Re: [PATCH v4 2/9] iio: core: annotate the conditional mode claims

[Jonathan Cameron]

On Wed, 28 Oct 2026 03:18:34 +0000
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

> On Tue, 27 Oct 2026 14:49:47 +0200
> Rupesh Majhi <zoone.rupert@xxxxxxxxx> wrote:
> 
> > iio_device_claim_direct() and iio_device_try_claim_buffer_mode() hold
> > the mode lock only when they return true, but carry no annotation. With
> > context analysis on, each caller that releases the claim is reported
> > for releasing a lock it never took.
> > 
> > Mark both __cond_acquires(true, indio_dev), the token that
> > __iio_dev_mode_lock() acquires. Sparse no longer reads these
> > annotations, so drop the comment about it.  
> Hi Rupesh,
> 
> The history of the definitions of these being in the header at
> all was to make them visible enough to sparse that it could
> figure out the locking and 'mostly' give good coverage.
> 
> Given that's no longer necessary, maybe it makes sense to make
> the implementations private. It doesn't really help us that
> much as those nasty helpers are still needed for the
> rarely used guard that claims the current mode.
> 
> So maybe it's worth this maybe not...  Anyhow, perhaps one for
> another day given I don't think it helps this effort.
> 

> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index e9a5d1d905c1..04c194bdda87 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -2269,6 +2269,33 @@ void __iio_dev_mode_unlock(struct iio_dev *indio_dev)
>  }
>  EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock);
>  
> +bool iio_device_claim_direct(struct iio_dev *indio_dev
> +{
> +       __iio_dev_mode_lock(indio_dev);
> +
> +       if (iio_buffer_enabled(indio_dev)) {
> +               __iio_dev_mode_unlock(indio_dev);
> +               return false;
> +       }
> +
> +       return true;
> +}
> +EXPORT_SYMBOL_GPL(iio_device_claim_direct);
> +
> +bool iio_device_try_claim_buffer_mode(struct iio_dev *indio_dev)
> +       __cond_acquires(true, indio_dev)

Dropping that marking. Turns out I even failed to build test this...


> +{
> +       __iio_dev_mode_lock(indio_dev);
> +
> +       if (!iio_buffer_enabled(indio_dev)) {
> +               __iio_dev_mode_unlock(indio_dev);
> +               return false;
> +       }
> +
> +       return true;
> +}
> +EXPORT_SYMBOL_GPL(iio_device_try_claim_buffer_mode);
> +
>  /**
>   * iio_device_get_current_mode() - helper function providing read-only access to
>   *                                the opaque @currentmode variable
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index ca91f5a64626..b9e4888e18bc 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -718,18 +718,7 @@ void __iio_dev_mode_unlock(struct iio_dev *indio_dev) __releases(indio_dev);
>   *
>   * Returns: true on success, false on failure.
>   */
> -static inline bool iio_device_claim_direct(struct iio_dev *indio_dev)
> -       __cond_acquires(true, indio_dev)
> -{
> -       __iio_dev_mode_lock(indio_dev);
> -
> -       if (iio_buffer_enabled(indio_dev)) {
> -               __iio_dev_mode_unlock(indio_dev);
> -               return false;
> -       }
> -
> -       return true;
> -}
> +bool iio_device_claim_direct(struct iio_dev *indio_dev) __cond_acquires(true, indio_dev);
>  
>  /**
>   * iio_device_release_direct() - Releases claim on direct mode
> @@ -753,18 +742,8 @@ static inline bool iio_device_claim_direct(struct iio_dev *indio_dev)
>   *
>   * Returns: true on success, false on failure.
>   */
> -static inline bool iio_device_try_claim_buffer_mode(struct iio_dev *indio_dev)
> -       __cond_acquires(true, indio_dev)
> -{
> -       __iio_dev_mode_lock(indio_dev);
> -
> -       if (!iio_buffer_enabled(indio_dev)) {
> -               __iio_dev_mode_unlock(indio_dev);
> -               return false;
> -       }
> -
> -       return true;
> -}
> +bool iio_device_try_claim_buffer_mode(struct iio_dev *indio_dev)
> +  __cond_acquires(true, indio_dev);
>  
>  /**
>   * iio_device_release_buffer_mode() - releases claim on buffer mode
> 
> 
> > 
> > Signed-off-by: Rupesh Majhi <zoone.rupert@xxxxxxxxx>
> > ---
> > Cc: Marco Elver <elver@xxxxxxxxxx>
> > Cc: Bart Van Assche <bvanassche@xxxxxxx>
> > 
> >  include/linux/iio/iio.h | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index 9b63785d5aec..ca91f5a64626 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -707,13 +707,6 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp);
> >  void __iio_dev_mode_lock(struct iio_dev *indio_dev) __acquires(indio_dev);
> >  void __iio_dev_mode_unlock(struct iio_dev *indio_dev) __releases(indio_dev);
> >  
> > -/*
> > - * Helper functions that allow claim and release of direct mode
> > - * in a fashion that doesn't generate many false positives from sparse.
> > - * Note this must remain static inline in the header so that sparse
> > - * can see the __acquires() and __releases() annotations.
> > - */
> > -
> >  /**
> >   * iio_device_claim_direct() - Keep device in direct mode
> >   * @indio_dev:	the iio_dev associated with the device
> > @@ -726,6 +719,7 @@ void __iio_dev_mode_unlock(struct iio_dev *indio_dev) __releases(indio_dev);
> >   * Returns: true on success, false on failure.
> >   */
> >  static inline bool iio_device_claim_direct(struct iio_dev *indio_dev)
> > +	__cond_acquires(true, indio_dev)
> >  {
> >  	__iio_dev_mode_lock(indio_dev);
> >  
> > @@ -760,6 +754,7 @@ static inline bool iio_device_claim_direct(struct iio_dev *indio_dev)
> >   * Returns: true on success, false on failure.
> >   */
> >  static inline bool iio_device_try_claim_buffer_mode(struct iio_dev *indio_dev)
> > +	__cond_acquires(true, indio_dev)
> >  {
> >  	__iio_dev_mode_lock(indio_dev);
> >    
> 
>