[PATCH v2 0/7] iio: core: Introduce cleanup.h support for mode locks

[PATCH v2 0/7] iio: core: Introduce cleanup.h support for mode locks

[Kurt Borja]

Hi,

In a recent driver review discussion [1], Andy Shevchenko suggested we
add cleanup.h support for the lock API:

	iio_device_claim_{direct,buffer_mode}().

Which would allow some nice code simplification in many places. Some
examples are given as patches, but the last two are the biggest
differences.

In this version I dropped the RFC tag, as the general feeling is to go
through with this after some modifications. Main one is the addition of
IIO_DEV_ACQUIRE_{BUFFER,CLAIM}_MODE() wrappers to avoid drivers using
the guard classes directly. I also added comments on the forbidden ways
to use this API but I definitely still take suggestions on this.

For now I dropped iio_device_claim_buffer_mode() rename, as this point
is still being discussed. My suggestion based on the RFC discussion is
to do it, but in a separate patch (using coccinelle) and while we're at
it rename the whole API like this:

	iio_dev_mode_lock()
	iio_dev_mode_direct_trylock()
	iio_dev_mode_buffer_trylock()
	iio_dev_mode_unlock()

Let me know what you think and thanks for taking a look! The RFC
discussion was a bit long so apologies if I overlooked some comment.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
v2:
  - Add __iio_dev_mode_lock() (formerly iio_device_claim()) in the first
    patch.
  - Added comments to make sure __iio_dev_mode_lock() is not used by
    drivers to protect internal state, or in general.
  - Add patch which re-implements iio_device_claim_direct() using
    __iio_dev_mode_lock().
  - Match iio_device_claim_buffer_mode() semantics by reimplementing it
    in the same way as iio_device_claim_direct().
  - Guard classes now are prefixed with __priv__ to make sure drivers
    don't use them directly.
  - Add IIO_DEV_ACQUIRE_{BUFFER,DIRECT}_MODE() documented wrappers
  - Avoid any function renames (for now).
  - Rename dummy variable `claim` instead of `busy` on vcnl4000 patch.
  - Avoid scoped guard in max30102.
  - Keep using iio_trigger_validate_own_device() insted of
    iio_trigger_using_own() in opt4060.
  - Link to v1: https://lore.kernel.org/r/20251203-lock-impr-v1-0-b4a1fd639423@gmail.com

---
Kurt Borja (7):
      iio: core: Add and export __iio_dev_mode_lock()
      iio: core: Refactor iio_device_claim_direct() implementation
      iio: core: Match iio_device_claim_*() semantics and implementation
      iio: core: Add cleanup.h support for iio_device_claim_*()
      iio: light: vcnl4000: Use IIO cleanup helpers
      iio: health: max30102: Use IIO cleanup helpers
      iio: light: opt4060: Use IIO cleanup helpers

 drivers/iio/adc/ade9000.c                          |   2 +-
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  |   5 +-
 drivers/iio/health/max30100.c                      |   2 +-
 drivers/iio/health/max30102.c                      |  33 ++---
 drivers/iio/industrialio-core.c                    |  86 ++---------
 drivers/iio/light/opt4060.c                        |  52 +++----
 drivers/iio/light/vcnl4000.c                       |  49 +++----
 include/linux/iio/iio.h                            | 162 +++++++++++++++++++--
 8 files changed, 210 insertions(+), 181 deletions(-)
---
base-commit: daea3a394a8b425a2dd206ab09eb37f0d1087d35
change-id: 20251130-lock-impr-6f22748c15e8

-- 
 ~ Kurt

[PATCH v2 1/7] iio: core: Add and export __iio_dev_mode_lock()

[Kurt Borja]

Add infallible wrappers around the internal IIO mode lock.

As mentioned in the documentation, this is not meant to be used by
drivers, instead this will aid in the eventual addition of cleanup
classes around conditional locks.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/iio/industrialio-core.c | 30 ++++++++++++++++++++++++++++++
 include/linux/iio/iio.h         |  3 +++
 2 files changed, 33 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index f69deefcfb6f..1cce2d1ecef1 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -2171,6 +2171,36 @@ int __devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev,
 }
 EXPORT_SYMBOL_GPL(__devm_iio_device_register);
 
+/**
+ * __iio_dev_mode_lock - Locks the current IIO device mode
+ * @indio_dev: the iio_dev associated with the device
+ *
+ * If the device is either in direct or buffer mode, it's guaranteed to stay
+ * that way until __iio_dev_mode_unlock() is called.
+ *
+ * This function is not meant to be used directly by drivers to protect internal
+ * state, a driver should have it's own mechanisms for that matter.
+ *
+ * There are very few cases where a driver actually needs to lock any mode. It's
+ * *strongly* recommended to use iio_device_claim_direct() or
+ * iio_device_claim_buffer_mode() pairs or related helpers.
+ */
+void __iio_dev_mode_lock(struct iio_dev *indio_dev)
+{
+	mutex_lock(&to_iio_dev_opaque(indio_dev)->mlock);
+}
+EXPORT_SYMBOL_GPL(__iio_dev_mode_lock);
+
+/**
+ * __iio_dev_mode_unlock - Unlocks the current IIO device mode
+ * @indio_dev: the iio_dev associated with the device
+ */
+void __iio_dev_mode_unlock(struct iio_dev *indio_dev)
+{
+	mutex_unlock(&to_iio_dev_opaque(indio_dev)->mlock);
+}
+EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock);
+
 /**
  * __iio_device_claim_direct - Keep device in direct mode
  * @indio_dev:	the iio_dev associated with the device
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 872ebdf0dd77..aecda887d833 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -661,6 +661,9 @@ void iio_device_unregister(struct iio_dev *indio_dev);
 int __devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev,
 			       struct module *this_mod);
 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);
 bool __iio_device_claim_direct(struct iio_dev *indio_dev);
 void __iio_device_release_direct(struct iio_dev *indio_dev);
 

-- 
2.52.0

[PATCH v2 2/7] iio: core: Refactor iio_device_claim_direct() implementation

[Kurt Borja]

In order to eventually unify the locking API, implement
iio_device_claim_direct() fully inline, with the use of
__iio_dev_mode_lock(), which takes care of sparse annotations.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/iio/industrialio-core.c | 44 -----------------------------------------
 include/linux/iio/iio.h         | 38 +++++++++++++++++++++++------------
 2 files changed, 25 insertions(+), 57 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 1cce2d1ecef1..cffc6efb6617 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -2201,50 +2201,6 @@ void __iio_dev_mode_unlock(struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock);
 
-/**
- * __iio_device_claim_direct - Keep device in direct mode
- * @indio_dev:	the iio_dev associated with the device
- *
- * If the device is in direct mode it is guaranteed to stay
- * that way until __iio_device_release_direct() is called.
- *
- * Use with __iio_device_release_direct().
- *
- * Drivers should only call iio_device_claim_direct().
- *
- * Returns: true on success, false on failure.
- */
-bool __iio_device_claim_direct(struct iio_dev *indio_dev)
-{
-	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-
-	mutex_lock(&iio_dev_opaque->mlock);
-
-	if (iio_buffer_enabled(indio_dev)) {
-		mutex_unlock(&iio_dev_opaque->mlock);
-		return false;
-	}
-	return true;
-}
-EXPORT_SYMBOL_GPL(__iio_device_claim_direct);
-
-/**
- * __iio_device_release_direct - releases claim on direct mode
- * @indio_dev:	the iio_dev associated with the device
- *
- * Release the claim. Device is no longer guaranteed to stay
- * in direct mode.
- *
- * Drivers should only call iio_device_release_direct().
- *
- * Use with __iio_device_claim_direct()
- */
-void __iio_device_release_direct(struct iio_dev *indio_dev)
-{
-	mutex_unlock(&to_iio_dev_opaque(indio_dev)->mlock);
-}
-EXPORT_SYMBOL_GPL(__iio_device_release_direct);
-
 /**
  * iio_device_claim_buffer_mode - Keep device in buffer mode
  * @indio_dev:	the iio_dev associated with the device
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index aecda887d833..76398dbfa5ca 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -664,30 +664,42 @@ 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);
-bool __iio_device_claim_direct(struct iio_dev *indio_dev);
-void __iio_device_release_direct(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()
+/**
+ * iio_device_claim_direct - Keep device in direct mode
+ * @indio_dev:	the iio_dev associated with the device
+ *
+ * If the device is in direct mode it is guaranteed to stay
+ * that way until iio_device_release_direct() is called.
+ *
+ * Use with iio_device_release_direct().
+ *
+ * Returns: true on success, false on failure.
  */
 static inline bool iio_device_claim_direct(struct iio_dev *indio_dev)
 {
-	if (!__iio_device_claim_direct(indio_dev))
-		return false;
+	__iio_dev_mode_lock(indio_dev);
 
-	__acquire(iio_dev);
+	if (iio_buffer_enabled(indio_dev)) {
+		__iio_dev_mode_unlock(indio_dev);
+		return false;
+	}
 
 	return true;
 }
 
