Re: [PATCH v11 2/6] iio: adc: ad4691: add initial driver for AD4691 family

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing power-on and reset recovery delays before accessing SPI registers.
--

commit 0154ecf4e1bff3698a674b01e6191a828b294b82
Author: Radu Sabau <radu.sabau@analog.com>

iio: adc: ad4691: add initial driver for AD4691 family

Add support for the Analog Devices AD4691 family of high-speed,
low-power multichannel SAR ADCs: AD4691 (16-ch, 500 kSPS),
AD4692 (16-ch, 1 MSPS), AD4693 (8-ch, 500 kSPS) and
AD4694 (8-ch, 1 MSPS).

> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> new file mode 100644
> index 0000000000000..ba77e1bfef166
> --- /dev/null
> +++ b/drivers/iio/adc/ad4691.c

[ ... ]

> @@ -0,0 +1,756 @@

[ ... ]

> +static int ad4691_reset(struct ad4691_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct reset_control *rst;
> +
> +	rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	if (IS_ERR(rst))
> +		return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset\n");
> +
> +	if (rst) {
> +		/*
> +		 * Assert the reset line before sleeping to guarantee a proper
> +		 * reset pulse on every probe, including driver reloads where
> +		 * the line may already be deasserted (reset_control_put() does
> +		 * not re-assert on release).
> +		 * devm_reset_control_get_optional_exclusive_deasserted() cannot
> +		 * be used because it deasserts immediately without delay; the
> +		 * datasheet (Table 5) requires a ≥300 µs reset pulse width
> +		 * before deassertion.
> +		 */
> +		reset_control_assert(rst);
> +		fsleep(300);
> +		return reset_control_deassert(rst);

Should there be a delay here after reset_control_deassert() to allow the
device to recover and load OTP trim data before the next SPI accesses?

Without a recovery delay, could subsequent configuration commands in
ad4691_config() be dropped by the hardware, leaving it in an uninitialized
state?

> +	}
> +
> +	/* No hardware reset available, fall back to software reset. */
> +	return regmap_write(st->regmap, AD4691_SPI_CONFIG_A_REG,
> +			    AD4691_SW_RESET);

Similarly, does this software reset require a recovery delay before we start
sending configuration commands to the device?

> +}

[ ... ]

> +static int ad4691_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct iio_dev *indio_dev;
> +	struct ad4691_state *st;
> +	int ret;

[ ... ]

> +	ret = ad4691_regulator_setup(st);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad4691_reset(st);

Is a power-on delay necessary here after ad4691_regulator_setup()
and before we attempt the first SPI communication in ad4691_reset()?

If the hardware reset is not available, ad4691_reset() sends a software reset
via SPI. If the internal POR circuit is still initializing immediately after
the regulators are enabled, could this initial SPI write be ignored by the
device?

> +	if (ret)
> +		return ret;
> +
> +	ret = ad4691_config(st);
> +	if (ret)
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-ad4692-multichannel-sar-adc-driver-v11-0-eab27d852ac2@analog.com?part=2