Re: [PATCH 2/7] iio: dac: ad5592r: use lock guards

[Jonathan Cameron]

On Mon, 07 Apr 2025 09:18:10 +0200
Bartosz Golaszewski <brgl@xxxxxxxx> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> 
> Use lock guards from linux/cleanup.h to simplify the code and remove
> some labels.
> 
> Note that we need to initialize some variables even though it's not
> technically required as scoped_guards() are implemented as for loops.
That is always a tiny bit irritating. Thanks for calling it out.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> ---
>  drivers/iio/dac/ad5592r-base.c | 119 ++++++++++++++++-------------------------
>  1 file changed, 47 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
> index fe4c35689d4d..bbe3b52c6a12 100644
> --- a/drivers/iio/dac/ad5592r-base.c
> +++ b/drivers/iio/dac/ad5592r-base.c

Just one case I'd prefer done a tiny bit differently so as to alway do
early returns rather than very nearly always.


>  	udelay(250);
> @@ -249,45 +235,43 @@ static int ad5592r_set_channel_modes(struct ad5592r_state *st)
>  		}
>  	}
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
>  
>  	/* Pull down unused pins to GND */
>  	ret = ops->reg_write(st, AD5592R_REG_PULLDOWN, pulldown);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_TRISTATE, tristate);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	/* Configure pins that we use */
>  	ret = ops->reg_write(st, AD5592R_REG_DAC_EN, dac);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_ADC_EN, adc);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_GPIO_SET, st->gpio_val);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_GPIO_OUT_EN, st->gpio_out);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_GPIO_IN_EN, st->gpio_in);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	/* Verify that we can read back at least one register */
>  	ret = ops->reg_read(st, AD5592R_REG_ADC_EN, &read_back);
>  	if (!ret && (read_back & 0xff) != adc)
>  		ret = -EIO;
		return -EIO; 
for this one.

>  
> -err_unlock:
> -	mutex_unlock(&st->lock);
>  	return ret;
return 0; here

>  }