Patchwork arm/mach-mx6: fix pll4 set_rate callback

login
register
mail settings
Submitter Alexander Smirnov
Date May 29, 2013, 12:23 p.m.
Message ID <1369830235-8476-2-git-send-email-alex.bluesman.smirnov@gmail.com>
Download mbox | patch
Permalink /patch/50693/
State Accepted
Delegated to: Otavio Salvador
Headers show

Comments

Alexander Smirnov - May 29, 2013, 12:23 p.m.
There is single method to set clock-rate for both audio and video pll-s
in i.MX6q clock system implementation. That's possible due to they have
similar set of registers with a different bases. But there is also one
common register: CCM_ANALOG_MISC2, which contains post-dividers.

In current implementation, independently of whether audio or video clock
is going to be set, the mask 0xc0000000 is applied to MISC2 register.
This means, that if the audio clock rate is changed, the video clock
post-dividers possibly will be corrupted.

This patch fixes the issue described above.

Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
 arch/arm/mach-mx6/clock.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Otavio Salvador - May 29, 2013, 2:41 p.m.
Hello Alexander,

On Wed, May 29, 2013 at 9:23 AM, Alexander Smirnov <
alex.bluesman.smirnov@gmail.com> wrote:

> There is single method to set clock-rate for both audio and video pll-s
> in i.MX6q clock system implementation. That's possible due to they have
> similar set of registers with a different bases. But there is also one
> common register: CCM_ANALOG_MISC2, which contains post-dividers.
>
> In current implementation, independently of whether audio or video clock
> is going to be set, the mask 0xc0000000 is applied to MISC2 register.
> This means, that if the audio clock rate is changed, the video clock
> post-dividers possibly will be corrupted.
>
> This patch fixes the issue described above.
>
> Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
>

I am adding Fabio and Mahesh in Cc so they can take a look in this patch.

Regards,
Alexander Smirnov - June 4, 2013, 12:43 p.m.
Hi Folks,



 >     There is single method to set clock-rate for both audio and video 
pll-s
>     in i.MX6q clock system implementation. That's possible due to they have
>     similar set of registers with a different bases. But there is also one
>     common register: CCM_ANALOG_MISC2, which contains post-dividers.
>
>     In current implementation, independently of whether audio or video clock
>     is going to be set, the mask 0xc0000000 is applied to MISC2 register.
>     This means, that if the audio clock rate is changed, the video clock
>     post-dividers possibly will be corrupted.
>
>     This patch fixes the issue described above.
>
>     Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com
>     <mailto:alex.bluesman.smirnov@gmail.com>>
>
>
> I am adding Fabio and Mahesh in Cc so they can take a look in this patch

has anybody looked at the patch I sent?

I also have a couple of fixes for imx audio, can I send them here?

With best regards,
Alex

>
> Regards,
>
> --
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br http://projetos.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
Eric BENARD - June 4, 2013, 3:54 p.m.
Hi Alexander,

Le Tue, 04 Jun 2013 16:43:53 +0400,
Alexander Smirnov <alex.bluesman.smirnov@gmail.com> a écrit :
>  >     There is single method to set clock-rate for both audio and video 
> pll-s
> >     in i.MX6q clock system implementation. That's possible due to they have
> >     similar set of registers with a different bases. But there is also one
> >     common register: CCM_ANALOG_MISC2, which contains post-dividers.
> >
> >     In current implementation, independently of whether audio or video clock
> >     is going to be set, the mask 0xc0000000 is applied to MISC2 register.
> >     This means, that if the audio clock rate is changed, the video clock
> >     post-dividers possibly will be corrupted.
> >
> >     This patch fixes the issue described above.
> >
> >     Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com
> >     <mailto:alex.bluesman.smirnov@gmail.com>>
> >
> >
> > I am adding Fabio and Mahesh in Cc so they can take a look in this patch
> 
> has anybody looked at the patch I sent?
> 
> I also have a couple of fixes for imx audio, can I send them here?
> 
please send them here even if that doesn't reach meta-fsl-arm, at least
other users of i.MX are reading this mailing list and may find your
patches interesting and test them if they face the same issues.

Thanks and best regards,
Eric
Mahadevan Mahesh-R9AADQ - June 4, 2013, 4:34 p.m.
Hello Alex,
Thanks for the patch. We have looked at it and it looks accurate. It has been sent to the clock driver developer for his comments and to integrate into the code-base.

