mbox series

[net-next,v4,0/7] Create a binding for the Marvell MV88E6xxx DSA switches

Message ID 20231018-marvell-88e6152-wan-led-v4-0-3ee0c67383be@linaro.org
Headers show
Series Create a binding for the Marvell MV88E6xxx DSA switches | expand

Message

Linus Walleij Oct. 18, 2023, 9:03 a.m. UTC
The Marvell switches are lacking DT bindings.

I need proper schema checking to add LED support to the
Marvell switch. Just how it is, it can't go on like this.

Some Device Tree fixes are included in the series, these
remove the major and most annoying warnings fallout noise:
some warnings remain, and these are of more serious nature,
such as missing phy-mode. They can be applied individually,
or to the networking tree with the rest of the patches.

This series requires Rob Herrings series
"dt-bindings: net: Child node schema cleanups" to be applied
first.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Changes in v4:
- Rebase the series on top of Rob's series
  "dt-bindings: net: Child node schema cleanups" (or the hex numbered
  ports will not work)
- Fix up a whitespacing error corrupting v3...
- Add a new patch making the generic DSA binding require ports or
  ethernet-ports in the switch node.
- Drop any corrections of port@a in the patches.
- Drop oneOf in the compatible enum for mv88e6xxx
- Use ethernet-switch, ethernet-ports and ethernet-phy in the examples
- Transclude the dsa.yaml#/$defs/ethernet-ports define for ports
- Move the DTS and binding fixes first, before the actual bindings,
  so they apply without (too many) warnings as fallout.
- Drop stray colon in text.
- Drop example port in the mveusb binding.
- Link to v3: https://lore.kernel.org/r/20231016-marvell-88e6152-wan-led-v3-0-38cd449dfb15@linaro.org

Changes in v3:
- Fix up a related mvusb example in a different binding that
  the scripts were complaining about.
- Fix up the wording on internal vs external MDIO buses in the
  mv88e6xxx binding document.
- Remove pointless label and put the right rev-mii into the
  MV88E6060 schema.
- Link to v2: https://lore.kernel.org/r/20231014-marvell-88e6152-wan-led-v2-0-7fca08b68849@linaro.org

Changes in v2:
- Break out a separate Marvell MV88E6060 binding file. I stand corrected.
- Drop the idea to rely on nodename mdio-external for the external
  MDIO bus, keep the compatible, drop patch for the driver.
- Fix more Marvell DT mistakes.
- Fix NXP DT mistakes in a separate patch.
- Fix Marvell ARM64 mistakes in a separate patch.
- Link to v1: https://lore.kernel.org/r/20231013-marvell-88e6152-wan-led-v1-0-0712ba99857c@linaro.org

---
Linus Walleij (7):
      dt-bindings: net: dsa: Require ports or ethernet-ports
      dt-bindings: net: mvusb: Fix up DSA example
      ARM: dts: marvell: Fix some common switch mistakes
      ARM: dts: nxp: Fix some common switch mistakes
      ARM64: dts: marvell: Fix some common switch mistakes
      dt-bindings: marvell: Rewrite MV88E6xxx in schema
      dt-bindings: marvell: Add Marvell MV88E6060 DSA schema

 Documentation/devicetree/bindings/net/dsa/dsa.yaml |   6 +
 .../bindings/net/dsa/marvell,mv88e6060.yaml        |  90 +++++++++
 .../bindings/net/dsa/marvell,mv88e6xxx.yaml        | 225 +++++++++++++++++++++
 .../devicetree/bindings/net/dsa/marvell.txt        | 109 ----------
 .../devicetree/bindings/net/marvell,mvusb.yaml     |   7 +-
 MAINTAINERS                                        |   3 +-
 arch/arm/boot/dts/marvell/armada-370-rd.dts        |   2 -
 .../dts/marvell/armada-381-netgear-gs110emx.dts    |   2 -
 .../dts/marvell/armada-385-clearfog-gtr-l8.dts     |   2 +-
 .../dts/marvell/armada-385-clearfog-gtr-s4.dts     |   2 +-
 arch/arm/boot/dts/marvell/armada-385-linksys.dtsi  |   2 -
 .../boot/dts/marvell/armada-385-turris-omnia.dts   |  16 +-
 arch/arm/boot/dts/marvell/armada-388-clearfog.dts  |   2 -
 .../boot/dts/marvell/armada-xp-linksys-mamba.dts   |   2 -
 arch/arm/boot/dts/nxp/vf/vf610-zii-cfu1.dts        |   2 +-
 arch/arm/boot/dts/nxp/vf/vf610-zii-scu4-aib.dts    |   8 +-
 arch/arm/boot/dts/nxp/vf/vf610-zii-spb4.dts        |   2 +-
 arch/arm/boot/dts/nxp/vf/vf610-zii-ssmb-dtu.dts    |   4 +-
 arch/arm/boot/dts/nxp/vf/vf610-zii-ssmb-spu3.dts   |   2 +-
 .../boot/dts/marvell/armada-3720-espressobin.dtsi  |   4 +-
 .../boot/dts/marvell/armada-3720-gl-mv1000.dts     |   4 +-
 .../boot/dts/marvell/armada-3720-turris-mox.dts    |  12 +-
 .../boot/dts/marvell/armada-7040-mochabin.dts      |   2 -
 .../dts/marvell/armada-8040-clearfog-gt-8k.dts     |   2 +-
 arch/arm64/boot/dts/marvell/cn9130-crb.dtsi        |   4 +-
 25 files changed, 356 insertions(+), 160 deletions(-)
