Re: [PATCH v2 01/27] iio: core: Rework claim and release of direct mode to work with sparse.

["Nuno Sá" <noname.nuno@gmail.com>]

On Sun, 2025-02-09 at 18:05 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Initial thought was to do something similar to __cond_lock()
> 
> 	do_iio_device_claim_direct_mode(iio_dev) ? : ({ __acquire(iio_dev);
> 0; })
> + Appropriate static inline iio_device_release_direct_mode()
> 
> However with that, sparse generates false positives. E.g.
> 
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c:1811:17: warning: context
> imbalance in 'st_lsm6dsx_read_raw' - unexpected unlock
> 
> So instead, this patch rethinks the return type and makes it more
> 'conditional lock like' (which is part of what is going on under the hood
> anyway) and return a boolean - true for successfully acquired, false for
> did not acquire.
> 
> To allow a migration path given the rework is now non trivial, take a leaf
> out of the naming of the conditional guard we currently have for IIO
> device direct mode and drop the _mode postfix from the new functions giving
> iio_device_claim_direct() and iio_device_release_direct()
> 
> Whilst the kernel supports __cond_acquires() upstream sparse does not
> yet do so.  Hence rely on sparse expanding a static inline wrapper
> to explicitly see whether __acquire() is called.
> 
> Note that even with the solution here, sparse sometimes gives false
> positives. However in the few cases seen they were complex code
> structures that benefited from simplification anyway.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v2: include linux/compiler_types.h (David)

UhU, I'm not seeing it?

> ---

With the above,

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  include/linux/iio/iio.h | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 56161e02f002..fe33835b19cf 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -662,6 +662,31 @@ int iio_push_event(struct iio_dev *indio_dev, u64
> ev_code, s64 timestamp);
>  int iio_device_claim_direct_mode(struct iio_dev *indio_dev);
>  void iio_device_release_direct_mode(struct iio_dev *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 __acquire() marking. Revisit when sparse supports
> + * __cond_acquires()
> + */
> +static inline bool iio_device_claim_direct(struct iio_dev *indio_dev)
> +{
> +	int ret = iio_device_claim_direct_mode(indio_dev);
> +
> +	if (ret)
> +		return false;
> +
> +	__acquire(iio_dev);
> +
> +	return true;
> +}
> +
> +static inline void iio_device_release_direct(struct iio_dev *indio_dev)
> +{
> +	iio_device_release_direct_mode(indio_dev);
> +	__release(indio_dev);
> +}
> +
>  /*
>   * This autocleanup logic is normally used via
>   * iio_device_claim_direct_scoped().