Re: [PATCH 2/2] arm64: dts: rockchip: rock-pi-e: fix location of snps properties

[Trevor Woerner]

On Wed 2024-01-17 @ 12:38:39 PM, Chen-Yu Tsai wrote:
> On Wed, Jan 17, 2024 at 4:41 AM Trevor Woerner <twoerner@xxxxxxxxx> wrote:
> >
> > A number of snps (Synopsys) properties are not in their correct location.
> 
> Nope. If you read the snps,dwmac.yaml binding file, you'll see that these
> properties have been deprecated. They are properties pertaining to the PHY
> and should be described under the PHY node. Support for reset GPIOs on PHY
> devices in phylib has been there since v4.16. The snps prefixed properties
> were deprecated in v5.3.

If that's the case, then the bindings and the drivers are out of sync in both
U-Boot and Linux. I discovered this issue while working with older and newer
revisions of the rock-pi-e board. The first three spins of the rock-pi-e have
the rtl8211e PHY but the last one (v1.21) has the rtl8211f PHY.

With the existing layout nothing works in U-Boot and in Linux the rtl8211e
works but not the rtl8211f. With this patch both the rtl8211e and the rtl8211f
PHYs work using the exact same device trees on both older and newer rock-pi-e
boards in both U-Boot and Linux.

Comparing the rock-pi-e's dts file with the one from the roc-rk3328-cc board,
which also uses the layout in this patch, and which also uses the rtl8211e
external PHY, is what led me in this direction.

> 
> ChenYu
> 
> > Fixes: b918e81f2145 ("arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E")
> > Signed-off-by: Trevor Woerner <twoerner@xxxxxxxxx>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
> > index 096cfa19036e..0739b8fec86e 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
> > @@ -150,8 +150,11 @@ &gmac2io {
> >         phy-mode = "rgmii";
> >         phy-supply = <&vcc_io>;
> >         pinctrl-names = "default";
> > -       pinctrl-0 = <&rgmiim1_pins>;
> > +       pinctrl-0 = <&rgmiim1_pins>, <&eth_phy_reset_pin>;
> >         snps,aal;
> > +       snps,reset-gpio = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>;
> > +       snps,reset-active-low;
> > +       snps,reset-delays-us = <0 10000 50000>;
> >         snps,rxpbl = <0x4>;
> >         snps,txpbl = <0x4>;
> >         tx_delay = <0x26>;
> > @@ -165,13 +168,10 @@ mdio {
> >
> >                 rtl8211: ethernet-phy@1 {
> >                         reg = <1>;
> > -                       pinctrl-0 = <&eth_phy_int_pin>, <&eth_phy_reset_pin>;
> > +                       pinctrl-0 = <&eth_phy_int_pin>;
> >                         pinctrl-names = "default";
> >                         interrupt-parent = <&gpio1>;
> >                         interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
> > -                       reset-assert-us = <10000>;
> > -                       reset-deassert-us = <50000>;
> > -                       reset-gpios = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>;
> >                 };
> >         };
> >  };
> > --
> > 2.43.0.76.g1a87c842ece3
> >