[PATCHv2 0/4] Rotary Encoder Push Button Support

[PATCHv2 0/4] Rotary Encoder Push Button Support

[Sebastian Reichel]

Hi,

The aim of this series is to add support for reporting push
events from rotary encoders with integrated push button
functionality (which are quite common).

I added a few more additional cleanup patches, since I worked
on the driver anyways.

Changes since PATCHv1 [0]:
 * Added patch converting the binding to YAML
 * Added patch fixing the steps default value
 * Added patch introducing dev_err_probe usage
 * Updated gpio push button patch from Mylène
  - use linux,push-code to be clear this is about the push button
  - add linux,push-type to support switches in addition to buttons
  - cleanup code a bit
   o 100 character line length
   o use dev_err_probe()
  - use EV_KEY and KEY_ENTER as default and make properties optional
  - use push-gpios instead of push-gpio in binding

[0] https://lore.kernel.org/linux-input/20190614133651.28396-1-mylene.josserand@bootlin.com/

-- Sebastian

Mylène Josserand (1):
  Input: rotary-encoder - Add gpio as push button

Sebastian Reichel (3):
  dt-bindings: input: Convert rotary-encoder bindings to schema
  Input: rotary-encoder - Fix steps property reading
  Input: rotary-encoder - Use dev_err_probe

 .../bindings/input/rotary-encoder.txt         |  50 --------
 .../bindings/input/rotary-encoder.yaml        | 121 ++++++++++++++++++
 drivers/input/misc/rotary_encoder.c           |  56 +++++++-
 3 files changed, 171 insertions(+), 56 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/rotary-encoder.txt
 create mode 100644 Documentation/devicetree/bindings/input/rotary-encoder.yaml

-- 
2.28.0

[PATCHv2 1/4] dt-bindings: input: Convert rotary-encoder bindings to schema

[Sebastian Reichel]

Convert rotary-encoder bindings to YAML schema.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 .../bindings/input/rotary-encoder.txt         |  50 ---------
 .../bindings/input/rotary-encoder.yaml        | 100 ++++++++++++++++++
 2 files changed, 100 insertions(+), 50 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/rotary-encoder.txt
 create mode 100644 Documentation/devicetree/bindings/input/rotary-encoder.yaml

diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
deleted file mode 100644
index a644408b33b8..000000000000
--- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
+++ /dev/null
@@ -1,50 +0,0 @@
-Rotary encoder DT bindings
-
-Required properties:
-- gpios: a spec for at least two GPIOs to be used, most significant first
-
-Optional properties:
-- linux,axis: the input subsystem axis to map to this rotary encoder.
-  Defaults to 0 (ABS_X / REL_X)
-- rotary-encoder,steps: Number of steps in a full turnaround of the
-  encoder. Only relevant for absolute axis. Defaults to 24 which is a
-  typical value for such devices.
-- rotary-encoder,relative-axis: register a relative axis rather than an
-  absolute one. Relative axis will only generate +1/-1 events on the input
-  device, hence no steps need to be passed.
-- rotary-encoder,rollover: Automatic rollover when the rotary value becomes
-  greater than the specified steps or smaller than 0. For absolute axis only.
-- rotary-encoder,steps-per-period: Number of steps (stable states) per period.
-  The values have the following meaning:
-  1: Full-period mode (default)
-  2: Half-period mode
-  4: Quarter-period mode
-- wakeup-source: Boolean, rotary encoder can wake up the system.
-- rotary-encoder,encoding: String, the method used to encode steps.
-  Supported are "gray" (the default and more common) and "binary".
-
-Deprecated properties:
-- rotary-encoder,half-period: Makes the driver work on half-period mode.
-  This property is deprecated. Instead, a 'steps-per-period ' value should
-  be used, such as "rotary-encoder,steps-per-period = <2>".
-
-See Documentation/input/devices/rotary-encoder.rst for more information.
-
-Example:
-
-		rotary@0 {
-			compatible = "rotary-encoder";
-			gpios = <&gpio 19 1>, <&gpio 20 0>; /* GPIO19 is inverted */
-			linux,axis = <0>; /* REL_X */
-			rotary-encoder,encoding = "gray";
-			rotary-encoder,relative-axis;
-		};
-
-		rotary@1 {
-			compatible = "rotary-encoder";
-			gpios = <&gpio 21 0>, <&gpio 22 0>;
-			linux,axis = <1>; /* ABS_Y */
-			rotary-encoder,steps = <24>;
-			rotary-encoder,encoding = "binary";
-			rotary-encoder,rollover;
-		};
diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.yaml b/Documentation/devicetree/bindings/input/rotary-encoder.yaml
new file mode 100644
index 000000000000..5b60ea86bd62
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/rotary-encoder.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/rotary-encoder.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic Rotary Encoder
+
+maintainers:
+  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
+
+description:
+  See Documentation/input/devices/rotary-encoder.rst for more information.
+
+properties:
+  compatible:
+    const: rotary-encoder
+
+  gpios:
+    minItems: 2
+    description: GPIOs for the rotation signals, most significant first
+
+  linux,axis:
+    description:
+      The input subsystem axis to map to this rotary encoder.
+      Defaults to (ABS_X / REL_X).
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0
+
+  rotary-encoder,rollover:
+    description:
+      Automatic rollover when the rotary value becomes greater than the
+      specified steps or smaller than 0. For absolute axis only.
+    type: boolean
+
+  rotary-encoder,steps-per-period:
+    description: The values have the following meaning
+      1 - Full-period mode
+      2 - Half-period mode
+      4 - Quarter-period mode
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2, 4]
+    default: 1
+
+  wakeup-source:
+    description: Rotary encoder can wake up the system.
+    type: boolean
+
+  rotary-encoder,encoding:
+    description:
+      Method used to encode steps. Gray code is more common.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: ["gray", "binary"]
+    default: "gray"
+
+  rotary-encoder,half-period:
+    description:
+      Deprecated, use "rotary-encoder,steps-per-period = <2>" instead.
+    type: boolean
+    deprecated: True
+
+  rotary-encoder,steps:
+    description:
+      Number of steps in a full turnaround of the encoder. Only relevant
+      for absolute axis. Defaults to 24 which is a typical value for such
+      devices.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 24
+
+  rotary-encoder,relative-axis:
+    description:
+      register a relative axis rather than an absolute one. Relative axis
+      will only generate +1/-1 events on the input device, hence no steps
+      need to be passed.
+    type: boolean
+
+required:
+  - compatible
+  - gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/gpio/gpio.h"
+    #include "dt-bindings/input/input.h"
+    rotary-encoder0 {
+            compatible = "rotary-encoder";
+            gpios = <&gpio 19 GPIO_ACTIVE_LOW>, <&gpio 20 GPIO_ACTIVE_HIGH>;
+            linux,axis = <REL_X>;
+            rotary-encoder,encoding = "gray";
+            rotary-encoder,relative-axis;
+    };
+    rotary-encoder1 {
+            compatible = "rotary-encoder";
+            gpios = <&gpio 21 GPIO_ACTIVE_HIGH>, <&gpio 22 GPIO_ACTIVE_HIGH>;
+            linux,axis = <ABS_Y>;
+            rotary-encoder,encoding = "binary";
+            rotary-encoder,rollover;
+    };
-- 
2.28.0

Re: [PATCHv2 1/4] dt-bindings: input: Convert rotary-encoder bindings to schema

[Rob Herring]

