Devicetree
 help / color / mirror / Atom feed
From: David Lechner <dlechner@baylibre.com>
To: Kurt Borja <kuurtb@gmail.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Linus Walleij <linusw@kernel.org>,
	Bartosz Golaszewski <brgl@kernel.org>
Cc: "Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org
Subject: Re: [PATCH v3 7/9] iio: adc: ti-ads1262: support triggered buffer sampling
Date: Mon, 10 Aug 2026 11:31:21 -0500	[thread overview]
Message-ID: <f3ea3859-aff1-4c15-9302-095119faec78@baylibre.com> (raw)
In-Reply-To: <DKK9S36LB543.2T124NE3TNETN@gmail.com>

On 8/9/26 3:28 AM, Kurt Borja wrote:
> On Sat Aug 8, 2026 at 1:39 PM -05, David Lechner wrote:
>> On 8/7/26 10:58 PM, Kurt Borja wrote:
>>> Add triggered buffer support and a data-ready (DRDY) hardware trigger.
>>>

...

>>
>>> +	u8 tx[11] __aligned(IIO_DMA_MINALIGN);
>>> +	u8 rx[11] __aligned(IIO_DMA_MINALIGN);
>>
>> don't need second one to be aligned, they aren't independent.
> 
> Ah, I forgot this observation in the last version. These are used in a
> full-duplex transfer, wouldn't that require for both to be on its own
> cache line? I just started learning about DMA.

No. In this case, it works roughly like this...

- We fill the TX buffer before the transfer. (CPU access to memory may
  just live in the cache at this point and not actually be sent to RAM)
- We request to start the SPI transfer.
- The core SPI code flushes (or maybe I should say invalidates) the cache
  on the TX buffer. This ensures that what we wrote with the CPU available
  to DMA.
- The actual SPI transfer happens that uses DMA to access both TX and
  RX buffers. (Again, could just live in a cache and not be sent to RAM)
- The SPI core code flushes the cache on the RX buffer. This ensures
  that when the CPU reads the memory, it will see what the DMA just
  wrote.

Since there isn't a time when CPU and DMA both write to the cache line
at the same time before a flush, there is never a time we could have
an issue with stale data replacing data that had not been flushed.

It does mean that we can't update the tx buffer for the next message
until after this message is done, but we have to do that anyway.

What does cause problems is if we just had a regular unrelated variable
after this in the cache line and the driver updated it during a SPI
transfer. If this new value was just living in the CPU cache, then
when the RX flush happened, it would write over that new value with
stale data from the DMA's version of the cache.


> 
> [...]
> 
>>> +static int ads1262_enable_and_read_last(struct ads1262 *st,
>>> +					const struct iio_chan_spec *spec,
>>> +					__be32 *val)
>>> +{
>>> +	struct ads1262_channel *chan;
>>> +	int ret;
>>> +
>>> +	lockdep_assert_held(&st->xfer_lock);
>>
>> What happens if something else (e.g. gpio in the future) decides to do a
>> register write here. If it wins the race, will it unintentionally read the
>> data? So do we also need to read the stored data via command here too?
> 
> On each trigger, we are holding the lock before we enable the first
> channel, until after we read the final conversion. So we don't really
> care if there's concurrent activity in-between triggers. Am I missing
> something?
> 
The datasheet doesn't seem clear on it, so I don't know if you are
missing something or not, that is why I asked. :-)

It looks to me like the way this is working is that that when data
is ready, on the very next SPI transfer, the data is placed in the
RX buffer no matter what is in the TX buffer. So I was imagining that
if we got a DRDY interrupt, but someone happened to set a gpio output
at the same time and the gpio request won the race for the lock, then
when the gpio request was made, it would receive the RX data and trigger
the next conversion while setting the gpio output, then then what was
supposed to be receiving the data would receive I don't know what.



  reply	other threads:[~2026-08-10 16:31 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-08  3:58 [PATCH v3 0/9] iio: adc: Add TI ADS126X ADC family support Kurt Borja
2026-08-08  3:58 ` [PATCH v3 1/9] dt-bindings: iio: adc: support the TI ADS126x ADC family Kurt Borja
2026-08-08  4:08   ` sashiko-bot
2026-08-08 18:38   ` David Lechner
2026-08-09  8:26     ` Kurt Borja
2026-08-10 16:42       ` David Lechner
2026-08-10  8:46   ` Bartosz Golaszewski
2026-08-08  3:58 ` [PATCH v3 2/9] iio: adc: add the ti-ads1262 driver Kurt Borja
2026-08-08  4:11   ` sashiko-bot
2026-08-08 18:39   ` David Lechner
2026-08-09  8:26     ` Kurt Borja
2026-08-10 16:42       ` David Lechner
2026-08-10 18:48       ` Andy Shevchenko
2026-08-08 22:28   ` Uwe Kleine-König
2026-08-09 16:24     ` Kurt Borja
2026-08-08  3:58 ` [PATCH v3 3/9] iio: adc: ti-ads1262: support per-channel sampling frequency Kurt Borja
2026-08-08  4:11   ` sashiko-bot
2026-08-08 18:39   ` David Lechner
2026-08-09  8:27     ` Kurt Borja
2026-08-08  3:58 ` [PATCH v3 4/9] iio: adc: ti-ads1262: support per-channel reference and gain Kurt Borja
2026-08-08 18:39   ` David Lechner
2026-08-09  8:28     ` Kurt Borja
2026-08-08  3:58 ` [PATCH v3 5/9] iio: adc: ti-ads1262: support input chopping Kurt Borja
2026-08-08 18:39   ` David Lechner
2026-08-08  3:58 ` [PATCH v3 6/9] iio: adc: ti-ads1262: support excitation currents Kurt Borja
2026-08-08  4:13   ` sashiko-bot
2026-08-08 18:39   ` David Lechner
2026-08-08  3:58 ` [PATCH v3 7/9] iio: adc: ti-ads1262: support triggered buffer sampling Kurt Borja
2026-08-08  4:09   ` sashiko-bot
2026-08-08 18:39   ` David Lechner
2026-08-09  8:28     ` Kurt Borja
2026-08-10 16:31       ` David Lechner [this message]
2026-08-08  3:58 ` [PATCH v3 8/9] iio: adc: ti-ads1262: support REFOUT and VBIAS regulators Kurt Borja
2026-08-08  4:11   ` sashiko-bot
2026-08-08 18:40   ` David Lechner
2026-08-09  8:28     ` Kurt Borja
2026-08-08  3:58 ` [PATCH v3 9/9] iio: adc: ti-ads1262: support common mode supplies Kurt Borja
2026-08-08 18:40   ` David Lechner
2026-08-09  8:29     ` Kurt Borja
2026-08-08 18:37 ` [PATCH v3 0/9] iio: adc: Add TI ADS126X ADC family support David Lechner
2026-08-09  8:29   ` Kurt Borja
2026-08-10 16:42     ` David Lechner

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=f3ea3859-aff1-4c15-9302-095119faec78@baylibre.com \
    --to=dlechner@baylibre.com \
    --cc=andy@kernel.org \
    --cc=brgl@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=kuurtb@gmail.com \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    /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