[PATCH 09/18] input: add input driver for polled GPIO buttons

[PATCH 09/18] input: add input driver for polled GPIO buttons

[Gabor Juhos]

The existing gpio-keys driver can be usable only for GPIO lines with
interrupt support. Several devices have buttons connected to a GPIO
line which is not capable to generate interrupts. This patch adds a
new input driver using the generic GPIO layer and the input-polldev
to support such buttons.

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: linux-input@vger.kernel.org
---

Changes since RFC: ---

 drivers/input/misc/Kconfig        |   16 +++
 drivers/input/misc/Makefile       |    1 +
 drivers/input/misc/gpio_buttons.c |  232 +++++++++++++++++++++++++++++++++++++
 include/linux/gpio_buttons.h      |   33 +++++
 4 files changed, 282 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/misc/gpio_buttons.c
 create mode 100644 include/linux/gpio_buttons.h

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b99b8cb..3439b79 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -448,4 +448,20 @@ config INPUT_ADXL34X_SPI
 	  To compile this driver as a module, choose M here: the
 	  module will be called adxl34x-spi.
 
+config INPUT_GPIO_BUTTONS
+	tristate "Polled GPIO buttons interface"
+	depends on GENERIC_GPIO
+	select INPUT_POLLDEV
+	help
+	  This driver implements support for buttons connected
+	  to GPIO pins of various CPUs (and some other chips).
+
+	  Say Y here if your device has buttons connected
+	  directly to such GPIO pins.  Your board-specific
+	  setup logic must also provide a platform device,
+	  with configuration data saying which GPIOs are used.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called gpio-buttons.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 1fe1f6c..3791050 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -42,4 +42,5 @@ obj-$(CONFIG_INPUT_WINBOND_CIR)		+= winbond-cir.o
 obj-$(CONFIG_INPUT_WISTRON_BTNS)	+= wistron_btns.o
 obj-$(CONFIG_INPUT_WM831X_ON)		+= wm831x-on.o
 obj-$(CONFIG_INPUT_YEALINK)		+= yealink.o
+obj-$(CONFIG_INPUT_GPIO_BUTTONS)	+= gpio_buttons.o
 