On Mon, Sep 07, 2020 at 10:40:42PM +0200, Sebastian Reichel wrote:
> Convert rotary-encoder bindings to YAML schema.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  .../bindings/input/rotary-encoder.txt         |  50 ---------
>  .../bindings/input/rotary-encoder.yaml        | 100 ++++++++++++++++++
>  2 files changed, 100 insertions(+), 50 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/input/rotary-encoder.txt
>  create mode 100644 Documentation/devicetree/bindings/input/rotary-encoder.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> deleted file mode 100644
> index a644408b33b8..000000000000
> --- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -Rotary encoder DT bindings
> -
> -Required properties:
> -- gpios: a spec for at least two GPIOs to be used, most significant first
> -
> -Optional properties:
> -- linux,axis: the input subsystem axis to map to this rotary encoder.
> -  Defaults to 0 (ABS_X / REL_X)
> -- rotary-encoder,steps: Number of steps in a full turnaround of the
> -  encoder. Only relevant for absolute axis. Defaults to 24 which is a
> -  typical value for such devices.
> -- rotary-encoder,relative-axis: register a relative axis rather than an
> -  absolute one. Relative axis will only generate +1/-1 events on the input
> -  device, hence no steps need to be passed.
> -- rotary-encoder,rollover: Automatic rollover when the rotary value becomes
> -  greater than the specified steps or smaller than 0. For absolute axis only.
> -- rotary-encoder,steps-per-period: Number of steps (stable states) per period.
> -  The values have the following meaning:
> -  1: Full-period mode (default)
> -  2: Half-period mode
> -  4: Quarter-period mode
> -- wakeup-source: Boolean, rotary encoder can wake up the system.
> -- rotary-encoder,encoding: String, the method used to encode steps.
> -  Supported are "gray" (the default and more common) and "binary".
> -
> -Deprecated properties:
> -- rotary-encoder,half-period: Makes the driver work on half-period mode.
> -  This property is deprecated. Instead, a 'steps-per-period ' value should
> -  be used, such as "rotary-encoder,steps-per-period = <2>".
> -
> -See Documentation/input/devices/rotary-encoder.rst for more information.
> -
> -Example:
> -
> -		rotary@0 {
> -			compatible = "rotary-encoder";
> -			gpios = <&gpio 19 1>, <&gpio 20 0>; /* GPIO19 is inverted */
> -			linux,axis = <0>; /* REL_X */
> -			rotary-encoder,encoding = "gray";
> -			rotary-encoder,relative-axis;
> -		};
> -
> -		rotary@1 {
> -			compatible = "rotary-encoder";
> -			gpios = <&gpio 21 0>, <&gpio 22 0>;
> -			linux,axis = <1>; /* ABS_Y */
> -			rotary-encoder,steps = <24>;
> -			rotary-encoder,encoding = "binary";
> -			rotary-encoder,rollover;
> -		};
> diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.yaml b/Documentation/devicetree/bindings/input/rotary-encoder.yaml
> new file mode 100644
> index 000000000000..5b60ea86bd62
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/rotary-encoder.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/rotary-encoder.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic Rotary Encoder
> +
> +maintainers:
> +  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
> +
> +description:
> +  See Documentation/input/devices/rotary-encoder.rst for more information.
> +
> +properties:
> +  compatible:
> +    const: rotary-encoder
> +
> +  gpios:
> +    minItems: 2

You need to specify maxItems too. A 'should be enough for everyone'TM 
value is fine here if there's no real max.

> +    description: GPIOs for the rotation signals, most significant first
> +
> +  linux,axis:
> +    description:
> +      The input subsystem axis to map to this rotary encoder.
> +      Defaults to (ABS_X / REL_X).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 0
> +
> +  rotary-encoder,rollover:
> +    description:
> +      Automatic rollover when the rotary value becomes greater than the
> +      specified steps or smaller than 0. For absolute axis only.
> +    type: boolean
> +
> +  rotary-encoder,steps-per-period:
> +    description: The values have the following meaning
> +      1 - Full-period mode
> +      2 - Half-period mode
> +      4 - Quarter-period mode

You need a '|' after 'descripton' to preserve formatting here.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2, 4]
> +    default: 1
> +
> +  wakeup-source:
> +    description: Rotary encoder can wake up the system.
> +    type: boolean
> +
> +  rotary-encoder,encoding:
> +    description:
> +      Method used to encode steps. Gray code is more common.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: ["gray", "binary"]
> +    default: "gray"

Don't need quotes.

