Subject: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards

Hello.

This is a small patch series setting the PHY address of MT7531 to 0x1f on
all boards that have the switch.

Signed-off-by: Arınç ÜNAL <[email protected]>
---
Arınç ÜNAL (2):
arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch to 0x1f

arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 4 ++--
arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts | 4 ++--
arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts | 4 ++--
arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts | 4 ++--
5 files changed, 10 insertions(+), 10 deletions(-)
---
base-commit: ba90af39ba57b3fe3ecfdba0c87a80d20c7b788d
change-id: 20240314-for-mediatek-mt7531-phy-address-9d0b4cfeca21

Best regards,
--
Arınç ÜNAL <[email protected]>



Subject: [PATCH 2/2] arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch to 0x1f

From: Arınç ÜNAL <[email protected]>

The MT7531 switch listens on PHY address 0x1f on an MDIO bus. I've got two
findings that support this. There's no bootstrapping option to change the
PHY address of the switch. The Linux driver hardcodes 0x1f as the PHY
address of the switch. So the reg property on the device tree is currently
ignored by the Linux driver.

Therefore, describe the correct PHY address on boards that have this
switch. This is already the case on all MT7986 boards here, so use
hexadecimal numbering and align the switch node name with the reg value.

Signed-off-by: Arınç ÜNAL <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts | 4 ++--
arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts | 4 ++--
arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
index e04b1c0c0ebb..2f92f8cfd8a3 100644
--- a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
@@ -200,9 +200,9 @@ mdio: mdio-bus {
};

