[net-next,v4,1/7] dt-bindings: net: dsa: Require ports or ethernet-ports

  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(+)
  

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.
  

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
  

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.
  

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
  

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!
  

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

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.
  

On Thu, Oct 19, 2023 at 01:28:27PM +0100, Russell King (Oracle) wrote:
> 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.

Sorry! I thought it would be less confusing to provide a diff that
resolved one issue only.
  

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

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.

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.
  

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
  

On 18/10/2023 13:11, 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

.........^ here

> +    - required:
> +      - ethernet-ports
> 
> Two spaces after the oneOf, 2 spaces after a required as usual.
> I don't get it.

Although YAML accepts your indentation, yamllint does not and we always,
always, expected yamllint flavor of syntax.


Best regards,
Krzysztof
  

On Fri, Oct 20, 2023 at 1:10 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 18/10/2023 13:11, 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:


> >> 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
>
> .........^ here
>
> > +    - required:
> > +      - ethernet-ports
> >
> > Two spaces after the oneOf, 2 spaces after a required as usual.
> > I don't get it.
>
> Although YAML accepts your indentation, yamllint does not and we always,
> always, expected yamllint flavor of syntax.

That's chill, however I can't reproduce this, make dt_bindings_check in the
mainline kernel does not yield this warning (after pip install
--upgrade --user dtschema
and yamllint is installed and all), so right now my only way of testing this
patch is to mail it to the mailing list and have it tested by Rob's bot.

I just don't understand what I'm supposed to do... drop the dash-space "- " in
front of "- ports"? Then the bot will be happy?

(This patch was added in response to Rob's comments
"this should probably be in dsa.yaml".)

We can also just drop the patch if this whole thing upsets the tooling, it's
just intended as a generalization of this requirement as can be seen in
Documentation/devicetree/bindings/net/dsa/qca8k.yaml
which in turn can do that because it is not using the generic def.

Yours,
Linus Walleij
  

On 20/10/2023 13:41, Linus Walleij wrote:
> On Fri, Oct 20, 2023 at 1:10 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 18/10/2023 13:11, 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:
> 
> 
>>>> 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
>>
>> .........^ here
>>
>>> +    - required:
>>> +      - ethernet-ports
>>>
>>> Two spaces after the oneOf, 2 spaces after a required as usual.
>>> I don't get it.
>>
>> Although YAML accepts your indentation, yamllint does not and we always,
>> always, expected yamllint flavor of syntax.
> 
> That's chill, however I can't reproduce this, make dt_bindings_check in the
> mainline kernel does not yield this warning (after pip install
> --upgrade --user dtschema
> and yamllint is installed and all), so right now my only way of testing this
> patch is to mail it to the mailing list and have it tested by Rob's bot.

Just tried on top of next/master and v6.5: correctly report this warning
from yamllint. Maybe you do not have yamlling installed or you have
Python mismatches (e.g. yamllint installed for Python2 but use Python3
here).

> 
> I just don't understand what I'm supposed to do... drop the dash-space "- " in
> front of "- ports"? Then the bot will be happy?

Missing two spaces, so 6 -> 8 spaces.

Best regards,
Krzysztof
  

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.
  

On Fri, Oct 20, 2023 at 2:27 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> 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.

Yep I added that.

(But I think the kernels dt_binding_check should ultimately add
the same flag, otherwise the world gets super confusing.)

Yours,
Linus Walleij