LinuxLists.cc - [PATCH v2 0/4] ARM: at91: prepare transition to common clk framework

2013-06-25 08:08:32

Subject: [PATCH v2 0/4] ARM: at91: prepare transition to common clk framework

Hello,

This patch series prepares the transition to common clk framework by
replacing all the clk_enable and clk_disable calls by clk_prepare_unable and
clk_disable_unprepare to avoid common clock framework warnings.

Best Regards,
Boris

Changes since v1:
- remove unneeded clk_unprepare in mci driver
- remove already applied patches (ehci, ohci, dma and serial)

Changes since common clk patch series:
- add clk_prepare_enable return check
- fix a deadlock in atmel-mci (missing spin_unlock)
- remove already applied patches (atmel-ssc and pwm-atmel-tcb)

Boris BREZILLON (4):
ARM: at91/tc/clocksource: replace clk_enable/disable with
clk_prepare_enable/disable_unprepare.
mmc: atmel-mci: prepare clk before calling enable
usb: gadget: at91_udc: prepare clk before calling enable
at91/avr32/atmel_lcdfb: prepare clk before calling enable

drivers/clocksource/tcb_clksrc.c | 10 +++++-----
drivers/mmc/host/atmel-mci.c | 27 ++++++++++++++++++---------
drivers/usb/gadget/at91_udc.c | 14 ++++++++------
drivers/video/atmel_lcdfb.c | 8 ++++----
4 files changed, 35 insertions(+), 24 deletions(-)

--
1.7.9.5

2013-06-25 08:10:14

by Boris BREZILLON

[permalink] [raw]

Subject: [PATCH v2 1/4] ARM: at91/tc/clocksource: replace clk_enable/disable with clk_prepare_enable/disable_unprepare.

Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.

Signed-off-by: Boris BREZILLON <[email protected]>
---
drivers/clocksource/tcb_clksrc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index 8a61872..229c019 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -100,7 +100,7 @@ static void tc_mode(enum clock_event_mode m, struct clock_event_device *d)
|| tcd->clkevt.mode == CLOCK_EVT_MODE_ONESHOT) {
__raw_writel(0xff, regs + ATMEL_TC_REG(2, IDR));
__raw_writel(ATMEL_TC_CLKDIS, regs + ATMEL_TC_REG(2, CCR));
- clk_disable(tcd->clk);
+ clk_disable_unprepare(tcd->clk);
}