> +
> +  rotary-encoder,half-period:
> +    description:
> +      Deprecated, use "rotary-encoder,steps-per-period = <2>" instead.
> +    type: boolean
> +    deprecated: True
> +
> +  rotary-encoder,steps:
> +    description:
> +      Number of steps in a full turnaround of the encoder. Only relevant
> +      for absolute axis. Defaults to 24 which is a typical value for such
> +      devices.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 24
> +
> +  rotary-encoder,relative-axis:
> +    description:
> +      register a relative axis rather than an absolute one. Relative axis
> +      will only generate +1/-1 events on the input device, hence no steps
> +      need to be passed.
> +    type: boolean
> +
> +required:
> +  - compatible
> +  - gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include "dt-bindings/gpio/gpio.h"
> +    #include "dt-bindings/input/input.h"
> +    rotary-encoder0 {
> +            compatible = "rotary-encoder";
> +            gpios = <&gpio 19 GPIO_ACTIVE_LOW>, <&gpio 20 GPIO_ACTIVE_HIGH>;
> +            linux,axis = <REL_X>;
> +            rotary-encoder,encoding = "gray";
> +            rotary-encoder,relative-axis;
> +    };
> +    rotary-encoder1 {
> +            compatible = "rotary-encoder";
> +            gpios = <&gpio 21 GPIO_ACTIVE_HIGH>, <&gpio 22 GPIO_ACTIVE_HIGH>;
> +            linux,axis = <ABS_Y>;
> +            rotary-encoder,encoding = "binary";
> +            rotary-encoder,rollover;
> +    };
> -- 
> 2.28.0
> 

[PATCHv2 2/4] Input: rotary-encoder - Fix steps property reading

[Sebastian Reichel]

The DT binding specifies 24 being used by default, but driver
initializes to 0. The steps variable will be used for a modulo
operation at a later step, so do not allow 0 (does not make
sense in any case).

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/input/misc/rotary_encoder.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 6d613f2a017c..e9a5dbb10513 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -198,7 +198,12 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 
 	mutex_init(&encoder->access_mutex);
 
+	encoder->steps = 24;
 	device_property_read_u32(dev, "rotary-encoder,steps", &encoder->steps);
+	if (!encoder->steps) {
+		dev_err(dev, "invalid steps setting\n");
+		return -EINVAL;
+	}
 
 	err = device_property_read_u32(dev, "rotary-encoder,steps-per-period",
 				       &steps_per_period);
-- 
2.28.0

[PATCHv2 3/4] Input: rotary-encoder - Use dev_err_probe

[Sebastian Reichel]

Simplify driver a bit by making use of dev_err_probe.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/input/misc/rotary_encoder.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index e9a5dbb10513..16ad86fad7cb 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -241,12 +241,8 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		device_property_read_bool(dev, "rotary-encoder,relative-axis");
 
 	encoder->gpios = devm_gpiod_get_array(dev, NULL, GPIOD_IN);
-	if (IS_ERR(encoder->gpios)) {
-		err = PTR_ERR(encoder->gpios);
-		if (err != -EPROBE_DEFER)
-			dev_err(dev, "unable to get gpios: %d\n", err);
-		return err;
-	}
+	if (IS_ERR(encoder->gpios))
+		return dev_err_probe(dev, PTR_ERR(encoder->gpios), "unable to get gpios\n");
 	if (encoder->gpios->ndescs < 2) {
 		dev_err(dev, "not enough gpios found\n");
 		return -EINVAL;
-- 
2.28.0

Re: [PATCHv2 3/4] Input: rotary-encoder - Use dev_err_probe

[Rob Herring]

On Mon, Sep 7, 2020 at 2:40 PM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Simplify driver a bit by making use of dev_err_probe.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/input/misc/rotary_encoder.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index e9a5dbb10513..16ad86fad7cb 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -241,12 +241,8 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>                 device_property_read_bool(dev, "rotary-encoder,relative-axis");
>
>         encoder->gpios = devm_gpiod_get_array(dev, NULL, GPIOD_IN);
> -       if (IS_ERR(encoder->gpios)) {
> -               err = PTR_ERR(encoder->gpios);
> -               if (err != -EPROBE_DEFER)
> -                       dev_err(dev, "unable to get gpios: %d\n", err);
> -               return err;
> -       }
> +       if (IS_ERR(encoder->gpios))
> +               return dev_err_probe(dev, PTR_ERR(encoder->gpios), "unable to get gpios\n");

I hadn't seen dev_err_probe...

Just FYI, I'm working on a different fix here which is to print errors
in the subsystems instead. We already do this for IRQs, so why not
everything else? The original reason was no resource is sometimes not
an error, but now we have *_optional calls to handle this case for
most all subsystems. It's a coccinelle script (hacked up from
platform_get_irq.cocci) to convert all the drivers.