---
base-commit: 1c9be5fea84e409542a186893d219bf7cff22f5a
change-id: 20231008-marvell-88e6152-wan-led-88c43b7fd2fd

Best regards,

Comments

Rob Herring Oct. 18, 2023, 10:32 a.m. UTC | #1
On Wed, 18 Oct 2023 11:03:40 +0200, Linus Walleij wrote:
> Bindings using dsa.yaml#/$defs/ethernet-ports specify that
> a DSA switch node need to have a ports or ethernet-ports
> subnode, and that is actually required, so add requirements
> using oneOf.
> 
> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  Documentation/devicetree/bindings/net/dsa/dsa.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/net/dsa/dsa.yaml:60:7: [warning] wrong indentation: expected 8 but found 6 (indentation)
./Documentation/devicetree/bindings/net/dsa/dsa.yaml:62:7: [warning] wrong indentation: expected 8 but found 6 (indentation)

dtschema/dtc warnings/errors:
Traceback (most recent call last):
  File "/usr/local/bin/dt-doc-validate", line 64, in <module>
    ret |= check_doc(f)
           ^^^^^^^^^^^^
  File "/usr/local/bin/dt-doc-validate", line 32, in check_doc
    for error in sorted(dtsch.iter_errors(), key=lambda e: e.linecol):
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/dtschema/schema.py", line 125, in iter_errors
    self.annotate_error(scherr, meta_schema, scherr.schema_path)
  File "/usr/local/lib/python3.11/dist-packages/dtschema/schema.py", line 104, in annotate_error
    schema = schema[p]
             ~~~~~~^^^
KeyError: 'type'

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231018-marvell-88e6152-wan-led-v4-1-3ee0c67383be@linaro.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Linus Walleij Oct. 18, 2023, 11:11 a.m. UTC | #2
On Wed, Oct 18, 2023 at 12:32 PM Rob Herring <robh@kernel.org> wrote:
> On Wed, 18 Oct 2023 11:03:40 +0200, Linus Walleij wrote:

> > Bindings using dsa.yaml#/$defs/ethernet-ports specify that
> > a DSA switch node need to have a ports or ethernet-ports
> > subnode, and that is actually required, so add requirements
> > using oneOf.
> >
> > Suggested-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/net/dsa/dsa.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/net/dsa/dsa.yaml:60:7: [warning] wrong indentation: expected 8 but found 6 (indentation)
> ./Documentation/devicetree/bindings/net/dsa/dsa.yaml:62:7: [warning] wrong indentation: expected 8 but found 6 (indentation)

Really?

+  oneOf:
+    - required:
+      - ports
+    - required:
+      - ethernet-ports

Two spaces after the oneOf, 2 spaces after a required as usual.
I don't get it.