+/**
+ * iio_device_release_direct - Releases claim on direct mode
+ * @indio_dev:	the iio_dev associated with the device
+ *
+ * Release the claim. Device is no longer guaranteed to stay
+ * in direct mode.
+ *
+ * Use with iio_device_claim_direct()
+ */
 static inline void iio_device_release_direct(struct iio_dev *indio_dev)
 {
-	__iio_device_release_direct(indio_dev);
-	__release(indio_dev);
+	__iio_dev_mode_unlock(indio_dev);
 }
 
 int iio_device_claim_buffer_mode(struct iio_dev *indio_dev);

-- 
2.52.0

[PATCH v2 3/7] iio: core: Match iio_device_claim_*() semantics and implementation

[Kurt Borja]

Implement iio_device_claim_buffer_mode() fully inline with the use of
__iio_dev_mode_lock(), which takes care of sparse annotations.

To completely match iio_device_claim_direct() semantics, we need to
also change iio_device_claim_buffer_mode() return semantics to usual
true/false conditional lock semantics.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/iio/adc/ade9000.c                          |  2 +-
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  |  5 +--
 drivers/iio/health/max30100.c                      |  2 +-
 drivers/iio/health/max30102.c                      |  2 +-
 drivers/iio/industrialio-core.c                    | 40 ----------------------
 drivers/iio/light/opt4060.c                        |  2 +-
 include/linux/iio/iio.h                            | 38 ++++++++++++++++++--
 7 files changed, 41 insertions(+), 50 deletions(-)

diff --git a/drivers/iio/adc/ade9000.c b/drivers/iio/adc/ade9000.c
index 2de8a718d62a..b345c4d1ef24 100644
--- a/drivers/iio/adc/ade9000.c
+++ b/drivers/iio/adc/ade9000.c
@@ -964,7 +964,7 @@ static irqreturn_t ade9000_dready_thread(int irq, void *data)
 	struct iio_dev *indio_dev = data;
 
 	/* Handle data ready interrupt from C4/EVENT/DREADY pin */
