On Jan 14, 2017 10:38, Jonathan Cameron wrote:
> As this has device tree bindings it should have gone to linux-devicetree,
> Rob and Mark (maintainers of bindings).
I will add them to CC on the new version.
> Spi buffers for spi_write need to be cacheline aligned. See below
> for roughly why.
I can recall that from my previous submissions... I made the same mistake.
Thank you once again for explaining that. I hope this will imprint in my mind
from now on.
Thank you!
> Jonathan
> > ---
> > +static int max5481_write_cmd(struct spi_device *spi, u8 cmd, u16 val)
> > +{
> > + /* SPI Format from MAX5481-MAX5484 (19-3708; Rev 5; 4/10) pg 15 */
> > + u8 msg[3];
> It's clearly one of those days - same issue in two drivers in a row. :(
> Still I can refine me response ;)
>
> There are requirements for buffers passed directly to spi_read / spi_write.
> They get passed to spi_sync which calls into the spi master drivers.
> SPI master drivers are explicitly allowed to directly use this buffer
> in dma. On most modern platforms it is fine to do DMA from any location...
>
> However, cacheline corruption comes in here. There is no guarantee that
> the SPI controller won't write back to this address, as it it will be
> bypassing the processor whilst doing this, that can result in a difference
> in other parts of the cacheline between what is in the cache and what is
> in main memory. At the end of the dma transfer any changes elsewhere in
> the cacheline can be wiped out as result. (or something like that ;)
>
> Anyhow, two solutions. Either allocate the memory in it's own cacheline
> which will naturally happen if you allocate on the heap using kmalloc
> or use the fact we carefully align the iio_priv memory to be cacheline
> aligned. This means that if you stick a __cacheline_aligned buffer at the
> end of your iio_priv structure it was also be in it's own cacheline.
>
> Not doing this is the source of really hard to track down bugs!
> > +
> > + msg[0] = cmd;
> > +
> > + switch (cmd) {
> > + case MAX5481_WRITE_WIPER:
> > + msg[1] = val >> 2;
> > + msg[2] = (val & 0x3) << 6;
> > + return spi_write(spi, msg, ARRAY_SIZE(msg));
> > +
> > + case MAX5481_COPY_AB_TO_NV:
> > + case MAX5481_COPY_NV_TO_AB:
> > + return spi_write(spi, msg, sizeof(u8));
> > +
> > + default:
> > + return -EIO;
> > + }
> > +}
--
Slawomir Stepien
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
