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

[Uwe Kleine-König]

On Tue, Mar 10, 2026 at 04:32:23PM +0200, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@xxxxxxxxxx>
> 
> 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.
> 
> Two buffered operating modes are supported, auto-detected from the
> device tree:
> 
>     - CNV Clock Mode:  an external PWM drives the CNV pin; the sampling
>                        rate is controlled via the PWM period. Requires a
>                        reference clock and a DATA_READY interrupt.
> 
>     - Manual Mode:     CNV is tied to SPI CS; each SPI transfer triggers
>                        a conversion and returns the previous result
>                        (pipelined). No external clock or interrupt needed.
> 
> In both modes the chip idles in Autonomous Mode so that single-shot
> read_raw can use the internal oscillator without disturbing the
> hardware configuration.
> 
> Signed-off-by: Radu Sabau <radu.sabau@xxxxxxxxxx>
> ---
>  MAINTAINERS              |   1 +
>  drivers/iio/adc/Kconfig  |  11 +
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad4691.c | 773 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 786 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9994d107d88d..5325f7d3b7f4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1490,6 +1490,7 @@ L:	linux-iio@xxxxxxxxxxxxxxx
>  S:	Supported
>  W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
> +F:	drivers/iio/adc/ad4691.c
>  F:	include/dt-bindings/iio/adc/adi,ad4691.h
>  
>  ANALOG DEVICES INC AD4695 DRIVER
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 60038ae8dfc4..3685a03aa8dc 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -139,6 +139,17 @@ config AD4170_4
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called ad4170-4.
>  
> +config AD4691
> +	tristate "Analog Devices AD4691 Family ADC Driver"
> +	depends on SPI
> +	select REGMAP
> +	help
> +	  Say yes here to build support for Analog Devices AD4691 Family MuxSAR
> +	  SPI analog to digital converters (ADC).
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ad4691.
> +
>  config AD4695
>  	tristate "Analog Device AD4695 ADC Driver"
>  	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index c76550415ff1..4ac1ea09d773 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_AD4080) += ad4080.o
>  obj-$(CONFIG_AD4130) += ad4130.o
>  obj-$(CONFIG_AD4134) += ad4134.o
>  obj-$(CONFIG_AD4170_4) += ad4170-4.o
> +obj-$(CONFIG_AD4691) += ad4691.o
>  obj-$(CONFIG_AD4695) += ad4695.o
>  obj-$(CONFIG_AD4851) += ad4851.o
>  obj-$(CONFIG_AD7091R) += ad7091r-base.o
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> new file mode 100644
> index 000000000000..528c37a9a383
> --- /dev/null
> +++ b/drivers/iio/adc/ad4691.c
> @@ -0,0 +1,773 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024-2026 Analog Devices, Inc.
> + * Author: Radu Sabau <radu.sabau@xxxxxxxxxx>
> + */
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/cleanup.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/math.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/util_macros.h>
> +#include <linux/units.h>
> +#include <linux/unaligned.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#include <dt-bindings/iio/adc/adi,ad4691.h>
> +
> +#define AD4691_VREF_MIN				2400000
> +#define AD4691_VREF_MAX				5250000
> +
> +/*
> + * Default sampling frequency for MANUAL_MODE.
> + * Each sample needs (num_channels + 1) SPI transfers of 24 bits.
> + * The factor 36 = 24 * 3/2 folds in a 50% scheduling margin:
> + *   freq = spi_hz / (24 * 3/2 * (num_channels + 1))
> + *        = spi_hz / (36 * (num_channels + 1))
> + */
> +#define AD4691_MANUAL_MODE_STD_FREQ(x, y)	((y) / (36 * ((x) + 1)))
> +#define AD4691_BITS_PER_XFER			24
> +#define AD4691_CNV_DUTY_CYCLE_NS		380
> +#define AD4691_MAX_CONV_PERIOD_US		800
> +
> +#define AD4691_SEQ_ALL_CHANNELS_OFF		0x00
> +#define AD4691_STATE_RESET_ALL			0x01
> +
> +#define AD4691_REF_CTRL_MASK			GENMASK(4, 2)
> +
> +#define AD4691_DEVICE_MANUAL			0x14
> +#define AD4691_DEVICE_REGISTER			0x10
> +#define AD4691_AUTONOMOUS_MODE_VAL		0x02
> +
> +#define AD4691_NOOP				0x00
> +#define AD4691_ADC_CHAN(ch)			((0x10 + (ch)) << 3)
> +
> +#define AD4691_STATUS_REG			0x14
> +#define AD4691_CLAMP_STATUS1_REG		0x1A
> +#define AD4691_CLAMP_STATUS2_REG		0x1B
> +#define AD4691_DEVICE_SETUP			0x20
> +#define AD4691_REF_CTRL				0x21
> +#define AD4691_OSC_FREQ_REG			0x23
> +#define AD4691_STD_SEQ_CONFIG			0x25
> +#define AD4691_SPARE_CONTROL			0x2A
> +
> +#define AD4691_OSC_EN_REG			0x180
> +#define AD4691_STATE_RESET_REG			0x181
> +#define AD4691_ADC_SETUP			0x182
> +#define AD4691_ACC_MASK1_REG			0x184
> +#define AD4691_ACC_MASK2_REG			0x185
> +#define AD4691_ACC_COUNT_LIMIT(n)		(0x186 + (n))
> +#define AD4691_ACC_COUNT_VAL			0x3F
> +#define AD4691_GPIO_MODE1_REG			0x196
> +#define AD4691_GPIO_MODE2_REG			0x197
> +#define AD4691_GPIO_READ			0x1A0
> +#define AD4691_ACC_STATUS_FULL1_REG		0x1B0
> +#define AD4691_ACC_STATUS_FULL2_REG		0x1B1
> +#define AD4691_ACC_STATUS_OVERRUN1_REG		0x1B2
> +#define AD4691_ACC_STATUS_OVERRUN2_REG		0x1B3
> +#define AD4691_ACC_STATUS_SAT1_REG		0x1B4
> +#define AD4691_ACC_STATUS_SAT2_REG		0x1BE
> +#define AD4691_ACC_SAT_OVR_REG(n)		(0x1C0 + (n))
> +#define AD4691_AVG_IN(n)			(0x201 + (2 * (n)))
> +#define AD4691_AVG_STS_IN(n)			(0x222 + (3 * (n)))
> +#define AD4691_ACC_IN(n)			(0x252 + (3 * (n)))
> +#define AD4691_ACC_STS_DATA(n)			(0x283 + (4 * (n)))
> +
> +enum ad4691_adc_mode {
> +	AD4691_CNV_CLOCK_MODE,
> +	AD4691_MANUAL_MODE,
> +};
> +
> +enum ad4691_gpio_mode {
> +	AD4691_ADC_BUSY = 4,
> +	AD4691_DATA_READY = 6,
> +};
> +
> +enum ad4691_ref_ctrl {
> +	AD4691_VREF_2P5 = 0,
> +	AD4691_VREF_3P0,
> +	AD4691_VREF_3P3,
> +	AD4691_VREF_4P096,
> +	AD4691_VREF_5P0,
> +};
> +
> +struct ad4691_chip_info {
> +	const struct iio_chan_spec *channels;
> +	const struct iio_chan_spec *manual_channels;
> +	const char *name;
> +	int num_channels;
> +	int max_rate;
> +};
> +
> +#define AD4691_CHANNEL(chan, index, real_bits, storage_bits, _shift)	\
> +	{								\
> +		.type = IIO_VOLTAGE,					\
> +		.indexed = 1,						\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ)	\
> +					   | BIT(IIO_CHAN_INFO_SCALE),	\
> +		.channel = chan,					\
> +		.scan_index = index,					\
> +		.scan_type = {						\
> +			.sign = 'u',					\
> +			.realbits = real_bits,				\
> +			.storagebits = storage_bits,			\
> +			.shift = _shift,				\
> +		},							\
> +	}
> +
> +static const struct iio_chan_spec ad4691_channels[] = {
> +	AD4691_CHANNEL(0, 0, 16, 32, 0),
> +	AD4691_CHANNEL(1, 1, 16, 32, 0),
> +	AD4691_CHANNEL(2, 2, 16, 32, 0),
> +	AD4691_CHANNEL(3, 3, 16, 32, 0),
> +	AD4691_CHANNEL(4, 4, 16, 32, 0),
> +	AD4691_CHANNEL(5, 5, 16, 32, 0),
> +	AD4691_CHANNEL(6, 6, 16, 32, 0),
> +	AD4691_CHANNEL(7, 7, 16, 32, 0),
> +	AD4691_CHANNEL(8, 8, 16, 32, 0),
> +	AD4691_CHANNEL(9, 9, 16, 32, 0),
> +	AD4691_CHANNEL(10, 10, 16, 32, 0),
> +	AD4691_CHANNEL(11, 11, 16, 32, 0),
> +	AD4691_CHANNEL(12, 12, 16, 32, 0),
> +	AD4691_CHANNEL(13, 13, 16, 32, 0),
> +	AD4691_CHANNEL(14, 14, 16, 32, 0),
> +	AD4691_CHANNEL(15, 15, 16, 32, 0)
> +};
> +
> +static const struct iio_chan_spec ad4693_channels[] = {
> +	AD4691_CHANNEL(0, 0, 16, 32, 0),
> +	AD4691_CHANNEL(1, 1, 16, 32, 0),
> +	AD4691_CHANNEL(2, 2, 16, 32, 0),
> +	AD4691_CHANNEL(3, 3, 16, 32, 0),
> +	AD4691_CHANNEL(4, 4, 16, 32, 0),
> +	AD4691_CHANNEL(5, 5, 16, 32, 0),
> +	AD4691_CHANNEL(6, 6, 16, 32, 0),
> +	AD4691_CHANNEL(7, 7, 16, 32, 0)
> +};
> +
> +static const struct iio_chan_spec ad4691_manual_channels[] = {
> +	AD4691_CHANNEL(0, 0, 16, 24, 8),
> +	AD4691_CHANNEL(1, 1, 16, 24, 8),
> +	AD4691_CHANNEL(2, 2, 16, 24, 8),
> +	AD4691_CHANNEL(3, 3, 16, 24, 8),
> +	AD4691_CHANNEL(4, 4, 16, 24, 8),
> +	AD4691_CHANNEL(5, 5, 16, 24, 8),
> +	AD4691_CHANNEL(6, 6, 16, 24, 8),
> +	AD4691_CHANNEL(7, 7, 16, 24, 8),
> +	AD4691_CHANNEL(8, 8, 16, 24, 8),
> +	AD4691_CHANNEL(9, 9, 16, 24, 8),
> +	AD4691_CHANNEL(10, 10, 16, 24, 8),
> +	AD4691_CHANNEL(11, 11, 16, 24, 8),
> +	AD4691_CHANNEL(12, 12, 16, 24, 8),
> +	AD4691_CHANNEL(13, 13, 16, 24, 8),
> +	AD4691_CHANNEL(14, 14, 16, 24, 8),
> +	AD4691_CHANNEL(15, 15, 16, 24, 8)
> +};
> +
> +static const struct iio_chan_spec ad4693_manual_channels[] = {
> +	AD4691_CHANNEL(0, 0, 16, 24, 8),
> +	AD4691_CHANNEL(1, 1, 16, 24, 8),
> +	AD4691_CHANNEL(2, 2, 16, 24, 8),
> +	AD4691_CHANNEL(3, 3, 16, 24, 8),
> +	AD4691_CHANNEL(4, 4, 16, 24, 8),
> +	AD4691_CHANNEL(5, 5, 16, 24, 8),
> +	AD4691_CHANNEL(6, 6, 16, 24, 8),
> +	AD4691_CHANNEL(7, 7, 16, 24, 8)
> +};
> +
> +static const struct ad4691_chip_info ad4691_ad4691 = {
> +	.channels = ad4691_channels,
> +	.manual_channels = ad4691_manual_channels,
> +	.name = "ad4691",
> +	.num_channels = ARRAY_SIZE(ad4691_channels),
> +	.max_rate = 500000,
> +};
> +
> +static const struct ad4691_chip_info ad4691_ad4692 = {
> +	.channels = ad4691_channels,
> +	.manual_channels = ad4691_manual_channels,
> +	.name = "ad4692",
> +	.num_channels = ARRAY_SIZE(ad4691_channels),
> +	.max_rate = 1000000,
> +};
> +
> +static const struct ad4691_chip_info ad4691_ad4693 = {
> +	.channels = ad4693_channels,
> +	.manual_channels = ad4693_manual_channels,
> +	.name = "ad4693",
> +	.num_channels = ARRAY_SIZE(ad4693_channels),
> +	.max_rate = 500000,
> +};
> +
> +static const struct ad4691_chip_info ad4691_ad4694 = {
> +	.channels = ad4693_channels,
> +	.manual_channels = ad4693_manual_channels,
> +	.name = "ad4694",
> +	.num_channels = ARRAY_SIZE(ad4693_channels),
> +	.max_rate = 1000000,
> +};
> +
> +struct ad4691_state {
> +	const struct ad4691_chip_info	*chip;
> +	struct spi_device		*spi;
> +	struct regmap			*regmap;
> +
> +	unsigned long			ref_clk_rate;
> +	struct pwm_device		*conv_trigger;
> +
> +	enum ad4691_adc_mode		adc_mode;
> +
> +	int				vref;
> +	u64				cnv_period;
> +	/*
> +	 * Synchronize access to members of the driver state, and ensure
> +	 * atomicity of consecutive SPI operations.
> +	 */
> +	struct mutex			lock;
> +};
> +
> +static void ad4691_disable_pwm(void *data)
> +{
> +	struct pwm_device *pwm = data;
> +	struct pwm_state state;
> +
> +	pwm_get_state(pwm, &state);
> +	state.enabled = false;
> +	pwm_apply_might_sleep(pwm, &state);

You can just do:

	struct pwm_state state = { .enabled = false, };
	pwm_apply_might_sleep(pwm, &state);

as there is no need to keep period and duty_cycle.

> +}
> +
> +static int ad4691_regulator_get(struct ad4691_state *st)
> +{
> +	struct device *dev = &st->spi->dev;
> +	int ret;
> +
> +	ret = devm_regulator_get_enable(dev, "vio");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get and enable VIO\n");
> +
> +	st->vref = devm_regulator_get_enable_read_voltage(dev, "vref");
> +	if (st->vref == -ENODEV)
> +		st->vref = devm_regulator_get_enable_read_voltage(dev, "vrefin");
> +	if (st->vref < 0)
> +		return dev_err_probe(dev, st->vref,
> +				     "Failed to get reference supply\n");
> +
> +	if (st->vref < AD4691_VREF_MIN || st->vref > AD4691_VREF_MAX)
> +		return dev_err_probe(dev, -EINVAL, "vref(%d) must be under [%u %u]\n",
> +				     st->vref, AD4691_VREF_MIN, AD4691_VREF_MAX);
> +
> +	return 0;
> +}
> +
> +static int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct ad4691_state *st = context;
> +	u8 tx[2], rx[4];
> +	int ret;
> +
> +	put_unaligned_be16(0x8000 | reg, tx);
> +
> +	switch (reg) {
> +	case 0 ... AD4691_OSC_FREQ_REG:
> +	case AD4691_SPARE_CONTROL ... AD4691_ACC_SAT_OVR_REG(15):
> +		ret = spi_write_then_read(st->spi, tx, 2, rx, 1);
> +		if (!ret)
> +			*val = rx[0];
> +		return ret;
> +	case AD4691_STD_SEQ_CONFIG:
> +	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
> +		ret = spi_write_then_read(st->spi, tx, 2, rx, 2);
> +		if (!ret)
> +			*val = get_unaligned_be16(rx);
> +		return ret;
> +	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
> +	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
> +		ret = spi_write_then_read(st->spi, tx, 2, rx, 3);
> +		if (!ret)
> +			*val = get_unaligned_be24(rx);
> +		return ret;
> +	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
> +		ret = spi_write_then_read(st->spi, tx, 2, rx, 4);
> +		if (!ret)
> +			*val = get_unaligned_be32(rx);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4691_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> +	struct ad4691_state *st = context;
> +	u8 tx[4];
> +
> +	put_unaligned_be16(reg, tx);
> +
> +	switch (reg) {
> +	case 0 ... AD4691_OSC_FREQ_REG:
> +	case AD4691_SPARE_CONTROL ... AD4691_GPIO_MODE2_REG:
> +		if (val > 0xFF)
> +			return -EINVAL;
> +		tx[2] = val;
> +		return spi_write_then_read(st->spi, tx, 3, NULL, 0);
> +	case AD4691_STD_SEQ_CONFIG:
> +		if (val > 0xFFFF)
> +			return -EINVAL;
> +		put_unaligned_be16(val, &tx[2]);
> +		return spi_write_then_read(st->spi, tx, 4, NULL, 0);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static bool ad4691_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case AD4691_STATUS_REG:
> +	case AD4691_CLAMP_STATUS1_REG:
> +	case AD4691_CLAMP_STATUS2_REG:
> +	case AD4691_GPIO_READ:
> +	case AD4691_ACC_STATUS_FULL1_REG ... AD4691_ACC_STATUS_SAT2_REG:
> +	case AD4691_ACC_SAT_OVR_REG(0) ... AD4691_ACC_SAT_OVR_REG(15):
> +	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
> +	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
> +	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
> +	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool ad4691_readable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case 0 ... AD4691_OSC_FREQ_REG:
> +	case AD4691_SPARE_CONTROL ... AD4691_ACC_SAT_OVR_REG(15):
> +	case AD4691_STD_SEQ_CONFIG:
> +	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
> +	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
> +	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
> +	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool ad4691_writeable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case 0 ... AD4691_OSC_FREQ_REG:
> +	case AD4691_STD_SEQ_CONFIG:
> +	case AD4691_SPARE_CONTROL ... AD4691_GPIO_MODE2_REG:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct regmap_config ad4691_regmap_config = {
> +	.reg_bits = 16,
> +	.val_bits = 32,
> +	.reg_read = ad4691_reg_read,
> +	.reg_write = ad4691_reg_write,
> +	.volatile_reg = ad4691_volatile_reg,
> +	.readable_reg = ad4691_readable_reg,
> +	.writeable_reg = ad4691_writeable_reg,
> +	.max_register = AD4691_ACC_STS_DATA(15),
> +	.cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int ad4691_get_sampling_freq(struct ad4691_state *st)
> +{
> +	if (st->adc_mode == AD4691_MANUAL_MODE) {
> +		return DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC,
> +					     ktime_to_ns(st->sampling_period));
> +	}
> +
> +	return DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC,
> +				     pwm_get_period(st->conv_trigger));

Other iio drivers (I looked at ad4062.c) use down-rounding division to
provide the sampling frequency. I wonder if there should be some
consistency about that in the different drivers?
(But having said that, the ad4062.c calculation looks somewhat fishy, is
AD4062_TCONV_NS really relevant to the frequency?)

> +}
> +
> +static int __ad4691_set_sampling_freq(struct ad4691_state *st, int freq)
> +{
> +	unsigned long long target, ref_clk_period_ns;
> +	struct pwm_state cnv_state;
> +
> +	pwm_init_state(st->conv_trigger, &cnv_state);
> +
> +	freq = clamp(freq, 1, st->chip->max_rate);
> +	target = DIV_ROUND_CLOSEST_ULL(st->ref_clk_rate, freq);
> +	ref_clk_period_ns = DIV_ROUND_CLOSEST_ULL(NANO, st->ref_clk_rate);
> +	st->cnv_period = ref_clk_period_ns * target;
> +	cnv_state.period = ref_clk_period_ns * target;

You're losing precision here. Consider ref_clk_rate = 64000000, freq =
1234560. Then
   cnv_state.period
 = ref_clk_period_ns * target
 = DIV_ROUND_CLOSEST_ULL(NANO, st->ref_clk_rate) * DIV_ROUND_CLOSEST_ULL(st->ref_clk_rate, freq) 
 = 16 * 52
 = 832

The exact value is NANO / freq (as st->ref_clk_rate cancels out) which
is 810.0051840331778.

So this either needs a comment why the calculation is as it is without
canceling st->ref_clk_rate and two divisions instead of only one, or it
should be simplified.

Also I wonder if DIV_ROUND_CLOSEST_ULL(NANO, freq) is right. With freq =
793336 we get:

	NANO / freq = 1260.499964706001

so 1260 is picked as period which results in an actual frequency of

	NANO / 1260 = 793650.7936507936

and thus a delta of 314.79365079361014 Hz. With period = 1261 however we
get

	NANO / 1261 = 793021.4115781126

which has a delta of 314.5884218873689 and thus is (maybe?) the better
choice? But maybe that is too theoretic as the underlying PWM hardware
probably cannot set 1260 ns or 1261 ns exactly anyhow and most probably
both result in the same HW setting (but if not, 1261 yields the better
setting).

> +	cnv_state.duty_cycle = AD4691_CNV_DUTY_CYCLE_NS;
> +	cnv_state.enabled = false;
> +
> +	return pwm_apply_might_sleep(st->conv_trigger, &cnv_state);
> +}

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature