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

[Maxwell Doose]

On Fri, May 15, 2026 at 9:18 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> 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.
>

TBH I'm fairly confused on what Andy wanted in terms of "what do I
wrap". If he wants me to put the guard()() into the function call
itself I can do that but see the (ps) in my other email.

> >  - 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!
>

Perhaps, we'll see I suppose.

best regards,
max


>
> >       if (ret)
> >               goto err;
> >