Re: [RFC PATCH] arm64: dts: rockchip: H28K: release RTL8211F reset before MDIO probing

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Device Tree binding schema violation and dependency failure due to plural `snps,reset-gpios` property.
- [Medium] Introduction of deprecated MAC-level PHY reset properties instead of using standard PHY or MDIO bus resets.
- [Medium] Pinctrl configuration mismatch caused by stranded `pinctrl-0` reference in the PHY node.
--

--- Patch [1]: [RFC PATCH] arm64: dts: rockchip: H28K: release RTL8211F reset before MDIO probing ---
commit b9933a1b302b2f683cf8eb84eb27dfe653094785
Author: Zhyi <[email protected]>

arm64: dts: rockchip: H28K: release RTL8211F reset before MDIO probing

This commit attempts to fix a PHY probe deadlock on the HINLINK H28K board
by moving the reset GPIO configuration from the PHY node to the MAC node.
It uses the snps,reset-gpios properties on the MAC to release the reset
before the MDIO bus scan happens.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-hinlink-h28k.dts b/arch/arm64/boot/dts/rockchip/rk3528-hinlink-h28k.dts
> index 0ec50e9156eb1..9d7e33a92cfed 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3528-hinlink-h28k.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3528-hinlink-h28k.dts
> @@ -191,6 +191,8 @@ &gmac1 {
>  		    <&rgmii_rx_bus2>,
>  		    <&rgmii_rgmii_clk>,
>  		    <&rgmii_rgmii_bus>;
> +	snps,reset-gpios = <&gpio4 RK_PC2 GPIO_ACTIVE_LOW>;
> +	snps,reset-delays-us = <0 20000 100000>;

[Severity: Medium]
Will this fail device tree schema validation? The snps,dwmac.yaml schema
strictly defines the singular snps,reset-gpio property rather than the plural
snps,reset-gpios used here. Furthermore, the schema has a strict dependency
where snps,reset-delays-us requires the singular snps,reset-gpio property
to be present, so this dependency check may fail.

[Severity: Medium]
Is this the preferred approach for handling early PHY resets? The patch moves
the reset properties into the MAC node using the deprecated snps prefix.

The upstream-approved solutions for early PHY resets prior to MDIO probing
are typically to either declare the PHY ID explicitly in the PHY node
compatible string (e.g. compatible = "ethernet-phy-idXXXX.XXXX") so the MDIO
core skips the hardware read, or to use the standard reset-gpios inside the
parent mdio node.

>  	status = "okay";
>  };
>  
> @@ -205,9 +207,6 @@ rgmii_phy: ethernet-phy@1 {
>  		reg = <0x1>;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&gmac1_rstn_l>;
> -		reset-assert-us = <20000>;
> -		reset-deassert-us = <100000>;
> -		reset-gpios = <&gpio4 RK_PC2 GPIO_ACTIVE_LOW>;
>  	};
>  };

[Severity: Medium]
Will this cause pinctrl settings for the reset line to be applied too late?

The consumer of the reset GPIO has moved to the MAC node (gmac1), which
will now request and toggle the GPIO during its probe. However, the pinctrl-0
reference for gmac1_rstn_l remains here in the PHY node. 

Since the PHY is probed after the MAC, the pin's pinctrl settings might not
be applied before the MAC toggles the reset. When the PHY later probes and
applies the pinctrl, could it glitch the already-active reset pin?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1