Rob

Re: [PATCHv2 3/4] Input: rotary-encoder - Use dev_err_probe

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2389 bytes --]

Hi Rob,

On Wed, Sep 09, 2020 at 01:48:49PM -0600, Rob Herring wrote:
> On Mon, Sep 7, 2020 at 2:40 PM Sebastian Reichel
> <sebastian.reichel@collabora.com> wrote:
> >
> > Simplify driver a bit by making use of dev_err_probe.
> >
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> >  drivers/input/misc/rotary_encoder.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> > index e9a5dbb10513..16ad86fad7cb 100644
> > --- a/drivers/input/misc/rotary_encoder.c
> > +++ b/drivers/input/misc/rotary_encoder.c
> > @@ -241,12 +241,8 @@ static int rotary_encoder_probe(struct platform_device *pdev)
> >                 device_property_read_bool(dev, "rotary-encoder,relative-axis");
> >
> >         encoder->gpios = devm_gpiod_get_array(dev, NULL, GPIOD_IN);
> > -       if (IS_ERR(encoder->gpios)) {
> > -               err = PTR_ERR(encoder->gpios);
> > -               if (err != -EPROBE_DEFER)
> > -                       dev_err(dev, "unable to get gpios: %d\n", err);
> > -               return err;
> > -       }
> > +       if (IS_ERR(encoder->gpios))
> > +               return dev_err_probe(dev, PTR_ERR(encoder->gpios), "unable to get gpios\n");
> 
> I hadn't seen dev_err_probe...

It got added in 5.8.

> Just FYI, I'm working on a different fix here which is to print errors
> in the subsystems instead. We already do this for IRQs, so why not
> everything else? The original reason was no resource is sometimes not
> an error, but now we have *_optional calls to handle this case for
> most all subsystems. It's a coccinelle script (hacked up from
> platform_get_irq.cocci) to convert all the drivers.

Makes sense. I suppose dev_err_probe could be used within the
framework(s) and is still useful for those resource frameworks not
having _optional variants.

FYI: There is a bunch of dev_err_probe for all kind of drivers
being send out at the moment. I already received quite a few
for the power-supply subsystem. Be prepared for conflicts.

If this is about this specific instance: No hard feelings, I
only cleaned up the driver a bit while adding new features
and being able to easily test the cleanups on real HW. With
some luck patch 4/4 applies without this one.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCHv2 4/4] Input: rotary-encoder - Add gpio as push button

[Sebastian Reichel]

From: Mylène Josserand <mylene.josserand@collabora.com>

Add the support of a gpio that can be defined as a push button.
Thanks to that, it is possible to emit a keycode in case of a
"push" event, if the rotary supports that.

The keycode to emit is defined using "linux,code" property
(such as in gpio-keys).

Signed-off-by: Mylène Josserand <mylene.josserand@collabora.com>
[code cleanup to current standards and renamed some properties]
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 .../bindings/input/rotary-encoder.yaml        | 21 +++++++++
 drivers/input/misc/rotary_encoder.c           | 43 +++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.yaml b/Documentation/devicetree/bindings/input/rotary-encoder.yaml
