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

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 22 Apr 2025 15:04:25 -0300
Nattan Ferreira <nattanferreira58@gmail.com> wrote:

> On Sun, Apr 20, 2025 at 6:17 PM Francesco Dolcini <francesco@dolcini.it> wrote:
> >
> > On Sun, Apr 20, 2025 at 01:07:28PM -0300, João Paulo Gonçalves wrote:  
> > > > Use iio_device_claim_direct() to protect register access via debugfs
> > > > from conflicting with buffered capture modes. This prevents data
> > > > corruption and ensures correct device operation when users access
> > > > registers while streaming data.
> > > >  
> > >
> > > but debugfs is meant to be used during development/integration,
> > > where this probably is not an issue.  
> >
> > Is even worth doing any such a change? I assume Jonathan will have an
> > opinion on what's the expectation for an IIO driver.
> >
> > Nattan, can you explain why you need such a change? What is the use
> > case?
> >
> > Francesco
> >  
> 
> Hi Francesco and João,
> 
> Thanks for the feedback and your thoughts on this.
> 
> To clarify the use case: The primary reason for preventing both read
> and write access when the buffer is active is to avoid potential data
> corruption and inconsistencies. When the buffer is streaming data,
> concurrent register access (either reading or writing) can lead to
> unexpected behaviors, such as incorrect register values or misaligned
> data. This can cause confusion and make it harder to debug or test the
> driver, as well as introduce risks in production if these issues
> persist unnoticed.
> 
> I understand that development and integration are critical stages, and
> while it's tempting to allow register access for diagnostic purposes,
> my concern is that even during this phase, uncoordinated access to
> registers could result in misleading data, which could cascade into
> errors or incorrect assumptions.
> 
> With that said, I do see the merit in João’s suggestion to allow read
> access while blocking writes. If that aligns better with the IIO
> driver expectations, I’m happy to update the patch accordingly.
> 
> Let me know your thoughts or if you think there’s a better approach here.

My personal view is that these debug interfaces provide a terrible way
to shoot yourself in the foot whenever we enable them.   As such I'm not
really convinced that we need protections.   Limiting write access is
probably fine but it also an ABI change. Technically someone might be using
that interface in their userspace code and see breakage at which point we
get into  the silly situation of having to revert this hardening...

I'd be interested in a proposal to lock these down in general via a config
parameter or new drivers restricting things as you have done here.

Jonathan

> 
> Best regards,
> Nattan Ferreira