[PATCH v3 4/7] iio: core: Add cleanup.h support for iio_device_claim_*()

Kurt Borja posted 7 patches 1 month ago
Only 6 patches received!
There is a newer version of this series
[PATCH v3 4/7] iio: core: Add cleanup.h support for iio_device_claim_*()
Posted by Kurt Borja 1 month ago
Add guard classes for iio_device_claim_*() conditional locks. This will
aid drivers write safer and cleaner code when dealing with some common
patterns.

These classes are not meant to be used directly by drivers (hence the
__priv__ prefix). Instead, documented wrapper macros are provided to
enforce the use of ACQUIRE() or guard() semantics and avoid the
problematic scoped guard.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 include/linux/iio/iio.h | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index d8af0456f966..c795f731f2d8 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -10,6 +10,7 @@
 #include <linux/align.h>
 #include <linux/device.h>
 #include <linux/cdev.h>
+#include <linux/cleanup.h>
 #include <linux/compiler_types.h>
 #include <linux/minmax.h>
 #include <linux/slab.h>
@@ -740,6 +741,76 @@ static inline bool iio_device_try_claim_buffer_mode(struct iio_dev *indio_dev)
  */
 #define iio_device_release_buffer_mode(indio_dev) __iio_dev_mode_unlock(indio_dev)
 
