On Sun, 2025-02-09 at 18:05 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> ---
> v2: include linux/compiler_types.h (David)
UhU, I'm not seeing it?
> ---
With the above,
Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> 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().