Yours,
Linus Walleij
Andrew Lunn Oct. 18, 2023, 1:43 p.m. UTC | #3
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
Conor Dooley Oct. 19, 2023, 11:58 a.m. UTC | #4
On Thu, Oct 19, 2023 at 11:58:49AM +0100, Russell King (Oracle) wrote:
> On Wed, Oct 18, 2023 at 01:11:45PM +0200, Linus Walleij wrote:
> > On Wed, Oct 18, 2023 at 12:32 PM Rob Herring <robh@kernel.org> wrote:
> > > On Wed, 18 Oct 2023 11:03:40 +0200, Linus Walleij wrote:
> > 
> > > > Bindings using dsa.yaml#/$defs/ethernet-ports specify that
> > > > a DSA switch node need to have a ports or ethernet-ports
> > > > subnode, and that is actually required, so add requirements
> > > > using oneOf.
> > > >
> > > > Suggested-by: Rob Herring <robh@kernel.org>
> > > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/net/dsa/dsa.yaml | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > >
> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > >
> > > yamllint warnings/errors:
> > > ./Documentation/devicetree/bindings/net/dsa/dsa.yaml:60:7: [warning] wrong indentation: expected 8 but found 6 (indentation)
> > > ./Documentation/devicetree/bindings/net/dsa/dsa.yaml:62:7: [warning] wrong indentation: expected 8 but found 6 (indentation)
> > 
> > Really?
> > 
> > +  oneOf:
> > +    - required:
> > +      - ports
> > +    - required:
> > +      - ethernet-ports
> > 
> > Two spaces after the oneOf, 2 spaces after a required as usual.
> > I don't get it.
> 
> Given the other python errors spat out in Rob's report, I would suggest
> that the "bot" is running a development version that hasn't been fully
> tested, so anything it spits out is suspect. Maybe Rob can comment on
> the validity of the warnings in the report.

In this case, I think it is correct.
2 spaces for the oneOf, 2 spaces the start of the required for the
nested list, so:
oneOf:
  - required:
      - ports
  - required:
      - ethernet-ports
Russell King (Oracle) Oct. 19, 2023, 12:28 p.m. UTC | #5
On Thu, Oct 19, 2023 at 01:03:41PM +0100, Conor Dooley wrote:
> On Wed, Oct 18, 2023 at 05:32:48AM -0500, Rob Herring wrote:
> > 
> > On Wed, 18 Oct 2023 11:03:40 +0200, Linus Walleij wrote:
> > > Bindings using dsa.yaml#/$defs/ethernet-ports specify that
> > > a DSA switch node need to have a ports or ethernet-ports
> > > subnode, and that is actually required, so add requirements
> > > using oneOf.
> > > 
> > > Suggested-by: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > > ---
> > >  Documentation/devicetree/bindings/net/dsa/dsa.yaml | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > 
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > 
> > yamllint warnings/errors:
> > ./Documentation/devicetree/bindings/net/dsa/dsa.yaml:60:7: [warning] wrong indentation: expected 8 but found 6 (indentation)
> > ./Documentation/devicetree/bindings/net/dsa/dsa.yaml:62:7: [warning] wrong indentation: expected 8 but found 6 (indentation)
> > 
> > dtschema/dtc warnings/errors:
> > Traceback (most recent call last):
> >   File "/usr/local/bin/dt-doc-validate", line 64, in <module>
> >     ret |= check_doc(f)
> >            ^^^^^^^^^^^^
> >   File "/usr/local/bin/dt-doc-validate", line 32, in check_doc
> >     for error in sorted(dtsch.iter_errors(), key=lambda e: e.linecol):
> >                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/usr/local/lib/python3.11/dist-packages/dtschema/schema.py", line 125, in iter_errors
> >     self.annotate_error(scherr, meta_schema, scherr.schema_path)
> >   File "/usr/local/lib/python3.11/dist-packages/dtschema/schema.py", line 104, in annotate_error
> >     schema = schema[p]
> >              ~~~~~~^^^
> > KeyError: 'type'
> 
> Locally, on an older version of dt-schema, I see
> /stuff/linux-dt/Documentation/devicetree/bindings/net/dsa/dsa.yaml: $defs: 'oneOf' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
> 	hint: A json-schema keyword was found in $defs key.
> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
> /stuff/linux-dt/Documentation/devicetree/bindings/net/dsa/dsa.yaml: $defs:oneOf: [{'required': ['ports']}, {'required': ['ethernet-ports']}] is not of type 'object'
> 	hint: $defs entries must contain schemas
> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> 
> On the latest version I see the error from the bot.
> 
> Doing 
> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
> index bd6948e4fd9e..25e5950d51ae 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
> @@ -55,10 +55,10 @@ $defs:
>              $ref: dsa-port.yaml#
>              unevaluatedProperties: false
>  
> -  oneOf:
> -    - required:
> -      - ports
> -    - required:
> -      - ethernet-ports
> +oneOf:
> +  - required:
> +    - ports
> +  - required:
> +    - ethernet-ports
>  
>  ...
> 
> resolves both issues, but the older version of dt-schema definitely had
> better error reporting in this case!

