Linux GPIO subsystem development
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: "David Lechner" <dlechner@baylibre.com>,
	"Sabau, Radu bogdan" <Radu.Sabau@analog.com>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Hennerich, Michael" <Michael.Hennerich@analog.com>,
	"Sa, Nuno" <Nuno.Sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Uwe Kleine-König" <ukleinek@kernel.org>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Linus Walleij" <linusw@kernel.org>,
	"Bartosz Golaszewski" <brgl@kernel.org>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pwm@vger.kernel.org" <linux-pwm@vger.kernel.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Subject: Re: [PATCH v11 4/6] iio: adc: ad4691: add SPI offload support
Date: Wed, 20 May 2026 11:36:01 +0100	[thread overview]
Message-ID: <20260520113601.2f13b9f0@jic23-huawei> (raw)
In-Reply-To: <agtZwbeVeZdnlXTI@ashevche-desk.local>

On Mon, 18 May 2026 21:26:09 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, May 18, 2026 at 10:16:38AM -0500, David Lechner wrote:
> > On 5/18/26 10:14 AM, Sabau, Radu bogdan wrote:  
> > >> -----Original Message-----
> > >> From: David Lechner <dlechner@baylibre.com>
> > >> Sent: Saturday, May 16, 2026 8:53 PM  
> 
> ...
> 
> > >>> +	if (st->manual_mode && st->offload)
> > >>> +		return sysfs_emit(buf, "%llu\n", READ_ONCE(st->offload-
> > >>> trigger_hz));  
> > >>
> > >> Why do we need READ_ONCE?  
> > > 
> > > trigger_hz is u64 and if the target is 32-bit, a 64-bit access compiles to two 32-bit
> > > instructions, so show() reading it without a lock and store() writing it concurrently
> > > can produce a torn value at the compiler level. READ_ONCE/WRITE_ONCE suppress
> > > the compiler transformations that would allow that splitting or caching. We could
> > > have st->lock in show() instead, but that felt heavier than necessary for a single
> > > scalar where a transiently stale-but-whole read is fine.  
> > 
> > I would go with the mutex. It will be easier for people to understand.  
> 
> But why? READ_ONCE() here is exactly enough. We do not care about
> serialisation, we care only about integrity. With mutex it will confuse
> (some) people more, e.g., me. Because in that case I would think about
> some specific access to it that may happen. Yes, I saw many times the show
> functions that do mutex and then print the result when mutex is not held
> anymore, but for simple cases like here, mutex is overkill. Interestingly
> that using guard()() inside show makes the mentioned functions to print
> (almost) latest value of the variable in question. It narrows window down
> as printing will go inside critical section.
> 

I think it's worth noting that we are very lax in IIO wrt to READ_ONCE()
usage.  It might be worth starting to tighten that up for state variable reads
etc whether they are 64 bit or not (that just increases the chances).
In theory compilers can do far too many evil things.  I've been scared
of pushing this because of the massive number of incorrect instances
(and the bad example I set with early drivers :(), but it would be good
to have a few examples in tree so we can start to encourage people to
do that stuff right.

Jonathan

  reply	other threads:[~2026-05-20 10:36 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15 13:31 [PATCH v11 0/6] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
2026-05-15 13:31 ` [PATCH v11 1/6] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
2026-05-15 13:31 ` [PATCH v11 2/6] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
2026-05-16 17:11   ` David Lechner
2026-05-17 11:50     ` Jonathan Cameron
2026-05-17 12:14     ` Jonathan Cameron
2026-05-18 14:59     ` Sabau, Radu bogdan
2026-05-18 15:05       ` David Lechner
2026-05-17 11:52   ` Jonathan Cameron
2026-05-18 15:05     ` Sabau, Radu bogdan
2026-05-17 12:19   ` Jonathan Cameron
2026-05-15 13:31 ` [PATCH v11 3/6] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
2026-05-16 17:32   ` David Lechner
2026-05-17 12:25     ` Jonathan Cameron
2026-05-17 19:21       ` David Lechner
2026-05-18 14:21         ` Jonathan Cameron
2026-05-18 14:36           ` David Lechner
2026-05-18 15:25             ` Jonathan Cameron
2026-05-16 18:48   ` Jonathan Cameron
2026-05-15 13:31 ` [PATCH v11 4/6] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
2026-05-16 17:53   ` David Lechner
2026-05-18 15:14     ` Sabau, Radu bogdan
2026-05-18 15:16       ` David Lechner
2026-05-18 18:26         ` Andy Shevchenko
2026-05-20 10:36           ` Jonathan Cameron [this message]
2026-05-15 13:31 ` [PATCH v11 5/6] iio: adc: ad4691: add oversampling support Radu Sabau via B4 Relay
2026-05-16 18:10   ` David Lechner
2026-05-16 18:55   ` Jonathan Cameron
2026-05-15 13:31 ` [PATCH v11 6/6] docs: iio: adc: ad4691: add driver documentation Radu Sabau via B4 Relay
2026-05-16 18:18   ` David Lechner
2026-05-17 12:32     ` Jonathan Cameron

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=20260520113601.2f13b9f0@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Michael.Hennerich@analog.com \
    --cc=Nuno.Sa@analog.com \
    --cc=Radu.Sabau@analog.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=brgl@kernel.org \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=lgirdwood@gmail.com \
    --cc=linusw@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=robh@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=ukleinek@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