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

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

[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.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));
 	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

[PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support

[Viresh Kumar]

From: Vipul Kumar Samar <vipulkumar.samar@st.com>

clk_{un}prepare is mandatory for platforms using common clock framework. Because
for SPEAr we don't do anything in clk_{un}prepare() calls, just call them ones
in probe/remove.

Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/input/keyboard/spear-keyboard.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c
index 1d24fb2..9792924 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -245,6 +245,10 @@ static int __devinit spear_kbd_probe(struct platform_device *pdev)
 		goto err_free_mem;
 	}
 
+	error = clk_prepare(kbd->clk);
+	if (error)
+		goto err_free_mem;
+
 	input_dev->name = "Spear Keyboard";
 	input_dev->phys = "keyboard/input0";
 	input_dev->dev.parent = &pdev->dev;
@@ -259,7 +263,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_free_mem;
+		goto err_unprepare_clk;
 	}
 
 	if (kbd->rep)
@@ -272,13 +276,13 @@ static int __devinit spear_kbd_probe(struct platform_device *pdev)
 			"keyboard", kbd);
 	if (error) {
 		dev_err(&pdev->dev, "request_irq fail\n");
-		goto err_free_mem;
+		goto err_unprepare_clk;
 	}
 
 	error = input_register_device(input_dev);
 	if (error) {
 		dev_err(&pdev->dev, "Unable to register keyboard device\n");
-		goto err_free_mem;
+		goto err_unprepare_clk;
 	}
 
 	device_init_wakeup(&pdev->dev, 1);
@@ -286,6 +290,8 @@ static int __devinit spear_kbd_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_unprepare_clk:
+	clk_unprepare(kbd->clk);
 err_free_mem:
 	input_free_device(input_dev);
 
@@ -297,6 +303,7 @@ 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);
 	platform_set_drvdata(pdev, NULL);
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support

[Viresh Kumar]

On 8 November 2012 19:10, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> From: Vipul Kumar Samar <vipulkumar.samar@st.com>
>
> clk_{un}prepare is mandatory for platforms using common clock framework. Because
> for SPEAr we don't do anything in clk_{un}prepare() calls, just call them ones
> in probe/remove.
>
> Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Missed applying this one ?

Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support

[Viresh Kumar]

On 12 November 2012 11:27, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 8 November 2012 19:10, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> From: Vipul Kumar Samar <vipulkumar.samar@st.com>
>>
>> clk_{un}prepare is mandatory for platforms using common clock framework. Because
>> for SPEAr we don't do anything in clk_{un}prepare() calls, just call them ones
>> in probe/remove.
>>
>> Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> Missed applying this one ?

This one you missed :)

Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support

[Dmitry Torokhov]

On Tue, Nov 20, 2012 at 01:26:29PM +0530, Viresh Kumar wrote:
> On 12 November 2012 11:27, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > On 8 November 2012 19:10, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >> From: Vipul Kumar Samar <vipulkumar.samar@st.com>
> >>
> >> clk_{un}prepare is mandatory for platforms using common clock framework. Because
> >> for SPEAr we don't do anything in clk_{un}prepare() calls, just call them ones
> >> in probe/remove.
> >>
> >> Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
> >> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> >
> > Missed applying this one ?
> 
> This one you missed :)

No, not really, it just does not work well with devm_* patches that got
applied: on removal you unprepare clock as the very first operation and
then devm_* does the rest which is wrong order.

I am looking at adding dem_* for clocks.

Thanks.

-- 
Dmitry

Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support

[Viresh Kumar]

On 20 November 2012 14:17, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> No, not really, it just does not work well with devm_* patches that got
> applied: on removal you unprepare clock as the very first operation and
> then devm_* does the rest which is wrong order.

Nice. I don't expect the order would do anything wrong, as we aren't
touching any registers between unpreparing clock and devm_ undo stuff.

> I am looking at adding dem_* for clocks.

You mean, adding devm_clk_prepare, devm_clk_enable, devm_clk_prepare_enable ?

Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support

[Viresh Kumar]

On 20 November 2012 14:17, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> No, not really, it just does not work well with devm_* patches that got
> applied: on removal you unprepare clock as the very first operation and
> then devm_* does the rest which is wrong order.
>
> I am looking at adding dem_* for clocks.

I haven't seen your clock patch in your tree. I am asking because i want to
push this patch :)

