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

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

[Petr Štetiar]

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(-)

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

[PATCH v2 2/3] ARM: dts: sun7i: add support for A20-olinuxino-lime2 Revisions G/G1/G2

[Petr Štetiar]

Lime2 HW first public revisions G/G1/G2 used RTL8211E PHYs. Later public
revision K and newest are using KSZ9031 PHYs. Up to commit bbc4d71d6354
("net: phy: realtek: fix rtl8211e rx/tx delay config") it was possible
to use same DTS for A20-olinuxino-lime2 with either KSZ9031 or RTL8211E
PHYs, thus on all HW revisions.

Having commit bbc4d71d6354 ("net: phy: realtek: fix rtl8211e rx/tx delay
config") applied, Bastien found out, that his board using RTL8211E PHY
doesn't work anymore and tried to fix it by providing proper `phy-mode`
in commit 55dd7e059098 ("ARM: dts: sun7i: A20-olinuxino-lime2: Fix
ethernet phy-mode") with following reasoning:

 Commit bbc4d71d6354 ("net: phy: realtek: fix rtl8211e rx/tx delay
 config") sets the RX/TX delay according to the phy-mode property in the
 device tree. For the A20-olinuxino-lime2 board this is "rgmii", which is
 the wrong setting.

Indeed, the settings were likely wrong, but only for boards with G/G1/G2
revisions using RTL8211E PHY. Those settings were still correct on
boards with HW revisions K and later with KSZ9031 PHY, so this fix was
incorrect.

So fix it properly by introducing separate DTS for boards with G/G1/G2
revisions using proper PHY mode for RTL8211E PHY.

Cc: stable@vger.kernel.org
Cc: Bastien Roucariès <rouca@debian.org>
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/Makefile                            |  2 ++
 .../boot/dts/sun7i-a20-olinuxino-lime2-emmc-revG.dts  | 11 +++++++++++
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-revG.dts  | 11 +++++++++++
 3 files changed, 24 insertions(+)
 create mode 100644 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-emmc-revG.dts
 create mode 100644 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-revG.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index e41eca79c950..241a1a229f0f 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1244,7 +1244,9 @@ dtb-$(CONFIG_MACH_SUN7I) += \
 	sun7i-a20-olinuxino-lime.dtb \
 	sun7i-a20-olinuxino-lime-emmc.dtb \
 	sun7i-a20-olinuxino-lime2.dtb \
+	sun7i-a20-olinuxino-lime2-revG.dtb \
 	sun7i-a20-olinuxino-lime2-emmc.dtb \
+	sun7i-a20-olinuxino-lime2-emmc-revG.dtb \
 	sun7i-a20-olinuxino-micro.dtb \
 	sun7i-a20-olinuxino-micro-emmc.dtb \
 	sun7i-a20-orangepi.dtb \
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-emmc-revG.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-emmc-revG.dts
new file mode 100644
index 000000000000..1e29f973614d
--- /dev/null
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-emmc-revG.dts
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include "sun7i-a20-olinuxino-lime2-emmc.dts"
+
+/ {
+	model = "Olimex A20-OLinuXino-LIME2-eMMC (Rev G/G1/G2)";
+	compatible = "olimex,a20-olinuxino-lime2-emmc-revG", "allwinner,sun7i-a20";
+};
+
+&gmac {
+	phy-mode = "rgmii-id";
+};
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-revG.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-revG.dts
new file mode 100644
index 000000000000..7122dc99810a
--- /dev/null
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-revG.dts
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include "sun7i-a20-olinuxino-lime2.dts"
+
+/ {
+	model = "Olimex A20-OLinuXino-LIME2 (Rev G/G1/G2)";
+	compatible = "olimex,a20-olinuxino-lime2-revG", "allwinner,sun7i-a20";
+};
+
+&gmac {
+	phy-mode = "rgmii-id";
+};

[PATCH v2 3/3] dt-bindings: arm: sunxi: add A20-olinuxino-lime2 Revisions G/G1/G2

[Petr Štetiar]

Add DT bindings for A20-olinuxino-lime2 Revisions G/G1/G2 boards.

Cc: stable@vger.kernel.org
Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
 Documentation/devicetree/bindings/arm/sunxi.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
index c8a3102c0fde..d142209e76a4 100644
--- a/Documentation/devicetree/bindings/arm/sunxi.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
@@ -610,11 +610,21 @@ properties:
           - const: olimex,a20-olinuxino-lime2
           - const: allwinner,sun7i-a20
 
+      - description: Olimex A20-OlinuXino LIME2 Revisions G/G1/G2
+        items:
+          - const: olimex,a20-olinuxino-lime2-revG
+          - const: allwinner,sun7i-a20
+
       - description: Olimex A20-OlinuXino LIME2 (with eMMC)
         items:
           - const: olimex,a20-olinuxino-lime2-emmc
           - const: allwinner,sun7i-a20
 
+      - description: Olimex A20-OlinuXino LIME2 Revisions G/G1/G2 (with eMMC)
+        items:
+          - const: olimex,a20-olinuxino-lime2-emmc-revG
+          - const: allwinner,sun7i-a20
+
       - description: Olimex A20-OlinuXino Micro
         items:
           - const: olimex,a20-olinuxino-micro

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

[Jernej Škrabec]

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

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

[Andre Przywara]

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

Re: [PATCH v2 3/3] dt-bindings: arm: sunxi: add A20-olinuxino-lime2 Revisions G/G1/G2

[Rob Herring]

On Tue, 15 Mar 2022 10:52:44 +0100, Petr Štetiar wrote:
> Add DT bindings for A20-olinuxino-lime2 Revisions G/G1/G2 boards.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Petr Štetiar <ynezz@true.cz>
> ---
>  Documentation/devicetree/bindings/arm/sunxi.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>