Hi Henrik,
On Wed, 15 May 2013 18:34:07 +0200, Henrik Nordström henrik@henriknordstrom.net wrote:
ons 2013-05-15 klockan 17:11 +0200 skrev Albert ARIBAUD:
What is the rationale behind putting it in arch/ rather than in common/ by adding this to the existing common/cmd_boot.c file under ARMv7 conditionals?
Only because of what I said earlier: blindly calling invalidate_icache_all() from the go command will cause loud complains on other arches where the icache is not supported/enabled.
I probably didn't make myself clear: why not put it in comm/cmd_boot.c *whthin a pair of #if ... ARMV7... /#endif conditionals*?
Also:
ARM v7 runs with icache enabled. For reliable results the go command needs to flush the icache before jumping or it may risk running cached instructions that differ from what currently is in memory.
IIUC, the issue is due to cache being enabled when doing the go, rather than due to armv7 per se.
Not entirely sure what you mean.
I mean that the problem is having icache and DDR inconsistency at the time of doing a go, whatever the CPU (or even architecture concerned). This happens because the icache and DDR are inconsistent, not because the CPU is an ARMv7. IOW, this could happen on an ARMv5 just as well, or on any architecture that has separate i an dcache and loads code in DDR.
So, should we not have this icache flush conditioned at compile time on cache being compiled in, and at run time on cache being enabled? This way, we would automatically cater for the same issue appearing in other ARM CPUs, and even more, in other architectures.
Except.. see common/cmd_cache.c top of file and you'll see the yelling I am talking about.
I'd rather you explained it, as you certainly read this file with the knowledge of the issue, and thus immediately focus on part of it that are salient to your eyes, while I will read the same file without such knowledge and might miss the point.
Regards Henrik
Amicalement,