| Message ID | 20201024162515.30032-2-wens@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [01/10] Revert "arm: sun8i: orangepi-pc-plus: Set EMAC activity LEDs to active high" | expand |
在 2020-10-25星期日的 00:25 +0800，Chen-Yu Tsai写道： > From: Chen-Yu Tsai <wens@csie.org> > > 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. (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@csie.org> > --- > 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"; > }; > > -- > 2.28.0 >
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@csie.org> > > > > 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. 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@csie.org> > > --- > > > > 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"; > > > > };
于 2020年10月25日 GMT+08:00 上午2:30:35, "Jernej Škrabec" <jernej.skrabec@siol.net> 写到: >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@csie.org> >> > >> > 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. > >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@csie.org> >> > --- >> > >> > 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"; >> > >> > };
Hi, On Sat, 24 Oct 2020 at 20:39, Icenowy Zheng <icenowy@aosc.io> wrote: > > > > 于 2020年10月25日 GMT+08:00 上午2:30:35, "Jernej Škrabec" <jernej.skrabec@siol.net> 写到: > >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@csie.org> > >> > > >> > 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@csie.org> > >> > --- > >> > > >> > 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@googlegroups.com. > To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/9D317C52-5F28-41A9-80DA-DBADA142B042%40aosc.io.
Dne sobota, 24. oktober 2020 ob 18:25:07 CET je Chen-Yu Tsai napisal(a): > From: Chen-Yu Tsai <wens@csie.org> > > 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"). > > Fixes: c220aec2bb79 ("ARM: dts: sun6i: Add Merrii A31 Hummingbird support") > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Acked-by: Jernej Skrabec <jernej.skrabec@siol.net> Best regards, Jernej
On Sat, 24 Oct 2020 at 20:40, Icenowy Zheng <icenowy@aosc.io> wrote: > > > > 于 2020年10月25日 GMT+08:00 上午2:30:35, "Jernej Škrabec" <jernej.skrabec@siol.net> 写到: > >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@csie.org> > >> > > >> > 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. > This same issue is under discussion in other places as well: The following commit from v5.2 f81dadbcf7fd ("net: phy: realtek: Add rtl8211e rx/tx delays config") added handling of the TX/RX delay controls on the PHY, but did so incorrectly, and therefore had no effect on any of these systems. Commit bbc4d71d6354 ("net: phy: realtek: fix rtl8211e rx/tx delay config") fixed TX/RX delay handling in v5.9, and now, incorrect phy-mode settings in DT means losing Ethernet connectivity. This patch has been backported to -stable, even though it is not clear at all that the patch fixes any regressions, it only fixes some broken code that did not turn out to be active in the first place. The result of this is that affected systems in the field using v5.4 or v5.8 stable kernels will lose their Ethernet connectivity once they upgrade to a newer release of the same v5.x tree that they shipped with. We may have ways to work around this more cleanly, but in the meantime, the stable backports of bbc4d71d6354 should simply be reverted (IMHO)
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"; };