diff mbox

[U-Boot,3/4,V2] i.MX28: Add delay after CPU bypass is cleared

Message ID 1336060041-8803-3-git-send-email-marex@denx.de
State Accepted
Commit 8f975865be2bb2d5eb2bdb9fc5ab09aae67ad8e1
Delegated to: Stefano Babic
Headers show

Commit Message

Marek Vasut May 3, 2012, 3:47 p.m. UTC
This solves issues when larger amount of DRAM is used. Behave the
same in case of CPU bypass as we do in case of EMI bypass, wait
15 ms. We need to wait until the clock domain stabilizes.

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>
---

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.

 arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c |    2 ++
 1 file changed, 2 insertions(+)

Comments

Detlev Zundel May 4, 2012, 9:20 a.m. UTC | #1
Hi Marek,

> This solves issues when larger amount of DRAM is used. Behave the
> same in case of CPU bypass as we do in case of EMI bypass, wait
> 15 ms. We need to wait until the clock domain stabilizes.

Sorry to be somewhat persistent here, but can you please include the
information what "larger amount of DRAM" is that this delay works for?
Also is this guessed, measured or calculated?

The reason why I am so persistent is that this is _available_
information now and it will be very valuable information for the next
person reading the code while pondering the question "ok, this worked in
the past, but maybe it is a problem on my brand new hardware".

Thanks
  Detlev
Marek Vasut May 4, 2012, 11:13 a.m. UTC | #2
Dear Detlev Zundel,

> Hi Marek,
> 
> > This solves issues when larger amount of DRAM is used. Behave the
> > same in case of CPU bypass as we do in case of EMI bypass, wait
> > 15 ms. We need to wait until the clock domain stabilizes.
> 
> Sorry to be somewhat persistent here, but can you please include the
> information what "larger amount of DRAM" is that this delay works for?
> Also is this guessed, measured or calculated?

Pure guesswork. I have no scientific background for this.

> The reason why I am so persistent is that this is _available_
> information now and it will be very valuable information for the next
> person reading the code while pondering the question "ok, this worked in
> the past, but maybe it is a problem on my brand new hardware".

There is zero information available, I'll need to dig deeper ... Fabio, can you 
comment on this? Is this needed or is it some other problem?

> Thanks
>   Detlev

Best regards,
Marek Vasut
Stefano Babic May 6, 2012, 4:28 p.m. UTC | #3
On 03/05/2012 17:47, Marek Vasut wrote:
> This solves issues when larger amount of DRAM is used. Behave the
> same in case of CPU bypass as we do in case of EMI bypass, wait
> 15 ms. We need to wait until the clock domain stabilizes.
> 
> 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>
> ---
> 
> 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.
> 
>  arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
> index 0d13537..a9b1bb6 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);

Is there some influence (I assume that from your commit message) between
size of the RAM and amount of time to wait ? Then yes, with boards with
even more memory (this patch touches a common MX28 file) this delay is
not enough. Should we correlate the delay maybe with PHYS_SDRAM_1_SIZE
(maybe early_delay(15000 * (PHYS_SDRAM_1_SIZE / 512MB)) ?

Best regards,
Stefano Babic
Marek Vasut May 6, 2012, 4:30 p.m. UTC | #4
Dear Stefano Babic,

> On 03/05/2012 17:47, Marek Vasut wrote:
> > This solves issues when larger amount of DRAM is used. Behave the
> > same in case of CPU bypass as we do in case of EMI bypass, wait
> > 15 ms. We need to wait until the clock domain stabilizes.
> > 
> > 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>
> > ---
> > 
> > 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.
> >  
> >  arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c |    2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
> > b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c index 0d13537..a9b1bb6
> > 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);
> 
> Is there some influence (I assume that from your commit message) between
> size of the RAM and amount of time to wait ?

Nope

> Then yes, with boards with
> even more memory (this patch touches a common MX28 file) this delay is
> not enough. Should we correlate the delay maybe with PHYS_SDRAM_1_SIZE
> (maybe early_delay(15000 * (PHYS_SDRAM_1_SIZE / 512MB)) ?

There was a new version of this patch explaining how I got to this delay (or how 
FSL hid it from me).

> 
> Best regards,
> Stefano Babic

Best regards,
Marek Vasut
diff mbox

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..a9b1bb6 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)