index 5b60ea86bd62..682fee104004 100644
--- a/Documentation/devicetree/bindings/input/rotary-encoder.yaml
+++ b/Documentation/devicetree/bindings/input/rotary-encoder.yaml
@@ -74,6 +74,24 @@ properties:
       need to be passed.
     type: boolean
 
+  push-gpios:
+    description: GPIO used as a detection of a push from the rotary-encoder.
+    maxItems: 1
+
+  linux,push-code:
+    description:
+      keycode to emit with the push-gpio of this rotary encoder.
+      If not specified defaults to <28> == KEY_ENTER.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 28
+
+  linux,push-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
+
 required:
   - compatible
   - gpios
@@ -97,4 +115,7 @@ examples:
             linux,axis = <ABS_Y>;
             rotary-encoder,encoding = "binary";
             rotary-encoder,rollover;
+            push-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
+            linux,push-code = <KEY_ENTER>;
+            linux,push-type = <EV_KEY>;
     };
diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 16ad86fad7cb..484042a5afa0 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -47,12 +47,33 @@ struct rotary_encoder {
 
 	unsigned int *irq;
 
+	struct gpio_desc *push_gpio;
+	unsigned int push_code;
+	unsigned int push_type;
+
 	bool armed;
 	signed char dir;	/* 1 - clockwise, -1 - CCW */
 
 	unsigned int last_stable;
 };
 
