From: Jonathan Cameron <jic23@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: rodrigo.alencar@analog.com, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org,
Stefan Popa <stefan.popa@analog.com>,
Jonathan Cameron <jic23@cam.ac.uk>,
Greg Kroah-Hartman <gregkh@suse.de>,
Michael Auchter <michael.auchter@ni.com>,
Lars-Peter Clausen <lars@metafoo.de>,
Michael Hennerich <Michael.Hennerich@analog.com>,
David Lechner <dlechner@baylibre.com>,
Andy Shevchenko <andy@kernel.org>,
Rodrigo Alencar <rdealencar@gmail.com>,
Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH v4 05/13] iio: dac: ad5686: fix overlapping DMA buffers in I2C read
Date: Wed, 29 Apr 2026 18:38:44 +0100 [thread overview]
Message-ID: <20260429183844.1744098a@jic23-huawei> (raw)
In-Reply-To: <afIQjU2TGrZ6P-3w@ashevche-desk.local>
On Wed, 29 Apr 2026 17:07:09 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> 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@analog.com/
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
next prev parent reply other threads:[~2026-04-29 17:38 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-29 13:07 [PATCH v4 00/13] Fixes and cleanups for the AD5686 IIO driver Rodrigo Alencar via B4 Relay
2026-04-29 13:07 ` [PATCH v4 01/13] iio: dac: ad5686: fix ref bit initialization for single-channel parts Rodrigo Alencar via B4 Relay
2026-04-29 13:07 ` [PATCH v4 02/13] iio: dac: ad5686: fix input raw value check Rodrigo Alencar via B4 Relay
2026-04-29 13:07 ` [PATCH v4 03/13] iio: dac: ad5686: acquire lock when doing powerdown control Rodrigo Alencar via B4 Relay
2026-04-29 13:07 ` [PATCH v4 04/13] iio: dac: ad5686: fix powerdown control on dual-channel devices Rodrigo Alencar via B4 Relay
2026-04-29 14:03 ` Andy Shevchenko
2026-04-29 14:21 ` Rodrigo Alencar
2026-04-29 13:07 ` [PATCH v4 05/13] iio: dac: ad5686: fix overlapping DMA buffers in I2C read Rodrigo Alencar via B4 Relay
2026-04-29 13:12 ` Rodrigo Alencar
2026-04-29 13:50 ` Andy Shevchenko
2026-04-29 17:11 ` Jonathan Cameron
2026-04-29 14:07 ` Andy Shevchenko
2026-04-29 17:38 ` Jonathan Cameron [this message]
2026-04-29 13:07 ` [PATCH v4 06/13] iio: dac: ad5686: refactor include headers Rodrigo Alencar via B4 Relay
2026-04-29 14:09 ` Andy Shevchenko
2026-04-29 14:38 ` Joshua Crofts
2026-04-29 17:41 ` Jonathan Cameron
2026-04-29 18:19 ` Andy Shevchenko
2026-04-30 9:18 ` Joshua Crofts
2026-04-30 11:47 ` Andy Shevchenko
2026-05-05 10:55 ` Jonathan Cameron
2026-05-05 12:46 ` Andy Shevchenko
2026-04-29 13:07 ` [PATCH v4 07/13] iio: dac: ad5686: remove redundant register definition Rodrigo Alencar via B4 Relay
2026-04-29 13:07 ` [PATCH v4 08/13] iio: dac: ad5686: drop enum id Rodrigo Alencar via B4 Relay
2026-04-29 13:07 ` [PATCH v4 09/13] iio: dac: ad5686: add of_match table to the spi driver Rodrigo Alencar via B4 Relay
2026-04-29 13:07 ` [PATCH v4 10/13] iio: dac: ad5686: remove powerdown mask magic number Rodrigo Alencar via B4 Relay
2026-04-29 14:19 ` Andy Shevchenko
2026-04-29 13:07 ` [PATCH v4 11/13] iio: dac: ad5686: add control_sync() for single-channel devices Rodrigo Alencar via B4 Relay
2026-04-29 13:07 ` [PATCH v4 12/13] iio: dac: ad5686: cleanup doc header of local structs Rodrigo Alencar via B4 Relay
2026-04-29 13:07 ` [PATCH v4 13/13] iio: dac: ad5686: create bus ops struct Rodrigo Alencar via B4 Relay
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260429183844.1744098a@jic23-huawei \
--to=jic23@kernel.org \
--cc=Michael.Hennerich@analog.com \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=broonie@kernel.org \
--cc=dlechner@baylibre.com \
--cc=gregkh@suse.de \
--cc=jic23@cam.ac.uk \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michael.auchter@ni.com \
--cc=rdealencar@gmail.com \
--cc=rodrigo.alencar@analog.com \
--cc=stefan.popa@analog.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox