On Sun, 2017-08-27 at 16:37 +0200, Wolfram Sang wrote: > Please use scripts/get_maintainers.pl as cc-cmd of git-send-email. This > would have added Seth and Neil (driver maintainers) to CC automatically. > I have done so now. My bad; sorry about that and thanks for adding Seth and Neil. > The IIO subsystems uses ____cacheline_aligned. I have never looked into > this closely, but it probably is cleaner than working with ALIGN on an > oversized buffer? Dunno why ____cacheline_aligned has four underscores > at the beginning, though... I haven't looked into that either; it seems that ____cacheline_aligned is just __attribute__((__aligned__(SMP_CACHE_BYTES))) unless already defined by the architecture (see include/linux/cache.h). I believe using this on a (whole) structure only makes sense with statically allocated structures and instructs the compiler to allocate the structure at an aligned address. My guess is that it has no effect on dynamically allocated structures (such as struct ismt_priv). If you were thinking about using ____cacheline_aligned just on the dma_buffer field, then it would align the field with respect to the beginning of the structure (i.e. insert the required padding), but the field would still be misaligned if the structure itself is misaligned. > @@ -320,7 +320,7 @@ static int ismt_process_desc(const struct > > ismt_desc *desc, > > struct ismt_priv *priv, int size, > > char read_write) > > { > > - u8 *dma_buffer = priv->dma_buffer; > > + u8 *dma_buffer = (u8*)ALIGN((unsigned long)&priv->buffer[0], 16); > > The fixed value here might not work on all (future?) generations? Good question. In theory this alignment should not be necessary at all. There is no such requirement in the public datasheet, which is probably why it was not implemented in the driver in the first place. This is related to some hardware errata and the problem probably occurs only with some specific hardware revisions. Hard to say what happens with future generations. But at least it should not make any difference to the other (existing) generations. In any case, it's probably better to use a macro (e.g. #define ISMT_DMA_ALIGN 16) rather than hardcoding the "16" value. Radu