[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Peter Korsgaard]

Commit c7190f02 (retain POR values of non-configured ACR, SPCR, SCCR,
and LCRR bitfields) moved the LCRR assignment to after relocation
to RAM because of the potential problem with changing the local bus
clock while executing from flash.

For some boards this isn't needed (E.G. when running from a normal async
NOR flash), and running all code up to cpu_init_r at the slow bootup
speed adversively affects the startup time.

E.G. on a 8347 board a bootup time increase of ~600ms has been observed:

   0.020 CPU:   e300c1, MPC8347_PBGA_EA, Rev: 3.0 at 400 MHz, CSB: 266.667 MHz
   0.168 RS:    232
   0.172 I2C:   ready
   0.176 DRAM:  64 MB
   1.236 FLASH: 32 MB

Versus:

   0.016 CPU:   e300c1, MPC8347_PBGA_EA, Rev: 3.0 at 400 MHz, CSB: 266.667 MHz
   0.092 RS:    232
   0.092 I2C:   ready
   0.096 DRAM:  64 MB
   0.644 FLASH: 32 MB

Fix it by introducing CONFIG_SYS_LCRR_EARLY, and set LCRR in cpu_init_f
instead of in cpu_init_r if set.

Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 cpu/mpc83xx/cpu_init.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/cpu/mpc83xx/cpu_init.c b/cpu/mpc83xx/cpu_init.c
index 031e8d5..ef4d694 100644
--- a/cpu/mpc83xx/cpu_init.c
+++ b/cpu/mpc83xx/cpu_init.c
@@ -171,6 +171,30 @@ void cpu_init_f (volatile immap_t * im)
 		(CONFIG_SYS_SCCR_SATACM << SCCR_SATACM_SHIFT) |
 #endif
 		0;
+#ifdef CONFIG_SYS_LCRR_EARLY
+	__be32 lcrr_mask =
+#ifdef CONFIG_SYS_LCRR_DBYP /* PLL bypass */
+		LCRR_DBYP |
+#endif
+#ifdef CONFIG_SYS_LCRR_EADC /* external address delay */
+		LCRR_EADC |
+#endif
+#ifdef CONFIG_SYS_LCRR_CLKDIV /* system clock divider */
+		LCRR_CLKDIV |
+#endif
+		0;
+	__be32 lcrr_val =
+#ifdef CONFIG_SYS_LCRR_DBYP /* PLL bypass */
+		CONFIG_SYS_LCRR_DBYP |
+#endif
+#ifdef CONFIG_SYS_LCRR_EADC
+		CONFIG_SYS_LCRR_EADC |
+#endif
+#ifdef CONFIG_SYS_LCRR_CLKDIV /* system clock divider */
+		CONFIG_SYS_LCRR_CLKDIV |
+#endif
+		0;
+#endif /* CONFIG_SYS_LCRR_EARLY */
 
 	/* Pointer is writable since we allocated a register for it */
 	gd = (gd_t *) (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET);
@@ -199,6 +223,15 @@ void cpu_init_f (volatile immap_t * im)
 	 */
 	__raw_writel(RMR_CSRE & (1<<RMR_CSRE_SHIFT), &im->reset.rmr);
 
+#ifdef CONFIG_SYS_LCRR_EARLY
+	/* LCRR - Clock Ratio Register (10.3.1.16)
+	 * write, read, and isync per MPC8379ERM rev.1 CLKDEV field description
+	 */
+	clrsetbits_be32(&im->lbus.lcrr, lcrr_mask, lcrr_val);
+	__raw_readl(&im->lbus.lcrr);
+	isync();
+#endif
+
 	/* Enable Time Base & Decrementer ( so we will have udelay() )*/
 	setbits_be32(&im->sysconf.spcr, SPCR_TBEN);
 
@@ -335,6 +368,7 @@ int cpu_init_r (void)
 #ifdef CONFIG_QE
 	uint qe_base = CONFIG_SYS_IMMR + 0x00100000; /* QE immr base */
 #endif
