[PATCH] input: add support for generic GPIO-based matrix keypad

[PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

Original patch by Marek Vasut, modified by Eric in:

1. use delayed work to simplify the debouncing
2. build keycode array for fast lookup
3. combine col_polarity/row_polarity into a single active_low field
(are there some cases where the GPIOs are externally connected
with an inverter and thus causing two different polarity requirement??)
4. use a generic bit array based XOR algorithm to detect key press/release,
which should make the column assertion time shorter and code a bit
cleaner
5. remove the ALT_FN handling, which is no way generic, the ALT_FN
key should be treated as no different from other keys, and translation
will be done by user space by commands like 'loadkeys'.

Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
controller, I have to configure those pins as generic GPIO to use this
driver, works quite well, though ;-)

I might left some points to make this generic and ideally simplified,
please feel free to add comments.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Signed-off-by: Eric Miao <eric.miao@marvell.com>
---
 drivers/input/keyboard/Kconfig         |   10 +
 drivers/input/keyboard/Makefile        |    1 +
 drivers/input/keyboard/matrix_keypad.c |  345 ++++++++++++++++++++++++++++++++
 include/linux/input/matrix_keypad.h    |   34 +++
 4 files changed, 390 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/matrix_keypad.c
 create mode 100644 include/linux/input/matrix_keypad.h

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index ea2638b..6b9f89c 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -332,4 +332,14 @@ config KEYBOARD_SH_KEYSC

 	  To compile this driver as a module, choose M here: the
 	  module will be called sh_keysc.
+
+config KEYBOARD_MATRIX
+	tristate "GPIO driven matrix keypad support"
+	depends on GENERIC_GPIO
+	help
+	  Enable support for GPIO driven matrix keypad
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called matrix_keypad.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 36351e1..1349408 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_KEYBOARD_HP7XX)		+= jornada720_kbd.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
 obj-$(CONFIG_KEYBOARD_SH_KEYSC)		+= sh_keysc.o
+obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
diff --git a/drivers/input/keyboard/matrix_keypad.c
b/drivers/input/keyboard/matrix_keypad.c
new file mode 100644
index 0000000..da9a3fc
--- /dev/null
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -0,0 +1,345 @@
+/*
+ * drivers/input/keyboard/matrix_keypad.c
+ *
+ *  GPIO driven matrix keyboard driver
+ *
+ *  Copyright (c) 2008 Marek Vasut <marek.vasut@gmail.com>
+ *
+ *  Based on corgikbd.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/input/matrix_keypad.h>
+
+struct matrix_keypad {
+	struct matrix_keypad_platform_data *pdata;
+	struct input_dev *input_dev;
+
+	uint32_t last_key_state[MATRIX_MAX_COLS];
+	uint32_t *keycodes;
+	struct delayed_work work;
+};
+
+static void build_keycodes(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	struct input_dev *input_dev = keypad->input_dev;
+	uint32_t *key;
+	int i;
+
+	keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
+
+	key = &pdata->key_map[0];
+	for (i = 0; i < pdata->key_map_size; i++, key++) {
+		keypad->keycodes[KEY_ROWCOL(*key)] = KEY_VAL(*key);
+		set_bit(KEY_VAL(*key), input_dev->keybit);
+	}
+}
+
+static unsigned int lookup_keycode(struct matrix_keypad *keypad,
+				   int row, int col)
+{
+	return keypad->keycodes[(row << 4) + col];
+}
+
+static void activate_all_cols(struct matrix_keypad_platform_data
*pdata, int on)
+{
+	int i;
+	
+	for (i = 0; i < pdata->num_col_gpios; i++)
+		gpio_set_value(pdata->col_gpios[i],
+			(on) ? !pdata->active_low : pdata->active_low);
+}
+
+static void activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	gpio_set_value(pdata->col_gpios[col],
+			(on) ? !pdata->active_low : pdata->active_low);
+
+	if (on && pdata->col_scan_delay_us)
+		udelay(pdata->col_scan_delay_us);
+}
+
+static int row_asserted(struct matrix_keypad_platform_data *pdata, int row)
+{
+	return gpio_get_value(pdata->row_gpios[row]) ?
+			!pdata->active_low : pdata->active_low;
+}
+
+static void enable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		enable_irq(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+static void disable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+/*
+ * This gets the keys from keyboard and reports it to input subsystem
+ */
+static void matrix_keypad_scan(struct work_struct *work)
+{
+	struct matrix_keypad *keypad =
+	       	container_of(work, struct matrix_keypad, work.work);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	uint32_t new_state[MATRIX_MAX_COLS];
+	int row, col;
+
+	/* de-activate all columns for scanning */
+	activate_all_cols(pdata, 0);
+
+	memset(new_state, 0, sizeof(new_state));
+
+	/* assert each column and read the row status out */
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+
+		activate_col(pdata, col, 1);
+
+		for (row = 0; row < pdata->num_row_gpios; row++)
+			new_state[col] |= row_asserted(pdata, row) ?
+						(1 << row) : 0;
+		activate_col(pdata, col, 0);
+	}
+
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+		uint32_t bits_changed;
+		
+		bits_changed = keypad->last_key_state[col] ^ new_state[col];
+		if (bits_changed == 0)
+			continue;
+
+		for (row = 0; row < pdata->num_row_gpios; row++) {
+			if ((bits_changed & (1 << row)) == 0)
+				continue;
+
+			input_report_key(keypad->input_dev,
+					lookup_keycode(keypad, row, col),
+					new_state[col] & (1 << row));
+		}
+	}
+	input_sync(keypad->input_dev);
+	memcpy(keypad->last_key_state, new_state, sizeof(new_state));
+
+	activate_all_cols(pdata, 1);
+	enable_row_irqs(keypad);
+}
+
+static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
+{
+	struct matrix_keypad *keypad = id;
+
+	disable_row_irqs(keypad);
+	schedule_delayed_work(&keypad->work,
+			msecs_to_jiffies(keypad->pdata->debounce_ms));
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM
+static int matrix_keypad_suspend(struct platform_device *pdev,
pm_message_t state)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	return 0;
+}
+
+static int matrix_keypad_resume(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	return 0;
+}
+#else
+#define matrix_keypad_suspend	NULL
+#define matrix_keypad_resume	NULL
+#endif
+
+static int __devinit init_matrix_gpio(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i, err = -EINVAL;
+
+	/* initialized strobe lines as outputs, activated */
+	for (i = 0; i < pdata->num_col_gpios; i++) {
+		err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
+		if (err) {
+			pr_err("failed to request GPIO%d for COL%d\n",
+					pdata->col_gpios[i], i);
+			goto err_free_cols;
+		}
+
+		gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
+		if (err) {
+			pr_err("failed to request GPIO%d for ROW%d\n",
+					pdata->row_gpios[i], i);
+			goto err_free_rows;
+		}
+
+		gpio_direction_input(pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
+				matrix_keypad_interrupt, IRQF_DISABLED |
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				"matrix-keypad", keypad);
+		if (err) {
+			pr_err("Unable to acquire interrupt for GPIO line %i\n",
+				pdata->row_gpios[i]);
+			goto err_free_irqs;
+		}
+	}
+	return 0;
+
+err_free_irqs:
+	for (i = i - 1; i >= 0; i--)
+		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+
+err_free_rows:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->row_gpios[i]);
+
+err_free_cols:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->col_gpios[i]);
+
+	return err;
+}
+
+static int __devinit matrix_keypad_probe(struct platform_device *pdev)
+{
+	struct matrix_keypad_platform_data *pdata;
+	struct matrix_keypad *keypad;
+	struct input_dev *input_dev;
+	int err = -ENOMEM;
+
+	if ((pdata = pdev->dev.platform_data) == NULL) {
+		dev_err(&pdev->dev, "no platform data defined\n");
+		return -EINVAL;
+	}
+
+	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
+	if (keypad == NULL)
+		return -ENOMEM;
+
+	input_dev = input_allocate_device();
+	if (!input_dev)
+		goto err_free_keypad;
+
+	platform_set_drvdata(pdev, keypad);
+
+	keypad->input_dev = input_dev;
+	keypad->pdata = pdata;
+	INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
+
+	input_dev->name		= pdev->name;
+	input_dev->id.bustype	= BUS_HOST;
+	input_dev->dev.parent	= &pdev->dev;
+	input_dev->evbit[0]	= BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+
+	build_keycodes(keypad);
+
+	err = input_register_device(keypad->input_dev);
+	if (err)
+		goto err_free_input;
+
+	err = init_matrix_gpio(keypad);
+	if (err)
+		goto err_unregister;
+
+	return 0;
+
+err_unregister:
+	input_unregister_device(input_dev);
+err_free_input:
+	input_free_device(input_dev);
+err_free_keypad:
+	kfree(keypad);
+	return err;
+}
+
+static int matrix_keypad_remove(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < keypad->pdata->num_row_gpios; i++) {
+		free_irq(gpio_to_irq(keypad->pdata->row_gpios[i]), keypad);
+		gpio_free(keypad->pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < keypad->pdata->num_col_gpios; i++)
+		gpio_free(keypad->pdata->col_gpios[i]);
+
+	input_unregister_device(keypad->input_dev);
+	kfree(keypad);
+	return 0;
+}
+
+static struct platform_driver matrix_keypad_driver = {
+	.probe		= matrix_keypad_probe,
+	.remove		= matrix_keypad_remove,
+	.suspend	= matrix_keypad_suspend,
+	.resume		= matrix_keypad_resume,
+	.driver		= {
+		.name	= "matrix-keypad",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __devinit matrix_keypad_init(void)
+{
+	return platform_driver_register(&matrix_keypad_driver);
+}
+
+static void __exit matrix_keypad_exit(void)
+{
+	platform_driver_unregister(&matrix_keypad_driver);
+}
+
+module_init(matrix_keypad_init);
+module_exit(matrix_keypad_exit);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
+MODULE_DESCRIPTION("GPIO Driven Matrix Keypad Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:matrix-keypad");
diff --git a/include/linux/input/matrix_keypad.h
b/include/linux/input/matrix_keypad.h
new file mode 100644
index 0000000..8b661cb
--- /dev/null
+++ b/include/linux/input/matrix_keypad.h
@@ -0,0 +1,34 @@
+#ifndef _MATRIX_KEYPAD_H
+#define _MATRIX_KEYPAD_H
+
+#include <linux/input.h>
+
+#define MATRIX_MAX_ROWS		16
+#define MATRIX_MAX_COLS		16
+#define MATRIX_MAX_KEYS		(MATRIX_MAX_ROWS * MATRIX_MAX_COLS)
+
+struct matrix_keypad_platform_data {
+	/* scancode map for the matrix keys */
+	uint32_t	*key_map;
+	int		key_map_size;
+
+	unsigned	row_gpios[MATRIX_MAX_ROWS];
+	unsigned	col_gpios[MATRIX_MAX_COLS];
+	int		num_row_gpios;
+	int		num_col_gpios;
+
+	unsigned int	active_low;
+	unsigned int	col_scan_delay_us;
+
+	/* key debounce interval in milli-second */
+	unsigned int	debounce_ms;
+};
+
+#define KEY(row, col, val)	((((row) & (MATRIX_MAX_ROWS - 1)) << 28) |\
+				 (((col) & (MATRIX_MAX_COLS - 1)) << 24) |\
+				 (val & 0xffffff))
+
+#define KEY_ROWCOL(k)		(((k) >> 24) & 0xff)
+#define KEY_VAL(k)		((k) & 0xffffff)
+
+#endif /* _MATRIX_KEYPAD_H */
-- 
1.6.0.4

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Eric,

On Thu, May 7, 2009 at 1:30 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
> Original patch by Marek Vasut, modified by Eric in:
>
> 1. use delayed work to simplify the debouncing
> 2. build keycode array for fast lookup
> 3. combine col_polarity/row_polarity into a single active_low field
> (are there some cases where the GPIOs are externally connected
> with an inverter and thus causing two different polarity requirement??)
> 4. use a generic bit array based XOR algorithm to detect key press/release,
> which should make the column assertion time shorter and code a bit
> cleaner
> 5. remove the ALT_FN handling, which is no way generic, the ALT_FN
> key should be treated as no different from other keys, and translation
> will be done by user space by commands like 'loadkeys'.
>
> Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
> controller, I have to configure those pins as generic GPIO to use this
> driver, works quite well, though ;-)

Any support about removing/clearing ghost/phantom keys? Also we assume
that all gpios in the matrix should be able to generate interrupts so
no polldev support required.


