[PATCH 4/5] arm: dts: ls1028a: Add Ethernet switch node and dependencies

[Claudiu Manoil]

>-----Original Message-----
>From: Michael Walle <michael at walle.cc>
>Sent: Wednesday, January 13, 2021 10:11 PM
[...]
>> --- a/arch/arm/dts/fsl-ls1028a.dtsi
>> +++ b/arch/arm/dts/fsl-ls1028a.dtsi
>> @@ -14,6 +14,17 @@
>>  	#address-cells = <2>;
>>  	#size-cells = <2>;
>>
>> +	aliases {
>> +		eth0 = &enetc0;
>> +		eth1 = &enetc1;
>> +		eth2 = &enetc2;
>> +		eth3 = &enetc6;
>> +		eth4 = &felix0;
>> +		eth5 = &felix1;
>> +		eth6 = &felix2;
>> +		eth7 = &felix3;
>> +	};
>
>Don't include the aliases in the common dtsi. There are serveral
>reasons for that:
>  (1) it is really board dependent. not every board has all these
>      ports.
>  (2) it will mess up the device numbering for boards which use
>      this dtsi. And with this it will also mess up the ethNaddr
>      environment variable logic.
>
>Please move them into the corresponding boards.
>

Hi Michael,

I think the point of this patch was to enable the felix switch for the LS1028A RDB only for now,
for simplicity, to make the patch set smaller. Follow-up patches would enable it for the remaining
boards.
But I understand that keeping the aliases in the fsl-ls1028a.dtsi can mess up other boards
that include it, including the Kontron boards.  Would you agree to update only the ls1028 RDB
board DT for now?

Alternatively you could test these patches on your boards and maybe provide the DT updates for
the Kontron boards as part of this patch set.

>> +
>>  	sysclk: sysclk {
>>  		compatible = "fixed-clock";
>>  		#clock-cells = <0>;
>> @@ -151,9 +162,51 @@
>>  			reg = <0x000300 0 0 0 0>;
>>  			status = "disabled";
>>  		};
>> +		ethsw: pci at 0,5 {
>> +			#address-cells=<0>;
>> +			#size-cells=<1>;
>> +			reg = <0x000500 0 0 0 0>;
>> +
>> +			ethsw_ports: ports {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +
>> +				felix0: port at 0 {
>> +					reg = <0>;
>> +					status = "disabled";
>> +					label = "swp0";
>> +				};
>> +				felix1: port at 1 {
>> +					reg = <1>;
>> +					status = "disabled";
>> +					label = "swp1";
>> +				};
>> +				felix2: port at 2 {
>> +					reg = <2>;
>> +					status = "disabled";
>> +					label = "swp2";
>> +				};
>> +				felix3: port at 3 {
>> +					reg = <3>;
>> +					status = "disabled";
>> +					label = "swp3";
>> +				};
>> +				port at 4 {
>> +					reg = <4>;
>> +					phy-mode = "internal";
>> +					status = "okay";
>> +					ethernet = <&enetc2>;
>> +				};
>
>status = "disabled".
>
>Why would you enable just this port if all the switch ports
>are disabled.
>

The number of active front panel ports may vary from board to board.
These ports are supposed to be enabled in each board DT, depending
on setup. But a DSA switch always needs a CPU port regardless of how
many front side ports are active in each particular setup, and port at 4 is
the CPU port.

Thanks,
Claudiu