[U-Boot] [v3] command/cache: Add flush command

[Scott Wood]

On 04/09/2013 12:45:31 PM, Wolfgang Denk wrote:
> Dear Scott,
> 
> In message <1365449512.28843.10 at snotra> you wrote:
> > 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.

If that means you have some other reason for objecting that you *do*  
want to go into, could you elaborate?

I do agree that flush is not a good name for what we do on the data  
side for PPC, given that we use dcbst and not dcbf.

> > > 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 }
> 
> would go into the "icache" command.
> Lines 36..42 would go into the "dcache" command, while lines 44..51
> 
> So far this code appears to work fine.  What am I missing?

This is just one implementation of flush_cache().  Here are all of them:

arch/sparc/lib/cache.c:void flush_cache(ulong start_addr, ulong size)
arch/openrisc/cpu/cache.c:void flush_cache(unsigned long addr, unsigned  
long size)
arch/powerpc/lib/cache.c:void flush_cache(ulong start_addr, ulong size)
arch/nds32/lib/cache.c:void flush_cache(unsigned long addr, unsigned  
long size)
arch/m68k/lib/cache.c:void flush_cache(ulong start_addr, ulong size)
arch/sh/cpu/sh4/cpu.c:void flush_cache (unsigned long addr, unsigned  
long size)
arch/sh/cpu/sh2/cpu.c:void flush_cache(unsigned long addr, unsigned  
long size)
arch/sh/cpu/sh3/cpu.c:void flush_cache(unsigned long addr, unsigned  
long size)
arch/microblaze/cpu/cache.c:void flush_cache (ulong addr, ulong size)
arch/arm/cpu/arm1136/cpu.c:void flush_cache(unsigned long start,  
unsigned long size)
arch/arm/cpu/arm1136/cpu.c:void flush_cache(unsigned long start,  
unsigned long size)
arch/arm/cpu/arm926ejs/cache.c:void flush_cache(unsigned long start,  
unsigned long size)
arch/arm/cpu/arm926ejs/cache.c:void flush_cache(unsigned long start,  
unsigned long size)
arch/blackfin/lib/cache.c:void flush_cache(unsigned long addr, unsigned  
long size)
arch/mips/cpu/mips64/cpu.c:void flush_cache(ulong start_addr, ulong  
size)
arch/mips/cpu/mips32/cpu.c:void flush_cache(ulong start_addr, ulong  
size)
arch/mips/cpu/xburst/cpu.c:void flush_cache(ulong start_addr, ulong  
size)

This is what we're trying to expose as a command.  If you want it split  
into icache and dcache, it needs to be rewritten for every architecture  
(or else we'd need to limit the command to only running on certain  
architectures), and not all of them are as easily splittable.  What is  
the actual need for splitting it?  The semantics we're trying to expose  
as a command are the same as this function is meant to implement,  
regardless of the choice of name.  Those semantics are "make the data  
we just wrote to this range visible to instruction fetches".

-Scott