Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Fri, 3 Jun 2022 22:04:55 -0500
From:   Jeff LaBundy <jeff@labundy.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-input@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Stefan Hansson <newbie13xd@gmail.com>,
        Andreas Kemnade <andreas@kemnade.info>
Subject: Re: [RFC PATCH 1/2] dt-bindings: input: gpio-keys: enforce node
 names to match all properties
Message-ID: <20220604030455.GA12308@nixie71>
References: <20220603101601.542054-1-krzysztof.kozlowski@linaro.org>
 <20220603101601.542054-2-krzysztof.kozlowski@linaro.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220603101601.542054-2-krzysztof.kozlowski@linaro.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?jM6IOViHCwJM0ymS4tKWPGOMa+XKyJ1I0+d2hYYSq4y4xqyTB42Wg0TO2Lq8?=
 =?us-ascii?Q?J7SQyQIj3r6IsLdfsqtO1U6VDKusSo9Tm4RTsdX+EmRvL6Xoj2IyRyDFxQ/M?=
 =?us-ascii?Q?8aNnaLrIw+EgA0FJvq4zuAovh5VFZfJEoXy+31stiGIvrbyQHt86oh6B6+oX?=
 =?us-ascii?Q?8hk3qjXhoboGfZNzy+rpV9AqrByMMuH7FE4IwTdojs5gl//pWLhYGMAgfLnk?=
 =?us-ascii?Q?UphKOC5T02KJ4KlwKIQOcmsTwN41JNYariISC04mT14LYolivMNIJSicY7Yn?=
 =?us-ascii?Q?PIL7hPbtCRu+iEJW/KKZ4Lg2p+zLZEGM/s3Zyk+oVvRImbX3N7GbJKwKr1QP?=
 =?us-ascii?Q?wlF2BAYkIYeqr6B1ByD7zN2XQlKsRqD3oQjvOx86XjuXrI2B0m+32xPeB9IB?=
 =?us-ascii?Q?p4zcMY3HgaVhBcJ34IkaxRx9lV5zj9D5sJsp2BFskFtNHkiA2+pstJDjfiD1?=
 =?us-ascii?Q?Ou1M+lmzkT4zsklNFI+nZK6ZMsQx0Nm9ponzWJeTUK9kKVwfSp5I442+Ve9/?=
 =?us-ascii?Q?fx7wndkjqFYBYnHtTgCt9GENDKj7HAevs3EbIo8Y8X1MoCYelpaJxaYNmsOB?=
 =?us-ascii?Q?EBUkWmy4/RqGfccYm5yQeMBy57z0+sY33tapbTp6F+o1lRCWLzx+NyUbvTd0?=
 =?us-ascii?Q?DiXxbhpjjJ/1Wb8faRwF3UUUrAGDzIWYsnG1P/ii8fhuxBy4AOWcDIABtVX8?=
 =?us-ascii?Q?dPcf+FSNwRAxExEKNDR/KZqoQJ/pyMZhoYYvtlNmuaO+/S8j110snAFznP+d?=
 =?us-ascii?Q?nJyHXRt41hV+RjHPqCqAeG0RwSZNdhHGX0cV3XRdgRvmJPPQrb+LKIZoYkg4?=
 =?us-ascii?Q?vqT/go6pxAFzn11P6m8XCllp4Z2+PkO1SvnGwR3e6TP0cgEHJRwNRHc12Z3+?=
 =?us-ascii?Q?XJHhx6fNR+0FP3ddc5MmKZHGbU6ltjxTUz7hGypkrKqnjexlfb1yE5OiSElt?=
 =?us-ascii?Q?isrAuk5e/pzN5tyyZvDT90H7IgqIb1FfYLYbOXPHhVm32dITau1ax5YY1e6m?=
 =?us-ascii?Q?HuUaL7ifkmr6NWya9jiNh1FqjQDS3wJbuobkYwEw9A3i/FTlm53ddpBGpUpA?=
 =?us-ascii?Q?Hj1OC6LXecM2l5g3+eNR//gCMRHwG3TyHTWTsv/y+Picgkkemx+JbdnAGYBX?=
 =?us-ascii?Q?PJqEco46o9vppI/fd9e4PENs/+CWIAaUt1HztHA20y6VtirSCS9ILXUdYPJV?=
 =?us-ascii?Q?t6HQ+uD9R0g/epu+4aocPTlkMnB0xezuqM1cB4pi9a34vbtYk0kqkT4DSqpC?=
 =?us-ascii?Q?jszG82Q5l5Rfe1vQ7OqcfDD/zLcF+wfxAfoZnSP2tPLwXz9MhHlsprhvaw6A?=
 =?us-ascii?Q?L/HoFBExHJ0m8JlqfDkbp7znwSoFqeZqkHgBDgKlKQ7LMP/U/q1383LrC3BO?=
 =?us-ascii?Q?+c9WFL8cPe2wmZLwKRzyjrfPcV1dk0YZIxF/qqrFlPRPCyyFQDTPyLsb1h8O?=
 =?us-ascii?Q?GsG692h/qRfBCUqDQQYEss6jXYp1ICTD4PT7+BFDSkQpX6O36UFWIAbjbyeR?=
 =?us-ascii?Q?gnXR44nw2Rg28oA+Y56D0NfZsb9L48ykxym6yKurvJu3+CmaIxoXH/b8J4V6?=
 =?us-ascii?Q?38xqQ0VuxTVEf/gAcw2QHA0ggJWniOgWJ/jxiGuCWij5F+JtXL0uUPs0Asyh?=
 =?us-ascii?Q?GlsHibjvGQOrOA68bcRWAY9eYxC929RvN0olzee18f+QWt18UG3M3b13JXtu?=
 =?us-ascii?Q?sb2rDx3iolkGKJ6Va2Rj19pTwEhY+2LTKRDNnTenBq+nWsHp4t9F3zFOvOik?=
 =?us-ascii?Q?USv2QqfeYA=3D=3D?=
X-OriginatorOrg: labundy.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1bb51037-936a-45bb-2da3-08da45d70400
X-MS-Exchange-CrossTenant-AuthSource: SN4PR0801MB3774.namprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jun 2022 03:05:01.7658
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 00b69d09-acab-4585-aca7-8fb7c6323e6f
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gDN9K7fG5XsUKo32W1ntBZMkIzK1SQPiyPJpQkpeCc6iBWXobbEtpMCBvng/bw145X7KaSf9VgFpw5SQbLIuVQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR08MB5257
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RDNS_NONE,
	SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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