arm64: dts: rockchip: rock-pi-e: adjust phy-handle name
Subsystems (2)
PATCH 1/2 arm64: dts: rockchip: rock-pi-e: adjust phy-handle name
The rock-pi-e currently comes in 4 board spins, the latest one (v1.21) swaps out the Realtek 8211e PHY for an 8211f PHY. Therefore modify the phy-handle name to be more generic.
--- arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts +2 -2
@@ -146,7 +146,7 @@ &gmac2io { assigned-clocks = <&cru SCLK_MAC2IO>, <&cru SCLK_MAC2IO_EXT>; assigned-clock-parents = <&gmac_clkin>, <&gmac_clkin>; clock_in_out = "input"; - phy-handle = <&rtl8211e>; + phy-handle = <&rtl8211>; phy-mode = "rgmii"; phy-supply = <&vcc_io>; pinctrl-names = "default";
@@ -163,7 +163,7 @@ mdio { #address-cells = <1>; #size-cells = <0>; - rtl8211e: ethernet-phy@1 { + rtl8211: ethernet-phy@1 { reg = <1>; pinctrl-0 = <ð_phy_int_pin>, <ð_phy_reset_pin>; pinctrl-names = "default";
Signature · 1 line
2.43.0.76.g1a87c842ece3
PATCH 2/2 arm64: dts: rockchip: rock-pi-e: fix location of snps properties
A number of snps (Synopsys) properties are not in their correct location.
--- arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts +5 -5
@@ -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>; }; }; };
Signature · 1 line
2.43.0.76.g1a87c842ece3
Re: [PATCH 2/2] arm64: dts: rockchip: rock-pi-e: fix location of snps properties
Quoted 2 lines · Trevor Woerner
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.
ChenYu
Quoted 41 lines
Fixes b918e81f2145 ("arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E")Signed-off-by Trevor Woerner <[email protected]>--- arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts +5
-6@@ -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
Re: [PATCH 2/2] arm64: dts: rockchip: rock-pi-e: fix location of snps properties
Quoted 9 lines · Chen-Yu Tsai
Quoted 2 lines · Trevor Woerner
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.
Quoted 44 lines
ChenYu
Quoted 41 lines
Fixes b918e81f2145 ("arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E")Signed-off-by Trevor Woerner <[email protected]>--- arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts +5
-6@@ -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
Re: [PATCH 2/2] arm64: dts: rockchip: rock-pi-e: fix location of snps properties
Quoted 16 lines · Trevor Woerner
Quoted 9 lines · Chen-Yu Tsai
Quoted 2 lines · Trevor Woerner
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.
Quoted 4 lines
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.
[1] https://lore.kernel.org/linux-rockchip/[email protected]/
Regards,
Jonas
Quoted 46 lines
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.
Quoted 41 lines
ChenYu
Quoted 38 lines
Fixes b918e81f2145 ("arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E")Signed-off-by Trevor Woerner <[email protected]>--- arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts +5
-5@@ -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>; }; }; };
Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
--
2.43.0.76.g1a87c842ece3_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH 2/2] arm64: dts: rockchip: rock-pi-e: fix location of snps properties
Quoted 30 lines · Jonas Karlman
Quoted 16 lines · Trevor Woerner
Quoted 9 lines · Chen-Yu Tsai
Quoted 2 lines · Trevor Woerner
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.
Quoted 4 lines
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.
Quoted 8 lines
Following config options is disabled for rock-pi-e-rk3328 defconfig:
#
CONFIG_DM_ETH_PHYis not set
#CONFIG_PHY_REALTEKis not setAlso the driver needs to be updated to make use eth-phy uclass for it to work correctly. More similar to how the
dwc_eth_qosdriver 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].
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.
[2] https://lore.kernel.org/u-boot/[email protected]/T/
Regards,
Jonas
Quoted 55 lines
[1] https://lore.kernel.org/linux-rockchip/[email protected]/
Regards,
JonasQuoted 49 lines
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.
Quoted 44 lines
ChenYu
Quoted 41 lines
Fixes b918e81f2145 ("arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E")Signed-off-by Trevor Woerner <[email protected]>--- arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts +5
-6@@ -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
Re: [PATCH 2/2] arm64: dts: rockchip: rock-pi-e: fix location of snps properties
Quoted 36 lines · Jonas Karlman
Quoted 35 lines · Jonas Karlman
Quoted 16 lines · Trevor Woerner
Quoted 9 lines · Chen-Yu Tsai
Quoted 2 lines · Trevor Woerner
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.
Quoted 4 lines
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_PHYis not set
#CONFIG_PHY_REALTEKis not set
Quoted 6 lines
Quoted 3 lines
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_qosdriver 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.
Quoted 5 lines
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.
Quoted 61 lines
[2] https://lore.kernel.org/u-boot/[email protected]/T/
Regards,
JonasQuoted 55 lines
[1] https://lore.kernel.org/linux-rockchip/[email protected]/
Regards,
JonasQuoted 49 lines
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.
Quoted 44 lines
ChenYu
Quoted 41 lines
Fixes b918e81f2145 ("arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E")Signed-off-by Trevor Woerner <[email protected]>--- arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts +5
-6@@ -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
Heiko Stuebner
maintainerRe: [PATCH 2/2] arm64: dts: rockchip: rock-pi-e: fix location of snps properties
Hi,
Am Freitag, 19. Januar 2024, 00:03:12 CET schrieb Trevor Woerner:
Quoted 7 lines · Jonas Karlman
Quoted 3 lines · Jonas Karlman
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.
if I'm reading this correctly, this patch is not needed anymore, right?
I'm not yet sure about the first patch, as it really is just a cosmetic ;-)
Heiko
Re: [PATCH 2/2] arm64: dts: rockchip: rock-pi-e: fix location of snps properties
Quoted 4 lines · Heiko Stuebner
Hi,
Am Freitag, 19. Januar 2024, 00:03:12 CET schrieb Trevor Woerner:
Quoted 1 line · Jonas Karlman
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines · Jonas Karlman
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.
if I'm reading this correctly, this patch is not needed anymore, right?
True, patch 2/2 is not needed (or correct).
Quoted 2 lines
I'm not yet sure about the first patch, as it really is just a cosmetic ;-)
I wouldn't say it's entirely cosmetic. Some rock-pi-e boards come with the 8211e PHY and others come with the 8211f PHY. Labelling the phandle as "rtl8211e" would be misleading/confusing for people who had boards whose boot logs would report otherwise.
Re: (subset) [PATCH 1/2] arm64: dts: rockchip: rock-pi-e: adjust phy-handle name
In reply to Trevor WoernerQuoted 5 lines · Trevor Woerner
The rock-pi-e currently comes in 4 board spins, the latest one (v1.21) swaps out the Realtek 8211e PHY for an 8211f PHY. Therefore modify the phy-handle name to be more generic.
Applied, thanks!
[1/2] arm64: dts: rockchip: rock-pi-e: adjust phy-handle name
commit: abe3426c8176b2713713e8ba7cbc7cecc5dedd81
Best regards,
Signature · 1 line
Heiko Stuebner <[email protected]>