[net-next,v4,5/7] ARM64: dts: marvell: Fix some common switch mistakes
Commit Message
Fix some errors in the Marvell MV88E6xxx switch descriptions:
- The top node had no address size or cells.
- switch0@0 is not OK, should be switch@0.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi | 4 +---
arch/arm64/boot/dts/marvell/armada-3720-gl-mv1000.dts | 4 +---
arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 12 ++++++------
arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts | 2 --
arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts | 2 +-
arch/arm64/boot/dts/marvell/cn9130-crb.dtsi | 4 +---
6 files changed, 10 insertions(+), 18 deletions(-)
Comments
On Wed, Oct 18, 2023 at 11:03:44AM +0200, Linus Walleij wrote:
> Fix some errors in the Marvell MV88E6xxx switch descriptions:
> - The top node had no address size or cells.
> - switch0@0 is not OK, should be switch@0.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
+Marek
On Wed, Oct 18, 2023 at 11:03:44AM +0200, Linus Walleij wrote:
> Fix some errors in the Marvell MV88E6xxx switch descriptions:
> - The top node had no address size or cells.
> - switch0@0 is not OK, should be switch@0.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 9eab2bb22134..c69cb4e191e5 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -305,7 +305,7 @@ phy1: ethernet-phy@1 {
> };
>
> /* switch nodes are enabled by U-Boot if modules are present */
> - switch0@10 {
> + switch@10 {
As the comment says: U-Boot
(https://elixir.bootlin.com/u-boot/latest/source/board/CZ.NIC/turris_mox/turris_mox.c#L728)
sets up status = "okay" for these nodes depending on the MOXTET
configuration. It doesn't look as if it's doing that by alias, just by
path ("%s/switch%i@%x").
I have a Turris MOX, please allow me some time to test if the node name
change is going to be significant and cause regressions. I expect the
answer to be yes (sadly).
> compatible = "marvell,mv88e6190";
> reg = <0x10>;
> dsa,member = <0 0>;
> @@ -430,7 +430,7 @@ port-sfp@a {
> };
> };
>
> - switch0@2 {
> + switch@2 {
> compatible = "marvell,mv88e6085";
> reg = <0x2>;
> dsa,member = <0 0>;
> @@ -497,7 +497,7 @@ port@5 {
> };
> };
>
> - switch1@11 {
> + switch@11 {
> compatible = "marvell,mv88e6190";
> reg = <0x11>;
> dsa,member = <0 1>;
> @@ -622,7 +622,7 @@ port-sfp@a {
> };
> };
>
> - switch1@2 {
> + switch@2 {
> compatible = "marvell,mv88e6085";
> reg = <0x2>;
> dsa,member = <0 1>;
> @@ -689,7 +689,7 @@ port@5 {
> };
> };
>
> - switch2@12 {
> + switch@12 {
> compatible = "marvell,mv88e6190";
> reg = <0x12>;
> dsa,member = <0 2>;
> @@ -805,7 +805,7 @@ port-sfp@a {
> };
> };
>
> - switch2@2 {
> + switch@2 {
> compatible = "marvell,mv88e6085";
> reg = <0x2>;
> dsa,member = <0 2>;
On Thu, Oct 19, 2023 at 05:40:22PM +0300, Vladimir Oltean wrote:
> +Marek
>
> On Wed, Oct 18, 2023 at 11:03:44AM +0200, Linus Walleij wrote:
> > Fix some errors in the Marvell MV88E6xxx switch descriptions:
> > - The top node had no address size or cells.
> > - switch0@0 is not OK, should be switch@0.
> >
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > index 9eab2bb22134..c69cb4e191e5 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > @@ -305,7 +305,7 @@ phy1: ethernet-phy@1 {
> > };
> >
> > /* switch nodes are enabled by U-Boot if modules are present */
> > - switch0@10 {
> > + switch@10 {
>
> As the comment says: U-Boot
> (https://elixir.bootlin.com/u-boot/latest/source/board/CZ.NIC/turris_mox/turris_mox.c#L728)
> sets up status = "okay" for these nodes depending on the MOXTET
> configuration. It doesn't look as if it's doing that by alias, just by
> path ("%s/switch%i@%x").
>
> I have a Turris MOX, please allow me some time to test if the node name
> change is going to be significant and cause regressions. I expect the
> answer to be yes (sadly).
Yeah, it's bad.
U-Boot 2018.11 (Dec 16 2018 - 12:50:19 +0000), Build: jenkins-turris-os-packages-kittens-mox-90
DRAM: 1 GiB
Enabling Armada 3720 wComphy-0: SGMII1 3.125 Gbps
Comphy-1: PEX0 5 Gbps
Comphy-2: USB3_HOST0 5 Gbps
MMC: sdhci@d8000: 0
Loading Environment from SPI Flash... SF: Detected w25q64dw with page size 256 Bytes, erase size 4 KiB, total 8 MiB
OK
Model: CZ.NIC Turris Mox Board
Net: eth0: neta@30000
Turris Mox:
Board version: 22
RAM size: 1024 MiB
SD/eMMC version: SD
Module Topology:
1: Peridot Switch Module (8-port)
2: Peridot Switch Module (8-port)
3: Peridot Switch Module (8-port)
4: SFP Module
Hit any key to stop autoboot: 0
=> run sd_tftp_boot
neta@30000 Waiting for PHY auto negotiation to complete....... done
BOOTP broadcast 1
BOOTP broadcast 2
DHCP client bound to address 10.0.0.117 (254 ms)
Using neta@30000 device
TFTP from server 10.0.0.1; our IP address is 10.0.0.117
Filename 'mox/armada-3720-turris-mox.dtb'.
Load address: 0x4f00000
Loading: ####
1.5 MiB/s
done
Bytes transferred = 19479 (4c17 hex)
Using neta@30000 device
TFTP from server 10.0.0.1; our IP address is 10.0.0.117
Filename 'mox/Image'.
Load address: 0x5000000
Loading: #################################################################
##########################################
6 MiB/s
done
Bytes transferred = 54069760 (3390a00 hex)
## Flattened Device Tree blob at 04f00000
Booting using the fdt blob at 0x4f00000
Loading Device Tree to 000000003bf16000, end 000000003bf1dc16 ... OK
ERROR: board-specific fdt fixup failed: FDT_ERR_NOTFOUND
- must RESET the board to recover.
FDT creation failed! hanging...### ERROR ### Please RESET the board ###
On Thu, 19 Oct 2023 17:49:35 +0300
Vladimir Oltean <olteanv@gmail.com> wrote:
> On Thu, Oct 19, 2023 at 05:40:22PM +0300, Vladimir Oltean wrote:
> > +Marek
> >
> > On Wed, Oct 18, 2023 at 11:03:44AM +0200, Linus Walleij wrote:
> > > Fix some errors in the Marvell MV88E6xxx switch descriptions:
> > > - The top node had no address size or cells.
> > > - switch0@0 is not OK, should be switch@0.
> > >
> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > > ---
> > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > index 9eab2bb22134..c69cb4e191e5 100644
> > > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > @@ -305,7 +305,7 @@ phy1: ethernet-phy@1 {
> > > };
> > >
> > > /* switch nodes are enabled by U-Boot if modules are present */
> > > - switch0@10 {
> > > + switch@10 {
> >
> > As the comment says: U-Boot
> > (https://elixir.bootlin.com/u-boot/latest/source/board/CZ.NIC/turris_mox/turris_mox.c#L728)
> > sets up status = "okay" for these nodes depending on the MOXTET
> > configuration. It doesn't look as if it's doing that by alias, just by
> > path ("%s/switch%i@%x").
> >
> > I have a Turris MOX, please allow me some time to test if the node name
> > change is going to be significant and cause regressions. I expect the
> > answer to be yes (sadly).
>
> Yeah, it's bad.
>
> U-Boot 2018.11 (Dec 16 2018 - 12:50:19 +0000), Build: jenkins-turris-os-packages-kittens-mox-90
>
> DRAM: 1 GiB
> Enabling Armada 3720 wComphy-0: SGMII1 3.125 Gbps
> Comphy-1: PEX0 5 Gbps
> Comphy-2: USB3_HOST0 5 Gbps
> MMC: sdhci@d8000: 0
> Loading Environment from SPI Flash... SF: Detected w25q64dw with page size 256 Bytes, erase size 4 KiB, total 8 MiB
> OK
> Model: CZ.NIC Turris Mox Board
> Net: eth0: neta@30000
> Turris Mox:
> Board version: 22
> RAM size: 1024 MiB
> SD/eMMC version: SD
> Module Topology:
> 1: Peridot Switch Module (8-port)
> 2: Peridot Switch Module (8-port)
> 3: Peridot Switch Module (8-port)
> 4: SFP Module
>
> Hit any key to stop autoboot: 0
> => run sd_tftp_boot
> neta@30000 Waiting for PHY auto negotiation to complete....... done
> BOOTP broadcast 1
> BOOTP broadcast 2
> DHCP client bound to address 10.0.0.117 (254 ms)
> Using neta@30000 device
> TFTP from server 10.0.0.1; our IP address is 10.0.0.117
> Filename 'mox/armada-3720-turris-mox.dtb'.
> Load address: 0x4f00000
> Loading: ####
> 1.5 MiB/s
> done
> Bytes transferred = 19479 (4c17 hex)
> Using neta@30000 device
> TFTP from server 10.0.0.1; our IP address is 10.0.0.117
> Filename 'mox/Image'.
> Load address: 0x5000000
> Loading: #################################################################
> ##########################################
> 6 MiB/s
> done
> Bytes transferred = 54069760 (3390a00 hex)
> ## Flattened Device Tree blob at 04f00000
> Booting using the fdt blob at 0x4f00000
> Loading Device Tree to 000000003bf16000, end 000000003bf1dc16 ... OK
> ERROR: board-specific fdt fixup failed: FDT_ERR_NOTFOUND
> - must RESET the board to recover.
>
> FDT creation failed! hanging...### ERROR ### Please RESET the board ###
Yes, unfortunately changing that node name will break booting.
Maybe we could add a comment into the DTS to describe this unfortunate
state of things? :)
Marek
On Thu, Oct 19, 2023 at 05:26:49PM +0200, Marek Behún wrote:
> Yes, unfortunately changing that node name will break booting.
>
> Maybe we could add a comment into the DTS to describe this unfortunate
> state of things? :)
Well, the fact that Linus didn't notice means that there are insufficient
signals currently, so I guess a more explicit comment would help. Could
you prepare a patch?
On Thu, Oct 19, 2023 at 6:22 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> On Thu, Oct 19, 2023 at 05:26:49PM +0200, Marek Behún wrote:
> > Yes, unfortunately changing that node name will break booting.
> >
> > Maybe we could add a comment into the DTS to describe this unfortunate
> > state of things? :)
>
> Well, the fact that Linus didn't notice means that there are insufficient
> signals currently, so I guess a more explicit comment would help. Could
> you prepare a patch?
I can just include a blurb in my patch so we don't get colliding
changes.
Yours,
Linus Walleij
On Fri, Oct 20, 2023 at 02:59:43PM +0200, Linus Walleij wrote:
> On Thu, Oct 19, 2023 at 6:22 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > On Thu, Oct 19, 2023 at 05:26:49PM +0200, Marek Behún wrote:
> > > Yes, unfortunately changing that node name will break booting.
> > >
> > > Maybe we could add a comment into the DTS to describe this unfortunate
> > > state of things? :)
> >
> > Well, the fact that Linus didn't notice means that there are insufficient
> > signals currently, so I guess a more explicit comment would help. Could
> > you prepare a patch?
>
> I can just include a blurb in my patch so we don't get colliding
> changes.
Colliding with what, if you shouldn't change the node names?
@@ -145,10 +145,8 @@ &usb2 {
};
&mdio {
- switch0: switch0@1 {
+ switch0: switch@1 {
compatible = "marvell,mv88e6085";
- #address-cells = <1>;
- #size-cells = <0>;
reg = <1>;
dsa,member = <0 0>;
@@ -152,10 +152,8 @@ &uart0 {
};
&mdio {
- switch0: switch0@1 {
+ switch0: switch@1 {
compatible = "marvell,mv88e6085";
- #address-cells = <1>;
- #size-cells = <0>;
reg = <1>;
dsa,member = <0 0>;
@@ -305,7 +305,7 @@ phy1: ethernet-phy@1 {
};
/* switch nodes are enabled by U-Boot if modules are present */
- switch0@10 {
+ switch@10 {
compatible = "marvell,mv88e6190";
reg = <0x10>;
dsa,member = <0 0>;
@@ -430,7 +430,7 @@ port-sfp@a {
};
};
- switch0@2 {
+ switch@2 {
compatible = "marvell,mv88e6085";
reg = <0x2>;
dsa,member = <0 0>;
@@ -497,7 +497,7 @@ port@5 {
};
};
- switch1@11 {
+ switch@11 {
compatible = "marvell,mv88e6190";
reg = <0x11>;
dsa,member = <0 1>;
@@ -622,7 +622,7 @@ port-sfp@a {
};
};
- switch1@2 {
+ switch@2 {
compatible = "marvell,mv88e6085";
reg = <0x2>;
dsa,member = <0 1>;
@@ -689,7 +689,7 @@ port@5 {
};
};
- switch2@12 {
+ switch@12 {
compatible = "marvell,mv88e6190";
reg = <0x12>;
dsa,member = <0 2>;
@@ -805,7 +805,7 @@ port-sfp@a {
};
};
- switch2@2 {
+ switch@2 {
compatible = "marvell,mv88e6085";
reg = <0x2>;
dsa,member = <0 2>;
@@ -303,8 +303,6 @@ eth2phy: ethernet-phy@1 {
/* 88E6141 Topaz switch */
switch: switch@3 {
compatible = "marvell,mv88e6085";
- #address-cells = <1>;
- #size-cells = <0>;
reg = <3>;
pinctrl-names = "default";
@@ -497,7 +497,7 @@ ge_phy: ethernet-phy@0 {
reset-deassert-us = <10000>;
};
- switch0: switch0@4 {
+ switch0: switch@4 {
compatible = "marvell,mv88e6085";
reg = <4>;
pinctrl-names = "default";
@@ -207,11 +207,9 @@ phy0: ethernet-phy@0 {
reg = <0>;
};
- switch6: switch0@6 {
+ switch6: switch@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>;