Message ID | E20F32A5-0EF5-4CC3-AB70-0C1E2330EF27@kernel.crashing.org |
---|---|
State | RFC |
Headers | show |
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
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 --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