linux-mediatek.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards
@ 2024-03-14 12:20 Arınç ÜNAL via B4 Relay
  2024-03-14 12:20 ` [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f Arınç ÜNAL via B4 Relay
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Arınç ÜNAL via B4 Relay @ 2024-03-14 12:20 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, Arınç ÜNAL

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 <arinc.unal@arinc9.com>
---
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 <arinc.unal@arinc9.com>



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
  2024-03-14 12:20 [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards Arınç ÜNAL via B4 Relay
@ 2024-03-14 12:20 ` Arınç ÜNAL via B4 Relay
  2024-03-15 17:26   ` Florian Fainelli
  2024-03-14 12:20 ` [PATCH 2/2] arm64: dts: mediatek: mt7986: " Arınç ÜNAL via B4 Relay
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Arınç ÜNAL via B4 Relay @ 2024-03-14 12:20 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, Arınç ÜNAL

From: Arınç ÜNAL <arinc.unal@arinc9.com>

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 <arinc.unal@arinc9.com>
---
 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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch to 0x1f
  2024-03-14 12:20 [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards Arınç ÜNAL via B4 Relay
  2024-03-14 12:20 ` [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f Arınç ÜNAL via B4 Relay
@ 2024-03-14 12:20 ` Arınç ÜNAL via B4 Relay
  2024-04-27  1:28   ` Daniel Golle
  2024-03-15 15:50 ` [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards Rob Herring
  2024-03-31  9:28 ` arinc.unal
  3 siblings, 1 reply; 15+ messages in thread
From: Arınç ÜNAL via B4 Relay @ 2024-03-14 12:20 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, Arınç ÜNAL

From: Arınç ÜNAL <arinc.unal@arinc9.com>

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 <arinc.unal@arinc9.com>
---
 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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards
  2024-03-14 12:20 [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards Arınç ÜNAL via B4 Relay
  2024-03-14 12:20 ` [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f Arınç ÜNAL via B4 Relay
  2024-03-14 12:20 ` [PATCH 2/2] arm64: dts: mediatek: mt7986: " Arınç ÜNAL via B4 Relay
@ 2024-03-15 15:50 ` Rob Herring
  2024-03-31  9:28 ` arinc.unal
  3 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2024-03-15 15:50 UTC (permalink / raw)
  To: Arınç ÜNAL
  Cc: Conor Dooley, linux-mediatek, mithat.guner, Matthias Brugger,
	linux-arm-kernel, linux-kernel, AngeloGioacchino Del Regno,
	erkin.bozoglu, devicetree, Krzysztof Kozlowski, Rob Herring


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 <arinc.unal@arinc9.com>
> ---
> 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 <arinc.unal@arinc9.com>
> 
> 


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#







^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
  2024-03-14 12:20 ` [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f Arınç ÜNAL via B4 Relay
@ 2024-03-15 17:26   ` Florian Fainelli
  2024-03-16  7:43     ` Arınç ÜNAL
  0 siblings, 1 reply; 15+ messages in thread
From: Florian Fainelli @ 2024-03-15 17:26 UTC (permalink / raw)
  To: arinc.unal, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

On 3/14/24 05:20, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> 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



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
  2024-03-15 17:26   ` Florian Fainelli
@ 2024-03-16  7:43     ` Arınç ÜNAL
  2024-03-18 13:02       ` Florian Fainelli
  0 siblings, 1 reply; 15+ messages in thread
From: Arınç ÜNAL @ 2024-03-16  7:43 UTC (permalink / raw)
  To: Florian Fainelli, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

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 <arinc.unal@arinc9.com>
>>
>> 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ç


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
  2024-03-16  7:43     ` Arınç ÜNAL
@ 2024-03-18 13:02       ` Florian Fainelli
  2024-03-18 15:26         ` Arınç ÜNAL
  0 siblings, 1 reply; 15+ messages in thread
From: Florian Fainelli @ 2024-03-18 13:02 UTC (permalink / raw)
  To: Arınç ÜNAL, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek



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 <arinc.unal@arinc9.com>
>>>
>>> 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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
  2024-03-18 13:02       ` Florian Fainelli
@ 2024-03-18 15:26         ` Arınç ÜNAL
  2024-03-18 15:38           ` Florian Fainelli
  0 siblings, 1 reply; 15+ messages in thread
From: Arınç ÜNAL @ 2024-03-18 15:26 UTC (permalink / raw)
  To: Florian Fainelli, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

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ç


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
  2024-03-18 15:26         ` Arınç ÜNAL
@ 2024-03-18 15:38           ` Florian Fainelli
  2024-03-18 15:50             ` Arınç ÜNAL
  0 siblings, 1 reply; 15+ messages in thread
From: Florian Fainelli @ 2024-03-18 15:38 UTC (permalink / raw)
  To: Arınç ÜNAL, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

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



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f
  2024-03-18 15:38           ` Florian Fainelli
@ 2024-03-18 15:50             ` Arınç ÜNAL
  0 siblings, 0 replies; 15+ messages in thread
From: Arınç ÜNAL @ 2024-03-18 15:50 UTC (permalink / raw)
  To: Florian Fainelli, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

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ç


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards
  2024-03-14 12:20 [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards Arınç ÜNAL via B4 Relay
                   ` (2 preceding siblings ...)
  2024-03-15 15:50 ` [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards Rob Herring
@ 2024-03-31  9:28 ` arinc.unal
  2024-04-08  7:22   ` Arınç ÜNAL
  3 siblings, 1 reply; 15+ messages in thread
From: arinc.unal @ 2024-03-31  9:28 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

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 <arinc.unal@arinc9.com>
> ---
> 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ç


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards
  2024-03-31  9:28 ` arinc.unal
@ 2024-04-08  7:22   ` Arınç ÜNAL
  2024-04-23  9:16     ` Arınç ÜNAL
  0 siblings, 1 reply; 15+ messages in thread
From: Arınç ÜNAL @ 2024-04-08  7:22 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

On 31.03.2024 12:28, arinc.unal@arinc9.com 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 <arinc.unal@arinc9.com>
>> ---
>> 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ç


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards
  2024-04-08  7:22   ` Arınç ÜNAL
@ 2024-04-23  9:16     ` Arınç ÜNAL
  2024-04-26 12:15       ` Arınç ÜNAL
  0 siblings, 1 reply; 15+ messages in thread
From: Arınç ÜNAL @ 2024-04-23  9:16 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

On 08/04/2024 10:22, Arınç ÜNAL wrote:
> On 31.03.2024 12:28, arinc.unal@arinc9.com 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 <arinc.unal@arinc9.com>
>>> ---
>>> 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ç


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards
  2024-04-23  9:16     ` Arınç ÜNAL
@ 2024-04-26 12:15       ` Arınç ÜNAL
  0 siblings, 0 replies; 15+ messages in thread
From: Arınç ÜNAL @ 2024-04-26 12:15 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: mithat.guner, erkin.bozoglu, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek

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, arinc.unal@arinc9.com 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 <arinc.unal@arinc9.com>
>>>> ---
>>>> 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ç


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] arm64: dts: mediatek: mt7986: set PHY address of MT7531 switch to 0x1f
  2024-03-14 12:20 ` [PATCH 2/2] arm64: dts: mediatek: mt7986: " Arınç ÜNAL via B4 Relay
@ 2024-04-27  1:28   ` Daniel Golle
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Golle @ 2024-04-27  1:28 UTC (permalink / raw)
  To: arinc.unal
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, mithat.guner, erkin.bozoglu,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

Hi Arınç,

On Thu, Mar 14, 2024 at 03:20:05PM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> 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 <arinc.unal@arinc9.com>
> ---
>  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
> 
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-04-27  1:29 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-14 12:20 [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards Arınç ÜNAL via B4 Relay
2024-03-14 12:20 ` [PATCH 1/2] arm64: dts: mediatek: mt7622: set PHY address of MT7531 switch to 0x1f Arınç ÜNAL via B4 Relay
2024-03-15 17:26   ` Florian Fainelli
2024-03-16  7:43     ` Arınç ÜNAL
2024-03-18 13:02       ` Florian Fainelli
2024-03-18 15:26         ` Arınç ÜNAL
2024-03-18 15:38           ` Florian Fainelli
2024-03-18 15:50             ` Arınç ÜNAL
2024-03-14 12:20 ` [PATCH 2/2] arm64: dts: mediatek: mt7986: " Arınç ÜNAL via B4 Relay
2024-04-27  1:28   ` Daniel Golle
2024-03-15 15:50 ` [PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards Rob Herring
2024-03-31  9:28 ` arinc.unal
2024-04-08  7:22   ` Arınç ÜNAL
2024-04-23  9:16     ` Arınç ÜNAL
2024-04-26 12:15       ` Arınç ÜNAL

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).