[U-Boot] [PATCH 7/8] mtd/nand/ubi: assortment of alignment fixes

[Marek Vasut]

On Monday, July 06, 2015 at 11:25:35 PM, Scott Wood wrote:
> On Fri, 2015-07-03 at 15:44 +0200, Marek Vasut wrote:
> > On Thursday, July 02, 2015 at 11:35:19 PM, Scott Wood wrote:
> > > On Thu, 2015-07-02 at 07:53 +0200, Marek Vasut wrote:
> > > > On Thursday, July 02, 2015 at 01:04:52 AM, Marcel Ziswiler wrote:
> > > > > From: Marcel Ziswiler <marcel.ziswiler at toradex.com>
> > > > > 
> > > > > Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take
> > > > > buffer
> > > > > alignment into account which led to failures of the following form:
> > > > > 
> > > > > ERROR: v7_dcache_inval_range - start address is not aligned -
> > > > > 0x1f7f0108 ERROR: v7_dcache_inval_range - stop address is not
> > > > > aligned - 0x1f7f1108
> > > > > 
> > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler at toradex.com>
> > > > 
> > > > What about using ALLOC_CACHE_ALIGN_BUFFER() and friends instead ? See
> > > > include/common.h for their definition, this is what those functions
> > > > are exactly for.
> > > 
> > > ALLOC_CACHE_ALIGN_BUFFER() is for statically allocating an aligned
> > > buffer.
> > 
> > You're confusing this with DEFINE_ALIGN_BUFFER, no ?
> 
> OK, not "statically", but on the stack.  It is not appropriate to turn
> dynamic allocations into stack allocations without considering how large
> the allocation can be.  It'd also be more intrusive a change than
> necessary, even if the sizes were small enough.

Fine.

> > > Dynamically allocating an aligned buffer is exactly what memalign() is
> > > for.
> > 
> > Isn't memalign()ed memory aligned only to the start address, while the
> > end address (and thus the length) is not aligned ?
> 
> The end address is aligned if the size passed to memalign is aligned. 
> Maybe add a wrapper that calls memalign() with the size rounded up to
> ARCH_DMA_MINALIGN?

I agree.

> >  This is what memalign(3) has
> > 
> > to say:
> > 
> > "
> > The function posix_memalign() allocates size bytes and places the
> > address of the allocated memory in *memptr.  The address of the
> > allo‐ cated memory will be a multiple of alignment, which must
> > be a power of two and a multiple of sizeof(void *).  If size is  0,
> > then  the value placed in *memptr is either NULL, or a unique pointer
> > value that can later be successfully passed to free(3).
> > 
> > The obsolete function memalign() allocates size bytes and returns a
> > pointer to the allocated memory.  The memory address will be a mul‐
> > tiple of alignment, which must be a power of two.
> > "
> 
> posix_memalign() does not exist in U-Boot, and it's not clear to me why
> memalign() should be considered obsolete.  Is the difference just the
> ability to return -EINVAL?

The args are also totally different.

Best regards,
Marek Vasut