On Wed, 22 Apr 2026 15:45:56 +0100 Rodrigo Alencar via B4 Relay <[email protected]> wrote:
> From: Rodrigo Alencar <[email protected]> > > 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 <[email protected]> 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; > + } > +} >