> diff --git a/drivers/input/keyboard/matrix_keypad.c
> b/drivers/input/keyboard/matrix_keypad.c
> new file mode 100644
> index 0000000..da9a3fc
> --- /dev/null
> +++ b/drivers/input/keyboard/matrix_keypad.c
> @@ -0,0 +1,345 @@
> +/*
> + * drivers/input/keyboard/matrix_keypad.c
> + *

Better to remove full path and just write matrix_keypad.c

> + *  GPIO driven matrix keyboard driver
> + *
> + *  Copyright (c) 2008 Marek Vasut <marek.vasut@gmail.com>

You may want to add your Copyright here.

> + *
> + *  Based on corgikbd.c
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/gpio.h>
> +#include <linux/input/matrix_keypad.h>
> +


> +
> +static void build_keycodes(struct matrix_keypad *keypad)
> +{
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       struct input_dev *input_dev = keypad->input_dev;
> +       uint32_t *key;
> +       int i;
> +
> +       keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
> +
> +       key = &pdata->key_map[0];
> +       for (i = 0; i < pdata->key_map_size; i++, key++) {
> +               keypad->keycodes[KEY_ROWCOL(*key)] = KEY_VAL(*key);
> +               set_bit(KEY_VAL(*key), input_dev->keybit);

__set_bit


> +
> +
> +#ifdef CONFIG_PM
> +static int matrix_keypad_suspend(struct platform_device *pdev,
> pm_message_t state)
> +{
> +       struct matrix_keypad *keypad = platform_get_drvdata(pdev);
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       int i;
> +
> +       if (device_may_wakeup(&pdev->dev))
> +               for (i = 0; i < pdata->num_row_gpios; i++)
> +                       enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
> +
> +       return 0;
> +}
> +
> +static int matrix_keypad_resume(struct platform_device *pdev)
> +{
> +       struct matrix_keypad *keypad = platform_get_drvdata(pdev);
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       int i;
> +
> +       if (device_may_wakeup(&pdev->dev))

I think I have not seen device_init_wakeup anywhere in this code.

> +               for (i = 0; i < pdata->num_row_gpios; i++)
> +                       disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
> +
> +       return 0;
> +}
> +#else
> +#define matrix_keypad_suspend  NULL
> +#define matrix_keypad_resume   NULL
> +#endif
> +
> +
> +static int __devinit matrix_keypad_probe(struct platform_device *pdev)
> +{


> +
> +err_unregister:
> +       input_unregister_device(input_dev);
> +err_free_input:
> +       input_free_device(input_dev);

No need of input_free_device after input_unregister_device(..). You
can just do input_dev = NULL after input_unregister_device instead?


> +
> +static struct platform_driver matrix_keypad_driver = {
> +       .probe          = matrix_keypad_probe,
> +       .remove         = matrix_keypad_remove,

__exit_p ?




-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

On Thu, May 7, 2009 at 4:41 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
> Hi Eric,
>
> On Thu, May 7, 2009 at 1:30 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>> Original patch by Marek Vasut, modified by Eric in:
>>
>> 1. use delayed work to simplify the debouncing
>> 2. build keycode array for fast lookup
>> 3. combine col_polarity/row_polarity into a single active_low field
>> (are there some cases where the GPIOs are externally connected
>> with an inverter and thus causing two different polarity requirement??)
>> 4. use a generic bit array based XOR algorithm to detect key press/release,
>> which should make the column assertion time shorter and code a bit
>> cleaner
>> 5. remove the ALT_FN handling, which is no way generic, the ALT_FN
>> key should be treated as no different from other keys, and translation
>> will be done by user space by commands like 'loadkeys'.
>>
>> Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
>> controller, I have to configure those pins as generic GPIO to use this
>> driver, works quite well, though ;-)
>
> Any support about removing/clearing ghost/phantom keys? Also we assume
> that all gpios in the matrix should be able to generate interrupts so
> no polldev support required.
>

Is there some info about such ghost/phantom keys? Otherwise I'll
assume these are something like sticky control keys?

I'm not sure if there're any such support in console tools and in
input subsystem. Hard to say...

And the comments below are of great value, I'll incorporate these
suggestions in later submission. Thanks very much.

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Eric,

On Thu, May 7, 2009 at 2:18 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
> On Thu, May 7, 2009 at 4:41 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>> Hi Eric,
>>
>> On Thu, May 7, 2009 at 1:30 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>> Original patch by Marek Vasut, modified by Eric in:
>>>
>>> 1. use delayed work to simplify the debouncing
>>> 2. build keycode array for fast lookup
>>> 3. combine col_polarity/row_polarity into a single active_low field
>>> (are there some cases where the GPIOs are externally connected
>>> with an inverter and thus causing two different polarity requirement??)
>>> 4. use a generic bit array based XOR algorithm to detect key press/release,
>>> which should make the column assertion time shorter and code a bit
>>> cleaner
>>> 5. remove the ALT_FN handling, which is no way generic, the ALT_FN
>>> key should be treated as no different from other keys, and translation
>>> will be done by user space by commands like 'loadkeys'.
>>>
>>> Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
>>> controller, I have to configure those pins as generic GPIO to use this
>>> driver, works quite well, though ;-)
>>
>> Any support about removing/clearing ghost/phantom keys? Also we assume
>> that all gpios in the matrix should be able to generate interrupts so
>> no polldev support required.
>>
>
> Is there some info about such ghost/phantom keys? Otherwise I'll
> assume these are something like sticky control keys?
>
> I'm not sure if there're any such support in console tools and in
> input subsystem. Hard to say...
>
> And the comments below are of great value, I'll incorporate these
> suggestions in later submission. Thanks very much.
>

Could you please have look at following gpio_matrix driver?

gpio_matrix.c:
http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_matrix.c;h=c1f47651a4937d5c976a9625ca5da389dd7e4a7c;hb=HEAD

gpio_input.c:
http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_input.c;h=7e307f267a2a059b64a3fb9c8a379b149016b2f8;hb=HEAD

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

On Thu, May 7, 2009 at 4:51 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
> Hi Eric,
>
> On Thu, May 7, 2009 at 2:18 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>> On Thu, May 7, 2009 at 4:41 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>>> Hi Eric,
>>>
>>> On Thu, May 7, 2009 at 1:30 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>>> Original patch by Marek Vasut, modified by Eric in:
>>>>
>>>> 1. use delayed work to simplify the debouncing
>>>> 2. build keycode array for fast lookup
>>>> 3. combine col_polarity/row_polarity into a single active_low field
>>>> (are there some cases where the GPIOs are externally connected
>>>> with an inverter and thus causing two different polarity requirement??)
>>>> 4. use a generic bit array based XOR algorithm to detect key press/release,
>>>> which should make the column assertion time shorter and code a bit
>>>> cleaner
>>>> 5. remove the ALT_FN handling, which is no way generic, the ALT_FN
>>>> key should be treated as no different from other keys, and translation
>>>> will be done by user space by commands like 'loadkeys'.
>>>>
>>>> Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
>>>> controller, I have to configure those pins as generic GPIO to use this
>>>> driver, works quite well, though ;-)
>>>
>>> Any support about removing/clearing ghost/phantom keys? Also we assume
>>> that all gpios in the matrix should be able to generate interrupts so
>>> no polldev support required.
>>>
>>
>> Is there some info about such ghost/phantom keys? Otherwise I'll
>> assume these are something like sticky control keys?
>>
>> I'm not sure if there're any such support in console tools and in
>> input subsystem. Hard to say...
>>
>> And the comments below are of great value, I'll incorporate these
>> suggestions in later submission. Thanks very much.
>>
>
> Could you please have look at following gpio_matrix driver?
>
> gpio_matrix.c:
> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_matrix.c;h=c1f47651a4937d5c976a9625ca5da389dd7e4a7c;hb=HEAD
>
> gpio_input.c:
> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_input.c;h=7e307f267a2a059b64a3fb9c8a379b149016b2f8;hb=HEAD
>

Mmm... looks not so generic, though. And I still have no idea
about how its phantom key works unless I see its hw.

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Eric,

On Fri, May 8, 2009 at 6:59 AM, Eric Miao <eric.y.miao@gmail.com> wrote:
> On Thu, May 7, 2009 at 4:51 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>> Hi Eric,
>>
>> On Thu, May 7, 2009 at 2:18 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>> On Thu, May 7, 2009 at 4:41 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>>>> Hi Eric,
>>>>
>>>> On Thu, May 7, 2009 at 1:30 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>>>> Original patch by Marek Vasut, modified by Eric in:
>>>>>
>>>>> 1. use delayed work to simplify the debouncing
>>>>> 2. build keycode array for fast lookup
>>>>> 3. combine col_polarity/row_polarity into a single active_low field
>>>>> (are there some cases where the GPIOs are externally connected
>>>>> with an inverter and thus causing two different polarity requirement??)
>>>>> 4. use a generic bit array based XOR algorithm to detect key press/release,
>>>>> which should make the column assertion time shorter and code a bit
>>>>> cleaner
>>>>> 5. remove the ALT_FN handling, which is no way generic, the ALT_FN
>>>>> key should be treated as no different from other keys, and translation
>>>>> will be done by user space by commands like 'loadkeys'.
>>>>>
>>>>> Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
>>>>> controller, I have to configure those pins as generic GPIO to use this
>>>>> driver, works quite well, though ;-)
>>>>
>>>> Any support about removing/clearing ghost/phantom keys? Also we assume
>>>> that all gpios in the matrix should be able to generate interrupts so
>>>> no polldev support required.
>>>>
>>>
>>> Is there some info about such ghost/phantom keys? Otherwise I'll
>>> assume these are something like sticky control keys?
>>>
>>> I'm not sure if there're any such support in console tools and in
>>> input subsystem. Hard to say...
>>>
>>> And the comments below are of great value, I'll incorporate these
>>> suggestions in later submission. Thanks very much.
>>>
>>
>> Could you please have look at following gpio_matrix driver?
>>
>> gpio_matrix.c:
>> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_matrix.c;h=c1f47651a4937d5c976a9625ca5da389dd7e4a7c;hb=HEAD
>>
>> gpio_input.c:
>> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_input.c;h=7e307f267a2a059b64a3fb9c8a379b149016b2f8;hb=HEAD
>>
>
> Mmm... looks not so generic, though. And I still have no idea
> about how its phantom key works unless I see its hw.
>

Right, it could be based on what multi-keys people are interested in
and how it has been placed on the matrix during h/w design time. So,
it highly depends on the h/w design. We don't need to put this support
right now. Above links were just for information purpose only, we can
add such support in future.


-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Arve Hjønnevåg]

On Thu, May 7, 2009 at 6:29 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
> On Thu, May 7, 2009 at 4:51 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>> Hi Eric,
>>
>> On Thu, May 7, 2009 at 2:18 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>> On Thu, May 7, 2009 at 4:41 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>>>> Hi Eric,
>>>>
>>>> On Thu, May 7, 2009 at 1:30 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>>>> Original patch by Marek Vasut, modified by Eric in:
>>>>>
>>>>> 1. use delayed work to simplify the debouncing
>>>>> 2. build keycode array for fast lookup
>>>>> 3. combine col_polarity/row_polarity into a single active_low field
>>>>> (are there some cases where the GPIOs are externally connected
>>>>> with an inverter and thus causing two different polarity requirement??)
>>>>> 4. use a generic bit array based XOR algorithm to detect key press/release,
>>>>> which should make the column assertion time shorter and code a bit
>>>>> cleaner
>>>>> 5. remove the ALT_FN handling, which is no way generic, the ALT_FN
>>>>> key should be treated as no different from other keys, and translation
>>>>> will be done by user space by commands like 'loadkeys'.
>>>>>
>>>>> Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
>>>>> controller, I have to configure those pins as generic GPIO to use this
>>>>> driver, works quite well, though ;-)
>>>>
>>>> Any support about removing/clearing ghost/phantom keys? Also we assume
>>>> that all gpios in the matrix should be able to generate interrupts so
>>>> no polldev support required.
>>>>
>>>
>>> Is there some info about such ghost/phantom keys? Otherwise I'll
>>> assume these are something like sticky control keys?
>>>
>>> I'm not sure if there're any such support in console tools and in
>>> input subsystem. Hard to say...
>>>
>>> And the comments below are of great value, I'll incorporate these
>>> suggestions in later submission. Thanks very much.
>>>
>>
>> Could you please have look at following gpio_matrix driver?
>>
>> gpio_matrix.c:
>> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_matrix.c;h=c1f47651a4937d5c976a9625ca5da389dd7e4a7c;hb=HEAD
>>
>> gpio_input.c:
>> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_input.c;h=7e307f267a2a059b64a3fb9c8a379b149016b2f8;hb=HEAD
>>
>
> Mmm... looks not so generic, though. And I still have no idea
> about how its phantom key works unless I see its hw.
>

There should be no hardware specific code in those drivers, and you
are free to use them. If you remove the early suspend and wakelock
code and replace disable_irq with disable_irq_nosync it may cover your
needs. We used to have a separate gpio_matrix (and gpio_keys) like you
are proposing to add, but it proved to be too limited since the
hardware vendors like to mix direct gpio keys and and matrix keys in
the same logical keypad.

For the phantom key problem, this occurs with most matrix based
keypads/keyboards. For instance, you press two keys in the same row
then a third key in another row, but in the same column as one of the
first two keys. The driver cannot tell which of the two column you
pressed since the current will flow through the first two keys to or
from the other column. If you do not have any code to detect this you
will report four keys down.

-- 
Arve Hjønnevåg

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Eric/Arve,

2009/5/8 Arve Hjønnevåg <arve@android.com>:
> On Thu, May 7, 2009 at 6:29 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>> On Thu, May 7, 2009 at 4:51 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>>> Hi Eric,
>>>
>>> On Thu, May 7, 2009 at 2:18 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>>> On Thu, May 7, 2009 at 4:41 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>>>>> Hi Eric,
>>>>>
>>>>> On Thu, May 7, 2009 at 1:30 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>>>>> Original patch by Marek Vasut, modified by Eric in:
>>>>>>
>>>>>> 1. use delayed work to simplify the debouncing
>>>>>> 2. build keycode array for fast lookup
>>>>>> 3. combine col_polarity/row_polarity into a single active_low field
>>>>>> (are there some cases where the GPIOs are externally connected
>>>>>> with an inverter and thus causing two different polarity requirement??)
>>>>>> 4. use a generic bit array based XOR algorithm to detect key press/release,
>>>>>> which should make the column assertion time shorter and code a bit
>>>>>> cleaner
>>>>>> 5. remove the ALT_FN handling, which is no way generic, the ALT_FN
>>>>>> key should be treated as no different from other keys, and translation
>>>>>> will be done by user space by commands like 'loadkeys'.
>>>>>>
>>>>>> Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
>>>>>> controller, I have to configure those pins as generic GPIO to use this
>>>>>> driver, works quite well, though ;-)
>>>>>
>>>>> Any support about removing/clearing ghost/phantom keys? Also we assume
>>>>> that all gpios in the matrix should be able to generate interrupts so
>>>>> no polldev support required.
>>>>>
>>>>
>>>> Is there some info about such ghost/phantom keys? Otherwise I'll
>>>> assume these are something like sticky control keys?
>>>>
>>>> I'm not sure if there're any such support in console tools and in
>>>> input subsystem. Hard to say...
>>>>
>>>> And the comments below are of great value, I'll incorporate these
>>>> suggestions in later submission. Thanks very much.
>>>>
>>>
>>> Could you please have look at following gpio_matrix driver?
>>>
>>> gpio_matrix.c:
>>> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_matrix.c;h=c1f47651a4937d5c976a9625ca5da389dd7e4a7c;hb=HEAD
>>>
>>> gpio_input.c:
>>> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_input.c;h=7e307f267a2a059b64a3fb9c8a379b149016b2f8;hb=HEAD
>>>
>>
>> Mmm... looks not so generic, though. And I still have no idea
>> about how its phantom key works unless I see its hw.
>>
>
> There should be no hardware specific code in those drivers, and you
> are free to use them. If you remove the early suspend and wakelock
> code and replace disable_irq with disable_irq_nosync it may cover your
> needs. We used to have a separate gpio_matrix (and gpio_keys) like you
> are proposing to add, but it proved to be too limited since the
> hardware vendors like to mix direct gpio keys and and matrix keys in
> the same logical keypad.
>
> For the phantom key problem, this occurs with most matrix based
> keypads/keyboards. For instance, you press two keys in the same row
> then a third key in another row, but in the same column as one of the
> first two keys. The driver cannot tell which of the two column you
> pressed since the current will flow through the first two keys to or
> from the other column. If you do not have any code to detect this you
> will report four keys down.

I would suggest that we better move ahead with current gpio_matrix
driver submitted by Eric and let's get reviewed by Dmitry. Other
features can be added as an when some one needs them. Is that sounds
good?

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

2009/5/25 Trilok Soni <soni.trilok@gmail.com>:
> Hi Eric/Arve,
>
> 2009/5/8 Arve Hjønnevåg <arve@android.com>:
>> On Thu, May 7, 2009 at 6:29 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>> On Thu, May 7, 2009 at 4:51 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>>>> Hi Eric,
>>>>
>>>> On Thu, May 7, 2009 at 2:18 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>>>> On Thu, May 7, 2009 at 4:41 PM, Trilok Soni <soni.trilok@gmail.com> wrote:
>>>>>> Hi Eric,
>>>>>>
>>>>>> On Thu, May 7, 2009 at 1:30 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
>>>>>>> Original patch by Marek Vasut, modified by Eric in:
>>>>>>>
>>>>>>> 1. use delayed work to simplify the debouncing
>>>>>>> 2. build keycode array for fast lookup
>>>>>>> 3. combine col_polarity/row_polarity into a single active_low field
>>>>>>> (are there some cases where the GPIOs are externally connected
>>>>>>> with an inverter and thus causing two different polarity requirement??)
>>>>>>> 4. use a generic bit array based XOR algorithm to detect key press/release,
>>>>>>> which should make the column assertion time shorter and code a bit
>>>>>>> cleaner
>>>>>>> 5. remove the ALT_FN handling, which is no way generic, the ALT_FN
>>>>>>> key should be treated as no different from other keys, and translation
>>>>>>> will be done by user space by commands like 'loadkeys'.
>>>>>>>
>>>>>>> Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
>>>>>>> controller, I have to configure those pins as generic GPIO to use this
>>>>>>> driver, works quite well, though ;-)
>>>>>>
>>>>>> Any support about removing/clearing ghost/phantom keys? Also we assume
>>>>>> that all gpios in the matrix should be able to generate interrupts so
>>>>>> no polldev support required.
>>>>>>
>>>>>
>>>>> Is there some info about such ghost/phantom keys? Otherwise I'll
>>>>> assume these are something like sticky control keys?
>>>>>
>>>>> I'm not sure if there're any such support in console tools and in
>>>>> input subsystem. Hard to say...
>>>>>
>>>>> And the comments below are of great value, I'll incorporate these
>>>>> suggestions in later submission. Thanks very much.
>>>>>
>>>>
>>>> Could you please have look at following gpio_matrix driver?
>>>>
>>>> gpio_matrix.c:
>>>> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_matrix.c;h=c1f47651a4937d5c976a9625ca5da389dd7e4a7c;hb=HEAD
>>>>
>>>> gpio_input.c:
>>>> http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/input/misc/gpio_input.c;h=7e307f267a2a059b64a3fb9c8a379b149016b2f8;hb=HEAD
>>>>
>>>
>>> Mmm... looks not so generic, though. And I still have no idea
>>> about how its phantom key works unless I see its hw.
>>>
>>
>> There should be no hardware specific code in those drivers, and you
>> are free to use them. If you remove the early suspend and wakelock
>> code and replace disable_irq with disable_irq_nosync it may cover your
>> needs. We used to have a separate gpio_matrix (and gpio_keys) like you
>> are proposing to add, but it proved to be too limited since the
>> hardware vendors like to mix direct gpio keys and and matrix keys in
>> the same logical keypad.
>>
>> For the phantom key problem, this occurs with most matrix based
>> keypads/keyboards. For instance, you press two keys in the same row
>> then a third key in another row, but in the same column as one of the
>> first two keys. The driver cannot tell which of the two column you
>> pressed since the current will flow through the first two keys to or
>> from the other column. If you do not have any code to detect this you
>> will report four keys down.
>
> I would suggest that we better move ahead with current gpio_matrix
> driver submitted by Eric and let's get reviewed by Dmitry. Other
> features can be added as an when some one needs them. Is that sounds
> good?
>

Yes, that will be good.

Dmitry,

Could you have a quick look and get some feedback to us, so that
we can still have time to fix and get this into the next merge window
hopefully?
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Uli Luckas]

On Thursday, 7. May 2009, Eric Miao wrote:
>+static int matrix_keypad_suspend(struct platform_device *pdev,
>pm_message_t state)
>+{
>+       struct matrix_keypad *keypad = platform_get_drvdata(pdev);
>+       struct matrix_keypad_platform_data *pdata = keypad->pdata;
>+       int i;
>+
>+       if (device_may_wakeup(&pdev->dev))
>+               for (i = 0; i < pdata->num_row_gpios; i++)
>+                       enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
>+
>+       return 0;
>+}
>+
Hi Eric,
what about the workqueue and interrupts? Shouldn't the interrupts be disabled and then the wq flushed?

> +static int matrix_keypad_resume(struct platform_device *pdev)
> +{
> +	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
> +	struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +	int i;
> +
> +	if (device_may_wakeup(&pdev->dev))
> +		for (i = 0; i < pdata->num_row_gpios; i++)
> +			disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
> +
> +	return 0;
> +}
>+
You should probably schedule an imediate key scan as all key state changes where lost while the device was suspended.

Uli

-- 

------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Head of Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 62 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing director: Hans-Peter Constien

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

Hi Uli,

Sorry for the delay, and yes, the suspend/resume process is a little
bit out of consideration, I've made some modifications according to
your review, together with several other minor changes:

1. disable row GPIO IRQs and flush work queue in suspend and
schedule an immediate key scan out of resume (columns will
be activated and row IRQs be enabled after scan)

2. de-activating column GPIO turns the GPIO into input, thus a
HiZ state on most platforms, causing no side effect to the scan
of other columns. (Not turning them into input makes detection
of multiple keys failed on PXA/Sharp Zaurus).

3. minor corrections of the __devinit, __devexit_p() stuffs

Patch updated as follows:

>From e2f972a03845b5f1eb631eca481c06b620a88f27 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Thu, 7 May 2009 15:49:32 +0800
Subject: [PATCH] input: add support for generic GPIO-based matrix keypad

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Reviewed-by: Uli Luckas <u.luckas@road.de>
Signed-off-by: Eric Miao <eric.miao@marvell.com>
---
 drivers/input/keyboard/Kconfig         |   10 +
 drivers/input/keyboard/Makefile        |    1 +
 drivers/input/keyboard/matrix_keypad.c |  368 ++++++++++++++++++++++++++++++++
 include/linux/input/matrix_keypad.h    |   34 +++
 4 files changed, 413 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/matrix_keypad.c
 create mode 100644 include/linux/input/matrix_keypad.h

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index ea2638b..6b9f89c 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -332,4 +332,14 @@ config KEYBOARD_SH_KEYSC

 	  To compile this driver as a module, choose M here: the
 	  module will be called sh_keysc.
+
+config KEYBOARD_MATRIX
+	tristate "GPIO driven matrix keypad support"
+	depends on GENERIC_GPIO
+	help
+	  Enable support for GPIO driven matrix keypad
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called matrix_keypad.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 36351e1..1349408 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_KEYBOARD_HP7XX)		+= jornada720_kbd.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
 obj-$(CONFIG_KEYBOARD_SH_KEYSC)		+= sh_keysc.o
+obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
diff --git a/drivers/input/keyboard/matrix_keypad.c
b/drivers/input/keyboard/matrix_keypad.c
new file mode 100644
index 0000000..affad7e
--- /dev/null
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -0,0 +1,368 @@
+/*
+ * drivers/input/keyboard/matrix_keypad.c
+ *
+ *  GPIO driven matrix keyboard driver
+ *
+ *  Copyright (c) 2008 Marek Vasut <marek.vasut@gmail.com>
+ *
+ *  Based on corgikbd.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/input/matrix_keypad.h>
+
+struct matrix_keypad {
+	struct matrix_keypad_platform_data *pdata;
+	struct input_dev *input_dev;
+
+	uint32_t last_key_state[MATRIX_MAX_COLS];
+	uint32_t *keycodes;
+	struct delayed_work work;
+};
+
+static void __devinit build_keycodes(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	struct input_dev *input_dev = keypad->input_dev;
+	uint32_t *key;
+	int i;
+
+	keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
+
+	key = &pdata->key_map[0];
+	for (i = 0; i < pdata->key_map_size; i++, key++) {
+		keypad->keycodes[KEY_ROWCOL(*key)] = KEY_VAL(*key);
+		set_bit(KEY_VAL(*key), input_dev->keybit);
+	}
+}
+
+static unsigned int lookup_keycode(struct matrix_keypad *keypad,
+				   int row, int col)
+{
+	return keypad->keycodes[(row << 4) + col];
+}
+
+/* NOTE: normally the GPIO has to be put into HiZ when de-activated to cause
+ * minimum side effect when scanning other columns, here it is configured to
+ * be input, and it should work on most platforms.
+ */
+static void __activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	int level_on = !pdata->active_low;
+
+	if (on)
+		gpio_direction_output(pdata->col_gpios[col], level_on);
+	else {
+		gpio_set_value(pdata->col_gpios[col], !level_on);
+		gpio_direction_input(pdata->col_gpios[col]);
+	}
+}
+
+static void activate_all_cols(struct matrix_keypad_platform_data
*pdata, int on)
+{
+	int col;
+
+	for (col = 0; col < pdata->num_col_gpios; col++)
+		__activate_col(pdata, col, on);
+}
+
+static void activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	__activate_col(pdata, col, on);
+
+	if (on && pdata->col_scan_delay_us)
+		udelay(pdata->col_scan_delay_us);
+}
+
+static int row_asserted(struct matrix_keypad_platform_data *pdata, int row)
+{
+	return gpio_get_value(pdata->row_gpios[row]) ?
+			!pdata->active_low : pdata->active_low;
+}
+
+static void enable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		enable_irq(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+static void disable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+/*
+ * This gets the keys from keyboard and reports it to input subsystem
+ */
+static void matrix_keypad_scan(struct work_struct *work)
+{
+	struct matrix_keypad *keypad =
+	       	container_of(work, struct matrix_keypad, work.work);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	uint32_t new_state[MATRIX_MAX_COLS];
+	int row, col;
+
+	/* de-activate all columns for scanning */
+	activate_all_cols(pdata, 0);
+
+	memset(new_state, 0, sizeof(new_state));
+
+	/* assert each column and read the row status out */
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+
+		activate_col(pdata, col, 1);
+
+		for (row = 0; row < pdata->num_row_gpios; row++)
+			new_state[col] |= row_asserted(pdata, row) ?
+						(1 << row) : 0;
+		activate_col(pdata, col, 0);
+	}
+
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+		uint32_t bits_changed;
+		
+		bits_changed = keypad->last_key_state[col] ^ new_state[col];
+		if (bits_changed == 0)
+			continue;
+
+		for (row = 0; row < pdata->num_row_gpios; row++) {
+			if ((bits_changed & (1 << row)) == 0)
+				continue;
+
+			input_report_key(keypad->input_dev,
+					lookup_keycode(keypad, row, col),
+					new_state[col] & (1 << row));
+		}
+	}
+	input_sync(keypad->input_dev);
+	memcpy(keypad->last_key_state, new_state, sizeof(new_state));
+
+	activate_all_cols(pdata, 1);
+	enable_row_irqs(keypad);
+}
+
+static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
+{
+	struct matrix_keypad *keypad = id;
+
+	disable_row_irqs(keypad);
+	schedule_delayed_work(&keypad->work,
+			msecs_to_jiffies(keypad->pdata->debounce_ms));
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM
+static int matrix_keypad_suspend(struct platform_device *pdev,
pm_message_t state)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	disable_row_irqs(keypad);
+	flush_work(&keypad->work.work);
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	return 0;
+}
+
+static int matrix_keypad_resume(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	/* Uli Luckas: schedule an immediate key scan as all key state changes
+	 * were lost while the device was suspended, columns will be activated
+	 * and IRQs be enabled after the scan.
+	 */
+	schedule_delayed_work(&keypad->work, 0);
+	return 0;
+}
+#else
+#define matrix_keypad_suspend	NULL
+#define matrix_keypad_resume	NULL
+#endif
+
+static int __devinit init_matrix_gpio(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i, err = -EINVAL;
+
+	/* initialized strobe lines as outputs, activated */
+	for (i = 0; i < pdata->num_col_gpios; i++) {
+		err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
+		if (err) {
+			pr_err("failed to request GPIO%d for COL%d\n",
+					pdata->col_gpios[i], i);
+			goto err_free_cols;
+		}
+
+		gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
+		if (err) {
+			pr_err("failed to request GPIO%d for ROW%d\n",
+					pdata->row_gpios[i], i);
+			goto err_free_rows;
+		}
+
+		gpio_direction_input(pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
+				matrix_keypad_interrupt, IRQF_DISABLED |
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				"matrix-keypad", keypad);
+		if (err) {
+			pr_err("Unable to acquire interrupt for GPIO line %i\n",
+				pdata->row_gpios[i]);
+			goto err_free_irqs;
+		}
+	}
+	return 0;
+
+err_free_irqs:
+	for (i = i - 1; i >= 0; i--)
+		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+
+err_free_rows:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->row_gpios[i]);
+
+err_free_cols:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->col_gpios[i]);
+
+	return err;
+}
+
+static int __devinit matrix_keypad_probe(struct platform_device *pdev)
+{
+	struct matrix_keypad_platform_data *pdata;
+	struct matrix_keypad *keypad;
+	struct input_dev *input_dev;
+	int err = -ENOMEM;
+
+	if ((pdata = pdev->dev.platform_data) == NULL) {
+		dev_err(&pdev->dev, "no platform data defined\n");
+		return -EINVAL;
+	}
+
+	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
+	if (keypad == NULL)
+		return -ENOMEM;
+
+	input_dev = input_allocate_device();
+	if (!input_dev)
+		goto err_free_keypad;
+
+	platform_set_drvdata(pdev, keypad);
+
+	keypad->input_dev = input_dev;
+	keypad->pdata = pdata;
+	INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
+
+	input_dev->name		= pdev->name;
+	input_dev->id.bustype	= BUS_HOST;
+	input_dev->dev.parent	= &pdev->dev;
+	input_dev->evbit[0]	= BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+
+	build_keycodes(keypad);
+
+	err = input_register_device(keypad->input_dev);
+	if (err)
+		goto err_free_input;
+
+	err = init_matrix_gpio(keypad);
+	if (err)
+		goto err_unregister;
+
+	return 0;
+
+err_unregister:
+	input_unregister_device(input_dev);
+err_free_input:
+	input_free_device(input_dev);
+err_free_keypad:
+	kfree(keypad);
+	return err;
+}
+
+static int __devexit matrix_keypad_remove(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < keypad->pdata->num_row_gpios; i++) {
+		free_irq(gpio_to_irq(keypad->pdata->row_gpios[i]), keypad);
+		gpio_free(keypad->pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < keypad->pdata->num_col_gpios; i++)
+		gpio_free(keypad->pdata->col_gpios[i]);
+
+	input_unregister_device(keypad->input_dev);
+	kfree(keypad);
+	return 0;
+}
+
+static struct platform_driver matrix_keypad_driver = {
+	.probe		= matrix_keypad_probe,
+	.remove		= __devexit_p(matrix_keypad_remove),
+	.suspend	= matrix_keypad_suspend,
+	.resume		= matrix_keypad_resume,
+	.driver		= {
+		.name	= "matrix-keypad",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init matrix_keypad_init(void)
+{
+	return platform_driver_register(&matrix_keypad_driver);
+}
+
+static void __exit matrix_keypad_exit(void)
+{
+	platform_driver_unregister(&matrix_keypad_driver);
+}
+
+module_init(matrix_keypad_init);
+module_exit(matrix_keypad_exit);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
+MODULE_DESCRIPTION("GPIO Driven Matrix Keypad Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:matrix-keypad");
diff --git a/include/linux/input/matrix_keypad.h
b/include/linux/input/matrix_keypad.h
new file mode 100644
index 0000000..8b661cb
--- /dev/null
+++ b/include/linux/input/matrix_keypad.h
@@ -0,0 +1,34 @@
+#ifndef _MATRIX_KEYPAD_H
+#define _MATRIX_KEYPAD_H
+
+#include <linux/input.h>
+
+#define MATRIX_MAX_ROWS		16
+#define MATRIX_MAX_COLS		16
+#define MATRIX_MAX_KEYS		(MATRIX_MAX_ROWS * MATRIX_MAX_COLS)
+
+struct matrix_keypad_platform_data {
+	/* scancode map for the matrix keys */
+	uint32_t	*key_map;
+	int		key_map_size;
+
+	unsigned	row_gpios[MATRIX_MAX_ROWS];
+	unsigned	col_gpios[MATRIX_MAX_COLS];
+	int		num_row_gpios;
+	int		num_col_gpios;
+
+	unsigned int	active_low;
+	unsigned int	col_scan_delay_us;
+
+	/* key debounce interval in milli-second */
+	unsigned int	debounce_ms;
+};
+
+#define KEY(row, col, val)	((((row) & (MATRIX_MAX_ROWS - 1)) << 28) |\
+				 (((col) & (MATRIX_MAX_COLS - 1)) << 24) |\
+				 (val & 0xffffff))
+
+#define KEY_ROWCOL(k)		(((k) >> 24) & 0xff)
+#define KEY_VAL(k)		((k) & 0xffffff)
+
+#endif /* _MATRIX_KEYPAD_H */
-- 
1.6.0.4

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Russell King - ARM Linux]

On Sun, May 31, 2009 at 10:12:25PM +0800, Eric Miao wrote:
> +static void __devinit build_keycodes(struct matrix_keypad *keypad)
> +{
> +	struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +	struct input_dev *input_dev = keypad->input_dev;
> +	uint32_t *key;
> +	int i;
> +
> +	keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);

This doesn't check for failure...

> +static int __devinit matrix_keypad_probe(struct platform_device *pdev)
> +{
> +	struct matrix_keypad_platform_data *pdata;
> +	struct matrix_keypad *keypad;
> +	struct input_dev *input_dev;
> +	int err = -ENOMEM;
> +
> +	if ((pdata = pdev->dev.platform_data) == NULL) {
> +		dev_err(&pdev->dev, "no platform data defined\n");
> +		return -EINVAL;
> +	}
> +
> +	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
> +	if (keypad == NULL)
> +		return -ENOMEM;
> +
> +	input_dev = input_allocate_device();
> +	if (!input_dev)
> +		goto err_free_keypad;
> +
> +	platform_set_drvdata(pdev, keypad);
> +
> +	keypad->input_dev = input_dev;
> +	keypad->pdata = pdata;
> +	INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
> +
> +	input_dev->name		= pdev->name;
> +	input_dev->id.bustype	= BUS_HOST;
> +	input_dev->dev.parent	= &pdev->dev;
> +	input_dev->evbit[0]	= BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
> +
> +	build_keycodes(keypad);
> +
> +	err = input_register_device(keypad->input_dev);
> +	if (err)
> +		goto err_free_input;
> +
> +	err = init_matrix_gpio(keypad);
> +	if (err)
> +		goto err_unregister;
> +
> +	return 0;
> +
> +err_unregister:
> +	input_unregister_device(input_dev);

I don't see keypad->keycodes being kfree'd

> +err_free_input:
> +	input_free_device(input_dev);
> +err_free_keypad:
> +	kfree(keypad);
> +	return err;
> +}
> +
> +static int __devexit matrix_keypad_remove(struct platform_device *pdev)
> +{
> +	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
> +	int i;
> +
> +	for (i = 0; i < keypad->pdata->num_row_gpios; i++) {
> +		free_irq(gpio_to_irq(keypad->pdata->row_gpios[i]), keypad);
> +		gpio_free(keypad->pdata->row_gpios[i]);
> +	}
> +
> +	for (i = 0; i < keypad->pdata->num_col_gpios; i++)
> +		gpio_free(keypad->pdata->col_gpios[i]);
> +
> +	input_unregister_device(keypad->input_dev);

Nor here.

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

On Sun, May 31, 2009 at 10:20 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, May 31, 2009 at 10:12:25PM +0800, Eric Miao wrote:
>> +static void __devinit build_keycodes(struct matrix_keypad *keypad)
>> +{
>> +     struct matrix_keypad_platform_data *pdata = keypad->pdata;
>> +     struct input_dev *input_dev = keypad->input_dev;
>> +     uint32_t *key;
>> +     int i;
>> +
>> +     keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
>
> This doesn't check for failure...

Oops, updated (together with the release) as below:

From 5a742cd04aaa0f48519f52583fc6043458922a31 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Thu, 7 May 2009 15:49:32 +0800
Subject: [PATCH] input: add support for generic GPIO-based matrix keypad

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Reviewed-by: Uli Luckas <u.luckas@road.de>
Signed-off-by: Eric Miao <eric.miao@marvell.com>
---
 drivers/input/keyboard/Kconfig         |   10 +
 drivers/input/keyboard/Makefile        |    1 +
 drivers/input/keyboard/matrix_keypad.c |  375 ++++++++++++++++++++++++++++++++
 include/linux/input/matrix_keypad.h    |   34 +++
 4 files changed, 420 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/matrix_keypad.c
 create mode 100644 include/linux/input/matrix_keypad.h

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index ea2638b..6b9f89c 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -332,4 +332,14 @@ config KEYBOARD_SH_KEYSC

 	  To compile this driver as a module, choose M here: the
 	  module will be called sh_keysc.
+
+config KEYBOARD_MATRIX
+	tristate "GPIO driven matrix keypad support"
+	depends on GENERIC_GPIO
+	help
+	  Enable support for GPIO driven matrix keypad
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called matrix_keypad.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 36351e1..1349408 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_KEYBOARD_HP7XX)		+= jornada720_kbd.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
 obj-$(CONFIG_KEYBOARD_SH_KEYSC)		+= sh_keysc.o
+obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
diff --git a/drivers/input/keyboard/matrix_keypad.c
b/drivers/input/keyboard/matrix_keypad.c
new file mode 100644
index 0000000..61c7153
--- /dev/null
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -0,0 +1,375 @@
+/*
+ * drivers/input/keyboard/matrix_keypad.c
+ *
+ *  GPIO driven matrix keyboard driver
+ *
+ *  Copyright (c) 2008 Marek Vasut <marek.vasut@gmail.com>
+ *
+ *  Based on corgikbd.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/input/matrix_keypad.h>
+
+struct matrix_keypad {
+	struct matrix_keypad_platform_data *pdata;
+	struct input_dev *input_dev;
+
+	uint32_t last_key_state[MATRIX_MAX_COLS];
+	uint32_t *keycodes;
+	struct delayed_work work;
+};
+
+static int __devinit build_keycodes(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	struct input_dev *input_dev = keypad->input_dev;
+	uint32_t *key;
+	int i;
+
+	keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
+	if (keypad->keycodes == NULL)
+		return -ENOMEM;
+
+	key = &pdata->key_map[0];
+	for (i = 0; i < pdata->key_map_size; i++, key++) {
+		keypad->keycodes[KEY_ROWCOL(*key)] = KEY_VAL(*key);
+		set_bit(KEY_VAL(*key), input_dev->keybit);
+	}
+}
+
+static unsigned int lookup_keycode(struct matrix_keypad *keypad,
+				   int row, int col)
+{
+	return keypad->keycodes[(row << 4) + col];
+}
+
+/* NOTE: normally the GPIO has to be put into HiZ when de-activated to cause
+ * minimum side effect when scanning other columns, here it is configured to
+ * be input, and it should work on most platforms.
+ */
+static void __activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	int level_on = !pdata->active_low;
+
+	if (on)
+		gpio_direction_output(pdata->col_gpios[col], level_on);
+	else {
+		gpio_set_value(pdata->col_gpios[col], !level_on);
+		gpio_direction_input(pdata->col_gpios[col]);
+	}
+}
+
+static void activate_all_cols(struct matrix_keypad_platform_data
*pdata, int on)
+{
+	int col;
+
+	for (col = 0; col < pdata->num_col_gpios; col++)
+		__activate_col(pdata, col, on);
+}
+
+static void activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	__activate_col(pdata, col, on);
+
+	if (on && pdata->col_scan_delay_us)
+		udelay(pdata->col_scan_delay_us);
+}
+
+static int row_asserted(struct matrix_keypad_platform_data *pdata, int row)
+{
+	return gpio_get_value(pdata->row_gpios[row]) ?
+			!pdata->active_low : pdata->active_low;
+}
+
+static void enable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		enable_irq(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+static void disable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+/*
+ * This gets the keys from keyboard and reports it to input subsystem
+ */
+static void matrix_keypad_scan(struct work_struct *work)
+{
+	struct matrix_keypad *keypad =
+	       	container_of(work, struct matrix_keypad, work.work);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	uint32_t new_state[MATRIX_MAX_COLS];
+	int row, col;
+
+	/* de-activate all columns for scanning */
+	activate_all_cols(pdata, 0);
+
+	memset(new_state, 0, sizeof(new_state));
+
+	/* assert each column and read the row status out */
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+
+		activate_col(pdata, col, 1);
+
+		for (row = 0; row < pdata->num_row_gpios; row++)
+			new_state[col] |= row_asserted(pdata, row) ?
+						(1 << row) : 0;
+		activate_col(pdata, col, 0);
+	}
+
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+		uint32_t bits_changed;
+		
+		bits_changed = keypad->last_key_state[col] ^ new_state[col];
+		if (bits_changed == 0)
+			continue;
+
+		for (row = 0; row < pdata->num_row_gpios; row++) {
+			if ((bits_changed & (1 << row)) == 0)
+				continue;
+
+			input_report_key(keypad->input_dev,
+					lookup_keycode(keypad, row, col),
+					new_state[col] & (1 << row));
+		}
+	}
+	input_sync(keypad->input_dev);
+	memcpy(keypad->last_key_state, new_state, sizeof(new_state));
+
+	activate_all_cols(pdata, 1);
+	enable_row_irqs(keypad);
+}
+
+static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
+{
+	struct matrix_keypad *keypad = id;
+
+	disable_row_irqs(keypad);
+	schedule_delayed_work(&keypad->work,
+			msecs_to_jiffies(keypad->pdata->debounce_ms));
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM
+static int matrix_keypad_suspend(struct platform_device *pdev,
pm_message_t state)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	disable_row_irqs(keypad);
+	flush_work(&keypad->work.work);
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	return 0;
+}
+
+static int matrix_keypad_resume(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	/* Uli Luckas: schedule an immediate key scan as all key state changes
+	 * were lost while the device was suspended, columns will be activated
+	 * and IRQs be enabled after the scan.
+	 */
+	schedule_delayed_work(&keypad->work, 0);
+	return 0;
+}
+#else
+#define matrix_keypad_suspend	NULL
+#define matrix_keypad_resume	NULL
+#endif
+
+static int __devinit init_matrix_gpio(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i, err = -EINVAL;
+
+	/* initialized strobe lines as outputs, activated */
+	for (i = 0; i < pdata->num_col_gpios; i++) {
+		err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
+		if (err) {
+			pr_err("failed to request GPIO%d for COL%d\n",
+					pdata->col_gpios[i], i);
+			goto err_free_cols;
+		}
+
+		gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
+		if (err) {
+			pr_err("failed to request GPIO%d for ROW%d\n",
+					pdata->row_gpios[i], i);
+			goto err_free_rows;
+		}
+
+		gpio_direction_input(pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
+				matrix_keypad_interrupt, IRQF_DISABLED |
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				"matrix-keypad", keypad);
+		if (err) {
+			pr_err("Unable to acquire interrupt for GPIO line %i\n",
+				pdata->row_gpios[i]);
+			goto err_free_irqs;
+		}
+	}
+	return 0;
+
+err_free_irqs:
+	for (i = i - 1; i >= 0; i--)
+		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+
+err_free_rows:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->row_gpios[i]);
+
+err_free_cols:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->col_gpios[i]);
+
+	return err;
+}
+
+static int __devinit matrix_keypad_probe(struct platform_device *pdev)
+{
+	struct matrix_keypad_platform_data *pdata;
+	struct matrix_keypad *keypad;
+	struct input_dev *input_dev;
+	int err = -ENOMEM;
+
+	if ((pdata = pdev->dev.platform_data) == NULL) {
+		dev_err(&pdev->dev, "no platform data defined\n");
+		return -EINVAL;
+	}
+
+	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
+	if (keypad == NULL)
+		return -ENOMEM;
+
+	input_dev = input_allocate_device();
+	if (!input_dev)
+		goto err_free_keypad;
+
+	platform_set_drvdata(pdev, keypad);
+
+	keypad->input_dev = input_dev;
+	keypad->pdata = pdata;
+	INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
+
+	input_dev->name		= pdev->name;
+	input_dev->id.bustype	= BUS_HOST;
+	input_dev->dev.parent	= &pdev->dev;
+	input_dev->evbit[0]	= BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+
+	err = build_keycodes(keypad);
+	if (err)
+		goto err_free_input;
+
+	err = input_register_device(keypad->input_dev);
+	if (err)
+		goto err_free_keycodes;
+
+	err = init_matrix_gpio(keypad);
+	if (err)
+		goto err_unregister;
+
+	return 0;
+
+err_unregister:
+	input_unregister_device(input_dev);
+err_free_keycodes:
+	kfree(keypad->keycodes);
+err_free_input:
+	input_free_device(input_dev);
+err_free_keypad:
+	kfree(keypad);
+	return err;
+}
+
+static int __devexit matrix_keypad_remove(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < keypad->pdata->num_row_gpios; i++) {
+		free_irq(gpio_to_irq(keypad->pdata->row_gpios[i]), keypad);
+		gpio_free(keypad->pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < keypad->pdata->num_col_gpios; i++)
+		gpio_free(keypad->pdata->col_gpios[i]);
+
+	input_unregister_device(keypad->input_dev);
+	kfree(keypad->keycodes);
+	kfree(keypad);
+	return 0;
+}
+
+static struct platform_driver matrix_keypad_driver = {
+	.probe		= matrix_keypad_probe,
+	.remove		= __devexit_p(matrix_keypad_remove),
+	.suspend	= matrix_keypad_suspend,
+	.resume		= matrix_keypad_resume,
+	.driver		= {
+		.name	= "matrix-keypad",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init matrix_keypad_init(void)
+{
+	return platform_driver_register(&matrix_keypad_driver);
+}
+
+static void __exit matrix_keypad_exit(void)
+{
+	platform_driver_unregister(&matrix_keypad_driver);
+}
+
+module_init(matrix_keypad_init);
+module_exit(matrix_keypad_exit);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
+MODULE_DESCRIPTION("GPIO Driven Matrix Keypad Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:matrix-keypad");
diff --git a/include/linux/input/matrix_keypad.h
b/include/linux/input/matrix_keypad.h
new file mode 100644
index 0000000..8b661cb
--- /dev/null
+++ b/include/linux/input/matrix_keypad.h
@@ -0,0 +1,34 @@
+#ifndef _MATRIX_KEYPAD_H
+#define _MATRIX_KEYPAD_H
+
+#include <linux/input.h>
+
+#define MATRIX_MAX_ROWS		16
+#define MATRIX_MAX_COLS		16
+#define MATRIX_MAX_KEYS		(MATRIX_MAX_ROWS * MATRIX_MAX_COLS)
+
+struct matrix_keypad_platform_data {
+	/* scancode map for the matrix keys */
+	uint32_t	*key_map;
+	int		key_map_size;
+
+	unsigned	row_gpios[MATRIX_MAX_ROWS];
+	unsigned	col_gpios[MATRIX_MAX_COLS];
+	int		num_row_gpios;
+	int		num_col_gpios;
+
+	unsigned int	active_low;
+	unsigned int	col_scan_delay_us;
+
+	/* key debounce interval in milli-second */
+	unsigned int	debounce_ms;
+};
+
+#define KEY(row, col, val)	((((row) & (MATRIX_MAX_ROWS - 1)) << 28) |\
+				 (((col) & (MATRIX_MAX_COLS - 1)) << 24) |\
+				 (val & 0xffffff))
+
+#define KEY_ROWCOL(k)		(((k) >> 24) & 0xff)
+#define KEY_VAL(k)		((k) & 0xffffff)
+
+#endif /* _MATRIX_KEYPAD_H */
-- 
1.6.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Dmitry Torokhov]

