diff mbox series

[3/7] Input: ep93xx_keypad: Prepare clock before using it

Message ID 20210613233041.128961-4-alexander.sverdlin@gmail.com (mailing list archive)
State Under Review
Headers show
Series Prepare EP93xx drivers for Common Clock Framework | expand

Commit Message

Alexander Sverdlin June 13, 2021, 11:30 p.m. UTC
Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
to Common Clock Framework.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
 drivers/input/keyboard/ep93xx_keypad.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dmitry Torokhov June 14, 2021, 9:55 p.m. UTC | #1
Hi Alexander,

On Mon, Jun 14, 2021 at 01:30:37AM +0200, Alexander Sverdlin wrote:
> Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
> to Common Clock Framework.

Can this be merged standalone?

> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> ---
>  drivers/input/keyboard/ep93xx_keypad.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c
> index c8194333d612..e0e931e796fa 100644
> --- a/drivers/input/keyboard/ep93xx_keypad.c
> +++ b/drivers/input/keyboard/ep93xx_keypad.c
> @@ -157,7 +157,7 @@ static int ep93xx_keypad_open(struct input_dev *pdev)
>  
>  	if (!keypad->enabled) {
>  		ep93xx_keypad_config(keypad);
> -		clk_enable(keypad->clk);
> +		clk_prepare_enable(keypad->clk);
>  		keypad->enabled = true;
>  	}
>  
> @@ -169,7 +169,7 @@ static void ep93xx_keypad_close(struct input_dev *pdev)
>  	struct ep93xx_keypad *keypad = input_get_drvdata(pdev);
>  
>  	if (keypad->enabled) {
> -		clk_disable(keypad->clk);
> +		clk_disable_unprepare(keypad->clk);
>  		keypad->enabled = false;

While we are at it, I wonder about handling suspend/resume. I see that
we disable the clock even if keyboard is configured as a wakeup source.
Is it really capable of waking up the system when clock is off?

Thanks.
Alexander Sverdlin June 15, 2021, 7:46 a.m. UTC | #2
Hello Dmitry!

On Mon, 2021-06-14 at 14:55 -0700, Dmitry Torokhov wrote:
> > Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
> > to Common Clock Framework.
> 
> Can this be merged standalone?

In principle, yes, but I thought it would be easier if the patches
would go via the same path as CCF conversion.

> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> > ---
> >   drivers/input/keyboard/ep93xx_keypad.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c
> > index c8194333d612..e0e931e796fa 100644
> > --- a/drivers/input/keyboard/ep93xx_keypad.c
> > +++ b/drivers/input/keyboard/ep93xx_keypad.c
> > @@ -157,7 +157,7 @@ static int ep93xx_keypad_open(struct input_dev *pdev)
> >   
> >         if (!keypad->enabled) {
> >                 ep93xx_keypad_config(keypad);
> > -               clk_enable(keypad->clk);
> > +               clk_prepare_enable(keypad->clk);
> >                 keypad->enabled = true;
> >         }
> >   
> > @@ -169,7 +169,7 @@ static void ep93xx_keypad_close(struct input_dev *pdev)
> >         struct ep93xx_keypad *keypad = input_get_drvdata(pdev);
> >   
> >         if (keypad->enabled) {
> > -               clk_disable(keypad->clk);
> > +               clk_disable_unprepare(keypad->clk);
> >                 keypad->enabled = false;
> 
> While we are at it, I wonder about handling suspend/resume. I see that
> we disable the clock even if keyboard is configured as a wakeup source.
> Is it really capable of waking up the system when clock is off?

That what "EP93xx User’s Guide" says:

26.2.4 Low Power Mode
The key scanning block also supports a low power wake-up mode. In this mode, a key press
generates a wake up interrupt. The key scan interrupt should be masked. Because the wake
up interrupt is asynchronous, and depends on external keypad lines which may have a large
capacitance value, glitches may occur on the interrupt when transitioning to low power mode.
After transitioning, all clocks to the key scanning circuitry can be shut down.
Dmitry Torokhov June 20, 2021, 3:23 a.m. UTC | #3
On Tue, Jun 15, 2021 at 09:46:51AM +0200, Alexander Sverdlin wrote:
> Hello Dmitry!
> 
> On Mon, 2021-06-14 at 14:55 -0700, Dmitry Torokhov wrote:
> > > Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
> > > to Common Clock Framework.
> > 
> > Can this be merged standalone?
> 
> In principle, yes, but I thought it would be easier if the patches
> would go via the same path as CCF conversion.

OK, in this case:

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
diff mbox series

Patch

diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c
index c8194333d612..e0e931e796fa 100644
--- a/drivers/input/keyboard/ep93xx_keypad.c
+++ b/drivers/input/keyboard/ep93xx_keypad.c
@@ -157,7 +157,7 @@  static int ep93xx_keypad_open(struct input_dev *pdev)
 
 	if (!keypad->enabled) {
 		ep93xx_keypad_config(keypad);
-		clk_enable(keypad->clk);
+		clk_prepare_enable(keypad->clk);
 		keypad->enabled = true;
 	}
 
@@ -169,7 +169,7 @@  static void ep93xx_keypad_close(struct input_dev *pdev)
 	struct ep93xx_keypad *keypad = input_get_drvdata(pdev);
 
 	if (keypad->enabled) {
-		clk_disable(keypad->clk);
+		clk_disable_unprepare(keypad->clk);
 		keypad->enabled = false;
 	}
 }