[PATCH v3 0/4] input: add GPIO-based charlieplex keypad

[PATCH v3 0/4] input: add GPIO-based charlieplex keypad

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Hello,
this patch series add a new GPIO charlieplex keypad driver.

The first two patches simply commonize two properties that are present in
a few bindings, so that the actual patches for the charlieplex keypad driver
can reuse them instead of also redefining them.

I have tested the driver on a custom board with a Solidrun RZ/G2LC SOM
with three charlieplex keyboards, all connected thru a single PCA9416 I2C GPIO
expander.

Link: [v1] https://lore.kernel.org/all/20260203155023.536103-1-hugo@hugovil.com/
Link: [v2] https://lore.kernel.org/all/20260213171431.2228814-1-hugo@hugovil.com/

Changes for v3:
- Add ASCII diagram in bindings, and reference to it in example
- Reorder properties alphabetically
- Add patch to define common input settling-time-us property
- Add patch to define common input debounce-delay-ms property

Changes for v2:
- Fix yamllint error for example
- Remove unused debug variable (nkeys)
- Remove support for custom linux,no-autorepeat DT property
- Remove support for custom gpio-activelow DT property

Thank you.

Hugo Villeneuve (4):
  dt-bindings: input: add debounce-delay-ms common property
  dt-bindings: input: add settling-time-us common property
  dt-bindings: input: add GPIO charlieplex keypad
  Input: charlieplex_keypad: add GPIO charlieplex keypad

 .../bindings/auxdisplay/holtek,ht16k33.yaml   |   5 +-
 .../bindings/input/cirrus,ep9307-keypad.yaml  |   7 +-
 .../input/gpio-charlieplex-keypad.yaml        | 106 +++++++++
 .../bindings/input/gpio-matrix-keypad.yaml    |   5 +-
 .../devicetree/bindings/input/input.yaml      |  16 ++
 .../input/mediatek,mt6779-keypad.yaml         |   1 +
 .../devicetree/bindings/mfd/fsl,mc13xxx.yaml  |   2 -
 MAINTAINERS                                   |   7 +
 drivers/input/keyboard/Kconfig                |  14 ++
 drivers/input/keyboard/Makefile               |   1 +
 drivers/input/keyboard/charlieplex_keypad.c   | 213 ++++++++++++++++++
 11 files changed, 365 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
 create mode 100644 drivers/input/keyboard/charlieplex_keypad.c


base-commit: ab2e361ca97a42b7af8be1d273646b30d3b75bf3
-- 
2.47.3

[PATCH v3 1/4] dt-bindings: input: add debounce-delay-ms common property

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

A few bindings are already defining a debounce-delay-ms property, so
add it to the input binding to reduce redundant redefines.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../devicetree/bindings/auxdisplay/holtek,ht16k33.yaml    | 5 ++---
 .../devicetree/bindings/input/cirrus,ep9307-keypad.yaml   | 7 +++----
 .../devicetree/bindings/input/gpio-matrix-keypad.yaml     | 5 ++---
 Documentation/devicetree/bindings/input/input.yaml        | 8 ++++++++
 .../devicetree/bindings/input/mediatek,mt6779-keypad.yaml | 1 +
 Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml    | 2 --
 6 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml b/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
index b90eec2077b4..c46a2471f8b1 100644
--- a/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
+++ b/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
@@ -10,6 +10,7 @@ maintainers:
   - Robin van der Gracht <robin@protonic.nl>
 
 allOf:
+  - $ref: /schemas/input/input.yaml#
   - $ref: /schemas/input/matrix-keymap.yaml#
 
 properties:
@@ -33,9 +34,7 @@ properties:
   interrupts:
     maxItems: 1
 
-  debounce-delay-ms:
-    maxItems: 1
-    description: Debouncing interval time in milliseconds
+  debounce-delay-ms: true
 
   linux,keymap: true
 
diff --git a/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
index a0d2460c55ab..25b8b29c87d7 100644
--- a/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
@@ -10,6 +10,7 @@ maintainers:
   - Alexander Sverdlin <alexander.sverdlin@gmail.com>
 
 allOf:
+  - $ref: input.yaml#
   - $ref: /schemas/input/matrix-keymap.yaml#
 
 description:
@@ -37,10 +38,8 @@ properties:
   clocks:
     maxItems: 1
 
-  debounce-delay-ms:
-    description: |
-          Time in microseconds that key must be pressed or
-          released for state change interrupt to trigger.
+  # Time for state change interrupt to trigger
+  debounce-delay-ms: true
 
   cirrus,prescale:
     description: row/column counter pre-scaler load value
diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
index ebfff9e42a36..69df24a5ae70 100644
--- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
@@ -18,6 +18,7 @@ description:
   report the event using GPIO interrupts to the cpu.
 
 allOf:
+  - $ref: input.yaml#
   - $ref: /schemas/input/matrix-keymap.yaml#
 
 properties:
@@ -46,9 +47,7 @@ properties:
       Force GPIO polarity to active low.
       In the absence of this property GPIOs are treated as active high.
 
-  debounce-delay-ms:
-    description: Debounce interval in milliseconds.
-    default: 0
+  debounce-delay-ms: true
 
   col-scan-delay-us:
     description:
diff --git a/Documentation/devicetree/bindings/input/input.yaml b/Documentation/devicetree/bindings/input/input.yaml
index 94f7942189e8..502e0b7eb500 100644
--- a/Documentation/devicetree/bindings/input/input.yaml
+++ b/Documentation/devicetree/bindings/input/input.yaml
@@ -14,6 +14,14 @@ properties:
     description: Enable autorepeat when key is pressed and held down.
     type: boolean
 
+  debounce-delay-ms:
+    description:
+      Debounce delay in milliseconds. This is the time during which the key
+      press or release signal must remain stable before it is considered valid.
+    minimum: 0
+    maximum: 999
+    default: 0
+
   linux,keycodes:
     description:
       Specifies an array of numeric keycode values to be used for reporting
diff --git a/Documentation/devicetree/bindings/input/mediatek,mt6779-keypad.yaml b/Documentation/devicetree/bindings/input/mediatek,mt6779-keypad.yaml
index e365413732e7..914dd3283df3 100644
--- a/Documentation/devicetree/bindings/input/mediatek,mt6779-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/mediatek,mt6779-keypad.yaml
@@ -10,6 +10,7 @@ maintainers:
   - Mattijs Korpershoek <mkorpershoek@kernel.org>
 
 allOf:
+  - $ref: input.yaml#
   - $ref: /schemas/input/matrix-keymap.yaml#
 
 description: |
diff --git a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
index d2886f2686a8..542ba5114424 100644
--- a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
+++ b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
@@ -76,8 +76,6 @@ properties:
           debounce-delay-ms:
             enum: [0, 30, 150, 750]
             default: 30
-            description:
-              Sets the debouncing delay in milliseconds.
 
           active-low:
             description: Set active when pin is pulled low.
-- 
2.47.3

[PATCH v3 2/4] dt-bindings: input: add settling-time-us common property

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Add common property that can be reused by other bindings.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
Note: a similar property is used by gpio-matrix-keypad.yaml:
  col-scan-delay-us
---
 Documentation/devicetree/bindings/input/input.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/input.yaml b/Documentation/devicetree/bindings/input/input.yaml
index 502e0b7eb500..14652d324e2c 100644
--- a/Documentation/devicetree/bindings/input/input.yaml
+++ b/Documentation/devicetree/bindings/input/input.yaml
@@ -66,6 +66,14 @@ properties:
       reset automatically. Device with key pressed reset feature can specify
       this property.
 
+  settling-time-us:
+    description:
+      Delay, in microseconds, when activating an output line/col/row GPIO
+      before we can reliably read other input GPIOs that maybe affected by this
+      output. This can be the case for an output with a RC circuit that affects
+      ramp-up/down times.
+    default: 0
+
 dependencies:
   linux,input-type: [ "linux,code" ]
 
-- 
2.47.3

[PATCH v3 3/4] dt-bindings: input: add GPIO charlieplex keypad

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Add DT bindings for GPIO charlieplex keypad.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../input/gpio-charlieplex-keypad.yaml        | 106 ++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml

diff --git a/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
new file mode 100644
index 000000000000..d3eeddc8e533
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO charlieplex keypad
+
+maintainers:
+  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
+
+description: |
+  The charlieplex keypad supports N^2)-N different key combinations (where N is
+  the number of lines). Key presses and releases are detected by configuring
+  only one line as output at a time, and reading other line states. This process
+  is repeated for each line. Diodes are required to ensure current flows in only
+  one direction between any pair of pins.
+  This mechanism doesn't allow to detect simultaneous key presses.
+
+  Wiring example for 3 lines keyboard with 6 switches and 3 diodes:
+
+  L0  --+---------------------+----------------------+
+        |                     |                      |
+  L1  -------+-----------+---------------------+     |
+        |    |           |    |                |     |
+  L2  -------------+----------------+-----+    |     |
+        |    |     |     |    |     |     |    |     |
+        |    |     |     |    |     |     |    |     |
+        |  S1 \  S2 \    |  S3 \  S4 \    |  S5 \  S6 \
+        |    |     |     |    |     |     |    |     |
+        |    +--+--+     |    +--+--+     |    +--+--+
+        |       |        |       |        |       |
+        |    D1 v        |    D2 v        |    D3 v
+        |       - (k)    |       - (k)    |       - (k)
+        |       |        |       |        |       |
+        +-------+        +-------+        +-------+
+
+  L: GPIO line
+  S: switch
+  D: diode (k indicates cathode)
+
+allOf:
+  - $ref: input.yaml#
+  - $ref: /schemas/input/matrix-keymap.yaml#
+
+properties:
+  compatible:
+    const: gpio-charlieplex-keypad
+
+  autorepeat: true
+
+  debounce-delay-ms:
+    default: 5
+
+  line-gpios:
+    description:
+      List of GPIOs used as lines. The gpio specifier for this property
+      depends on the gpio controller to which these lines are connected.
+
+  linux,keymap: true
+
+  poll-interval: true
+
+  settling-time-us: true
+
+  wakeup-source: true
+
+required:
+  - compatible
+  - line-gpios
+  - linux,keymap
+  - poll-interval
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/input/input.h>
+
+    charlieplex-keypad {
+        compatible = "gpio-charlieplex-keypad";
+        debounce-delay-ms = <20>;
+        poll-interval = <5>;
+        settling-time-us = <2>;
+
+        line-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH
+                      &gpio2 26 GPIO_ACTIVE_HIGH
+                      &gpio2 27 GPIO_ACTIVE_HIGH>;
+
+        /* MATRIX_KEY(output, input, key-code) */
+        linux,keymap = <
+            /*
+             * According to wiring diagram above, if L1 is configured as
+             * output and HIGH, and we detect a HIGH level on input L0,
+             * then it means S1 is pressed: MATRIX_KEY(L1, L0, KEY...)
+             */
+            MATRIX_KEY(1, 0, KEY_F1) /* S1 */
+            MATRIX_KEY(2, 0, KEY_F2) /* S2 */
+            MATRIX_KEY(0, 1, KEY_F3) /* S3 */
+            MATRIX_KEY(2, 1, KEY_F4) /* S4 */
+            MATRIX_KEY(1, 2, KEY_F5) /* S5 */
+            MATRIX_KEY(0, 2, KEY_F6) /* S6 */
+        >;
+    };
-- 
2.47.3

Re: [PATCH v3 3/4] dt-bindings: input: add GPIO charlieplex keypad

[Geert Uytterhoeven]

Hi Hugo,

On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> Add DT bindings for GPIO charlieplex keypad.
>
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> @@ -0,0 +1,106 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO charlieplex keypad
> +
> +maintainers:
> +  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> +
> +description: |
> +  The charlieplex keypad supports N^2)-N different key combinations (where N is
> +  the number of lines). Key presses and releases are detected by configuring
> +  only one line as output at a time, and reading other line states. This process
> +  is repeated for each line. Diodes are required to ensure current flows in only
> +  one direction between any pair of pins.
> +  This mechanism doesn't allow to detect simultaneous key presses.

Indeed, e.g. pressing S1 and S2 simultaneously will show a ghost
S5 keypress.

> +
> +  Wiring example for 3 lines keyboard with 6 switches and 3 diodes:
> +
> +  L0  --+---------------------+----------------------+
> +        |                     |                      |
> +  L1  -------+-----------+---------------------+     |
> +        |    |           |    |                |     |
> +  L2  -------------+----------------+-----+    |     |
> +        |    |     |     |    |     |     |    |     |
> +        |    |     |     |    |     |     |    |     |
> +        |  S1 \  S2 \    |  S3 \  S4 \    |  S5 \  S6 \
> +        |    |     |     |    |     |     |    |     |
> +        |    +--+--+     |    +--+--+     |    +--+--+
> +        |       |        |       |        |       |
> +        |    D1 v        |    D2 v        |    D3 v
> +        |       - (k)    |       - (k)    |       - (k)
> +        |       |        |       |        |       |
> +        +-------+        +-------+        +-------+

Don't you need pull-down resistors on L[0-2], and/or a way to specify
in DT to enable internal poll-down on GPIO controllers that support it?
Some controllers may support internal pull-up only, but I guess that
can be handled using GPIO_ACTIVE_LOW?

> +
> +  L: GPIO line
> +  S: switch
> +  D: diode (k indicates cathode)
> +
> +allOf:
> +  - $ref: input.yaml#
> +  - $ref: /schemas/input/matrix-keymap.yaml#
> +
> +properties:
> +  compatible:
> +    const: gpio-charlieplex-keypad
> +
> +  autorepeat: true
> +
> +  debounce-delay-ms:
> +    default: 5
> +
> +  line-gpios:
> +    description:
> +      List of GPIOs used as lines. The gpio specifier for this property
> +      depends on the gpio controller to which these lines are connected.
> +
> +  linux,keymap: true
> +
> +  poll-interval: true
> +
> +  settling-time-us: true
> +
> +  wakeup-source: true
> +
> +required:
> +  - compatible
> +  - line-gpios
> +  - linux,keymap
> +  - poll-interval
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/input/input.h>
> +
> +    charlieplex-keypad {

"keyboard", as per Devicetree Specification Generic Names
Recommendation.

> +        compatible = "gpio-charlieplex-keypad";
> +        debounce-delay-ms = <20>;
> +        poll-interval = <5>;
> +        settling-time-us = <2>;
> +
> +        line-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH
> +                      &gpio2 26 GPIO_ACTIVE_HIGH
> +                      &gpio2 27 GPIO_ACTIVE_HIGH>;
> +
> +        /* MATRIX_KEY(output, input, key-code) */
> +        linux,keymap = <
> +            /*
> +             * According to wiring diagram above, if L1 is configured as
> +             * output and HIGH, and we detect a HIGH level on input L0,
> +             * then it means S1 is pressed: MATRIX_KEY(L1, L0, KEY...)
> +             */
> +            MATRIX_KEY(1, 0, KEY_F1) /* S1 */
> +            MATRIX_KEY(2, 0, KEY_F2) /* S2 */
> +            MATRIX_KEY(0, 1, KEY_F3) /* S3 */
> +            MATRIX_KEY(2, 1, KEY_F4) /* S4 */
> +            MATRIX_KEY(1, 2, KEY_F5) /* S5 */
> +            MATRIX_KEY(0, 2, KEY_F6) /* S6 */
> +        >;
> +    };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 3/4] dt-bindings: input: add GPIO charlieplex keypad

[Hugo Villeneuve]

Hi Geert,

On Thu, 26 Feb 2026 10:32:30 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Hugo,
> 
> On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >
> > Add DT bindings for GPIO charlieplex keypad.
> >
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > @@ -0,0 +1,106 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GPIO charlieplex keypad
> > +
> > +maintainers:
> > +  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > +
> > +description: |
> > +  The charlieplex keypad supports N^2)-N different key combinations (where N is
> > +  the number of lines). Key presses and releases are detected by configuring
> > +  only one line as output at a time, and reading other line states. This process
> > +  is repeated for each line. Diodes are required to ensure current flows in only
> > +  one direction between any pair of pins.
> > +  This mechanism doesn't allow to detect simultaneous key presses.
> 
> Indeed, e.g. pressing S1 and S2 simultaneously will show a ghost
> S5 keypress.
> 
> > +
> > +  Wiring example for 3 lines keyboard with 6 switches and 3 diodes:
> > +
> > +  L0  --+---------------------+----------------------+
> > +        |                     |                      |
> > +  L1  -------+-----------+---------------------+     |
> > +        |    |           |    |                |     |
> > +  L2  -------------+----------------+-----+    |     |
> > +        |    |     |     |    |     |     |    |     |
> > +        |    |     |     |    |     |     |    |     |
> > +        |  S1 \  S2 \    |  S3 \  S4 \    |  S5 \  S6 \
> > +        |    |     |     |    |     |     |    |     |
> > +        |    +--+--+     |    +--+--+     |    +--+--+
> > +        |       |        |       |        |       |
> > +        |    D1 v        |    D2 v        |    D3 v
> > +        |       - (k)    |       - (k)    |       - (k)
> > +        |       |        |       |        |       |
> > +        +-------+        +-------+        +-------+
> 
> Don't you need pull-down resistors on L[0-2], and/or a way to specify
> in DT to enable internal poll-down on GPIO controllers that support it?
> Some controllers may support internal pull-up only, but I guess that
> can be handled using GPIO_ACTIVE_LOW?