diff --git a/drivers/input/misc/gpio_buttons.c b/drivers/input/misc/gpio_buttons.c
new file mode 100644
index 0000000..51288a3
--- /dev/null
+++ b/drivers/input/misc/gpio_buttons.c
@@ -0,0 +1,232 @@
+/*
+ *  Driver for buttons on GPIO lines not capable of generating interrupts
+ *
+ *  Copyright (C) 2007-2010 Gabor Juhos <juhosg@openwrt.org>
+ *  Copyright (C) 2010 Nuno Goncalves <nunojpg@gmail.com>
+ *
+ *  This file was based on: /drivers/input/misc/cobalt_btns.c
+ *	Copyright (C) 2007 Yoichi Yuasa <yoichi_yuasa@tripeaks.co.jp>
+ *
+ *  also was based on: /drivers/input/keyboard/gpio_keys.c
+ *	Copyright 2005 Phil Blundell
+ *
+ *  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/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/input.h>
+#include <linux/input-polldev.h>
+#include <linux/ioport.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/gpio_buttons.h>
+
+#define DRV_NAME	"gpio-buttons"
+
+struct gpio_button_data {
+	int last_state;
+	int count;
+	int can_sleep;
+};
+
+struct gpio_buttons_dev {
+	struct input_polled_dev *poll_dev;
+	struct gpio_buttons_platform_data *pdata;
+	struct gpio_button_data *data;
+};
+
+static void gpio_buttons_check_state(struct input_dev *input,
+				      struct gpio_button *button,
+				      struct gpio_button_data *bdata)
+{
+	int state;
+
+	if (bdata->can_sleep)
+		state = !!gpio_get_value_cansleep(button->gpio);
+	else
+		state = !!gpio_get_value(button->gpio);
+
+	if (state != bdata->last_state) {
+		unsigned int type = button->type ?: EV_KEY;
+
+		input_event(input, type, button->code,
+			    !!(state ^ button->active_low));
+		input_sync(input);
+		bdata->count = 0;
+		bdata->last_state = state;
+	}
+}
+
+static void gpio_buttons_poll(struct input_polled_dev *dev)
+{
+	struct gpio_buttons_dev *bdev = dev->private;
+	struct gpio_buttons_platform_data *pdata = bdev->pdata;
+	struct input_dev *input = dev->input;
+	int i;
+
+	for (i = 0; i < bdev->pdata->nbuttons; i++) {
+		struct gpio_button *button = &pdata->buttons[i];
+		struct gpio_button_data *bdata = &bdev->data[i];
+
+		if (bdata->count < button->threshold)
+			bdata->count++;
+		else
+			gpio_buttons_check_state(input, button, bdata);
+
+	}
+}
+
+static int __devinit gpio_buttons_probe(struct platform_device *pdev)
+{
+	struct gpio_buttons_platform_data *pdata = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	struct gpio_buttons_dev *bdev;
+	struct input_polled_dev *poll_dev;
+	struct input_dev *input;
+	int error;
+	int i;
+
+	if (!pdata)
+		return -ENXIO;
+
+	bdev = kzalloc(sizeof(struct gpio_buttons_dev) +
+		       pdata->nbuttons * sizeof(struct gpio_button_data),
+		       GFP_KERNEL);
+	if (!bdev) {
+		dev_err(dev, "no memory for private data\n");
+		return -ENOMEM;
+	}
+
+	bdev->data = (struct gpio_button_data *) &bdev[1];
+
+	poll_dev = input_allocate_polled_device();
+	if (!poll_dev) {
+		dev_err(dev, "no memory for polled device\n");
+		error = -ENOMEM;
+		goto err_free_bdev;
+	}
+
+	poll_dev->private = bdev;
+	poll_dev->poll = gpio_buttons_poll;
+	poll_dev->poll_interval = pdata->poll_interval;
+
+	input = poll_dev->input;
+
+	input->evbit[0] = BIT(EV_KEY);
+	input->name = pdev->name;
+	input->phys = "gpio-buttons/input0";
+	input->dev.parent = &pdev->dev;
+
+	input->id.bustype = BUS_HOST;
+	input->id.vendor = 0x0001;
+	input->id.product = 0x0001;
+	input->id.version = 0x0100;
+
+	for (i = 0; i < pdata->nbuttons; i++) {
+		struct gpio_button *button = &pdata->buttons[i];
+		unsigned int gpio = button->gpio;
+		unsigned int type = button->type ?: EV_KEY;
+
+		error = gpio_request(gpio,
+				     button->desc ? button->desc : DRV_NAME);
+		if (error) {
+			dev_err(dev, "unable to claim gpio %u, err=%d\n",
+				gpio, error);
+			goto err_free_gpio;
+		}
+
+		error = gpio_direction_input(gpio);
+		if (error) {
+			dev_err(dev,
+				"unable to set direction on gpio %u, err=%d\n",
+				gpio, error);
+			goto err_free_gpio;
+		}
+
+		bdev->data[i].can_sleep = gpio_cansleep(gpio);
+		bdev->data[i].last_state = -1;
+
+		input_set_capability(input, type, button->code);
+	}
+
+	bdev->poll_dev = poll_dev;
+	bdev->pdata = pdata;
+	platform_set_drvdata(pdev, bdev);
+
+	error = input_register_polled_device(poll_dev);
+	if (error) {
+		dev_err(dev, "unable to register polled device, err=%d\n",
+			error);
+		goto err_free_gpio;
+	}
+
+	/* report initial state of the buttons */
+	for (i = 0; i < pdata->nbuttons; i++)
+		gpio_buttons_check_state(input, &pdata->buttons[i],
+					 &bdev->data[i]);
+
+	return 0;
+
+err_free_gpio:
+	for (i = i - 1; i >= 0; i--)
+		gpio_free(pdata->buttons[i].gpio);
+
+	input_free_polled_device(poll_dev);
+
+err_free_bdev:
+	kfree(bdev);
+
+	platform_set_drvdata(pdev, NULL);
+	return error;
+}
+
+static int __devexit gpio_buttons_remove(struct platform_device *pdev)
+{
+	struct gpio_buttons_dev *bdev = platform_get_drvdata(pdev);
+	struct gpio_buttons_platform_data *pdata = bdev->pdata;
+	int i;
+
+	input_unregister_polled_device(bdev->poll_dev);
+
+	for (i = 0; i < pdata->nbuttons; i++)
+		gpio_free(pdata->buttons[i].gpio);
+
+	input_free_polled_device(bdev->poll_dev);
+
+	kfree(bdev);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver gpio_buttons_driver = {
+	.probe	= gpio_buttons_probe,
+	.remove	= __devexit_p(gpio_buttons_remove),
+	.driver	= {
+		.name	= DRV_NAME,
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init gpio_buttons_init(void)
+{
+	return platform_driver_register(&gpio_buttons_driver);
+}
+
+static void __exit gpio_buttons_exit(void)
+{
+	platform_driver_unregister(&gpio_buttons_driver);
+}
+
+module_init(gpio_buttons_init);
+module_exit(gpio_buttons_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Gabor Juhos <juhosg@openwrt.org>");
+MODULE_DESCRIPTION("Polled GPIO Buttons driver");
diff --git a/include/linux/gpio_buttons.h b/include/linux/gpio_buttons.h
new file mode 100644
index 0000000..f85b993
--- /dev/null
+++ b/include/linux/gpio_buttons.h
@@ -0,0 +1,33 @@
+/*
+ *  Definitions for the GPIO buttons interface driver
+ *
+ *  Copyright (C) 2007-2010 Gabor Juhos <juhosg@openwrt.org>
+ *
+ *  This file was based on: /include/linux/gpio_keys.h
+ *	The original gpio_keys.h seems not to have a license.
+ *
+ *  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.
+ *
+ */
+
+#ifndef _GPIO_BUTTONS_H_
+#define _GPIO_BUTTONS_H_
+
+struct gpio_button {
+	int	gpio;		/* GPIO line number */
+	int	active_low;
+	char	*desc;		/* button description */
+	int	type;		/* input event type (EV_KEY, EV_SW) */
+	int	code;		/* input event code (KEY_*, SW_*) */
+	int	threshold;	/* count threshold */
+};
+
+struct gpio_buttons_platform_data {
+	struct gpio_button *buttons;
+	int	nbuttons;		/* number of buttons */
+	int	poll_interval;		/* polling interval */
+};
+
+#endif /* _GPIO_BUTTONS_H_ */
-- 
1.7.2.1

Re: [PATCH 09/18] input: add input driver for polled GPIO buttons

[Ben Gardiner]

Hi Gabor,

On Tue, Nov 23, 2010 at 10:06 AM, Gabor Juhos <juhosg@openwrt.org> wrote:
> The existing gpio-keys driver can be usable only for GPIO lines with
> interrupt support. Several devices have buttons connected to a GPIO
> line which is not capable to generate interrupts. This patch adds a
> new input driver using the generic GPIO layer and the input-polldev
> to support such buttons.
>
> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Mike Frysinger <vapier@gentoo.org>
> Cc: linux-input@vger.kernel.org

I'm excited to see this patch posted. As I understand it, this
"separate polling gpio button driver" approach is preferred to the
"polling extensions in gpio-keys approach". I'm sorry I missed the RFC
[1] : I posted a revised version of Alex Clouter's and Paul Mundt's
polling gpio-keys support [2]. But now I can try integrating this
driver. :)

I will be posting a version of the patch shortly that is still using
the polling-gpio-keys support -- because there are discussions in the
machine-specific parts. Then I will try to integrate this driver into
the series.

Best Regards,
Ben Gardiner

[1] http://article.gmane.org/gmane.linux.kernel.input/16468
[2] http://article.gmane.org/gmane.linux.kernel.input/16587

---
Nanometrics Inc.
http://www.nanometrics.ca

Re: [PATCH 09/18] input: add input driver for polled GPIO buttons

[Ben Gardiner]

Hello Gabor,

On Tue, Nov 23, 2010 at 10:06 AM, Gabor Juhos <juhosg@openwrt.org> wrote:
> The existing gpio-keys driver can be usable only for GPIO lines with
> interrupt support. Several devices have buttons connected to a GPIO
> line which is not capable to generate interrupts. This patch adds a
> new input driver using the generic GPIO layer and the input-polldev
> to support such buttons.
>
> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Mike Frysinger <vapier@gentoo.org>
> Cc: linux-input@vger.kernel.org
> ---

I've tested this driver with the da850-evm pushbuttons and switches
connected to i2c gpio expanders. It works well. The changes to the
patch series were straightforward: .config, "gpio-keys" ->
"gpio-buttons", struct gpio_key -> struct gpio_button etc.

I do have some comments about this patch. But the new driver is
functional as-is.

Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>

>  drivers/input/misc/Kconfig        |   16 +++
>  drivers/input/misc/Makefile       |    1 +
>  drivers/input/misc/gpio_buttons.c |  232 +++++++++++++++++++++++++++++++++++++

Since the new gpio_buttons.c driver presents the same input event
device as the gpio_keys.c driver, I think it should also be a
drivers/input/keys device.

>  [...]
> diff --git a/drivers/input/misc/gpio_buttons.c b/drivers/input/misc/gpio_buttons.c

When I was converting the da850-evm platform code to use the new
driver I felt that the changes did not indicate a switch to a polled
driver as seems to be the intent with the introduction of a separate
driver. All that was different in the platform code was 'button' where
there use to be 'key' and button does not itself convey the knowledge
that it is a polled input device.

I know names of drivers can be contentions but I will propose
regardless that this driver be called gpio-polled-keys /
gpio_polled_keys.c

> new file mode 100644
> index 0000000..51288a3
> --- /dev/null
> +++ b/drivers/input/misc/gpio_buttons.c
> [...]
> +static void gpio_buttons_poll(struct input_polled_dev *dev)
> +{
> +       struct gpio_buttons_dev *bdev = dev->private;
> +       struct gpio_buttons_platform_data *pdata = bdev->pdata;
> +       struct input_dev *input = dev->input;
> +       int i;
> +
> +       for (i = 0; i < bdev->pdata->nbuttons; i++) {
> +               struct gpio_button *button = &pdata->buttons[i];
> +               struct gpio_button_data *bdata = &bdev->data[i];
> +
> +               if (bdata->count < button->threshold)
> +                       bdata->count++;
> +               else
> +                       gpio_buttons_check_state(input, button, bdata);

I think that a count-theshold can still be performed here, but using
the debounce_interval and polling_interval field specified in the
gpio_button and gpio_buttons_platform_data structures, respectively,
to calculate a threshold value.

In this way the gpio_button and gpio_keys_button structs are made more
similar -- differing only in the presence of .wakeup in
gpio_keys_button but not in gpio_button. Which may make it possible to
re-use the gpio_keys_button structure.

> [...]
> diff --git a/include/linux/gpio_buttons.h b/include/linux/gpio_buttons.h
> new file mode 100644
> index 0000000..f85b993
> --- /dev/null
> +++ b/include/linux/gpio_buttons.h
> @@ -0,0 +1,33 @@
> +/*
> + *  Definitions for the GPIO buttons interface driver
> + *
> + *  Copyright (C) 2007-2010 Gabor Juhos <juhosg@openwrt.org>
> + *
> + *  This file was based on: /include/linux/gpio_keys.h
> + *     The original gpio_keys.h seems not to have a license.
> + *
> + *  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.
> + *
> + */
> +
> +#ifndef _GPIO_BUTTONS_H_
> +#define _GPIO_BUTTONS_H_
> +
> +struct gpio_button {
> +       int     gpio;           /* GPIO line number */
> +       int     active_low;
> +       char    *desc;          /* button description */
> +       int     type;           /* input event type (EV_KEY, EV_SW) */
> +       int     code;           /* input event code (KEY_*, SW_*) */
> +       int     threshold;      /* count threshold */

Could we instead use the existing struct gpio_keys_button; we could
transform debounce_interval into a threshold as described above and
add an error when a gpio_button_probe() sees a gpio_key with .wakeup
== TRUE? It seems that this structure duplicates alot of the
gpio_keys_button structure.

> [...]
> +struct gpio_buttons_platform_data {
> +       struct gpio_button *buttons;
> +       int     nbuttons;               /* number of buttons */
> +       int     poll_interval;          /* polling interval */
> +};

I think the units of poll_interval should be included in the comment.
i.e. /* polling interval in msecs */

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca
--
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 09/18] input: add input driver for polled GPIO buttons

[Gabor Juhos]

Hi Ben,

> <...>
> I've tested this driver with the da850-evm pushbuttons and switches
> connected to i2c gpio expanders. It works well. The changes to the
> patch series were straightforward: .config, "gpio-keys" ->
> "gpio-buttons", struct gpio_key -> struct gpio_button etc.
> 
> I do have some comments about this patch. But the new driver is
> functional as-is.
> 
> Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>

Thanks!

>><...>
> 
> Since the new gpio_buttons.c driver presents the same input event
> device as the gpio_keys.c driver, I think it should also be a
> drivers/input/keys device.

Makes sense.

> 
>>  [...]
>> diff --git a/drivers/input/misc/gpio_buttons.c b/drivers/input/misc/gpio_buttons.c
> 
> When I was converting the da850-evm platform code to use the new
> driver I felt that the changes did not indicate a switch to a polled
> driver as seems to be the intent with the introduction of a separate
> driver. All that was different in the platform code was 'button' where
> there use to be 'key' and button does not itself convey the knowledge
> that it is a polled input device.
> 
> I know names of drivers can be contentions but I will propose
> regardless that this driver be called gpio-polled-keys /
> gpio_polled_keys.c

I agree, this would be more informative.

>> <...>
>> +       for (i = 0; i < bdev->pdata->nbuttons; i++) {
>> +               struct gpio_button *button = &pdata->buttons[i];
>> +               struct gpio_button_data *bdata = &bdev->data[i];
>> +
>> +               if (bdata->count < button->threshold)
>> +                       bdata->count++;
>> +               else
>> +                       gpio_buttons_check_state(input, button, bdata);
> 
> I think that a count-theshold can still be performed here, but using
> the debounce_interval and polling_interval field specified in the
> gpio_button and gpio_buttons_platform_data structures, respectively,
> to calculate a threshold value.

Good idea. We don't even have to compute a threshold value, we can use the
debounce_interval and poll_interval fields directly. I mean something similar to
this:

<...>
	if (bdata->interval < button->debounce_interval)
		bdata->interval += pdata->poll_interval;
	else
		gpio_buttons_check_state(input, button, bdata);
<...>

> In this way the gpio_button and gpio_keys_button structs are made more
> similar -- differing only in the presence of .wakeup in
> gpio_keys_button but not in gpio_button. Which may make it possible to
> re-use the gpio_keys_button structure.
> 
>> <...>
>> +struct gpio_button {
>> +       int     gpio;           /* GPIO line number */
>> +       int     active_low;
>> +       char    *desc;          /* button description */
>> +       int     type;           /* input event type (EV_KEY, EV_SW) */
>> +       int     code;           /* input event code (KEY_*, SW_*) */
>> +       int     threshold;      /* count threshold */
> 
> Could we instead use the existing struct gpio_keys_button; we could
> transform debounce_interval into a threshold as described above

Sure, we can use that...

> and add an error when a gpio_button_probe() sees a gpio_key with .wakeup ==
> TRUE?

I don't think we should check that, we can simply ignore this field. Maybe we
should add a comment to the .wakeup field to state that the polled driver does
not use that.

> It seems that this structure duplicates alot of the gpio_keys_button
> structure.

Yes, it is almost the same.

>> [...]
>> +struct gpio_buttons_platform_data {
>> +       struct gpio_button *buttons;
>> +       int     nbuttons;               /* number of buttons */
>> +       int     poll_interval;          /* polling interval */
>> +};
> 
> I think the units of poll_interval should be included in the comment.
> i.e. /* polling interval in msecs */

Yes, you are right. Additionally, we should move this structure into
gpio_keys.h, so we can get rid of the gpio_buttons.h file.

Thank you for the valuable comments. I will create a new patch.

Regards,
Gabor

Re: [PATCH 09/18] input: add input driver for polled GPIO buttons

[Ben Gardiner]

Hi Gabor,

On Wed, Nov 24, 2010 at 1:54 PM, Gabor Juhos <juhosg@openwrt.org> wrote:
> Hi Ben,
>> [...]
>> Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>
>
> Thanks!

You are most welcome.

>[...]
>
> Thank you for the valuable comments. I will create a new patch.

Actually, I've done a little hacking on the patch here while
integrating with the da850-evm changes. I will post the series here,
feel free to pick an choose the changes as you see fit -- hopefully it
can save you some time.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[PATCH WIP 0/6] suggested changes to gpio_buttons driver

[Ben Gardiner]

These are just some suggested changes from the review in patch-form. I will
do my best to make time for reviewing and testing any future versions of the
patch that you submit but I will regrettably be moving to other projects 
for awhile. Until then I will not be able to spend much time on integration.

With these patches applied I was able to build the da850-evm buttons and keys
support with minimal changes.As I said, these are just suggested changes,
please feel free to squash, edit or drop them as you see fit. My motivation 
here is that if you pickup the driver name and config symbol names as-is then
I will have no remaining integration to perform in the da850-evm series. I
hope you don't mind that I have to patch and run here.

I look forward to your next version of the patch. 

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

Ben Gardiner (6):
  fixup gpio_buttons: use the same debounce_interval member found in
    gpio_key to obtain a threshold count based on polling interval
  fixup gpio_buttons: use existing gpio_keys_button structure instead
    of introducing new gpio_button structure
  fixup gpio_buttons: detect and error-out if a button is requested for
    wakeup
  fixup gpio_buttons: show units of poll_interval platform data member
  fixup gpio_buttons: move gpio_buttons.c to drivers/input/keyboard
    from drivers/input/misc
  fixup gpio_buttons : rename gpio-buttons / gpio_buttons to
    gpio-keys-polled / gpio_keys_polled

 drivers/input/keyboard/Kconfig                     |   16 ++++
 drivers/input/keyboard/Makefile                    |    2 +
 .../gpio_buttons.c => keyboard/gpio_keys_polled.c} |   86 +++++++++++---------
 drivers/input/misc/Kconfig                         |   16 ----
 drivers/input/misc/Makefile                        |    1 -
 .../linux/{gpio_buttons.h => gpio_keys_polled.h}   |   15 +---
 6 files changed, 69 insertions(+), 67 deletions(-)
 rename drivers/input/{misc/gpio_buttons.c => keyboard/gpio_keys_polled.c} (64%)
 rename include/linux/{gpio_buttons.h => gpio_keys_polled.h} (60%)

[PATCH WIP 1/6] fixup gpio_buttons: use the same debounce_interval member found in gpio_key to obtain a threshold count based on polling interval

[Ben Gardiner]

my reasoning was that if the debounce interval was set then the platform 
_wants_ to supress spurious events so it was better to roundup and swallow 
real events than round down and not supress. 

I'm fine also with using the poll_interval and debuonce_interval directly as
you suggested.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 drivers/input/misc/gpio_buttons.c |    7 +++++--
 include/linux/gpio_buttons.h      |    2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/input/misc/gpio_buttons.c b/drivers/input/misc/gpio_buttons.c
index 51288a3..f42906c 100644
--- a/drivers/input/misc/gpio_buttons.c
+++ b/drivers/input/misc/gpio_buttons.c
@@ -68,13 +68,16 @@ static void gpio_buttons_poll(struct input_polled_dev *dev)
 	struct gpio_buttons_dev *bdev = dev->private;
 	struct gpio_buttons_platform_data *pdata = bdev->pdata;
 	struct input_dev *input = dev->input;
-	int i;
+	int i, threshold;
 
 	for (i = 0; i < bdev->pdata->nbuttons; i++) {
 		struct gpio_button *button = &pdata->buttons[i];
 		struct gpio_button_data *bdata = &bdev->data[i];
 
-		if (bdata->count < button->threshold)
+		threshold = round_up(button->debounce_interval,
+				pdata->poll_interval) /
+				pdata->poll_interval;
+		if (bdata->count < threshold)
 			bdata->count++;
 		else
 			gpio_buttons_check_state(input, button, bdata);
diff --git a/include/linux/gpio_buttons.h b/include/linux/gpio_buttons.h
index f85b993..c016f07 100644
--- a/include/linux/gpio_buttons.h
+++ b/include/linux/gpio_buttons.h
@@ -21,7 +21,7 @@ struct gpio_button {
 	char	*desc;		/* button description */
 	int	type;		/* input event type (EV_KEY, EV_SW) */
 	int	code;		/* input event code (KEY_*, SW_*) */
-	int	threshold;	/* count threshold */
+	int debounce_interval;	/* debounce ticks interval in msecs */
 };
 
 struct gpio_buttons_platform_data {
-- 
1.7.0.4

[PATCH WIP 2/6] fixup gpio_buttons: use existing gpio_keys_button structure instead of introducing new gpio_button structure

[Ben Gardiner]

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 drivers/input/misc/gpio_buttons.c |    6 +++---
 include/linux/gpio_buttons.h      |   11 ++---------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/input/misc/gpio_buttons.c b/drivers/input/misc/gpio_buttons.c
index f42906c..c34e978 100644
--- a/drivers/input/misc/gpio_buttons.c
+++ b/drivers/input/misc/gpio_buttons.c
@@ -42,7 +42,7 @@ struct gpio_buttons_dev {
 };
 
 static void gpio_buttons_check_state(struct input_dev *input,
-				      struct gpio_button *button,
+				      struct gpio_keys_button *button,
 				      struct gpio_button_data *bdata)
 {
 	int state;
@@ -71,7 +71,7 @@ static void gpio_buttons_poll(struct input_polled_dev *dev)
 	int i, threshold;
 
 	for (i = 0; i < bdev->pdata->nbuttons; i++) {
-		struct gpio_button *button = &pdata->buttons[i];
+		struct gpio_keys_button *button = &pdata->buttons[i];
 		struct gpio_button_data *bdata = &bdev->data[i];
 
 		threshold = round_up(button->debounce_interval,
@@ -132,7 +132,7 @@ static int __devinit gpio_buttons_probe(struct platform_device *pdev)
 	input->id.version = 0x0100;
 
 	for (i = 0; i < pdata->nbuttons; i++) {
-		struct gpio_button *button = &pdata->buttons[i];
+		struct gpio_keys_button *button = &pdata->buttons[i];
 		unsigned int gpio = button->gpio;
 		unsigned int type = button->type ?: EV_KEY;
 
diff --git a/include/linux/gpio_buttons.h b/include/linux/gpio_buttons.h
index c016f07..4d43de9 100644
--- a/include/linux/gpio_buttons.h
+++ b/include/linux/gpio_buttons.h
@@ -15,17 +15,10 @@
 #ifndef _GPIO_BUTTONS_H_
 #define _GPIO_BUTTONS_H_
 
-struct gpio_button {
-	int	gpio;		/* GPIO line number */
-	int	active_low;
-	char	*desc;		/* button description */
-	int	type;		/* input event type (EV_KEY, EV_SW) */
-	int	code;		/* input event code (KEY_*, SW_*) */
-	int debounce_interval;	/* debounce ticks interval in msecs */
-};
+#include <linux/gpio_keys.h>
 
 struct gpio_buttons_platform_data {
-	struct gpio_button *buttons;
+	struct gpio_keys_button *buttons;
 	int	nbuttons;		/* number of buttons */
 	int	poll_interval;		/* polling interval */
 };
-- 
1.7.0.4

[PATCH WIP 3/6] fixup gpio_buttons: detect and error-out if a button is requested for wakeup

[Ben Gardiner]

I'm fine also with ignoring the value of wakeup as you suggested.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 drivers/input/misc/gpio_buttons.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/input/misc/gpio_buttons.c b/drivers/input/misc/gpio_buttons.c
index c34e978..0859b4d 100644
--- a/drivers/input/misc/gpio_buttons.c
+++ b/drivers/input/misc/gpio_buttons.c
@@ -136,6 +136,11 @@ static int __devinit gpio_buttons_probe(struct platform_device *pdev)
 		unsigned int gpio = button->gpio;
 		unsigned int type = button->type ?: EV_KEY;
 
+		if (button->wakeup) {
+			dev_err(dev, DRV_NAME " does not support wakeup\n");
+			goto err_free_gpio;
+		}
+
 		error = gpio_request(gpio,
 				     button->desc ? button->desc : DRV_NAME);
 		if (error) {
-- 
1.7.0.4

[PATCH WIP 4/6] fixup gpio_buttons: show units of poll_interval platform data member

[Ben Gardiner]

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 include/linux/gpio_buttons.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/gpio_buttons.h b/include/linux/gpio_buttons.h
index 4d43de9..f7f1467 100644
--- a/include/linux/gpio_buttons.h
+++ b/include/linux/gpio_buttons.h
@@ -20,7 +20,7 @@
 struct gpio_buttons_platform_data {
 	struct gpio_keys_button *buttons;
 	int	nbuttons;		/* number of buttons */
-	int	poll_interval;		/* polling interval */
+	int	poll_interval;		/* polling interval in msecs*/
 };
 
 #endif /* _GPIO_BUTTONS_H_ */
-- 
1.7.0.4

[PATCH WIP 5/6] fixup gpio_buttons: move gpio_buttons.c to drivers/input/keyboard from drivers/input/misc

[Ben Gardiner]

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 drivers/input/keyboard/Kconfig                  |   16 ++++++++++++++++
 drivers/input/keyboard/Makefile                 |    2 ++
 drivers/input/{misc => keyboard}/gpio_buttons.c |    0
 drivers/input/misc/Kconfig                      |   16 ----------------
 drivers/input/misc/Makefile                     |    1 -
 5 files changed, 18 insertions(+), 17 deletions(-)
 rename drivers/input/{misc => keyboard}/gpio_buttons.c (100%)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index b8c51b9..4ab13fc 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -485,4 +485,20 @@ config KEYBOARD_W90P910
 	  To compile this driver as a module, choose M here: the
 	  module will be called w90p910_keypad.
 
+config INPUT_GPIO_BUTTONS
+	tristate "Polled GPIO buttons interface"
+	depends on GENERIC_GPIO
+	select INPUT_POLLDEV
+	help
+	  This driver implements support for buttons connected
+	  to GPIO pins of various CPUs (and some other chips).
+
+	  Say Y here if your device has buttons connected
+	  directly to such GPIO pins.  Your board-specific
+	  setup logic must also provide a platform device,
+	  with configuration data saying which GPIOs are used.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called gpio-buttons.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index a34452e..2a52e0c 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -44,3 +44,5 @@ obj-$(CONFIG_KEYBOARD_TNETV107X)	+= tnetv107x-keypad.o
 obj-$(CONFIG_KEYBOARD_TWL4030)		+= twl4030_keypad.o
 obj-$(CONFIG_KEYBOARD_XTKBD)		+= xtkbd.o
 obj-$(CONFIG_KEYBOARD_W90P910)		+= w90p910_keypad.o
+obj-$(CONFIG_INPUT_GPIO_BUTTONS)	+= gpio_buttons.o
+
diff --git a/drivers/input/misc/gpio_buttons.c b/drivers/input/keyboard/gpio_buttons.c
similarity index 100%
rename from drivers/input/misc/gpio_buttons.c
rename to drivers/input/keyboard/gpio_buttons.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index ef46534..a3adef6 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -457,20 +457,4 @@ config INPUT_ADXL34X_SPI
 	  To compile this driver as a module, choose M here: the
 	  module will be called adxl34x-spi.
 
-config INPUT_GPIO_BUTTONS
-	tristate "Polled GPIO buttons interface"
-	depends on GENERIC_GPIO
-	select INPUT_POLLDEV
-	help
-	  This driver implements support for buttons connected
-	  to GPIO pins of various CPUs (and some other chips).
-
-	  Say Y here if your device has buttons connected
-	  directly to such GPIO pins.  Your board-specific
-	  setup logic must also provide a platform device,
-	  with configuration data saying which GPIOs are used.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called gpio-buttons.
-
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 1f9f7bd..b041f5f 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -43,5 +43,4 @@ obj-$(CONFIG_INPUT_WINBOND_CIR)		+= winbond-cir.o
 obj-$(CONFIG_INPUT_WISTRON_BTNS)	+= wistron_btns.o
 obj-$(CONFIG_INPUT_WM831X_ON)		+= wm831x-on.o
 obj-$(CONFIG_INPUT_YEALINK)		+= yealink.o
-obj-$(CONFIG_INPUT_GPIO_BUTTONS)	+= gpio_buttons.o
 
-- 
1.7.0.4

[PATCH WIP 6/6] fixup gpio_buttons : rename gpio-buttons / gpio_buttons to gpio-keys-polled / gpio_keys_polled

[Ben Gardiner]

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 drivers/input/keyboard/Kconfig                     |    4 +-
 drivers/input/keyboard/Makefile                    |    2 +-
 .../{gpio_buttons.c => gpio_keys_polled.c}         |   68 ++++++++++----------
 .../linux/{gpio_buttons.h => gpio_keys_polled.h}   |    2 +-
 4 files changed, 38 insertions(+), 38 deletions(-)
 rename drivers/input/keyboard/{gpio_buttons.c => gpio_keys_polled.c} (71%)
 rename include/linux/{gpio_buttons.h => gpio_keys_polled.h} (94%)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 4ab13fc..9648ff4 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -485,8 +485,8 @@ config KEYBOARD_W90P910
 	  To compile this driver as a module, choose M here: the
 	  module will be called w90p910_keypad.
 
-config INPUT_GPIO_BUTTONS
-	tristate "Polled GPIO buttons interface"
+config KEYBOARD_GPIO_POLLED
+	tristate "Polled GPIO buttons"
 	depends on GENERIC_GPIO
 	select INPUT_POLLDEV
 	help
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 2a52e0c..e6da817 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -44,5 +44,5 @@ obj-$(CONFIG_KEYBOARD_TNETV107X)	+= tnetv107x-keypad.o
 obj-$(CONFIG_KEYBOARD_TWL4030)		+= twl4030_keypad.o
 obj-$(CONFIG_KEYBOARD_XTKBD)		+= xtkbd.o
 obj-$(CONFIG_KEYBOARD_W90P910)		+= w90p910_keypad.o
-obj-$(CONFIG_INPUT_GPIO_BUTTONS)	+= gpio_buttons.o
+obj-$(CONFIG_KEYBOARD_GPIO_POLLED)	+= gpio_keys_polled.o
 
diff --git a/drivers/input/keyboard/gpio_buttons.c b/drivers/input/keyboard/gpio_keys_polled.c
similarity index 71%
rename from drivers/input/keyboard/gpio_buttons.c
rename to drivers/input/keyboard/gpio_keys_polled.c
index 0859b4d..390ed93 100644
--- a/drivers/input/keyboard/gpio_buttons.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -25,25 +25,25 @@
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
-#include <linux/gpio_buttons.h>
+#include <linux/gpio_keys_polled.h>
 
-#define DRV_NAME	"gpio-buttons"
+#define DRV_NAME	"gpio-keys-polled"
 
-struct gpio_button_data {
+struct gpio_keys_button_data {
 	int last_state;
 	int count;
 	int can_sleep;
 };
 
-struct gpio_buttons_dev {
+struct gpio_keys_polled_dev {
 	struct input_polled_dev *poll_dev;
-	struct gpio_buttons_platform_data *pdata;
-	struct gpio_button_data *data;
+	struct gpio_keys_polled_platform_data *pdata;
+	struct gpio_keys_button_data *data;
 };
 
-static void gpio_buttons_check_state(struct input_dev *input,
+static void gpio_keys_polled_check_state(struct input_dev *input,
 				      struct gpio_keys_button *button,
-				      struct gpio_button_data *bdata)
+				      struct gpio_keys_button_data *bdata)
 {
 	int state;
 
@@ -63,16 +63,16 @@ static void gpio_buttons_check_state(struct input_dev *input,
 	}
 }
 
-static void gpio_buttons_poll(struct input_polled_dev *dev)
+static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 {
-	struct gpio_buttons_dev *bdev = dev->private;
-	struct gpio_buttons_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_polled_dev *bdev = dev->private;
+	struct gpio_keys_polled_platform_data *pdata = bdev->pdata;
 	struct input_dev *input = dev->input;
 	int i, threshold;
 
 	for (i = 0; i < bdev->pdata->nbuttons; i++) {
 		struct gpio_keys_button *button = &pdata->buttons[i];
-		struct gpio_button_data *bdata = &bdev->data[i];
+		struct gpio_keys_button_data *bdata = &bdev->data[i];
 
 		threshold = round_up(button->debounce_interval,
 				pdata->poll_interval) /
@@ -80,16 +80,16 @@ static void gpio_buttons_poll(struct input_polled_dev *dev)
 		if (bdata->count < threshold)
 			bdata->count++;
 		else
-			gpio_buttons_check_state(input, button, bdata);
+			gpio_keys_polled_check_state(input, button, bdata);
 
 	}
 }
 
-static int __devinit gpio_buttons_probe(struct platform_device *pdev)
+static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 {
-	struct gpio_buttons_platform_data *pdata = pdev->dev.platform_data;
+	struct gpio_keys_polled_platform_data *pdata = pdev->dev.platform_data;
 	struct device *dev = &pdev->dev;
-	struct gpio_buttons_dev *bdev;
+	struct gpio_keys_polled_dev *bdev;
 	struct input_polled_dev *poll_dev;
 	struct input_dev *input;
 	int error;
@@ -98,15 +98,15 @@ static int __devinit gpio_buttons_probe(struct platform_device *pdev)
 	if (!pdata)
 		return -ENXIO;
 
-	bdev = kzalloc(sizeof(struct gpio_buttons_dev) +
-		       pdata->nbuttons * sizeof(struct gpio_button_data),
+	bdev = kzalloc(sizeof(struct gpio_keys_polled_dev) +
+		       pdata->nbuttons * sizeof(struct gpio_keys_button_data),
 		       GFP_KERNEL);
 	if (!bdev) {
 		dev_err(dev, "no memory for private data\n");
 		return -ENOMEM;
 	}
 
-	bdev->data = (struct gpio_button_data *) &bdev[1];
+	bdev->data = (struct gpio_keys_button_data *) &bdev[1];
 
 	poll_dev = input_allocate_polled_device();
 	if (!poll_dev) {
@@ -116,14 +116,14 @@ static int __devinit gpio_buttons_probe(struct platform_device *pdev)
 	}
 
 	poll_dev->private = bdev;
-	poll_dev->poll = gpio_buttons_poll;
+	poll_dev->poll = gpio_keys_polled_poll;
 	poll_dev->poll_interval = pdata->poll_interval;
 
 	input = poll_dev->input;
 
 	input->evbit[0] = BIT(EV_KEY);
 	input->name = pdev->name;
-	input->phys = "gpio-buttons/input0";
+	input->phys = DRV_NAME"/input0";
 	input->dev.parent = &pdev->dev;
 
 	input->id.bustype = BUS_HOST;
@@ -176,7 +176,7 @@ static int __devinit gpio_buttons_probe(struct platform_device *pdev)
 
 	/* report initial state of the buttons */
 	for (i = 0; i < pdata->nbuttons; i++)
-		gpio_buttons_check_state(input, &pdata->buttons[i],
+		gpio_keys_polled_check_state(input, &pdata->buttons[i],
 					 &bdev->data[i]);
 
 	return 0;
@@ -194,10 +194,10 @@ err_free_bdev:
 	return error;
 }
 
-static int __devexit gpio_buttons_remove(struct platform_device *pdev)
+static int __devexit gpio_keys_polled_remove(struct platform_device *pdev)
 {
-	struct gpio_buttons_dev *bdev = platform_get_drvdata(pdev);
-	struct gpio_buttons_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_polled_dev *bdev = platform_get_drvdata(pdev);
+	struct gpio_keys_polled_platform_data *pdata = bdev->pdata;
 	int i;
 
 	input_unregister_polled_device(bdev->poll_dev);
@@ -213,27 +213,27 @@ static int __devexit gpio_buttons_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver gpio_buttons_driver = {
-	.probe	= gpio_buttons_probe,
-	.remove	= __devexit_p(gpio_buttons_remove),
+static struct platform_driver gpio_keys_polled_driver = {
+	.probe	= gpio_keys_polled_probe,
+	.remove	= __devexit_p(gpio_keys_polled_remove),
 	.driver	= {
 		.name	= DRV_NAME,
 		.owner	= THIS_MODULE,
 	},
 };
 
-static int __init gpio_buttons_init(void)
+static int __init gpio_keys_polled_init(void)
 {
-	return platform_driver_register(&gpio_buttons_driver);
+	return platform_driver_register(&gpio_keys_polled_driver);
 }
 
-static void __exit gpio_buttons_exit(void)
+static void __exit gpio_keys_polled_exit(void)
 {
-	platform_driver_unregister(&gpio_buttons_driver);
+	platform_driver_unregister(&gpio_keys_polled_driver);
 }
 
-module_init(gpio_buttons_init);
-module_exit(gpio_buttons_exit);
+module_init(gpio_keys_polled_init);
+module_exit(gpio_keys_polled_exit);
 
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Gabor Juhos <juhosg@openwrt.org>");
diff --git a/include/linux/gpio_buttons.h b/include/linux/gpio_keys_polled.h
similarity index 94%
rename from include/linux/gpio_buttons.h
rename to include/linux/gpio_keys_polled.h
index f7f1467..bf7f94a 100644
--- a/include/linux/gpio_buttons.h
+++ b/include/linux/gpio_keys_polled.h
@@ -17,7 +17,7 @@
 
 #include <linux/gpio_keys.h>
 
-struct gpio_buttons_platform_data {
+struct gpio_keys_polled_platform_data {
 	struct gpio_keys_button *buttons;
 	int	nbuttons;		/* number of buttons */
 	int	poll_interval;		/* polling interval in msecs*/
-- 
1.7.0.4

Re: [PATCH 09/18] input: add input driver for polled GPIO buttons

[Dmitry Torokhov]

On Tue, Nov 23, 2010 at 04:06:31PM +0100, Gabor Juhos wrote:
> + *
> + *  This file was based on: /include/linux/gpio_keys.h
> + *	The original gpio_keys.h seems not to have a license.
> + *

This is incorrect statement. Unless otherwise specified Linux kernel
sources are licensed under GPL v2.

Thanks.

-- 
Dmitry

Re: [PATCH WIP 0/6] suggested changes to gpio_buttons driver

[Dmitry Torokhov]

Hi Gabor, Ben,

On Wed, Nov 24, 2010 at 04:01:34PM -0500, Ben Gardiner wrote:
> These are just some suggested changes from the review in patch-form. I will
> do my best to make time for reviewing and testing any future versions of the
> patch that you submit but I will regrettably be moving to other projects 
> for awhile. Until then I will not be able to spend much time on integration.
> 
> With these patches applied I was able to build the da850-evm buttons and keys
> support with minimal changes.As I said, these are just suggested changes,
> please feel free to squash, edit or drop them as you see fit. My motivation 
> here is that if you pickup the driver name and config symbol names as-is then
> I will have no remaining integration to perform in the da850-evm series. I
> hope you don't mind that I have to patch and run here.
> 
> I look forward to your next version of the patch. 

I do like the original patch and even more so Ben changes. Could you
tell me if the following still works for you (goes on top of Ben's)
and if it does I'll fold everything together and queue for 2.6.38.

Thanks!

-- 
Dmitry

Input: gpio-buttons - misc changes

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/keyboard/gpio_keys_polled.c |   38 ++++++++++++++++++++++-------
 include/linux/gpio_keys.h                 |    2 ++
 include/linux/gpio_keys_polled.h          |   26 --------------------
 3 files changed, 31 insertions(+), 35 deletions(-)
 delete mode 100644 include/linux/gpio_keys_polled.h


diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 390ed93..6ed6910 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -25,7 +25,7 @@
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
-#include <linux/gpio_keys_polled.h>
+#include <linux/gpio_keys.h>
 
 #define DRV_NAME	"gpio-keys-polled"
 
@@ -37,8 +37,9 @@ struct gpio_keys_button_data {
 
 struct gpio_keys_polled_dev {
 	struct input_polled_dev *poll_dev;
-	struct gpio_keys_polled_platform_data *pdata;
-	struct gpio_keys_button_data *data;
+	struct device *dev;
+	struct gpio_keys_platform_data *pdata;
+	struct gpio_keys_button_data data[0];
 };
 
 static void gpio_keys_polled_check_state(struct input_dev *input,
@@ -66,7 +67,7 @@ static void gpio_keys_polled_check_state(struct input_dev *input,
 static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 {
 	struct gpio_keys_polled_dev *bdev = dev->private;
-	struct gpio_keys_polled_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = bdev->pdata;
 	struct input_dev *input = dev->input;
 	int i, threshold;
 
@@ -85,9 +86,27 @@ static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 	}
 }
 
+static void gpio_keys_polled_open(struct input_polled_dev *dev)
+{
+	struct gpio_keys_polled_dev *bdev = dev->private;
+	struct gpio_keys_platform_data *pdata = bdev->pdata;
+
+	if (pdata->enable)
+		pdata->enable(bdev->dev);
+}
+
+static void gpio_keys_polled_close(struct input_polled_dev *dev)
+{
+	struct gpio_keys_polled_dev *bdev = dev->private;
+	struct gpio_keys_platform_data *pdata = bdev->pdata;
+
+	if (pdata->disable)
+		pdata->disable(bdev->dev);
+}
+
 static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 {
-	struct gpio_keys_polled_platform_data *pdata = pdev->dev.platform_data;
+	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
 	struct device *dev = &pdev->dev;
 	struct gpio_keys_polled_dev *bdev;
 	struct input_polled_dev *poll_dev;
@@ -106,8 +125,6 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	bdev->data = (struct gpio_keys_button_data *) &bdev[1];
-
 	poll_dev = input_allocate_polled_device();
 	if (!poll_dev) {
 		dev_err(dev, "no memory for polled device\n");
@@ -118,6 +135,8 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 	poll_dev->private = bdev;
 	poll_dev->poll = gpio_keys_polled_poll;
 	poll_dev->poll_interval = pdata->poll_interval;
+	poll_dev->open = gpio_keys_polled_open;
+	poll_dev->close = gpio_keys_polled_close;
 
 	input = poll_dev->input;
 
@@ -164,6 +183,7 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 	}
 
 	bdev->poll_dev = poll_dev;
+	bdev->dev = dev;
 	bdev->pdata = pdata;
 	platform_set_drvdata(pdev, bdev);
 
@@ -182,7 +202,7 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 	return 0;
 
 err_free_gpio:
-	for (i = i - 1; i >= 0; i--)
+	while (--i >= 0)
 		gpio_free(pdata->buttons[i].gpio);
 
 	input_free_polled_device(poll_dev);
@@ -197,7 +217,7 @@ err_free_bdev:
 static int __devexit gpio_keys_polled_remove(struct platform_device *pdev)
 {
 	struct gpio_keys_polled_dev *bdev = platform_get_drvdata(pdev);
-	struct gpio_keys_polled_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = bdev->pdata;
 	int i;
 
 	input_unregister_polled_device(bdev->poll_dev);
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index ce73a30..dd1a56f 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -16,6 +16,8 @@ struct gpio_keys_button {
 struct gpio_keys_platform_data {
 	struct gpio_keys_button *buttons;
 	int nbuttons;
+	unsigned int poll_interval;	/* polling interval in msecs -
+					   for polling driver only */
 	unsigned int rep:1;		/* enable input subsystem auto repeat */
 	int (*enable)(struct device *dev);
 	void (*disable)(struct device *dev);
diff --git a/include/linux/gpio_keys_polled.h b/include/linux/gpio_keys_polled.h
deleted file mode 100644
index bf7f94a..0000000
--- a/include/linux/gpio_keys_polled.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- *  Definitions for the GPIO buttons interface driver
- *
- *  Copyright (C) 2007-2010 Gabor Juhos <juhosg@openwrt.org>
- *
- *  This file was based on: /include/linux/gpio_keys.h
- *	The original gpio_keys.h seems not to have a license.
- *
- *  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.
- *
- */
-
-#ifndef _GPIO_BUTTONS_H_
-#define _GPIO_BUTTONS_H_
-
-#include <linux/gpio_keys.h>
-
-struct gpio_keys_polled_platform_data {
-	struct gpio_keys_button *buttons;
-	int	nbuttons;		/* number of buttons */
-	int	poll_interval;		/* polling interval in msecs*/
-};
-
-#endif /* _GPIO_BUTTONS_H_ */

Re: [PATCH WIP 0/6] suggested changes to gpio_buttons driver

[Gabor Juhos]

Hi Dmitry,

> On Wed, Nov 24, 2010 at 04:01:34PM -0500, Ben Gardiner wrote:
>> These are just some suggested changes from the review in patch-form. I will
>> do my best to make time for reviewing and testing any future versions of the
>> patch that you submit but I will regrettably be moving to other projects 
>> for awhile. Until then I will not be able to spend much time on integration.
>>
>> With these patches applied I was able to build the da850-evm buttons and keys
>> support with minimal changes.As I said, these are just suggested changes,
>> please feel free to squash, edit or drop them as you see fit. My motivation 
>> here is that if you pickup the driver name and config symbol names as-is then
>> I will have no remaining integration to perform in the da850-evm series. I
>> hope you don't mind that I have to patch and run here.
>>
>> I look forward to your next version of the patch. 
> 
> I do like the original patch and even more so Ben changes. Could you
> tell me if the following still works for you (goes on top of Ben's)
> and if it does I'll fold everything together and queue for 2.6.38.

It works for me, however I have additional fixes on top of Ben's changes. I will
post an updated version soon.

The original driver was a part of a patch-set which adds support for a new MIPS
platform. There are a couple of patches in that series which requires this one.
It would be nice if the driver would be able to go through the MIPS tree to
avoid cross-tree dependencies.

Regards,
Gabor

Re: [PATCH WIP 0/6] suggested changes to gpio_buttons driver

[Ben Gardiner]

Hi Dmitry, Gabor,

On Tue, Nov 30, 2010 at 2:56 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> [...]
> I do like the original patch and even more so Ben changes. Could you
> tell me if the following still works for you (goes on top of Ben's)
> and if it does I'll fold everything together and queue for 2.6.38.

Thank you for considering the proposed changes and also for you r kind
words: it is encouraging to hear as a newcomer to kernel development.\

> [...]
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>
>  drivers/input/keyboard/gpio_keys_polled.c |   38 ++++++++++++++++++++++-------
>  include/linux/gpio_keys.h                 |    2 ++
>  include/linux/gpio_keys_polled.h          |   26 --------------------
>  3 files changed, 31 insertions(+), 35 deletions(-)
>  delete mode 100644 include/linux/gpio_keys_polled.h

I have tested your patch with the "da850-evm: add gpio-{keys,leds} for
UI and BB expanders" on top of Gabor's "[PATCH 09/18] input: add input
driver for polled GPIO buttons" patch and my "[PATCH WIP 0/6]
suggested changes to gpio_buttons driver" series. Only minimal changes
were needed in the later patches of the da850-evm series.

They can be summarized (as you probably expected) by:
s/gpio_keys_polled\.h/gpio_keys.\h/g and
s/gpio_keys_polled_platform_data/gpio_keys_platform_data/g.

With those changes I observed the expected input events with evtest.

Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>

On Tue, Nov 30, 2010 at 5:30 AM, Gabor Juhos <juhosg@openwrt.org> wrote:
>> [...]
> It works for me, however I have additional fixes on top of Ben's changes. I will
> post an updated version soon.

This "polled gpio keys" driver was your invention. FWIW, I think your
upcoming updated version should be given consideration before
proceeding with the collection of patches discussed above.

> The original driver was a part of a patch-set which adds support for a new MIPS
> platform. There are a couple of patches in that series which requires this one.
> It would be nice if the driver would be able to go through the MIPS tree to
> avoid cross-tree dependencies.

Since the da850-evm series which depends on this patch is proposed for
the linux-davinci tree I think that to avoid cross-tree dependencies
they would all need to be taken in one tree. That is, unless there is
zero chance of the da850-evm series being taken-up for 2.6.38 along
with this driver. I have added Kevin Hilman, the linux-davinci
maintainer, and Sekhar Nori, the da850-evm maintainer, to the CC of
this email so that they can participate in the discussion of when and
how the patches should be merged.

Kevin, Sekhar, do you think that "da850-evm: add gpio-{keys,leds} for
UI and BB expanders" could be queued for 2.6.38?

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca
--
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 WIP 0/6] suggested changes to gpio_buttons driver

[Gabor Juhos]

Hi Ben, Dmitry,

>> <...>
>> It works for me, however I have additional fixes on top of Ben's changes. I will
>> post an updated version soon.
> 
> This "polled gpio keys" driver was your invention. FWIW, I think your
> upcoming updated version should be given consideration before
> proceeding with the collection of patches discussed above.
> 
>> The original driver was a part of a patch-set which adds support for a new MIPS
>> platform. There are a couple of patches in that series which requires this one.
>> It would be nice if the driver would be able to go through the MIPS tree to
>> avoid cross-tree dependencies.
> 
> Since the da850-evm series which depends on this patch is proposed for
> the linux-davinci tree I think that to avoid cross-tree dependencies
> they would all need to be taken in one tree. That is, unless there is
> zero chance of the da850-evm series being taken-up for 2.6.38 along
> with this driver. I have added Kevin Hilman, the linux-davinci
> maintainer, and Sekhar Nori, the da850-evm maintainer, to the CC of
> this email so that they can participate in the discussion of when and
> how the patches should be merged.
> 
> Kevin, Sekhar, do you think that "da850-evm: add gpio-{keys,leds} for
> UI and BB expanders" could be queued for 2.6.38?

Well, I had to rethink that a bit. In my original patch-set I have several
patches which must be acked by other subsystem maintainers.

Given the possibility that my patch-set will not be merged for 2.6.38 I will
drop the driver from that set so the driver can go through any tree.
Additionally I will post my changes in a follow up e-mail. Those changes goes on
top of Dmitry's changes, so the whole stuff can be folded into the original patch.

Regards,
Gabor

[PATCH 0/8] input: gpio_keys_polled fixes

[Gabor Juhos]

This set contains my latest fixes on top of Dmitry's changes.

Gabor Juhos (8):
  input: gpio_keys_polled: add MODULE_ALIAS
  input: gpio_keys_polled: return -EINVAL if wakeup specified
  input: gpio_keys_polled: avoid possible division by zero
  input: gpio_keys_polled: use DIV_ROUND_UP to compute the threshold value
  input: gpio_keys_polled: precompute threshold value in the probe routine
  input: gpio_keys_polled: use tabs instead of spaces for indentation
  input: gpio_keys_polled: remove a local variable
  input: gpio_keys_polled: fix Kconfig help text

 drivers/input/keyboard/Kconfig            |    4 +-
 drivers/input/keyboard/gpio_keys_polled.c |   30 +++++++++++++++-------------
 2 files changed, 18 insertions(+), 16 deletions(-)

-- 
1.7.2.1

[PATCH 1/8] input: gpio_keys_polled: add MODULE_ALIAS

[Gabor Juhos]

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/input/keyboard/gpio_keys_polled.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 6ed6910..1bd335b 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -258,3 +258,4 @@ module_exit(gpio_keys_polled_exit);
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Gabor Juhos <juhosg@openwrt.org>");
 MODULE_DESCRIPTION("Polled GPIO Buttons driver");
+MODULE_ALIAS("platform:" DRV_NAME)
-- 
1.7.2.1

[PATCH 2/8] input: gpio_keys_polled: return -EINVAL if wakeup specified

[Gabor Juhos]

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/input/keyboard/gpio_keys_polled.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 1bd335b..5275c9e 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -157,6 +157,7 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 
 		if (button->wakeup) {
 			dev_err(dev, DRV_NAME " does not support wakeup\n");
+			error = -EINVAL;
 			goto err_free_gpio;
 		}
 
-- 
1.7.2.1

[PATCH 3/8] input: gpio_keys_polled: avoid possible division by zero

[Gabor Juhos]

The poll_interval field is used as a divider in the computation of
the threshold value. Check the passed value in the probe routine,
and return -EINVAL if it has not been set in the platform data.

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/input/keyboard/gpio_keys_polled.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 5275c9e..9636c2f 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -114,8 +114,8 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 	int error;
 	int i;
 
-	if (!pdata)
-		return -ENXIO;
+	if (!pdata || !pdata->poll_interval)
+		return -EINVAL;
 
 	bdev = kzalloc(sizeof(struct gpio_keys_polled_dev) +
 		       pdata->nbuttons * sizeof(struct gpio_keys_button_data),
-- 
1.7.2.1

[PATCH 4/8] input: gpio_keys_polled: use DIV_ROUND_UP to compute the threshold value

[Gabor Juhos]

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/input/keyboard/gpio_keys_polled.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 9636c2f..c9e0d1c 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -75,9 +75,8 @@ static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 		struct gpio_keys_button *button = &pdata->buttons[i];
 		struct gpio_keys_button_data *bdata = &bdev->data[i];
 
-		threshold = round_up(button->debounce_interval,
-				pdata->poll_interval) /
-				pdata->poll_interval;
+		threshold = DIV_ROUND_UP(button->debounce_interval,
+					 pdata->poll_interval);
 		if (bdata->count < threshold)
 			bdata->count++;
 		else
-- 
1.7.2.1

[PATCH 5/8] input: gpio_keys_polled: precompute threshold value in the probe routine

[Gabor Juhos]

The threshold value will be a constant, so precompute that at probe time
to save a few cycles in the polling thread. Also introduce a new bdata
local variable in the probe routine to improve readability.

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/input/keyboard/gpio_keys_polled.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index c9e0d1c..5af0be9 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -32,6 +32,7 @@
 struct gpio_keys_button_data {
 	int last_state;
 	int count;
+	int threshold;
 	int can_sleep;
 };
 
@@ -69,15 +70,13 @@ static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 	struct gpio_keys_polled_dev *bdev = dev->private;
 	struct gpio_keys_platform_data *pdata = bdev->pdata;
 	struct input_dev *input = dev->input;
-	int i, threshold;
+	int i;
 
 	for (i = 0; i < bdev->pdata->nbuttons; i++) {
 		struct gpio_keys_button *button = &pdata->buttons[i];
 		struct gpio_keys_button_data *bdata = &bdev->data[i];
 
-		threshold = DIV_ROUND_UP(button->debounce_interval,
-					 pdata->poll_interval);
-		if (bdata->count < threshold)
+		if (bdata->count < bdata->threshold)
 			bdata->count++;
 		else
 			gpio_keys_polled_check_state(input, button, bdata);
@@ -151,6 +150,7 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 
 	for (i = 0; i < pdata->nbuttons; i++) {
 		struct gpio_keys_button *button = &pdata->buttons[i];
+		struct gpio_keys_button_data *bdata = &bdev->data[i];
 		unsigned int gpio = button->gpio;
 		unsigned int type = button->type ?: EV_KEY;
 
@@ -176,8 +176,10 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 			goto err_free_gpio;
 		}
 
-		bdev->data[i].can_sleep = gpio_cansleep(gpio);
-		bdev->data[i].last_state = -1;
+		bdata->can_sleep = gpio_cansleep(gpio);
+		bdata->last_state = -1;
+		bdata->threshold = DIV_ROUND_UP(button->debounce_interval,
+						pdata->poll_interval);
 
 		input_set_capability(input, type, button->code);
 	}
-- 
1.7.2.1

[PATCH 6/8] input: gpio_keys_polled: use tabs instead of spaces for indentation

[Gabor Juhos]

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/input/keyboard/gpio_keys_polled.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 5af0be9..7b6a333 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -44,8 +44,8 @@ struct gpio_keys_polled_dev {
 };
 
 static void gpio_keys_polled_check_state(struct input_dev *input,
-				      struct gpio_keys_button *button,
-				      struct gpio_keys_button_data *bdata)
+					 struct gpio_keys_button *button,
+					 struct gpio_keys_button_data *bdata)
 {
 	int state;
 
-- 
1.7.2.1

[PATCH 7/8] input: gpio_keys_polled: remove a local variable

[Gabor Juhos]

The 'button' variable is used only once in the gpio_keys_polled_poll
routine. Remove that and use the '&pdata->buttons[i]' value directly
instead.

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/input/keyboard/gpio_keys_polled.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 7b6a333..61e1a92 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -73,14 +73,13 @@ static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 	int i;
 
 	for (i = 0; i < bdev->pdata->nbuttons; i++) {
-		struct gpio_keys_button *button = &pdata->buttons[i];
 		struct gpio_keys_button_data *bdata = &bdev->data[i];
 
 		if (bdata->count < bdata->threshold)
 			bdata->count++;
 		else
-			gpio_keys_polled_check_state(input, button, bdata);
-
+			gpio_keys_polled_check_state(input, &pdata->buttons[i],
+						     bdata);
 	}
 }
 
-- 
1.7.2.1

[PATCH 8/8] input: gpio_keys_polled: fix Kconfig help text

[Gabor Juhos]

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/input/keyboard/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 9648ff4..897b758 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -491,7 +491,7 @@ config KEYBOARD_GPIO_POLLED
 	select INPUT_POLLDEV
 	help
 	  This driver implements support for buttons connected
-	  to GPIO pins of various CPUs (and some other chips).
+	  to GPIO pins which is not capable to generate interrupts.
 
 	  Say Y here if your device has buttons connected
 	  directly to such GPIO pins.  Your board-specific
@@ -499,6 +499,6 @@ config KEYBOARD_GPIO_POLLED
 	  with configuration data saying which GPIOs are used.
 
 	  To compile this driver as a module, choose M here: the
-	  module will be called gpio-buttons.
+	  module will be called gpio_keys_polled.
 
 endif
-- 
1.7.2.1

Re: [PATCH 0/8] input: gpio_keys_polled fixes

[Ben Gardiner]

Hi Gabor,

On Tue, Nov 30, 2010 at 1:44 PM, Gabor Juhos <juhosg@openwrt.org> wrote:
> This set contains my latest fixes on top of Dmitry's changes.
>
> Gabor Juhos (8):
>  input: gpio_keys_polled: add MODULE_ALIAS

>  input: gpio_keys_polled: return -EINVAL if wakeup specified
>  input: gpio_keys_polled: avoid possible division by zero
Good idea to return -EINVAL in those cases.

>  input: gpio_keys_polled: use DIV_ROUND_UP to compute the threshold value
That's what I was looking for, thank you for fixing that up.

>  input: gpio_keys_polled: precompute threshold value in the probe routine
Even better.

>  input: gpio_keys_polled: use tabs instead of spaces for indentation
>  input: gpio_keys_polled: remove a local variable
>  input: gpio_keys_polled: fix Kconfig help text
I missed that remaining gpio-buttons reference, thanks for catching that.

>  drivers/input/keyboard/Kconfig            |    4 +-
>  drivers/input/keyboard/gpio_keys_polled.c |   30 +++++++++++++++-------------
>  2 files changed, 18 insertions(+), 16 deletions(-)

I have tested this series with the "da850-evm: add gpio-{keys,leds}
for UI and BB expanders" on top of your "[PATCH 09/18] input:  add
input driver for polled GPIO buttons" patch, my "[PATCH WIP 0/6]
suggested changes to gpio_buttons driver" series and Dmitry's "Input:
gpio-buttons - misc changes"  patch.

Only minimal changes were needed in the later patches of the da850-evm
series. They can be summarized by:
s/gpio_keys_polled\.h/gpio_keys.\h/g and
s/gpio_keys_polled_platform_data/gpio_keys_platform_data/g.

With those changes I observed the expected input events with evtest.

Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca
--
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 8/8] input: gpio_keys_polled: fix Kconfig help text

[Ferenc Wagner]

Gabor Juhos <juhosg@openwrt.org> writes:

> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -491,7 +491,7 @@ config KEYBOARD_GPIO_POLLED
> -	  to GPIO pins of various CPUs (and some other chips).
> +	  to GPIO pins which is not capable to generate interrupts.

I suggest s/is/are/ or perhaps even s/which is //.
-- 
Great stuff, thanks!
Feri.

Re: [PATCH 0/8] input: gpio_keys_polled fixes

[Gabor Juhos]

Hi Ben,

> <...>
> I have tested this series with the "da850-evm: add gpio-{keys,leds}
> for UI and BB expanders" on top of your "[PATCH 09/18] input:  add
> input driver for polled GPIO buttons" patch, my "[PATCH WIP 0/6]
> suggested changes to gpio_buttons driver" series and Dmitry's "Input:
> gpio-buttons - misc changes"  patch.
> 
> Only minimal changes were needed in the later patches of the da850-evm
> series. They can be summarized by:
> s/gpio_keys_polled\.h/gpio_keys.\h/g and
> s/gpio_keys_polled_platform_data/gpio_keys_platform_data/g.
> 
> With those changes I observed the expected input events with evtest.

Superb.

> Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>

Thank you for the continuous testing.

Regards,
Gabor

Re: [PATCH 8/8] input: gpio_keys_polled: fix Kconfig help text

[Gabor Juhos]

2010.12.01. 23:53 keltezéssel, Ferenc Wagner írta:
> Gabor Juhos <juhosg@openwrt.org> writes:
> 
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -491,7 +491,7 @@ config KEYBOARD_GPIO_POLLED
>> -	  to GPIO pins of various CPUs (and some other chips).
>> +	  to GPIO pins which is not capable to generate interrupts.
> 
> I suggest s/is/are/ or perhaps even s/which is //.

Ok, i will send an updated patch.

Thanks,
Gabor
--
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

[PATCH v2 8/8] input: gpio_keys_polled: fix Kconfig help text

[Gabor Juhos]

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
Changes since v1:
  - remove 'which is ' from the help text

 drivers/input/keyboard/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 9648ff4..897b758 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -491,7 +491,7 @@ config KEYBOARD_GPIO_POLLED
 	select INPUT_POLLDEV
 	help
 	  This driver implements support for buttons connected
-	  to GPIO pins of various CPUs (and some other chips).
+	  to GPIO pins not capable to generate interrupts.
 
 	  Say Y here if your device has buttons connected
 	  directly to such GPIO pins.  Your board-specific
@@ -499,6 +499,6 @@ config KEYBOARD_GPIO_POLLED
 	  with configuration data saying which GPIOs are used.
 
 	  To compile this driver as a module, choose M here: the
-	  module will be called gpio-buttons.
+	  module will be called gpio_keys_polled.
 
 endif
-- 
1.7.2.1

Re: [PATCH 0/8] input: gpio_keys_polled fixes

[Dmitry Torokhov]

On Tue, Nov 30, 2010 at 05:16:15PM -0500, Ben Gardiner wrote:
> Hi Gabor,
> 
> On Tue, Nov 30, 2010 at 1:44 PM, Gabor Juhos <juhosg@openwrt.org> wrote:
> > This set contains my latest fixes on top of Dmitry's changes.
> >
> > Gabor Juhos (8):
> >  input: gpio_keys_polled: add MODULE_ALIAS
> 
> >  input: gpio_keys_polled: return -EINVAL if wakeup specified
> >  input: gpio_keys_polled: avoid possible division by zero
> Good idea to return -EINVAL in those cases.
> 
> >  input: gpio_keys_polled: use DIV_ROUND_UP to compute the threshold value
> That's what I was looking for, thank you for fixing that up.
> 
> >  input: gpio_keys_polled: precompute threshold value in the probe routine
> Even better.
> 
> >  input: gpio_keys_polled: use tabs instead of spaces for indentation
> >  input: gpio_keys_polled: remove a local variable
> >  input: gpio_keys_polled: fix Kconfig help text
> I missed that remaining gpio-buttons reference, thanks for catching that.
> 
> >  drivers/input/keyboard/Kconfig            |    4 +-
> >  drivers/input/keyboard/gpio_keys_polled.c |   30 +++++++++++++++-------------
> >  2 files changed, 18 insertions(+), 16 deletions(-)
> 
> I have tested this series with the "da850-evm: add gpio-{keys,leds}
> for UI and BB expanders" on top of your "[PATCH 09/18] input:  add
> input driver for polled GPIO buttons" patch, my "[PATCH WIP 0/6]
> suggested changes to gpio_buttons driver" series and Dmitry's "Input:
> gpio-buttons - misc changes"  patch.

Ok, so driver is in mainline now and you should have no problems with
merging your platform code in .38 merge window.

Thanks.

-- 
Dmitry
--
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 0/8] input: gpio_keys_polled fixes

[Gabor Juhos]

Hi Dmitry,

> <...>
> Ok, so driver is in mainline now and you should have no problems with
> merging your platform code in .38 merge window.

Thank you!

-Gabor

Re: [PATCH 0/8] input: gpio_keys_polled fixes

[Ben Gardiner]

Hi Dmitry,

On Wed, Dec 8, 2010 at 7:06 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Ok, so driver is in mainline now [...]

I see it in v2.6.37-rc5 (at 0e7d0c860a0dee49dacb7bbb248d1eba637075ad
). With all the changes proposed folded-in and even a little Kconfig
help text fixup. It is (still) working as expected.

Thank you very much for picking up the driver changes...

> [...] and you should have no problems with
> merging your platform code in .38 merge window.

and thank you for the vote of confidence. I will post an updated
version shortly.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca