On 04/10/2013 05:50:56 PM, Wolfgang Denk wrote:
Dear Scott Wood,
In message 1365628243.8381.20@snotra you wrote:
testing that we cannot do). Blackfin is weird -- if we did a
simple
split at the C-code level it looks like we'd have two dummy
loops
executing.
Huh? flush_cache() does not include any loop for BF.
Yes it does. It's in assembly code (do_flush macro in cache.S).
Or at
least, it looks sort of like a loop -- I'm not familiar with
blackfin
assembly code.
do_flush is not used anywhere within flush_cache(); we are talking about "arch/blackfin/lib/cache.c" here, don't we?
Sigh. blackfin's flush_cache() calls blackfin_icache_dcache_flush_range(), which is in cache.S.
blackfin_icache_dcache_flush_range() calls do_flush.
What the actual implementation of blackfin_icache_dcache_flush_range() resp. blackfin_icache_flush_range() resp. blackfin_dcache_flush_range() might look like is of no real interest for this discussion here, right? We just need to call these functions, there is no need to touch these in any way in the context of what we are discussing here.
Really, you'd be OK with leaving dead code around? Nothing else would call blackfin_icache_dcache_flush_range() if we remove the combined flushing in cache.c. Even if we remove blackfin_icache_dcache_flush_range(), that leaves mechanics in do_flush that are no longer needed (the ability to do two different cache ops inside the loop).
Do you have any real technical arguments?
I'm done arguing and have already recommended we just keep this patch in our local tree. We do not have unlimited time to spend messing around with other arch code to produce a result that wouldn't be better in any meaningful way. If someone else wants to hack up flush_cache(), they're welcome to.
-Scott