Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-09 Thread Viresh Kumar 
On 9 November 2012 11:03, Dmitry Torokhov  wrote:
> On Fri, Nov 09, 2012 at 08:06:29AM +0530, Viresh Kumar wrote:
>> On 8 November 2012 22:08, Dmitry Torokhov  wrote:
>> > On Thu, Nov 08, 2012 at 07:10:47PM +0530, Viresh Kumar wrote:
>> > It also breaks the error unwinding/removal of the driver as it frees
>> > input device while IRQ handler is still active.
>>
>> I have heard of this argument before, probably from you. :)
>> Just need clarification again. How will we get an interrupt when the 
>> controller
>> is stopped, unless we have a shared irq.
>
> My bad, I missed that spear-keyboard driver implements open() and
> close() methods and shuts off the device properly. Still, thanks for
> switching everything to devm_*, I think it is much cleaner this way as
> opposed to mixing managed and unmanaged resources.

Another fixup:

commit ebdec75f51050cf4a5ac41d3a1e880e14bf96a95
Author: Viresh Kumar 
Date:   Fri Nov 9 22:28:48 2012 +0530

fixup! input: spear-keyboard: Use devm_*() routines
---
 drivers/input/keyboard/spear-keyboard.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/keyboard/spear-keyboard.c
b/drivers/input/keyboard/spear-keyboard.c
index 25e0a3b..65f5950 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -292,7 +292,6 @@ static int __devexit spear_kbd_remove(struct
platform_device *pdev)
 {
struct spear_kbd *kbd = platform_get_drvdata(pdev);

-   input_unregister_device(kbd->input);
clk_unprepare(kbd->clk);

device_init_wakeup(>dev, 0);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-09 Thread Viresh Kumar 
On 9 November 2012 11:03, Dmitry Torokhov dmitry.torok...@gmail.com wrote:
 On Fri, Nov 09, 2012 at 08:06:29AM +0530, Viresh Kumar wrote:
 On 8 November 2012 22:08, Dmitry Torokhov dmitry.torok...@gmail.com wrote:
  On Thu, Nov 08, 2012 at 07:10:47PM +0530, Viresh Kumar wrote:
  It also breaks the error unwinding/removal of the driver as it frees
  input device while IRQ handler is still active.

 I have heard of this argument before, probably from you. :)
 Just need clarification again. How will we get an interrupt when the 
 controller
 is stopped, unless we have a shared irq.

 My bad, I missed that spear-keyboard driver implements open() and
 close() methods and shuts off the device properly. Still, thanks for
 switching everything to devm_*, I think it is much cleaner this way as
 opposed to mixing managed and unmanaged resources.

Another fixup:

commit ebdec75f51050cf4a5ac41d3a1e880e14bf96a95
Author: Viresh Kumar viresh.ku...@linaro.org
Date:   Fri Nov 9 22:28:48 2012 +0530

fixup! input: spear-keyboard: Use devm_*() routines
---
 drivers/input/keyboard/spear-keyboard.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/keyboard/spear-keyboard.c
b/drivers/input/keyboard/spear-keyboard.c
index 25e0a3b..65f5950 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -292,7 +292,6 @@ static int __devexit spear_kbd_remove(struct
platform_device *pdev)
 {
struct spear_kbd *kbd = platform_get_drvdata(pdev);

-   input_unregister_device(kbd-input);
clk_unprepare(kbd-clk);

device_init_wakeup(pdev-dev, 0);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Dmitry Torokhov 
On Fri, Nov 09, 2012 at 08:06:29AM +0530, Viresh Kumar wrote:
> On 8 November 2012 22:08, Dmitry Torokhov  wrote:
> > On Thu, Nov 08, 2012 at 07:10:47PM +0530, Viresh Kumar wrote:
> > It also breaks the error unwinding/removal of the driver as it frees
> > input device while IRQ handler is still active.
> 
> I have heard of this argument before, probably from you. :)
> Just need clarification again. How will we get an interrupt when the 
> controller
> is stopped, unless we have a shared irq.

