[RFC PATCH 0/2] dt-bindings: input: gpio-keys: rework matching children

[RFC PATCH 0/2] dt-bindings: input: gpio-keys: rework matching children

[Krzysztof Kozlowski]

Hi,

Currently the gpio-keys schema allows any property to be present, even
undocumented.  Narrow the pattern for children to require specific key naming like:

    gpio-keys {
        compatible = "gpio-keys";

        // "up" is wrong
        key-up {
            label = "GPIO Key UP";
            linux,code = <103>;
            gpios = <&gpio1 0 1>;
        };
    };

This will cause many, many DTS warnings, which I can fix. But before I start
such big work, let's agree whether the approach is correct.

Best regards,
Krzysztof

Krzysztof Kozlowski (2):
  dt-bindings: input: gpio-keys: enforce node names to match all
    properties
  dt-bindings: input: gpio-keys: document label and autorepeat
    properties

 .../devicetree/bindings/input/gpio-keys.yaml  | 177 +++++++++---------
 1 file changed, 91 insertions(+), 86 deletions(-)

-- 
2.34.1

[RFC PATCH 1/2] dt-bindings: input: gpio-keys: enforce node names to match all properties

[Krzysztof Kozlowski]

The gpio-keys DT schema matches all properties with a wide pattern and
applies specific schema to children.  This has drawback - all regular
properties are also matched and are silently ignored, even if they are
not described in schema.  Basically this allows any non-object property
to be present.

Enforce specific naming pattern for children (keys) to narrow the
pattern thus do not match other properties.  This will require all
children to be named with 'key-' prefix or '-key' suffix.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/input/gpio-keys.yaml  | 169 +++++++++---------
 1 file changed, 83 insertions(+), 86 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
index 93f601c58984..49d388dc8d78 100644
--- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
@@ -16,92 +16,89 @@ properties:
       - gpio-keys-polled
 
 patternProperties:
