[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