-	if (!iio_device_claim_buffer_mode(indio_dev)) {
+	if (iio_device_claim_buffer_mode(indio_dev)) {
 		ade9000_iio_push_buffer(indio_dev);
 		iio_device_release_buffer_mode(indio_dev);
 	}
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 9ac80e4b7d75..8ed4b2e410c8 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -188,11 +188,8 @@ int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
 	/*
 	 * Ignore samples if the buffer is not set: it is needed if the ODR is
 	 * set but the buffer is not enabled yet.
-	 *
-	 * Note: iio_device_claim_buffer_mode() returns -EBUSY if the buffer
-	 * is not enabled.
 	 */
-	if (iio_device_claim_buffer_mode(indio_dev) < 0)
+	if (!iio_device_claim_buffer_mode(indio_dev))
 		return 0;
 
 	out = (s16 *)st->samples;
diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c
index 3d441013893c..3f3680c4b42f 100644
--- a/drivers/iio/health/max30100.c
+++ b/drivers/iio/health/max30100.c
@@ -417,7 +417,7 @@ static int max30100_read_raw(struct iio_dev *indio_dev,
 		 * Temperature reading can only be acquired while engine
 		 * is running
 		 */
-		if (iio_device_claim_buffer_mode(indio_dev)) {
+		if (!iio_device_claim_buffer_mode(indio_dev)) {
 			/*
 			 * Replacing -EBUSY or other error code
 			 * returned by iio_device_claim_buffer_mode()
diff --git a/drivers/iio/health/max30102.c b/drivers/iio/health/max30102.c
index a48c0881a4c7..288c2f37a4a2 100644
--- a/drivers/iio/health/max30102.c
+++ b/drivers/iio/health/max30102.c
@@ -476,7 +476,7 @@ static int max30102_read_raw(struct iio_dev *indio_dev,
 		 * shutdown; leave shutdown briefly when buffer not running
 		 */
 any_mode_retry:
-		if (iio_device_claim_buffer_mode(indio_dev)) {
+		if (!iio_device_claim_buffer_mode(indio_dev)) {
 			/*
 			 * This one is a *bit* hacky. If we cannot claim buffer
 			 * mode, then try direct mode so that we make sure
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index cffc6efb6617..46e336bff64c 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -2201,46 +2201,6 @@ void __iio_dev_mode_unlock(struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock);
 
-/**
- * iio_device_claim_buffer_mode - Keep device in buffer mode
- * @indio_dev:	the iio_dev associated with the device
- *
- * If the device is in buffer mode it is guaranteed to stay
- * that way until iio_device_release_buffer_mode() is called.
- *
- * Use with iio_device_release_buffer_mode().
- *
- * Returns: 0 on success, -EBUSY on failure.
- */
-int iio_device_claim_buffer_mode(struct iio_dev *indio_dev)
-{
-	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-
-	mutex_lock(&iio_dev_opaque->mlock);
-
-	if (iio_buffer_enabled(indio_dev))
-		return 0;
-
-	mutex_unlock(&iio_dev_opaque->mlock);
-	return -EBUSY;
-}
-EXPORT_SYMBOL_GPL(iio_device_claim_buffer_mode);
-
-/**
- * iio_device_release_buffer_mode - releases claim on buffer mode
- * @indio_dev:	the iio_dev associated with the device
- *
- * Release the claim. Device is no longer guaranteed to stay
- * in buffer mode.
- *
- * Use with iio_device_claim_buffer_mode().
- */
-void iio_device_release_buffer_mode(struct iio_dev *indio_dev)
-{
-	mutex_unlock(&to_iio_dev_opaque(indio_dev)->mlock);
-}
-EXPORT_SYMBOL_GPL(iio_device_release_buffer_mode);
-
 /**
  * iio_device_get_current_mode() - helper function providing read-only access to
  *				   the opaque @currentmode variable
diff --git a/drivers/iio/light/opt4060.c b/drivers/iio/light/opt4060.c
index 981c704e7df5..8cb3fa38077e 100644
--- a/drivers/iio/light/opt4060.c
+++ b/drivers/iio/light/opt4060.c
@@ -304,7 +304,7 @@ static int opt4060_set_driver_state(struct iio_dev *indio_dev,
 	struct opt4060_chip *chip = iio_priv(indio_dev);
 	int ret = 0;
 any_mode_retry:
-	if (iio_device_claim_buffer_mode(indio_dev)) {
+	if (!iio_device_claim_buffer_mode(indio_dev)) {
 		/*
 		 * This one is a *bit* hacky. If we cannot claim buffer mode,
 		 * then try direct mode so that we make sure things cannot
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 76398dbfa5ca..f8a7ef709210 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -702,8 +702,42 @@ static inline void iio_device_release_direct(struct iio_dev *indio_dev)
 	__iio_dev_mode_unlock(indio_dev);
 }
 
-int iio_device_claim_buffer_mode(struct iio_dev *indio_dev);
-void iio_device_release_buffer_mode(struct iio_dev *indio_dev);
+/**
+ * iio_device_claim_buffer_mode - Keep device in buffer mode
+ * @indio_dev:	the iio_dev associated with the device
+ *
+ * If the device is in buffer mode it is guaranteed to stay
+ * that way until iio_device_release_buffer_mode() is called.
+ *
+ * Use with iio_device_release_buffer_mode().
+ *
+ * Returns: true on success, false on failure.
+ */
+static inline bool iio_device_claim_buffer_mode(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;
+}
+
+/**
+ * iio_device_release_buffer_mode - releases claim on buffer mode
+ * @indio_dev:	the iio_dev associated with the device
+ *
+ * Release the claim. Device is no longer guaranteed to stay
+ * in buffer mode.
+ *
+ * Use with iio_device_claim_buffer_mode().
+ */
+static inline void iio_device_release_buffer_mode(struct iio_dev *indio_dev)
+{
+	__iio_dev_mode_unlock(indio_dev);
+}
 
 extern const struct bus_type iio_bus_type;
 

-- 
2.52.0

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

[Kurt Borja]

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 | 83 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index f8a7ef709210..c84853c7a37f 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>
@@ -739,6 +740,88 @@ static inline void iio_device_release_buffer_mode(struct iio_dev *indio_dev)
 	__iio_dev_mode_unlock(indio_dev);
 }
 
+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_buffer,
+		  iio_device_claim_buffer_mode(_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
+ * @_var: Dummy 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_ERR(&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_ERR(&claim))
+ *			return -EBUSY;
+ *
+ *		...
+ *	}
+ *	case IIO_CHAN_INFO_SCALE:
+ *		...
+ *	...
+ *	}
+ *
+ * Context: Can sleep
+ */
+#define IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) \
+	ACQUIRE(__priv__iio_dev_mode_lock_try_direct, _var)(_dev)
+
+/**
+ * IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) - Tries to acquire the buffer mode
+ *                                           lock with automatic release
+ * @_dev: IIO device instance
+ * @_var: Dummy variable identifier to store acquire result
+ *
+ * Tries to acquire the direct mode lock and automatically releases it at the
+ * end of the scope. It most be paired with IIO_DEV_ACQUIRE_ERR(), for example::
+ *
+ *	IIO_DEV_ACQUIRE_BUFFER_MODE(indio_dev, claim);
+ *	if (IIO_DEV_ACQUIRE_ERR(&claim))
+ *		return IRQ_HANDLED;
+ *
+ * Context: Can sleep
+ */
+#define IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) \
+	ACQUIRE(__priv__iio_dev_mode_lock_try_buffer, _var)(_dev)
+
+/**
+ * IIO_DEV_ACQUIRE_ERR() - ACQUIRE_ERR() wrapper
+ * @_var: Dummy variable passed to IIO_DEV_ACQUIRE_*_MODE()
+ *
+ * Return: true on success, false on error
+ */
+#define IIO_DEV_ACQUIRE_ERR(_var_ptr) \
+	ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_buffer, _var_ptr)
+
+/**
+ * IIO_DEV_GUARD_ANY_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.
+ */
+#define IIO_DEV_GUARD_ANY_MODE(_dev) \
+	guard(__priv__iio_dev_mode_lock)(_dev)
+
 extern const struct bus_type iio_bus_type;
 
 /**

-- 
2.52.0

[PATCH v2 5/7] iio: light: vcnl4000: Use IIO cleanup helpers

[Kurt Borja]

Use IIO_DEV_ACQUIRE_DIRECT_MODE() helper to automatically release direct
mode.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/iio/light/vcnl4000.c | 49 ++++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 4dbb2294a843..4184104ac8f2 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -1078,20 +1078,17 @@ static int vcnl4010_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-	case IIO_CHAN_INFO_SCALE:
-		if (!iio_device_claim_direct(indio_dev))
+	case IIO_CHAN_INFO_SCALE: {
+		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+		if (IIO_DEV_ACQUIRE_ERR(&claim))
 			return -EBUSY;
 
 		/* Protect against event capture. */
-		if (vcnl4010_is_in_periodic_mode(data)) {
-			ret = -EBUSY;
-		} else {
-			ret = vcnl4000_read_raw(indio_dev, chan, val, val2,
-						mask);
-		}
+		if (vcnl4010_is_in_periodic_mode(data))
+			return -EBUSY;
 
-		iio_device_release_direct(indio_dev);
-		return ret;
+		return vcnl4000_read_raw(indio_dev, chan, val, val2, mask);
+	}
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		switch (chan->type) {
 		case IIO_PROXIMITY:
@@ -1148,36 +1145,27 @@ static int vcnl4010_write_raw(struct iio_dev *indio_dev,
 			      struct iio_chan_spec const *chan,
 			      int val, int val2, long mask)
 {
-	int ret;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
 
-	if (!iio_device_claim_direct(indio_dev))
+	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+	if (IIO_DEV_ACQUIRE_ERR(&claim))
 		return -EBUSY;
 
 	/* Protect against event capture. */
-	if (vcnl4010_is_in_periodic_mode(data)) {
-		ret = -EBUSY;
-		goto end;
-	}
+	if (vcnl4010_is_in_periodic_mode(data))
+		return -EBUSY;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		switch (chan->type) {
 		case IIO_PROXIMITY:
-			ret = vcnl4010_write_proxy_samp_freq(data, val, val2);
-			goto end;
+			return vcnl4010_write_proxy_samp_freq(data, val, val2);
 		default:
-			ret = -EINVAL;
-			goto end;
+			return -EINVAL;
 		}
 	default:
-		ret = -EINVAL;
-		goto end;
+		return -EINVAL;
 	}
-
-end:
-	iio_device_release_direct(indio_dev);
-	return ret;
 }
 
 static int vcnl4010_read_event(struct iio_dev *indio_dev,
@@ -1438,14 +1426,13 @@ static int vcnl4010_config_threshold_disable(struct vcnl4000_data *data)
 static int vcnl4010_config_threshold(struct iio_dev *indio_dev, bool state)
 {
 	struct vcnl4000_data *data = iio_priv(indio_dev);
-	int ret;
 
 	if (state) {
-		if (!iio_device_claim_direct(indio_dev))
+		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+		if (IIO_DEV_ACQUIRE_ERR(&claim))
 			return -EBUSY;
-		ret = vcnl4010_config_threshold_enable(data);
-		iio_device_release_direct(indio_dev);
-		return ret;
+
+		return vcnl4010_config_threshold_enable(data);
 	} else {
 		return vcnl4010_config_threshold_disable(data);
 	}

-- 
2.52.0

[PATCH v2 6/7] iio: health: max30102: Use IIO cleanup helpers

[Kurt Borja]

Use IIO_DEV_GUARD_ANY_MODE() cleanup helper to simplify and drop
busy-waiting code in max30102_read_raw().

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/iio/health/max30102.c | 33 +++++++++------------------------
 1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/health/max30102.c b/drivers/iio/health/max30102.c
index 288c2f37a4a2..94e15598ffe7 100644
--- a/drivers/iio/health/max30102.c
+++ b/drivers/iio/health/max30102.c
@@ -467,44 +467,29 @@ static int max30102_read_raw(struct iio_dev *indio_dev,
 			     int *val, int *val2, long mask)
 {
 	struct max30102_data *data = iio_priv(indio_dev);
-	int ret = -EINVAL;
+	int ret;
 
 	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_RAW: {
 		/*
 		 * Temperature reading can only be acquired when not in
 		 * shutdown; leave shutdown briefly when buffer not running
 		 */
-any_mode_retry:
-		if (!iio_device_claim_buffer_mode(indio_dev)) {
-			/*
-			 * This one is a *bit* hacky. If we cannot claim buffer
-			 * mode, then try direct mode so that we make sure
-			 * things cannot concurrently change. And we just keep
-			 * trying until we get one of the modes...
-			 */
-			if (!iio_device_claim_direct(indio_dev))
-				goto any_mode_retry;
+		IIO_DEV_GUARD_ANY_MODE(indio_dev);
 
-			ret = max30102_get_temp(data, val, true);
-			iio_device_release_direct(indio_dev);
-		} else {
-			ret = max30102_get_temp(data, val, false);
-			iio_device_release_buffer_mode(indio_dev);
-		}
+		ret = max30102_get_temp(data, val, !iio_buffer_enabled(indio_dev));
 		if (ret)
 			return ret;
 
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
+	}
 	case IIO_CHAN_INFO_SCALE:
 		*val = 1000;  /* 62.5 */
 		*val2 = 16;
-		ret = IIO_VAL_FRACTIONAL;
-		break;
+		return IIO_VAL_FRACTIONAL;
+	default:
+		return -EINVAL;
 	}
-
-	return ret;
 }
 
 static const struct iio_info max30102_info = {

-- 
2.52.0

[PATCH v2 7/7] iio: light: opt4060: Use IIO cleanup helpers

[Kurt Borja]

Use IIO_DEV_GUARD_ANY_MODE() cleanup helper to simplify and drop
busy-waiting code in opt4060_set_driver_state().

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/iio/light/opt4060.c | 52 +++++++++++++++------------------------------
 1 file changed, 17 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/light/opt4060.c b/drivers/iio/light/opt4060.c
index 8cb3fa38077e..943e5963d568 100644
--- a/drivers/iio/light/opt4060.c
+++ b/drivers/iio/light/opt4060.c
@@ -302,41 +302,23 @@ static int opt4060_set_driver_state(struct iio_dev *indio_dev,
 				    bool continuous_irq)
 {
 	struct opt4060_chip *chip = iio_priv(indio_dev);
-	int ret = 0;
-any_mode_retry:
-	if (!iio_device_claim_buffer_mode(indio_dev)) {
-		/*
-		 * This one is a *bit* hacky. If we cannot claim buffer mode,
-		 * then try direct mode so that we make sure things cannot
-		 * concurrently change. And we just keep trying until we get one
-		 * of the modes...
-		 */
-		if (!iio_device_claim_direct(indio_dev))
-			goto any_mode_retry;
-		/*
-		 * This path means that we managed to claim direct mode. In
-		 * this case the buffer isn't enabled and it's okay to leave
-		 * continuous mode for sampling and/or irq.
-		 */
-		ret = opt4060_set_state_common(chip, continuous_sampling,
-					       continuous_irq);
-		iio_device_release_direct(indio_dev);
-		return ret;
-	} else {
-		/*
-		 * This path means that we managed to claim buffer mode. In
-		 * this case the buffer is enabled and irq and sampling must go
-		 * to or remain continuous, but only if the trigger is from this
-		 * device.
-		 */
-		if (!iio_trigger_validate_own_device(indio_dev->trig, indio_dev))
-			ret = opt4060_set_state_common(chip, true, true);
-		else
-			ret = opt4060_set_state_common(chip, continuous_sampling,
-						       continuous_irq);
-		iio_device_release_buffer_mode(indio_dev);
-	}
-	return ret;
+
+	IIO_DEV_GUARD_ANY_MODE(indio_dev);
+
+	/*
+	 * If we manage to claim buffer mode and we are using our own trigger,
+	 * IRQ and sampling must go to or remain continuous.
+	 */
+	if (iio_buffer_enabled(indio_dev) &&
+	    iio_trigger_validate_own_device(indio_dev->trig, indio_dev))
+		return opt4060_set_state_common(chip, true, true);
+
+	/*
+	 * This path means that we managed to claim direct mode. In this case
+	 * the buffer isn't enabled and it's okay to leave continuous mode for
+	 * sampling and/or irq.
+	 */
+	return opt4060_set_state_common(chip, continuous_sampling, continuous_irq);
 }
 
 /*

-- 
2.52.0

Re: [PATCH v2 1/7] iio: core: Add and export __iio_dev_mode_lock()

[Andy Shevchenko]

On Thu, Dec 11, 2025 at 09:45:19PM -0500, Kurt Borja wrote:
> Add infallible wrappers around the internal IIO mode lock.
> 
> As mentioned in the documentation, this is not meant to be used by
> drivers, instead this will aid in the eventual addition of cleanup
> classes around conditional locks.

...

> +EXPORT_SYMBOL_GPL(__iio_dev_mode_lock);

> +EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock);

Put them to "IIO_CORE" namespace.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/7] iio: core: Refactor iio_device_claim_direct() implementation

[Andy Shevchenko]

On Thu, Dec 11, 2025 at 09:45:20PM -0500, Kurt Borja wrote:
> In order to eventually unify the locking API, implement
> iio_device_claim_direct() fully inline, with the use of
> __iio_dev_mode_lock(), which takes care of sparse annotations.

...

> +/**
> + * iio_device_claim_direct - Keep device in direct mode
> + * @indio_dev:	the iio_dev associated with the device
> + *
> + * If the device is in direct mode it is guaranteed to stay
> + * that way until iio_device_release_direct() is called.
> + *
> + * Use with iio_device_release_direct().

Nice.

> + * Returns: true on success, false on failure.
>   */

...


> +/**
> + * iio_device_release_direct - Releases claim on direct mode
> + * @indio_dev:	the iio_dev associated with the device
> + *
> + * Release the claim. Device is no longer guaranteed to stay
> + * in direct mode.
> + *
> + * Use with iio_device_claim_direct()

Not nice — missed period.

> + */

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/7] iio: core: Match iio_device_claim_*() semantics and implementation

[Andy Shevchenko]

On Thu, Dec 11, 2025 at 09:45:21PM -0500, Kurt Borja wrote:
> Implement iio_device_claim_buffer_mode() fully inline with the use of
> __iio_dev_mode_lock(), which takes care of sparse annotations.
> 
> To completely match iio_device_claim_direct() semantics, we need to
> also change iio_device_claim_buffer_mode() return semantics to usual
> true/false conditional lock semantics.

...

> -		if (iio_device_claim_buffer_mode(indio_dev)) {
> +		if (!iio_device_claim_buffer_mode(indio_dev)) {
>  			/*
>  			 * Replacing -EBUSY or other error code
>  			 * returned by iio_device_claim_buffer_mode()

In one of the previous files you removed a comment, and here it's still present.

...

> -		if (iio_device_claim_buffer_mode(indio_dev)) {
> +		if (!iio_device_claim_buffer_mode(indio_dev)) {
>  			/*
>  			 * This one is a *bit* hacky. If we cannot claim buffer
>  			 * mode, then try direct mode so that we make sure

Also check if this comment needs to be amended.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/7] iio: core: Add and export __iio_dev_mode_lock()

[Kurt Borja]

On Fri Dec 12, 2025 at 1:21 PM -05, Andy Shevchenko wrote:
> On Thu, Dec 11, 2025 at 09:45:19PM -0500, Kurt Borja wrote:
>> Add infallible wrappers around the internal IIO mode lock.
>> 
>> As mentioned in the documentation, this is not meant to be used by
>> drivers, instead this will aid in the eventual addition of cleanup
>> classes around conditional locks.
>
> ...
>
>> +EXPORT_SYMBOL_GPL(__iio_dev_mode_lock);
>
>> +EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock);
>
> Put them to "IIO_CORE" namespace.

I will!

Thanks for your comments. If this gets no more comments for a couple of
days I'll send v3 including the rest of your suggestions.

-- 
 ~ Kurt

Re: [PATCH v2 1/7] iio: core: Add and export __iio_dev_mode_lock()

[David Lechner]

On 12/11/25 8:45 PM, Kurt Borja wrote:
> Add infallible wrappers around the internal IIO mode lock.

Not sure what "infallible" is supposed to mean in this context. Maybe
referring to autocleanup?

> 
> As mentioned in the documentation, this is not meant to be used by
> drivers, instead this will aid in the eventual addition of cleanup
> classes around conditional locks.
> 
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>  drivers/iio/industrialio-core.c | 30 ++++++++++++++++++++++++++++++
>  include/linux/iio/iio.h         |  3 +++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index f69deefcfb6f..1cce2d1ecef1 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -2171,6 +2171,36 @@ int __devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev,
>  }
>  EXPORT_SYMBOL_GPL(__devm_iio_device_register);
>  
> +/**
> + * __iio_dev_mode_lock - Locks the current IIO device mode
> + * @indio_dev: the iio_dev associated with the device
> + *
> + * If the device is either in direct or buffer mode, it's guaranteed to stay
> + * that way until __iio_dev_mode_unlock() is called.
> + *
> + * This function is not meant to be used directly by drivers to protect internal
> + * state, a driver should have it's own mechanisms for that matter.
> + *
> + * There are very few cases where a driver actually needs to lock any mode. It's
> + * *strongly* recommended to use iio_device_claim_direct() or

I wouldn't even say "strongly recommend". Just say "use these instead".

In the exceptions, likely the autocleanup version should be used as is
the case currently.

> + * iio_device_claim_buffer_mode() pairs or related helpers.
> + */
> +void __iio_dev_mode_lock(struct iio_dev *indio_dev)
> +{
> +	mutex_lock(&to_iio_dev_opaque(indio_dev)->mlock);
> +}
> +EXPORT_SYMBOL_GPL(__iio_dev_mode_lock);
> +
> +/**
> + * __iio_dev_mode_unlock - Unlocks the current IIO device mode
> + * @indio_dev: the iio_dev associated with the device
> + */
> +void __iio_dev_mode_unlock(struct iio_dev *indio_dev)
> +{
> +	mutex_unlock(&to_iio_dev_opaque(indio_dev)->mlock);
> +}
> +EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock);
> +
>  /**
>   * __iio_device_claim_direct - Keep device in direct mode
>   * @indio_dev:	the iio_dev associated with the device
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 872ebdf0dd77..aecda887d833 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -661,6 +661,9 @@ void iio_device_unregister(struct iio_dev *indio_dev);
>  int __devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev,
>  			       struct module *this_mod);
>  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);
>  bool __iio_device_claim_direct(struct iio_dev *indio_dev);
>  void __iio_device_release_direct(struct iio_dev *indio_dev);
>  
> 

Re: [PATCH v2 2/7] iio: core: Refactor iio_device_claim_direct() implementation

[David Lechner]

On 12/11/25 8:45 PM, Kurt Borja wrote:
> In order to eventually unify the locking API, implement
> iio_device_claim_direct() fully inline, with the use of
> __iio_dev_mode_lock(), which takes care of sparse annotations.
> 
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>  drivers/iio/industrialio-core.c | 44 -----------------------------------------
>  include/linux/iio/iio.h         | 38 +++++++++++++++++++++++------------
>  2 files changed, 25 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 1cce2d1ecef1..cffc6efb6617 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -2201,50 +2201,6 @@ void __iio_dev_mode_unlock(struct iio_dev *indio_dev)
>  }
>  EXPORT_SYMBOL_GPL(__iio_dev_mode_unlock);
>  
> -/**
> - * __iio_device_claim_direct - Keep device in direct mode
> - * @indio_dev:	the iio_dev associated with the device
> - *
> - * If the device is in direct mode it is guaranteed to stay
> - * that way until __iio_device_release_direct() is called.
> - *
> - * Use with __iio_device_release_direct().
> - *
> - * Drivers should only call iio_device_claim_direct().
> - *
> - * Returns: true on success, false on failure.
> - */
> -bool __iio_device_claim_direct(struct iio_dev *indio_dev)
> -{
> -	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> -
> -	mutex_lock(&iio_dev_opaque->mlock);
> -
> -	if (iio_buffer_enabled(indio_dev)) {
> -		mutex_unlock(&iio_dev_opaque->mlock);
> -		return false;
> -	}
> -	return true;
> -}
> -EXPORT_SYMBOL_GPL(__iio_device_claim_direct);
> -
> -/**
> - * __iio_device_release_direct - releases claim on direct mode
> - * @indio_dev:	the iio_dev associated with the device
> - *
> - * Release the claim. Device is no longer guaranteed to stay
> - * in direct mode.
> - *
> - * Drivers should only call iio_device_release_direct().
> - *
> - * Use with __iio_device_claim_direct()
> - */
> -void __iio_device_release_direct(struct iio_dev *indio_dev)
> -{
> -	mutex_unlock(&to_iio_dev_opaque(indio_dev)->mlock);
> -}
> -EXPORT_SYMBOL_GPL(__iio_device_release_direct);
> -
>  /**
>   * iio_device_claim_buffer_mode - Keep device in buffer mode
>   * @indio_dev:	the iio_dev associated with the device
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index aecda887d833..76398dbfa5ca 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -664,30 +664,42 @@ 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);
> -bool __iio_device_claim_direct(struct iio_dev *indio_dev);
> -void __iio_device_release_direct(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()

I think we should leave a comment that says these functions need to stay
as static inline so that sparse works. Although I'm wondering how sparse
could still work since __acquire() and __release() are removed here. IIRC
it was a bit finicky about that.

> +/**
> + * iio_device_claim_direct - Keep device in direct mode
> + * @indio_dev:	the iio_dev associated with the device
> + *
> + * If the device is in direct mode it is guaranteed to stay
> + * that way until iio_device_release_direct() is called.
> + *
> + * Use with iio_device_release_direct().
> + *
> + * Returns: true on success, false on failure.
>   */
>  static inline bool iio_device_claim_direct(struct iio_dev *indio_dev)
>  {
> -	if (!__iio_device_claim_direct(indio_dev))
> -		return false;
> +	__iio_dev_mode_lock(indio_dev);
>  
> -	__acquire(iio_dev);
> +	if (iio_buffer_enabled(indio_dev)) {
> +		__iio_dev_mode_unlock(indio_dev);
> +		return false;
> +	}
>  
>  	return true;
>  }
>  
> +/**
> + * iio_device_release_direct - Releases claim on direct mode
> + * @indio_dev:	the iio_dev associated with the device
> + *
> + * Release the claim. Device is no longer guaranteed to stay
> + * in direct mode.
> + *
> + * Use with iio_device_claim_direct()
> + */
>  static inline void iio_device_release_direct(struct iio_dev *indio_dev)

It could make more sense to make this:

#define iio_device_release_direct __iio_dev_mode_unlock

To make it clear that the auto cleanup functions that come later
don't call iio_device_release_direct() but rather call
__iio_dev_mode_unlock() directly.

Otherwise, someone could be tempted to modify the iio_device_release_direct()
function and the changes would not have an effect when auto cleanup is used.

(Same applies to the release buffer mode function.)

>  {
> -	__iio_device_release_direct(indio_dev);
> -	__release(indio_dev);
> +	__iio_dev_mode_unlock(indio_dev);
>  }
>  
>  int iio_device_claim_buffer_mode(struct iio_dev *indio_dev);
> 

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

[David Lechner]

On 12/11/25 8:45 PM, 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.

Would be useful to repeat this in a comment in the code.

> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>  include/linux/iio/iio.h | 83 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index f8a7ef709210..c84853c7a37f 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>
> @@ -739,6 +740,88 @@ static inline void iio_device_release_buffer_mode(struct iio_dev *indio_dev)
>  	__iio_dev_mode_unlock(indio_dev);
>  }
>  
> +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_buffer,
> +		  iio_device_claim_buffer_mode(_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
> + * @_var: Dummy variable identifier to store acquire result

It's not a dummy if it does something. :-) (so we can drop that word)

Also, I would call it _claim instead of _var to to match the example
and encourage people to use the same name everywhere.

And for that matter, we don't really need the leading underscores in either
parameter since there are no name conflicts.

> + *
> + * 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_ERR(&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_ERR(&claim))
> + *			return -EBUSY;
> + *
> + *		...
> + *	}
> + *	case IIO_CHAN_INFO_SCALE:
> + *		...
> + *	...
> + *	}
> + *
> + * Context: Can sleep
> + */
> +#define IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) \
> +	ACQUIRE(__priv__iio_dev_mode_lock_try_direct, _var)(_dev)
> +
> +/**
> + * IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) - Tries to acquire the buffer mode
> + *                                           lock with automatic release
> + * @_dev: IIO device instance
> + * @_var: Dummy variable identifier to store acquire result
> + *
> + * Tries to acquire the direct mode lock and automatically releases it at the
> + * end of the scope. It most be paired with IIO_DEV_ACQUIRE_ERR(), for example::
> + *
> + *	IIO_DEV_ACQUIRE_BUFFER_MODE(indio_dev, claim);
> + *	if (IIO_DEV_ACQUIRE_ERR(&claim))
> + *		return IRQ_HANDLED;
> + *
> + * Context: Can sleep
> + */
> +#define IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) \
> +	ACQUIRE(__priv__iio_dev_mode_lock_try_buffer, _var)(_dev)
> +
> +/**
> + * IIO_DEV_ACQUIRE_ERR() - ACQUIRE_ERR() wrapper
> + * @_var: Dummy variable passed to IIO_DEV_ACQUIRE_*_MODE()
> + *
> + * Return: true on success, false on error

This could be clarified more. Based on the example, this sounds
backwards.

	Returns: true if acquiring the mode failed, otherwise false.

> + */
> +#define IIO_DEV_ACQUIRE_ERR(_var_ptr) \
> +	ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_buffer, _var_ptr)

There is no error code here, so calling it "ERR" seems wrong.
Maybe IIO_DEV_ACQUIRE_FAILED()?

> +
> +/**
> + * IIO_DEV_GUARD_ANY_MODE - Acquires the mode lock with automatic release
> + * @_dev: IIO device instance

It would be helpful to explain more about the use case here and that this
is used rarely.

The point to get across is that it is used when we need to do something
that doesn't depend on the current mode, but would be affected by a mode
switch. So it guards against changing the mode without caring what the
current mode is. If it is in buffer mode, it stays in buffer mode, otherwise
direct mode is claimed.

> + *
> + * 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.

Might as well add Context: here like the others.

> + */
> +#define IIO_DEV_GUARD_ANY_MODE(_dev) \

Accordingly, I would be inclined to call it IIO_DEV_GUARD_CURRENT_MODE()

> +	guard(__priv__iio_dev_mode_lock)(_dev)
> +
>  extern const struct bus_type iio_bus_type;
>  
>  /**
> 

Re: [PATCH v2 3/7] iio: core: Match iio_device_claim_*() semantics and implementation

[Jonathan Cameron]

On Thu, 11 Dec 2025 21:45:21 -0500
Kurt Borja <kuurtb@gmail.com> wrote:

> Implement iio_device_claim_buffer_mode() fully inline with the use of
> __iio_dev_mode_lock(), which takes care of sparse annotations.
> 
> To completely match iio_device_claim_direct() semantics, we need to
> also change iio_device_claim_buffer_mode() return semantics to usual
> true/false conditional lock semantics.

I wasn't rushing to review this set because I want it to sit
a little longer than a typical series to get more eyes on it.
Anyhow, long enough for this version at least!

Whilst I find it hard to care strongly about out of tree drivers
and in place flip of the return logic seems a bit unfair on anyone
trying to keep those rebased on mainline!

So with that in mind, maybe we need to name it differently even
if we are getting rid of the old implementation all in one patch.

Given earlier discussion about this one being rather more tricky
to name than the claim_direct because claim_buffer sounds like
we are grabbing the buffer, I'm not sure on the best naming to have
here. iio_device_claim_buffer_m maybe?  Ugly though and
these are super rare so maybe this isn't a particularly major
concern.

Given I think the people maintaining most out of tree drivers
are Analog Devices maybe this is a question Nuno can answer
for us?


> 
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>

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

[Jonathan Cameron]

On Tue, 23 Dec 2025 11:23:24 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 12/11/25 8:45 PM, 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.  
> 
> Would be useful to repeat this in a comment in the code.
> 
Given David did a much more thorough review than me I've just
added a few comments on top.

Jonathan

> > + * Context: Can sleep
> > + */
> > +#define IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) \
> > +	ACQUIRE(__priv__iio_dev_mode_lock_try_direct, _var)(_dev)
> > +
> > +/**
> > + * IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) - Tries to acquire the buffer mode
> > + *                                           lock with automatic release
> > + * @_dev: IIO device instance
> > + * @_var: Dummy variable identifier to store acquire result
> > + *
> > + * Tries to acquire the direct mode lock and automatically releases it at the
> > + * end of the scope. It most be paired with IIO_DEV_ACQUIRE_ERR(), for example::
> > + *
> > + *	IIO_DEV_ACQUIRE_BUFFER_MODE(indio_dev, claim);
> > + *	if (IIO_DEV_ACQUIRE_ERR(&claim))
> > + *		return IRQ_HANDLED;
> > + *
> > + * Context: Can sleep
> > + */
> > +#define IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) \
> > +	ACQUIRE(__priv__iio_dev_mode_lock_try_buffer, _var)(_dev)

This one doesn't actually have any users I think?  Do we want to introduce
it without them? I'm not sure we do.

> > +
> > +/**
> > + * IIO_DEV_ACQUIRE_ERR() - ACQUIRE_ERR() wrapper
> > + * @_var: Dummy variable passed to IIO_DEV_ACQUIRE_*_MODE()
> > + *
> > + * Return: true on success, false on error  
> 
> This could be clarified more. Based on the example, this sounds
> backwards.
> 
> 	Returns: true if acquiring the mode failed, otherwise false.
> 
> > + */
> > +#define IIO_DEV_ACQUIRE_ERR(_var_ptr) \
> > +	ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_buffer, _var_ptr)  
> 
> There is no error code here, so calling it "ERR" seems wrong.
> Maybe IIO_DEV_ACQUIRE_FAILED()?
> 
> > +
> > +/**
> > + * IIO_DEV_GUARD_ANY_MODE - Acquires the mode lock with automatic release
> > + * @_dev: IIO device instance  
> 
> It would be helpful to explain more about the use case here and that this
> is used rarely.
> 
> The point to get across is that it is used when we need to do something
> that doesn't depend on the current mode, but would be affected by a mode
> switch. So it guards against changing the mode without caring what the
> current mode is. If it is in buffer mode, it stays in buffer mode, otherwise
> direct mode is claimed.
> 
> > + *
> > + * 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.  
> 
> Might as well add Context: here like the others.
> 
> > + */
> > +#define IIO_DEV_GUARD_ANY_MODE(_dev) \  
> 
> Accordingly, I would be inclined to call it IIO_DEV_GUARD_CURRENT_MODE()