Yes, I did not put any resistors in the diagram in order to keep it
simple and clear.

You need pull-down resistors either on the board, or specified in the
DT like we do for our board (we use the PCAL6416 with pull-up/down
support):

  line-gpios = <&gpio20 0 (GPIO_ACTIVE_HIGH | GPIO_PULL_DOWN)
                 ...

I will add this to the example, and add a few lines explaining this in
the description.


> > +
> > +  L: GPIO line
> > +  S: switch
> > +  D: diode (k indicates cathode)
> > +
> > +allOf:
> > +  - $ref: input.yaml#
> > +  - $ref: /schemas/input/matrix-keymap.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: gpio-charlieplex-keypad
> > +
> > +  autorepeat: true
> > +
> > +  debounce-delay-ms:
> > +    default: 5
> > +
> > +  line-gpios:
> > +    description:
> > +      List of GPIOs used as lines. The gpio specifier for this property
> > +      depends on the gpio controller to which these lines are connected.
> > +
> > +  linux,keymap: true
> > +
> > +  poll-interval: true
> > +
> > +  settling-time-us: true
> > +
> > +  wakeup-source: true
> > +
> > +required:
> > +  - compatible
> > +  - line-gpios
> > +  - linux,keymap
> > +  - poll-interval
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    #include <dt-bindings/input/input.h>
> > +
> > +    charlieplex-keypad {
> 
> "keyboard", as per Devicetree Specification Generic Names
> Recommendation.

Ok.

Thank you,
Hugo.

> 
> > +        compatible = "gpio-charlieplex-keypad";
> > +        debounce-delay-ms = <20>;
> > +        poll-interval = <5>;
> > +        settling-time-us = <2>;
> > +
> > +        line-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH
> > +                      &gpio2 26 GPIO_ACTIVE_HIGH
> > +                      &gpio2 27 GPIO_ACTIVE_HIGH>;
> > +
> > +        /* MATRIX_KEY(output, input, key-code) */
> > +        linux,keymap = <
> > +            /*
> > +             * According to wiring diagram above, if L1 is configured as
> > +             * output and HIGH, and we detect a HIGH level on input L0,
> > +             * then it means S1 is pressed: MATRIX_KEY(L1, L0, KEY...)
> > +             */
> > +            MATRIX_KEY(1, 0, KEY_F1) /* S1 */
> > +            MATRIX_KEY(2, 0, KEY_F2) /* S2 */
> > +            MATRIX_KEY(0, 1, KEY_F3) /* S3 */
> > +            MATRIX_KEY(2, 1, KEY_F4) /* S4 */
> > +            MATRIX_KEY(1, 2, KEY_F5) /* S5 */
> > +            MATRIX_KEY(0, 2, KEY_F6) /* S6 */
> > +        >;
> > +    };
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 


-- 
Hugo Villeneuve

Re: [PATCH v3 3/4] dt-bindings: input: add GPIO charlieplex keypad

[Hugo Villeneuve]

Hi Geert,

On Thu, 26 Feb 2026 10:32:30 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Hugo,
> 
> On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >
> > Add DT bindings for GPIO charlieplex keypad.
> >
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > @@ -0,0 +1,106 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GPIO charlieplex keypad
> > +
> > +maintainers:
> > +  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > +
> > +description: |
> > +  The charlieplex keypad supports N^2)-N different key combinations (where N is
> > +  the number of lines). Key presses and releases are detected by configuring
> > +  only one line as output at a time, and reading other line states. This process
> > +  is repeated for each line. Diodes are required to ensure current flows in only
> > +  one direction between any pair of pins.
> > +  This mechanism doesn't allow to detect simultaneous key presses.
> 
> Indeed, e.g. pressing S1 and S2 simultaneously will show a ghost
> S5 keypress.
> 
> > +
> > +  Wiring example for 3 lines keyboard with 6 switches and 3 diodes:
> > +
> > +  L0  --+---------------------+----------------------+
> > +        |                     |                      |
> > +  L1  -------+-----------+---------------------+     |
> > +        |    |           |    |                |     |
> > +  L2  -------------+----------------+-----+    |     |
> > +        |    |     |     |    |     |     |    |     |
> > +        |    |     |     |    |     |     |    |     |
> > +        |  S1 \  S2 \    |  S3 \  S4 \    |  S5 \  S6 \
> > +        |    |     |     |    |     |     |    |     |
> > +        |    +--+--+     |    +--+--+     |    +--+--+
> > +        |       |        |       |        |       |
> > +        |    D1 v        |    D2 v        |    D3 v
> > +        |       - (k)    |       - (k)    |       - (k)
> > +        |       |        |       |        |       |
> > +        +-------+        +-------+        +-------+
> 
> Don't you need pull-down resistors on L[0-2], and/or a way to specify
> in DT to enable internal poll-down on GPIO controllers that support it?
> Some controllers may support internal pull-up only, but I guess that
> can be handled using GPIO_ACTIVE_LOW?

Yes, using something like this:

  line-gpios = <&gpio2 25 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)

should work I think, although with my hardware unfortunately I cannot
test it.

Hugo.


> 
> > +
> > +  L: GPIO line
> > +  S: switch
> > +  D: diode (k indicates cathode)
> > +
> > +allOf:
> > +  - $ref: input.yaml#
> > +  - $ref: /schemas/input/matrix-keymap.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: gpio-charlieplex-keypad
> > +
> > +  autorepeat: true
> > +
> > +  debounce-delay-ms:
> > +    default: 5
> > +
> > +  line-gpios:
> > +    description:
> > +      List of GPIOs used as lines. The gpio specifier for this property
> > +      depends on the gpio controller to which these lines are connected.
> > +
> > +  linux,keymap: true
> > +
> > +  poll-interval: true
> > +
> > +  settling-time-us: true
> > +
> > +  wakeup-source: true
> > +
> > +required:
> > +  - compatible
> > +  - line-gpios
> > +  - linux,keymap
> > +  - poll-interval
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    #include <dt-bindings/input/input.h>
> > +
> > +    charlieplex-keypad {
> 
> "keyboard", as per Devicetree Specification Generic Names
> Recommendation.
> 
> > +        compatible = "gpio-charlieplex-keypad";
> > +        debounce-delay-ms = <20>;
> > +        poll-interval = <5>;
> > +        settling-time-us = <2>;
> > +
> > +        line-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH
> > +                      &gpio2 26 GPIO_ACTIVE_HIGH
> > +                      &gpio2 27 GPIO_ACTIVE_HIGH>;
> > +
> > +        /* MATRIX_KEY(output, input, key-code) */
> > +        linux,keymap = <
> > +            /*
> > +             * According to wiring diagram above, if L1 is configured as
> > +             * output and HIGH, and we detect a HIGH level on input L0,
> > +             * then it means S1 is pressed: MATRIX_KEY(L1, L0, KEY...)
> > +             */
> > +            MATRIX_KEY(1, 0, KEY_F1) /* S1 */
> > +            MATRIX_KEY(2, 0, KEY_F2) /* S2 */
> > +            MATRIX_KEY(0, 1, KEY_F3) /* S3 */
> > +            MATRIX_KEY(2, 1, KEY_F4) /* S4 */
> > +            MATRIX_KEY(1, 2, KEY_F5) /* S5 */
> > +            MATRIX_KEY(0, 2, KEY_F6) /* S6 */
> > +        >;
> > +    };
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 


-- 
Hugo Villeneuve

