[U-Boot] [PATCH 6/9] CACHE: nand read/write: Test if start address is aligned

[Marek Vasut <marex@denx.de>]

Dear Scott Wood,

> On 06/24/2012 07:17 PM, Marek Vasut wrote:
> > This prevents the scenario where data cache is on and the
> > device uses DMA to deploy data. In that case, it might not
> > be possible to flush/invalidate data to RAM properly. The
> > other option is to use bounce buffer,
> 
> Or get cache coherent hardware. :-)

Oh ... you mean powerpc? Or rather something like this 
http://cache.freescale.com/files/32bit/doc/fact_sheet/QORIQLS2FAMILYFS.pdf ? :-D

> > but that involves a lot of copying and therefore degrades performance
> > rapidly. Therefore disallow this possibility of unaligned load
> > address altogether if data cache is on.
> 
> How about use the bounce buffer only if the address is misaligned?

Not happening, bounce buffer is bullshit, especially if we can prevent it by 
teaching user not to do retarded things.

It's like driving a car in the wrong lane. Sure, you can do it, but it'll 
eventually have some consequences. And using a bounce buffer is like driving a 
tank in the wrong lane ...

> The
> corrective action a user has to take is the same as with this patch,
> except for an additional option of living with the slight performance
> penalty.

Slight is very weak word here.

> How often does this actually happen?  How much does it
> actually slow things down compared to the speed of the NAND chip?

If the user is dumb, always. But if you tell the user how to milk the most of 
the hardware, he'll be happier.

> I'm hesitant to break something -- even if it's odd (literally in this
> case) -- that currently works on most hardware, just because one or two
> drivers can't handle it.  It feels kind of like changing the read() and
> write() system calls to require cacheline alignment. :-P

That's actually almost right, we're doing a bootloader here, it might have 
limitations. We're not writing yet another operating system with no bounds on 
possibilities!

> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Scott Wood <scottwood@freescale.com>
> > ---
> > 
> >  common/cmd_nand.c |    9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> > index a91ccf4..122a91c 100644
> > --- a/common/cmd_nand.c
> > +++ b/common/cmd_nand.c
> > @@ -609,6 +609,8 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc,
> > char * const argv[])
> > 
> >  			goto usage;
> >  		
> >  		addr = (ulong)simple_strtoul(argv[2], NULL, 16);
> > 
> > +		if (!cacheline_aligned(addr))
> > +			return 1;
> 
> There's no way you can just return like this without printing an error
> that lets the user know that the operation wasn't performed, and why.
> 
> -Scott

Best regards,
Marek Vasut