Input: gpio-keys - clean up series
Subsystems (2)
v3 cover Input: gpio-keys - clean up series
This is a 3rd iteration of the small cleanup against gpio-keys driver.
Changes since v2:
- append received Acks
- rebase against recent linux-next
- test on Intel Medfield based product
Andy Shevchenko (3):
Input: gpio_keys - use dev instead of pdev in gpio_keys_setup_key()
Input: gpio_keys - convert to use devm_*
Input: gpio_keys - convert struct descriptions to kernel-doc
drivers/input/keyboard/gpio_keys.c | 82 +++++++++++++------------------------- include/linux/gpio_keys.h | 45 +++++++++++++++------ 2 files changed, 61 insertions(+), 66 deletions(-)
Signature · 1 line
1.9.2
PATCH v3 1/3 Input: gpio_keys - use dev instead of pdev in gpio_keys_setup_key()
The platform device is not used in gpio_keys_setup_key(). This patch substitutes it by struct device.
--- drivers/input/keyboard/gpio_keys.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/input/keyboard/gpio_keys.c +3 -4
@@ -424,13 +424,12 @@ out:return IRQ_HANDLED ; } -static int gpio_keys_setup_key (struct platform_device *pdev, +static int gpio_keys_setup_key (struct device *dev,struct input_dev *input,struct gpio_button_data *bdata,const struct gpio_keys_button *button) {const char *desc = button->desc ? button->desc :"gpio_keys" ; -struct device *dev = &pdev->dev;irq_handler_t isr;unsigned long irqflags;int irq, error;
@@ -719,7 +718,7 @@ static int gpio_keys_probe(struct platform_device *pdev) 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;
@@ -736,7 +735,7 @@ static int gpio_keys_probe(struct platform_device *pdev)const struct gpio_keys_button *button = &pdata->buttons[i];struct gpio_button_data *bdata = &ddata->data[i]; - error =gpio_keys_setup_key (pdev, input, bdata, button); + error =gpio_keys_setup_key (dev, input, bdata, button);if (error) goto fail2;
Signature · 1 line
1.9.2
Dmitry Torokhov
maintainerRe: [PATCH v3 1/3] Input: gpio_keys - use dev instead of pdev in gpio_keys_setup_key()
Quoted 2 lines · Andy Shevchenko
The platform device is not used in
gpio_keys_setup_key(). This patch substitutes it by struct device.
And the benefit of this is...?
Quoted 46 lines
Signed-off-by Andy Shevchenko <[email protected]>--- drivers/input/keyboard/gpio_keys.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)drivers/input/keyboard/gpio_keys.c +3
-4@@ -424,13 +424,12 @@ out:return IRQ_HANDLED ; } -static int gpio_keys_setup_key (struct platform_device *pdev, +static int gpio_keys_setup_key (struct device *dev,struct input_dev *input,struct gpio_button_data *bdata,const struct gpio_keys_button *button) {const char *desc = button->desc ? button->desc :"gpio_keys" ; -struct device *dev = &pdev->dev;irq_handler_t isr;unsigned long irqflags;int irq, error;@@ -719,7 +718,7 @@ static int gpio_keys_probe(struct platform_device *pdev) 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;@@ -736,7 +735,7 @@ static int gpio_keys_probe(struct platform_device *pdev)const struct gpio_keys_button *button = &pdata->buttons[i];struct gpio_button_data *bdata = &ddata->data[i]; - error =gpio_keys_setup_key (pdev, input, bdata, button); + error =gpio_keys_setup_key (dev, input, bdata, button);if (error) goto fail2;Signature · 1 line
1.9.2
Thanks.
Signature · 1 line
Dmitry
Re: [PATCH v3 1/3] Input: gpio_keys - use dev instead of pdev in gpio_keys_setup_key()
Quoted 5 lines · Dmitry Torokhov
Quoted 2 lines · Andy Shevchenko
The platform device is not used in
gpio_keys_setup_key(). This patch substitutes it by struct device.And the benefit of this is...?
Probably no benefit for now. Let's drop it for good.
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
PATCH v3 2/3 Input: gpio_keys - convert to use devm_*
In reply to Andy ShevchenkoThis makes the error handling much more simpler than open-coding everything and in addition makes the probe function smaller an tidier.
--- drivers/input/keyboard/gpio_keys.c | 75 +++++++++++++------------------------- 1 file changed, 25 insertions(+), 50 deletions(-)
drivers/input/keyboard/gpio_keys.c +25 -50
@@ -47,7 +47,7 @@ struct gpio_keys_drvdata {const struct gpio_keys_platform_data *pdata;struct input_dev *input;struct mutex disable_lock; -struct gpio_button_data data[0 ]; +struct gpio_button_data *data; };/*
@@ -577,25 +577,22 @@ gpio_keys_get_devtree_pdata(struct device *dev)int i; node = dev->of_node; -if (!node) { - error = -ENODEV ; - goto err_out; - } +if (!node) +return ERR_PTR (-ENODEV ); nbuttons =of_get_child_count (node); -if (nbuttons ==0 ) { - error = -ENODEV ; - goto err_out; - } +if (nbuttons ==0 ) +return ERR_PTR (-ENODEV ); - pdata =kzalloc (sizeof (*pdata) + nbuttons * (sizeof *button), -GFP_KERNEL ); -if (!pdata) { - error = -ENOMEM ; - goto err_out; - } + pdata =devm_kzalloc (dev,sizeof (*pdata),GFP_KERNEL ); +if (!pdata) +return ERR_PTR (-ENOMEM ); + + pdata->buttons =devm_kcalloc (dev, nbuttons,sizeof (*button), +GFP_KERNEL ); +if (!pdata->buttons) +return ERR_PTR (-ENOMEM ); - pdata->buttons = (struct gpio_keys_button *)(pdata +1 ); pdata->nbuttons = nbuttons; pdata->rep = !!of_get_property (node,"autorepeat" ,NULL );
@@ -618,7 +615,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)dev_err (dev,"Failed to get gpio flags, error: %d\n" , error); - goto err_free_pdata; +return ERR_PTR (error); } button = &pdata->buttons[i++];
@@ -629,8 +626,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)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; +return ERR_PTR (-EINVAL ); } button->desc =of_get_property (pp,"label" ,NULL );
@@ -645,17 +641,10 @@ gpio_keys_get_devtree_pdata(struct device *dev) button->debounce_interval =5 ; } -if (pdata->nbuttons ==0 ) { - error = -EINVAL ; - goto err_free_pdata; - } +if (pdata->nbuttons ==0 ) +return ERR_PTR (-EINVAL );return pdata; - -err_free_pdata: -kfree (pdata); -err_out: -return ERR_PTR (error); }static struct of_device_id gpio_keys_of_match[] = {
@@ -699,16 +688,18 @@ static int gpio_keys_probe(struct platform_device *pdev)return PTR_ERR (pdata); } - ddata =kzalloc (sizeof (struct gpio_keys_drvdata ) + - pdata->nbuttons *sizeof (struct gpio_button_data ), -GFP_KERNEL ); - input =input_allocate_device (); + ddata =devm_kzalloc (dev,sizeof (*ddata),GFP_KERNEL ); + input =devm_input_allocate_device (dev);if (!ddata || !input) {dev_err (dev,"failed to allocate state\n" ); - error = -ENOMEM ; - goto fail1; +return -ENOMEM ; } + ddata->data =devm_kcalloc (dev, pdata->nbuttons,sizeof (*ddata->data), +GFP_KERNEL ); +if (!ddata->data) +return -ENOMEM ; + ddata->pdata = pdata; ddata->input = input;mutex_init (&ddata->disable_lock);
@@ -767,20 +758,12 @@ static int gpio_keys_probe(struct platform_device *pdev)while (--i >=0 )gpio_remove_key (&ddata->data[i]); - fail1: -input_free_device (input); -kfree (ddata); -/* If we have no platform data, we allocated pdata dynamically. */ -if (!dev_get_platdata (&pdev->dev)) -kfree (pdata); -return error; }static int gpio_keys_remove (struct platform_device *pdev) {struct gpio_keys_drvdata *ddata =platform_get_drvdata (pdev); -struct input_dev *input = ddata->input;int i;sysfs_remove_group (&pdev->dev.kobj, &gpio_keys_attr_group);
@@ -790,14 +773,6 @@ static int gpio_keys_remove(struct platform_device *pdev)for (i =0 ; i < ddata->pdata->nbuttons; i++)gpio_remove_key (&ddata->data[i]); -input_unregister_device (input); - -/* If we have no platform data, we allocated pdata dynamically. */ -if (!dev_get_platdata (&pdev->dev)) -kfree (ddata->pdata); - -kfree (ddata); -return 0 ; }
Signature · 1 line
1.9.2
Dmitry Torokhov
maintainerRe: [PATCH v3 2/3] Input: gpio_keys - convert to use devm_*
Quoted 45 lines · Andy Shevchenko
This makes the error handling much more simpler than open-coding everything and in addition makes the probe function smaller an tidier.
Signed-off-by Andy Shevchenko <[email protected]>Acked-by Linus Walleij <[email protected]>--- drivers/input/keyboard/gpio_keys.c | 75 +++++++++++++------------------------- 1 file changed, 25 insertions(+), 50 deletions(-)drivers/input/keyboard/gpio_keys.c +5
-13@@ -47,7 +47,7 @@ struct gpio_keys_drvdata {const struct gpio_keys_platform_data *pdata;struct input_dev *input;struct mutex disable_lock; -struct gpio_button_data data[0 ]; +struct gpio_button_data *data; };/* @@ -577,25 +577,22 @@ gpio_keys_get_devtree_pdata(struct device *dev)int i; node = dev->of_node; -if (!node) { - error = -ENODEV ; - goto err_out; - } +if (!node) +return ERR_PTR (-ENODEV ); nbuttons =of_get_child_count (node); -if (nbuttons ==0 ) { - error = -ENODEV ; - goto err_out; - } +if (nbuttons ==0 ) +return ERR_PTR (-ENODEV ); - pdata =kzalloc (sizeof (*pdata) + nbuttons * (sizeof *button), -GFP_KERNEL ); -if (!pdata) { - error = -ENOMEM ;
drivers/input/keyboard/gpio_keys.c gpio_keys_get_devtree_pdata(struct device *dev) · line 594
Quoted 10 lines
drivers/input/keyboard/gpio_keys.c +8
-2- goto err_out; - } + pdata =devm_kzalloc (dev,sizeof (*pdata),GFP_KERNEL ); +if (!pdata) +return ERR_PTR (-ENOMEM ); + + pdata->buttons =devm_kcalloc (dev, nbuttons,sizeof (*button), +GFP_KERNEL ); +if (!pdata->buttons) +return ERR_PTR (-ENOMEM );
Why are we splitting the allocation in 2?
Quoted 56 lines
drivers/input/keyboard/gpio_keys.c +6
-17- pdata->buttons = (struct gpio_keys_button *)(pdata +1 ); pdata->nbuttons = nbuttons; pdata->rep = !!of_get_property (node,"autorepeat" ,NULL );@@ -618,7 +615,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)dev_err (dev,"Failed to get gpio flags, error: %d\n" , error); - goto err_free_pdata; +return ERR_PTR (error); } button = &pdata->buttons[i++];@@ -629,8 +626,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)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; +return ERR_PTR (-EINVAL ); } button->desc =of_get_property (pp,"label" ,NULL );@@ -645,17 +641,10 @@ gpio_keys_get_devtree_pdata(struct device *dev) button->debounce_interval =5 ; } -if (pdata->nbuttons ==0 ) { - error = -EINVAL ; - goto err_free_pdata; - } +if (pdata->nbuttons ==0 ) +return ERR_PTR (-EINVAL );return pdata; - -err_free_pdata: -kfree (pdata); -err_out: -return ERR_PTR (error); }static struct of_device_id gpio_keys_of_match[] = {@@ -699,16 +688,18 @@ static int gpio_keys_probe(struct platform_device *pdev)return PTR_ERR (pdata); } - ddata =kzalloc (sizeof (struct gpio_keys_drvdata ) + - pdata->nbuttons *sizeof (struct gpio_button_data ), -GFP_KERNEL ); - input =input_allocate_device (); + ddata =devm_kzalloc (dev,sizeof (*ddata),GFP_KERNEL ); + input =devm_input_allocate_device (dev);if (!ddata || !input) {dev_err (dev,"failed to allocate state\n" );
drivers/input/keyboard/gpio_keys.c static int gpio_keys_probe(struct platform_device *pdev) · line 702
Quoted 10 lines
drivers/input/keyboard/gpio_keys.c +6
-2- error = -ENOMEM ; - goto fail1; +return -ENOMEM ; } + ddata->data =devm_kcalloc (dev, pdata->nbuttons,sizeof (*ddata->data), +GFP_KERNEL ); +if (!ddata->data) +return -ENOMEM ; +
Same here. I dropped the separate allocations and applied.
Thanks.
Signature · 1 line
Dmitry
Re: [PATCH v3 2/3] Input: gpio_keys - convert to use devm_*
Quoted 3 lines · Dmitry Torokhov
Quoted 2 lines · Andy Shevchenko
This makes the error handling much more simpler than open-coding everything and in addition makes the probe function smaller an tidier.
[]
Quoted 8 lines
Quoted 8 lines
drivers/input/keyboard/gpio_keys.c +2
-6- pdata =kzalloc (sizeof (*pdata) + nbuttons * (sizeof *button), -GFP_KERNEL ); -if (!pdata) { - error = -ENOMEM ; - goto err_out; - } + pdata =devm_kzalloc (dev,sizeof (*pdata),GFP_KERNEL ); +if (!pdata)
Quoted 8 lines
drivers/input/keyboard/gpio_keys.c gpio_keys_get_devtree_pdata(struct device *dev) · line 594
Quoted 6 lines
drivers/input/keyboard/gpio_keys.c +6
-0+return ERR_PTR (-ENOMEM ); + + pdata->buttons =devm_kcalloc (dev, nbuttons,sizeof (*button), +GFP_KERNEL ); +if (!pdata->buttons) +return ERR_PTR (-ENOMEM );Why are we splitting the allocation in 2?
Just for logical distinguishing, but I'm okay with one allocation.
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
PATCH v3 3/3 Input: gpio_keys - convert struct descriptions to kernel-doc
In reply to Andy ShevchenkoThis patch converts descriptions of the structures defined in linux/gpio_keys.h to follow kernel-doc format.
There is no functional change.
--- include/linux/gpio_keys.h | 45 +++++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-)
include/linux/gpio_keys.h +33 -12
@@ -3,29 +3,50 @@struct device ; +/** +* struct gpio_keys_button - configuration parameters +* @code: input event code (KEY_*, SW_*) +* @gpio: %-1 if this key does not support gpio +* @active_low: +* @desc: +* @type: input event type (%EV_KEY, %EV_SW, %EV_ABS) +* @wakeup: configure the button as a wake-up source +* @debounce_interval: debounce ticks interval in msecs +* @can_disable: +* @value: axis value for %EV_ABS +* @irq: Irq number in case of interrupt keys +*/ struct gpio_keys_button { -/* Configuration parameters */ -unsigned int code;/* input event code (KEY_*, SW_*) */ -int gpio;/* -1 if this key does not support gpio */ +unsigned int code; +int gpio;int active_low;const char *desc; -unsigned int type;/* input event type (EV_KEY, EV_SW, EV_ABS) */ -int wakeup;/* configure the button as a wake-up source */ -int debounce_interval;/* debounce ticks interval in msecs */ +unsigned int type; +int wakeup; +int debounce_interval;bool can_disable; -int value;/* axis value for EV_ABS */ -unsigned int irq;/* Irq number in case of interrupt keys */ +int value; +unsigned int irq; }; +/** +* struct gpio_keys_platform_data - platform data for gpio_keys driver +* @buttons: +* @nbuttons: +* @poll_interval: polling interval in msecs - for polling driver only +* @rep: enable input subsystem auto repeat +* @enable: +* @disable: +* @name: input device name +*/ struct gpio_keys_platform_data {struct gpio_keys_button *buttons;int nbuttons; -unsigned int poll_interval;/* polling interval in msecs - -for polling driver only */ -unsigned int rep:1 ;/* enable input subsystem auto repeat */ +unsigned int poll_interval; +unsigned int rep:1 ;int (*enable)(struct device *dev);void (*disable)(struct device *dev); -const char *name;/* input device name */ +const char *name; };#endif
Signature · 1 line
1.9.2
Re: [PATCH v3 3/3] Input: gpio_keys - convert struct descriptions to kernel-doc
Quoted 7 lines · Andy Shevchenko
This patch converts descriptions of the structures defined in
linux/gpio_keys.hto follow kernel-doc format.There is no functional change.
Signed-off-by Andy Shevchenko <[email protected]>Acked-by Linus Walleij <[email protected]>
Applied, thank you.
Quoted 74 lines
--- include/linux/gpio_keys.h | 45 +++++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-)include/linux/gpio_keys.h +33
-12@@ -3,29 +3,50 @@struct device ; +/** +* struct gpio_keys_button - configuration parameters +* @code: input event code (KEY_*, SW_*) +* @gpio: %-1 if this key does not support gpio +* @active_low: +* @desc: +* @type: input event type (%EV_KEY, %EV_SW, %EV_ABS) +* @wakeup: configure the button as a wake-up source +* @debounce_interval: debounce ticks interval in msecs +* @can_disable: +* @value: axis value for %EV_ABS +* @irq: Irq number in case of interrupt keys +*/ struct gpio_keys_button { -/* Configuration parameters */ -unsigned int code;/* input event code (KEY_*, SW_*) */ -int gpio;/* -1 if this key does not support gpio */ +unsigned int code; +int gpio;int active_low;const char *desc; -unsigned int type;/* input event type (EV_KEY, EV_SW, EV_ABS) */ -int wakeup;/* configure the button as a wake-up source */ -int debounce_interval;/* debounce ticks interval in msecs */ +unsigned int type; +int wakeup; +int debounce_interval;bool can_disable; -int value;/* axis value for EV_ABS */ -unsigned int irq;/* Irq number in case of interrupt keys */ +int value; +unsigned int irq; }; +/** +* struct gpio_keys_platform_data - platform data for gpio_keys driver +* @buttons: +* @nbuttons: +* @poll_interval: polling interval in msecs - for polling driver only +* @rep: enable input subsystem auto repeat +* @enable: +* @disable: +* @name: input device name +*/ struct gpio_keys_platform_data {struct gpio_keys_button *buttons;int nbuttons; -unsigned int poll_interval;/* polling interval in msecs - -for polling driver only */ -unsigned int rep:1 ;/* enable input subsystem auto repeat */ +unsigned int poll_interval; +unsigned int rep:1 ;int (*enable)(struct device *dev);void (*disable)(struct device *dev); -const char *name;/* input device name */ +const char *name; };#endif Signature · 1 line
1.9.2
Signature · 1 line
Dmitry