Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] arm64: dts: marvell: espressobin: Add ethernet switch
 aliases
To:     =?UTF-8?Q?Pali_Roh=c3=a1r?= <pali@kernel.org>,
        Jason Cooper <jason@lakedaemon.net>,
        Andrew Lunn <andrew@lunn.ch>,
        Gregory Clement <gregory.clement@bootlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Tomasz Maciej Nowak <tmn505@gmail.com>
Cc:     linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20200907112718.5994-1-pali@kernel.org>
From:   Andre Heider <a.heider@gmail.com>
Message-ID: <3ec54259-4bfe-8462-e8d5-083fc009707a@gmail.com>
Date:   Mon, 7 Sep 2020 19:13:41 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.2.1
MIME-Version: 1.0
In-Reply-To: <20200907112718.5994-1-pali@kernel.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Pali,

On 07/09/2020 13:27, Pali Rohár wrote:
> 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 <pali@kernel.org>
> ---
>   .../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(-)
> 
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7-emmc.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7-emmc.dts
> index 03733fd92732..215d2f702623 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7-emmc.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7-emmc.dts
> @@ -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>;

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";
};

?

> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7.dts
> index 8570c5f47d7d..b6f4af8ebafb 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin-v7.dts
> @@ -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>;
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi
> index b97218c72727..0775c16e0ec8 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi
> @@ -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 {

This label is unused it seems.

Regards,
Andre

>   				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>;
>