[PATCH v2 0/4] iio: accel: convert to guard(mutex)

[PATCH v2 0/4] iio: accel: convert to guard(mutex)

[Rajveer Chaudhari]

This series converts manual mutex_lock/mutex_unlock pairs to
guard(mutex) in two ADXL accelerometer drivers. Each conversion
also simplifies error handling by removing goto labels and
returning directly on error paths.

Alphabetically Header alignment dropped in v2.

Link to v1: https://lore.kernel.org/all/20260309142352.57407-1-
rajveer.chaudhari.linux@gmail.com/

Rajveer Chaudhari (4):
  iio: accel: bmc150: convert to guard(mutex)
  iio: accel: mma8452: convert to guard(mutex)
  iio: accel: mma9551: convert to guard(mutex)
  iio: accel: sca3000: convert to guard(mutex)

 drivers/iio/accel/bmc150-accel-core.c | 17 ++---
 drivers/iio/accel/mma8452.c           | 21 +++---
 drivers/iio/accel/mma9551.c           | 10 ++-
 drivers/iio/accel/sca3000.c           | 97 ++++++++++++---------------
 4 files changed, 60 insertions(+), 85 deletions(-)

-- 
2.53.0

[PATCH v2 1/4] iio: accel: bmc150: convert to guard(mutex)

[Rajveer Chaudhari]

Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
bmc150_accel_buffer_predisable() and
bmc150_accel_buffer_postenable(). This ensures the mutex is
released on all return paths and allows returning directly
without a goto label.

Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
---
v2: Cleaned mutex_unlock and goto in bmc150_accel_buffer_postenable(),
Dropped Header alignment change.
---
 drivers/iio/accel/bmc150-accel-core.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 42ccf0316ce5..bd9791c9fcf7 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -7,6 +7,7 @@
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
@@ -1485,15 +1486,15 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
 	if (iio_device_get_current_mode(indio_dev) == INDIO_BUFFER_TRIGGERED)
 		return 0;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (!data->watermark)
-		goto out;
+		return ret;
 
 	ret = bmc150_accel_set_interrupt(data, BMC150_ACCEL_INT_WATERMARK,
 					 true);
 	if (ret)
-		goto out;
+		return ret;
 
 	data->fifo_mode = BMC150_ACCEL_FIFO_MODE_FIFO;
 
@@ -1504,9 +1505,6 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
 					   false);
 	}
 
-out:
-	mutex_unlock(&data->mutex);
-
 	return ret;
 }
 
@@ -1517,19 +1515,16 @@ static int bmc150_accel_buffer_predisable(struct iio_dev *indio_dev)
 	if (iio_device_get_current_mode(indio_dev) == INDIO_BUFFER_TRIGGERED)
 		return 0;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (!data->fifo_mode)
-		goto out;
+		return 0;
 
 	bmc150_accel_set_interrupt(data, BMC150_ACCEL_INT_WATERMARK, false);
 	__bmc150_accel_fifo_flush(indio_dev, BMC150_ACCEL_FIFO_LENGTH, false);
 	data->fifo_mode = 0;
 	bmc150_accel_fifo_set_mode(data);
 
-out:
-	mutex_unlock(&data->mutex);
-
 	return 0;
 }
 
-- 
2.53.0

Re: [PATCH v2 1/4] iio: accel: bmc150: convert to guard(mutex)

[Andy Shevchenko]

On Mon, Mar 09, 2026 at 09:04:05PM +0530, Rajveer Chaudhari wrote:
> Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
> bmc150_accel_buffer_predisable() and
> bmc150_accel_buffer_postenable(). This ensures the mutex is
> released on all return paths and allows returning directly
> without a goto label.

Wait _at least_ 24h between the versions of the patch or patch series.
Let others give a chance to comment, review, et cetera!

So, based on the discussion in v1 it might make sense to have a sorting patches
as precursor for each of the drivers in question. But since it's indeed cosmetic
change I leave it to maintainers and other reviewers to judge.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/4] iio: accel: bmc150: convert to guard(mutex)

[Jonathan Cameron]

