2012-11-01 14:21:01

by Ulf Hansson

[permalink] [raw]
Subject: [PATCH 0/2] Input: nomadik-ske-keypad - clk fixups

From: Ulf Hansson <[email protected]>

Due to the convert to the common clk driver these changes for clks are needed.

Ulf Hansson (2):
Input: nomadik-ske-keypad - fixup use of clk
Input: nomadik-ske-keypad - start using the apb_pclk

drivers/input/keyboard/nomadik-ske-keypad.c | 34 +++++++++++++++++++++++----
1 file changed, 29 insertions(+), 5 deletions(-)

--
1.7.10


2012-11-01 14:20:38

by Ulf Hansson

[permalink] [raw]
Subject: [PATCH 2/2] Input: nomadik-ske-keypad - start using the apb_pclk

From: Ulf Hansson <[email protected]>

Previously this clock was handled internally by the clockdriver,
but now this is separate clk. So we need take care of it.

Signed-off-by: Ulf Hansson <[email protected]>
---
drivers/input/keyboard/nomadik-ske-keypad.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/nomadik-ske-keypad.c b/drivers/input/keyboard/nomadik-ske-keypad.c
index 95dcc9b..a1a9375 100644
--- a/drivers/input/keyboard/nomadik-ske-keypad.c
+++ b/drivers/input/keyboard/nomadik-ske-keypad.c
@@ -67,6 +67,7 @@ struct ske_keypad {
const struct ske_keypad_platform_data *board;
unsigned short keymap[SKE_KPD_NUM_ROWS * SKE_KPD_NUM_COLS];
struct clk *clk;
+ struct clk *pclk;
spinlock_t ske_keypad_lock;
};

@@ -271,11 +272,18 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
goto err_free_mem_region;
}

+ keypad->pclk = clk_get(&pdev->dev, "apb_pclk");
+ if (IS_ERR(keypad->pclk)) {
+ dev_err(&pdev->dev, "failed to get pclk\n");
+ error = PTR_ERR(keypad->pclk);
+ goto err_iounmap;
+ }
+
keypad->clk = clk_get(&pdev->dev, NULL);
if (IS_ERR(keypad->clk)) {
dev_err(&pdev->dev, "failed to get clk\n");
error = PTR_ERR(keypad->clk);
- goto err_iounmap;
+ goto err_pclk;
}

input->id.bustype = BUS_HOST;
@@ -294,10 +302,16 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
if (!plat->no_autorepeat)
__set_bit(EV_REP, input->evbit);

+ error = clk_prepare_enable(keypad->pclk);
+ if (error) {
+ dev_err(&pdev->dev, "Failed to prepare/enable pclk\n");
+ goto err_clk;
+ }
+
error = clk_prepare_enable(keypad->clk);
if (error) {
dev_err(&pdev->dev, "Failed to prepare/enable clk\n");
- goto err_clk;
+ goto err_pclk_disable;
}


@@ -336,8 +350,12 @@ err_free_irq:
free_irq(keypad->irq, keypad);
err_clk_disable:
clk_disable_unprepare(keypad->clk);
+err_pclk_disable:
+ clk_disable_unprepare(keypad->pclk);
err_clk:
clk_put(keypad->clk);
+err_pclk:
+ clk_put(keypad->pclk);
err_iounmap:
iounmap(keypad->reg_base);
err_free_mem_region:
--
1.7.10

2012-11-01 14:20:40

by Ulf Hansson

[permalink] [raw]
Subject: [PATCH 1/2] Input: nomadik-ske-keypad - fixup use of clk

From: Ulf Hansson <[email protected]>

Do proper error handling for clk and make sure clocks are being
prepared|unprepared as well as enabled|disabled.

Signed-off-by: Ulf Hansson <[email protected]>
---
drivers/input/keyboard/nomadik-ske-keypad.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/nomadik-ske-keypad.c b/drivers/input/keyboard/nomadik-ske-keypad.c
index 49f5fa6..95dcc9b 100644
--- a/drivers/input/keyboard/nomadik-ske-keypad.c
+++ b/drivers/input/keyboard/nomadik-ske-keypad.c
@@ -287,14 +287,19 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
keypad->keymap, input);
if (error) {
dev_err(&pdev->dev, "Failed to build keymap\n");
- goto err_iounmap;
+ goto err_clk;
}

input_set_capability(input, EV_MSC, MSC_SCAN);
if (!plat->no_autorepeat)
__set_bit(EV_REP, input->evbit);

- clk_enable(keypad->clk);
+ error = clk_prepare_enable(keypad->clk);
+ if (error) {
+ dev_err(&pdev->dev, "Failed to prepare/enable clk\n");
+ goto err_clk;
+ }
+

/* go through board initialization helpers */
if (keypad->board->init)
@@ -330,7 +335,8 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
err_free_irq:
free_irq(keypad->irq, keypad);
err_clk_disable:
- clk_disable(keypad->clk);
+ clk_disable_unprepare(keypad->clk);
+err_clk:
clk_put(keypad->clk);
err_iounmap:
iounmap(keypad->reg_base);
@@ -351,7 +357,7 @@ static int __devexit ske_keypad_remove(struct platform_device *pdev)

input_unregister_device(keypad->input);

- clk_disable(keypad->clk);
+ clk_disable_unprepare(keypad->clk);
clk_put(keypad->clk);

if (keypad->board->exit)
--
1.7.10

