2022-06-06 03:45:40

by Krzysztof Kozlowski

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

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 <[email protected]>
---
.../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


2022-06-06 04:19:51

by Jeff LaBundy

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

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