Patchwork [1/1] ARM: mx51/53: correct clocks that take ipg_perclk as parent

login
register
mail settings
Submitter Richard Zhao
Date Nov. 4, 2011, 9:34 a.m.
Message ID <1320399263-14885-1-git-send-email-richard.zhao@linaro.org>
Download mbox | patch
Permalink /patch/123578/
State New
Headers show

Comments

Richard Zhao - Nov. 4, 2011, 9:34 a.m.
gpt_clk, pwm1_clk and pwm2_clk parent clock are all ipg_perclk.

Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
---
 arch/arm/mach-mx5/clock-mx51-mx53.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Sascha Hauer - Nov. 7, 2011, 7:52 a.m.
On Fri, Nov 04, 2011 at 05:34:23PM +0800, Richard Zhao wrote:
> gpt_clk, pwm1_clk and pwm2_clk parent clock are all ipg_perclk.
> 
> Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> ---
>  arch/arm/mach-mx5/clock-mx51-mx53.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
> index 2aacf41..a3ca4ce 100644
> --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
> +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
> @@ -1278,12 +1278,12 @@ DEFINE_CLOCK(uart5_clk, 4, MXC_CCM_CCGR7, MXC_CCM_CCGRx_CG7_OFFSET,
>  DEFINE_CLOCK(gpt_ipg_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG10_OFFSET,
>  	NULL,  NULL, &ipg_clk, NULL);
>  DEFINE_CLOCK(gpt_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG9_OFFSET,
> -	NULL,  NULL, &ipg_clk, &gpt_ipg_clk);
> +	NULL,  NULL, &ipg_perclk, &gpt_ipg_clk);

No. The gpt has several parents and the one we are using is the ipg
clock. Try a sleep 10 with your patch applied.

Sascha

>  
>  DEFINE_CLOCK(pwm1_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG6_OFFSET,
> -	NULL, NULL, &ipg_clk, NULL);
> +	NULL, NULL, &ipg_perclk, NULL);
>  DEFINE_CLOCK(pwm2_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG8_OFFSET,
> -	NULL, NULL, &ipg_clk, NULL);
> +	NULL, NULL, &ipg_perclk, NULL);
>  
>  /* I2C */
>  DEFINE_CLOCK(i2c1_clk, 0, MXC_CCM_CCGR1, MXC_CCM_CCGRx_CG9_OFFSET,
> -- 
> 1.7.5.4
> 
> 
>
Richard Zhao - Nov. 8, 2011, 12:41 a.m.
On Mon, Nov 07, 2011 at 08:52:54AM +0100, Sascha Hauer wrote:
> On Fri, Nov 04, 2011 at 05:34:23PM +0800, Richard Zhao wrote:
> > gpt_clk, pwm1_clk and pwm2_clk parent clock are all ipg_perclk.
> > 
> > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > ---
> >  arch/arm/mach-mx5/clock-mx51-mx53.c |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
> > index 2aacf41..a3ca4ce 100644
> > --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
> > +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
> > @@ -1278,12 +1278,12 @@ DEFINE_CLOCK(uart5_clk, 4, MXC_CCM_CCGR7, MXC_CCM_CCGRx_CG7_OFFSET,
> >  DEFINE_CLOCK(gpt_ipg_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG10_OFFSET,
> >  	NULL,  NULL, &ipg_clk, NULL);
> >  DEFINE_CLOCK(gpt_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG9_OFFSET,
> > -	NULL,  NULL, &ipg_clk, &gpt_ipg_clk);
> > +	NULL,  NULL, &ipg_perclk, &gpt_ipg_clk);
> 
> No. The gpt has several parents and the one we are using is the ipg
> clock. Try a sleep 10 with your patch applied.
You are right. Sounds like gpt_clk can not be defined here. Is it better to pass
three clocks (ipg, ipg_per, ckil) to gpt driver and let gpt decide what to use?
Anyway, it's beyond this patch. I'll keep this line as it was.

Richard
> 
> Sascha
> 
> >  
> >  DEFINE_CLOCK(pwm1_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG6_OFFSET,
> > -	NULL, NULL, &ipg_clk, NULL);
> > +	NULL, NULL, &ipg_perclk, NULL);
> >  DEFINE_CLOCK(pwm2_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG8_OFFSET,
> > -	NULL, NULL, &ipg_clk, NULL);
> > +	NULL, NULL, &ipg_perclk, NULL);
> >  
> >  /* I2C */
> >  DEFINE_CLOCK(i2c1_clk, 0, MXC_CCM_CCGR1, MXC_CCM_CCGRx_CG9_OFFSET,
> > -- 
> > 1.7.5.4
> > 
> > 
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Sascha Hauer - Nov. 8, 2011, 1:08 p.m.
On Tue, Nov 08, 2011 at 08:41:31AM +0800, Richard Zhao wrote:
> On Mon, Nov 07, 2011 at 08:52:54AM +0100, Sascha Hauer wrote:
> > On Fri, Nov 04, 2011 at 05:34:23PM +0800, Richard Zhao wrote:
> > > gpt_clk, pwm1_clk and pwm2_clk parent clock are all ipg_perclk.
> > > 
> > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > > ---
> > >  arch/arm/mach-mx5/clock-mx51-mx53.c |    6 +++---
> > >  1 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
> > > index 2aacf41..a3ca4ce 100644
> > > --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
> > > +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
> > > @@ -1278,12 +1278,12 @@ DEFINE_CLOCK(uart5_clk, 4, MXC_CCM_CCGR7, MXC_CCM_CCGRx_CG7_OFFSET,
> > >  DEFINE_CLOCK(gpt_ipg_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG10_OFFSET,
> > >  	NULL,  NULL, &ipg_clk, NULL);
> > >  DEFINE_CLOCK(gpt_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG9_OFFSET,
> > > -	NULL,  NULL, &ipg_clk, &gpt_ipg_clk);
> > > +	NULL,  NULL, &ipg_perclk, &gpt_ipg_clk);
> > 
> > No. The gpt has several parents and the one we are using is the ipg
> > clock. Try a sleep 10 with your patch applied.
> You are right. Sounds like gpt_clk can not be defined here. Is it better to pass
> three clocks (ipg, ipg_per, ckil) to gpt driver and let gpt decide what to use?
> Anyway, it's beyond this patch. I'll keep this line as it was.

Yes, this would be the correct solution.

Sascha

Patch

diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 2aacf41..a3ca4ce 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1278,12 +1278,12 @@  DEFINE_CLOCK(uart5_clk, 4, MXC_CCM_CCGR7, MXC_CCM_CCGRx_CG7_OFFSET,
 DEFINE_CLOCK(gpt_ipg_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG10_OFFSET,
 	NULL,  NULL, &ipg_clk, NULL);
 DEFINE_CLOCK(gpt_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG9_OFFSET,
-	NULL,  NULL, &ipg_clk, &gpt_ipg_clk);
+	NULL,  NULL, &ipg_perclk, &gpt_ipg_clk);
 
 DEFINE_CLOCK(pwm1_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG6_OFFSET,
-	NULL, NULL, &ipg_clk, NULL);
+	NULL, NULL, &ipg_perclk, NULL);
 DEFINE_CLOCK(pwm2_clk, 0, MXC_CCM_CCGR2, MXC_CCM_CCGRx_CG8_OFFSET,
-	NULL, NULL, &ipg_clk, NULL);
+	NULL, NULL, &ipg_perclk, NULL);
 
 /* I2C */
 DEFINE_CLOCK(i2c1_clk, 0, MXC_CCM_CCGR1, MXC_CCM_CCGRx_CG9_OFFSET,