On Sunday 31 May 2009 07:29:08 Eric Miao wrote:
> On Sun, May 31, 2009 at 10:20 PM, Russell King - ARM Linux
>
> <linux@arm.linux.org.uk> wrote:
> > On Sun, May 31, 2009 at 10:12:25PM +0800, Eric Miao wrote:
> >> +static void __devinit build_keycodes(struct matrix_keypad *keypad)
> >> +{
> >> +     struct matrix_keypad_platform_data *pdata = keypad->pdata;
> >> +     struct input_dev *input_dev = keypad->input_dev;
> >> +     uint32_t *key;
> >> +     int i;
> >> +
> >> +     keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int),
> >> GFP_KERNEL);
> >
> > This doesn't check for failure...
>
> Oops, updated (together with the release) as below:

Eric, could you please send me this patch as a diff to the original
one you sent?

Thanks a lot.

-- 
Dmitry

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Robert Jarzmik]

Eric Miao <eric.y.miao@gmail.com> writes:

Hi Eric, some minor cosmetic points ...

<snip>

> +static void activate_all_cols(struct matrix_keypad_platform_data
> *pdata, int on)
That function declaration spacing looks suspicious, looks like my mailer (or
yours) ate a bit out of it, and patch doesn't accept to apply it.

<snip>

+static void activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	__activate_col(pdata, col, on);
+
+	if (on && pdata->col_scan_delay_us)
+		udelay(pdata->col_scan_delay_us);
+}
That function is called is the worst case 16 times in a row (if I understood
correctly matrix_keypad_scan()). If delay is 100us, that makes 1.6ms for a
keystroke with all kernel blocked (udelay).
I didn't follow the discussion before, so maybe that's the only way. Still,
that's too bad ...

<snip>

> +/*
> + * This gets the keys from keyboard and reports it to input subsystem
> + */
> +static void matrix_keypad_scan(struct work_struct *work)
> +{
> +	struct matrix_keypad *keypad =
> +	       	container_of(work, struct matrix_keypad, work.work);
> +	struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +	uint32_t new_state[MATRIX_MAX_COLS];
> +	int row, col;
> +
> +	/* de-activate all columns for scanning */
> +	activate_all_cols(pdata, 0);
> +
> +	memset(new_state, 0, sizeof(new_state));
> +
> +	/* assert each column and read the row status out */
> +	for (col = 0; col < pdata->num_col_gpios; col++) {
> +
> +		activate_col(pdata, col, 1);
> +
> +		for (row = 0; row < pdata->num_row_gpios; row++)
> +			new_state[col] |= row_asserted(pdata, row) ?
> +						(1 << row) : 0;
> +		activate_col(pdata, col, 0);
> +	}
> +
> +	for (col = 0; col < pdata->num_col_gpios; col++) {
> +		uint32_t bits_changed;
> +		
Nitpicking: extra space on that line.

<snip>

Cheers.

--
Robert

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

On Mon, Jun 1, 2009 at 1:38 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Sunday 31 May 2009 07:29:08 Eric Miao wrote:
>> On Sun, May 31, 2009 at 10:20 PM, Russell King - ARM Linux
>>
>> <linux@arm.linux.org.uk> wrote:
>> > On Sun, May 31, 2009 at 10:12:25PM +0800, Eric Miao wrote:
>> >> +static void __devinit build_keycodes(struct matrix_keypad *keypad)
>> >> +{
>> >> +     struct matrix_keypad_platform_data *pdata = keypad->pdata;
>> >> +     struct input_dev *input_dev = keypad->input_dev;
>> >> +     uint32_t *key;
>> >> +     int i;
>> >> +
>> >> +     keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int),
>> >> GFP_KERNEL);
>> >
>> > This doesn't check for failure...
>>
>> Oops, updated (together with the release) as below:
>
> Eric, could you please send me this patch as a diff to the original
> one you sent?
>

Sorry, I used 'git commit --amend' for this and I'm not sure if I can
recover the diff. I'll paste the changed part here instead:

+	keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
+	if (keypad->keycodes == NULL)
+		return -ENOMEM;
+
+	key = &pdata->key_map[0];
+	for (i = 0; i < pdata->key_map_size; i++, key++) {
+		keypad->keycodes[KEY_ROWCOL(*key)] = KEY_VAL(*key);
+		set_bit(KEY_VAL(*key), input_dev->keybit);
+	}
+	return 0;

returns something in 'build_keycodes()'.

+	err = build_keycodes(keypad);
+	if (err)
+		goto err_free_input;
+
...
+
+err_unregister:
+	input_unregister_device(input_dev);
+err_free_keycodes:
+	kfree(keypad->keycodes);
+err_free_input:
+	input_free_device(input_dev);
+err_free_keypad:
+	kfree(keypad);
+	return err;

add one more exiting path when build_keycodes() failed

+
+	input_unregister_device(keypad->input_dev);
+	kfree(keypad->keycodes);
+	kfree(keypad);
+	return 0;

kfree(keypad->keycodes) in matrix_keypad_remove()
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

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

On Mon, Jun 1, 2009 at 1:39 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Eric Miao <eric.y.miao@gmail.com> writes:
>
> Hi Eric, some minor cosmetic points ...
>
> <snip>
>
>> +static void activate_all_cols(struct matrix_keypad_platform_data
>> *pdata, int on)
> That function declaration spacing looks suspicious, looks like my mailer (or
> yours) ate a bit out of it, and patch doesn't accept to apply it.
>
> <snip>
>
> +static void activate_col(struct matrix_keypad_platform_data *pdata,
> +                        int col, int on)
> +{
> +       __activate_col(pdata, col, on);
> +
> +       if (on && pdata->col_scan_delay_us)
> +               udelay(pdata->col_scan_delay_us);
> +}
> That function is called is the worst case 16 times in a row (if I understood
> correctly matrix_keypad_scan()). If delay is 100us, that makes 1.6ms for a
> keystroke with all kernel blocked (udelay).
> I didn't follow the discussion before, so maybe that's the only way. Still,
> that's too bad ...
>

This is AFAIK not necessary. This originally came from the
{corgi,spitz}kbd.c, where it is believed a small delay after
de-activating all columns, and a delay after activating one
specific column are needed (though the delay is suggested
in the {corgi,spitz}kbd.c as 10us). I'll check if it still works
without this delay (I believe it still works).


> <snip>
>
>> +/*
>> + * This gets the keys from keyboard and reports it to input subsystem
>> + */
>> +static void matrix_keypad_scan(struct work_struct *work)
>> +{
>> +     struct matrix_keypad *keypad =
>> +             container_of(work, struct matrix_keypad, work.work);
>> +     struct matrix_keypad_platform_data *pdata = keypad->pdata;
>> +     uint32_t new_state[MATRIX_MAX_COLS];
>> +     int row, col;
>> +
>> +     /* de-activate all columns for scanning */
>> +     activate_all_cols(pdata, 0);
>> +
>> +     memset(new_state, 0, sizeof(new_state));
>> +
>> +     /* assert each column and read the row status out */
>> +     for (col = 0; col < pdata->num_col_gpios; col++) {
>> +
>> +             activate_col(pdata, col, 1);
>> +
>> +             for (row = 0; row < pdata->num_row_gpios; row++)
>> +                     new_state[col] |= row_asserted(pdata, row) ?
>> +                                             (1 << row) : 0;
>> +             activate_col(pdata, col, 0);
>> +     }
>> +
>> +     for (col = 0; col < pdata->num_col_gpios; col++) {
>> +             uint32_t bits_changed;
>> +
> Nitpicking: extra space on that line.
>

OK.

I've attached the updated one as attachment here.

[-- Attachment #2: 0001-input-add-support-for-generic-GPIO-based-matrix-key.patch --]
[-- Type: text/x-patch, Size: 13064 bytes --]

From d903ce5d97de09901987b20332d37982942a5627 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Thu, 7 May 2009 15:49:32 +0800
Subject: [PATCH] input: add support for generic GPIO-based matrix keypad

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Reviewed-by: Uli Luckas <u.luckas@road.de>
Signed-off-by: Eric Miao <eric.miao@marvell.com>
---
 drivers/input/keyboard/Kconfig         |   10 +
 drivers/input/keyboard/Makefile        |    1 +
 drivers/input/keyboard/matrix_keypad.c |  378 ++++++++++++++++++++++++++++++++
 include/linux/input/matrix_keypad.h    |   34 +++
 4 files changed, 423 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/matrix_keypad.c
 create mode 100644 include/linux/input/matrix_keypad.h

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index ea2638b..6b9f89c 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -332,4 +332,14 @@ config KEYBOARD_SH_KEYSC
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called sh_keysc.
+
+config KEYBOARD_MATRIX
+	tristate "GPIO driven matrix keypad support"
+	depends on GENERIC_GPIO
+	help
+	  Enable support for GPIO driven matrix keypad
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called matrix_keypad.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 36351e1..1349408 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_KEYBOARD_HP7XX)		+= jornada720_kbd.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
 obj-$(CONFIG_KEYBOARD_SH_KEYSC)		+= sh_keysc.o
+obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
new file mode 100644
index 0000000..2debc8a
--- /dev/null
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -0,0 +1,378 @@
+/*
+ * drivers/input/keyboard/matrix_keypad.c
+ *
+ *  GPIO driven matrix keyboard driver
+ *
+ *  Copyright (c) 2008 Marek Vasut <marek.vasut@gmail.com>
+ *
+ *  Based on corgikbd.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/input/matrix_keypad.h>
+
+struct matrix_keypad {
+	struct matrix_keypad_platform_data *pdata;
+	struct input_dev *input_dev;
+
+	uint32_t last_key_state[MATRIX_MAX_COLS];
+	uint32_t *keycodes;
+	struct delayed_work work;
+};
+
+static int __devinit build_keycodes(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	struct input_dev *input_dev = keypad->input_dev;
+	uint32_t *key;
+	int i;
+
+	keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
+	if (keypad->keycodes == NULL)
+		return -ENOMEM;
+
+	key = &pdata->key_map[0];
+	for (i = 0; i < pdata->key_map_size; i++, key++) {
+		keypad->keycodes[KEY_ROWCOL(*key)] = KEY_VAL(*key);
+		set_bit(KEY_VAL(*key), input_dev->keybit);
+	}
+	return 0;
+}
+
+static unsigned int lookup_keycode(struct matrix_keypad *keypad,
+				   int row, int col)
+{
+	return keypad->keycodes[(row << 4) + col];
+}
+
+/* NOTE: normally the GPIO has to be put into HiZ when de-activated to cause
+ * minimum side effect when scanning other columns, here it is configured to
+ * be input, and it should work on most platforms.
+ */
+static void __activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	int level_on = !pdata->active_low;
+
+	if (on)
+		gpio_direction_output(pdata->col_gpios[col], level_on);
+	else {
+		gpio_set_value(pdata->col_gpios[col], !level_on);
+		gpio_direction_input(pdata->col_gpios[col]);
+	}
+}
+
+static void activate_all_cols(struct matrix_keypad_platform_data *pdata, int on)
+{
+	int col;
+
+	for (col = 0; col < pdata->num_col_gpios; col++)
+		__activate_col(pdata, col, on);
+}
+
+static void activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	__activate_col(pdata, col, on);
+
+	if (on && pdata->col_scan_delay_us)
+		udelay(pdata->col_scan_delay_us);
+}
+
+static int row_asserted(struct matrix_keypad_platform_data *pdata, int row)
+{
+	return gpio_get_value(pdata->row_gpios[row]) ?
+			!pdata->active_low : pdata->active_low;
+}
+
+static void enable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		enable_irq(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+static void disable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+/*
+ * This gets the keys from keyboard and reports it to input subsystem
+ */
+static void matrix_keypad_scan(struct work_struct *work)
+{
+	struct matrix_keypad *keypad =
+			container_of(work, struct matrix_keypad, work.work);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	uint32_t new_state[MATRIX_MAX_COLS];
+	int row, col;
+
+	/* de-activate all columns for scanning */
+	activate_all_cols(pdata, 0);
+
+	memset(new_state, 0, sizeof(new_state));
+
+	/* assert each column and read the row status out */
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+
+		activate_col(pdata, col, 1);
+
+		for (row = 0; row < pdata->num_row_gpios; row++)
+			new_state[col] |= row_asserted(pdata, row) ?
+						(1 << row) : 0;
+		activate_col(pdata, col, 0);
+	}
+
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+		uint32_t bits_changed;
+
+		bits_changed = keypad->last_key_state[col] ^ new_state[col];
+		if (bits_changed == 0)
+			continue;
+
+		for (row = 0; row < pdata->num_row_gpios; row++) {
+			if ((bits_changed & (1 << row)) == 0)
+				continue;
+
+			input_report_key(keypad->input_dev,
+					lookup_keycode(keypad, row, col),
+					new_state[col] & (1 << row));
+		}
+	}
+	input_sync(keypad->input_dev);
+	memcpy(keypad->last_key_state, new_state, sizeof(new_state));
+
+	activate_all_cols(pdata, 1);
+	enable_row_irqs(keypad);
+}
+
+static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
+{
+	struct matrix_keypad *keypad = id;
+
+	disable_row_irqs(keypad);
+	schedule_delayed_work(&keypad->work,
+			msecs_to_jiffies(keypad->pdata->debounce_ms));
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM
+static int matrix_keypad_suspend(struct platform_device *pdev,
+				 pm_message_t state)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	disable_row_irqs(keypad);
+	flush_work(&keypad->work.work);
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	return 0;
+}
+
+static int matrix_keypad_resume(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	/* Uli Luckas: schedule an immediate key scan as all key state changes
+	 * were lost while the device was suspended, columns will be activated
+	 * and IRQs be enabled after the scan.
+	 */
+	schedule_delayed_work(&keypad->work, 0);
+	return 0;
+}
+#else
+#define matrix_keypad_suspend	NULL
+#define matrix_keypad_resume	NULL
+#endif
+
+static int __devinit init_matrix_gpio(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i, err = -EINVAL;
+
+	/* initialized strobe lines as outputs, activated */
+	for (i = 0; i < pdata->num_col_gpios; i++) {
+		err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
+		if (err) {
+			pr_err("failed to request GPIO%d for COL%d\n",
+					pdata->col_gpios[i], i);
+			goto err_free_cols;
+		}
+
+		gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
+		if (err) {
+			pr_err("failed to request GPIO%d for ROW%d\n",
+					pdata->row_gpios[i], i);
+			goto err_free_rows;
+		}
+
+		gpio_direction_input(pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
+				matrix_keypad_interrupt, IRQF_DISABLED |
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				"matrix-keypad", keypad);
+		if (err) {
+			pr_err("Unable to acquire interrupt for GPIO line %i\n",
+				pdata->row_gpios[i]);
+			goto err_free_irqs;
+		}
+	}
+	return 0;
+
+err_free_irqs:
+	for (i = i - 1; i >= 0; i--)
+		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+
+err_free_rows:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->row_gpios[i]);
+
+err_free_cols:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->col_gpios[i]);
+
+	return err;
+}
+
+static int __devinit matrix_keypad_probe(struct platform_device *pdev)
+{
+	struct matrix_keypad_platform_data *pdata;
+	struct matrix_keypad *keypad;
+	struct input_dev *input_dev;
+	int err = -ENOMEM;
+
+	pdata = pdev->dev.platform_data;
+	if (pdata == NULL) {
+		dev_err(&pdev->dev, "no platform data defined\n");
+		return -EINVAL;
+	}
+
+	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
+	if (keypad == NULL)
+		return -ENOMEM;
+
+	input_dev = input_allocate_device();
+	if (!input_dev)
+		goto err_free_keypad;
+
+	platform_set_drvdata(pdev, keypad);
+
+	keypad->input_dev = input_dev;
+	keypad->pdata = pdata;
+	INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
+
+	input_dev->name		= pdev->name;
+	input_dev->id.bustype	= BUS_HOST;
+	input_dev->dev.parent	= &pdev->dev;
+	input_dev->evbit[0]	= BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+
+	err = build_keycodes(keypad);
+	if (err)
+		goto err_free_input;
+
+	err = input_register_device(keypad->input_dev);
+	if (err)
+		goto err_free_keycodes;
+
+	err = init_matrix_gpio(keypad);
+	if (err)
+		goto err_unregister;
+
+	return 0;
+
+err_unregister:
+	input_unregister_device(input_dev);
+err_free_keycodes:
+	kfree(keypad->keycodes);
+err_free_input:
+	input_free_device(input_dev);
+err_free_keypad:
+	kfree(keypad);
+	return err;
+}
+
+static int __devexit matrix_keypad_remove(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < keypad->pdata->num_row_gpios; i++) {
+		free_irq(gpio_to_irq(keypad->pdata->row_gpios[i]), keypad);
+		gpio_free(keypad->pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < keypad->pdata->num_col_gpios; i++)
+		gpio_free(keypad->pdata->col_gpios[i]);
+
+	input_unregister_device(keypad->input_dev);
+	kfree(keypad->keycodes);
+	kfree(keypad);
+	return 0;
+}
+
+static struct platform_driver matrix_keypad_driver = {
+	.probe		= matrix_keypad_probe,
+	.remove		= __devexit_p(matrix_keypad_remove),
+	.suspend	= matrix_keypad_suspend,
+	.resume		= matrix_keypad_resume,
+	.driver		= {
+		.name	= "matrix-keypad",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init matrix_keypad_init(void)
+{
+	return platform_driver_register(&matrix_keypad_driver);
+}
+
+static void __exit matrix_keypad_exit(void)
+{
+	platform_driver_unregister(&matrix_keypad_driver);
+}
+
+module_init(matrix_keypad_init);
+module_exit(matrix_keypad_exit);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
+MODULE_DESCRIPTION("GPIO Driven Matrix Keypad Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:matrix-keypad");
diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
new file mode 100644
index 0000000..8b661cb
--- /dev/null
+++ b/include/linux/input/matrix_keypad.h
@@ -0,0 +1,34 @@
+#ifndef _MATRIX_KEYPAD_H
+#define _MATRIX_KEYPAD_H
+
+#include <linux/input.h>
+
+#define MATRIX_MAX_ROWS		16
+#define MATRIX_MAX_COLS		16
+#define MATRIX_MAX_KEYS		(MATRIX_MAX_ROWS * MATRIX_MAX_COLS)
+
+struct matrix_keypad_platform_data {
+	/* scancode map for the matrix keys */
+	uint32_t	*key_map;
+	int		key_map_size;
+
+	unsigned	row_gpios[MATRIX_MAX_ROWS];
+	unsigned	col_gpios[MATRIX_MAX_COLS];
+	int		num_row_gpios;
+	int		num_col_gpios;
+
+	unsigned int	active_low;
+	unsigned int	col_scan_delay_us;
+
+	/* key debounce interval in milli-second */
+	unsigned int	debounce_ms;
+};
+
+#define KEY(row, col, val)	((((row) & (MATRIX_MAX_ROWS - 1)) << 28) |\
+				 (((col) & (MATRIX_MAX_COLS - 1)) << 24) |\
+				 (val & 0xffffff))
+
+#define KEY_ROWCOL(k)		(((k) >> 24) & 0xff)
+#define KEY_VAL(k)		((k) & 0xffffff)
+
+#endif /* _MATRIX_KEYPAD_H */
-- 
1.6.0.4

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Uli Luckas]

On Sunday, 31. May 2009, Eric Miao wrote:
> +#ifdef CONFIG_PM
> +static int matrix_keypad_suspend(struct platform_device *pdev,
> pm_message_t state)
> +{
> +       struct matrix_keypad *keypad = platform_get_drvdata(pdev);
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       int i;
> +
> +       disable_row_irqs(keypad);
> +       flush_work(&keypad->work.work);
> 
Hi Eric,
thanks for fixing the problems. One comment on the approach:
disable_row_irqs uses disable_irq_nosync. I guess this still works as
the interrupt handler is requested with IRQF_DISABLED.

Some more non-show stoppers:
1) On some SoCs wakeup gpios are a precoius resource. Therfore a hardware design which ORs all row gpios and connects the result to a single row_interrupt might be smart. Could 
the driver be made generic enought to support this?
2) Did anyone ever wonder why this driver is able to detect a second key press on the same row? Actually the driver has polling behavior while any key is pressed. This is 
because scanning the key matix triggers a new interrupt on every row. Every one of these interrupts will fire as soon as enable_irq is called, scheduling a new scan after the 
debounce period. I think that is worth being documented in the source code.

