[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 7/7] pwm: ep93xx: 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/pwm/pwm-ep93xx.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pwm/pwm-ep93xx.c b/drivers/pwm/pwm-ep93xx.c
index 4ca70794ad96..8c0d4d69d9e6 100644
--- a/drivers/pwm/pwm-ep93xx.c
+++ b/drivers/pwm/pwm-ep93xx.c
@@ -74,7 +74,7 @@ static int ep93xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * Configuration can be changed at any time.
 	 */
 	if (!pwm_is_enabled(pwm)) {
-		ret = clk_enable(ep93xx_pwm->clk);
+		ret = clk_prepare_enable(ep93xx_pwm->clk);
 		if (ret)
 			return ret;
 	}
@@ -105,7 +105,7 @@ static int ep93xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	}
 
 	if (!pwm_is_enabled(pwm))
-		clk_disable(ep93xx_pwm->clk);
+		clk_disable_unprepare(ep93xx_pwm->clk);
 
 	return ret;
 }
@@ -120,7 +120,7 @@ static int ep93xx_pwm_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * The clock needs to be enabled to access the PWM registers.
 	 * Polarity can only be changed when the PWM is disabled.
 	 */
-	ret = clk_enable(ep93xx_pwm->clk);
+	ret = clk_prepare_enable(ep93xx_pwm->clk);
 	if (ret)
 		return ret;
 
@@ -129,7 +129,7 @@ static int ep93xx_pwm_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
 	else
 		writew(0x0, ep93xx_pwm->base + EP93XX_PWMx_INVERT);
 
-	clk_disable(ep93xx_pwm->clk);
+	clk_disable_unprepare(ep93xx_pwm->clk);
 
 	return 0;
 }
@@ -139,7 +139,7 @@ static int ep93xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	struct ep93xx_pwm *ep93xx_pwm = to_ep93xx_pwm(chip);
 	int ret;
 
-	ret = clk_enable(ep93xx_pwm->clk);
+	ret = clk_prepare_enable(ep93xx_pwm->clk);
 	if (ret)
 		return ret;
 
@@ -153,7 +153,7 @@ static void ep93xx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	struct ep93xx_pwm *ep93xx_pwm = to_ep93xx_pwm(chip);
 
 	writew(0x0, ep93xx_pwm->base + EP93XX_PWMx_ENABLE);
-	clk_disable(ep93xx_pwm->clk);
+	clk_disable_unprepare(ep93xx_pwm->clk);
 }
 
 static const struct pwm_ops ep93xx_pwm_ops = {
-- 
2.32.0

Re: [PATCH 7/7] pwm: ep93xx: Prepare clock before using it

[Uwe Kleine-König]

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

On Mon, Jun 14, 2021 at 01:30:41AM +0200, Alexander Sverdlin wrote:
> Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
> to Common Clock Framework.
> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>

Maybe it would make sense to move the prepare into the probe function?!
Anyhow, for now preparing the driver for the common-clk switch is the
focus and for that the conversion is correct, so:

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

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 7/7] pwm: ep93xx: Prepare clock before using it

[Alexander Sverdlin]

Hi Uwe!

On Mon, 2021-06-14 at 09:22 +0200, Uwe Kleine-König wrote:
> > Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
> > to Common Clock Framework.
> > 
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> 
> Maybe it would make sense to move the prepare into the probe function?!

If one thinks about real meaningful clk_prepare(), not like in EP93xx,
then clk_is_enabled_when_prepared() shall be considered and this might
change behaviour. That's why this "stupid" approach was chosen for this
conversion. Also, unfortunately, I don't have a test setup for PWM, this
made me shy towards this driver ;)

-- 
Alexander Sverdlin.

Re: [PATCH 7/7] pwm: ep93xx: Prepare clock before using it

[Alexander Sverdlin]

Hello Thierry,

On Mon, 2021-06-14 at 09:22 +0200, Uwe Kleine-König wrote:
> On Mon, Jun 14, 2021 at 01:30:41AM +0200, Alexander Sverdlin wrote:
> > Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
> > to Common Clock Framework.
> > 
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> 
> Maybe it would make sense to move the prepare into the probe function?!
> Anyhow, for now preparing the driver for the common-clk switch is the
> focus and for that the conversion is correct, so:
> 
> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

would you take this patch only, please?
It didn't work out to sell the whole series as one piece and
most of them were taken individually...

-- 
Alexander Sverdlin.

Re: [PATCH 7/7] pwm: ep93xx: Prepare clock before using it

[Uwe Kleine-König]

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