On Mon, 9 Mar 2026 18:28:58 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Mar 09, 2026 at 09:04:05PM +0530, Rajveer Chaudhari wrote:
> > Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
> > bmc150_accel_buffer_predisable() and
> > bmc150_accel_buffer_postenable(). This ensures the mutex is
> > released on all return paths and allows returning directly
> > without a goto label.  
> 
> Wait _at least_ 24h between the versions of the patch or patch series.
> Let others give a chance to comment, review, et cetera!
> 
> So, based on the discussion in v1 it might make sense to have a sorting patches
> as precursor for each of the drivers in question. But since it's indeed cosmetic
> change I leave it to maintainers and other reviewers to judge.

Given the code is being 'touched' anyway, I'd welcome precursor patches
sorting the headers prior to adding a new one.
Not something I'll insist on though.

Thanks,

Jonathan
> 

Re: [PATCH v2 1/4] iio: accel: bmc150: convert to guard(mutex)

[Jonathan Cameron]

On Mon,  9 Mar 2026 21:04:05 +0530
Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote:

> Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
Wrap commit messages up to 75 chars.
 mutex_lock()/mutex_unlock() pair with guard(mutex)()

> bmc150_accel_buffer_predisable() and
> bmc150_accel_buffer_postenable(). This ensures the mutex is
> released on all return paths and allows returning directly
> without a goto label.
> 
> Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
> ---
> v2: Cleaned mutex_unlock and goto in bmc150_accel_buffer_postenable(),
> Dropped Header alignment change.
> ---
>  drivers/iio/accel/bmc150-accel-core.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> index 42ccf0316ce5..bd9791c9fcf7 100644
> --- a/drivers/iio/accel/bmc150-accel-core.c
> +++ b/drivers/iio/accel/bmc150-accel-core.c
> @@ -7,6 +7,7 @@
>  #include <linux/module.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> +#include <linux/cleanup.h>
>  #include <linux/delay.h>
>  #include <linux/slab.h>
>  #include <linux/acpi.h>
> @@ -1485,15 +1486,15 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
>  	if (iio_device_get_current_mode(indio_dev) == INDIO_BUFFER_TRIGGERED)
>  		return 0;
>  
> -	mutex_lock(&data->mutex);
> +	guard(mutex)(&data->mutex);
>  
>  	if (!data->watermark)
> -		goto out;
> +		return ret;

return 0; and stop initializing ret to 0 at the start of the function.
That will make it clear that this is a 'good' exit path not an error one.
Its the path where we don't turn on the fifo.

>  
>  	ret = bmc150_accel_set_interrupt(data, BMC150_ACCEL_INT_WATERMARK,
>  					 true);
>  	if (ret)
> -		goto out;
> +		return ret;
>  
>  	data->fifo_mode = BMC150_ACCEL_FIFO_MODE_FIFO;
>  
> @@ -1504,9 +1505,6 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
>  					   false);
>  	}
>  
> -out:
> -	mutex_unlock(&data->mutex);
> -
>  	return ret;
I'd slightly prefer return ret moves up into the if (ret) {} block above and
we return 0 here.  Again to make it easier to spot where the good and bad paths are.

>  }
>  
> @@ -1517,19 +1515,16 @@ static int bmc150_accel_buffer_predisable(struct iio_dev *indio_dev)
>  	if (iio_device_get_current_mode(indio_dev) == INDIO_BUFFER_TRIGGERED)
>  		return 0;
>  
> -	mutex_lock(&data->mutex);
> +	guard(mutex)(&data->mutex);
>  
>  	if (!data->fifo_mode)
> -		goto out;
> +		return 0;
>  
>  	bmc150_accel_set_interrupt(data, BMC150_ACCEL_INT_WATERMARK, false);
>  	__bmc150_accel_fifo_flush(indio_dev, BMC150_ACCEL_FIFO_LENGTH, false);
>  	data->fifo_mode = 0;
>  	bmc150_accel_fifo_set_mode(data);
>  
> -out:
> -	mutex_unlock(&data->mutex);
> -
>  	return 0;
>  }
>  

[PATCH v2 2/4] iio: accel: mma8452: convert to guard(mutex)

