2021-06-13 23:35:14

by Alexander Sverdlin

[permalink] [raw]
Subject: [PATCH 0/7] Prepare EP93xx drivers for Common Clock Framework

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


2021-06-13 23:35:18

by Alexander Sverdlin

[permalink] [raw]
Subject: [PATCH 7/7] pwm: ep93xx: Prepare clock before using it

Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
to Common Clock Framework.

Signed-off-by: Alexander Sverdlin <[email protected]>
---
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

2021-06-13 23:35:28

by Alexander Sverdlin

[permalink] [raw]
Subject: [PATCH 5/7] dmaengine: ep93xx: Prepare clock before using it

Use clk_prepare_enable()/clk_disable_unprepare() in preparation for switch
to Common Clock Framework, otherwise the following is visible:

WARNING: CPU: 0 PID: 1 at drivers/clk/clk.c:1011 clk_core_enable+0x9c/0xbc
Enabling unprepared m2p0
CPU: 0 PID: 1 Comm: swapper Not tainted 5.13.0-rc5-tekon #1
Hardware name: Cirrus Logic EDB9302 Evaluation Board
[<c000d5b0>] (unwind_backtrace) from [<c000c590>] (show_stack+0x10/0x18)
[<c000c590>] (show_stack) from [<c03a5fb8>] (dump_stack+0x20/0x2c)
[<c03a5fb8>] (dump_stack) from [<c03a2118>] (__warn+0x98/0xc0)
[<c03a2118>] (__warn) from [<c03a21d0>] (warn_slowpath_fmt+0x90/0xc0)
[<c03a21d0>] (warn_slowpath_fmt) from [<c01d8358>] (clk_core_enable+0x9c/0xbc)
[<c01d8358>] (clk_core_enable) from [<c01d8698>] (clk_core_enable_lock+0x18/0x30)
[<c01d8698>] (clk_core_enable_lock) from [<c01e1844>] (ep93xx_dma_alloc_chan_resources+0x94/0x244)
[<c01e1844>] (ep93xx_dma_alloc_chan_resources) from [<c01df7d4>] (dma_chan_get+0x90/0x124)
[<c01df7d4>] (dma_chan_get) from [<c01df92c>] (find_candidate+0xc4/0x188)
[<c01df92c>] (find_candidate) from [<c01dff30>] (__dma_request_channel+0x68/0xb0)
[<c01dff30>] (__dma_request_channel) from [<c027d2e4>] (snd_dmaengine_pcm_request_channel+0x68/0x90)
[<c027d2e4>] (snd_dmaengine_pcm_request_channel) from [<c0290618>] (dmaengine_pcm_new+0x254/0x29c)
[<c0290618>] (dmaengine_pcm_new) from [<c0289b84>] (snd_soc_pcm_component_new+0x40/0xa0)
[<c0289b84>] (snd_soc_pcm_component_new) from [<c028c7f8>] (soc_new_pcm+0x47c/0x5fc)
[<c028c7f8>] (soc_new_pcm) from [<c027f300>] (snd_soc_bind_card+0x73c/0x8a8)
[<c027f300>] (snd_soc_bind_card) from [<c029180c>] (edb93xx_probe+0x34/0x5c)
[<c029180c>] (edb93xx_probe) from [<c02126e0>] (platform_probe+0x34/0x80)
[<c02126e0>] (platform_probe) from [<c0210bf8>] (really_probe+0xe8/0x394)
[<c0210bf8>] (really_probe) from [<c0211464>] (device_driver_attach+0x5c/0x64)
[<c0211464>] (device_driver_attach) from [<c02114e8>] (__driver_attach+0x7c/0xec)
[<c02114e8>] (__driver_attach) from [<c020f1b4>] (bus_for_each_dev+0x78/0xc4)
[<c020f1b4>] (bus_for_each_dev) from [<c0211570>] (driver_attach+0x18/0x24)
[<c0211570>] (driver_attach) from [<c020fab4>] (bus_add_driver+0x140/0x1cc)
[<c020fab4>] (bus_add_driver) from [<c0211c44>] (driver_register+0x74/0x114)
[<c0211c44>] (driver_register) from [<c02134f8>] (__platform_driver_register+0x18/0x24)
[<c02134f8>] (__platform_driver_register) from [<c047084c>] (edb93xx_driver_init+0x10/0x1c)
[<c047084c>] (edb93xx_driver_init) from [<c045ce88>] (do_one_initcall+0x7c/0x1a4)
[<c045ce88>] (do_one_initcall) from [<c045d184>] (kernel_init_freeable+0x17c/0x1fc)
[<c045d184>] (kernel_init_freeable) from [<c03a6550>] (kernel_init+0x8/0xf8)
[<c03a6550>] (kernel_init) from [<c00082d8>] (ret_from_fork+0x14/0x3c)
...
ep93xx-i2s ep93xx-i2s: Missing dma channel for stream: 0
ep93xx-i2s ep93xx-i2s: ASoC: error at snd_soc_pcm_component_new on ep93xx-i2s: -22
edb93xx-audio edb93xx-audio: ASoC: can't create pcm CS4271 HiFi :-22
edb93xx-audio edb93xx-audio: snd_soc_register_card() failed: -22
edb93xx-audio: probe of edb93xx-audio failed with error -22

