From 219edc71784ff8a3537ffbe64baded61ce4048b9 Mon Sep 17 00:00:00 2001 From: Alexandre Pereira da Silva Date: Sun, 29 Jul 2012 22:18:47 -0700 Subject: [PATCH] Input: gpio_keys - clean up device tree parser - fix sizeof in memset; - clean up dt properties extraction; - use for_each_chil_of_node macro; - use of_get_child_count(); - use of_match_ptr macro. Signed-off-by: Alexandre Pereira da Silva Acked-by: Rob Herring [Fabio Estevam : fix fix NULL pointer dereference for dt case - pdata->buttons wasn't initialized] Signed-off-by: Dmitry Torokhov --- drivers/input/keyboard/gpio_keys.c | 163 ++++++++++++++--------------- 1 file changed, 81 insertions(+), 82 deletions(-) diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index cbb1add43d5e..b890231debd8 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -43,11 +43,9 @@ struct gpio_button_data { }; struct gpio_keys_drvdata { + const struct gpio_keys_platform_data *pdata; struct input_dev *input; struct mutex disable_lock; - unsigned int n_buttons; - int (*enable)(struct device *dev); - void (*disable)(struct device *dev); struct gpio_button_data data[0]; }; @@ -171,7 +169,7 @@ static ssize_t gpio_keys_attr_show_helper(struct gpio_keys_drvdata *ddata, if (!bits) return -ENOMEM; - for (i = 0; i < ddata->n_buttons; i++) { + for (i = 0; i < ddata->pdata->nbuttons; i++) { struct gpio_button_data *bdata = &ddata->data[i]; if (bdata->button->type != type) @@ -219,7 +217,7 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata, goto out; /* First validate */ - for (i = 0; i < ddata->n_buttons; i++) { + for (i = 0; i < ddata->pdata->nbuttons; i++) { struct gpio_button_data *bdata = &ddata->data[i]; if (bdata->button->type != type) @@ -234,7 +232,7 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata, mutex_lock(&ddata->disable_lock); - for (i = 0; i < ddata->n_buttons; i++) { + for (i = 0; i < ddata->pdata->nbuttons; i++) { struct gpio_button_data *bdata = &ddata->data[i]; if (bdata->button->type != type) @@ -523,56 +521,64 @@ fail: static int gpio_keys_open(struct input_dev *input) { struct gpio_keys_drvdata *ddata = input_get_drvdata(input); + const struct gpio_keys_platform_data *pdata = ddata->pdata; - return ddata->enable ? ddata->enable(input->dev.parent) : 0; + return pdata->enable ? pdata->enable(input->dev.parent) : 0; } static void gpio_keys_close(struct input_dev *input) { struct gpio_keys_drvdata *ddata = input_get_drvdata(input); + const struct gpio_keys_platform_data *pdata = ddata->pdata; - if (ddata->disable) - ddata->disable(input->dev.parent); + if (pdata->disable) + pdata->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) +static struct gpio_keys_platform_data * __devinit +gpio_keys_get_devtree_pdata(struct device *dev) { struct device_node *node, *pp; + struct gpio_keys_platform_data *pdata; + struct gpio_keys_button *button; + int error; + int nbuttons; int i; - struct gpio_keys_button *buttons; - u32 reg; node = dev->of_node; - if (node == NULL) - return -ENODEV; + if (!node) { + error = -ENODEV; + goto err_out; + } - memset(pdata, 0, sizeof *pdata); + nbuttons = of_get_child_count(node); + if (nbuttons == 0) { + error = -ENODEV; + goto err_out; + } + + pdata = kzalloc(sizeof(*pdata) + nbuttons * (sizeof *button), + GFP_KERNEL); + if (!pdata) { + error = -ENOMEM; + goto err_out; + } + + pdata->buttons = (struct gpio_keys_button *)(pdata + 1); + pdata->nbuttons = nbuttons; pdata->rep = !!of_get_property(node, "autorepeat", NULL); - /* First count the subnodes */ - 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 -ENOMEM; - - pp = NULL; i = 0; - while ((pp = of_get_next_child(node, pp))) { + for_each_child_of_node(node, pp) { enum of_gpio_flags flags; if (!of_find_property(pp, "gpios", NULL)) { @@ -580,39 +586,42 @@ static int gpio_keys_get_devtree_pdata(struct device *dev, 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; - if (of_property_read_u32(pp, "linux,code", ®)) { - dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio); - goto out_fail; + button = &pdata->buttons[i++]; + + button->gpio = of_get_gpio_flags(pp, 0, &flags); + button->active_low = flags & OF_GPIO_ACTIVE_LOW; + + if (of_property_read_u32(pp, "linux,code", &button->code)) { + dev_err(dev, "Button without keycode: 0x%x\n", + button->gpio); + error = -EINVAL; + goto err_free_pdata; } - buttons[i].code = reg; - buttons[i].desc = of_get_property(pp, "label", NULL); + button->desc = of_get_property(pp, "label", NULL); - if (of_property_read_u32(pp, "linux,input-type", ®) == 0) - buttons[i].type = reg; - else - buttons[i].type = EV_KEY; + if (of_property_read_u32(pp, "linux,input-type", &button->type)) + button->type = EV_KEY; - buttons[i].wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL); + button->wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL); - if (of_property_read_u32(pp, "debounce-interval", ®) == 0) - buttons[i].debounce_interval = reg; - else - buttons[i].debounce_interval = 5; - - i++; + if (of_property_read_u32(pp, "debounce-interval", + &button->debounce_interval)) + button->debounce_interval = 5; } - pdata->buttons = buttons; + if (pdata->nbuttons == 0) { + error = -EINVAL; + goto err_free_pdata; + } - return 0; + return pdata; -out_fail: - kfree(buttons); - return -ENODEV; +err_free_pdata: + kfree(pdata); +err_out: + return ERR_PTR(error); } static struct of_device_id gpio_keys_of_match[] = { @@ -623,14 +632,12 @@ 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) +static inline struct gpio_keys_platform_data * +gpio_keys_get_devtree_pdata(struct device *dev) { - return -ENODEV; + return ERR_PTR(-ENODEV); } -#define gpio_keys_of_match NULL - #endif static void gpio_remove_key(struct gpio_button_data *bdata) @@ -645,19 +652,17 @@ static void gpio_remove_key(struct gpio_button_data *bdata) static int __devinit gpio_keys_probe(struct platform_device *pdev) { - const 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; + const struct gpio_keys_platform_data *pdata = dev_get_platdata(dev); + struct gpio_keys_drvdata *ddata; 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; + pdata = gpio_keys_get_devtree_pdata(dev); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); } ddata = kzalloc(sizeof(struct gpio_keys_drvdata) + @@ -670,10 +675,8 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) goto fail1; } + ddata->pdata = pdata; ddata->input = input; - ddata->n_buttons = pdata->nbuttons; - ddata->enable = pdata->enable; - ddata->disable = pdata->disable; mutex_init(&ddata->disable_lock); platform_set_drvdata(pdev, ddata); @@ -742,9 +745,9 @@ 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); + /* If we have no platform data, we allocated pdata dynamically. */ + if (!dev_get_platdata(&pdev->dev)) + kfree(pdata); return error; } @@ -759,18 +762,14 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev) device_init_wakeup(&pdev->dev, 0); - for (i = 0; i < ddata->n_buttons; i++) + for (i = 0; i < ddata->pdata->nbuttons; i++) gpio_remove_key(&ddata->data[i]); 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); + /* If we have no platform data, we allocated pdata dynamically. */ + if (!dev_get_platdata(&pdev->dev)) + kfree(ddata->pdata); kfree(ddata); @@ -784,7 +783,7 @@ static int gpio_keys_suspend(struct device *dev) int i; if (device_may_wakeup(dev)) { - for (i = 0; i < ddata->n_buttons; i++) { + for (i = 0; i < ddata->pdata->nbuttons; i++) { struct gpio_button_data *bdata = &ddata->data[i]; if (bdata->button->wakeup) enable_irq_wake(bdata->irq); @@ -799,7 +798,7 @@ static int gpio_keys_resume(struct device *dev) struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); int i; - for (i = 0; i < ddata->n_buttons; i++) { + for (i = 0; i < ddata->pdata->nbuttons; i++) { struct gpio_button_data *bdata = &ddata->data[i]; if (bdata->button->wakeup && device_may_wakeup(dev)) disable_irq_wake(bdata->irq); @@ -822,7 +821,7 @@ static struct platform_driver gpio_keys_device_driver = { .name = "gpio-keys", .owner = THIS_MODULE, .pm = &gpio_keys_pm_ops, - .of_match_table = gpio_keys_of_match, + .of_match_table = of_match_ptr(gpio_keys_of_match), } };