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

[Wolfgang Denk]

Dear Scott,

In message <1365449512.28843.10 at 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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You don't stop doing things because you get old.  You get old because
you stop doing things.                            - Rosamunde Pilcher