-  ".*":
-    if:
-      type: object
-    then:
-      $ref: input.yaml#
-
-      properties:
-        gpios:
-          maxItems: 1
-
-        interrupts:
-          maxItems: 1
-
-        label:
-          description: Descriptive name of the key.
-
-        linux,code:
-          description: Key / Axis code to emit.
-          $ref: /schemas/types.yaml#/definitions/uint32
-
-        linux,input-type:
-          description:
-            Specify event type this button/key generates. If not specified defaults to
-            <1> == EV_KEY.
-          $ref: /schemas/types.yaml#/definitions/uint32
-
-          default: 1
-
-        linux,input-value:
-          description: |
-            If linux,input-type is EV_ABS or EV_REL then this
-            value is sent for events this button generates when pressed.
-            EV_ABS/EV_REL axis will generate an event with a value of 0
-            when all buttons with linux,input-type == type and
-            linux,code == axis are released. This value is interpreted
-            as a signed 32 bit value, e.g. to make a button generate a
-            value of -1 use:
-
-            linux,input-value = <0xffffffff>; /* -1 */
-
-          $ref: /schemas/types.yaml#/definitions/uint32
-
-        debounce-interval:
-          description:
-            Debouncing interval time in milliseconds. If not specified defaults to 5.
-          $ref: /schemas/types.yaml#/definitions/uint32
-
-          default: 5
-
-        wakeup-source:
-          description: Button can wake-up the system.
-
-        wakeup-event-action:
-          description: |
-            Specifies whether the key should wake the system when asserted, when
-            deasserted, or both. This property is only valid for keys that wake up the
-            system (e.g., when the "wakeup-source" property is also provided).
-
-            Supported values are defined in linux-event-codes.h:
-
-              EV_ACT_ANY        - both asserted and deasserted
-              EV_ACT_ASSERTED   - asserted
-              EV_ACT_DEASSERTED - deasserted
-          $ref: /schemas/types.yaml#/definitions/uint32
-          enum: [0, 1, 2]
-
-        linux,can-disable:
-          description:
-            Indicates that button is connected to dedicated (not shared) interrupt
-            which can be disabled to suppress events from the button.
-          type: boolean
-
-      required:
-        - linux,code
-
-      anyOf:
-        - required:
-            - interrupts
-        - required:
-            - gpios
-
-      dependencies:
-        wakeup-event-action: [ wakeup-source ]
-        linux,input-value: [ gpios ]
-
-      unevaluatedProperties: false
+  "^(key|key-[a-z0-9-]+|[a-z0-9-]+-key)$":
+    $ref: input.yaml#
+
+    properties:
+      gpios:
+        maxItems: 1
+
+      interrupts:
+        maxItems: 1
+
+      label:
+        description: Descriptive name of the key.
+
+      linux,code:
+        description: Key / Axis code to emit.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      linux,input-type:
+        description:
+          Specify event type this button/key generates. If not specified defaults to
+          <1> == EV_KEY.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+        default: 1
+
+      linux,input-value:
+        description: |
+          If linux,input-type is EV_ABS or EV_REL then this
+          value is sent for events this button generates when pressed.
+          EV_ABS/EV_REL axis will generate an event with a value of 0
+          when all buttons with linux,input-type == type and
+          linux,code == axis are released. This value is interpreted
+          as a signed 32 bit value, e.g. to make a button generate a
+          value of -1 use:
+
+          linux,input-value = <0xffffffff>; /* -1 */
+
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      debounce-interval:
+        description:
+          Debouncing interval time in milliseconds. If not specified defaults to 5.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+        default: 5
+
+      wakeup-source:
+        description: Button can wake-up the system.
+
+      wakeup-event-action:
+        description: |
+          Specifies whether the key should wake the system when asserted, when
+          deasserted, or both. This property is only valid for keys that wake up the
+          system (e.g., when the "wakeup-source" property is also provided).
+
+          Supported values are defined in linux-event-codes.h:
+
+            EV_ACT_ANY        - both asserted and deasserted
+            EV_ACT_ASSERTED   - asserted
+            EV_ACT_DEASSERTED - deasserted
+        $ref: /schemas/types.yaml#/definitions/uint32
+        enum: [0, 1, 2]
+
+      linux,can-disable:
+        description:
+          Indicates that button is connected to dedicated (not shared) interrupt
+          which can be disabled to suppress events from the button.
+        type: boolean
+
+    required:
+      - linux,code
+
+    anyOf:
+      - required:
+          - interrupts
+      - required:
+          - gpios
+
+    dependencies:
+      wakeup-event-action: [ wakeup-source ]
+      linux,input-value: [ gpios ]
+
+    unevaluatedProperties: false
 
 if:
   properties:
-- 
2.34.1

Re: [RFC PATCH 1/2] dt-bindings: input: gpio-keys: enforce node names to match all properties

[Rob Herring]

On Fri, 03 Jun 2022 12:16:00 +0200, Krzysztof Kozlowski wrote:
> The gpio-keys DT schema matches all properties with a wide pattern and
> applies specific schema to children.  This has drawback - all regular
> properties are also matched and are silently ignored, even if they are
> not described in schema.  Basically this allows any non-object property
> to be present.
> 
> Enforce specific naming pattern for children (keys) to narrow the
> pattern thus do not match other properties.  This will require all
> children to be named with 'key-' prefix or '-key' suffix.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../devicetree/bindings/input/gpio-keys.yaml  | 169 +++++++++---------
>  1 file changed, 83 insertions(+), 86 deletions(-)
> 

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:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/nuvoton,wpcm450-pinctrl.example.dtb: gpio-keys: 'uid' does not match any of the regexes: '^(key|key-[a-z0-9-]+|[a-z0-9-]+-key)$', 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/input/gpio-keys.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/input/gpio-keys.example.dtb: gpio-keys: 'autorepeat', 'down', 'up' do not match any of the regexes: '^(key|key-[a-z0-9-]+|[a-z0-9-]+-key)$', 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/input/gpio-keys.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

