| Message ID | 20200917165301.23100-2-krzk@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | gpio: add common dtschema | expand |
Hi Krzysztof, Thank you for the patch. On Thu, Sep 17, 2020 at 06:52:49PM +0200, Krzysztof Kozlowski wrote: > Convert parts of gpio.txt bindings into common dtschema file for GPIO > controllers. How about deleting the part that has been converted from gpio.txt ? > The schema enforces proper naming of GPIO controller nodes and GPIO > hogs. > > The schema should be included by specific GPIO controllers bindings. Instead of including it manually, could we use a conditional select: to apply the schema automatically when a gpio-controller property is present ? > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > --- > > Changes since v1: > 1. Do not require compatible (some child nodes are gpio-controllers > without the compatible). > --- > .../devicetree/bindings/gpio/gpio-common.yaml | 125 ++++++++++++++++++ > 1 file changed, 125 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/gpio-common.yaml > > diff --git a/Documentation/devicetree/bindings/gpio/gpio-common.yaml b/Documentation/devicetree/bindings/gpio/gpio-common.yaml > new file mode 100644 > index 000000000000..af9f6c7feeec > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/gpio-common.yaml > @@ -0,0 +1,125 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/gpio/gpio-common.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Common GPIO controller properties > + > +maintainers: > + - Krzysztof Kozlowski <krzk@kernel.org> > + - Linus Walleij <linus.walleij@linaro.org> > + > +properties: > + nodename: > + pattern: "^(gpio-controller|gpio)(@[0-9a-f]+|-[0-9a-f]+)?$" > + > + '#gpio-cells': true > + gpio-controller: true > + gpio-ranges: true > + > + gpio-line-names: > + description: | > + Optionally, a GPIO controller may have a "gpio-line-names" property. This > + is an array of strings defining the names of the GPIO lines going out of > + the GPIO controller. This name should be the most meaningful producer > + name for the system, such as a rail name indicating the usage. Package > + names such as pin name are discouraged: such lines have opaque names > + (since they are by definition generic purpose) and such names are usually > + not very helpful. > + > + For example "MMC-CD", "Red LED Vdd" and "ethernet reset" are reasonable > + line names as they describe what the line is used for. "GPIO0" is not a > + good name to give to a GPIO line. > + > + Placeholders are discouraged: rather use the "" (blank string) if the use > + of the GPIO line is undefined in your design. The names are assigned > + starting from line offset 0 from left to right from the passed array. An > + incomplete array (where the number of passed named are less than ngpios) > + will still be used up until the last provided valid line index. > + > + gpio-reserved-ranges: > + description: > + Indicates the start and size of the GPIOs that can't be used. > + > + ngpios: > + description: | > + Optionally, a GPIO controller may have a "ngpios" property. This property > + indicates the number of in-use slots of available slots for GPIOs. The > + typical example is something like this: the hardware register is 32 bits > + wide, but only 18 of the bits have a physical counterpart. The driver is > + generally written so that all 32 bits can be used, but the IP block is > + reused in a lot of designs, some using all 32 bits, some using 18 and > + some using 12. In this case, setting "ngpios = <18>;" informs the driver > + that only the first 18 GPIOs, at local offset 0 .. 17, are in use. > + > + If these GPIOs do not happen to be the first N GPIOs at offset 0...N-1, > + an additional set of tuples is needed to specify which GPIOs are > + unusable, with the gpio-reserved-ranges binding. > + > +patternProperties: > + "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$": > + type: object > + description: > + The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism > + providing automatic GPIO request and configuration as part of the > + gpio-controller's driver probe function. > + Each GPIO hog definition is represented as a child node of the GPIO controller. > + > + properties: > + gpio-hog: true > + gpios: true > + input: true > + output-high: true > + output-low: true > + line-name: > + description: > + The GPIO label name. If not present the node name is used. > + > + required: > + - gpio-hog > + - gpios > + > + oneOf: > + - required: > + - input > + - required: > + - output-high > + - required: > + - output-low > + > + additionalProperties: false > + > +required: > + - "#gpio-cells" > + - gpio-controller > + > +examples: > + - | > + gpio-controller@15000000 { > + compatible = "foo"; > + reg = <0x15000000 0x1000>; > + gpio-controller; > + #gpio-cells = <2>; > + ngpios = <18>; > + gpio-reserved-ranges = <0 4>, <12 2>; > + gpio-line-names = "MMC-CD", "MMC-WP", "VDD eth", "RST eth", "LED R", > + "LED G", "LED B", "Col A", "Col B", "Col C", "Col D", > + "Row A", "Row B", "Row C", "Row D", "NMI button", > + "poweroff", "reset"; > + }; > + > + - | > + gpio-controller@1400 { > + compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > + reg = <0x1400 0x18>; > + gpio-controller; > + #gpio-cells = <2>; > + > + line-b-hog { > + gpio-hog; > + gpios = <6 0>; > + input; > + line-name = "foo-bar-gpio"; > + }; > + };
Hi Krzysztof, On Fri, Sep 18, 2020 at 09:52:57AM +0200, Krzysztof Kozlowski wrote: > On Thu, 17 Sep 2020 at 22:10, Laurent Pinchart wrote: > > On Thu, Sep 17, 2020 at 06:52:49PM +0200, Krzysztof Kozlowski wrote: > > > Convert parts of gpio.txt bindings into common dtschema file for GPIO > > > controllers. > > > > How about deleting the part that has been converted from gpio.txt ? > > I did not move everything from the gpio.txt and it is really nicely > explained there. I think to leave it as it works as a overview/guide > better than YAML. I'm just a bit worried that duplicating some of the information in two places will lead to them becoming out of sync, but maybe the risk isn't that high. > > > The schema enforces proper naming of GPIO controller nodes and GPIO > > > hogs. > > > > > > The schema should be included by specific GPIO controllers bindings. > > > > Instead of including it manually, could we use a conditional select: to > > apply the schema automatically when a gpio-controller property is > > present ? > > You mean the same way as generic schema for GPIO controllers work? > This could be done but the point is to enforce the GPIO controller > bindings in GPIO controllers, so also in cases when someone forgets to > add "gpio-controller" property. Although, if given GPIO controller > schema requires "gpio-controller" then indeed select would work... You could just make gpio-controller mandatory in the schema, and get everything else automatically selected based on that, without needing a manual $ref.
On Thu, Sep 17, 2020 at 10:53 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > Convert parts of gpio.txt bindings into common dtschema file for GPIO > controllers. The schema enforces proper naming of GPIO controller nodes > and GPIO hogs. Did you not see my previous reply about a common schema? We already have a common GPIO and hog schema in dtschema. Please add to it whatever is missing. My goal is all common schema end up in dtschema, but I haven't pushed folks to do that yet. Ones I've done are there though. One issue is what's in dtschema should be GPL/BSD and the existing text bindings are default GPL, so there's a relicensing exercise. In some cases, the schema is there but I haven't copied over the descriptions. Rob > + description: > + Indicates the start and size of the GPIOs that can't be used. > + > + ngpios: > + description: | > + Optionally, a GPIO controller may have a "ngpios" property. This property > + indicates the number of in-use slots of available slots for GPIOs. The > + typical example is something like this: the hardware register is 32 bits > + wide, but only 18 of the bits have a physical counterpart. The driver is > + generally written so that all 32 bits can be used, but the IP block is > + reused in a lot of designs, some using all 32 bits, some using 18 and > + some using 12. In this case, setting "ngpios = <18>;" informs the driver > + that only the first 18 GPIOs, at local offset 0 .. 17, are in use. > + > + If these GPIOs do not happen to be the first N GPIOs at offset 0...N-1, > + an additional set of tuples is needed to specify which GPIOs are > + unusable, with the gpio-reserved-ranges binding. > + > +patternProperties: > + "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$": > + type: object > + description: > + The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism > + providing automatic GPIO request and configuration as part of the > + gpio-controller's driver probe function. > + Each GPIO hog definition is represented as a child node of the GPIO controller. > + > + properties: > + gpio-hog: true > + gpios: true > + input: true > + output-high: true > + output-low: true > + line-name: > + description: > + The GPIO label name. If not present the node name is used. > + > + required: > + - gpio-hog > + - gpios > + > + oneOf: > + - required: > + - input > + - required: > + - output-high > + - required: > + - output-low > + > + additionalProperties: false > + > +required: > + - "#gpio-cells" > + - gpio-controller > + > +examples: > + - | > + gpio-controller@15000000 { > + compatible = "foo"; > + reg = <0x15000000 0x1000>; > + gpio-controller; > + #gpio-cells = <2>; > + ngpios = <18>; > + gpio-reserved-ranges = <0 4>, <12 2>; > + gpio-line-names = "MMC-CD", "MMC-WP", "VDD eth", "RST eth", "LED R", > + "LED G", "LED B", "Col A", "Col B", "Col C", "Col D", > + "Row A", "Row B", "Row C", "Row D", "NMI button", > + "poweroff", "reset"; > + }; > + > + - | > + gpio-controller@1400 { > + compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > + reg = <0x1400 0x18>; > + gpio-controller; > + #gpio-cells = <2>; > + > + line-b-hog { > + gpio-hog; > + gpios = <6 0>; > + input; > + line-name = "foo-bar-gpio"; > + }; > + }; > -- > 2.17.1 >
On Fri, Sep 18, 2020 at 08:30:02AM -0600, Rob Herring wrote: > On Thu, Sep 17, 2020 at 10:53 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > Convert parts of gpio.txt bindings into common dtschema file for GPIO > > controllers. The schema enforces proper naming of GPIO controller nodes > > and GPIO hogs. > > Did you not see my previous reply about a common schema? We already > have a common GPIO and hog schema in dtschema. Please add to it > whatever is missing. Indeed, I'll enhance the dt-schema. The trouble is that each in-kernel YAML file still has to mention possible gpio-hogs nodes. Is the proper solution to put them in common YAML inside kernel sources? > > My goal is all common schema end up in dtschema, but I haven't pushed > folks to do that yet. Ones I've done are there though. One issue is > what's in dtschema should be GPL/BSD and the existing text bindings > are default GPL, so there's a relicensing exercise. In some cases, the > schema is there but I haven't copied over the descriptions. Right, I'll skip the descriptions when posting to dt-schema. Best regards, Krzysztof
On Sun, Sep 20, 2020 at 09:39:15PM +0200, Krzysztof Kozlowski wrote: > On Fri, Sep 18, 2020 at 08:30:02AM -0600, Rob Herring wrote: > > On Thu, Sep 17, 2020 at 10:53 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > Convert parts of gpio.txt bindings into common dtschema file for GPIO > > > controllers. The schema enforces proper naming of GPIO controller nodes > > > and GPIO hogs. > > > > Did you not see my previous reply about a common schema? We already > > have a common GPIO and hog schema in dtschema. Please add to it > > whatever is missing. > > Indeed, I'll enhance the dt-schema. > > The trouble is that each in-kernel YAML file still has to mention > possible gpio-hogs nodes. Is the proper solution to put them in common > YAML inside kernel sources? Currently, the gpio.yaml schema is applied to all nodes. That has the advantage that GPIO related properties are always checked whether we have a device specific schema or not. It has the disadvantage that you can't do some constraints like required properties or what's in child nodes. We could (and probably should) change it to be referenced by specific gpio controller schemas like we do for i2c, spi, etc. Then you can define required properties there and do something like: "-hogs$": type: object $ref: gpio-hogs.yaml# > > My goal is all common schema end up in dtschema, but I haven't pushed > > folks to do that yet. Ones I've done are there though. One issue is > > what's in dtschema should be GPL/BSD and the existing text bindings > > are default GPL, so there's a relicensing exercise. In some cases, the > > schema is there but I haven't copied over the descriptions. > > Right, I'll skip the descriptions when posting to dt-schema. I was hoping someone would add the descriptions. :) Rob
diff --git a/Documentation/devicetree/bindings/gpio/gpio-common.yaml b/Documentation/devicetree/bindings/gpio/gpio-common.yaml new file mode 100644 index 000000000000..af9f6c7feeec --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-common.yaml @@ -0,0 +1,125 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/gpio/gpio-common.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Common GPIO controller properties + +maintainers: + - Krzysztof Kozlowski <krzk@kernel.org> + - Linus Walleij <linus.walleij@linaro.org> + +properties: + nodename: + pattern: "^(gpio-controller|gpio)(@[0-9a-f]+|-[0-9a-f]+)?$" + + '#gpio-cells': true + gpio-controller: true + gpio-ranges: true + + gpio-line-names: + description: | + Optionally, a GPIO controller may have a "gpio-line-names" property. This + is an array of strings defining the names of the GPIO lines going out of + the GPIO controller. This name should be the most meaningful producer + name for the system, such as a rail name indicating the usage. Package + names such as pin name are discouraged: such lines have opaque names + (since they are by definition generic purpose) and such names are usually + not very helpful. + + For example "MMC-CD", "Red LED Vdd" and "ethernet reset" are reasonable + line names as they describe what the line is used for. "GPIO0" is not a + good name to give to a GPIO line. + + Placeholders are discouraged: rather use the "" (blank string) if the use + of the GPIO line is undefined in your design. The names are assigned + starting from line offset 0 from left to right from the passed array. An + incomplete array (where the number of passed named are less than ngpios) + will still be used up until the last provided valid line index. + + gpio-reserved-ranges: + description: + Indicates the start and size of the GPIOs that can't be used. + + ngpios: + description: | + Optionally, a GPIO controller may have a "ngpios" property. This property + indicates the number of in-use slots of available slots for GPIOs. The + typical example is something like this: the hardware register is 32 bits + wide, but only 18 of the bits have a physical counterpart. The driver is + generally written so that all 32 bits can be used, but the IP block is + reused in a lot of designs, some using all 32 bits, some using 18 and + some using 12. In this case, setting "ngpios = <18>;" informs the driver + that only the first 18 GPIOs, at local offset 0 .. 17, are in use. + + If these GPIOs do not happen to be the first N GPIOs at offset 0...N-1, + an additional set of tuples is needed to specify which GPIOs are + unusable, with the gpio-reserved-ranges binding. + +patternProperties: + "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$": + type: object + description: + The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism + providing automatic GPIO request and configuration as part of the + gpio-controller's driver probe function. + Each GPIO hog definition is represented as a child node of the GPIO controller. + + properties: + gpio-hog: true + gpios: true + input: true + output-high: true + output-low: true + line-name: + description: + The GPIO label name. If not present the node name is used. + + required: + - gpio-hog + - gpios + + oneOf: + - required: + - input + - required: + - output-high + - required: + - output-low + + additionalProperties: false + +required: + - "#gpio-cells" + - gpio-controller + +examples: + - | + gpio-controller@15000000 { + compatible = "foo"; + reg = <0x15000000 0x1000>; + gpio-controller; + #gpio-cells = <2>; + ngpios = <18>; + gpio-reserved-ranges = <0 4>, <12 2>; + gpio-line-names = "MMC-CD", "MMC-WP", "VDD eth", "RST eth", "LED R", + "LED G", "LED B", "Col A", "Col B", "Col C", "Col D", + "Row A", "Row B", "Row C", "Row D", "NMI button", + "poweroff", "reset"; + }; + + - | + gpio-controller@1400 { + compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; + reg = <0x1400 0x18>; + gpio-controller; + #gpio-cells = <2>; + + line-b-hog { + gpio-hog; + gpios = <6 0>; + input; + line-name = "foo-bar-gpio"; + }; + };
Convert parts of gpio.txt bindings into common dtschema file for GPIO controllers. The schema enforces proper naming of GPIO controller nodes and GPIO hogs. The schema should be included by specific GPIO controllers bindings. Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- Changes since v1: 1. Do not require compatible (some child nodes are gpio-controllers without the compatible). --- .../devicetree/bindings/gpio/gpio-common.yaml | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-common.yaml