On Thu 2024-01-18 @ 08:31:30 AM, Jonas Karlman wrote: > On 2024-01-17 09:15, Jonas Karlman wrote: > > On 2024-01-17 06:47, Trevor Woerner wrote: > >> 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. > > > > For linux this is probably related to the same chicken-and-egg reset > > issue outlined at [1]. The phy is not reset before it is probed and > > cannot be probed because it is not reset. > > > > As for U-Boot the designware/gmac_rockchip ethernet driver may need some > > adjustments to properly integrate with eth-phy uclass to properly reset > > the phy described in a ethernet-phy node before it can be probed. > > > > Following config options is disabled for rock-pi-e-rk3328 defconfig: > > > > # CONFIG_DM_ETH_PHY is not set > > # CONFIG_PHY_REALTEK is not set > > > > Also the driver needs to be updated to make use eth-phy uclass for it to > > work correctly. More similar to how the dwc_eth_qos driver works related > > to ethernet phy. > > I have just sent out a U-Boot series that fix ethernet on the v1.21 > revision of the ROCK Pi E board, see [2]. Thank you! I have tested your patches both in U-Boot and Linux and they work perfectly. > Would expect that ethernet also starts to work in linux with those > patches applied to U-Boot. I only tested and verified ethernet in U-Boot. > > Hopefully someone can take a closer look at the issue on linux side > so that it does not need to depend on PHY being reset by the bootloader. Yes, as you suspected this is exactly what happened: providing a fix in U-Boot caused everything to work again in Linux. It would appear as though Linux is expecting the PHY to be reset/setup/configured by the bootloader in the case of this board/SoC/PHY. > [2] https://lore.kernel.org/u-boot/20240118071949.927089-1-jonas@xxxxxxxxx/T/ > > Regards, > Jonas > > > > > [1] https://lore.kernel.org/linux-rockchip/47d55aca-bee6-810f-379f-9431649fefa6@xxxxxxxxx/ > > > > Regards, > > Jonas > > > >> > >> 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>, <ð_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 = <ð_phy_int_pin>, <ð_phy_reset_pin>; > >>>> + pinctrl-0 = <ð_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 > >>>> >