[PATCH v2 0/3] INPUT: gpio_keys.c: Added OF support and enabled use with I2C GPIO expanders

[PATCH v2 0/3] INPUT: gpio_keys.c: Added OF support and enabled use with I2C GPIO expanders

[David Jander]

This is the second version of this patch, spit-out in three separate patches.

[PATCH 1/3] Input: gpio_keys.c: Simplify platform_device -> device casting

[David Jander]

This patch factors out the use of struct platform_device *pdev in most
places.

Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/input/keyboard/gpio_keys.c |   46 ++++++++++++++++-------------------
 1 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 6e6145b..987498e 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -251,8 +251,7 @@ static ssize_t gpio_keys_show_##name(struct device *dev,		\
 				     struct device_attribute *attr,	\
 				     char *buf)				\
 {									\
-	struct platform_device *pdev = to_platform_device(dev);		\
-	struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev);	\
+	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);		\
 									\
 	return gpio_keys_attr_show_helper(ddata, buf,			\
 					  type, only_disabled);		\
@@ -278,8 +277,7 @@ static ssize_t gpio_keys_store_##name(struct device *dev,		\
 				      const char *buf,			\
 				      size_t count)			\
 {									\
-	struct platform_device *pdev = to_platform_device(dev);		\
-	struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev);	\
+	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);		\
 	ssize_t error;							\
 									\
 	error = gpio_keys_attr_store_helper(ddata, buf, type);		\
@@ -364,12 +362,11 @@ static irqreturn_t gpio_keys_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int __devinit gpio_keys_setup_key(struct platform_device *pdev,
+static int __devinit gpio_keys_setup_key(struct device *dev,
 					 struct gpio_button_data *bdata,
 					 struct gpio_keys_button *button)
 {
 	const char *desc = button->desc ? button->desc : "gpio_keys";
-	struct device *dev = &pdev->dev;
 	unsigned long irqflags;
 	int irq, error;
 
@@ -447,9 +444,9 @@ static void gpio_keys_close(struct input_dev *input)
 
 static int __devinit gpio_keys_probe(struct platform_device *pdev)
 {
-	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
 	struct gpio_keys_drvdata *ddata;
 	struct device *dev = &pdev->dev;
+	struct gpio_keys_platform_data *pdata = dev->platform_data;
 	struct input_dev *input;
 	int i, error;
 	int wakeup = 0;
@@ -470,12 +467,12 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 	ddata->disable = pdata->disable;
 	mutex_init(&ddata->disable_lock);
 
-	platform_set_drvdata(pdev, ddata);
+	dev_set_drvdata(dev, ddata);
 	input_set_drvdata(input, ddata);
 
 	input->name = pdata->name ? : pdev->name;
 	input->phys = "gpio-keys/input0";
-	input->dev.parent = &pdev->dev;
+	input->dev.parent = dev;
 	input->open = gpio_keys_open;
 	input->close = gpio_keys_close;
 
@@ -496,7 +493,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 		bdata->input = input;
 		bdata->button = button;
 
-		error = gpio_keys_setup_key(pdev, bdata, button);
+		error = gpio_keys_setup_key(dev, bdata, button);
 		if (error)
 			goto fail2;
 
@@ -506,7 +503,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 		input_set_capability(input, type, button->code);
 	}
 
-	error = sysfs_create_group(&pdev->dev.kobj, &gpio_keys_attr_group);
+	error = sysfs_create_group(&dev->kobj, &gpio_keys_attr_group);
 	if (error) {
 		dev_err(dev, "Unable to export keys/switches, error: %d\n",
 			error);
@@ -525,12 +522,12 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 		gpio_keys_report_event(&ddata->data[i]);
 	input_sync(input);
 
-	device_init_wakeup(&pdev->dev, wakeup);
+	device_init_wakeup(dev, wakeup);
 
 	return 0;
 
  fail3:
-	sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group);
+	sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group);
  fail2:
 	while (--i >= 0) {
 		free_irq(gpio_to_irq(pdata->buttons[i].gpio), &ddata->data[i]);
@@ -540,7 +537,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 		gpio_free(pdata->buttons[i].gpio);
 	}
 
-	platform_set_drvdata(pdev, NULL);
+	dev_set_drvdata(dev, NULL);
  fail1:
 	input_free_device(input);
 	kfree(ddata);
@@ -550,14 +547,15 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 
 static int __devexit gpio_keys_remove(struct platform_device *pdev)
 {
-	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
-	struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+	struct gpio_keys_platform_data *pdata = dev->platform_data;
+	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
 	struct input_dev *input = ddata->input;
 	int i;
 
-	sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group);
+	sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group);
 