Re: [RFC PATCH 1/2] dt-bindings: input: gpio-keys: enforce node names to match all properties

[Jeff LaBundy]

Hi Krzysztof,

On Fri, Jun 03, 2022 at 12:16:00PM +0200, Krzysztof Kozlowski wrote:
> The gpio-keys DT schema matches all properties with a wide pattern and
> applies specific schema to children.  This has drawback - all regular
> properties are also matched and are silently ignored, even if they are
> not described in schema.  Basically this allows any non-object property
> to be present.
> 
> Enforce specific naming pattern for children (keys) to narrow the
> pattern thus do not match other properties.  This will require all
> children to be named with 'key-' prefix or '-key' suffix.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../devicetree/bindings/input/gpio-keys.yaml  | 169 +++++++++---------
>  1 file changed, 83 insertions(+), 86 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
> index 93f601c58984..49d388dc8d78 100644
> --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
> +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
> @@ -16,92 +16,89 @@ properties:
>        - gpio-keys-polled
>  
>  patternProperties:
> -  ".*":
> -    if:
> -      type: object
> -    then:
> -      $ref: input.yaml#
> -
> -      properties:
> -        gpios:
> -          maxItems: 1
> -
> -        interrupts:
> -          maxItems: 1
> -
> -        label:
> -          description: Descriptive name of the key.
> -
> -        linux,code:
> -          description: Key / Axis code to emit.
> -          $ref: /schemas/types.yaml#/definitions/uint32
> -
> -        linux,input-type:
> -          description:
> -            Specify event type this button/key generates. If not specified defaults to
> -            <1> == EV_KEY.
> -          $ref: /schemas/types.yaml#/definitions/uint32
> -
> -          default: 1
> -
> -        linux,input-value:
> -          description: |
> -            If linux,input-type is EV_ABS or EV_REL then this
> -            value is sent for events this button generates when pressed.
> -            EV_ABS/EV_REL axis will generate an event with a value of 0
> -            when all buttons with linux,input-type == type and
> -            linux,code == axis are released. This value is interpreted
> -            as a signed 32 bit value, e.g. to make a button generate a
> -            value of -1 use:
> -
> -            linux,input-value = <0xffffffff>; /* -1 */
> -
> -          $ref: /schemas/types.yaml#/definitions/uint32
> -
> -        debounce-interval:
> -          description:
> -            Debouncing interval time in milliseconds. If not specified defaults to 5.
> -          $ref: /schemas/types.yaml#/definitions/uint32
> -
> -          default: 5
> -
> -        wakeup-source:
> -          description: Button can wake-up the system.
> -
> -        wakeup-event-action:
> -          description: |
> -            Specifies whether the key should wake the system when asserted, when
> -            deasserted, or both. This property is only valid for keys that wake up the
> -            system (e.g., when the "wakeup-source" property is also provided).
> -
> -            Supported values are defined in linux-event-codes.h:
> -
> -              EV_ACT_ANY        - both asserted and deasserted
> -              EV_ACT_ASSERTED   - asserted
> -              EV_ACT_DEASSERTED - deasserted
> -          $ref: /schemas/types.yaml#/definitions/uint32
> -          enum: [0, 1, 2]
> -
> -        linux,can-disable:
> -          description:
> -            Indicates that button is connected to dedicated (not shared) interrupt
> -            which can be disabled to suppress events from the button.
> -          type: boolean
> -
> -      required:
> -        - linux,code
> -
> -      anyOf:
> -        - required:
> -            - interrupts
> -        - required:
> -            - gpios
> -
> -      dependencies:
> -        wakeup-event-action: [ wakeup-source ]
> -        linux,input-value: [ gpios ]
> -
> -      unevaluatedProperties: false
> +  "^(key|key-[a-z0-9-]+|[a-z0-9-]+-key)$":

