[U-Boot] [PATCH, REGRESSION] mpc83xx: provide option to set LCRR early
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);
"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(-)
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
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
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
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
"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.
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
"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.
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
"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>.
participants (3)
-
Kim Phillips -
Peter Korsgaard -
Wolfgang Denk