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

Rajveer Chaudhari posted 4 patches 1 month ago
There is a newer version of this series
[PATCH v2 1/4] iio: accel: bmc150: convert to guard(mutex)
Posted by Rajveer Chaudhari 1 month ago
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)
Posted by Jonathan Cameron 1 month ago
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;
>  }
>
Re: [PATCH v2 1/4] iio: accel: bmc150: convert to guard(mutex)
Posted by Andy Shevchenko 1 month ago
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)
Posted by Jonathan Cameron 1 month ago
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
>