[PATCH 0/4] net: stmmac: dwmac-mediatek: Fix inverted logic for mediatek,mac-wol

[PATCH 0/4] net: stmmac: dwmac-mediatek: Fix inverted logic for mediatek,mac-wol

[Nícolas F. R. A. Prado]

This series fixes the inverted handling of the mediatek,mac-wol DT
property while keeping backward compatibility. It does so by introducing
a new property on patch 1 and updating the driver to handle it on patch
2. Patch 3 adds this property on the Genio 700 EVK DT, where this issue
was noticed, to get WOL working on that platform. Patch 4 adds the new
property on all DTs with the MediaTek DWMAC ethernet node enabled
and inverts the presence of mediatek,mac-wol to maintain the
current behavior and have it match the description in the binding.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
Nícolas F. R. A. Prado (4):
      net: dt-bindings: dwmac: Introduce mediatek,mac-wol-noninverted
      net: stmmac: dwmac-mediatek: Handle non-inverted mediatek,mac-wol
      arm64: dts: mediatek: mt8390-genio-700-evk: Enable ethernet MAC WOL
      arm64: dts: mediatek: Add mediatek,mac-wol-noninverted to ethernet nodes

 Documentation/devicetree/bindings/net/mediatek-dwmac.yaml     | 11 +++++++++++
 arch/arm64/boot/dts/mediatek/mt2712-evb.dts                   |  2 ++
 arch/arm64/boot/dts/mediatek/mt8195-demo.dts                  |  2 ++
 arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts         |  1 +
 arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts        |  2 +-
 arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts |  2 ++
 arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts         |  2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c          |  9 ++++++---
 8 files changed, 26 insertions(+), 5 deletions(-)
---
base-commit: c88416ba074a8913cf6d61b789dd834bbca6681c
change-id: 20241101-mediatek-mac-wol-noninverted-198c6c404536

Best regards,
-- 
Nícolas F. R. A. Prado <nfraprado@collabora.com>

[PATCH 1/4] net: dt-bindings: dwmac: Introduce mediatek,mac-wol-noninverted

[Nícolas F. R. A. Prado]

The mediatek,mac-wol property, as described, was intended to indicate
that the MAC supports Wake-On-LAN (WOL) and that it should be used
instead of the PHY WOL. However, the driver code currently handles it
backwards: setting up PHY WOL when the property is present and vice
versa.

In order to rectify the property's handling while still maintaining
backwards compatibility, introduce a new property to indicate that the
mediatek,mac-wol property should be handled as it is described, not
inverted, and make it required for new DTs.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 Documentation/devicetree/bindings/net/mediatek-dwmac.yaml | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
index ed9d845f600804964e0000dd4354898673fafe08..0f20c4e09e79ff722d53a364825da29bd6323c31 100644
--- a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
@@ -125,6 +125,15 @@ properties:
       3. the inside clock, which be sent to MAC, will be inversed in RMII case when
          the reference clock is from MAC.
 
+  mediatek,mac-wol-noninverted:
+    type: boolean
+    description:
+      Previously the driver parsed the mediatek,mac-wol property backwards,
+      enabling the PHY WOL when the property was present, and vice versa. That
+      behavior is kept for backwards compatility, but newer DTs should specify
+      this property to have the driver handle the mediatek,mac-wol property as
+      it is described in this binding.
+
   mediatek,mac-wol:
     type: boolean
     description:
@@ -140,6 +149,7 @@ required:
   - clock-names
   - phy-mode
   - mediatek,pericfg
+  - mediatek,mac-wol-noninverted
 
 unevaluatedProperties: false
 
@@ -181,4 +191,5 @@ examples:
         snps,rxpbl = <1>;
         snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
         snps,reset-delays-us = <0 10000 10000>;
+        mediatek,mac-wol-noninverted;
     };

-- 
2.47.0

[PATCH 3/4] arm64: dts: mediatek: mt8390-genio-700-evk: Enable ethernet MAC WOL

[Nícolas F. R. A. Prado]

