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

[Christian Riesch]

Hi,

On Monday, January 30, 2012, Heiko Schocher <hs at denx.de> wrote:
> Hello Christian,
>
> Christian Riesch wrote:
>> 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.
>
> Yep.
>
>> 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?
>
> or we should introduce a "CONFIG_SKIP_CPU_CRIT_INIT" define, which
> if defined, prevent the jump to cpu_init_crit ... so we have the same
> behaviour as before commit "ca4b55800ed74207c35271bf7335a092d4955416"
>
> Boards which have problems with cpu_crit_init, should define
> this new define ... but it would be better to find out, what
> is really going on here ...

Yes, that would be good of course.

Another idea: Actually the init code that we are discussing here is
executed twice. First in the SPL and then in the full-blown u-boot.
Sughosh, are you sure it is the execution of the code in SPL that causes
the trouble? I'm asking since we don't do any memory barrier related stuff
in the code that loads u-boot from flash in SPL. Of course, dcache is off
but icache is on.

Regards, Christian