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@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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;
>  }
>