Re: [PATCH 2/3] Input: gpio_keys.c: Added support for device-tree platform data
From: Grant Likely <hidden>
Date: 2011-06-06 15:52:51
On Mon, Jun 6, 2011 at 9:12 AM, David Jander [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Signed-off-by: David Jander <redacted> --- .../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.txtdiff --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.
quoted hunk ↗ jump to hunk
+ +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 [off-list ref] + * * 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);
quoted hunk ↗ jump to hunk
+ 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.
quoted hunk ↗ jump to hunk
+ {}, +}; #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