Agreed. That's a clearer name. 

> 
> > +	guard(__priv__iio_dev_mode_lock)(_dev)
> > +
> >  extern const struct bus_type iio_bus_type;
> >  
> >  /**
> >   
> 

Re: [PATCH v2 1/7] iio: core: Add and export __iio_dev_mode_lock()

[Kurt Borja]

Hi David,

On Tue Dec 23, 2025 at 12:19 PM -05, David Lechner wrote:
> On 12/11/25 8:45 PM, Kurt Borja wrote:
>> Add infallible wrappers around the internal IIO mode lock.
>
> Not sure what "infallible" is supposed to mean in this context. Maybe
> referring to autocleanup?

I meant wrappers that do not fail i.e. return void. Should I word it
differently?

>
>> 
>> As mentioned in the documentation, this is not meant to be used by
>> drivers, instead this will aid in the eventual addition of cleanup
>> classes around conditional locks.
>> 
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>>  drivers/iio/industrialio-core.c | 30 ++++++++++++++++++++++++++++++
>>  include/linux/iio/iio.h         |  3 +++
>>  2 files changed, 33 insertions(+)
>> 
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index f69deefcfb6f..1cce2d1ecef1 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -2171,6 +2171,36 @@ int __devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev,
>>  }
>>  EXPORT_SYMBOL_GPL(__devm_iio_device_register);
>>  
>> +/**
>> + * __iio_dev_mode_lock - Locks the current IIO device mode
>> + * @indio_dev: the iio_dev associated with the device
>> + *
>> + * If the device is either in direct or buffer mode, it's guaranteed to stay
>> + * that way until __iio_dev_mode_unlock() is called.
>> + *
>> + * This function is not meant to be used directly by drivers to protect internal
>> + * state, a driver should have it's own mechanisms for that matter.
>> + *
>> + * There are very few cases where a driver actually needs to lock any mode. It's
>> + * *strongly* recommended to use iio_device_claim_direct() or
>
> I wouldn't even say "strongly recommend". Just say "use these instead".
>
> In the exceptions, likely the autocleanup version should be used as is
> the case currently.

Sure!

-- 
 ~ Kurt

Re: [PATCH v2 2/7] iio: core: Refactor iio_device_claim_direct() implementation

[Kurt Borja]

On Tue Dec 23, 2025 at 12:20 PM -05, David Lechner wrote:
> On 12/11/25 8:45 PM, Kurt Borja wrote:
>> In order to eventually unify the locking API, implement
>> iio_device_claim_direct() fully inline, with the use of
>> __iio_dev_mode_lock(), which takes care of sparse annotations.
>> 
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---

...

>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index aecda887d833..76398dbfa5ca 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -664,30 +664,42 @@ 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);
>> -bool __iio_device_claim_direct(struct iio_dev *indio_dev);
>> -void __iio_device_release_direct(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()
>
> I think we should leave a comment that says these functions need to stay
> as static inline so that sparse works. Although I'm wondering how sparse

I'll keep that part!

> could still work since __acquire() and __release() are removed here. IIRC
> it was a bit finicky about that.

Now __iio_dev_mode_{lock,unlock}() are declared with __acquires() and
__releases() tags. These are then used conditionally in
iio_device_claim_direct().

I did test every driver with `make C=2` and it seems to work in the same
way.

...

>> +/**
>> + * iio_device_release_direct - Releases claim on direct mode
>> + * @indio_dev:	the iio_dev associated with the device
>> + *
>> + * Release the claim. Device is no longer guaranteed to stay
>> + * in direct mode.
>> + *
>> + * Use with iio_device_claim_direct()
>> + */
>>  static inline void iio_device_release_direct(struct iio_dev *indio_dev)
>
> It could make more sense to make this:
>
> #define iio_device_release_direct __iio_dev_mode_unlock
>
> To make it clear that the auto cleanup functions that come later
> don't call iio_device_release_direct() but rather call
> __iio_dev_mode_unlock() directly.
>
> Otherwise, someone could be tempted to modify the iio_device_release_direct()
> function and the changes would not have an effect when auto cleanup is used.
>
> (Same applies to the release buffer mode function.)
>

