Re: [linux-next-v1 3/3] arm64: dts: rockchip: Fix ethernet reset node Rock 3A sbc

[Michael Riesch <michael.riesch@xxxxxxxxxxxxxx>]

Hi Anand,

On 11/11/22 10:42, Anand Moon wrote:
> Add support for snps,reset-gpio reset ethernet gpio pins and
> drop the mdio reset code.

But why? Is it preferred that the MAC driver performs the reset? Or what
does this change fix?

> Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx>
> ---
>  .../boot/dts/rockchip/rk3568-rock-3a.dts      | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index 16fff1ada195..9172cf60b867 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -258,6 +258,11 @@ &gmac1 {
>  	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
>  	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&gmac1_clkin>;
>  	clock_in_out = "input";
> +	snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> +	snps,reset-active-low;
> +	/* Reset time is 20ms, 100ms for rtl8211f */
> +	snps,reset-delays-us = <0 20000 100000>;
> +	phy-supply = <&vcc_3v3>;
>  	phy-handle = <&rgmii_phy1>;
>  	phy-mode = "rgmii";
>  	pinctrl-names = "default";
> @@ -267,6 +272,9 @@ &gmac1m1_rx_bus2
>  		     &gmac1m1_rgmii_clk
>  		     &gmac1m1_clkinout
>  		     &gmac1m1_rgmii_bus>;
> +
> +	tx_delay = <0x4f>;
> +	rx_delay = <0x26>;

Ah, here they are. I would say these delays should be added in patch 1/3
along a comment why the rgmii-id approach does not work.

>  	status = "okay";
>  };
>  
> @@ -583,11 +591,6 @@ &mdio1 {
>  	rgmii_phy1: ethernet-phy@0 {
>  		compatible = "ethernet-phy-ieee802.3-c22";
>  		reg = <0x0>;
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&eth_phy_rst>;
> -		reset-assert-us = <20000>;
> -		reset-deassert-us = <100000>;
> -		reset-gpios = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
>  	};
>  };
>  
> @@ -625,12 +628,6 @@ vcc_mipi_en: vcc_mipi_en {
>  		};
>  	};
>  
> -	ethernet {
> -		eth_phy_rst: eth_phy_rst {
> -			rockchip,pins = <3 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
> -		};
> -	};

I would assume the pinctrl is still required no matter whether the MAC
driver or the PHY driver asserts the GPIO reset.

Best regards,
Michael

> -
>  	hym8563 {
>  		hym8563_int: hym8563-int {
>  			rockchip,pins = <0 RK_PD3 RK_FUNC_GPIO &pcfg_pull_up>;