[PATCH net-next v7 5/7] ARM64: dts: marvell: Fix some common switch mistakes

[Chris Packham]

Hi All,

On 25/10/23 07:28, Vladimir Oltean wrote:
> Linus,
>
> On Tue, Oct 24, 2023 at 03:20:31PM +0200, Linus Walleij wrote:
>> Fix some errors in the Marvell MV88E6xxx switch descriptions:
>> - The top node had no address size or cells.
>> - switch0 at 0 is not OK, should be ethernet-switch at 0.
>> - ports should be ethernet-ports
>> - port at 0 should be ethernet-port at 0
>> - PHYs should be named ethernet-phy@
>>
>> Reviewed-by: Andrew Lunn <andrew at lunn.ch>
>> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
<snip>
>> ---
>> diff --git a/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi b/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
>> index 32cfb3e2efc3..7538ed56053b 100644
>> --- a/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
>> @@ -207,11 +207,9 @@ phy0: ethernet-phy at 0 {
>>   		reg = <0>;
>>   	};
>>   
>> -	switch6: switch0 at 6 {
>> +	switch6: ethernet-switch at 6 {
>>   		/* Actual device is MV88E6393X */
>>   		compatible = "marvell,mv88e6190";
>> -		#address-cells = <1>;
>> -		#size-cells = <0>;
>>   		reg = <6>;
>>   		interrupt-parent = <&cp0_gpio1>;
>>   		interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
>> @@ -220,59 +218,59 @@ switch6: switch0 at 6 {
>>   
>>   		dsa,member = <0 0>;
>>   
>> -		ports {
>> +		ethernet-ports {
>>   			#address-cells = <1>;
>>   			#size-cells = <0>;
>>   
>> -			port at 1 {
>> +			ethernet-port at 1 {
>>   				reg = <1>;
>>   				label = "p1";
>>   				phy-handle = <&switch0phy1>;
>>   			};
>>   
>> -			port at 2 {
>> +			ethernet-port at 2 {
>>   				reg = <2>;
>>   				label = "p2";
>>   				phy-handle = <&switch0phy2>;
>>   			};
>>   
>> -			port at 3 {
>> +			ethernet-port at 3 {
>>   				reg = <3>;
>>   				label = "p3";
>>   				phy-handle = <&switch0phy3>;
>>   			};
>>   
>> -			port at 4 {
>> +			ethernet-port at 4 {
>>   				reg = <4>;
>>   				label = "p4";
>>   				phy-handle = <&switch0phy4>;
>>   			};
>>   
>> -			port at 5 {
>> +			ethernet-port at 5 {
>>   				reg = <5>;
>>   				label = "p5";
>>   				phy-handle = <&switch0phy5>;
>>   			};
>>   
>> -			port at 6 {
>> +			ethernet-port at 6 {
>>   				reg = <6>;
>>   				label = "p6";
>>   				phy-handle = <&switch0phy6>;
>>   			};
>>   
>> -			port at 7 {
>> +			ethernet-port at 7 {
>>   				reg = <7>;
>>   				label = "p7";
>>   				phy-handle = <&switch0phy7>;
>>   			};
>>   
>> -			port at 8 {
>> +			ethernet-port at 8 {
>>   				reg = <8>;
>>   				label = "p8";
>>   				phy-handle = <&switch0phy8>;
>>   			};
>>   
>> -			port at 9 {
>> +			ethernet-port at 9 {
>>   				reg = <9>;
>>   				label = "p9";
>>   				phy-mode = "10gbase-r";
>> @@ -280,7 +278,7 @@ port at 9 {
>>   				managed = "in-band-status";
>>   			};
>>   
>> -			port at a {
>> +			ethernet-port at a {
>>   				reg = <10>;
>>   				ethernet = <&cp0_eth0>;
>>   				phy-mode = "10gbase-r";
>> @@ -293,35 +291,35 @@ mdio {
>>   			#address-cells = <1>;
>>   			#size-cells = <0>;
>>   
>> -			switch0phy1: switch0phy1 at 1 {
>> +			switch0phy1: ethernet-phy at 1 {
>>   				reg = <0x1>;
>>   			};
>>   
>> -			switch0phy2: switch0phy2 at 2 {
>> +			switch0phy2: ethernet-phy at 2 {
>>   				reg = <0x2>;
>>   			};
>>   
>> -			switch0phy3: switch0phy3 at 3 {
>> +			switch0phy3: ethernet-phy at 3 {
>>   				reg = <0x3>;
>>   			};
>>   
>> -			switch0phy4: switch0phy4 at 4 {
>> +			switch0phy4: ethernet-phy at 4 {
>>   				reg = <0x4>;
>>   			};
>>   
>> -			switch0phy5: switch0phy5 at 5 {
>> +			switch0phy5: ethernet-phy at 5 {
>>   				reg = <0x5>;
>>   			};
>>   
>> -			switch0phy6: switch0phy6 at 6 {
>> +			switch0phy6: ethernet-phy at 6 {
>>   				reg = <0x6>;
>>   			};
>>   
>> -			switch0phy7: switch0phy7 at 7 {
>> +			switch0phy7: ethernet-phy at 7 {
>>   				reg = <0x7>;
>>   			};
>>   
>> -			switch0phy8: switch0phy8 at 8 {
>> +			switch0phy8: ethernet-phy at 8 {
>>   				reg = <0x8>;
>>   			};
>>   		};
> Chris, does this look okay?

There's nothing in the u-boot code for the CN9130-CRB that cares about 
the switch so I don't think there's any issue ABI wise. We are working 
on our own CN9130 based router with a L2 switch but it's at a point we 
can follow whatever upstream decide is the final schema.

In terms of my personal preference the schema quoted up thread has the 
pattern  '^(ethernet-)?switch(@.*)?$' (i.e. the 'ethernet-' part is 
optional) so I'd personally prefer switch0 at 6 -> switch at 6 but that's only 
a slight preference because I deal with Ethernet switches day in day out.