diff mbox series

[v2,7/9] arm64: dts: rockchip: use generic Ethernet PHY reset bindings for Lunzn Fastrhino R68S

Message ID 20240630150010.55729-8-amadeus@jmu.edu.cn (mailing list archive)
State New
Headers show
Series arm64: dts: rockchip: fixes support for Lunzn Fastrhino R6xS | expand

Commit Message

Chukun Pan June 30, 2024, 3 p.m. UTC
Replace the deprecated snps,reset-xxx bindings to the generic Ethernet
PHY reset GPIO bindings. Also updates the delays based on the vendor
recommendations.

Fixes: b9f8ca655d80 ("arm64: dts: rockchip: Add Lunzn Fastrhino R68S")
Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
---
 .../boot/dts/rockchip/rk3568-fastrhino-r68s.dts    | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

Comments

Heiko Stuebner July 4, 2024, 7:12 p.m. UTC | #1
Hi,

Am Sonntag, 30. Juni 2024, 17:00:08 CEST schrieb Chukun Pan:
> Replace the deprecated snps,reset-xxx bindings to the generic Ethernet
> PHY reset GPIO bindings. Also updates the delays based on the vendor
> recommendations.
> 
> Fixes: b9f8ca655d80 ("arm64: dts: rockchip: Add Lunzn Fastrhino R68S")
> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
> ---
>  .../boot/dts/rockchip/rk3568-fastrhino-r68s.dts    | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-fastrhino-r68s.dts b/arch/arm64/boot/dts/rockchip/rk3568-fastrhino-r68s.dts
> index ce2a5e1ccefc..02d966d218fd 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-fastrhino-r68s.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-fastrhino-r68s.dts
> @@ -39,10 +39,6 @@ &gmac0_tx_bus2
>  		     &gmac0_rx_bus2
>  		     &gmac0_rgmii_clk
>  		     &gmac0_rgmii_bus>;
> -	snps,reset-gpio = <&gpio1 RK_PB0 GPIO_ACTIVE_LOW>;
> -	snps,reset-active-low;
> -	/* Reset time is 15ms, 50ms for rtl8211f */
> -	snps,reset-delays-us = <0 15000 50000>;
>  	tx_delay = <0x3c>;
>  	rx_delay = <0x2f>;
>  	status = "okay";
> @@ -61,10 +57,6 @@ &gmac1m1_tx_bus2
>  		     &gmac1m1_rx_bus2
>  		     &gmac1m1_rgmii_clk
>  		     &gmac1m1_rgmii_bus>;
> -	snps,reset-gpio = <&gpio1 RK_PB1 GPIO_ACTIVE_LOW>;
> -	snps,reset-active-low;
> -	/* Reset time is 15ms, 50ms for rtl8211f */
> -	snps,reset-delays-us = <0 15000 50000>;
>  	tx_delay = <0x4f>;
>  	rx_delay = <0x26>;
>  	status = "okay";
> @@ -76,6 +68,9 @@ rgmii_phy0: ethernet-phy@1 {
>  		reg = <0x1>;
>  		pinctrl-0 = <&eth_phy0_reset_pin>;
>  		pinctrl-names = "default";
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;

what's the reason behind the changed timings?

The original comment stated,
	/* Reset time is 15ms, 50ms for rtl8211f */
so that timing change needs an explanation please :-)

Thanks
Heiko

> +		reset-gpios = <&gpio1 RK_PB0 GPIO_ACTIVE_LOW>;
>  	};
>  };
>  
> @@ -85,6 +80,9 @@ rgmii_phy1: ethernet-phy@1 {
>  		reg = <0x1>;
>  		pinctrl-0 = <&eth_phy1_reset_pin>;
>  		pinctrl-names = "default";
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;
> +		reset-gpios = <&gpio1 RK_PB1 GPIO_ACTIVE_LOW>;
>  	};
>  };
>  
>
Chukun Pan July 10, 2024, 1:30 p.m. UTC | #2
> what's the reason behind the changed timings?
>
> The original comment stated,
>	/* Reset time is 15ms, 50ms for rtl8211f */
> so that timing change needs an explanation please :-)

I don't know why this comment says that, but it's clearly wrong.
According to the PHY datasheet, the RTL8211F PHY needs a 10ms
assert delay and at least 72ms deassert delay. Considering the
possibility of mixed use of PHY chips, the reset time should be
further increased. 

Thanks,
Chukun
Jonas Karlman July 10, 2024, 2:30 p.m. UTC | #3
Hi,

On 2024-07-10 15:30, Chukun Pan wrote:
>> what's the reason behind the changed timings?
>>
>> The original comment stated,
>> 	/* Reset time is 15ms, 50ms for rtl8211f */
>> so that timing change needs an explanation please :-)
> 
> I don't know why this comment says that, but it's clearly wrong.
> According to the PHY datasheet, the RTL8211F PHY needs a 10ms
> assert delay and at least 72ms deassert delay. Considering the
> possibility of mixed use of PHY chips, the reset time should be
> further increased.

