[U-Boot] [PATCH 1/2 V2] arm926: Flush the data cache before disabling it.

[Christian Riesch]

Hi all,

On Fri, Jan 27, 2012 at 7:33 PM, Tom Rini <tom.rini at gmail.com> wrote:
> So, what do we want to do here?  We really want to get this fix in so
> we can get the hawkboard SPL changes in, and the other platforms /
> fixups that are gated by that.
>
> If I can sum it up, in the relevant section of code we have incorrect
> comments and the init code is not following what the manual says the
> correct sequence is.  However, given the (potentially wide) impact the
> changes would have, Albert had previously requested making the change
> opt-in (but I believe this request came before the "the manual says we
> must do ...").  If this is still the case?  If so, can we get an
> updated patch?  Thanks!

A few thoughts:

1) Before commit ca4b55800ed74207c35271bf7335a092d4955416 the low
level initialization code that we are discussing here was only
executed if CONFIG_SKIP_LOWLEVEL_INIT is not defined. For ARM926EJS
the relevant section in start.S looked like this

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
flush caches
turn off dcache, do some other configurations of the CPU, enable icache
call the SoC specific lowlevel_init
#endif

For DA850 SoCs we had no lowlevel_init routine and therefore
CONFIG_SKIP_LOWLEVEL_INIT defined in all board configurations. The
lowlevel initialization was done by TI's UBL boot loader. Now we have
boards that don't use UBL (e.g. enbw_cmc, calimain, da850evm when used
with the SPL), therefore we need some lowlevel init. Most important
configuration is enabling icache, otherwise u-boot startup gets really
slow.

Since commit ca4b55800ed74207c35271bf7335a092d4955416 it is

flush caches
turn off dcache, do some other configurations of the CPU, enable icache
#ifndef CONFIG_SKIP_LOWLEVEL_INIT
call the SoC specific lowlevel_init
#endif

So the change that has a big impact is already done and in mainline.

Perhaps we should revert that change and instead remove
CONFIG_SKIP_LOWLEVEL_INIT from the da850 board config files. But since
we don't need the lowlevel_init function for DA850 SoCs we must either
add a dummy function or add some additional defines that allow
removing the CONFIG_SKIP_LOWLEVEL_INIT define without calling
lowlevel_init. Any suggestions how such defines should be called or
how the logic behind them should be so it doesn't cause breakage of
existing boards?

What do you think? Or is reverting to dangerous since we would change
the behavior of start.S twice if we do so?

2) The current version of Sughosh's patch does not change the logic
behind the LOWLEVEL_INIT defines but just fixes the code to agree with
ARM's manual. Instead of invalidating the cache it now is flushed. I
think we should therefore merge his patch (@Tom: Yes, the manual says
we must do.). The big change that Albert fears was already done
earlier in commit ca4b55800ed74207c35271bf7335a092d4955416. And while
we are doing this we also get the comments right.

3) As Sughosh pointed out, the current code changes the V bit
(location of exceptions). Sughosh's patch removes this code that does
this change.  I'm not sure if this is correct or not, so maybe you,
Tom, could put your TI hat on again and clarify this?

4) The current code turns on icache. This is great since my board
executes u-boot directly from flash until it relocates itself and thus
the startup time is greatly reduced by using icache. However, there is
this CONFIG_SYS_ICACHE_OFF define. Should the code be changed to
respect this define?

Regards, Christian