Re: [PATCH v11 4/6] iio: adc: ad4691: add SPI offload support

[Jonathan Cameron]

On Mon, 18 May 2026 21:26:09 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> 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@xxxxxxxxxxxx>
> > >> 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