[Rajveer Chaudhari]

Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
mma8452_change_config(). This ensures the mutex is
released on all return paths and allows returning directly
without a goto label.

Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
---
v2 : Dropped Header alignment change
---
 drivers/iio/accel/mma8452.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 15172ba2972c..09bb205a1b8f 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -20,6 +20,7 @@
 
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
+#include <linux/cleanup.h>
 #include <linux/property.h>
 #include <linux/i2c.h>
 #include <linux/iio/iio.h>
@@ -597,36 +598,30 @@ static int mma8452_change_config(struct mma8452_data *data, u8 reg, u8 val)
 	int ret;
 	int is_active;
 
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
 
 	is_active = mma8452_is_active(data);
-	if (is_active < 0) {
-		ret = is_active;
-		goto fail;
-	}
+	if (is_active < 0)
+		return is_active;
 
 	/* config can only be changed when in standby */
 	if (is_active > 0) {
 		ret = mma8452_standby(data);
 		if (ret < 0)
-			goto fail;
+			return ret;
 	}
 
 	ret = i2c_smbus_write_byte_data(data->client, reg, val);
 	if (ret < 0)
-		goto fail;
+		return ret;
 
 	if (is_active > 0) {
 		ret = mma8452_active(data);
 		if (ret < 0)
-			goto fail;
+			return ret;
 	}
 
-	ret = 0;
-fail:
-	mutex_unlock(&data->lock);
-
-	return ret;
+	return 0;
 }
 
 static int mma8452_set_power_mode(struct mma8452_data *data, u8 mode)
-- 
2.53.0

[PATCH v2 3/4] iio: accel: mma9551: convert to guard(mutex)

[Rajveer Chaudhari]

Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
mma9551_event_handler(). This ensures the mutex is
released on all return paths and allows returning directly
without a goto label.

Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
---
v2: Dropped Header alignment change
---
 drivers/iio/accel/mma9551.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
index 02195deada49..65a913972ce2 100644
--- a/drivers/iio/accel/mma9551.c
+++ b/drivers/iio/accel/mma9551.c
@@ -9,6 +9,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/iio/iio.h>
@@ -337,7 +338,7 @@ static irqreturn_t mma9551_event_handler(int irq, void *private)
 	u16 reg;
 	u8 val;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	for (i = 0; i < 3; i++)
 		if (irq == data->irqs[i]) {
@@ -349,7 +350,7 @@ static irqreturn_t mma9551_event_handler(int irq, void *private)
 		/* IRQ was triggered on 4th line, which we don't use. */
 		dev_warn(&data->client->dev,
 			 "irq triggered on unused line %d\n", data->irqs[3]);
-		goto out;
+		return IRQ_HANDLED;
 	}
 
 	switch (mma_axis) {
@@ -373,7 +374,7 @@ static irqreturn_t mma9551_event_handler(int irq, void *private)
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"error %d reading tilt register in IRQ\n", ret);
-		goto out;
+		return IRQ_HANDLED;
 	}
 
 	iio_push_event(indio_dev,
@@ -381,9 +382,6 @@ static irqreturn_t mma9551_event_handler(int irq, void *private)
 					  IIO_EV_TYPE_ROC, IIO_EV_DIR_RISING),
 		       iio_get_time_ns(indio_dev));
 
-out:
-	mutex_unlock(&data->mutex);
-
 	return IRQ_HANDLED;
 }
 
-- 
2.53.0

[PATCH v2 4/4] iio: accel: sca3000: convert to guard(mutex)

