Re: [PATCH] iio: adc: ti-ads1119: Prevent concurrent access during

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 21 Apr 2025 11:42:36 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> ...
> > For debug access it 'depends'....
> > 
> > Some cases are easy reasons to add this protection
> > 1) Accesses can't happen at all because to talk to the device and read
> >    registers etc requires some mode change (e.g. recent drivers that
> >    can only access config registers when the bus is operating in particular
> >    modes, or where we have to use a slower SPI bus rate).
> > 2) There are register banks involved. So a single write can leave the
> >    driver talking to the wrong registers...
> > 3) Driver uses multipart reads / writes (similar to register banks)
> >    In most cases this is a device specific thing and should use a local
> >    lock to serialize accesses.  On occasion that is too complex to make
> >    work with debug so we restrict debugfs access in general.
> > 
> > Other cases are less obvious.
> > 1) Bus traffic in general might slow down a transfer and break things
> >    because of timing. I.e. missed samples.  That can happen for all sorts
> >    of reasons anyway so should only be a momentary problem.
> > 2) They might changes settings.  
> 
> Does the case of configuration/setting change apply to IIO device properties?
> Let's say for example a device starts running a buffered capture with some
> certain input gain configuration and goes filling IIO buffer with the data
> from the device. At some point while the buffered capture is running, the
> user uses debugfs to change the gain configuration. The _scale attribute
> will not correctly convert all buffer data to mV units. Would cases like that
> be something to prevent? Or we consider those to be fine since a debug feature
> was used?
Fine.  This isn't really any different to them changing the scale when using
non buffered capture.

The debug interfaces in general are optional and I've always wondered if we should
have gated them more strongly.  Maybe a CONFIG_IIO_DRIVER_DEBUG or similar
to hide them all on production systems and remove the footguns.

Jonathan
 
> 
> Thanks,
> Marcelo
> 
> > 
> > These less obvious things are a case of thinking it's a debug
> > access. We tend to not prevent actual deliberate state changes as that's
> > someone shooting them selves in the foot and they get what they deserve.
> > 
> > So what is the case we are protecting against here?  The description
> > definitely needs more information to justify this patch.
> > 
> > Jonathan
> >   
> > >   
> > 
> >