Dear Scott,
In message 1365449512.28843.10@snotra you wrote:
Maybe "cache" should be the toplevel command, with "icache" and "dcache" refactored to be subcommands? Of course, then you're making an incompatible interface change. How much is consistency worth?
I think backward compatibility is mandatory here. We cannot break existing user scripts.
Sure. But if the main reason for the icache/dcache split is compatibility, I don't think that should constrict the form of new commands.
Backward compatibility is just the argument for not changing the existing command interfaces. I tried to explain why I do not want to see the flush_cache() combination of functions exposed to end users.
I think the terminology is just fine. It's not just invalidating the icache (flushing and invalidating are the same thing for cache lines which are not modified -- or are incapable of being modified). It's flushing the region out of *all* caches.
There is a well-defined difference between flushing and invalidating a cache, and I don't intend to go into hair-splitting mode here just to follow your path which would lead me to where I d not want to go.
See above. In such a case "icache" and "dcache" can just call the same underlying code.
And then we end up having to do the flush twice, if the user follows the "first flush dcache, then flush icache" instructions. That's not an ideal interface.
OK, this is indeed a good argument. But is this really the case? When looking at the current code of flash_cache() for PPC, it seems actually easy to split:
28 void flush_cache(ulong start_addr, ulong size) 29 { 30 #ifndef CONFIG_5xx 31 ulong addr, start, end; 32 33 start = start_addr & ~(CONFIG_SYS_CACHELINE_SIZE - 1); 34 end = start_addr + size - 1; 35 36 for (addr = start; (addr <= end) && (addr >= start); 37 addr += CONFIG_SYS_CACHELINE_SIZE) { 38 asm volatile("dcbst 0,%0" : : "r" (addr) : "memory"); 39 WATCHDOG_RESET(); 40 } 41 /* wait for all dcbst to complete on bus */ 42 asm volatile("sync" : : : "memory"); 43 44 for (addr = start; (addr <= end) && (addr >= start); 45 addr += CONFIG_SYS_CACHELINE_SIZE) { 46 asm volatile("icbi 0,%0" : : "r" (addr) : "memory"); 47 WATCHDOG_RESET(); 48 } 49 asm volatile("sync" : : : "memory"); 50 /* flush prefetch queue */ 51 asm volatile("isync" : : : "memory"); 52 #endif 53 }
Lines 36..42 would go into the "dcache" command, while lines 44..51 would go into the "icache" command.
So far this code appears to work fine. What am I missing?
Best regards,
Wolfgang Denk