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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The hardware reset logic fails to guarantee a reset pulse and skips software reset, breaking module reload.
--

commit e9e9fe5da17e349af411f3cecc999fcff44de1e2
Author: Janani Sunil <janani.sunil@analog.com>

iio: dac: Add AD5529R DAC driver support

This commit adds driver support for the Analog Devices AD5529R 16-channel,
12/16-bit DAC. It provides basic IIO voltage output capabilities and supports
configuring output ranges.

> diff --git a/drivers/iio/dac/ad5529r.c b/drivers/iio/dac/ad5529r.c
> new file mode 100644
> index 0000000000000..d2d0287d0f951
> --- /dev/null
> +++ b/drivers/iio/dac/ad5529r.c

[ ... ]

> +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);
> +	if (IS_ERR(rst))
> +		return PTR_ERR(rst);
> +
> +	if (rst) {
> +		ret = reset_control_deassert(rst);
> +		if (ret)
> +			return ret;
> +	} else {
> +		ret = regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
> +				   AD5529R_INTERFACE_CONFIG_A_SW_RESET);
> +		if (ret)
> +			return ret;
> +	}

[Severity: High]
Does this code ensure the device is actually reset on driver reload?

If the driver is unbound and rebound, the hardware reset line might already
be deasserted. Since reset_control_deassert() is called without a prior
assert, and there is no teardown action registered to assert the line on
unbind, will the device fail to receive a physical reset pulse?

Because a hardware reset control is present, the software reset in the else
block is also skipped. Could this combination leave the device in an
unpredictable state on subsequent probes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-ad5529r-driver-v4-0-2e4c02234a1a@analog.com?part=2