regards,
Uli

-- 

------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Head of Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 62 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing director: Hans-Peter Constien

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Uli Luckas]

One more...

As all gpio handling is done in a workqueue, 
gpio_set_value_cansleep/gpio_get_value_cansleep could probably be used. This 
would make the driver ready for gpios on external controllers (via i2c for 
example).

regards,
Uli

-- 

------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Head of Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 62 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing director: Hans-Peter Constien

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

On Tue, Jun 2, 2009 at 10:55 PM, Uli Luckas <u.luckas@road.de> wrote:
> On Sunday, 31. May 2009, Eric Miao wrote:
>> +#ifdef CONFIG_PM
>> +static int matrix_keypad_suspend(struct platform_device *pdev,
>> pm_message_t state)
>> +{
>> +       struct matrix_keypad *keypad = platform_get_drvdata(pdev);
>> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
>> +       int i;
>> +
>> +       disable_row_irqs(keypad);
>> +       flush_work(&keypad->work.work);
>>
> Hi Eric,
> thanks for fixing the problems. One comment on the approach:
> disable_row_irqs uses disable_irq_nosync. I guess this still works as
> the interrupt handler is requested with IRQF_DISABLED.
>
> Some more non-show stoppers:
> 1) On some SoCs wakeup gpios are a precoius resource. Therfore a hardware design which ORs all row gpios and connects the result to a single row_interrupt might be smart. Could
> the driver be made generic enought to support this?

I'd prefer this being left for further improvement when someone comes
with such a hardware and the patch.

> 2) Did anyone ever wonder why this driver is able to detect a second key press on the same row? Actually the driver has polling behavior while any key is pressed. This is
> because scanning the key matix triggers a new interrupt on every row. Every one of these interrupts will fire as soon as enable_irq is called, scheduling a new scan after the
> debounce period. I think that is worth being documented in the source code.
>

You see, I'm not a good writer, so ... And I think the reader
will be able to figure out. Well, just feel free to send me a
copy if you have a good description of this :)
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

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

On Tue, Jun 2, 2009 at 11:14 PM, Uli Luckas <u.luckas@road.de> wrote:
> One more...
>
> As all gpio handling is done in a workqueue,
> gpio_set_value_cansleep/gpio_get_value_cansleep could probably be used. This
> would make the driver ready for gpios on external controllers (via i2c for
> example).
>

Sounds good, patch updated again, with attachment.

>From 76776dfc468dc35f0d8394a2331e1a91f390e642 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Thu, 7 May 2009 15:49:32 +0800
Subject: [PATCH] input: add support for generic GPIO-based matrix keypad

Original patch by Marek Vasut, modified by Eric in:

1. use delayed work to simplify the debouncing process

2. build keycode array for fast lookup

3. combine col_polarity/row_polarity into a single active_low
   field (are there some cases where the GPIOs are externally
   connected with an inverter and thus causing two different
   polarity requirement??)

4. use a generic bit array based XOR algorithm to detect key
   press/release, which should make the column assertion time
   shorter and code a bit cleaner

5. remove the ALT_FN handling, which is no way generic, ALT_FN
   key should be treated as no different from other keys, and
   translation can be done by commands like 'loadkeys'.

6. explicitly disable row IRQs and flush potential pending work,
   and schedule an immediate scan after resuming as suggested
   by Uli Luckas

7. incorporate review comments from many others

Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
controller, I have to configure those pins as generic GPIO to use this
driver, works quite well, though ;-), and Sharp Zaurus model SL-C7x0
and SL-C1000.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Reviewed-by: Trilok Soni <soni.trilok@gmail.com>
Reviewed-by: Uli Luckas <u.luckas@road.de>
Reviewed-by: Russell King <linux@arm.linux.org.uk>
Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Eric Miao <eric.miao@marvell.com>
---
 drivers/input/keyboard/Kconfig         |   10 +
 drivers/input/keyboard/Makefile        |    1 +
 drivers/input/keyboard/matrix_keypad.c |  379 ++++++++++++++++++++++++++++++++
 include/linux/input/matrix_keypad.h    |   34 +++
 4 files changed, 424 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/matrix_keypad.c
 create mode 100644 include/linux/input/matrix_keypad.h

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index ea2638b..6b9f89c 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -332,4 +332,14 @@ config KEYBOARD_SH_KEYSC

 	  To compile this driver as a module, choose M here: the
 	  module will be called sh_keysc.
+
+config KEYBOARD_MATRIX
+	tristate "GPIO driven matrix keypad support"
+	depends on GENERIC_GPIO
+	help
+	  Enable support for GPIO driven matrix keypad
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called matrix_keypad.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 36351e1..1349408 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_KEYBOARD_HP7XX)		+= jornada720_kbd.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
 obj-$(CONFIG_KEYBOARD_SH_KEYSC)		+= sh_keysc.o
+obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
diff --git a/drivers/input/keyboard/matrix_keypad.c
b/drivers/input/keyboard/matrix_keypad.c
new file mode 100644
index 0000000..2078d64
--- /dev/null
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -0,0 +1,379 @@
+/*
+ * drivers/input/keyboard/matrix_keypad.c
+ *
+ *  GPIO driven matrix keyboard driver
+ *
+ *  Copyright (c) 2008 Marek Vasut <marek.vasut@gmail.com>
+ *
+ *  Based on corgikbd.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/input/matrix_keypad.h>
+
+struct matrix_keypad {
+	struct matrix_keypad_platform_data *pdata;
+	struct input_dev *input_dev;
+
+	uint32_t last_key_state[MATRIX_MAX_COLS];
+	uint32_t *keycodes;
+	struct delayed_work work;
+};
+
+static int __devinit build_keycodes(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	struct input_dev *input_dev = keypad->input_dev;
+	uint32_t *key;
+	int i;
+
+	keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
+	if (keypad->keycodes == NULL)
+		return -ENOMEM;
+
+	key = &pdata->key_map[0];
+	for (i = 0; i < pdata->key_map_size; i++, key++) {
+		keypad->keycodes[KEY_ROWCOL(*key)] = KEY_VAL(*key);
+		set_bit(KEY_VAL(*key), input_dev->keybit);
+	}
+	return 0;
+}
+
+static unsigned int lookup_keycode(struct matrix_keypad *keypad,
+				   int row, int col)
+{
+	return keypad->keycodes[(row << 4) + col];
+}
+
+/* NOTE: normally the GPIO has to be put into HiZ when de-activated to cause
+ * minimum side effect when scanning other columns, here it is configured to
+ * be input, and it should work on most platforms.
+ */
+static void __activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	int level_on = !pdata->active_low;
+
+	if (on)
+		gpio_direction_output(pdata->col_gpios[col], level_on);
+	else {
+		gpio_set_value_cansleep(pdata->col_gpios[col], !level_on);
+		gpio_direction_input(pdata->col_gpios[col]);
+	}
+}
+
+static void activate_all_cols(struct matrix_keypad_platform_data
*pdata, int on)
+{
+	int col;
+
+	for (col = 0; col < pdata->num_col_gpios; col++)
+		__activate_col(pdata, col, on);
+}
+
+static void activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	__activate_col(pdata, col, on);
+
+	if (on && pdata->col_scan_delay_us)
+		udelay(pdata->col_scan_delay_us);
+}
+
+static int row_asserted(struct matrix_keypad_platform_data *pdata, int row)
+{
+	return gpio_get_value_cansleep(pdata->row_gpios[row]) ?
+			!pdata->active_low : pdata->active_low;
+}
+
+static void enable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		enable_irq(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+static void disable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+/*
+ * This gets the keys from keyboard and reports it to input subsystem
+ */
+static void matrix_keypad_scan(struct work_struct *work)
+{
+	struct matrix_keypad *keypad =
+			container_of(work, struct matrix_keypad, work.work);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	uint32_t new_state[MATRIX_MAX_COLS];
+	int row, col;
+
+	/* de-activate all columns for scanning */
+	activate_all_cols(pdata, 0);
+
+	memset(new_state, 0, sizeof(new_state));
+
+	/* assert each column and read the row status out */
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+
+		activate_col(pdata, col, 1);
+
+		for (row = 0; row < pdata->num_row_gpios; row++)
+			new_state[col] |= row_asserted(pdata, row) ?
+						(1 << row) : 0;
+		activate_col(pdata, col, 0);
+	}
+
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+		uint32_t bits_changed;
+
+		bits_changed = keypad->last_key_state[col] ^ new_state[col];
+		if (bits_changed == 0)
+			continue;
+
+		for (row = 0; row < pdata->num_row_gpios; row++) {
+			if ((bits_changed & (1 << row)) == 0)
+				continue;
+
+			input_report_key(keypad->input_dev,
+					lookup_keycode(keypad, row, col),
+					new_state[col] & (1 << row));
+		}
+	}
+	input_sync(keypad->input_dev);
+	memcpy(keypad->last_key_state, new_state, sizeof(new_state));
+
+	activate_all_cols(pdata, 1);
+	enable_row_irqs(keypad);
+}
+
+static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
+{
+	struct matrix_keypad *keypad = id;
+
+	disable_row_irqs(keypad);
+	schedule_delayed_work(&keypad->work,
+			msecs_to_jiffies(keypad->pdata->debounce_ms));
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM
+static int matrix_keypad_suspend(struct platform_device *pdev,
+				 pm_message_t state)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	disable_row_irqs(keypad);
+	flush_work(&keypad->work.work);
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	return 0;
+}
+
+static int matrix_keypad_resume(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	/* Uli Luckas: schedule an immediate key scan as all key state changes
+	 * were lost while the device was suspended, columns will be activated
+	 * and IRQs be enabled after the scan.
+	 */
+	schedule_delayed_work(&keypad->work, 0);
+	return 0;
+}
+#else
+#define matrix_keypad_suspend	NULL
+#define matrix_keypad_resume	NULL
+#endif
+
+static int __devinit init_matrix_gpio(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i, err = -EINVAL;
+
+	/* initialized strobe lines as outputs, activated */
+	for (i = 0; i < pdata->num_col_gpios; i++) {
+		err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
+		if (err) {
+			pr_err("failed to request GPIO%d for COL%d\n",
+					pdata->col_gpios[i], i);
+			goto err_free_cols;
+		}
+
+		gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
+		if (err) {
+			pr_err("failed to request GPIO%d for ROW%d\n",
+					pdata->row_gpios[i], i);
+			goto err_free_rows;
+		}
+
+		gpio_direction_input(pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
+				matrix_keypad_interrupt, IRQF_DISABLED |
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				"matrix-keypad", keypad);
+		if (err) {
+			pr_err("Unable to acquire interrupt for GPIO line %i\n",
+				pdata->row_gpios[i]);
+			goto err_free_irqs;
+		}
+	}
+	return 0;
+
+err_free_irqs:
+	for (i = i - 1; i >= 0; i--)
+		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+
+err_free_rows:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->row_gpios[i]);
+
+err_free_cols:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->col_gpios[i]);
+
+	return err;
+}
+
+static int __devinit matrix_keypad_probe(struct platform_device *pdev)
+{
+	struct matrix_keypad_platform_data *pdata;
+	struct matrix_keypad *keypad;
+	struct input_dev *input_dev;
+	int err = -ENOMEM;
+
+	pdata = pdev->dev.platform_data;
+	if (pdata == NULL) {
+		dev_err(&pdev->dev, "no platform data defined\n");
+		return -EINVAL;
+	}
+
+	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
+	if (keypad == NULL)
+		return -ENOMEM;
+
+	input_dev = input_allocate_device();
+	if (!input_dev)
+		goto err_free_keypad;
+
+	platform_set_drvdata(pdev, keypad);
+
+	keypad->input_dev = input_dev;
+	keypad->pdata = pdata;
+	INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
+
+	input_dev->name		= pdev->name;
+	input_dev->id.bustype	= BUS_HOST;
+	input_dev->dev.parent	= &pdev->dev;
+	input_dev->evbit[0]	= BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+
+	err = build_keycodes(keypad);
+	if (err)
+		goto err_free_input;
+
+	err = input_register_device(keypad->input_dev);
+	if (err)
+		goto err_free_keycodes;
+
+	err = init_matrix_gpio(keypad);
+	if (err)
+		goto err_unregister;
+
+	device_init_wakeup(&pdev->dev, 1);
+	return 0;
+
+err_unregister:
+	input_unregister_device(input_dev);
+err_free_keycodes:
+	kfree(keypad->keycodes);
+err_free_input:
+	input_free_device(input_dev);
+err_free_keypad:
+	kfree(keypad);
+	return err;
+}
+
+static int __devexit matrix_keypad_remove(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < keypad->pdata->num_row_gpios; i++) {
+		free_irq(gpio_to_irq(keypad->pdata->row_gpios[i]), keypad);
+		gpio_free(keypad->pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < keypad->pdata->num_col_gpios; i++)
+		gpio_free(keypad->pdata->col_gpios[i]);
+
+	input_unregister_device(keypad->input_dev);
+	kfree(keypad->keycodes);
+	kfree(keypad);
+	return 0;
+}
+
+static struct platform_driver matrix_keypad_driver = {
+	.probe		= matrix_keypad_probe,
+	.remove		= __devexit_p(matrix_keypad_remove),
+	.suspend	= matrix_keypad_suspend,
+	.resume		= matrix_keypad_resume,
+	.driver		= {
+		.name	= "matrix-keypad",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init matrix_keypad_init(void)
+{
+	return platform_driver_register(&matrix_keypad_driver);
+}
+
+static void __exit matrix_keypad_exit(void)
+{
+	platform_driver_unregister(&matrix_keypad_driver);
+}
+
+module_init(matrix_keypad_init);
+module_exit(matrix_keypad_exit);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
+MODULE_DESCRIPTION("GPIO Driven Matrix Keypad Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:matrix-keypad");
diff --git a/include/linux/input/matrix_keypad.h
b/include/linux/input/matrix_keypad.h
new file mode 100644
index 0000000..8b661cb
--- /dev/null
+++ b/include/linux/input/matrix_keypad.h
@@ -0,0 +1,34 @@
+#ifndef _MATRIX_KEYPAD_H
+#define _MATRIX_KEYPAD_H
+
+#include <linux/input.h>
+
+#define MATRIX_MAX_ROWS		16
+#define MATRIX_MAX_COLS		16
+#define MATRIX_MAX_KEYS		(MATRIX_MAX_ROWS * MATRIX_MAX_COLS)
+
+struct matrix_keypad_platform_data {
+	/* scancode map for the matrix keys */
+	uint32_t	*key_map;
+	int		key_map_size;
+
+	unsigned	row_gpios[MATRIX_MAX_ROWS];
+	unsigned	col_gpios[MATRIX_MAX_COLS];
+	int		num_row_gpios;
+	int		num_col_gpios;
+
+	unsigned int	active_low;
+	unsigned int	col_scan_delay_us;
+
+	/* key debounce interval in milli-second */
+	unsigned int	debounce_ms;
+};
+
+#define KEY(row, col, val)	((((row) & (MATRIX_MAX_ROWS - 1)) << 28) |\
+				 (((col) & (MATRIX_MAX_COLS - 1)) << 24) |\
+				 (val & 0xffffff))
+
+#define KEY_ROWCOL(k)		(((k) >> 24) & 0xff)
+#define KEY_VAL(k)		((k) & 0xffffff)
+
+#endif /* _MATRIX_KEYPAD_H */
-- 
1.6.0.4

[-- Attachment #2: 0001-input-add-support-for-generic-GPIO-based-matrix-key.patch --]
[-- Type: text/x-diff, Size: 14393 bytes --]

From 76776dfc468dc35f0d8394a2331e1a91f390e642 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Thu, 7 May 2009 15:49:32 +0800
Subject: [PATCH] input: add support for generic GPIO-based matrix keypad

Original patch by Marek Vasut, modified by Eric in:

1. use delayed work to simplify the debouncing process

2. build keycode array for fast lookup

3. combine col_polarity/row_polarity into a single active_low
   field (are there some cases where the GPIOs are externally
   connected with an inverter and thus causing two different
   polarity requirement??)

4. use a generic bit array based XOR algorithm to detect key
   press/release, which should make the column assertion time
   shorter and code a bit cleaner

5. remove the ALT_FN handling, which is no way generic, ALT_FN
   key should be treated as no different from other keys, and
   translation can be done by commands like 'loadkeys'.

6. explicitly disable row IRQs and flush potential pending work,
   and schedule an immediate scan after resuming as suggested
   by Uli Luckas

7. incorporate review comments from many others

Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
controller, I have to configure those pins as generic GPIO to use this
driver, works quite well, though ;-), and Sharp Zaurus model SL-C7x0
and SL-C1000.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Reviewed-by: Trilok Soni <soni.trilok@gmail.com>
Reviewed-by: Uli Luckas <u.luckas@road.de>
Reviewed-by: Russell King <linux@arm.linux.org.uk>
Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Eric Miao <eric.miao@marvell.com>
---
 drivers/input/keyboard/Kconfig         |   10 +
 drivers/input/keyboard/Makefile        |    1 +
 drivers/input/keyboard/matrix_keypad.c |  379 ++++++++++++++++++++++++++++++++
 include/linux/input/matrix_keypad.h    |   34 +++
 4 files changed, 424 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/keyboard/matrix_keypad.c
 create mode 100644 include/linux/input/matrix_keypad.h

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index ea2638b..6b9f89c 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -332,4 +332,14 @@ config KEYBOARD_SH_KEYSC
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called sh_keysc.
+
+config KEYBOARD_MATRIX
+	tristate "GPIO driven matrix keypad support"
+	depends on GENERIC_GPIO
+	help
+	  Enable support for GPIO driven matrix keypad
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called matrix_keypad.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 36351e1..1349408 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_KEYBOARD_HP7XX)		+= jornada720_kbd.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
 obj-$(CONFIG_KEYBOARD_SH_KEYSC)		+= sh_keysc.o
+obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
new file mode 100644
index 0000000..2078d64
--- /dev/null
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -0,0 +1,379 @@
+/*
+ * drivers/input/keyboard/matrix_keypad.c
+ *
+ *  GPIO driven matrix keyboard driver
+ *
+ *  Copyright (c) 2008 Marek Vasut <marek.vasut@gmail.com>
+ *
+ *  Based on corgikbd.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/input/matrix_keypad.h>
+
+struct matrix_keypad {
+	struct matrix_keypad_platform_data *pdata;
+	struct input_dev *input_dev;
+
+	uint32_t last_key_state[MATRIX_MAX_COLS];
+	uint32_t *keycodes;
+	struct delayed_work work;
+};
+
+static int __devinit build_keycodes(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	struct input_dev *input_dev = keypad->input_dev;
+	uint32_t *key;
+	int i;
+
+	keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
+	if (keypad->keycodes == NULL)
+		return -ENOMEM;
+
+	key = &pdata->key_map[0];
+	for (i = 0; i < pdata->key_map_size; i++, key++) {
+		keypad->keycodes[KEY_ROWCOL(*key)] = KEY_VAL(*key);
+		set_bit(KEY_VAL(*key), input_dev->keybit);
+	}
+	return 0;
+}
+
+static unsigned int lookup_keycode(struct matrix_keypad *keypad,
+				   int row, int col)
+{
+	return keypad->keycodes[(row << 4) + col];
+}
+
+/* NOTE: normally the GPIO has to be put into HiZ when de-activated to cause
+ * minimum side effect when scanning other columns, here it is configured to
+ * be input, and it should work on most platforms.
+ */
+static void __activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	int level_on = !pdata->active_low;
+
+	if (on)
+		gpio_direction_output(pdata->col_gpios[col], level_on);
+	else {
+		gpio_set_value_cansleep(pdata->col_gpios[col], !level_on);
+		gpio_direction_input(pdata->col_gpios[col]);
+	}
+}
+
+static void activate_all_cols(struct matrix_keypad_platform_data *pdata, int on)
+{
+	int col;
+
+	for (col = 0; col < pdata->num_col_gpios; col++)
+		__activate_col(pdata, col, on);
+}
+
+static void activate_col(struct matrix_keypad_platform_data *pdata,
+			 int col, int on)
+{
+	__activate_col(pdata, col, on);
+
+	if (on && pdata->col_scan_delay_us)
+		udelay(pdata->col_scan_delay_us);
+}
+
+static int row_asserted(struct matrix_keypad_platform_data *pdata, int row)
+{
+	return gpio_get_value_cansleep(pdata->row_gpios[row]) ?
+			!pdata->active_low : pdata->active_low;
+}
+
+static void enable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		enable_irq(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+static void disable_row_irqs(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+/*
+ * This gets the keys from keyboard and reports it to input subsystem
+ */
+static void matrix_keypad_scan(struct work_struct *work)
+{
+	struct matrix_keypad *keypad =
+			container_of(work, struct matrix_keypad, work.work);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	uint32_t new_state[MATRIX_MAX_COLS];
+	int row, col;
+
+	/* de-activate all columns for scanning */
+	activate_all_cols(pdata, 0);
+
+	memset(new_state, 0, sizeof(new_state));
+
+	/* assert each column and read the row status out */
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+
+		activate_col(pdata, col, 1);
+
+		for (row = 0; row < pdata->num_row_gpios; row++)
+			new_state[col] |= row_asserted(pdata, row) ?
+						(1 << row) : 0;
+		activate_col(pdata, col, 0);
+	}
+
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+		uint32_t bits_changed;
+
+		bits_changed = keypad->last_key_state[col] ^ new_state[col];
+		if (bits_changed == 0)
+			continue;
+
+		for (row = 0; row < pdata->num_row_gpios; row++) {
+			if ((bits_changed & (1 << row)) == 0)
+				continue;
+
+			input_report_key(keypad->input_dev,
+					lookup_keycode(keypad, row, col),
+					new_state[col] & (1 << row));
+		}
+	}
+	input_sync(keypad->input_dev);
+	memcpy(keypad->last_key_state, new_state, sizeof(new_state));
+
+	activate_all_cols(pdata, 1);
+	enable_row_irqs(keypad);
+}
+
+static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
+{
+	struct matrix_keypad *keypad = id;
+
+	disable_row_irqs(keypad);
+	schedule_delayed_work(&keypad->work,
+			msecs_to_jiffies(keypad->pdata->debounce_ms));
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM
+static int matrix_keypad_suspend(struct platform_device *pdev,
+				 pm_message_t state)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	disable_row_irqs(keypad);
+	flush_work(&keypad->work.work);
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	return 0;
+}
+
+static int matrix_keypad_resume(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	/* Uli Luckas: schedule an immediate key scan as all key state changes
+	 * were lost while the device was suspended, columns will be activated
+	 * and IRQs be enabled after the scan.
+	 */
+	schedule_delayed_work(&keypad->work, 0);
+	return 0;
+}
+#else
+#define matrix_keypad_suspend	NULL
+#define matrix_keypad_resume	NULL
+#endif
+
+static int __devinit init_matrix_gpio(struct matrix_keypad *keypad)
+{
+	struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i, err = -EINVAL;
+
+	/* initialized strobe lines as outputs, activated */
+	for (i = 0; i < pdata->num_col_gpios; i++) {
+		err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
+		if (err) {
+			pr_err("failed to request GPIO%d for COL%d\n",
+					pdata->col_gpios[i], i);
+			goto err_free_cols;
+		}
+
+		gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
+		if (err) {
+			pr_err("failed to request GPIO%d for ROW%d\n",
+					pdata->row_gpios[i], i);
+			goto err_free_rows;
+		}
+
+		gpio_direction_input(pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
+				matrix_keypad_interrupt, IRQF_DISABLED |
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				"matrix-keypad", keypad);
+		if (err) {
+			pr_err("Unable to acquire interrupt for GPIO line %i\n",
+				pdata->row_gpios[i]);
+			goto err_free_irqs;
+		}
+	}
+	return 0;
+
+err_free_irqs:
+	for (i = i - 1; i >= 0; i--)
+		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+
+err_free_rows:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->row_gpios[i]);
+
+err_free_cols:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->col_gpios[i]);
+
+	return err;
+}
+
+static int __devinit matrix_keypad_probe(struct platform_device *pdev)
+{
+	struct matrix_keypad_platform_data *pdata;
+	struct matrix_keypad *keypad;
+	struct input_dev *input_dev;
+	int err = -ENOMEM;
+
+	pdata = pdev->dev.platform_data;
+	if (pdata == NULL) {
+		dev_err(&pdev->dev, "no platform data defined\n");
+		return -EINVAL;
+	}
+
+	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
+	if (keypad == NULL)
+		return -ENOMEM;
+
+	input_dev = input_allocate_device();
+	if (!input_dev)
+		goto err_free_keypad;
+
+	platform_set_drvdata(pdev, keypad);
+
+	keypad->input_dev = input_dev;
+	keypad->pdata = pdata;
+	INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
+
+	input_dev->name		= pdev->name;
+	input_dev->id.bustype	= BUS_HOST;
+	input_dev->dev.parent	= &pdev->dev;
+	input_dev->evbit[0]	= BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+
+	err = build_keycodes(keypad);
+	if (err)
+		goto err_free_input;
+
+	err = input_register_device(keypad->input_dev);
+	if (err)
+		goto err_free_keycodes;
+
+	err = init_matrix_gpio(keypad);
+	if (err)
+		goto err_unregister;
+
+	device_init_wakeup(&pdev->dev, 1);
+	return 0;
+
+err_unregister:
+	input_unregister_device(input_dev);
+err_free_keycodes:
+	kfree(keypad->keycodes);
+err_free_input:
+	input_free_device(input_dev);
+err_free_keypad:
+	kfree(keypad);
+	return err;
+}
+
+static int __devexit matrix_keypad_remove(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < keypad->pdata->num_row_gpios; i++) {
+		free_irq(gpio_to_irq(keypad->pdata->row_gpios[i]), keypad);
+		gpio_free(keypad->pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < keypad->pdata->num_col_gpios; i++)
+		gpio_free(keypad->pdata->col_gpios[i]);
+
+	input_unregister_device(keypad->input_dev);
+	kfree(keypad->keycodes);
+	kfree(keypad);
+	return 0;
+}
+
+static struct platform_driver matrix_keypad_driver = {
+	.probe		= matrix_keypad_probe,
+	.remove		= __devexit_p(matrix_keypad_remove),
+	.suspend	= matrix_keypad_suspend,
+	.resume		= matrix_keypad_resume,
+	.driver		= {
+		.name	= "matrix-keypad",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init matrix_keypad_init(void)
+{
+	return platform_driver_register(&matrix_keypad_driver);
+}
+
+static void __exit matrix_keypad_exit(void)
+{
+	platform_driver_unregister(&matrix_keypad_driver);
+}
+
+module_init(matrix_keypad_init);
+module_exit(matrix_keypad_exit);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
+MODULE_DESCRIPTION("GPIO Driven Matrix Keypad Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:matrix-keypad");
diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
new file mode 100644
index 0000000..8b661cb
--- /dev/null
+++ b/include/linux/input/matrix_keypad.h
@@ -0,0 +1,34 @@
+#ifndef _MATRIX_KEYPAD_H
+#define _MATRIX_KEYPAD_H
+
+#include <linux/input.h>
+
+#define MATRIX_MAX_ROWS		16
+#define MATRIX_MAX_COLS		16
+#define MATRIX_MAX_KEYS		(MATRIX_MAX_ROWS * MATRIX_MAX_COLS)
+
+struct matrix_keypad_platform_data {
+	/* scancode map for the matrix keys */
+	uint32_t	*key_map;
+	int		key_map_size;
+
+	unsigned	row_gpios[MATRIX_MAX_ROWS];
+	unsigned	col_gpios[MATRIX_MAX_COLS];
+	int		num_row_gpios;
+	int		num_col_gpios;
+
+	unsigned int	active_low;
+	unsigned int	col_scan_delay_us;
+
+	/* key debounce interval in milli-second */
+	unsigned int	debounce_ms;
+};
+
+#define KEY(row, col, val)	((((row) & (MATRIX_MAX_ROWS - 1)) << 28) |\
+				 (((col) & (MATRIX_MAX_COLS - 1)) << 24) |\
+				 (val & 0xffffff))
+
+#define KEY_ROWCOL(k)		(((k) >> 24) & 0xff)
+#define KEY_VAL(k)		((k) & 0xffffff)
+
+#endif /* _MATRIX_KEYPAD_H */
-- 
1.6.0.4

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Uli Luckas,

On Tue, Jun 2, 2009 at 8:25 PM, Uli Luckas <u.luckas@road.de> wrote:
> On Sunday, 31. May 2009, Eric Miao wrote:
>> +#ifdef CONFIG_PM
>> +static int matrix_keypad_suspend(struct platform_device *pdev,
>> pm_message_t state)
>> +{
>> +       struct matrix_keypad *keypad = platform_get_drvdata(pdev);
>> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
>> +       int i;
>> +
>> +       disable_row_irqs(keypad);
>> +       flush_work(&keypad->work.work);
>>
> Hi Eric,
> thanks for fixing the problems. One comment on the approach:
> disable_row_irqs uses disable_irq_nosync. I guess this still works as
> the interrupt handler is requested with IRQF_DISABLED.
>
> Some more non-show stoppers:
> 1) On some SoCs wakeup gpios are a precoius resource. Therfore a hardware design which ORs all row gpios and connects the result to a single row_interrupt might be smart. Could
> the driver be made generic enought to support this?
> 2) Did anyone ever wonder why this driver is able to detect a second key press on the same row? Actually the driver has polling behavior while any key is pressed. This is
> because scanning the key matix triggers a new interrupt on every row. Every one of these interrupts will fire as soon as enable_irq is called, scheduling a new scan after the
> debounce period. I think that is worth being documented in the source code.