My bad, I missed that spear-keyboard driver implements open() and
close() methods and shuts off the device properly. Still, thanks for
switching everything to devm_*, I think it is much cleaner this way as
opposed to mixing managed and unmanaged resources.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Viresh Kumar 
On 9 November 2012 08:06, Viresh Kumar  wrote:
> On 8 November 2012 22:08, Dmitry Torokhov  wrote:
>> There is devm_request_and_ioremap() which you can use here.
>
> Should have been done in V1 only.

Hi Dmitry,

Please apply below fixup to original patch:

x-x

From: Viresh Kumar 
Date: Fri, 9 Nov 2012 08:22:28 +0530
Subject: [PATCH] fixup! input: spear-keyboard: Use devm_*() routines

---
 drivers/input/keyboard/spear-keyboard.c | 41 -
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/input/keyboard/spear-keyboard.c
b/drivers/input/keyboard/spear-keyboard.c
index b8784df..25e0a3b 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -55,7 +55,6 @@

 struct spear_kbd {
struct input_dev *input;
-   struct resource *res;
void __iomem *io_base;
struct clk *clk;
unsigned int irq;
@@ -205,11 +204,15 @@ static int __devinit spear_kbd_probe(struct
platform_device *pdev)
}

kbd = devm_kzalloc(>dev, sizeof(*kbd), GFP_KERNEL);
-   input_dev = input_allocate_device();
-   if (!kbd || !input_dev) {
+   if (!kbd) {
+   dev_err(>dev, "out of memory\n");
+   return -ENOMEM;
+   }
+
+   input_dev = devm_input_allocate_device(>dev);
+   if (!input_dev) {
dev_err(>dev, "out of memory\n");
-   error = -ENOMEM;
-   goto err_free_mem;
+   return -ENOMEM;
}

kbd->input = input_dev;
@@ -218,41 +221,29 @@ static int __devinit spear_kbd_probe(struct
platform_device *pdev)
if (!pdata) {
error = spear_kbd_parse_dt(pdev, kbd);
if (error)
-   goto err_free_mem;
+   return error;
} else {
kbd->mode = pdata->mode;
kbd->rep = pdata->rep;
kbd->suspended_rate = pdata->suspended_rate;
}

-   kbd->res = devm_request_mem_region(>dev, res->start,
-   resource_size(res), pdev->name);
-   if (!kbd->res) {
-   dev_err(>dev, "keyboard region already claimed\n");
-   error = -EBUSY;
-   goto err_free_mem;
-   }
-
-   kbd->io_base = devm_ioremap(>dev, res->start, resource_size(res));
+   kbd->io_base = devm_request_and_ioremap(>dev, res);
if (!kbd->io_base) {
-   dev_err(>dev, "ioremap failed for kbd_region\n");
-   error = -ENOMEM;
-   goto err_free_mem;
+   dev_err(>dev, "request-ioremap failed for kbd_region\n");
+   return -ENOMEM;
}

kbd->clk = devm_clk_get(>dev, NULL);
-   if (IS_ERR(kbd->clk)) {
-   error = PTR_ERR(kbd->clk);
-   goto err_free_mem;
-   }
+   if (IS_ERR(kbd->clk))
+   return PTR_ERR(kbd->clk);

error = clk_prepare(kbd->clk);
if (error)
-   goto err_free_mem;
+   return error;

input_dev->name = "Spear Keyboard";
input_dev->phys = "keyboard/input0";
-   input_dev->dev.parent = >dev;
input_dev->id.bustype = BUS_HOST;
input_dev->id.vendor = 0x0001;
input_dev->id.product = 0x0001;
@@ -293,8 +284,6 @@ static int __devinit spear_kbd_probe(struct
platform_device *pdev)

 err_unprepare_clk:
clk_unprepare(kbd->clk);
-err_free_mem:
-   input_free_device(input_dev);

return error;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Viresh Kumar 
On 8 November 2012 22:08, Dmitry Torokhov  wrote:
> On Thu, Nov 08, 2012 at 07:10:47PM +0530, Viresh Kumar wrote:
> It also breaks the error unwinding/removal of the driver as it frees
> input device while IRQ handler is still active.

I have heard of this argument before, probably from you. :)
Just need clarification again. How will we get an interrupt when the controller
is stopped, unless we have a shared irq.

