| Message ID | 20231018-marvell-88e6152-wan-led-v4-1-3ee0c67383be@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Create a binding for the Marvell MV88E6xxx DSA switches | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
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
diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml index 6107189d276a..368169f7fd37 100644 --- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml @@ -46,4 +46,10 @@ $defs: $ref: dsa-port.yaml# unevaluatedProperties: false + oneOf: + - required: + - ports + - required: + - 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(+)