[PATCH 1/2] arm64: dts: rockchip: rock-pi-e: adjust phy-handle name

[PATCH 1/2] arm64: dts: rockchip: rock-pi-e: adjust phy-handle name

[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.

Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 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 018a3a5075c7..096cfa19036e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock-pi-e.dts
@@ -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 = <&eth_phy_int_pin>, <&eth_phy_reset_pin>;
 			pinctrl-names = "default";
-- 
2.43.0.76.g1a87c842ece3

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

[Trevor Woerner]

A number of snps (Synopsys) properties are not in their correct location.

Fixes: b918e81f2145 ("arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E")
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 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

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

[Chen-Yu Tsai]

On Wed, Jan 17, 2024 at 4:41 AM Trevor Woerner <twoerner@gmail.com> 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.

ChenYu

> Fixes: b918e81f2145 ("arm64: dts: rockchip: rk3328: Add Radxa ROCK Pi E")
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> ---
>  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
>

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@gmail.com> 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@gmail.com>
> > ---
> >  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
> >

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

[Jonas Karlman]

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@gmail.com> 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.

[1] https://lore.kernel.org/linux-rockchip/47d55aca-bee6-810f-379f-9431649fefa6@kwiboo.se/

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@gmail.com>
>>> ---
>>>  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
>>>
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

[Jonas Karlman]

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@gmail.com> 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].

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/20240118071949.927089-1-jonas@kwiboo.se/T/

Regards,
Jonas

> 
> [1] https://lore.kernel.org/linux-rockchip/47d55aca-bee6-810f-379f-9431649fefa6@kwiboo.se/
> 
> 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@gmail.com>
>>>> ---
>>>>  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
>>>>

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

[Trevor Woerner]

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@gmail.com> 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@kwiboo.se/T/
> 
> Regards,
> Jonas
> 
> > 
> > [1] https://lore.kernel.org/linux-rockchip/47d55aca-bee6-810f-379f-9431649fefa6@kwiboo.se/
> > 
> > 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@gmail.com>
> >>>> ---
> >>>>  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
> >>>>
> 

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

[Heiko Stuebner]

Hi,

Am Freitag, 19. Januar 2024, 00:03:12 CET schrieb Trevor Woerner:
> On Thu 2024-01-18 @ 08:31:30 AM, Jonas Karlman wrote:
> > On 2024-01-17 09:15, Jonas Karlman wrote:
> > 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

[Trevor Woerner]

On Thu 2024-01-25 @ 09:12:46 PM, Heiko Stuebner wrote:
> Hi,
> 
> Am Freitag, 19. Januar 2024, 00:03:12 CET schrieb Trevor Woerner:
> > On Thu 2024-01-18 @ 08:31:30 AM, Jonas Karlman wrote:
> > > On 2024-01-17 09:15, Jonas Karlman wrote:
> > > 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).

> 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

[Heiko Stuebner]

On Tue, 16 Jan 2024 15:41:01 -0500, Trevor Woerner wrote:
> 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,
-- 
Heiko Stuebner <heiko@sntech.de>