[U-Boot] [PATCH 1/3] mtd: onenand: Fix unaligned access

[Marek Vasut <marex@denx.de>]

On Tuesday, December 31, 2013 at 11:43:48 AM, Lukasz Majewski wrote:
> Hi Marek,
> 
> > On Saturday, December 28, 2013 at 05:06:28 PM, Lukasz Majewski wrote:
> > > Hi Marek,
> > > 
> > > > On Thursday, December 26, 2013 at 01:01:24 AM, Marek Vasut wrote:
> > > > > Fix unaligned access in OneNAND core. The problem is that the
> > > > > ffchars[] array is an array of "unsigned char", but in
> > > > > onenand_write_ops_nolock() can be passed to the memcpy_16()
> > > > > function. The memcpy_16() function will treat the buffer as an
> > > > > array of "unsigned short", thus triggering unaligned access if
> > > > > the compiler decided ffchars[] to be not aligned.
> > > > > 
> > > > > I managed to trigger the problem with regular ELDK 5.4 GCC
> > > > > compiler.
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > > > > Cc: Scott Wood <scottwood@freescale.com>
> > > > > Cc: Tom Rini <trini@ti.com>
> > > > > ---
> > > > > 
> > > > >  drivers/mtd/onenand/onenand_base.c | 8 +++++++-
> > > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/mtd/onenand/onenand_base.c
> > > > > b/drivers/mtd/onenand/onenand_base.c index 979e4af..e33e8d3
> > > > > 100644 --- a/drivers/mtd/onenand/onenand_base.c
> > > > > +++ b/drivers/mtd/onenand/onenand_base.c
> > > > > @@ -91,7 +91,13 @@ static struct nand_ecclayout onenand_oob_32
> > > > > = {
> > > > > 
> > > > >  	.oobfree	= { {2, 3}, {14, 2}, {18, 3}, {30, 2} }
> > > > >  
> > > > >  };
> > > > > 
> > > > > -static const unsigned char ffchars[] = {
> > > > > +/*
> > > > > + * Warning! This array is used with the memcpy_16() function,
> > > > > thus
> > > > > + * it must be aligned to 2 bytes. GCC can make this array
> > > > > unaligned
> > > > > + * as the array is made of unsigned char, which memcpy16()
> > > > > doesn't
> > > > > + * like and will cause unaligned access.
> > > > > + */
> > > > > +static const unsigned char __aligned(2) ffchars[] = {
> > > > > 
> > > > >  	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > > > >  	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > > > > 
> > > > > 0xff,	/*
> > > > > 
> > > > > 16 */ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > > > 
> > > > Lukasz, can you please review this one?
> > > 
> > > No problem, I will review the patch when I come back to the office
> > > (and be able to test it on the proper HW).
> > 
> > Thanks, I'd love to get this fix into 2014.01 . Maybe the real fix
> > here would be to fix the memcpy_16() instead tho.
> 
> I've tested it with eldk-5.4 toolchain on GONI device. It seems that no
> regression is caused by this patch.
> 
> As a side note - this problem didn't show up for GONI board. Apparently
> I was lucky.
> 
> I will scrutinize this code in respect of code alignment.
> 
> Applied to u-boot-onenand.

Thank you very much!

Have a happy new year, guys !

Best regards,
Marek Vasut