[PATCH v3 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Add support for GPIO-based charlieplex keypad, allowing to control
N^2-N keys using N GPIO lines.

Reuse matrix keypad keymap to simplify, even if there is no concept
of rows and columns in this type of keyboard.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 MAINTAINERS                                 |   7 +
 drivers/input/keyboard/Kconfig              |  14 ++
 drivers/input/keyboard/Makefile             |   1 +
 drivers/input/keyboard/charlieplex_keypad.c | 213 ++++++++++++++++++++
 4 files changed, 235 insertions(+)
 create mode 100644 drivers/input/keyboard/charlieplex_keypad.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9ed6d11a7746..0b2d71f32b40 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5765,6 +5765,13 @@ S:	Maintained
 F:	Documentation/hwmon/powerz.rst
 F:	drivers/hwmon/powerz.c
 
+CHARLIEPLEX KEYPAD DRIVER
+M:	Hugo Villeneuve <hvilleneuve@dimonoff.com>
+S:	Supported
+W:	http://www.mosaic-industries.com/embedded-systems/microcontroller-projects/electronic-circuits/matrix-keypad-scan-decode
+F:	Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
+F:	drivers/input/keyboard/charlieplex_keypad.c
+
 CHECKPATCH
 M:	Andy Whitcroft <apw@canonical.com>
 M:	Joe Perches <joe@perches.com>
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 2ff4fef322c2..ae54b4b7e2d8 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -289,6 +289,20 @@ config KEYBOARD_MATRIX
 	  To compile this driver as a module, choose M here: the
 	  module will be called matrix_keypad.
 
+config KEYBOARD_CHARLIEPLEX
+	tristate "GPIO driven chearlieplex keypad support"
+	depends on GPIOLIB || COMPILE_TEST
+	select INPUT_MATRIXKMAP
+	help
+	  Enable support for GPIO driven charlieplex keypad. A charlieplex
+	  keypad allows to use fewer GPIO lines to interface to key switches.
+	  For example, an N lines charlieplex keypad can be used to interface
+	  to N^2-N different key switches. However, this type of keypad
+	  cannot detect more than one key press at a time.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called charlieplex_keypad.
+
 config KEYBOARD_HIL_OLD
 	tristate "HP HIL keyboard support (simple driver)"
 	depends on GSC || HP300
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 2d906e14f3e2..40b5cf5d374d 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_KEYBOARD_LOCOMO)		+= locomokbd.o
 obj-$(CONFIG_KEYBOARD_LPC32XX)		+= lpc32xx-keys.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
+obj-$(CONFIG_KEYBOARD_CHARLIEPLEX)	+= charlieplex_keypad.o
 obj-$(CONFIG_KEYBOARD_MAX7359)		+= max7359_keypad.o
 obj-$(CONFIG_KEYBOARD_MAX7360)		+= max7360-keypad.o
 obj-$(CONFIG_KEYBOARD_MPR121)		+= mpr121_touchkey.o