[Rajveer Chaudhari]

Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
sca3000_print_rev(), sca3000_ring_int_process(),
sca3000_read_event_config(), __sca3000_hw_ring_state_set(),
sca3000_hw_ring_preenable(), sca3000_hw_ring_postdisable(),
sca3000_clean_setup(), sca3000_stop_all_interrupts().
This ensures the mutex is released on all return paths and
allows returning directly without a goto label.

Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
---
v2: Dropped Header alignment change
---
 drivers/iio/accel/sca3000.c | 97 ++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 55 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 4a827be439a2..8a06602ab41c 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -9,6 +9,7 @@
 
 #include <linux/interrupt.h>
 #include <linux/fs.h>
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
@@ -424,16 +425,16 @@ static int sca3000_print_rev(struct iio_dev *indio_dev)
 	int ret;
 	struct sca3000_state *st = iio_priv(indio_dev);
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	ret = sca3000_read_data_short(st, SCA3000_REG_REVID_ADDR, 1);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
+
 	dev_info(&indio_dev->dev,
 		 "sca3000 revision major=%lu, minor=%lu\n",
 		 st->rx[0] & SCA3000_REG_REVID_MAJOR_MASK,
 		 st->rx[0] & SCA3000_REG_REVID_MINOR_MASK);
-error_ret:
-	mutex_unlock(&st->lock);
 
 	return ret;
 }
@@ -996,13 +997,14 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
 	struct sca3000_state *st = iio_priv(indio_dev);
 	int ret, i, num_available;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	if (val & SCA3000_REG_INT_STATUS_HALF) {
 		ret = sca3000_read_data_short(st, SCA3000_REG_BUF_COUNT_ADDR,
 					      1);
 		if (ret)
-			goto error_ret;
+			return;
+
 		num_available = st->rx[0];
 		/*
 		 * num_available is the total number of samples available
@@ -1011,7 +1013,8 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
 		ret = sca3000_read_data(st, SCA3000_REG_RING_OUT_ADDR, st->rx,
 					num_available * 2);
 		if (ret)
-			goto error_ret;
+			return;
+
 		for (i = 0; i < num_available / 3; i++) {
 			/*
 			 * Dirty hack to cover for 11 bit in fifo, 13 bit
@@ -1023,8 +1026,6 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
 			iio_push_to_buffers(indio_dev, st->rx + i * 3 * 2);
 		}
 	}
-error_ret:
-	mutex_unlock(&st->lock);
 }
 
 /**
@@ -1110,11 +1111,11 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
 	struct sca3000_state *st = iio_priv(indio_dev);
 	int ret;
 	/* read current value of mode register */
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	ret = sca3000_read_data_short(st, SCA3000_REG_MODE_ADDR, 1);
 	if (ret)
-		goto error_ret;
+		return ret;
 
 	switch (chan->channel2) {
 	case IIO_MOD_X_AND_Y_AND_Z:
@@ -1134,7 +1135,7 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
 			ret = sca3000_read_ctrl_reg(st,
 						SCA3000_REG_CTRL_SEL_MD_CTRL);
 			if (ret < 0)
-				goto error_ret;
+				return ret;
 			/* only supporting logical or's for now */
 			ret = !!(ret & sca3000_addresses[chan->address][2]);
 		}
@@ -1143,9 +1144,6 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
 		ret = -EINVAL;
 	}
 
-error_ret:
-	mutex_unlock(&st->lock);
-
 	return ret;
 }
 
@@ -1277,10 +1275,12 @@ int __sca3000_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
 	struct sca3000_state *st = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	ret = sca3000_read_data_short(st, SCA3000_REG_MODE_ADDR, 1);
 	if (ret)
-		goto error_ret;
+		return ret;
+
 	if (state) {
 		dev_info(&indio_dev->dev, "supposedly enabling ring buffer\n");
 		ret = sca3000_write_reg(st,
@@ -1290,8 +1290,6 @@ int __sca3000_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
 		ret = sca3000_write_reg(st,
 			SCA3000_REG_MODE_ADDR,
 			(st->rx[0] & ~SCA3000_REG_MODE_RING_BUF_ENABLE));
-error_ret:
-	mutex_unlock(&st->lock);
 
 	return ret;
 }
@@ -1310,26 +1308,20 @@ static int sca3000_hw_ring_preenable(struct iio_dev *indio_dev)
 	int ret;
 	struct sca3000_state *st = iio_priv(indio_dev);
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	/* Enable the 50% full interrupt */
 	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
 	if (ret)
-		goto error_unlock;
+		return ret;
+
 	ret = sca3000_write_reg(st,
 				SCA3000_REG_INT_MASK_ADDR,
 				st->rx[0] | SCA3000_REG_INT_MASK_RING_HALF);
 	if (ret)
-		goto error_unlock;
-
-	mutex_unlock(&st->lock);
+		return ret;
 
 	return __sca3000_hw_ring_state_set(indio_dev, 1);
-
-error_unlock:
-	mutex_unlock(&st->lock);
-
-	return ret;
 }
 
 static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