Where do you find the 72ms in the datasheet?

In RTL8211F-CG v1.1 I see 10ms and minimum of 30ms, in v1.2 and v1.4
I see 10ms and minimum of 50ms.

I have used 50ms on a few recently added boards and they seem to all
work fine with 50ms, wonder if the deassert delay should be changed for
those boards.

Regards,
Jonas

> 
> Thanks,
> Chukun
>
Chukun Pan July 10, 2024, 2:50 p.m. UTC | #4
> Where do you find the 72ms in the datasheet?

I refer to this commit:
https://github.com/torvalds/linux/commit/1c7412530d5d0e0a0b27f1642f5c13c8b9f36f05
BTW I found that some boards use the RTL8211F-VD PHY,
but I can't find the datasheet.

> In RTL8211F-CG v1.1 I see 10ms and minimum of 30ms, in v1.2 and v1.4
> I see 10ms and minimum of 50ms.

> I have used 50ms on a few recently added boards and they seem to all
> work fine with 50ms, wonder if the deassert delay should be changed for
> those boards.

Thanks,
Chukun
Jonas Karlman July 10, 2024, 3:07 p.m. UTC | #5
On 2024-07-10 16:50, Chukun Pan wrote:
>> Where do you find the 72ms in the datasheet?
> 
> I refer to this commit:
> https://github.com/torvalds/linux/commit/1c7412530d5d0e0a0b27f1642f5c13c8b9f36f05
> BTW I found that some boards use the RTL8211F-VD PHY,
> but I can't find the datasheet.

Thanks, I can also see in RTL8211F-VD-CG (Rev 1.0 - 08 February 2022):

  For a complete PHY reset, this pin must be asserted low for at least
  10ms for the internal regulator. Wait for at least 72ms* before
  accessing the PHY register. * Note: Not included the 0.9V rise time.

So using 80-100ms may be the best option to be on the safe side, should
probably send a fix for recently added boards where I used 50ms.

Regards,
Jonas

> 
>> In RTL8211F-CG v1.1 I see 10ms and minimum of 30ms, in v1.2 and v1.4
>> I see 10ms and minimum of 50ms.
> 
>> I have used 50ms on a few recently added boards and they seem to all
>> work fine with 50ms, wonder if the deassert delay should be changed for
>> those boards.
> 
> Thanks,
> Chukun
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-fastrhino-r68s.dts b/arch/arm64/boot/dts/rockchip/rk3568-fastrhino-r68s.dts
index ce2a5e1ccefc..02d966d218fd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-fastrhino-r68s.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-fastrhino-r68s.dts
@@ -39,10 +39,6 @@  &gmac0_tx_bus2
 		     &gmac0_rx_bus2
 		     &gmac0_rgmii_clk
 		     &gmac0_rgmii_bus>;
-	snps,reset-gpio = <&gpio1 RK_PB0 GPIO_ACTIVE_LOW>;
-	snps,reset-active-low;
-	/* Reset time is 15ms, 50ms for rtl8211f */
-	snps,reset-delays-us = <0 15000 50000>;
 	tx_delay = <0x3c>;
 	rx_delay = <0x2f>;
 	status = "okay";
@@ -61,10 +57,6 @@  &gmac1m1_tx_bus2
 		     &gmac1m1_rx_bus2
 		     &gmac1m1_rgmii_clk
 		     &gmac1m1_rgmii_bus>;
-	snps,reset-gpio = <&gpio1 RK_PB1 GPIO_ACTIVE_LOW>;
-	snps,reset-active-low;
-	/* Reset time is 15ms, 50ms for rtl8211f */
-	snps,reset-delays-us = <0 15000 50000>;
 	tx_delay = <0x4f>;
 	rx_delay = <0x26>;
 	status = "okay";
@@ -76,6 +68,9 @@  rgmii_phy0: ethernet-phy@1 {
 		reg = <0x1>;
 		pinctrl-0 = <&eth_phy0_reset_pin>;
 		pinctrl-names = "default";
+		reset-assert-us = <20000>;
+		reset-deassert-us = <100000>;
+		reset-gpios = <&gpio1 RK_PB0 GPIO_ACTIVE_LOW>;
 	};
 };
 
@@ -85,6 +80,9 @@  rgmii_phy1: ethernet-phy@1 {
 		reg = <0x1>;
 		pinctrl-0 = <&eth_phy1_reset_pin>;
 		pinctrl-names = "default";
+		reset-assert-us = <20000>;
+		reset-deassert-us = <100000>;
+		reset-gpios = <&gpio1 RK_PB1 GPIO_ACTIVE_LOW>;
 	};
 };