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

From: Andre Przywara <andre.przywara@arm.com>
To: "Jernej Škrabec" <jernej.skrabec@gmail.com>,
	"Petr Štetiar" <ynezz@true.cz>
Cc: "Rob Herring" <robh+dt@kernel.org>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Chen-Yu Tsai" <wens@csie.org>,
	stable@vger.kernel.org, "Bastien Roucariès" <rouca@debian.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] Revert "ARM: dts: sun7i: A20-olinuxino-lime2: Fix ethernet phy-mode"
Date: Tue, 15 Mar 2022 22:44:44 +0000	[thread overview]
Message-ID: <20220315224444.1825833e@slackpad.lan> (raw)
In-Reply-To: <44524634.fMDQidcC6G@kista>

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";
> >  };
> >  
> >   
> 
> 
> 