@@ -1342,17 +1334,15 @@ static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
 		return ret;
 
 	/* Disable the 50% full interrupt */
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
 	if (ret)
-		goto unlock;
-	ret = sca3000_write_reg(st,
+		return ret;
+
+	return sca3000_write_reg(st,
 				SCA3000_REG_INT_MASK_ADDR,
 				st->rx[0] & ~SCA3000_REG_INT_MASK_RING_HALF);
-unlock:
-	mutex_unlock(&st->lock);
-	return ret;
 }
 
 static const struct iio_buffer_setup_ops sca3000_ring_setup_ops = {
@@ -1372,25 +1362,26 @@ static int sca3000_clean_setup(struct sca3000_state *st)
 {
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	/* Ensure all interrupts have been acknowledged */
 	ret = sca3000_read_data_short(st, SCA3000_REG_INT_STATUS_ADDR, 1);
 	if (ret)
-		goto error_ret;
+		return ret;
 
 	/* Turn off all motion detection channels */
 	ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 	ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL,
 				     ret & SCA3000_MD_CTRL_PROT_MASK);
 	if (ret)
-		goto error_ret;
+		return ret;
 
 	/* Disable ring buffer */
 	ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 	ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL,
 				     (ret & SCA3000_REG_OUT_CTRL_PROT_MASK)
 				     | SCA3000_REG_OUT_CTRL_BUF_X_EN
@@ -1398,17 +1389,17 @@ static int sca3000_clean_setup(struct sca3000_state *st)
 				     | SCA3000_REG_OUT_CTRL_BUF_Z_EN
 				     | SCA3000_REG_OUT_CTRL_BUF_DIV_4);
 	if (ret)
-		goto error_ret;
+		return ret;
 	/* Enable interrupts, relevant to mode and set up as active low */
 	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
 	if (ret)
-		goto error_ret;
+		return ret;
 	ret = sca3000_write_reg(st,
 				SCA3000_REG_INT_MASK_ADDR,
 				(ret & SCA3000_REG_INT_MASK_PROT_MASK)
 				| SCA3000_REG_INT_MASK_ACTIVE_LOW);
 	if (ret)
-		goto error_ret;
+		return ret;
 	/*
 	 * Select normal measurement mode, free fall off, ring off
 	 * Ring in 12 bit mode - it is fine to overwrite reserved bits 3,5
@@ -1416,13 +1407,10 @@ static int sca3000_clean_setup(struct sca3000_state *st)
 	 */
 	ret = sca3000_read_data_short(st, SCA3000_REG_MODE_ADDR, 1);
 	if (ret)
-		goto error_ret;
-	ret = sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
-				(st->rx[0] & SCA3000_MODE_PROT_MASK));
+		return ret;
 
-error_ret:
-	mutex_unlock(&st->lock);
-	return ret;
+	return sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
+				(st->rx[0] & SCA3000_MODE_PROT_MASK));
 }
 
 static const struct iio_info sca3000_info = {
@@ -1504,18 +1492,17 @@ static int sca3000_stop_all_interrupts(struct sca3000_state *st)
 {
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
 	if (ret)
-		goto error_ret;
-	ret = sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
+		return ret;
+
+	return sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
 				(st->rx[0] &
 				 ~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
 				   SCA3000_REG_INT_MASK_RING_HALF |
 				   SCA3000_REG_INT_MASK_ALL_INTS)));
-error_ret:
-	mutex_unlock(&st->lock);
-	return ret;
 }
 
 static void sca3000_remove(struct spi_device *spi)
-- 
2.53.0

Re: [PATCH v2 4/4] iio: accel: sca3000: convert to guard(mutex)

[Jonathan Cameron]

On Mon,  9 Mar 2026 21:04:08 +0530
Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote:

> Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
> sca3000_print_rev(), sca3000_ring_int_process(),
> sca3000_read_event_config(), __sca3000_hw_ring_state_set(),
> sca3000_hw_ring_preenable(), sca3000_hw_ring_postdisable(),
> sca3000_clean_setup(), sca3000_stop_all_interrupts().
> This ensures the mutex is released on all return paths and
> allows returning directly without a goto label.
> 
> Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
Hi Rajveer,

This one isn't so simple. May need a couple of steps to get to the
code we want to end up with that takes full advantage of guard()

> ---
> v2: Dropped Header alignment change
> ---
>  drivers/iio/accel/sca3000.c | 97 ++++++++++++++++---------------------
>  1 file changed, 42 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
> index 4a827be439a2..8a06602ab41c 100644
> --- a/drivers/iio/accel/sca3000.c
> +++ b/drivers/iio/accel/sca3000.c
> @@ -9,6 +9,7 @@
>  
>  #include <linux/interrupt.h>
>  #include <linux/fs.h>
> +#include <linux/cleanup.h>
>  #include <linux/device.h>
>  #include <linux/slab.h>
>  #include <linux/kernel.h>
> @@ -424,16 +425,16 @@ static int sca3000_print_rev(struct iio_dev *indio_dev)
>  	int ret;
>  	struct sca3000_state *st = iio_priv(indio_dev);
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
> +
>  	ret = sca3000_read_data_short(st, SCA3000_REG_REVID_ADDR, 1);
>  	if (ret < 0)
> -		goto error_ret;
> +		return ret;
> +
>  	dev_info(&indio_dev->dev,
>  		 "sca3000 revision major=%lu, minor=%lu\n",
>  		 st->rx[0] & SCA3000_REG_REVID_MAJOR_MASK,
>  		 st->rx[0] & SCA3000_REG_REVID_MINOR_MASK);
> -error_ret:
> -	mutex_unlock(&st->lock);
>  
>  	return ret;

	return 0;

To make it clear that if we get here it is a good path.


>  }
> @@ -996,13 +997,14 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
>  	struct sca3000_state *st = iio_priv(indio_dev);
>  	int ret, i, num_available;
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
>  
>  	if (val & SCA3000_REG_INT_STATUS_HALF) {
Hmm. This code is unnecessarily complex - Probably written by a younger more foolish me ;)

So I think we really want to head towards something like the following.
Probably break this up though.  guard() patch first, then flip the logic in
a second patch.


Val is a parameter so the test can be outside the lock.  Also, we can just
exit early if the bit isn't set, reducing the indent considerably.
	
	if (!(val & SCA3000_REG_INT_STATUS_HALF))
		return;

	guard(mutex)(&st->lock);

	ret = sca3000_read_data_short(st, SCA3000_REG_BUF_COUNT_ADDR,  1);
	if (ret)
		return;

	num_available = st->rx[0];
	/*
	 * num_available is the total number of samples available
	 * i.e. number of time points * number of channels.
	 */
	ret = sca3000_read_data(st, SCA3000_REG_RING_OUT_ADDR, st->rx,
				num_available * 2);
	if (ret)
		return;

	for (i = 0; i < num_available / 3; i++) {
		/*
		 * Dirty hack to cover for 11 bit in fifo, 13 bit direct reading.
		 *
		 * In theory the bottom two bits are undefined. In reality they
		 * appear to always be 0.
		 */
		iio_push_to_buffers(indio_dev, st->rx + i * 3 * 2);
	}

