| Message ID | 20231018-marvell-88e6152-wan-led-v4-5-3ee0c67383be@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Create a binding for the Marvell MV88E6xxx DSA switches | expand |
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?
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi index 5fc613d24151..b526efeee293 100644 --- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi +++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dtsi @@ -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>; diff --git a/arch/arm64/boot/dts/marvell/armada-3720-gl-mv1000.dts b/arch/arm64/boot/dts/marvell/armada-3720-gl-mv1000.dts index b1b45b4fa9d4..5de4417f929c 100644 --- a/arch/arm64/boot/dts/marvell/armada-3720-gl-mv1000.dts +++ b/arch/arm64/boot/dts/marvell/armada-3720-gl-mv1000.dts @@ -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>; 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 { 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>; diff --git a/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts b/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts index 48202810bf78..3cc794fcf12e 100644 --- a/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts +++ b/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts @@ -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"; diff --git a/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts b/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts index 4125202028c8..7a25ea36b565 100644 --- a/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts +++ b/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts @@ -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"; diff --git a/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi b/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi index 32cfb3e2efc3..110d4c9898bc 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@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>;
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(-)