| Message ID | 20200907112718.5994-1-pali@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: marvell: espressobin: Add ethernet switch aliases | expand |
On Mon, Sep 07, 2020 at 01:27:17PM +0200, 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> I'm not sure a Fixes: is appropriate here. What is actually broken? This just seems like a new feature. Andrew
On Monday 07 September 2020 16:42:28 Andrew Lunn wrote: > On Mon, Sep 07, 2020 at 01:27:17PM +0200, 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> > > 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.
> 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
On Monday 07 September 2020 17:43:53 Andrew Lunn wrote: > > 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 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.
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 = ð0; > + /* 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 = <ð0>; > @@ -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>; >
> 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
On Mon, Sep 07, 2020 at 06:13:16PM +0200, Pali Rohár wrote: > On Monday 07 September 2020 17:43:53 Andrew Lunn wrote: > > > 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 > > 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 <andrew@lunn.ch> Andrew
On Monday 07 September 2020 19:23:03 Andrew Lunn wrote: > > 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. 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?
On Monday 07 September 2020 19:13:41 Andre Heider wrote: > > @@ -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. > > > reg = <0>; > > label = "cpu"; > > ethernet = <ð0>; > > @@ -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>; > > >
On 07/09/2020 19:35, Pali Rohár wrote: > On Monday 07 September 2020 19:23:03 Andrew Lunn wrote: >>> 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. > > 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
On 07/09/2020 19:42, Pali Rohár wrote: > On Monday 07 September 2020 19:13:41 Andre Heider wrote: >>> @@ -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. Alright, sounds good: Reviewed-by: Andre Heider <a.heider@gmail.com> > >> >>> reg = <0>; >>> label = "cpu"; >>> ethernet = <ð0>; >>> @@ -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>; >>> >>
On Monday 07 September 2020 19:43:08 Andre Heider wrote: > On 07/09/2020 19:35, Pali Rohár wrote: > > On Monday 07 September 2020 19:23:03 Andrew Lunn wrote: > > > > 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. > > > > 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.
On 07/09/2020 19:47, Pali Rohár wrote: > On Monday 07 September 2020 19:43:08 Andre Heider wrote: >> On 07/09/2020 19:35, Pali Rohár wrote: >>> On Monday 07 September 2020 19:23:03 Andrew Lunn wrote: >>>>> 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. >>> >>> 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. Ok, will do.
> 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
On Monday 07 September 2020 19:23:45 Andrew Lunn wrote: > On Mon, Sep 07, 2020 at 06:13:16PM +0200, Pali Rohár wrote: > > On Monday 07 September 2020 17:43:53 Andrew Lunn wrote: > > > > 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 > > > > 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 <andrew@lunn.ch> > > 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.
Hello Pali, > 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> Applied on mvebu/fixes Thanks, Gregory > --- > .../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>; > 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 = ð0; > + /* 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 = <ð0>; > @@ -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>; > -- > 2.20.1 >
On Tuesday 08 September 2020 09:47:33 Pali Rohár wrote: > On Monday 07 September 2020 19:23:45 Andrew Lunn wrote: > > On Mon, Sep 07, 2020 at 06:13:16PM +0200, Pali Rohár wrote: > > > On Monday 07 September 2020 17:43:53 Andrew Lunn wrote: > > > > > 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 > > > > > > 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 <andrew@lunn.ch> > > > > 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: <stable@vger.kernel.org> # 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?
Hi Pali, > On Tuesday 08 September 2020 09:47:33 Pali Rohár wrote: >> On Monday 07 September 2020 19:23:45 Andrew Lunn wrote: >> > On Mon, Sep 07, 2020 at 06:13:16PM +0200, Pali Rohár wrote: >> > > On Monday 07 September 2020 17:43:53 Andrew Lunn wrote: >> > > > > 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 >> > > >> > > 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 <andrew@lunn.ch> >> > >> > 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: <stable@vger.kernel.org> # 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
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>; 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 = ð0; + /* 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 = <ð0>; @@ -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>;
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(-)