> 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.

Will surely use that.

>> + kbd->io_base = devm_ioremap(>dev, res->start, 
>> resource_size(res));
>
> There is devm_request_and_ioremap() which you can use here.

Should have been done in V1 only.

--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Dmitry Torokhov 
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 
> ---
>  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(>dev, sizeof(*kbd), GFP_KERNEL);
>   input_dev = input_allocate_device();
>   if (!kbd || !input_dev) {
>   dev_err(>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(>dev, res->start,
> + resource_size(res), pdev->name);
>   if (!kbd->res) {
>   dev_err(>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(>dev, res->start, resource_size(res));

There is devm_request_and_ioremap() which you can use here.

>   if (!kbd->io_base) {
>   dev_err(>dev, "ioremap failed for kbd_region\n");
>   error = -ENOMEM;
> - goto err_release_mem_region;
> + goto err_free_mem;
>   }
>  
> - kbd->clk = clk_get(>dev, NULL);
> + kbd->clk = devm_clk_get(>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(>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(>dev, irq, spear_kbd_interrupt, 0,
> + "keyboard", kbd);
>   if (error) {
>   dev_err(>dev, "request_irq fail\n");
> - goto err_put_clk;
> + goto err_free_mem;
>   }
>  
>   error = input_register_device(input_dev);
>   if (error) {
>   dev_err(>dev, "Unable to register keyboard device\n");
> - goto err_free_irq;
> + goto err_free_mem;
>   }
>  
>   device_init_wakeup(>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(>dev, 0);
>   platform_set_drvdata(pdev, NULL);
> -- 
> 1.7.12.rc2.18.g61b472e
> 

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Viresh Kumar 
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.

Signed-off-by: Viresh Kumar 
---
 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(>dev, sizeof(*kbd), GFP_KERNEL);
input_dev = input_allocate_device();
if (!kbd || !input_dev) {
dev_err(>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(>dev, res->start,
+   resource_size(res), pdev->name);
if (!kbd->res) {
dev_err(>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(>dev, res->start, resource_size(res));
if (!kbd->io_base) {
dev_err(>dev, "ioremap failed for kbd_region\n");
error = -ENOMEM;
-   goto err_release_mem_region;
+   goto err_free_mem;
}
 
-   kbd->clk = clk_get(>dev, NULL);
+   kbd->clk = devm_clk_get(>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(>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(>dev, irq, spear_kbd_interrupt, 0,
+   "keyboard", kbd);
if (error) {
dev_err(>dev, "request_irq fail\n");
-   goto err_put_clk;
+   goto err_free_mem;
}
 
error = input_register_device(input_dev);
if (error) {
dev_err(>dev, "Unable to register keyboard device\n");
-   goto err_free_irq;
+   goto err_free_mem;
}
 
device_init_wakeup(>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(>dev, 0);
platform_set_drvdata(pdev, NULL);
-- 
1.7.12.rc2.18.g61b472e

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Viresh Kumar 
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.

Signed-off-by: Viresh Kumar viresh.ku...@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));
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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Dmitry Torokhov 
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.ku...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Viresh Kumar 
On 8 November 2012 22:08, Dmitry Torokhov dmitry.torok...@gmail.com wrote:
 On Thu, Nov 08, 2012 at 07:10:47PM +0530, Viresh Kumar wrote:
 It also breaks the error unwinding/removal of the driver as it frees
 input device while IRQ handler is still active.

I have heard of this argument before, probably from you. :)
Just need clarification again. How will we get an interrupt when the controller
is stopped, unless we have a shared irq.

 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.

Will surely use that.

 + kbd-io_base = devm_ioremap(pdev-dev, res-start, 
 resource_size(res));

 There is devm_request_and_ioremap() which you can use here.

Should have been done in V1 only.

--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Viresh Kumar 
On 9 November 2012 08:06, Viresh Kumar viresh.ku...@linaro.org wrote:
 On 8 November 2012 22:08, Dmitry Torokhov dmitry.torok...@gmail.com wrote:
 There is devm_request_and_ioremap() which you can use here.

 Should have been done in V1 only.

Hi Dmitry,

Please apply below fixup to original patch:

x-x

From: Viresh Kumar viresh.ku...@linaro.org
Date: Fri, 9 Nov 2012 08:22:28 +0530
Subject: [PATCH] fixup! input: spear-keyboard: Use devm_*() routines

---
 drivers/input/keyboard/spear-keyboard.c | 41 -
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/input/keyboard/spear-keyboard.c
b/drivers/input/keyboard/spear-keyboard.c
index b8784df..25e0a3b 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -55,7 +55,6 @@

 struct spear_kbd {
struct input_dev *input;
-   struct resource *res;
void __iomem *io_base;
struct clk *clk;
unsigned int irq;
@@ -205,11 +204,15 @@ static int __devinit spear_kbd_probe(struct
platform_device *pdev)
}

kbd = devm_kzalloc(pdev-dev, sizeof(*kbd), GFP_KERNEL);
-   input_dev = input_allocate_device();
-   if (!kbd || !input_dev) {
+   if (!kbd) {
+   dev_err(pdev-dev, out of memory\n);
+   return -ENOMEM;
+   }
+
+   input_dev = devm_input_allocate_device(pdev-dev);
+   if (!input_dev) {
dev_err(pdev-dev, out of memory\n);
-   error = -ENOMEM;
-   goto err_free_mem;
+   return -ENOMEM;
}

kbd-input = input_dev;
@@ -218,41 +221,29 @@ static int __devinit spear_kbd_probe(struct
platform_device *pdev)
if (!pdata) {
error = spear_kbd_parse_dt(pdev, kbd);
if (error)
-   goto err_free_mem;
+   return error;
} else {
kbd-mode = pdata-mode;
kbd-rep = pdata-rep;
kbd-suspended_rate = pdata-suspended_rate;
}

-   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 = devm_ioremap(pdev-dev, res-start, resource_size(res));
+   kbd-io_base = devm_request_and_ioremap(pdev-dev, res);
if (!kbd-io_base) {
-   dev_err(pdev-dev, ioremap failed for kbd_region\n);
-   error = -ENOMEM;
-   goto err_free_mem;
+   dev_err(pdev-dev, request-ioremap failed for kbd_region\n);
+   return -ENOMEM;
}

kbd-clk = devm_clk_get(pdev-dev, NULL);
-   if (IS_ERR(kbd-clk)) {
-   error = PTR_ERR(kbd-clk);
-   goto err_free_mem;
-   }
+   if (IS_ERR(kbd-clk))
+   return PTR_ERR(kbd-clk);

error = clk_prepare(kbd-clk);
if (error)
-   goto err_free_mem;
+   return error;

input_dev-name = Spear Keyboard;
input_dev-phys = keyboard/input0;
-   input_dev-dev.parent = pdev-dev;
input_dev-id.bustype = BUS_HOST;
input_dev-id.vendor = 0x0001;
input_dev-id.product = 0x0001;
@@ -293,8 +284,6 @@ static int __devinit spear_kbd_probe(struct
platform_device *pdev)

 err_unprepare_clk:
clk_unprepare(kbd-clk);
-err_free_mem:
-   input_free_device(input_dev);

return error;
 }
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines

2012-11-08 Thread Dmitry Torokhov 
On Fri, Nov 09, 2012 at 08:06:29AM +0530, Viresh Kumar wrote:
 On 8 November 2012 22:08, Dmitry Torokhov dmitry.torok...@gmail.com wrote:
  On Thu, Nov 08, 2012 at 07:10:47PM +0530, Viresh Kumar wrote:
  It also breaks the error unwinding/removal of the driver as it frees
  input device while IRQ handler is still active.
 
 I have heard of this argument before, probably from you. :)
 Just need clarification again. How will we get an interrupt when the 
 controller
 is stopped, unless we have a shared irq.

My bad, I missed that spear-keyboard driver implements open() and
close() methods and shuts off the device properly. Still, thanks for
switching everything to devm_*, I think it is much cleaner this way as
opposed to mixing managed and unmanaged resources.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/