On Wed, May 15, 2013 at 06:44:10PM +0200, Albert ARIBAUD wrote:
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*?
There is no such #if test we can do for ARMv7 I believe. There was, long ago, a CONFIG_ARMV7 but that's been removed for ages.
[snip]
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.
To summarize why I rejected the last patch in the end, most arches do not define an invalidate_icache_all so they will see for every "go": No arch specific invalidate_icache_all available! And many other arches simply won't compile now as they don't link common/cmd_cache.o and do not have an invalidate_icache_all anywhere.