lkml.org 
[lkml]   [2012]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines
Hi Viresh,

On Thu, Nov 08, 2012 at 07:10:47PM +0530, Viresh Kumar wrote:
> This patch frees spear-keyboard driver from tension of freeing resources :)
> devm_* derivatives of multiple routines are used while allocating resources,
> which would be freed automatically by kernel.

It also breaks the error unwinding/removal of the driver as it frees
input device while IRQ handler is still active.

I will push the patch which implements devm_input_allocate_device() to
my 'next' branch in a few, please use it because it will make sure input
device will stick around long enough.

Also below.

>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/input/keyboard/spear-keyboard.c | 37 +++++++++++----------------------
> 1 file changed, 12 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c
> index c7ca97f..1d24fb2 100644
> --- a/drivers/input/keyboard/spear-keyboard.c
> +++ b/drivers/input/keyboard/spear-keyboard.c
> @@ -203,7 +203,7 @@ static int __devinit spear_kbd_probe(struct platform_device *pdev)
> return irq;
> }
>
> - kbd = kzalloc(sizeof(*kbd), GFP_KERNEL);
> + kbd = devm_kzalloc(&pdev->dev, sizeof(*kbd), GFP_KERNEL);
> input_dev = input_allocate_device();
> if (!kbd || !input_dev) {
> dev_err(&pdev->dev, "out of memory\n");
> @@ -224,25 +224,25 @@ static int __devinit spear_kbd_probe(struct platform_device *pdev)
> kbd->suspended_rate = pdata->suspended_rate;
> }
>
> - kbd->res = request_mem_region(res->start, resource_size(res),
> - pdev->name);
> + kbd->res = devm_request_mem_region(&pdev->dev, res->start,
> + resource_size(res), pdev->name);
> if (!kbd->res) {
> dev_err(&pdev->dev, "keyboard region already claimed\n");
> error = -EBUSY;
> goto err_free_mem;
> }
>
> - kbd->io_base = ioremap(res->start, resource_size(res));
> + kbd->io_base = devm_ioremap(&pdev->dev, res->start, resource_size(res));

There is devm_request_and_ioremap() which you can use here.

> if (!kbd->io_base) {
> dev_err(&pdev->dev, "ioremap failed for kbd_region\n");
> error = -ENOMEM;
> - goto err_release_mem_region;
> + goto err_free_mem;
> }
>
> - kbd->clk = clk_get(&pdev->dev, NULL);
> + kbd->clk = devm_clk_get(&pdev->dev, NULL);
> if (IS_ERR(kbd->clk)) {
> error = PTR_ERR(kbd->clk);
> - goto err_iounmap;
> + goto err_free_mem;
> }
>
> input_dev->name = "Spear Keyboard";
> @@ -259,7 +259,7 @@ static int __devinit spear_kbd_probe(struct platform_device *pdev)
> kbd->keycodes, input_dev);
> if (error) {
> dev_err(&pdev->dev, "Failed to build keymap\n");
> - goto err_put_clk;
> + goto err_free_mem;
> }
>
> if (kbd->rep)
> @@ -268,16 +268,17 @@ static int __devinit spear_kbd_probe(struct platform_device *pdev)
>
> input_set_drvdata(input_dev, kbd);
>
> - error = request_irq(irq, spear_kbd_interrupt, 0, "keyboard", kbd);
> + error = devm_request_irq(&pdev->dev, irq, spear_kbd_interrupt, 0,
> + "keyboard", kbd);
> if (error) {
> dev_err(&pdev->dev, "request_irq fail\n");
> - goto err_put_clk;
> + goto err_free_mem;
> }
>
> error = input_register_device(input_dev);
> if (error) {
> dev_err(&pdev->dev, "Unable to register keyboard device\n");
> - goto err_free_irq;
> + goto err_free_mem;
> }
>
> device_init_wakeup(&pdev->dev, 1);
> @@ -285,17 +286,8 @@ static int __devinit spear_kbd_probe(struct platform_device *pdev)
>
> return 0;
>
> -err_free_irq:
> - free_irq(kbd->irq, kbd);
> -err_put_clk:
> - clk_put(kbd->clk);
> -err_iounmap:
> - iounmap(kbd->io_base);
> -err_release_mem_region:
> - release_mem_region(res->start, resource_size(res));
> err_free_mem:
> input_free_device(input_dev);
> - kfree(kbd);
>
> return error;
> }
> @@ -304,12 +296,7 @@ static int __devexit spear_kbd_remove(struct platform_device *pdev)
> {
> struct spear_kbd *kbd = platform_get_drvdata(pdev);
>
> - free_irq(kbd->irq, kbd);
> input_unregister_device(kbd->input);
> - clk_put(kbd->clk);
> - iounmap(kbd->io_base);
> - release_mem_region(kbd->res->start, resource_size(kbd->res));
> - kfree(kbd);
>
> device_init_wakeup(&pdev->dev, 0);
> platform_set_drvdata(pdev, NULL);
> --
> 1.7.12.rc2.18.g61b472e
>

Thanks.

--
Dmitry


\
 
 \ /
  Last update: 2012-11-08 18:21    [W:0.207 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site