diff mbox series

[v2,2/2] arm64: dts: mediatek: Set mediatek,mac-wol on DWMAC node for all boards

Message ID 20241109-mediatek-mac-wol-noninverted-v2-2-0e264e213878@collabora.com (mailing list archive)
State New
Headers show
Series net: stmmac: dwmac-mediatek: Fix inverted logic for mediatek,mac-wol | expand

Commit Message

Nícolas F. R. A. Prado Nov. 9, 2024, 3:16 p.m. UTC 
Due to the mediatek,mac-wol property previously being handled backwards
by the dwmac-mediatek driver, its use in the DTs seems to have been
inconsistent.

Now that the driver has been fixed, correct this description. All the
currently upstream boards support MAC WOL, so add the mediatek,mac-wol
property to the missing ones.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt2712-evb.dts                   | 1 +
 arch/arm64/boot/dts/mediatek/mt8195-demo.dts                  | 1 +
 arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts | 1 +
 3 files changed, 3 insertions(+)

Comments

AngeloGioacchino Del Regno Nov. 14, 2024, 9:26 a.m. UTC | #1 
Il 09/11/24 16:16, Nícolas F. R. A. Prado ha scritto:
> Due to the mediatek,mac-wol property previously being handled backwards
> by the dwmac-mediatek driver, its use in the DTs seems to have been
> inconsistent.
> 
> Now that the driver has been fixed, correct this description. All the
> currently upstream boards support MAC WOL, so add the mediatek,mac-wol
> property to the missing ones.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt2712-evb.dts                   | 1 +
>   arch/arm64/boot/dts/mediatek/mt8195-demo.dts                  | 1 +
>   arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts | 1 +
>   3 files changed, 3 insertions(+)
> 

..snip..

> diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> index 31d424b8fc7cedef65489392eb279b7fd2194a4a..c12684e8c449b2d7b3b3a79086925bfe5ae0d8f8 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> @@ -109,6 +109,7 @@ &eth {
>   	pinctrl-names = "default", "sleep";
>   	pinctrl-0 = <&eth_default_pins>;
>   	pinctrl-1 = <&eth_sleep_pins>;
> +	mediatek,mac-wol;

The demo board has the same WoL capability as the EVK, so you can avoid adding the
mac-wol property here.

>   	status = "okay";
>   
>   	mdio {
> 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..4985b65925a9ed10ad44a6e58b9657a9dd48751f 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,7 @@ &eth {
>   	pinctrl-names = "default", "sleep";
>   	pinctrl-0 = <&eth_default_pins>;
>   	pinctrl-1 = <&eth_sleep_pins>;
> +	mediatek,mac-wol;

I'm mostly sure that Kontron's i1200 works the same as the EVK in regards to WoL.

Michael, I recall you worked on this board - can you please confirm?

Thanks,
Angelo
Michael Walle Nov. 14, 2024, 12:29 p.m. UTC | #2 
Hi,

On Thu Nov 14, 2024 at 10:26 AM CET, AngeloGioacchino Del Regno wrote:
> Il 09/11/24 16:16, Nícolas F. R. A. Prado ha scritto:
> > Due to the mediatek,mac-wol property previously being handled backwards
> > by the dwmac-mediatek driver, its use in the DTs seems to have been
> > inconsistent.
> > 
> > Now that the driver has been fixed, correct this description. All the
> > currently upstream boards support MAC WOL, so add the mediatek,mac-wol
> > property to the missing ones.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >   arch/arm64/boot/dts/mediatek/mt2712-evb.dts                   | 1 +
> >   arch/arm64/boot/dts/mediatek/mt8195-demo.dts                  | 1 +
> >   arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts | 1 +
> >   3 files changed, 3 insertions(+)
> > 
>
> ..snip..
>
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > index 31d424b8fc7cedef65489392eb279b7fd2194a4a..c12684e8c449b2d7b3b3a79086925bfe5ae0d8f8 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > @@ -109,6 +109,7 @@ &eth {
> >   	pinctrl-names = "default", "sleep";
> >   	pinctrl-0 = <&eth_default_pins>;
> >   	pinctrl-1 = <&eth_sleep_pins>;
> > +	mediatek,mac-wol;
>
> The demo board has the same WoL capability as the EVK, so you can avoid adding the
> mac-wol property here.

Not sure I can follow you here.

>
> >   	status = "okay";
> >   
> >   	mdio {
> > 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..4985b65925a9ed10ad44a6e58b9657a9dd48751f 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,7 @@ &eth {
> >   	pinctrl-names = "default", "sleep";
> >   	pinctrl-0 = <&eth_default_pins>;
> >   	pinctrl-1 = <&eth_sleep_pins>;
> > +	mediatek,mac-wol;
>
> I'm mostly sure that Kontron's i1200 works the same as the EVK in regards to WoL.
>
> Michael, I recall you worked on this board - can you please confirm?

I'd say so. Honestly, I've never tried WoL on this board, but I'm
not aware of any difference to the *demo* board (not the EVK).

-michael
AngeloGioacchino Del Regno Nov. 14, 2024, 1:56 p.m. UTC | #3 
Il 14/11/24 13:29, Michael Walle ha scritto:
> Hi,
> 
> On Thu Nov 14, 2024 at 10:26 AM CET, AngeloGioacchino Del Regno wrote:
>> Il 09/11/24 16:16, Nícolas F. R. A. Prado ha scritto:
>>> Due to the mediatek,mac-wol property previously being handled backwards
>>> by the dwmac-mediatek driver, its use in the DTs seems to have been
>>> inconsistent.
>>>
>>> Now that the driver has been fixed, correct this description. All the
>>> currently upstream boards support MAC WOL, so add the mediatek,mac-wol
>>> property to the missing ones.
>>>
>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>>> ---
>>>    arch/arm64/boot/dts/mediatek/mt2712-evb.dts                   | 1 +
>>>    arch/arm64/boot/dts/mediatek/mt8195-demo.dts                  | 1 +
>>>    arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts | 1 +
>>>    3 files changed, 3 insertions(+)
>>>
>>
>> ..snip..
>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> index 31d424b8fc7cedef65489392eb279b7fd2194a4a..c12684e8c449b2d7b3b3a79086925bfe5ae0d8f8 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> @@ -109,6 +109,7 @@ &eth {
>>>    	pinctrl-names = "default", "sleep";
>>>    	pinctrl-0 = <&eth_default_pins>;
>>>    	pinctrl-1 = <&eth_sleep_pins>;
>>> +	mediatek,mac-wol;
>>
>> The demo board has the same WoL capability as the EVK, so you can avoid adding the
>> mac-wol property here.
> 
> Not sure I can follow you here.
> 

That's in the sense that they do WoL through the MAC and not through the PHY (as
in, it's the MAC that has to be configured for WoL and not *only* the PHY).

>>
>>>    	status = "okay";
>>>    
>>>    	mdio {
>>> 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..4985b65925a9ed10ad44a6e58b9657a9dd48751f 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,7 @@ &eth {
>>>    	pinctrl-names = "default", "sleep";
>>>    	pinctrl-0 = <&eth_default_pins>;
>>>    	pinctrl-1 = <&eth_sleep_pins>;
>>> +	mediatek,mac-wol;
>>
>> I'm mostly sure that Kontron's i1200 works the same as the EVK in regards to WoL.
>>
>> Michael, I recall you worked on this board - can you please confirm?
> 
> I'd say so. Honestly, I've never tried WoL on this board, but I'm
> not aware of any difference to the *demo* board (not the EVK).

Thanks for confirming. I will ignore the devicetree commit entirely then, as this
would ...un...fix the fix (meaning that patch [1/2] is good!).

Cheers,
Angelo

> 
> -michael
Nícolas F. R. A. Prado Nov. 14, 2024, 7:22 p.m. UTC | #4 
On Thu, Nov 14, 2024 at 10:26:34AM +0100, AngeloGioacchino Del Regno wrote:
> Il 09/11/24 16:16, Nícolas F. R. A. Prado ha scritto:
> > Due to the mediatek,mac-wol property previously being handled backwards
> > by the dwmac-mediatek driver, its use in the DTs seems to have been
> > inconsistent.
> > 
> > Now that the driver has been fixed, correct this description. All the
> > currently upstream boards support MAC WOL, so add the mediatek,mac-wol
> > property to the missing ones.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >   arch/arm64/boot/dts/mediatek/mt2712-evb.dts                   | 1 +
> >   arch/arm64/boot/dts/mediatek/mt8195-demo.dts                  | 1 +
> >   arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts | 1 +
> >   3 files changed, 3 insertions(+)
> > 
> 
> ..snip..
> 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > index 31d424b8fc7cedef65489392eb279b7fd2194a4a..c12684e8c449b2d7b3b3a79086925bfe5ae0d8f8 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > @@ -109,6 +109,7 @@ &eth {
> >   	pinctrl-names = "default", "sleep";
> >   	pinctrl-0 = <&eth_default_pins>;
> >   	pinctrl-1 = <&eth_sleep_pins>;
> > +	mediatek,mac-wol;
> 
> The demo board has the same WoL capability as the EVK, so you can avoid adding the
> mac-wol property here.

Not sure I follow... If we omit the property here it will use PHY WOL instead,
while the genio 1200 EVK has the property, so it will be using MAC WOL, so
they're already the same and omitting will make them behave differently...

Let me recap to make sure we're all on the same page:

This was the WOL configuration for each board before this series:
MAC mt2712-evb.dts
MAC mt8195-demo.dts
PHY mt8395-genio-1200-evk.dts
MAC mt8395-kontron-3-5-sbc-i1200.dts
PHY mt8395-radxa-nio-12l.dts
PHY mt8390-genio-700-evk.dts

After patch 1, they all get inverted:
PHY mt2712-evb.dts
PHY mt8195-demo.dts
MAC mt8395-genio-1200-evk.dts
PHY mt8395-kontron-3-5-sbc-i1200.dts
MAC mt8395-radxa-nio-12l.dts
MAC mt8390-genio-700-evk.dts

And after patch 2, the remaining PHY ones are set to MAC:
MAC mt2712-evb.dts
MAC mt8195-demo.dts
MAC mt8395-genio-1200-evk.dts
MAC mt8395-kontron-3-5-sbc-i1200.dts
MAC mt8395-radxa-nio-12l.dts
MAC mt8390-genio-700-evk.dts

The only board I have in hands and am able to test is mt8390-genio-700-evk.dts,
which requires MAC WOL to work. For the others, your feedback on v1 was that
they should all be set to MAC WOL. Except for mt2712, which you were not sure
about, but it was already set to MAC WOL so we're keeping the same behavior.

That's how we got to adding mediatek,mac-wol to mt8195-demo.dts,
mt8395-kontron-3-5-sbc-i1200.dts and mt2712-evb.dts. Let me know if there has
been some misunderstanding.

Thanks,
Nícolas

> 
> >   	status = "okay";
> >   	mdio {
> > 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..4985b65925a9ed10ad44a6e58b9657a9dd48751f 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,7 @@ &eth {
> >   	pinctrl-names = "default", "sleep";
> >   	pinctrl-0 = <&eth_default_pins>;
> >   	pinctrl-1 = <&eth_sleep_pins>;
> > +	mediatek,mac-wol;
> 
> I'm mostly sure that Kontron's i1200 works the same as the EVK in regards to WoL.
> 
> Michael, I recall you worked on this board - can you please confirm?
> 
> Thanks,
> Angelo
>
AngeloGioacchino Del Regno Nov. 15, 2024, 9:26 a.m. UTC | #5 
Il 14/11/24 20:22, Nícolas F. R. A. Prado ha scritto:
> On Thu, Nov 14, 2024 at 10:26:34AM +0100, AngeloGioacchino Del Regno wrote:
>> Il 09/11/24 16:16, Nícolas F. R. A. Prado ha scritto:
>>> Due to the mediatek,mac-wol property previously being handled backwards
>>> by the dwmac-mediatek driver, its use in the DTs seems to have been
>>> inconsistent.
>>>
>>> Now that the driver has been fixed, correct this description. All the
>>> currently upstream boards support MAC WOL, so add the mediatek,mac-wol
>>> property to the missing ones.
>>>
>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>>> ---
>>>    arch/arm64/boot/dts/mediatek/mt2712-evb.dts                   | 1 +
>>>    arch/arm64/boot/dts/mediatek/mt8195-demo.dts                  | 1 +
>>>    arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts | 1 +
>>>    3 files changed, 3 insertions(+)
>>>
>>
>> ..snip..
>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> index 31d424b8fc7cedef65489392eb279b7fd2194a4a..c12684e8c449b2d7b3b3a79086925bfe5ae0d8f8 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> @@ -109,6 +109,7 @@ &eth {
>>>    	pinctrl-names = "default", "sleep";
>>>    	pinctrl-0 = <&eth_default_pins>;
>>>    	pinctrl-1 = <&eth_sleep_pins>;
>>> +	mediatek,mac-wol;
>>
>> The demo board has the same WoL capability as the EVK, so you can avoid adding the
>> mac-wol property here.
> 
> Not sure I follow... If we omit the property here it will use PHY WOL instead,
> while the genio 1200 EVK has the property, so it will be using MAC WOL, so
> they're already the same and omitting will make them behave differently...
> 
> Let me recap to make sure we're all on the same page:
> 
> This was the WOL configuration for each board before this series:
> MAC mt2712-evb.dts
> MAC mt8195-demo.dts
> PHY mt8395-genio-1200-evk.dts
> MAC mt8395-kontron-3-5-sbc-i1200.dts
> PHY mt8395-radxa-nio-12l.dts
> PHY mt8390-genio-700-evk.dts
> 
> After patch 1, they all get inverted:
> PHY mt2712-evb.dts
> PHY mt8195-demo.dts
> MAC mt8395-genio-1200-evk.dts
> PHY mt8395-kontron-3-5-sbc-i1200.dts
> MAC mt8395-radxa-nio-12l.dts
> MAC mt8390-genio-700-evk.dts
> 
> And after patch 2, the remaining PHY ones are set to MAC:
> MAC mt2712-evb.dts
> MAC mt8195-demo.dts
> MAC mt8395-genio-1200-evk.dts
> MAC mt8395-kontron-3-5-sbc-i1200.dts
> MAC mt8395-radxa-nio-12l.dts
> MAC mt8390-genio-700-evk.dts
> 
> The only board I have in hands and am able to test is mt8390-genio-700-evk.dts,
> which requires MAC WOL to work. For the others, your feedback on v1 was that
> they should all be set to MAC WOL. Except for mt2712, which you were not sure
> about, but it was already set to MAC WOL so we're keeping the same behavior.
> 
> That's how we got to adding mediatek,mac-wol to mt8195-demo.dts,
> mt8395-kontron-3-5-sbc-i1200.dts and mt2712-evb.dts. Let me know if there has
> been some misunderstanding.
> 

No, it's me getting confused about the current status - and I'm sorry about that.

So just ignore me saying "we can avoid adding" - we can't.
This commit is definitely needed.

Cheers,
Angelo

> Thanks,
> Nícolas
> 
>>
>>>    	status = "okay";
>>>    	mdio {
>>> 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..4985b65925a9ed10ad44a6e58b9657a9dd48751f 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,7 @@ &eth {
>>>    	pinctrl-names = "default", "sleep";
>>>    	pinctrl-0 = <&eth_default_pins>;
>>>    	pinctrl-1 = <&eth_sleep_pins>;
>>> +	mediatek,mac-wol;
>>
>> I'm mostly sure that Kontron's i1200 works the same as the EVK in regards to WoL.
>>
>> Michael, I recall you worked on this board - can you please confirm?
>>
>> Thanks,
>> Angelo
>>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
index c84c47c1352fba49d219fb8ace17a74953927fdc..0449686bd06ba17c5798aafdfb3fa071fca7e2f2 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
@@ -115,6 +115,7 @@  &eth {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&eth_default>;
 	pinctrl-1 = <&eth_sleep>;
+	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..c12684e8c449b2d7b3b3a79086925bfe5ae0d8f8 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
@@ -109,6 +109,7 @@  &eth {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&eth_default_pins>;
 	pinctrl-1 = <&eth_sleep_pins>;
+	mediatek,mac-wol;
 	status = "okay";
 
 	mdio {
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..4985b65925a9ed10ad44a6e58b9657a9dd48751f 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,7 @@  &eth {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&eth_default_pins>;
 	pinctrl-1 = <&eth_sleep_pins>;
+	mediatek,mac-wol;
 	status = "okay";
 
 	mdio {