Maybe this would be better as:

"^((key|switch|axis)|(key|switch|axis)-[a-z0-9-]+|[a-z0-9-]+-(key|switch|axis))$":

...or perhaps a more efficient version of my counter-proposal.

The reason is because it is confusing to see a lid or dock switch named
as "key-lid", etc.

> +    $ref: input.yaml#
> +
> +    properties:
> +      gpios:
> +        maxItems: 1
> +
> +      interrupts:
> +        maxItems: 1
> +
> +      label:
> +        description: Descriptive name of the key.
> +
> +      linux,code:
> +        description: Key / Axis code to emit.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +      linux,input-type:
> +        description:
> +          Specify event type this button/key generates. If not specified defaults to
> +          <1> == EV_KEY.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +        default: 1
> +
> +      linux,input-value:
> +        description: |
> +          If linux,input-type is EV_ABS or EV_REL then this
> +          value is sent for events this button generates when pressed.
> +          EV_ABS/EV_REL axis will generate an event with a value of 0
> +          when all buttons with linux,input-type == type and
> +          linux,code == axis are released. This value is interpreted
> +          as a signed 32 bit value, e.g. to make a button generate a
> +          value of -1 use:
> +
> +          linux,input-value = <0xffffffff>; /* -1 */
> +
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +      debounce-interval:
> +        description:
> +          Debouncing interval time in milliseconds. If not specified defaults to 5.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +        default: 5
> +
> +      wakeup-source:
> +        description: Button can wake-up the system.
> +
> +      wakeup-event-action:
> +        description: |
> +          Specifies whether the key should wake the system when asserted, when
> +          deasserted, or both. This property is only valid for keys that wake up the
> +          system (e.g., when the "wakeup-source" property is also provided).
> +
> +          Supported values are defined in linux-event-codes.h:
> +
> +            EV_ACT_ANY        - both asserted and deasserted
> +            EV_ACT_ASSERTED   - asserted
> +            EV_ACT_DEASSERTED - deasserted
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [0, 1, 2]
> +
> +      linux,can-disable:
> +        description:
> +          Indicates that button is connected to dedicated (not shared) interrupt
> +          which can be disabled to suppress events from the button.
> +        type: boolean
> +
> +    required:
> +      - linux,code
> +
> +    anyOf:
> +      - required:
> +          - interrupts
> +      - required:
> +          - gpios
> +
> +    dependencies:
> +      wakeup-event-action: [ wakeup-source ]
> +      linux,input-value: [ gpios ]
> +
> +    unevaluatedProperties: false
>  
>  if:
>    properties:
> -- 
> 2.34.1
> 

Kind regards,
Jeff LaBundy

Re: [RFC PATCH 1/2] dt-bindings: input: gpio-keys: enforce node names to match all properties

[Krzysztof Kozlowski]

On 04/06/2022 05:04, Jeff LaBundy wrote:
>> -      dependencies:
>> -        wakeup-event-action: [ wakeup-source ]
>> -        linux,input-value: [ gpios ]
>> -
>> -      unevaluatedProperties: false
>> +  "^(key|key-[a-z0-9-]+|[a-z0-9-]+-key)$":
> 
> Maybe this would be better as:
> 
> "^((key|switch|axis)|(key|switch|axis)-[a-z0-9-]+|[a-z0-9-]+-(key|switch|axis))$":
> 
> ...or perhaps a more efficient version of my counter-proposal.
> 
> The reason is because it is confusing to see a lid or dock switch named
> as "key-lid", etc.

Nice point. "switch" I understand, but can you really have "axis" on
GPIO keys? I had impression axis is related to joysticks.


Best regards,
Krzysztof

Re: [RFC PATCH 1/2] dt-bindings: input: gpio-keys: enforce node names to match all properties

[Jeff LaBundy]

Hi Krzysztof,

