Patchwork [v2,2/3] ARM: mx28evk: set a initial clock rate for saif

login
register
mail settings
Submitter Dong Aisheng
Date Sept. 7, 2011, 1:57 p.m.
Message ID <1315403858-9904-3-git-send-email-b29396@freescale.com>
Download mbox | patch
Permalink /patch/113768/
State New
Headers show

Comments

Dong Aisheng - Sept. 7, 2011, 1:57 p.m.
Signed-off-by: Dong Aisheng <b29396@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Liam Girdwood <lrg@ti.com>

---
No changes since v1.

Note this patch still needs another patch from Wolfram to work.
I send out it in this series as
[PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks

As discussed with Wolfram, that patch may still have some later work
to do. However, we just send out it for testers to work first.
---
 arch/arm/mach-mxs/clock-mx28.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
Uwe Kleine-König - Sept. 7, 2011, 6:21 p.m.
On Wed, Sep 07, 2011 at 09:57:37PM +0800, Dong Aisheng wrote:
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: Liam Girdwood <lrg@ti.com>
> 
> ---
> No changes since v1.
> 
> Note this patch still needs another patch from Wolfram to work.
> I send out it in this series as
> [PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks
If your patch 2 depends on patch 3 there is something fishy.

> 
> As discussed with Wolfram, that patch may still have some later work
> to do. However, we just send out it for testers to work first.
> ---
>  arch/arm/mach-mxs/clock-mx28.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
> index f0c7cb6..33cc2ff 100644
> --- a/arch/arm/mach-mxs/clock-mx28.c
> +++ b/arch/arm/mach-mxs/clock-mx28.c
> @@ -826,6 +826,15 @@ int __init mx28_clocks_init(void)
>  	clk_set_parent(&saif0_clk, &pll0_clk);
>  	clk_set_parent(&saif1_clk, &pll0_clk);
>  
> +	/*
> +	 * Set an initial clk rate for saif's internal logic to work properly,
> +	 * this is especially for the saif working on EXTMASTER mode that who
> +	 * uses other saif's BITCLK&LRCLK but it still needs a basic clk which
> +	 * should be bigger enough for its internal logic.
This is not a proper (German) English sentence. Maybe this is better?:

	Set an initial clock rate for the saif internal logic to work
	properly. This is important when working in EXTMASTER mode that
	uses the other saif's BITCLK&LRCLK but it still needs a basic
	clock which should be fast enough for the internal logic.

> +	 */
> +	clk_set_rate(&saif0_clk, 24000000);
> +	clk_set_rate(&saif1_clk, 24000000);
> +
Do you need to check for clk_set_rate returning -ESOMETHING here?

Uwe
>  	clkdev_add_table(lookups, ARRAY_SIZE(lookups));
>  
>  	mxs_timer_init(&clk32k_clk, MX28_INT_TIMER0);
Dong Aisheng - Sept. 8, 2011, 8:03 a.m.
> -----Original Message-----
> From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> Sent: Thursday, September 08, 2011 2:22 AM
> To: Dong Aisheng-B29396
> Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de;
> broonie@opensource.wolfsonmicro.com; lrg@ti.com; linux-arm-
> kernel@lists.infradead.org; w.sang@pengutronix.de
> Subject: Re: [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for
> saif
> 
> On Wed, Sep 07, 2011 at 09:57:37PM +0800, Dong Aisheng wrote:
> > Signed-off-by: Dong Aisheng <b29396@freescale.com>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > Cc: Liam Girdwood <lrg@ti.com>
> >
> > ---
> > No changes since v1.
> >
> > Note this patch still needs another patch from Wolfram to work.
> > I send out it in this series as
> > [PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks
> If your patch 2 depends on patch 3 there is something fishy.
Yes, because the original saif clk setting code has some issues
Which can be fixed in patch 3.
 
> >
> > As discussed with Wolfram, that patch may still have some later work
> > to do. However, we just send out it for testers to work first.
> > ---
> >  arch/arm/mach-mxs/clock-mx28.c |    9 +++++++++
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-mxs/clock-mx28.c
> > b/arch/arm/mach-mxs/clock-mx28.c index f0c7cb6..33cc2ff 100644
> > --- a/arch/arm/mach-mxs/clock-mx28.c
> > +++ b/arch/arm/mach-mxs/clock-mx28.c
> > @@ -826,6 +826,15 @@ int __init mx28_clocks_init(void)
> >  	clk_set_parent(&saif0_clk, &pll0_clk);
> >  	clk_set_parent(&saif1_clk, &pll0_clk);
> >
> > +	/*
> > +	 * Set an initial clk rate for saif's internal logic to work
> properly,
> > +	 * this is especially for the saif working on EXTMASTER mode that
> who
> > +	 * uses other saif's BITCLK&LRCLK but it still needs a basic clk
> which
> > +	 * should be bigger enough for its internal logic.
> This is not a proper (German) English sentence. Maybe this is better?:
> 
> 	Set an initial clock rate for the saif internal logic to work
> 	properly. This is important when working in EXTMASTER mode that
> 	uses the other saif's BITCLK&LRCLK but it still needs a basic
> 	clock which should be fast enough for the internal logic.
Yes, I think it's better. :)
I will change like that.

> > +	 */
> > +	clk_set_rate(&saif0_clk, 24000000);
> > +	clk_set_rate(&saif1_clk, 24000000);
> > +
> Do you need to check for clk_set_rate returning -ESOMETHING here?
Basically this rate setting will not fail in clock_init.
I noticed arch/arm/mach-mx5/clock-mx51-mx53.c also used like this.
Should I add an error checking here?

Regards
Dong Aisheng
Uwe Kleine-König - Sept. 8, 2011, 8:56 a.m.
Hello (must I say Dong or Aisheng here?)

On Thu, Sep 08, 2011 at 08:03:17AM +0000, Dong Aisheng-B29396 wrote:
> > -----Original Message-----
> > From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> > Sent: Thursday, September 08, 2011 2:22 AM
> > To: Dong Aisheng-B29396
> > Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de;
> > broonie@opensource.wolfsonmicro.com; lrg@ti.com; linux-arm-
> > kernel@lists.infradead.org; w.sang@pengutronix.de
> > Subject: Re: [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for
> > saif
> > 
> > On Wed, Sep 07, 2011 at 09:57:37PM +0800, Dong Aisheng wrote:
> > > Signed-off-by: Dong Aisheng <b29396@freescale.com>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > > Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > > Cc: Liam Girdwood <lrg@ti.com>
> > >
> > > ---
> > > No changes since v1.
> > >
> > > Note this patch still needs another patch from Wolfram to work.
> > > I send out it in this series as
> > > [PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks
> > If your patch 2 depends on patch 3 there is something fishy.
> Yes, because the original saif clk setting code has some issues
> Which can be fixed in patch 3.
So patch 3 should come before patch 2 in the series.

> > > As discussed with Wolfram, that patch may still have some later work
> > > to do. However, we just send out it for testers to work first.
> > > ---
> > >  arch/arm/mach-mxs/clock-mx28.c |    9 +++++++++
> > >  1 files changed, 9 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-mxs/clock-mx28.c
> > > b/arch/arm/mach-mxs/clock-mx28.c index f0c7cb6..33cc2ff 100644
> > > --- a/arch/arm/mach-mxs/clock-mx28.c
> > > +++ b/arch/arm/mach-mxs/clock-mx28.c
> > > @@ -826,6 +826,15 @@ int __init mx28_clocks_init(void)
> > >  	clk_set_parent(&saif0_clk, &pll0_clk);
> > >  	clk_set_parent(&saif1_clk, &pll0_clk);
> > >
> > > +	/*
> > > +	 * Set an initial clk rate for saif's internal logic to work
> > properly,
> > > +	 * this is especially for the saif working on EXTMASTER mode that
> > who
> > > +	 * uses other saif's BITCLK&LRCLK but it still needs a basic clk
> > which
> > > +	 * should be bigger enough for its internal logic.
> > This is not a proper (German) English sentence. Maybe this is better?:
> > 
> > 	Set an initial clock rate for the saif internal logic to work
> > 	properly. This is important when working in EXTMASTER mode that
> > 	uses the other saif's BITCLK&LRCLK but it still needs a basic
> > 	clock which should be fast enough for the internal logic.
> Yes, I think it's better. :)
> I will change like that.
> 
> > > +	 */
> > > +	clk_set_rate(&saif0_clk, 24000000);
> > > +	clk_set_rate(&saif1_clk, 24000000);
> > > +
> > Do you need to check for clk_set_rate returning -ESOMETHING here?
> Basically this rate setting will not fail in clock_init.
> I noticed arch/arm/mach-mx5/clock-mx51-mx53.c also used like this.
> Should I add an error checking here?
Probably it's OK as is then.

Uwe
Dong Aisheng - Sept. 8, 2011, 9:06 a.m.
> -----Original Message-----
> From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> Sent: Thursday, September 08, 2011 4:56 PM
> To: Dong Aisheng-B29396
> Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de;
> broonie@opensource.wolfsonmicro.com; lrg@ti.com; linux-arm-
> kernel@lists.infradead.org; w.sang@pengutronix.de
> Subject: Re: [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for
> saif
> 
> Hello (must I say Dong or Aisheng here?)
Both are ok.
If conveniently, I prefer Aisheng. :)

> On Thu, Sep 08, 2011 at 08:03:17AM +0000, Dong Aisheng-B29396 wrote:
> > > -----Original Message-----
> > > From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> > > Sent: Thursday, September 08, 2011 2:22 AM
> > > To: Dong Aisheng-B29396
> > > Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de;
> > > broonie@opensource.wolfsonmicro.com; lrg@ti.com; linux-arm-
> > > kernel@lists.infradead.org; w.sang@pengutronix.de
> > > Subject: Re: [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate
> > > for saif
> > >
> > > On Wed, Sep 07, 2011 at 09:57:37PM +0800, Dong Aisheng wrote:
> > > > Signed-off-by: Dong Aisheng <b29396@freescale.com>
> > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > > > Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > > > Cc: Liam Girdwood <lrg@ti.com>
> > > >
> > > > ---
> > > > No changes since v1.
> > > >
> > > > Note this patch still needs another patch from Wolfram to work.
> > > > I send out it in this series as
> > > > [PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks
> > > If your patch 2 depends on patch 3 there is something fishy.
> > Yes, because the original saif clk setting code has some issues Which
> > can be fixed in patch 3.
> So patch 3 should come before patch 2 in the series.
Question is that do you think the patch could be accepted?
If yes, I will change it as patch 2.
Or it may still be patch3 just for testing and wait for the further fixing.

> > > > As discussed with Wolfram, that patch may still have some later
> > > > work to do. However, we just send out it for testers to work first.
> > > > ---
> > > >  arch/arm/mach-mxs/clock-mx28.c |    9 +++++++++
> > > >  1 files changed, 9 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/arch/arm/mach-mxs/clock-mx28.c
> > > > b/arch/arm/mach-mxs/clock-mx28.c index f0c7cb6..33cc2ff 100644
> > > > --- a/arch/arm/mach-mxs/clock-mx28.c
> > > > +++ b/arch/arm/mach-mxs/clock-mx28.c
> > > > @@ -826,6 +826,15 @@ int __init mx28_clocks_init(void)
> > > >  	clk_set_parent(&saif0_clk, &pll0_clk);
> > > >  	clk_set_parent(&saif1_clk, &pll0_clk);
> > > >
> > > > +	/*
> > > > +	 * Set an initial clk rate for saif's internal logic to work
> > > properly,
> > > > +	 * this is especially for the saif working on EXTMASTER mode
> > > > +that
> > > who
> > > > +	 * uses other saif's BITCLK&LRCLK but it still needs a basic
> clk
> > > which
> > > > +	 * should be bigger enough for its internal logic.
> > > This is not a proper (German) English sentence. Maybe this is better?:
> > >
> > > 	Set an initial clock rate for the saif internal logic to work
> > > 	properly. This is important when working in EXTMASTER mode that
> > > 	uses the other saif's BITCLK&LRCLK but it still needs a basic
> > > 	clock which should be fast enough for the internal logic.
> > Yes, I think it's better. :)
> > I will change like that.
> >
> > > > +	 */
> > > > +	clk_set_rate(&saif0_clk, 24000000);
> > > > +	clk_set_rate(&saif1_clk, 24000000);
> > > > +
> > > Do you need to check for clk_set_rate returning -ESOMETHING here?
> > Basically this rate setting will not fail in clock_init.
> > I noticed arch/arm/mach-mx5/clock-mx51-mx53.c also used like this.
> > Should I add an error checking here?
> Probably it's OK as is then.
Ok, thanks.

Regards
Dong Aisheng

Patch

diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
index f0c7cb6..33cc2ff 100644
--- a/arch/arm/mach-mxs/clock-mx28.c
+++ b/arch/arm/mach-mxs/clock-mx28.c
@@ -826,6 +826,15 @@  int __init mx28_clocks_init(void)
 	clk_set_parent(&saif0_clk, &pll0_clk);
 	clk_set_parent(&saif1_clk, &pll0_clk);
 
+	/*
+	 * Set an initial clk rate for saif's internal logic to work properly,
+	 * this is especially for the saif working on EXTMASTER mode that who
+	 * uses other saif's BITCLK&LRCLK but it still needs a basic clk which
+	 * should be bigger enough for its internal logic.
+	 */
+	clk_set_rate(&saif0_clk, 24000000);
+	clk_set_rate(&saif1_clk, 24000000);
+
 	clkdev_add_table(lookups, ARRAY_SIZE(lookups));
 
 	mxs_timer_init(&clk32k_clk, MX28_INT_TIMER0);