Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1616577 > unrolled thread

[PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port Ralph Sennhauser <ralph.sennhauser@gmail.com> - 2017-04-05 07:30 +0200
    Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead  of internet for DSA port Andrew Lunn <andrew@lunn.ch> - 2017-04-05 16:40 +0200
      Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead  of internet for DSA port Ralph Sennhauser <ralph.sennhauser@gmail.com> - 2017-04-05 17:30 +0200
        Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead  of internet for DSA port Andrew Lunn <andrew@lunn.ch> - 2017-04-05 18:00 +0200
          Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead  of internet for DSA port Ralph Sennhauser <ralph.sennhauser@gmail.com> - 2017-04-05 18:10 +0200

#1616577 — [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port

The LEDs for the "wan" port are already labeled "mamba:amber:wan" resp.
"mamba:white:wan". So besides being an outlier with regard to the rest
of the product line (see table below) changing the label fixes an
internal inconsistency as well.

This will be visible in user space. Given commit cb4f71c42988 ("ARM:
dts: armada-38x: change order of ethernet DT nodes on Armada 38x") it's
expected to happen anyway. Commit 499400c9ac20 ("ARM: dts:
armada-xp-linksys-mamba: Utilize new DSA binding") switches to the new
bindings, use this opportunity to do it now rather than later.

|-----------------------------------------------------------------|
| Labels used for the case and those used for the DSA ports       |
|-----------------------------------------------------------------|
| case labels	| armada-385-linksys-*	| armada-xp-linksys-mamba |
|---------------|-----------------------|-------------------------|
| internet	| wan			| internet		  |
| 1		| lan1			| lan1			  |
| 2		| lan2			| lan2			  |
| 3		| lan3			| lan3			  |
| 4		| lan4			| lan4			  |
|-----------------------------------------------------------------|

Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
---

Hi everybody,

I underestimated the urge of people to make all the same before. While I do not
particularly like this sort of change I see it coming anyway. So this patch is
meant to make it a deliberate decision so it no longer is an item lurking in
the shadows. Whether this patch gets taken or rejected my goal is reached.

In hindsight wan would have been the better choice.

Ralph

 arch/arm/boot/dts/armada-xp-linksys-mamba.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
index 9efcf59..0143aed 100644
--- a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
+++ b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
@@ -354,7 +354,7 @@
 
 			port@4 {
 				reg = <4>;
-				label = "internet";
+				label = "wan";
 			};
 
 			port@5 {
@@ -452,7 +452,7 @@
 
 			port@4 {
 				reg = <4>;
-				label = "internet";
+				label = "wan";
 			};
 
 			port@5 {
-- 
2.10.2

[toc] | [next] | [standalone]

#1617010 — Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port

On Wed, Apr 05, 2017 at 07:28:46AM +0200, Ralph Sennhauser wrote:
> The LEDs for the "wan" port are already labeled "mamba:amber:wan" resp.
> "mamba:white:wan". So besides being an outlier with regard to the rest
> of the product line (see table below) changing the label fixes an
> internal inconsistency as well.
> 
> This will be visible in user space. Given commit cb4f71c42988 ("ARM:
> dts: armada-38x: change order of ethernet DT nodes on Armada 38x") it's
> expected to happen anyway. Commit 499400c9ac20 ("ARM: dts:
> armada-xp-linksys-mamba: Utilize new DSA binding") switches to the new
> bindings, use this opportunity to do it now rather than later.
> 
> |-----------------------------------------------------------------|
> | Labels used for the case and those used for the DSA ports       |
> |-----------------------------------------------------------------|
> | case labels	| armada-385-linksys-*	| armada-xp-linksys-mamba |
> |---------------|-----------------------|-------------------------|
> | internet	| wan			| internet		  |
> | 1		| lan1			| lan1			  |
> | 2		| lan2			| lan2			  |
> | 3		| lan3			| lan3			  |
> | 4		| lan4			| lan4			  |
> |-----------------------------------------------------------------|

Hi Ralph

I always encourage people to use the case labels as interface names.
So if i was going to make a change, it would be wan->internet for the 
armada-385-linksys-*.

> I underestimated the urge of people to make all the same before. While I do not
> particularly like this sort of change I see it coming anyway. So this patch is
> meant to make it a deliberate decision so it no longer is an item lurking in
> the shadows. Whether this patch gets taken or rejected my goal is reached.

I would say, this is now too late. Changing an interface name will
break configuration scripts. We are stuck with it.

      Andrew

[toc] | [prev] | [next] | [standalone]

#1617051 — Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port

On Wed, 5 Apr 2017 16:28:24 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Wed, Apr 05, 2017 at 07:28:46AM +0200, Ralph Sennhauser wrote:
> > The LEDs for the "wan" port are already labeled "mamba:amber:wan"
> > resp. "mamba:white:wan". So besides being an outlier with regard to
> > the rest of the product line (see table below) changing the label
> > fixes an internal inconsistency as well.
> > 
> > This will be visible in user space. Given commit cb4f71c42988 ("ARM:
> > dts: armada-38x: change order of ethernet DT nodes on Armada 38x")
> > it's expected to happen anyway. Commit 499400c9ac20 ("ARM: dts:
> > armada-xp-linksys-mamba: Utilize new DSA binding") switches to the
> > new bindings, use this opportunity to do it now rather than later.
> > 
> > |-----------------------------------------------------------------|
> > | Labels used for the case and those used for the DSA ports       |
> > |-----------------------------------------------------------------|
> > | case labels	| armada-385-linksys-*	|
> > armada-xp-linksys-mamba |
> > |---------------|-----------------------|-------------------------|
> > | internet	| wan			|
> > internet		  | | 1		|
> > lan1			| lan1			  | |
> > 2		| lan2			|
> > lan2			  | | 3		|
> > lan3			| lan3			  | |
> > 4		| lan4			|
> > lan4			  |
> > |-----------------------------------------------------------------|  
> 
> Hi Ralph
> 
> I always encourage people to use the case labels as interface names.
> So if i was going to make a change, it would be wan->internet for the 
> armada-385-linksys-*.

Glad Imre used lan1 instead of 1 ;) 

The reverse I have appended to this mail so people know what it would
look like.

In fact "internet" label on the case is uppercase, would this matter
for you for new bindings, or would you still use the lowercase version?
It's not relevant here, just wondering.

> 
> > I underestimated the urge of people to make all the same before.
> > While I do not particularly like this sort of change I see it
> > coming anyway. So this patch is meant to make it a deliberate
> > decision so it no longer is an item lurking in the shadows. Whether
> > this patch gets taken or rejected my goal is reached.  
> 
> I would say, this is now too late. Changing an interface name will
> break configuration scripts. We are stuck with it.
> 

If it weren't for commit cb4f71c42988 that would have been obvious for
me as well.

Thanks
Ralph



The inverse (following trough all the way) would look the following,
same changes applies to Rango which is not yet supported by vanilla
kernel.

diff --git a/arch/arm/boot/dts/armada-385-linksys-caiman.dts b/arch/arm/boot/dts/armada-385-linksys-caiman.dts
index a1b6e68..3efceda 100644
--- a/arch/arm/boot/dts/armada-385-linksys-caiman.dts
+++ b/arch/arm/boot/dts/armada-385-linksys-caiman.dts
@@ -53,13 +53,13 @@
 				#address-cells = <1>;
 				#size-cells = <0>;
 
-					wan_amber@0 {
-						label = "caiman:amber:wan";
+					internet_amber@0 {
+						label = "caiman:amber:internet";
 						reg = <0x0>;
 					};
 
-					wan_white@1 {
-						label = "caiman:white:wan";
+					internet_white@1 {
+						label = "caiman:white:internet";
 						reg = <0x1>;
 					};
 
diff --git a/arch/arm/boot/dts/armada-385-linksys-cobra.dts b/arch/arm/boot/dts/armada-385-linksys-cobra.dts
index a1a75af..4f2e7f6 100644
--- a/arch/arm/boot/dts/armada-385-linksys-cobra.dts
+++ b/arch/arm/boot/dts/armada-385-linksys-cobra.dts
@@ -53,13 +53,13 @@
 				#address-cells = <1>;
 				#size-cells = <0>;
 
-					wan_amber@0 {
-						label = "cobra:amber:wan";
+					internet_amber@0 {
+						label = "cobra:amber:internet";
 						reg = <0x0>;
 					};
 
-					wan_white@1 {
-						label = "cobra:white:wan";
+					internet_white@1 {
+						label = "cobra:white:internet";
 						reg = <0x1>;
 					};
 
diff --git a/arch/arm/boot/dts/armada-385-linksys-shelby.dts b/arch/arm/boot/dts/armada-385-linksys-shelby.dts
index c7a8ddd..a7b47f7 100644
--- a/arch/arm/boot/dts/armada-385-linksys-shelby.dts
+++ b/arch/arm/boot/dts/armada-385-linksys-shelby.dts
@@ -53,13 +53,13 @@
 				#address-cells = <1>;
 				#size-cells = <0>;
 
-					wan_amber@0 {
-						label = "shelby:amber:wan";
+					internet_amber@0 {
+						label = "shelby:amber:internet";
 						reg = <0x0>;
 					};
 
-					wan_white@1 {
-						label = "shelby:white:wan";
+					internet_white@1 {
+						label = "shelby:white:internet";
 						reg = <0x1>;
 					};
 
diff --git a/arch/arm/boot/dts/armada-385-linksys.dtsi b/arch/arm/boot/dts/armada-385-linksys.dtsi
index aa0d225..8460066 100644
--- a/arch/arm/boot/dts/armada-385-linksys.dtsi
+++ b/arch/arm/boot/dts/armada-385-linksys.dtsi
@@ -145,7 +145,7 @@
 
 						port@4 {
 							reg = <4>;
-							label = "wan";
+							label = "internet";
 						};
 
 						port@5 {
@@ -360,7 +360,7 @@
 
 			port@4 {
 				reg = <4>;
-				label = "wan";
+				label = "internet";
 			};
 
 			port@5 {
diff --git a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
index 9efcf59..ed2fe8f 100644
--- a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
+++ b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
@@ -140,13 +140,13 @@
 					compatible = "ti,tlc59116";
 					reg = <0x68>;
 
-					wan_amber@0 {
-						label = "mamba:amber:wan";
+					internet_amber@0 {
+						label = "mamba:amber:internet";
 						reg = <0x0>;
 					};
 
-					wan_white@1 {
-						label = "mamba:white:wan";
+					internet_white@1 {
+						label = "mamba:white:internet";
 						reg = <0x1>;
 					};
 

[toc] | [prev] | [next] | [standalone]

#1617079 — Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port

> In fact "internet" label on the case is uppercase, would this matter
> for you for new bindings, or would you still use the lowercase version?

I would use lower case, just to fix with the general convention that
interface names are lower case.

> > I would say, this is now too late. Changing an interface name will
> > break configuration scripts. We are stuck with it.
> > 
> 
> If it weren't for commit cb4f71c42988 that would have been obvious for
> me as well.

Yes, that was not nice. But it was also very earlier in the life of
38x, so it just affected a few developers with reference boards, not
real products out in the wild, as far as i remember.

     Andrew

[toc] | [prev] | [next] | [standalone]

#1617094 — Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port

Hi Andrew,

On Wed, 5 Apr 2017 17:50:32 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > In fact "internet" label on the case is uppercase, would this matter
> > for you for new bindings, or would you still use the lowercase
> > version?  
> 
> I would use lower case, just to fix with the general convention that
> interface names are lower case.

Thanks

> 
> > > I would say, this is now too late. Changing an interface name will
> > > break configuration scripts. We are stuck with it.
> > >   
> > 
> > If it weren't for commit cb4f71c42988 that would have been obvious
> > for me as well.  
> 
> Yes, that was not nice. But it was also very earlier in the life of
> 38x, so it just affected a few developers with reference boards, not
> real products out in the wild, as far as i remember.
> 
>      Andrew

The mentioned commit bit me amongst others, see
https://lkml.org/lkml/2016/8/21/62

In fact I expect the impact to be much lower this time around, before
4.10 DSA was no contender for swconfig (out of tree switch driver as
used by OpenWrt and others). On Mamba ~200Mbit was top speed. Now they
perform roughly the same.


Thanks
Ralph

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web