On Thu, 2018-01-04 at 08:46 -0500, Neil Horman wrote: > On Fri, Aug 18, 2017 at 05:01:27PM +0100, Radu Rendec wrote: > > Use only a portion of the data buffer for DMA transfers, which is always > > 16-byte aligned. This makes the DMA buffer address 16-byte aligned and > > compensates for spurious hardware parity errors that may appear when the > > DMA buffer address is not 16-byte aligned. > > > > The data buffer is enlarged in order to accommodate any possible 16-byte > > alignment offset and changes the DMA code to only use a portion of the > > data buffer, which is 16-byte aligned. > > > > The symptom of the hardware issue is the same as the one addressed in > > v3.12-rc2-5-gbf41691 and manifests by transfers failing with EIO, with > > bit 9 being set in the ERRSTS register. > > > > Signed-off-by: Radu Rendec <radu.rendec@xxxxxxxxx> > > Why not just use the alligned attribute here for buffer? > > https://gcc.gnu.org/onlinedocs/gcc-3.3/gcc/Type-Attributes.html > > That would save you over allocation when possible and keep you from needing to > create a private aligned variable. First of all, thanks for reviewing this! I believe the aligned() attribute cannot be used here because the whole ismt_priv structure is dynamically allocated (the allocation is done at the beginning of the ismt_probe() function). Even with a statically allocated structure, aligning the whole structure does not guarantee the alignment of the dma_buffer field, because the field offset within the structure can change (e.g. if other fields are added, removed, moved around etc.). On the other hand, I realized I could have used the PTR_ALIGN macro to calculate the aligned address. That would probably make the code less ugly and more intuitive. So I would at least resubmit the patch using PTR_ALIGN instead of ALIGN and explicit casts - if you agree to the current solution, of course. Thanks, Radu