Agree. I'll implement it like this.

-- 
 ~ Kurt

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

[Kurt Borja]

On Tue Dec 23, 2025 at 12:23 PM -05, David Lechner wrote:
> On 12/11/25 8:45 PM, 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.
>
> Would be useful to repeat this in a comment in the code.

Sure!

>
>> 
>> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>>  include/linux/iio/iio.h | 83 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 83 insertions(+)
>> 
>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index f8a7ef709210..c84853c7a37f 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>
>> @@ -739,6 +740,88 @@ static inline void iio_device_release_buffer_mode(struct iio_dev *indio_dev)
>>  	__iio_dev_mode_unlock(indio_dev);
>>  }
>>  
>> +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_buffer,
>> +		  iio_device_claim_buffer_mode(_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
>> + * @_var: Dummy variable identifier to store acquire result
>
> It's not a dummy if it does something. :-) (so we can drop that word)

That's true :).

>
> Also, I would call it _claim instead of _var to to match the example
> and encourage people to use the same name everywhere.
>
> And for that matter, we don't really need the leading underscores in either
> parameter since there are no name conflicts.

Ack.

...

>> +/**
>> + * IIO_DEV_ACQUIRE_ERR() - ACQUIRE_ERR() wrapper
>> + * @_var: Dummy variable passed to IIO_DEV_ACQUIRE_*_MODE()
>> + *
>> + * Return: true on success, false on error
>
> This could be clarified more. Based on the example, this sounds
> backwards.
>
> 	Returns: true if acquiring the mode failed, otherwise false.
>
>> + */
>> +#define IIO_DEV_ACQUIRE_ERR(_var_ptr) \
>> +	ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_buffer, _var_ptr)
>
> There is no error code here, so calling it "ERR" seems wrong.
> Maybe IIO_DEV_ACQUIRE_FAILED()?