diff --git a/drivers/input/keyboard/charlieplex_keypad.c b/drivers/input/keyboard/charlieplex_keypad.c
new file mode 100644
index 000000000000..fae0aeb1d167
--- /dev/null
+++ b/drivers/input/keyboard/charlieplex_keypad.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  GPIO driven charlieplex keypad driver
+ *
+ *  Copyright (c) 2025 Hugo Villeneuve <hvilleneuve@dimonoff.com>
+ *
+ *  Based on matrix_keyboard.c
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/types.h>
+
+struct charlieplex_keypad {
+	struct input_dev *input_dev;
+	struct gpio_descs *line_gpios;
+	unsigned int nlines;
+	unsigned int settling_time_us;
+	unsigned int debounce_threshold;
+	unsigned int debounce_count;
+	int debounce_code;
+	int current_code;
+};
+
+static void charlieplex_keypad_report_key(struct input_dev *input)
+{
+	struct charlieplex_keypad *keypad = input_get_drvdata(input);
+	const unsigned short *keycodes = input->keycode;
+
+	if (keypad->current_code > 0) {
+		input_event(input, EV_MSC, MSC_SCAN, keypad->current_code);
+		input_report_key(input, keycodes[keypad->current_code], 0);
+	}
+
+	if (keypad->debounce_code) {
+		input_event(input, EV_MSC, MSC_SCAN, keypad->debounce_code);
+		input_report_key(input, keycodes[keypad->debounce_code], 1);
+	}
+
+	input_sync(input);
+	keypad->current_code = keypad->debounce_code;
+}
+
+static void charlieplex_keypad_check_switch_change(struct input_dev *input,
+						   int code)
+{
+	struct charlieplex_keypad *keypad = input_get_drvdata(input);
+
+	if (code != keypad->debounce_code) {
+		keypad->debounce_count = 0;
+		keypad->debounce_code = code;
+	} else if (keypad->debounce_count < keypad->debounce_threshold) {
+		keypad->debounce_count++;
+
+		if (keypad->debounce_count >= keypad->debounce_threshold &&
+		    keypad->debounce_code != keypad->current_code)
+			charlieplex_keypad_report_key(input);
+	}
+}
+
+static void charlieplex_keypad_poll(struct input_dev *input)
+{
+	struct charlieplex_keypad *keypad = input_get_drvdata(input);
+	int oline;
+	int code;
+
+	for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
+		DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
+		int iline;
+		int rc;
+
+		/* Activate only one line as output at a time. */
+		gpiod_direction_output(keypad->line_gpios->desc[oline], 1);
+
+		if (keypad->settling_time_us)
+			fsleep(keypad->settling_time_us);
+
+		/* Read input on all other lines. */
+		rc = gpiod_get_array_value_cansleep(keypad->line_gpios->ndescs,
+						    keypad->line_gpios->desc,
+						    keypad->line_gpios->info, values);
+		if (rc)
+			return;
+
+		for (iline = 0; iline < keypad->nlines; iline++) {
+			if (iline == oline)
+				continue; /* Do not read active output line. */
+
+			/* Check if GPIO is asserted. */
+			if (test_bit(iline, values)) {
+				code = MATRIX_SCAN_CODE(oline, iline,
+							get_count_order(keypad->nlines));
+				/*
+				 * Exit loop immediately since we cannot detect
+				 * more than one key press at a time.
+				 */
+				break;
+			}
+		}
+
+		gpiod_direction_input(keypad->line_gpios->desc[oline]);
+
+		if (code)
+			break;
+	}
+
+	charlieplex_keypad_check_switch_change(input, code);
+}
+
+static int charlieplex_keypad_init_gpio(struct platform_device *pdev,
+					struct charlieplex_keypad *keypad)
+{
+	int i;
+
+	keypad->line_gpios = devm_gpiod_get_array(&pdev->dev, "line", GPIOD_IN);
+	if (IS_ERR(keypad->line_gpios))
+		return PTR_ERR(keypad->line_gpios);
+
+	keypad->nlines = keypad->line_gpios->ndescs;
+
+	if (keypad->nlines > MATRIX_MAX_ROWS)
+		return -EINVAL;
+
+	for (i = 0; i < keypad->nlines; i++)
+		gpiod_set_consumer_name(keypad->line_gpios->desc[i], "charlieplex_kbd_line");
+
+	return 0;
+}
+
+static int charlieplex_keypad_probe(struct platform_device *pdev)
+{
+	struct charlieplex_keypad *keypad;
+	unsigned int debounce_interval_ms;
+	unsigned int poll_interval_ms;
+	struct input_dev *input_dev;
+	int err;
+
+	keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
+	if (!keypad)
+		return -ENOMEM;
+
+	input_dev = devm_input_allocate_device(&pdev->dev);
+	if (!input_dev)
+		return -ENOMEM;
+
+	keypad->input_dev = input_dev;
+
+	device_property_read_u32(&pdev->dev, "poll-interval", &poll_interval_ms);
+	device_property_read_u32(&pdev->dev, "debounce-delay-ms", &debounce_interval_ms);
+	device_property_read_u32(&pdev->dev, "settling-time-us", &keypad->settling_time_us);
+
+	keypad->current_code = -1;
+	keypad->debounce_code = -1;
+	keypad->debounce_threshold = DIV_ROUND_UP(debounce_interval_ms, poll_interval_ms);
+
+	err = charlieplex_keypad_init_gpio(pdev, keypad);
+	if (err)
+		return err;
+
+	input_dev->name		= pdev->name;
+	input_dev->id.bustype	= BUS_HOST;
+
+	err = matrix_keypad_build_keymap(NULL, NULL, keypad->nlines,
+					 keypad->nlines, NULL, input_dev);
+	if (err)
+		dev_err_probe(&pdev->dev, -ENOMEM, "failed to build keymap\n");
+
+	if (device_property_read_bool(&pdev->dev, "autorepeat"))
+		__set_bit(EV_REP, input_dev->evbit);
+
+	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
+
+	err = input_setup_polling(input_dev, charlieplex_keypad_poll);
+	if (err)
+		dev_err_probe(&pdev->dev, err, "unable to set up polling\n");
+
+	input_set_poll_interval(input_dev, poll_interval_ms);
+
+	input_set_drvdata(input_dev, keypad);
+
+	err = input_register_device(keypad->input_dev);
+	if (err)
+		return err;
+
+	platform_set_drvdata(pdev, keypad);
+
+	return 0;
+}
+
+static const struct of_device_id charlieplex_keypad_dt_match[] = {
+	{ .compatible = "gpio-charlieplex-keypad" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, charlieplex_keypad_dt_match);
+
+static struct platform_driver charlieplex_keypad_driver = {
+	.probe		= charlieplex_keypad_probe,
+	.driver		= {
+		.name	= "charlieplex-keypad",
+		.of_match_table = of_match_ptr(charlieplex_keypad_dt_match),
+	},
+};
+module_platform_driver(charlieplex_keypad_driver);
+
+MODULE_AUTHOR("Hugo Villeneuve <hvilleneuve@dimonoff.com>");
+MODULE_DESCRIPTION("GPIO driven charlieplex keypad driver");
+MODULE_LICENSE("GPL");
-- 
2.47.3

Re: [PATCH v3 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad

[Andy Shevchenko]

On Wed, Feb 25, 2026 at 10:54:01AM -0500, Hugo Villeneuve wrote:

> Add support for GPIO-based charlieplex keypad, allowing to control
> N^2-N keys using N GPIO lines.
> 
> Reuse matrix keypad keymap to simplify, even if there is no concept
> of rows and columns in this type of keyboard.

...

> +/*
> + *  GPIO driven charlieplex keypad driver
> + *
> + *  Copyright (c) 2025 Hugo Villeneuve <hvilleneuve@dimonoff.com>
> + *
> + *  Based on matrix_keyboard.c

A single space after asterisk is enough.

> + */

...

+ bitops.h

> +#include <linux/delay.h>

+ dev_printk.h
+ device/devres.h
+ err.h

> +#include <linux/gpio/consumer.h>
> +#include <linux/input.h>
> +#include <linux/input/matrix_keypad.h>

+ math.h

> +#include <linux/module.h>

> +#include <linux/of.h>

Is this in use? Or you wanted mod_devicetable.h for OF ID table?

> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/types.h>

...

> +	for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
> +		DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
> +		int iline;

> +		int rc;

I think Dmitry prefers 'error' name for this kind of variables.

> +		/* Activate only one line as output at a time. */
> +		gpiod_direction_output(keypad->line_gpios->desc[oline], 1);
> +
> +		if (keypad->settling_time_us)
> +			fsleep(keypad->settling_time_us);
> +
> +		/* Read input on all other lines. */
> +		rc = gpiod_get_array_value_cansleep(keypad->line_gpios->ndescs,
> +						    keypad->line_gpios->desc,
> +						    keypad->line_gpios->info, values);
> +		if (rc)
> +			return;
> +
> +		for (iline = 0; iline < keypad->nlines; iline++) {
> +			if (iline == oline)
> +				continue; /* Do not read active output line. */
> +
> +			/* Check if GPIO is asserted. */
> +			if (test_bit(iline, values)) {
> +				code = MATRIX_SCAN_CODE(oline, iline,
> +							get_count_order(keypad->nlines));
> +				/*
> +				 * Exit loop immediately since we cannot detect
> +				 * more than one key press at a time.
> +				 */
> +				break;
> +			}
> +		}
> +
> +		gpiod_direction_input(keypad->line_gpios->desc[oline]);
> +
> +		if (code)
> +			break;
> +	}

...

> +static int charlieplex_keypad_init_gpio(struct platform_device *pdev,
> +					struct charlieplex_keypad *keypad)
> +{
> +	int i;

Why signed? But see below as well.

> +	keypad->line_gpios = devm_gpiod_get_array(&pdev->dev, "line", GPIOD_IN);
> +	if (IS_ERR(keypad->line_gpios))
> +		return PTR_ERR(keypad->line_gpios);
> +
> +	keypad->nlines = keypad->line_gpios->ndescs;
> +
> +	if (keypad->nlines > MATRIX_MAX_ROWS)
> +		return -EINVAL;

> +	for (i = 0; i < keypad->nlines; i++)

iterator is local to the loop, hence

	for (unsigned int i = 0; i < keypad->nlines; i++)

> +		gpiod_set_consumer_name(keypad->line_gpios->desc[i], "charlieplex_kbd_line");
> +
> +	return 0;
> +}

...

> +static int charlieplex_keypad_probe(struct platform_device *pdev)
> +{
> +	struct charlieplex_keypad *keypad;
> +	unsigned int debounce_interval_ms;
> +	unsigned int poll_interval_ms;
> +	struct input_dev *input_dev;

> +	int err;

The naming is even inconsistent between the functions...

> +	keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> +	if (!keypad)
> +		return -ENOMEM;
> +
> +	input_dev = devm_input_allocate_device(&pdev->dev);
> +	if (!input_dev)
> +		return -ENOMEM;
> +
> +	keypad->input_dev = input_dev;
> +
> +	device_property_read_u32(&pdev->dev, "poll-interval", &poll_interval_ms);
> +	device_property_read_u32(&pdev->dev, "debounce-delay-ms", &debounce_interval_ms);
> +	device_property_read_u32(&pdev->dev, "settling-time-us", &keypad->settling_time_us);
> +
> +	keypad->current_code = -1;
> +	keypad->debounce_code = -1;
> +	keypad->debounce_threshold = DIV_ROUND_UP(debounce_interval_ms, poll_interval_ms);
> +
> +	err = charlieplex_keypad_init_gpio(pdev, keypad);
> +	if (err)
> +		return err;
> +
> +	input_dev->name		= pdev->name;
> +	input_dev->id.bustype	= BUS_HOST;
> +
> +	err = matrix_keypad_build_keymap(NULL, NULL, keypad->nlines,
> +					 keypad->nlines, NULL, input_dev);
> +	if (err)
> +		dev_err_probe(&pdev->dev, -ENOMEM, "failed to build keymap\n");
> +
> +	if (device_property_read_bool(&pdev->dev, "autorepeat"))
> +		__set_bit(EV_REP, input_dev->evbit);
> +
> +	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
> +
> +	err = input_setup_polling(input_dev, charlieplex_keypad_poll);
> +	if (err)
> +		dev_err_probe(&pdev->dev, err, "unable to set up polling\n");
> +
> +	input_set_poll_interval(input_dev, poll_interval_ms);
> +
> +	input_set_drvdata(input_dev, keypad);
> +
> +	err = input_register_device(keypad->input_dev);
> +	if (err)
> +		return err;

> +	platform_set_drvdata(pdev, keypad);

Is this needed?

> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad

[Hugo Villeneuve]

Hi Andy,
thank you for the review.

On Wed, 25 Feb 2026 18:12:13 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Feb 25, 2026 at 10:54:01AM -0500, Hugo Villeneuve wrote:
> 
> > Add support for GPIO-based charlieplex keypad, allowing to control
> > N^2-N keys using N GPIO lines.
> > 
> > Reuse matrix keypad keymap to simplify, even if there is no concept
> > of rows and columns in this type of keyboard.
> 
> ...
> 
> > +/*
> > + *  GPIO driven charlieplex keypad driver
> > + *
> > + *  Copyright (c) 2025 Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > + *
> > + *  Based on matrix_keyboard.c
> 
> A single space after asterisk is enough.

Ok, leftover from copy/paste from matrix_keyboard.c :)

> 
> > + */
> 
> ...
> 
> + bitops.h
> 
> > +#include <linux/delay.h>
> 
> + dev_printk.h
> + device/devres.h
> + err.h
> 
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/input.h>
> > +#include <linux/input/matrix_keypad.h>
> 
> + math.h

Ok.


> 
> > +#include <linux/module.h>
> 
> > +#include <linux/of.h>
> 
> Is this in use? Or you wanted mod_devicetable.h for OF ID table?

I need only OF ID table, so will replace with mod_devicetable.h.


> 
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/types.h>
> 
> ...
> 
> > +	for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
> > +		DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
> > +		int iline;
> 
> > +		int rc;
> 
> I think Dmitry prefers 'error' name for this kind of variables.

I hate using "error", can be so misleading :)

I would prefer to use "rc" everywhere, but if Dmitry chimes in and
specifies "error" or "err", then so it will be.


> 
> > +		/* Activate only one line as output at a time. */
> > +		gpiod_direction_output(keypad->line_gpios->desc[oline], 1);
> > +
> > +		if (keypad->settling_time_us)
> > +			fsleep(keypad->settling_time_us);
> > +
> > +		/* Read input on all other lines. */
> > +		rc = gpiod_get_array_value_cansleep(keypad->line_gpios->ndescs,
> > +						    keypad->line_gpios->desc,
> > +						    keypad->line_gpios->info, values);
> > +		if (rc)
> > +			return;
> > +
> > +		for (iline = 0; iline < keypad->nlines; iline++) {
> > +			if (iline == oline)
> > +				continue; /* Do not read active output line. */
> > +
> > +			/* Check if GPIO is asserted. */
> > +			if (test_bit(iline, values)) {
> > +				code = MATRIX_SCAN_CODE(oline, iline,
> > +							get_count_order(keypad->nlines));
> > +				/*
> > +				 * Exit loop immediately since we cannot detect
> > +				 * more than one key press at a time.
> > +				 */
> > +				break;
> > +			}
> > +		}
> > +
> > +		gpiod_direction_input(keypad->line_gpios->desc[oline]);
> > +
> > +		if (code)
> > +			break;
> > +	}
> 
> ...
> 
> > +static int charlieplex_keypad_init_gpio(struct platform_device *pdev,
> > +					struct charlieplex_keypad *keypad)
> > +{
> > +	int i;
> 
> Why signed? But see below as well.

Will switch to unsigned.


> 
> > +	keypad->line_gpios = devm_gpiod_get_array(&pdev->dev, "line", GPIOD_IN);
> > +	if (IS_ERR(keypad->line_gpios))
> > +		return PTR_ERR(keypad->line_gpios);
> > +
> > +	keypad->nlines = keypad->line_gpios->ndescs;
> > +
> > +	if (keypad->nlines > MATRIX_MAX_ROWS)
> > +		return -EINVAL;
> 
> > +	for (i = 0; i < keypad->nlines; i++)
> 
> iterator is local to the loop, hence
> 
> 	for (unsigned int i = 0; i < keypad->nlines; i++)

Ok


> 
> > +		gpiod_set_consumer_name(keypad->line_gpios->desc[i], "charlieplex_kbd_line");
> > +
> > +	return 0;
> > +}
> 
> ...
> 
> > +static int charlieplex_keypad_probe(struct platform_device *pdev)
> > +{
> > +	struct charlieplex_keypad *keypad;
> > +	unsigned int debounce_interval_ms;
> > +	unsigned int poll_interval_ms;
> > +	struct input_dev *input_dev;
> 
> > +	int err;
> 
> The naming is even inconsistent between the functions...

Agreed, will fix as stated above.


> 
> > +	keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> > +	if (!keypad)
> > +		return -ENOMEM;
> > +
> > +	input_dev = devm_input_allocate_device(&pdev->dev);
> > +	if (!input_dev)
> > +		return -ENOMEM;
> > +
> > +	keypad->input_dev = input_dev;
> > +
> > +	device_property_read_u32(&pdev->dev, "poll-interval", &poll_interval_ms);
> > +	device_property_read_u32(&pdev->dev, "debounce-delay-ms", &debounce_interval_ms);
> > +	device_property_read_u32(&pdev->dev, "settling-time-us", &keypad->settling_time_us);
> > +
> > +	keypad->current_code = -1;
> > +	keypad->debounce_code = -1;
> > +	keypad->debounce_threshold = DIV_ROUND_UP(debounce_interval_ms, poll_interval_ms);
> > +
> > +	err = charlieplex_keypad_init_gpio(pdev, keypad);
> > +	if (err)
> > +		return err;
> > +
> > +	input_dev->name		= pdev->name;
> > +	input_dev->id.bustype	= BUS_HOST;
> > +
> > +	err = matrix_keypad_build_keymap(NULL, NULL, keypad->nlines,
> > +					 keypad->nlines, NULL, input_dev);
> > +	if (err)
> > +		dev_err_probe(&pdev->dev, -ENOMEM, "failed to build keymap\n");
> > +
> > +	if (device_property_read_bool(&pdev->dev, "autorepeat"))
> > +		__set_bit(EV_REP, input_dev->evbit);
> > +
> > +	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
> > +
> > +	err = input_setup_polling(input_dev, charlieplex_keypad_poll);
> > +	if (err)
> > +		dev_err_probe(&pdev->dev, err, "unable to set up polling\n");
> > +
> > +	input_set_poll_interval(input_dev, poll_interval_ms);
> > +
> > +	input_set_drvdata(input_dev, keypad);
> > +
> > +	err = input_register_device(keypad->input_dev);
> > +	if (err)
> > +		return err;
> 
> > +	platform_set_drvdata(pdev, keypad);
> 
> Is this needed?

No, will remove it, and replace last lines with:

   return input_register_device(keypad->input_dev);


> 
> > +	return 0;
> > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
> 

Hugo Villeneuve

Re: [PATCH v3 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad

[Hugo Villeneuve]

On Wed, 25 Feb 2026 11:41:55 -0500
Hugo Villeneuve <hugo@hugovil.com> wrote:

> Hi Andy,
> thank you for the review.
> 
> On Wed, 25 Feb 2026 18:12:13 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
> > On Wed, Feb 25, 2026 at 10:54:01AM -0500, Hugo Villeneuve wrote:
> > 
> > > Add support for GPIO-based charlieplex keypad, allowing to control
> > > N^2-N keys using N GPIO lines.
> > > 
> > > Reuse matrix keypad keymap to simplify, even if there is no concept
> > > of rows and columns in this type of keyboard.
> > 
> > ...
> > 
> > > +/*
> > > + *  GPIO driven charlieplex keypad driver
> > > + *
> > > + *  Copyright (c) 2025 Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > + *
> > > + *  Based on matrix_keyboard.c
> > 
> > A single space after asterisk is enough.
> 
> Ok, leftover from copy/paste from matrix_keyboard.c :)
> 
> > 
> > > + */
> > 
> > ...
> > 
> > + bitops.h
> > 
> > > +#include <linux/delay.h>
> > 
> > + dev_printk.h
> > + device/devres.h
> > + err.h
> > 
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/input.h>
> > > +#include <linux/input/matrix_keypad.h>
> > 
> > + math.h
> 
> Ok.
> 
> 
> > 
> > > +#include <linux/module.h>
> > 
> > > +#include <linux/of.h>
> > 
> > Is this in use? Or you wanted mod_devicetable.h for OF ID table?
> 
> I need only OF ID table, so will replace with mod_devicetable.h.

Hi Andy,
finally I need <linux/of.h> for of_match_ptr()...

But I will keep <mod_devicetable.h> ...


> > 
> > > +#include <linux/platform_device.h>
> > > +#include <linux/property.h>
> > > +#include <linux/types.h>
> > 
> > ...
> > 
> > > +	for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
> > > +		DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
> > > +		int iline;
> > 
> > > +		int rc;
> > 
> > I think Dmitry prefers 'error' name for this kind of variables.
> 
> I hate using "error", can be so misleading :)
> 
> I would prefer to use "rc" everywhere, but if Dmitry chimes in and
> specifies "error" or "err", then so it will be.
> 
> 
> > 
> > > +		/* Activate only one line as output at a time. */
> > > +		gpiod_direction_output(keypad->line_gpios->desc[oline], 1);
> > > +
> > > +		if (keypad->settling_time_us)
> > > +			fsleep(keypad->settling_time_us);
> > > +
> > > +		/* Read input on all other lines. */
> > > +		rc = gpiod_get_array_value_cansleep(keypad->line_gpios->ndescs,
> > > +						    keypad->line_gpios->desc,
> > > +						    keypad->line_gpios->info, values);
> > > +		if (rc)
> > > +			return;
> > > +
> > > +		for (iline = 0; iline < keypad->nlines; iline++) {
> > > +			if (iline == oline)
> > > +				continue; /* Do not read active output line. */
> > > +
> > > +			/* Check if GPIO is asserted. */
> > > +			if (test_bit(iline, values)) {
> > > +				code = MATRIX_SCAN_CODE(oline, iline,
> > > +							get_count_order(keypad->nlines));
> > > +				/*
> > > +				 * Exit loop immediately since we cannot detect
> > > +				 * more than one key press at a time.
> > > +				 */
> > > +				break;
> > > +			}
> > > +		}
> > > +
> > > +		gpiod_direction_input(keypad->line_gpios->desc[oline]);
> > > +
> > > +		if (code)
> > > +			break;
> > > +	}
> > 
> > ...
> > 
> > > +static int charlieplex_keypad_init_gpio(struct platform_device *pdev,
> > > +					struct charlieplex_keypad *keypad)
> > > +{
> > > +	int i;
> > 
> > Why signed? But see below as well.
> 
> Will switch to unsigned.
> 
> 
> > 
> > > +	keypad->line_gpios = devm_gpiod_get_array(&pdev->dev, "line", GPIOD_IN);
> > > +	if (IS_ERR(keypad->line_gpios))
> > > +		return PTR_ERR(keypad->line_gpios);
> > > +
> > > +	keypad->nlines = keypad->line_gpios->ndescs;
> > > +
> > > +	if (keypad->nlines > MATRIX_MAX_ROWS)
> > > +		return -EINVAL;
> > 
> > > +	for (i = 0; i < keypad->nlines; i++)
> > 
> > iterator is local to the loop, hence
> > 
> > 	for (unsigned int i = 0; i < keypad->nlines; i++)
> 
> Ok
> 
> 
> > 
> > > +		gpiod_set_consumer_name(keypad->line_gpios->desc[i], "charlieplex_kbd_line");
> > > +
> > > +	return 0;
> > > +}
> > 
> > ...
> > 
> > > +static int charlieplex_keypad_probe(struct platform_device *pdev)
> > > +{
> > > +	struct charlieplex_keypad *keypad;
> > > +	unsigned int debounce_interval_ms;
> > > +	unsigned int poll_interval_ms;
> > > +	struct input_dev *input_dev;
> > 
> > > +	int err;
> > 
> > The naming is even inconsistent between the functions...
> 
> Agreed, will fix as stated above.
> 
> 
> > 
> > > +	keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> > > +	if (!keypad)
> > > +		return -ENOMEM;
> > > +
> > > +	input_dev = devm_input_allocate_device(&pdev->dev);
> > > +	if (!input_dev)
> > > +		return -ENOMEM;
> > > +
> > > +	keypad->input_dev = input_dev;
> > > +
> > > +	device_property_read_u32(&pdev->dev, "poll-interval", &poll_interval_ms);
> > > +	device_property_read_u32(&pdev->dev, "debounce-delay-ms", &debounce_interval_ms);
> > > +	device_property_read_u32(&pdev->dev, "settling-time-us", &keypad->settling_time_us);
> > > +
> > > +	keypad->current_code = -1;
> > > +	keypad->debounce_code = -1;
> > > +	keypad->debounce_threshold = DIV_ROUND_UP(debounce_interval_ms, poll_interval_ms);
> > > +
> > > +	err = charlieplex_keypad_init_gpio(pdev, keypad);
> > > +	if (err)
> > > +		return err;
> > > +
> > > +	input_dev->name		= pdev->name;
> > > +	input_dev->id.bustype	= BUS_HOST;
> > > +
> > > +	err = matrix_keypad_build_keymap(NULL, NULL, keypad->nlines,
> > > +					 keypad->nlines, NULL, input_dev);
> > > +	if (err)
> > > +		dev_err_probe(&pdev->dev, -ENOMEM, "failed to build keymap\n");
> > > +
> > > +	if (device_property_read_bool(&pdev->dev, "autorepeat"))
> > > +		__set_bit(EV_REP, input_dev->evbit);
> > > +
> > > +	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
> > > +
> > > +	err = input_setup_polling(input_dev, charlieplex_keypad_poll);
> > > +	if (err)
> > > +		dev_err_probe(&pdev->dev, err, "unable to set up polling\n");
> > > +
> > > +	input_set_poll_interval(input_dev, poll_interval_ms);
> > > +
> > > +	input_set_drvdata(input_dev, keypad);
> > > +
> > > +	err = input_register_device(keypad->input_dev);
> > > +	if (err)
> > > +		return err;
> > 
> > > +	platform_set_drvdata(pdev, keypad);
> > 
> > Is this needed?
> 
> No, will remove it, and replace last lines with:
> 
>    return input_register_device(keypad->input_dev);
> 
> 
> > 
> > > +	return 0;
> > > +}
> > 
> > -- 
> > With Best Regards,
> > Andy Shevchenko
> > 
> > 
> > 
> 
> Hugo Villeneuve


