Dear Scott,
In message 1365634846.8381.24@snotra you wrote:
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.
Let me try to summarize the situation:
- You want to add a user command that implements some cache operations for a range of addresses. The needed operations are 1) flushing the data cache; and 2) invalidating the instruction cache.
- U-Boot currently provides a common, architecture-independent C API that implements this functionality:
void flush_cache(ulong start_addr, ulong size)
All architectures provide such a function, but implementation differs: some provide functions like
void flush_dcache_range(unsigned long addr, unsigned long end) void invalidate_icache_range(unsigned long addr, unsigned long end)
while others (like SPARC) always flush the whole cache, and even other implement only parts like data cache flushing (like sh4) or just an empty dummy function (like sh2).
- The cheapest way (in terms of efforts) to provide the requested command line API would be to add a new command that maps to the existing flush_cache() function.
- However, we already have a command line API that deals with cache operations: the commands "icache" and "dcache".
- The existing command line API could be extended like this:
dcache flush addr size => flush_dcache_range(addr, addr+size) icache invalidate addr size => invalidate_icache_range(addr, addr+size)
[Additional functionality, like flushing/invalidating the whole caches if arguments are omitted, are straightforward to add.]
- This implementation requires more effort, since all architectures need to be touched (and then tested), and some of the architecture specific code needs a closer look. Also. this approach is less convenient to the user who now needs to run two commands instead of one.
---------------------------------------------------------------------
Do you agree to this summary, or am I missing important points or is anything wrong?
Cache handling has always been a much neglected area in U-Boot. There has never been an attempt to provide a common, generic API (neither in C nor on the command line) for it. Instead, cache support has always been added only where needed, i. e. when some cache related bug popped up. You can see the result of this by just comparing the implemen- tations of flush_cache() for the different architectures: basically evera architecture invented her approach and interfaces. This is an area where the code is not exactly in perfect shape - any work to unify this wuld be more than welcome.
When we are now discussing to provide a command line API to more cache related functions we should not make the same mistakes again. That means that we should not just take existing code and use it just because it is existing (and thus convenient to use), but we should spend a thought or two to come up with a simple yet powerful (and backward compatible) command line API that can be used across all currently known architectures.
This is why I resist to the cheap and easy way of just adding somthing like a "flush_cache" command.
Best regards,
Wolfgang Denk