+/*
+ * These classes are not meant to be used directly by drivers (hence the
+ * __priv__ prefix). Instead, documented wrapper macros are provided bellow to
+ * enforce the use of ACQUIRE() or guard() semantics and avoid the problematic
+ * scoped guard variants.
+ */
+DEFINE_GUARD(__priv__iio_dev_mode_lock, struct iio_dev *,
+	     __iio_dev_mode_lock(_T), __iio_dev_mode_unlock(_T));
+DEFINE_GUARD_COND(__priv__iio_dev_mode_lock, _try_direct,
+		  iio_device_claim_direct(_T));
+
+/**
+ * IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) - Tries to acquire the direct mode
+ *                                           lock with automatic release
+ * @dev: IIO device instance
+ * @claim: Variable identifier to store acquire result
+ *
+ * Tries to acquire the direct mode lock with cleanup ACQUIRE() semantics and
+ * automatically releases it at the end of the scope. It most be always paired
+ * with IIO_DEV_ACQUIRE_ERR(), for example::
+ *
+ *	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+ *	if (IIO_DEV_ACQUIRE_FAILED(&claim))
+ *		return -EBUSY;
+ *
+ * ...or a more common scenario (notice scope the braces)::
+ *
+ *	switch() {
+ *	case IIO_CHAN_INFO_RAW: {
+ *		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+ *		if (IIO_DEV_ACQUIRE_FAILED(&claim))
+ *			return -EBUSY;
+ *
+ *		...
+ *	}
+ *	case IIO_CHAN_INFO_SCALE:
+ *		...
+ *	...
+ *	}
+ *
+ * Context: Can sleep
+ */
+#define IIO_DEV_ACQUIRE_DIRECT_MODE(dev, claim) \
+	ACQUIRE(__priv__iio_dev_mode_lock_try_direct, claim)(dev)
+
+/**
+ * IIO_DEV_ACQUIRE_FAILED() - ACQUIRE_ERR() wrapper
+ * @claim_ptr: Pointer to the claim variable passed to IIO_DEV_ACQUIRE_*_MODE()
+ *
+ * Return: true if acquired the mode failed, otherwise false.
+ */
+#define IIO_DEV_ACQUIRE_FAILED(claim_ptr) \
+	ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_direct, claim_ptr)
+
+/**
+ * IIO_DEV_GUARD_CURRENT_MODE - Acquires the mode lock with automatic release
+ * @_dev: IIO device instance
+ *
+ * Acquires the mode lock with cleanup guard() semantics. It is usually paired
+ * with iio_buffer_enabled().
+ *
+ * This should *not* be used to protect internal driver state and it's use in
+ * general is *strongly* discouraged. Use any of the IIO_DEV_ACQUIRE_*_MODE()
+ * variants.
+ *
+ * Context: Can sleep
+ */
+#define IIO_DEV_GUARD_CURRENT_MODE(dev) \
+	guard(__priv__iio_dev_mode_lock)(dev)
+
 extern const struct bus_type iio_bus_type;
 
 /**

-- 
2.52.0
Re: [PATCH v3 4/7] iio: core: Add cleanup.h support for iio_device_claim_*()
Posted by David Lechner 3 weeks, 1 day ago
On 1/6/26 2:06 AM, Kurt Borja wrote:
> Add guard classes for iio_device_claim_*() conditional locks. This will
> aid drivers write safer and cleaner code when dealing with some common
> patterns.
> 
> These classes are not meant to be used directly by drivers (hence the
> __priv__ prefix). Instead, documented wrapper macros are provided to
> enforce the use of ACQUIRE() or guard() semantics and avoid the
> problematic scoped guard.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>  include/linux/iio/iio.h | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index d8af0456f966..c795f731f2d8 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -10,6 +10,7 @@
>  #include <linux/align.h>
>  #include <linux/device.h>
>  #include <linux/cdev.h>
> +#include <linux/cleanup.h>
>  #include <linux/compiler_types.h>
>  #include <linux/minmax.h>
>  #include <linux/slab.h>
> @@ -740,6 +741,76 @@ static inline bool iio_device_try_claim_buffer_mode(struct iio_dev *indio_dev)
>   */
>  #define iio_device_release_buffer_mode(indio_dev) __iio_dev_mode_unlock(indio_dev)
>  
> +/*
> + * These classes are not meant to be used directly by drivers (hence the
> + * __priv__ prefix). Instead, documented wrapper macros are provided bellow to
> + * enforce the use of ACQUIRE() or guard() semantics and avoid the problematic
> + * scoped guard variants.
> + */
> +DEFINE_GUARD(__priv__iio_dev_mode_lock, struct iio_dev *,
> +	     __iio_dev_mode_lock(_T), __iio_dev_mode_unlock(_T));
> +DEFINE_GUARD_COND(__priv__iio_dev_mode_lock, _try_direct,
> +		  iio_device_claim_direct(_T));
> +
> +/**
> + * IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) - Tries to acquire the direct mode
> + *                                           lock with automatic release

I don't think it is usual to put the function parameters in the
doc comment like this. They don't match the actual names anyway.

> + * @dev: IIO device instance
> + * @claim: Variable identifier to store acquire result
> + *
> + * Tries to acquire the direct mode lock with cleanup ACQUIRE() semantics and
> + * automatically releases it at the end of the scope. It most be always paired
> + * with IIO_DEV_ACQUIRE_ERR(), for example::
> + *
> + *	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> + *	if (IIO_DEV_ACQUIRE_FAILED(&claim))
> + *		return -EBUSY;
> + *
> + * ...or a more common scenario (notice scope the braces)::
> + *
> + *	switch() {
> + *	case IIO_CHAN_INFO_RAW: {
> + *		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> + *		if (IIO_DEV_ACQUIRE_FAILED(&claim))
> + *			return -EBUSY;
> + *
> + *		...
> + *	}
> + *	case IIO_CHAN_INFO_SCALE:
> + *		...
> + *	...
> + *	}
> + *
> + * Context: Can sleep
> + */
> +#define IIO_DEV_ACQUIRE_DIRECT_MODE(dev, claim) \
> +	ACQUIRE(__priv__iio_dev_mode_lock_try_direct, claim)(dev)
> +
> +/**
> + * IIO_DEV_ACQUIRE_FAILED() - ACQUIRE_ERR() wrapper
> + * @claim_ptr: Pointer to the claim variable passed to IIO_DEV_ACQUIRE_*_MODE()
> + *
> + * Return: true if acquired the mode failed, otherwise false.
> + */
> +#define IIO_DEV_ACQUIRE_FAILED(claim_ptr) \
> +	ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_direct, claim_ptr)
> +

