diff mbox

[U-Boot] RFC: Move when we call cpu_init_r on PPC

Message ID E20F32A5-0EF5-4CC3-AB70-0C1E2330EF27@kernel.crashing.org
State RFC
Headers show

Commit Message

Kumar Gala Nov. 13, 2010, 5 p.m. UTC
Wolfgang, Stefan, Kim, (other PPC arch maintainers)

I'd like to move cpu_init_r to after env_relocate().  This is desirable on 85xx based systems to deal with an issue when we boot from NAND (and probably SPI or SDHC/MMC) in which we use the L2 cache in SRAM mode to load the u-boot image from the storage device (NAND, SPI, SDHC/MMC, etc.).  In these cases we have an ordering issue between freeing up the L2 cache and when the environment is relocated.

If we move cpu_init_r() after env_relocate() that addresses the issue.  I did an audit of all the other PPC chips and I dont believe it impacts any of them if we move cpu_init_r after spi_init_{f,r}, nand_init, mmc_initialize, & env_relocate.

Please let me know if this is acceptable solution to my problem.  I've provided both the 'audit' details and example diff.

- k

----

My audit of what we do in cpu_init_r on all PPCs:

74xx_7xx/cpu_init.c:int cpu_init_r (void)
        Does nothing
mpc512x/cpu_init.c:int cpu_init_r (void)
        Does nothing
mpc5xx/cpu_init.c:int cpu_init_r (void)
        Does nothing
mpc5xxx/cpu_init.c:int cpu_init_r (void)
        Mask interrupts, loadtask/microcode
mpc8220/cpu_init.c:int cpu_init_r (void)
        Mask interrupts, loadtask/microcode
mpc824x/cpu_init.c:int cpu_init_r (void)
        init EUMMBAR on MPC10x
mpc8260/cpu_init.c:int cpu_init_r (void)
        init RISC Controller Cfg Register (most boards either 0 or modify TIMEP)
mpc83xx/cpu_init.c:int cpu_init_r (void)
        init QE
mpc85xx/cpu_init.c:int cpu_init_r(void)
        init L2/L3 caches
        init QE
        init SERDES
        setup secondary cores (MP)
        set LCRR
mpc86xx/cpu_init.c:int cpu_init_r(void)
        setup secondary cores (MP)
mpc8xx/cpu_init.c:int cpu_init_r (void)
        setup RTCSC
        setup RMDS
ppc4xx/cpu_init.c:int cpu_init_r (void)
        sets edge conditioning circuitry on PPC405GPr

----

Comments

Wolfgang Denk Nov. 13, 2010, 6:22 p.m. UTC | #1
Dear Kumar Gala,

In message <E20F32A5-0EF5-4CC3-AB70-0C1E2330EF27@kernel.crashing.org> you wrote:
> 
> I'd like to move cpu_init_r to after env_relocate().  This is desirable
> on 85xx based systems to deal with an issue when we boot from NAND (and
> probably SPI or SDHC/MMC) in which we use the L2 cache in SRAM mode to
> load the u-boot image from the storage device (NAND, SPI, SDHC/MMC,
> etc.).  In these cases we have an ordering issue between freeing up the
> L2 cache and when the environment is relocated.

I don't think that's a good idea. cpu_init_r() starts a number of
pretty basic servies and must run as early as possible after
relocation. For example, it may initialize caches, load microcode
needed for some drivers soon, initialize interrupts and timers and
such...

Moving these things around requires extreme care and really intensive
testing on a lots of boards.

> If we move cpu_init_r() after env_relocate() that addresses the issue.
> I did an audit of all the other PPC chips and I dont believe it impacts
> any of them if we move cpu_init_r after spi_init_{f,r}, nand_init,
> mmc_initialize, & env_relocate.

Um.. I doubt that. My fingers still carry the fire scars from my last
attempt to do something like that.

> Please let me know if this is acceptable solution to my problem.  I've
> provided both the 'audit' details and example diff.

Thanks for that. It only shows that there is alot of pretty basic
stuff done, which might be needed.  You will need to retest a ton of
boards.

Best regards,

Wolfgang Denk
Kumar Gala Nov. 15, 2010, 3:12 p.m. UTC | #2
On Nov 13, 2010, at 12:22 PM, Wolfgang Denk wrote:

> Dear Kumar Gala,
> 
> In message <E20F32A5-0EF5-4CC3-AB70-0C1E2330EF27@kernel.crashing.org> you wrote:
>> 
>> I'd like to move cpu_init_r to after env_relocate().  This is desirable
>> on 85xx based systems to deal with an issue when we boot from NAND (and
>> probably SPI or SDHC/MMC) in which we use the L2 cache in SRAM mode to
>> load the u-boot image from the storage device (NAND, SPI, SDHC/MMC,
>> etc.).  In these cases we have an ordering issue between freeing up the
>> L2 cache and when the environment is relocated.
> 
> I don't think that's a good idea. cpu_init_r() starts a number of
> pretty basic servies and must run as early as possible after
> relocation. For example, it may initialize caches, load microcode
> needed for some drivers soon, initialize interrupts and timers and
> such...
> 
> Moving these things around requires extreme care and really intensive
> testing on a lots of boards.
> 
>> If we move cpu_init_r() after env_relocate() that addresses the issue.
>> I did an audit of all the other PPC chips and I dont believe it impacts
>> any of them if we move cpu_init_r after spi_init_{f,r}, nand_init,
>> mmc_initialize, & env_relocate.
> 
> Um.. I doubt that. My fingers still carry the fire scars from my last
> attempt to do something like that.
> 
>> Please let me know if this is acceptable solution to my problem.  I've
>> provided both the 'audit' details and example diff.
> 
> Thanks for that. It only shows that there is alot of pretty basic
> stuff done, which might be needed.  You will need to retest a ton of
> boards.

Well I'm still stuck for a solution to my problem :)

So is a post_env_relocate_cleanup() the way to go?  I can than differ L2 init to that point in the case we use the L2 SRAM for boot memory?

- k
diff mbox

Patch

diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c
index 2e0749d..aaa5d1e 100644
--- a/arch/powerpc/lib/board.c
+++ b/arch/powerpc/lib/board.c
@@ -754,11 +754,6 @@  void board_init_r (gd_t *id, ulong dest_addr)
 
        WATCHDOG_RESET ();
 
-       /* initialize higher level parts of CPU like time base and timers */
-       cpu_init_r ();
-
-       WATCHDOG_RESET ();
-
 #ifdef CONFIG_SPI
 # if !defined(CONFIG_ENV_IS_IN_EEPROM)
        spi_init_f ();
@@ -786,6 +781,11 @@  void board_init_r (gd_t *id, ulong dest_addr)
        /* relocate environment function pointers etc. */
        env_relocate ();
 
+       /* initialize higher level parts of CPU like time base and timers */
+       cpu_init_r ();
+
+       WATCHDOG_RESET ();
+
        /*
         * Fill in missing fields of bd_info.
         * We do this here, where we have "normal" access to the