diff mbox series

[v2,1/3] Revert "ARM: dts: sun7i: A20-olinuxino-lime2: Fix ethernet phy-mode"

Message ID 20220315095244.29718-2-ynezz@true.cz (mailing list archive)
State New, archived
Headers show
Series [v2,1/3] Revert "ARM: dts: sun7i: A20-olinuxino-lime2: Fix ethernet phy-mode" | expand

Commit Message

Petr Štetiar March 15, 2022, 9:52 a.m. UTC
This reverts commit 55dd7e059098ce4bd0a55c251cb78e74604abb57 as it
breaks network on my A20-olinuxino-lime2 hardware revision "K" which has
Micrel KSZ9031RNXCC-TR Gigabit PHY. Bastien has probably some previous
hardware revisions which were based on RTL8211E-VB-CG1 PHY and thus this
fix was working on his board.

Cc: stable@vger.kernel.org
Cc: Bastien Roucariès <rouca@debian.org>
References: https://github.com/openwrt/openwrt/issues/9153
References: https://github.com/OLIMEX/OLINUXINO/blob/master/HARDWARE/A20-OLinuXino-LIME2/hardware_revision_changes_log.txt
Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jernej Škrabec March 15, 2022, 6:50 p.m. UTC | #1
Hi Petr!

Dne torek, 15. marec 2022 ob 10:52:42 CET je Petr Štetiar napisal(a):
> This reverts commit 55dd7e059098ce4bd0a55c251cb78e74604abb57 as it
> breaks network on my A20-olinuxino-lime2 hardware revision "K" which has
> Micrel KSZ9031RNXCC-TR Gigabit PHY. Bastien has probably some previous
> hardware revisions which were based on RTL8211E-VB-CG1 PHY and thus this
> fix was working on his board.

NAK.

As Corentin mentioned in another discussion, new DT variant should be 
introduced for newer board model. Otherwise we can play this revert game with 
each new revision which changes Ethernet PHY behaviour. It also makes most 
sense to have naming chronologically sorted. If board name in DT file doesn't 
have any postfix, it should be compatible with earliest publicly available 
board. If board manufacturer releases new board variant with incompatible 
changes, new DT with appropriate postfix should be introduced.

I understand that this is frustrating for you, but whole situation around 
mentioned commit is unfortunate and we can't satisfy everyone.

Also good way to solve such issues is to apply DT overlay in bootloader based 
on board revision number. I know Olimex implemented DT fixup in their 
downstream U-Boot fork.

Best regards,
Jernej

> 
> Cc: stable@vger.kernel.org
> Cc: Bastien Roucariès <rouca@debian.org>
> References: https://github.com/openwrt/openwrt/issues/9153
> References: https://github.com/OLIMEX/OLINUXINO/blob/master/HARDWARE/A20-OLinuXino-LIME2/hardware_revision_changes_log.txt
> Signed-off-by: Petr Štetiar <ynezz@true.cz>
> ---
>  arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/
dts/sun7i-a20-olinuxino-lime2.dts
> index ecb91fb899ff..8077f1716fbc 100644
> --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> @@ -112,7 +112,7 @@ &gmac {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&gmac_rgmii_pins>;
>  	phy-handle = <&phy1>;
> -	phy-mode = "rgmii-id";
> +	phy-mode = "rgmii";
>  	status = "okay";
>  };
>  
>
Andre Przywara March 15, 2022, 10:44 p.m. UTC | #2
On Tue, 15 Mar 2022 19:50:23 +0100
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

> Hi Petr!
> 
> Dne torek, 15. marec 2022 ob 10:52:42 CET je Petr Štetiar napisal(a):
> > This reverts commit 55dd7e059098ce4bd0a55c251cb78e74604abb57 as it
> > breaks network on my A20-olinuxino-lime2 hardware revision "K" which has
> > Micrel KSZ9031RNXCC-TR Gigabit PHY. Bastien has probably some previous
> > hardware revisions which were based on RTL8211E-VB-CG1 PHY and thus this
> > fix was working on his board.  
> 
> NAK.
> 
> As Corentin mentioned in another discussion, new DT variant should be 
> introduced for newer board model. Otherwise we can play this revert game with 
> each new revision which changes Ethernet PHY behaviour. It also makes most 
> sense to have naming chronologically sorted. If board name in DT file doesn't 
> have any postfix, it should be compatible with earliest publicly available 
> board. If board manufacturer releases new board variant with incompatible 
> changes, new DT with appropriate postfix should be introduced.
> 
> I understand that this is frustrating for you, but whole situation around 
> mentioned commit is unfortunate and we can't satisfy everyone.
> 
> Also good way to solve such issues is to apply DT overlay in bootloader based 
> on board revision number. I know Olimex implemented DT fixup in their 
> downstream U-Boot fork.

I agree with Jernej's here.
I had a quick look into the U-Boot source, and it seem like the Micrel
PHY should work there, since its phy_driver.config routine seems to
ignore the phy-mode property (in contrast to its 9131 sibling in the
same file). So we can go with the Realtek setting in the DT.

If U-Boot's networking itself is fine, we can just try to fix up the
DT. Looks like board/sunxi/board.c:ft_board_setup() is the place. The
PHY is autodetected, I am pretty sure we can somehow read the PHY
driver name, and depending on that just patch the phy-mode property.

Does that sound like a way out?

Cheers,
Andre

> Best regards,
> Jernej
> 
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Bastien Roucariès <rouca@debian.org>
> > References: https://github.com/openwrt/openwrt/issues/9153
> > References: https://github.com/OLIMEX/OLINUXINO/blob/master/HARDWARE/A20-OLinuXino-LIME2/hardware_revision_changes_log.txt
> > Signed-off-by: Petr Štetiar <ynezz@true.cz>
> > ---
> >  arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/  
> dts/sun7i-a20-olinuxino-lime2.dts
> > index ecb91fb899ff..8077f1716fbc 100644
> > --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > @@ -112,7 +112,7 @@ &gmac {
> >  	pinctrl-names = "default";
> >  	pinctrl-0 = <&gmac_rgmii_pins>;
> >  	phy-handle = <&phy1>;
> > -	phy-mode = "rgmii-id";
> > +	phy-mode = "rgmii";
> >  	status = "okay";
> >  };
> >  
> >   
> 
> 
>
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
index ecb91fb899ff..8077f1716fbc 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
@@ -112,7 +112,7 @@  &gmac {
 	pinctrl-names = "default";
 	pinctrl-0 = <&gmac_rgmii_pins>;
 	phy-handle = <&phy1>;
-	phy-mode = "rgmii-id";
+	phy-mode = "rgmii";
 	status = "okay";
 };