Here I'd prefer to keep it as _ERR so users can make the immediate
association. But I don't feel strongly about it.

>
>> +
>> +/**
>> + * IIO_DEV_GUARD_ANY_MODE - Acquires the mode lock with automatic release
>> + * @_dev: IIO device instance
>
> It would be helpful to explain more about the use case here and that this
> is used rarely.
>
> The point to get across is that it is used when we need to do something
> that doesn't depend on the current mode, but would be affected by a mode
> switch. So it guards against changing the mode without caring what the
> current mode is. If it is in buffer mode, it stays in buffer mode, otherwise
> direct mode is claimed.

I'll add a comment on this.

>
>> + *
>> + * 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.
>
> Might as well add Context: here like the others.
>
>> + */
>> +#define IIO_DEV_GUARD_ANY_MODE(_dev) \
>
> Accordingly, I would be inclined to call it IIO_DEV_GUARD_CURRENT_MODE()

I like this better.

Thank you for your comments!

-- 
 ~ Kurt

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

[Kurt Borja]

On Sat Dec 27, 2025 at 9:53 AM -05, Jonathan Cameron wrote:
> On Tue, 23 Dec 2025 11:23:24 -0600
> David Lechner <dlechner@baylibre.com> wrote:
>
>> On 12/11/25 8:45 PM, 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.  
>> 
>> Would be useful to repeat this in a comment in the code.
>> 
> Given David did a much more thorough review than me I've just
> added a few comments on top.