Best regards,
-Mahesh

-----Original Message-----
From: Alexander Smirnov [mailto:alex.bluesman.smirnov@gmail.com] 
Sent: Tuesday, June 04, 2013 7:44 AM
To: Otavio Salvador
Cc: Estevam Fabio-R49496; Mahadevan Mahesh-R9AADQ; meta-freescale@yoctoproject.org
Subject: Re: [meta-freescale] [PATCH] arm/mach-mx6: fix pll4 set_rate callback

Hi Folks,



 >     There is single method to set clock-rate for both audio and video 
pll-s
>     in i.MX6q clock system implementation. That's possible due to they have
>     similar set of registers with a different bases. But there is also one
>     common register: CCM_ANALOG_MISC2, which contains post-dividers.
>
>     In current implementation, independently of whether audio or video clock
>     is going to be set, the mask 0xc0000000 is applied to MISC2 register.
>     This means, that if the audio clock rate is changed, the video clock
>     post-dividers possibly will be corrupted.
>
>     This patch fixes the issue described above.
>
>     Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com
>     <mailto:alex.bluesman.smirnov@gmail.com>>
>
>
> I am adding Fabio and Mahesh in Cc so they can take a look in this patch

has anybody looked at the patch I sent?

I also have a couple of fixes for imx audio, can I send them here?

With best regards,
Alex

>
> Regards,
>
> --
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br http://projetos.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
Daiane Angolini - June 4, 2013, 4:42 p.m.
On 06/04/2013 09:43 AM, Alexander Smirnov wrote:
> Hi Folks,
>
>
>
>  >     There is single method to set clock-rate for both audio and video
> pll-s
>>     in i.MX6q clock system implementation. That's possible due to they
>> have
>>     similar set of registers with a different bases. But there is also
>> one
>>     common register: CCM_ANALOG_MISC2, which contains post-dividers.
>>
>>     In current implementation, independently of whether audio or video
>> clock
>>     is going to be set, the mask 0xc0000000 is applied to MISC2 register.
>>     This means, that if the audio clock rate is changed, the video clock
>>     post-dividers possibly will be corrupted.
>>
>>     This patch fixes the issue described above.
>>
>>     Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com
>>     <mailto:alex.bluesman.smirnov@gmail.com>>
>>
>>
>> I am adding Fabio and Mahesh in Cc so they can take a look in this patch
>
> has anybody looked at the patch I sent?

Yes, it looks right for everyone and we are working to get it included 
in the linux tree.

But, even if was not included in the linux tree (we should know it in 1 
or 2 days), I do think we should include it on linux-imx recipe.


>
> I also have a couple of fixes for imx audio, can I send them here?

Please, let the patches come!


>
> With best regards,
> Alex


Thanks a lot, Alex, for sharing this ;)

Daiane
Otavio Salvador - June 4, 2013, 4:51 p.m.
Hello Mahesh,

On Tue, Jun 4, 2013 at 1:34 PM, Mahadevan Mahesh-R9AADQ <
r9aadq@freescale.com> wrote:
>
> Thanks for the patch. We have looked at it and it looks accurate. It has
> been sent to the clock driver developer for his comments and to integrate
> into the code-base.
>

When this get reviewed (and integrated) could you try to cherry-pick it to
4.0.0 branch?

Regards,

Patch

diff --git a/arch/arm/mach-mx6/clock.c b/arch/arm/mach-mx6/clock.c
index 8c590b7..8706c32 100644
--- a/arch/arm/mach-mx6/clock.c
+++ b/arch/arm/mach-mx6/clock.c
@@ -1023,7 +1023,8 @@  static int _clk_audio_video_set_rate(struct clk *clk, unsigned long rate)
 	__raw_writel(mfn, pllbase + PLL_NUM_DIV_OFFSET);
 	__raw_writel(mfd, pllbase + PLL_DENOM_DIV_OFFSET);
 
-	if (rev >= IMX_CHIP_REVISION_1_1) {
+	if ((rev >= IMX_CHIP_REVISION_1_1) &&
+	    (pllbase == PLL5_VIDEO_BASE_ADDR)) {
 		reg = __raw_readl(ANA_MISC2_BASE_ADDR)
 			& ~ANADIG_ANA_MISC2_CONTROL3_MASK;
 		reg |= control3 << ANADIG_ANA_MISC2_CONTROL3_OFFSET;