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

[Sughosh Ganu <urwithsughosh@gmail.com>]

hi Christian,

On Sun Jan 29, 2012 at 02:36:39PM +0100, Christian Riesch wrote:
> Hi all,
> 
> On Fri, Jan 27, 2012 at 7:33 PM, Tom Rini <tom.rini@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

  This is correct, the only differece being, 

  flush caches, is what the comment said, but it was actually doing a
  invalidate caches.

<snip> 


> 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?

  Actually, even i can do with the enabling of the icache :). The only
  change i made was that instead of invalidating the data cache, i do
  doing a "test and flush".

> 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.

  Actually, the part of the code that Albert NAK'ed was changing the
  setting of the V bit. To this he had proposed to make this a config
  option.

  The current behaviour clears the V bit by default in the
  cpu_init_crit function, which maps the exception vectors at 0x0. But
  some SoC's don't have anything mapped at 0x0(eg. omapl-138), so i
  had proposed not to clear this bit by default.

  http://www.mail-archive.com/u-boot at lists.denx.de/msg75271.html

> 
> 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?

  Please check the link i have provided above.

-sughosh