+#ifndef CONFIG_SYS_LCRR_EARLY
 	__be32 lcrr_mask =
 #ifdef CONFIG_SYS_LCRR_DBYP /* PLL bypass */
 		LCRR_DBYP |
@@ -364,6 +398,7 @@ int cpu_init_r (void)
 	clrsetbits_be32(&im->lbus.lcrr, lcrr_mask, lcrr_val);
 	__raw_readl(&im->lbus.lcrr);
 	isync();
+#endif /* !CONFIG_SYS_LCRR_EARLY */
 
 #ifdef CONFIG_QE
 	qe_init(qe_base);
-- 
1.6.3.3

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Peter Korsgaard]

>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> writes:

 Peter> Commit c7190f02 (retain POR values of non-configured ACR, SPCR, SCCR,
 Peter> and LCRR bitfields) moved the LCRR assignment to after relocation
 Peter> to RAM because of the potential problem with changing the local bus
 Peter> clock while executing from flash.

Comments?

 Peter> For some boards this isn't needed (E.G. when running from a normal async
 Peter> NOR flash), and running all code up to cpu_init_r at the slow bootup
 Peter> speed adversively affects the startup time.

 Peter> E.G. on a 8347 board a bootup time increase of ~600ms has been observed:

 Peter>    0.020 CPU:   e300c1, MPC8347_PBGA_EA, Rev: 3.0 at 400 MHz, CSB: 266.667 MHz
 Peter>    0.168 RS:    232
 Peter>    0.172 I2C:   ready
 Peter>    0.176 DRAM:  64 MB
 Peter>    1.236 FLASH: 32 MB

 Peter> Versus:

 Peter>    0.016 CPU:   e300c1, MPC8347_PBGA_EA, Rev: 3.0 at 400 MHz, CSB: 266.667 MHz
 Peter>    0.092 RS:    232
 Peter>    0.092 I2C:   ready
 Peter>    0.096 DRAM:  64 MB
 Peter>    0.644 FLASH: 32 MB

 Peter> Fix it by introducing CONFIG_SYS_LCRR_EARLY, and set LCRR in cpu_init_f
 Peter> instead of in cpu_init_r if set.

 Peter> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
 Peter> ---
 Peter>  cpu/mpc83xx/cpu_init.c |   35 +++++++++++++++++++++++++++++++++++
 Peter>  1 files changed, 35 insertions(+), 0 deletions(-)

-- 
Bye, Peter Korsgaard

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Wolfgang Denk]

Dear Peter Korsgaard,

In message <87pr7472vw.fsf@macbook.be.48ers.dk> you wrote:
> >>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> writes:
> 
>  Peter> Commit c7190f02 (retain POR values of non-configured ACR, SPCR, SCCR,
>  Peter> and LCRR bitfields) moved the LCRR assignment to after relocation
>  Peter> to RAM because of the potential problem with changing the local bus
>  Peter> clock while executing from flash.
> 
> Comments?

It seems Kim is on vacation or something like that.

Kumar, is there anybody else who can pick up MPC83xx related stuff?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Gew?hnlich glaubt der Mensch,  wenn er nur Worte h?rt,  es m?sse sich
dabei doch auch was denken lassen.                 -- Goethe, Faust I

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Kim Phillips]

On Sat, 5 Dec 2009 01:29:25 +0100
Wolfgang Denk <wd@denx.de> wrote:

> Dear Peter Korsgaard,
> 
> In message <87pr7472vw.fsf@macbook.be.48ers.dk> you wrote:
> > >>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> writes:
> > 
> >  Peter> Commit c7190f02 (retain POR values of non-configured ACR, SPCR, SCCR,
> >  Peter> and LCRR bitfields) moved the LCRR assignment to after relocation
> >  Peter> to RAM because of the potential problem with changing the local bus
> >  Peter> clock while executing from flash.
> > 
> > Comments?
> 
> It seems Kim is on vacation or something like that.

I was, but I'm back now.

> Kumar, is there anybody else who can pick up MPC83xx related stuff?

I'll get them - I'm still sifting through my backlog here.

Kim

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Wolfgang Denk]

Dear Kim,

