Re: [1/2] i2c: ismt: 16-byte align the DMA buffer address

[Radu Rendec <radu.rendec@gmail.com>]

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@gmail.com>
> 
> 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