| Message ID | 20221210033033.662553-5-colin.foster@in-advantage.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | dt-binding preparation for ocelot switches | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 30 of 30 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 115 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On 10.12.2022 06:30, Colin Foster wrote: > DSA switches can fall into one of two categories: switches where all ports > follow standard '(ethernet-)?port' properties, and switches that have > additional properties for the ports. > > The scenario where DSA ports are all standardized can be handled by > switches with a reference to the new 'dsa.yaml#/$defs/ethernet-ports'. > > The scenario where DSA ports require additional properties can reference > '$dsa.yaml#' directly. This will allow switches to reference these standard > definitions of the DSA switch, but add additional properties under the port > nodes. > > Suggested-by: Rob Herring <robh@kernel.org> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > Reviewed-by: Rob Herring <robh@kernel.org> > Acked-by: Alvin Šipraga <alsi@bang-olufsen.dk> # realtek > Acked-by: Arınç ÜNAL <arinc.unal@arinc9.com> > --- > > v4 -> v5 > * Add Rob Reviewed, Arınç Acked > * Defer the removal of "^(ethernet-)?switch(@.*)?$" in dsa.yaml until a > later patch > * Undo the move of ethernet switch ports description in mediatek,mt7530.yaml > * Fix typos in commit message > > v3 -> v4 > * Rename "$defs/base" to "$defs/ethernet-ports" to avoid implication of a > "base class" and fix commit message accordingly > * Add the following to the common etherent-ports node: > "additionalProperties: false" > "#address-cells" property > "#size-cells" property > * Fix "etherenet-ports@[0-9]+" to correctly be "ethernet-port@[0-9]+" > * Remove unnecessary newline > * Apply changes to mediatek,mt7530.yaml that were previously in a separate patch > * Add Reviewed and Acked tags > > v3 > * New patch > > --- > .../bindings/net/dsa/arrow,xrs700x.yaml | 2 +- > .../devicetree/bindings/net/dsa/brcm,b53.yaml | 2 +- > .../devicetree/bindings/net/dsa/dsa.yaml | 22 +++++++++++++++++++ > .../net/dsa/hirschmann,hellcreek.yaml | 2 +- > .../bindings/net/dsa/mediatek,mt7530.yaml | 5 +---- > .../bindings/net/dsa/microchip,ksz.yaml | 2 +- > .../bindings/net/dsa/microchip,lan937x.yaml | 2 +- > .../bindings/net/dsa/mscc,ocelot.yaml | 2 +- > .../bindings/net/dsa/nxp,sja1105.yaml | 2 +- > .../devicetree/bindings/net/dsa/realtek.yaml | 2 +- > .../bindings/net/dsa/renesas,rzn1-a5psw.yaml | 2 +- > 11 files changed, 32 insertions(+), 13 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml b/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml > index 259a0c6547f3..5888e3a0169a 100644 > --- a/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml > @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > title: Arrow SpeedChips XRS7000 Series Switch Device Tree Bindings > > allOf: > - - $ref: dsa.yaml# > + - $ref: dsa.yaml#/$defs/ethernet-ports > > maintainers: > - George McCollister <george.mccollister@gmail.com> > diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml > index 1219b830b1a4..5bef4128d175 100644 > --- a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml > @@ -66,7 +66,7 @@ required: > - reg > > allOf: > - - $ref: dsa.yaml# > + - $ref: dsa.yaml#/$defs/ethernet-ports > - if: > properties: > compatible: > diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > index 5efc0ee8edcb..9375cdcfbf96 100644 > --- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > @@ -58,4 +58,26 @@ oneOf: > > additionalProperties: true > > +$defs: > + ethernet-ports: > + description: A DSA switch without any extra port properties > + $ref: '#/' > + > + patternProperties: > + "^(ethernet-)?ports$": > + type: object > + additionalProperties: false > + > + properties: > + '#address-cells': > + const: 1 > + '#size-cells': > + const: 0 > + > + patternProperties: > + "^(ethernet-)?port@[0-9]+$": > + description: Ethernet switch ports > + $ref: dsa-port.yaml# > + unevaluatedProperties: false I've got moderate experience in json-schema but shouldn't you put 'type: object' here like you did for "^(ethernet-)?ports$"? > + > ... > diff --git a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml > index 73b774eadd0b..748ef9983ce2 100644 > --- a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml > @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > title: Hirschmann Hellcreek TSN Switch Device Tree Bindings > > allOf: > - - $ref: dsa.yaml# > + - $ref: dsa.yaml#/$defs/ethernet-ports > > maintainers: > - Andrew Lunn <andrew@lunn.ch> > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > index f2e9ff3f580b..20312f5d1944 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > @@ -157,9 +157,6 @@ patternProperties: > patternProperties: > "^(ethernet-)?port@[0-9]+$": > type: object This line was being removed on the previous version. Must be related to above. > - description: Ethernet switch ports > - > - unevaluatedProperties: false > > properties: > reg: Arınç
Hi Arınç, On Sat, Dec 10, 2022 at 07:24:42PM +0300, Arınç ÜNAL wrote: > On 10.12.2022 06:30, Colin Foster wrote: > > DSA a/Documentation/devicetree/bindings/net/dsa/dsa.yaml > > +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > > @@ -58,4 +58,26 @@ oneOf: > > additionalProperties: true > > +$defs: > > + ethernet-ports: > > + description: A DSA switch without any extra port properties > > + $ref: '#/' > > + > > + patternProperties: > > + "^(ethernet-)?ports$": > > + type: object > > + additionalProperties: false > > + > > + properties: > > + '#address-cells': > > + const: 1 > > + '#size-cells': > > + const: 0 > > + > > + patternProperties: > > + "^(ethernet-)?port@[0-9]+$": > > + description: Ethernet switch ports > > + $ref: dsa-port.yaml# > > + unevaluatedProperties: false > > I've got moderate experience in json-schema but shouldn't you put 'type: > object' here like you did for "^(ethernet-)?ports$"? I can't say for sure, but adding "type: object" here and removing it from mediatek,mt7530.yaml still causes the same issue I mention below. Rob's initial suggestion for this patch set (which was basically the entire implementation... many thanks again Rob) can be found here: https://lore.kernel.org/netdev/20221104200212.GA2315642-robh@kernel.org/ From what I can tell, the omission of "type: object" here was intentional. At the very least, it doesn't seem to have any effect on warnings. > > > + > > ... > > diff --git a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml > > index 73b774eadd0b..748ef9983ce2 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml > > +++ b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml > > @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > > title: Hirschmann Hellcreek TSN Switch Device Tree Bindings > > allOf: > > - - $ref: dsa.yaml# > > + - $ref: dsa.yaml#/$defs/ethernet-ports > > maintainers: > > - Andrew Lunn <andrew@lunn.ch> > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > index f2e9ff3f580b..20312f5d1944 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > @@ -157,9 +157,6 @@ patternProperties: > > patternProperties: > > "^(ethernet-)?port@[0-9]+$": > > type: object > > This line was being removed on the previous version. Must be related to > above. Without the 'object' type here, I get the following warning: Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml: patternProperties:^(ethernet-)?ports$:patternProperties:^(ethernet-)?port@[0-9]+$: 'anyOf' conditional failed, one must be fixed: 'type' is a required property '$ref' is a required property hint: node schemas must have a type or $ref from schema $id: http://devicetree.org/meta-schemas/core.yaml# ./Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml SCHEMA Documentation/devicetree/bindings/processed-schema.json /home/colin/src/work/linux_vsc/linux-imx/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml: ignoring, error in schema: patternProperties: ^(ethernet-)?ports$: patternProperties: ^(ethernet-)?port@[0-9]+$ I'm testing this now and I'm noticing something is going on with the "ref: dsa-port.yaml" Everything seems to work fine (in that I don't see any warnings) when I have this diff: diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml index 20312f5d1944..db0122020f98 100644 --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yam @@ -156,8 +156,7 @@ patternProperties: patternProperties: "^(ethernet-)?port@[0-9]+$": - type: object - + $ref: dsa-port.yaml# properties: reg: description: @@ -165,7 +164,6 @@ patternProperties: for user ports. allOf: - - $ref: dsa-port.yaml# - if: required: [ ethernet ] then: This one has me [still] scratching my head... > > > - description: Ethernet switch ports > > - > > - unevaluatedProperties: false > > properties: > > reg: > > Arınç
On 10.12.2022 21:02, Colin Foster wrote: > Hi Arınç, > On Sat, Dec 10, 2022 at 07:24:42PM +0300, Arınç ÜNAL wrote: >> On 10.12.2022 06:30, Colin Foster wrote: >>> DSA a/Documentation/devicetree/bindings/net/dsa/dsa.yaml >>> +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml >>> @@ -58,4 +58,26 @@ oneOf: >>> additionalProperties: true >>> +$defs: >>> + ethernet-ports: >>> + description: A DSA switch without any extra port properties >>> + $ref: '#/' >>> + >>> + patternProperties: >>> + "^(ethernet-)?ports$": >>> + type: object >>> + additionalProperties: false >>> + >>> + properties: >>> + '#address-cells': >>> + const: 1 >>> + '#size-cells': >>> + const: 0 >>> + >>> + patternProperties: >>> + "^(ethernet-)?port@[0-9]+$": >>> + description: Ethernet switch ports >>> + $ref: dsa-port.yaml# >>> + unevaluatedProperties: false >> >> I've got moderate experience in json-schema but shouldn't you put 'type: >> object' here like you did for "^(ethernet-)?ports$"? > > I can't say for sure, but adding "type: object" here and removing it > from mediatek,mt7530.yaml still causes the same issue I mention below. > > Rob's initial suggestion for this patch set (which was basically the > entire implementation... many thanks again Rob) can be found here: > https://lore.kernel.org/netdev/20221104200212.GA2315642-robh@kernel.org/ > > From what I can tell, the omission of "type: object" here was > intentional. At the very least, it doesn't seem to have any effect on > warnings. > >> >>> + >>> ... >>> diff --git a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml >>> index 73b774eadd0b..748ef9983ce2 100644 >>> --- a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml >>> +++ b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml >>> @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# >>> title: Hirschmann Hellcreek TSN Switch Device Tree Bindings >>> allOf: >>> - - $ref: dsa.yaml# >>> + - $ref: dsa.yaml#/$defs/ethernet-ports >>> maintainers: >>> - Andrew Lunn <andrew@lunn.ch> >>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> index f2e9ff3f580b..20312f5d1944 100644 >>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> @@ -157,9 +157,6 @@ patternProperties: >>> patternProperties: >>> "^(ethernet-)?port@[0-9]+$": >>> type: object >> >> This line was being removed on the previous version. Must be related to >> above. > > Without the 'object' type here, I get the following warning: > > Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml: patternProperties:^(ethernet-)?ports$:patternProperties:^(ethernet-)?port@[0-9]+$: 'anyOf' conditional failed, one must be fixed: > 'type' is a required property > '$ref' is a required property > hint: node schemas must have a type or $ref > from schema $id: http://devicetree.org/meta-schemas/core.yaml# > ./Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml > SCHEMA Documentation/devicetree/bindings/processed-schema.json > /home/colin/src/work/linux_vsc/linux-imx/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml: ignoring, error in schema: patternProperties: ^(ethernet-)?ports$: patternProperties: ^(ethernet-)?port@[0-9]+$ > > > I'm testing this now and I'm noticing something is going on with the > "ref: dsa-port.yaml" > > > Everything seems to work fine (in that I don't see any warnings) when I > have this diff: > > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > index 20312f5d1944..db0122020f98 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yam > @@ -156,8 +156,7 @@ patternProperties: > > patternProperties: > "^(ethernet-)?port@[0-9]+$": > - type: object > - > + $ref: dsa-port.yaml# > properties: > reg: > description: > @@ -165,7 +164,6 @@ patternProperties: > for user ports. > > allOf: > - - $ref: dsa-port.yaml# > - if: > required: [ ethernet ] > then: > > > > This one has me [still] scratching my head... Right there with you. In addition to this, having or deleting type object on/from "^(ethernet-)?ports$" and "^(ethernet-)?port@[0-9]+$" on dsa.yaml doesn't cause any warnings (checked with make dt_binding_check DT_SCHEMA_FILES=net/dsa) which makes me question why it's there in the first place. Arınç
On Mon, Dec 12, 2022 at 12:28:06PM +0300, Arınç ÜNAL wrote: > On 10.12.2022 21:02, Colin Foster wrote: > > Hi Arınç, > > On Sat, Dec 10, 2022 at 07:24:42PM +0300, Arınç ÜNAL wrote: > > > On 10.12.2022 06:30, Colin Foster wrote: > > > > DSA a/Documentation/devicetree/bindings/net/dsa/dsa.yaml > > > > +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > > > > @@ -58,4 +58,26 @@ oneOf: > > > > additionalProperties: true > > > > +$defs: > > > > + ethernet-ports: > > > > + description: A DSA switch without any extra port properties > > > > + $ref: '#/' > > > > + > > > > + patternProperties: > > > > + "^(ethernet-)?ports$": > > > > + type: object > > > > + additionalProperties: false > > > > + > > > > + properties: > > > > + '#address-cells': > > > > + const: 1 > > > > + '#size-cells': > > > > + const: 0 > > > > + > > > > + patternProperties: > > > > + "^(ethernet-)?port@[0-9]+$": > > > > + description: Ethernet switch ports > > > > + $ref: dsa-port.yaml# > > > > + unevaluatedProperties: false > > > > > > I've got moderate experience in json-schema but shouldn't you put 'type: > > > object' here like you did for "^(ethernet-)?ports$"? > > > > I can't say for sure, but adding "type: object" here and removing it > > from mediatek,mt7530.yaml still causes the same issue I mention below. > > > > Rob's initial suggestion for this patch set (which was basically the > > entire implementation... many thanks again Rob) can be found here: > > https://lore.kernel.org/netdev/20221104200212.GA2315642-robh@kernel.org/ > > > > From what I can tell, the omission of "type: object" here was > > intentional. At the very least, it doesn't seem to have any effect on > > warnings. > > > > > > > > > + > > > > ... > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml > > > > index 73b774eadd0b..748ef9983ce2 100644 > > > > --- a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml > > > > +++ b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml > > > > @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > > > > title: Hirschmann Hellcreek TSN Switch Device Tree Bindings > > > > allOf: > > > > - - $ref: dsa.yaml# > > > > + - $ref: dsa.yaml#/$defs/ethernet-ports > > > > maintainers: > > > > - Andrew Lunn <andrew@lunn.ch> > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > > > index f2e9ff3f580b..20312f5d1944 100644 > > > > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > > > @@ -157,9 +157,6 @@ patternProperties: > > > > patternProperties: > > > > "^(ethernet-)?port@[0-9]+$": > > > > type: object > > > > > > This line was being removed on the previous version. Must be related to > > > above. > > > > Without the 'object' type here, I get the following warning: > > > > Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml: patternProperties:^(ethernet-)?ports$:patternProperties:^(ethernet-)?port@[0-9]+$: 'anyOf' conditional failed, one must be fixed: > > 'type' is a required property > > '$ref' is a required property > > hint: node schemas must have a type or $ref > > from schema $id: http://devicetree.org/meta-schemas/core.yaml# > > ./Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml > > SCHEMA Documentation/devicetree/bindings/processed-schema.json > > /home/colin/src/work/linux_vsc/linux-imx/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml: ignoring, error in schema: patternProperties: ^(ethernet-)?ports$: patternProperties: ^(ethernet-)?port@[0-9]+$ Is the above warning not clear? We require either 'type' or a $ref to define nodes as json-schema objects. > > I'm testing this now and I'm noticing something is going on with the > > "ref: dsa-port.yaml" > > > > > > Everything seems to work fine (in that I don't see any warnings) when I > > have this diff: > > > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > index 20312f5d1944..db0122020f98 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yam > > @@ -156,8 +156,7 @@ patternProperties: > > > > patternProperties: > > "^(ethernet-)?port@[0-9]+$": > > - type: object > > - > > + $ref: dsa-port.yaml# > > properties: > > reg: > > description: > > @@ -165,7 +164,6 @@ patternProperties: > > for user ports. > > > > allOf: > > - - $ref: dsa-port.yaml# > > - if: > > required: [ ethernet ] > > then: > > > > > > > > This one has me [still] scratching my head... > > Right there with you. In addition to this, having or deleting type object > on/from "^(ethernet-)?ports$" and "^(ethernet-)?port@[0-9]+$" on dsa.yaml > doesn't cause any warnings (checked with make dt_binding_check > DT_SCHEMA_FILES=net/dsa) which makes me question why it's there in the first > place. That check probably doesn't consider an ref being under an 'allOf'. Perhaps what is missing in understanding is every schema at the top-level has an implicit 'type: object'. But nothing is ever implicit in json-schema which will silently ignore keywords which don't make sense for an instance type. Instead of a bunch of boilerplate, the processed schema has 'type' added in lots of cases such as this one. Rob
On Fri, Dec 09, 2022 at 07:30:27PM -0800, Colin Foster wrote: > DSA switches can fall into one of two categories: switches where all ports > follow standard '(ethernet-)?port' properties, and switches that have > additional properties for the ports. > > The scenario where DSA ports are all standardized can be handled by > switches with a reference to the new 'dsa.yaml#/$defs/ethernet-ports'. > > The scenario where DSA ports require additional properties can reference > '$dsa.yaml#' directly. This will allow switches to reference these standard > definitions of the DSA switch, but add additional properties under the port > nodes. > > Suggested-by: Rob Herring <robh@kernel.org> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > Reviewed-by: Rob Herring <robh@kernel.org> > Acked-by: Alvin Šipraga <alsi@bang-olufsen.dk> # realtek > Acked-by: Arınç ÜNAL <arinc.unal@arinc9.com> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Hi Rob, Arınç, On Mon, Dec 12, 2022 at 10:51:47AM -0600, Rob Herring wrote: > On Mon, Dec 12, 2022 at 12:28:06PM +0300, Arınç ÜNAL wrote: > > On 10.12.2022 21:02, Colin Foster wrote: > > > Hi Arınç, > > > On Sat, Dec 10, 2022 at 07:24:42PM +0300, Arınç ÜNAL wrote: > > > > On 10.12.2022 06:30, Colin Foster wrote: > > > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yam > > > @@ -156,8 +156,7 @@ patternProperties: > > > > > > patternProperties: > > > "^(ethernet-)?port@[0-9]+$": > > > - type: object > > > - > > > + $ref: dsa-port.yaml# > > > properties: > > > reg: > > > description: > > > @@ -165,7 +164,6 @@ patternProperties: > > > for user ports. > > > > > > allOf: > > > - - $ref: dsa-port.yaml# > > > - if: > > > required: [ ethernet ] > > > then: > > > > > > > > > > > > This one has me [still] scratching my head... > > > > Right there with you. In addition to this, having or deleting type object > > on/from "^(ethernet-)?ports$" and "^(ethernet-)?port@[0-9]+$" on dsa.yaml > > doesn't cause any warnings (checked with make dt_binding_check > > DT_SCHEMA_FILES=net/dsa) which makes me question why it's there in the first > > place. > > > That check probably doesn't consider an ref being under an 'allOf'. > Perhaps what is missing in understanding is every schema at the > top-level has an implicit 'type: object'. But nothing is ever implicit > in json-schema which will silently ignore keywords which don't make > sense for an instance type. Instead of a bunch of boilerplate, the > processed schema has 'type' added in lots of cases such as this one. > > Rob What do you suggest on this set? I think this is the only outstanding issue, and Jakub brought up the possibility of applying end of today (maybe 5-6 hours from now in the US). It seems like there's an issue with the dt_bindings_check that causes the "allOf: $ref" to throw a warning when it shouldn't. So removing the "type: object" was supposed to be correct, but throws warnings. It seems like keeping this patch as-is and updating it when the check gets fixed might be an acceptable path, but I'd understand if you disagree and prefer a resubmission.
On Mon, Dec 12, 2022 at 10:55:19AM -0800, Colin Foster wrote: > Hi Rob, Arınç, > > On Mon, Dec 12, 2022 at 10:51:47AM -0600, Rob Herring wrote: > > On Mon, Dec 12, 2022 at 12:28:06PM +0300, Arınç ÜNAL wrote: > > > On 10.12.2022 21:02, Colin Foster wrote: > > > > Hi Arınç, > > > > On Sat, Dec 10, 2022 at 07:24:42PM +0300, Arınç ÜNAL wrote: > > > > > On 10.12.2022 06:30, Colin Foster wrote: > > > > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yam > > > > @@ -156,8 +156,7 @@ patternProperties: > > > > > > > > patternProperties: > > > > "^(ethernet-)?port@[0-9]+$": > > > > - type: object > > > > - > > > > + $ref: dsa-port.yaml# > > > > properties: > > > > reg: > > > > description: > > > > @@ -165,7 +164,6 @@ patternProperties: > > > > for user ports. > > > > > > > > allOf: > > > > - - $ref: dsa-port.yaml# > > > > - if: > > > > required: [ ethernet ] > > > > then: > > > > > > > > > > > > > > > > This one has me [still] scratching my head... > > > > > > Right there with you. In addition to this, having or deleting type object > > > on/from "^(ethernet-)?ports$" and "^(ethernet-)?port@[0-9]+$" on dsa.yaml > > > doesn't cause any warnings (checked with make dt_binding_check > > > DT_SCHEMA_FILES=net/dsa) which makes me question why it's there in the first > > > place. > > > > > > That check probably doesn't consider an ref being under an 'allOf'. > > Perhaps what is missing in understanding is every schema at the > > top-level has an implicit 'type: object'. But nothing is ever implicit > > in json-schema which will silently ignore keywords which don't make > > sense for an instance type. Instead of a bunch of boilerplate, the > > processed schema has 'type' added in lots of cases such as this one. > > > > Rob > > What do you suggest on this set? I think this is the only outstanding > issue, and Jakub brought up the possibility of applying end of today > (maybe 5-6 hours from now in the US). > > It seems like there's an issue with the dt_bindings_check that causes > the "allOf: $ref" to throw a warning when it shouldn't. So removing the > "type: object" was supposed to be correct, but throws warnings. > > It seems like keeping this patch as-is and updating it when the check > gets fixed might be an acceptable path, but I'd understand if you > disagree and prefer a resubmission. Heads up on my plan for this. I plan to re-submit this on Monday after the merge window with the change where I move the $ref: dsa-port.yaml# to outside the allOf: section, and remove the object type as the above code suggests. Hopefully that's the right step to take.
On Thu, 22 Dec 2022 12:08:30 -0800 Colin Foster wrote: > Heads up on my plan for this. I plan to re-submit this on Monday after > the merge window with the change where I move the $ref: dsa-port.yaml# > to outside the allOf: section, and remove the object type as the above > code suggests. Hopefully that's the right step to take. FWIW in case you mean Mon, Dec 26th and net-next -- we extended the period of net-next being closed until Jan: https://lore.kernel.org/all/20221215092531.133ce653@kernel.org/
diff --git a/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml b/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml index 259a0c6547f3..5888e3a0169a 100644 --- a/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml +++ b/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Arrow SpeedChips XRS7000 Series Switch Device Tree Bindings allOf: - - $ref: dsa.yaml# + - $ref: dsa.yaml#/$defs/ethernet-ports maintainers: - George McCollister <george.mccollister@gmail.com> diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml index 1219b830b1a4..5bef4128d175 100644 --- a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml +++ b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml @@ -66,7 +66,7 @@ required: - reg allOf: - - $ref: dsa.yaml# + - $ref: dsa.yaml#/$defs/ethernet-ports - if: properties: compatible: diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml index 5efc0ee8edcb..9375cdcfbf96 100644 --- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml @@ -58,4 +58,26 @@ oneOf: additionalProperties: true +$defs: + ethernet-ports: + description: A DSA switch without any extra port properties + $ref: '#/' + + patternProperties: + "^(ethernet-)?ports$": + type: object + additionalProperties: false + + properties: + '#address-cells': + const: 1 + '#size-cells': + const: 0 + + patternProperties: + "^(ethernet-)?port@[0-9]+$": + description: Ethernet switch ports + $ref: dsa-port.yaml# + unevaluatedProperties: false + ... diff --git a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml index 73b774eadd0b..748ef9983ce2 100644 --- a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml +++ b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Hirschmann Hellcreek TSN Switch Device Tree Bindings allOf: - - $ref: dsa.yaml# + - $ref: dsa.yaml#/$defs/ethernet-ports maintainers: - Andrew Lunn <andrew@lunn.ch> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml index f2e9ff3f580b..20312f5d1944 100644 --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml @@ -157,9 +157,6 @@ patternProperties: patternProperties: "^(ethernet-)?port@[0-9]+$": type: object - description: Ethernet switch ports - - unevaluatedProperties: false properties: reg: @@ -238,7 +235,7 @@ $defs: - sgmii allOf: - - $ref: dsa.yaml# + - $ref: dsa.yaml#/$defs/ethernet-ports - if: required: - mediatek,mcm diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml index 4da75b1f9533..a4b53434c85c 100644 --- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml @@ -11,7 +11,7 @@ maintainers: - Woojung Huh <Woojung.Huh@microchip.com> allOf: - - $ref: dsa.yaml# + - $ref: dsa.yaml#/$defs/ethernet-ports - $ref: /schemas/spi/spi-peripheral-props.yaml# properties: diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml index 630bf0f8294b..4aee3bf4c2f4 100644 --- a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml @@ -10,7 +10,7 @@ maintainers: - UNGLinuxDriver@microchip.com allOf: - - $ref: dsa.yaml# + - $ref: dsa.yaml#/$defs/ethernet-ports properties: compatible: diff --git a/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml b/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml index 8d93ed9c172c..85014a590a35 100644 --- a/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml +++ b/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml @@ -78,7 +78,7 @@ required: - reg allOf: - - $ref: dsa.yaml# + - $ref: dsa.yaml#/$defs/ethernet-ports - if: properties: compatible: diff --git a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml index 1e26d876d146..826e2db98974 100644 --- a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml +++ b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml @@ -13,7 +13,7 @@ description: depends on the SPI bus master driver. allOf: - - $ref: "dsa.yaml#" + - $ref: dsa.yaml#/$defs/ethernet-ports - $ref: /schemas/spi/spi-peripheral-props.yaml# maintainers: diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml index 1a7d45a8ad66..cfd69c2604ea 100644 --- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml +++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Realtek switches for unmanaged switches allOf: - - $ref: dsa.yaml# + - $ref: dsa.yaml#/$defs/ethernet-ports maintainers: - Linus Walleij <linus.walleij@linaro.org> diff --git a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml index 0a0d62b6c00e..833d2f68daa1 100644 --- a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml +++ b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml @@ -14,7 +14,7 @@ description: | handles 4 ports + 1 CPU management port. allOf: - - $ref: dsa.yaml# + - $ref: dsa.yaml#/$defs/ethernet-ports properties: compatible: