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

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 19 May 2026 15:20:23 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:

> From: Radu Sabau <radu.sabau@analog.com>
> 
> 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).
> 
> The driver implements a custom regmap layer over raw SPI to handle the
> device's mixed 1/2/3/4-byte register widths and uses the standard IIO
> read_raw/write_raw interface for single-channel reads.
> 
> The chip idles in Autonomous Mode so that single-shot read_raw can use
> the internal oscillator without disturbing the hardware configuration.
> 
> Three voltage supply domains are managed: avdd (required), vio, and a
> reference supply on either the REF pin (ref-supply, external buffer)
> or the REFIN pin (refin-supply, uses the on-chip reference buffer;
> REFBUF_EN is set accordingly). Hardware reset is performed by asserting
> the reset-gpios GPIO line for at least 300 µs then deasserting it;
> a software reset via SPI_CONFIG_A is used as fallback when no reset
> GPIO is provided.
> 
> Accumulator channel masking for single-shot reads uses ACC_MASK_REG via
> an ADDR_DESCENDING SPI write, which covers both mask bytes in a single
> 16-bit transfer.
> 
> IIO_CHAN_INFO_SAMP_FREQ is exposed as info_mask_separate. The oscillator
> is shared hardware — writing any channel's sampling_frequency attribute
> sets it for all others — but per-channel attributes are used throughout
> the series to avoid an ABI change when per-channel oversampling ratios
> are introduced in a later commit, at which point the effective output
> rate (osc_freq / osr[N]) becomes genuinely per-channel.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>

One really small thing given you mention you'll be doing a v13.

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



> +static int ad4691_reset(struct ad4691_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct reset_control *rst;
> +	int ret;
> +
> +	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 to guarantee a clean reset pulse on
> +		 * every probe, including driver reloads where the line may
> +		 * already be deasserted (reset_control_put() does not
> +		 * re-assert on release). tRESETL (minimum pulse width) = 10 ns
> +		 * (Table 5); kernel function-call overhead alone exceeds this,
> +		 * so no explicit delay is needed between assert and deassert.
> +		 */
> +		reset_control_assert(rst);
> +		ret = reset_control_deassert(rst);
> +		if (ret)
> +			return ret;

Really trivial but seems like this could be refactored to share the sleep 
code + perhaps more usefully the comment.



	} else {
		/* No hardware reset available, fall back to software reset. */
		ret = regmap_write(st->regmap, AD4691_SPI_CONFIG_A_REG, AD4691_SW_RESET);
		if (ret)
			return ret;
	}
	/*
	 * Wait tHWR = 300 µs (Table 5) for the device to complete its
	 * internal reset sequence before accepting SPI commands.
	 */
	fsleep(300);

	return 0;
}
> +		/*
> +		 * Wait tHWR = 300 µs (Table 5) for the device to complete its
> +		 * internal reset sequence before accepting SPI commands.
> +		 */
> +		fsleep(300);
> +		return 0;
> +	}
> +
> +	/* No hardware reset available, fall back to software reset. */
> +	ret = regmap_write(st->regmap, AD4691_SPI_CONFIG_A_REG, AD4691_SW_RESET);
> +	if (ret)
> +		return ret;
> +	/*
> +	 * Wait tSWR = 300 µs (Table 5) for the device to complete its
> +	 * internal reset sequence before accepting SPI commands.
> +	 */
> +	fsleep(300);
> +	return 0;
> +}