[U-Boot] [PATCH] lib_ppc: rework the flush_cache

[Wolfgang Denk]

Dear "Liu Dave",

In message <79C363B768933F4FB918025FF7EB888856A046 at zch01exm21.fsl.freescale.net> you wrote:
>  
> > That comment was on the version you posted in the NAND patch; the 
> > lib_ppc version actually looks worse -- it tried to round 
> > down to avoid 
> > the issue, but it was missing a ~.  Thus, it flushed everything from 
> > address 0 to the end.
> 
> the lib_ppc version basically is as below.
> void flush_cache (ulong start_addr, ulong size)
> {
> 	ulong addr, end_addr = start_addr + size;
> 	addr = start_addr & (CONFIG_SYS_CACHELINE_SIZE - 1);
> 	for (addr = start_addr; addr < end_addr; addr +=
> CONFIG_SYS_CACHELINE_SIZE) {
> 		asm ("dcbst 0,%0": :"r" (addr));
> 	}
> }
> 
> so, you are not completely right, the flush is from start_addr.
> I believe my commit log also is proper for lib_ppc version.
> 
> > > +	start = start_addr & ~(CONFIG_SYS_CACHELINE_SIZE - 1);
> > > +	end = (start_addr + size) & ~(CONFIG_SYS_CACHELINE_SIZE - 1);
> > 
> > end = start_addr + size - 1;
> > 
> > The rounding is unnecessary for end, and without the - 1, if 
> > start_addr 
> > + size is on a cacheline boundary, you'll flush one cache 
> > line too many 
> > (which might not be mapped, or might cause end to wrap around 
> > to zero if 
> > flushing at the end of the address space).
> 
> I don't see what is the problem in my patch at here.

Scott explained that instead of

	end = (start_addr + size) ...

you should use

	end = (start_addr + size - 1) ...

(wether or not the rounding makes sense is not really clear to me.

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
The nice thing about  standards  is that there are  so many to choose
from.                                           - Andrew S. Tanenbaum