You want me to repost it over your patches.

--
viresh

Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support

[Dmitry Torokhov]

On Mon, Nov 26, 2012 at 09:57:45PM +0530, Viresh Kumar wrote:
> On 20 November 2012 14:17, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > No, not really, it just does not work well with devm_* patches that got
> > applied: on removal you unprepare clock as the very first operation and
> > then devm_* does the rest which is wrong order.
> >
> > I am looking at adding dem_* for clocks.
> 
> I haven't seen your clock patch in your tree. I am asking because i want to
> push this patch :)

Yeah, I think I am not going to rush it and let it take its normal
course instead of short-cutting through my tree. I am going to apply the
patcch below in the meantime.

Thanks.

-- 
Dmitry


Input: spear-keyboard - Add clk_{un}prepare() support

From: Vipul Kumar Samar <vipulkumar.samar@st.com>

clk_{un}prepare is mandatory for platforms using common clock framework.
Because for SPEAr we don't do anything in clk_{un}prepare() calls, just
call them once in probe/remove.

Signed-off-by: Vipul Kumar Samar <vipulkumar.samar@st.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/spear-keyboard.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c
index d70093b..695d237 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -267,9 +267,14 @@ static int spear_kbd_probe(struct platform_device *pdev)
 		return error;
 	}
 
+	error = clk_prepare(kbd->clk);
+	if (error)
+		return error;
+
 	error = input_register_device(input_dev);
 	if (error) {
 		dev_err(&pdev->dev, "Unable to register keyboard device\n");
+		clk_unprepare(kbd->clk);
 		return error;
 	}
 
@@ -281,6 +286,11 @@ static int spear_kbd_probe(struct platform_device *pdev)
 
 static int 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);
 	platform_set_drvdata(pdev, NULL);
 

[PATCH 3/3] input: spear-keyboard: Fix for balancing the enable_irq_wake

[Viresh Kumar]

From: Deepak Sikri <deepak.sikri@st.com>

This patch handles the fix for unbalanced irq for the cases when
enable_irq_wake fails, and a warning related to same is displayed
on the console. The workaround is handled at the driver level.

Signed-off-by: Deepak Sikri <deepak.sikri@st.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/input/keyboard/spear-keyboard.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c
index 9792924..b8784df 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -60,6 +60,7 @@ struct spear_kbd {
 	struct clk *clk;
 	unsigned int irq;
 	unsigned int mode;
+	unsigned int irq_wake;
 	unsigned short last_key;
 	unsigned short keycodes[NUM_ROWS * NUM_COLS];
 	bool rep;
@@ -327,7 +328,8 @@ static int spear_kbd_suspend(struct device *dev)
 	mode_ctl_reg = readl_relaxed(kbd->io_base + MODE_CTL_REG);
 
 	if (device_may_wakeup(&pdev->dev)) {
-		enable_irq_wake(kbd->irq);
+		if (!enable_irq_wake(kbd->irq))
+			kbd->irq_wake = 1;
 
 		/*
 		 * reprogram the keyboard operating frequency as on some
@@ -373,7 +375,10 @@ static int spear_kbd_resume(struct device *dev)
 	mutex_lock(&input_dev->mutex);
 
 	if (device_may_wakeup(&pdev->dev)) {
-		disable_irq_wake(kbd->irq);
+		if (kbd->irq_wake) {
+			kbd->irq_wake = 0;
+			disable_irq_wake(kbd->irq);
+		}
 	} else {
 		if (input_dev->users)
 			clk_enable(kbd->clk);
-- 
1.7.12.rc2.18.g61b472e

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

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

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

[Viresh Kumar]

On 8 November 2012 22:08, Dmitry Torokhov <dmitry.torokhov@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

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

[Viresh Kumar]

On 9 November 2012 08:06, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 8 November 2012 22:08, Dmitry Torokhov <dmitry.torokhov@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.kumar@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;
 }

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

[Dmitry Torokhov]

On Fri, Nov 09, 2012 at 08:06:29AM +0530, Viresh Kumar wrote:
> On 8 November 2012 22:08, Dmitry Torokhov <dmitry.torokhov@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

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

[Viresh Kumar]

On 9 November 2012 11:03, Dmitry Torokhov <dmitry.torokhov@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.torokhov@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.kumar@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);