Re: [PATCH] staging: iio: frequency: ad9834: use guard() for mutex locking

[Jonathan Cameron]

On Thu, 25 Jun 2026 15:04:55 +0300
Batu Ada Tutkun <batuadatutkun@xxxxxxxxx> wrote:

> Replace manual mutex_lock()/mutex_unlock() pairs with guard(mutex) in
> ad9834_write() and ad9834_store_wavetype(). The mutex is now released
> automatically when each function returns, removing the need for
> explicit unlock calls.
> 
> Signed-off-by: Batu Ada Tutkun <batuadatutkun@xxxxxxxxx>

Whilst we aren't taking this forwards, a few comments inline for future
reference.

> ---
>  drivers/staging/iio/frequency/ad9834.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> index bdb2580e2..f15819eaf 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -17,6 +17,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/err.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>

I'm guessing cleanup.h is missing as well. You should have both
when guard(mutex) is is in use.

>  #include <asm/div64.h>
>  
>  #include <linux/iio/iio.h>
> @@ -152,7 +153,7 @@ static ssize_t ad9834_write(struct device *dev,
>  	if (ret)
>  		return ret;
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
>  	switch ((u32)this_attr->address) {
>  	case AD9834_REG_FREQ0:
>  	case AD9834_REG_FREQ1:
> @@ -210,7 +211,6 @@ static ssize_t ad9834_write(struct device *dev,
>  	default:
>  		ret = -ENODEV;

The big advantage of using guard() is not removing mutex_unlock() but
instead that you can use simpler code flow.  Here you should return early.

There are a lot of error returns in this function and it may even
make sense to return len early if no error.

>  	}
> -	mutex_unlock(&st->lock);
>  
>  	return ret ? ret : len;
>  }
> @@ -226,7 +226,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
>  	int ret = 0;
>  	bool is_ad9833_7 = (st->devid == ID_AD9833) || (st->devid == ID_AD9837);
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
>  
>  	switch ((u32)this_attr->address) {
>  	case 0:
> @@ -269,7 +269,6 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
>  		st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
>  		ret = spi_sync(st->spi, &st->msg);
>  	}
> -	mutex_unlock(&st->lock);
Similar applies in this function where there are a lot of paths
that can become early error returns if guard() is used.

Jonathan

>  
>  	return ret ? ret : len;
>  }