Patchwork [U-Boot] i.MX28: Add delay after CPU bypass is cleared

login
register
mail settings
Submitter Marek Vasut
Date May 4, 2012, 11:32 a.m.
Message ID <1336131170-7621-1-git-send-email-marex@denx.de>
Download mbox | patch
Permalink /patch/156874/
State Awaiting Upstream
Headers show

Comments

Marek Vasut - May 4, 2012, 11:32 a.m.
This solves issues when larger amount of DRAM is used, like 256MB.
Behave the same in case of CPU bypass as we do in case of EMI
bypass, but wait 15 ms. We need to wait until the clock domain
stabilizes.

This issue seemed to have been caused by not waiting after frobbing
with the CPU bypass, it was unrelated to memory, but had a direct
impact, causing trouble. This was yet another X-File of the
imx-bootlets, sigh. The conclusion is, trying a semi-random delay
(there is delay after the EMI bypass change), the issue is fixed.

Another possible explanation is that we do not do the "simple memory
test" FSL does in their imx-bootlets (1000 R/W cycles to/from piece of
the memory, while also outputing something on the serial port). This
might have caused the similar delay in the imx-bootlets and therefore
they didn't need to add this explicitly.

For now, this seems good fix enough, but to me, whole that memory
init code in imx-bootlets is completely flunked and it'd need deeper
investigation.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
---
 arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c |    2 ++
 1 file changed, 2 insertions(+)

V2: Change the description, this issue seemed to have been caused by not
    waiting after frobbing with the CPU bypass, it was unrelated to memory,
    but had a direct impact, causing trouble. This was yet another X-File
    of the imx-bootlets, sigh.
V3: Add more conspiracy theories into the commit message.
Stefano Babic - May 6, 2012, 4:38 p.m.
On 04/05/2012 13:32, Marek Vasut wrote:
> This solves issues when larger amount of DRAM is used, like 256MB.
> Behave the same in case of CPU bypass as we do in case of EMI
> bypass, but wait 15 ms. We need to wait until the clock domain
> stabilizes.
> 
> This issue seemed to have been caused by not waiting after frobbing
> with the CPU bypass, it was unrelated to memory, but had a direct
> impact, causing trouble. This was yet another X-File of the
> imx-bootlets, sigh. The conclusion is, trying a semi-random delay
> (there is delay after the EMI bypass change), the issue is fixed.
> 
> Another possible explanation is that we do not do the "simple memory
> test" FSL does in their imx-bootlets (1000 R/W cycles to/from piece of
> the memory, while also outputing something on the serial port). This
> might have caused the similar delay in the imx-bootlets and therefore
> they didn't need to add this explicitly.
> 
> For now, this seems good fix enough, but to me, whole that memory
> init code in imx-bootlets is completely flunked and it'd need deeper
> investigation.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> ---
>  arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> V2: Change the description, this issue seemed to have been caused by not
>     waiting after frobbing with the CPU bypass, it was unrelated to memory,
>     but had a direct impact, causing trouble. This was yet another X-File
>     of the imx-bootlets, sigh.
> V3: Add more conspiracy theories into the commit message.
> 
> diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
> index 0d13537..9fa5d29 100644
> --- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
> +++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
> @@ -149,6 +149,8 @@ void mx28_mem_setup_cpu_and_hbus(void)
>  	/* Disable CPU bypass */
>  	writel(CLKCTRL_CLKSEQ_BYPASS_CPU,
>  		&clkctrl_regs->hw_clkctrl_clkseq_clr);
> +
> +	early_delay(15000);
>  }
>  

It is fine with me

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic
Marek Vasut - May 6, 2012, 4:39 p.m.
Dear Stefano Babic,

> On 04/05/2012 13:32, Marek Vasut wrote:
> > This solves issues when larger amount of DRAM is used, like 256MB.
> > Behave the same in case of CPU bypass as we do in case of EMI
> > bypass, but wait 15 ms. We need to wait until the clock domain
> > stabilizes.
> > 
> > This issue seemed to have been caused by not waiting after frobbing
> > with the CPU bypass, it was unrelated to memory, but had a direct
> > impact, causing trouble. This was yet another X-File of the
> > imx-bootlets, sigh. The conclusion is, trying a semi-random delay
> > (there is delay after the EMI bypass change), the issue is fixed.
> > 
> > Another possible explanation is that we do not do the "simple memory
> > test" FSL does in their imx-bootlets (1000 R/W cycles to/from piece of
> > the memory, while also outputing something on the serial port). This
> > might have caused the similar delay in the imx-bootlets and therefore
> > they didn't need to add this explicitly.

