* [PATCH 1/3] input: spear-keyboard: Use devm_*() routines
@ 2012-11-08 13:40 Viresh Kumar
2012-11-08 13:40 ` [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support Viresh Kumar
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Viresh Kumar @ 2012-11-08 13:40 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, spear-devel, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support
2012-11-08 13:40 [PATCH 1/3] input: spear-keyboard: Use devm_*() routines Viresh Kumar
@ 2012-11-08 13:40 ` Viresh Kumar
2012-11-12 5:57 ` Viresh Kumar
2012-11-08 13:40 ` [PATCH 3/3] input: spear-keyboard: Fix for balancing the enable_irq_wake Viresh Kumar
2012-11-08 16:38 ` [PATCH 1/3] input: spear-keyboard: Use devm_*() routines Dmitry Torokhov
2 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2012-11-08 13:40 UTC (permalink / raw)
To: dmitry.torokhov
Cc: linux-input, linux-kernel, spear-devel, Vipul Kumar Samar, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] input: spear-keyboard: Fix for balancing the enable_irq_wake
2012-11-08 13:40 [PATCH 1/3] input: spear-keyboard: Use devm_*() routines Viresh Kumar
2012-11-08 13:40 ` [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support Viresh Kumar
@ 2012-11-08 13:40 ` Viresh Kumar
2012-11-08 16:38 ` [PATCH 1/3] input: spear-keyboard: Use devm_*() routines Dmitry Torokhov
2 siblings, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2012-11-08 13:40 UTC (permalink / raw)
To: dmitry.torokhov
Cc: linux-input, linux-kernel, spear-devel, Deepak Sikri, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines
2012-11-08 13:40 [PATCH 1/3] input: spear-keyboard: Use devm_*() routines Viresh Kumar
2012-11-08 13:40 ` [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support Viresh Kumar
2012-11-08 13:40 ` [PATCH 3/3] input: spear-keyboard: Fix for balancing the enable_irq_wake Viresh Kumar
@ 2012-11-08 16:38 ` Dmitry Torokhov
2012-11-09 2:36 ` Viresh Kumar
2 siblings, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2012-11-08 16:38 UTC (permalink / raw)
To: Viresh Kumar; +Cc: linux-input, linux-kernel, spear-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines
2012-11-08 16:38 ` [PATCH 1/3] input: spear-keyboard: Use devm_*() routines Dmitry Torokhov
@ 2012-11-09 2:36 ` Viresh Kumar
2012-11-09 3:03 ` Viresh Kumar
2012-11-09 5:33 ` Dmitry Torokhov
0 siblings, 2 replies; 14+ messages in thread
From: Viresh Kumar @ 2012-11-09 2:36 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, spear-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines
2012-11-09 2:36 ` Viresh Kumar
@ 2012-11-09 3:03 ` Viresh Kumar
2012-11-09 5:33 ` Dmitry Torokhov
1 sibling, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2012-11-09 3:03 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, spear-devel
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;
}
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines
2012-11-09 2:36 ` Viresh Kumar
2012-11-09 3:03 ` Viresh Kumar
@ 2012-11-09 5:33 ` Dmitry Torokhov
2012-11-09 17:05 ` Viresh Kumar
1 sibling, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2012-11-09 5:33 UTC (permalink / raw)
To: Viresh Kumar; +Cc: linux-input, linux-kernel, spear-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] input: spear-keyboard: Use devm_*() routines
2012-11-09 5:33 ` Dmitry Torokhov
@ 2012-11-09 17:05 ` Viresh Kumar
0 siblings, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2012-11-09 17:05 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, spear-devel
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);
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support
2012-11-08 13:40 ` [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support Viresh Kumar
@ 2012-11-12 5:57 ` Viresh Kumar
2012-11-20 7:56 ` Viresh Kumar
0 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2012-11-12 5:57 UTC (permalink / raw)
To: dmitry.torokhov
Cc: linux-input, linux-kernel, spear-devel, Vipul Kumar Samar, 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 ?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support
2012-11-12 5:57 ` Viresh Kumar
@ 2012-11-20 7:56 ` Viresh Kumar
2012-11-20 8:47 ` Dmitry Torokhov
0 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2012-11-20 7:56 UTC (permalink / raw)
To: dmitry.torokhov
Cc: linux-input, linux-kernel, spear-devel, Vipul Kumar Samar, 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 :)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support
2012-11-20 7:56 ` Viresh Kumar
@ 2012-11-20 8:47 ` Dmitry Torokhov
2012-11-20 8:54 ` Viresh Kumar
2012-11-26 16:27 ` Viresh Kumar
0 siblings, 2 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2012-11-20 8:47 UTC (permalink / raw)
To: Viresh Kumar; +Cc: linux-input, linux-kernel, spear-devel, Vipul Kumar Samar
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support
2012-11-20 8:47 ` Dmitry Torokhov
@ 2012-11-20 8:54 ` Viresh Kumar
2012-11-26 16:27 ` Viresh Kumar
1 sibling, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2012-11-20 8:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, spear-devel, Vipul Kumar Samar
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 ?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support
2012-11-20 8:47 ` Dmitry Torokhov
2012-11-20 8:54 ` Viresh Kumar
@ 2012-11-26 16:27 ` Viresh Kumar
2012-11-26 16:56 ` Dmitry Torokhov
1 sibling, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2012-11-26 16:27 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, spear-devel, Vipul Kumar Samar
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support
2012-11-26 16:27 ` Viresh Kumar
@ 2012-11-26 16:56 ` Dmitry Torokhov
0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2012-11-26 16:56 UTC (permalink / raw)
To: Viresh Kumar; +Cc: linux-input, linux-kernel, spear-devel, Vipul Kumar Samar
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);
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-11-26 16:56 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-08 13:40 [PATCH 1/3] input: spear-keyboard: Use devm_*() routines Viresh Kumar
2012-11-08 13:40 ` [PATCH 2/3] Input: spear-keyboard: Add clk_{un}prepare() support Viresh Kumar
2012-11-12 5:57 ` Viresh Kumar
2012-11-20 7:56 ` Viresh Kumar
2012-11-20 8:47 ` Dmitry Torokhov
2012-11-20 8:54 ` Viresh Kumar
2012-11-26 16:27 ` Viresh Kumar
2012-11-26 16:56 ` Dmitry Torokhov
2012-11-08 13:40 ` [PATCH 3/3] input: spear-keyboard: Fix for balancing the enable_irq_wake Viresh Kumar
2012-11-08 16:38 ` [PATCH 1/3] input: spear-keyboard: Use devm_*() routines Dmitry Torokhov
2012-11-09 2:36 ` Viresh Kumar
2012-11-09 3:03 ` Viresh Kumar
2012-11-09 5:33 ` Dmitry Torokhov
2012-11-09 17:05 ` Viresh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).