Linux IIO development
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Maxwell Doose <m32285159@gmail.com>
Cc: "Tomasz Duszynski" <tduszyns@gmail.com>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	linux-iio@vger.kernel.org (open list:IIO SUBSYSTEM AND DRIVERS),
	linux-kernel@vger.kernel.org (open list)
Subject: Re: [PATCH v3] iio: chemical: sps30: Replace manual locking with RAII locking
Date: Fri, 15 May 2026 15:18:40 +0100	[thread overview]
Message-ID: <20260515151840.48a42705@jic23-huawei> (raw)
In-Reply-To: <20260515031343.56039-1-m32285159@gmail.com>

On Thu, 14 May 2026 22:13:42 -0500
Maxwell Doose <m32285159@gmail.com> 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@gmail.com>
> ---
> 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@mail.gmail.com/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@gmail.com>
>   */
>  
> +#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;
>  

  reply	other threads:[~2026-05-15 14:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15  3:13 [PATCH v3] iio: chemical: sps30: Replace manual locking with RAII locking Maxwell Doose
2026-05-15 14:18 ` Jonathan Cameron [this message]
2026-05-15 14:35   ` Maxwell Doose
2026-05-15 14:57     ` Jonathan Cameron
2026-05-17  8:05       ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260515151840.48a42705@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m32285159@gmail.com \
    --cc=nuno.sa@analog.com \
    --cc=tduszyns@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox