Re: [PATCH v9 5/6] iio: adc: ad4691: add oversampling support

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Thu, Apr 30, 2026 at 01:16:47PM +0300, Radu Sabau via B4 Relay wrote:

> Add per-channel oversampling ratio (OSR) support for CNV burst mode.
> The accumulator depth register (ACC_DEPTH_IN) is programmed with the
> selected OSR at buffer enable time and before each single-shot read.
> 
> Supported OSR values: 1, 2, 4, 8, 16, 32.
> 
> Introduce AD4691_MANUAL_CHANNEL() for manual mode channels, which do
> not expose the oversampling ratio attribute since OSR is not applicable
> in that mode. A separate manual_channels array is added to
> struct ad4691_channel_info and selected at probe time; offload paths
> reuse the same arrays with num_channels capping access before the soft
> timestamp entry.
> 
> in_voltageN_sampling_frequency represents the effective output rate for
> channel N, defined as osc_freq / osr[N]. The chip has one internal
> oscillator shared by all channels; each channel independently
> accumulates osr[N] oscillator cycles before producing a result.
> 
> Writing sampling_frequency computes needed_osc = freq * osr[N] and
> snaps down to the largest oscillator table entry that satisfies both
> osc <= needed_osc and osc % osr[N] == 0, guaranteeing an exact integer
> read-back. The result is stored in target_osc_freq_Hz and written to
> OSC_FREQ_REG at buffer enable and single-shot time, so sampling_frequency
> and oversampling_ratio can be set in any order.
> 
> in_voltageN_sampling_frequency_available is computed dynamically from
> the channel's current OSR, listing only oscillator table entries that
> divide evenly by osr[N], expressed as effective rates. The list becomes
> sparser as OSR increases, capping at max_rate / osr[N].
> 
> Writing oversampling_ratio stores the new OSR for that channel;
> target_osc_freq_Hz is left unchanged. The effective rate read back via
> in_voltageN_sampling_frequency becomes target_osc_freq_Hz / new_osr
> automatically. The two attributes are orthogonal: sampling_frequency
> controls the oscillator, oversampling_ratio controls the averaging depth.
> 
> OSR defaults to 1 (no accumulation) for all channels.

...

> +/* Write target_osc_freq_Hz to OSC_FREQ_REG. Called at use time. */
> +static int ad4691_write_osc_freq(struct ad4691_state *st)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {

It can be

	for (unsigned int i = 0; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {

> +		if (ad4691_osc_freqs_Hz[i] == st->target_osc_freq_Hz)
> +			return regmap_write(st->regmap, AD4691_OSC_FREQ_REG, i);
> +	}
> +	return -EINVAL;
> +}

...

> +/*
> + * Find the largest oscillator table entry that is both <= needed_osc and
> + * evenly divisible by osr (guaranteeing an integer effective rate on
> + * read-back). Returns 0 if no such entry exists in the chip's valid range.
> + */
> +static unsigned int ad4691_find_osc_freq(struct ad4691_state *st,
> +					 unsigned int needed_osc,
> +					 unsigned int osr)
>  {
> +	unsigned int start = ad4691_samp_freq_start(st->info);
>  
> +	for (unsigned int i = start; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {
> +		if ((unsigned int)ad4691_osc_freqs_Hz[i] > needed_osc)
> +			continue;
> +		if (ad4691_osc_freqs_Hz[i] % osr != 0)

' != 0' is redundant.

> +			continue;
> +		return ad4691_osc_freqs_Hz[i];
> +	}
> +	return 0;
> +}

> +static int ad4691_get_sampling_freq(struct ad4691_state *st, u8 osr, int *val)
> +{
> +	*val = st->target_osc_freq_Hz / osr;
>  	return IIO_VAL_INT;
>  }

...

> -static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
> +static int ad4691_set_sampling_freq(struct iio_dev *indio_dev,
> +				    struct iio_chan_spec const *chan, int freq)

Can you use --histogram algo when preparing patches? This might make them look
better.

...

> +	if (freq <= 0 || (unsigned int)freq > st->info->max_rate / osr)

Why casting?

> +		return -EINVAL;

...

> -	/* Wait 2 oscillator periods for the conversion to complete. */
> -	period_us = DIV_ROUND_UP(2UL * USEC_PER_SEC, ad4691_osc_freqs_Hz[osc_idx]);
> +	/*
> +	 * Wait osr + 1 oscillator periods: osr for accumulation, +1 for the
> +	 * pipeline margin (one extra period ensures the final result is ready).
> +	 */
> +	period_us = DIV_ROUND_UP((unsigned long)(st->osr[chan->channel] + 1) * USEC_PER_SEC,

Why casting?

> +				 st->target_osc_freq_Hz);
>  	fsleep(period_us);

-- 
With Best Regards,
Andy Shevchenko