Re: [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support

[Jonathan Cameron]

On Mon, 22 Jun 2026 11:12:19 +0200
Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:

> On Di, 2026-05-19 at 17:42 +0200, Janani Sunil wrote:
> > Add support for AD5529R 16-channel, 12/16 bit Digital to Analog Converter
> > 
> > Signed-off-by: Janani Sunil <janani.sunil@xxxxxxxxxx>
> > ---
> >  MAINTAINERS               |   1 +
> >  drivers/iio/dac/Kconfig   |  17 ++
> >  drivers/iio/dac/Makefile  |   1 +
> >  drivers/iio/dac/ad5529r.c | 527 ++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 546 insertions(+)
> >   
> [...]
> > diff --git a/drivers/iio/dac/ad5529r.c b/drivers/iio/dac/ad5529r.c
> > new file mode 100644
> > index 000000000000..9bb63030db95
> > --- /dev/null
> > +++ b/drivers/iio/dac/ad5529r.c
> > @@ -0,0 +1,527 @@  
> [...]
> > +static int ad5529r_reset(struct ad5529r_state *st)
> > +{
> > +	struct reset_control *rst;
> > +	int ret;
> > +
> > +	rst = devm_reset_control_get_optional_exclusive(&st->spi->dev, NULL);  
> 
> Consider using devm_reset_control_get_optional_exclusive_deasserted()
> to save a few lines, and to make sure the reset line is asserted again
> when the driver is unbound.

Given we can't assume it's a gpio reset at this level (it is but meh,
we shouldn't use the API that way), we can't guarantee it was ever asserted
so we probably have to do a dance of assert then deassert.

This is one of Sashiko's favourite things to complain about so
we ended up doing some digging into that path a few weeks back.

I'd really like that not to be the case so maybe that analysis is wrong.

Jonathan




> 
> > +	if (IS_ERR(rst))
> > +		return PTR_ERR(rst);
> > +
> > +	if (rst) {
> > +		ret = reset_control_deassert(rst);
> > +		if (ret)
> > +			return ret;  
> 
> This branch could then be removed.
> 
> > +	} else {
> > +		ret = regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
> > +				   AD5529R_INTERFACE_CONFIG_A_SW_RESET);
> > +		if (ret)
> > +			return ret;
> > +	}  
> 
> regards
> Philipp