On Sun, Jun 05, 2022 at 05:12:42PM +0200, Krzysztof Kozlowski wrote:
> On 04/06/2022 05:04, Jeff LaBundy wrote:
> >> -      dependencies:
> >> -        wakeup-event-action: [ wakeup-source ]
> >> -        linux,input-value: [ gpios ]
> >> -
> >> -      unevaluatedProperties: false
> >> +  "^(key|key-[a-z0-9-]+|[a-z0-9-]+-key)$":
> > 
> > Maybe this would be better as:
> > 
> > "^((key|switch|axis)|(key|switch|axis)-[a-z0-9-]+|[a-z0-9-]+-(key|switch|axis))$":
> > 
> > ...or perhaps a more efficient version of my counter-proposal.
> > 
> > The reason is because it is confusing to see a lid or dock switch named
> > as "key-lid", etc.
> 
> Nice point. "switch" I understand, but can you really have "axis" on
> GPIO keys? I had impression axis is related to joysticks.

I do not think it is very common, but technically we can use gpio-keys
to create coarse sliders as follows:

- linux,code = ABS_X (or ABS_Y, etc.)
- linux,input-type = EV_ABS
- linux,input-value = 0, 10, 20...

Trying to encode all possible values for linux,input-type (EV_*) in the
pattern is not reasonable, so maybe a compromise would be to use 'event'
in place of 'key|switch' because events are what we are ultimately trying
to describe here.

That being said, these are special cases and I don't feel strongly against
simply using 'key|switch' for now as those are by far the most common use-
cases. Another compromise is 'key|switch|event', with 'event' available as
a catch-all for these special cases.

> 
> 
> Best regards,
> Krzysztof

Kind regards,
Jeff LaBundy

[RFC PATCH 2/2] dt-bindings: input: gpio-keys: document label and autorepeat properties

[Krzysztof Kozlowski]

The original text bindings documented "autorepeat" and "label"
properties (in the device node, beside the nodes with keys).

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/input/gpio-keys.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
index 49d388dc8d78..b1c910a5e233 100644
--- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
@@ -15,6 +15,14 @@ properties:
       - gpio-keys
       - gpio-keys-polled
 
+  autorepeat:
+    type: boolean
+    description:
+      Enable operating system (not hardware) key auto repeat feature.
+
+  label:
+    description: Name of entire device
+
 patternProperties:
   "^(key|key-[a-z0-9-]+|[a-z0-9-]+-key)$":
     $ref: input.yaml#
-- 
2.34.1

Re: [RFC PATCH 2/2] dt-bindings: input: gpio-keys: document label and autorepeat properties

[Dmitry Torokhov]

On Fri, Jun 03, 2022 at 12:16:01PM +0200, Krzysztof Kozlowski wrote:
> The original text bindings documented "autorepeat" and "label"
> properties (in the device node, beside the nodes with keys).
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  Documentation/devicetree/bindings/input/gpio-keys.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
> index 49d388dc8d78..b1c910a5e233 100644
> --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
> +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
> @@ -15,6 +15,14 @@ properties:
>        - gpio-keys
>        - gpio-keys-polled
>  
> +  autorepeat:
> +    type: boolean
> +    description:
> +      Enable operating system (not hardware) key auto repeat feature.

Should we refer to the generic input device property here instead (one
on described in input.yaml)?

Thanks.

-- 
Dmitry

Re: [RFC PATCH 2/2] dt-bindings: input: gpio-keys: document label and autorepeat properties

[Krzysztof Kozlowski]

On 03/06/2022 18:43, Dmitry Torokhov wrote:
> On Fri, Jun 03, 2022 at 12:16:01PM +0200, Krzysztof Kozlowski wrote:
>> The original text bindings documented "autorepeat" and "label"
>> properties (in the device node, beside the nodes with keys).
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  Documentation/devicetree/bindings/input/gpio-keys.yaml | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
>> index 49d388dc8d78..b1c910a5e233 100644
>> --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
>> +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
>> @@ -15,6 +15,14 @@ properties:
>>        - gpio-keys
>>        - gpio-keys-polled
>>  
>> +  autorepeat:
>> +    type: boolean
>> +    description:
>> +      Enable operating system (not hardware) key auto repeat feature.
> 
> Should we refer to the generic input device property here instead (one
> on described in input.yaml)?