switch (m) {
@@ -109,7 +109,7 @@ static void tc_mode(enum clock_event_mode m, struct clock_event_device *d)
* of oneshot, we get lower overhead and improved accuracy.
*/
case CLOCK_EVT_MODE_PERIODIC:
- clk_enable(tcd->clk);
+ clk_prepare_enable(tcd->clk);

/* slow clock, count up to RC, then irq and restart */
__raw_writel(timer_clock
@@ -126,7 +126,7 @@ static void tc_mode(enum clock_event_mode m, struct clock_event_device *d)
break;

case CLOCK_EVT_MODE_ONESHOT:
- clk_enable(tcd->clk);
+ clk_prepare_enable(tcd->clk);

/* slow clock, count up to RC, then irq and stop */
__raw_writel(timer_clock | ATMEL_TC_CPCSTOP
@@ -275,7 +275,7 @@ static int __init tcb_clksrc_init(void)
pdev = tc->pdev;

t0_clk = tc->clk[0];
- clk_enable(t0_clk);
+ clk_prepare_enable(t0_clk);

/* How fast will we be counting? Pick something over 5 MHz. */
rate = (u32) clk_get_rate(t0_clk);
@@ -313,7 +313,7 @@ static int __init tcb_clksrc_init(void)
/* tclib will give us three clocks no matter what the
* underlying platform supports.
*/
- clk_enable(tc->clk[1]);
+ clk_prepare_enable(tc->clk[1]);
/* setup both channel 0 & 1 */
tcb_setup_dual_chan(tc, best_divisor_idx);
}
--
1.7.9.5

2013-06-25 08:11:46

by Boris BREZILLON

[permalink] [raw]

Subject: [PATCH v2 2/4] mmc: atmel-mci: prepare clk before calling enable

Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.

Signed-off-by: Boris BREZILLON <[email protected]>
---
drivers/mmc/host/atmel-mci.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index e75774f..631c4cf 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -380,6 +380,8 @@ static int atmci_regs_show(struct seq_file *s, void *v)
{
struct atmel_mci *host = s->private;
u32 *buf;
+ int ret = 0;
+

buf = kmalloc(ATMCI_REGS_SIZE, GFP_KERNEL);
if (!buf)
@@ -391,9 +393,13 @@ static int atmci_regs_show(struct seq_file *s, void *v)
* consistent.
*/
spin_lock_bh(&host->lock);
- clk_enable(host->mck);
+ ret = clk_prepare_enable(host->mck);
+ if (ret) {
+ spin_unlock_bh(&host->lock);
+ goto out;
+ }
memcpy_fromio(buf, host->regs, ATMCI_REGS_SIZE);
- clk_disable(host->mck);
+ clk_disable_unprepare(host->mck);
spin_unlock_bh(&host->lock);

seq_printf(s, "MR:\t0x%08x%s%s ",
@@ -444,9 +450,10 @@ static int atmci_regs_show(struct seq_file *s, void *v)
val & ATMCI_CFG_LSYNC ? " LSYNC" : "");
}

+out:
kfree(buf);

- return 0;
+ return ret;
}

static int atmci_regs_open(struct inode *inode, struct file *file)
@@ -1281,7 +1288,7 @@ static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

spin_lock_bh(&host->lock);
if (!host->mode_reg) {
- clk_enable(host->mck);
+ clk_prepare_enable(host->mck);
atmci_writel(host, ATMCI_CR, ATMCI_CR_SWRST);
atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIEN);
if (host->caps.has_cfg_reg)
@@ -1361,7 +1368,7 @@ static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIDIS);
if (host->mode_reg) {
atmci_readl(host, ATMCI_MR);
- clk_disable(host->mck);
+ clk_disable_unprepare(host->mck);
}
host->mode_reg = 0;
}
@@ -2379,10 +2386,12 @@ static int __init atmci_probe(struct platform_device *pdev)
if (!host->regs)
goto err_ioremap;

- clk_enable(host->mck);
+ ret = clk_prepare_enable(host->mck);
+ if (ret)
+ goto err_request_irq;
atmci_writel(host, ATMCI_CR, ATMCI_CR_SWRST);
host->bus_hz = clk_get_rate(host->mck);
- clk_disable(host->mck);
+ clk_disable_unprepare(host->mck);

host->mapbase = regs->start;

@@ -2487,11 +2496,11 @@ static int __exit atmci_remove(struct platform_device *pdev)
atmci_cleanup_slot(host->slot[i], i);
}

- clk_enable(host->mck);
+ clk_prepare_enable(host->mck);
atmci_writel(host, ATMCI_IDR, ~0UL);
atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIDIS);
atmci_readl(host, ATMCI_SR);
- clk_disable(host->mck);
+ clk_disable_unprepare(host->mck);

if (host->dma.chan)
dma_release_channel(host->dma.chan);
--
1.7.9.5

2013-06-25 08:13:04

by Boris BREZILLON

[permalink] [raw]

Subject: [PATCH v2 3/4] usb: gadget: at91_udc: prepare clk before calling enable

2013-06-25 08:16:11

by Nicolas Ferre

[permalink] [raw]

Subject: Re: [PATCH v2 4/4] at91/avr32/atmel_lcdfb: prepare clk before calling enable

On 25/06/2013 10:14, Boris BREZILLON :
> Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
> avoid common clk framework warnings.
>
> Signed-off-by: Boris BREZILLON <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>