+static irqreturn_t rotary_push_irq(int irq, void *dev_id)
+{
+	struct rotary_encoder *encoder = dev_id;
+	int val;
+
+	mutex_lock(&encoder->access_mutex);
+
+	val = gpiod_get_value_cansleep(encoder->push_gpio);
+
+	input_report_key(encoder->input, encoder->push_code, val);
+	input_sync(encoder->input);
+
+	mutex_unlock(&encoder->access_mutex);
+
+	return IRQ_HANDLED;
+}
+
 static unsigned int rotary_encoder_get_state(struct rotary_encoder *encoder)
 {
 	int i;
@@ -248,6 +269,16 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	encoder->push_gpio = devm_gpiod_get_optional(dev, "push", GPIOD_IN);
+	if (IS_ERR(encoder->push_gpio))
+		return dev_err_probe(dev, PTR_ERR(encoder->push_gpio), "failed to get push-gpio\n");
+
+	encoder->push_code = KEY_ENTER;
+	device_property_read_u32(dev, "linux,push-code", &encoder->push_code);
+
+	encoder->push_type = EV_KEY;
+	device_property_read_u32(dev, "linux,push-type", &encoder->push_type);
+
 	input = devm_input_allocate_device(dev);
 	if (!input)
 		return -ENOMEM;
@@ -304,6 +335,18 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (encoder->push_gpio) {
+		input_set_capability(encoder->input, encoder->push_type, encoder->push_code);
+		err = devm_request_threaded_irq(dev, gpiod_to_irq(encoder->push_gpio),
+						NULL, rotary_push_irq,
+						IRQF_TRIGGER_RISING |
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
+						DRV_NAME, encoder);
+		if (err)
+			return dev_err_probe(dev, err, "unable to request push IRQ\n");
+	}
+
 	err = input_register_device(input);
 	if (err) {
 		dev_err(dev, "failed to register input device\n");
-- 
2.28.0

Re: [PATCHv2 4/4] Input: rotary-encoder - Add gpio as push button

[Rob Herring]

On Mon, Sep 07, 2020 at 10:40:45PM +0200, Sebastian Reichel wrote:
> From: Mylène Josserand <mylene.josserand@collabora.com>
> 
> Add the support of a gpio that can be defined as a push button.
> Thanks to that, it is possible to emit a keycode in case of a
> "push" event, if the rotary supports that.
> 
> The keycode to emit is defined using "linux,code" property
> (such as in gpio-keys).

But it is not...

> Signed-off-by: Mylène Josserand <mylene.josserand@collabora.com>
> [code cleanup to current standards and renamed some properties]
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  .../bindings/input/rotary-encoder.yaml        | 21 +++++++++

Bindings should be a separate patch.

>  drivers/input/misc/rotary_encoder.c           | 43 +++++++++++++++++++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.yaml b/Documentation/devicetree/bindings/input/rotary-encoder.yaml
> index 5b60ea86bd62..682fee104004 100644
> --- a/Documentation/devicetree/bindings/input/rotary-encoder.yaml
> +++ b/Documentation/devicetree/bindings/input/rotary-encoder.yaml
> @@ -74,6 +74,24 @@ properties:
>        need to be passed.
>      type: boolean
>  
> +  push-gpios:
> +    description: GPIO used as a detection of a push from the rotary-encoder.
> +    maxItems: 1
> +
> +  linux,push-code:
> +    description:
> +      keycode to emit with the push-gpio of this rotary encoder.
> +      If not specified defaults to <28> == KEY_ENTER.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 28
> +
> +  linux,push-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

Can we just use 'linux,code' and 'linux,input-type' here. That's 
problematic if there's ever more than 1, but so is the current solution 
having to make up new property names every time.

Why can't the gpio-keys binding just be used here? Is it important to 
have the events on the same device? Is there some coordination needed 
between the functions?

> +
>  required:
>    - compatible
>    - gpios
> @@ -97,4 +115,7 @@ examples:
>              linux,axis = <ABS_Y>;
>              rotary-encoder,encoding = "binary";
>              rotary-encoder,rollover;
> +            push-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
> +            linux,push-code = <KEY_ENTER>;
> +            linux,push-type = <EV_KEY>;
>      };
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index 16ad86fad7cb..484042a5afa0 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -47,12 +47,33 @@ struct rotary_encoder {
>  
>  	unsigned int *irq;
>  
> +	struct gpio_desc *push_gpio;
> +	unsigned int push_code;
> +	unsigned int push_type;
> +
>  	bool armed;
>  	signed char dir;	/* 1 - clockwise, -1 - CCW */
>  
>  	unsigned int last_stable;
>  };
>  
> +static irqreturn_t rotary_push_irq(int irq, void *dev_id)
> +{
> +	struct rotary_encoder *encoder = dev_id;
> +	int val;
> +
> +	mutex_lock(&encoder->access_mutex);
> +
> +	val = gpiod_get_value_cansleep(encoder->push_gpio);
> +
> +	input_report_key(encoder->input, encoder->push_code, val);
> +	input_sync(encoder->input);
> +
> +	mutex_unlock(&encoder->access_mutex);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static unsigned int rotary_encoder_get_state(struct rotary_encoder *encoder)
>  {
>  	int i;
> @@ -248,6 +269,16 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> +	encoder->push_gpio = devm_gpiod_get_optional(dev, "push", GPIOD_IN);
> +	if (IS_ERR(encoder->push_gpio))
> +		return dev_err_probe(dev, PTR_ERR(encoder->push_gpio), "failed to get push-gpio\n");
> +
> +	encoder->push_code = KEY_ENTER;
> +	device_property_read_u32(dev, "linux,push-code", &encoder->push_code);
> +
> +	encoder->push_type = EV_KEY;
> +	device_property_read_u32(dev, "linux,push-type", &encoder->push_type);
> +
>  	input = devm_input_allocate_device(dev);
>  	if (!input)
>  		return -ENOMEM;
> @@ -304,6 +335,18 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	if (encoder->push_gpio) {
> +		input_set_capability(encoder->input, encoder->push_type, encoder->push_code);
> +		err = devm_request_threaded_irq(dev, gpiod_to_irq(encoder->push_gpio),
> +						NULL, rotary_push_irq,
> +						IRQF_TRIGGER_RISING |
> +						IRQF_TRIGGER_FALLING |
> +						IRQF_ONESHOT,
> +						DRV_NAME, encoder);
> +		if (err)
> +			return dev_err_probe(dev, err, "unable to request push IRQ\n");
> +	}
> +
>  	err = input_register_device(input);
>  	if (err) {
>  		dev_err(dev, "failed to register input device\n");
> -- 
> 2.28.0
>