Re: [PATCH v5 06/15] iio: adc: ad4134: Protect conventional device access from debug accesses

[Marcelo Schmitt]

On 10/17, Jonathan Cameron wrote:
> On Fri, 16 Oct 2026 22:58:05 -0300
> Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx> wrote:
> 
> > Require debug access to acquire device synchronization locks before
> > allowing any transfer from the debug interface so to avoid disrupting
> > operations issued from usual (non-debug) interfaces.
> 
> These needs more.  What sort of disruption are we talking?
> 
> A deliberate write to change state that breaks normal operation
> is a debug mess up that we don't guard against, but if a read
> is enough to break things, or there is an inherent disruption
> of other operations by using the bus at all then state that.

A read is enough to break any ongoing buffered read because register reads will
cause the bus configuration to change to a state that invalidates any sample read
that happens afterwards. 

> 
> What isn't regmap locking covering?  

regmap doens't cover IIO device direct/buffered modes.

> 
> If this is all about the mux stuff and backend usage,
> then move it to nearer that in the series and refer to that.

We also need the mux usage to be synchronized which is why debugfs_reg_access
now also locks on device state sync_lock.

In practice, because every device access will select then deselect the mux state,
we are not able to have two operations going on simultaneously (not even two
register read can run concurrently due to the needed mux state care).

On v3, there was a dedicated mutex for mux access synchronization. In an atempt
to make things more simple, that second mutex was removed and sync_lock is now
also utilized to prevent multiple threads from changing the mux state simultaneously. 
It shall work properly as long as any device access first
claims IIO direct + get sync_lock or claims IIO buffered mode.
Is it okay to have it like that?

> 
> Jonathan
> 
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx>
> > ---
> > New patch in v5.
> > 
> > I'm not sure about this one and have no strong opinion about it.
> > Providing it according to suggestion from Nuno.
> > https://lore.kernel.org/linux-iio/179199692826.658576.4841754843069240356.b4-review@b4/
> > From previous discussions I guess this patch might not be welcome.
> > https://lore.kernel.org/linux-iio/20250421142359.11fb483f@jic23-huawei/
> > In that case, completely fine with it being dropped.
> > 
> >  drivers/iio/adc/ad4134.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad4134.c b/drivers/iio/adc/ad4134.c
> > index dc4d801490db..42a1272c2894 100644
> > --- a/drivers/iio/adc/ad4134.c
> > +++ b/drivers/iio/adc/ad4134.c
> > @@ -302,6 +302,12 @@ static int ad4134_debugfs_reg_access(struct iio_dev *indio_dev,
> >  {
> >  	struct ad4134_state *st = iio_priv(indio_dev);
> >  
> > +	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> > +	if (IIO_DEV_ACQUIRE_FAILED(claim))
> > +		return -EBUSY;
> > +
> > +	guard(mutex)(&st->sync_lock);
> > +
> >  	if (readval)
> >  		return regmap_read(st->regmap, reg, readval);
> >  
>