Jean-Christophe, can you take this one?

Thanks, best regards,

> ---
> drivers/video/atmel_lcdfb.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 540909d..8525457 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -893,14 +893,14 @@ static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
>
> static void atmel_lcdfb_start_clock(struct atmel_lcdfb_info *sinfo)
> {
> - clk_enable(sinfo->bus_clk);
> - clk_enable(sinfo->lcdc_clk);
> + clk_prepare_enable(sinfo->bus_clk);
> + clk_prepare_enable(sinfo->lcdc_clk);
> }
>
> static void atmel_lcdfb_stop_clock(struct atmel_lcdfb_info *sinfo)
> {
> - clk_disable(sinfo->bus_clk);
> - clk_disable(sinfo->lcdc_clk);
> + clk_disable_unprepare(sinfo->bus_clk);
> + clk_disable_unprepare(sinfo->lcdc_clk);
> }
>
>
>

--
Nicolas Ferre

2013-06-25 08:20:48

by Boris BREZILLON

[permalink] [raw]

Subject: [PATCH v2 4/4] at91/avr32/atmel_lcdfb: prepare clk before calling enable

2013-06-28 07:44:47

by Nicolas Ferre

[permalink] [raw]

Subject: Re: [PATCH v2 2/4] mmc: atmel-mci: prepare clk before calling enable

On 25/06/2013 10:11, Boris BREZILLON :
> Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
> avoid common clk framework warnings.
>
> Signed-off-by: Boris BREZILLON <[email protected]>

It seems that you forgot the Acked-by line from Ludovic.

One comment...

> ---
> drivers/mmc/host/atmel-mci.c | 27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index e75774f..631c4cf 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -380,6 +380,8 @@ static int atmci_regs_show(struct seq_file *s, void *v)
> {
> struct atmel_mci *host = s->private;
> u32 *buf;
> + int ret = 0;
> +
>
> buf = kmalloc(ATMCI_REGS_SIZE, GFP_KERNEL);
> if (!buf)
> @@ -391,9 +393,13 @@ static int atmci_regs_show(struct seq_file *s, void *v)
> * consistent.
> */
> spin_lock_bh(&host->lock);
> - clk_enable(host->mck);
> + ret = clk_prepare_enable(host->mck);

Do you think that it is needed here? I do not think it can be called
before the probe() function and I do not know if the result can change
depending on when clk_prepare_enable() is called...

> + if (ret) {
> + spin_unlock_bh(&host->lock);
> + goto out;
> + }
> memcpy_fromio(buf, host->regs, ATMCI_REGS_SIZE);
> - clk_disable(host->mck);
> + clk_disable_unprepare(host->mck);
> spin_unlock_bh(&host->lock);
>
> seq_printf(s, "MR:\t0x%08x%s%s ",
> @@ -444,9 +450,10 @@ static int atmci_regs_show(struct seq_file *s, void *v)
> val & ATMCI_CFG_LSYNC ? " LSYNC" : "");
> }
>
> +out:
> kfree(buf);
>
> - return 0;
> + return ret;
> }
>
> static int atmci_regs_open(struct inode *inode, struct file *file)
> @@ -1281,7 +1288,7 @@ static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>
> spin_lock_bh(&host->lock);
> if (!host->mode_reg) {
> - clk_enable(host->mck);
> + clk_prepare_enable(host->mck);
> atmci_writel(host, ATMCI_CR, ATMCI_CR_SWRST);
> atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIEN);
> if (host->caps.has_cfg_reg)
> @@ -1361,7 +1368,7 @@ static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIDIS);
> if (host->mode_reg) {
> atmci_readl(host, ATMCI_MR);
> - clk_disable(host->mck);
> + clk_disable_unprepare(host->mck);
> }
> host->mode_reg = 0;
> }
> @@ -2379,10 +2386,12 @@ static int __init atmci_probe(struct platform_device *pdev)
> if (!host->regs)
> goto err_ioremap;
>
> - clk_enable(host->mck);
> + ret = clk_prepare_enable(host->mck);
> + if (ret)
> + goto err_request_irq;
> atmci_writel(host, ATMCI_CR, ATMCI_CR_SWRST);
> host->bus_hz = clk_get_rate(host->mck);
> - clk_disable(host->mck);
> + clk_disable_unprepare(host->mck);
>
> host->mapbase = regs->start;
>
> @@ -2487,11 +2496,11 @@ static int __exit atmci_remove(struct platform_device *pdev)
> atmci_cleanup_slot(host->slot[i], i);
> }
>
> - clk_enable(host->mck);
> + clk_prepare_enable(host->mck);
> atmci_writel(host, ATMCI_IDR, ~0UL);
> atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIDIS);
> atmci_readl(host, ATMCI_SR);
> - clk_disable(host->mck);
> + clk_disable_unprepare(host->mck);
>
> if (host->dma.chan)
> dma_release_channel(host->dma.chan);
>