Following is on radar after this driver gets picked up for the
mainline merge window (should open soon as 2.6.30-rc8 already out) by
Dmitry.

1. passing settling time from platform data as mentioned by you.
Android gpio matrix driver I have pointed out earlier is already doing
this.
2. adding phantom key removal support - probably picking it up
straight from android driver.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Dmitry,

>
> Sounds good, patch updated again, with attachment.
>
> >From 76776dfc468dc35f0d8394a2331e1a91f390e642 Mon Sep 17 00:00:00 2001
> From: Marek Vasut <marek.vasut@gmail.com>
> Date: Thu, 7 May 2009 15:49:32 +0800
> Subject: [PATCH] input: add support for generic GPIO-based matrix keypad
>

Could you please review this patch and see if it can be acked as merge
window will open in couple of days? Thanks.


> Original patch by Marek Vasut, modified by Eric in:
>
> 1. use delayed work to simplify the debouncing process
>
> 2. build keycode array for fast lookup
>
> 3. combine col_polarity/row_polarity into a single active_low
>   field (are there some cases where the GPIOs are externally
>   connected with an inverter and thus causing two different
>   polarity requirement??)
>
> 4. use a generic bit array based XOR algorithm to detect key
>   press/release, which should make the column assertion time
>   shorter and code a bit cleaner
>
> 5. remove the ALT_FN handling, which is no way generic, ALT_FN
>   key should be treated as no different from other keys, and
>   translation can be done by commands like 'loadkeys'.
>
> 6. explicitly disable row IRQs and flush potential pending work,
>   and schedule an immediate scan after resuming as suggested
>   by Uli Luckas
>
> 7. incorporate review comments from many others
>
> Patch tested on Littleton/PXA310 (though PXA310 has a dedicate keypad
> controller, I have to configure those pins as generic GPIO to use this
> driver, works quite well, though ;-), and Sharp Zaurus model SL-C7x0
> and SL-C1000.
>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Reviewed-by: Trilok Soni <soni.trilok@gmail.com>
> Reviewed-by: Uli Luckas <u.luckas@road.de>
> Reviewed-by: Russell King <linux@arm.linux.org.uk>
> Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>
> Signed-off-by: Eric Miao <eric.miao@marvell.com>
> ---
>  drivers/input/keyboard/Kconfig         |   10 +
>  drivers/input/keyboard/Makefile        |    1 +
>  drivers/input/keyboard/matrix_keypad.c |  379 ++++++++++++++++++++++++++++++++
>  include/linux/input/matrix_keypad.h    |   34 +++
>  4 files changed, 424 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/keyboard/matrix_keypad.c
>  create mode 100644 include/linux/input/matrix_keypad.h
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index ea2638b..6b9f89c 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -332,4 +332,14 @@ config KEYBOARD_SH_KEYSC
>
>          To compile this driver as a module, choose M here: the
>          module will be called sh_keysc.
> +
> +config KEYBOARD_MATRIX
> +       tristate "GPIO driven matrix keypad support"
> +       depends on GENERIC_GPIO
> +       help
> +         Enable support for GPIO driven matrix keypad
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called matrix_keypad.
> +
>  endif
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 36351e1..1349408 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -28,3 +28,4 @@ obj-$(CONFIG_KEYBOARD_HP7XX)          += jornada720_kbd.o
>  obj-$(CONFIG_KEYBOARD_MAPLE)           += maple_keyb.o
>  obj-$(CONFIG_KEYBOARD_BFIN)            += bf54x-keys.o
>  obj-$(CONFIG_KEYBOARD_SH_KEYSC)                += sh_keysc.o
> +obj-$(CONFIG_KEYBOARD_MATRIX)          += matrix_keypad.o
> diff --git a/drivers/input/keyboard/matrix_keypad.c
> b/drivers/input/keyboard/matrix_keypad.c
> new file mode 100644
> index 0000000..2078d64
> --- /dev/null
> +++ b/drivers/input/keyboard/matrix_keypad.c
> @@ -0,0 +1,379 @@
> +/*
> + * drivers/input/keyboard/matrix_keypad.c
> + *
> + *  GPIO driven matrix keyboard driver
> + *
> + *  Copyright (c) 2008 Marek Vasut <marek.vasut@gmail.com>
> + *
> + *  Based on corgikbd.c
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/gpio.h>
> +#include <linux/input/matrix_keypad.h>
> +
> +struct matrix_keypad {
> +       struct matrix_keypad_platform_data *pdata;
> +       struct input_dev *input_dev;
> +
> +       uint32_t last_key_state[MATRIX_MAX_COLS];
> +       uint32_t *keycodes;
> +       struct delayed_work work;
> +};
> +
> +static int __devinit build_keycodes(struct matrix_keypad *keypad)
> +{
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       struct input_dev *input_dev = keypad->input_dev;
> +       uint32_t *key;
> +       int i;
> +
> +       keypad->keycodes = kzalloc(MATRIX_MAX_KEYS * sizeof(int), GFP_KERNEL);
> +       if (keypad->keycodes == NULL)
> +               return -ENOMEM;
> +
> +       key = &pdata->key_map[0];
> +       for (i = 0; i < pdata->key_map_size; i++, key++) {
> +               keypad->keycodes[KEY_ROWCOL(*key)] = KEY_VAL(*key);
> +               set_bit(KEY_VAL(*key), input_dev->keybit);
> +       }
> +       return 0;
> +}
> +
> +static unsigned int lookup_keycode(struct matrix_keypad *keypad,
> +                                  int row, int col)
> +{
> +       return keypad->keycodes[(row << 4) + col];
> +}
> +
> +/* NOTE: normally the GPIO has to be put into HiZ when de-activated to cause
> + * minimum side effect when scanning other columns, here it is configured to
> + * be input, and it should work on most platforms.
> + */
> +static void __activate_col(struct matrix_keypad_platform_data *pdata,
> +                        int col, int on)
> +{
> +       int level_on = !pdata->active_low;
> +
> +       if (on)
> +               gpio_direction_output(pdata->col_gpios[col], level_on);
> +       else {
> +               gpio_set_value_cansleep(pdata->col_gpios[col], !level_on);
> +               gpio_direction_input(pdata->col_gpios[col]);
> +       }
> +}
> +
> +static void activate_all_cols(struct matrix_keypad_platform_data
> *pdata, int on)
> +{
> +       int col;
> +
> +       for (col = 0; col < pdata->num_col_gpios; col++)
> +               __activate_col(pdata, col, on);
> +}
> +
> +static void activate_col(struct matrix_keypad_platform_data *pdata,
> +                        int col, int on)
> +{
> +       __activate_col(pdata, col, on);
> +
> +       if (on && pdata->col_scan_delay_us)
> +               udelay(pdata->col_scan_delay_us);
> +}
> +
> +static int row_asserted(struct matrix_keypad_platform_data *pdata, int row)
> +{
> +       return gpio_get_value_cansleep(pdata->row_gpios[row]) ?
> +                       !pdata->active_low : pdata->active_low;
> +}
> +
> +static void enable_row_irqs(struct matrix_keypad *keypad)
> +{
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       int i;
> +
> +       for (i = 0; i < pdata->num_row_gpios; i++)
> +               enable_irq(gpio_to_irq(pdata->row_gpios[i]));
> +}
> +
> +static void disable_row_irqs(struct matrix_keypad *keypad)
> +{
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       int i;
> +
> +       for (i = 0; i < pdata->num_row_gpios; i++)
> +               disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
> +}
> +
> +/*
> + * This gets the keys from keyboard and reports it to input subsystem
> + */
> +static void matrix_keypad_scan(struct work_struct *work)
> +{
> +       struct matrix_keypad *keypad =
> +                       container_of(work, struct matrix_keypad, work.work);
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       uint32_t new_state[MATRIX_MAX_COLS];
> +       int row, col;
> +
> +       /* de-activate all columns for scanning */
> +       activate_all_cols(pdata, 0);
> +
> +       memset(new_state, 0, sizeof(new_state));
> +
> +       /* assert each column and read the row status out */
> +       for (col = 0; col < pdata->num_col_gpios; col++) {
> +
> +               activate_col(pdata, col, 1);
> +
> +               for (row = 0; row < pdata->num_row_gpios; row++)
> +                       new_state[col] |= row_asserted(pdata, row) ?
> +                                               (1 << row) : 0;
> +               activate_col(pdata, col, 0);
> +       }
> +
> +       for (col = 0; col < pdata->num_col_gpios; col++) {
> +               uint32_t bits_changed;
> +
> +               bits_changed = keypad->last_key_state[col] ^ new_state[col];
> +               if (bits_changed == 0)
> +                       continue;
> +
> +               for (row = 0; row < pdata->num_row_gpios; row++) {
> +                       if ((bits_changed & (1 << row)) == 0)
> +                               continue;
> +
> +                       input_report_key(keypad->input_dev,
> +                                       lookup_keycode(keypad, row, col),
> +                                       new_state[col] & (1 << row));
> +               }
> +       }
> +       input_sync(keypad->input_dev);
> +       memcpy(keypad->last_key_state, new_state, sizeof(new_state));
> +
> +       activate_all_cols(pdata, 1);
> +       enable_row_irqs(keypad);
> +}
> +
> +static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
> +{
> +       struct matrix_keypad *keypad = id;
> +
> +       disable_row_irqs(keypad);
> +       schedule_delayed_work(&keypad->work,
> +                       msecs_to_jiffies(keypad->pdata->debounce_ms));
> +       return IRQ_HANDLED;
> +}
> +
> +#ifdef CONFIG_PM
> +static int matrix_keypad_suspend(struct platform_device *pdev,
> +                                pm_message_t state)
> +{
> +       struct matrix_keypad *keypad = platform_get_drvdata(pdev);
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       int i;
> +
> +       disable_row_irqs(keypad);
> +       flush_work(&keypad->work.work);
> +
> +       if (device_may_wakeup(&pdev->dev))
> +               for (i = 0; i < pdata->num_row_gpios; i++)
> +                       enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
> +
> +       return 0;
> +}
> +
> +static int matrix_keypad_resume(struct platform_device *pdev)
> +{
> +       struct matrix_keypad *keypad = platform_get_drvdata(pdev);
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       int i;
> +
> +       if (device_may_wakeup(&pdev->dev))
> +               for (i = 0; i < pdata->num_row_gpios; i++)
> +                       disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
> +
> +       /* Uli Luckas: schedule an immediate key scan as all key state changes
> +        * were lost while the device was suspended, columns will be activated
> +        * and IRQs be enabled after the scan.
> +        */
> +       schedule_delayed_work(&keypad->work, 0);
> +       return 0;
> +}
> +#else
> +#define matrix_keypad_suspend  NULL
> +#define matrix_keypad_resume   NULL
> +#endif
> +
> +static int __devinit init_matrix_gpio(struct matrix_keypad *keypad)
> +{
> +       struct matrix_keypad_platform_data *pdata = keypad->pdata;
> +       int i, err = -EINVAL;
> +
> +       /* initialized strobe lines as outputs, activated */
> +       for (i = 0; i < pdata->num_col_gpios; i++) {
> +               err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
> +               if (err) {
> +                       pr_err("failed to request GPIO%d for COL%d\n",
> +                                       pdata->col_gpios[i], i);
> +                       goto err_free_cols;
> +               }
> +
> +               gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
> +       }
> +
> +       for (i = 0; i < pdata->num_row_gpios; i++) {
> +               err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
> +               if (err) {
> +                       pr_err("failed to request GPIO%d for ROW%d\n",
> +                                       pdata->row_gpios[i], i);
> +                       goto err_free_rows;
> +               }
> +
> +               gpio_direction_input(pdata->row_gpios[i]);
> +       }
> +
> +       for (i = 0; i < pdata->num_row_gpios; i++) {
> +               err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
> +                               matrix_keypad_interrupt, IRQF_DISABLED |
> +                               IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +                               "matrix-keypad", keypad);
> +               if (err) {
> +                       pr_err("Unable to acquire interrupt for GPIO line %i\n",
> +                               pdata->row_gpios[i]);
> +                       goto err_free_irqs;
> +               }
> +       }
> +       return 0;
> +
> +err_free_irqs:
> +       for (i = i - 1; i >= 0; i--)
> +               free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
> +
> +err_free_rows:
> +       for (i = i - 1; i >= 0; i--)
> +               gpio_free(pdata->row_gpios[i]);
> +
> +err_free_cols:
> +       for (i = i - 1; i >= 0; i--)
> +               gpio_free(pdata->col_gpios[i]);
> +
> +       return err;
> +}
> +
> +static int __devinit matrix_keypad_probe(struct platform_device *pdev)
> +{
> +       struct matrix_keypad_platform_data *pdata;
> +       struct matrix_keypad *keypad;
> +       struct input_dev *input_dev;
> +       int err = -ENOMEM;
> +
> +       pdata = pdev->dev.platform_data;
> +       if (pdata == NULL) {
> +               dev_err(&pdev->dev, "no platform data defined\n");
> +               return -EINVAL;
> +       }
> +
> +       keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
> +       if (keypad == NULL)
> +               return -ENOMEM;
> +
> +       input_dev = input_allocate_device();
> +       if (!input_dev)
> +               goto err_free_keypad;
> +
> +       platform_set_drvdata(pdev, keypad);
> +
> +       keypad->input_dev = input_dev;
> +       keypad->pdata = pdata;
> +       INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
> +
> +       input_dev->name         = pdev->name;
> +       input_dev->id.bustype   = BUS_HOST;
> +       input_dev->dev.parent   = &pdev->dev;
> +       input_dev->evbit[0]     = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
> +
> +       err = build_keycodes(keypad);
> +       if (err)
> +               goto err_free_input;
> +
> +       err = input_register_device(keypad->input_dev);
> +       if (err)
> +               goto err_free_keycodes;
> +
> +       err = init_matrix_gpio(keypad);
> +       if (err)
> +               goto err_unregister;
> +
> +       device_init_wakeup(&pdev->dev, 1);
> +       return 0;
> +
> +err_unregister:
> +       input_unregister_device(input_dev);
> +err_free_keycodes:
> +       kfree(keypad->keycodes);
> +err_free_input:
> +       input_free_device(input_dev);
> +err_free_keypad:
> +       kfree(keypad);
> +       return err;
> +}
> +
> +static int __devexit matrix_keypad_remove(struct platform_device *pdev)
> +{
> +       struct matrix_keypad *keypad = platform_get_drvdata(pdev);
> +       int i;
> +
> +       for (i = 0; i < keypad->pdata->num_row_gpios; i++) {
> +               free_irq(gpio_to_irq(keypad->pdata->row_gpios[i]), keypad);
> +               gpio_free(keypad->pdata->row_gpios[i]);
> +       }
> +
> +       for (i = 0; i < keypad->pdata->num_col_gpios; i++)
> +               gpio_free(keypad->pdata->col_gpios[i]);
> +
> +       input_unregister_device(keypad->input_dev);
> +       kfree(keypad->keycodes);
> +       kfree(keypad);
> +       return 0;
> +}
> +
> +static struct platform_driver matrix_keypad_driver = {
> +       .probe          = matrix_keypad_probe,
> +       .remove         = __devexit_p(matrix_keypad_remove),
> +       .suspend        = matrix_keypad_suspend,
> +       .resume         = matrix_keypad_resume,
> +       .driver         = {
> +               .name   = "matrix-keypad",
> +               .owner  = THIS_MODULE,
> +       },
> +};
> +
> +static int __init matrix_keypad_init(void)
> +{
> +       return platform_driver_register(&matrix_keypad_driver);
> +}
> +
> +static void __exit matrix_keypad_exit(void)
> +{
> +       platform_driver_unregister(&matrix_keypad_driver);
> +}
> +
> +module_init(matrix_keypad_init);
> +module_exit(matrix_keypad_exit);
> +
> +MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
> +MODULE_DESCRIPTION("GPIO Driven Matrix Keypad Driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:matrix-keypad");
> diff --git a/include/linux/input/matrix_keypad.h
> b/include/linux/input/matrix_keypad.h
> new file mode 100644
> index 0000000..8b661cb
> --- /dev/null
> +++ b/include/linux/input/matrix_keypad.h
> @@ -0,0 +1,34 @@
> +#ifndef _MATRIX_KEYPAD_H
> +#define _MATRIX_KEYPAD_H
> +
> +#include <linux/input.h>
> +
> +#define MATRIX_MAX_ROWS                16
> +#define MATRIX_MAX_COLS                16
> +#define MATRIX_MAX_KEYS                (MATRIX_MAX_ROWS * MATRIX_MAX_COLS)
> +
> +struct matrix_keypad_platform_data {
> +       /* scancode map for the matrix keys */
> +       uint32_t        *key_map;
> +       int             key_map_size;
> +
> +       unsigned        row_gpios[MATRIX_MAX_ROWS];
> +       unsigned        col_gpios[MATRIX_MAX_COLS];
> +       int             num_row_gpios;
> +       int             num_col_gpios;
> +
> +       unsigned int    active_low;
> +       unsigned int    col_scan_delay_us;
> +
> +       /* key debounce interval in milli-second */
> +       unsigned int    debounce_ms;
> +};
> +
> +#define KEY(row, col, val)     ((((row) & (MATRIX_MAX_ROWS - 1)) << 28) |\
> +                                (((col) & (MATRIX_MAX_COLS - 1)) << 24) |\
> +                                (val & 0xffffff))
> +
> +#define KEY_ROWCOL(k)          (((k) >> 24) & 0xff)
> +#define KEY_VAL(k)             ((k) & 0xffffff)
> +
> +#endif /* _MATRIX_KEYPAD_H */
> --
> 1.6.0.4
>




-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Dmitry Torokhov]

On Sunday 07 June 2009 07:04:09 Eric Miao wrote:
> On Wed, Jun 3, 2009 at 2:24 PM, Eric Miao<eric.y.miao@gmail.com> wrote:
> > On Tue, Jun 2, 2009 at 11:14 PM, Uli Luckas <u.luckas@road.de> wrote:
> >> One more...
> >>
> >> As all gpio handling is done in a workqueue,
> >> gpio_set_value_cansleep/gpio_get_value_cansleep could probably be used.
> >> This would make the driver ready for gpios on external controllers (via
> >> i2c for example).
> >
> > Sounds good, patch updated again, with attachment.
>
> Hi Dmitry,
>
> I'd like to know if this patch is able to get into the boat of the
> next merge window, I have several patches depending on
> this, and it seems that Trilok has something as well. I know
> it might take some time to review all this, but let me know
> if there are anything I can help.
>

Eric,

I tried to merge your latest patch with some work I have done earlier,
the result is below. Please give it a try and if it still works then
I will queue it for pull.

The most important change IMHO is the way IRQs are enabled and disabled.
We cannot simply do that without any locking because (on SMP) more than
one interrupt handler may be executing at the same time causing interrupts
being disabled several times. Same goes for enabling interrupts at the end
of the scan - once first is enabled we have a chance of getting it raised
right away and then we have 2 pieces racing against each other, one
enabling and another disabling interrupts. Also care is needed to flush
work properly at remove time. Please check out the logic for holes in case
I missed something.

Other changes:
- Make key table definition useable to other drivers;
- Allow changing keymap from userspace
- open/close
- various cleanups.

Thanks!

--
Dmitry

Input: add support for generic GPIO-based matrix keypad

From: Eric Miao <eric.y.miao@gmail.com>

Original patch by Marek Vasut, modified by Eric in:

1. use delayed work to simplify the debouncing
2. combine col_polarity/row_polarity into a single active_low field
3. use a generic bit array based XOR algorithm to detect key
   press/release, which should make the column assertion time
   shorter and code a bit cleaner
4. remove the ALT_FN handling, which is no way generic, the ALT_FN
   key should be treated as no different from other keys, and
   translation will be done by user space by commands like 'loadkeys'.

[dtor@mail.ru: fix error unwinding path, support changing keymap
 from userspace]
Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Signed-off-by: Eric Miao <eric.miao@marvell.com>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/keyboard/Kconfig         |   13 +
 drivers/input/keyboard/Makefile        |    1 
 drivers/input/keyboard/matrix_keypad.c |  453 ++++++++++++++++++++++++++++++++
 include/linux/input/matrix_keypad.h    |   65 +++++
 4 files changed, 530 insertions(+), 2 deletions(-)
 create mode 100644 drivers/input/keyboard/matrix_keypad.c
 create mode 100644 include/linux/input/matrix_keypad.h


diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index d2df103..a6b989a 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -158,7 +158,16 @@ config KEYBOARD_GPIO
 	  with configuration data saying which GPIOs are used.
 
 	  To compile this driver as a module, choose M here: the
-	  module will be called gpio-keys.
+	  module will be called gpio_keys.
+
+config KEYBOARD_MATRIX
+	tristate "GPIO driven matrix keypad support"
+	depends on GENERIC_GPIO
+	help
+	  Enable support for GPIO driven matrix keypad.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called matrix_keypad.
 
 config KEYBOARD_HIL_OLD
 	tristate "HP HIL keyboard support (simple driver)"
@@ -254,7 +263,7 @@ config KEYBOARD_PXA27x
 	tristate "PXA27x/PXA3xx keypad support"
 	depends on PXA27x || PXA3xx
 	help
-	  Enable support for PXA27x/PXA3xx keypad controller
+	  Enable support for PXA27x/PXA3xx keypad controller.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called pxa27x_keypad.
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 632efbc..b5b5eae 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_KEYBOARD_LKKBD)		+= lkkbd.o
 obj-$(CONFIG_KEYBOARD_LM8323)		+= lm8323.o
 obj-$(CONFIG_KEYBOARD_LOCOMO)		+= locomokbd.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
+obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
 obj-$(CONFIG_KEYBOARD_NEWTON)		+= newtonkbd.o
 obj-$(CONFIG_KEYBOARD_OMAP)		+= omap-keypad.o
 obj-$(CONFIG_KEYBOARD_PXA27x)		+= pxa27x_keypad.o
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
new file mode 100644
index 0000000..4040cf0
--- /dev/null
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -0,0 +1,453 @@
+/*
+ *  GPIO driven matrix keyboard driver
+ *
+ *  Copyright (c) 2008 Marek Vasut <marek.vasut@gmail.com>
+ *
+ *  Based on corgikbd.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/input/matrix_keypad.h>
+
+struct matrix_keypad {
+	const struct matrix_keypad_platform_data *pdata;
+	struct input_dev *input_dev;
+	unsigned short *keycodes;
+
+	uint32_t last_key_state[MATRIX_MAX_COLS];
+	struct delayed_work work;
+	bool scan_pending;
+	bool stopped;
+	spinlock_t lock;
+};
+
+/*
+ * NOTE: normally the GPIO has to be put into HiZ when de-activated to cause
+ * minmal side effect when scanning other columns, here it is configured to
+ * be input, and it should work on most platforms.
+ */
+static void __activate_col(const struct matrix_keypad_platform_data *pdata,
+			   int col, bool on)
+{
+	bool level_on = !pdata->active_low;
+
+	if (on) {
+		gpio_direction_output(pdata->col_gpios[col], level_on);
+	} else {
+		gpio_set_value_cansleep(pdata->col_gpios[col], !level_on);
+		gpio_direction_input(pdata->col_gpios[col]);
+	}
+}
+
+static void activate_col(const struct matrix_keypad_platform_data *pdata,
+			 int col, bool on)
+{
+	__activate_col(pdata, col, on);
+
+	if (on && pdata->col_scan_delay_us)
+		udelay(pdata->col_scan_delay_us);
+}
+
+static void activate_all_cols(const struct matrix_keypad_platform_data *pdata,
+			      bool on)
+{
+	int col;
+
+	for (col = 0; col < pdata->num_col_gpios; col++)
+		__activate_col(pdata, col, on);
+}
+
+static bool row_asserted(const struct matrix_keypad_platform_data *pdata,
+			 int row)
+{
+	return gpio_get_value_cansleep(pdata->row_gpios[row]) ?
+			!pdata->active_low : pdata->active_low;
+}
+
+static void enable_row_irqs(struct matrix_keypad *keypad)
+{
+	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		enable_irq(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+static void disable_row_irqs(struct matrix_keypad *keypad)
+{
+	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	for (i = 0; i < pdata->num_row_gpios; i++)
+		disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
+}
+
+/*
+ * This gets the keys from keyboard and reports it to input subsystem
+ */
+static void matrix_keypad_scan(struct work_struct *work)
+{
+	struct matrix_keypad *keypad =
+		container_of(work, struct matrix_keypad, work.work);
+	struct input_dev *input_dev = keypad->input_dev;
+	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	uint32_t new_state[MATRIX_MAX_COLS];
+	int row, col, code;
+
+	/* de-activate all columns for scanning */
+	activate_all_cols(pdata, false);
+
+	memset(new_state, 0, sizeof(new_state));
+
+	/* assert each column and read the row status out */
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+
+		activate_col(pdata, col, true);
+
+		for (row = 0; row < pdata->num_row_gpios; row++)
+			new_state[col] |=
+				row_asserted(pdata, row) ? (1 << row) : 0;
+
+		activate_col(pdata, col, false);
+	}
+
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+		uint32_t bits_changed;
+
+		bits_changed = keypad->last_key_state[col] ^ new_state[col];
+		if (bits_changed == 0)
+			continue;
+
+		for (row = 0; row < pdata->num_row_gpios; row++) {
+			if ((bits_changed & (1 << row)) == 0)
+				continue;
+
+			code = (row << 4) + col;
+			input_event(input_dev, EV_MSC, MSC_SCAN, code);
+			input_report_key(input_dev,
+					 keypad->keycodes[code],
+					 new_state[col] & (1 << row));
+		}
+	}
+	input_sync(input_dev);
+
+	memcpy(keypad->last_key_state, new_state, sizeof(new_state));
+
+	activate_all_cols(pdata, true);
+
+	/* Enable IRQs again */
+	spin_lock_irq(&keypad->lock);
+	keypad->scan_pending = false;
+	enable_row_irqs(keypad);
+	spin_unlock_irq(&keypad->lock);
+}
+
+static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
+{
+	struct matrix_keypad *keypad = id;
+	unsigned long flags;
+
+	spin_lock_irqsave(&keypad->lock, flags);
+
+	/*
+	 * See if another IRQ beaten us to it and scheduled the
+	 * scan already. In that case we should not try to
+	 * disable IRQs again.
+	 */
+	if (unlikely(keypad->scan_pending || keypad->stopped))
+		goto out;
+
+	disable_row_irqs(keypad);
+	keypad->scan_pending = true;
+	schedule_delayed_work(&keypad->work,
+		msecs_to_jiffies(keypad->pdata->debounce_ms));
+
+out:
+	spin_unlock_irqrestore(&keypad->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static int matrix_keypad_start(struct input_dev *dev)
+{
+	struct matrix_keypad *keypad = input_get_drvdata(dev);
+
+	keypad->stopped = false;
+	mb();
+
+	/*
+	 * Schedule an immediate key scan to capture current key state;
+	 * columns will be activated and IRQs be enabled after the scan.
+	 */
+	schedule_delayed_work(&keypad->work, 0);
+
+	return 0;
+}
+
+static void matrix_keypad_stop(struct input_dev *dev)
+{
+	struct matrix_keypad *keypad = input_get_drvdata(dev);
+
+	keypad->stopped = true;
+	mb();
+	flush_work(&keypad->work.work);
+	/*
+	 * matrix_keypad_scan() will leave IRQs enabled;
+	 * we should disable them now.
+	 */
+	disable_row_irqs(keypad);
+}
+
+#ifdef CONFIG_PM
+static int matrix_keypad_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	matrix_keypad_stop(keypad->input_dev);
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	return 0;
+}
+
+static int matrix_keypad_resume(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	if (device_may_wakeup(&pdev->dev))
+		for (i = 0; i < pdata->num_row_gpios; i++)
+			disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
+
+	matrix_keypad_start(keypad->input_dev);
+
+	return 0;
+}
+#else
+#define matrix_keypad_suspend	NULL
+#define matrix_keypad_resume	NULL
+#endif
+
+static int __devinit init_matrix_gpio(struct platform_device *pdev,
+					struct matrix_keypad *keypad)
+{
+	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i, err = -EINVAL;
+
+	/* initialized strobe lines as outputs, activated */
+	for (i = 0; i < pdata->num_col_gpios; i++) {
+		err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
+		if (err) {
+			dev_err(&pdev->dev,
+				"failed to request GPIO%d for COL%d\n",
+				pdata->col_gpios[i], i);
+			goto err_free_cols;
+		}
+
+		gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
+		if (err) {
+			dev_err(&pdev->dev,
+				"failed to request GPIO%d for ROW%d\n",
+				pdata->row_gpios[i], i);
+			goto err_free_rows;
+		}
+
+		gpio_direction_input(pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
+				matrix_keypad_interrupt,
+				IRQF_DISABLED |
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				"matrix-keypad", keypad);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Unable to acquire interrupt for GPIO line %i\n",
+				pdata->row_gpios[i]);
+			goto err_free_irqs;
+		}
+	}
+	return 0;
+
+err_free_irqs:
+	while (--i >= 0)
+		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+	i = pdata->num_row_gpios;
+err_free_rows:
+	while (--i >= 0)
+		gpio_free(pdata->row_gpios[i]);
+	i = pdata->num_col_gpios;
+err_free_cols:
+	while (--i >= 0)
+		gpio_free(pdata->col_gpios[i]);
+
+	return err;
+}
+
+static int __devinit matrix_keypad_probe(struct platform_device *pdev)
+{
+	const struct matrix_keypad_platform_data *pdata;
+	const struct matrix_keymap_data *keymap_data;
+	struct matrix_keypad *keypad;
+	struct input_dev *input_dev;
+	unsigned short *keycodes;
+	int i;
+	int err;
+
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		dev_err(&pdev->dev, "no platform data defined\n");
+		return -EINVAL;
+	}
+
+	keymap_data = pdata->keymap_data;
+	if (!keymap_data) {
+		dev_err(&pdev->dev, "no keymap data defined\n");
+		return -EINVAL;
+	}
+
+	if (!keymap_data->max_keymap_size) {
+		dev_err(&pdev->dev, "invalid keymap data supplied\n");
+		return -EINVAL;
+	}
+
+	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
+	keycodes = kzalloc(keymap_data->max_keymap_size *
+				sizeof(keypad->keycodes),
+			   GFP_KERNEL);
+	input_dev = input_allocate_device();
+	if (!keypad || !keycodes || !input_dev) {
+		err = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	keypad->input_dev = input_dev;
+	keypad->pdata = pdata;
+	keypad->keycodes = keycodes;
+	keypad->stopped = true;
+	INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
+	spin_lock_init(&keypad->lock);
+
+	input_dev->name		= pdev->name;
+	input_dev->id.bustype	= BUS_HOST;
+	input_dev->dev.parent	= &pdev->dev;
+	input_dev->evbit[0]	= BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+	input_dev->open		= matrix_keypad_start;
+	input_dev->close	= matrix_keypad_stop;
+
+	input_dev->keycode	= keycodes;
+	input_dev->keycodesize	= sizeof(*keycodes);
+	input_dev->keycodemax	= keymap_data->max_keymap_size;
+
+	for (i = 0; i < keymap_data->keymap_size; i++) {
+		unsigned int key = keymap_data->keymap[i];
+		unsigned int row = KEY_ROW(key);
+		unsigned int col = KEY_COL(key);
+		unsigned short code = KEY_VAL(key);
+
+		keycodes[(row << 4) + col] = code;
+		__set_bit(code, input_dev->keybit);
+	}
+	__clear_bit(KEY_RESERVED, input_dev->keybit);
+
+	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
+	input_set_drvdata(input_dev, keypad);
+
+	err = input_register_device(keypad->input_dev);
+	if (err)
+		goto err_free_mem;
+
+	err = init_matrix_gpio(pdev, keypad);
+	if (err)
+		goto err_unregister;
+
+	device_init_wakeup(&pdev->dev, pdata->wakeup);
+	platform_set_drvdata(pdev, keypad);
+
+	return 0;
+
+err_unregister:
+	input_unregister_device(input_dev);
+	input_dev = NULL;
+err_free_mem:
+	input_free_device(input_dev);
+	kfree(keycodes);
+	kfree(keypad);
+	return err;
+}
+
+static int __devexit matrix_keypad_remove(struct platform_device *pdev)
+{
+	struct matrix_keypad *keypad = platform_get_drvdata(pdev);
+	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+	int i;
+
+	device_init_wakeup(&pdev->dev, 0);
+
+	for (i = 0; i < pdata->num_row_gpios; i++) {
+		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+		gpio_free(pdata->row_gpios[i]);
+	}
+
+	for (i = 0; i < pdata->num_col_gpios; i++)
+		gpio_free(pdata->col_gpios[i]);
+
+	input_unregister_device(keypad->input_dev);
+	platform_set_drvdata(pdev, NULL);
+	kfree(keypad->keycodes);
+	kfree(keypad);
+
+	return 0;
+}
+
+static struct platform_driver matrix_keypad_driver = {
+	.probe		= matrix_keypad_probe,
+	.remove		= __devexit_p(matrix_keypad_remove),
+	.suspend	= matrix_keypad_suspend,
+	.resume		= matrix_keypad_resume,
+	.driver		= {
+		.name	= "matrix-keypad",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init matrix_keypad_init(void)
+{
+	return platform_driver_register(&matrix_keypad_driver);
+}
+
+static void __exit matrix_keypad_exit(void)
+{
+	platform_driver_unregister(&matrix_keypad_driver);
+}
+
+module_init(matrix_keypad_init);
+module_exit(matrix_keypad_exit);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
+MODULE_DESCRIPTION("GPIO Driven Matrix Keypad Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:matrix-keypad");
diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
new file mode 100644
index 0000000..f945402
--- /dev/null
+++ b/include/linux/input/matrix_keypad.h
@@ -0,0 +1,65 @@
+#ifndef _MATRIX_KEYPAD_H
+#define _MATRIX_KEYPAD_H
+
+#include <linux/types.h>
+#include <linux/input.h>
+
+#define MATRIX_MAX_ROWS		16
+#define MATRIX_MAX_COLS		16
+
+#define KEY(row, col, val)	((((row) & (MATRIX_MAX_ROWS - 1)) << 24) |\
+				 (((col) & (MATRIX_MAX_COLS - 1)) << 16) |\
+				 (val & 0xffff))
+
+#define KEY_ROW(k)		(((k) >> 24) & 0xff)
+#define KEY_COL(k)		(((k) >> 16) & 0xff)
+#define KEY_VAL(k)		((k) & 0xffff)
+
+/**
+ * struct matrix_keymap_data - keymap for matrix keyboards
+ * @keymap: pointer to array of uint32 values encoded with KEY() macro
+ * 	representing keymap
+ * @keymap_size: number of entries (initialized) in this keymap
+ * @max_keymap_size: maximum size of keymap supported by the device
+ *
+ * This structure is supposed to be used by platform code to supply
+ * keymaps to drivers that implement matrix-like keypads/keyboards.
+ */
+struct matrix_keymap_data {
+	const uint32_t *keymap;
+	unsigned int	keymap_size;
+	unsigned int	max_keymap_size;
+};
+
+/**
+ * struct matrix_keypad_platform_data - platform-dependent keypad data
+ * @keymap_data: pointer to &matrix_keymap_data
+ * @row_gpios: array of gpio numbers reporesenting rows
+ * @col_gpios: array of gpio numbers reporesenting colums
+ * @num_row_gpios: actual number of row gpios used by device
+ * @num_col_gpios: actual number of col gpios used by device
+ * @col_scan_delay_us: delay, measured in microseconds, that is
+ * 	needed before we can keypad after activating column gpio
+ * @debounce_ms: debounce interval in milliseconds
+ *
+ * This structure represents platform-specific data that use used by
+ * matrix_keypad driver to perform proper initialization.
+ */
+struct matrix_keypad_platform_data {
+	const struct matrix_keymap_data *keymap_data;
+
+	unsigned int	row_gpios[MATRIX_MAX_ROWS];
+	unsigned int	col_gpios[MATRIX_MAX_COLS];
+	unsigned int	num_row_gpios;
+	unsigned int	num_col_gpios;
+
+	unsigned int	col_scan_delay_us;
+
+	/* key debounce interval in milli-second */
+	unsigned int	debounce_ms;
+
+	bool		active_low;
+	bool		wakeup;
+};
+
+#endif /* _MATRIX_KEYPAD_H */

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Dmitry,

>
> Input: add support for generic GPIO-based matrix keypad
>
> From: Eric Miao <eric.y.miao@gmail.com>
>
> Original patch by Marek Vasut, modified by Eric in:
>
> 1. use delayed work to simplify the debouncing
> 2. combine col_polarity/row_polarity into a single active_low field
> 3. use a generic bit array based XOR algorithm to detect key
>   press/release, which should make the column assertion time
>   shorter and code a bit cleaner
> 4. remove the ALT_FN handling, which is no way generic, the ALT_FN
>   key should be treated as no different from other keys, and
>   translation will be done by user space by commands like 'loadkeys'.
>
> [dtor@mail.ru: fix error unwinding path, support changing keymap
>  from userspace]
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Signed-off-by: Eric Miao <eric.miao@marvell.com>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---

Did you took latest patch submitted from Eric? Because it had more
signed-off and acked-by lines, like this.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Reviewed-by: Trilok Soni <soni.trilok@gmail.com>
Reviewed-by: Uli Luckas <u.luckas@road.de>
Reviewed-by: Russell King <linux@arm.linux.org.uk>
Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Eric Miao <eric.miao@marvell.com>

http://markmail.org/message/2wrr2b6mr6qsd4xs#query:+page:1+mid:fkkfxlumfm4mjhk4+state:results

Eric can confirm otherwise.

>
>  config KEYBOARD_HIL_OLD
>        tristate "HP HIL keyboard support (simple driver)"
> @@ -254,7 +263,7 @@ config KEYBOARD_PXA27x
>        tristate "PXA27x/PXA3xx keypad support"
>        depends on PXA27x || PXA3xx
>        help
> -         Enable support for PXA27x/PXA3xx keypad controller
> +         Enable support for PXA27x/PXA3xx keypad controller.

Why this change in this patch?

> +
> +                       code = (row << 4) + col;

<< 4 logic will break once MAX_ROWS increased, right?


-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

On Fri, Jun 12, 2009 at 9:01 PM, Trilok Soni<soni.trilok@gmail.com> wrote:
> Hi Dmitry,
>
>>
>> Input: add support for generic GPIO-based matrix keypad
>>
>> From: Eric Miao <eric.y.miao@gmail.com>
>>
>> Original patch by Marek Vasut, modified by Eric in:
>>
>> 1. use delayed work to simplify the debouncing
>> 2. combine col_polarity/row_polarity into a single active_low field
>> 3. use a generic bit array based XOR algorithm to detect key
>>   press/release, which should make the column assertion time
>>   shorter and code a bit cleaner
>> 4. remove the ALT_FN handling, which is no way generic, the ALT_FN
>>   key should be treated as no different from other keys, and
>>   translation will be done by user space by commands like 'loadkeys'.
>>
>> [dtor@mail.ru: fix error unwinding path, support changing keymap
>>  from userspace]
>> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>> Signed-off-by: Eric Miao <eric.miao@marvell.com>
>> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
>> ---
>
> Did you took latest patch submitted from Eric? Because it had more
> signed-off and acked-by lines, like this.
>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Reviewed-by: Trilok Soni <soni.trilok@gmail.com>
> Reviewed-by: Uli Luckas <u.luckas@road.de>
> Reviewed-by: Russell King <linux@arm.linux.org.uk>
> Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>
> Signed-off-by: Eric Miao <eric.miao@marvell.com>
>
> http://markmail.org/message/2wrr2b6mr6qsd4xs#query:+page:1+mid:fkkfxlumfm4mjhk4+state:results
>
> Eric can confirm otherwise.
>
>>
>>  config KEYBOARD_HIL_OLD
>>        tristate "HP HIL keyboard support (simple driver)"
>> @@ -254,7 +263,7 @@ config KEYBOARD_PXA27x
>>        tristate "PXA27x/PXA3xx keypad support"
>>        depends on PXA27x || PXA3xx
>>        help
>> -         Enable support for PXA27x/PXA3xx keypad controller
>> +         Enable support for PXA27x/PXA3xx keypad controller.
>
> Why this change in this patch?
>
>> +
>> +                       code = (row << 4) + col;
>
> << 4 logic will break once MAX_ROWS increased, right?
>

Hi Dmitry,

I've tested the driver code, and it's basically OK except for two minor
fixes:

1) GPIO and IRQs have to be initialized before input_register_device(),
otherwise input->open() will be invoked before that, which will schedule
an immediate scan work and fail.

2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
initialized to disabled - and will be enabled by input->open()

Diff follows:

--- drivers/input/keyboard/matrix_keypad.c.orig	2009-06-12
21:18:20.000000000 +0800
+++ drivers/input/keyboard/matrix_keypad.c	2009-06-12 21:07:26.000000000 +0800
@@ -290,6 +290,9 @@ static int __devinit init_matrix_gpio(st
 			goto err_free_irqs;
 		}
 	}
+
+	/* initialized as disabled - enabled by input->open */
+	disable_row_irqs(keypad);
 	return 0;

 err_free_irqs:
@@ -376,22 +379,19 @@ static int __devinit matrix_keypad_probe
 	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
 	input_set_drvdata(input_dev, keypad);

-	err = input_register_device(keypad->input_dev);
+	err = init_matrix_gpio(pdev, keypad);
 	if (err)
 		goto err_free_mem;

-	err = init_matrix_gpio(pdev, keypad);
+	err = input_register_device(keypad->input_dev);
 	if (err)
-		goto err_unregister;
+		goto err_free_mem;

 	device_init_wakeup(&pdev->dev, pdata->wakeup);
 	platform_set_drvdata(pdev, keypad);

 	return 0;

-err_unregister:
-	input_unregister_device(input_dev);
-	input_dev = NULL;
 err_free_mem:
 	input_free_device(input_dev);
 	kfree(keycodes);
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

On Fri, Jun 12, 2009 at 6:56 PM, Eric Miao<eric.y.miao@gmail.com> wrote:
> On Fri, Jun 12, 2009 at 9:01 PM, Trilok Soni<soni.trilok@gmail.com> wrote:
>> Hi Dmitry,
>>
>>>
>>> Input: add support for generic GPIO-based matrix keypad
>>>
>>> From: Eric Miao <eric.y.miao@gmail.com>
>>>
>>> Original patch by Marek Vasut, modified by Eric in:
>>>
>>> 1. use delayed work to simplify the debouncing
>>> 2. combine col_polarity/row_polarity into a single active_low field
>>> 3. use a generic bit array based XOR algorithm to detect key
>>>   press/release, which should make the column assertion time
>>>   shorter and code a bit cleaner
>>> 4. remove the ALT_FN handling, which is no way generic, the ALT_FN
>>>   key should be treated as no different from other keys, and
>>>   translation will be done by user space by commands like 'loadkeys'.
>>>
>>> [dtor@mail.ru: fix error unwinding path, support changing keymap
>>>  from userspace]
>>> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>>> Signed-off-by: Eric Miao <eric.miao@marvell.com>
>>> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
>>> ---
>>
>> Did you took latest patch submitted from Eric? Because it had more
>> signed-off and acked-by lines, like this.
>>
>> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>> Reviewed-by: Trilok Soni <soni.trilok@gmail.com>
>> Reviewed-by: Uli Luckas <u.luckas@road.de>
>> Reviewed-by: Russell King <linux@arm.linux.org.uk>
>> Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>
>> Signed-off-by: Eric Miao <eric.miao@marvell.com>
>>
>> http://markmail.org/message/2wrr2b6mr6qsd4xs#query:+page:1+mid:fkkfxlumfm4mjhk4+state:results
>>
>> Eric can confirm otherwise.
>>
>>>
>>>  config KEYBOARD_HIL_OLD
>>>        tristate "HP HIL keyboard support (simple driver)"
>>> @@ -254,7 +263,7 @@ config KEYBOARD_PXA27x
>>>        tristate "PXA27x/PXA3xx keypad support"
>>>        depends on PXA27x || PXA3xx
>>>        help
>>> -         Enable support for PXA27x/PXA3xx keypad controller
>>> +         Enable support for PXA27x/PXA3xx keypad controller.
>>
>> Why this change in this patch?
>>
>>> +
>>> +                       code = (row << 4) + col;
>>
>> << 4 logic will break once MAX_ROWS increased, right?
>>
>
> Hi Dmitry,
>
> I've tested the driver code, and it's basically OK except for two minor
> fixes:
>
> 1) GPIO and IRQs have to be initialized before input_register_device(),
> otherwise input->open() will be invoked before that, which will schedule
> an immediate scan work and fail.
>
> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
> initialized to disabled - and will be enabled by input->open()
>
> Diff follows:
>
> --- drivers/input/keyboard/matrix_keypad.c.orig 2009-06-12
> 21:18:20.000000000 +0800
> +++ drivers/input/keyboard/matrix_keypad.c      2009-06-12 21:07:26.000000000 +0800
> @@ -290,6 +290,9 @@ static int __devinit init_matrix_gpio(st
>                        goto err_free_irqs;
>                }
>        }
> +
> +       /* initialized as disabled - enabled by input->open */
> +       disable_row_irqs(keypad);
>        return 0;
>
>  err_free_irqs:
> @@ -376,22 +379,19 @@ static int __devinit matrix_keypad_probe
>        input_set_capability(input_dev, EV_MSC, MSC_SCAN);
>        input_set_drvdata(input_dev, keypad);
>
> -       err = input_register_device(keypad->input_dev);
> +       err = init_matrix_gpio(pdev, keypad);
>        if (err)
>                goto err_free_mem;
>
> -       err = init_matrix_gpio(pdev, keypad);
> +       err = input_register_device(keypad->input_dev);
>        if (err)
> -               goto err_unregister;
> +               goto err_free_mem;
>
>        device_init_wakeup(&pdev->dev, pdata->wakeup);
>        platform_set_drvdata(pdev, keypad);
>
>        return 0;
>
> -err_unregister:
> -       input_unregister_device(input_dev);
> -       input_dev = NULL;
>  err_free_mem:
>        input_free_device(input_dev);
>        kfree(keycodes);
>

Ping? We are already half-way through open merge window.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Dmitry Torokhov]

Hi Eric,

On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
> 
> Hi Dmitry,
> 
> I've tested the driver code, and it's basically OK except for two minor
> fixes:
> 
> 1) GPIO and IRQs have to be initialized before input_register_device(),
> otherwise input->open() will be invoked before that, which will schedule
> an immediate scan work and fail.
> 
> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
> initialized to disabled - and will be enabled by input->open()
> 

Could you please take a look at the driver as it is commited to
'for-linus' branch of my tree and let me know if you see anything wrong
there? Otherwise I inted to submit it in my next update to Linus.

Thanks!

-- 
Dmitry

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Dmitry,

On Mon, Jun 29, 2009 at 9:56 PM, Dmitry
Torokhov<dmitry.torokhov@gmail.com> wrote:
> Hi Eric,
>
> On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
>>
>> Hi Dmitry,
>>
>> I've tested the driver code, and it's basically OK except for two minor
>> fixes:
>>
>> 1) GPIO and IRQs have to be initialized before input_register_device(),
>> otherwise input->open() will be invoked before that, which will schedule
>> an immediate scan work and fail.
>>
>> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
>> initialized to disabled - and will be enabled by input->open()
>>
>
> Could you please take a look at the driver as it is commited to
> 'for-linus' branch of my tree and let me know if you see anything wrong
> there? Otherwise I inted to submit it in my next update to Linus.
>

Are you planning to take "MAX7359 input switch controller driver" too
for submission to Linus in -rc1?

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Kim Kyuwon]

On Tue, Jun 30, 2009 at 3:43 PM, Trilok Soni<soni.trilok@gmail.com> wrote:
> Hi Dmitry,
>
> On Mon, Jun 29, 2009 at 9:56 PM, Dmitry
> Torokhov<dmitry.torokhov@gmail.com> wrote:
>> Hi Eric,
>>
>> On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
>>>
>>> Hi Dmitry,
>>>
>>> I've tested the driver code, and it's basically OK except for two minor
>>> fixes:
>>>
>>> 1) GPIO and IRQs have to be initialized before input_register_device(),
>>> otherwise input->open() will be invoked before that, which will schedule
>>> an immediate scan work and fail.
>>>
>>> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
>>> initialized to disabled - and will be enabled by input->open()
>>>
>>
>> Could you please take a look at the driver as it is commited to
>> 'for-linus' branch of my tree and let me know if you see anything wrong
>> there? Otherwise I inted to submit it in my next update to Linus.
>>
>
> Are you planning to take "MAX7359 input switch controller driver" too
> for submission to Linus in -rc1?

Thanks, Trilok

Dmitry, can I know when MAX7359 keypad driver will be merged to mainline?

Regards,
Kyuwon

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Marek Vasut]

Dne St 10. června 2009 06:10:31 Trilok Soni napsal(a):
> Hi Dmitry,
>
> > Sounds good, patch updated again, with attachment.
> >
> > >From 76776dfc468dc35f0d8394a2331e1a91f390e642 Mon Sep 17 00:00:00 2001
> >
> > From: Marek Vasut <marek.vasut@gmail.com>
> > Date: Thu, 7 May 2009 15:49:32 +0800
> > Subject: [PATCH] input: add support for generic GPIO-based matrix keypad
>
> Could you please review this patch and see if it can be acked as merge
> window will open in couple of days? Thanks.

Hi, I have but one comment (see further), otherwise works well on Palm 
Tungsten C (PXA255A0 CPU).

btw Eric, please add yourself to the credits, you did most of the work on it 
anyway ;-) .
>
> > Original patch by Marek Vasut, modified by Eric in:
> >
> > 1. use delayed work to simplify the debouncing process

...

