Re: [PATCH 22/22] iio: dac: ad5686: add gain control support

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 22 Apr 2026 15:45:56 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> Most of the supported devices rely on a GAIN pin to control a 2x
> multiplier applied to the output voltage. Other devices, e.g. the
> single-channel ones, provides a gain control through a bit field in the
> control register. Some designs might have the GAIN pin hardwired to
> VDD/VLOGIC or GND, which would still be fine for this patch, that allows
> the scale property to be configurable with two available options. In
> read_raw() and write_raw() implementations mutex guards are used to allow
> early returns.
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Minor stuff inline.

Thanks,

Jonathan

> ---
>  drivers/iio/dac/ad5686.c | 110 +++++++++++++++++++++++++++++++++++++++--------
>  drivers/iio/dac/ad5686.h |   8 ++++
>  2 files changed, 101 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index bec951afe8d0..adbf62848697 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c

>  static inline int ad5686_pd_mask_shift(const struct iio_chan_spec *chan)
> @@ -163,20 +167,25 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
>  	struct ad5686_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> +	guard(mutex)(&st->lock);
As below. I'd break out the guard() usage as a precusor cleanup patch.

> +
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
> -		mutex_lock(&st->lock);
>  		ret = ad5686_read(st, chan->address);
> -		mutex_unlock(&st->lock);
>  		if (ret < 0)
>  			return ret;
>  		*val = (ret >> chan->scan_type.shift) &
>  			GENMASK(chan->scan_type.realbits - 1, 0);
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
> -		*val = st->vref_mv;
> -		*val2 = chan->scan_type.realbits;
> -		return IIO_VAL_FRACTIONAL_LOG2;
> +		if (st->double_scale) {
> +			*val = st->scale_avail[2];
> +			*val2 = st->scale_avail[3];
> +		} else {
> +			*val = st->scale_avail[0];
> +			*val2 = st->scale_avail[1];
> +		}
> +		return IIO_VAL_INT_PLUS_NANO;
>  	}
>  	return -EINVAL;
>  }
> @@ -188,28 +197,77 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
>  			    long mask)
>  {
>  	struct ad5686_state *st = iio_priv(indio_dev);
> -	int ret;
> +
> +	guard(mutex)(&st->lock);
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
>  		if (!in_range(val, 0, 1 << chan->scan_type.realbits))
>  			return -EINVAL;
>  
> -		mutex_lock(&st->lock);
> -		ret = ad5686_write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
> -				   chan->address, val << chan->scan_type.shift);
> -		mutex_unlock(&st->lock);

As Andy pointed out, move this switch to guard() magic earlier in series.


> -		break;
> -	default:
> -		ret = -EINVAL;
> -	}
> +		return ad5686_write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
> +				    chan->address, val << chan->scan_type.shift);
> +	case IIO_CHAN_INFO_SCALE:
> +		if (val == st->scale_avail[0] && val2 == st->scale_avail[1])
> +			st->double_scale = false;
> +		else if (val == st->scale_avail[2] && val2 == st->scale_avail[3])
> +			st->double_scale = true;
> +		else
> +			return -EINVAL;
>  
> -	return ret;
> +		switch (st->chip_info->regmap_type) {
> +		case AD5310_REGMAP:
> +			return ad5310_control_sync(st);
> +		case AD5683_REGMAP:
> +			return ad5683_control_sync(st);
> +		case AD5686_REGMAP:
> +			gpiod_set_value_cansleep(st->gain_gpio, st->double_scale);
> +			return 0;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
>