2024-02-29 16:28:05

by Roger Quadros

[permalink] [raw]
Subject: [PATCH] arm: dts: ti: beagleplay: Fix Ethernet PHY RESET GPIOs

The RESET GPIO pinmux should be part of MDIO bus node
so that they can be in the right state before the PHY
can be probed via MDIO bus scan.

Add GPIO reset for the Gigabit Ethernet PHY. As per
RTL8211F datasheet, reset assert width is 10ms and
PHY registers can be access accessed after 50ms of
reset deassert.

Signed-off-by: Roger Quadros <[email protected]>
---
arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
index a34e0df2ab86..77240cf3ae4d 100644
--- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
+++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
@@ -292,6 +292,8 @@ mdio0_pins_default: mdio0-default-pins {
pinctrl-single,pins = <
AM62X_IOPAD(0x0160, PIN_OUTPUT, 0) /* (AD24) MDIO0_MDC */
AM62X_IOPAD(0x015c, PIN_INPUT, 0) /* (AB22) MDIO0_MDIO */
+ AM62X_IOPAD(0x003c, PIN_INPUT, 7) /* (M25) GPMC0_AD0.GPIO0_15 */
+ AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */
>;
};

@@ -383,7 +385,6 @@ AM62X_IOPAD(0x017c, PIN_INPUT, 1) /* (AD22) RGMII2_RX_CTL.RMII2_RX_ER */
AM62X_IOPAD(0x016c, PIN_INPUT, 1) /* (Y18) RGMII2_TD0.RMII2_TXD0 */
AM62X_IOPAD(0x0170, PIN_INPUT, 1) /* (AA18) RGMII2_TD1.RMII2_TXD1 */
AM62X_IOPAD(0x0164, PIN_INPUT, 1) /* (AA19) RGMII2_TX_CTL.RMII2_TX_EN */
- AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */
AM62X_IOPAD(0x0190, PIN_INPUT, 7) /* (AE22) RGMII2_RD3.GPIO1_6 */
AM62X_IOPAD(0x01f0, PIN_OUTPUT, 5) /* (A18) EXT_REFCLK1.CLKOUT0 */
>;
@@ -597,6 +598,9 @@ &cpsw3g_mdio {

cpsw3g_phy0: ethernet-phy@0 {
reg = <0>;
+ reset-gpios = <&main_gpio0 15 GPIO_ACTIVE_LOW>;
+ reset-assert-us = <10000>;
+ reset-deassert-us = <50000>;
};

cpsw3g_phy1: ethernet-phy@1 {
@@ -615,7 +619,7 @@ &main_gpio0 {
"USR0", "USR1", "USR2", "USR3", "", "", "USR4", /* 3-9 */
"EEPROM_WP", /* 10 */
"CSI2_CAMERA_GPIO1", "CSI2_CAMERA_GPIO2", /* 11-12 */
- "CC1352P7_BOOT", "CC1352P7_RSTN", "", "", "", /* 13-17 */
+ "CC1352P7_BOOT", "CC1352P7_RSTN", "GBE_RSTN", "", "", /* 13-17 */
"USR_BUTTON", "", "", "", "", "", "", "", "", /* 18-26 */
"", "", "", "", "", "", "", "", "", "HDMI_INT", /* 27-36 */
"", "VDD_WLAN_EN", "", "", "WL_IRQ", "GBE_INTN",/* 37-42 */

---
base-commit: bbef42084cc170cbfc035bf784f2ff055c939d7e
change-id: 20240229-b4-for-v6-9-am65-beagleplay-ethernet-reset-098f274fbf15

Best regards,
--
Roger Quadros <[email protected]>



2024-03-01 21:12:00

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH] arm: dts: ti: beagleplay: Fix Ethernet PHY RESET GPIOs



On 29/02/2024 18:25, Roger Quadros wrote:
> The RESET GPIO pinmux should be part of MDIO bus node
> so that they can be in the right state before the PHY
> can be probed via MDIO bus scan.
>
> Add GPIO reset for the Gigabit Ethernet PHY. As per
> RTL8211F datasheet, reset assert width is 10ms and
> PHY registers can be access accessed after 50ms of
> reset deassert.
>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
> arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
> index a34e0df2ab86..77240cf3ae4d 100644
> --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
> @@ -292,6 +292,8 @@ mdio0_pins_default: mdio0-default-pins {
> pinctrl-single,pins = <
> AM62X_IOPAD(0x0160, PIN_OUTPUT, 0) /* (AD24) MDIO0_MDC */
> AM62X_IOPAD(0x015c, PIN_INPUT, 0) /* (AB22) MDIO0_MDIO */
> + AM62X_IOPAD(0x003c, PIN_INPUT, 7) /* (M25) GPMC0_AD0.GPIO0_15 */

This should be PIN_OUTPUT.
Will fix in next spin.

> + AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */
> >;
> };
>
> @@ -383,7 +385,6 @@ AM62X_IOPAD(0x017c, PIN_INPUT, 1) /* (AD22) RGMII2_RX_CTL.RMII2_RX_ER */
> AM62X_IOPAD(0x016c, PIN_INPUT, 1) /* (Y18) RGMII2_TD0.RMII2_TXD0 */
> AM62X_IOPAD(0x0170, PIN_INPUT, 1) /* (AA18) RGMII2_TD1.RMII2_TXD1 */
> AM62X_IOPAD(0x0164, PIN_INPUT, 1) /* (AA19) RGMII2_TX_CTL.RMII2_TX_EN */
> - AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */
> AM62X_IOPAD(0x0190, PIN_INPUT, 7) /* (AE22) RGMII2_RD3.GPIO1_6 */
> AM62X_IOPAD(0x01f0, PIN_OUTPUT, 5) /* (A18) EXT_REFCLK1.CLKOUT0 */
> >;
> @@ -597,6 +598,9 @@ &cpsw3g_mdio {
>
> cpsw3g_phy0: ethernet-phy@0 {
> reg = <0>;
> + reset-gpios = <&main_gpio0 15 GPIO_ACTIVE_LOW>;
> + reset-assert-us = <10000>;
> + reset-deassert-us = <50000>;
> };
>
> cpsw3g_phy1: ethernet-phy@1 {
> @@ -615,7 +619,7 @@ &main_gpio0 {
> "USR0", "USR1", "USR2", "USR3", "", "", "USR4", /* 3-9 */
> "EEPROM_WP", /* 10 */
> "CSI2_CAMERA_GPIO1", "CSI2_CAMERA_GPIO2", /* 11-12 */
> - "CC1352P7_BOOT", "CC1352P7_RSTN", "", "", "", /* 13-17 */
> + "CC1352P7_BOOT", "CC1352P7_RSTN", "GBE_RSTN", "", "", /* 13-17 */
> "USR_BUTTON", "", "", "", "", "", "", "", "", /* 18-26 */
> "", "", "", "", "", "", "", "", "", "HDMI_INT", /* 27-36 */
> "", "VDD_WLAN_EN", "", "", "WL_IRQ", "GBE_INTN",/* 37-42 */
>
> ---
> base-commit: bbef42084cc170cbfc035bf784f2ff055c939d7e
> change-id: 20240229-b4-for-v6-9-am65-beagleplay-ethernet-reset-098f274fbf15
>
> Best regards,

--
cheers,
-roger

2024-03-01 21:29:39

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH] arm: dts: ti: beagleplay: Fix Ethernet PHY RESET GPIOs



On 01/03/2024 22:58, Roger Quadros wrote:
>
>
> On 29/02/2024 18:25, Roger Quadros wrote:
>> The RESET GPIO pinmux should be part of MDIO bus node
>> so that they can be in the right state before the PHY
>> can be probed via MDIO bus scan.
>>
>> Add GPIO reset for the Gigabit Ethernet PHY. As per
>> RTL8211F datasheet, reset assert width is 10ms and
>> PHY registers can be access accessed after 50ms of
>> reset deassert.
>>
>> Signed-off-by: Roger Quadros <[email protected]>
>> ---
>> arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> index a34e0df2ab86..77240cf3ae4d 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> @@ -292,6 +292,8 @@ mdio0_pins_default: mdio0-default-pins {
>> pinctrl-single,pins = <
>> AM62X_IOPAD(0x0160, PIN_OUTPUT, 0) /* (AD24) MDIO0_MDC */
>> AM62X_IOPAD(0x015c, PIN_INPUT, 0) /* (AB22) MDIO0_MDIO */
>> + AM62X_IOPAD(0x003c, PIN_INPUT, 7) /* (M25) GPMC0_AD0.GPIO0_15 */
>
> This should be PIN_OUTPUT.
> Will fix in next spin.

Actually PIN_INPUT is correct else we won't be able to read the correct GPIO pin status
on gpio read.
I observe this issue on u-boot at least.

>
>> + AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */

This one needs to be fixed to PIN_INPUT.

>> >;
>> };
>>
>> @@ -383,7 +385,6 @@ AM62X_IOPAD(0x017c, PIN_INPUT, 1) /* (AD22) RGMII2_RX_CTL.RMII2_RX_ER */
>> AM62X_IOPAD(0x016c, PIN_INPUT, 1) /* (Y18) RGMII2_TD0.RMII2_TXD0 */
>> AM62X_IOPAD(0x0170, PIN_INPUT, 1) /* (AA18) RGMII2_TD1.RMII2_TXD1 */
>> AM62X_IOPAD(0x0164, PIN_INPUT, 1) /* (AA19) RGMII2_TX_CTL.RMII2_TX_EN */
>> - AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */
>> AM62X_IOPAD(0x0190, PIN_INPUT, 7) /* (AE22) RGMII2_RD3.GPIO1_6 */
>> AM62X_IOPAD(0x01f0, PIN_OUTPUT, 5) /* (A18) EXT_REFCLK1.CLKOUT0 */
>> >;
>> @@ -597,6 +598,9 @@ &cpsw3g_mdio {
>>
>> cpsw3g_phy0: ethernet-phy@0 {
>> reg = <0>;
>> + reset-gpios = <&main_gpio0 15 GPIO_ACTIVE_LOW>;
>> + reset-assert-us = <10000>;
>> + reset-deassert-us = <50000>;
>> };
>>
>> cpsw3g_phy1: ethernet-phy@1 {
>> @@ -615,7 +619,7 @@ &main_gpio0 {
>> "USR0", "USR1", "USR2", "USR3", "", "", "USR4", /* 3-9 */
>> "EEPROM_WP", /* 10 */
>> "CSI2_CAMERA_GPIO1", "CSI2_CAMERA_GPIO2", /* 11-12 */
>> - "CC1352P7_BOOT", "CC1352P7_RSTN", "", "", "", /* 13-17 */
>> + "CC1352P7_BOOT", "CC1352P7_RSTN", "GBE_RSTN", "", "", /* 13-17 */
>> "USR_BUTTON", "", "", "", "", "", "", "", "", /* 18-26 */
>> "", "", "", "", "", "", "", "", "", "HDMI_INT", /* 27-36 */
>> "", "VDD_WLAN_EN", "", "", "WL_IRQ", "GBE_INTN",/* 37-42 */
>>
>> ---
>> base-commit: bbef42084cc170cbfc035bf784f2ff055c939d7e
>> change-id: 20240229-b4-for-v6-9-am65-beagleplay-ethernet-reset-098f274fbf15
>>
>> Best regards,
>

--
cheers,
-roger

2024-03-04 04:24:55

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH] arm: dts: ti: beagleplay: Fix Ethernet PHY RESET GPIOs

Hi Roger,

On 02/03/24 02:59, Roger Quadros wrote:
>
>
> On 01/03/2024 22:58, Roger Quadros wrote:
>>
>>
>> On 29/02/2024 18:25, Roger Quadros wrote:
>>> The RESET GPIO pinmux should be part of MDIO bus node
>>> so that they can be in the right state before the PHY
>>> can be probed via MDIO bus scan.
>>>
>>> Add GPIO reset for the Gigabit Ethernet PHY. As per
>>> RTL8211F datasheet, reset assert width is 10ms and
>>> PHY registers can be access accessed after 50ms of
>>> reset deassert.
>>>
>>> Signed-off-by: Roger Quadros <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 8 ++++++--
>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>>> index a34e0df2ab86..77240cf3ae4d 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>>> +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>>> @@ -292,6 +292,8 @@ mdio0_pins_default: mdio0-default-pins {
>>> pinctrl-single,pins = <
>>> AM62X_IOPAD(0x0160, PIN_OUTPUT, 0) /* (AD24) MDIO0_MDC */
>>> AM62X_IOPAD(0x015c, PIN_INPUT, 0) /* (AB22) MDIO0_MDIO */
>>> + AM62X_IOPAD(0x003c, PIN_INPUT, 7) /* (M25) GPMC0_AD0.GPIO0_15 */
>>
>> This should be PIN_OUTPUT.
>> Will fix in next spin.
>
> Actually PIN_INPUT is correct else we won't be able to read the correct GPIO pin status
> on gpio read.
> I observe this issue on u-boot at least.
>
>>
>>> + AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */
>
> This one needs to be fixed to PIN_INPUT.

While at it, please fix the $subject prefix:

arm64: dts: ti: beagleplay: ...

Do we need a Fixes: Tag too?

>
>>> >;
>>> };
>>>
>>> @@ -383,7 +385,6 @@ AM62X_IOPAD(0x017c, PIN_INPUT, 1) /* (AD22) RGMII2_RX_CTL.RMII2_RX_ER */
>>> AM62X_IOPAD(0x016c, PIN_INPUT, 1) /* (Y18) RGMII2_TD0.RMII2_TXD0 */
>>> AM62X_IOPAD(0x0170, PIN_INPUT, 1) /* (AA18) RGMII2_TD1.RMII2_TXD1 */
>>> AM62X_IOPAD(0x0164, PIN_INPUT, 1) /* (AA19) RGMII2_TX_CTL.RMII2_TX_EN */
>>> - AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */
>>> AM62X_IOPAD(0x0190, PIN_INPUT, 7) /* (AE22) RGMII2_RD3.GPIO1_6 */
>>> AM62X_IOPAD(0x01f0, PIN_OUTPUT, 5) /* (A18) EXT_REFCLK1.CLKOUT0 */
>>> >;
>>> @@ -597,6 +598,9 @@ &cpsw3g_mdio {
>>>
>>> cpsw3g_phy0: ethernet-phy@0 {
>>> reg = <0>;
>>> + reset-gpios = <&main_gpio0 15 GPIO_ACTIVE_LOW>;
>>> + reset-assert-us = <10000>;
>>> + reset-deassert-us = <50000>;
>>> };
>>>
>>> cpsw3g_phy1: ethernet-phy@1 {
>>> @@ -615,7 +619,7 @@ &main_gpio0 {
>>> "USR0", "USR1", "USR2", "USR3", "", "", "USR4", /* 3-9 */
>>> "EEPROM_WP", /* 10 */
>>> "CSI2_CAMERA_GPIO1", "CSI2_CAMERA_GPIO2", /* 11-12 */
>>> - "CC1352P7_BOOT", "CC1352P7_RSTN", "", "", "", /* 13-17 */
>>> + "CC1352P7_BOOT", "CC1352P7_RSTN", "GBE_RSTN", "", "", /* 13-17 */
>>> "USR_BUTTON", "", "", "", "", "", "", "", "", /* 18-26 */
>>> "", "", "", "", "", "", "", "", "", "HDMI_INT", /* 27-36 */
>>> "", "VDD_WLAN_EN", "", "", "WL_IRQ", "GBE_INTN",/* 37-42 */
>>>
>>> ---
>>> base-commit: bbef42084cc170cbfc035bf784f2ff055c939d7e
>>> change-id: 20240229-b4-for-v6-9-am65-beagleplay-ethernet-reset-098f274fbf15
>>>
>>> Best regards,
>>
>

--
Regards
Vignesh

2024-03-05 08:43:22

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH] arm: dts: ti: beagleplay: Fix Ethernet PHY RESET GPIOs



On 04/03/2024 06:24, Vignesh Raghavendra wrote:
> Hi Roger,
>
> On 02/03/24 02:59, Roger Quadros wrote:
>>
>>
>> On 01/03/2024 22:58, Roger Quadros wrote:
>>>
>>>
>>> On 29/02/2024 18:25, Roger Quadros wrote:
>>>> The RESET GPIO pinmux should be part of MDIO bus node
>>>> so that they can be in the right state before the PHY
>>>> can be probed via MDIO bus scan.
>>>>
>>>> Add GPIO reset for the Gigabit Ethernet PHY. As per
>>>> RTL8211F datasheet, reset assert width is 10ms and
>>>> PHY registers can be access accessed after 50ms of
>>>> reset deassert.
>>>>
>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>> ---
>>>> arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 8 ++++++--
>>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>>>> index a34e0df2ab86..77240cf3ae4d 100644
>>>> --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>>>> +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>>>> @@ -292,6 +292,8 @@ mdio0_pins_default: mdio0-default-pins {
>>>> pinctrl-single,pins = <
>>>> AM62X_IOPAD(0x0160, PIN_OUTPUT, 0) /* (AD24) MDIO0_MDC */
>>>> AM62X_IOPAD(0x015c, PIN_INPUT, 0) /* (AB22) MDIO0_MDIO */
>>>> + AM62X_IOPAD(0x003c, PIN_INPUT, 7) /* (M25) GPMC0_AD0.GPIO0_15 */
>>>
>>> This should be PIN_OUTPUT.
>>> Will fix in next spin.
>>
>> Actually PIN_INPUT is correct else we won't be able to read the correct GPIO pin status
>> on gpio read.
>> I observe this issue on u-boot at least.
>>
>>>
>>>> + AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */
>>
>> This one needs to be fixed to PIN_INPUT.
>
> While at it, please fix the $subject prefix:
>
> arm64: dts: ti: beagleplay: ...

Right.

>
> Do we need a Fixes: Tag too?

Sure, I'll add.

>
>>
>>>> >;
>>>> };
>>>>
>>>> @@ -383,7 +385,6 @@ AM62X_IOPAD(0x017c, PIN_INPUT, 1) /* (AD22) RGMII2_RX_CTL.RMII2_RX_ER */
>>>> AM62X_IOPAD(0x016c, PIN_INPUT, 1) /* (Y18) RGMII2_TD0.RMII2_TXD0 */
>>>> AM62X_IOPAD(0x0170, PIN_INPUT, 1) /* (AA18) RGMII2_TD1.RMII2_TXD1 */
>>>> AM62X_IOPAD(0x0164, PIN_INPUT, 1) /* (AA19) RGMII2_TX_CTL.RMII2_TX_EN */
>>>> - AM62X_IOPAD(0x018c, PIN_OUTPUT, 7) /* (AC21) RGMII2_RD2.GPIO1_5 */
>>>> AM62X_IOPAD(0x0190, PIN_INPUT, 7) /* (AE22) RGMII2_RD3.GPIO1_6 */
>>>> AM62X_IOPAD(0x01f0, PIN_OUTPUT, 5) /* (A18) EXT_REFCLK1.CLKOUT0 */
>>>> >;
>>>> @@ -597,6 +598,9 @@ &cpsw3g_mdio {
>>>>
>>>> cpsw3g_phy0: ethernet-phy@0 {
>>>> reg = <0>;
>>>> + reset-gpios = <&main_gpio0 15 GPIO_ACTIVE_LOW>;
>>>> + reset-assert-us = <10000>;
>>>> + reset-deassert-us = <50000>;
>>>> };
>>>>
>>>> cpsw3g_phy1: ethernet-phy@1 {
>>>> @@ -615,7 +619,7 @@ &main_gpio0 {
>>>> "USR0", "USR1", "USR2", "USR3", "", "", "USR4", /* 3-9 */
>>>> "EEPROM_WP", /* 10 */
>>>> "CSI2_CAMERA_GPIO1", "CSI2_CAMERA_GPIO2", /* 11-12 */
>>>> - "CC1352P7_BOOT", "CC1352P7_RSTN", "", "", "", /* 13-17 */
>>>> + "CC1352P7_BOOT", "CC1352P7_RSTN", "GBE_RSTN", "", "", /* 13-17 */
>>>> "USR_BUTTON", "", "", "", "", "", "", "", "", /* 18-26 */
>>>> "", "", "", "", "", "", "", "", "", "HDMI_INT", /* 27-36 */
>>>> "", "VDD_WLAN_EN", "", "", "WL_IRQ", "GBE_INTN",/* 37-42 */
>>>>
>>>> ---
>>>> base-commit: bbef42084cc170cbfc035bf784f2ff055c939d7e
>>>> change-id: 20240229-b4-for-v6-9-am65-beagleplay-ethernet-reset-098f274fbf15
>>>>
>>>> Best regards,
>>>
>>
>

--
cheers,
-roger