[PATCH v4 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad
Hugo Villeneuve
hugo at hugovil.com
Thu Mar 12 09:12:17 PDT 2026
Hi Andy,
On Fri, 6 Mar 2026 16:21:09 +0200
Andy Shevchenko <andriy.shevchenko at intel.com> wrote:
> On Thu, Mar 05, 2026 at 02:20:50PM -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.
>
> ...
>
> > --- 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
>
> Seem unordered. At least the all around it is ordered AFAICS.
Will fix it.
>
> > obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
> > obj-$(CONFIG_KEYBOARD_MAX7360) += max7360-keypad.o
> > obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
>
> ...
>
> > +/*
> > + * GPIO driven charlieplex keypad driver
> > + *
> > + * Copyright (c) 2025 Hugo Villeneuve <hvilleneuve at dimonoff.com>
>
> 2026?
Ok
>
> > + *
> > + * Based on matrix_keyboard.c
> > + */
>
> ...
>
> > +#include <linux/bitops.h>
> > +#include <linux/delay.h>
> > +#include <linux/dev_printk.h>
> > +#include <linux/device/devres.h>
> > +#include <linux/err.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/input.h>
> > +#include <linux/input/matrix_keypad.h>
> > +#include <linux/math.h>
> > +#include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/types.h>
>
> > +static void charlieplex_keypad_poll(struct input_dev *input)
> > +{
> > + struct charlieplex_keypad *keypad = input_get_drvdata(input);
>
> > + int oline;
>
> Why signed?
Will change this one, and others noted below to unsigned.
>
> > + int code;
> > +
> > + for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
>
> Can be like
>
> code = 0;
> for (unsigned int oline = 0; oline < keypad->nlines; oline++) {
>
> as iterator is not used outside the loop.
Ok, will do.
>
> > + DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
>
> > + int iline;
>
> Why signed?
>
> > + int err;
> > +
> > + /* 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. */
> > + err = gpiod_get_array_value_cansleep(keypad->line_gpios->ndescs,
> > + keypad->line_gpios->desc,
> > + keypad->line_gpios->info, values);
> > + if (err)
> > + return;
>
> > + for (iline = 0; iline < keypad->nlines; iline++) {
>
> Can be just
>
> for (unsigned int iline = 0; iline < keypad->nlines; iline++) {
>
> as iterator is not used outside the loop.
Ok.
>
> > + 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);
> > +}
>
> ...
>
> > + for (unsigned int i = 0; i < keypad->nlines; i++)
> > + (keypad->line_gpios->desc[i], "charlieplex_kbd_line");
>
> Hmm... Don't you want to give it an index?
Makes sense, will do.
I will also change for "keypad->line_gpios->ndescs" in the loop
iterator.
>
> (In case you go this direction, see the kasprintf_strarray() or
> its managed variant.)
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
--
Hugo Villeneuve
More information about the Linux-mediatek
mailing list