On Mon, Sep 13, 2021 at 11:46:41PM +0200, Alexander Sverdlin wrote:
> Hello Thierry,
> 
> On Mon, 2021-06-14 at 09:22 +0200, Uwe Kleine-König wrote:
> > On Mon, Jun 14, 2021 at 01:30:41AM +0200, Alexander Sverdlin wrote:
> > > Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
> > > to Common Clock Framework.
> > > 
> > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> > 
> > Maybe it would make sense to move the prepare into the probe function?!
> > Anyhow, for now preparing the driver for the common-clk switch is the
> > focus and for that the conversion is correct, so:
> > 
> > Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> would you take this patch only, please?
> It didn't work out to sell the whole series as one piece and
> most of them were taken individually...

Hmm, this patch is marked as accepted in patchwork
(http://patchwork.ozlabs.org/project/linux-pwm/patch/20210613233041.128961-8-alexander.sverdlin@gmail.com/).
There is also a v2, that is also marked as accepted
(http://patchwork.ozlabs.org/project/linux-pwm/patch/20210726140001.24820-8-nikita.shubin@maquefel.me/).

Not sure what want wrong here

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 7/7] pwm: ep93xx: Prepare clock before using it

[Alexander Sverdlin]

Thank you, Uwe,

On Tue, 2021-09-14 at 09:58 +0200, Uwe Kleine-König wrote:
> > > On Mon, Jun 14, 2021 at 01:30:41AM +0200, Alexander Sverdlin wrote:
> > > > Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
> > > > to Common Clock Framework.
> > > > 
> > > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> > > 
> > > Maybe it would make sense to move the prepare into the probe function?!
> > > Anyhow, for now preparing the driver for the common-clk switch is the
> > > focus and for that the conversion is correct, so:
> > > 
> > > Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > would you take this patch only, please?
> > It didn't work out to sell the whole series as one piece and
> > most of them were taken individually...
> 
> Hmm, this patch is marked as accepted in patchwork
> (http://patchwork.ozlabs.org/project/linux-pwm/patch/20210613233041.128961-8-alexander.sverdlin@gmail.com/).
> There is also a v2, that is also marked as accepted
> (http://patchwork.ozlabs.org/project/linux-pwm/patch/20210726140001.24820-8-nikita.shubin@maquefel.me/).
> 
> Not sure what want wrong here

Sorry for the noise!

-- 
Alexander Sverdlin.

Re: [PATCH 7/7] pwm: ep93xx: Prepare clock before using it

[Uwe Kleine-König]

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

On Tue, Sep 14, 2021 at 12:15:14PM +0200, Alexander Sverdlin wrote:
> Thank you, Uwe,
> 
> On Tue, 2021-09-14 at 09:58 +0200, Uwe Kleine-König wrote:
> > > > On Mon, Jun 14, 2021 at 01:30:41AM +0200, Alexander Sverdlin wrote:
> > > > > Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
> > > > > to Common Clock Framework.
> > > > > 
> > > > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> > > > 
> > > > Maybe it would make sense to move the prepare into the probe function?!
> > > > Anyhow, for now preparing the driver for the common-clk switch is the
> > > > focus and for that the conversion is correct, so:
> > > > 
> > > > Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > 
> > > would you take this patch only, please?
> > > It didn't work out to sell the whole series as one piece and
> > > most of them were taken individually...
> > 
> > Hmm, this patch is marked as accepted in patchwork
> > (http://patchwork.ozlabs.org/project/linux-pwm/patch/20210613233041.128961-8-alexander.sverdlin@gmail.com/).
> > There is also a v2, that is also marked as accepted
> > (http://patchwork.ozlabs.org/project/linux-pwm/patch/20210726140001.24820-8-nikita.shubin@maquefel.me/).
> > 
> > Not sure what want wrong here
> 
> Sorry for the noise!

No, it's no noise because I didn't see either of the versions in any
tree. So the patches were marked as applied without being applied ...

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 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 0/7] Prepare EP93xx drivers for Common Clock Framework

[Andy Shevchenko]

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

On Monday, June 14, 2021, Alexander Sverdlin <alexander.sverdlin@gmail.com>
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.
>
> Link: https://lore.kernel.org/patchwork/patch/1435884/
>
>
For all commit messages: please, reduce the noise in them as much as
possible, i.e. leave only up to ~3-4 most significant lines out of
trackbacks.



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

-- 
With Best Regards,
Andy Shevchenko

[-- Attachment #2: Type: text/html, Size: 2065 bytes --]