--
Nicolas Ferre

2013-06-28 08:06:30

by Boris BREZILLON

[permalink] [raw]

Subject: Re: [PATCH v2 2/4] mmc: atmel-mci: prepare clk before calling enable

On 28/06/2013 09:44, Nicolas Ferre wrote:
> On 25/06/2013 10:11, Boris BREZILLON :
>> Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
>> avoid common clk framework warnings.
>>
>> Signed-off-by: Boris BREZILLON <[email protected]>
>
> It seems that you forgot the Acked-by line from Ludovic.
>
> One comment...
>
>> ---
>> drivers/mmc/host/atmel-mci.c | 27 ++++++++++++++++++---------
>> 1 file changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
>> index e75774f..631c4cf 100644
>> --- a/drivers/mmc/host/atmel-mci.c
>> +++ b/drivers/mmc/host/atmel-mci.c
>> @@ -380,6 +380,8 @@ static int atmci_regs_show(struct seq_file *s,
>> void *v)
>> {
>> struct atmel_mci *host = s->private;
>> u32 *buf;
>> + int ret = 0;
>> +
>>
>> buf = kmalloc(ATMCI_REGS_SIZE, GFP_KERNEL);
>> if (!buf)
>> @@ -391,9 +393,13 @@ static int atmci_regs_show(struct seq_file *s,
>> void *v)
>> * consistent.
>> */
>> spin_lock_bh(&host->lock);
>> - clk_enable(host->mck);
>> + ret = clk_prepare_enable(host->mck);
>
> Do you think that it is needed here? I do not think it can be called
> before the probe() function and I do not know if the result can change
> depending on when clk_prepare_enable() is called...
Same as for other patches: it should not fail if this function is called
after probe,
because the prepare and enable process already succeed once.

But I think this is better to keep the check wherever we can reflect the
error to the caller:
the code will be more robust to clk framework or at91 clk implementation
changes.

I know I told you this kind of check could be removed from dma driver,
so this is a bit inconsistent.

It's up to you, tell me if you want me to remove this part.

>
>> + if (ret) {
>> + spin_unlock_bh(&host->lock);
>> + goto out;
>> + }
>> memcpy_fromio(buf, host->regs, ATMCI_REGS_SIZE);
>> - clk_disable(host->mck);
>> + clk_disable_unprepare(host->mck);
>> spin_unlock_bh(&host->lock);
>>
>> seq_printf(s, "MR:\t0x%08x%s%s ",
>> @@ -444,9 +450,10 @@ static int atmci_regs_show(struct seq_file *s,
>> void *v)
>> val & ATMCI_CFG_LSYNC ? " LSYNC" : "");
>> }
>>
>> +out:
>> kfree(buf);
>>
>> - return 0;
>> + return ret;
>> }
>>
>> static int atmci_regs_open(struct inode *inode, struct file *file)
>> @@ -1281,7 +1288,7 @@ static void atmci_set_ios(struct mmc_host *mmc,
>> struct mmc_ios *ios)
>>
>> spin_lock_bh(&host->lock);
>> if (!host->mode_reg) {
>> - clk_enable(host->mck);
>> + clk_prepare_enable(host->mck);
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_SWRST);
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIEN);
>> if (host->caps.has_cfg_reg)
>> @@ -1361,7 +1368,7 @@ static void atmci_set_ios(struct mmc_host *mmc,
>> struct mmc_ios *ios)
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIDIS);
>> if (host->mode_reg) {
>> atmci_readl(host, ATMCI_MR);
>> - clk_disable(host->mck);
>> + clk_disable_unprepare(host->mck);
>> }
>> host->mode_reg = 0;
>> }
>> @@ -2379,10 +2386,12 @@ static int __init atmci_probe(struct
>> platform_device *pdev)
>> if (!host->regs)
>> goto err_ioremap;
>>
>> - clk_enable(host->mck);
>> + ret = clk_prepare_enable(host->mck);
>> + if (ret)
>> + goto err_request_irq;
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_SWRST);
>> host->bus_hz = clk_get_rate(host->mck);
>> - clk_disable(host->mck);
>> + clk_disable_unprepare(host->mck);
>>
>> host->mapbase = regs->start;
>>
>> @@ -2487,11 +2496,11 @@ static int __exit atmci_remove(struct
>> platform_device *pdev)
>> atmci_cleanup_slot(host->slot[i], i);
>> }
>>
>> - clk_enable(host->mck);
>> + clk_prepare_enable(host->mck);
>> atmci_writel(host, ATMCI_IDR, ~0UL);
>> atmci_writel(host, ATMCI_CR, ATMCI_CR_MCIDIS);
>> atmci_readl(host, ATMCI_SR);
>> - clk_disable(host->mck);
>> + clk_disable_unprepare(host->mck);
>>
>> if (host->dma.chan)
>> dma_release_channel(host->dma.chan);
>>
>
>

