Patchwork ARM: mx28: Clear CLKGATE bit prior to changing DIV field

login
register
mail settings
Submitter Fabio Estevam
Date Jan. 19, 2012, 3:57 a.m.
Message ID <CAOMZO5CeW08kb4q3UVbpQ1tfJPotoVWV9dD7ycokpEbp2a9mow@mail.gmail.com>
Download mbox | patch
Permalink /patch/136756/
State New
Headers show

Comments

Fabio Estevam - Jan. 19, 2012, 3:57 a.m.
2012/1/19 Shawn Guo <shawn.guo@linaro.org>:

> My proposal is we need to clk_prepare_enable before clk_set_rate a clock
> if the clock is gated, and then clk_disable_unprepare the clock after
> clk_set_rate is done.  This applies whatever codes that want to
> clk_set_rate a mxs clock.

So something like:

didn't work on my tests (probe of mxs-saif fails with the same timeout message).

However if I use the clk_disable versions:

+       clk_disable(&saif0_clk);
+       clk_disable(&saif1_clk);

Then it works fine.

Regards,

Fabio Estevam
Shawn Guo - Jan. 19, 2012, 4:20 a.m.
On Thu, Jan 19, 2012 at 01:57:45AM -0200, Fabio Estevam wrote:
> 2012/1/19 Shawn Guo <shawn.guo@linaro.org>:
> 
> > My proposal is we need to clk_prepare_enable before clk_set_rate a clock
> > if the clock is gated, and then clk_disable_unprepare the clock after
> > clk_set_rate is done.  This applies whatever codes that want to
> > clk_set_rate a mxs clock.
> 
> So something like:
> 
> --- a/arch/arm/mach-mxs/clock-mx28.c
> +++ b/arch/arm/mach-mxs/clock-mx28.c
> @@ -809,6 +809,8 @@ int __init mx28_clocks_init(void)
>         clk_prepare_enable(&xbus_clk);
>         clk_prepare_enable(&emi_clk);
>         clk_prepare_enable(&uart_clk);
> +       clk_prepare_enable(&saif0_clk);
> +       clk_prepare_enable(&saif1_clk);
> 
>         clk_set_parent(&lcdif_clk, &ref_pix_clk);
>         clk_set_parent(&saif0_clk, &pll0_clk);
> @@ -822,6 +824,8 @@ int __init mx28_clocks_init(void)
>          */
>         clk_set_rate(&saif0_clk, 24000000);
>         clk_set_rate(&saif1_clk, 24000000);
> +       clk_disable_unprepare(&saif0_clk);
> +       clk_disable_unprepare(&saif1_clk);
> 
>         clkdev_add_table(lookups, ARRAY_SIZE(lookups));
> ---
> 
> didn't work on my tests (probe of mxs-saif fails with the same timeout message).
> 
As far as I know, mxs-saif driver also has one clk_set_rate() being
called with the clock gated.
Dong Aisheng - Jan. 19, 2012, 4:55 a.m.
> -----Original Message-----

> From: Fabio Estevam [mailto:festevam@gmail.com]

> Sent: Thursday, January 19, 2012 11:58 AM

> To: Shawn Guo

> Cc: Lothar Waßmann; Estevam Fabio-R49496; w.sang@pengutronix.de;

> marek.vasut@gmail.com; linux-arm-kernel@lists.infradead.org;

> kernel@pengutronix.de; Guo Shawn-R65073; Dong Aisheng-B29396

> Subject: Re: [PATCH] ARM: mx28: Clear CLKGATE bit prior to changing DIV field

> Importance: High

> 

> 2012/1/19 Shawn Guo <shawn.guo@linaro.org>:

> 

> > My proposal is we need to clk_prepare_enable before clk_set_rate a

> > clock if the clock is gated, and then clk_disable_unprepare the clock

> > after clk_set_rate is done.  This applies whatever codes that want to

> > clk_set_rate a mxs clock.

> 

> So something like:

> 

> --- a/arch/arm/mach-mxs/clock-mx28.c

> +++ b/arch/arm/mach-mxs/clock-mx28.c

> @@ -809,6 +809,8 @@ int __init mx28_clocks_init(void)

>         clk_prepare_enable(&xbus_clk);

>         clk_prepare_enable(&emi_clk);

>         clk_prepare_enable(&uart_clk);

> +       clk_prepare_enable(&saif0_clk);

> +       clk_prepare_enable(&saif1_clk);

> 

>         clk_set_parent(&lcdif_clk, &ref_pix_clk);

>         clk_set_parent(&saif0_clk, &pll0_clk); @@ -822,6 +824,8 @@ int __init