2012-11-04 18:09:55

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/2] Input: nomadik-ske-keypad - fixup use of clk

On Thu, Nov 1, 2012 at 3:20 PM, Ulf Hansson <[email protected]> wrote:

> From: Ulf Hansson <[email protected]>
>
> Do proper error handling for clk and make sure clocks are being
> prepared|unprepared as well as enabled|disabled.
>
> Signed-off-by: Ulf Hansson <[email protected]>

Acked-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2012-11-04 18:12:11

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 2/2] Input: nomadik-ske-keypad - start using the apb_pclk

On Thu, Nov 1, 2012 at 3:20 PM, Ulf Hansson <[email protected]> wrote:

> From: Ulf Hansson <[email protected]>
>
> Previously this clock was handled internally by the clockdriver,
> but now this is separate clk. So we need take care of it.
>
> Signed-off-by: Ulf Hansson <[email protected]>

So this is a silicon block clock and falls into the category of things
we've been discussing...

If I understand correctly, the only "real" solution is to
implement the PM domains and have these enable the clocks.

An alternative may be to move this driver over to the AMBA bus,
because I think this device actually has primecell registers.
Then the bus will take care of the pclk for starters.

Yours,
Linus Walleij

2012-11-05 12:25:15

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Input: nomadik-ske-keypad - start using the apb_pclk

On 4 November 2012 19:12, Linus Walleij <[email protected]> wrote:
> On Thu, Nov 1, 2012 at 3:20 PM, Ulf Hansson <[email protected]> wrote:
>
>> From: Ulf Hansson <[email protected]>
>>
>> Previously this clock was handled internally by the clockdriver,
>> but now this is separate clk. So we need take care of it.
>>
>> Signed-off-by: Ulf Hansson <[email protected]>
>
> So this is a silicon block clock and falls into the category of things
> we've been discussing...
>
> If I understand correctly, the only "real" solution is to
> implement the PM domains and have these enable the clocks.

Agree.

Although, since the pm_domain not yet exist, this as a way forward for
now - to fix what is broken.
When the pm_domain is in place and when we decide to fold in the clock
handling in there, we can move it.

>
> An alternative may be to move this driver over to the AMBA bus,
> because I think this device actually has primecell registers.
> Then the bus will take care of the pclk for starters.

You are definitely right, this driver can be converted into using the AMBA bus.
Although, do you think that should be done _instead_ of going ahead
with this patch or do you want that to be handled as a next and a
separate step?

Kind regards
Ulf Hansson

2012-11-05 12:37:06

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 2/2] Input: nomadik-ske-keypad - start using the apb_pclk

On Mon, Nov 5, 2012 at 1:25 PM, Ulf Hansson <[email protected]> wrote:
> On 4 November 2012 19:12, Linus Walleij <[email protected]> wrote:

>> If I understand correctly, the only "real" solution is to
>> implement the PM domains and have these enable the clocks.
>
> Agree.
>
> Although, since the pm_domain not yet exist, this as a way forward for
> now - to fix what is broken.

True. Fixing regressions is more important.
Acked-by: Linus Walleij <[email protected]>

> When the pm_domain is in place and when we decide to fold in the clock
> handling in there, we can move it.

OK for me.

>> An alternative may be to move this driver over to the AMBA bus,
>> because I think this device actually has primecell registers.
>> Then the bus will take care of the pclk for starters.
>
> You are definitely right, this driver can be converted into using the AMBA bus.
> Although, do you think that should be done _instead_ of going ahead
> with this patch or do you want that to be handled as a next and a
> separate step?

No, just a suggestion of possibilities...

Yours,
Linus Walleij

2012-11-23 15:54:13

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 0/2] Input: nomadik-ske-keypad - clk fixups

On 1 November 2012 15:20, Ulf Hansson <[email protected]> wrote:
> From: Ulf Hansson <[email protected]>
>
> Due to the convert to the common clk driver these changes for clks are needed.
>
> Ulf Hansson (2):
> Input: nomadik-ske-keypad - fixup use of clk
> Input: nomadik-ske-keypad - start using the apb_pclk
>
> drivers/input/keyboard/nomadik-ske-keypad.c | 34 +++++++++++++++++++++++----
> 1 file changed, 29 insertions(+), 5 deletions(-)
>
> --
> 1.7.10
>

Just a kind reminder here. Do we see any issues with merging this?

Kind regards
Ulf Hansson

2012-11-24 08:15:45

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 0/2] Input: nomadik-ske-keypad - clk fixups

Hi Ulf,

On Fri, Nov 23, 2012 at 04:54:09PM +0100, Ulf Hansson wrote:
> On 1 November 2012 15:20, Ulf Hansson <[email protected]> wrote:
> > From: Ulf Hansson <[email protected]>
> >
> > Due to the convert to the common clk driver these changes for clks are needed.
> >
> > Ulf Hansson (2):
> > Input: nomadik-ske-keypad - fixup use of clk
> > Input: nomadik-ske-keypad - start using the apb_pclk
> >
> > drivers/input/keyboard/nomadik-ske-keypad.c | 34 +++++++++++++++++++++++----
> > 1 file changed, 29 insertions(+), 5 deletions(-)
> >
> > --
> > 1.7.10
> >
>
> Just a kind reminder here. Do we see any issues with merging this?

Both are in my 'next' branch, sorry for not notifying you earlier.

--
Dmitry