[PATCH 0/7] Prepare EP93xx drivers for Common Clock Framework

[PATCH 0/7] Prepare EP93xx drivers for Common Clock Framework

[Alexander Sverdlin]

Nikita posted a patch converting EP93xx to use Common Clock Framework. It
turns out some cleanup is necessary in the EP93xx drivers to avoid
"Enabling unprepared" clock warnings.

Patches with stack traces in the commit messages are tested on EP9302.

Link: https://lore.kernel.org/patchwork/patch/1435884/

Alexander Sverdlin (7):
  iio: ep93xx: Prepare clock before using it
  spi: spi-ep93xx: Prepare clock before using it
  Input: ep93xx_keypad: Prepare clock before using it
  video: ep93xx: Prepare clock before using it
  dmaengine: ep93xx: Prepare clock before using it
  ASoC: cirrus: i2s: Prepare clock before using it
  pwm: ep93xx: Prepare clock before using it

 drivers/dma/ep93xx_dma.c               |  6 +++---
 drivers/iio/adc/ep93xx_adc.c           |  6 +++---
 drivers/input/keyboard/ep93xx_keypad.c |  4 ++--
 drivers/pwm/pwm-ep93xx.c               | 12 ++++++------
 drivers/spi/spi-ep93xx.c               |  4 ++--
 drivers/video/fbdev/ep93xx-fb.c        |  4 ++--
 sound/soc/cirrus/ep93xx-i2s.c          | 12 ++++++------
 7 files changed, 24 insertions(+), 24 deletions(-)

-- 
2.32.0

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

[Alexander Sverdlin]

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(-)

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;
 	}
 }
-- 
2.32.0

Re: [PATCH 0/7] Prepare EP93xx drivers for Common Clock Framework

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

On Mon, Jun 14, 2021 at 01:30:34AM +0200, Alexander Sverdlin wrote:
> Nikita posted a patch converting EP93xx to use Common Clock Framework. It
> turns out some cleanup is necessary in the EP93xx drivers to avoid
> "Enabling unprepared" clock warnings.
> 
> Patches with stack traces in the commit messages are tested on EP9302.

One thing to note is: ep93xx currently doesn't provide a clk_prepare
function, this isn't a problem though because include/linux/clk.h
provides a dummy if CONFIG_HAVE_CLK_PREPARE isn't defined. So as ep93xx
doesn't define this symbol the changes here effectively only add a
might_sleep.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Dmitry Torokhov]

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.

-- 
Dmitry

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

[Alexander Sverdlin]

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.

-- 
Alexander Sverdlin.

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

[Dmitry Torokhov]

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>

-- 
Dmitry

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

[Alexander Sverdlin]

Hello Dmitry!

On 20/06/2021 05:23, 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>

Seems that this doesn't work as we planned and two patches of the series
were already taken in by the respective maintainers.
Could you please apply this patch as well to your tree?

--
Alex.

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

[Alexander Sverdlin]

Hello Dmitry,

On Sat, 2021-06-19 at 20:23 -0700, Dmitry Torokhov wrote:
> > 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>

our initial attempt to find a maintainer for the whole series
didn't work out. Would you take this single patch, please?
Three others were already taken into respective subsystems
and I'll ping the rest of maintainers individually...

-- 
Alexander Sverdlin.

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

[Dmitry Torokhov]

Hi Alexander,

On Mon, Sep 13, 2021 at 11:29:14PM +0200, Alexander Sverdlin wrote:
> Hello Dmitry,
> 
> On Sat, 2021-06-19 at 20:23 -0700, Dmitry Torokhov wrote:
> > > 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>
> 
> our initial attempt to find a maintainer for the whole series
> didn't work out. Would you take this single patch, please?
> Three others were already taken into respective subsystems
> and I'll ping the rest of maintainers individually...

It looks like I forgot to mention it, but I applied this patch and it
should be in mainline now. I also CCed you on a few patches to
ep93xx_keyboard driver and woudl appreciate if you looked them over as I
do not have the hardware.

Thanks.

-- 
Dmitry

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

[Alexander Sverdlin]

Hello Dmitry,

On Mon, 2021-10-11 at 18:43 -0700, Dmitry Torokhov wrote:
> It looks like I forgot to mention it, but I applied this patch and it
> should be in mainline now. I also CCed you on a few patches to
> ep93xx_keyboard driver and woudl appreciate if you looked them over as I
> do not have the hardware.

thank you for the note!
I've acked 3 of the 4 patches you've sent, but will have comments on the
4th one...

-- 
Alexander Sverdlin.