> mx28_clocks_init(void)

>          */

>         clk_set_rate(&saif0_clk, 24000000);

>         clk_set_rate(&saif1_clk, 24000000);

> +       clk_disable_unprepare(&saif0_clk);

> +       clk_disable_unprepare(&saif1_clk);

> 

>         clkdev_add_table(lookups, ARRAY_SIZE(lookups));

> ---

> 

> didn't work on my tests (probe of mxs-saif fails with the same timeout message).

> 

> However if I use the clk_disable versions:

> 

> +       clk_disable(&saif0_clk);

> +       clk_disable(&saif1_clk);

> 

> Then it works fine.

> 


It looks strange to me.
Is there any big difference between clk_disable and clk_disable_unprepare for MXS?
Why clk_disable works?

Regards
Dong Aisheng
Dong Aisheng - Jan. 19, 2012, 5:14 a.m.
> -----Original Message-----

> From: Fabio Estevam [mailto:festevam@gmail.com]

> Sent: Thursday, January 19, 2012 12:45 PM

> To: Shawn Guo

> Cc: Lothar Waßmann; Estevam Fabio-R49496; w.sang@pengutronix.de;

> marek.vasut@gmail.com; linux-arm-kernel@lists.infradead.org;

> kernel@pengutronix.de; Guo Shawn-R65073; Dong Aisheng-B29396

> Subject: Re: [PATCH] ARM: mx28: Clear CLKGATE bit prior to changing DIV field

> Importance: High

> 

> 2012/1/19 Shawn Guo <shawn.guo@linaro.org>:

> 

> > As far as I know, mxs-saif driver also has one clk_set_rate() being

> > called with the clock gated.

> 

> Yes, correct. Is this a good fix?

> 

> diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c index

> dccfb37..c5a29a8 100644

> --- a/sound/soc/mxs/mxs-saif.c

> +++ b/sound/soc/mxs/mxs-saif.c

> @@ -124,6 +124,8 @@ static int mxs_saif_set_clk(struct mxs_saif *saif,

>          *

>          * If MCLK is not used, we just set saif clk to 512*fs.

>          */

> +       clk_prepare_enable(master_saif->clk);

> +

>         if (master_saif->mclk_in_use) {

>                 if (mclk % 32 == 0) {

>                         scr &= ~BM_SAIF_CTRL_BITCLK_BASE_RATE; @@ -133,6 +135,7

> @@ static int mxs_saif_set_clk(struct mxs_saif *saif,

>                         ret = clk_set_rate(master_saif->clk, 384 * rate);

>                 } else {

>                         /* SAIF MCLK should be either 32x or 48x */

> +                       clk_disable_unprepare(master_saif->clk);

>                         return -EINVAL;

>                 }

>         } else {

> @@ -140,6 +143,8 @@ static int mxs_saif_set_clk(struct mxs_saif *saif,

>                 scr &= ~BM_SAIF_CTRL_BITCLK_BASE_RATE;

>         }

> 

> +       clk_disable_unprepare(master_saif->clk);

> +

>         if (ret)

>                 return ret;

> 

The change looks ok to me.
If the test is ok, I will ack this patch.

Regards
Dong Aisheng
Fabio Estevam - Jan. 19, 2012, 2:04 p.m.
On 1/19/12, Dong Aisheng-B29396 <B29396@freescale.com> wrote:

> The change looks ok to me.
> If the test is ok, I will ack this patch.

Yes, audio works with this change. I submitted the patch to the alsa-devel list.

Regards,

Fabio Estevam

Patch

--- a/arch/arm/mach-mxs/clock-mx28.c
+++ b/arch/arm/mach-mxs/clock-mx28.c
@@ -809,6 +809,8 @@  int __init mx28_clocks_init(void)
        clk_prepare_enable(&xbus_clk);
        clk_prepare_enable(&emi_clk);
        clk_prepare_enable(&uart_clk);
+       clk_prepare_enable(&saif0_clk);
+       clk_prepare_enable(&saif1_clk);

        clk_set_parent(&lcdif_clk, &ref_pix_clk);
        clk_set_parent(&saif0_clk, &pll0_clk);
@@ -822,6 +824,8 @@  int __init mx28_clocks_init(void)
         */
        clk_set_rate(&saif0_clk, 24000000);
        clk_set_rate(&saif1_clk, 24000000);
+       clk_disable_unprepare(&saif0_clk);
+       clk_disable_unprepare(&saif1_clk);

        clkdev_add_table(lookups, ARRAY_SIZE(lookups));
---