[PATCH v2 00/27] iio: improve handling of direct mode claim and release

[PATCH v2 00/27] iio: improve handling of direct mode claim and release

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

v2: Changes listed in individual patches.  A few typos + missing or
    missordered headers (thanks David!)

I could have just applied this with tweaks, but given the large number
of drivers touched I'd like it to be on list a little longer for
others to see it. Might as well make that the updated version.

This is the first of perhaps 4 related series, broken up mostly to keep
them bite sized enough for people to review.

Difference from RFC.
- Some refactoring etc
- Patch to drop the _scoped infrastructure.

Effectively two linked things in this series:

1) Ripping out iio_device_claim_direct_scoped()
2) Enabling use of sparse to check the claim is always released.

The iio_device_claim_direct_scoped() was an interesting experiment built
on conditional scoped guards, but it has been the source of a range of
esoteric build warnings and is awkward to use.

Several attempts have been made to improve the related handling but in the
end it looks like this is just too hard to use and too prone to tripping up
the compiler.  So time to rip it out.

Most recent discussion was around if_not_cond_guard()
https://lore.kernel.org/all/CAHk-=wi8C2yZF_y_T180-v+dSZAhps5QghS_2tKfn-+xAghYPQ@mail.gmail.com/
which looked like a promising avenue but ran into problems and
got reverted.

A large part of the advantage of scoped cleanup is that it removes the
possibility of failing to call the 'destructor' which here released the
claim on direct mode (implementation detail is that corresponds to
unlocking a mutex).  It's a shame to lose that but we do have other
infrastructure to prevent such common mistakes, lock markings + sparse.
Hence I thought we could simply enable those for existing
iio_device_claim_direct_mode() / iio_device_release_direct_mode() via
similar magic to that used for __cond_lock() that is rename existing
iio_device_claim_direct_mode to do_iio_device_claim_direct_mode and

#define iio_device_claim_direct_mode(iio_dev) \
	do_iio_device_claim_direct_mode(iio_dev) ? : ({ __acquire(iio_dev); 0; })

Unfortunatley that gives a few false positives. Seems sparse is tripping
up on this magic in some more complex switch statements.
Instead this series makes the iio specific infrastructure look more like
a normal conditional lock.  Given that a failure to claim direect mode only
ever resulted in -EBUSY, we can make it a boolean return instead.
Note the sense is switched so true now means successfully claimed the mode.

Ideally we'd use the sparse markings __cond_acquires() and __releases()
but appropriate support has not yet landed in upstream sparse. It
can be found in this thread.
https://lore.kernel.org/all/CAHk-=wjZfO9hGqJ2_hGQG3U_XzSh9_XaXze=HgPdvJbgrvASfA@mail.gmail.com/

As David pointed out on the RFC the reason this series worked at all
was because we had __acquire() in the static inline wrapper. Given that
works, it will do for now. Added some docs to stop us accidentally hiding
that wrapper in a .c file and breaking everything.

These new wrappers are called iio_device_claim_direct() and
iio_device_claim_release().  Naming is based on no one seeming bothered
that we dropped mode from the scoped case. Long term plan would be to
drop the _direct_mode() calls at the end of all these series.
That will cause some rebase complexity for drivers that race with this.
I'll handle those whilst merging.

In this first set we have a false positive that showed up with one of my
earlier attempts + dropping of all uses and the infrastructure for
iio_device_claim_direct_scoped().

I've tried to +CC people on individual patches, but in some cases there
is no active maintainer.  Hence if you are looking at all, please take
a look at a few of them!

Jonathan


Jonathan Cameron (27):
  iio: core: Rework claim and release of direct mode to work with
    sparse.
  iio: chemical: scd30: Use guard(mutex) to allow early returns
  iio: chemical: scd30: Switch to sparse friendly claim/release_direct()
  iio: temperature: tmp006: Stop using iio_device_claim_direct_scoped()
  iio: proximity: sx9310: Stop using iio_device_claim_direct_scoped()
  iio: proximity: sx9324: Stop using iio_device_claim_direct_scoped()
  iio: proximity: sx9360: Stop using iio_device_claim_direct_scoped()
  iio: accel: adxl367: Stop using iio_device_claim_direct_scoped()
  iio: adc: ad4000: Stop using iio_device_claim_direct_scoped()
  iio: adc: ad4130: Stop using iio_device_claim_direct_scoped()
  iio: adc: ad4695: Stop using iio_device_claim_direct_scoped()
  iio: adc: ad7606: Stop using iio_device_claim_direct_scoped()
  iio: adc: ad7625: Stop using iio_device_claim_direct_scoped()
  iio: adc: ad7779: Stop using iio_device_claim_direct_scoped()
  iio: adc: ad9467: Stop using iio_device_claim_direct_scoped()
  iio: adc: max1363: Stop using iio_device_claim_direct_scoped()
  iio: adc: rtq6056: Stop using iio_device_claim_direct_scoped()
  iio: adc: ti-adc161s626: Stop using iio_device_claim_direct_scoped()
  iio: adc: ti-ads1119: Stop using iio_device_claim_direct_scoped()
  iio: addac: ad74413r: Stop using iio_device_claim_direct_scoped()
  iio: chemical: ens160: Stop using iio_device_claim_direct_scoped()
  iio: dac: ad3552r-hs: Stop using iio_device_claim_direct_scoped()
  iio: dac: ad8460: Stop using iio_device_claim_direct_scoped()
  iio: dummy: Stop using iio_device_claim_direct_scoped()
  iio: imu: bmi323: Stop using iio_device_claim_direct_scoped()
  iio: light: bh1745: Stop using iio_device_claim_direct_scoped()
  iio: Drop iio_device_claim_direct_scoped() and related infrastructure

 drivers/iio/accel/adxl367.c          | 194 ++++++++++++----------
 drivers/iio/adc/ad4000.c             |  60 ++++---
 drivers/iio/adc/ad4130.c             |  18 +-
 drivers/iio/adc/ad4695.c             | 240 +++++++++++++++------------
 drivers/iio/adc/ad7606.c             |  14 +-
 drivers/iio/adc/ad7625.c             |   9 +-
 drivers/iio/adc/ad7779.c             | 101 ++++++-----
 drivers/iio/adc/ad9467.c             |  23 ++-
 drivers/iio/adc/max1363.c            | 165 +++++++++---------
 drivers/iio/adc/rtq6056.c            |  46 +++--
 drivers/iio/adc/ti-adc161s626.c      |  14 +-
 drivers/iio/adc/ti-ads1119.c         |  17 +-
 drivers/iio/addac/ad74413r.c         |  14 +-
 drivers/iio/chemical/ens160_core.c   |  32 ++--
 drivers/iio/chemical/scd30_core.c    |  70 ++++----
 drivers/iio/dac/ad3552r-hs.c         |  15 +-
 drivers/iio/dac/ad8460.c             |  18 +-
 drivers/iio/dummy/iio_simple_dummy.c | 119 +++++++------
 drivers/iio/imu/bmi323/bmi323_core.c |  44 ++---
 drivers/iio/light/bh1745.c           |  18 +-
 drivers/iio/proximity/sx9310.c       |  19 ++-
 drivers/iio/proximity/sx9324.c       |  19 ++-
 drivers/iio/proximity/sx9360.c       |  19 ++-
 drivers/iio/temperature/tmp006.c     |  33 ++--
 include/linux/iio/iio.h              |  41 +++--
 25 files changed, 777 insertions(+), 585 deletions(-)

-- 
2.48.1

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

[Jonathan Cameron]

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)
---
 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().
-- 
2.48.1

[PATCH v2 02/27] iio: chemical: scd30: Use guard(mutex) to allow early returns

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Auto cleanup based release of the lock allows for simpler code flow in a
few functions with large multiplexing style switch statements and no
common operations following the switch.

Suggested-by: David Lechner <dlechner@baylibre.com>
Cc: Tomasz Duszynski <tomasz.duszynski@octakon.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v2: Fix include ordering (David)
---
 drivers/iio/chemical/scd30_core.c | 63 ++++++++++++++-----------------
 include/linux/iio/iio.h           |  1 +
 2 files changed, 29 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
index d613c54cb28d..23ba46f7ca32 100644
--- a/drivers/iio/chemical/scd30_core.c
+++ b/drivers/iio/chemical/scd30_core.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2020 Tomasz Duszynski <tomasz.duszynski@octakon.com>
  */
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -198,112 +199,104 @@ static int scd30_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
 			  int *val, int *val2, long mask)
 {
 	struct scd30_state *state = iio_priv(indio_dev);
-	int ret = -EINVAL;
+	int ret;
 	u16 tmp;
 
-	mutex_lock(&state->lock);
+	guard(mutex)(&state->lock);
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 	case IIO_CHAN_INFO_PROCESSED:
 		if (chan->output) {
 			*val = state->pressure_comp;
-			ret = IIO_VAL_INT;
-			break;
+			return IIO_VAL_INT;
 		}
 
 		ret = iio_device_claim_direct_mode(indio_dev);
 		if (ret)
-			break;
+			return ret;
 
 		ret = scd30_read(state);
 		if (ret) {
 			iio_device_release_direct_mode(indio_dev);
-			break;
+			return ret;
 		}
 
 		*val = state->meas[chan->address];
 		iio_device_release_direct_mode(indio_dev);
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		*val = 0;
 		*val2 = 1;
-		ret = IIO_VAL_INT_PLUS_MICRO;
-		break;
+		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		ret = scd30_command_read(state, CMD_MEAS_INTERVAL, &tmp);
 		if (ret)
-			break;
+			return ret;
 
 		*val = 0;
 		*val2 = 1000000000 / tmp;
-		ret = IIO_VAL_INT_PLUS_NANO;
-		break;
+		return IIO_VAL_INT_PLUS_NANO;
 	case IIO_CHAN_INFO_CALIBBIAS:
 		ret = scd30_command_read(state, CMD_TEMP_OFFSET, &tmp);
 		if (ret)
-			break;
+			return ret;
 
 		*val = tmp;
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
 	}
-	mutex_unlock(&state->lock);
-
-	return ret;
 }
 
 static int scd30_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
 			   int val, int val2, long mask)
 {
 	struct scd30_state *state = iio_priv(indio_dev);
-	int ret = -EINVAL;
+	int ret;
 
-	mutex_lock(&state->lock);
+	guard(mutex)(&state->lock);
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		if (val)
-			break;
+			return -EINVAL;
 
 		val = 1000000000 / val2;
 		if (val < SCD30_MEAS_INTERVAL_MIN_S || val > SCD30_MEAS_INTERVAL_MAX_S)
-			break;
+			return -EINVAL;
 
 		ret = scd30_command_write(state, CMD_MEAS_INTERVAL, val);
 		if (ret)
-			break;
+			return ret;
 
 		state->meas_interval = val;
-		break;
+		return 0;
 	case IIO_CHAN_INFO_RAW:
 		switch (chan->type) {
 		case IIO_PRESSURE:
 			if (val < SCD30_PRESSURE_COMP_MIN_MBAR ||
 			    val > SCD30_PRESSURE_COMP_MAX_MBAR)
-				break;
+				return -EINVAL;
 
 			ret = scd30_command_write(state, CMD_START_MEAS, val);
 			if (ret)
-				break;
+				return ret;
 
 			state->pressure_comp = val;
-			break;
+			return 0;
 		default:
-			break;
+			return -EINVAL;
 		}
-		break;
 	case IIO_CHAN_INFO_CALIBBIAS:
 		if (val < 0 || val > SCD30_TEMP_OFFSET_MAX)
-			break;
+			return -EINVAL;
 		/*
 		 * Manufacturer does not explicitly specify min/max sensible
 		 * values hence check is omitted for simplicity.
 		 */
-		ret = scd30_command_write(state, CMD_TEMP_OFFSET / 10, val);
+		return scd30_command_write(state, CMD_TEMP_OFFSET / 10, val);
+	default:
+		return -EINVAL;
 	}
-	mutex_unlock(&state->lock);
-
-	return ret;
 }
 
 static int scd30_write_raw_get_fmt(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index fe33835b19cf..5ed03e36178f 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -10,6 +10,7 @@
 #include <linux/device.h>
 #include <linux/cdev.h>
 #include <linux/cleanup.h>
+#include <linux/compiler_types.h>
 #include <linux/slab.h>
 #include <linux/iio/types.h>
 /* IIO TODO LIST */
-- 
2.48.1

[PATCH v2 03/27] iio: chemical: scd30: Switch to sparse friendly claim/release_direct()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This driver caused a false positive with __cond_lock() style solution
but is fine with the simple boolean return approach now used.

Cc: Tomasz Duszynski <tomasz.duszynski@octakon.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/chemical/scd30_core.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
index 23ba46f7ca32..3fed6b63710f 100644
--- a/drivers/iio/chemical/scd30_core.c
+++ b/drivers/iio/chemical/scd30_core.c
@@ -211,18 +211,17 @@ static int scd30_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
 			return IIO_VAL_INT;
 		}
 
-		ret = iio_device_claim_direct_mode(indio_dev);
-		if (ret)
-			return ret;
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
 
 		ret = scd30_read(state);
 		if (ret) {
-			iio_device_release_direct_mode(indio_dev);
+			iio_device_release_direct(indio_dev);
 			return ret;
 		}
 
 		*val = state->meas[chan->address];
-		iio_device_release_direct_mode(indio_dev);
+		iio_device_release_direct(indio_dev);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		*val = 0;
-- 
2.48.1

