Re: [PATCH v3] iio: chemical: sps30: Replace manual locking with RAII locking

[Jonathan Cameron]

On Thu, 14 May 2026 22:13:42 -0500
Maxwell Doose <m32285159@xxxxxxxxx> wrote:

> Replace manual mutex_lock() and mutex_unlock() calls with the much newer
> guard(mutex)() and scoped_guard() macros to enable RAII patterns,
> modernize the driver, and to increase readability.
> 
> Signed-off-by: Maxwell Doose <m32285159@xxxxxxxxx>
> ---
> v2:
>  - Switch over some scoped_guard()s to guard(mutex)() per David's
>    suggestion.
>  - Remove redundant whitespace per Andy's suggestion.
>  - Add new wrapper sps30_do_meas() per Andy's suggestion (see commit
>    message).
>  - Add Joshua's RB
>    (link: https://lore.kernel.org/linux-iio/CAKqfh0FWig8mRR-xhvnfcFeSinR6RySyPaf9Gbpb6WU+diiiUQ@xxxxxxxxxxxxxx/T/#t)
> 
> v3:
>  - Remove sps30_do_meas() wrapper per Andy's suggestion.
I think this wasn't what Andy meant. I think he meant just
push the guard into the the function you were wrapping.

>  - Remove Joshua's RB (major changes).
> 
>  drivers/iio/chemical/sps30.c | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
> index a934bf0298dd..9cb201cf0f15 100644
> --- a/drivers/iio/chemical/sps30.c
> +++ b/drivers/iio/chemical/sps30.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) Tomasz Duszynski <tduszyns@xxxxxxxxx>
>   */
>  
> +#include <linux/cleanup.h>
>  #include <linux/crc8.h>
>  #include <linux/delay.h>
>  #include <linux/i2c.h>
> @@ -111,9 +112,8 @@ static irqreturn_t sps30_trigger_handler(int irq, void *p)
>  		aligned_s64 ts;
>  	} scan;
>  
> -	mutex_lock(&state->lock);
> -	ret = sps30_do_meas(state, scan.data, ARRAY_SIZE(scan.data));
> -	mutex_unlock(&state->lock);
> +	scoped_guard(mutex, &state->lock)
> +		ret = sps30_do_meas(state, scan.data, ARRAY_SIZE(scan.data));
Every call to sps30_do_meas() is done with the lock held just over that call.

So just move the lock (as a guard) into sps30_do_meas().

I think that's what Andy was suggesting in v2 review, but maybe not!


>  	if (ret)
>  		goto err;
>