| Message ID | CAOMZO5CeW08kb4q3UVbpQ1tfJPotoVWV9dD7ycokpEbp2a9mow@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
> -----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
> -----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
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
--- 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)); ---