And now I'm even more confused... your example in your other reply had
six spaces before "- ports" and "- ethernet-ports" but here you're
using four spaces.
Rob Herring Oct. 19, 2023, 1:41 p.m. UTC | #6
On Thu, Oct 19, 2023 at 01:46:36PM +0100, Conor Dooley wrote:
> On Thu, Oct 19, 2023 at 01:27:09PM +0100, Russell King (Oracle) wrote:
> > On Thu, Oct 19, 2023 at 12:58:46PM +0100, Conor Dooley wrote:
> > > On Thu, Oct 19, 2023 at 11:58:49AM +0100, Russell King (Oracle) wrote:
> > > > On Wed, Oct 18, 2023 at 01:11:45PM +0200, Linus Walleij wrote:
> > > > > On Wed, Oct 18, 2023 at 12:32 PM Rob Herring <robh@kernel.org> wrote:
> > > > > > On Wed, 18 Oct 2023 11:03:40 +0200, Linus Walleij wrote:
> > > > > 
> > > > > > > Bindings using dsa.yaml#/$defs/ethernet-ports specify that
> > > > > > > a DSA switch node need to have a ports or ethernet-ports
> > > > > > > subnode, and that is actually required, so add requirements
> > > > > > > using oneOf.
> > > > > > >
> > > > > > > Suggested-by: Rob Herring <robh@kernel.org>
> > > > > > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > > > > > > ---
> > > > > > >  Documentation/devicetree/bindings/net/dsa/dsa.yaml | 6 ++++++
> > > > > > >  1 file changed, 6 insertions(+)
> > > > > > >
> > > > > >
> > > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > > > > > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > > > > >
> > > > > > yamllint warnings/errors:
> > > > > > ./Documentation/devicetree/bindings/net/dsa/dsa.yaml:60:7: [warning] wrong indentation: expected 8 but found 6 (indentation)
> > > > > > ./Documentation/devicetree/bindings/net/dsa/dsa.yaml:62:7: [warning] wrong indentation: expected 8 but found 6 (indentation)
> > > > > 
> > > > > Really?
> > > > > 
> > > > > +  oneOf:
> > > > > +    - required:
> > > > > +      - ports
> > > > > +    - required:
> > > > > +      - ethernet-ports
> > > > > 
> > > > > Two spaces after the oneOf, 2 spaces after a required as usual.
> > > > > I don't get it.

Either way is valid. It's just 2 different common styles and I picked 
the other way. The reason is to look different for a sequence vs. 
mapping:

- required:
  - ethernet-ports

- required:
    ethernet-ports

It's easy to miss the missing '-'.


> > > > Given the other python errors spat out in Rob's report, I would suggest
> > > > that the "bot" is running a development version that hasn't been fully
> > > > tested, so anything it spits out is suspect. Maybe Rob can comment on
> > > > the validity of the warnings in the report.
> > > 
> > > In this case, I think it is correct.
> > > 2 spaces for the oneOf, 2 spaces the start of the required for the
> > > nested list, so:
> > > oneOf:
> > >   - required:
> > 
> > This is a total of two spaces indentation.
> > 
> > >       - ports
> > 
> > This is a total of six spaces indentation.
> > 
> > You mention 2 spaces for the oneOf, which explains why the "- required"
> > needs to be indented by two spaces. You also say 2 spaces for the
> > required nested list, but what about the other two spaces?
> 
> I a word that might've made it more clear.
> It is 2 spaces for the oneOf and 2 spaces _from_ the start of the
> required for the nested list.