> > diff --git a/include/linux/input/matrix_keypad.h
> > b/include/linux/input/matrix_keypad.h
> > new file mode 100644
> > index 0000000..8b661cb
> > --- /dev/null
> > +++ b/include/linux/input/matrix_keypad.h
> > @@ -0,0 +1,34 @@
> > +#ifndef _MATRIX_KEYPAD_H
> > +#define _MATRIX_KEYPAD_H
> > +
> > +#include <linux/input.h>
> > +
> > +#define MATRIX_MAX_ROWS                16
> > +#define MATRIX_MAX_COLS                16
> > +#define MATRIX_MAX_KEYS                (MATRIX_MAX_ROWS *
> > MATRIX_MAX_COLS) +
> > +struct matrix_keypad_platform_data {
> > +       /* scancode map for the matrix keys */
> > +       uint32_t        *key_map;
> > +       int             key_map_size;
> > +
> > +       unsigned        row_gpios[MATRIX_MAX_ROWS];
> > +       unsigned        col_gpios[MATRIX_MAX_COLS];

Why not doing the above like the following?
+       unsigned        *row_gpios;
+       unsigned        *col_gpios;

> > +       int             num_row_gpios;
> > +       int             num_col_gpios;
> > +
> > +       unsigned int    active_low;

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

Marek Vasut wrote:
> Dne St 10. června 2009 06:10:31 Trilok Soni napsal(a):
>> Hi Dmitry,
>>
>>> Sounds good, patch updated again, with attachment.
>>>
>>> >From 76776dfc468dc35f0d8394a2331e1a91f390e642 Mon Sep 17 00:00:00 2001
>>>
>>> From: Marek Vasut <marek.vasut@gmail.com>
>>> Date: Thu, 7 May 2009 15:49:32 +0800
>>> Subject: [PATCH] input: add support for generic GPIO-based matrix keypad
>> Could you please review this patch and see if it can be acked as merge
>> window will open in couple of days? Thanks.
> 
> Hi, I have but one comment (see further), otherwise works well on Palm 
> Tungsten C (PXA255A0 CPU).
> 
> btw Eric, please add yourself to the credits, you did most of the work on it 
> anyway ;-) .
>>> Original patch by Marek Vasut, modified by Eric in:
>>>
>>> 1. use delayed work to simplify the debouncing process
> 
> ...
> 
>>> diff --git a/include/linux/input/matrix_keypad.h
>>> b/include/linux/input/matrix_keypad.h
>>> new file mode 100644
>>> index 0000000..8b661cb
>>> --- /dev/null
>>> +++ b/include/linux/input/matrix_keypad.h
>>> @@ -0,0 +1,34 @@
>>> +#ifndef _MATRIX_KEYPAD_H
>>> +#define _MATRIX_KEYPAD_H
>>> +
>>> +#include <linux/input.h>
>>> +
>>> +#define MATRIX_MAX_ROWS                16
>>> +#define MATRIX_MAX_COLS                16
>>> +#define MATRIX_MAX_KEYS                (MATRIX_MAX_ROWS *
>>> MATRIX_MAX_COLS) +
>>> +struct matrix_keypad_platform_data {
>>> +       /* scancode map for the matrix keys */
>>> +       uint32_t        *key_map;
>>> +       int             key_map_size;
>>> +
>>> +       unsigned        row_gpios[MATRIX_MAX_ROWS];
>>> +       unsigned        col_gpios[MATRIX_MAX_COLS];
> 
> Why not doing the above like the following?
> +       unsigned        *row_gpios;
> +       unsigned        *col_gpios;
> 

By having an array, one can specify the GPIOs inside the
structure initialization instead of a dedicated array outside,
which might look a little bit better.

>>> +       int             num_row_gpios;
>>> +       int             num_col_gpios;
>>> +
>>> +       unsigned int    active_low;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Marek Vasut]

Dne Po 6. července 2009 08:02:10 Eric Miao napsal(a):
> Marek Vasut wrote:
> > Dne St 10. června 2009 06:10:31 Trilok Soni napsal(a):
> >> Hi Dmitry,
> >>
> >>> Sounds good, patch updated again, with attachment.
> >>>
> >>> >From 76776dfc468dc35f0d8394a2331e1a91f390e642 Mon Sep 17 00:00:00 2001
> >>>
> >>> From: Marek Vasut <marek.vasut@gmail.com>
> >>> Date: Thu, 7 May 2009 15:49:32 +0800
> >>> Subject: [PATCH] input: add support for generic GPIO-based matrix
> >>> keypad
> >>
> >> Could you please review this patch and see if it can be acked as merge
> >> window will open in couple of days? Thanks.
> >
> > Hi, I have but one comment (see further), otherwise works well on Palm
> > Tungsten C (PXA255A0 CPU).
> >
> > btw Eric, please add yourself to the credits, you did most of the work on
> > it anyway ;-) .
> >
> >>> Original patch by Marek Vasut, modified by Eric in:
> >>>
> >>> 1. use delayed work to simplify the debouncing process
> >
> > ...
> >
> >>> diff --git a/include/linux/input/matrix_keypad.h
> >>> b/include/linux/input/matrix_keypad.h
> >>> new file mode 100644
> >>> index 0000000..8b661cb
> >>> --- /dev/null
> >>> +++ b/include/linux/input/matrix_keypad.h
> >>> @@ -0,0 +1,34 @@
> >>> +#ifndef _MATRIX_KEYPAD_H
> >>> +#define _MATRIX_KEYPAD_H
> >>> +
> >>> +#include <linux/input.h>
> >>> +
> >>> +#define MATRIX_MAX_ROWS                16
> >>> +#define MATRIX_MAX_COLS                16
> >>> +#define MATRIX_MAX_KEYS                (MATRIX_MAX_ROWS *
> >>> MATRIX_MAX_COLS) +
> >>> +struct matrix_keypad_platform_data {
> >>> +       /* scancode map for the matrix keys */
> >>> +       uint32_t        *key_map;
> >>> +       int             key_map_size;
> >>> +
> >>> +       unsigned        row_gpios[MATRIX_MAX_ROWS];
> >>> +       unsigned        col_gpios[MATRIX_MAX_COLS];
> >
> > Why not doing the above like the following?
> > +       unsigned        *row_gpios;
> > +       unsigned        *col_gpios;
>
> By having an array, one can specify the GPIOs inside the
> structure initialization instead of a dedicated array outside,
> which might look a little bit better.

But this way we will have the MATRIX_MAX_COLS MATRIX_MAX_ROWS used on less 
places (and we can eventually get rid of it easier in the future if someone 
wants to). Well I don't really care about the way you do it, both are OK with 
me ;-)
>
> >>> +       int             num_row_gpios;
> >>> +       int             num_col_gpios;
> >>> +
> >>> +       unsigned int    active_low;

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

Dmitry Torokhov wrote:
> Hi Eric,
> 
> On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
>> Hi Dmitry,
>>
>> I've tested the driver code, and it's basically OK except for two minor
>> fixes:
>>
>> 1) GPIO and IRQs have to be initialized before input_register_device(),
>> otherwise input->open() will be invoked before that, which will schedule
>> an immediate scan work and fail.
>>
>> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
>> initialized to disabled - and will be enabled by input->open()
>>
> 
> Could you please take a look at the driver as it is commited to
> 'for-linus' branch of my tree and let me know if you see anything wrong
> there? Otherwise I inted to submit it in my next update to Linus.
> 

Sorry Dmitry,

Reply so late and til now I have some time to test this. The patch there
still has at least one issue I guess - the keycodes[] allocation size
should be MATRIX_MAX_ROWS * MATRIX_MAX_COLS if I understand correctly.

I'm experiencing a problem of keypad->pdata being overwritten, and found
keycodes are actually accessed by [(row << 4) + col], so that should be
a full range.

And one more minor issue that may not be significant is the hardcoding of
the '4' in this (row << 4).

Some of the key hits on my Littleton just worked smoothly with the above
correction, but not all - some key hits just gave intermittent events, I
have no idea how's that - maybe related to my scan delay and debouncing
settings

> Thanks!
> 

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Eric,

On Thu, Jul 9, 2009 at 3:16 PM, Eric Miao<eric.y.miao@gmail.com> wrote:
> Dmitry Torokhov wrote:
>> Hi Eric,
>>
>> On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
>>> Hi Dmitry,
>>>
>>> I've tested the driver code, and it's basically OK except for two minor
>>> fixes:
>>>
>>> 1) GPIO and IRQs have to be initialized before input_register_device(),
>>> otherwise input->open() will be invoked before that, which will schedule
>>> an immediate scan work and fail.
>>>
>>> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
>>> initialized to disabled - and will be enabled by input->open()
>>>
>>
>> Could you please take a look at the driver as it is commited to
>> 'for-linus' branch of my tree and let me know if you see anything wrong
>> there? Otherwise I inted to submit it in my next update to Linus.
>>
>
> Sorry Dmitry,
>
> Reply so late and til now I have some time to test this. The patch there
> still has at least one issue I guess - the keycodes[] allocation size
> should be MATRIX_MAX_ROWS * MATRIX_MAX_COLS if I understand correctly.
>
> I'm experiencing a problem of keypad->pdata being overwritten, and found
> keycodes are actually accessed by [(row << 4) + col], so that should be
> a full range.
>
> And one more minor issue that may not be significant is the hardcoding of
> the '4' in this (row << 4).

I had pointed out this. We should probably do fls(MATRIX_MAX_COLS) there.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Paulius Zaleckas]

Eric Miao wrote:
> Dmitry Torokhov wrote:
>> Hi Eric,
>>
>> On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
>>> Hi Dmitry,
>>>
>>> I've tested the driver code, and it's basically OK except for two minor
>>> fixes:
>>>
>>> 1) GPIO and IRQs have to be initialized before input_register_device(),
>>> otherwise input->open() will be invoked before that, which will schedule
>>> an immediate scan work and fail.
>>>
>>> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
>>> initialized to disabled - and will be enabled by input->open()
>>>
>> Could you please take a look at the driver as it is commited to
>> 'for-linus' branch of my tree and let me know if you see anything wrong
>> there? Otherwise I inted to submit it in my next update to Linus.
>>
> 
> Sorry Dmitry,
> 
> Reply so late and til now I have some time to test this. The patch there
> still has at least one issue I guess - the keycodes[] allocation size
> should be MATRIX_MAX_ROWS * MATRIX_MAX_COLS if I understand correctly.
> 
> I'm experiencing a problem of keypad->pdata being overwritten, and found
> keycodes are actually accessed by [(row << 4) + col], so that should be
> a full range.

As this driver is already in the mainline and I was trying to use it...
I get various kernel crashes due to this keycodes[] allocation size bug.
Care to send a patch for this bug?

> And one more minor issue that may not be significant is the hardcoding of
> the '4' in this (row << 4).
> 
> Some of the key hits on my Littleton just worked smoothly with the above
> correction, but not all - some key hits just gave intermittent events, I
> have no idea how's that - maybe related to my scan delay and debouncing
> settings
> 
>> Thanks!
>>

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Paulius,

On Fri, Jul 17, 2009 at 1:21 PM, Paulius
Zaleckas<paulius.zaleckas@teltonika.lt> wrote:
> Eric Miao wrote:
>> Dmitry Torokhov wrote:
>>> Hi Eric,
>>>
>>> On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
>>>> Hi Dmitry,
>>>>
>>>> I've tested the driver code, and it's basically OK except for two minor
>>>> fixes:
>>>>
>>>> 1) GPIO and IRQs have to be initialized before input_register_device(),
>>>> otherwise input->open() will be invoked before that, which will schedule
>>>> an immediate scan work and fail.
>>>>
>>>> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
>>>> initialized to disabled - and will be enabled by input->open()
>>>>
>>> Could you please take a look at the driver as it is commited to
>>> 'for-linus' branch of my tree and let me know if you see anything wrong
>>> there? Otherwise I inted to submit it in my next update to Linus.
>>>
>>
>> Sorry Dmitry,
>>
>> Reply so late and til now I have some time to test this. The patch there
>> still has at least one issue I guess - the keycodes[] allocation size
>> should be MATRIX_MAX_ROWS * MATRIX_MAX_COLS if I understand correctly.
>>
>> I'm experiencing a problem of keypad->pdata being overwritten, and found
>> keycodes are actually accessed by [(row << 4) + col], so that should be
>> a full range.
>
> As this driver is already in the mainline and I was trying to use it...
> I get various kernel crashes due to this keycodes[] allocation size bug.
> Care to send a patch for this bug?
>
>> And one more minor issue that may not be significant is the hardcoding of
>> the '4' in this (row << 4).
>>
>> Some of the key hits on my Littleton just worked smoothly with the above
>> correction, but not all - some key hits just gave intermittent events, I
>> have no idea how's that - maybe related to my scan delay and debouncing
>> settings

Did you tried assigning max_keypmap_size in platform data to
MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Paulius Zaleckas]

Trilok Soni wrote:
> Hi Paulius,
> 
> On Fri, Jul 17, 2009 at 1:21 PM, Paulius
> Zaleckas<paulius.zaleckas@teltonika.lt> wrote:
>> Eric Miao wrote:
>>> Dmitry Torokhov wrote:
>>>> Hi Eric,
>>>>
>>>> On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
>>>>> Hi Dmitry,
>>>>>
>>>>> I've tested the driver code, and it's basically OK except for two minor
>>>>> fixes:
>>>>>
>>>>> 1) GPIO and IRQs have to be initialized before input_register_device(),
>>>>> otherwise input->open() will be invoked before that, which will schedule
>>>>> an immediate scan work and fail.
>>>>>
>>>>> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
>>>>> initialized to disabled - and will be enabled by input->open()
>>>>>
>>>> Could you please take a look at the driver as it is commited to
>>>> 'for-linus' branch of my tree and let me know if you see anything wrong
>>>> there? Otherwise I inted to submit it in my next update to Linus.
>>>>
>>> Sorry Dmitry,
>>>
>>> Reply so late and til now I have some time to test this. The patch there
>>> still has at least one issue I guess - the keycodes[] allocation size
>>> should be MATRIX_MAX_ROWS * MATRIX_MAX_COLS if I understand correctly.
>>>
>>> I'm experiencing a problem of keypad->pdata being overwritten, and found
>>> keycodes are actually accessed by [(row << 4) + col], so that should be
>>> a full range.
>> As this driver is already in the mainline and I was trying to use it...
>> I get various kernel crashes due to this keycodes[] allocation size bug.
>> Care to send a patch for this bug?
>>
>>> And one more minor issue that may not be significant is the hardcoding of
>>> the '4' in this (row << 4).
>>>
>>> Some of the key hits on my Littleton just worked smoothly with the above
>>> correction, but not all - some key hits just gave intermittent events, I
>>> have no idea how's that - maybe related to my scan delay and debouncing
>>> settings
> 
> Did you tried assigning max_keypmap_size in platform data to
> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
> 

Yes, this fixes crashes. But this is just workaround for bug in driver.

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Paulius,

On Fri, Jul 17, 2009 at 2:50 PM, Paulius
Zaleckas<paulius.zaleckas@teltonika.lt> wrote:
> Trilok Soni wrote:
>> Hi Paulius,
>>
>> On Fri, Jul 17, 2009 at 1:21 PM, Paulius
>> Zaleckas<paulius.zaleckas@teltonika.lt> wrote:
>>> Eric Miao wrote:
>>>> Dmitry Torokhov wrote:
>>>>> Hi Eric,
>>>>>
>>>>> On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
>>>>>> Hi Dmitry,
>>>>>>
>>>>>> I've tested the driver code, and it's basically OK except for two minor
>>>>>> fixes:
>>>>>>
>>>>>> 1) GPIO and IRQs have to be initialized before input_register_device(),
>>>>>> otherwise input->open() will be invoked before that, which will schedule
>>>>>> an immediate scan work and fail.
>>>>>>
>>>>>> 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's
>>>>>> initialized to disabled - and will be enabled by input->open()
>>>>>>
>>>>> Could you please take a look at the driver as it is commited to
>>>>> 'for-linus' branch of my tree and let me know if you see anything wrong
>>>>> there? Otherwise I inted to submit it in my next update to Linus.
>>>>>
>>>> Sorry Dmitry,
>>>>
>>>> Reply so late and til now I have some time to test this. The patch there
>>>> still has at least one issue I guess - the keycodes[] allocation size
>>>> should be MATRIX_MAX_ROWS * MATRIX_MAX_COLS if I understand correctly.
>>>>
>>>> I'm experiencing a problem of keypad->pdata being overwritten, and found
>>>> keycodes are actually accessed by [(row << 4) + col], so that should be
>>>> a full range.
>>> As this driver is already in the mainline and I was trying to use it...
>>> I get various kernel crashes due to this keycodes[] allocation size bug.
>>> Care to send a patch for this bug?
>>>
>>>> And one more minor issue that may not be significant is the hardcoding of
>>>> the '4' in this (row << 4).
>>>>
>>>> Some of the key hits on my Littleton just worked smoothly with the above
>>>> correction, but not all - some key hits just gave intermittent events, I
>>>> have no idea how's that - maybe related to my scan delay and debouncing
>>>> settings
>>
>> Did you tried assigning max_keypmap_size in platform data to
>> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
>>
>
> Yes, this fixes crashes. But this is just workaround for bug in driver.
>

As you have access to h/w, care to submit a patch which fixes this?



-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

>>> Did you tried assigning max_keypmap_size in platform data to
>>> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
>>>
>> Yes, this fixes crashes. But this is just workaround for bug in driver.
>>
> 
> As you have access to h/w, care to submit a patch which fixes this?
> 

Dmitry & Trilok,

How about this?  Due to the fact that we are not able to sort out the
proper solution for a dynamic maximum of columns/rows, let's simplify
the fix to the patch below:


>From 61ea1bd16a3636f526fb12619e84a75fa16b7f38 Mon Sep 17 00:00:00 2001
From: Eric Miao <eric.y.miao@gmail.com>
Date: Mon, 20 Jul 2009 11:31:08 +0800
Subject: [PATCH] input: matrix keymap size fixed to maximum

Introduced KEY_IDX(), merged keymap_data into 'matrix_keypad_platform_data'.

Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
---
 drivers/input/keyboard/matrix_keypad.c |   22 ++++++----------------
 include/linux/input/matrix_keypad.h    |   21 +++++----------------
 2 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index e9b2e7c..a0ba134 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -136,7 +136,7 @@ static void matrix_keypad_scan(struct work_struct *work)
 			if ((bits_changed & (1 << row)) == 0)
 				continue;
 
