On Fri, Mar 14, 2025 at 01:56:32PM -0500, David Lechner wrote:
> On 3/14/25 1:13 PM, Jorge Marques wrote:
> > On Mon, Mar 10, 2025 at 07:54:16PM +0000, Jonathan Cameron wrote:
> >> On Sun, 9 Mar 2025 21:49:24 +0100
> >> Jorge Marques <gastmaier@xxxxxxxxx> wrote:
> >>
> >>>>> +.. list-table:: Driver attributes
> >>>>> + :header-rows: 1
> >>>>> +
> >>>>> + * - Attribute
> >>>>> + - Description
> >>>>> + * - ``in_voltage0_raw``
> >>>>> + - Raw ADC voltage value
> >>>>> + * - ``in_voltage0_oversampling_ratio``
> >>>>> + - Enable the device's burst averaging mode to over sample using
> >>>>> + the internal sample rate.
> >>>>> + * - ``in_voltage0_oversampling_ratio_available``
> >>>>> + - List of available oversampling values. Value 0 disable the burst
> >>>>> + averaging mode.
> >>>>> + * - ``sample_rate``
> >>>>> + - Device internal sample rate used in the burst averaging mode.
> >>>>> + * - ``sample_rate_available``
> >>>>> + - List of available sample rates.
> >>>>
> >>>> Why not using the standard sampling_frequency[_available] attributes?
> >>> Because sampling_frequency is the sampling frequency for the pwm trigger
> >>> during buffer readings.
> >>> sample_rate is the internal device clock used during monitor and burst
> >>> averaging modes.
> >>
> >> For an ABI that is very vague and the two use cases seem to be logically
> >> quite different.
> >>
> >> Seems that for each trigger we have an oversampling ratio controlled number
> >> of samples at this rate. It is unusual to be able to control oversampling
> >> rate separately from the trigger clock, hence the lack of ABI. If
> >> we add something new for this it should something relating to oversampling.
> >> oversampling_frequency perhaps.
> >>
> >> For monitor mode, it is tied to the sampling frequency for most devices.
> >> But there are exceptions. E.g. the max1363. Trick is to make it an event
> >> ABI property and hence under events/ rather than in the root directory.
> >>
> >> In this case you'll have to store two values and write the appropriate
> >> one into the register to suit a given operating mode.
> >>
> >
> > If doing buffer captures with oversampling enabled, both sampling
> > frequencies have an impact:
> >
> > e.g.,
> > oversampling: 4
> > sample_rate: 2MHz
> > PWM sampling frequency: 500KHz
> >
> > PWM trigger out (CNV) | | | | |
> > ADC conversion ++++ ++++ ++++ ++++ ++++
> > ADC data ready (GP) * * * * *
> >
> > For monitor mode, it will constantly be doing conversion to check for
> > threshold crossings, at the defined sample_rate.
> >
> > I like the idea of having the device's sample_rate as
> > conversion_frequency.
>
> In addition to what makes sense for this chip, we should also consider what
> makes sense other chips with similar features. For example, I am working on
> ad7606c which has control for the oversampling burst frequency (frequency of
> "+" in the diagram above). So it would make sense to have a standard attribute
> that would work for both chips.
>
> On ad4052, just because we have a single register that controls two different
> functions doesn't mean we have to be limited to a single attribute that controls
> that register.
>
I looked into the ad7606c driver and summarized below to organize our
ideas:
PADDING OVERSAMPLING
--------------------
Delay between conversions:
OS_CLOCK(Hz) = 1 / (1+OS_PAD/16)
OS_CLOCK: internal clock, reg
0x08 OVERSAMPLING
OS_PAD[7:4]: Extends the internal oversampling period allowing
evenly spaced sampling between CONVST rising edges,
from 0 to 15
OS_RATIO[3:0]: from off(1) to 256
Therefore, OS_CLOCK range is therefore 1Hz .. 0.516Hz
(1) from previous discussion, iio oversampling 1 equals off.
EXTERNAL OVERSAMPLING CLOCK
---------------------------
Use CONVST as the external trigger for
each conversion
On AD4052 family:
BURST AVERAGING MODE
--------------------
Delay between conversions
Total latency:
(AVG_WIN_LEN-1)/FS_BURST_AUTO + t_CONV
0x23 AVG_CONFIG
AVG_WIN_LEN[3:0]: Averaging ratio/number of samples
0x27 TIMER_CONFIG
FS_BURST_AUTO[7:4]: from 111Hz to 2 MHz, internal sample rate
AVERAGING MODE
--------------
Use CONVST as the external trigger for
each conversion
So, we can say that
PADDING OVERSAMPLING == BURST AVERAGING MODE, and
EXTERNAL OVERSAMPLING CLOCK == AVERAGING MODE
> So I would create the events/sampling_frequency{,_available} attributes like
> Jonathan suggested for controlling the sampling frequency in monitor mode and
> introduce new oversampling_burst_frequency{,_available} attributes for
> controlling the conversion frequency when oversampling. When an attribute is
> written, we can cache the requested value in the state struct instead of
> writing it directly to the register on the ADC if we want the attributes to be
> independent. Then only write the register when we enable monitor mode or when
> we start reading samples with oversampling enabled.
>
> Sure, it is more work to implement it in the driver this way, but that shouldn't
> be an an excuse to do things in a way that isn't compatible with other ADCs.
>
I am alright with that and will follow the suggestion of having the
values independent through cache.
So, two new attributes will be implemented:
* oversampling_[burst_]frequency{,_available} (new ABI required)
* events/sampling_frequency{,_available}
And I will drop conversion_frequency (early sample_rate) attribute.