If we always have to add the & at the call site, could we just
put that in the macro instead? Then the parameter would just be
claim instead of claim_ptr.
Re: [PATCH v3 4/7] iio: core: Add cleanup.h support for iio_device_claim_*()
Posted by Kurt Borja 2 weeks, 6 days ago
On Fri Jan 16, 2026 at 5:03 PM -05, David Lechner wrote:
> On 1/6/26 2:06 AM, Kurt Borja wrote:
>> Add guard classes for iio_device_claim_*() conditional locks. This will
>> aid drivers write safer and cleaner code when dealing with some common
>> patterns.
>> 
>> These classes are not meant to be used directly by drivers (hence the
>> __priv__ prefix). Instead, documented wrapper macros are provided to
>> enforce the use of ACQUIRE() or guard() semantics and avoid the
>> problematic scoped guard.
>> 
>> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>>  include/linux/iio/iio.h | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 71 insertions(+)
>> 
>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index d8af0456f966..c795f731f2d8 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -10,6 +10,7 @@
>>  #include <linux/align.h>
>>  #include <linux/device.h>
>>  #include <linux/cdev.h>
>> +#include <linux/cleanup.h>
>>  #include <linux/compiler_types.h>
>>  #include <linux/minmax.h>
>>  #include <linux/slab.h>
>> @@ -740,6 +741,76 @@ static inline bool iio_device_try_claim_buffer_mode(struct iio_dev *indio_dev)
>>   */
>>  #define iio_device_release_buffer_mode(indio_dev) __iio_dev_mode_unlock(indio_dev)
>>  
>> +/*
>> + * These classes are not meant to be used directly by drivers (hence the
>> + * __priv__ prefix). Instead, documented wrapper macros are provided bellow to
>> + * enforce the use of ACQUIRE() or guard() semantics and avoid the problematic
>> + * scoped guard variants.
>> + */
>> +DEFINE_GUARD(__priv__iio_dev_mode_lock, struct iio_dev *,
>> +	     __iio_dev_mode_lock(_T), __iio_dev_mode_unlock(_T));
>> +DEFINE_GUARD_COND(__priv__iio_dev_mode_lock, _try_direct,
>> +		  iio_device_claim_direct(_T));
>> +
>> +/**
>> + * IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) - Tries to acquire the direct mode
>> + *                                           lock with automatic release
>
> I don't think it is usual to put the function parameters in the
> doc comment like this. They don't match the actual names anyway.

Hi David,

This format of kernel-doc applies to function-like macros too [1]. I'll
match the name of the variables though.

>
>> + * @dev: IIO device instance
>> + * @claim: Variable identifier to store acquire result
>> + *
>> + * Tries to acquire the direct mode lock with cleanup ACQUIRE() semantics and
>> + * automatically releases it at the end of the scope. It most be always paired
>> + * with IIO_DEV_ACQUIRE_ERR(), for example::
>> + *
>> + *	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
>> + *	if (IIO_DEV_ACQUIRE_FAILED(&claim))
>> + *		return -EBUSY;
>> + *
>> + * ...or a more common scenario (notice scope the braces)::
>> + *
>> + *	switch() {
>> + *	case IIO_CHAN_INFO_RAW: {
>> + *		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
>> + *		if (IIO_DEV_ACQUIRE_FAILED(&claim))
>> + *			return -EBUSY;
>> + *
>> + *		...
>> + *	}
>> + *	case IIO_CHAN_INFO_SCALE:
>> + *		...
>> + *	...
>> + *	}
>> + *
>> + * Context: Can sleep
>> + */
>> +#define IIO_DEV_ACQUIRE_DIRECT_MODE(dev, claim) \
>> +	ACQUIRE(__priv__iio_dev_mode_lock_try_direct, claim)(dev)
>> +
>> +/**
>> + * IIO_DEV_ACQUIRE_FAILED() - ACQUIRE_ERR() wrapper
>> + * @claim_ptr: Pointer to the claim variable passed to IIO_DEV_ACQUIRE_*_MODE()
>> + *
>> + * Return: true if acquired the mode failed, otherwise false.
>> + */
>> +#define IIO_DEV_ACQUIRE_FAILED(claim_ptr) \
>> +	ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_direct, claim_ptr)
>> +
>
> If we always have to add the & at the call site, could we just
> put that in the macro instead? Then the parameter would just be
> claim instead of claim_ptr.

I'll add this in the next revision.

[1] https://docs.kernel.org/doc-guide/kernel-doc.html#function-documentation

-- 
Thanks,
 ~ Kurt
Re: [PATCH v3 4/7] iio: core: Add cleanup.h support for iio_device_claim_*()
Posted by David Lechner 2 weeks, 6 days ago
On 1/18/26 9:23 AM, Kurt Borja wrote:
> On Fri Jan 16, 2026 at 5:03 PM -05, David Lechner wrote:
>> On 1/6/26 2:06 AM, Kurt Borja wrote:
>>> Add guard classes for iio_device_claim_*() conditional locks. This will
>>> aid drivers write safer and cleaner code when dealing with some common
>>> patterns.
>>>
>>> These classes are not meant to be used directly by drivers (hence the
>>> __priv__ prefix). Instead, documented wrapper macros are provided to
>>> enforce the use of ACQUIRE() or guard() semantics and avoid the
>>> problematic scoped guard.
>>>
>>> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>>> ---
>>>  include/linux/iio/iio.h | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 71 insertions(+)
>>>
>>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>>> index d8af0456f966..c795f731f2d8 100644
>>> --- a/include/linux/iio/iio.h
>>> +++ b/include/linux/iio/iio.h
>>> @@ -10,6 +10,7 @@
>>>  #include <linux/align.h>
>>>  #include <linux/device.h>
>>>  #include <linux/cdev.h>
>>> +#include <linux/cleanup.h>
>>>  #include <linux/compiler_types.h>
>>>  #include <linux/minmax.h>
>>>  #include <linux/slab.h>
>>> @@ -740,6 +741,76 @@ static inline bool iio_device_try_claim_buffer_mode(struct iio_dev *indio_dev)
>>>   */
>>>  #define iio_device_release_buffer_mode(indio_dev) __iio_dev_mode_unlock(indio_dev)
>>>  
>>> +/*
>>> + * These classes are not meant to be used directly by drivers (hence the
>>> + * __priv__ prefix). Instead, documented wrapper macros are provided bellow to
>>> + * enforce the use of ACQUIRE() or guard() semantics and avoid the problematic
>>> + * scoped guard variants.
>>> + */
>>> +DEFINE_GUARD(__priv__iio_dev_mode_lock, struct iio_dev *,
>>> +	     __iio_dev_mode_lock(_T), __iio_dev_mode_unlock(_T));
>>> +DEFINE_GUARD_COND(__priv__iio_dev_mode_lock, _try_direct,
>>> +		  iio_device_claim_direct(_T));
>>> +
>>> +/**
>>> + * IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) - Tries to acquire the direct mode
>>> + *                                           lock with automatic release
>>
>> I don't think it is usual to put the function parameters in the
>> doc comment like this. They don't match the actual names anyway.
> 
> Hi David,
> 
> This format of kernel-doc applies to function-like macros too [1]. I'll
> match the name of the variables though.

Right. And it has no parameters between the () on the first line
in that documentation.

/*
 * function_name() - Brief description of function.


So it should be just `IIO_DEV_ACQUIRE_DIRECT_MODE() - ...

>
>> + * @dev: IIO device instance
>> + * @claim: Variable identifier to store acquire result
>> + *

The parameters go below like this, which is already correct.
Re: [PATCH v3 4/7] iio: core: Add cleanup.h support for iio_device_claim_*()
Posted by Jonathan Cameron 3 weeks, 1 day ago
On Tue, 06 Jan 2026 03:06:59 -0500
Kurt Borja <kuurtb@gmail.com> wrote:

> Add guard classes for iio_device_claim_*() conditional locks. This will
> aid drivers write safer and cleaner code when dealing with some common
> patterns.
> 
> These classes are not meant to be used directly by drivers (hence the
> __priv__ prefix). Instead, documented wrapper macros are provided to
> enforce the use of ACQUIRE() or guard() semantics and avoid the
> problematic scoped guard.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
Very nice. Trivial comments inline.

J
> ---
>  include/linux/iio/iio.h | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index d8af0456f966..c795f731f2d8 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -10,6 +10,7 @@
>  #include <linux/align.h>
>  #include <linux/device.h>
>  #include <linux/cdev.h>
> +#include <linux/cleanup.h>
>  #include <linux/compiler_types.h>
>  #include <linux/minmax.h>
>  #include <linux/slab.h>
> @@ -740,6 +741,76 @@ static inline bool iio_device_try_claim_buffer_mode(struct iio_dev *indio_dev)
>   */
>  #define iio_device_release_buffer_mode(indio_dev) __iio_dev_mode_unlock(indio_dev)
>  
> +/*
> + * These classes are not meant to be used directly by drivers (hence the
> + * __priv__ prefix). Instead, documented wrapper macros are provided bellow to

below

> + * enforce the use of ACQUIRE() or guard() semantics and avoid the problematic
> + * scoped guard variants.
> + */
> +DEFINE_GUARD(__priv__iio_dev_mode_lock, struct iio_dev *,
> +	     __iio_dev_mode_lock(_T), __iio_dev_mode_unlock(_T));
> +DEFINE_GUARD_COND(__priv__iio_dev_mode_lock, _try_direct,
> +		  iio_device_claim_direct(_T));
> +
> +/**
> + * IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) - Tries to acquire the direct mode
> + *                                           lock with automatic release
> + * @dev: IIO device instance
> + * @claim: Variable identifier to store acquire result
> + *
> + * Tries to acquire the direct mode lock with cleanup ACQUIRE() semantics and
> + * automatically releases it at the end of the scope. It most be always paired
> + * with IIO_DEV_ACQUIRE_ERR(), for example::
> + *
> + *	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> + *	if (IIO_DEV_ACQUIRE_FAILED(&claim))
> + *		return -EBUSY;

I don't think we need two examples. I'd just go with the second one as
the braces is indeed something people get wrong with the other cleanup.h stuff.

> + *
> + * ...or a more common scenario (notice scope the braces)::
> + *
> + *	switch() {
> + *	case IIO_CHAN_INFO_RAW: {
> + *		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> + *		if (IIO_DEV_ACQUIRE_FAILED(&claim))
> + *			return -EBUSY;
> + *
> + *		...
> + *	}
> + *	case IIO_CHAN_INFO_SCALE:
> + *		...
> + *	...
> + *	}
> + *
> + * Context: Can sleep
> + */
> +#define IIO_DEV_ACQUIRE_DIRECT_MODE(dev, claim) \
> +	ACQUIRE(__priv__iio_dev_mode_lock_try_direct, claim)(dev)
> +
> +/**
> + * IIO_DEV_ACQUIRE_FAILED() - ACQUIRE_ERR() wrapper
> + * @claim_ptr: Pointer to the claim variable passed to IIO_DEV_ACQUIRE_*_MODE()
> + *
> + * Return: true if acquired the mode failed, otherwise false.
> + */
> +#define IIO_DEV_ACQUIRE_FAILED(claim_ptr) \
> +	ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_direct, claim_ptr)
> +
> +/**
> + * IIO_DEV_GUARD_CURRENT_MODE - Acquires the mode lock with automatic release
> + * @_dev: IIO device instance
> + *
> + * Acquires the mode lock with cleanup guard() semantics. It is usually paired
> + * with iio_buffer_enabled().
> + *
> + * This should *not* be used to protect internal driver state and it's use in
> + * general is *strongly* discouraged. Use any of the IIO_DEV_ACQUIRE_*_MODE()
> + * variants.
> + *
> + * Context: Can sleep
> + */
> +#define IIO_DEV_GUARD_CURRENT_MODE(dev) \
> +	guard(__priv__iio_dev_mode_lock)(dev)
> +
>  extern const struct bus_type iio_bus_type;
>  
>  /**
>