-- 
Hugo Villeneuve

Re: [PATCH v3 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad

[Dmitry Torokhov]

On Wed, Feb 25, 2026 at 11:56:52AM -0500, Hugo Villeneuve wrote:
> On Wed, 25 Feb 2026 11:41:55 -0500
> Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > Hi Andy,
> > thank you for the review.
> > 
> > On Wed, 25 Feb 2026 18:12:13 +0200
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > 
> > > On Wed, Feb 25, 2026 at 10:54:01AM -0500, Hugo Villeneuve wrote:
> > > 
> > > > Add support for GPIO-based charlieplex keypad, allowing to control
> > > > N^2-N keys using N GPIO lines.
> > > > 
> > > > Reuse matrix keypad keymap to simplify, even if there is no concept
> > > > of rows and columns in this type of keyboard.
> > > 
> > > ...
> > > 
> > > > +/*
> > > > + *  GPIO driven charlieplex keypad driver
> > > > + *
> > > > + *  Copyright (c) 2025 Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > + *
> > > > + *  Based on matrix_keyboard.c
> > > 
> > > A single space after asterisk is enough.
> > 
> > Ok, leftover from copy/paste from matrix_keyboard.c :)
> > 
> > > 
> > > > + */
> > > 
> > > ...
> > > 
> > > + bitops.h
> > > 
> > > > +#include <linux/delay.h>
> > > 
> > > + dev_printk.h
> > > + device/devres.h
> > > + err.h
> > > 
> > > > +#include <linux/gpio/consumer.h>
> > > > +#include <linux/input.h>
> > > > +#include <linux/input/matrix_keypad.h>
> > > 
> > > + math.h
> > 
> > Ok.
> > 
> > 
> > > 
> > > > +#include <linux/module.h>
> > > 
> > > > +#include <linux/of.h>
> > > 
> > > Is this in use? Or you wanted mod_devicetable.h for OF ID table?
> > 
> > I need only OF ID table, so will replace with mod_devicetable.h.
> 
> Hi Andy,
> finally I need <linux/of.h> for of_match_ptr()...
> 
> But I will keep <mod_devicetable.h> ...

Do we need the dependency on OF? We may include of match pointer
unconditionally and the driver could be used on ACPI systems with
PRP0001 HID.

Thanks.

-- 
Dmitry

Re: [PATCH v3 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad

[Andy Shevchenko]

On Wed, Feb 25, 2026 at 09:14:16AM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 25, 2026 at 11:56:52AM -0500, Hugo Villeneuve wrote:
> > On Wed, 25 Feb 2026 11:41:55 -0500
> > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > On Wed, 25 Feb 2026 18:12:13 +0200
> > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > > > On Wed, Feb 25, 2026 at 10:54:01AM -0500, Hugo Villeneuve wrote:

...

> > > > > +#include <linux/of.h>
> > > > 
> > > > Is this in use? Or you wanted mod_devicetable.h for OF ID table?
> > > 
> > > I need only OF ID table, so will replace with mod_devicetable.h.
> > 
> > Hi Andy,
> > finally I need <linux/of.h> for of_match_ptr()...
> > 
> > But I will keep <mod_devicetable.h> ...
> 
> Do we need the dependency on OF? We may include of match pointer
> unconditionally and the driver could be used on ACPI systems with
> PRP0001 HID.

Not only that. of_match_ptr() or ACPI_PTR() shouldn't be used in a new code
(there are, of course, _rare_ corner cases, which this one is not one of).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad

[Hugo Villeneuve]

On Wed, 25 Feb 2026 19:21:36 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Feb 25, 2026 at 09:14:16AM -0800, Dmitry Torokhov wrote:
> > On Wed, Feb 25, 2026 at 11:56:52AM -0500, Hugo Villeneuve wrote:
> > > On Wed, 25 Feb 2026 11:41:55 -0500
> > > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > On Wed, 25 Feb 2026 18:12:13 +0200
> > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > > > > On Wed, Feb 25, 2026 at 10:54:01AM -0500, Hugo Villeneuve wrote:
> 
> ...
> 
> > > > > > +#include <linux/of.h>
> > > > > 
> > > > > Is this in use? Or you wanted mod_devicetable.h for OF ID table?
> > > > 
> > > > I need only OF ID table, so will replace with mod_devicetable.h.
> > > 
> > > Hi Andy,
> > > finally I need <linux/of.h> for of_match_ptr()...
> > > 
> > > But I will keep <mod_devicetable.h> ...
> > 
> > Do we need the dependency on OF? We may include of match pointer
> > unconditionally and the driver could be used on ACPI systems with
> > PRP0001 HID.
> 
> Not only that. of_match_ptr() or ACPI_PTR() shouldn't be used in a new code
> (there are, of course, _rare_ corner cases, which this one is not one of).

Ok, will remove of_match_ptr() and also include <linux/of.h>

Hugo Villeneuve

Re: [PATCH v3 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad

[Dmitry Torokhov]

Hi Hugo,

On Wed, Feb 25, 2026 at 11:41:55AM -0500, Hugo Villeneuve wrote:
> Hi Andy,
> thank you for the review.
> 
> On Wed, 25 Feb 2026 18:12:13 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
> > On Wed, Feb 25, 2026 at 10:54:01AM -0500, Hugo Villeneuve wrote:
> > 
> > > Add support for GPIO-based charlieplex keypad, allowing to control
> > > N^2-N keys using N GPIO lines.
> > > 
> > > Reuse matrix keypad keymap to simplify, even if there is no concept
> > > of rows and columns in this type of keyboard.
> > 
> > ...
> > 
> > > +/*
> > > + *  GPIO driven charlieplex keypad driver
> > > + *
> > > + *  Copyright (c) 2025 Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > + *
> > > + *  Based on matrix_keyboard.c
> > 
> > A single space after asterisk is enough.
> 
> Ok, leftover from copy/paste from matrix_keyboard.c :)
> 
> > 
> > > + */
> > 
> > ...
> > 
> > + bitops.h
> > 
> > > +#include <linux/delay.h>
> > 
> > + dev_printk.h
> > + device/devres.h
> > + err.h
> > 
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/input.h>
> > > +#include <linux/input/matrix_keypad.h>
> > 
> > + math.h
> 
> Ok.
> 
> 
> > 
> > > +#include <linux/module.h>
> > 
> > > +#include <linux/of.h>
> > 
> > Is this in use? Or you wanted mod_devicetable.h for OF ID table?
> 
> I need only OF ID table, so will replace with mod_devicetable.h.
> 
> 
> > 
> > > +#include <linux/platform_device.h>
> > > +#include <linux/property.h>
> > > +#include <linux/types.h>
> > 
> > ...
> > 
> > > +	for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
> > > +		DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
> > > +		int iline;
> > 
> > > +		int rc;
> > 
> > I think Dmitry prefers 'error' name for this kind of variables.
> 
> I hate using "error", can be so misleading :)
> 
> I would prefer to use "rc" everywhere, but if Dmitry chimes in and
> specifies "error" or "err", then so it will be.

Yes, I prefer err or error for variables that carry error code or 0.
This allows to write

	error = action(...);
	if (error) {
		// handle it
	}

which is very clear IMO.


> > > +
> > > +	err = input_register_device(keypad->input_dev);
> > > +	if (err)
> > > +		return err;
> > 
> > > +	platform_set_drvdata(pdev, keypad);
> > 
> > Is this needed?
> 
> No, will remove it, and replace last lines with:
> 
>    return input_register_device(keypad->input_dev);

Please use

	err = input_register_device(...);
	if (err)
		return err;

	return 0;

It clearly differentiates error an normal paths, shows that function
returns 0 and not anything else on success, and allows to reorder
or add additional actions easily.

Thanks.

-- 
Dmitry

Re: [PATCH v3 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad

[Hugo Villeneuve]

Hi Dmitry,

On Wed, 25 Feb 2026 09:12:57 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> Hi Hugo,
> 
> On Wed, Feb 25, 2026 at 11:41:55AM -0500, Hugo Villeneuve wrote:
> > Hi Andy,
> > thank you for the review.
> > 
> > On Wed, 25 Feb 2026 18:12:13 +0200
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > 
> > > On Wed, Feb 25, 2026 at 10:54:01AM -0500, Hugo Villeneuve wrote:
> > > 
> > > > Add support for GPIO-based charlieplex keypad, allowing to control
> > > > N^2-N keys using N GPIO lines.
> > > > 
> > > > Reuse matrix keypad keymap to simplify, even if there is no concept
> > > > of rows and columns in this type of keyboard.
> > > 
> > > ...
> > > 
> > > > +/*
> > > > + *  GPIO driven charlieplex keypad driver
> > > > + *
> > > > + *  Copyright (c) 2025 Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > + *
> > > > + *  Based on matrix_keyboard.c
> > > 
> > > A single space after asterisk is enough.
> > 
> > Ok, leftover from copy/paste from matrix_keyboard.c :)
> > 
> > > 
> > > > + */
> > > 
> > > ...
> > > 
> > > + bitops.h
> > > 
> > > > +#include <linux/delay.h>
> > > 
> > > + dev_printk.h
> > > + device/devres.h
> > > + err.h
> > > 
> > > > +#include <linux/gpio/consumer.h>
> > > > +#include <linux/input.h>
> > > > +#include <linux/input/matrix_keypad.h>
> > > 
> > > + math.h
> > 
> > Ok.
> > 
> > 
> > > 
> > > > +#include <linux/module.h>
> > > 
> > > > +#include <linux/of.h>
> > > 
> > > Is this in use? Or you wanted mod_devicetable.h for OF ID table?
> > 
> > I need only OF ID table, so will replace with mod_devicetable.h.
> > 
> > 
> > > 
> > > > +#include <linux/platform_device.h>
> > > > +#include <linux/property.h>
> > > > +#include <linux/types.h>
> > > 
> > > ...
> > > 
> > > > +	for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
> > > > +		DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
> > > > +		int iline;
> > > 
> > > > +		int rc;
> > > 
> > > I think Dmitry prefers 'error' name for this kind of variables.
> > 
> > I hate using "error", can be so misleading :)
> > 
> > I would prefer to use "rc" everywhere, but if Dmitry chimes in and
> > specifies "error" or "err", then so it will be.
> 
> Yes, I prefer err or error for variables that carry error code or 0.
> This allows to write
> 
> 	error = action(...);
> 	if (error) {
> 		// handle it
> 	}
> 
> which is very clear IMO.

Ok, will fix it globally to "err" everywhere in V4.


> 
> 
> > > > +
> > > > +	err = input_register_device(keypad->input_dev);
> > > > +	if (err)
> > > > +		return err;
> > > 
> > > > +	platform_set_drvdata(pdev, keypad);
> > > 
> > > Is this needed?
> > 
> > No, will remove it, and replace last lines with:
> > 
> >    return input_register_device(keypad->input_dev);
> 
> Please use
> 
> 	err = input_register_device(...);
> 	if (err)
> 		return err;
> 
> 	return 0;
> 
> It clearly differentiates error an normal paths, shows that function
> returns 0 and not anything else on success, and allows to reorder
> or add additional actions easily.

Ok.

Thank you,
Hugo

Re: [PATCH v3 0/4] input: add GPIO-based charlieplex keypad

[Geert Uytterhoeven]

Hi Hugo,

Thanks for your series!

On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> Hello,
> this patch series add a new GPIO charlieplex keypad driver.
>
> The first two patches simply commonize two properties that are present in
> a few bindings, so that the actual patches for the charlieplex keypad driver
> can reuse them instead of also redefining them.
>
> I have tested the driver on a custom board with a Solidrun RZ/G2LC SOM
> with three charlieplex keyboards, all connected thru a single PCA9416 I2C GPIO

PCA9416? The closest I could find is TCA9416, which is something
different.

> expander.
>
> Link: [v1] https://lore.kernel.org/all/20260203155023.536103-1-hugo@hugovil.com/
> Link: [v2] https://lore.kernel.org/all/20260213171431.2228814-1-hugo@hugovil.com/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 0/4] input: add GPIO-based charlieplex keypad

[Hugo Villeneuve]

Hi Geert,

On Thu, 26 Feb 2026 10:20:33 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Hugo,
> 
> Thanks for your series!
> 
> On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >
> > Hello,
> > this patch series add a new GPIO charlieplex keypad driver.
> >
> > The first two patches simply commonize two properties that are present in
> > a few bindings, so that the actual patches for the charlieplex keypad driver
> > can reuse them instead of also redefining them.
> >
> > I have tested the driver on a custom board with a Solidrun RZ/G2LC SOM
> > with three charlieplex keyboards, all connected thru a single PCA9416 I2C GPIO
> 
> PCA9416? The closest I could find is TCA9416, which is something
> different.

Its a typo, it is PCA6416, thanks for catching that!

Hugo.


> 
> > expander.
> >
> > Link: [v1] https://lore.kernel.org/all/20260203155023.536103-1-hugo@hugovil.com/
> > Link: [v2] https://lore.kernel.org/all/20260213171431.2228814-1-hugo@hugovil.com/
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 


-- 
Hugo Villeneuve