Hi Jonathan,

...

>> > +/**
>> > + * IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) - Tries to acquire the buffer mode
>> > + *                                           lock with automatic release
>> > + * @_dev: IIO device instance
>> > + * @_var: Dummy variable identifier to store acquire result
>> > + *
>> > + * Tries to acquire the direct mode lock and automatically releases it at the
>> > + * end of the scope. It most be paired with IIO_DEV_ACQUIRE_ERR(), for example::
>> > + *
>> > + *	IIO_DEV_ACQUIRE_BUFFER_MODE(indio_dev, claim);
>> > + *	if (IIO_DEV_ACQUIRE_ERR(&claim))
>> > + *		return IRQ_HANDLED;
>> > + *
>> > + * Context: Can sleep
>> > + */
>> > +#define IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) \
>> > +	ACQUIRE(__priv__iio_dev_mode_lock_try_buffer, _var)(_dev)
>
> This one doesn't actually have any users I think?  Do we want to introduce
> it without them? I'm not sure we do.

I can drop it.

I would personally keep it for completeness, but you're right, the
buffer variant is very niche in general.

-- 
 ~ Kurt

Re: [PATCH v2 1/7] iio: core: Add and export __iio_dev_mode_lock()

[David Lechner]

On 12/27/25 11:51 AM, Kurt Borja wrote:
> Hi David,
> 
> On Tue Dec 23, 2025 at 12:19 PM -05, David Lechner wrote:
>> On 12/11/25 8:45 PM, Kurt Borja wrote:
>>> Add infallible wrappers around the internal IIO mode lock.
>>
>> Not sure what "infallible" is supposed to mean in this context. Maybe
>> referring to autocleanup?
> 
> I meant wrappers that do not fail i.e. return void. Should I word it
> differently?
> 
Ah, I think you mean "unconditional" - as in "not a conditional
lock".

