arm64: dts: marvell: espressobin: Add ethernet switch aliases

PATCH In review Pali Rohár 4 participants 19 messages Sep 7, 2020 – Sep 24, 2020

Subsystems (2)

Device tree bindings Arm/Marvell Kirkwood and Armada 370, 375, 38x, 39x, XP, 3700, 7K/8K, CN9130 SoC support

lore

Share & tools

lore

Pali Rohár

author

2020-09-07 11:28 UTC #0 lore raw

PATCH 1/1 arm64: dts: marvell: espressobin: Add ethernet switch aliases

Espressobin boards have 3 ethernet ports and some of them got assigned more then one MAC address. MAC addresses are stored in U-Boot environment.

Since commit a2c7023f7075c ("net: dsa: read mac address from DT for slave device") kernel can use MAC addresses from DT for particular DSA port.

Currently Espressobin DTS file contains alias just for ethernet0.

This patch defines additional ethernet aliases in Espressobin DTS files, so bootloader can fill correct MAC address for DSA switch ports if more MAC addresses were specified.

DT alias ethernet1 is used for wan port, DT aliases ethernet2 and ethernet3 are used for lan ports for both Espressobin revisions (V5 and V7).

Fixes 5253cb8c00a6f ("arm64: dts: marvell: espressobin: add ethernet alias")

Signed-off-by Pali Rohár <[email protected]>

---
 .../dts/marvell/armada-3720-espressobin-v7-emmc.dts  | 10 ++++++++--
 .../boot/dts/marvell/armada-3720-espressobin-v7.dts  | 10 ++++++++--
 .../boot/dts/marvell/armada-3720-espressobin.dtsi    | 12 ++++++++----
 3 files changed, 24 insertions(+), 8 deletions(-)

arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7-emmc.dts +8 -2

index 03733fd92732..215d2f702623 100644
@@ -20,17 +20,23 @@
 	compatible = "globalscale,espressobin-v7-emmc", "globalscale,espressobin-v7",
 		     "globalscale,espressobin", "marvell,armada3720",
 		     "marvell,armada3710";
