| Message ID | 20230811110432.3968-1-tony@atomide.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] dt-bindings: input: gpio-keys: Allow optional dedicated wakeirq | expand |
On Fri, 11 Aug 2023 14:04:31 +0300, Tony Lindgren wrote: > Allow configuring optional dedicated wakeirq that some SoCs have. > Let's use the interrupt naming "irq" and "wakeup" that we already have > in use for some drivers and subsystems like i2c. > > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > .../devicetree/bindings/input/gpio-keys.yaml | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > 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/input/gpio-keys.yaml:40:1: [error] syntax error: found character '\t' that cannot start any token (syntax) dtschema/dtc warnings/errors: make[2]: *** Deleting file 'Documentation/devicetree/bindings/input/gpio-keys.example.dts' Documentation/devicetree/bindings/input/gpio-keys.yaml:40:1: found character '\t' that cannot start any token make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/input/gpio-keys.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/input/gpio-keys.yaml:40:1: found character '\t' that cannot start any token /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/gpio-keys.yaml: ignoring, error parsing file make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1500: dt_binding_check] Error 2 make: *** [Makefile:234: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230811110432.3968-1-tony@atomide.com 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 Fri, Aug 11, 2023 at 02:04:31PM +0300, Tony Lindgren wrote: > Allow configuring optional dedicated wakeirq that some SoCs have. > Let's use the interrupt naming "irq" and "wakeup" that we already have > in use for some drivers and subsystems like i2c. > > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > .../devicetree/bindings/input/gpio-keys.yaml | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml > --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml > +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml > @@ -31,7 +31,17 @@ patternProperties: > maxItems: 1 > > interrupts: > - maxItems: 1 > + description: > + Optional interrupts if different from the gpio interrupt > + maxItems: 2 > + > + interrupt-names: > + description: > + Optional interrupt names, can be used to specify a separate > + dedicated wake-up interrupt > + items: > + -const: irq > + -const: wakeup Also need a space after '-'. > > label: > description: Descriptive name of the key. > @@ -130,6 +140,9 @@ examples: > label = "GPIO Key UP"; > linux,code = <103>; > gpios = <&gpio1 0 1>; > + interrupts-extended = <&intc_wakeup 0 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "wakeup"; That's not what your schema allows. You need: minItems: 1 items: - enum: [ irq, wakeup ] - const: wakeup (repeating 'wakeup' is disallowed globally for ".*-names".) > + wakeup-source; Of course with this, a single interrupt is the wake-up source and doesn't need a name. So you could define that 'interrupt-names' is only used when there are 2 interrupts. In that case, the schema is right and the example is wrong. Rob
* Rob Herring <robh@kernel.org> [230811 15:10]: > On Fri, Aug 11, 2023 at 02:04:31PM +0300, Tony Lindgren wrote: > > Allow configuring optional dedicated wakeirq that some SoCs have. > > Let's use the interrupt naming "irq" and "wakeup" that we already have > > in use for some drivers and subsystems like i2c. > > > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > --- > > .../devicetree/bindings/input/gpio-keys.yaml | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml > > --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml > > +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml > > @@ -31,7 +31,17 @@ patternProperties: > > maxItems: 1 > > > > interrupts: > > - maxItems: 1 > > + description: > > + Optional interrupts if different from the gpio interrupt > > + maxItems: 2 > > + > > + interrupt-names: > > + description: > > + Optional interrupt names, can be used to specify a separate > > + dedicated wake-up interrupt > > + items: > > + -const: irq > > + -const: wakeup > > Also need a space after '-'. Oops sorry about that, obviously I did not run make dtbs_check on this binding. I guess I just grepped so we don't have interrupt-names in use right now. > > > > label: > > description: Descriptive name of the key. > > @@ -130,6 +140,9 @@ examples: > > label = "GPIO Key UP"; > > linux,code = <103>; > > gpios = <&gpio1 0 1>; > > + interrupts-extended = <&intc_wakeup 0 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-names = "wakeup"; > > That's not what your schema allows. You need: > > minItems: 1 > items: > - enum: [ irq, wakeup ] > - const: wakeup > > (repeating 'wakeup' is disallowed globally for ".*-names".) OK > > + wakeup-source; > > Of course with this, a single interrupt is the wake-up source and > doesn't need a name. So you could define that 'interrupt-names' is only > used when there are 2 interrupts. In that case, the schema is right and > the example is wrong. OK. So here's what gpio-keys currently allows: 1. gpios property with no interrupt in the dts, the driver tries to find the interrupt based on the gpio 2. gpios property with one interrupts property and no interrupt-names And here's what we could allow in the binding with the wakeirq support added: 1. gpios property with no interrupt in the dts, the driver tries to find the interrupt based on the gpio 2. gpios property with one interrupts property and no interrupt-names 3. gpios property with one interrupts property and interrupt-names = "irq" 4. gpios property with one wakeirq and interrupt-names = "wakeirq", the driver tries to find the io interrupt based on the gpio 5. gpios property with two interrupts and interrupt-names = "irq", "wakeirq" So yeah we could only allow interrupt-names if there are two interrupts like the attempted binding has. This would leave out #3 and #4 options above. No need to limit these options from driver point of view though. Any preferences on what the binding should have? Regards, Tony
diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml @@ -31,7 +31,17 @@ patternProperties: maxItems: 1 interrupts: - maxItems: 1 + description: + Optional interrupts if different from the gpio interrupt + maxItems: 2 + + interrupt-names: + description: + Optional interrupt names, can be used to specify a separate + dedicated wake-up interrupt + items: + -const: irq + -const: wakeup label: description: Descriptive name of the key. @@ -130,6 +140,9 @@ examples: label = "GPIO Key UP"; linux,code = <103>; gpios = <&gpio1 0 1>; + interrupts-extended = <&intc_wakeup 0 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "wakeup"; + wakeup-source; }; key-down {
Allow configuring optional dedicated wakeirq that some SoCs have. Let's use the interrupt naming "irq" and "wakeup" that we already have in use for some drivers and subsystems like i2c. Signed-off-by: Tony Lindgren <tony@atomide.com> --- .../devicetree/bindings/input/gpio-keys.yaml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)