linux-kernel - Re: [PATCH] arm64: dts: rockchip: ROCK3B: Correct clock rates for Ethernet PHYs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <17251add-40b7-4d18-8626-2d74b29532fc@kwiboo.se>
Date: Fri, 20 Dec 2024 18:09:01 +0100
From: Jonas Karlman <jonas@...boo.se>
To: Alicja Michalska <alicja.michalska@...ements.com>
Cc: heiko@...ech.de, linux-rockchip@...ts.infradead.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: dts: rockchip: ROCK3B: Correct clock rates for
 Ethernet PHYs

Hi Alicja,

On 2024-12-20 17:32, Alicja Michalska wrote:
> Built-in ethernet PHYs did not work on mainline kernel:
> 
> fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY

Have you used mainline or vendor u-boot when testing?

There is an existing issue with RTL8211F PHYs that they must be reset
before Linux can identify them, see [1] for more details on the issue.

Mainline U-Boot will reset the Ethernet PHYs on this board and that
should allow for Linux to identify and use the Ethernet PHYs.

[1] https://lore.kernel.org/r/47d55aca-bee6-810f-379f-9431649fefa6@kwiboo.se/

> 
> According to the board design, they need to be configured as output with
> static TX/RX delay. This patch sets it accordingly.
> 
> Signed-off-by: Alicja Michalska <alicja.michalska@...ements.com>
> ---
>  .../boot/dts/rockchip/rk3568-rock-3b.dts      | 68 +++++++++++--------
>  1 file changed, 38 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> index 3d0c1ccfaa79..5350158302e4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> @@ -183,37 +183,65 @@ &cpu3 {
>  &gmac0 {
>  	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
>  	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>, <&cru CLK_MAC0_2TOP>;
> -	clock_in_out = "input";
> -	phy-handle = <&rgmii_phy0>;
> -	phy-mode = "rgmii-id";
> +	assigned-clock-rates = <0>, <125000000>;
> +	clock_in_out = "output";
>  	phy-supply = <&vcc_3v3>;
> +	phy-mode = "rgmii";
> +	phy-handle = <&rgmii_phy0>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&gmac0_miim
>  		     &gmac0_tx_bus2
>  		     &gmac0_rx_bus2
>  		     &gmac0_rgmii_clk
> -		     &gmac0_rgmii_bus
> -		     &gmac0_clkinout>;
> +		     &gmac0_rgmii_bus>;
> +	snps,reset-gpio = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> +	snps,reset-active-low;
> +	snps,reset-delays-us = <0 20000 100000>;

The snps,reset props is deprecated and reset- props should be used in
the PHY node.

This also changes to use 20/100 ms delay instead of current 20/50ms
delay. If anything it could be changed to 20/80ms, I have only seen
datasheets for rtl8211f mention minimum 10 ms and minimum 30-76 ms.

> +
> +	tx_delay = <0x36>;
> +	rx_delay = <0x2d>;

Are you having issue with use of rgmii-id on your board?

> +
>  	status = "okay";
>  };
>  
>  &gmac1 {
>  	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
>  	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
> -	clock_in_out = "input";
> -	phy-handle = <&rgmii_phy1>;
> -	phy-mode = "rgmii-id";
> +	assigned-clock-rates = <0>, <125000000>;
> +	clock_in_out = "output";
>  	phy-supply = <&vcc_3v3>;
> +	phy-mode = "rgmii";
> +	phy-handle = <&rgmii_phy1>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&gmac1m1_miim
>  		     &gmac1m1_tx_bus2
>  		     &gmac1m1_rx_bus2
>  		     &gmac1m1_rgmii_clk
> -		     &gmac1m1_rgmii_bus
> -		     &gmac1m1_clkinout>;
> +		     &gmac1m1_rgmii_bus>;
> +	snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> +	snps,reset-active-low;
> +	snps,reset-delays-us = <0 50000 150000>;

Same here, but now this is 50/150 ms?

> +
> +	tx_delay = <0x47>;
> +	rx_delay = <0x28>;

Same here, is use of rgmii-id causing issues on your boards?

> +
>  	status = "okay";
>  };
>  
> +&mdio0 {
> +	rgmii_phy0: ethernet-phy@0 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0>;

The phy addr used for these boards is 0x1.

> +	};
> +};
> +
> +&mdio1 {
> +	rgmii_phy1: ethernet-phy@0 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0>;

Same here.

> +	};
> +};

There should be no need to move these nodes, they should already be in
alphabetical order?

> +
>  &gpu {
>  	mali-supply = <&vdd_gpu>;
>  	status = "okay";
> @@ -512,26 +540,6 @@ &i2s1m0_sdi0
>  	status = "okay";
>  };
>  
> -&mdio0 {
> -	rgmii_phy0: ethernet-phy@1 {
> -		compatible = "ethernet-phy-ieee802.3-c22";

If you want to use vendor u-boot you can probably change this to
"ethernet-phy-id001c.c916".

> -		reg = <1>;
> -		reset-assert-us = <20000>;
> -		reset-deassert-us = <50000>;
> -		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> -	};
> -};
> -
> -&mdio1 {
> -	rgmii_phy1: ethernet-phy@1 {
> -		compatible = "ethernet-phy-ieee802.3-c22";

Same here.

Regards,
Jonas

> -		reg = <1>;
> -		reset-assert-us = <20000>;
> -		reset-deassert-us = <50000>;
> -		reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> -	};
> -};
> -
>  &pcie2x1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie20m1_pins>;