Re: [linux-sunxi] [PATCH 02/10] ARM: dts: sun6i: a31-hummingbird: Enable RGMII RX/TX delay on Ethernet PHY

[Clément Péron]

Hi,

On Sat, 24 Oct 2020 at 20:39, Icenowy Zheng <icenowy@xxxxxxx> wrote:
>
>
>
> 于 2020年10月25日 GMT+08:00 上午2:30:35, "Jernej Škrabec" <jernej.skrabec@xxxxxxxx> 写到:
> >Dne sobota, 24. oktober 2020 ob 19:51:06 CEST je Icenowy Zheng
> >napisal(a):
> >> 在 2020-10-25星期日的 00:25 +0800，Chen-Yu Tsai写道：
> >>
> >> > From: Chen-Yu Tsai <wens@xxxxxxxx>
> >> >
> >> > The Ethernet PHY on the A31 Hummingbird has the RX and TX delays
> >> > enabled on the PHY, using pull-ups on the RXDLY and TXDLY pins.
> >> >
> >> > Fix the phy-mode description to correct reflect this so that the
> >> > implementation doesn't reconfigure the delays incorrectly. This
> >> > happened with commit bbc4d71d6354 ("net: phy: realtek: fix rtl8211e
> >> > rx/tx delay config").
> >>
> >> Personally I think they should revert this commit, and consider other
> >> solution.
> >>
> >> This commit breaks everything.
> >>
> >
> >Previously broken driver allowed inproper DT to work, so you have to
> >fix
> >everything eventually.
>
> There is no "improper DT" if it's already shipped, DT should suffer driver
> change, not changed to match driver.
>
> I think at least Fedora tends to ship a DT with a system image that will
> not get updated when kernel gets updated.

I think we are missing a phy-mode saying use RGMII but don't change
the delay set by the hardware.
Or maybe introduce a phy-mode='rgmii-noid' to explicitly disable the
RX/TD delay using software bits
and use phy-mode='rgmii' to keep the delay set by the hardware.

Clement

>
> >
> >Plus side, there is no need to have hack for Pine64 Plus ethernet
> >anymore.
> >
> >Best regards,
> >Jernej
> >
> >> (Although the patch on individual DT patches are technically correct)
> >>
> >> > Fixes: c220aec2bb79 ("ARM: dts: sun6i: Add Merrii A31 Hummingbird
> >> > support")
> >> > Signed-off-by: Chen-Yu Tsai <wens@xxxxxxxx>
> >> > ---
> >> >
> >> >  arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> >> > b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> >> > index 049e6ab3cf56..73de34ae37fd 100644
> >> > --- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> >> > +++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> >> > @@ -154,7 +154,7 @@ &gmac {
> >> >
> >> >    pinctrl-names = "default";
> >> >    pinctrl-0 = <&gmac_rgmii_pins>;
> >> >    phy-handle = <&phy1>;
> >> >
> >> > -  phy-mode = "rgmii";
> >> > +  phy-mode = "rgmii-id";
> >> >
> >> >    status = "okay";
> >> >
> >> >  };
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@xxxxxxxxxxxxxxxx.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/9D317C52-5F28-41A9-80DA-DBADA142B042%40aosc.io.