[PATCH v5 0/2] Input: gpio_keys.c: Add support for OF and I2C GPIO chips

[PATCH v5 0/2] Input: gpio_keys.c: Add support for OF and I2C GPIO chips

[David Jander]

This patch series adds support for OF device-tree platform data and non-local
GPIO chips, such as I2C I/O expanders.
This is version 5 of the patches. Feedback from Grant Likely and Dmitry
Torokhov has been processed.

Changed in this version:
 - Removed first patch: Simplify platform_device -> device casting.
   It is not necessary anymore since of_platform bus has been merged into
   platform_bus.
 - Fixed copyright message.
 - Removed support for numeric GPIO specifiers in "reg" property.
 - Renamed "wakeup" property.
 - Improved error handling of gpio_keys_get_devtree_pdata().
 - Misc cleanups.

David Jander (2):
  Input: gpio_keys.c: Added support for device-tree platform data
  Input: gpio_keys.c: Enable use with non-local GPIO chips.

 .../devicetree/bindings/gpio/gpio_keys.txt         |   37 +++++
 drivers/input/keyboard/gpio_keys.c                 |  152 +++++++++++++++++---
 2 files changed, 171 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_keys.txt

-- 
1.7.4.1

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

[David Jander]

This patch enables fetching configuration data which is normally provided via
platform_data from the device-tree instead.
If the device is configured from device-tree data, the platform_data struct
is not used, and button data needs to be allocated dynamically.
Big part of this patch deals with confining pdata usage to the probe function,
to make this possible.

