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

[Jeff LaBundy <jeff@labundy.com>]

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