2013-07-03 11:51:08

by Jean-Christophe PLAGNIOL-VILLARD

[permalink] [raw]

Subject: Re: [PATCH v2 4/4] at91/avr32/atmel_lcdfb: prepare clk before calling enable

On 10:16 Tue 25 Jun , Nicolas Ferre wrote:
> On 25/06/2013 10:14, Boris BREZILLON :
> >Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
> >avoid common clk framework warnings.
> >
> >Signed-off-by: Boris BREZILLON <[email protected]>
>
> Acked-by: Nicolas Ferre <[email protected]>
>
> Jean-Christophe, can you take this one?

Yes I'll

Best Regards,
J.
>
> Thanks, best regards,
>
> >---
> > drivers/video/atmel_lcdfb.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> >index 540909d..8525457 100644
> >--- a/drivers/video/atmel_lcdfb.c
> >+++ b/drivers/video/atmel_lcdfb.c
> >@@ -893,14 +893,14 @@ static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
> >
> > static void atmel_lcdfb_start_clock(struct atmel_lcdfb_info *sinfo)
> > {
> >- clk_enable(sinfo->bus_clk);
> >- clk_enable(sinfo->lcdc_clk);
> >+ clk_prepare_enable(sinfo->bus_clk);
> >+ clk_prepare_enable(sinfo->lcdc_clk);
> > }
> >
> > static void atmel_lcdfb_stop_clock(struct atmel_lcdfb_info *sinfo)
> > {
> >- clk_disable(sinfo->bus_clk);
> >- clk_disable(sinfo->lcdc_clk);
> >+ clk_disable_unprepare(sinfo->bus_clk);
> >+ clk_disable_unprepare(sinfo->lcdc_clk);
> > }
> >
> >
> >
>
>
> --
> Nicolas Ferre