Yes, 'oneOf' here is not a json-schema keyword, but a key under $defs 
because it is indented. 

'$defs' entries must be a schema/dict/mapping (json-schema/python/yaml 
terms). 

> 
> In theory you might have a contrived example that looks like:
> 
> oneOf:
>   - required:
>       - ports
>     properties:
>       ethernet-ports: false
> 
>   - required:
>       - ethernet-ports
>     properties:
>       ports: false
> 
> Maybe with that example you can see that each option of the oneOf
> contains a `required` and a `properties` component at 4 spaces of
> indent, and then in turn the required properties, being sub-components
> of `required` grow 2 more spaces for 6.
> 
> > I guess if you're a YAML expert, this all makes sense, but to those of
> > us who aren't, these quirky "features" of it just seem totally
> > illogical.

Indentation being significant is not quirky. Languages choose either 
indentation or brackets of some form. YAML uses one and JSON uses the 
other.

> If I were a yaml expert, I would probably be able to use the correct
> terminology to explain this better, but hopefully the example is useful.

It has little to do with YAML other than indentation is *very* 
significant in YAML. It's actually valid YAML. It's probably valid 
json-schema, but questionable use in terms of how $defs is typically 
used.

Anyways, I'm working on a fix for the meta-schema.

Rob
Vladimir Oltean Oct. 19, 2023, 2:29 p.m. UTC | #7
On Thu, Oct 19, 2023 at 12:04:46PM +0100, Russell King (Oracle) wrote:
> On Wed, Oct 18, 2023 at 11:03:42AM +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.
> > - The ports node should have port@0 etc children, no
> >   plural "ports".
> > 
> > This serves as an example of fixes needed for introducing a
> > schema for the bindings, but the patch can simply be applied.
> 
> In patch 2, you mention that things should be named ethernet-switch and
> ethernet-port. As you're renaming the nodes in this patch, wouldn't it
> make sense to use those names instead now, rather than at some point in
> the future a patch that converts to these names?

I agree, and I was wondering the same thing.
Vladimir Oltean Oct. 19, 2023, 2:49 p.m. UTC | #8
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 ###
Vladimir Oltean Oct. 19, 2023, 4:11 p.m. UTC | #9
On Wed, Oct 18, 2023 at 11:03:46AM +0200, Linus Walleij wrote:
> The Marvell MV88E6060 is one of the oldest DSA switches from
> Marvell, and it has DT bindings used in the wild. Let's define
> them properly.
> 
> It is different enough from the rest of the MV88E6xxx switches
> that it deserves its own binding.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../bindings/net/dsa/marvell,mv88e6060.yaml        | 90 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 91 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell,mv88e6060.yaml b/Documentation/devicetree/bindings/net/dsa/marvell,mv88e6060.yaml
> new file mode 100644
> index 000000000000..787f328551f6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/marvell,mv88e6060.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/marvell,mv88e6060.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell MV88E6060 DSA switch
> +
> +maintainers:
> +  - Andrew Lunn <andrew@lunn.ch>
> +
> +description:
> +  The Marvell MV88E6060 switch has been produced and sold by Marvell
> +  since at least 2010. The switch has one pin ADDR4 that controls the
> +  MDIO address of the switch to be 0x10 or 0x00, and on the MDIO bus
> +  connected to the switch, the PHYs inside the switch appear as
> +  independent devices on address 0x00-0x04 or 0x10-0x14, so in difference
> +  from many other DSA switches this switch does not have an internal
> +  MDIO bus for the PHY devices.

Where does 2010 come from (both here and in the other Marvell schema)?
Lennert Buytenhek added Linux support for this switch family in 2008.

Anyway,

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Vladimir Oltean Oct. 20, 2023, 12:27 p.m. UTC | #10
On Fri, Oct 20, 2023 at 01:41:22PM +0200, Linus Walleij wrote:
> I can't reproduce this, make dt_bindings_check in the mainline kernel
> does not yield this warning

You used the actual command that the bot posted, right? aka "make DT_CHECKER_FLAGS=-m dt_binding_check"?
I am also seeing the yamllint warning.
Linus Walleij Oct. 20, 2023, 12:59 p.m. UTC | #11
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