Yes Stefano ... I meant this patch ... and the above explanation :-( This is all 
so messed up :-/

> > 
> > For now, this seems good fix enough, but to me, whole that memory
> > init code in imx-bootlets is completely flunked and it'd need deeper
> > investigation.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Detlev Zundel <dzu@denx.de>
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Fabio Estevam <festevam@gmail.com>
> > ---
> > 
> >  arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c |    2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > V2: Change the description, this issue seemed to have been caused by not
> > 
> >     waiting after frobbing with the CPU bypass, it was unrelated to
> >     memory, but had a direct impact, causing trouble. This was yet
> >     another X-File of the imx-bootlets, sigh.
> > 
> > V3: Add more conspiracy theories into the commit message.
> > 
> > diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
> > b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c index 0d13537..9fa5d29
> > 100644
> > --- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
> > +++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
> > @@ -149,6 +149,8 @@ void mx28_mem_setup_cpu_and_hbus(void)
> > 
> >  	/* Disable CPU bypass */
> >  	writel(CLKCTRL_CLKSEQ_BYPASS_CPU,
> >  	
> >  		&clkctrl_regs->hw_clkctrl_clkseq_clr);
> > 
> > +
> > +	early_delay(15000);
> > 
> >  }
> 
> It is fine with me
> 
> Acked-by: Stefano Babic <sbabic@denx.de>
> 
> Best regards,
> Stefano Babic

Best regards,
Marek Vasut
Detlev Zundel - May 7, 2012, 9:49 a.m.
Hi Stefano,

> On 04/05/2012 13:32, Marek Vasut wrote:
>> This solves issues when larger amount of DRAM is used, like 256MB.
>> Behave the same in case of CPU bypass as we do in case of EMI
>> bypass, but wait 15 ms. We need to wait until the clock domain
>> stabilizes.
>> 
>> This issue seemed to have been caused by not waiting after frobbing
>> with the CPU bypass, it was unrelated to memory, but had a direct
>> impact, causing trouble. This was yet another X-File of the
>> imx-bootlets, sigh. The conclusion is, trying a semi-random delay
>> (there is delay after the EMI bypass change), the issue is fixed.
>> 
>> Another possible explanation is that we do not do the "simple memory
>> test" FSL does in their imx-bootlets (1000 R/W cycles to/from piece of
>> the memory, while also outputing something on the serial port). This
>> might have caused the similar delay in the imx-bootlets and therefore
>> they didn't need to add this explicitly.
>> 
>> For now, this seems good fix enough, but to me, whole that memory
>> init code in imx-bootlets is completely flunked and it'd need deeper
>> investigation.
>> 
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Wolfgang Denk <wd@denx.de>
>> Cc: Detlev Zundel <dzu@denx.de>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Cc: Fabio Estevam <festevam@gmail.com>
>> ---
>>  arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c |    2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> V2: Change the description, this issue seemed to have been caused by not
>>     waiting after frobbing with the CPU bypass, it was unrelated to memory,
>>     but had a direct impact, causing trouble. This was yet another X-File
>>     of the imx-bootlets, sigh.
>> V3: Add more conspiracy theories into the commit message.
>> 
>> diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
>> index 0d13537..9fa5d29 100644
>> --- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
>> +++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
>> @@ -149,6 +149,8 @@ void mx28_mem_setup_cpu_and_hbus(void)
>>  	/* Disable CPU bypass */
>>  	writel(CLKCTRL_CLKSEQ_BYPASS_CPU,
>>  		&clkctrl_regs->hw_clkctrl_clkseq_clr);
>> +
>> +	early_delay(15000);
>>  }
>>  
>
> It is fine with me
>
> Acked-by: Stefano Babic <sbabic@denx.de>

I'm also content with the commit message now, so I don't want to block
this anymore.

Acked-by: Detlev Zundel <dzu@denx.de>

Cheers
  Detlev

Patch

diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
index 0d13537..9fa5d29 100644
--- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
+++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
@@ -149,6 +149,8 @@  void mx28_mem_setup_cpu_and_hbus(void)
 	/* Disable CPU bypass */
 	writel(CLKCTRL_CLKSEQ_BYPASS_CPU,
 		&clkctrl_regs->hw_clkctrl_clkseq_clr);
+
+	early_delay(15000);
 }
 
 void mx28_mem_setup_vdda(void)