Re: [PATCH] iio: adc: ti-ads7924: Use guard(mutex) for locking

[Jonathan Cameron]

On Sat, 18 Apr 2026 18:44:32 +0400
Giorgi Tchankvetadze <giorgitchankvetadze1997@xxxxxxxxx> wrote:

> Replace mutex_lock()/mutex_unlock() calls with guard(mutex)() to
> simplify locking and make cleanup automatic when the lock goes out
> of scope.
> 
> Signed-off-by: Giorgi Tchankvetadze <giorgitchankvetadze1997@xxxxxxxxx>
Hi Giorgi.

This is a slightly odd case as this is the only use of the mutex,
so all it currently protects against is concurrent calls via this path.
That's valid, but it lets us think about neater solutions.

That made me wonder if this is the best level to do the locking at.
I think it would be neater to push the guard() down to the top
of ads7924_get_adc_result() rather than having any lock handling
in ads924_read_raw()

You won't need the extra scope defined here and it will make it easier
to see why the lock is held.

Despite the comment saying it's about concurrent reads, I think it
is really about queuing multiple reads up so the sleep isn't repeated
if they are racing.  The regmap internal lock would otherwise be sufficient.

Thanks

Jonathan

> ---
>  drivers/iio/adc/ti-ads7924.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads7924.c b/drivers/iio/adc/ti-ads7924.c
> index bbcc4fc22b6e..21bd5cb973f4 100644
> --- a/drivers/iio/adc/ti-ads7924.c
> +++ b/drivers/iio/adc/ti-ads7924.c
> @@ -12,6 +12,7 @@
>   */
>  
>  #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/init.h>
> @@ -226,14 +227,13 @@ static int ads7924_read_raw(struct iio_dev *indio_dev,
>  	struct ads7924_data *data = iio_priv(indio_dev);
>  
>  	switch (mask) {
> -	case IIO_CHAN_INFO_RAW:
> -		mutex_lock(&data->lock);
> +	case IIO_CHAN_INFO_RAW: {
> +		guard(mutex)(&data->lock);
>  		ret = ads7924_get_adc_result(data, chan, val);
> -		mutex_unlock(&data->lock);
>  		if (ret < 0)
>  			return ret;
> -
>  		return IIO_VAL_INT;
> +		}
>  	case IIO_CHAN_INFO_SCALE:
>  		vref_uv = regulator_get_voltage(data->vref_reg);
>  		if (vref_uv < 0)