[PATCH] dt-bindings: input: Convert gpio-keys bindings to schema

[PATCH] dt-bindings: input: Convert gpio-keys bindings to schema

[Rob Herring]

Convert the gpio-keys and gpio-keys-polled bindings to a DT schema. As
both bindings are almost the same, combine them into a single schema.

The binding said 'interrupts' was required, but testing on dts files
showed that it isn't required.

'linux,input-value' was only documented for gpio-keys-polled, but there
doesn't seem to be any reason for it to be specific to that.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../bindings/input/gpio-keys-polled.txt       |  45 ------
 .../devicetree/bindings/input/gpio-keys.txt   |  58 -------
 .../devicetree/bindings/input/gpio-keys.yaml  | 151 ++++++++++++++++++
 3 files changed, 151 insertions(+), 103 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/gpio-keys-polled.txt
 delete mode 100644 Documentation/devicetree/bindings/input/gpio-keys.txt
 create mode 100644 Documentation/devicetree/bindings/input/gpio-keys.yaml

diff --git a/Documentation/devicetree/bindings/input/gpio-keys-polled.txt b/Documentation/devicetree/bindings/input/gpio-keys-polled.txt
deleted file mode 100644
index 4d9a3717eaaf..000000000000
--- a/Documentation/devicetree/bindings/input/gpio-keys-polled.txt
+++ /dev/null
@@ -1,45 +0,0 @@
-Device-Tree bindings for input/gpio_keys_polled.c keyboard driver
-
-Required properties:
-	- compatible = "gpio-keys-polled";
-	- poll-interval: Poll interval time in milliseconds
-
-Optional properties:
-	- autorepeat: Boolean, Enable auto repeat feature of Linux input
-	  subsystem.
-
-Each button (key) is represented as a sub-node of "gpio-keys-polled":
-Subnode properties:
-
-	- gpios: OF device-tree gpio specification.
-	- label: Descriptive name of the key.
-	- linux,code: Key / Axis code to emit.
-
-Optional subnode-properties:
-	- linux,input-type: Specify event type this button/key generates.
-	  If not specified defaults to <1> == EV_KEY.
-	- linux,input-value: 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 */
-	- debounce-interval: Debouncing interval time in milliseconds.
-	  If not specified defaults to 5.
-	- wakeup-source: Boolean, button can wake-up the system.
-			 (Legacy property supported: "gpio-key,wakeup")
-
-Example nodes:
-
-	gpio_keys_polled {
-			compatible = "gpio-keys-polled";
-			poll-interval = <100>;
-			autorepeat;
-
-			button21 {
-				label = "GPIO Key UP";
-				linux,code = <103>;
-				gpios = <&gpio1 0 1>;
-			};
-			...
diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
deleted file mode 100644
index 7cccc49b6bea..000000000000
--- a/Documentation/devicetree/bindings/input/gpio-keys.txt
+++ /dev/null
@@ -1,58 +0,0 @@
-Device-Tree bindings for input/keyboard/gpio_keys.c keyboard driver
-
-Required properties:
-	- compatible = "gpio-keys";
-
-Optional properties:
-	- autorepeat: Boolean, Enable auto repeat feature of Linux input
-	  subsystem.
-	- label: String, name of the input device.
-
-Each button (key) is represented as a sub-node of "gpio-keys":
-Subnode properties:
-
-	- gpios: OF device-tree gpio specification.
-	- interrupts: the interrupt line for that input.
-	- label: Descriptive name of the key.
-	- linux,code: Keycode to emit.
-
-Note that either "interrupts" or "gpios" properties can be omitted, but not
-both at the same time. Specifying both properties is allowed.
-
-Optional subnode-properties:
-	- linux,input-type: Specify event type this button/key generates.
-	  If not specified defaults to <1> == EV_KEY.
-	- debounce-interval: Debouncing interval time in milliseconds.
-	  If not specified defaults to 5.
-	- wakeup-source: Boolean, button can wake-up the system.
-			 (Legacy property supported: "gpio-key,wakeup")
-	- wakeup-event-action: 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_ASSERTED		- asserted
-		EV_ACT_DEASSERTED	- deasserted
-		EV_ACT_ANY		- both asserted and deasserted
-	- linux,can-disable: Boolean, indicates that button is connected
-	  to dedicated (not shared) interrupt which can be disabled to
-	  suppress events from the button.
-
-Example nodes:
-
-	gpio-keys {
-			compatible = "gpio-keys";
-			autorepeat;
-
-			up {
-				label = "GPIO Key UP";
-				linux,code = <103>;
-				gpios = <&gpio1 0 1>;
-			};
-
-			down {
-				label = "GPIO Key DOWN";
-				linux,code = <108>;
-				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
-			};
-			...
diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
new file mode 100644
index 000000000000..05b96a17af21
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
@@ -0,0 +1,151 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/gpio-keys.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Device-Tree bindings for GPIO attached keys
+
+maintainers:
+  - Rob Herring <robh@kernel.org>
+
+properties:
+  compatible:
+    enum:
+      - gpio-keys
+      - gpio-keys-polled
+
+patternProperties:
+  ".*":
+    if:
+      type: object
+    then:
+      allOf:
+        - $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.
+          allOf:
+            - $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 */
+
+          allOf:
+            - $ref: /schemas/types.yaml#definitions/uint32
+
+        debounce-interval:
+          description:
+            Debouncing interval time in milliseconds. If not specified defaults to 5.
+          allOf:
+            - $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
+          allOf:
+            - $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
+
+        pinctrl-0:
+          maxItems: 1
+
+        pinctrl-names:
+          maxItems: 1
+
+      required:
+        - linux,code
+
+      anyOf:
+        - required:
+            - interrupts
+        - required:
+            - gpios
+
+      dependencies:
+        wakeup-event-action: [ wakeup-source ]
+
+      unevaluatedProperties: false
+
+if:
+  properties:
+    compatible:
+      const: gpio-keys-polled
+then:
+  properties:
+    poll-interval:
+      description:
+        Poll interval time in milliseconds
+      $ref: /schemas/types.yaml#definitions/uint32
+
+  required:
+    - poll-interval
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    gpio-keys {
+        compatible = "gpio-keys";
+        autorepeat;
+
+        up {
+            label = "GPIO Key UP";
+            linux,code = <103>;
+            gpios = <&gpio1 0 1>;
+        };
+
+        down {
+            label = "GPIO Key DOWN";
+            linux,code = <108>;
+            interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
+        };
+    };
+
+...
-- 
2.20.1

Re: [PATCH] dt-bindings: input: Convert gpio-keys bindings to schema

[Dmitry Torokhov]

Hi Rob,

On Thu, Jan 23, 2020 at 03:42:22PM -0600, Rob Herring wrote:
> Convert the gpio-keys and gpio-keys-polled bindings to a DT schema. As
> both bindings are almost the same, combine them into a single schema.
> 
> The binding said 'interrupts' was required, but testing on dts files
> showed that it isn't required.
> 
> 'linux,input-value' was only documented for gpio-keys-polled, but there
> doesn't seem to be any reason for it to be specific to that.

Actually, there is: with gpio-keys-polled we take a "snapshot" of the
entire device state, so we know when to generate a 0 event (the example
we have a device with several GPIOs with values assigned 1, 2, 3, 4, 5..
values, when one of the gpios is active we generate event with given
value, when all are inactive we generate 0 event). This does not work
for interrupt-only driven device.

> +
> +      properties:
> +        gpios:
> +          maxItems: 1
> +
> +        interrupts:
> +          maxItems: 1

We support "interrupt-only" mode where we do not have GPIO, so for
"gpio-keys" we need either interrupts or gpios or both, and for polled
we must have gpios (and I guess we simply ignore interrupts if they are
specified).

Does this binding manages to enforce this?

Thanks.

-- 
Dmitry

Re: [PATCH] dt-bindings: input: Convert gpio-keys bindings to schema

[Rob Herring]

On Thu, Jan 23, 2020 at 4:25 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Rob,
>
> On Thu, Jan 23, 2020 at 03:42:22PM -0600, Rob Herring wrote:
> > Convert the gpio-keys and gpio-keys-polled bindings to a DT schema. As
> > both bindings are almost the same, combine them into a single schema.
> >
> > The binding said 'interrupts' was required, but testing on dts files
> > showed that it isn't required.
> >
> > 'linux,input-value' was only documented for gpio-keys-polled, but there
> > doesn't seem to be any reason for it to be specific to that.
>
> Actually, there is: with gpio-keys-polled we take a "snapshot" of the
> entire device state, so we know when to generate a 0 event (the example
> we have a device with several GPIOs with values assigned 1, 2, 3, 4, 5..
> values, when one of the gpios is active we generate event with given
> value, when all are inactive we generate 0 event). This does not work
> for interrupt-only driven device.

Okay, it wasn't clear to me reading the binding doc. I'll make it conditional.


> > +      properties:
> > +        gpios:
> > +          maxItems: 1

> > +
> > +        interrupts:
> > +          maxItems: 1
>
> We support "interrupt-only" mode where we do not have GPIO, so for
> "gpio-keys" we need either interrupts or gpios or both, and for polled
> we must have gpios (and I guess we simply ignore interrupts if they are
> specified).
>
> Does this binding manages to enforce this?

Yes, this hunk does that:

+      anyOf:
+        - required:
+            - interrupts
+        - required:
+            - gpios

Rob

Re: [PATCH] dt-bindings: input: Convert gpio-keys bindings to schema

[Dmitry Torokhov]

On Thu, Jan 23, 2020 at 07:35:06PM -0600, Rob Herring wrote:
> On Thu, Jan 23, 2020 at 4:25 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hi Rob,
> >
> > On Thu, Jan 23, 2020 at 03:42:22PM -0600, Rob Herring wrote:
> > > Convert the gpio-keys and gpio-keys-polled bindings to a DT schema. As
> > > both bindings are almost the same, combine them into a single schema.
> > >
> > > The binding said 'interrupts' was required, but testing on dts files
> > > showed that it isn't required.
> > >
> > > 'linux,input-value' was only documented for gpio-keys-polled, but there
> > > doesn't seem to be any reason for it to be specific to that.
> >
> > Actually, there is: with gpio-keys-polled we take a "snapshot" of the
> > entire device state, so we know when to generate a 0 event (the example
> > we have a device with several GPIOs with values assigned 1, 2, 3, 4, 5..
> > values, when one of the gpios is active we generate event with given
> > value, when all are inactive we generate 0 event). This does not work
> > for interrupt-only driven device.
> 
> Okay, it wasn't clear to me reading the binding doc. I'll make it conditional.

Actually, I think we can make it usable in interrupt-driver driver. For
EV_REL events we do not need to "go back to 0", and for EV_ABS, if
desired, we could allow specifying an option to scan all GPIOs
on any interrupt. This obviously will not work for pure interrupt
devices (where we do not have GPIOs).

Thanks.

-- 
Dmitry