>  		ret = sca3000_read_data_short(st, SCA3000_REG_BUF_COUNT_ADDR,
>  					      1);
>  		if (ret)
> -			goto error_ret;
> +			return;
> +
>  		num_available = st->rx[0];
>  		/*
>  		 * num_available is the total number of samples available
> @@ -1011,7 +1013,8 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
>  		ret = sca3000_read_data(st, SCA3000_REG_RING_OUT_ADDR, st->rx,
>  					num_available * 2);
>  		if (ret)
> -			goto error_ret;
> +			return;
> +
>  		for (i = 0; i < num_available / 3; i++) {
>  			/*
>  			 * Dirty hack to cover for 11 bit in fifo, 13 bit
> @@ -1023,8 +1026,6 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
>  			iio_push_to_buffers(indio_dev, st->rx + i * 3 * 2);
>  		}
>  	}
> -error_ret:
> -	mutex_unlock(&st->lock);
>  }
>  
>  /**
> @@ -1110,11 +1111,11 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
>  	struct sca3000_state *st = iio_priv(indio_dev);
>  	int ret;
>  	/* read current value of mode register */
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
>  
>  	ret = sca3000_read_data_short(st, SCA3000_REG_MODE_ADDR, 1);
>  	if (ret)
> -		goto error_ret;
> +		return ret;
>  
>  	switch (chan->channel2) {
>  	case IIO_MOD_X_AND_Y_AND_Z:
> @@ -1134,7 +1135,7 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
With a return where there is currently a ret = 0; just above here you can drop
the else and reduce the indent of this code.

>  			ret = sca3000_read_ctrl_reg(st,
>  						SCA3000_REG_CTRL_SEL_MD_CTRL);
>  			if (ret < 0)
> -				goto error_ret;
> +				return ret;
>  			/* only supporting logical or's for now */
>  			ret = !!(ret & sca3000_addresses[chan->address][2]);
Can return here too. It's a 'good' path but that's fine.

There are a couple above here as well that aren't in the context.

>  		}
> @@ -1143,9 +1144,6 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
>  		ret = -EINVAL;
return there.
>  	}
>  
> -error_ret:
> -	mutex_unlock(&st->lock);
> -
And ultimately there should be no way of getting here, so this can go.

>  	return ret;
>  }
>  
> @@ -1277,10 +1275,12 @@ int __sca3000_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
>  	struct sca3000_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
> +
>  	ret = sca3000_read_data_short(st, SCA3000_REG_MODE_ADDR, 1);
>  	if (ret)
> -		goto error_ret;
> +		return ret;
> +
>  	if (state) {
>  		dev_info(&indio_dev->dev, "supposedly enabling ring buffer\n");
>  		ret = sca3000_write_reg(st,
return in both these legs. No point in having the reader have to look down
to see if anything else happens if we just end up with return ret;

> @@ -1290,8 +1290,6 @@ int __sca3000_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
>  		ret = sca3000_write_reg(st,
>  			SCA3000_REG_MODE_ADDR,
>  			(st->rx[0] & ~SCA3000_REG_MODE_RING_BUF_ENABLE));
> -error_ret:
> -	mutex_unlock(&st->lock);
>  
>  	return ret;
>  }
>  static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
> @@ -1342,17 +1334,15 @@ static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
>  		return ret;
>  
>  	/* Disable the 50% full interrupt */
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
>  
>  	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
>  	if (ret)
> -		goto unlock;
> -	ret = sca3000_write_reg(st,
> +		return ret;
> +
> +	return sca3000_write_reg(st,
>  				SCA3000_REG_INT_MASK_ADDR,

Check indentation. I suspect this is all 1 space too little now.

>  				st->rx[0] & ~SCA3000_REG_INT_MASK_RING_HALF);
> -unlock:
> -	mutex_unlock(&st->lock);
> -	return ret;
>  }


>  
>  static const struct iio_info sca3000_info = {
> @@ -1504,18 +1492,17 @@ static int sca3000_stop_all_interrupts(struct sca3000_state *st)
>  {
>  	int ret;
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
> +
>  	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
>  	if (ret)
> -		goto error_ret;
> -	ret = sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
> +		return ret;
> +
> +	return sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
>  				(st->rx[0] &
As above. Indentation needs an update.

>  				 ~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
>  				   SCA3000_REG_INT_MASK_RING_HALF |
>  				   SCA3000_REG_INT_MASK_ALL_INTS)));
> -error_ret:
> -	mutex_unlock(&st->lock);
> -	return ret;
>  }
>  
>  static void sca3000_remove(struct spi_device *spi)

Re: [PATCH v2 4/4] iio: accel: sca3000: convert to guard(mutex)

