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.r...@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@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