In message <20091204185150.1e2939e8.kim.phillips@freescale.com> you wrote:
> 
> > It seems Kim is on vacation or something like that.
> 
> I was, but I'm back now.

I hope you had a good time. Welcome back.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Sorry, but my karma just ran over your dogma.

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Kim Phillips]

On Fri, 20 Nov 2009 12:42:43 +0100
Peter Korsgaard <jacmet@sunsite.dk> wrote:

> E.G. on a 8347 board a bootup time increase of ~600ms has been observed:

heh, even more on an 8313!  Thanks for this - I hadn't realized the
difference was so large (or neglected it since the move to init_r was
done at the last moment).

> Fix it by introducing CONFIG_SYS_LCRR_EARLY, and set LCRR in cpu_init_f
> instead of in cpu_init_r if set.

instead of introducing the new CONFIG_SYS_LCRR_EARLY, shouldn't we
check for something like:

!defined(CONFIG_NAND_SPL) && !defined(CONFIG_SYS_RAMBOOT)

?

> +++ b/cpu/mpc83xx/cpu_init.c
> @@ -171,6 +171,30 @@ void cpu_init_f (volatile immap_t * im)
>  		(CONFIG_SYS_SCCR_SATACM << SCCR_SATACM_SHIFT) |
>  #endif
>  		0;
> +#ifdef CONFIG_SYS_LCRR_EARLY

btw, this generates:

cpu_init.c: In function 'cpu_init_r':
cpu_init.c:367: warning: unused variable 'im'

Kim

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Peter Korsgaard]

>>>>> "Kim" == Kim Phillips <kim.phillips@freescale.com> writes:

 Kim> On Fri, 20 Nov 2009 12:42:43 +0100
 Kim> Peter Korsgaard <jacmet@sunsite.dk> wrote:

 >> E.G. on a 8347 board a bootup time increase of ~600ms has been observed:

 Kim> heh, even more on an 8313!  Thanks for this - I hadn't realized the
 Kim> difference was so large (or neglected it since the move to init_r was
 Kim> done at the last moment).

OK, why exactly was it moved? What do you want to protect against? I
don't remember seing anyone complaining about the old location.

 >> Fix it by introducing CONFIG_SYS_LCRR_EARLY, and set LCRR in cpu_init_f
 >> instead of in cpu_init_r if set.

 Kim> instead of introducing the new CONFIG_SYS_LCRR_EARLY, shouldn't we
 Kim> check for something like:

 Kim> !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SYS_RAMBOOT)

As in do the reconfig early if we're running from RAM right away?
It's not that simple - I have a board which boots from NOR flash. As
that is an async device there isn't any problem in changing the LBC
settings while you're running from flash (as long as you respect the
min access time). I have another design where the flash sits behind a
FPGA (for signal integrity reasons), and there I have to wait until
we're running in RAM before changing the LBC clock.

On the 2nd design I even have to tell the FPGA to resync (through a GPIO
pin) and wait a bit before I can continue, so I'm doing the LCRR config
in board code (in board_early_init_r). The move to cpu_init_r broke that
as well as the LCRR value is overwritten there.
 
 Kim> btw, this generates:

 Kim> cpu_init.c: In function 'cpu_init_r':
 Kim> cpu_init.c:367: warning: unused variable 'im'

Ups, I'll fix that and send a new patch once we agree on the form.

-- 
Bye, Peter Korsgaard

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Kim Phillips]

On Sat, 5 Dec 2009 21:27:30 +0100
Peter Korsgaard <jacmet@sunsite.dk> wrote:

> >>>>> "Kim" == Kim Phillips <kim.phillips@freescale.com> writes:
> 
>  Kim> On Fri, 20 Nov 2009 12:42:43 +0100
>  Kim> Peter Korsgaard <jacmet@sunsite.dk> wrote:
> 
>  >> E.G. on a 8347 board a bootup time increase of ~600ms has been observed:
> 
>  Kim> heh, even more on an 8313!  Thanks for this - I hadn't realized the
>  Kim> difference was so large (or neglected it since the move to init_r was
>  Kim> done at the last moment).
> 
> OK, why exactly was it moved? What do you want to protect against? I
> don't remember seing anyone complaining about the old location.