[Rajveer Chaudhari]

Hello Jonathan,

Thanks for the review, I will add all of your suggested return path,
indentation fixes in v3.

But I have a question, should I submit the next version of patch right
now as you reviewed
the whole patch, or wait for 24 hrs as instructed by Andy Shevchenko
to let others
have a chance to comment, review.

On Tue, Mar 10, 2026 at 1:00 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> So I think we really want to head towards something like the following.
> Probably break this up though.  guard() patch first, then flip the logic in
> a second patch.

Yes, I will prepare and submit a separate patch for this logic flip.

Thankyou,
Rajveer

Re: [PATCH v2 4/4] iio: accel: sca3000: convert to guard(mutex)

[David Lechner]

On 3/9/26 2:58 PM, Rajveer Chaudhari wrote:
> Hello Jonathan,
> 
> Thanks for the review, I will add all of your suggested return path,
> indentation fixes in v3.
> 
> But I have a question, should I submit the next version of patch right
> now as you reviewed
> the whole patch, or wait for 24 hrs as instructed by Andy Shevchenko
> to let others
> have a chance to comment, review.

Waiting even longer than 24 hours is better. Not everyone has time to
review every day. Ideally, wait 1 week between revisions. If it is something
urgent, it can be sooner. Converting things to guard() is certainly not
urgent.

Re: [PATCH v2 4/4] iio: accel: sca3000: convert to guard(mutex)

[Rajveer Chaudhari]

On Tue, Mar 10, 2026 at 1:33 AM David Lechner <dlechner@baylibre.com> wrote:
>
> Waiting even longer than 24 hours is better. Not everyone has time to
> review every day. Ideally, wait 1 week between revisions. If it is something
> urgent, it can be sooner. Converting things to guard() is certainly not
> urgent.

Yes, understood.

Thank you

Re: [PATCH v2 0/4] iio: accel: convert to guard(mutex)

[Andy Shevchenko]

On Mon, Mar 09, 2026 at 09:04:04PM +0530, Rajveer Chaudhari wrote:
> This series converts manual mutex_lock/mutex_unlock pairs to
> guard(mutex) in two ADXL accelerometer drivers. Each conversion
> also simplifies error handling by removing goto labels and
> returning directly on error paths.

> Alphabetically Header alignment dropped in v2.

Why? It's valid change when in the series.

And again to make sure you got it, send a new version at least 24h later than
the previous one, not 2h.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/4] iio: accel: convert to guard(mutex)

[Rajveer Chaudhari]

On Mon, Mar 9, 2026 at 10:00 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> Why? It's valid change when in the series.

Hello Andy Thanks for the review, As Waqar Hameed previously
instructed that alphabetically aligning
headers is just a cosmetic change and to be kept in seperate patch. I
thought it would be a good idea to keep
only one logical change in the series.

> And again to make sure you got it, send a new version at least 24h later than
> the previous one, not 2h.

Yes, I will keep that in mind for future patches.

With Best Regards,
Rajveer Chaudhari

Re: [PATCH v2 0/4] iio: accel: convert to guard(mutex)

[Waqar Hameed]

On Mon, Mar 09, 2026 at 23:03 +0530 Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote:

> On Mon, Mar 9, 2026 at 10:00 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
>>
>> Why? It's valid change when in the series.
>
> Hello Andy Thanks for the review, As Waqar Hameed previously
> instructed that alphabetically aligning
> headers is just a cosmetic change and to be kept in seperate patch. I
> thought it would be a good idea to keep
> only one logical change in the series.

A separate patch can still be in the same patch _series_. I would advise
you to not change anything until we have a consensus here (as already
mentioned in other thread).

[...]

Re: [PATCH v2 0/4] iio: accel: convert to guard(mutex)

[Rajveer Chaudhari]

On Mon, Mar 9, 2026 at 11:27 PM Waqar Hameed <waqar.hameed@axis.com> wrote:
>
> A separate patch can still be in the same patch _series_. I would advise
> you to not change anything until we have a consensus here (as already
> mentioned in other thread).

Yes, understood.

Thanks,
Rajveer