Signed-off-by: Alexander Sverdlin <[email protected]>
---
drivers/dma/ep93xx_dma.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
index 01027779beb8..98f9ee70362e 100644
--- a/drivers/dma/ep93xx_dma.c
+++ b/drivers/dma/ep93xx_dma.c
@@ -897,7 +897,7 @@ static int ep93xx_dma_alloc_chan_resources(struct dma_chan *chan)
if (data && data->name)
name = data->name;

- ret = clk_enable(edmac->clk);
+ ret = clk_prepare_enable(edmac->clk);
if (ret)
return ret;

@@ -936,7 +936,7 @@ static int ep93xx_dma_alloc_chan_resources(struct dma_chan *chan)
fail_free_irq:
free_irq(edmac->irq, edmac);
fail_clk_disable:
- clk_disable(edmac->clk);
+ clk_disable_unprepare(edmac->clk);

return ret;
}
@@ -969,7 +969,7 @@ static void ep93xx_dma_free_chan_resources(struct dma_chan *chan)
list_for_each_entry_safe(desc, d, &list, node)
kfree(desc);

- clk_disable(edmac->clk);
+ clk_disable_unprepare(edmac->clk);
free_irq(edmac->irq, edmac);
}

--
2.32.0

2021-06-14 07:17:49

by Uwe Kleine-König

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

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


Attachments:
(No filename) (809.00 B)
signature.asc (499.00 B)
Download all attachments

2021-06-14 07:24:27

by Uwe Kleine-König

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

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 <[email protected]>

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 <[email protected]>

Best regards
Uwe

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


Attachments:
(No filename) (692.00 B)
signature.asc (499.00 B)
Download all attachments

2021-06-14 07:34:25

by Alexander Sverdlin

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

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 <[email protected]>
>
> 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.


2021-09-14 01:14:58

by Alexander Sverdlin

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

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 <[email protected]>
>
> 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 <[email protected]>

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.


2021-09-14 07:59:16

by Uwe Kleine-König

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

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 <[email protected]>
> >
> > 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 <[email protected]>
>
> 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/[email protected]/).
There is also a v2, that is also marked as accepted
(http://patchwork.ozlabs.org/project/linux-pwm/patch/[email protected]/).

Not sure what want wrong here

Best regards
Uwe

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


Attachments:
(No filename) (1.37 kB)
signature.asc (499.00 B)
Download all attachments

2021-09-14 10:16:50

by Alexander Sverdlin

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

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 <[email protected]>
> > >
> > > 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 <[email protected]>
> >
> > 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/[email protected]/).
> There is also a v2, that is also marked as accepted
> (http://patchwork.ozlabs.org/project/linux-pwm/patch/[email protected]/).
>
> Not sure what want wrong here

Sorry for the noise!

--
Alexander Sverdlin.


2021-09-14 10:22:27

by Uwe Kleine-König

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

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 <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > >
> > > 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/[email protected]/).
> > There is also a v2, that is also marked as accepted
> > (http://patchwork.ozlabs.org/project/linux-pwm/patch/[email protected]/).
> >
> > 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/ |


Attachments:
(No filename) (1.62 kB)
signature.asc (499.00 B)
Download all attachments