Re: [PATCH v4 05/13] iio: dac: ad5686: fix overlapping DMA buffers in I2C read
From: Jonathan Cameron
Date: Wed Apr 29 2026 - 13:39:39 EST
On Wed, 29 Apr 2026 17:07:09 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
> On Wed, Apr 29, 2026 at 02:07:35PM +0100, Rodrigo Alencar via B4 Relay wrote:
> >
> > The TX and RX buffers in ad5686_i2c_read() both reference data[0], causing
> > byte d8[1] to be shared between the TX buffer and the RX buffer. I2C
> > controller drivers that map all message buffers for DMA before initiating
> > the hardware transaction will map overlapping memory ranges with
> > conflicting DMA directions (DMA_TO_DEVICE and DMA_FROM_DEVICE). This issue
> > was reported by sashiko.
>
> Not sure how this patch helps with that. The minimum granularity for DMA is
> a cache line size. Do we have data[0] and data[1] be cache line separated?
>
> It might be I miss something obvious, but this topic is always confusing
> to me (DMA alignment).
>
I thought I understood :( From a real hardware point of view the cache flushes
that occur for non coherent DMA should be safe with tx and rx overlap but
'maybe' there is a framework problem if they buffer is simultaneously marked
in both directions or indeed there might be spi controller drivers that
can't cope with this.
Flush wise need to flush the tx and rx at start to ensure the tx data
is written back and any copies of rx in the CPU caches are clean.
Then after DMA need to flush rx again to ensure an prefetched cachelines
(which will be clean because of earlier flush) are dropped and new
reads come from the device. These will all happen on a cacheline but
shouldn't matter if it's the same one as the post DMA flushes are just
to drop clean lines and it's clean either way.
+CC Mark for whether this corner has come up in SPI before.
Short question is can we pass same buffer for rx and tx on an spi transfer?
https://lore.kernel.org/all/20260429-ad5686-fixes-v4-5-bb8f1cbd68e1@xxxxxxxxxx/
Is the patch, based on a bug report from an LLM.
hmm. In the docs for struct spi_transfer we have a not entirely confident
sounding comment on this.
/*
* It's okay if tx_buf == rx_buf (right?).
* For MicroWire, one buffer must be NULL.
* Buffers must work with dma_*map_single() calls.
*/
We don't care about MicroWire here, but the other two lines matter.
This particular case is even weirder as I think they are offset and overlapping
but if it works for them equal I'd assume that's fine too.
Jonathan