&mdio {
- switch: switch@31 {
+ switch: switch@1f {
compatible = "mediatek,mt7531";
- reg = <31>;
+ reg = <0x1f>;
interrupt-controller;
#interrupt-cells = <1>;
interrupts-extended = <&pio 66 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
index 5d8e3d3f6c20..47f75ece1872 100644
--- a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
@@ -84,9 +84,9 @@ mdio: mdio-bus {
};

&mdio {
- switch: switch@0 {
+ switch: switch@1f {
compatible = "mediatek,mt7531";
- reg = <31>;
+ reg = <0x1f>;
reset-gpios = <&pio 5 0>;
};
};
diff --git a/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts b/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
index 58f77d932429..5148a69f4729 100644
--- a/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
@@ -61,9 +61,9 @@ mdio: mdio-bus {
#address-cells = <1>;
#size-cells = <0>;

- switch@0 {
+ switch@1f {
compatible = "mediatek,mt7531";
- reg = <31>;
+ reg = <0x1f>;
reset-gpios = <&pio 5 0>;

ports {

--
2.40.1


Subject: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f

From: Arınç ÜNAL <[email protected]>

The MT7531 switch listens on PHY address 0x1f on an MDIO bus. I've got two
findings that support this. There's no bootstrapping option to change the
PHY address of the switch. The Linux driver hardcodes 0x1f as the PHY
address of the switch. So the reg property on the device tree is currently
ignored by the Linux driver.

Therefore, describe the correct PHY address on boards that have this
switch.

Signed-off-by: Arınç ÜNAL <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
index 224bb289660c..811b227d6f50 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -149,9 +149,9 @@ mdio: mdio-bus {
#address-cells = <1>;
#size-cells = <0>;

- switch@0 {
+ switch@1f {
compatible = "mediatek,mt7531";
- reg = <0>;
+ reg = <0x1f>;
interrupt-controller;
#interrupt-cells = <1>;
interrupts-extended = <&pio 53 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
index 41629769bdc8..3c2423cb38fd 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
@@ -134,9 +134,9 @@ mdio-bus {
#address-cells = <1>;
#size-cells = <0>;

- switch@0 {
+ switch@1f {
compatible = "mediatek,mt7531";
- reg = <0>;
+ reg = <0x1f>;
reset-gpios = <&pio 54 0>;

ports {

--
2.40.1


2024-03-15 15:52:03

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards


On Thu, 14 Mar 2024 15:20:03 +0300, Arınç ÜNAL wrote:
> Hello.
>
> This is a small patch series setting the PHY address of MT7531 to 0x1f on
> all boards that have the switch.
>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---
> Arınç ÜNAL (2):
> arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
> arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch to 0x1f
>
> arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts | 4 ++--
> 5 files changed, 10 insertions(+), 10 deletions(-)
> ---
> base-commit: ba90af39ba57b3fe3ecfdba0c87a80d20c7b788d
> change-id: 20240314-for-mediatek-mt7531-phy-address-9d0b4cfeca21
>
> Best regards,
> --
> Arınç ÜNAL <[email protected]>
>
>


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y mediatek/mt7622-bananapi-bpi-r64.dtb mediatek/mt7622-rfb1.dtb mediatek/mt7986a-bananapi-bpi-r3.dtb mediatek/mt7986a-rfb.dtb mediatek/mt7986b-rfb.dtb' for 20240314-for-mediatek-mt7531-phy-address-v1-0-52f58db01acd@arinc9.com:

arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dtb: switch@1f: 'interrupts' is a dependency of 'interrupt-controller'
from schema $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#
arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dtb: switch@1f: 'interrupts' is a dependency of 'interrupt-controller'
from schema $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#






2024-03-15 17:37:17

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f

On 3/14/24 05:20, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> The MT7531 switch listens on PHY address 0x1f on an MDIO bus. I've got two
> findings that support this. There's no bootstrapping option to change the
> PHY address of the switch. The Linux driver hardcodes 0x1f as the PHY
> address of the switch. So the reg property on the device tree is currently
> ignored by the Linux driver.
>
> Therefore, describe the correct PHY address on boards that have this
> switch.

Can we call it a pseudo PHY to use a similar terminology as what is done
through drivers/net/dsa/{bcm_sf2,b53}*?

This is not a real PHY as in it has no actual transceiver/digital signal
processing logic, this is a piece of logic that snoops for MDIO
transactions at that specific address and lets you access the switch's
internal register as if it was a MDIO device.

LGTM otherwise!
--
Florian


2024-03-16 07:44:15

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f

On 15.03.2024 20:26, Florian Fainelli wrote:
> On 3/14/24 05:20, Arınç ÜNAL via B4 Relay wrote:
>> From: Arınç ÜNAL <[email protected]>
>>
>> The MT7531 switch listens on PHY address 0x1f on an MDIO bus. I've got two
>> findings that support this. There's no bootstrapping option to change the
>> PHY address of the switch. The Linux driver hardcodes 0x1f as the PHY
>> address of the switch. So the reg property on the device tree is currently
>> ignored by the Linux driver.
>>
>> Therefore, describe the correct PHY address on boards that have this
>> switch.
>
> Can we call it a pseudo PHY to use a similar terminology as what is done through drivers/net/dsa/{bcm_sf2,b53}*?
>
> This is not a real PHY as in it has no actual transceiver/digital signal processing logic, this is a piece of logic that snoops for MDIO transactions at that specific address and lets you access the switch's internal register as if it was a MDIO device.

I can get behind calling the switch a psuedo-PHY in the context of MDIO.
However, as described on "22.2.4.5.5 PHYAD (PHY Address)" of "22.2.4.5
Management frame structure" of the active standard IEEE Std 802.3™‐2022,
the field is called "PHY Address". The patch log doesn't give an identifier
as to what a switch is in the context of MDIO. Only that it listens on a
certain PHY address which the term complies with IEEE Std 802.3™‐2022.

So I don't see an improvement to be made on the patch log. Feel free to
elaborate further.

Arınç

2024-03-18 13:02:22

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f



On 3/16/2024 12:43 AM, Arınç ÜNAL wrote:
> On 15.03.2024 20:26, Florian Fainelli wrote:
>> On 3/14/24 05:20, Arınç ÜNAL via B4 Relay wrote:
>>> From: Arınç ÜNAL <[email protected]>
>>>
>>> The MT7531 switch listens on PHY address 0x1f on an MDIO bus. I've
>>> got two
>>> findings that support this. There's no bootstrapping option to change
>>> the
>>> PHY address of the switch. The Linux driver hardcodes 0x1f as the PHY
>>> address of the switch. So the reg property on the device tree is
>>> currently
>>> ignored by the Linux driver.
>>>
>>> Therefore, describe the correct PHY address on boards that have this
>>> switch.
>>
>> Can we call it a pseudo PHY to use a similar terminology as what is
>> done through drivers/net/dsa/{bcm_sf2,b53}*?
>>
>> This is not a real PHY as in it has no actual transceiver/digital
>> signal processing logic, this is a piece of logic that snoops for MDIO
>> transactions at that specific address and lets you access the switch's
>> internal register as if it was a MDIO device.
>
> I can get behind calling the switch a psuedo-PHY in the context of MDIO.
> However, as described on "22.2.4.5.5 PHYAD (PHY Address)" of "22.2.4.5
> Management frame structure" of the active standard IEEE Std 802.3™‐2022,
> the field is called "PHY Address". The patch log doesn't give an identifier
> as to what a switch is in the context of MDIO. Only that it listens on a
> certain PHY address which the term complies with IEEE Std 802.3™‐2022.
>
> So I don't see an improvement to be made on the patch log. Feel free to
> elaborate further.

I would just s/PHY/MDIO bus address/ since that is simply more generic,
but if it is not written as-is in the spec, then I won't fight it much
more than I already did.
--
Florian

2024-03-18 15:27:20

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f

On 18.03.2024 16:02, Florian Fainelli wrote:
>>> Can we call it a pseudo PHY to use a similar terminology as what is done through drivers/net/dsa/{bcm_sf2,b53}*?
>>>
>>> This is not a real PHY as in it has no actual transceiver/digital signal processing logic, this is a piece of logic that snoops for MDIO transactions at that specific address and lets you access the switch's internal register as if it was a MDIO device.
>>
>> I can get behind calling the switch a psuedo-PHY in the context of MDIO.
>> However, as described on "22.2.4.5.5 PHYAD (PHY Address)" of "22.2.4.5
>> Management frame structure" of the active standard IEEE Std 802.3™‐2022,
>> the field is called "PHY Address". The patch log doesn't give an identifier
>> as to what a switch is in the context of MDIO. Only that it listens on a
>> certain PHY address which the term complies with IEEE Std 802.3™‐2022.
>>
>> So I don't see an improvement to be made on the patch log. Feel free to
>> elaborate further.
>
> I would just s/PHY/MDIO bus address/ since that is simply more generic, but if it is not written as-is in the spec, then I won't fight it much more than I already did.

I'm not sure what you're referring to by spec. Are you asking how specific
the name of the PHYAD field is described on the standard?

Arınç

2024-03-18 15:39:09

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f

On 3/18/24 08:26, Arınç ÜNAL wrote:
> On 18.03.2024 16:02, Florian Fainelli wrote:
>>>> Can we call it a pseudo PHY to use a similar terminology as what is
>>>> done through drivers/net/dsa/{bcm_sf2,b53}*?
>>>>
>>>> This is not a real PHY as in it has no actual transceiver/digital
>>>> signal processing logic, this is a piece of logic that snoops for
>>>> MDIO transactions at that specific address and lets you access the
>>>> switch's internal register as if it was a MDIO device.
>>>
>>> I can get behind calling the switch a psuedo-PHY in the context of MDIO.
>>> However, as described on "22.2.4.5.5 PHYAD (PHY Address)" of "22.2.4.5
>>> Management frame structure" of the active standard IEEE Std 802.3™‐2022,
>>> the field is called "PHY Address". The patch log doesn't give an
>>> identifier
>>> as to what a switch is in the context of MDIO. Only that it listens on a
>>> certain PHY address which the term complies with IEEE Std 802.3™‐2022.
>>>
>>> So I don't see an improvement to be made on the patch log. Feel free to
>>> elaborate further.
>>
>> I would just s/PHY/MDIO bus address/ since that is simply more
>> generic, but if it is not written as-is in the spec, then I won't
>> fight it much more than I already did.
>
> I'm not sure what you're referring to by spec. Are you asking how specific
> the name of the PHYAD field is described on the standard?

Spec = IEEE Std 802.3-2022 standard, aka the document you are quoting.
--
Florian


2024-03-18 15:52:57

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f

On 18.03.2024 18:38, Florian Fainelli wrote:
> On 3/18/24 08:26, Arınç ÜNAL wrote:
>> On 18.03.2024 16:02, Florian Fainelli wrote:
>>>>> Can we call it a pseudo PHY to use a similar terminology as what is done through drivers/net/dsa/{bcm_sf2,b53}*?
>>>>>
>>>>> This is not a real PHY as in it has no actual transceiver/digital signal processing logic, this is a piece of logic that snoops for MDIO transactions at that specific address and lets you access the switch's internal register as if it was a MDIO device.
>>>>
>>>> I can get behind calling the switch a psuedo-PHY in the context of MDIO.
>>>> However, as described on "22.2.4.5.5 PHYAD (PHY Address)" of "22.2.4.5
>>>> Management frame structure" of the active standard IEEE Std 802.3™‐2022,
>>>> the field is called "PHY Address". The patch log doesn't give an identifier
>>>> as to what a switch is in the context of MDIO. Only that it listens on a
>>>> certain PHY address which the term complies with IEEE Std 802.3™‐2022.
>>>>
>>>> So I don't see an improvement to be made on the patch log. Feel free to
>>>> elaborate further.
>>>
>>> I would just s/PHY/MDIO bus address/ since that is simply more generic, but if it is not written as-is in the spec, then I won't fight it much more than I already did.
>>
>> I'm not sure what you're referring to by spec. Are you asking how specific
>> the name of the PHYAD field is described on the standard?
>
> Spec = IEEE Std 802.3-2022 standard, aka the document you are quoting.

Ok. The field is referred to as "PHY Address" (capital A) in a sentence on
the standard. I interpret that as it defines the term to mention the field
in a sentence which is why I'd like to stick to it on my patch log.

Arınç

2024-03-31 09:28:28

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards

On 14.03.2024 15:20, Arınç ÜNAL via B4 Relay wrote:
> Hello.
>
> This is a small patch series setting the PHY address of MT7531 to 0x1f
> on
> all boards that have the switch.
>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---
> Arınç ÜNAL (2):
> arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch
> to 0x1f
> arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch
> to 0x1f
>
> arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts | 4 ++--
> 5 files changed, 10 insertions(+), 10 deletions(-)
> ---
> base-commit: ba90af39ba57b3fe3ecfdba0c87a80d20c7b788d
> change-id: 20240314-for-mediatek-mt7531-phy-address-9d0b4cfeca21
>
> Best regards,

Reminder that this patch series is waiting.

Arınç

2024-04-08 07:23:20

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards

On 31.03.2024 12:28, [email protected] wrote:
> On 14.03.2024 15:20, Arınç ÜNAL via B4 Relay wrote:
>> Hello.
>>
>> This is a small patch series setting the PHY address of MT7531 to 0x1f on
>> all boards that have the switch.
>>
>> Signed-off-by: Arınç ÜNAL <[email protected]>
>> ---
>> Arınç ÜNAL (2):
>>        arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
>>        arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch to 0x1f
>>
>>   arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
>>   arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts             | 4 ++--
>>   arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts | 4 ++--
>>   arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts             | 4 ++--
>>   arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts             | 4 ++--
>>   5 files changed, 10 insertions(+), 10 deletions(-)
>> ---
>> base-commit: ba90af39ba57b3fe3ecfdba0c87a80d20c7b788d
>> change-id: 20240314-for-mediatek-mt7531-phy-address-9d0b4cfeca21
>>
>> Best regards,
>
> Reminder that this patch series is waiting.

Another reminder that this patch series is waiting to be applied.

Arınç

2024-04-23 09:16:21

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards

On 08/04/2024 10:22, Arınç ÜNAL wrote:
> On 31.03.2024 12:28, [email protected] wrote:
>> On 14.03.2024 15:20, Arınç ÜNAL via B4 Relay wrote:
>>> Hello.
>>>
>>> This is a small patch series setting the PHY address of MT7531 to 0x1f on
>>> all boards that have the switch.
>>>
>>> Signed-off-by: Arınç ÜNAL <[email protected]>
>>> ---
>>> Arınç ÜNAL (2):
>>>        arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
>>>        arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch to 0x1f
>>>
>>>   arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
>>>   arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts             | 4 ++--
>>>   arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts | 4 ++--
>>>   arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts             | 4 ++--
>>>   arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts             | 4 ++--
>>>   5 files changed, 10 insertions(+), 10 deletions(-)
>>> ---
>>> base-commit: ba90af39ba57b3fe3ecfdba0c87a80d20c7b788d
>>> change-id: 20240314-for-mediatek-mt7531-phy-address-9d0b4cfeca21
>>>
>>> Best regards,
>>
>> Reminder that this patch series is waiting.
>
> Another reminder that this patch series is waiting to be applied.

Here's the third reminder for someone to apply this. From now on, I am
going to reply to this thread once every day until this patch series is
applied.

Arınç

2024-04-26 12:16:28

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards

On 23.04.2024 12:16, Arınç ÜNAL wrote:
> On 08/04/2024 10:22, Arınç ÜNAL wrote:
>> On 31.03.2024 12:28, [email protected] wrote:
>>> On 14.03.2024 15:20, Arınç ÜNAL via B4 Relay wrote:
>>>> Hello.
>>>>
>>>> This is a small patch series setting the PHY address of MT7531 to 0x1f on
>>>> all boards that have the switch.
>>>>
>>>> Signed-off-by: Arınç ÜNAL <[email protected]>
>>>> ---
>>>> Arınç ÜNAL (2):
>>>>        arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
>>>>        arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch to 0x1f
>>>>
>>>>   arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
>>>>   arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts             | 4 ++--
>>>>   arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts | 4 ++--
>>>>   arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts             | 4 ++--
>>>>   arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts             | 4 ++--
>>>>   5 files changed, 10 insertions(+), 10 deletions(-)
>>>> ---
>>>> base-commit: ba90af39ba57b3fe3ecfdba0c87a80d20c7b788d
>>>> change-id: 20240314-for-mediatek-mt7531-phy-address-9d0b4cfeca21
>>>>
>>>> Best regards,
>>>
>>> Reminder that this patch series is waiting.
>>
>> Another reminder that this patch series is waiting to be applied.
>
> Here's the third reminder for someone to apply this. From now on, I am
> going to reply to this thread once every day until this patch series is
> applied.

Fourth.

Arınç

2024-04-27 01:29:27

by Daniel Golle

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch to 0x1f

Hi Arınç,

On Thu, Mar 14, 2024 at 03:20:05PM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> The MT7531 switch listens on PHY address 0x1f on an MDIO bus. I've got two
> findings that support this. There's no bootstrapping option to change the
> PHY address of the switch. The Linux driver hardcodes 0x1f as the PHY
> address of the switch. So the reg property on the device tree is currently
> ignored by the Linux driver.
>
> Therefore, describe the correct PHY address on boards that have this
> switch. This is already the case on all MT7986 boards here, so use
> hexadecimal numbering and align the switch node name with the reg value.

Sorry for the late reply to this series which I had not noticed earlier.
My comment below applies for the whole series.

While this is generally correct, you are mixing cosmetic with functional
changes here.

Replacing
reg = <31>;
with
reg = <0x1f>;
is a purely cosmetic change (and it's up to maintainers taste to like
one or the other notation more).

On the other hand replacing
switch@0 or switch@31
with
switch@1f
is fixing a bug. Yet even that doesn't have any functional impact on the
affected boards as there aren't any other DT nodes which would collide
with that incorrect address, nor does the driver actually care about the
node name (not before and not after your patch
"net: dsa: mt7530-mdio: read PHY address of switch from device tree").
Anyway, there *is* something wrong and everybody should agree it's a
good thing to fix it.

So imho you should start with a patch only replacing all instances of
mt7530/mt7531 switch nodes named 'switch@0' or 'switch@31' with
'switch@1f' as that's fixing an actual (minor) bug.

The other thing, using hexadecimal notation for the 'reg' property is
a mere matter of (statistically unusual) taste. I fully get the point
that using hexdecimal for both, the address in the node name as well
as the 'reg' property avoids the exact divergence of the two you are
fixing now.

Byt statistically unusual I mean:
$ find -name \*.dts\* | while read line; do grep 'reg.*=.*<[0-9]*>' $line ; done | wc -l
37284
$ find -name \*.dts\* | while read line; do grep 'reg.*=.*<0x[0-9a-fA-F]*>' $line ; done | wc -l
10339

(I know the above regexp could be done more accurately, but it's good
enough to demonstrate my point)

So please make this four patches. Two fixing the wrong node names.
And another two opening the (purely cosmetic) debate to use hexademical
notation for the 'reg' property.


Cheers


Daniel



>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---
> arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts | 4 ++--
> arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts | 4 ++--
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
> index e04b1c0c0ebb..2f92f8cfd8a3 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
> @@ -200,9 +200,9 @@ mdio: mdio-bus {
> };
>
> &mdio {
> - switch: switch@31 {
> + switch: switch@1f {
> compatible = "mediatek,mt7531";
> - reg = <31>;
> + reg = <0x1f>;
> interrupt-controller;
> #interrupt-cells = <1>;
> interrupts-extended = <&pio 66 IRQ_TYPE_LEVEL_HIGH>;
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> index 5d8e3d3f6c20..47f75ece1872 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> @@ -84,9 +84,9 @@ mdio: mdio-bus {
> };
>
> &mdio {
> - switch: switch@0 {
> + switch: switch@1f {
> compatible = "mediatek,mt7531";
> - reg = <31>;
> + reg = <0x1f>;
> reset-gpios = <&pio 5 0>;
> };
> };
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts b/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
> index 58f77d932429..5148a69f4729 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
> @@ -61,9 +61,9 @@ mdio: mdio-bus {
> #address-cells = <1>;
> #size-cells = <0>;
>
> - switch@0 {
> + switch@1f {
> compatible = "mediatek,mt7531";
> - reg = <31>;
> + reg = <0x1f>;
> reset-gpios = <&pio 5 0>;
>
> ports {
>
> --
> 2.40.1
>
>