none, really.  Just safety from any potential unknown behaviour:

http://lists.denx.de/pipermail/u-boot/2009-September/061580.html

>  >> Fix it by introducing CONFIG_SYS_LCRR_EARLY, and set LCRR in cpu_init_f
>  >> instead of in cpu_init_r if set.
> 
>  Kim> instead of introducing the new CONFIG_SYS_LCRR_EARLY, shouldn't we
>  Kim> check for something like:
> 
>  Kim> !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SYS_RAMBOOT)
> 
> As in do the reconfig early if we're running from RAM right away?
> It's not that simple - I have a board which boots from NOR flash. As
> that is an async device there isn't any problem in changing the LBC
> settings while you're running from flash (as long as you respect the
> min access time). I have another design where the flash sits behind a
> FPGA (for signal integrity reasons), and there I have to wait until
> we're running in RAM before changing the LBC clock.
> 
> On the 2nd design I even have to tell the FPGA to resync (through a GPIO
> pin) and wait a bit before I can continue, so I'm doing the LCRR config
> in board code (in board_early_init_r). The move to cpu_init_r broke that
> as well as the LCRR value is overwritten there.

ok let's leave it as _EARLY then.  wrt boards in current u-boot, can
you add the _EARLY define in their default config files?  I'm trying to
extend the benefit of the patch to existing boards, not only out-of-tree
boards.  Speaking of which, new board patches are welcome.

Kim

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Peter Korsgaard]

>>>>> "Kim" == Kim Phillips <kim.phillips@freescale.com> writes:

Hi,

 Kim> ok let's leave it as _EARLY then.  wrt boards in current u-boot, can
 Kim> you add the _EARLY define in their default config files?

Sure - But if we're adding _EARLY to all the configs, shouldn't we just
make _EARLY default (or simply get rid of the late variant?)

 Kim> I'm trying to extend the benefit of the patch to existing boards,
 Kim> not only out-of-tree boards.  Speaking of which, new board patches
 Kim> are welcome.

Also for specialized (E.G. non-evaluation boards / consumer products)
boards? I have no problem submitting them, I just see very limited added
value for others.

-- 
Bye, Peter Korsgaard

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Kim Phillips]

On Mon, 7 Dec 2009 22:10:49 +0100
Peter Korsgaard <jacmet@sunsite.dk> wrote:

> >>>>> "Kim" == Kim Phillips <kim.phillips@freescale.com> writes:
>  Kim> ok let's leave it as _EARLY then.  wrt boards in current u-boot, can
>  Kim> you add the _EARLY define in their default config files?
> 
> Sure - But if we're adding _EARLY to all the configs, shouldn't we just
> make _EARLY default (or simply get rid of the late variant?)

probably the latter.  Thanks!

>  Kim> I'm trying to extend the benefit of the patch to existing boards,
>  Kim> not only out-of-tree boards.  Speaking of which, new board patches
>  Kim> are welcome.
> 
> Also for specialized (E.G. non-evaluation boards / consumer products)
> boards? I have no problem submitting them, I just see very limited added
> value for others.

sure, why not?  It allows me and others to see and allow for such real
use-cases, plus you get the benefit of not having to maintain patches
externally.

Kim

[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early

[Peter Korsgaard]

>>>>> "Kim" == Kim Phillips <kim.phillips@freescale.com> writes:

Hi,

 >> Sure - But if we're adding _EARLY to all the configs, shouldn't we just
 >> make _EARLY default (or simply get rid of the late variant?)

 Kim> probably the latter.  Thanks!

Great - I'll send an updated patch just doing that then instead.

 >> Also for specialized (E.G. non-evaluation boards / consumer products)
 >> boards? I have no problem submitting them, I just see very limited added
 >> value for others.

 Kim> sure, why not?  It allows me and others to see and allow for such real
 Kim> use-cases, plus you get the benefit of not having to maintain patches
 Kim> externally.

Ok. I'll sort out what to do with the ancient board/barco directory that
already is in the tree, and then submit board/barco/<boards>.

-- 
Bye, Peter Korsgaard