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

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko