2022-06-06 03:45:17

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 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 <[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)$":

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


2022-06-06 06:09:41

by Krzysztof Kozlowski

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

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