Add the mediatek,mac-wol-noninverted property to the ethernet node to
make the driver parse the mediatek,mac-wol property as originally
intended: enabling the MAC WOL. This gets WOL working on the Genio 700
EVK board.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts b/arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts
index 13f2e0e3fa8ab6679f843693230b9661d323a705..83c10517458d1df2f14e41baeff628f46ded1618 100644
--- a/arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts
@@ -898,6 +898,7 @@ &eth {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&eth_default_pins>;
 	pinctrl-1 = <&eth_sleep_pins>;
+	mediatek,mac-wol-noninverted;
 	mediatek,mac-wol;
 	snps,reset-gpio = <&pio 147 GPIO_ACTIVE_HIGH>;
 	snps,reset-delays-us = <0 10000 10000>;

-- 
2.47.0

[PATCH 4/4] arm64: dts: mediatek: Add mediatek,mac-wol-noninverted to ethernet nodes

[Nícolas F. R. A. Prado]

Add the newly introduced and now required mediatek,mac-wol-noninverted
property to the dwmac ethernet nodes and invert the presence of the
mediatek,mac-wol property to make it align with the description on the
binding and maintain the current behavior.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt2712-evb.dts                   | 2 ++
 arch/arm64/boot/dts/mediatek/mt8195-demo.dts                  | 2 ++
 arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts        | 2 +-
 arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts | 2 ++
 arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts         | 2 +-
 5 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
index c84c47c1352fba49d219fb8ace17a74953927fdc..09760a0784bfb59511ea64fb44b7aeb66326f81b 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
@@ -115,6 +115,8 @@ &eth {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&eth_default>;
 	pinctrl-1 = <&eth_sleep>;
+	mediatek,mac-wol-noninverted;
+	mediatek,mac-wol;
 	status = "okay";
 
 	mdio {
diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
index 31d424b8fc7cedef65489392eb279b7fd2194a4a..f48baa0b7dcbb95816517b7e501d87e39ac63a2d 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
@@ -109,6 +109,8 @@ &eth {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&eth_default_pins>;
 	pinctrl-1 = <&eth_sleep_pins>;
+	mediatek,mac-wol-noninverted;
+	mediatek,mac-wol;
 	status = "okay";
 
 	mdio {
diff --git a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
index 5f16fb82058056cf8cf6318c9fc373601bd6eb60..290fcdce1c9f49c475403fa4aa7a0911605d4abd 100644
--- a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
@@ -177,7 +177,7 @@ &eth {
 	snps,reset-gpio = <&pio 93 GPIO_ACTIVE_HIGH>;
 	snps,reset-delays-us = <0 10000 10000>;
 	mediatek,tx-delay-ps = <2030>;
-	mediatek,mac-wol;
+	mediatek,mac-wol-noninverted;
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&eth_default_pins>;
 	pinctrl-1 = <&eth_sleep_pins>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts b/arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts
index e2e75b8ff91880711c82f783c7ccbef4128b7ab4..ebd0deb4e9ec1d67182c7602203e4fa1a0fb1c0c 100644
--- a/arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts
@@ -271,6 +271,8 @@ &eth {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&eth_default_pins>;
 	pinctrl-1 = <&eth_sleep_pins>;
+	mediatek,mac-wol-noninverted;
+	mediatek,mac-wol;
 	status = "okay";
 
 	mdio {
diff --git a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
index 14ec970c4e491fbd69bf2800639abf726d47589a..a541d4fb9621c55f789d55a1eb985030827b158b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
@@ -179,7 +179,7 @@ &eth {
 	pinctrl-0 = <&eth_default_pins>;
 	pinctrl-1 = <&eth_sleep_pins>;
 	mediatek,tx-delay-ps = <2030>;
-	mediatek,mac-wol;
+	mediatek,mac-wol-noninverted;
 	snps,reset-gpio = <&pio 93 GPIO_ACTIVE_HIGH>;
 	snps,reset-delays-us = <0 20000 100000>;
 	status = "okay";

-- 
2.47.0

Re: [PATCH 0/4] net: stmmac: dwmac-mediatek: Fix inverted logic for mediatek,mac-wol

[AngeloGioacchino Del Regno]

Il 01/11/24 16:20, Nícolas F. R. A. Prado ha scritto:
> This series fixes the inverted handling of the mediatek,mac-wol DT
> property while keeping backward compatibility. It does so by introducing
> a new property on patch 1 and updating the driver to handle it on patch
> 2. Patch 3 adds this property on the Genio 700 EVK DT, where this issue
> was noticed, to get WOL working on that platform. Patch 4 adds the new
> property on all DTs with the MediaTek DWMAC ethernet node enabled
> and inverts the presence of mediatek,mac-wol to maintain the
> current behavior and have it match the description in the binding.
> 

Actually, I'm sure that all of these boards *do* need MAC WOL and *not* PHY WOL.

The only one I'm unsure about is MT2712, but that's an evaluation board and not
a retail product with "that kind of diffusion".

I think you can just fix the bug in the driver without getting new properties
and such. One commit, two lines.

Cheers,
Angelo

> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> Nícolas F. R. A. Prado (4):
>        net: dt-bindings: dwmac: Introduce mediatek,mac-wol-noninverted
>        net: stmmac: dwmac-mediatek: Handle non-inverted mediatek,mac-wol
>        arm64: dts: mediatek: mt8390-genio-700-evk: Enable ethernet MAC WOL
>        arm64: dts: mediatek: Add mediatek,mac-wol-noninverted to ethernet nodes
> 
>   Documentation/devicetree/bindings/net/mediatek-dwmac.yaml     | 11 +++++++++++
>   arch/arm64/boot/dts/mediatek/mt2712-evb.dts                   |  2 ++
>   arch/arm64/boot/dts/mediatek/mt8195-demo.dts                  |  2 ++
>   arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts         |  1 +
>   arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts        |  2 +-
>   arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts |  2 ++
>   arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts         |  2 +-
>   drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c          |  9 ++++++---
>   8 files changed, 26 insertions(+), 5 deletions(-)
> ---
> base-commit: c88416ba074a8913cf6d61b789dd834bbca6681c
> change-id: 20241101-mediatek-mac-wol-noninverted-198c6c404536
> 
> Best regards,

Re: [PATCH 0/4] net: stmmac: dwmac-mediatek: Fix inverted logic for mediatek,mac-wol

[Nícolas F. R. A. Prado]

On Mon, Nov 04, 2024 at 11:18:48AM +0100, AngeloGioacchino Del Regno wrote:
> Il 01/11/24 16:20, Nícolas F. R. A. Prado ha scritto:
> > This series fixes the inverted handling of the mediatek,mac-wol DT
> > property while keeping backward compatibility. It does so by introducing
> > a new property on patch 1 and updating the driver to handle it on patch
> > 2. Patch 3 adds this property on the Genio 700 EVK DT, where this issue
> > was noticed, to get WOL working on that platform. Patch 4 adds the new
> > property on all DTs with the MediaTek DWMAC ethernet node enabled
> > and inverts the presence of mediatek,mac-wol to maintain the
> > current behavior and have it match the description in the binding.
> > 
> 
> Actually, I'm sure that all of these boards *do* need MAC WOL and *not* PHY WOL.
> 
> The only one I'm unsure about is MT2712, but that's an evaluation board and not
> a retail product with "that kind of diffusion".
> 
> I think you can just fix the bug in the driver without getting new properties
> and such. One commit, two lines.

Alright, since you're sure all the boards need MAC WOL this is simpler: I can
just fix the driver handling, and also update all the DTs to make sure they use
MAC WOL. (Right now some are enabling PHY WOL, mt8395-genio-1200-evk.dts and
mt8395-radxa-nio-12l.dts, while the others are enabling the MAC WOL)

The MT2712 EVB currently enables MAC WOL, so even though you're not sure about
that one, I think it makes sense to keep the current behavior ie add the
mediatek,mac-wol property just like for the others, so it keeps using MAC WOL.

I'll wait a couple days in case there are more comments and then I'll send v2
with those changes.

Thanks,
Nícolas