-	device_init_wakeup(&pdev->dev, 0);
+	device_init_wakeup(dev, 0);
 
 	for (i = 0; i < pdata->nbuttons; i++) {
 		int irq = gpio_to_irq(pdata->buttons[i].gpio);
@@ -577,11 +575,10 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM
 static int gpio_keys_suspend(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
+	struct gpio_keys_platform_data *pdata = dev->platform_data;
 	int i;
 
-	if (device_may_wakeup(&pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		for (i = 0; i < pdata->nbuttons; i++) {
 			struct gpio_keys_button *button = &pdata->buttons[i];
 			if (button->wakeup) {
@@ -596,15 +593,14 @@ static int gpio_keys_suspend(struct device *dev)
 
 static int gpio_keys_resume(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev);
-	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
+	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
+	struct gpio_keys_platform_data *pdata = dev->platform_data;
 	int i;
 
 	for (i = 0; i < pdata->nbuttons; i++) {
 
 		struct gpio_keys_button *button = &pdata->buttons[i];
-		if (button->wakeup && device_may_wakeup(&pdev->dev)) {
+		if (button->wakeup && device_may_wakeup(dev)) {
 			int irq = gpio_to_irq(button->gpio);
 			disable_irq_wake(irq);
 		}
-- 
1.7.4.1

[PATCH 2/3] Input: gpio_keys.c: Added support for device-tree platform data

[David Jander]

Signed-off-by: David Jander <david@protonic.nl>
---
 .../devicetree/bindings/gpio/gpio_keys.txt         |   49 ++++++++
 drivers/input/keyboard/gpio_keys.c                 |  131 ++++++++++++++++++++
 2 files changed, 180 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_keys.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio_keys.txt b/Documentation/devicetree/bindings/gpio/gpio_keys.txt
new file mode 100644
index 0000000..ff8ba2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio_keys.txt
@@ -0,0 +1,49 @@
+Device-Tree bindings for input/gpio_keys.c keyboard driver
+
+Required properties:
+	- compatible = "linux,gpio-keys", "gpio-keys";
+
+Optional properties:
+	- linux,autorepeat: Boolean, Enable auto repeat feature of Linux input
+	  subsystem.
+
+Each button (key) is represented as a sub-node of "gpio-keys":
+Subnode properties:
+
+	- reg: GPIO number the key is bound to.
+	- label: Descriptive name of the key.
+	- linux,code: Keycode to emit.
+
+Optional subnode-properties:
+	- linux,active_low: Boolean flag to specify active-low GPIO input.
+	- linux,input_type: Specify event type this button/key generates.
+	  Default if unspecified is <1> == EV_KEY.
+	- linux,debounc_interval: Debouncing interval time in milliseconds.
+	  Default if unspecified is 5.
+	- linux,can_disable: Boolean, specify if key can be disabled via
+	  module parameter.
+	- linux,wakeup: Boolean, button can wake-up the system.
+
+Example nodes:
+
+	gpio_keys {
+			compatible = "linux,gpio-keys", "gpio-keys";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			linux,autorepeat;
+			button@20 {
+				label = "GPIO Key ESC";
+				linux,code = <1>;
+				reg = <0x20>;
+				linux,active_low;
+				linux,input_type = <1>;
+			};
+			button@21 {
+				label = "GPIO Key UP";
+				linux,code = <103>;
+				reg = <0x21>;
+				linux,active_low;
+				linux,input_type = <1>;
+			};
+			...
+
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 987498e..4efd067 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -3,6 +3,9 @@
  *
  * Copyright 2005 Phil Blundell
  *
+ * Added OF support:
+ * Copyright 2010 David Jander <david@protonic.nl>
+ *
  * 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.
@@ -25,6 +28,8 @@
 #include <linux/gpio_keys.h>
 #include <linux/workqueue.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 
 struct gpio_button_data {
 	struct gpio_keys_button *button;
@@ -42,6 +47,7 @@ struct gpio_keys_drvdata {
 	int (*enable)(struct device *dev);
 	void (*disable)(struct device *dev);
 	struct gpio_button_data data[0];
+	struct gpio_keys_platform_data *dyn_pdata;
 };
 
 /*
@@ -442,18 +448,131 @@ static void gpio_keys_close(struct input_dev *input)
 		ddata->disable(input->dev.parent);
 }
 
+/*
+ * Handlers for alternative sources of platform_data
+ */
+#ifdef CONFIG_OF
+/*
+ * Translate OpenFirmware node properties into platform_data
+ */
+static struct gpio_keys_platform_data *
+gpio_keys_get_devtree_pdata(struct device *dev)
+{
+	struct gpio_keys_platform_data *pdata;
+	struct device_node *node, *pp;
+	int i;
+	struct gpio_keys_button *buttons;
+	const u32 *reg;
+	int len;
+
+	node = dev->of_node;
+	if (node == NULL)
+		return NULL;
+
+	pdata = kzalloc(sizeof(struct gpio_keys_platform_data), GFP_KERNEL);
+	if (pdata == NULL) {
+		dev_err(dev, "Unable to allocate platform_data\n");
+		return NULL;
+	}
+
+	pdata->rep = !!of_get_property(node, "linux,autorepeat", &len);
+
+	/* First count the subnodes */
+	pdata->nbuttons = 0;
+	pp = NULL;
+	while ((pp = of_get_next_child(node, pp)))
+		pdata->nbuttons++;
+
+	if (pdata->nbuttons == 0)
+		return NULL;
+
+	buttons = kzalloc(pdata->nbuttons * sizeof(*buttons), GFP_KERNEL);
+	if (!buttons)
+		return NULL;
+
+	pp = NULL;
+	i = 0;
+	while ((pp = of_get_next_child(node, pp))) {
+		const char *lbl;
+
+		reg = of_get_property(pp, "reg", &len);
+		if (!reg) {
+			pdata->nbuttons--;
+			printk("Found button without reg\n");
+			continue;
+		}
+		buttons[i].gpio = reg[0];
+
+		reg = of_get_property(pp, "linux,code", &len);
+		if (!reg) {
+			dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
+			return NULL;
+		}
+		buttons[i].code = reg[0];
+
+		lbl = of_get_property(pp, "label", &len);
+		buttons[i].desc = (char *)lbl;
+
+		buttons[i].active_low = !!of_get_property(pp, "linux,active_low", NULL);
+
+		reg = of_get_property(pp, "linux,input_type", &len);
+		if (reg)
+			buttons[i].type = reg[0];
+		else
+			buttons[i].type = EV_KEY;
+
+		buttons[i].wakeup = !!of_get_property(pp, "linux,wakeup", NULL);
+
+		reg = of_get_property(pp, "linux,debounc_interval", &len);
+		if (reg)
+			buttons[i].debounce_interval = reg[0];
+		else
+			buttons[i].debounce_interval = 5;
+
+		buttons[i].can_disable = (bool)!!of_get_property(pp, "linux,can_disable", NULL);
+		i++;
+	}
+
+	pdata->buttons = buttons;
+
+	return pdata;
+}
+#else
+static struct gpio_keys_platform_data *
+gpio_keys_get_devtree_pdata(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int __devinit gpio_keys_probe(struct platform_device *pdev)
 {
 	struct gpio_keys_drvdata *ddata;
 	struct device *dev = &pdev->dev;
 	struct gpio_keys_platform_data *pdata = dev->platform_data;
+	struct gpio_keys_platform_data *dyn_pdata = NULL;
 	struct input_dev *input;
 	int i, error;
 	int wakeup = 0;
 
+	if (!pdata) {
+		pdata = gpio_keys_get_devtree_pdata(dev);
+		if (!pdata)
+			return -ENODEV;
+		/*
+		 * Unlike normal platform_data, this is allocated
+		 * dynamically and must be freed in the driver
+		 */
+		dev->platform_data = pdata;
+		dyn_pdata = pdata;
+	}
+
 	ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
 			pdata->nbuttons * sizeof(struct gpio_button_data),
 			GFP_KERNEL);
+
+	ddata->dyn_pdata = dyn_pdata;
+
 	input = input_allocate_device();
 	if (!ddata || !input) {
 		dev_err(dev, "failed to allocate state\n");
@@ -565,12 +684,21 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
 		cancel_work_sync(&ddata->data[i].work);
 		gpio_free(pdata->buttons[i].gpio);
 	}
+	if (ddata->dyn_pdata) {
+		kfree(pdata->buttons);
+		kfree(pdata);
+	}
 
 	input_unregister_device(input);
 
 	return 0;
 }
 
+static struct of_device_id gpio_keys_of_match[] = {
+	{ .compatible = "linux,gpio-keys", },
+	{ .compatible = "gpio-keys", },
+	{},
+};
 
 #ifdef CONFIG_PM
 static int gpio_keys_suspend(struct device *dev)
@@ -618,6 +746,8 @@ static const struct dev_pm_ops gpio_keys_pm_ops = {
 };
 #endif
 
+MODULE_DEVICE_TABLE(of, gpio_keys_of_match);
+
 static struct platform_driver gpio_keys_device_driver = {
 	.probe		= gpio_keys_probe,
 	.remove		= __devexit_p(gpio_keys_remove),
@@ -627,6 +757,7 @@ static struct platform_driver gpio_keys_device_driver = {
 #ifdef CONFIG_PM
 		.pm	= &gpio_keys_pm_ops,
 #endif
+		.of_match_table = gpio_keys_of_match,
 	}
 };
 
-- 
1.7.4.1

[PATCH 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

[David Jander]

Use a threaded interrupt handler in order to permit the handler to use
a GPIO driver that causes things like I2C transactions being done inside
the handler context.
Also, gpio_keys_init needs to be declared as a late_initcall, to make sure
all needed GPIO drivers have been loaded if the drivers are built into the
kernel.

Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/input/keyboard/gpio_keys.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 4efd067..bfbdfe8 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -3,7 +3,7 @@
  *
  * Copyright 2005 Phil Blundell
  *
- * Added OF support:
+ * Added OF support and enabled use with I2C GPIO expanders:
  * Copyright 2010 David Jander <david@protonic.nl>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -418,7 +418,7 @@ static int __devinit gpio_keys_setup_key(struct device *dev,
 	if (!button->can_disable)
 		irqflags |= IRQF_SHARED;
 
-	error = request_any_context_irq(irq, gpio_keys_isr, irqflags, desc, bdata);
+	error = request_threaded_irq(irq, NULL, gpio_keys_isr, irqflags, desc, bdata);
 	if (error < 0) {
 		dev_err(dev, "Unable to claim irq %d; error %d\n",
 			irq, error);
@@ -771,10 +771,10 @@ static void __exit gpio_keys_exit(void)
 	platform_driver_unregister(&gpio_keys_device_driver);
 }
 
-module_init(gpio_keys_init);
+late_initcall(gpio_keys_init);
 module_exit(gpio_keys_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Phil Blundell <pb@handhelds.org>");
-MODULE_DESCRIPTION("Keyboard driver for CPU GPIOs");
+MODULE_DESCRIPTION("Keyboard driver for GPIOs");
 MODULE_ALIAS("platform:gpio-keys");
-- 
1.7.4.1

Re: [PATCH 1/3] Input: gpio_keys.c: Simplify platform_device -> device casting

[Grant Likely]

On Mon, Jun 6, 2011 at 9:12 AM, David Jander <david@protonic.nl> wrote:
> This patch factors out the use of struct platform_device *pdev in most
> places.
>
> Signed-off-by: David Jander <david@protonic.nl>

Looks right.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
>  drivers/input/keyboard/gpio_keys.c |   46 ++++++++++++++++-------------------
>  1 files changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 6e6145b..987498e 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -251,8 +251,7 @@ static ssize_t gpio_keys_show_##name(struct device *dev,            \
>                                     struct device_attribute *attr,     \
>                                     char *buf)                         \
>  {                                                                      \
> -       struct platform_device *pdev = to_platform_device(dev);         \
> -       struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev);   \
> +       struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);         \
>                                                                        \
>        return gpio_keys_attr_show_helper(ddata, buf,                   \
>                                          type, only_disabled);         \
> @@ -278,8 +277,7 @@ static ssize_t gpio_keys_store_##name(struct device *dev,           \
>                                      const char *buf,                  \
>                                      size_t count)                     \
>  {                                                                      \
> -       struct platform_device *pdev = to_platform_device(dev);         \
> -       struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev);   \
> +       struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);         \
>        ssize_t error;                                                  \
>                                                                        \
>        error = gpio_keys_attr_store_helper(ddata, buf, type);          \
> @@ -364,12 +362,11 @@ static irqreturn_t gpio_keys_isr(int irq, void *dev_id)
>        return IRQ_HANDLED;
>  }
>
> -static int __devinit gpio_keys_setup_key(struct platform_device *pdev,
> +static int __devinit gpio_keys_setup_key(struct device *dev,
>                                         struct gpio_button_data *bdata,
>                                         struct gpio_keys_button *button)
>  {
>        const char *desc = button->desc ? button->desc : "gpio_keys";
> -       struct device *dev = &pdev->dev;
>        unsigned long irqflags;
>        int irq, error;
>
> @@ -447,9 +444,9 @@ static void gpio_keys_close(struct input_dev *input)
>
>  static int __devinit gpio_keys_probe(struct platform_device *pdev)
>  {
> -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
>        struct gpio_keys_drvdata *ddata;
>        struct device *dev = &pdev->dev;
> +       struct gpio_keys_platform_data *pdata = dev->platform_data;
>        struct input_dev *input;
>        int i, error;
>        int wakeup = 0;
> @@ -470,12 +467,12 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
>        ddata->disable = pdata->disable;
>        mutex_init(&ddata->disable_lock);
>
> -       platform_set_drvdata(pdev, ddata);
> +       dev_set_drvdata(dev, ddata);
>        input_set_drvdata(input, ddata);
>
>        input->name = pdata->name ? : pdev->name;
>        input->phys = "gpio-keys/input0";
> -       input->dev.parent = &pdev->dev;
> +       input->dev.parent = dev;
>        input->open = gpio_keys_open;
>        input->close = gpio_keys_close;
>
> @@ -496,7 +493,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
>                bdata->input = input;
>                bdata->button = button;
>
> -               error = gpio_keys_setup_key(pdev, bdata, button);
> +               error = gpio_keys_setup_key(dev, bdata, button);
>                if (error)
>                        goto fail2;
>
> @@ -506,7 +503,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
>                input_set_capability(input, type, button->code);
>        }
>
> -       error = sysfs_create_group(&pdev->dev.kobj, &gpio_keys_attr_group);
> +       error = sysfs_create_group(&dev->kobj, &gpio_keys_attr_group);
>        if (error) {
>                dev_err(dev, "Unable to export keys/switches, error: %d\n",
>                        error);
> @@ -525,12 +522,12 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
>                gpio_keys_report_event(&ddata->data[i]);
>        input_sync(input);
>
> -       device_init_wakeup(&pdev->dev, wakeup);
> +       device_init_wakeup(dev, wakeup);
>
>        return 0;
>
>  fail3:
> -       sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group);
> +       sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group);
>  fail2:
>        while (--i >= 0) {
>                free_irq(gpio_to_irq(pdata->buttons[i].gpio), &ddata->data[i]);
> @@ -540,7 +537,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
>                gpio_free(pdata->buttons[i].gpio);
>        }
>
> -       platform_set_drvdata(pdev, NULL);
> +       dev_set_drvdata(dev, NULL);
>  fail1:
>        input_free_device(input);
>        kfree(ddata);
> @@ -550,14 +547,15 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
>
>  static int __devexit gpio_keys_remove(struct platform_device *pdev)
>  {
> -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> -       struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev);
> +       struct device *dev = &pdev->dev;
> +       struct gpio_keys_platform_data *pdata = dev->platform_data;
> +       struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
>        struct input_dev *input = ddata->input;
>        int i;
>
> -       sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group);
> +       sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group);
>
> -       device_init_wakeup(&pdev->dev, 0);
> +       device_init_wakeup(dev, 0);
>
>        for (i = 0; i < pdata->nbuttons; i++) {
>                int irq = gpio_to_irq(pdata->buttons[i].gpio);
> @@ -577,11 +575,10 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
>  #ifdef CONFIG_PM
>  static int gpio_keys_suspend(struct device *dev)
>  {
> -       struct platform_device *pdev = to_platform_device(dev);
> -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> +       struct gpio_keys_platform_data *pdata = dev->platform_data;
>        int i;
>
> -       if (device_may_wakeup(&pdev->dev)) {
> +       if (device_may_wakeup(dev)) {
>                for (i = 0; i < pdata->nbuttons; i++) {
>                        struct gpio_keys_button *button = &pdata->buttons[i];
>                        if (button->wakeup) {
> @@ -596,15 +593,14 @@ static int gpio_keys_suspend(struct device *dev)
>
>  static int gpio_keys_resume(struct device *dev)
>  {
> -       struct platform_device *pdev = to_platform_device(dev);
> -       struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev);
> -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> +       struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
> +       struct gpio_keys_platform_data *pdata = dev->platform_data;
>        int i;
>
>        for (i = 0; i < pdata->nbuttons; i++) {
>
>                struct gpio_keys_button *button = &pdata->buttons[i];
> -               if (button->wakeup && device_may_wakeup(&pdev->dev)) {
> +               if (button->wakeup && device_may_wakeup(dev)) {
>                        int irq = gpio_to_irq(button->gpio);
>                        disable_irq_wake(irq);
>                }
> --
> 1.7.4.1
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
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 2/3] Input: gpio_keys.c: Added support for device-tree platform data

[Grant Likely]

On Mon, Jun 6, 2011 at 9:12 AM, David Jander <david@protonic.nl> wrote:
> Signed-off-by: David Jander <david@protonic.nl>
> ---
>  .../devicetree/bindings/gpio/gpio_keys.txt         |   49 ++++++++
>  drivers/input/keyboard/gpio_keys.c                 |  131 ++++++++++++++++++++
>  2 files changed, 180 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio_keys.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio_keys.txt b/Documentation/devicetree/bindings/gpio/gpio_keys.txt
> new file mode 100644
> index 0000000..ff8ba2e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio_keys.txt
> @@ -0,0 +1,49 @@
> +Device-Tree bindings for input/gpio_keys.c keyboard driver
> +
> +Required properties:
> +       - compatible = "linux,gpio-keys", "gpio-keys";

Drop linux,gpio-keys

> +
> +Optional properties:
> +       - linux,autorepeat: Boolean, Enable auto repeat feature of Linux input
> +         subsystem.

Drop the linux specific bits.  Define the properties you need to
describe the behaviour or the keypad without making it linux-specific.
 The binding should make sense regardless of either future changes to
the driver, or when using a different OS.


> +
> +Each button (key) is represented as a sub-node of "gpio-keys":
> +Subnode properties:
> +
> +       - reg: GPIO number the key is bound to.
> +       - label: Descriptive name of the key.
> +       - linux,code: Keycode to emit.

Hmm, is there an OS-neutral way to specify this?  There may not be,
but if there is then it should be looked at seriously.

> +
> +Optional subnode-properties:
> +       - linux,active_low: Boolean flag to specify active-low GPIO input.

This isn't really a linux-specific thing.  Perhaps use the name
"key-active-low".

Also, use '-' instead of '_' in property names.

> +       - linux,input_type: Specify event type this button/key generates.
> +         Default if unspecified is <1> == EV_KEY.


> +       - linux,debounc_interval: Debouncing interval time in milliseconds.
> +         Default if unspecified is 5.

Debounce is spelled with 2 'e's.  :-)  Also not linux specific.
Should be renamed.  It would probably make sense to also allow a
debounce intervale property in the parent node.  I suspect that a
single debounce interval value will simply be used by all keys in a
gpio-keys block.

> +       - linux,can_disable: Boolean, specify if key can be disabled via
> +         module parameter.

Is this really needed?  Or even a good idea?  It looks to be encoding
a driver-specific implementation detail that could potentially change
in the future.

> +       - linux,wakeup: Boolean, button can wake-up the system.

Again, not linux-specific.

> +
> +Example nodes:
> +
> +       gpio_keys {
> +                       compatible = "linux,gpio-keys", "gpio-keys";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       linux,autorepeat;
> +                       button@20 {
> +                               label = "GPIO Key ESC";
> +                               linux,code = <1>;
> +                               reg = <0x20>;
> +                               linux,active_low;
> +                               linux,input_type = <1>;
> +                       };
> +                       button@21 {
> +                               label = "GPIO Key UP";
> +                               linux,code = <103>;
> +                               reg = <0x21>;
> +                               linux,active_low;
> +                               linux,input_type = <1>;
> +                       };
> +                       ...
> +
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 987498e..4efd067 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -3,6 +3,9 @@
>  *
>  * Copyright 2005 Phil Blundell
>  *
> + * Added OF support:
> + * Copyright 2010 David Jander <david@protonic.nl>
> + *
>  * 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.
> @@ -25,6 +28,8 @@
>  #include <linux/gpio_keys.h>
>  #include <linux/workqueue.h>
>  #include <linux/gpio.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
>
>  struct gpio_button_data {
>        struct gpio_keys_button *button;
> @@ -42,6 +47,7 @@ struct gpio_keys_drvdata {
>        int (*enable)(struct device *dev);
>        void (*disable)(struct device *dev);
>        struct gpio_button_data data[0];
> +       struct gpio_keys_platform_data *dyn_pdata;
>  };
>
>  /*
> @@ -442,18 +448,131 @@ static void gpio_keys_close(struct input_dev *input)
>                ddata->disable(input->dev.parent);
>  }
>
> +/*
> + * Handlers for alternative sources of platform_data
> + */
> +#ifdef CONFIG_OF
> +/*
> + * Translate OpenFirmware node properties into platform_data
> + */
> +static struct gpio_keys_platform_data *
> +gpio_keys_get_devtree_pdata(struct device *dev)
> +{
> +       struct gpio_keys_platform_data *pdata;
> +       struct device_node *node, *pp;
> +       int i;
> +       struct gpio_keys_button *buttons;
> +       const u32 *reg;
> +       int len;
> +
> +       node = dev->of_node;
> +       if (node == NULL)
> +               return NULL;
> +
> +       pdata = kzalloc(sizeof(struct gpio_keys_platform_data), GFP_KERNEL);
> +       if (pdata == NULL) {
> +               dev_err(dev, "Unable to allocate platform_data\n");
> +               return NULL;
> +       }
> +
> +       pdata->rep = !!of_get_property(node, "linux,autorepeat", &len);
> +
> +       /* First count the subnodes */
> +       pdata->nbuttons = 0;
> +       pp = NULL;
> +       while ((pp = of_get_next_child(node, pp)))
> +               pdata->nbuttons++;
> +
> +       if (pdata->nbuttons == 0)
> +               return NULL;
> +
> +       buttons = kzalloc(pdata->nbuttons * sizeof(*buttons), GFP_KERNEL);
> +       if (!buttons)
> +               return NULL;
> +
> +       pp = NULL;
> +       i = 0;
> +       while ((pp = of_get_next_child(node, pp))) {
> +               const char *lbl;
> +
> +               reg = of_get_property(pp, "reg", &len);
> +               if (!reg) {
> +                       pdata->nbuttons--;
> +                       printk("Found button without reg\n");
> +                       continue;
> +               }
> +               buttons[i].gpio = reg[0];
> +
> +               reg = of_get_property(pp, "linux,code", &len);
> +               if (!reg) {
> +                       dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
> +                       return NULL;
> +               }
> +               buttons[i].code = reg[0];
> +
> +               lbl = of_get_property(pp, "label", &len);
> +               buttons[i].desc = (char *)lbl;
> +
> +               buttons[i].active_low = !!of_get_property(pp, "linux,active_low", NULL);
> +
> +               reg = of_get_property(pp, "linux,input_type", &len);
> +               if (reg)
> +                       buttons[i].type = reg[0];

You need to do an endian conversion on device tree data.  be32_to_cpup(reg);

> +               else
> +                       buttons[i].type = EV_KEY;
> +
> +               buttons[i].wakeup = !!of_get_property(pp, "linux,wakeup", NULL);
> +
> +               reg = of_get_property(pp, "linux,debounc_interval", &len);
> +               if (reg)
> +                       buttons[i].debounce_interval = reg[0];
> +               else
> +                       buttons[i].debounce_interval = 5;
> +
> +               buttons[i].can_disable = (bool)!!of_get_property(pp, "linux,can_disable", NULL);
> +               i++;
> +       }
> +
> +       pdata->buttons = buttons;
> +
> +       return pdata;
> +}
> +#else
> +static struct gpio_keys_platform_data *
> +gpio_keys_get_devtree_pdata(struct device *dev)
> +{
> +       return NULL;
> +}
> +#endif
> +
>  static int __devinit gpio_keys_probe(struct platform_device *pdev)
>  {
>        struct gpio_keys_drvdata *ddata;
>        struct device *dev = &pdev->dev;
>        struct gpio_keys_platform_data *pdata = dev->platform_data;
> +       struct gpio_keys_platform_data *dyn_pdata = NULL;
>        struct input_dev *input;
>        int i, error;
>        int wakeup = 0;
>
> +       if (!pdata) {
> +               pdata = gpio_keys_get_devtree_pdata(dev);
> +               if (!pdata)
> +                       return -ENODEV;
> +               /*
> +                * Unlike normal platform_data, this is allocated
> +                * dynamically and must be freed in the driver
> +                */
> +               dev->platform_data = pdata;
> +               dyn_pdata = pdata;
> +       }
> +
>        ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
>                        pdata->nbuttons * sizeof(struct gpio_button_data),
>                        GFP_KERNEL);
> +
> +       ddata->dyn_pdata = dyn_pdata;
> +
>        input = input_allocate_device();
>        if (!ddata || !input) {
>                dev_err(dev, "failed to allocate state\n");
> @@ -565,12 +684,21 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
>                cancel_work_sync(&ddata->data[i].work);
>                gpio_free(pdata->buttons[i].gpio);
>        }
> +       if (ddata->dyn_pdata) {
> +               kfree(pdata->buttons);
> +               kfree(pdata);
> +       }
>
>        input_unregister_device(input);
>
>        return 0;
>  }
>
> +static struct of_device_id gpio_keys_of_match[] = {
> +       { .compatible = "linux,gpio-keys", },
> +       { .compatible = "gpio-keys", },

You don't need both values here.  "gpio-keys" is fine.

> +       {},
> +};
>
>  #ifdef CONFIG_PM
>  static int gpio_keys_suspend(struct device *dev)
> @@ -618,6 +746,8 @@ static const struct dev_pm_ops gpio_keys_pm_ops = {
>  };
>  #endif
>
> +MODULE_DEVICE_TABLE(of, gpio_keys_of_match);
> +
>  static struct platform_driver gpio_keys_device_driver = {
>        .probe          = gpio_keys_probe,
>        .remove         = __devexit_p(gpio_keys_remove),
> @@ -627,6 +757,7 @@ static struct platform_driver gpio_keys_device_driver = {
>  #ifdef CONFIG_PM
>                .pm     = &gpio_keys_pm_ops,
>  #endif
> +               .of_match_table = gpio_keys_of_match,
>        }
>  };
>
> --
> 1.7.4.1
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
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 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

[Grant Likely]

On Mon, Jun 6, 2011 at 9:12 AM, David Jander <david@protonic.nl> wrote:
> Use a threaded interrupt handler in order to permit the handler to use
> a GPIO driver that causes things like I2C transactions being done inside
> the handler context.
> Also, gpio_keys_init needs to be declared as a late_initcall, to make sure
> all needed GPIO drivers have been loaded if the drivers are built into the
> kernel.
>
> Signed-off-by: David Jander <david@protonic.nl>

Looks okay to me.  Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
>  drivers/input/keyboard/gpio_keys.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 4efd067..bfbdfe8 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -3,7 +3,7 @@
>  *
>  * Copyright 2005 Phil Blundell
>  *
> - * Added OF support:
> + * Added OF support and enabled use with I2C GPIO expanders:
>  * Copyright 2010 David Jander <david@protonic.nl>
>  *
>  * This program is free software; you can redistribute it and/or modify
> @@ -418,7 +418,7 @@ static int __devinit gpio_keys_setup_key(struct device *dev,
>        if (!button->can_disable)
>                irqflags |= IRQF_SHARED;
>
> -       error = request_any_context_irq(irq, gpio_keys_isr, irqflags, desc, bdata);
> +       error = request_threaded_irq(irq, NULL, gpio_keys_isr, irqflags, desc, bdata);
>        if (error < 0) {
>                dev_err(dev, "Unable to claim irq %d; error %d\n",
>                        irq, error);
> @@ -771,10 +771,10 @@ static void __exit gpio_keys_exit(void)
>        platform_driver_unregister(&gpio_keys_device_driver);
>  }
>
> -module_init(gpio_keys_init);
> +late_initcall(gpio_keys_init);
>  module_exit(gpio_keys_exit);
>
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Phil Blundell <pb@handhelds.org>");
> -MODULE_DESCRIPTION("Keyboard driver for CPU GPIOs");
> +MODULE_DESCRIPTION("Keyboard driver for GPIOs");
>  MODULE_ALIAS("platform:gpio-keys");
> --
> 1.7.4.1
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
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 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

[David Jander]

Use a threaded interrupt handler in order to permit the handler to use
a GPIO driver that causes things like I2C transactions being done inside
the handler context.
Also, gpio_keys_init needs to be declared as a late_initcall, to make sure
all needed GPIO drivers have been loaded if the drivers are built into the
kernel.

Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/input/keyboard/gpio_keys.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 4539b0b..bbb4e4d 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -3,7 +3,7 @@
  *
  * Copyright 2005 Phil Blundell
  *
- * Added OF support:
+ * Added OF support and enabled use with I2C GPIO expanders:
  * Copyright 2010 David Jander <david@protonic.nl>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -418,7 +418,7 @@ static int __devinit gpio_keys_setup_key(struct device *dev,
 	if (!button->can_disable)
 		irqflags |= IRQF_SHARED;
 
-	error = request_any_context_irq(irq, gpio_keys_isr, irqflags, desc, bdata);
+	error = request_threaded_irq(irq, NULL, gpio_keys_isr, irqflags, desc, bdata);
 	if (error < 0) {
 		dev_err(dev, "Unable to claim irq %d; error %d\n",
 			irq, error);
@@ -770,10 +770,10 @@ static void __exit gpio_keys_exit(void)
 	platform_driver_unregister(&gpio_keys_device_driver);
 }
 
-module_init(gpio_keys_init);
+late_initcall(gpio_keys_init);
 module_exit(gpio_keys_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Phil Blundell <pb@handhelds.org>");
-MODULE_DESCRIPTION("Keyboard driver for CPU GPIOs");
+MODULE_DESCRIPTION("Keyboard driver for GPIOs");
 MODULE_ALIAS("platform:gpio-keys");
-- 
1.7.4.1