Signed-off-by: David Jander <david@protonic.nl>
---
 .../devicetree/bindings/gpio/gpio_keys.txt         |   37 +++++
 drivers/input/keyboard/gpio_keys.c                 |  146 ++++++++++++++++++--
 2 files changed, 168 insertions(+), 15 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..a0fed97
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio_keys.txt
@@ -0,0 +1,37 @@
+Device-Tree bindings for input/gpio_keys.c keyboard driver
+
+Required properties:
+	- compatible = "gpio-keys";
+
+Optional properties:
+	- autorepeat: Boolean, Enable auto repeat feature of Linux input
+	  subsystem.
+
+Each button (key) is represented as a sub-node of "gpio-keys":
+Subnode properties:
+
+	- gpios: OF devcie-tree gpio specificatin.
+	- label: Descriptive name of the key.
+	- linux,code: Keycode to emit.
+
+Optional subnode-properties:
+	- linux,input-type: Specify event type this button/key generates.
+	  Default if unspecified is <1> == EV_KEY.
+	- debounce-interval: Debouncing interval time in milliseconds.
+	  Default if unspecified is 5.
+	- gpio-key,wakeup: Boolean, button can wake-up the system.
+
+Example nodes:
+
+	gpio_keys {
+			compatible = "gpio-keys";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			autorepeat;
+			button@21 {
+				label = "GPIO Key UP";
+				linux,code = <103>;
+				gpios = <&gpio1 0 1>;
+			};
+			...
+
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 6e6145b..2db7db2 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, 2011 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_platform.h>
+#include <linux/of_gpio.h>
 
 struct gpio_button_data {
 	struct gpio_keys_button *button;
@@ -445,15 +450,111 @@ 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 int 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 -ENODEV;
+
+	memset(pdata, 0, sizeof *pdata);
+
+	pdata->rep = !!of_get_property(node, "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 -ENODEV;
+
+	buttons = kzalloc(pdata->nbuttons * (sizeof *buttons), GFP_KERNEL);
+	if (!buttons)
+		return -ENODEV;
+
+	pp = NULL;
+	i = 0;
+	while ((pp = of_get_next_child(node, pp))) {
+		const char *lbl;
+		enum of_gpio_flags flags;
+
+		if (!of_find_property(pp, "gpios", NULL)) {
+			pdata->nbuttons--;
+			dev_warn(dev, "Found button without gpios\n");
+			continue;
+		}
+		buttons[i].gpio = of_get_gpio_flags(pp, 0, &flags);
+		buttons[i].active_low = !!(flags & OF_GPIO_ACTIVE_LOW);
+
+		reg = of_get_property(pp, "linux,code", &len);
+		if (!reg) {
+			dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
+			goto out_fail;
+		}
+		buttons[i].code = be32_to_cpup(reg);
+
+		lbl = of_get_property(pp, "label", &len);
+		buttons[i].desc = (char *)lbl;
+
+		reg = of_get_property(pp, "linux,input-type", &len);
+		buttons[i].type = reg ? be32_to_cpup(reg) : EV_KEY;
+
+		buttons[i].wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
+
+		reg = of_get_property(pp, "debounce-interval", &len);
+		buttons[i].debounce_interval = reg ? be32_to_cpup(reg) : 5;
+
+		i++;
+	}
+
+	pdata->buttons = buttons;
+
+	return 0;
+
+out_fail:
+	kfree(buttons);
+	return -ENODEV;
+}
+#else
+static int gpio_keys_get_devtree_pdata(struct device *dev,
+			    struct gpio_keys_platform_data *altp)
+{
+	return -ENODEV;
+}
+#endif
+
 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 alt_pdata;
 	struct input_dev *input;
 	int i, error;
 	int wakeup = 0;
 
+	if (!pdata) {
+		error = gpio_keys_get_devtree_pdata(dev, &alt_pdata);
+		if (error)
+			return error;
+		pdata = &alt_pdata;
+	}
+
 	ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
 			pdata->nbuttons * sizeof(struct gpio_button_data),
 			GFP_KERNEL);
@@ -550,7 +651,6 @@ 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 input_dev *input = ddata->input;
 	int i;
@@ -559,31 +659,42 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
 
 	device_init_wakeup(&pdev->dev, 0);
 
-	for (i = 0; i < pdata->nbuttons; i++) {
-		int irq = gpio_to_irq(pdata->buttons[i].gpio);
+	for (i = 0; i < ddata->n_buttons; i++) {
+		int irq = gpio_to_irq(ddata->data[i].button->gpio);
 		free_irq(irq, &ddata->data[i]);
 		if (ddata->data[i].timer_debounce)
 			del_timer_sync(&ddata->data[i].timer);
 		cancel_work_sync(&ddata->data[i].work);
-		gpio_free(pdata->buttons[i].gpio);
+		gpio_free(ddata->data[i].button->gpio);
 	}
 
+	/*
+	 * If we had no platform_data, we allocated buttons dynamically, and
+	 * must free them here. ddata->data[0].button is the pointer to the
+	 * beginning of the allocated array.
+	 */
+	if (!pdev->dev.platform_data)
+		kfree(ddata->data[0].button);
+
 	input_unregister_device(input);
 
 	return 0;
 }
 
+static struct of_device_id gpio_keys_of_match[] = {
+	{ .compatible = "gpio-keys", },
+	{},
+};
 
 #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_drvdata *ddata = dev_get_drvdata(dev);
 	int i;
 
-	if (device_may_wakeup(&pdev->dev)) {
-		for (i = 0; i < pdata->nbuttons; i++) {
-			struct gpio_keys_button *button = &pdata->buttons[i];
+	if (device_may_wakeup(dev)) {
+		for (i = 0; i < ddata->n_buttons; i++) {
+			struct gpio_keys_button *button = ddata->data[i].button;
 			if (button->wakeup) {
 				int irq = gpio_to_irq(button->gpio);
 				enable_irq_wake(irq);
@@ -596,15 +707,13 @@ 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);
 	int i;
 
-	for (i = 0; i < pdata->nbuttons; i++) {
+	for (i = 0; i < ddata->n_buttons; i++) {
 
-		struct gpio_keys_button *button = &pdata->buttons[i];
-		if (button->wakeup && device_may_wakeup(&pdev->dev)) {
+		struct gpio_keys_button *button = ddata->data[i].button;
+		if (button->wakeup && device_may_wakeup(dev)) {
 			int irq = gpio_to_irq(button->gpio);
 			disable_irq_wake(irq);
 		}
@@ -622,6 +731,12 @@ static const struct dev_pm_ops gpio_keys_pm_ops = {
 };
 #endif
 
+#ifdef CONFIG_OF
+MODULE_DEVICE_TABLE(of, gpio_keys_of_match);
+#else
+#define gpio_keys_of_match NULL
+#endif
+
 static struct platform_driver gpio_keys_device_driver = {
 	.probe		= gpio_keys_probe,
 	.remove		= __devexit_p(gpio_keys_remove),
@@ -631,6 +746,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 v5 2/2] 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 2db7db2..689150c 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, 2011 David Jander <david@protonic.nl>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -420,7 +420,7 @@ static int __devinit gpio_keys_setup_key(struct platform_device *pdev,
 	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);
@@ -760,10 +760,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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[Grant Likely]

On Thu, Jun 23, 2011 at 4:04 AM, David Jander <david@protonic.nl> wrote:
> This patch enables fetching configuration data which is normally provided via
> platform_data from the device-tree instead.
> If the device is configured from device-tree data, the platform_data struct
> is not used, and button data needs to be allocated dynamically.
> Big part of this patch deals with confining pdata usage to the probe function,
> to make this possible.
>
> Signed-off-by: David Jander <david@protonic.nl>
> ---
>  .../devicetree/bindings/gpio/gpio_keys.txt         |   37 +++++
>  drivers/input/keyboard/gpio_keys.c                 |  146 ++++++++++++++++++--
>  2 files changed, 168 insertions(+), 15 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..a0fed97
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio_keys.txt
> @@ -0,0 +1,37 @@
> +Device-Tree bindings for input/gpio_keys.c keyboard driver
> +
> +Required properties:
> +       - compatible = "gpio-keys";
> +
> +Optional properties:
> +       - autorepeat: Boolean, Enable auto repeat feature of Linux input
> +         subsystem.
> +
> +Each button (key) is represented as a sub-node of "gpio-keys":
> +Subnode properties:
> +
> +       - gpios: OF devcie-tree gpio specificatin.
> +       - label: Descriptive name of the key.
> +       - linux,code: Keycode to emit.

The fact that this is the Linux internal keycode definitions still
makes me nervous.  Is there no existing standard for keycodes emitted
by keyboard devices?

> +
> +Optional subnode-properties:
> +       - linux,input-type: Specify event type this button/key generates.
> +         Default if unspecified is <1> == EV_KEY.
> +       - debounce-interval: Debouncing interval time in milliseconds.
> +         Default if unspecified is 5.
> +       - gpio-key,wakeup: Boolean, button can wake-up the system.
> +
> +Example nodes:
> +
> +       gpio_keys {
> +                       compatible = "gpio-keys";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       autorepeat;
> +                       button@21 {
> +                               label = "GPIO Key UP";
> +                               linux,code = <103>;
> +                               gpios = <&gpio1 0 1>;
> +                       };
> +                       ...
> +
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 6e6145b..2db7db2 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, 2011 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_platform.h>
> +#include <linux/of_gpio.h>
>
>  struct gpio_button_data {
>        struct gpio_keys_button *button;
> @@ -445,15 +450,111 @@ 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 int 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 -ENODEV;
> +
> +       memset(pdata, 0, sizeof *pdata);
> +
> +       pdata->rep = !!of_get_property(node, "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 -ENODEV;
> +
> +       buttons = kzalloc(pdata->nbuttons * (sizeof *buttons), GFP_KERNEL);
> +       if (!buttons)
> +               return -ENODEV;
> +
> +       pp = NULL;
> +       i = 0;
> +       while ((pp = of_get_next_child(node, pp))) {
> +               const char *lbl;
> +               enum of_gpio_flags flags;
> +
> +               if (!of_find_property(pp, "gpios", NULL)) {
> +                       pdata->nbuttons--;
> +                       dev_warn(dev, "Found button without gpios\n");
> +                       continue;
> +               }
> +               buttons[i].gpio = of_get_gpio_flags(pp, 0, &flags);
> +               buttons[i].active_low = !!(flags & OF_GPIO_ACTIVE_LOW);
> +
> +               reg = of_get_property(pp, "linux,code", &len);
> +               if (!reg) {
> +                       dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
> +                       goto out_fail;
> +               }
> +               buttons[i].code = be32_to_cpup(reg);
> +
> +               lbl = of_get_property(pp, "label", &len);
> +               buttons[i].desc = (char *)lbl;
> +
> +               reg = of_get_property(pp, "linux,input-type", &len);
> +               buttons[i].type = reg ? be32_to_cpup(reg) : EV_KEY;
> +
> +               buttons[i].wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
> +
> +               reg = of_get_property(pp, "debounce-interval", &len);
> +               buttons[i].debounce_interval = reg ? be32_to_cpup(reg) : 5;
> +
> +               i++;
> +       }
> +
> +       pdata->buttons = buttons;
> +
> +       return 0;
> +
> +out_fail:
> +       kfree(buttons);
> +       return -ENODEV;
> +}
> +#else
> +static int gpio_keys_get_devtree_pdata(struct device *dev,
> +                           struct gpio_keys_platform_data *altp)
> +{
> +       return -ENODEV;
> +}
> +#endif
> +
>  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 alt_pdata;
>        struct input_dev *input;
>        int i, error;
>        int wakeup = 0;
>
> +       if (!pdata) {
> +               error = gpio_keys_get_devtree_pdata(dev, &alt_pdata);
> +               if (error)
> +                       return error;
> +               pdata = &alt_pdata;
> +       }
> +
>        ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
>                        pdata->nbuttons * sizeof(struct gpio_button_data),
>                        GFP_KERNEL);
> @@ -550,7 +651,6 @@ 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 input_dev *input = ddata->input;
>        int i;
> @@ -559,31 +659,42 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
>
>        device_init_wakeup(&pdev->dev, 0);
>
> -       for (i = 0; i < pdata->nbuttons; i++) {
> -               int irq = gpio_to_irq(pdata->buttons[i].gpio);
> +       for (i = 0; i < ddata->n_buttons; i++) {
> +               int irq = gpio_to_irq(ddata->data[i].button->gpio);
>                free_irq(irq, &ddata->data[i]);
>                if (ddata->data[i].timer_debounce)
>                        del_timer_sync(&ddata->data[i].timer);
>                cancel_work_sync(&ddata->data[i].work);
> -               gpio_free(pdata->buttons[i].gpio);
> +               gpio_free(ddata->data[i].button->gpio);
>        }
>
> +       /*
> +        * If we had no platform_data, we allocated buttons dynamically, and
> +        * must free them here. ddata->data[0].button is the pointer to the
> +        * beginning of the allocated array.
> +        */
> +       if (!pdev->dev.platform_data)
> +               kfree(ddata->data[0].button);
> +
>        input_unregister_device(input);
>
>        return 0;
>  }
>
> +static struct of_device_id gpio_keys_of_match[] = {
> +       { .compatible = "gpio-keys", },
> +       {},
> +};

This gpio_keys_of_match[]...

> @@ -622,6 +731,12 @@ static const struct dev_pm_ops gpio_keys_pm_ops = {
>  };
>  #endif
>
> +#ifdef CONFIG_OF
> +MODULE_DEVICE_TABLE(of, gpio_keys_of_match);
> +#else
> +#define gpio_keys_of_match NULL
> +#endif

...belongs inside this #ifdef CONFIG_OF block.

In fact, there should only need to be one #ifdef CONFIG_OF/#else block
in this .c file.

g.
--
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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[Alan Cox]

> > +       - gpios: OF devcie-tree gpio specificatin.
> > +       - label: Descriptive name of the key.
> > +       - linux,code: Keycode to emit.
> 
> The fact that this is the Linux internal keycode definitions still
> makes me nervous.  Is there no existing standard for keycodes emitted
> by keyboard devices?

There is but no standard lookup table. For Intel MID we do a translation
between Linux key names in the firmware and keycodes but there isn't a
generic helper for it.

--
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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[Grant Likely]

On Thu, Jun 23, 2011 at 6:39 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> > +       - gpios: OF devcie-tree gpio specificatin.
>> > +       - label: Descriptive name of the key.
>> > +       - linux,code: Keycode to emit.
>>
>> The fact that this is the Linux internal keycode definitions still
>> makes me nervous.  Is there no existing standard for keycodes emitted
>> by keyboard devices?
>
> There is but no standard lookup table. For Intel MID we do a translation
> between Linux key names in the firmware and keycodes but there isn't a
> generic helper for it.

I suppose the Linux keycodes are exported out to userspace, and are
therefore an ABI which will not change.  Okay.

g.
--
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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[David Jander]

On Thu, 23 Jun 2011 07:25:37 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> On Thu, Jun 23, 2011 at 6:39 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >> > +       - gpios: OF devcie-tree gpio specificatin.
> >> > +       - label: Descriptive name of the key.
> >> > +       - linux,code: Keycode to emit.
> >>
> >> The fact that this is the Linux internal keycode definitions still
> >> makes me nervous.  Is there no existing standard for keycodes emitted
> >> by keyboard devices?
> >
> > There is but no standard lookup table. For Intel MID we do a translation
> > between Linux key names in the firmware and keycodes but there isn't a
> > generic helper for it.
> 
> I suppose the Linux keycodes are exported out to userspace, and are
> therefore an ABI which will not change.  Okay.

Right.
If I am not mistaken, it started out being AT keyboard scancodes one day, but
nowadays it is a huge (and very useful) list of all possible keyboard-like
input events that can exist in linux. We use one such handy niche-code:
CYCLE_WINDOWS. We couldn't use this code if we were forced to emulate some
existing keyboard which probably wouldn't have this nifty key. It just fits
perfectly the intended function of the key. I have openbox configured to
handle it more or less like ALT+TAB.

Best regards,

-- 
David Jander
Protonic Holland.
--
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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[Dmitry Torokhov]

On Thu, Jun 23, 2011 at 07:25:37AM -0600, Grant Likely wrote:
> On Thu, Jun 23, 2011 at 6:39 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >> > +       - gpios: OF devcie-tree gpio specificatin.
> >> > +       - label: Descriptive name of the key.
> >> > +       - linux,code: Keycode to emit.
> >>
> >> The fact that this is the Linux internal keycode definitions still
> >> makes me nervous.  Is there no existing standard for keycodes emitted
> >> by keyboard devices?
> >
> > There is but no standard lookup table. For Intel MID we do a translation
> > between Linux key names in the firmware and keycodes but there isn't a
> > generic helper for it.
> 
> I suppose the Linux keycodes are exported out to userspace, and are
> therefore an ABI which will not change.  Okay.

Right, keycodes form ABI that will not change.

Another option would be to use codes from HID usage tables but then
they would have to be translated to Linux ones.

-- 
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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[David Jander]

On Thu, 23 Jun 2011 11:01:22 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Thu, Jun 23, 2011 at 07:25:37AM -0600, Grant Likely wrote:
> > On Thu, Jun 23, 2011 at 6:39 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > >> > +       - gpios: OF devcie-tree gpio specificatin.
> > >> > +       - label: Descriptive name of the key.
> > >> > +       - linux,code: Keycode to emit.
> > >>
> > >> The fact that this is the Linux internal keycode definitions still
> > >> makes me nervous.  Is there no existing standard for keycodes emitted
> > >> by keyboard devices?
> > >
> > > There is but no standard lookup table. For Intel MID we do a translation
> > > between Linux key names in the firmware and keycodes but there isn't a
> > > generic helper for it.
> > 
> > I suppose the Linux keycodes are exported out to userspace, and are
> > therefore an ABI which will not change.  Okay.
> 
> Right, keycodes form ABI that will not change.
> 
> Another option would be to use codes from HID usage tables but then
> they would have to be translated to Linux ones.

Dmitry, will you accept this patch also?
Until now, part 2/2 is in your tree, thanks for that, but I'd like to know if
this part (1/2) will also be accepted?

Best regards,

-- 
David Jander
Protonic Holland.
--
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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[Dmitry Torokhov]

David Jander <david.jander@protonic.nl> wrote:

>On Thu, 23 Jun 2011 11:01:22 -0700
>Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
>> On Thu, Jun 23, 2011 at 07:25:37AM -0600, Grant Likely wrote:
>> > On Thu, Jun 23, 2011 at 6:39 AM, Alan Cox
><alan@lxorguk.ukuu.org.uk> wrote:
>> > >> > +       - gpios: OF devcie-tree gpio specificatin.
>> > >> > +       - label: Descriptive name of the key.
>> > >> > +       - linux,code: Keycode to emit.
>> > >>
>> > >> The fact that this is the Linux internal keycode definitions
>still
>> > >> makes me nervous.  Is there no existing standard for keycodes
>emitted
>> > >> by keyboard devices?
>> > >
>> > > There is but no standard lookup table. For Intel MID we do a
>translation
>> > > between Linux key names in the firmware and keycodes but there
>isn't a
>> > > generic helper for it.
>> > 
>> > I suppose the Linux keycodes are exported out to userspace, and are
>> > therefore an ABI which will not change.  Okay.
>> 
>> Right, keycodes form ABI that will not change.
>> 
>> Another option would be to use codes from HID usage tables but then
>> they would have to be translated to Linux ones.
>
>Dmitry, will you accept this patch also?
>Until now, part 2/2 is in your tree, thanks for that, but I'd like to
>know if
>this part (1/2) will also be accepted?
>

Yes, I will since there was no more discussion about hid codes and I do believe that using linux definitions is fine.


-- 
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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[David Jander]

On Mon, 04 Jul 2011 10:52:55 +0400
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> David Jander <david.jander@protonic.nl> wrote:
> 
> >On Thu, 23 Jun 2011 11:01:22 -0700
> >Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >
> >> On Thu, Jun 23, 2011 at 07:25:37AM -0600, Grant Likely wrote:
> >> > On Thu, Jun 23, 2011 at 6:39 AM, Alan Cox
> ><alan@lxorguk.ukuu.org.uk> wrote:
> >> > >> > +       - gpios: OF devcie-tree gpio specificatin.
> >> > >> > +       - label: Descriptive name of the key.
> >> > >> > +       - linux,code: Keycode to emit.
> >> > >>
> >> > >> The fact that this is the Linux internal keycode definitions
> >still
> >> > >> makes me nervous.  Is there no existing standard for keycodes
> >emitted
> >> > >> by keyboard devices?
> >> > >
> >> > > There is but no standard lookup table. For Intel MID we do a
> >translation
> >> > > between Linux key names in the firmware and keycodes but there
> >isn't a
> >> > > generic helper for it.
> >> > 
> >> > I suppose the Linux keycodes are exported out to userspace, and are
> >> > therefore an ABI which will not change.  Okay.
> >> 
> >> Right, keycodes form ABI that will not change.
> >> 
> >> Another option would be to use codes from HID usage tables but then
> >> they would have to be translated to Linux ones.
> >
> >Dmitry, will you accept this patch also?
> >Until now, part 2/2 is in your tree, thanks for that, but I'd like to
> >know if
> >this part (1/2) will also be accepted?
> >
> 
> Yes, I will since there was no more discussion about hid codes and I do
> believe that using linux definitions is fine.

Ok, thanks.

Best regards,

-- 
David Jander
Protonic Holland.
--
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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[Dmitry Torokhov]

On Mon, Jul 04, 2011 at 08:56:51AM +0200, David Jander wrote:
> On Mon, 04 Jul 2011 10:52:55 +0400
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > David Jander <david.jander@protonic.nl> wrote:
> > 
> > >On Thu, 23 Jun 2011 11:01:22 -0700
> > >Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > >
> > >> On Thu, Jun 23, 2011 at 07:25:37AM -0600, Grant Likely wrote:
> > >> > On Thu, Jun 23, 2011 at 6:39 AM, Alan Cox
> > ><alan@lxorguk.ukuu.org.uk> wrote:
> > >> > >> > +       - gpios: OF devcie-tree gpio specificatin.
> > >> > >> > +       - label: Descriptive name of the key.
> > >> > >> > +       - linux,code: Keycode to emit.
> > >> > >>
> > >> > >> The fact that this is the Linux internal keycode definitions
> > >still
> > >> > >> makes me nervous.  Is there no existing standard for keycodes
> > >emitted
> > >> > >> by keyboard devices?
> > >> > >
> > >> > > There is but no standard lookup table. For Intel MID we do a
> > >translation
> > >> > > between Linux key names in the firmware and keycodes but there
> > >isn't a
> > >> > > generic helper for it.
> > >> > 
> > >> > I suppose the Linux keycodes are exported out to userspace, and are
> > >> > therefore an ABI which will not change.  Okay.
> > >> 
> > >> Right, keycodes form ABI that will not change.
> > >> 
> > >> Another option would be to use codes from HID usage tables but then
> > >> they would have to be translated to Linux ones.
> > >
> > >Dmitry, will you accept this patch also?
> > >Until now, part 2/2 is in your tree, thanks for that, but I'd like to
> > >know if
> > >this part (1/2) will also be accepted?
> > >
> > 
> > Yes, I will since there was no more discussion about hid codes and I do
> > believe that using linux definitions is fine.
> 
> Ok, thanks.
> 

Noticed that we leaked dynamically allocated button data in case when
gpio_keys_probe() fails. Also removed changelog from copyright notice
(we have SCM for it) and got rid of a warning in case of !CONFIG_OF.

Could you please tell me if the patch below still work for you?

Grant, can I get your formal Ack for OF bindings?

Thanks.

-- 
Dmitry


Input: gpio_keys - add support for device-tree platform data

From: David Jander <david@protonic.nl>

This patch enables fetching configuration data, which is normally provided
via platform_data, from the device-tree instead.

If the device is configured from device-tree data, the platform_data struct
is not used, and button data needs to be allocated dynamically. Big part of
this patch deals with confining pdata usage to the probe function, to make
this possible.

Signed-off-by: David Jander <david@protonic.nl>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 .../devicetree/bindings/gpio/gpio_keys.txt         |   36 +++++
 drivers/input/keyboard/gpio_keys.c                 |  148 ++++++++++++++++++--
 2 files changed, 168 insertions(+), 16 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..7190c99
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio_keys.txt
@@ -0,0 +1,36 @@
+Device-Tree bindings for input/gpio_keys.c keyboard driver
+
+Required properties:
+	- compatible = "gpio-keys";
+
+Optional properties:
+	- autorepeat: Boolean, Enable auto repeat feature of Linux input
+	  subsystem.
+
+Each button (key) is represented as a sub-node of "gpio-keys":
+Subnode properties:
+
+	- gpios: OF devcie-tree gpio specificatin.
+	- label: Descriptive name of the key.
+	- linux,code: Keycode to emit.
+
+Optional subnode-properties:
+	- linux,input-type: Specify event type this button/key generates.
+	  If not specified defaults to <1> == EV_KEY.
+	- debounce-interval: Debouncing interval time in milliseconds.
+	  If not specified defaults to 5.
+	- gpio-key,wakeup: Boolean, button can wake-up the system.
+
+Example nodes:
+
+	gpio_keys {
+			compatible = "gpio-keys";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			autorepeat;
+			button@21 {
+				label = "GPIO Key UP";
+				linux,code = <103>;
+				gpios = <&gpio1 0 1>;
+			};
+			...
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 97bada4..ad11e86 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -2,6 +2,7 @@
  * Driver for keys on GPIO lines capable of generating interrupts.
  *
  * Copyright 2005 Phil Blundell
+ * Copyright 2010, 2011 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
@@ -25,6 +26,8 @@
 #include <linux/gpio_keys.h>
 #include <linux/workqueue.h>
 #include <linux/gpio.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
 
 struct gpio_button_data {
 	struct gpio_keys_button *button;
@@ -445,15 +448,120 @@ 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 int 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 -ENODEV;
+
+	memset(pdata, 0, sizeof *pdata);
+
+	pdata->rep = !!of_get_property(node, "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 -ENODEV;
+
+	buttons = kzalloc(pdata->nbuttons * (sizeof *buttons), GFP_KERNEL);
+	if (!buttons)
+		return -ENODEV;
+
+	pp = NULL;
+	i = 0;
+	while ((pp = of_get_next_child(node, pp))) {
+		enum of_gpio_flags flags;
+
+		if (!of_find_property(pp, "gpios", NULL)) {
+			pdata->nbuttons--;
+			dev_warn(dev, "Found button without gpios\n");
+			continue;
+		}
+		buttons[i].gpio = of_get_gpio_flags(pp, 0, &flags);
+		buttons[i].active_low = flags & OF_GPIO_ACTIVE_LOW;
+
+		reg = of_get_property(pp, "linux,code", &len);
+		if (!reg) {
+			dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
+			goto out_fail;
+		}
+		buttons[i].code = be32_to_cpup(reg);
+
+		buttons[i].desc = of_get_property(pp, "label", &len);
+
+		reg = of_get_property(pp, "linux,input-type", &len);
+		buttons[i].type = reg ? be32_to_cpup(reg) : EV_KEY;
+
+		buttons[i].wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
+
+		reg = of_get_property(pp, "debounce-interval", &len);
+		buttons[i].debounce_interval = reg ? be32_to_cpup(reg) : 5;
+
+		i++;
+	}
+
+	pdata->buttons = buttons;
+
+	return 0;
+
+out_fail:
+	kfree(buttons);
+	return -ENODEV;
+}
+
+static struct of_device_id gpio_keys_of_match[] = {
+	{ .compatible = "gpio-keys", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, gpio_keys_of_match);
+
+#else
+
+static int gpio_keys_get_devtree_pdata(struct device *dev,
+			    struct gpio_keys_platform_data *altp)
+{
+	return -ENODEV;
+}
+
+#define gpio_keys_of_match NULL
+
+#endif
+
 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 alt_pdata;
 	struct input_dev *input;
 	int i, error;
 	int wakeup = 0;
 
+	if (!pdata) {
+		error = gpio_keys_get_devtree_pdata(dev, &alt_pdata);
+		if (error)
+			return error;
+		pdata = &alt_pdata;
+	}
+
 	ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
 			pdata->nbuttons * sizeof(struct gpio_button_data),
 			GFP_KERNEL);
@@ -544,13 +652,15 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
  fail1:
 	input_free_device(input);
 	kfree(ddata);
+	/* If we have no platform_data, we allocated buttons dynamically. */
+	if (!pdev->dev.platform_data)
+		kfree(pdata->buttons);
 
 	return error;
 }
 
 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 input_dev *input = ddata->input;
 	int i;
@@ -559,32 +669,39 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
 
 	device_init_wakeup(&pdev->dev, 0);
 
-	for (i = 0; i < pdata->nbuttons; i++) {
-		int irq = gpio_to_irq(pdata->buttons[i].gpio);
+	for (i = 0; i < ddata->n_buttons; i++) {
+		int irq = gpio_to_irq(ddata->data[i].button->gpio);
 		free_irq(irq, &ddata->data[i]);
 		if (ddata->data[i].timer_debounce)
 			del_timer_sync(&ddata->data[i].timer);
 		cancel_work_sync(&ddata->data[i].work);
-		gpio_free(pdata->buttons[i].gpio);
+		gpio_free(ddata->data[i].button->gpio);
 	}
 
 	input_unregister_device(input);
+
+	/*
+	 * If we had no platform_data, we allocated buttons dynamically, and
+	 * must free them here. ddata->data[0].button is the pointer to the
+	 * beginning of the allocated array.
+	 */
+	if (!pdev->dev.platform_data)
+		kfree(ddata->data[0].button);
+
 	kfree(ddata);
 
 	return 0;
 }
 
-
 #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_drvdata *ddata = dev_get_drvdata(dev);
 	int i;
 
-	if (device_may_wakeup(&pdev->dev)) {
-		for (i = 0; i < pdata->nbuttons; i++) {
-			struct gpio_keys_button *button = &pdata->buttons[i];
+	if (device_may_wakeup(dev)) {
+		for (i = 0; i < ddata->n_buttons; i++) {
+			struct gpio_keys_button *button = ddata->data[i].button;
 			if (button->wakeup) {
 				int irq = gpio_to_irq(button->gpio);
 				enable_irq_wake(irq);
@@ -597,15 +714,13 @@ 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);
 	int i;
 
-	for (i = 0; i < pdata->nbuttons; i++) {
+	for (i = 0; i < ddata->n_buttons; i++) {
 
-		struct gpio_keys_button *button = &pdata->buttons[i];
-		if (button->wakeup && device_may_wakeup(&pdev->dev)) {
+		struct gpio_keys_button *button = ddata->data[i].button;
+		if (button->wakeup && device_may_wakeup(dev)) {
 			int irq = gpio_to_irq(button->gpio);
 			disable_irq_wake(irq);
 		}
@@ -632,6 +747,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,
 	}
 };
 
--
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 v5 1/2] Input: gpio_keys.c: Added support for device-tree platform data

[David Jander]

On Mon, 4 Jul 2011 10:28:06 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Mon, Jul 04, 2011 at 08:56:51AM +0200, David Jander wrote:
> > On Mon, 04 Jul 2011 10:52:55 +0400
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > 
> > > David Jander <david.jander@protonic.nl> wrote:
> > > 
> > > >On Thu, 23 Jun 2011 11:01:22 -0700
> > > >Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > >
> > > >> On Thu, Jun 23, 2011 at 07:25:37AM -0600, Grant Likely wrote:
> > > >> > On Thu, Jun 23, 2011 at 6:39 AM, Alan Cox
> > > ><alan@lxorguk.ukuu.org.uk> wrote:
> > > >> > >> > +       - gpios: OF devcie-tree gpio specificatin.
> > > >> > >> > +       - label: Descriptive name of the key.
> > > >> > >> > +       - linux,code: Keycode to emit.
> > > >> > >>
> > > >> > >> The fact that this is the Linux internal keycode definitions
> > > >still
> > > >> > >> makes me nervous.  Is there no existing standard for keycodes
> > > >emitted
> > > >> > >> by keyboard devices?
> > > >> > >
> > > >> > > There is but no standard lookup table. For Intel MID we do a
> > > >translation
> > > >> > > between Linux key names in the firmware and keycodes but there
> > > >isn't a
> > > >> > > generic helper for it.
> > > >> > 
> > > >> > I suppose the Linux keycodes are exported out to userspace, and are
> > > >> > therefore an ABI which will not change.  Okay.
> > > >> 
> > > >> Right, keycodes form ABI that will not change.
> > > >> 
> > > >> Another option would be to use codes from HID usage tables but then
> > > >> they would have to be translated to Linux ones.
> > > >
> > > >Dmitry, will you accept this patch also?
> > > >Until now, part 2/2 is in your tree, thanks for that, but I'd like to
> > > >know if
> > > >this part (1/2) will also be accepted?
> > > >
> > > 
> > > Yes, I will since there was no more discussion about hid codes and I do
> > > believe that using linux definitions is fine.
> > 
> > Ok, thanks.
> > 
> 
> Noticed that we leaked dynamically allocated button data in case when
> gpio_keys_probe() fails.

Eeek. Sorry. Thanks for pointing out.

> Also removed changelog from copyright notice
> (we have SCM for it) and got rid of a warning in case of !CONFIG_OF.

Ah, ok. I was just imitating others here, but this is a good point ;-)

> Could you please tell me if the patch below still work for you?

Yes, it seems to work fine.

Best regards,

-- 
David Jander
Protonic Holland.
--
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