From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Mon, 6 Jul 2015 16:25:35 -0500 Subject: [U-Boot] [PATCH 7/8] mtd/nand/ubi: assortment of alignment fixes In-Reply-To: <201507031544.05678.marex@denx.de> References: <201507020753.36195.marex@denx.de> <1435872919.10531.1.camel@freescale.com> <201507031544.05678.marex@denx.de> Message-ID: <1436217935.2658.8.camel@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > > > > > > > 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 > > > > > > 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. > > 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? > 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? -Scott