-			code = (row << 4) + col;
+			code = KEY_IDX(row, col);
 			input_event(input_dev, EV_MSC, MSC_SCAN, code);
 			input_report_key(input_dev,
 					 keypad->keycodes[code],
@@ -313,7 +313,6 @@ err_free_cols:
 static int __devinit matrix_keypad_probe(struct platform_device *pdev)
 {
 	const struct matrix_keypad_platform_data *pdata;
-	const struct matrix_keymap_data *keymap_data;
 	struct matrix_keypad *keypad;
 	struct input_dev *input_dev;
 	unsigned short *keycodes;
@@ -326,20 +325,13 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	keymap_data = pdata->keymap_data;
-	if (!keymap_data) {
+	if (!pdata->keymap) {
 		dev_err(&pdev->dev, "no keymap data defined\n");
 		return -EINVAL;
 	}
 
-	if (!keymap_data->max_keymap_size) {
-		dev_err(&pdev->dev, "invalid keymap data supplied\n");
-		return -EINVAL;
-	}
-
 	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
-	keycodes = kzalloc(keymap_data->max_keymap_size *
-				sizeof(keypad->keycodes),
+	keycodes = kzalloc(sizeof(keypad->keycodes) * MATRIX_MAX_KEYS,
 			   GFP_KERNEL);
 	input_dev = input_allocate_device();
 	if (!keypad || !keycodes || !input_dev) {
@@ -362,16 +354,14 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
 	input_dev->close	= matrix_keypad_stop;
 
 	input_dev->keycode	= keycodes;
-	input_dev->keycodesize	= sizeof(*keycodes);
-	input_dev->keycodemax	= keymap_data->max_keymap_size;
 
-	for (i = 0; i < keymap_data->keymap_size; i++) {
-		unsigned int key = keymap_data->keymap[i];
+	for (i = 0; i < pdata->keymap_size; i++) {
+		unsigned int key = pdata->keymap[i];
 		unsigned int row = KEY_ROW(key);
 		unsigned int col = KEY_COL(key);
 		unsigned short code = KEY_VAL(key);
 
-		keycodes[(row << 4) + col] = code;
+		keycodes[KEY_IDX(row, col)] = code;
 		__set_bit(code, input_dev->keybit);
 	}
 	__clear_bit(KEY_RESERVED, input_dev->keybit);
diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
index 7964516..59173c9 100644
--- a/include/linux/input/matrix_keypad.h
+++ b/include/linux/input/matrix_keypad.h
@@ -6,6 +6,7 @@
 
 #define MATRIX_MAX_ROWS		16
 #define MATRIX_MAX_COLS		16
+#define MATRIX_MAX_KEYS		(MATRIX_MAX_ROWS * MATRIX_MAX_COLS)
 
 #define KEY(row, col, val)	((((row) & (MATRIX_MAX_ROWS - 1)) << 24) |\
 				 (((col) & (MATRIX_MAX_COLS - 1)) << 16) |\
@@ -14,26 +15,13 @@
 #define KEY_ROW(k)		(((k) >> 24) & 0xff)
 #define KEY_COL(k)		(((k) >> 16) & 0xff)
 #define KEY_VAL(k)		((k) & 0xffff)
+#define KEY_IDX(row, col)	(((row) << 4) + (col))
 
 /**
- * struct matrix_keymap_data - keymap for matrix keyboards
+ * struct matrix_keypad_platform_data - platform-dependent keypad data
  * @keymap: pointer to array of uint32 values encoded with KEY() macro
  *	representing keymap
  * @keymap_size: number of entries (initialized) in this keymap
- * @max_keymap_size: maximum size of keymap supported by the device
- *
- * This structure is supposed to be used by platform code to supply
- * keymaps to drivers that implement matrix-like keypads/keyboards.
- */
-struct matrix_keymap_data {
-	const uint32_t *keymap;
-	unsigned int	keymap_size;
-	unsigned int	max_keymap_size;
-};
-
-/**
- * struct matrix_keypad_platform_data - platform-dependent keypad data
- * @keymap_data: pointer to &matrix_keymap_data
  * @row_gpios: array of gpio numbers reporesenting rows
  * @col_gpios: array of gpio numbers reporesenting colums
  * @num_row_gpios: actual number of row gpios used by device
@@ -46,7 +34,8 @@ struct matrix_keymap_data {
  * matrix_keypad driver to perform proper initialization.
  */
 struct matrix_keypad_platform_data {
-	const struct matrix_keymap_data *keymap_data;
+	const uint32_t *keymap;
+	unsigned int	keymap_size;
 
 	unsigned int	row_gpios[MATRIX_MAX_ROWS];
 	unsigned int	col_gpios[MATRIX_MAX_COLS];
-- 
1.6.0.4

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Trilok Soni]

Hi Eric,

On Mon, Jul 20, 2009 at 4:07 PM, Eric Miao<eric.y.miao@gmail.com> wrote:
>
>>>> Did you tried assigning max_keypmap_size in platform data to
>>>> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
>>>>
>>> Yes, this fixes crashes. But this is just workaround for bug in driver.
>>>
>>
>> As you have access to h/w, care to submit a patch which fixes this?
>>
>
> Dmitry & Trilok,
>
> How about this?  Due to the fact that we are not able to sort out the
> proper solution for a dynamic maximum of columns/rows, let's simplify
> the fix to the patch below:
>
>
> From 61ea1bd16a3636f526fb12619e84a75fa16b7f38 Mon Sep 17 00:00:00 2001
> From: Eric Miao <eric.y.miao@gmail.com>
> Date: Mon, 20 Jul 2009 11:31:08 +0800
> Subject: [PATCH] input: matrix keymap size fixed to maximum
>
> Introduced KEY_IDX(), merged keymap_data into 'matrix_keypad_platform_data'.
>
> Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
> ---
>  drivers/input/keyboard/matrix_keypad.c |   22 ++++++----------------
>  include/linux/input/matrix_keypad.h    |   21 +++++----------------
>  2 files changed, 11 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
> index e9b2e7c..a0ba134 100644
> --- a/drivers/input/keyboard/matrix_keypad.c
> +++ b/drivers/input/keyboard/matrix_keypad.c
> @@ -136,7 +136,7 @@ static void matrix_keypad_scan(struct work_struct *work)
>                        if ((bits_changed & (1 << row)) == 0)
>                                continue;
>
> -                       code = (row << 4) + col;
> +                       code = KEY_IDX(row, col);
>                        input_event(input_dev, EV_MSC, MSC_SCAN, code);
>                        input_report_key(input_dev,
>                                         keypad->keycodes[code],
> @@ -313,7 +313,6 @@ err_free_cols:
>  static int __devinit matrix_keypad_probe(struct platform_device *pdev)
>  {
>        const struct matrix_keypad_platform_data *pdata;
> -       const struct matrix_keymap_data *keymap_data;
>        struct matrix_keypad *keypad;
>        struct input_dev *input_dev;
>        unsigned short *keycodes;
> @@ -326,20 +325,13 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
>                return -EINVAL;
>        }
>
> -       keymap_data = pdata->keymap_data;
> -       if (!keymap_data) {
> +       if (!pdata->keymap) {
>                dev_err(&pdev->dev, "no keymap data defined\n");
>                return -EINVAL;
>        }
>
> -       if (!keymap_data->max_keymap_size) {
> -               dev_err(&pdev->dev, "invalid keymap data supplied\n");
> -               return -EINVAL;
> -       }
> -
>        keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
> -       keycodes = kzalloc(keymap_data->max_keymap_size *
> -                               sizeof(keypad->keycodes),
> +       keycodes = kzalloc(sizeof(keypad->keycodes) * MATRIX_MAX_KEYS,
>                           GFP_KERNEL);
>        input_dev = input_allocate_device();
>        if (!keypad || !keycodes || !input_dev) {
> @@ -362,16 +354,14 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
>        input_dev->close        = matrix_keypad_stop;
>
>        input_dev->keycode      = keycodes;
> -       input_dev->keycodesize  = sizeof(*keycodes);
> -       input_dev->keycodemax   = keymap_data->max_keymap_size;
>
> -       for (i = 0; i < keymap_data->keymap_size; i++) {
> -               unsigned int key = keymap_data->keymap[i];
> +       for (i = 0; i < pdata->keymap_size; i++) {
> +               unsigned int key = pdata->keymap[i];
>                unsigned int row = KEY_ROW(key);
>                unsigned int col = KEY_COL(key);
>                unsigned short code = KEY_VAL(key);
>
> -               keycodes[(row << 4) + col] = code;
> +               keycodes[KEY_IDX(row, col)] = code;


How about just doing like this

keycodes[(row << ((fls(MATRIX_MAX_COLS) - 1))) + col] ?

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

Trilok Soni wrote:
> Hi Eric,
> 
> On Mon, Jul 20, 2009 at 4:07 PM, Eric Miao<eric.y.miao@gmail.com> wrote:
>>>>> Did you tried assigning max_keypmap_size in platform data to
>>>>> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
>>>>>
>>>> Yes, this fixes crashes. But this is just workaround for bug in driver.
>>>>
>>> As you have access to h/w, care to submit a patch which fixes this?
>>>
>> Dmitry & Trilok,
>>
>> How about this?  Due to the fact that we are not able to sort out the
>> proper solution for a dynamic maximum of columns/rows, let's simplify
>> the fix to the patch below:
>>
>>
>> From 61ea1bd16a3636f526fb12619e84a75fa16b7f38 Mon Sep 17 00:00:00 2001
>> From: Eric Miao <eric.y.miao@gmail.com>
>> Date: Mon, 20 Jul 2009 11:31:08 +0800
>> Subject: [PATCH] input: matrix keymap size fixed to maximum
>>
>> Introduced KEY_IDX(), merged keymap_data into 'matrix_keypad_platform_data'.
>>
>> Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
>> ---
>>  drivers/input/keyboard/matrix_keypad.c |   22 ++++++----------------
>>  include/linux/input/matrix_keypad.h    |   21 +++++----------------
>>  2 files changed, 11 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
>> index e9b2e7c..a0ba134 100644
>> --- a/drivers/input/keyboard/matrix_keypad.c
>> +++ b/drivers/input/keyboard/matrix_keypad.c
>> @@ -136,7 +136,7 @@ static void matrix_keypad_scan(struct work_struct *work)
>>                        if ((bits_changed & (1 << row)) == 0)
>>                                continue;
>>
>> -                       code = (row << 4) + col;
>> +                       code = KEY_IDX(row, col);
>>                        input_event(input_dev, EV_MSC, MSC_SCAN, code);
>>                        input_report_key(input_dev,
>>                                         keypad->keycodes[code],
>> @@ -313,7 +313,6 @@ err_free_cols:
>>  static int __devinit matrix_keypad_probe(struct platform_device *pdev)
>>  {
>>        const struct matrix_keypad_platform_data *pdata;
>> -       const struct matrix_keymap_data *keymap_data;
>>        struct matrix_keypad *keypad;
>>        struct input_dev *input_dev;
>>        unsigned short *keycodes;
>> @@ -326,20 +325,13 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
>>                return -EINVAL;
>>        }
>>
>> -       keymap_data = pdata->keymap_data;
>> -       if (!keymap_data) {
>> +       if (!pdata->keymap) {
>>                dev_err(&pdev->dev, "no keymap data defined\n");
>>                return -EINVAL;
>>        }
>>
>> -       if (!keymap_data->max_keymap_size) {
>> -               dev_err(&pdev->dev, "invalid keymap data supplied\n");
>> -               return -EINVAL;
>> -       }
>> -
>>        keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
>> -       keycodes = kzalloc(keymap_data->max_keymap_size *
>> -                               sizeof(keypad->keycodes),
>> +       keycodes = kzalloc(sizeof(keypad->keycodes) * MATRIX_MAX_KEYS,
>>                           GFP_KERNEL);
>>        input_dev = input_allocate_device();
>>        if (!keypad || !keycodes || !input_dev) {
>> @@ -362,16 +354,14 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
>>        input_dev->close        = matrix_keypad_stop;
>>
>>        input_dev->keycode      = keycodes;
>> -       input_dev->keycodesize  = sizeof(*keycodes);
>> -       input_dev->keycodemax   = keymap_data->max_keymap_size;
>>
>> -       for (i = 0; i < keymap_data->keymap_size; i++) {
>> -               unsigned int key = keymap_data->keymap[i];
>> +       for (i = 0; i < pdata->keymap_size; i++) {
>> +               unsigned int key = pdata->keymap[i];
>>                unsigned int row = KEY_ROW(key);
>>                unsigned int col = KEY_COL(key);
>>                unsigned short code = KEY_VAL(key);
>>
>> -               keycodes[(row << 4) + col] = code;
>> +               keycodes[KEY_IDX(row, col)] = code;
> 
> 
> How about just doing like this
> 
> keycodes[(row << ((fls(MATRIX_MAX_COLS) - 1))) + col] ?
> 

I doubt this will work correctly if MATRIX_MAX_COLS isn't something 2 ^ N

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Dmitry Torokhov]

Hi Eric,

On Mon, Jul 20, 2009 at 06:37:04PM +0800, Eric Miao wrote:
> 
> >>> Did you tried assigning max_keypmap_size in platform data to
> >>> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
> >>>
> >> Yes, this fixes crashes. But this is just workaround for bug in driver.
> >>
> > 
> > As you have access to h/w, care to submit a patch which fixes this?
> > 
> 
> Dmitry & Trilok,
> 
> How about this?  Due to the fact that we are not able to sort out the
> proper solution for a dynamic maximum of columns/rows, let's simplify
> the fix to the patch below:
> 
> 
> From 61ea1bd16a3636f526fb12619e84a75fa16b7f38 Mon Sep 17 00:00:00 2001
> From: Eric Miao <eric.y.miao@gmail.com>
> Date: Mon, 20 Jul 2009 11:31:08 +0800
> Subject: [PATCH] input: matrix keymap size fixed to maximum
> 
> Introduced KEY_IDX(), merged keymap_data into 'matrix_keypad_platform_data'.
> 

I would like to keep definitions in matrix_keymap.h useable to other
drivers so we either make KEY_IDX() work with different number of
columns or drop it. I would also like to keep keymap data separate so
drivers that don't use matrix encoding could still use it.

Overall, I don't quite understand what the problem with the current
drive is since it works fine as long as we set up max_keymap_size
properly. We could improve diagnostic by checking row and cols values
and warning users when they supply suspicious data and maybe adjust the
documentation, right?

-- 
Dmitry

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

Dmitry Torokhov wrote:
> Hi Eric,
> 
> On Mon, Jul 20, 2009 at 06:37:04PM +0800, Eric Miao wrote:
>>>>> Did you tried assigning max_keypmap_size in platform data to
>>>>> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
>>>>>
>>>> Yes, this fixes crashes. But this is just workaround for bug in driver.
>>>>
>>> As you have access to h/w, care to submit a patch which fixes this?
>>>
>> Dmitry & Trilok,
>>
>> How about this?  Due to the fact that we are not able to sort out the
>> proper solution for a dynamic maximum of columns/rows, let's simplify
>> the fix to the patch below:
>>
>>
>> From 61ea1bd16a3636f526fb12619e84a75fa16b7f38 Mon Sep 17 00:00:00 2001
>> From: Eric Miao <eric.y.miao@gmail.com>
>> Date: Mon, 20 Jul 2009 11:31:08 +0800
>> Subject: [PATCH] input: matrix keymap size fixed to maximum
>>
>> Introduced KEY_IDX(), merged keymap_data into 'matrix_keypad_platform_data'.
>>
> 
> I would like to keep definitions in matrix_keymap.h useable to other
> drivers so we either make KEY_IDX() work with different number of
> columns or drop it.

What about:

#define KEY_IDX(row, col)	(((row) * keypad->num_columns) + (col))

if we want to make it dynamic.

> I would also like to keep keymap data separate so
> drivers that don't use matrix encoding could still use it.
> 

Do you mean there are possibilities that some drivers are not going to
define any matrix keycodes, and depend on EV_MSC to know the position
happened? That way, we may want to omit the ->keycodes[] accesses.

> Overall, I don't quite understand what the problem with the current
> drive is since it works fine as long as we set up max_keymap_size
> properly.

I think the root of this problem lies in the code below:

                        code = (row << 4) + col;
                        input_event(input_dev, EV_MSC, MSC_SCAN, code);
                        input_report_key(input_dev,
                                         keypad->keycodes[code],
                                         new_state[col] & (1 << row));

that 'code = (row << 4) + col;' is hardcoded to shift left by '4', and
is then used to index into keypad->keycodes[] array, the size of which
in turn is specified by 'max_keymap_size'. This is a bit inconsistent.

If written as (row << 4) + col, it means the max_keymap_size should be
setup as 'max_rows * 16', instead of expected 'max_rows * max_cols'.

And there seems to be a typo in the allocation:

        keycodes = kzalloc(keymap_data->max_keymap_size *
                                sizeof(keypad->keycodes),
                           GFP_KERNEL);

that, 'sizeof(keypad->keycodes)' should be written as
'sizeof(keypad->keycodes[0])' if I guess it correct.

> We could improve diagnostic by checking row and cols values
> and warning users when they supply suspicious data and maybe adjust the
> documentation, right?
> 

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Dmitry Torokhov]

On Tue, Jul 21, 2009 at 04:26:47PM +0800, Eric Miao wrote:
> Dmitry Torokhov wrote:
> > Hi Eric,
> > 
> > On Mon, Jul 20, 2009 at 06:37:04PM +0800, Eric Miao wrote:
> >>>>> Did you tried assigning max_keypmap_size in platform data to
> >>>>> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
> >>>>>
> >>>> Yes, this fixes crashes. But this is just workaround for bug in driver.
> >>>>
> >>> As you have access to h/w, care to submit a patch which fixes this?
> >>>
> >> Dmitry & Trilok,
> >>
> >> How about this?  Due to the fact that we are not able to sort out the
> >> proper solution for a dynamic maximum of columns/rows, let's simplify
> >> the fix to the patch below:
> >>
> >>
> >> From 61ea1bd16a3636f526fb12619e84a75fa16b7f38 Mon Sep 17 00:00:00 2001
> >> From: Eric Miao <eric.y.miao@gmail.com>
> >> Date: Mon, 20 Jul 2009 11:31:08 +0800
> >> Subject: [PATCH] input: matrix keymap size fixed to maximum
> >>
> >> Introduced KEY_IDX(), merged keymap_data into 'matrix_keypad_platform_data'.
> >>
> > 
> > I would like to keep definitions in matrix_keymap.h useable to other
> > drivers so we either make KEY_IDX() work with different number of
> > columns or drop it.
> 
> What about:
> 
> #define KEY_IDX(row, col)	(((row) * keypad->num_columns) + (col))
> 
> if we want to make it dynamic.
> 

I'd rather not have any references to particular data structures there,
so something like KEY_IDX(row, col, shift) or KEY_IDX(row, col, maxcol).
Would that work for you?


> > I would also like to keep keymap data separate so
> > drivers that don't use matrix encoding could still use it.
> > 
> 
> Do you mean there are possibilities that some drivers are not going to
> define any matrix keycodes, and depend on EV_MSC to know the position
> happened? That way, we may want to omit the ->keycodes[] accesses.
> 

Poorly spoken on my part. I should have said "not use entire structure
from the matrix_keymap.h" but only the keymap part.

> > Overall, I don't quite understand what the problem with the current
> > drive is since it works fine as long as we set up max_keymap_size
> > properly.
> 
> I think the root of this problem lies in the code below:
> 
>                         code = (row << 4) + col;
>                         input_event(input_dev, EV_MSC, MSC_SCAN, code);
>                         input_report_key(input_dev,
>                                          keypad->keycodes[code],
>                                          new_state[col] & (1 << row));
> 
> that 'code = (row << 4) + col;' is hardcoded to shift left by '4', and
> is then used to index into keypad->keycodes[] array, the size of which
> in turn is specified by 'max_keymap_size'. This is a bit inconsistent.

Right, I agree.

> 
> If written as (row << 4) + col, it means the max_keymap_size should be
> setup as 'max_rows * 16', instead of expected 'max_rows * max_cols'.
> 

Yes, at the moment... I guess we need to put the true dimensions instead
of max size into the keymap data, right? Then we'd be able to calculate
proper shift value.

> And there seems to be a typo in the allocation:
> 
>         keycodes = kzalloc(keymap_data->max_keymap_size *
>                                 sizeof(keypad->keycodes),
>                            GFP_KERNEL);
> 
> that, 'sizeof(keypad->keycodes)' should be written as
> 'sizeof(keypad->keycodes[0])' if I guess it correct.

It should not hurt anything (since sizeof(keypad->keycodes) is 4 bytes)
but indeed I need to fix that.

> 
> > We could improve diagnostic by checking row and cols values
> > and warning users when they supply suspicious data and maybe adjust the
> > documentation, right?
> > 
> 

-- 
Dmitry

Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Eric Miao]

Dmitry Torokhov wrote:
> On Tue, Jul 21, 2009 at 04:26:47PM +0800, Eric Miao wrote:
>> Dmitry Torokhov wrote:
>>> Hi Eric,
>>>
>>> On Mon, Jul 20, 2009 at 06:37:04PM +0800, Eric Miao wrote:
>>>>>>> Did you tried assigning max_keypmap_size in platform data to
>>>>>>> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
>>>>>>>
>>>>>> Yes, this fixes crashes. But this is just workaround for bug in driver.
>>>>>>
>>>>> As you have access to h/w, care to submit a patch which fixes this?
>>>>>
>>>> Dmitry & Trilok,
>>>>
>>>> How about this?  Due to the fact that we are not able to sort out the
>>>> proper solution for a dynamic maximum of columns/rows, let's simplify
>>>> the fix to the patch below:
>>>>
>>>>
>>>> From 61ea1bd16a3636f526fb12619e84a75fa16b7f38 Mon Sep 17 00:00:00 2001
>>>> From: Eric Miao <eric.y.miao@gmail.com>
>>>> Date: Mon, 20 Jul 2009 11:31:08 +0800
>>>> Subject: [PATCH] input: matrix keymap size fixed to maximum
>>>>
>>>> Introduced KEY_IDX(), merged keymap_data into 'matrix_keypad_platform_data'.
>>>>
>>> I would like to keep definitions in matrix_keymap.h useable to other
>>> drivers so we either make KEY_IDX() work with different number of
>>> columns or drop it.
>> What about:
>>
>> #define KEY_IDX(row, col)	(((row) * keypad->num_columns) + (col))
>>
>> if we want to make it dynamic.
>>
> 
> I'd rather not have any references to particular data structures there,
> so something like KEY_IDX(row, col, shift) or KEY_IDX(row, col, maxcol).
> Would that work for you?
> 
> 
>>> I would also like to keep keymap data separate so
>>> drivers that don't use matrix encoding could still use it.
>>>
>> Do you mean there are possibilities that some drivers are not going to
>> define any matrix keycodes, and depend on EV_MSC to know the position
>> happened? That way, we may want to omit the ->keycodes[] accesses.
>>
> 
> Poorly spoken on my part. I should have said "not use entire structure
> from the matrix_keymap.h" but only the keymap part.
> 
>>> Overall, I don't quite understand what the problem with the current
>>> drive is since it works fine as long as we set up max_keymap_size
>>> properly.
>> I think the root of this problem lies in the code below:
>>
>>                         code = (row << 4) + col;
>>                         input_event(input_dev, EV_MSC, MSC_SCAN, code);
>>                         input_report_key(input_dev,
>>                                          keypad->keycodes[code],
>>                                          new_state[col] & (1 << row));
>>
>> that 'code = (row << 4) + col;' is hardcoded to shift left by '4', and
>> is then used to index into keypad->keycodes[] array, the size of which
>> in turn is specified by 'max_keymap_size'. This is a bit inconsistent.
> 
> Right, I agree.
> 
>> If written as (row << 4) + col, it means the max_keymap_size should be
>> setup as 'max_rows * 16', instead of expected 'max_rows * max_cols'.
>>
> 
> Yes, at the moment... I guess we need to put the true dimensions instead
> of max size into the keymap data, right? Then we'd be able to calculate
> proper shift value.
> 
>> And there seems to be a typo in the allocation:
>>
>>         keycodes = kzalloc(keymap_data->max_keymap_size *
>>                                 sizeof(keypad->keycodes),
>>                            GFP_KERNEL);
>>
>> that, 'sizeof(keypad->keycodes)' should be written as
>> 'sizeof(keypad->keycodes[0])' if I guess it correct.
> 
> It should not hurt anything (since sizeof(keypad->keycodes) is 4 bytes)
> but indeed I need to fix that.
> 
>>> We could improve diagnostic by checking row and cols values
>>> and warning users when they supply suspicious data and maybe adjust the
>>> documentation, right?
>>>
> 

OK, then how about this one?

>From be59051b471dc87a0cd846630dd9964602b310f6 Mon Sep 17 00:00:00 2001
From: Eric Miao <eric.y.miao@gmail.com>
Date: Mon, 20 Jul 2009 11:31:08 +0800
Subject: [PATCH] input: make matrix keymap size dynamic

The number of rows and columns should really belong to 'struct keymap_data',
and assumption on the shift and size of rows/columns removed.

Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
---
 drivers/input/keyboard/matrix_keypad.c |   85 +++++++++++++++++---------------
 include/linux/input/matrix_keypad.h    |    9 ++--
 2 files changed, 49 insertions(+), 45 deletions(-)

diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index e9b2e7c..aa9442a 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -26,6 +26,8 @@
 struct matrix_keypad {
 	const struct matrix_keypad_platform_data *pdata;
 	struct input_dev *input_dev;
+	int num_rows;
+	int num_cols;
 	unsigned short *keycodes;
 
 	uint32_t last_key_state[MATRIX_MAX_COLS];
@@ -35,14 +37,16 @@ struct matrix_keypad {
 	spinlock_t lock;
 };
 
+#define KEY_IDX(kp, row, col)	(((row) * (kp)->num_cols) + col)
+
 /*
  * NOTE: normally the GPIO has to be put into HiZ when de-activated to cause
  * minmal side effect when scanning other columns, here it is configured to
  * be input, and it should work on most platforms.
  */
-static void __activate_col(const struct matrix_keypad_platform_data *pdata,
-			   int col, bool on)
+static void __activate_col(struct matrix_keypad *keypad, int col, bool on)
 {
+	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
 	bool level_on = !pdata->active_low;
 
 	if (on) {
@@ -53,22 +57,20 @@ static void __activate_col(const struct matrix_keypad_platform_data *pdata,
 	}
 }
 
-static void activate_col(const struct matrix_keypad_platform_data *pdata,
-			 int col, bool on)
+static void activate_col(struct matrix_keypad *keypad, int col, bool on)
 {
-	__activate_col(pdata, col, on);
+	__activate_col(keypad, col, on);
 
-	if (on && pdata->col_scan_delay_us)
-		udelay(pdata->col_scan_delay_us);
+	if (on && keypad->pdata->col_scan_delay_us)
+		udelay(keypad->pdata->col_scan_delay_us);
 }
 
-static void activate_all_cols(const struct matrix_keypad_platform_data *pdata,
-			      bool on)
+static void activate_all_cols(struct matrix_keypad *keypad, bool on)
 {
 	int col;
 
-	for (col = 0; col < pdata->num_col_gpios; col++)
-		__activate_col(pdata, col, on);
+	for (col = 0; col < keypad->num_cols; col++)
+		__activate_col(keypad, col, on);
 }
 
 static bool row_asserted(const struct matrix_keypad_platform_data *pdata,
@@ -83,7 +85,7 @@ static void enable_row_irqs(struct matrix_keypad *keypad)
 	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
 	int i;
 
-	for (i = 0; i < pdata->num_row_gpios; i++)
+	for (i = 0; i < keypad->num_rows; i++)
 		enable_irq(gpio_to_irq(pdata->row_gpios[i]));
 }
 
@@ -92,7 +94,7 @@ static void disable_row_irqs(struct matrix_keypad *keypad)
 	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
 	int i;
 
-	for (i = 0; i < pdata->num_row_gpios; i++)
+	for (i = 0; i < keypad->num_rows; i++)
 		disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
 }
 
@@ -109,34 +111,34 @@ static void matrix_keypad_scan(struct work_struct *work)
 	int row, col, code;
 
 	/* de-activate all columns for scanning */
-	activate_all_cols(pdata, false);
+	activate_all_cols(keypad, false);
 
 	memset(new_state, 0, sizeof(new_state));
 
 	/* assert each column and read the row status out */
-	for (col = 0; col < pdata->num_col_gpios; col++) {
+	for (col = 0; col < keypad->num_cols; col++) {
 
-		activate_col(pdata, col, true);
+		activate_col(keypad, col, true);
 
-		for (row = 0; row < pdata->num_row_gpios; row++)
+		for (row = 0; row < keypad->num_rows; row++)
 			new_state[col] |=
 				row_asserted(pdata, row) ? (1 << row) : 0;
 
-		activate_col(pdata, col, false);
+		activate_col(keypad, col, false);
 	}
 
-	for (col = 0; col < pdata->num_col_gpios; col++) {
+	for (col = 0; col < keypad->num_cols; col++) {
 		uint32_t bits_changed;
 
 		bits_changed = keypad->last_key_state[col] ^ new_state[col];
 		if (bits_changed == 0)
 			continue;
 
-		for (row = 0; row < pdata->num_row_gpios; row++) {
+		for (row = 0; row < keypad->num_rows; row++) {
 			if ((bits_changed & (1 << row)) == 0)
 				continue;
 
-			code = (row << 4) + col;
+			code = KEY_IDX(keypad, row, col);
 			input_event(input_dev, EV_MSC, MSC_SCAN, code);
 			input_report_key(input_dev,
 					 keypad->keycodes[code],
@@ -147,7 +149,7 @@ static void matrix_keypad_scan(struct work_struct *work)
 
 	memcpy(keypad->last_key_state, new_state, sizeof(new_state));
 
-	activate_all_cols(pdata, true);
+	activate_all_cols(keypad, true);
 
 	/* Enable IRQs again */
 	spin_lock_irq(&keypad->lock);
@@ -221,7 +223,7 @@ static int matrix_keypad_suspend(struct platform_device *pdev, pm_message_t stat
 	matrix_keypad_stop(keypad->input_dev);
 
 	if (device_may_wakeup(&pdev->dev))
-		for (i = 0; i < pdata->num_row_gpios; i++)
+		for (i = 0; i < keypad->num_rows; i++)
 			enable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
 
 	return 0;
@@ -234,7 +236,7 @@ static int matrix_keypad_resume(struct platform_device *pdev)
 	int i;
 
 	if (device_may_wakeup(&pdev->dev))
-		for (i = 0; i < pdata->num_row_gpios; i++)
+		for (i = 0; i < keypad->num_rows; i++)
 			disable_irq_wake(gpio_to_irq(pdata->row_gpios[i]));
 
 	matrix_keypad_start(keypad->input_dev);
@@ -253,7 +255,7 @@ static int __devinit init_matrix_gpio(struct platform_device *pdev,
 	int i, err = -EINVAL;
 
 	/* initialized strobe lines as outputs, activated */
-	for (i = 0; i < pdata->num_col_gpios; i++) {
+	for (i = 0; i < keypad->num_cols; i++) {
 		err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
 		if (err) {
 			dev_err(&pdev->dev,
@@ -265,7 +267,7 @@ static int __devinit init_matrix_gpio(struct platform_device *pdev,
 		gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
 	}
 
-	for (i = 0; i < pdata->num_row_gpios; i++) {
+	for (i = 0; i < keypad->num_rows; i++) {
 		err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
 		if (err) {
 			dev_err(&pdev->dev,
@@ -277,7 +279,7 @@ static int __devinit init_matrix_gpio(struct platform_device *pdev,
 		gpio_direction_input(pdata->row_gpios[i]);
 	}
 
-	for (i = 0; i < pdata->num_row_gpios; i++) {
+	for (i = 0; i < keypad->num_rows; i++) {
 		err = request_irq(gpio_to_irq(pdata->row_gpios[i]),
 				matrix_keypad_interrupt,
 				IRQF_DISABLED |
@@ -298,11 +300,11 @@ static int __devinit init_matrix_gpio(struct platform_device *pdev,
 err_free_irqs:
 	while (--i >= 0)
 		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
-	i = pdata->num_row_gpios;
+	i = keypad->num_rows;
 err_free_rows:
 	while (--i >= 0)
 		gpio_free(pdata->row_gpios[i]);
-	i = pdata->num_col_gpios;
+	i = keypad->num_cols;
 err_free_cols:
 	while (--i >= 0)
 		gpio_free(pdata->col_gpios[i]);
@@ -317,7 +319,7 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
 	struct matrix_keypad *keypad;
 	struct input_dev *input_dev;
 	unsigned short *keycodes;
-	int i;
+	int i, num_rows, num_cols;
 	int err;
 
 	pdata = pdev->dev.platform_data;
@@ -332,14 +334,17 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	if (!keymap_data->max_keymap_size) {
+	if (!keymap_data->keymap_rows || !keymap_data->keymap_cols) {
 		dev_err(&pdev->dev, "invalid keymap data supplied\n");
 		return -EINVAL;
 	}
 
 	keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
-	keycodes = kzalloc(keymap_data->max_keymap_size *
-				sizeof(keypad->keycodes),
+
+	num_rows = pdata->keymap_data->keymap_rows;
+	num_cols = pdata->keymap_data->keymap_cols;
+	keycodes = kzalloc(num_rows * num_cols *
+				sizeof(keypad->keycodes[0]),
 			   GFP_KERNEL);
 	input_dev = input_allocate_device();
 	if (!keypad || !keycodes || !input_dev) {
@@ -349,6 +354,8 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
 
 	keypad->input_dev = input_dev;
 	keypad->pdata = pdata;
+	keypad->num_rows = num_rows;
+	keypad->num_cols = num_cols;
 	keypad->keycodes = keycodes;
 	keypad->stopped = true;
 	INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
@@ -362,16 +369,14 @@ static int __devinit matrix_keypad_probe(struct platform_device *pdev)
 	input_dev->close	= matrix_keypad_stop;
 
 	input_dev->keycode	= keycodes;
-	input_dev->keycodesize	= sizeof(*keycodes);
-	input_dev->keycodemax	= keymap_data->max_keymap_size;
 
-	for (i = 0; i < keymap_data->keymap_size; i++) {
-		unsigned int key = keymap_data->keymap[i];
+	for (i = 0; i < pdata->keymap_data->keymap_size; i++) {
+		unsigned int key = pdata->keymap_data->keymap[i];
 		unsigned int row = KEY_ROW(key);
 		unsigned int col = KEY_COL(key);
 		unsigned short code = KEY_VAL(key);
 
-		keycodes[(row << 4) + col] = code;
+		keypad->keycodes[KEY_IDX(keypad, row, col)] = code;
 		__set_bit(code, input_dev->keybit);
 	}
 	__clear_bit(KEY_RESERVED, input_dev->keybit);
@@ -407,12 +412,12 @@ static int __devexit matrix_keypad_remove(struct platform_device *pdev)
 
 	device_init_wakeup(&pdev->dev, 0);
 
-	for (i = 0; i < pdata->num_row_gpios; i++) {
+	for (i = 0; i < keypad->num_rows; i++) {
 		free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
 		gpio_free(pdata->row_gpios[i]);
 	}
 
-	for (i = 0; i < pdata->num_col_gpios; i++)
+	for (i = 0; i < keypad->num_cols; i++)
 		gpio_free(pdata->col_gpios[i]);
 
 	input_unregister_device(keypad->input_dev);
diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
index 7964516..46cb3db 100644
--- a/include/linux/input/matrix_keypad.h
+++ b/include/linux/input/matrix_keypad.h
@@ -27,8 +27,9 @@
  */
 struct matrix_keymap_data {
 	const uint32_t *keymap;
+	unsigned int	keymap_rows;
+	unsigned int	keymap_cols;
 	unsigned int	keymap_size;
-	unsigned int	max_keymap_size;
 };
 
 /**
@@ -48,10 +49,8 @@ struct matrix_keymap_data {
 struct matrix_keypad_platform_data {
 	const struct matrix_keymap_data *keymap_data;
 
-	unsigned int	row_gpios[MATRIX_MAX_ROWS];
-	unsigned int	col_gpios[MATRIX_MAX_COLS];
-	unsigned int	num_row_gpios;
-	unsigned int	num_col_gpios;
+	const int	*row_gpios;
+	const int	*col_gpios;
 
 	unsigned int	col_scan_delay_us;
 
-- 
1.6.0.4