| Message ID | 20240926134218.20863-1-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dt-bindings: examples: Add GPIO hogs and variable-lists | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Thu, Sep 26, 2024 at 03:42:18PM +0200, Krzysztof Kozlowski wrote: > Same Devicetree binding can be expressed multiple ways thus no wonder > we have multiple variants of similar concept. All such code could be > cleaned up, but even then new contributor might not be able to find > good, existing binding solving their case. > > Add subdirectory with two Devicetree schema examples (GPIO hog and > variable lists like clocks), hoping it will grow with reference-designs. > All these bindings use valid compatibles, although with "test" vendor > prefix, so they will be validated by DT schema. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > If approach is accepted, I will come with more ideas, as commented > during LPC DT BoF to creates examples based on review. > > Cc: Nishanth Menon <nm@ti.com> > Cc: Bjorn Andersson <andersson@kernel.org> > --- > .../devicetree/bindings/example-schema.yaml | 2 + > .../examples/gpio-controller-and-hogs.yaml | 72 ++++++++++++ > .../multiple-variants-and-variable-lists.yaml | 110 ++++++++++++++++++ > .../devicetree/bindings/writing-schema.rst | 6 +- > 4 files changed, 189 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml > create mode 100644 Documentation/devicetree/bindings/examples/multiple-variants-and-variable-lists.yaml > > diff --git a/Documentation/devicetree/bindings/example-schema.yaml b/Documentation/devicetree/bindings/example-schema.yaml > index a41f9b9a196b..0aece1823fde 100644 > --- a/Documentation/devicetree/bindings/example-schema.yaml > +++ b/Documentation/devicetree/bindings/example-schema.yaml > @@ -25,6 +25,8 @@ description: | > indentation less than the first line of the literal block. Lines also cannot > begin with a tab character. > > + See also examples/ subdirectory for more detailed code snippets. > + > select: false > # 'select' is a schema applied to a DT node to determine if this binding > # schema should be applied to the node. It is optional and by default the > diff --git a/Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml b/Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml > new file mode 100644 > index 000000000000..597a847daa12 > --- /dev/null > +++ b/Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml > @@ -0,0 +1,72 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/examples/gpio-controller-and-hogs.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: An Example GPIO Controller with Hogs > + > +maintainers: > + - Rob Herring <robh@kernel.org> > + - Krzysztof Kozlowski <krzk@kernel.org> > + - Conor Dooley <conor@kernel.org> > + > +properties: > + compatible: > + const: test,gpio-controller-and-hogs > + > + gpio-controller: true > + > + "#gpio-cells": > + const: 2 > + > + gpio-line-names: > + minItems: 1 > + maxItems: 32 > + > + gpio-ranges: > + minItems: 1 > + maxItems: 16 > + > + # Not every GPIO controller is an interrupt-controller > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 2 > + > +patternProperties: > + "-hog(-[0-9]+)?$": > + type: object > + required: > + - gpio-hog The best way to ensure this is done right is eliminate the need rather than adding examples to be ignored. I think we need to make gpio provider bindings reference gpio.yaml and then handle hogs there. Generally, the pattern is that providers which just list properties don't have a $ref to the provider schema and are always applied. Schemas that have child nodes like bus schemas are referenced by the specific controller schemas. GPIO started out as the former, but the addition of hogs made it more like the latter. That of course means that every GPIO provider can have hogs, but I think that's the right answer even though I'd really prefer no hogs at all. Always applying schemas like gpio.yaml has some benefits over relying on applying it via $ref, but that's diminished once everything has a schema. Of course, we can do both at the expense of applying the schema twice. Rob
On Thu, Sep 26, 2024 at 05:37:18PM -0500, Rob Herring wrote: > On Thu, Sep 26, 2024 at 03:42:18PM +0200, Krzysztof Kozlowski wrote: > > Same Devicetree binding can be expressed multiple ways thus no wonder > > we have multiple variants of similar concept. All such code could be > > cleaned up, but even then new contributor might not be able to find > > good, existing binding solving their case. > > > > Add subdirectory with two Devicetree schema examples (GPIO hog and > > variable lists like clocks), hoping it will grow with reference-designs. > > All these bindings use valid compatibles, although with "test" vendor > > prefix, so they will be validated by DT schema. > > > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > > > --- > > > > If approach is accepted, I will come with more ideas, as commented > > during LPC DT BoF to creates examples based on review. > > > > Cc: Nishanth Menon <nm@ti.com> > > Cc: Bjorn Andersson <andersson@kernel.org> > > --- > > .../devicetree/bindings/example-schema.yaml | 2 + > > .../examples/gpio-controller-and-hogs.yaml | 72 ++++++++++++ > > .../multiple-variants-and-variable-lists.yaml | 110 ++++++++++++++++++ > > .../devicetree/bindings/writing-schema.rst | 6 +- > > 4 files changed, 189 insertions(+), 1 deletion(-) > > create mode 100644 Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml > > create mode 100644 Documentation/devicetree/bindings/examples/multiple-variants-and-variable-lists.yaml > > > > diff --git a/Documentation/devicetree/bindings/example-schema.yaml b/Documentation/devicetree/bindings/example-schema.yaml > > index a41f9b9a196b..0aece1823fde 100644 > > --- a/Documentation/devicetree/bindings/example-schema.yaml > > +++ b/Documentation/devicetree/bindings/example-schema.yaml > > @@ -25,6 +25,8 @@ description: | > > indentation less than the first line of the literal block. Lines also cannot > > begin with a tab character. > > > > + See also examples/ subdirectory for more detailed code snippets. > > + > > select: false > > # 'select' is a schema applied to a DT node to determine if this binding > > # schema should be applied to the node. It is optional and by default the > > diff --git a/Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml b/Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml > > new file mode 100644 > > index 000000000000..597a847daa12 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml > > @@ -0,0 +1,72 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/examples/gpio-controller-and-hogs.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: An Example GPIO Controller with Hogs > > + > > +maintainers: > > + - Rob Herring <robh@kernel.org> > > + - Krzysztof Kozlowski <krzk@kernel.org> > > + - Conor Dooley <conor@kernel.org> > > + > > +properties: > > + compatible: > > + const: test,gpio-controller-and-hogs > > + > > + gpio-controller: true > > + > > + "#gpio-cells": > > + const: 2 > > + > > + gpio-line-names: > > + minItems: 1 > > + maxItems: 32 > > + > > + gpio-ranges: > > + minItems: 1 > > + maxItems: 16 > > + > > + # Not every GPIO controller is an interrupt-controller > > + interrupt-controller: true > > + > > + "#interrupt-cells": > > + const: 2 > > + > > +patternProperties: > > + "-hog(-[0-9]+)?$": > > + type: object > > + required: > > + - gpio-hog > > The best way to ensure this is done right is eliminate the need rather > than adding examples to be ignored. > > I think we need to make gpio provider bindings reference gpio.yaml and > then handle hogs there. Generally, the pattern is that providers which > just list properties don't have a $ref to the provider schema and are > always applied. Schemas that have child nodes like bus schemas are > referenced by the specific controller schemas. GPIO started out as the > former, but the addition of hogs made it more like the latter. That of > course means that every GPIO provider can have hogs, but I think that's > the right answer even though I'd really prefer no hogs at all. > > Always applying schemas like gpio.yaml has some benefits over relying > on applying it via $ref, but that's diminished once everything has a > schema. Of course, we can do both at the expense of applying the schema > twice. Ack, I will take a look at applying gpio.yaml to all controllers. At least looking at Linux, all GPIO chips and pinctrl with GPIO chip code go via generic of_gpiochip_add() which later goes via children for_each_available_child_of_node_scoped() to add hogs, so adding hogs to each schema seems matching reality. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/example-schema.yaml b/Documentation/devicetree/bindings/example-schema.yaml index a41f9b9a196b..0aece1823fde 100644 --- a/Documentation/devicetree/bindings/example-schema.yaml +++ b/Documentation/devicetree/bindings/example-schema.yaml @@ -25,6 +25,8 @@ description: | indentation less than the first line of the literal block. Lines also cannot begin with a tab character. + See also examples/ subdirectory for more detailed code snippets. + select: false # 'select' is a schema applied to a DT node to determine if this binding # schema should be applied to the node. It is optional and by default the diff --git a/Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml b/Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml new file mode 100644 index 000000000000..597a847daa12 --- /dev/null +++ b/Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml @@ -0,0 +1,72 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/examples/gpio-controller-and-hogs.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: An Example GPIO Controller with Hogs + +maintainers: + - Rob Herring <robh@kernel.org> + - Krzysztof Kozlowski <krzk@kernel.org> + - Conor Dooley <conor@kernel.org> + +properties: + compatible: + const: test,gpio-controller-and-hogs + + gpio-controller: true + + "#gpio-cells": + const: 2 + + gpio-line-names: + minItems: 1 + maxItems: 32 + + gpio-ranges: + minItems: 1 + maxItems: 16 + + # Not every GPIO controller is an interrupt-controller + interrupt-controller: true + + "#interrupt-cells": + const: 2 + +patternProperties: + "-hog(-[0-9]+)?$": + type: object + required: + - gpio-hog + +required: + - compatible + - interrupt-controller + - "#interrupt-cells" + - gpio-controller + - "#gpio-cells" + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + gpio { + compatible = "test,gpio-controller-and-hogs"; + + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&iomuxc 0 0 32>; + interrupt-controller; + #interrupt-cells = <2>; + + usb3-sata-sel-hog { + gpio-hog; + gpios = <4 GPIO_ACTIVE_HIGH>; + line-name = "usb3_sata_sel"; + output-low; + }; + }; diff --git a/Documentation/devicetree/bindings/examples/multiple-variants-and-variable-lists.yaml b/Documentation/devicetree/bindings/examples/multiple-variants-and-variable-lists.yaml new file mode 100644 index 000000000000..8763791a8adb --- /dev/null +++ b/Documentation/devicetree/bindings/examples/multiple-variants-and-variable-lists.yaml @@ -0,0 +1,110 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/examples/multiple-variants-and-variable-lists.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: An Example Device with Multiple Variants and Variable Clock Inputs + +maintainers: + - Rob Herring <robh@kernel.org> + - Krzysztof Kozlowski <krzk@kernel.org> + - Conor Dooley <conor@kernel.org> + +properties: + compatible: + enum: + - test,variant-2-clocks + - test,variant-3-clocks + - test,variant-4-clocks + + # Same principles as for clocks apply also to all list-based properties, like + # power-domains, reg, resets, etc. + clocks: + minItems: 2 + maxItems: 4 + + # It is preferred that variants (devices) share as much as possible from + # lists, e.g. the list has common part with 'minItems: X'. Example shows + # case when list cannot be constructed that way. + clock-names: + minItems: 2 + maxItems: 4 + +required: + - compatible + - clocks + - clock-names + +allOf: + - if: + properties: + compatible: + const: test,variant-2-clocks + then: + properties: + clocks: + maxItems: 2 + clock-names: + items: + - const: bus + - const: core + + - if: + properties: + compatible: + const: test,variant-3-clocks + then: + properties: + clocks: + minItems: 3 + maxItems: 3 + clock-names: + items: + - const: bus + - const: ref + - const: lane + + - if: + properties: + compatible: + const: test,variant-4-clocks + then: + properties: + clocks: + minItems: 4 + clock-names: + items: + - const: bus + - const: ref + - const: tx_lane + - const: rx_lane + +additionalProperties: false + +# Usually, if examples different by one property only one example is useful. +# For the purpose of the excercise all variants are described. +examples: + - | + device { + compatible = "test,variant-2-clocks"; + + clocks = <&cc 1>, <&cc 2>; + clock-names = "bus", "core"; + }; + + - | + device { + compatible = "test,variant-3-clocks"; + + clocks = <&cc 1>, <&cc 2>, <&cc 3>; + clock-names = "bus", "ref", "lane"; + }; + + - | + device { + compatible = "test,variant-4-clocks"; + + clocks = <&cc 1>, <&cc 2>, <&cc 3>, <&cc 4>; + clock-names = "bus", "ref", "tx_lane", "rx_lane"; + }; diff --git a/Documentation/devicetree/bindings/writing-schema.rst b/Documentation/devicetree/bindings/writing-schema.rst index 7e71cdd1d6de..4ecd4b309276 100644 --- a/Documentation/devicetree/bindings/writing-schema.rst +++ b/Documentation/devicetree/bindings/writing-schema.rst @@ -206,6 +206,10 @@ json-schema Resources Annotated Example Schema ------------------------ -Also available as a separate file: :download:`example-schema.yaml` +Several examples for typical cases: + - :download:`examples/gpio-controller-and-hogs.yaml` + - :download:`examples/multiple-variants-and-variable-lists.yaml` + +Example-schema available as a separate file: :download:`example-schema.yaml` .. literalinclude:: example-schema.yaml
Same Devicetree binding can be expressed multiple ways thus no wonder we have multiple variants of similar concept. All such code could be cleaned up, but even then new contributor might not be able to find good, existing binding solving their case. Add subdirectory with two Devicetree schema examples (GPIO hog and variable lists like clocks), hoping it will grow with reference-designs. All these bindings use valid compatibles, although with "test" vendor prefix, so they will be validated by DT schema. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- If approach is accepted, I will come with more ideas, as commented during LPC DT BoF to creates examples based on review. Cc: Nishanth Menon <nm@ti.com> Cc: Bjorn Andersson <andersson@kernel.org> --- .../devicetree/bindings/example-schema.yaml | 2 + .../examples/gpio-controller-and-hogs.yaml | 72 ++++++++++++ .../multiple-variants-and-variable-lists.yaml | 110 ++++++++++++++++++ .../devicetree/bindings/writing-schema.rst | 6 +- 4 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/examples/gpio-controller-and-hogs.yaml create mode 100644 Documentation/devicetree/bindings/examples/multiple-variants-and-variable-lists.yaml