In the body of the commit message, you could spell this out more that
normally, this is used as a conditional lock, but to prepare for autocleanup
and we need an unconditional lock for the base guard() to be extended
with the conditional ACQUIRE().

Re: [PATCH v2 3/7] iio: core: Match iio_device_claim_*() semantics and implementation

[Kurt Borja]

On Sat Dec 27, 2025 at 9:47 AM -05, Jonathan Cameron wrote:
> On Thu, 11 Dec 2025 21:45:21 -0500
> Kurt Borja <kuurtb@gmail.com> wrote:
>
>> Implement iio_device_claim_buffer_mode() fully inline with the use of
>> __iio_dev_mode_lock(), which takes care of sparse annotations.
>> 
>> To completely match iio_device_claim_direct() semantics, we need to
>> also change iio_device_claim_buffer_mode() return semantics to usual
>> true/false conditional lock semantics.
>
> I wasn't rushing to review this set because I want it to sit
> a little longer than a typical series to get more eyes on it.
> Anyhow, long enough for this version at least!
>
> Whilst I find it hard to care strongly about out of tree drivers
> and in place flip of the return logic seems a bit unfair on anyone
> trying to keep those rebased on mainline!
>
> So with that in mind, maybe we need to name it differently even
> if we are getting rid of the old implementation all in one patch.

You're right, I didn't really consider out-of-tree drivers.

>
> Given earlier discussion about this one being rather more tricky
> to name than the claim_direct because claim_buffer sounds like
> we are grabbing the buffer, I'm not sure on the best naming to have
> here. iio_device_claim_buffer_m maybe?  Ugly though and
> these are super rare so maybe this isn't a particularly major
> concern.

Yes, it's a bit ugly, but as I proposed in the cover letter, if we go
for a full API rename, it shouldn't matter for now?

What do you think about that?

I'll go for iio_device_claim_buffer_m() if I can't think of something
better.

>
> Given I think the people maintaining most out of tree drivers
> are Analog Devices maybe this is a question Nuno can answer
> for us?

-- 
 ~ Kurt

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

[David Lechner]

On 12/27/25 12:04 PM, Kurt Borja wrote:
> On Tue Dec 23, 2025 at 12:23 PM -05, David Lechner wrote:
>> On 12/11/25 8:45 PM, 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.
>>>
>>

...

>>> + */
>>> +#define IIO_DEV_ACQUIRE_ERR(_var_ptr) \
>>> +	ACQUIRE_ERR(__priv__iio_dev_mode_lock_try_buffer, _var_ptr)
>>
>> There is no error code here, so calling it "ERR" seems wrong.
>> Maybe IIO_DEV_ACQUIRE_FAILED()?
> 
> Here I'd prefer to keep it as _ERR so users can make the immediate
> association. But I don't feel strongly about it.

I'm afraid I fail to make any association with _ERR() and something that
doesn't have an error code.

Is this about following the established pattern with ACQUIRE() and
ACQUIRE_ERR()? Usually, I am strongly in favor of following patterns
like this. But in this case, I just doesn't make logical sense.

Re: [PATCH v2 3/7] iio: core: Match iio_device_claim_*() semantics and implementation

[David Lechner]

On 12/27/25 12:14 PM, Kurt Borja wrote:
> On Sat Dec 27, 2025 at 9:47 AM -05, Jonathan Cameron wrote:
>> On Thu, 11 Dec 2025 21:45:21 -0500
>> Kurt Borja <kuurtb@gmail.com> wrote:
>>

...

>> Given earlier discussion about this one being rather more tricky
>> to name than the claim_direct because claim_buffer sounds like
>> we are grabbing the buffer, I'm not sure on the best naming to have
>> here. iio_device_claim_buffer_m maybe?  Ugly though and
>> these are super rare so maybe this isn't a particularly major
>> concern.
> 
> Yes, it's a bit ugly, but as I proposed in the cover letter, if we go
> for a full API rename, it shouldn't matter for now?
> 
> What do you think about that?
> 
> I'll go for iio_device_claim_buffer_m() if I can't think of something
> better.

iio_device_try_claim_buffer_mode()?

> 
>>
>> Given I think the people maintaining most out of tree drivers
>> are Analog Devices maybe this is a question Nuno can answer
>> for us?
> 

Re: [PATCH v2 3/7] iio: core: Match iio_device_claim_*() semantics and implementation

[Kurt Borja]

On Sat Dec 27, 2025 at 1:24 PM -05, David Lechner wrote:
> On 12/27/25 12:14 PM, Kurt Borja wrote:
>> On Sat Dec 27, 2025 at 9:47 AM -05, Jonathan Cameron wrote:
>>> On Thu, 11 Dec 2025 21:45:21 -0500
>>> Kurt Borja <kuurtb@gmail.com> wrote:
>>>
>
> ...
>
>>> Given earlier discussion about this one being rather more tricky
>>> to name than the claim_direct because claim_buffer sounds like
>>> we are grabbing the buffer, I'm not sure on the best naming to have
>>> here. iio_device_claim_buffer_m maybe?  Ugly though and
>>> these are super rare so maybe this isn't a particularly major
>>> concern.
>> 
>> Yes, it's a bit ugly, but as I proposed in the cover letter, if we go
>> for a full API rename, it shouldn't matter for now?
>> 
>> What do you think about that?
>> 
>> I'll go for iio_device_claim_buffer_m() if I can't think of something
>> better.
>
> iio_device_try_claim_buffer_mode()?
>

Yes, that's better.

-- 
 ~ Kurt