You mean copy the description from input.yaml or say something like:
"see input.yaml"?


Best regards,
Krzysztof

Re: [RFC PATCH 2/2] dt-bindings: input: gpio-keys: document label and autorepeat properties

[Rob Herring]

On Sun, Jun 5, 2022 at 9:15 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 03/06/2022 18:43, Dmitry Torokhov wrote:
> > On Fri, Jun 03, 2022 at 12:16:01PM +0200, Krzysztof Kozlowski wrote:
> >> The original text bindings documented "autorepeat" and "label"
> >> properties (in the device node, beside the nodes with keys).
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >>  Documentation/devicetree/bindings/input/gpio-keys.yaml | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
> >> index 49d388dc8d78..b1c910a5e233 100644
> >> --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
> >> +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
> >> @@ -15,6 +15,14 @@ properties:
> >>        - gpio-keys
> >>        - gpio-keys-polled
> >>
> >> +  autorepeat:
> >> +    type: boolean
> >> +    description:
> >> +      Enable operating system (not hardware) key auto repeat feature.
> >
> > Should we refer to the generic input device property here instead (one
> > on described in input.yaml)?
>
> You mean copy the description from input.yaml or say something like:
> "see input.yaml"?

No, just:

$ref: input.yaml#
properties:
  autorepeat: true

And 'poll-interval' needs its definition removed.

It's a bit strange for input.yaml to be referenced in both the parent
and child nodes, but that's the nature of the input bindings. Maybe
input.yaml could be split? Doesn't really look like it to me. The main
issue with one file is the users need to list out which properties
they use (not a bad thing).

Note that this series (patch 1) is going to conflict with what I just
sent out[1].

Rob

[1] https://lore.kernel.org/all/20220608211207.2058487-1-robh@kernel.org/

Re: [RFC PATCH 2/2] dt-bindings: input: gpio-keys: document label and autorepeat properties

[Krzysztof Kozlowski]

On 08/06/2022 23:20, Rob Herring wrote:
> On Sun, Jun 5, 2022 at 9:15 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 03/06/2022 18:43, Dmitry Torokhov wrote:
>>> On Fri, Jun 03, 2022 at 12:16:01PM +0200, Krzysztof Kozlowski wrote:
>>>> The original text bindings documented "autorepeat" and "label"
>>>> properties (in the device node, beside the nodes with keys).
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/input/gpio-keys.yaml | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
>>>> index 49d388dc8d78..b1c910a5e233 100644
>>>> --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
>>>> +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
>>>> @@ -15,6 +15,14 @@ properties:
>>>>        - gpio-keys
>>>>        - gpio-keys-polled
>>>>
>>>> +  autorepeat:
>>>> +    type: boolean
>>>> +    description:
>>>> +      Enable operating system (not hardware) key auto repeat feature.
>>>
>>> Should we refer to the generic input device property here instead (one
>>> on described in input.yaml)?
>>
>> You mean copy the description from input.yaml or say something like:
>> "see input.yaml"?
> 
> No, just:
> 
> $ref: input.yaml#
> properties:
>   autorepeat: true
> 
> And 'poll-interval' needs its definition removed.
> 
> It's a bit strange for input.yaml to be referenced in both the parent
> and child nodes, but that's the nature of the input bindings. Maybe
> input.yaml could be split? Doesn't really look like it to me. The main
> issue with one file is the users need to list out which properties
> they use (not a bad thing).
> 
> Note that this series (patch 1) is going to conflict with what I just
> sent out[1].

I can rebase on top of it.

I understand that idea of the series looks good, so I will work on DTSes
and v2 of this.


Best regards,
Krzysztof