RE: [PATCH v7 5/6] iio: adc: ad4691: add oversampling support

[Sabau, Radu bogdan]

> -----Original Message-----
> From: Nuno Sá <noname.nuno@xxxxxxxxx>
> Sent: Wednesday, April 15, 2026 11:21 AM

...

> > >
> > > More than this, if the OSR is 32 the maximum effective rate would be
> 31250, so 25kHz
> > > would make it the closes available one. If the user would select 1MHz from
> the available
> > > list it would be weird I would say. So perhaps a solution for this is to display
> the avail list
> > > depending on the set OSR value.
> >
> > Yes, the available list should reflect the current state of any other attributes
> > that affect it.
> 
> IMO, the above makes total sense to me.
> 
> - Nuno Sá
> 

Hi everyone and thank you so much for your feedback!

After thinking this through carefully and testing on hardware (ad4692), here is
the design I have in mind:

in_voltageN_sampling_frequency = effective rate = `osc_freq / osr[N]`:

The chip has a single internal oscillator shred by all channels; each channel
independently accumulating osc[N] oscillator cycles before producing a result.

Writing in_voltageN_sampling_frequency = freq:

The driver computes the needed_osc = freq * osr[N] and snaps down to the largest
available oscillator table entry satisfying both `osc <= needed_osc` and an exact
division to osr. The divisibility constraint ensures the read-back is always an exact
integer.
The result is stored in a single shared `target_osc_freq_Hz` - writing the attribute
for any channel changes the shared oscillator and therefore the read-back of all
other channels.

in_voltageN_sampling_frequency_available:

Computed dynamically from the channel's current OSR. The list naturally becomes
sparser as OSR increases, capping at `max_rate / osr[N]` which is exactly the chip's
behaviour, and therefore more intuitive for the user.

OSC_FREQ_REG write timing:

`target_osc_freq_Hz` is written to hardware at two points:
- Single-shot read: immediately before starting accumulation.
- CNV busrt buffer enable: inside enter_conversion_mode, after the manual mode
early return (manual mode uses SPI CS toggling, not the internal oscillator, so the
write is skipped there).

This keeps the deffered-write benefit - both sampling_frequency and osr can be
set in any order before enabling the buffer/single-shot reading.

Buffer Mode:

After desired rates/osr are set by the user for each channel, reading back the sampling
frequency of each channel gives him the true effective rate for each. Therefore
he can use that information in order to set the buffer sampling frequency accordingly
and helping him use the chip with correct synchronization more intuitively.

I have also performed the next test using the hardware and got correct results:
- test case (ad4692, 1MHz maximum internal oscillator rate):

1. Set channel 0 OSR=32. Available list: {31250, 15625, 12500, 6250, 3125}.
    Write sampling_frequency=10000 (not in the list) -> snaps to 6250 (osc=200000Hz).
    Correct readback = 6250.
2. Set channel 1 OSR=4. Read channel 1 sampling frequency -> 50000 (=200000/4).
    Shared oscillator correctly reflected across channels.
3. Change channel 0 OSR from 32 to 8. Driver recomputes as follows : effective stays
    6250 as before and needed_osc becomes 50000, exact table hit. Readback channel 0:
    6250 (rate preserved). Readback channel 1 (OSR=4): 12500. (oscillator change visible).
    The sampling for channel 0 can be of course set to another available value as well and
    Make match with the initial requested 50k of channel 1. (in this case, set channel 0 to
    25k).
4. -EINVAL rejection is atomic: with OSR=1 and SF=1250 at start for lets say channel 0, writing
    OSR=32 is rejected since the needed_osc=40000, which is not a table entry and also has no
    table entry <= 40000 that is divisible by 32). Both OSR and SF remain unchanged. Raising SF
    to 500000 first then writing OSR=32 succeeds - osc snaps to 1000000, readback SF=31250.
    
    In (4) case we could still let the user have its sampling frequency as is (1250/32=39.0625),
    though it won't result in a precise true integer value, but a rounded (39) one, and when
    other channel would have OSR/rate changed it would imply a messy change in the previous
    channel's SF and requiring a non-existent/matching internal osc value (most of the times
    a float one), and true SF would be lost.

Do you guys think this approach suits the best?

Thanks,
Radu

    >
> > >
> > > Linking the two together is perhaps wrong to begin with from my end,
> since in this
> > > driver's case, the per-channel sampling frequency is controlled by the
> internal oscillator
> > > which has static available values. So perhaps sampling frequency should be
> separate, and
> > > OSR separate as well, which would make everything cleaner.
> > >
> > > Indeed, the effective rate is changed by OSR, but perhaps that is something
> the user
> > > should be aware of, since the sampling frequency is the rate at which the
> channel samples
> > > (1 sample per period) and OSR is how many times the channel samples
> upon a final sample
> > > is to be read. The user already has to take this into account when setting
> the buffer
> > > sampling frequency, so it would make sense to take this into account here
> too.
> >
> > We can't change the definition of the IIO ABI just to make one driver simpler
> > to implement. The OSR and sample rate can't be completely independent.
> >
> > If you want to leave it the way it is currently implemented though, that is
> fine.
> >
> > >
> > > Please let me know you thoughts on this,
> > > Radu
> >