+
+	aliases {
+		/* ethernet1 is wan port */
+		ethernet1 = &switch0port3;
+		ethernet3 = &switch0port1;
+	};
 };
 
 &switch0 {
 	ports {
-		port@1 {
+		switch0port1: port@1 {
 			reg = <1>;
 			label = "lan1";
 			phy-handle = <&switch0phy0>;
 		};
 
-		port@3 {
+		switch0port3: port@3 {
 			reg = <3>;
 			label = "wan";
 			phy-handle = <&switch0phy2>;

arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7.dts +8 -2

index 8570c5f47d7d..b6f4af8ebafb 100644
@@ -19,17 +19,23 @@
 	model = "Globalscale Marvell ESPRESSOBin Board V7";
 	compatible = "globalscale,espressobin-v7", "globalscale,espressobin",
 		     "marvell,armada3720", "marvell,armada3710";
+
+	aliases {
+		/* ethernet1 is wan port */
+		ethernet1 = &switch0port3;
+		ethernet3 = &switch0port1;
+	};
 };
 
 &switch0 {
 	ports {
-		port@1 {
+		switch0port1: port@1 {
 			reg = <1>;
 			label = "lan1";
 			phy-handle = <&switch0phy0>;
 		};
 
-		port@3 {
+		switch0port3: port@3 {
 			reg = <3>;
 			label = "wan";
 			phy-handle = <&switch0phy2>;

arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +8 -4

index b97218c72727..0775c16e0ec8 100644
@@ -13,6 +13,10 @@
 / {
 	aliases {
 		ethernet0 = &eth0;
+		/* for dsa slave device */
+		ethernet1 = &switch0port1;
+		ethernet2 = &switch0port2;
+		ethernet3 = &switch0port3;
 		serial0 = &uart0;
 		serial1 = &uart1;
 	};

@@ -120,7 +124,7 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 
-			port@0 {
+			switch0port0: port@0 {
 				reg = <0>;
 				label = "cpu";
 				ethernet = <&eth0>;

@@ -131,19 +135,19 @@
 				};
 			};
 
-			port@1 {
+			switch0port1: port@1 {
 				reg = <1>;
 				label = "wan";
 				phy-handle = <&switch0phy0>;
 			};
 
-			port@2 {
+			switch0port2: port@2 {
 				reg = <2>;
 				label = "lan0";
 				phy-handle = <&switch0phy1>;
 			};
 
-			port@3 {
+			switch0port3: port@3 {
 				reg = <3>;
 				label = "lan1";
 				phy-handle = <&switch0phy2>;

Signature · 1 line

2.20.1

Andrew Lunn

maintainer

2020-09-07 14:43 UTC #1 lore raw

Quoted 12 lines · Pali Rohár

Espressobin boards have 3 ethernet ports and some of them got assigned more then one MAC address. MAC addresses are stored in U-Boot environment.
Since commit a2c7023f7075c ("net: dsa: read mac address from DT for slave device") kernel can use MAC addresses from DT for particular DSA port.
Currently Espressobin DTS file contains alias just for ethernet0.
This patch defines additional ethernet aliases in Espressobin DTS files, so bootloader can fill correct MAC address for DSA switch ports if more MAC addresses were specified.

Quoted 5 lines

DT alias ethernet1 is used for wan port, DT aliases ethernet2 and ethernet3 are used for lan ports for both Espressobin revisions (V5 and V7).
Fixes 5253cb8c00a6f ("arm64: dts: marvell: espressobin: add ethernet alias")
Signed-off-by Pali Rohár <[email protected]>

I'm not sure a Fixes: is appropriate here. What is actually broken? This just seems like a new feature.

Andrew

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Pali Rohár

author

2020-09-07 14:52 UTC #2 lore raw

Quoted 13 lines · On Monday 07 September 2020 16:42:28 Andrew Lunn wrote:

Quoted 12 lines · Pali Rohár
Espressobin boards have 3 ethernet ports and some of them got assigned more then one MAC address. MAC addresses are stored in U-Boot environment.
Since commit a2c7023f7075c ("net: dsa: read mac address from DT for slave device") kernel can use MAC addresses from DT for particular DSA port.
Currently Espressobin DTS file contains alias just for ethernet0.
This patch defines additional ethernet aliases in Espressobin DTS files, so bootloader can fill correct MAC address for DSA switch ports if more MAC addresses were specified.

Quoted 8 lines

Quoted 5 lines
DT alias ethernet1 is used for wan port, DT aliases ethernet2 and ethernet3 are used for lan ports for both Espressobin revisions (V5 and V7).
Fixes 5253cb8c00a6f ("arm64: dts: marvell: espressobin: add ethernet alias")
Signed-off-by Pali Rohár <[email protected]>
I'm not sure a Fixes: is appropriate here. What is actually broken? This just seems like a new feature.

Hello Andrew! With "fixes" I mean that this patch fixes aliases list in that mentioned commit as it was incomplete. Probably better would be "related" or "extended" keyword in this case. But I do not know.

I would not say it is a "new feature". But rather that patch in this email fixes issue that Linux kernel did not set correct MAC address for DSA slave ports. I think it is something which could be backported also to stable releases as "ignoring" vendor/factory MAC address is not correct behavior.

If you have an idea how to reformulate commit description I will do it.

Andrew Lunn

maintainer

2020-09-07 15:45 UTC #3 lore raw

Quoted 5 lines

I would not say it is a "new feature". But rather that patch in this email fixes issue that Linux kernel did not set correct MAC address for DSA slave ports. I think it is something which could be backported also to stable releases as "ignoring" vendor/factory MAC address is not correct behavior.

Hi Pali

The rules for stable are here:

https://www.kernel.org/doc/html/v5.8/process/stable-kernel-rules.html

Do you think it fits?

Andrew

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Pali Rohár

author

2020-09-07 16:18 UTC #4 lore raw

Quoted 8 lines · On Monday 07 September 2020 17:43:53 Andrew Lunn wrote:

Quoted 5 lines
I would not say it is a "new feature". But rather that patch in this email fixes issue that Linux kernel did not set correct MAC address for DSA slave ports. I think it is something which could be backported also to stable releases as "ignoring" vendor/factory MAC address is not correct behavior.
Hi Pali

Quoted 7 lines

The rules for stable are here:
https://www.kernel.org/doc/html/v5.8/process/stable-kernel-rules.html
Do you think it fits?
Andrew

Hello Andrew! I think it fits into those rules. As I wrote it fixes real bug that Linux kernel does not use correct MAC address for particular DSA slaves / ethernet ports. But if you or other people have opposite opinion I will of course respect it.

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Andrew Lunn

maintainer Reviewed-by

2020-09-07 17:24 UTC #5 lore raw

Quoted 13 lines · Pali Rohár

Quoted 12 lines · On Monday 07 September 2020 17:43:53 Andrew Lunn wrote:
Quoted 5 lines
I would not say it is a "new feature". But rather that patch in this email fixes issue that Linux kernel did not set correct MAC address for DSA slave ports. I think it is something which could be backported also to stable releases as "ignoring" vendor/factory MAC address is not correct behavior.
Hi Pali
The rules for stable are here:
https://www.kernel.org/doc/html/v5.8/process/stable-kernel-rules.html

Quoted 7 lines

Quoted 3 lines
Do you think it fits?
Andrew
Hello Andrew! I think it fits into those rules. As I wrote it fixes real bug that Linux kernel does not use correct MAC address for particular DSA slaves / ethernet ports.

O.K, then:

Reviewed-by Andrew Lunn <[email protected]>

Andrew

Pali Rohár

author

2020-09-08 07:47 UTC #6 lore raw

Quoted 22 lines · On Monday 07 September 2020 19:23:45 Andrew Lunn wrote:

Quoted 13 lines · Pali Rohár
Quoted 12 lines · On Monday 07 September 2020 17:43:53 Andrew Lunn wrote:
Quoted 5 lines
I would not say it is a "new feature". But rather that patch in this email fixes issue that Linux kernel did not set correct MAC address for DSA slave ports. I think it is something which could be backported also to stable releases as "ignoring" vendor/factory MAC address is not correct behavior.
Hi Pali
The rules for stable are here:
https://www.kernel.org/doc/html/v5.8/process/stable-kernel-rules.html
Quoted 7 lines
Quoted 3 lines
Do you think it fits?
Andrew
Hello Andrew! I think it fits into those rules. As I wrote it fixes real bug that Linux kernel does not use correct MAC address for particular DSA slaves / ethernet ports.

Quoted 5 lines

O.K, then:
Reviewed-by Andrew Lunn <[email protected]>
Andrew

Ok! Andrew, I would like to ask another question, how to correctly define that this patch depends on a2c7023f7075c? I specified it in human-readable part of commit description, but for backporting it would also need some machine-readable format. So patch would not be occasionally backported to older/stable kernel where a2c7023f7075c is not available.

Pali Rohár

author

2020-09-23 16:19 UTC #7 lore raw

Quoted 27 lines · On Tuesday 08 September 2020 09:47:33 Pali Rohár wrote:

Quoted 26 lines · On Monday 07 September 2020 19:23:45 Andrew Lunn wrote:
Quoted 13 lines · Pali Rohár
Quoted 12 lines · On Monday 07 September 2020 17:43:53 Andrew Lunn wrote:
Quoted 5 lines
I would not say it is a "new feature". But rather that patch in this email fixes issue that Linux kernel did not set correct MAC address for DSA slave ports. I think it is something which could be backported also to stable releases as "ignoring" vendor/factory MAC address is not correct behavior.
Hi Pali
The rules for stable are here:
https://www.kernel.org/doc/html/v5.8/process/stable-kernel-rules.html
Quoted 7 lines
Quoted 3 lines
Do you think it fits?
Andrew
Hello Andrew! I think it fits into those rules. As I wrote it fixes real bug that Linux kernel does not use correct MAC address for particular DSA slaves / ethernet ports.
O.K, then:
Reviewed-by Andrew Lunn <[email protected]>

Quoted 8 lines

Quoted 1 line
Andrew
Ok! Andrew, I would like to ask another question, how to correctly define that this patch depends on a2c7023f7075c? I specified it in human-readable part of commit description, but for backporting it would also need some machine-readable format. So patch would not be occasionally backported to older/stable kernel where a2c7023f7075c is not available.

Based on stable-kernel-rules.html document I think that following line should define this dependency in machine readable format:

Cc <[email protected]> # a2c7023f7075c: dsa: read mac address

Gregory, if it is correct, would you add that line into commit sign-off area where is existing Fixes: line?

Gregory CLEMENT

maintainer

2020-09-24 08:15 UTC #8 lore raw

Hi Pali,

Quoted 37 lines · On Tuesday 08 September 2020 09:47:33 Pali Rohár wrote:

Quoted 27 lines · On Monday 07 September 2020 19:23:45 Andrew Lunn wrote:
Quoted 26 lines · Pali Rohár
Quoted 13 lines · On Monday 07 September 2020 17:43:53 Andrew Lunn wrote:
Quoted 12 lines
Quoted 5 lines
I would not say it is a "new feature". But rather that patch in this email fixes issue that Linux kernel did not set correct MAC address for DSA slave ports. I think it is something which could be backported also to stable releases as "ignoring" vendor/factory MAC address is not correct behavior.
Hi Pali
The rules for stable are here:
https://www.kernel.org/doc/html/v5.8/process/stable-kernel-rules.html
Quoted 7 lines
Quoted 3 lines
Do you think it fits?
Andrew
Hello Andrew! I think it fits into those rules. As I wrote it fixes real bug that Linux kernel does not use correct MAC address for particular DSA slaves / ethernet ports.
O.K, then:
Reviewed-by Andrew Lunn <[email protected]>
Quoted 8 lines
Quoted 1 line
Andrew
Ok! Andrew, I would like to ask another question, how to correctly define that this patch depends on a2c7023f7075c? I specified it in human-readable part of commit description, but for backporting it would also need some machine-readable format. So patch would not be occasionally backported to older/stable kernel where a2c7023f7075c is not available.

Quoted 7 lines

Based on stable-kernel-rules.html document I think that following line should define this dependency in machine readable format:
Cc <[email protected]> # a2c7023f7075c: dsa: read mac address
Gregory, if it is correct, would you add that line into commit sign-off area where is existing Fixes: line?

I amended the commit log with this change.

Thanks,

Gregory

Signature · 3 lines

Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

Andre Heider

2020-09-07 17:13 UTC #9 lore raw

In reply to Pali Rohár

Hi Pali,

Quoted 43 lines · Pali Rohár

Espressobin boards have 3 ethernet ports and some of them got assigned more then one MAC address. MAC addresses are stored in U-Boot environment.
Since commit a2c7023f7075c ("net: dsa: read mac address from DT for slave device") kernel can use MAC addresses from DT for particular DSA port.
Currently Espressobin DTS file contains alias just for ethernet0.
This patch defines additional ethernet aliases in Espressobin DTS files, so bootloader can fill correct MAC address for DSA switch ports if more MAC addresses were specified.
DT alias ethernet1 is used for wan port, DT aliases ethernet2 and ethernet3 are used for lan ports for both Espressobin revisions (V5 and V7).
Fixes 5253cb8c00a6f ("arm64: dts: marvell: espressobin: add ethernet alias")
Signed-off-by Pali Rohár <[email protected]>
---
  .../dts/marvell/armada-3720-espressobin-v7-emmc.dts  | 10 ++++++++--
  .../boot/dts/marvell/armada-3720-espressobin-v7.dts  | 10 ++++++++--
  .../boot/dts/marvell/armada-3720-espressobin.dtsi    | 12 ++++++++----
  3 files changed, 24 insertions(+), 8 deletions(-)
arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7-emmc.dts +7 -1
index 03733fd92732..215d2f702623 100644
@@ -20,17 +20,23 @@
  	compatible = "globalscale,espressobin-v7-emmc", "globalscale,espressobin-v7",
  		     "globalscale,espressobin", "marvell,armada3720",
  		     "marvell,armada3710";
+
+	aliases {
+		/* ethernet1 is wan port */
+		ethernet1 = &switch0port3;
+		ethernet3 = &switch0port1;
+	};
  };
 
  &switch0 {
  	ports {
-		port@1 {
+		switch0port1: port@1 {

arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7-emmc.dts line 42

Quoted 10 lines

arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7-emmc.dts +1 -1

  			reg = <1>;
  			label = "lan1";
  			phy-handle = <&switch0phy0>;
  		};
 
-		port@3 {
+		switch0port3: port@3 {
  			reg = <3>;
  			label = "wan";
  			phy-handle = <&switch0phy2>;

My dts-foo is a little rusty, but now that you labeled the ports in the .dtsi, can this whole "switch0" block reduced to something like:

&switch0port1 {
	label = "lan1";
};

&switch0port3 {
	label = "wan";
};

Quoted 46 lines

arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7.dts +8 -2

index 8570c5f47d7d..b6f4af8ebafb 100644
@@ -19,17 +19,23 @@
  	model = "Globalscale Marvell ESPRESSOBin Board V7";
  	compatible = "globalscale,espressobin-v7", "globalscale,espressobin",
  		     "marvell,armada3720", "marvell,armada3710";
+
+	aliases {
+		/* ethernet1 is wan port */
+		ethernet1 = &switch0port3;
+		ethernet3 = &switch0port1;
+	};
  };
 
  &switch0 {
  	ports {
-		port@1 {
+		switch0port1: port@1 {
  			reg = <1>;
  			label = "lan1";
  			phy-handle = <&switch0phy0>;
  		};
 
-		port@3 {
+		switch0port3: port@3 {
  			reg = <3>;
  			label = "wan";
  			phy-handle = <&switch0phy2>;

arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +4 -0

index b97218c72727..0775c16e0ec8 100644
@@ -13,6 +13,10 @@
  / {
  	aliases {
  		ethernet0 = &eth0;
+		/* for dsa slave device */
+		ethernet1 = &switch0port1;
+		ethernet2 = &switch0port2;
+		ethernet3 = &switch0port3;
  		serial0 = &uart0;
  		serial1 = &uart1;
  	};

arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi line 127

Quoted 5 lines

arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +1 -1

@@ -120,7 +124,7 @@
  			#address-cells = <1>;
  			#size-cells = <0>;
 
-			port@0 {
+			switch0port0: port@0 {

This label is unused it seems.

Regards,
Andre

Quoted 27 lines

arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +3 -3

  				reg = <0>;
  				label = "cpu";
  				ethernet = <&eth0>;

@@ -131,19 +135,19 @@
  				};
  			};
 
-			port@1 {
+			switch0port1: port@1 {
  				reg = <1>;
  				label = "wan";
  				phy-handle = <&switch0phy0>;
  			};
 
-			port@2 {
+			switch0port2: port@2 {
  				reg = <2>;
  				label = "lan0";
  				phy-handle = <&switch0phy1>;
  			};
 
-			port@3 {
+			switch0port3: port@3 {
  				reg = <3>;
  				label = "lan1";
  				phy-handle = <&switch0phy2>;

Andrew Lunn

maintainer

2020-09-07 17:23 UTC #10 lore raw

Quoted 10 lines

My dts-foo is a little rusty, but now that you labeled the ports in the .dtsi, can this whole "switch0" block reduced to something like:
&switch0port1 {
	label = "lan1";
};
&switch0port3 {
	label = "wan";
};

Probably yes.

But that is definitely too much for stable.

Andrew

Pali Rohár

author

2020-09-07 17:35 UTC #11 lore raw

Quoted 7 lines · On Monday 07 September 2020 19:23:03 Andrew Lunn wrote:

Quoted 7 lines
My dts-foo is a little rusty, but now that you labeled the ports in the .dtsi, can this whole "switch0" block reduced to something like:
&switch0port1 {
	label = "lan1";
};

Quoted 7 lines

Quoted 3 lines
&switch0port3 {
	label = "wan";
};
Probably yes.
But that is definitely too much for stable.

Yes, this suggested change is not for stable, but looks like a nice cleanup. So it could be done in followup patch.

Andre, are you going to prepare and test this followup change?

Andre Heider

2020-09-07 17:43 UTC #12 lore raw

Quoted 12 lines · Pali Rohár

Quoted 11 lines · On Monday 07 September 2020 19:23:03 Andrew Lunn wrote:
Quoted 10 lines
My dts-foo is a little rusty, but now that you labeled the ports in the .dtsi, can this whole "switch0" block reduced to something like:
&switch0port1 {
	label = "lan1";
};
&switch0port3 {
	label = "wan";
};

Quoted 8 lines

Quoted 3 lines
Probably yes.
But that is definitely too much for stable.
Yes, this suggested change is not for stable, but looks like a nice cleanup. So it could be done in followup patch.
Andre, are you going to prepare and test this followup change?

I can prep the patch if you like, but the suggested cleanup only affects the v7 dts files. I don't have that hardware version to test it, so could only send an untested patch.

Regards,
Andre

Pali Rohár

author

2020-09-07 17:47 UTC #13 lore raw

Quoted 17 lines · On Monday 07 September 2020 19:43:08 Andre Heider wrote:

Quoted 16 lines · Pali Rohár
Quoted 14 lines · On Monday 07 September 2020 19:23:03 Andrew Lunn wrote:
Quoted 10 lines
My dts-foo is a little rusty, but now that you labeled the ports in the .dtsi, can this whole "switch0" block reduced to something like:
&switch0port1 {
	label = "lan1";
};
&switch0port3 {
	label = "wan";
};
Probably yes.
But that is definitely too much for stable.

Quoted 8 lines

Quoted 4 lines
Yes, this suggested change is not for stable, but looks like a nice cleanup. So it could be done in followup patch.
Andre, are you going to prepare and test this followup change?
I can prep the patch if you like, but the suggested cleanup only affects the v7 dts files. I don't have that hardware version to test it, so could only send an untested patch.

As a result of this cleanup should be binary DTB file for V7 with same structure as DTB file without such cleanup patch, right? And this test (structure / content of compiled file) does not need particular hardware.

Andre Heider

2020-09-07 17:51 UTC #14 lore raw

Quoted 23 lines · Pali Rohár

Quoted 22 lines · On Monday 07 September 2020 19:43:08 Andre Heider wrote:
Quoted 20 lines · Pali Rohár
Quoted 7 lines · On Monday 07 September 2020 19:23:03 Andrew Lunn wrote:
Quoted 7 lines
My dts-foo is a little rusty, but now that you labeled the ports in the .dtsi, can this whole "switch0" block reduced to something like:
&switch0port1 {
	label = "lan1";
};
Quoted 7 lines
Quoted 3 lines
&switch0port3 {
	label = "wan";
};
Probably yes.
But that is definitely too much for stable.
Yes, this suggested change is not for stable, but looks like a nice cleanup. So it could be done in followup patch.
Andre, are you going to prepare and test this followup change?

Quoted 7 lines

Quoted 3 lines
I can prep the patch if you like, but the suggested cleanup only affects the v7 dts files. I don't have that hardware version to test it, so could only send an untested patch.
As a result of this cleanup should be binary DTB file for V7 with same structure as DTB file without such cleanup patch, right? And this test (structure / content of compiled file) does not need particular hardware.

Ok, will do.

Andrew Lunn

maintainer

2020-09-07 18:50 UTC #15 lore raw

In reply to Pali Rohár

Quoted 2 lines

As a result of this cleanup should be binary DTB file for V7 with same structure as DTB file without such cleanup patch, right?

Should be. If need be, you can decompile the DTB back to a DTS and make sure it looks correct.

Andrew

Pali Rohár

author

2020-09-07 17:42 UTC #16 lore raw

In reply to Andre Heider

Quoted 7 lines · On Monday 07 September 2020 19:13:41 Andre Heider wrote:

arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi line 126
Quoted 5 lines
arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +1 -1
@@ -120,7 +124,7 @@
  			#address-cells = <1>;
  			#size-cells = <0>;
-			port@0 {
+			switch0port0: port@0 {
This label is unused it seems.

Yes, it is unused, but I defined labels for all ports so it would be clean that ports are indexed from zero and not from one. Also it looks inconsistent if some of DSA ports have labels and some does not.

Quoted 26 lines

Quoted 25 lines

arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +3 -3

  				reg = <0>;
  				label = "cpu";
  				ethernet = <&eth0>;

@@ -131,19 +135,19 @@
  				};
  			};
-			port@1 {
+			switch0port1: port@1 {
  				reg = <1>;
  				label = "wan";
  				phy-handle = <&switch0phy0>;
  			};
-			port@2 {
+			switch0port2: port@2 {
  				reg = <2>;
  				label = "lan0";
  				phy-handle = <&switch0phy1>;
  			};
-			port@3 {
+			switch0port3: port@3 {
  				reg = <3>;
  				label = "lan1";
  				phy-handle = <&switch0phy2>;

Andre Heider

Reviewed-by

2020-09-07 17:44 UTC #17 lore raw

Quoted 9 lines · Pali Rohár

Quoted 7 lines · On Monday 07 September 2020 19:13:41 Andre Heider wrote:
arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi line 126
Quoted 5 lines
arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +1 -1
@@ -120,7 +124,7 @@
   			#address-cells = <1>;
   			#size-cells = <0>;
-			port@0 {
+			switch0port0: port@0 {
This label is unused it seems.

Quoted 3 lines

Yes, it is unused, but I defined labels for all ports so it would be clean that ports are indexed from zero and not from one. Also it looks inconsistent if some of DSA ports have labels and some does not.

Alright, sounds good:

Reviewed-by Andre Heider <[email protected]>

Quoted 27 lines

Quoted 26 lines

arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +3 -3

   				reg = <0>;
   				label = "cpu";
   				ethernet = <&eth0>;

@@ -131,19 +135,19 @@
   				};
   			};
-			port@1 {
+			switch0port1: port@1 {
   				reg = <1>;
   				label = "wan";
   				phy-handle = <&switch0phy0>;
   			};
-			port@2 {
+			switch0port2: port@2 {
   				reg = <2>;
   				label = "lan0";
   				phy-handle = <&switch0phy1>;
   			};
-			port@3 {
+			switch0port3: port@3 {
   				reg = <3>;
   				label = "lan1";
   				phy-handle = <&switch0phy2>;

Gregory CLEMENT

maintainer

2020-09-23 15:02 UTC #18 lore raw

In reply to Pali Rohár

Hello Pali,

Quoted 12 lines

Espressobin boards have 3 ethernet ports and some of them got assigned more then one MAC address. MAC addresses are stored in U-Boot environment.
Since commit a2c7023f7075c ("net: dsa: read mac address from DT for slave device") kernel can use MAC addresses from DT for particular DSA port.
Currently Espressobin DTS file contains alias just for ethernet0.
This patch defines additional ethernet aliases in Espressobin DTS files, so bootloader can fill correct MAC address for DSA switch ports if more MAC addresses were specified.

Quoted 5 lines

DT alias ethernet1 is used for wan port, DT aliases ethernet2 and ethernet3 are used for lan ports for both Espressobin revisions (V5 and V7).
Fixes 5253cb8c00a6f ("arm64: dts: marvell: espressobin: add ethernet alias")
Signed-off-by Pali Rohár <[email protected]>

Applied on mvebu/fixes

Thanks,

Gregory

Quoted 116 lines

---
 .../dts/marvell/armada-3720-espressobin-v7-emmc.dts  | 10 ++++++++--
 .../boot/dts/marvell/armada-3720-espressobin-v7.dts  | 10 ++++++++--
 .../boot/dts/marvell/armada-3720-espressobin.dtsi    | 12 ++++++++----
 3 files changed, 24 insertions(+), 8 deletions(-)

arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7-emmc.dts +8 -2

index 03733fd92732..215d2f702623 100644
@@ -20,17 +20,23 @@
 	compatible = "globalscale,espressobin-v7-emmc", "globalscale,espressobin-v7",
 		     "globalscale,espressobin", "marvell,armada3720",
 		     "marvell,armada3710";
+
+	aliases {
+		/* ethernet1 is wan port */
+		ethernet1 = &switch0port3;
+		ethernet3 = &switch0port1;
+	};
 };
 
 &switch0 {
 	ports {
-		port@1 {
+		switch0port1: port@1 {
 			reg = <1>;
 			label = "lan1";
 			phy-handle = <&switch0phy0>;
 		};
 
-		port@3 {
+		switch0port3: port@3 {
 			reg = <3>;
 			label = "wan";
 			phy-handle = <&switch0phy2>;

arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7.dts +8 -2

index 8570c5f47d7d..b6f4af8ebafb 100644
@@ -19,17 +19,23 @@
 	model = "Globalscale Marvell ESPRESSOBin Board V7";
 	compatible = "globalscale,espressobin-v7", "globalscale,espressobin",
 		     "marvell,armada3720", "marvell,armada3710";
+
+	aliases {
+		/* ethernet1 is wan port */
+		ethernet1 = &switch0port3;
+		ethernet3 = &switch0port1;
+	};
 };
 
 &switch0 {
 	ports {
-		port@1 {
+		switch0port1: port@1 {
 			reg = <1>;
 			label = "lan1";
 			phy-handle = <&switch0phy0>;
 		};
 
-		port@3 {
+		switch0port3: port@3 {
 			reg = <3>;
 			label = "wan";
 			phy-handle = <&switch0phy2>;

arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +8 -4

index b97218c72727..0775c16e0ec8 100644
@@ -13,6 +13,10 @@
 / {
 	aliases {
 		ethernet0 = &eth0;
+		/* for dsa slave device */
+		ethernet1 = &switch0port1;
+		ethernet2 = &switch0port2;
+		ethernet3 = &switch0port3;
 		serial0 = &uart0;
 		serial1 = &uart1;
 	};

@@ -120,7 +124,7 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 
-			port@0 {
+			switch0port0: port@0 {
 				reg = <0>;
 				label = "cpu";
 				ethernet = <&eth0>;

@@ -131,19 +135,19 @@
 				};
 			};
 
-			port@1 {
+			switch0port1: port@1 {
 				reg = <1>;
 				label = "wan";
 				phy-handle = <&switch0phy0>;
 			};
 
-			port@2 {
+			switch0port2: port@2 {
 				reg = <2>;
 				label = "lan0";
 				phy-handle = <&switch0phy1>;
 			};
 
-			port@3 {
+			switch0port3: port@3 {
 				reg = <3>;
 				label = "lan1";
 				phy-handle = <&switch0phy2>;

Signature · 1 line

2.20.1

Signature · 3 lines

Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com