[PATCH v2 04/27] iio: temperature: tmp006: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Cc: Antoni Pokusinski <apokusinski01@gmail.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/temperature/tmp006.c | 33 ++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/temperature/tmp006.c b/drivers/iio/temperature/tmp006.c
index 1998047a1f24..b5c94b7492f5 100644
--- a/drivers/iio/temperature/tmp006.c
+++ b/drivers/iio/temperature/tmp006.c
@@ -85,19 +85,25 @@ static int tmp006_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		if (channel->type == IIO_VOLTAGE) {
 			/* LSB is 156.25 nV */
-			iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-				ret = tmp006_read_measurement(data, TMP006_VOBJECT);
-				if (ret < 0)
-					return ret;
-			}
+			if (!iio_device_claim_direct(indio_dev))
+				return -EBUSY;
+
+			ret = tmp006_read_measurement(data, TMP006_VOBJECT);
+			iio_device_release_direct(indio_dev);
+			if (ret < 0)
+				return ret;
+
 			*val = sign_extend32(ret, 15);
 		} else if (channel->type == IIO_TEMP) {
 			/* LSB is 0.03125 degrees Celsius */
-			iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-				ret = tmp006_read_measurement(data, TMP006_TAMBIENT);
-				if (ret < 0)
-					return ret;
-			}
+			if (!iio_device_claim_direct(indio_dev))
+				return -EBUSY;
+
+			ret = tmp006_read_measurement(data, TMP006_TAMBIENT);
+			iio_device_release_direct(indio_dev);
+			if (ret < 0)
+				return ret;
+
 			*val = sign_extend32(ret, 15) >> TMP006_TAMBIENT_SHIFT;
 		} else {
 			break;
@@ -142,9 +148,8 @@ static int tmp006_write_raw(struct iio_dev *indio_dev,
 	for (i = 0; i < ARRAY_SIZE(tmp006_freqs); i++)
 		if ((val == tmp006_freqs[i][0]) &&
 		    (val2 == tmp006_freqs[i][1])) {
-			ret = iio_device_claim_direct_mode(indio_dev);
-			if (ret)
-				return ret;
+			if (!iio_device_claim_direct(indio_dev))
+				return -EBUSY;
 
 			data->config &= ~TMP006_CONFIG_CR_MASK;
 			data->config |= i << TMP006_CONFIG_CR_SHIFT;
@@ -153,7 +158,7 @@ static int tmp006_write_raw(struct iio_dev *indio_dev,
 							   TMP006_CONFIG,
 							   data->config);
 
-			iio_device_release_direct_mode(indio_dev);
+			iio_device_release_direct(indio_dev);
 			return ret;
 		}
 	return -EINVAL;
-- 
2.48.1

[PATCH v2 05/27] iio: proximity: sx9310: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/proximity/sx9310.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index 0d7f0518d4fb..b60707eba39d 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -337,19 +337,26 @@ static int sx9310_read_raw(struct iio_dev *indio_dev,
 			   int *val2, long mask)
 {
 	struct sx_common_data *data = iio_priv(indio_dev);
+	int ret;
 
 	if (chan->type != IIO_PROXIMITY)
 		return -EINVAL;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return sx_common_read_proximity(data, chan, val);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = sx_common_read_proximity(data, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_HARDWAREGAIN:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return sx9310_read_gain(data, chan, val);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = sx9310_read_gain(data, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		return sx9310_read_samp_freq(data, val, val2);
 	default:
-- 
2.48.1

[PATCH v2 06/27] iio: proximity: sx9324: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context

Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/proximity/sx9324.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
index f7819dd2775c..73d972416c01 100644
--- a/drivers/iio/proximity/sx9324.c
+++ b/drivers/iio/proximity/sx9324.c
@@ -429,16 +429,23 @@ static int sx9324_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long mask)
 {
 	struct sx_common_data *data = iio_priv(indio_dev);
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return sx_common_read_proximity(data, chan, val);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = sx_common_read_proximity(data, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_HARDWAREGAIN:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return sx9324_read_gain(data, chan, val);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = sx9324_read_gain(data, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		return sx9324_read_samp_freq(data, val, val2);
 	default:
-- 
2.48.1

[PATCH v2 07/27] iio: proximity: sx9360: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context

Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/proximity/sx9360.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
index a6ff16e33c1e..4448988d4e7e 100644
--- a/drivers/iio/proximity/sx9360.c
+++ b/drivers/iio/proximity/sx9360.c
@@ -321,16 +321,23 @@ static int sx9360_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long mask)
 {
 	struct sx_common_data *data = iio_priv(indio_dev);
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return sx_common_read_proximity(data, chan, val);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = sx_common_read_proximity(data, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_HARDWAREGAIN:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return sx9360_read_gain(data, chan, val);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = sx9360_read_gain(data, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		return sx9360_read_samp_freq(data, val, val2);
 	default:
-- 
2.48.1

[PATCH v2 08/27] iio: accel: adxl367: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context

In some cases there is a convenient wrapper function to which
the handling can be moved. Do that instead of introducing
another layer of wrappers. In others an outer wrapper is added
which claims direct mode, runs the original function with the
scoped claim logic removed, releases direct mode and then checks
for errors.

Cc: Cosmin Tanislav <demonsingur@gmail.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/accel/adxl367.c | 194 ++++++++++++++++++++----------------
 1 file changed, 106 insertions(+), 88 deletions(-)

diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
index a48ac0d7bd96..add4053e7a02 100644
--- a/drivers/iio/accel/adxl367.c
+++ b/drivers/iio/accel/adxl367.c
@@ -477,45 +477,42 @@ static int adxl367_set_fifo_watermark(struct adxl367_state *st,
 static int adxl367_set_range(struct iio_dev *indio_dev,
 			     enum adxl367_range range)
 {
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		struct adxl367_state *st = iio_priv(indio_dev);
-		int ret;
+	struct adxl367_state *st = iio_priv(indio_dev);
+	int ret;
 
-		guard(mutex)(&st->lock);
+	guard(mutex)(&st->lock);
 
-		ret = adxl367_set_measure_en(st, false);
-		if (ret)
-			return ret;
+	ret = adxl367_set_measure_en(st, false);
+	if (ret)
+		return ret;
 
-		ret = regmap_update_bits(st->regmap, ADXL367_REG_FILTER_CTL,
-					 ADXL367_FILTER_CTL_RANGE_MASK,
-					 FIELD_PREP(ADXL367_FILTER_CTL_RANGE_MASK,
-						    range));
-		if (ret)
-			return ret;
+	ret = regmap_update_bits(st->regmap, ADXL367_REG_FILTER_CTL,
+				 ADXL367_FILTER_CTL_RANGE_MASK,
+				 FIELD_PREP(ADXL367_FILTER_CTL_RANGE_MASK,
+					    range));
+	if (ret)
+		return ret;
 
-		adxl367_scale_act_thresholds(st, st->range, range);
+	adxl367_scale_act_thresholds(st, st->range, range);
 
-		/* Activity thresholds depend on range */
-		ret = _adxl367_set_act_threshold(st, ADXL367_ACTIVITY,
-						 st->act_threshold);
-		if (ret)
-			return ret;
+	/* Activity thresholds depend on range */
+	ret = _adxl367_set_act_threshold(st, ADXL367_ACTIVITY,
+					 st->act_threshold);
+	if (ret)
+		return ret;
 
-		ret = _adxl367_set_act_threshold(st, ADXL367_INACTIVITY,
-						 st->inact_threshold);
-		if (ret)
-			return ret;
+	ret = _adxl367_set_act_threshold(st, ADXL367_INACTIVITY,
+					 st->inact_threshold);
+	if (ret)
+		return ret;
 
-		ret = adxl367_set_measure_en(st, true);
-		if (ret)
-			return ret;
+	ret = adxl367_set_measure_en(st, true);
+	if (ret)
+		return ret;
 
-		st->range = range;
+	st->range = range;
 
-		return 0;
-	}
-	unreachable();
+	return 0;
 }
 
 static int adxl367_time_ms_to_samples(struct adxl367_state *st, unsigned int ms)
@@ -620,23 +617,20 @@ static int _adxl367_set_odr(struct adxl367_state *st, enum adxl367_odr odr)
 
 static int adxl367_set_odr(struct iio_dev *indio_dev, enum adxl367_odr odr)
 {
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		struct adxl367_state *st = iio_priv(indio_dev);
-		int ret;
+	struct adxl367_state *st = iio_priv(indio_dev);
+	int ret;
 
-		guard(mutex)(&st->lock);
+	guard(mutex)(&st->lock);
 
-		ret = adxl367_set_measure_en(st, false);
-		if (ret)
-			return ret;
+	ret = adxl367_set_measure_en(st, false);
+	if (ret)
+		return ret;
 
-		ret = _adxl367_set_odr(st, odr);
-		if (ret)
-			return ret;
+	ret = _adxl367_set_odr(st, odr);
+	if (ret)
+		return ret;
 
-		return adxl367_set_measure_en(st, true);
-	}
-	unreachable();
+	return adxl367_set_measure_en(st, true);
 }
 
 static int adxl367_set_temp_adc_en(struct adxl367_state *st, unsigned int reg,
@@ -725,32 +719,29 @@ static int adxl367_read_sample(struct iio_dev *indio_dev,
 			       struct iio_chan_spec const *chan,
 			       int *val)
 {
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		struct adxl367_state *st = iio_priv(indio_dev);
-		u16 sample;
-		int ret;
+	struct adxl367_state *st = iio_priv(indio_dev);
+	u16 sample;
+	int ret;
 
-		guard(mutex)(&st->lock);
+	guard(mutex)(&st->lock);
 
-		ret = adxl367_set_temp_adc_reg_en(st, chan->address, true);
-		if (ret)
-			return ret;
+	ret = adxl367_set_temp_adc_reg_en(st, chan->address, true);
+	if (ret)
+		return ret;
 
-		ret = regmap_bulk_read(st->regmap, chan->address, &st->sample_buf,
-				       sizeof(st->sample_buf));
-		if (ret)
-			return ret;
+	ret = regmap_bulk_read(st->regmap, chan->address, &st->sample_buf,
+			       sizeof(st->sample_buf));
+	if (ret)
+		return ret;
 
-		sample = FIELD_GET(ADXL367_DATA_MASK, be16_to_cpu(st->sample_buf));
-		*val = sign_extend32(sample, chan->scan_type.realbits - 1);
+	sample = FIELD_GET(ADXL367_DATA_MASK, be16_to_cpu(st->sample_buf));
+	*val = sign_extend32(sample, chan->scan_type.realbits - 1);
 
-		ret = adxl367_set_temp_adc_reg_en(st, chan->address, false);
-		if (ret)
-			return ret;
+	ret = adxl367_set_temp_adc_reg_en(st, chan->address, false);
+	if (ret)
+		return ret;
 
-		return IIO_VAL_INT;
-	}
-	unreachable();
+	return IIO_VAL_INT;
 }
 
 static int adxl367_get_status(struct adxl367_state *st, u8 *status,
@@ -852,10 +843,15 @@ static int adxl367_read_raw(struct iio_dev *indio_dev,
 			    int *val, int *val2, long info)
 {
 	struct adxl367_state *st = iio_priv(indio_dev);
+	int ret;
 
 	switch (info) {
 	case IIO_CHAN_INFO_RAW:
-		return adxl367_read_sample(indio_dev, chan, val);
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = adxl367_read_sample(indio_dev, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_ACCEL: {
@@ -912,7 +908,12 @@ static int adxl367_write_raw(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 
-		return adxl367_set_odr(indio_dev, odr);
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = adxl367_set_odr(indio_dev, odr);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	}
 	case IIO_CHAN_INFO_SCALE: {
 		enum adxl367_range range;
@@ -921,7 +922,12 @@ static int adxl367_write_raw(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 
-		return adxl367_set_range(indio_dev, range);
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = adxl367_set_range(indio_dev, range);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	}
 	default:
 		return -EINVAL;
@@ -1069,13 +1075,15 @@ static int adxl367_read_event_config(struct iio_dev *indio_dev,
 	}
 }
 
-static int adxl367_write_event_config(struct iio_dev *indio_dev,
-				      const struct iio_chan_spec *chan,
-				      enum iio_event_type type,
-				      enum iio_event_direction dir,
-				      bool state)
+static int __adxl367_write_event_config(struct iio_dev *indio_dev,
+					const struct iio_chan_spec *chan,
+					enum iio_event_type type,
+					enum iio_event_direction dir,
+					bool state)
 {
+	struct adxl367_state *st = iio_priv(indio_dev);
 	enum adxl367_activity_type act;
+	int ret;
 
 	switch (dir) {
 	case IIO_EV_DIR_RISING:
@@ -1088,28 +1096,38 @@ static int adxl367_write_event_config(struct iio_dev *indio_dev,
 		return -EINVAL;
 	}
 
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		struct adxl367_state *st = iio_priv(indio_dev);
-		int ret;
+	guard(mutex)(&st->lock);
+
+	ret = adxl367_set_measure_en(st, false);
+	if (ret)
+		return ret;
 
-		guard(mutex)(&st->lock);
+	ret = adxl367_set_act_interrupt_en(st, act, state);
+	if (ret)
+		return ret;
 
-		ret = adxl367_set_measure_en(st, false);
-		if (ret)
-			return ret;
+	ret = adxl367_set_act_en(st, act, state ? ADCL367_ACT_REF_ENABLED
+				 : ADXL367_ACT_DISABLED);
+	if (ret)
+		return ret;
 
-		ret = adxl367_set_act_interrupt_en(st, act, state);
-		if (ret)
-			return ret;
+	return adxl367_set_measure_en(st, true);
+}
 
-		ret = adxl367_set_act_en(st, act, state ? ADCL367_ACT_REF_ENABLED
-					 : ADXL367_ACT_DISABLED);
-		if (ret)
-			return ret;
+static int adxl367_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      bool state)
+{
+	int ret;
 
-		return adxl367_set_measure_en(st, true);
-	}
-	unreachable();
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = __adxl367_write_event_config(indio_dev, chan, type, dir, state);
+	iio_device_release_direct(indio_dev);
+	return ret;
 }
 
 static ssize_t adxl367_get_fifo_enabled(struct device *dev,
-- 
2.48.1

[PATCH v2 09/27] iio: adc: ad4000: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Tested-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ad4000.c | 60 +++++++++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
index 1d556a842a68..4fe8dee48da9 100644
--- a/drivers/iio/adc/ad4000.c
+++ b/drivers/iio/adc/ad4000.c
@@ -535,12 +535,16 @@ static int ad4000_read_raw(struct iio_dev *indio_dev,
 			   int *val2, long info)
 {
 	struct ad4000_state *st = iio_priv(indio_dev);
+	int ret;
 
 	switch (info) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return ad4000_single_conversion(indio_dev, chan, val);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = ad4000_single_conversion(indio_dev, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SCALE:
 		*val = st->scale_tbl[st->span_comp][0];
 		*val2 = st->scale_tbl[st->span_comp][1];
@@ -585,36 +589,46 @@ static int ad4000_write_raw_get_fmt(struct iio_dev *indio_dev,
 	}
 }
 
-static int ad4000_write_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan, int val, int val2,
-			    long mask)
+static int __ad4000_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val2)
 {
 	struct ad4000_state *st = iio_priv(indio_dev);
 	unsigned int reg_val;
 	bool span_comp_en;
 	int ret;
 
-	switch (mask) {
-	case IIO_CHAN_INFO_SCALE:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			guard(mutex)(&st->lock);
+	guard(mutex)(&st->lock);
+
+	ret = ad4000_read_reg(st, &reg_val);
+	if (ret < 0)
+		return ret;
+
+	span_comp_en = val2 == st->scale_tbl[1][1];
+	reg_val &= ~AD4000_CFG_SPAN_COMP;
+	reg_val |= FIELD_PREP(AD4000_CFG_SPAN_COMP, span_comp_en);
 
-			ret = ad4000_read_reg(st, &reg_val);
-			if (ret < 0)
-				return ret;
+	ret = ad4000_write_reg(st, reg_val);
+	if (ret < 0)
+		return ret;
 
-			span_comp_en = val2 == st->scale_tbl[1][1];
-			reg_val &= ~AD4000_CFG_SPAN_COMP;
-			reg_val |= FIELD_PREP(AD4000_CFG_SPAN_COMP, span_comp_en);
+	st->span_comp = span_comp_en;
+	return 0;
+}
 
-			ret = ad4000_write_reg(st, reg_val);
-			if (ret < 0)
-				return ret;
+static int ad4000_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	int ret;
 
-			st->span_comp = span_comp_en;
-			return 0;
-		}
-		unreachable();
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = __ad4000_write_raw(indio_dev, chan, val2);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	default:
 		return -EINVAL;
 	}
-- 
2.48.1

[PATCH v2 10/27] iio: adc: ad4130: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Cc: Cosmin Tanislav <demonsingur@gmail.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ad4130.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/ad4130.c b/drivers/iio/adc/ad4130.c
index acc241cc0a7a..061eeb9b1f8d 100644
--- a/drivers/iio/adc/ad4130.c
+++ b/drivers/iio/adc/ad4130.c
@@ -1067,13 +1067,11 @@ static int _ad4130_read_sample(struct iio_dev *indio_dev, unsigned int channel,
 static int ad4130_read_sample(struct iio_dev *indio_dev, unsigned int channel,
 			      int *val)
 {
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		struct ad4130_state *st = iio_priv(indio_dev);
+	struct ad4130_state *st = iio_priv(indio_dev);
 
-		guard(mutex)(&st->lock);
-		return _ad4130_read_sample(indio_dev, channel, val);
-	}
-	unreachable();
+	guard(mutex)(&st->lock);
+
+	return _ad4130_read_sample(indio_dev, channel, val);
 }
 
 static int ad4130_read_raw(struct iio_dev *indio_dev,
@@ -1083,10 +1081,16 @@ static int ad4130_read_raw(struct iio_dev *indio_dev,
 	struct ad4130_state *st = iio_priv(indio_dev);
 	unsigned int channel = chan->scan_index;
 	struct ad4130_setup_info *setup_info = &st->chans_info[channel].setup;
+	int ret;
 
 	switch (info) {
 	case IIO_CHAN_INFO_RAW:
-		return ad4130_read_sample(indio_dev, channel, val);
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
+		ret = ad4130_read_sample(indio_dev, channel, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SCALE: {
 		guard(mutex)(&st->lock);
 		*val = st->scale_tbls[setup_info->ref_sel][setup_info->pga][0];
-- 
2.48.1

[PATCH v2 11/27] iio: adc: ad4695: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.  In some cases code is factored
out to utility functions that can do a direct return with the
claim and release around the call.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v2: Typo in commit description (David).
Note there are several sets current in flight that touch this driver.
I'll rebase as necessary depending on what order the dependencies resolve.
---
 drivers/iio/adc/ad4695.c | 240 ++++++++++++++++++++++-----------------
 1 file changed, 133 insertions(+), 107 deletions(-)

diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
index 13cf01d35301..4bb22f4d739b 100644
--- a/drivers/iio/adc/ad4695.c
+++ b/drivers/iio/adc/ad4695.c
@@ -738,6 +738,25 @@ static int ad4695_read_one_sample(struct ad4695_state *st, unsigned int address)
 	return spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
 }
 
+static int __ad4695_read_info_raw(struct ad4695_state *st,
+				  struct iio_chan_spec const *chan,
+				  int *val)
+{
+	u8 realbits = chan->scan_type.realbits;
+	int ret;
+
+	ret = ad4695_read_one_sample(st, chan->address);
+	if (ret)
+		return ret;
+
+	if (chan->scan_type.sign == 's')
+		*val = sign_extend32(st->raw_data, realbits - 1);
+	else
+		*val = st->raw_data;
+
+	return IIO_VAL_INT;
+}
+
 static int ad4695_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long mask)
@@ -750,19 +769,12 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			ret = ad4695_read_one_sample(st, chan->address);
-			if (ret)
-				return ret;
-
-			if (chan->scan_type.sign == 's')
-				*val = sign_extend32(st->raw_data, realbits - 1);
-			else
-				*val = st->raw_data;
 
-			return IIO_VAL_INT;
-		}
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = __ad4695_read_info_raw(st, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
@@ -800,45 +812,45 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_CALIBSCALE:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
-			iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-				ret = regmap_read(st->regmap16,
-					AD4695_REG_GAIN_IN(chan->scan_index),
-					&reg_val);
-				if (ret)
-					return ret;
+			if (!iio_device_claim_direct(indio_dev))
+				return -EBUSY;
+			ret = regmap_read(st->regmap16,
+					  AD4695_REG_GAIN_IN(chan->scan_index),
+					  &reg_val);
+			iio_device_release_direct(indio_dev);
+			if (ret)
+				return ret;
 
-				*val = reg_val;
-				*val2 = 15;
+			*val = reg_val;
+			*val2 = 15;
 
-				return IIO_VAL_FRACTIONAL_LOG2;
-			}
-			unreachable();
+			return IIO_VAL_FRACTIONAL_LOG2;
 		default:
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_CALIBBIAS:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
-			iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-				ret = regmap_read(st->regmap16,
-					AD4695_REG_OFFSET_IN(chan->scan_index),
-					&reg_val);
-				if (ret)
-					return ret;
-
-				tmp = sign_extend32(reg_val, 15);
+			if (!iio_device_claim_direct(indio_dev))
+				return -EBUSY;
+			ret = regmap_read(st->regmap16,
+					  AD4695_REG_OFFSET_IN(chan->scan_index),
+					  &reg_val);
+			iio_device_release_direct(indio_dev);
+			if (ret)
+				return ret;
 
-				*val = tmp / 4;
-				*val2 = abs(tmp) % 4 * MICRO / 4;
+			tmp = sign_extend32(reg_val, 15);
 
-				if (tmp < 0 && *val2) {
-					*val *= -1;
-					*val2 *= -1;
-				}
+			*val = tmp / 4;
+			*val2 = abs(tmp) % 4 * MICRO / 4;
 
-				return IIO_VAL_INT_PLUS_MICRO;
+			if (tmp < 0 && *val2) {
+				*val *= -1;
+				*val2 *= -1;
 			}
-			unreachable();
+
+			return IIO_VAL_INT_PLUS_MICRO;
 		default:
 			return -EINVAL;
 		}
@@ -847,64 +859,75 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
-static int ad4695_write_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan,
-			    int val, int val2, long mask)
+static int __ad4695_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
 {
 	struct ad4695_state *st = iio_priv(indio_dev);
 	unsigned int reg_val;
 
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		switch (mask) {
-		case IIO_CHAN_INFO_CALIBSCALE:
-			switch (chan->type) {
-			case IIO_VOLTAGE:
-				if (val < 0 || val2 < 0)
-					reg_val = 0;
-				else if (val > 1)
-					reg_val = U16_MAX;
-				else
-					reg_val = (val * (1 << 16) +
-						   mul_u64_u32_div(val2, 1 << 16,
-								   MICRO)) / 2;
-
-				return regmap_write(st->regmap16,
-					AD4695_REG_GAIN_IN(chan->scan_index),
-					reg_val);
-			default:
-				return -EINVAL;
-			}
-		case IIO_CHAN_INFO_CALIBBIAS:
-			switch (chan->type) {
-			case IIO_VOLTAGE:
-				if (val2 >= 0 && val > S16_MAX / 4)
-					reg_val = S16_MAX;
-				else if ((val2 < 0 ? -val : val) < S16_MIN / 4)
-					reg_val = S16_MIN;
-				else if (val2 < 0)
-					reg_val = clamp_t(int,
-						-(val * 4 + -val2 * 4 / MICRO),
-						S16_MIN, S16_MAX);
-				else if (val < 0)
-					reg_val = clamp_t(int,
-						val * 4 - val2 * 4 / MICRO,
-						S16_MIN, S16_MAX);
-				else
-					reg_val = clamp_t(int,
-						val * 4 + val2 * 4 / MICRO,
-						S16_MIN, S16_MAX);
-
-				return regmap_write(st->regmap16,
-					AD4695_REG_OFFSET_IN(chan->scan_index),
-					reg_val);
-			default:
-				return -EINVAL;
-			}
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			if (val < 0 || val2 < 0)
+				reg_val = 0;
+			else if (val > 1)
+				reg_val = U16_MAX;
+			else
+				reg_val = (val * (1 << 16) +
+					   mul_u64_u32_div(val2, 1 << 16,
+							   MICRO)) / 2;
+
+			return regmap_write(st->regmap16,
+					    AD4695_REG_GAIN_IN(chan->scan_index),
+					    reg_val);
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_CALIBBIAS:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			if (val2 >= 0 && val > S16_MAX / 4)
+				reg_val = S16_MAX;
+			else if ((val2 < 0 ? -val : val) < S16_MIN / 4)
+				reg_val = S16_MIN;
+			else if (val2 < 0)
+				reg_val = clamp_t(int,
+						  -(val * 4 + -val2 * 4 / MICRO),
+						  S16_MIN, S16_MAX);
+			else if (val < 0)
+				reg_val = clamp_t(int,
+						  val * 4 - val2 * 4 / MICRO,
+						  S16_MIN, S16_MAX);
+			else
+				reg_val = clamp_t(int,
+						  val * 4 + val2 * 4 / MICRO,
+						  S16_MIN, S16_MAX);
+
+			return regmap_write(st->regmap16,
+					    AD4695_REG_OFFSET_IN(chan->scan_index),
+					    reg_val);
 		default:
 			return -EINVAL;
 		}
+	default:
+		return -EINVAL;
 	}
-	unreachable();
+}
+
+static int ad4695_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	int ret;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+	ret = __ad4695_write_raw(indio_dev, chan, val, val2, mask);
+	iio_device_release_direct(indio_dev);
+
+	return ret;
 }
 
 static int ad4695_read_avail(struct iio_dev *indio_dev,
@@ -954,26 +977,29 @@ static int ad4695_debugfs_reg_access(struct iio_dev *indio_dev,
 				     unsigned int *readval)
 {
 	struct ad4695_state *st = iio_priv(indio_dev);
-
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		if (readval) {
-			if (regmap_check_range_table(st->regmap, reg,
-						     &ad4695_regmap_rd_table))
-				return regmap_read(st->regmap, reg, readval);
-			if (regmap_check_range_table(st->regmap16, reg,
-						     &ad4695_regmap16_rd_table))
-				return regmap_read(st->regmap16, reg, readval);
-		} else {
-			if (regmap_check_range_table(st->regmap, reg,
-						     &ad4695_regmap_wr_table))
-				return regmap_write(st->regmap, reg, writeval);
-			if (regmap_check_range_table(st->regmap16, reg,
-						     &ad4695_regmap16_wr_table))
-				return regmap_write(st->regmap16, reg, writeval);
-		}
+	int ret = -EINVAL;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	if (readval) {
+		if (regmap_check_range_table(st->regmap, reg,
+					     &ad4695_regmap_rd_table))
+			ret = regmap_read(st->regmap, reg, readval);
+		if (regmap_check_range_table(st->regmap16, reg,
+					     &ad4695_regmap16_rd_table))
+			ret = regmap_read(st->regmap16, reg, readval);
+	} else {
+		if (regmap_check_range_table(st->regmap, reg,
+					     &ad4695_regmap_wr_table))
+			ret = regmap_write(st->regmap, reg, writeval);
+		if (regmap_check_range_table(st->regmap16, reg,
+					     &ad4695_regmap16_wr_table))
+			ret = regmap_write(st->regmap16, reg, writeval);
 	}
+	iio_device_release_direct(indio_dev);
 
-	return -EINVAL;
+	return ret;
 }
 
 static const struct iio_info ad4695_info = {
-- 
2.48.1

[PATCH v2 12/27] iio: adc: ad7606: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Cc: Guillaume Stols <gstols@baylibre.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ad7606.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index d8e3c7a43678..39b5bf09e46d 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -752,13 +752,13 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
 
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			ret = ad7606_scan_direct(indio_dev, chan->address, val);
-			if (ret < 0)
-				return ret;
-			return IIO_VAL_INT;
-		}
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = ad7606_scan_direct(indio_dev, chan->address, val);
+		iio_device_release_direct(indio_dev);
+		if (ret < 0)
+			return ret;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		if (st->sw_mode_en)
 			ch = chan->address;
-- 
2.48.1

[PATCH v2 13/27] iio: adc: ad7625: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Reviewed-by: Trevor Gamblin <tgamblin@baylibre.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ad7625.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad7625.c b/drivers/iio/adc/ad7625.c
index adb4e25dd7ea..0466c0c7eae4 100644
--- a/drivers/iio/adc/ad7625.c
+++ b/drivers/iio/adc/ad7625.c
@@ -248,12 +248,15 @@ static int ad7625_write_raw(struct iio_dev *indio_dev,
 			    int val, int val2, long info)
 {
 	struct ad7625_state *st = iio_priv(indio_dev);
+	int ret;
 
 	switch (info) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return ad7625_set_sampling_freq(st, val);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = ad7625_set_sampling_freq(st, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	default:
 		return -EINVAL;
 	}
-- 
2.48.1

[PATCH v2 14/27] iio: adc: ad7779: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Cc: Ramona Alexandra Nechita <ramona.nechita@analog.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ad7779.c | 101 ++++++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c
index 2537dab69a35..a5d87faa5e12 100644
--- a/drivers/iio/adc/ad7779.c
+++ b/drivers/iio/adc/ad7779.c
@@ -467,59 +467,82 @@ static int ad7779_set_calibbias(struct ad7779_state *st, int channel, int val)
 				calibbias[2]);
 }
 
+static int __ad7779_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int *val,
+			     int *val2, long mask)
+{
+	struct ad7779_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		ret = ad7779_get_calibscale(st, chan->channel);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		*val2 = GAIN_REL;
+		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		ret = ad7779_get_calibbias(st, chan->channel);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = st->sampling_freq;
+		if (*val < 0)
+			return -EINVAL;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int ad7779_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan, int *val,
 			   int *val2, long mask)
 {
-	struct ad7779_state *st = iio_priv(indio_dev);
 	int ret;
 
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		switch (mask) {
-		case IIO_CHAN_INFO_CALIBSCALE:
-			ret = ad7779_get_calibscale(st, chan->channel);
-			if (ret < 0)
-				return ret;
-			*val = ret;
-			*val2 = GAIN_REL;
-			return IIO_VAL_FRACTIONAL;
-		case IIO_CHAN_INFO_CALIBBIAS:
-			ret = ad7779_get_calibbias(st, chan->channel);
-			if (ret < 0)
-				return ret;
-			*val = ret;
-			return IIO_VAL_INT;
-		case IIO_CHAN_INFO_SAMP_FREQ:
-			*val = st->sampling_freq;
-			if (*val < 0)
-				return -EINVAL;
-			return IIO_VAL_INT;
-		default:
-			return -EINVAL;
-		}
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = __ad7779_read_raw(indio_dev, chan, val, val2, mask);
+	iio_device_release_direct(indio_dev);
+	return ret;
+}
+
+static int __ad7779_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2,
+			      long mask)
+{
+	struct ad7779_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		return ad7779_set_calibscale(st, chan->channel, val2);
+	case IIO_CHAN_INFO_CALIBBIAS:
+		return ad7779_set_calibbias(st, chan->channel, val);
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return ad7779_set_sampling_frequency(st, val);
+	default:
+		return -EINVAL;
 	}
-	unreachable();
 }
 
 static int ad7779_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan, int val, int val2,
 			    long mask)
 {
-	struct ad7779_state *st = iio_priv(indio_dev);
+	int ret;
 
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		switch (mask) {
-		case IIO_CHAN_INFO_CALIBSCALE:
-			return ad7779_set_calibscale(st, chan->channel, val2);
-		case IIO_CHAN_INFO_CALIBBIAS:
-			return ad7779_set_calibbias(st, chan->channel, val);
-		case IIO_CHAN_INFO_SAMP_FREQ:
-			return ad7779_set_sampling_frequency(st, val);
-		default:
-			return -EINVAL;
-		}
-	}
-	unreachable();
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = __ad7779_write_raw(indio_dev, chan, val, val2, mask);
+	iio_device_release_direct(indio_dev);
+	return ret;
 }
 
 static int ad7779_buffer_preenable(struct iio_dev *indio_dev)
-- 
2.48.1

[PATCH v2 15/27] iio: adc: ad9467: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context. Also use guard() to simplify mutex
unlock paths.

Cc: Nuno Sa <nuno.sa@analog.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ad9467.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index f30119b42ba0..f7a9f46ea0dc 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -813,6 +813,18 @@ static int ad9467_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int __ad9467_update_clock(struct ad9467_state *st, long r_clk)
+{
+	int ret;
+
+	ret = clk_set_rate(st->clk, r_clk);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+	return ad9467_calibrate(st);
+}
+
 static int ad9467_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int val, int val2, long mask)
@@ -842,14 +854,11 @@ static int ad9467_write_raw(struct iio_dev *indio_dev,
 		if (sample_rate == r_clk)
 			return 0;
 
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			ret = clk_set_rate(st->clk, r_clk);
-			if (ret)
-				return ret;
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
 
-			guard(mutex)(&st->lock);
-			ret = ad9467_calibrate(st);
-		}
+		ret = __ad9467_update_clock(st, r_clk);
+		iio_device_release_direct(indio_dev);
 		return ret;
 	default:
 		return -EINVAL;
-- 
2.48.1

[PATCH v2 16/27] iio: adc: max1363: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/max1363.c | 165 ++++++++++++++++++++------------------
 1 file changed, 89 insertions(+), 76 deletions(-)

diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
index e8d731bc34e0..35717ec082ce 100644
--- a/drivers/iio/adc/max1363.c
+++ b/drivers/iio/adc/max1363.c
@@ -364,55 +364,52 @@ static int max1363_read_single_chan(struct iio_dev *indio_dev,
 				    int *val,
 				    long m)
 {
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		s32 data;
-		u8 rxbuf[2];
-		struct max1363_state *st = iio_priv(indio_dev);
-		struct i2c_client *client = st->client;
-
-		guard(mutex)(&st->lock);
-
-		/*
-		 * If monitor mode is enabled, the method for reading a single
-		 * channel will have to be rather different and has not yet
-		 * been implemented.
-		 *
-		 * Also, cannot read directly if buffered capture enabled.
-		 */
-		if (st->monitor_on)
-			return -EBUSY;
+	s32 data;
+	u8 rxbuf[2];
+	struct max1363_state *st = iio_priv(indio_dev);
+	struct i2c_client *client = st->client;
 
-		/* Check to see if current scan mode is correct */
-		if (st->current_mode != &max1363_mode_table[chan->address]) {
-			int ret;
+	guard(mutex)(&st->lock);
 
-			/* Update scan mode if needed */
-			st->current_mode = &max1363_mode_table[chan->address];
-			ret = max1363_set_scan_mode(st);
-			if (ret < 0)
-				return ret;
-		}
-		if (st->chip_info->bits != 8) {
-			/* Get reading */
-			data = st->recv(client, rxbuf, 2);
-			if (data < 0)
-				return data;
-
-			data = get_unaligned_be16(rxbuf) &
-				((1 << st->chip_info->bits) - 1);
-		} else {
-			/* Get reading */
-			data = st->recv(client, rxbuf, 1);
-			if (data < 0)
-				return data;
-
-			data = rxbuf[0];
-		}
-		*val = data;
+	/*
+	 * If monitor mode is enabled, the method for reading a single
+	 * channel will have to be rather different and has not yet
+	 * been implemented.
+	 *
+	 * Also, cannot read directly if buffered capture enabled.
+	 */
+	if (st->monitor_on)
+		return -EBUSY;
+
+	/* Check to see if current scan mode is correct */
+	if (st->current_mode != &max1363_mode_table[chan->address]) {
+		int ret;
+
+		/* Update scan mode if needed */
+		st->current_mode = &max1363_mode_table[chan->address];
+		ret = max1363_set_scan_mode(st);
+		if (ret < 0)
+			return ret;
+	}
+	if (st->chip_info->bits != 8) {
+		/* Get reading */
+		data = st->recv(client, rxbuf, 2);
+		if (data < 0)
+			return data;
+
+		data = get_unaligned_be16(rxbuf) &
+			((1 << st->chip_info->bits) - 1);
+	} else {
+		/* Get reading */
+		data = st->recv(client, rxbuf, 1);
+		if (data < 0)
+			return data;
 
-		return 0;
+		data = rxbuf[0];
 	}
-	unreachable();
+	*val = data;
+
+	return 0;
 }
 
 static int max1363_read_raw(struct iio_dev *indio_dev,
@@ -426,7 +423,11 @@ static int max1363_read_raw(struct iio_dev *indio_dev,
 
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+
 		ret = max1363_read_single_chan(indio_dev, chan, val, m);
+		iio_device_release_direct(indio_dev);
 		if (ret < 0)
 			return ret;
 		return IIO_VAL_INT;
@@ -947,46 +948,58 @@ static inline int __max1363_check_event_mask(int thismask, int checkmask)
 	return ret;
 }
 
-static int max1363_write_event_config(struct iio_dev *indio_dev,
-	const struct iio_chan_spec *chan, enum iio_event_type type,
+static int __max1363_write_event_config(struct max1363_state *st,
+	const struct iio_chan_spec *chan,
 	enum iio_event_direction dir, bool state)
 {
-	struct max1363_state *st = iio_priv(indio_dev);
-
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		int number = chan->channel;
-		u16 unifiedmask;
-		int ret;
+	int number = chan->channel;
+	u16 unifiedmask;
+	int ret;
 
-		guard(mutex)(&st->lock);
+	guard(mutex)(&st->lock);
 
-		unifiedmask = st->mask_low | st->mask_high;
-		if (dir == IIO_EV_DIR_FALLING) {
+	unifiedmask = st->mask_low | st->mask_high;
+	if (dir == IIO_EV_DIR_FALLING) {
 
-			if (state == 0)
-				st->mask_low &= ~(1 << number);
-			else {
-				ret = __max1363_check_event_mask((1 << number),
-								 unifiedmask);
-				if (ret)
-					return ret;
-				st->mask_low |= (1 << number);
-			}
-		} else {
-			if (state == 0)
-				st->mask_high &= ~(1 << number);
-			else {
-				ret = __max1363_check_event_mask((1 << number),
-								 unifiedmask);
-				if (ret)
-					return ret;
-				st->mask_high |= (1 << number);
-			}
+		if (state == 0)
+			st->mask_low &= ~(1 << number);
+		else {
+			ret = __max1363_check_event_mask((1 << number),
+							 unifiedmask);
+			if (ret)
+				return ret;
+			st->mask_low |= (1 << number);
+		}
+	} else {
+		if (state == 0)
+			st->mask_high &= ~(1 << number);
+		else {
+			ret = __max1363_check_event_mask((1 << number),
+							 unifiedmask);
+			if (ret)
+				return ret;
+			st->mask_high |= (1 << number);
 		}
 	}
-	max1363_monitor_mode_update(st, !!(st->mask_high | st->mask_low));
 
 	return 0;
+
+}
+static int max1363_write_event_config(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, bool state)
+{
+	struct max1363_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = __max1363_write_event_config(st, chan,  dir, state);
+	iio_device_release_direct(indio_dev);
+	max1363_monitor_mode_update(st, !!(st->mask_high | st->mask_low));
+
+	return ret;
 }
 
 /*
-- 
2.48.1

[PATCH v2 17/27] iio: adc: rtq6056: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context

Reviewed-by: ChiYuan Huang <cy_huang@richtek.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/rtq6056.c | 46 ++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/adc/rtq6056.c b/drivers/iio/adc/rtq6056.c
index 337bc8b31b2c..54239df61d86 100644
--- a/drivers/iio/adc/rtq6056.c
+++ b/drivers/iio/adc/rtq6056.c
@@ -514,26 +514,37 @@ static int rtq6056_adc_read_avail(struct iio_dev *indio_dev,
 	}
 }
 
-static int rtq6056_adc_write_raw(struct iio_dev *indio_dev,
-				 struct iio_chan_spec const *chan, int val,
-				 int val2, long mask)
+static int __rtq6056_adc_write_raw(struct iio_dev *indio_dev,
+				   struct iio_chan_spec const *chan, int val,
+				   long mask)
 {
 	struct rtq6056_priv *priv = iio_priv(indio_dev);
 	const struct richtek_dev_data *devdata = priv->devdata;
 
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		switch (mask) {
-		case IIO_CHAN_INFO_SAMP_FREQ:
-			if (devdata->fixed_samp_freq)
-				return -EINVAL;
-			return rtq6056_adc_set_samp_freq(priv, chan, val);
-		case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
-			return devdata->set_average(priv, val);
-		default:
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (devdata->fixed_samp_freq)
 			return -EINVAL;
-		}
+		return rtq6056_adc_set_samp_freq(priv, chan, val);
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return devdata->set_average(priv, val);
+	default:
+		return -EINVAL;
 	}
-	unreachable();
+}
+
+static int rtq6056_adc_write_raw(struct iio_dev *indio_dev,
+				 struct iio_chan_spec const *chan, int val,
+				 int val2, long mask)
+{
+	int ret;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = __rtq6056_adc_write_raw(indio_dev, chan, val, mask);
+	iio_device_release_direct(indio_dev);
+	return ret;
 }
 
 static const char *rtq6056_channel_labels[RTQ6056_MAX_CHANNEL] = {
@@ -590,9 +601,8 @@ static ssize_t shunt_resistor_store(struct device *dev,
 	struct rtq6056_priv *priv = iio_priv(indio_dev);
 	int val, val_fract, ret;
 
-	ret = iio_device_claim_direct_mode(indio_dev);
-	if (ret)
-		return ret;
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
 
 	ret = iio_str_to_fixpoint(buf, 100000, &val, &val_fract);
 	if (ret)
@@ -601,7 +611,7 @@ static ssize_t shunt_resistor_store(struct device *dev,
 	ret = rtq6056_set_shunt_resistor(priv, val * 1000000 + val_fract);
 
 out_store:
-	iio_device_release_direct_mode(indio_dev);
+	iio_device_release_direct(indio_dev);
 
 	return ret ?: len;
 }
-- 
2.48.1

[PATCH v2 18/27] iio: adc: ti-adc161s626: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ti-adc161s626.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
index 474e733fb8e0..28aa6b80160c 100644
--- a/drivers/iio/adc/ti-adc161s626.c
+++ b/drivers/iio/adc/ti-adc161s626.c
@@ -137,13 +137,13 @@ static int ti_adc_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			ret = ti_adc_read_measurement(data, chan, val);
-			if (ret)
-				return ret;
-			return IIO_VAL_INT;
-		}
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = ti_adc_read_measurement(data, chan, val);
+		iio_device_release_direct(indio_dev);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		ret = regulator_get_voltage(data->ref);
 		if (ret < 0)
-- 
2.48.1

[PATCH v2 19/27] iio: adc: ti-ads1119: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Cc: João Paulo Gonçalves <joao.goncalves@toradex.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ti-ads1119.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1119.c b/drivers/iio/adc/ti-ads1119.c
index de019b3faa48..f120e7e21cff 100644
--- a/drivers/iio/adc/ti-ads1119.c
+++ b/drivers/iio/adc/ti-ads1119.c
@@ -336,19 +336,24 @@ static int ads1119_read_raw(struct iio_dev *indio_dev,
 {
 	struct ads1119_state *st = iio_priv(indio_dev);
 	unsigned int index = chan->address;
+	int ret;
 
 	if (index >= st->num_channels_cfg)
 		return -EINVAL;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return ads1119_single_conversion(st, chan, val, false);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = ads1119_single_conversion(st, chan, val, false);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_OFFSET:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return ads1119_single_conversion(st, chan, val, true);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = ads1119_single_conversion(st, chan, val, true);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SCALE:
 		*val = st->vref_uV / 1000;
 		*val /= st->channels_cfg[index].gain;
-- 
2.48.1

[PATCH v2 20/27] iio: addac: ad74413r: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context. Includes moving a mutex lock
into a function rather than around it to simplify the error handling.

Cc: Nuno Sa <nuno.sa@analog.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/addac/ad74413r.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index daea2bde7acf..f14d12b03da6 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -826,6 +826,8 @@ static int _ad74413r_get_single_adc_result(struct ad74413r_state *st,
 	unsigned int uval;
 	int ret;
 
+	guard(mutex)(&st->lock);
+
 	reinit_completion(&st->adc_data_completion);
 
 	ret = ad74413r_set_adc_channel_enable(st, channel, true);
@@ -865,12 +867,14 @@ static int ad74413r_get_single_adc_result(struct iio_dev *indio_dev,
 					  unsigned int channel, int *val)
 {
 	struct ad74413r_state *st = iio_priv(indio_dev);
+	int ret;
 
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		guard(mutex)(&st->lock);
-		return _ad74413r_get_single_adc_result(st, channel, val);
-	}
-	unreachable();
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = _ad74413r_get_single_adc_result(st, channel, val);
+	iio_device_release_direct(indio_dev);
+	return ret;
 }
 
 static void ad74413r_adc_to_resistance_result(int adc_result, int *val)
-- 
2.48.1

[PATCH v2 21/27] iio: chemical: ens160: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Reviewed-by: Gustavo Silva <gustavograzs@gmail.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/chemical/ens160_core.c | 32 ++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/chemical/ens160_core.c b/drivers/iio/chemical/ens160_core.c
index 48d5ad2075b6..152f81ff57e3 100644
--- a/drivers/iio/chemical/ens160_core.c
+++ b/drivers/iio/chemical/ens160_core.c
@@ -100,25 +100,35 @@ static const struct iio_chan_spec ens160_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(2),
 };
 
+static int __ens160_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val)
+{
+	struct ens160_data *data = iio_priv(indio_dev);
+	int ret;
+
+	guard(mutex)(&data->mutex);
+	ret = regmap_bulk_read(data->regmap, chan->address,
+			       &data->buf, sizeof(data->buf));
+	if (ret)
+		return ret;
+	*val = le16_to_cpu(data->buf);
+	return IIO_VAL_INT;
+}
+
 static int ens160_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long mask)
 {
-	struct ens160_data *data = iio_priv(indio_dev);
 	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			guard(mutex)(&data->mutex);
-			ret = regmap_bulk_read(data->regmap, chan->address,
-					       &data->buf, sizeof(data->buf));
-			if (ret)
-				return ret;
-			*val = le16_to_cpu(data->buf);
-			return IIO_VAL_INT;
-		}
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = __ens160_read_raw(indio_dev, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->channel2) {
 		case IIO_MOD_CO2:
-- 
2.48.1

[PATCH v2 22/27] iio: dac: ad3552r-hs: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Cc: Angelo Dureghello <adureghello@baylibre.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/dac/ad3552r-hs.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index c1dae58c1975..cd8dabb60c55 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -129,16 +129,19 @@ static int ad3552r_hs_write_raw(struct iio_dev *indio_dev,
 				int val, int val2, long mask)
 {
 	struct ad3552r_hs_state *st = iio_priv(indio_dev);
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
 		/* For RAW accesses, stay always in simple-spi. */
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			return st->data->bus_reg_write(st->back,
-				    AD3552R_REG_ADDR_CH_DAC_16B(chan->channel),
-				    val, 2);
-		}
-		unreachable();
+		ret = st->data->bus_reg_write(st->back,
+			AD3552R_REG_ADDR_CH_DAC_16B(chan->channel),
+			val, 2);
+
+		iio_device_release_direct(indio_dev);
+		return ret;
 	default:
 		return -EINVAL;
 	}
-- 
2.48.1

[PATCH v2 23/27] iio: dac: ad8460: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Cc: Mariel Tinaco <Mariel.Tinaco@analog.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/dac/ad8460.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/dac/ad8460.c b/drivers/iio/dac/ad8460.c
index 535ee3105af6..6e45686902dd 100644
--- a/drivers/iio/dac/ad8460.c
+++ b/drivers/iio/dac/ad8460.c
@@ -264,9 +264,12 @@ static ssize_t ad8460_write_toggle_en(struct iio_dev *indio_dev, uintptr_t priva
 	if (ret)
 		return ret;
 
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-		return ad8460_enable_apg_mode(state, toggle_en);
-	unreachable();
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = ad8460_enable_apg_mode(state, toggle_en);
+	iio_device_release_direct(indio_dev);
+	return ret;
 }
 
 static ssize_t ad8460_read_powerdown(struct iio_dev *indio_dev, uintptr_t private,
@@ -421,14 +424,17 @@ static int ad8460_write_raw(struct iio_dev *indio_dev,
 			    long mask)
 {
 	struct ad8460_state *state = iio_priv(indio_dev);
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
-			iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-				return ad8460_set_sample(state, val);
-			unreachable();
+			if (!iio_device_claim_direct(indio_dev))
+				return -EBUSY;
+			ret = ad8460_set_sample(state, val);
+			iio_device_release_direct(indio_dev);
+			return ret;
 		case IIO_CURRENT:
 			return regmap_write(state->regmap, AD8460_CTRL_REG(0x04),
 					    FIELD_PREP(AD8460_QUIESCENT_CURRENT_MSK, val));
-- 
2.48.1

[PATCH v2 24/27] iio: dummy: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context. Introduce two new utility functions
to allow for direct returns with claim and release of direct mode
in the caller.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/dummy/iio_simple_dummy.c | 119 ++++++++++++++++-----------
 1 file changed, 70 insertions(+), 49 deletions(-)

diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c
index 09efacaf8f78..8575d4a08963 100644
--- a/drivers/iio/dummy/iio_simple_dummy.c
+++ b/drivers/iio/dummy/iio_simple_dummy.c
@@ -267,6 +267,65 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
 	},
 };
 
+static int __iio_dummy_read_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int *val)
+{
+	struct iio_dummy_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+	switch (chan->type) {
+	case IIO_VOLTAGE:
+		if (chan->output) {
+			/* Set integer part to cached value */
+			*val = st->dac_val;
+			return IIO_VAL_INT;
+		} else if (chan->differential) {
+			if (chan->channel == 1)
+				*val = st->differential_adc_val[0];
+			else
+				*val = st->differential_adc_val[1];
+			return IIO_VAL_INT;
+		} else {
+			*val = st->single_ended_adc_val;
+			return IIO_VAL_INT;
+		}
+
+	case IIO_ACCEL:
+		*val = st->accel_val;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int __iio_dummy_read_processed(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan,
+				      int *val)
+{
+	struct iio_dummy_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+	switch (chan->type) {
+	case IIO_STEPS:
+		*val = st->steps;
+		return IIO_VAL_INT;
+	case IIO_ACTIVITY:
+		switch (chan->channel2) {
+		case IIO_MOD_RUNNING:
+			*val = st->activity_running;
+			return IIO_VAL_INT;
+		case IIO_MOD_WALKING:
+			*val = st->activity_walking;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
 /**
  * iio_dummy_read_raw() - data read function.
  * @indio_dev:	the struct iio_dev associated with this device instance
@@ -283,59 +342,21 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev,
 			      long mask)
 {
 	struct iio_dummy_state *st = iio_priv(indio_dev);
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW: /* magic value - channel value read */
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			guard(mutex)(&st->lock);
-			switch (chan->type) {
-			case IIO_VOLTAGE:
-				if (chan->output) {
-					/* Set integer part to cached value */
-					*val = st->dac_val;
-					return IIO_VAL_INT;
-				} else if (chan->differential) {
-					if (chan->channel == 1)
-						*val = st->differential_adc_val[0];
-					else
-						*val = st->differential_adc_val[1];
-					return IIO_VAL_INT;
-				} else {
-					*val = st->single_ended_adc_val;
-					return IIO_VAL_INT;
-				}
-
-			case IIO_ACCEL:
-				*val = st->accel_val;
-				return IIO_VAL_INT;
-			default:
-				return -EINVAL;
-			}
-		}
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = __iio_dummy_read_raw(indio_dev, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_PROCESSED:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			guard(mutex)(&st->lock);
-			switch (chan->type) {
-			case IIO_STEPS:
-				*val = st->steps;
-				return IIO_VAL_INT;
-			case IIO_ACTIVITY:
-				switch (chan->channel2) {
-				case IIO_MOD_RUNNING:
-					*val = st->activity_running;
-					return IIO_VAL_INT;
-				case IIO_MOD_WALKING:
-					*val = st->activity_walking;
-					return IIO_VAL_INT;
-				default:
-					return -EINVAL;
-				}
-			default:
-				return -EINVAL;
-			}
-		}
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = __iio_dummy_read_processed(indio_dev, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_OFFSET:
 		/* only single ended adc -> 7 */
 		*val = 7;
-- 
2.48.1

[PATCH v2 25/27] iio: imu: bmi323: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Cc: Julien Stephan <jstephan@baylibre.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/imu/bmi323/bmi323_core.c | 44 ++++++++++++++++------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
index 7f386c5e58b4..fc54d464a3ae 100644
--- a/drivers/iio/imu/bmi323/bmi323_core.c
+++ b/drivers/iio/imu/bmi323/bmi323_core.c
@@ -1702,26 +1702,30 @@ static int bmi323_write_raw(struct iio_dev *indio_dev,
 			    int val2, long mask)
 {
 	struct bmi323_data *data = iio_priv(indio_dev);
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return bmi323_set_odr(data,
-					      bmi323_iio_to_sensor(chan->type),
-					      val, val2);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = bmi323_set_odr(data, bmi323_iio_to_sensor(chan->type),
+				     val, val2);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SCALE:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return bmi323_set_scale(data,
-						bmi323_iio_to_sensor(chan->type),
-						val, val2);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = bmi323_set_scale(data, bmi323_iio_to_sensor(chan->type),
+				       val, val2);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
-			return bmi323_set_average(data,
-						  bmi323_iio_to_sensor(chan->type),
-						  val);
-		unreachable();
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = bmi323_set_average(data, bmi323_iio_to_sensor(chan->type),
+					 val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_ENABLE:
 		return bmi323_enable_steps(data, val);
 	case IIO_CHAN_INFO_PROCESSED: {
@@ -1747,6 +1751,7 @@ static int bmi323_read_raw(struct iio_dev *indio_dev,
 			   int *val2, long mask)
 {
 	struct bmi323_data *data = iio_priv(indio_dev);
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_PROCESSED:
@@ -1755,10 +1760,11 @@ static int bmi323_read_raw(struct iio_dev *indio_dev,
 		switch (chan->type) {
 		case IIO_ACCEL:
 		case IIO_ANGL_VEL:
-			iio_device_claim_direct_scoped(return -EBUSY,
-						       indio_dev)
-				return bmi323_read_axis(data, chan, val);
-			unreachable();
+			if (!iio_device_claim_direct(indio_dev))
+				return -EBUSY;
+			ret = bmi323_read_axis(data, chan, val);
+			iio_device_release_direct(indio_dev);
+			return ret;
 		case IIO_TEMP:
 			return bmi323_get_temp_data(data, val);
 		default:
-- 
2.48.1

[PATCH v2 26/27] iio: light: bh1745: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This complex cleanup.h use case of conditional guards has proved
to be more trouble that it is worth in terms of false positive compiler
warnings and hard to read code.

Move directly to the new claim/release_direct() that allow sparse
to check for unbalanced context.

Reviewed-by: Mudit Sharma <muditsharma.info@gmail.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/bh1745.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/light/bh1745.c b/drivers/iio/light/bh1745.c
index 3b4056be54a0..56ab5fe90ff9 100644
--- a/drivers/iio/light/bh1745.c
+++ b/drivers/iio/light/bh1745.c
@@ -426,16 +426,16 @@ static int bh1745_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			ret = regmap_bulk_read(data->regmap, chan->address,
-					       &value, 2);
-			if (ret)
-				return ret;
-			*val = value;
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
 
-			return IIO_VAL_INT;
-		}
-		unreachable();
+		ret = regmap_bulk_read(data->regmap, chan->address, &value, 2);
+		iio_device_release_direct(indio_dev);
+		if (ret)
+			return ret;
+		*val = value;
+
+		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_SCALE: {
 			guard(mutex)(&data->lock);
-- 
2.48.1

[PATCH v2 27/27] iio: Drop iio_device_claim_direct_scoped() and related infrastructure

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Scoped conditional automated cleanup turned out to be harder to work
with than expected. Despite several attempts to find a better solution
non have surfaced. As such rip it out of the IIO code.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 include/linux/iio/iio.h | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 5ed03e36178f..07a0e8132e88 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -9,7 +9,6 @@
 
 #include <linux/device.h>
 #include <linux/cdev.h>
-#include <linux/cleanup.h>
 #include <linux/compiler_types.h>
 #include <linux/slab.h>
 #include <linux/iio/types.h>
@@ -688,32 +687,6 @@ static inline void iio_device_release_direct(struct iio_dev *indio_dev)
 	__release(indio_dev);
 }
 
-/*
- * This autocleanup logic is normally used via
- * iio_device_claim_direct_scoped().
- */
-DEFINE_GUARD(iio_claim_direct, struct iio_dev *, iio_device_claim_direct_mode(_T),
-	     iio_device_release_direct_mode(_T))
-
-DEFINE_GUARD_COND(iio_claim_direct, _try, ({
-			struct iio_dev *dev;
-			int d = iio_device_claim_direct_mode(_T);
-
-			if (d < 0)
-				dev = NULL;
-			else
-				dev = _T;
-			dev;
-		}))
-
-/**
- * iio_device_claim_direct_scoped() - Scoped call to iio_device_claim_direct.
- * @fail: What to do on failure to claim device.
- * @iio_dev: Pointer to the IIO devices structure
- */
-#define iio_device_claim_direct_scoped(fail, iio_dev) \
-	scoped_cond_guard(iio_claim_direct_try, fail, iio_dev)
-
 int iio_device_claim_buffer_mode(struct iio_dev *indio_dev);
 void iio_device_release_buffer_mode(struct iio_dev *indio_dev);
 
-- 
2.48.1

Re: [PATCH v2 11/27] iio: adc: ad4695: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

On Sun,  9 Feb 2025 18:06:08 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.  In some cases code is factored
> out to utility functions that can do a direct return with the
> claim and release around the call.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v2: Typo in commit description (David).
> Note there are several sets current in flight that touch this driver.
> I'll rebase as necessary depending on what order the dependencies resolve.
I've done this rebase and applied on the testing branch of iio.git.

Would appreciate a sanity check if anyone has time though!

New code is as follows.  The one corner I was not sure on was
that for calibbias reading the direct mode claim was held for a long
time.  That seems to be unnecessary as we have a copy of osr anyway
in that function used for other purposes.

diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
index 3a1a6f96480f..9dbf326b6273 100644
--- a/drivers/iio/adc/ad4695.c
+++ b/drivers/iio/adc/ad4695.c
@@ -1029,6 +1029,25 @@ static int ad4695_read_one_sample(struct ad4695_state *st, unsigned int address)
 	return spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
 }
 
+static int __ad4695_read_info_raw(struct ad4695_state *st,
+				  struct iio_chan_spec const *chan,
+				  int *val)
+{
+	u8 realbits = chan->scan_type.realbits;
+	int ret;
+
+	ret = ad4695_read_one_sample(st, chan->address);
+	if (ret)
+		return ret;
+
+	if (chan->scan_type.sign == 's')
+		*val = sign_extend32(st->raw_data, realbits - 1);
+	else
+		*val = st->raw_data;
+
+	return IIO_VAL_INT;
+}
+
 static int ad4695_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long mask)
@@ -1049,19 +1068,12 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-			ret = ad4695_read_one_sample(st, chan->address);
-			if (ret)
-				return ret;
-
-			if (scan_type->sign == 's')
-				*val = sign_extend32(st->raw_data, realbits - 1);
-			else
-				*val = st->raw_data;
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
 
-			return IIO_VAL_INT;
-		}
-		unreachable();
+		ret = __ad4695_read_info_raw(st, chan, val);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
@@ -1099,63 +1111,62 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_CALIBSCALE:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
-			iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-				ret = regmap_read(st->regmap16,
-					AD4695_REG_GAIN_IN(chan->scan_index),
-					&reg_val);
-				if (ret)
-					return ret;
-
-				*val = reg_val;
-				*val2 = 15;
+			if (!iio_device_claim_direct(indio_dev))
+				return -EBUSY;
+			ret = regmap_read(st->regmap16,
+					  AD4695_REG_GAIN_IN(chan->scan_index),
+					  &reg_val);
+			iio_device_release_direct(indio_dev);
+			if (ret)
+				return ret;
+			*val = reg_val;
+			*val2 = 15;
 
-				return IIO_VAL_FRACTIONAL_LOG2;
-			}
-			unreachable();
+			return IIO_VAL_FRACTIONAL_LOG2;
 		default:
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_CALIBBIAS:
-		switch (chan->type) {
-		case IIO_VOLTAGE:
-			iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-				ret = regmap_read(st->regmap16,
-					AD4695_REG_OFFSET_IN(chan->scan_index),
-					&reg_val);
-				if (ret)
-					return ret;
-
-				tmp = sign_extend32(reg_val, 15);
-
-				switch (cfg->oversampling_ratio) {
-				case 1:
-					*val = tmp / 4;
-					*val2 = abs(tmp) % 4 * MICRO / 4;
-					break;
-				case 4:
-					*val = tmp / 2;
-					*val2 = abs(tmp) % 2 * MICRO / 2;
-					break;
-				case 16:
-					*val = tmp;
-					*val2 = 0;
-					break;
-				case 64:
-					*val = tmp * 2;
-					*val2 = 0;
-					break;
-				default:
-					return -EINVAL;
-				}
-
-				if (tmp < 0 && *val2) {
-					*val *= -1;
-					*val2 *= -1;
-				}
-
-				return IIO_VAL_INT_PLUS_MICRO;
+		switch (chan->type)
+		case IIO_VOLTAGE: {
+			if (!iio_device_claim_direct(indio_dev))
+				return -EBUSY;
+			ret = regmap_read(st->regmap16,
+					  AD4695_REG_OFFSET_IN(chan->scan_index),
+					  &reg_val);
+			iio_device_release_direct(indio_dev);
+			if (ret)
+				return ret;
////THIS IS THE BIT I WOuLD LIKE EYES on.
+
+			tmp = sign_extend32(reg_val, 15);
+
+			switch (osr) {
+			case 1:
+				*val = tmp / 4;
+				*val2 = abs(tmp) % 4 * MICRO / 4;
+				break;
+			case 4:
+				*val = tmp / 2;
+				*val2 = abs(tmp) % 2 * MICRO / 2;
+				break;
+			case 16:
+				*val = tmp;
+				*val2 = 0;
+				break;
+			case 64:
+				*val = tmp * 2;
+				*val2 = 0;
+				break;
+			default:
+				return -EINVAL;
+			}
+
+			if (tmp < 0 && *val2) {
+				*val *= -1;
+				*val2 *= -1;
 			}
-			unreachable();
+
+			return IIO_VAL_INT_PLUS_MICRO;
 		default:
 			return -EINVAL;
 		}
@@ -1255,72 +1266,83 @@ static unsigned int ad4695_get_calibbias(int val, int val2, int osr)
 	return clamp_t(int, val_calc, S16_MIN, S16_MAX);
 }
 
-static int ad4695_write_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan,
-			    int val, int val2, long mask)
+static int __ad4695_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
 {
 	struct ad4695_state *st = iio_priv(indio_dev);
 	unsigned int reg_val;
 	unsigned int osr = st->channels_cfg[chan->scan_index].oversampling_ratio;
 
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		switch (mask) {
-		case IIO_CHAN_INFO_CALIBSCALE:
-			switch (chan->type) {
-			case IIO_VOLTAGE:
-				if (val < 0 || val2 < 0)
-					reg_val = 0;
-				else if (val > 1)
-					reg_val = U16_MAX;
-				else
-					reg_val = (val * (1 << 16) +
-						   mul_u64_u32_div(val2, 1 << 16,
-								   MICRO)) / 2;
-
-				return regmap_write(st->regmap16,
-					AD4695_REG_GAIN_IN(chan->scan_index),
-					reg_val);
-			default:
-				return -EINVAL;
-			}
-		case IIO_CHAN_INFO_CALIBBIAS:
-			switch (chan->type) {
-			case IIO_VOLTAGE:
-				reg_val = ad4695_get_calibbias(val, val2, osr);
-				return regmap_write(st->regmap16,
-					AD4695_REG_OFFSET_IN(chan->scan_index),
-					reg_val);
-			default:
-				return -EINVAL;
-			}
-		case IIO_CHAN_INFO_SAMP_FREQ: {
-			struct pwm_state state;
-			/*
-			 * Limit the maximum acceptable sample rate according to
-			 * the channel's oversampling ratio.
-			 */
-			u64 max_osr_rate = DIV_ROUND_UP_ULL(st->chip_info->max_sample_rate,
-							    osr);
-
-			if (val <= 0 || val > max_osr_rate)
-				return -EINVAL;
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			if (val < 0 || val2 < 0)
+				reg_val = 0;
+			else if (val > 1)
+				reg_val = U16_MAX;
+			else
+				reg_val = (val * (1 << 16) +
+					   mul_u64_u32_div(val2, 1 << 16,
+							   MICRO)) / 2;
 
-			guard(mutex)(&st->cnv_pwm_lock);
-			pwm_get_state(st->cnv_pwm, &state);
-			/*
-			 * The required sample frequency for a given OSR is the
-			 * input frequency multiplied by it.
-			 */
-			state.period = DIV_ROUND_UP_ULL(NSEC_PER_SEC, val * osr);
-			return pwm_apply_might_sleep(st->cnv_pwm, &state);
+			return regmap_write(st->regmap16,
+					    AD4695_REG_GAIN_IN(chan->scan_index),
+					    reg_val);
+		default:
+			return -EINVAL;
 		}
-		case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
-			return ad4695_set_osr_val(st, chan, val);
+	case IIO_CHAN_INFO_CALIBBIAS:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			reg_val = ad4695_get_calibbias(val, val2, osr);
+			return regmap_write(st->regmap16,
+					    AD4695_REG_OFFSET_IN(chan->scan_index),
+					    reg_val);
 		default:
 			return -EINVAL;
 		}
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		struct pwm_state state;
+		/*
+		 * Limit the maximum acceptable sample rate according to
+		 * the channel's oversampling ratio.
+		 */
+		u64 max_osr_rate = DIV_ROUND_UP_ULL(st->chip_info->max_sample_rate,
+						    osr);
+
+		if (val <= 0 || val > max_osr_rate)
+			return -EINVAL;
+
+		guard(mutex)(&st->cnv_pwm_lock);
+		pwm_get_state(st->cnv_pwm, &state);
+		/*
+		 * The required sample frequency for a given OSR is the
+		 * input frequency multiplied by it.
+		 */
+		state.period = DIV_ROUND_UP_ULL(NSEC_PER_SEC, val * osr);
+		return pwm_apply_might_sleep(st->cnv_pwm, &state);
+	}
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return ad4695_set_osr_val(st, chan, val);
+	default:
+		return -EINVAL;
 	}
-	unreachable();
+}
+
+static int ad4695_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	int ret;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+	ret = __ad4695_write_raw(indio_dev, chan, val, val2, mask);
+	iio_device_release_direct(indio_dev);
+
+	return ret;
 }
 
 static int ad4695_read_avail(struct iio_dev *indio_dev,
@@ -1417,26 +1439,29 @@ static int ad4695_debugfs_reg_access(struct iio_dev *indio_dev,
 				     unsigned int *readval)
 {
 	struct ad4695_state *st = iio_priv(indio_dev);
-
-	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
-		if (readval) {
-			if (regmap_check_range_table(st->regmap, reg,
-						     &ad4695_regmap_rd_table))
-				return regmap_read(st->regmap, reg, readval);
-			if (regmap_check_range_table(st->regmap16, reg,
-						     &ad4695_regmap16_rd_table))
-				return regmap_read(st->regmap16, reg, readval);
-		} else {
-			if (regmap_check_range_table(st->regmap, reg,
-						     &ad4695_regmap_wr_table))
-				return regmap_write(st->regmap, reg, writeval);
-			if (regmap_check_range_table(st->regmap16, reg,
-						     &ad4695_regmap16_wr_table))
-				return regmap_write(st->regmap16, reg, writeval);
-		}
+	int ret = -EINVAL;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	if (readval) {
+		if (regmap_check_range_table(st->regmap, reg,
+					     &ad4695_regmap_rd_table))
+			ret = regmap_read(st->regmap, reg, readval);
+		if (regmap_check_range_table(st->regmap16, reg,
+					     &ad4695_regmap16_rd_table))
+			ret = regmap_read(st->regmap16, reg, readval);
+	} else {
+		if (regmap_check_range_table(st->regmap, reg,
+					     &ad4695_regmap_wr_table))
+			ret = regmap_write(st->regmap, reg, writeval);
+		if (regmap_check_range_table(st->regmap16, reg,
+					     &ad4695_regmap16_wr_table))
+			ret = regmap_write(st->regmap16, reg, writeval);
 	}
+	iio_device_release_direct(indio_dev);
 
-	return -EINVAL;
+	return ret;
 }
 
 static int ad4695_get_current_scan_type(const struct iio_dev *indio_dev,


Jonathan

> ---
>  drivers/iio/adc/ad4695.c | 240 ++++++++++++++++++++++-----------------
>  1 file changed, 133 insertions(+), 107 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
> index 13cf01d35301..4bb22f4d739b 100644
> --- a/drivers/iio/adc/ad4695.c
> +++ b/drivers/iio/adc/ad4695.c
> @@ -738,6 +738,25 @@ static int ad4695_read_one_sample(struct ad4695_state *st, unsigned int address)
>  	return spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
>  }
>  
> +static int __ad4695_read_info_raw(struct ad4695_state *st,
> +				  struct iio_chan_spec const *chan,
> +				  int *val)
> +{
> +	u8 realbits = chan->scan_type.realbits;
> +	int ret;
> +
> +	ret = ad4695_read_one_sample(st, chan->address);
> +	if (ret)
> +		return ret;
> +
> +	if (chan->scan_type.sign == 's')
> +		*val = sign_extend32(st->raw_data, realbits - 1);
> +	else
> +		*val = st->raw_data;
> +
> +	return IIO_VAL_INT;
> +}
> +
>  static int ad4695_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val, int *val2, long mask)
> @@ -750,19 +769,12 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			ret = ad4695_read_one_sample(st, chan->address);
> -			if (ret)
> -				return ret;
> -
> -			if (chan->scan_type.sign == 's')
> -				*val = sign_extend32(st->raw_data, realbits - 1);
> -			else
> -				*val = st->raw_data;
>  
> -			return IIO_VAL_INT;
> -		}
> -		unreachable();
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = __ad4695_read_info_raw(st, chan, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> @@ -800,45 +812,45 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_CALIBSCALE:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> -			iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -				ret = regmap_read(st->regmap16,
> -					AD4695_REG_GAIN_IN(chan->scan_index),
> -					&reg_val);
> -				if (ret)
> -					return ret;
> +			if (!iio_device_claim_direct(indio_dev))
> +				return -EBUSY;
> +			ret = regmap_read(st->regmap16,
> +					  AD4695_REG_GAIN_IN(chan->scan_index),
> +					  &reg_val);
> +			iio_device_release_direct(indio_dev);
> +			if (ret)
> +				return ret;
>  
> -				*val = reg_val;
> -				*val2 = 15;
> +			*val = reg_val;
> +			*val2 = 15;
>  
> -				return IIO_VAL_FRACTIONAL_LOG2;
> -			}
> -			unreachable();
> +			return IIO_VAL_FRACTIONAL_LOG2;
>  		default:
>  			return -EINVAL;
>  		}
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> -			iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -				ret = regmap_read(st->regmap16,
> -					AD4695_REG_OFFSET_IN(chan->scan_index),
> -					&reg_val);
> -				if (ret)
> -					return ret;
> -
> -				tmp = sign_extend32(reg_val, 15);
> +			if (!iio_device_claim_direct(indio_dev))
> +				return -EBUSY;
> +			ret = regmap_read(st->regmap16,
> +					  AD4695_REG_OFFSET_IN(chan->scan_index),
> +					  &reg_val);
> +			iio_device_release_direct(indio_dev);
> +			if (ret)
> +				return ret;
>  
> -				*val = tmp / 4;
> -				*val2 = abs(tmp) % 4 * MICRO / 4;
> +			tmp = sign_extend32(reg_val, 15);
>  
> -				if (tmp < 0 && *val2) {
> -					*val *= -1;
> -					*val2 *= -1;
> -				}
> +			*val = tmp / 4;
> +			*val2 = abs(tmp) % 4 * MICRO / 4;
>  
> -				return IIO_VAL_INT_PLUS_MICRO;
> +			if (tmp < 0 && *val2) {
> +				*val *= -1;
> +				*val2 *= -1;
>  			}
> -			unreachable();
> +
> +			return IIO_VAL_INT_PLUS_MICRO;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -847,64 +859,75 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> -static int ad4695_write_raw(struct iio_dev *indio_dev,
> -			    struct iio_chan_spec const *chan,
> -			    int val, int val2, long mask)
> +static int __ad4695_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long mask)
>  {
>  	struct ad4695_state *st = iio_priv(indio_dev);
>  	unsigned int reg_val;
>  
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		switch (mask) {
> -		case IIO_CHAN_INFO_CALIBSCALE:
> -			switch (chan->type) {
> -			case IIO_VOLTAGE:
> -				if (val < 0 || val2 < 0)
> -					reg_val = 0;
> -				else if (val > 1)
> -					reg_val = U16_MAX;
> -				else
> -					reg_val = (val * (1 << 16) +
> -						   mul_u64_u32_div(val2, 1 << 16,
> -								   MICRO)) / 2;
> -
> -				return regmap_write(st->regmap16,
> -					AD4695_REG_GAIN_IN(chan->scan_index),
> -					reg_val);
> -			default:
> -				return -EINVAL;
> -			}
> -		case IIO_CHAN_INFO_CALIBBIAS:
> -			switch (chan->type) {
> -			case IIO_VOLTAGE:
> -				if (val2 >= 0 && val > S16_MAX / 4)
> -					reg_val = S16_MAX;
> -				else if ((val2 < 0 ? -val : val) < S16_MIN / 4)
> -					reg_val = S16_MIN;
> -				else if (val2 < 0)
> -					reg_val = clamp_t(int,
> -						-(val * 4 + -val2 * 4 / MICRO),
> -						S16_MIN, S16_MAX);
> -				else if (val < 0)
> -					reg_val = clamp_t(int,
> -						val * 4 - val2 * 4 / MICRO,
> -						S16_MIN, S16_MAX);
> -				else
> -					reg_val = clamp_t(int,
> -						val * 4 + val2 * 4 / MICRO,
> -						S16_MIN, S16_MAX);
> -
> -				return regmap_write(st->regmap16,
> -					AD4695_REG_OFFSET_IN(chan->scan_index),
> -					reg_val);
> -			default:
> -				return -EINVAL;
> -			}
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			if (val < 0 || val2 < 0)
> +				reg_val = 0;
> +			else if (val > 1)
> +				reg_val = U16_MAX;
> +			else
> +				reg_val = (val * (1 << 16) +
> +					   mul_u64_u32_div(val2, 1 << 16,
> +							   MICRO)) / 2;
> +
> +			return regmap_write(st->regmap16,
> +					    AD4695_REG_GAIN_IN(chan->scan_index),
> +					    reg_val);
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			if (val2 >= 0 && val > S16_MAX / 4)
> +				reg_val = S16_MAX;
> +			else if ((val2 < 0 ? -val : val) < S16_MIN / 4)
> +				reg_val = S16_MIN;
> +			else if (val2 < 0)
> +				reg_val = clamp_t(int,
> +						  -(val * 4 + -val2 * 4 / MICRO),
> +						  S16_MIN, S16_MAX);
> +			else if (val < 0)
> +				reg_val = clamp_t(int,
> +						  val * 4 - val2 * 4 / MICRO,
> +						  S16_MIN, S16_MAX);
> +			else
> +				reg_val = clamp_t(int,
> +						  val * 4 + val2 * 4 / MICRO,
> +						  S16_MIN, S16_MAX);
> +
> +			return regmap_write(st->regmap16,
> +					    AD4695_REG_OFFSET_IN(chan->scan_index),
> +					    reg_val);
>  		default:
>  			return -EINVAL;
>  		}
> +	default:
> +		return -EINVAL;
>  	}
> -	unreachable();
> +}
> +
> +static int ad4695_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	int ret;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +	ret = __ad4695_write_raw(indio_dev, chan, val, val2, mask);
> +	iio_device_release_direct(indio_dev);
> +
> +	return ret;
>  }
>  
>  static int ad4695_read_avail(struct iio_dev *indio_dev,
> @@ -954,26 +977,29 @@ static int ad4695_debugfs_reg_access(struct iio_dev *indio_dev,
>  				     unsigned int *readval)
>  {
>  	struct ad4695_state *st = iio_priv(indio_dev);
> -
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		if (readval) {
> -			if (regmap_check_range_table(st->regmap, reg,
> -						     &ad4695_regmap_rd_table))
> -				return regmap_read(st->regmap, reg, readval);
> -			if (regmap_check_range_table(st->regmap16, reg,
> -						     &ad4695_regmap16_rd_table))
> -				return regmap_read(st->regmap16, reg, readval);
> -		} else {
> -			if (regmap_check_range_table(st->regmap, reg,
> -						     &ad4695_regmap_wr_table))
> -				return regmap_write(st->regmap, reg, writeval);
> -			if (regmap_check_range_table(st->regmap16, reg,
> -						     &ad4695_regmap16_wr_table))
> -				return regmap_write(st->regmap16, reg, writeval);
> -		}
> +	int ret = -EINVAL;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	if (readval) {
> +		if (regmap_check_range_table(st->regmap, reg,
> +					     &ad4695_regmap_rd_table))
> +			ret = regmap_read(st->regmap, reg, readval);
> +		if (regmap_check_range_table(st->regmap16, reg,
> +					     &ad4695_regmap16_rd_table))
> +			ret = regmap_read(st->regmap16, reg, readval);
> +	} else {
> +		if (regmap_check_range_table(st->regmap, reg,
> +					     &ad4695_regmap_wr_table))
> +			ret = regmap_write(st->regmap, reg, writeval);
> +		if (regmap_check_range_table(st->regmap16, reg,
> +					     &ad4695_regmap16_wr_table))
> +			ret = regmap_write(st->regmap16, reg, writeval);
>  	}
> +	iio_device_release_direct(indio_dev);
>  
> -	return -EINVAL;
> +	return ret;
>  }
>  
>  static const struct iio_info ad4695_info = {

Re: [PATCH v2 11/27] iio: adc: ad4695: Stop using iio_device_claim_direct_scoped()

[David Lechner]

On 2/16/25 12:19 PM, Jonathan Cameron wrote:
> On Sun,  9 Feb 2025 18:06:08 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
>> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>
>> This complex cleanup.h use case of conditional guards has proved
>> to be more trouble that it is worth in terms of false positive compiler
>> warnings and hard to read code.
>>
>> Move directly to the new claim/release_direct() that allow sparse
>> to check for unbalanced context.  In some cases code is factored
>> out to utility functions that can do a direct return with the
>> claim and release around the call.
>>
>> Reviewed-by: David Lechner <dlechner@baylibre.com>
>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> ---
>> v2: Typo in commit description (David).
>> Note there are several sets current in flight that touch this driver.
>> I'll rebase as necessary depending on what order the dependencies resolve.
> I've done this rebase and applied on the testing branch of iio.git.
> 
> Would appreciate a sanity check if anyone has time though!
> 
> New code is as follows.  The one corner I was not sure on was
> that for calibbias reading the direct mode claim was held for a long
> time.  That seems to be unnecessary as we have a copy of osr anyway
> in that function used for other purposes.
> 

...

>  	case IIO_CHAN_INFO_CALIBBIAS:
> -		switch (chan->type) {
> -		case IIO_VOLTAGE:
> -			iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -				ret = regmap_read(st->regmap16,
> -					AD4695_REG_OFFSET_IN(chan->scan_index),
> -					&reg_val);
> -				if (ret)
> -					return ret;
> -
> -				tmp = sign_extend32(reg_val, 15);
> -
> -				switch (cfg->oversampling_ratio) {
> -				case 1:
> -					*val = tmp / 4;
> -					*val2 = abs(tmp) % 4 * MICRO / 4;
> -					break;
> -				case 4:
> -					*val = tmp / 2;
> -					*val2 = abs(tmp) % 2 * MICRO / 2;
> -					break;
> -				case 16:
> -					*val = tmp;
> -					*val2 = 0;
> -					break;
> -				case 64:
> -					*val = tmp * 2;
> -					*val2 = 0;
> -					break;
> -				default:
> -					return -EINVAL;
> -				}
> -
> -				if (tmp < 0 && *val2) {
> -					*val *= -1;
> -					*val2 *= -1;
> -				}
> -
> -				return IIO_VAL_INT_PLUS_MICRO;
> +		switch (chan->type)
> +		case IIO_VOLTAGE: {
> +			if (!iio_device_claim_direct(indio_dev))
> +				return -EBUSY;
> +			ret = regmap_read(st->regmap16,
> +					  AD4695_REG_OFFSET_IN(chan->scan_index),
> +					  &reg_val);
> +			iio_device_release_direct(indio_dev);
> +			if (ret)
> +				return ret;
> ////THIS IS THE BIT I WOuLD LIKE EYES on.

Looks fine to me.

> +
> +			tmp = sign_extend32(reg_val, 15);
> +
> +			switch (osr) {
> +			case 1:
> +				*val = tmp / 4;
> +				*val2 = abs(tmp) % 4 * MICRO / 4;
> +				break;
> +			case 4:
> +				*val = tmp / 2;
> +				*val2 = abs(tmp) % 2 * MICRO / 2;
> +				break;
> +			case 16:
> +				*val = tmp;
> +				*val2 = 0;
> +				break;
> +			case 64:
> +				*val = tmp * 2;
> +				*val2 = 0;
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +
> +			if (tmp < 0 && *val2) {
> +				*val *= -1;
> +				*val2 *= -1;
>  			}
> -			unreachable();
> +
> +			return IIO_VAL_INT_PLUS_MICRO;
>  		default:
>  			return -EINVAL;
>  		}

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

[Nuno Sá]

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().

Re: [PATCH v2 08/27] iio: accel: adxl367: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context
> 
> In some cases there is a convenient wrapper function to which
> the handling can be moved. Do that instead of introducing
> another layer of wrappers. In others an outer wrapper is added
> which claims direct mode, runs the original function with the
> scoped claim logic removed, releases direct mode and then checks
> for errors.
> 
> Cc: Cosmin Tanislav <demonsingur@gmail.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/accel/adxl367.c | 194 ++++++++++++++++++++----------------
>  1 file changed, 106 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
> index a48ac0d7bd96..add4053e7a02 100644
> --- a/drivers/iio/accel/adxl367.c
> +++ b/drivers/iio/accel/adxl367.c
> @@ -477,45 +477,42 @@ static int adxl367_set_fifo_watermark(struct
> adxl367_state *st,
>  static int adxl367_set_range(struct iio_dev *indio_dev,
>  			     enum adxl367_range range)
>  {
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		struct adxl367_state *st = iio_priv(indio_dev);
> -		int ret;
> +	struct adxl367_state *st = iio_priv(indio_dev);
> +	int ret;
>  
> -		guard(mutex)(&st->lock);
> +	guard(mutex)(&st->lock);
>  
> -		ret = adxl367_set_measure_en(st, false);
> -		if (ret)
> -			return ret;
> +	ret = adxl367_set_measure_en(st, false);
> +	if (ret)
> +		return ret;
>  
> -		ret = regmap_update_bits(st->regmap, ADXL367_REG_FILTER_CTL,
> -					 ADXL367_FILTER_CTL_RANGE_MASK,
> -					
> FIELD_PREP(ADXL367_FILTER_CTL_RANGE_MASK,
> -						    range));
> -		if (ret)
> -			return ret;
> +	ret = regmap_update_bits(st->regmap, ADXL367_REG_FILTER_CTL,
> +				 ADXL367_FILTER_CTL_RANGE_MASK,
> +				 FIELD_PREP(ADXL367_FILTER_CTL_RANGE_MASK,
> +					    range));
> +	if (ret)
> +		return ret;
>  
> -		adxl367_scale_act_thresholds(st, st->range, range);
> +	adxl367_scale_act_thresholds(st, st->range, range);
>  
> -		/* Activity thresholds depend on range */
> -		ret = _adxl367_set_act_threshold(st, ADXL367_ACTIVITY,
> -						 st->act_threshold);
> -		if (ret)
> -			return ret;
> +	/* Activity thresholds depend on range */
> +	ret = _adxl367_set_act_threshold(st, ADXL367_ACTIVITY,
> +					 st->act_threshold);
> +	if (ret)
> +		return ret;
>  
> -		ret = _adxl367_set_act_threshold(st, ADXL367_INACTIVITY,
> -						 st->inact_threshold);
> -		if (ret)
> -			return ret;
> +	ret = _adxl367_set_act_threshold(st, ADXL367_INACTIVITY,
> +					 st->inact_threshold);
> +	if (ret)
> +		return ret;
>  
> -		ret = adxl367_set_measure_en(st, true);
> -		if (ret)
> -			return ret;
> +	ret = adxl367_set_measure_en(st, true);
> +	if (ret)
> +		return ret;
>  
> -		st->range = range;
> +	st->range = range;
>  
> -		return 0;
> -	}
> -	unreachable();
> +	return 0;
>  }
>  
>  static int adxl367_time_ms_to_samples(struct adxl367_state *st, unsigned int
> ms)
> @@ -620,23 +617,20 @@ static int _adxl367_set_odr(struct adxl367_state *st,
> enum adxl367_odr odr)
>  
>  static int adxl367_set_odr(struct iio_dev *indio_dev, enum adxl367_odr odr)
>  {
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		struct adxl367_state *st = iio_priv(indio_dev);
> -		int ret;
> +	struct adxl367_state *st = iio_priv(indio_dev);
> +	int ret;
>  
> -		guard(mutex)(&st->lock);
> +	guard(mutex)(&st->lock);
>  
> -		ret = adxl367_set_measure_en(st, false);
> -		if (ret)
> -			return ret;
> +	ret = adxl367_set_measure_en(st, false);
> +	if (ret)
> +		return ret;
>  
> -		ret = _adxl367_set_odr(st, odr);
> -		if (ret)
> -			return ret;
> +	ret = _adxl367_set_odr(st, odr);
> +	if (ret)
> +		return ret;
>  
> -		return adxl367_set_measure_en(st, true);
> -	}
> -	unreachable();
> +	return adxl367_set_measure_en(st, true);
>  }
>  
>  static int adxl367_set_temp_adc_en(struct adxl367_state *st, unsigned int
> reg,
> @@ -725,32 +719,29 @@ static int adxl367_read_sample(struct iio_dev
> *indio_dev,
>  			       struct iio_chan_spec const *chan,
>  			       int *val)
>  {
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		struct adxl367_state *st = iio_priv(indio_dev);
> -		u16 sample;
> -		int ret;
> +	struct adxl367_state *st = iio_priv(indio_dev);
> +	u16 sample;
> +	int ret;
>  
> -		guard(mutex)(&st->lock);
> +	guard(mutex)(&st->lock);
>  
> -		ret = adxl367_set_temp_adc_reg_en(st, chan->address, true);
> -		if (ret)
> -			return ret;
> +	ret = adxl367_set_temp_adc_reg_en(st, chan->address, true);
> +	if (ret)
> +		return ret;
>  
> -		ret = regmap_bulk_read(st->regmap, chan->address, &st-
> >sample_buf,
> -				       sizeof(st->sample_buf));
> -		if (ret)
> -			return ret;
> +	ret = regmap_bulk_read(st->regmap, chan->address, &st->sample_buf,
> +			       sizeof(st->sample_buf));
> +	if (ret)
> +		return ret;
>  
> -		sample = FIELD_GET(ADXL367_DATA_MASK, be16_to_cpu(st-
> >sample_buf));
> -		*val = sign_extend32(sample, chan->scan_type.realbits - 1);
> +	sample = FIELD_GET(ADXL367_DATA_MASK, be16_to_cpu(st->sample_buf));
> +	*val = sign_extend32(sample, chan->scan_type.realbits - 1);
>  
> -		ret = adxl367_set_temp_adc_reg_en(st, chan->address, false);
> -		if (ret)
> -			return ret;
> +	ret = adxl367_set_temp_adc_reg_en(st, chan->address, false);
> +	if (ret)
> +		return ret;
>  
> -		return IIO_VAL_INT;
> -	}
> -	unreachable();
> +	return IIO_VAL_INT;
>  }
>  
>  static int adxl367_get_status(struct adxl367_state *st, u8 *status,
> @@ -852,10 +843,15 @@ static int adxl367_read_raw(struct iio_dev *indio_dev,
>  			    int *val, int *val2, long info)
>  {
>  	struct adxl367_state *st = iio_priv(indio_dev);
> +	int ret;
>  
>  	switch (info) {
>  	case IIO_CHAN_INFO_RAW:
> -		return adxl367_read_sample(indio_dev, chan, val);
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = adxl367_read_sample(indio_dev, chan, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
>  		case IIO_ACCEL: {
> @@ -912,7 +908,12 @@ static int adxl367_write_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  
> -		return adxl367_set_odr(indio_dev, odr);
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +
> +		ret = adxl367_set_odr(indio_dev, odr);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	}
>  	case IIO_CHAN_INFO_SCALE: {
>  		enum adxl367_range range;
> @@ -921,7 +922,12 @@ static int adxl367_write_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  
> -		return adxl367_set_range(indio_dev, range);
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +
> +		ret = adxl367_set_range(indio_dev, range);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	}
>  	default:
>  		return -EINVAL;
> @@ -1069,13 +1075,15 @@ static int adxl367_read_event_config(struct iio_dev
> *indio_dev,
>  	}
>  }
>  
> -static int adxl367_write_event_config(struct iio_dev *indio_dev,
> -				      const struct iio_chan_spec *chan,
> -				      enum iio_event_type type,
> -				      enum iio_event_direction dir,
> -				      bool state)
> +static int __adxl367_write_event_config(struct iio_dev *indio_dev,
> +					const struct iio_chan_spec *chan,
> +					enum iio_event_type type,
> +					enum iio_event_direction dir,
> +					bool state)
>  {
> +	struct adxl367_state *st = iio_priv(indio_dev);
>  	enum adxl367_activity_type act;
> +	int ret;
>  
>  	switch (dir) {
>  	case IIO_EV_DIR_RISING:
> @@ -1088,28 +1096,38 @@ static int adxl367_write_event_config(struct iio_dev
> *indio_dev,
>  		return -EINVAL;
>  	}
>  
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		struct adxl367_state *st = iio_priv(indio_dev);
> -		int ret;
> +	guard(mutex)(&st->lock);
> +
> +	ret = adxl367_set_measure_en(st, false);
> +	if (ret)
> +		return ret;
>  
> -		guard(mutex)(&st->lock);
> +	ret = adxl367_set_act_interrupt_en(st, act, state);
> +	if (ret)
> +		return ret;
>  
> -		ret = adxl367_set_measure_en(st, false);
> -		if (ret)
> -			return ret;
> +	ret = adxl367_set_act_en(st, act, state ? ADCL367_ACT_REF_ENABLED
> +				 : ADXL367_ACT_DISABLED);
> +	if (ret)
> +		return ret;
>  
> -		ret = adxl367_set_act_interrupt_en(st, act, state);
> -		if (ret)
> -			return ret;
> +	return adxl367_set_measure_en(st, true);
> +}
>  
> -		ret = adxl367_set_act_en(st, act, state ?
> ADCL367_ACT_REF_ENABLED
> -					 : ADXL367_ACT_DISABLED);
> -		if (ret)
> -			return ret;
> +static int adxl367_write_event_config(struct iio_dev *indio_dev,
> +				      const struct iio_chan_spec *chan,
> +				      enum iio_event_type type,
> +				      enum iio_event_direction dir,
> +				      bool state)
> +{
> +	int ret;
>  
> -		return adxl367_set_measure_en(st, true);
> -	}
> -	unreachable();
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	ret = __adxl367_write_event_config(indio_dev, chan, type, dir,
> state);
> +	iio_device_release_direct(indio_dev);
> +	return ret;
>  }
>  
>  static ssize_t adxl367_get_fifo_enabled(struct device *dev,

Re: [PATCH v2 09/27] iio: adc: ad4000: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.
> 
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> Tested-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/adc/ad4000.c | 60 +++++++++++++++++++++++++---------------
>  1 file changed, 37 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> index 1d556a842a68..4fe8dee48da9 100644
> --- a/drivers/iio/adc/ad4000.c
> +++ b/drivers/iio/adc/ad4000.c
> @@ -535,12 +535,16 @@ static int ad4000_read_raw(struct iio_dev *indio_dev,
>  			   int *val2, long info)
>  {
>  	struct ad4000_state *st = iio_priv(indio_dev);
> +	int ret;
>  
>  	switch (info) {
>  	case IIO_CHAN_INFO_RAW:
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
> -			return ad4000_single_conversion(indio_dev, chan,
> val);
> -		unreachable();
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +
> +		ret = ad4000_single_conversion(indio_dev, chan, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = st->scale_tbl[st->span_comp][0];
>  		*val2 = st->scale_tbl[st->span_comp][1];
> @@ -585,36 +589,46 @@ static int ad4000_write_raw_get_fmt(struct iio_dev
> *indio_dev,
>  	}
>  }
>  
> -static int ad4000_write_raw(struct iio_dev *indio_dev,
> -			    struct iio_chan_spec const *chan, int val, int
> val2,
> -			    long mask)
> +static int __ad4000_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val2)
>  {
>  	struct ad4000_state *st = iio_priv(indio_dev);
>  	unsigned int reg_val;
>  	bool span_comp_en;
>  	int ret;
>  
> -	switch (mask) {
> -	case IIO_CHAN_INFO_SCALE:
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			guard(mutex)(&st->lock);
> +	guard(mutex)(&st->lock);
> +
> +	ret = ad4000_read_reg(st, &reg_val);
> +	if (ret < 0)
> +		return ret;
> +
> +	span_comp_en = val2 == st->scale_tbl[1][1];
> +	reg_val &= ~AD4000_CFG_SPAN_COMP;
> +	reg_val |= FIELD_PREP(AD4000_CFG_SPAN_COMP, span_comp_en);
>  
> -			ret = ad4000_read_reg(st, &reg_val);
> -			if (ret < 0)
> -				return ret;
> +	ret = ad4000_write_reg(st, reg_val);
> +	if (ret < 0)
> +		return ret;
>  
> -			span_comp_en = val2 == st->scale_tbl[1][1];
> -			reg_val &= ~AD4000_CFG_SPAN_COMP;
> -			reg_val |= FIELD_PREP(AD4000_CFG_SPAN_COMP,
> span_comp_en);
> +	st->span_comp = span_comp_en;
> +	return 0;
> +}
>  
> -			ret = ad4000_write_reg(st, reg_val);
> -			if (ret < 0)
> -				return ret;
> +static int ad4000_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	int ret;
>  
> -			st->span_comp = span_comp_en;
> -			return 0;
> -		}
> -		unreachable();
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = __ad4000_write_raw(indio_dev, chan, val2);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	default:
>  		return -EINVAL;
>  	}

Re: [PATCH v2 10/27] iio: adc: ad4130: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.
> 
> Cc: Cosmin Tanislav <demonsingur@gmail.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/adc/ad4130.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4130.c b/drivers/iio/adc/ad4130.c
> index acc241cc0a7a..061eeb9b1f8d 100644
> --- a/drivers/iio/adc/ad4130.c
> +++ b/drivers/iio/adc/ad4130.c
> @@ -1067,13 +1067,11 @@ static int _ad4130_read_sample(struct iio_dev
> *indio_dev, unsigned int channel,
>  static int ad4130_read_sample(struct iio_dev *indio_dev, unsigned int
> channel,
>  			      int *val)
>  {
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		struct ad4130_state *st = iio_priv(indio_dev);
> +	struct ad4130_state *st = iio_priv(indio_dev);
>  
> -		guard(mutex)(&st->lock);
> -		return _ad4130_read_sample(indio_dev, channel, val);
> -	}
> -	unreachable();
> +	guard(mutex)(&st->lock);
> +
> +	return _ad4130_read_sample(indio_dev, channel, val);
>  }
>  
>  static int ad4130_read_raw(struct iio_dev *indio_dev,
> @@ -1083,10 +1081,16 @@ static int ad4130_read_raw(struct iio_dev *indio_dev,
>  	struct ad4130_state *st = iio_priv(indio_dev);
>  	unsigned int channel = chan->scan_index;
>  	struct ad4130_setup_info *setup_info = &st-
> >chans_info[channel].setup;
> +	int ret;
>  
>  	switch (info) {
>  	case IIO_CHAN_INFO_RAW:
> -		return ad4130_read_sample(indio_dev, channel, val);
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +
> +		ret = ad4130_read_sample(indio_dev, channel, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	case IIO_CHAN_INFO_SCALE: {
>  		guard(mutex)(&st->lock);
>  		*val = st->scale_tbls[setup_info->ref_sel][setup_info-
> >pga][0];

Re: [PATCH v2 11/27] iio: adc: ad4695: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-16 at 13:00 -0600, David Lechner wrote:
> On 2/16/25 12:19 PM, Jonathan Cameron wrote:
> > On Sun,  9 Feb 2025 18:06:08 +0000
> > Jonathan Cameron <jic23@kernel.org> wrote:
> > 
> > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > 
> > > This complex cleanup.h use case of conditional guards has proved
> > > to be more trouble that it is worth in terms of false positive compiler
> > > warnings and hard to read code.
> > > 
> > > Move directly to the new claim/release_direct() that allow sparse
> > > to check for unbalanced context.  In some cases code is factored
> > > out to utility functions that can do a direct return with the
> > > claim and release around the call.
> > > 
> > > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > ---
> > > v2: Typo in commit description (David).
> > > Note there are several sets current in flight that touch this driver.
> > > I'll rebase as necessary depending on what order the dependencies resolve.
> > I've done this rebase and applied on the testing branch of iio.git.
> > 
> > Would appreciate a sanity check if anyone has time though!
> > 
> > New code is as follows.  The one corner I was not sure on was
> > that for calibbias reading the direct mode claim was held for a long
> > time.  That seems to be unnecessary as we have a copy of osr anyway
> > in that function used for other purposes.
> > 
> 
> ...
> 
> >  	case IIO_CHAN_INFO_CALIBBIAS:
> > -		switch (chan->type) {
> > -		case IIO_VOLTAGE:
> > -			iio_device_claim_direct_scoped(return -EBUSY,
> > indio_dev) {
> > -				ret = regmap_read(st->regmap16,
> > -					AD4695_REG_OFFSET_IN(chan-
> > >scan_index),
> > -					&reg_val);
> > -				if (ret)
> > -					return ret;
> > -
> > -				tmp = sign_extend32(reg_val, 15);
> > -
> > -				switch (cfg->oversampling_ratio) {
> > -				case 1:
> > -					*val = tmp / 4;
> > -					*val2 = abs(tmp) % 4 * MICRO / 4;
> > -					break;
> > -				case 4:
> > -					*val = tmp / 2;
> > -					*val2 = abs(tmp) % 2 * MICRO / 2;
> > -					break;
> > -				case 16:
> > -					*val = tmp;
> > -					*val2 = 0;
> > -					break;
> > -				case 64:
> > -					*val = tmp * 2;
> > -					*val2 = 0;
> > -					break;
> > -				default:
> > -					return -EINVAL;
> > -				}
> > -
> > -				if (tmp < 0 && *val2) {
> > -					*val *= -1;
> > -					*val2 *= -1;
> > -				}
> > -
> > -				return IIO_VAL_INT_PLUS_MICRO;
> > +		switch (chan->type)
> > +		case IIO_VOLTAGE: {
> > +			if (!iio_device_claim_direct(indio_dev))
> > +				return -EBUSY;
> > +			ret = regmap_read(st->regmap16,
> > +					  AD4695_REG_OFFSET_IN(chan-
> > >scan_index),
> > +					  &reg_val);
> > +			iio_device_release_direct(indio_dev);
> > +			if (ret)
> > +				return ret;
> > ////THIS IS THE BIT I WOuLD LIKE EYES on.
> 
> Looks fine to me.

+1

- Nuno Sá
> 
> > +
> > +			tmp = sign_extend32(reg_val, 15);
> > +
> > +			switch (osr) {
> > +			case 1:
> > +				*val = tmp / 4;
> > +				*val2 = abs(tmp) % 4 * MICRO / 4;
> > +				break;
> > +			case 4:
> > +				*val = tmp / 2;
> > +				*val2 = abs(tmp) % 2 * MICRO / 2;
> > +				break;
> > +			case 16:
> > +				*val = tmp;
> > +				*val2 = 0;
> > +				break;
> > +			case 64:
> > +				*val = tmp * 2;
> > +				*val2 = 0;
> > +				break;
> > +			default:
> > +				return -EINVAL;
> > +			}
> > +
> > +			if (tmp < 0 && *val2) {
> > +				*val *= -1;
> > +				*val2 *= -1;
> >  			}
> > -			unreachable();
> > +
> > +			return IIO_VAL_INT_PLUS_MICRO;
> >  		default:
> >  			return -EINVAL;
> >  		}

Re: [PATCH v2 11/27] iio: adc: ad4695: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.  In some cases code is factored
> out to utility functions that can do a direct return with the
> claim and release around the call.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v2: Typo in commit description (David).
> Note there are several sets current in flight that touch this driver.
> I'll rebase as necessary depending on what order the dependencies resolve.
> ---

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

>  drivers/iio/adc/ad4695.c | 240 ++++++++++++++++++++++-----------------
>  1 file changed, 133 insertions(+), 107 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
> index 13cf01d35301..4bb22f4d739b 100644
> --- a/drivers/iio/adc/ad4695.c
> +++ b/drivers/iio/adc/ad4695.c
> @@ -738,6 +738,25 @@ static int ad4695_read_one_sample(struct ad4695_state
> *st, unsigned int address)
>  	return spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
>  }
>  
> +static int __ad4695_read_info_raw(struct ad4695_state *st,
> +				  struct iio_chan_spec const *chan,
> +				  int *val)
> +{
> +	u8 realbits = chan->scan_type.realbits;
> +	int ret;
> +
> +	ret = ad4695_read_one_sample(st, chan->address);
> +	if (ret)
> +		return ret;
> +
> +	if (chan->scan_type.sign == 's')
> +		*val = sign_extend32(st->raw_data, realbits - 1);
> +	else
> +		*val = st->raw_data;
> +
> +	return IIO_VAL_INT;
> +}
> +
>  static int ad4695_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val, int *val2, long mask)
> @@ -750,19 +769,12 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			ret = ad4695_read_one_sample(st, chan->address);
> -			if (ret)
> -				return ret;
> -
> -			if (chan->scan_type.sign == 's')
> -				*val = sign_extend32(st->raw_data, realbits -
> 1);
> -			else
> -				*val = st->raw_data;
>  
> -			return IIO_VAL_INT;
> -		}
> -		unreachable();
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = __ad4695_read_info_raw(st, chan, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> @@ -800,45 +812,45 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_CALIBSCALE:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> -			iio_device_claim_direct_scoped(return -EBUSY,
> indio_dev) {
> -				ret = regmap_read(st->regmap16,
> -					AD4695_REG_GAIN_IN(chan->scan_index),
> -					&reg_val);
> -				if (ret)
> -					return ret;
> +			if (!iio_device_claim_direct(indio_dev))
> +				return -EBUSY;
> +			ret = regmap_read(st->regmap16,
> +					  AD4695_REG_GAIN_IN(chan-
> >scan_index),
> +					  &reg_val);
> +			iio_device_release_direct(indio_dev);
> +			if (ret)
> +				return ret;
>  
> -				*val = reg_val;
> -				*val2 = 15;
> +			*val = reg_val;
> +			*val2 = 15;
>  
> -				return IIO_VAL_FRACTIONAL_LOG2;
> -			}
> -			unreachable();
> +			return IIO_VAL_FRACTIONAL_LOG2;
>  		default:
>  			return -EINVAL;
>  		}
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> -			iio_device_claim_direct_scoped(return -EBUSY,
> indio_dev) {
> -				ret = regmap_read(st->regmap16,
> -					AD4695_REG_OFFSET_IN(chan-
> >scan_index),
> -					&reg_val);
> -				if (ret)
> -					return ret;
> -
> -				tmp = sign_extend32(reg_val, 15);
> +			if (!iio_device_claim_direct(indio_dev))
> +				return -EBUSY;
> +			ret = regmap_read(st->regmap16,
> +					  AD4695_REG_OFFSET_IN(chan-
> >scan_index),
> +					  &reg_val);
> +			iio_device_release_direct(indio_dev);
> +			if (ret)
> +				return ret;
>  
> -				*val = tmp / 4;
> -				*val2 = abs(tmp) % 4 * MICRO / 4;
> +			tmp = sign_extend32(reg_val, 15);
>  
> -				if (tmp < 0 && *val2) {
> -					*val *= -1;
> -					*val2 *= -1;
> -				}
> +			*val = tmp / 4;
> +			*val2 = abs(tmp) % 4 * MICRO / 4;
>  
> -				return IIO_VAL_INT_PLUS_MICRO;
> +			if (tmp < 0 && *val2) {
> +				*val *= -1;
> +				*val2 *= -1;
>  			}
> -			unreachable();
> +
> +			return IIO_VAL_INT_PLUS_MICRO;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -847,64 +859,75 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> -static int ad4695_write_raw(struct iio_dev *indio_dev,
> -			    struct iio_chan_spec const *chan,
> -			    int val, int val2, long mask)
> +static int __ad4695_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long mask)
>  {
>  	struct ad4695_state *st = iio_priv(indio_dev);
>  	unsigned int reg_val;
>  
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		switch (mask) {
> -		case IIO_CHAN_INFO_CALIBSCALE:
> -			switch (chan->type) {
> -			case IIO_VOLTAGE:
> -				if (val < 0 || val2 < 0)
> -					reg_val = 0;
> -				else if (val > 1)
> -					reg_val = U16_MAX;
> -				else
> -					reg_val = (val * (1 << 16) +
> -						   mul_u64_u32_div(val2, 1 <<
> 16,
> -								   MICRO)) /
> 2;
> -
> -				return regmap_write(st->regmap16,
> -					AD4695_REG_GAIN_IN(chan->scan_index),
> -					reg_val);
> -			default:
> -				return -EINVAL;
> -			}
> -		case IIO_CHAN_INFO_CALIBBIAS:
> -			switch (chan->type) {
> -			case IIO_VOLTAGE:
> -				if (val2 >= 0 && val > S16_MAX / 4)
> -					reg_val = S16_MAX;
> -				else if ((val2 < 0 ? -val : val) < S16_MIN /
> 4)
> -					reg_val = S16_MIN;
> -				else if (val2 < 0)
> -					reg_val = clamp_t(int,
> -						-(val * 4 + -val2 * 4 /
> MICRO),
> -						S16_MIN, S16_MAX);
> -				else if (val < 0)
> -					reg_val = clamp_t(int,
> -						val * 4 - val2 * 4 / MICRO,
> -						S16_MIN, S16_MAX);
> -				else
> -					reg_val = clamp_t(int,
> -						val * 4 + val2 * 4 / MICRO,
> -						S16_MIN, S16_MAX);
> -
> -				return regmap_write(st->regmap16,
> -					AD4695_REG_OFFSET_IN(chan-
> >scan_index),
> -					reg_val);
> -			default:
> -				return -EINVAL;
> -			}
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			if (val < 0 || val2 < 0)
> +				reg_val = 0;
> +			else if (val > 1)
> +				reg_val = U16_MAX;
> +			else
> +				reg_val = (val * (1 << 16) +
> +					   mul_u64_u32_div(val2, 1 << 16,
> +							   MICRO)) / 2;
> +
> +			return regmap_write(st->regmap16,
> +					    AD4695_REG_GAIN_IN(chan-
> >scan_index),
> +					    reg_val);
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			if (val2 >= 0 && val > S16_MAX / 4)
> +				reg_val = S16_MAX;
> +			else if ((val2 < 0 ? -val : val) < S16_MIN / 4)
> +				reg_val = S16_MIN;
> +			else if (val2 < 0)
> +				reg_val = clamp_t(int,
> +						  -(val * 4 + -val2 * 4 /
> MICRO),
> +						  S16_MIN, S16_MAX);
> +			else if (val < 0)
> +				reg_val = clamp_t(int,
> +						  val * 4 - val2 * 4 / MICRO,
> +						  S16_MIN, S16_MAX);
> +			else
> +				reg_val = clamp_t(int,
> +						  val * 4 + val2 * 4 / MICRO,
> +						  S16_MIN, S16_MAX);
> +
> +			return regmap_write(st->regmap16,
> +					    AD4695_REG_OFFSET_IN(chan-
> >scan_index),
> +					    reg_val);
>  		default:
>  			return -EINVAL;
>  		}
> +	default:
> +		return -EINVAL;
>  	}
> -	unreachable();
> +}
> +
> +static int ad4695_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	int ret;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +	ret = __ad4695_write_raw(indio_dev, chan, val, val2, mask);
> +	iio_device_release_direct(indio_dev);
> +
> +	return ret;
>  }
>  
>  static int ad4695_read_avail(struct iio_dev *indio_dev,
> @@ -954,26 +977,29 @@ static int ad4695_debugfs_reg_access(struct iio_dev
> *indio_dev,
>  				     unsigned int *readval)
>  {
>  	struct ad4695_state *st = iio_priv(indio_dev);
> -
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		if (readval) {
> -			if (regmap_check_range_table(st->regmap, reg,
> -						    
> &ad4695_regmap_rd_table))
> -				return regmap_read(st->regmap, reg, readval);
> -			if (regmap_check_range_table(st->regmap16, reg,
> -						    
> &ad4695_regmap16_rd_table))
> -				return regmap_read(st->regmap16, reg,
> readval);
> -		} else {
> -			if (regmap_check_range_table(st->regmap, reg,
> -						    
> &ad4695_regmap_wr_table))
> -				return regmap_write(st->regmap, reg,
> writeval);
> -			if (regmap_check_range_table(st->regmap16, reg,
> -						    
> &ad4695_regmap16_wr_table))
> -				return regmap_write(st->regmap16, reg,
> writeval);
> -		}
> +	int ret = -EINVAL;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	if (readval) {
> +		if (regmap_check_range_table(st->regmap, reg,
> +					     &ad4695_regmap_rd_table))
> +			ret = regmap_read(st->regmap, reg, readval);
> +		if (regmap_check_range_table(st->regmap16, reg,
> +					     &ad4695_regmap16_rd_table))
> +			ret = regmap_read(st->regmap16, reg, readval);
> +	} else {
> +		if (regmap_check_range_table(st->regmap, reg,
> +					     &ad4695_regmap_wr_table))
> +			ret = regmap_write(st->regmap, reg, writeval);
> +		if (regmap_check_range_table(st->regmap16, reg,
> +					     &ad4695_regmap16_wr_table))
> +			ret = regmap_write(st->regmap16, reg, writeval);
>  	}
> +	iio_device_release_direct(indio_dev);
>  
> -	return -EINVAL;
> +	return ret;
>  }
>  
>  static const struct iio_info ad4695_info = {

Re: [PATCH v2 12/27] iio: adc: ad7606: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.
> 
> Cc: Guillaume Stols <gstols@baylibre.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/adc/ad7606.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index d8e3c7a43678..39b5bf09e46d 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -752,13 +752,13 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
>  
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			ret = ad7606_scan_direct(indio_dev, chan->address,
> val);
> -			if (ret < 0)
> -				return ret;
> -			return IIO_VAL_INT;
> -		}
> -		unreachable();
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = ad7606_scan_direct(indio_dev, chan->address, val);
> +		iio_device_release_direct(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
>  		if (st->sw_mode_en)
>  			ch = chan->address;

Re: [PATCH v2 13/27] iio: adc: ad7625: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.
> 
> Reviewed-by: Trevor Gamblin <tgamblin@baylibre.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/adc/ad7625.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7625.c b/drivers/iio/adc/ad7625.c
> index adb4e25dd7ea..0466c0c7eae4 100644
> --- a/drivers/iio/adc/ad7625.c
> +++ b/drivers/iio/adc/ad7625.c
> @@ -248,12 +248,15 @@ static int ad7625_write_raw(struct iio_dev *indio_dev,
>  			    int val, int val2, long info)
>  {
>  	struct ad7625_state *st = iio_priv(indio_dev);
> +	int ret;
>  
>  	switch (info) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
> -			return ad7625_set_sampling_freq(st, val);
> -		unreachable();
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = ad7625_set_sampling_freq(st, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	default:
>  		return -EINVAL;
>  	}

Re: [PATCH v2 14/27] iio: adc: ad7779: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.
> 
> Cc: Ramona Alexandra Nechita <ramona.nechita@analog.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/adc/ad7779.c | 101 ++++++++++++++++++++++++---------------
>  1 file changed, 62 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c
> index 2537dab69a35..a5d87faa5e12 100644
> --- a/drivers/iio/adc/ad7779.c
> +++ b/drivers/iio/adc/ad7779.c
> @@ -467,59 +467,82 @@ static int ad7779_set_calibbias(struct ad7779_state *st,
> int channel, int val)
>  				calibbias[2]);
>  }
>  
> +static int __ad7779_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, int *val,
> +			     int *val2, long mask)
> +{
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		ret = ad7779_get_calibscale(st, chan->channel);
> +		if (ret < 0)
> +			return ret;
> +		*val = ret;
> +		*val2 = GAIN_REL;
> +		return IIO_VAL_FRACTIONAL;
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		ret = ad7779_get_calibbias(st, chan->channel);
> +		if (ret < 0)
> +			return ret;
> +		*val = ret;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = st->sampling_freq;
> +		if (*val < 0)
> +			return -EINVAL;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static int ad7779_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan, int *val,
>  			   int *val2, long mask)
>  {
> -	struct ad7779_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		switch (mask) {
> -		case IIO_CHAN_INFO_CALIBSCALE:
> -			ret = ad7779_get_calibscale(st, chan->channel);
> -			if (ret < 0)
> -				return ret;
> -			*val = ret;
> -			*val2 = GAIN_REL;
> -			return IIO_VAL_FRACTIONAL;
> -		case IIO_CHAN_INFO_CALIBBIAS:
> -			ret = ad7779_get_calibbias(st, chan->channel);
> -			if (ret < 0)
> -				return ret;
> -			*val = ret;
> -			return IIO_VAL_INT;
> -		case IIO_CHAN_INFO_SAMP_FREQ:
> -			*val = st->sampling_freq;
> -			if (*val < 0)
> -				return -EINVAL;
> -			return IIO_VAL_INT;
> -		default:
> -			return -EINVAL;
> -		}
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	ret = __ad7779_read_raw(indio_dev, chan, val, val2, mask);
> +	iio_device_release_direct(indio_dev);
> +	return ret;
> +}
> +
> +static int __ad7779_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2,
> +			      long mask)
> +{
> +	struct ad7779_state *st = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		return ad7779_set_calibscale(st, chan->channel, val2);
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		return ad7779_set_calibbias(st, chan->channel, val);
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return ad7779_set_sampling_frequency(st, val);
> +	default:
> +		return -EINVAL;
>  	}
> -	unreachable();
>  }
>  
>  static int ad7779_write_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan, int val, int
> val2,
>  			    long mask)
>  {
> -	struct ad7779_state *st = iio_priv(indio_dev);
> +	int ret;
>  
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		switch (mask) {
> -		case IIO_CHAN_INFO_CALIBSCALE:
> -			return ad7779_set_calibscale(st, chan->channel,
> val2);
> -		case IIO_CHAN_INFO_CALIBBIAS:
> -			return ad7779_set_calibbias(st, chan->channel, val);
> -		case IIO_CHAN_INFO_SAMP_FREQ:
> -			return ad7779_set_sampling_frequency(st, val);
> -		default:
> -			return -EINVAL;
> -		}
> -	}
> -	unreachable();
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	ret = __ad7779_write_raw(indio_dev, chan, val, val2, mask);
> +	iio_device_release_direct(indio_dev);
> +	return ret;
>  }
>  
>  static int ad7779_buffer_preenable(struct iio_dev *indio_dev)

Re: [PATCH v2 15/27] iio: adc: ad9467: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context. Also use guard() to simplify mutex
> unlock paths.
> 
> Cc: Nuno Sa <nuno.sa@analog.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/adc/ad9467.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> index f30119b42ba0..f7a9f46ea0dc 100644
> --- a/drivers/iio/adc/ad9467.c
> +++ b/drivers/iio/adc/ad9467.c
> @@ -813,6 +813,18 @@ static int ad9467_read_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static int __ad9467_update_clock(struct ad9467_state *st, long r_clk)
> +{
> +	int ret;
> +
> +	ret = clk_set_rate(st->clk, r_clk);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&st->lock);
> +	return ad9467_calibrate(st);
> +}
> +
>  static int ad9467_write_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
>  			    int val, int val2, long mask)
> @@ -842,14 +854,11 @@ static int ad9467_write_raw(struct iio_dev *indio_dev,
>  		if (sample_rate == r_clk)
>  			return 0;
>  
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			ret = clk_set_rate(st->clk, r_clk);
> -			if (ret)
> -				return ret;
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
>  
> -			guard(mutex)(&st->lock);
> -			ret = ad9467_calibrate(st);
> -		}
> +		ret = __ad9467_update_clock(st, r_clk);
> +		iio_device_release_direct(indio_dev);
>  		return ret;
>  	default:
>  		return -EINVAL;

Re: [PATCH v2 16/27] iio: adc: max1363: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/adc/max1363.c | 165 ++++++++++++++++++++------------------
>  1 file changed, 89 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> index e8d731bc34e0..35717ec082ce 100644
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -364,55 +364,52 @@ static int max1363_read_single_chan(struct iio_dev
> *indio_dev,
>  				    int *val,
>  				    long m)
>  {
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		s32 data;
> -		u8 rxbuf[2];
> -		struct max1363_state *st = iio_priv(indio_dev);
> -		struct i2c_client *client = st->client;
> -
> -		guard(mutex)(&st->lock);
> -
> -		/*
> -		 * If monitor mode is enabled, the method for reading a
> single
> -		 * channel will have to be rather different and has not yet
> -		 * been implemented.
> -		 *
> -		 * Also, cannot read directly if buffered capture enabled.
> -		 */
> -		if (st->monitor_on)
> -			return -EBUSY;
> +	s32 data;
> +	u8 rxbuf[2];
> +	struct max1363_state *st = iio_priv(indio_dev);
> +	struct i2c_client *client = st->client;
>  
> -		/* Check to see if current scan mode is correct */
> -		if (st->current_mode != &max1363_mode_table[chan->address]) {
> -			int ret;
> +	guard(mutex)(&st->lock);
>  
> -			/* Update scan mode if needed */
> -			st->current_mode = &max1363_mode_table[chan-
> >address];
> -			ret = max1363_set_scan_mode(st);
> -			if (ret < 0)
> -				return ret;
> -		}
> -		if (st->chip_info->bits != 8) {
> -			/* Get reading */
> -			data = st->recv(client, rxbuf, 2);
> -			if (data < 0)
> -				return data;
> -
> -			data = get_unaligned_be16(rxbuf) &
> -				((1 << st->chip_info->bits) - 1);
> -		} else {
> -			/* Get reading */
> -			data = st->recv(client, rxbuf, 1);
> -			if (data < 0)
> -				return data;
> -
> -			data = rxbuf[0];
> -		}
> -		*val = data;
> +	/*
> +	 * If monitor mode is enabled, the method for reading a single
> +	 * channel will have to be rather different and has not yet
> +	 * been implemented.
> +	 *
> +	 * Also, cannot read directly if buffered capture enabled.
> +	 */
> +	if (st->monitor_on)
> +		return -EBUSY;
> +
> +	/* Check to see if current scan mode is correct */
> +	if (st->current_mode != &max1363_mode_table[chan->address]) {
> +		int ret;
> +
> +		/* Update scan mode if needed */
> +		st->current_mode = &max1363_mode_table[chan->address];
> +		ret = max1363_set_scan_mode(st);
> +		if (ret < 0)
> +			return ret;
> +	}
> +	if (st->chip_info->bits != 8) {
> +		/* Get reading */
> +		data = st->recv(client, rxbuf, 2);
> +		if (data < 0)
> +			return data;
> +
> +		data = get_unaligned_be16(rxbuf) &
> +			((1 << st->chip_info->bits) - 1);
> +	} else {
> +		/* Get reading */
> +		data = st->recv(client, rxbuf, 1);
> +		if (data < 0)
> +			return data;
>  
> -		return 0;
> +		data = rxbuf[0];
>  	}
> -	unreachable();
> +	*val = data;
> +
> +	return 0;
>  }
>  
>  static int max1363_read_raw(struct iio_dev *indio_dev,
> @@ -426,7 +423,11 @@ static int max1363_read_raw(struct iio_dev *indio_dev,
>  
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +
>  		ret = max1363_read_single_chan(indio_dev, chan, val, m);
> +		iio_device_release_direct(indio_dev);
>  		if (ret < 0)
>  			return ret;
>  		return IIO_VAL_INT;
> @@ -947,46 +948,58 @@ static inline int __max1363_check_event_mask(int
> thismask, int checkmask)
>  	return ret;
>  }
>  
> -static int max1363_write_event_config(struct iio_dev *indio_dev,
> -	const struct iio_chan_spec *chan, enum iio_event_type type,
> +static int __max1363_write_event_config(struct max1363_state *st,
> +	const struct iio_chan_spec *chan,
>  	enum iio_event_direction dir, bool state)
>  {
> -	struct max1363_state *st = iio_priv(indio_dev);
> -
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		int number = chan->channel;
> -		u16 unifiedmask;
> -		int ret;
> +	int number = chan->channel;
> +	u16 unifiedmask;
> +	int ret;
>  
> -		guard(mutex)(&st->lock);
> +	guard(mutex)(&st->lock);
>  
> -		unifiedmask = st->mask_low | st->mask_high;
> -		if (dir == IIO_EV_DIR_FALLING) {
> +	unifiedmask = st->mask_low | st->mask_high;
> +	if (dir == IIO_EV_DIR_FALLING) {
>  
> -			if (state == 0)
> -				st->mask_low &= ~(1 << number);
> -			else {
> -				ret = __max1363_check_event_mask((1 <<
> number),
> -								
> unifiedmask);
> -				if (ret)
> -					return ret;
> -				st->mask_low |= (1 << number);
> -			}
> -		} else {
> -			if (state == 0)
> -				st->mask_high &= ~(1 << number);
> -			else {
> -				ret = __max1363_check_event_mask((1 <<
> number),
> -								
> unifiedmask);
> -				if (ret)
> -					return ret;
> -				st->mask_high |= (1 << number);
> -			}
> +		if (state == 0)
> +			st->mask_low &= ~(1 << number);
> +		else {
> +			ret = __max1363_check_event_mask((1 << number),
> +							 unifiedmask);
> +			if (ret)
> +				return ret;
> +			st->mask_low |= (1 << number);
> +		}
> +	} else {
> +		if (state == 0)
> +			st->mask_high &= ~(1 << number);
> +		else {
> +			ret = __max1363_check_event_mask((1 << number),
> +							 unifiedmask);
> +			if (ret)
> +				return ret;
> +			st->mask_high |= (1 << number);
>  		}
>  	}
> -	max1363_monitor_mode_update(st, !!(st->mask_high | st->mask_low));
>  
>  	return 0;
> +
> +}
> +static int max1363_write_event_config(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, enum iio_event_type type,
> +	enum iio_event_direction dir, bool state)
> +{
> +	struct max1363_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	ret = __max1363_write_event_config(st, chan,  dir, state);
> +	iio_device_release_direct(indio_dev);
> +	max1363_monitor_mode_update(st, !!(st->mask_high | st->mask_low));
> +
> +	return ret;
>  }
>  
>  /*

Re: [PATCH v2 20/27] iio: addac: ad74413r: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context. Includes moving a mutex lock
> into a function rather than around it to simplify the error handling.
> 
> Cc: Nuno Sa <nuno.sa@analog.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/addac/ad74413r.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> index daea2bde7acf..f14d12b03da6 100644
> --- a/drivers/iio/addac/ad74413r.c
> +++ b/drivers/iio/addac/ad74413r.c
> @@ -826,6 +826,8 @@ static int _ad74413r_get_single_adc_result(struct
> ad74413r_state *st,
>  	unsigned int uval;
>  	int ret;
>  
> +	guard(mutex)(&st->lock);
> +
>  	reinit_completion(&st->adc_data_completion);
>  
>  	ret = ad74413r_set_adc_channel_enable(st, channel, true);
> @@ -865,12 +867,14 @@ static int ad74413r_get_single_adc_result(struct iio_dev
> *indio_dev,
>  					  unsigned int channel, int *val)
>  {
>  	struct ad74413r_state *st = iio_priv(indio_dev);
> +	int ret;
>  
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		guard(mutex)(&st->lock);
> -		return _ad74413r_get_single_adc_result(st, channel, val);
> -	}
> -	unreachable();
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	ret = _ad74413r_get_single_adc_result(st, channel, val);
> +	iio_device_release_direct(indio_dev);
> +	return ret;
>  }
>  
>  static void ad74413r_adc_to_resistance_result(int adc_result, int *val)

Re: [PATCH v2 23/27] iio: dac: ad8460: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.
> 
> Cc: Mariel Tinaco <Mariel.Tinaco@analog.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/dac/ad8460.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad8460.c b/drivers/iio/dac/ad8460.c
> index 535ee3105af6..6e45686902dd 100644
> --- a/drivers/iio/dac/ad8460.c
> +++ b/drivers/iio/dac/ad8460.c
> @@ -264,9 +264,12 @@ static ssize_t ad8460_write_toggle_en(struct iio_dev
> *indio_dev, uintptr_t priva
>  	if (ret)
>  		return ret;
>  
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
> -		return ad8460_enable_apg_mode(state, toggle_en);
> -	unreachable();
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	ret = ad8460_enable_apg_mode(state, toggle_en);
> +	iio_device_release_direct(indio_dev);
> +	return ret;
>  }
>  
>  static ssize_t ad8460_read_powerdown(struct iio_dev *indio_dev, uintptr_t
> private,
> @@ -421,14 +424,17 @@ static int ad8460_write_raw(struct iio_dev *indio_dev,
>  			    long mask)
>  {
>  	struct ad8460_state *state = iio_priv(indio_dev);
> +	int ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
>  		switch (chan->type) {
>  		case IIO_VOLTAGE:
> -			iio_device_claim_direct_scoped(return -EBUSY,
> indio_dev)
> -				return ad8460_set_sample(state, val);
> -			unreachable();
> +			if (!iio_device_claim_direct(indio_dev))
> +				return -EBUSY;
> +			ret = ad8460_set_sample(state, val);
> +			iio_device_release_direct(indio_dev);
> +			return ret;
>  		case IIO_CURRENT:
>  			return regmap_write(state->regmap,
> AD8460_CTRL_REG(0x04),
>  					   
> FIELD_PREP(AD8460_QUIESCENT_CURRENT_MSK, val));

Re: [PATCH v2 22/27] iio: dac: ad3552r-hs: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context.
> 
> Cc: Angelo Dureghello <adureghello@baylibre.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/dac/ad3552r-hs.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index c1dae58c1975..cd8dabb60c55 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -129,16 +129,19 @@ static int ad3552r_hs_write_raw(struct iio_dev
> *indio_dev,
>  				int val, int val2, long mask)
>  {
>  	struct ad3552r_hs_state *st = iio_priv(indio_dev);
> +	int ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
>  		/* For RAW accesses, stay always in simple-spi. */
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			return st->data->bus_reg_write(st->back,
> -				    AD3552R_REG_ADDR_CH_DAC_16B(chan-
> >channel),
> -				    val, 2);
> -		}
> -		unreachable();
> +		ret = st->data->bus_reg_write(st->back,
> +			AD3552R_REG_ADDR_CH_DAC_16B(chan->channel),
> +			val, 2);
> +
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	default:
>  		return -EINVAL;
>  	}

Re: [PATCH v2 24/27] iio: dummy: Stop using iio_device_claim_direct_scoped()

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context. Introduce two new utility functions
> to allow for direct returns with claim and release of direct mode
> in the caller.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/dummy/iio_simple_dummy.c | 119 ++++++++++++++++-----------
>  1 file changed, 70 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/iio/dummy/iio_simple_dummy.c
> b/drivers/iio/dummy/iio_simple_dummy.c
> index 09efacaf8f78..8575d4a08963 100644
> --- a/drivers/iio/dummy/iio_simple_dummy.c
> +++ b/drivers/iio/dummy/iio_simple_dummy.c
> @@ -267,6 +267,65 @@ static const struct iio_chan_spec iio_dummy_channels[] =
> {
>  	},
>  };
>  
> +static int __iio_dummy_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val)
> +{
> +	struct iio_dummy_state *st = iio_priv(indio_dev);
> +
> +	guard(mutex)(&st->lock);
> +	switch (chan->type) {
> +	case IIO_VOLTAGE:
> +		if (chan->output) {
> +			/* Set integer part to cached value */
> +			*val = st->dac_val;
> +			return IIO_VAL_INT;
> +		} else if (chan->differential) {
> +			if (chan->channel == 1)
> +				*val = st->differential_adc_val[0];
> +			else
> +				*val = st->differential_adc_val[1];
> +			return IIO_VAL_INT;
> +		} else {
> +			*val = st->single_ended_adc_val;
> +			return IIO_VAL_INT;
> +		}
> +
> +	case IIO_ACCEL:
> +		*val = st->accel_val;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int __iio_dummy_read_processed(struct iio_dev *indio_dev,
> +				      struct iio_chan_spec const *chan,
> +				      int *val)
> +{
> +	struct iio_dummy_state *st = iio_priv(indio_dev);
> +
> +	guard(mutex)(&st->lock);
> +	switch (chan->type) {
> +	case IIO_STEPS:
> +		*val = st->steps;
> +		return IIO_VAL_INT;
> +	case IIO_ACTIVITY:
> +		switch (chan->channel2) {
> +		case IIO_MOD_RUNNING:
> +			*val = st->activity_running;
> +			return IIO_VAL_INT;
> +		case IIO_MOD_WALKING:
> +			*val = st->activity_walking;
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  /**
>   * iio_dummy_read_raw() - data read function.
>   * @indio_dev:	the struct iio_dev associated with this device instance
> @@ -283,59 +342,21 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev,
>  			      long mask)
>  {
>  	struct iio_dummy_state *st = iio_priv(indio_dev);
> +	int ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW: /* magic value - channel value read */
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			guard(mutex)(&st->lock);
> -			switch (chan->type) {
> -			case IIO_VOLTAGE:
> -				if (chan->output) {
> -					/* Set integer part to cached value
> */
> -					*val = st->dac_val;
> -					return IIO_VAL_INT;
> -				} else if (chan->differential) {
> -					if (chan->channel == 1)
> -						*val = st-
> >differential_adc_val[0];
> -					else
> -						*val = st-
> >differential_adc_val[1];
> -					return IIO_VAL_INT;
> -				} else {
> -					*val = st->single_ended_adc_val;
> -					return IIO_VAL_INT;
> -				}
> -
> -			case IIO_ACCEL:
> -				*val = st->accel_val;
> -				return IIO_VAL_INT;
> -			default:
> -				return -EINVAL;
> -			}
> -		}
> -		unreachable();
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = __iio_dummy_read_raw(indio_dev, chan, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	case IIO_CHAN_INFO_PROCESSED:
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			guard(mutex)(&st->lock);
> -			switch (chan->type) {
> -			case IIO_STEPS:
> -				*val = st->steps;
> -				return IIO_VAL_INT;
> -			case IIO_ACTIVITY:
> -				switch (chan->channel2) {
> -				case IIO_MOD_RUNNING:
> -					*val = st->activity_running;
> -					return IIO_VAL_INT;
> -				case IIO_MOD_WALKING:
> -					*val = st->activity_walking;
> -					return IIO_VAL_INT;
> -				default:
> -					return -EINVAL;
> -				}
> -			default:
> -				return -EINVAL;
> -			}
> -		}
> -		unreachable();
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = __iio_dummy_read_processed(indio_dev, chan, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	case IIO_CHAN_INFO_OFFSET:
>  		/* only single ended adc -> 7 */
>  		*val = 7;

Re: [PATCH v2 02/27] iio: chemical: scd30: Use guard(mutex) to allow early returns

[Nuno Sá]

On Sun, 2025-02-09 at 18:05 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Auto cleanup based release of the lock allows for simpler code flow in a
> few functions with large multiplexing style switch statements and no
> common operations following the switch.
> 
> Suggested-by: David Lechner <dlechner@baylibre.com>
> Cc: Tomasz Duszynski <tomasz.duszynski@octakon.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> ---
> v2: Fix include ordering (David)
> ---
>  drivers/iio/chemical/scd30_core.c | 63 ++++++++++++++-----------------
>  include/linux/iio/iio.h           |  1 +
>  2 files changed, 29 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/iio/chemical/scd30_core.c
> b/drivers/iio/chemical/scd30_core.c
> index d613c54cb28d..23ba46f7ca32 100644
> --- a/drivers/iio/chemical/scd30_core.c
> +++ b/drivers/iio/chemical/scd30_core.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2020 Tomasz Duszynski <tomasz.duszynski@octakon.com>
>   */
>  #include <linux/bits.h>
> +#include <linux/cleanup.h>
>  #include <linux/completion.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
> @@ -198,112 +199,104 @@ static int scd30_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const
>  			  int *val, int *val2, long mask)
>  {
>  	struct scd30_state *state = iio_priv(indio_dev);
> -	int ret = -EINVAL;
> +	int ret;
>  	u16 tmp;
>  
> -	mutex_lock(&state->lock);
> +	guard(mutex)(&state->lock);
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
>  	case IIO_CHAN_INFO_PROCESSED:
>  		if (chan->output) {
>  			*val = state->pressure_comp;
> -			ret = IIO_VAL_INT;
> -			break;
> +			return IIO_VAL_INT;
>  		}
>  
>  		ret = iio_device_claim_direct_mode(indio_dev);
>  		if (ret)
> -			break;
> +			return ret;
>  
>  		ret = scd30_read(state);
>  		if (ret) {
>  			iio_device_release_direct_mode(indio_dev);
> -			break;
> +			return ret;
>  		}
>  
>  		*val = state->meas[chan->address];
>  		iio_device_release_direct_mode(indio_dev);
> -		ret = IIO_VAL_INT;
> -		break;
> +		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 0;
>  		*val2 = 1;
> -		ret = IIO_VAL_INT_PLUS_MICRO;
> -		break;
> +		return IIO_VAL_INT_PLUS_MICRO;
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		ret = scd30_command_read(state, CMD_MEAS_INTERVAL, &tmp);
>  		if (ret)
> -			break;
> +			return ret;
>  
>  		*val = 0;
>  		*val2 = 1000000000 / tmp;
> -		ret = IIO_VAL_INT_PLUS_NANO;
> -		break;
> +		return IIO_VAL_INT_PLUS_NANO;
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		ret = scd30_command_read(state, CMD_TEMP_OFFSET, &tmp);
>  		if (ret)
> -			break;
> +			return ret;
>  
>  		*val = tmp;
> -		ret = IIO_VAL_INT;
> -		break;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
>  	}
> -	mutex_unlock(&state->lock);
> -
> -	return ret;
>  }
>  
>  static int scd30_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec
> const *chan,
>  			   int val, int val2, long mask)
>  {
>  	struct scd30_state *state = iio_priv(indio_dev);
> -	int ret = -EINVAL;
> +	int ret;
>  
> -	mutex_lock(&state->lock);
> +	guard(mutex)(&state->lock);
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		if (val)
> -			break;
> +			return -EINVAL;
>  
>  		val = 1000000000 / val2;
>  		if (val < SCD30_MEAS_INTERVAL_MIN_S || val >
> SCD30_MEAS_INTERVAL_MAX_S)
> -			break;
> +			return -EINVAL;
>  
>  		ret = scd30_command_write(state, CMD_MEAS_INTERVAL, val);
>  		if (ret)
> -			break;
> +			return ret;
>  
>  		state->meas_interval = val;
> -		break;
> +		return 0;
>  	case IIO_CHAN_INFO_RAW:
>  		switch (chan->type) {
>  		case IIO_PRESSURE:
>  			if (val < SCD30_PRESSURE_COMP_MIN_MBAR ||
>  			    val > SCD30_PRESSURE_COMP_MAX_MBAR)
> -				break;
> +				return -EINVAL;
>  
>  			ret = scd30_command_write(state, CMD_START_MEAS,
> val);
>  			if (ret)
> -				break;
> +				return ret;
>  
>  			state->pressure_comp = val;
> -			break;
> +			return 0;
>  		default:
> -			break;
> +			return -EINVAL;
>  		}
> -		break;
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		if (val < 0 || val > SCD30_TEMP_OFFSET_MAX)
> -			break;
> +			return -EINVAL;
>  		/*
>  		 * Manufacturer does not explicitly specify min/max sensible
>  		 * values hence check is omitted for simplicity.
>  		 */
> -		ret = scd30_command_write(state, CMD_TEMP_OFFSET / 10, val);
> +		return scd30_command_write(state, CMD_TEMP_OFFSET / 10, val);
> +	default:
> +		return -EINVAL;
>  	}
> -	mutex_unlock(&state->lock);
> -
> -	return ret;
>  }
>  
>  static int scd30_write_raw_get_fmt(struct iio_dev *indio_dev, struct
> iio_chan_spec const *chan,
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index fe33835b19cf..5ed03e36178f 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -10,6 +10,7 @@
>  #include <linux/device.h>
>  #include <linux/cdev.h>
>  #include <linux/cleanup.h>
> +#include <linux/compiler_types.h>

Hmm, here it is... I guess some messed rebase and squash :)

- Nuno Sá

Re: [PATCH v2 27/27] iio: Drop iio_device_claim_direct_scoped() and related infrastructure

[Nuno Sá]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Scoped conditional automated cleanup turned out to be harder to work
> with than expected. Despite several attempts to find a better solution
> non have surfaced. As such rip it out of the IIO code.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  include/linux/iio/iio.h | 27 ---------------------------
>  1 file changed, 27 deletions(-)
> 
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 5ed03e36178f..07a0e8132e88 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -9,7 +9,6 @@
>  
>  #include <linux/device.h>
>  #include <linux/cdev.h>
> -#include <linux/cleanup.h>
>  #include <linux/compiler_types.h>
>  #include <linux/slab.h>
>  #include <linux/iio/types.h>
> @@ -688,32 +687,6 @@ static inline void iio_device_release_direct(struct
> iio_dev *indio_dev)
>  	__release(indio_dev);
>  }
>  
> -/*
> - * This autocleanup logic is normally used via
> - * iio_device_claim_direct_scoped().
> - */
> -DEFINE_GUARD(iio_claim_direct, struct iio_dev *,
> iio_device_claim_direct_mode(_T),
> -	     iio_device_release_direct_mode(_T))
> -
> -DEFINE_GUARD_COND(iio_claim_direct, _try, ({
> -			struct iio_dev *dev;
> -			int d = iio_device_claim_direct_mode(_T);
> -
> -			if (d < 0)
> -				dev = NULL;
> -			else
> -				dev = _T;
> -			dev;
> -		}))
> -
> -/**
> - * iio_device_claim_direct_scoped() - Scoped call to iio_device_claim_direct.
> - * @fail: What to do on failure to claim device.
> - * @iio_dev: Pointer to the IIO devices structure
> - */
> -#define iio_device_claim_direct_scoped(fail, iio_dev) \
> -	scoped_cond_guard(iio_claim_direct_try, fail, iio_dev)
> -
>  int iio_device_claim_buffer_mode(struct iio_dev *indio_dev);
>  void iio_device_release_buffer_mode(struct iio_dev *indio_dev);
>  

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

[Jonathan Cameron]

On Mon, 17 Feb 2025 10:38:11 +0000
Nuno Sá <noname.nuno@gmail.com> wrote:

> 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?
> 
Fixed up.  Thanks!
> > ---  
> 
> 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().  
> 

Re: [PATCH v2 11/27] iio: adc: ad4695: Stop using iio_device_claim_direct_scoped()

[Jonathan Cameron]

On Mon, 17 Feb 2025 10:48:03 +0000
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Sun, 2025-02-16 at 13:00 -0600, David Lechner wrote:
> > On 2/16/25 12:19 PM, Jonathan Cameron wrote:  
> > > On Sun,  9 Feb 2025 18:06:08 +0000
> > > Jonathan Cameron <jic23@kernel.org> wrote:
> > >   
> > > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > > 
> > > > This complex cleanup.h use case of conditional guards has proved
> > > > to be more trouble that it is worth in terms of false positive compiler
> > > > warnings and hard to read code.
> > > > 
> > > > Move directly to the new claim/release_direct() that allow sparse
> > > > to check for unbalanced context.  In some cases code is factored
> > > > out to utility functions that can do a direct return with the
> > > > claim and release around the call.
> > > > 
> > > > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > > ---
> > > > v2: Typo in commit description (David).
> > > > Note there are several sets current in flight that touch this driver.
> > > > I'll rebase as necessary depending on what order the dependencies resolve.  
> > > I've done this rebase and applied on the testing branch of iio.git.
> > > 
> > > Would appreciate a sanity check if anyone has time though!
> > > 
> > > New code is as follows.  The one corner I was not sure on was
> > > that for calibbias reading the direct mode claim was held for a long
> > > time.  That seems to be unnecessary as we have a copy of osr anyway
> > > in that function used for other purposes.
> > >   
> > 
> > ...
> >   
> > >  	case IIO_CHAN_INFO_CALIBBIAS:
> > > -		switch (chan->type) {
> > > -		case IIO_VOLTAGE:
> > > -			iio_device_claim_direct_scoped(return -EBUSY,
> > > indio_dev) {
> > > -				ret = regmap_read(st->regmap16,
> > > -					AD4695_REG_OFFSET_IN(chan-  
> > > >scan_index),  
> > > -					&reg_val);
> > > -				if (ret)
> > > -					return ret;
> > > -
> > > -				tmp = sign_extend32(reg_val, 15);
> > > -
> > > -				switch (cfg->oversampling_ratio) {
> > > -				case 1:
> > > -					*val = tmp / 4;
> > > -					*val2 = abs(tmp) % 4 * MICRO / 4;
> > > -					break;
> > > -				case 4:
> > > -					*val = tmp / 2;
> > > -					*val2 = abs(tmp) % 2 * MICRO / 2;
> > > -					break;
> > > -				case 16:
> > > -					*val = tmp;
> > > -					*val2 = 0;
> > > -					break;
> > > -				case 64:
> > > -					*val = tmp * 2;
> > > -					*val2 = 0;
> > > -					break;
> > > -				default:
> > > -					return -EINVAL;
> > > -				}
> > > -
> > > -				if (tmp < 0 && *val2) {
> > > -					*val *= -1;
> > > -					*val2 *= -1;
> > > -				}
> > > -
> > > -				return IIO_VAL_INT_PLUS_MICRO;
> > > +		switch (chan->type)
> > > +		case IIO_VOLTAGE: {
> > > +			if (!iio_device_claim_direct(indio_dev))
> > > +				return -EBUSY;
> > > +			ret = regmap_read(st->regmap16,
> > > +					  AD4695_REG_OFFSET_IN(chan-  
> > > >scan_index),  
> > > +					  &reg_val);
> > > +			iio_device_release_direct(indio_dev);
> > > +			if (ret)
> > > +				return ret;
> > > ////THIS IS THE BIT I WOuLD LIKE EYES on.  
> > 
> > Looks fine to me.  
> 
> +1
Thanks to you both for the quick replies.

Added Nuno's tags (and the fixlet on the first path - doh!)

Jonathan

> 
> - Nuno Sá
> >   
> > > +
> > > +			tmp = sign_extend32(reg_val, 15);
> > > +
> > > +			switch (osr) {
> > > +			case 1:
> > > +				*val = tmp / 4;
> > > +				*val2 = abs(tmp) % 4 * MICRO / 4;
> > > +				break;
> > > +			case 4:
> > > +				*val = tmp / 2;
> > > +				*val2 = abs(tmp) % 2 * MICRO / 2;
> > > +				break;
> > > +			case 16:
> > > +				*val = tmp;
> > > +				*val2 = 0;
> > > +				break;
> > > +			case 64:
> > > +				*val = tmp * 2;
> > > +				*val2 = 0;
> > > +				break;
> > > +			default:
> > > +				return -EINVAL;
> > > +			}
> > > +
> > > +			if (tmp < 0 && *val2) {
> > > +				*val *= -1;
> > > +				*val2 *= -1;
> > >  			}
> > > -			unreachable();
> > > +
> > > +			return IIO_VAL_INT_PLUS_MICRO;
> > >  		default:
> > >  			return -EINVAL;
> > >  		}  
> 

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

[Andy Shevchenko]

Sun, Feb 09, 2025 at 06:05:58PM +0000, Jonathan Cameron kirjoitti:
> 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.

...


> +/*
> + * 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;

While I understand the intention, I dislike the function return boolean and
hide the actual error code, it calls user to misuse and replace boolean false
by arbitrary error codes.

Can we rather return an error code, please?
(as a side effect it reduces the churn in the followup changes)

> +}

-- 
With Best Regards,
Andy Shevchenko

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

[Jonathan Cameron]

On Sat, 22 Feb 2025 17:51:02 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> Sun, Feb 09, 2025 at 06:05:58PM +0000, Jonathan Cameron kirjoitti:
> > 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.  
> 
> ...
> 
> 
> > +/*
> > + * 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;  
> 
> While I understand the intention, I dislike the function return boolean and
> hide the actual error code, it calls user to misuse and replace boolean false
> by arbitrary error codes.
> 
> Can we rather return an error code, please?
> (as a side effect it reduces the churn in the followup changes)
> 
Hi Andy,

I tried - see above.  It plays badly with sparse which is the whole point of
this exercise. Note that iio_device_claim_direct_mode() only ever returns one
error code -EBUSY. So reality is it's a boolean and this is a lot close
to mutex_trylock() than anything else hence the switch to a boolean return.

At the end of the full series (not yet posted) is a patch that gets rid
of their being any pretence this isn't a yes / no question and can
return other error values. This intermediate step does leave it looking
more confusing.

Churn wise if we'd been able to do keep the error return and make sparse
work I could have just applied this to the original functions and made
no changes at all to the vast majority of drivers.  Sadly that wasn't
to be. End result of ending up with a trylock type approach is cleaner
and more compact even if it's not what we have gotten used to for this
particular function.

Jonathan


> > +}  
> 

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

[Andy Shevchenko]

On Sat, Feb 22, 2025 at 7:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Sat, 22 Feb 2025 17:51:02 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > Sun, Feb 09, 2025 at 06:05:58PM +0000, Jonathan Cameron kirjoitti:
> > > 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.

...

> > > +/*
> > > + * 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;
> >
> > While I understand the intention, I dislike the function return boolean and
> > hide the actual error code, it calls user to misuse and replace boolean false
> > by arbitrary error codes.
> >
> > Can we rather return an error code, please?
> > (as a side effect it reduces the churn in the followup changes)
> >
> Hi Andy,
>
> I tried - see above.  It plays badly with sparse which is the whole point of
> this exercise. Note that iio_device_claim_direct_mode() only ever returns one
> error code -EBUSY. So reality is it's a boolean and this is a lot close
> to mutex_trylock() than anything else hence the switch to a boolean return.

Hmm... You mean that the following (still as a macro)

static inline int ...
{
  int ret;
  ...
  if (ret)
    return ret;

  __acquire_lock(...);
  return 0;
}

triggers the sparse warning?

> At the end of the full series (not yet posted) is a patch that gets rid
> of their being any pretence this isn't a yes / no question and can
> return other error values. This intermediate step does leave it looking
> more confusing.
>
> Churn wise if we'd been able to do keep the error return and make sparse
> work I could have just applied this to the original functions and made
> no changes at all to the vast majority of drivers.  Sadly that wasn't
> to be. End result of ending up with a trylock type approach is cleaner
> and more compact even if it's not what we have gotten used to for this
> particular function.

> > > +}

-- 
With Best Regards,
Andy Shevchenko

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

[Jonathan Cameron]

On Sat, 22 Feb 2025 22:58:00 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Feb 22, 2025 at 7:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > On Sat, 22 Feb 2025 17:51:02 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> > > Sun, Feb 09, 2025 at 06:05:58PM +0000, Jonathan Cameron kirjoitti:  
> > > > 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.  
> 
> ...
> 
> > > > +/*
> > > > + * 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;  
> > >
> > > While I understand the intention, I dislike the function return boolean and
> > > hide the actual error code, it calls user to misuse and replace boolean false
> > > by arbitrary error codes.
> > >
> > > Can we rather return an error code, please?
> > > (as a side effect it reduces the churn in the followup changes)
> > >  
> > Hi Andy,
> >
> > I tried - see above.  It plays badly with sparse which is the whole point of
> > this exercise. Note that iio_device_claim_direct_mode() only ever returns one
> > error code -EBUSY. So reality is it's a boolean and this is a lot close
> > to mutex_trylock() than anything else hence the switch to a boolean return.  
> 
> Hmm... You mean that the following (still as a macro)
> 
> static inline int ...
> {
>   int ret;
>   ...
>   if (ret)
>     return ret;
> 
>   __acquire_lock(...);
>   return 0;
> }
> 
> triggers the sparse warning?

Doing it in the ternary form of the same thing did trigger issues on fairly
simple cases. I don't recall if I tried this precise combination. The motivation
for this form originally being the __cond_acquires() markings (See later).
However, I 'think' based on other false positives including the smaller
set that required refactors to avoid triggering in this series, is that
sparse isn't coping well with more complex control flows.  So if we
assign a local integer variable and then check it, it seems to loose
track of what is going on in more cases than if we can do

	if (!trylock())
		return;

I'm not sure on what is going wrong.  However it seems sparse is effectively
unmaintained at the moment so even if we could rectify things without the
code upstream it gets us nowhere.  Hence my motivation to make this 'look
like' existing stuff sparse is checking. The nearest being trylock.
It makes me have more warm fuzzy feelings to be the same style of code
as the other cases using the same infrastructure, than doing something new.

Ideally sparse will eventually wake up again and we can have the __cond_acquires()
markings that we had in v1 and not have to have the implementation in the
header. That currently requires trylock type boolean returns.

So overall I think this direction makes sense.  Also can't complain that it
saves 1 line of code for every instance and removes false pretense that
this thing returned a useful error code.

Jonathan
> 
> > At the end of the full series (not yet posted) is a patch that gets rid
> > of their being any pretence this isn't a yes / no question and can
> > return other error values. This intermediate step does leave it looking
> > more confusing.
> >
> > Churn wise if we'd been able to do keep the error return and make sparse
> > work I could have just applied this to the original functions and made
> > no changes at all to the vast majority of drivers.  Sadly that wasn't
> > to be. End result of ending up with a trylock type approach is cleaner
> > and more compact even if it's not what we have gotten used to for this
> > particular function.  
> 
> > > > +}  
> 

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

[Andy Shevchenko]

On Tue, Feb 25, 2025 at 8:25 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
> On Sat, 22 Feb 2025 22:58:00 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Sat, Feb 22, 2025 at 7:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > On Sat, 22 Feb 2025 17:51:02 +0200
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > Sun, Feb 09, 2025 at 06:05:58PM +0000, Jonathan Cameron kirjoitti:

...

> > > > > +/*
> > > > > + * 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;
> > > >
> > > > While I understand the intention, I dislike the function return boolean and
> > > > hide the actual error code, it calls user to misuse and replace boolean false
> > > > by arbitrary error codes.
> > > >
> > > > Can we rather return an error code, please?
> > > > (as a side effect it reduces the churn in the followup changes)
> > >
> > > I tried - see above.  It plays badly with sparse which is the whole point of
> > > this exercise. Note that iio_device_claim_direct_mode() only ever returns one
> > > error code -EBUSY. So reality is it's a boolean and this is a lot close
> > > to mutex_trylock() than anything else hence the switch to a boolean return.
> >
> > Hmm... You mean that the following (still as a macro)
> >
> > static inline int ...
> > {
> >   int ret;
> >   ...
> >   if (ret)
> >     return ret;
> >
> >   __acquire_lock(...);
> >   return 0;
> > }
> >
> > triggers the sparse warning?
>
> Doing it in the ternary form of the same thing did trigger issues on fairly
> simple cases.

Oh...

> I don't recall if I tried this precise combination. The motivation
> for this form originally being the __cond_acquires() markings (See later).
> However, I 'think' based on other false positives including the smaller
> set that required refactors to avoid triggering in this series, is that
> sparse isn't coping well with more complex control flows.  So if we
> assign a local integer variable and then check it, it seems to loose
> track of what is going on in more cases than if we can do
>
>         if (!trylock())
>                 return;
>
> I'm not sure on what is going wrong.  However it seems sparse is effectively
> unmaintained at the moment so even if we could rectify things without the
> code upstream it gets us nowhere.  Hence my motivation to make this 'look
> like' existing stuff sparse is checking. The nearest being trylock.
> It makes me have more warm fuzzy feelings to be the same style of code
> as the other cases using the same infrastructure, than doing something new.

Me neither. I haven't looked at the sparse, I don't know how it works.

> Ideally sparse will eventually wake up again and we can have the __cond_acquires()
> markings that we had in v1 and not have to have the implementation in the
> header. That currently requires trylock type boolean returns.

FWIW, Torvalds recently uploaded a new PoC branch in his sparse tree,
perhaps he can do something about it at the end of the day. I would Cc
him to the most needed sparse updates, like __cond_acquires().

> So overall I think this direction makes sense.  Also can't complain that it
> saves 1 line of code for every instance and removes false pretense that
> this thing returned a useful error code.

Right, so we have a trylock semantics for time being...

> > > At the end of the full series (not yet posted) is a patch that gets rid
> > > of their being any pretence this isn't a yes / no question and can
> > > return other error values. This intermediate step does leave it looking
> > > more confusing.
> > >
> > > Churn wise if we'd been able to do keep the error return and make sparse
> > > work I could have just applied this to the original functions and made
> > > no changes at all to the vast majority of drivers.  Sadly that wasn't
> > > to be. End result of ending up with a trylock type approach is cleaner
> > > and more compact even if it's not what we have gotten used to for this
> > > particular function.
> >
> > > > > +}

-- 
With Best Regards,
Andy Shevchenko