diff mbox series

[2/2] Documentation: ABI: add oversampling frequency in sysfs-bus-iio

Message ID 20250321-abi-oversampling-events-frequency-v1-2-794c1ab2f079@analog.com (mailing list archive)
State Changes Requested
Headers show
Series Add sys-bus-iio ABI frequency options to events and oversampling | expand

Commit Message

Jorge Marques March 21, 2025, 2:50 p.m. UTC
Some devices have an internal clock used to space out the conversion
trigger for the oversampling filter,
Consider an ADC with conversion and data ready pins topology:

  Sampling trigger |       |       |       |       |
  ADC conversion   ++++    ++++    ++++    ++++    ++++
  ADC data ready      *       *       *       *       *

With the oversampling frequency, conversions can be evenly space between
the sampling edge:

  Sampling trigger |       |       |       |       |
  ADC conversion   + + + + + + + + + + + + + + + + + + + +
  ADC data ready         *       *       *       *       *

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Jonathan Cameron March 30, 2025, 5:13 p.m. UTC | #1
On Fri, 21 Mar 2025 15:50:02 +0100
Jorge Marques <jorge.marques@analog.com> wrote:

> Some devices have an internal clock used to space out the conversion
> trigger for the oversampling filter,
> Consider an ADC with conversion and data ready pins topology:
> 
>   Sampling trigger |       |       |       |       |
>   ADC conversion   ++++    ++++    ++++    ++++    ++++
>   ADC data ready      *       *       *       *       *
> 
> With the oversampling frequency, conversions can be evenly space between
> the sampling edge:

I'm not sure what this second example is providing.  Are you suggesting
that if we don't provide oversampling frequency we should assume this
pattern?  i.e. it is the default?

> 
>   Sampling trigger |       |       |       |       |
>   ADC conversion   + + + + + + + + + + + + + + + + + + + +
>   ADC data ready         *       *       *       *       *
> 
In general this patch needs to go in with the first driver using it.
I don't think we have any such driver yet?

> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 33c09c4ac60a4feec82308461643134f5ba84b66..2317bacf6a2884691a08725d6f01d18555a96227 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -139,6 +139,23 @@ Contact:	linux-iio@vger.kernel.org
>  Description:
>  		Hardware dependent values supported by the oversampling filter.
>  
> +What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency
> +KernelVersion:	6.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Some devices have internal clocks for the ADC oversampling.
I wonder if we can hint at your diagram above?
Maybe
		Some devices have internal clocks for the ADC oversampling allowing
		the over samples to be bunched up, rather than evenly spread over the
		period set by the sampling frequency.

> +		Sets the resulting sampling frequency to trigger a conversion
> +		used by the oversampling filter.
> +		Can be used to evenly space conversion between the sampling edge
> +		on some devices.
I'd skip this last line, or maybe say something like:

		If not provided, the default assumption is that the oversamples
		are evenly spread over the period of the sample.

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency_available
> +KernelVersion:	6.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Hardware dependent values supported by the oversampling
> +		frequency.
> +
>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
>
David Lechner March 30, 2025, 5:34 p.m. UTC | #2
On 3/30/25 12:13 PM, Jonathan Cameron wrote:
> On Fri, 21 Mar 2025 15:50:02 +0100
> Jorge Marques <jorge.marques@analog.com> wrote:
> 
>> Some devices have an internal clock used to space out the conversion
>> trigger for the oversampling filter,
>> Consider an ADC with conversion and data ready pins topology:
>>
>>   Sampling trigger |       |       |       |       |
>>   ADC conversion   ++++    ++++    ++++    ++++    ++++
>>   ADC data ready      *       *       *       *       *
>>
>> With the oversampling frequency, conversions can be evenly space between
>> the sampling edge:
> 
> I'm not sure what this second example is providing.  Are you suggesting
> that if we don't provide oversampling frequency we should assume this
> pattern?  i.e. it is the default?
> 
>>
>>   Sampling trigger |       |       |       |       |
>>   ADC conversion   + + + + + + + + + + + + + + + + + + + +
>>   ADC data ready         *       *       *       *       *
>>
> In general this patch needs to go in with the first driver using it.
> I don't think we have any such driver yet?
> 
>> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index 33c09c4ac60a4feec82308461643134f5ba84b66..2317bacf6a2884691a08725d6f01d18555a96227 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -139,6 +139,23 @@ Contact:	linux-iio@vger.kernel.org
>>  Description:
>>  		Hardware dependent values supported by the oversampling filter.
>>  
>> +What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency
>> +KernelVersion:	6.15
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Some devices have internal clocks for the ADC oversampling.
> I wonder if we can hint at your diagram above?
> Maybe
> 		Some devices have internal clocks for the ADC oversampling allowing
> 		the over samples to be bunched up, rather than evenly spread over the
> 		period set by the sampling frequency.
> 
>> +		Sets the resulting sampling frequency to trigger a conversion
>> +		used by the oversampling filter.
>> +		Can be used to evenly space conversion between the sampling edge
>> +		on some devices.
> I'd skip this last line, or maybe say something like:
> 
> 		If not provided, the default assumption is that the oversamples
> 		are evenly spread over the period of the sample.

Does that mean we should go through existing drivers and add this new
attribute if appropriate? For example, ad7380 comes to mind. It has a
fixed-rate internal clock for oversampling, so would have a read-only
oversampling_frequency attribute.

> 
>> +
>> +What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency_available
>> +KernelVersion:	6.15
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Hardware dependent values supported by the oversampling
>> +		frequency.
>> +
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
>>
>
Jonathan Cameron March 30, 2025, 5:53 p.m. UTC | #3
On Sun, 30 Mar 2025 12:34:39 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 3/30/25 12:13 PM, Jonathan Cameron wrote:
> > On Fri, 21 Mar 2025 15:50:02 +0100
> > Jorge Marques <jorge.marques@analog.com> wrote:
> >   
> >> Some devices have an internal clock used to space out the conversion
> >> trigger for the oversampling filter,
> >> Consider an ADC with conversion and data ready pins topology:
> >>
> >>   Sampling trigger |       |       |       |       |
> >>   ADC conversion   ++++    ++++    ++++    ++++    ++++
> >>   ADC data ready      *       *       *       *       *
> >>
> >> With the oversampling frequency, conversions can be evenly space between
> >> the sampling edge:  
> > 
> > I'm not sure what this second example is providing.  Are you suggesting
> > that if we don't provide oversampling frequency we should assume this
> > pattern?  i.e. it is the default?
> >   
> >>
> >>   Sampling trigger |       |       |       |       |
> >>   ADC conversion   + + + + + + + + + + + + + + + + + + + +
> >>   ADC data ready         *       *       *       *       *
> >>  
> > In general this patch needs to go in with the first driver using it.
> > I don't think we have any such driver yet?
> >   
> >> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> >> ---
> >>  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
> >>  1 file changed, 17 insertions(+)
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> >> index 33c09c4ac60a4feec82308461643134f5ba84b66..2317bacf6a2884691a08725d6f01d18555a96227 100644
> >> --- a/Documentation/ABI/testing/sysfs-bus-iio
> >> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> >> @@ -139,6 +139,23 @@ Contact:	linux-iio@vger.kernel.org
> >>  Description:
> >>  		Hardware dependent values supported by the oversampling filter.
> >>  
> >> +What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency
> >> +KernelVersion:	6.15
> >> +Contact:	linux-iio@vger.kernel.org
> >> +Description:
> >> +		Some devices have internal clocks for the ADC oversampling.  
> > I wonder if we can hint at your diagram above?
> > Maybe
> > 		Some devices have internal clocks for the ADC oversampling allowing
> > 		the over samples to be bunched up, rather than evenly spread over the
> > 		period set by the sampling frequency.
> >   
> >> +		Sets the resulting sampling frequency to trigger a conversion
> >> +		used by the oversampling filter.
> >> +		Can be used to evenly space conversion between the sampling edge
> >> +		on some devices.  
> > I'd skip this last line, or maybe say something like:
> > 
> > 		If not provided, the default assumption is that the oversamples
> > 		are evenly spread over the period of the sample.  
> 
> Does that mean we should go through existing drivers and add this new
> attribute if appropriate? For example, ad7380 comes to mind. It has a
> fixed-rate internal clock for oversampling, so would have a read-only
> oversampling_frequency attribute.

Good point.

It is possibly a useful thing to do if that fixed rate clock is not
jut the appropriate fraction of the sampling_frequency clock.

Requiring drivers conform provide a new ABI is a non starter though.

I guess rewrite the above suggestion to be more vague.

		If not provided, either over samples are evenly spread over the
		period of the sample, or no information is available.

Or just don't have anything for that last sentence on basis if no
rules set there are no rules.

Jonathan

> 
> >   
> >> +
> >> +What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency_available
> >> +KernelVersion:	6.15
> >> +Contact:	linux-iio@vger.kernel.org
> >> +Description:
> >> +		Hardware dependent values supported by the oversampling
> >> +		frequency.
> >> +
> >>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
> >>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
> >>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
> >>  
> >   
>
Jorge Marques April 2, 2025, 1:47 p.m. UTC | #4
On Sun, Mar 30, 2025 at 06:53:53PM +0100, Jonathan Cameron wrote:
> On Sun, 30 Mar 2025 12:34:39 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > On 3/30/25 12:13 PM, Jonathan Cameron wrote:
> > > On Fri, 21 Mar 2025 15:50:02 +0100
> > > Jorge Marques <jorge.marques@analog.com> wrote:
> > >   
> > >> Some devices have an internal clock used to space out the conversion
> > >> trigger for the oversampling filter,
> > >> Consider an ADC with conversion and data ready pins topology:
> > >>
> > >>   Sampling trigger |       |       |       |       |
> > >>   ADC conversion   ++++    ++++    ++++    ++++    ++++
> > >>   ADC data ready      *       *       *       *       *
> > >>
> > >> With the oversampling frequency, conversions can be evenly space between
> > >> the sampling edge:  
> > > 
> > > I'm not sure what this second example is providing.  Are you suggesting
> > > that if we don't provide oversampling frequency we should assume this
> > > pattern?  i.e. it is the default?
> > >   

The default is to do the n-conversions sequentially (n*t_conv),
"left-aligned" as in the diagram above.
The main application for oversampling is to average out the noise over a wider
bandwidth.

I looked into some of the drivers with oversampling and the supported devices
datasheets:

* ADS1298: Single field for sampling rate and oversampling,
           I assume the values are the maximum values that the
	   oversampling time does not exceed the sampling period.
* RTQ6056: Field for oversampling and conversion time,
           maximum sampling period is roughly n*t_ovr.
* MCP3561: Field for oversampling and conversion time.
           maximum sampling period is roughly n*t_ovr.
* AD7380:  Field for oversampling and fixed conversion time,
           3 MSPS for the AD7380 and 4 MSPS for AD7381,
           maximum sampling period is n*t_ovr, e.g. f_samp=(6/4MSPS).

None will or claim to stretch over the sampling period the oversampling
conversions, but rather, do the n-conversions at oversampling rate,
providing the conversion as soon as it is ready and idling until the
next edge of the sampling frequency.

> > >>
> > >>   Sampling trigger |       |       |       |       |
> > >>   ADC conversion   + + + + + + + + + + + + + + + + + + + +
> > >>   ADC data ready         *       *       *       *       *
> > >>  
> > > In general this patch needs to go in with the first driver using it.
> > > I don't think we have any such driver yet?
> > >   

The AD4052 family that I will re-submit exposes this feature.
The AD7606c that David is working on can also expose.
I can add this patch to the AD4052 V2 series.

> > >> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> > >> ---
> > >>  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
> > >>  1 file changed, 17 insertions(+)
> > >>
> > >> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > >> index 33c09c4ac60a4feec82308461643134f5ba84b66..2317bacf6a2884691a08725d6f01d18555a96227 100644
> > >> --- a/Documentation/ABI/testing/sysfs-bus-iio
> > >> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > >> @@ -139,6 +139,23 @@ Contact:	linux-iio@vger.kernel.org
> > >>  Description:
> > >>  		Hardware dependent values supported by the oversampling filter.
> > >>  
> > >> +What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency
> > >> +KernelVersion:	6.15
> > >> +Contact:	linux-iio@vger.kernel.org
> > >> +Description:
> > >> +		Some devices have internal clocks for the ADC oversampling.  
> > > I wonder if we can hint at your diagram above?
> > > Maybe
> > > 		Some devices have internal clocks for the ADC oversampling allowing
> > > 		the over samples to be bunched up, rather than evenly spread over the
> > > 		period set by the sampling frequency.
> > >   
> > >> +		Sets the resulting sampling frequency to trigger a conversion
> > >> +		used by the oversampling filter.
> > >> +		Can be used to evenly space conversion between the sampling edge
> > >> +		on some devices.  
> > > I'd skip this last line, or maybe say something like:
> > > 
> > > 		If not provided, the default assumption is that the oversamples
> > > 		are evenly spread over the period of the sample.  
> > 
> > Does that mean we should go through existing drivers and add this new
> > attribute if appropriate? For example, ad7380 comes to mind. It has a
> > fixed-rate internal clock for oversampling, so would have a read-only
> > oversampling_frequency attribute.
> 
> Good point.
> 
> It is possibly a useful thing to do if that fixed rate clock is not
> jut the appropriate fraction of the sampling_frequency clock.
> 
> Requiring drivers conform provide a new ABI is a non starter though.
> 
> I guess rewrite the above suggestion to be more vague.
> 
> 		If not provided, either over samples are evenly spread over the
> 		period of the sample, or no information is available.
> 

I believe removing the paragraph is better, because the statement is false
most of the times. As investigated above, the norm is to bunch up the over
samples to the left (n*t_ovr).

My original paragraph was to nudge the user that he can adjust the
number of samples and conversion period to achieve evenly spaced,
but now I would rather not include this statement.

> Or just don't have anything for that last sentence on basis if no
> rules set there are no rules.

Agreed

> 
> Jonathan


> 
> > 
> > >   
> > >> +
> > >> +What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency_available
> > >> +KernelVersion:	6.15
> > >> +Contact:	linux-iio@vger.kernel.org
> > >> +Description:
> > >> +		Hardware dependent values supported by the oversampling
> > >> +		frequency.
> > >> +
> > >>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
> > >>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
> > >>  What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
> > >>  
> > >   
> > 
>
David Lechner April 2, 2025, 8:27 p.m. UTC | #5
On 4/2/25 8:47 AM, Jorge Marques wrote:
> On Sun, Mar 30, 2025 at 06:53:53PM +0100, Jonathan Cameron wrote:
>> On Sun, 30 Mar 2025 12:34:39 -0500
>> David Lechner <dlechner@baylibre.com> wrote:
>>
>>> On 3/30/25 12:13 PM, Jonathan Cameron wrote:
>>>> On Fri, 21 Mar 2025 15:50:02 +0100
>>>> Jorge Marques <jorge.marques@analog.com> wrote:

Once you dig into it, the current situation is even more complicated than
I expected. :-o

This reply has ended up being mostly a brain dump of my observations. I hope
it isn't too confusing. Consider this more of a brainstorm on a future
documentation page on sampling rates rather than commenting on what to do
on this particular patch. :-)

---

To make sure we are clear, I think we need to define some precise terminology,
especially with regard to "sample rate" since that can be used to mean a
lot of different things.

There is the "IIO sample rate" (could also call it the "effective sample rate")
that is the rate that we push one complete set of data to an IIO buffer. In
many cases, this would be the frequency of the hrtimer trigger that is configured
as the trigger for the iio:deviceX.

On the other end of the spectrum, we have the "conversion rate" which is the
rate that individual ADC conversions can happen.

What I had not seen before, but now I see in existing drivers, is that these
may actually be completely independent. In other words, the hrtimer trigger
only triggers reading the most recent set of conversions and conversions are
driven by a completely separate trigger, generally some sort of clock in the
ADC itself.

So I think we should expand the diagrams below to show more layers for the
completely general case.

>>>>   
>>>>> Some devices have an internal clock used to space out the conversion
>>>>> trigger for the oversampling filter,
>>>>> Consider an ADC with conversion and data ready pins topology:
>>>>>
>>>>>   Sampling trigger |       |       |       |       |
>>>>>   ADC conversion   ++++    ++++    ++++    ++++    ++++
>>>>>   ADC data ready      *       *       *       *       *
>>>>>

For terminology, let's call this "burst mode" oversampling (maybe also
referred to as "triggered mode" in some data sheets).

>>>>> With the oversampling frequency, conversions can be evenly space between
>>>>> the sampling edge:  
>>>>
>>>> I'm not sure what this second example is providing.  Are you suggesting
>>>> that if we don't provide oversampling frequency we should assume this
>>>> pattern?  i.e. it is the default?
>>>>   
> 
> The default is to do the n-conversions sequentially (n*t_conv),
> "left-aligned" as in the diagram above.
> The main application for oversampling is to average out the noise over a wider
> bandwidth.
> 
> I looked into some of the drivers with oversampling and the supported devices
> datasheets:
> 
> * ADS1298: Single field for sampling rate and oversampling,
>            I assume the values are the maximum values that the
> 	   oversampling time does not exceed the sampling period.
> * RTQ6056: Field for oversampling and conversion time,
>            maximum sampling period is roughly n*t_ovr.
> * MCP3561: Field for oversampling and conversion time.
>            maximum sampling period is roughly n*t_ovr.
> * AD7380:  Field for oversampling and fixed conversion time,
>            3 MSPS for the AD7380 and 4 MSPS for AD7381,
>            maximum sampling period is n*t_ovr, e.g. f_samp=(6/4MSPS).
> 
> None will or claim to stretch over the sampling period the oversampling
> conversions, but rather, do the n-conversions at oversampling rate,
> providing the conversion as soon as it is ready and idling until the
> next edge of the sampling frequency.
> 
>>>>>
>>>>>   Sampling trigger |       |       |       |       |
>>>>>   ADC conversion   + + + + + + + + + + + + + + + + + + + +
>>>>>   ADC data ready         *       *       *       *       *
>>>>> 

And let's call this one "continuous mode".

But as we will see, both of these are a bit ambiguous in their current
form. The complete picture is a bit more nuanced.

---

Let's take the RTQ6056 case (since I actually looked at that one before
as inspiration for developing another driver).

The chip itself is programmable and can operate in either a burst/triggered
mode or in a continuous mode. However, the way the IIO driver is implemented,
it is configured in continuous mode to trigger ADC conversions. But uses an
independent IIO trigger that triggers reading sample data. So from the point of
view of the data in a buffered read, it looks like burst mode. But the value
of the sampling_frequency attribute (the ADC device attribute, not the hrtimer
trigger attribute) is for the hardware continuous mode.

Hardware:
sampling_frequency   |       |       |       |       |
ADC conversion       + + + + + + + + + + + + + + + + + + + +
ADC data ready             *       *       *       *       *
sample number              S0      S1      S2      S3      S4

IIO:
hrtimer frequency               |                     |
I2C read                         *                     *
push to buffer                   S0                    S3

The IIO (hrtimer) trigger only reads the most recently available data, it
doesn't trigger any conversion. The clocks are asynchronous.

I think adding an oversampling_frequency attribute to this driver could make
it easier to used/understand since oversampling_frequency would be exactly
the "conversion rate". Compared to the current situation where the "conversion
rate" is the sampling_frequency / oversampling_ratio.

It is also interesting to consider that if someone decided to add SPI offload
support to this driver, then there would be the possibility of using burst/
trigggered mode or continuous mode and might want to support both even. In
fact this is exactly the possibility we have with ad7606 that I mentioned in
a previous reply. So we might even need an oversampling_mode attribute to allow
selecting one or the other. But that is something to save for a ad7606 patch
series.

---

Another driver probably worth considering is ad4030. In this one, there is no
internal clock to drive conversions. So, for oversampling, the sample rate is
just "as fast as possible" (currently bit-banging a GPIO). So it doesn't actually
have an oversampling frequency.

If someone ever decided to hook it up to some hardware that could actually
trigger a finite number of pulses at a specific rate, then this new attribute
for oversampling_frequency would become useful. For this particular driver,
the presence or absence of an oversampling_frequency attribute would have
a meaning, but I don't think this generalizes to other ADCs.

---

AD4695 is an interesting case to consider as well. When used without SPI offload
support, we actually don't allow oversampling currently. If we did though, it
be similar to the ad4030 in that we could either make it "as fast as possible"
by banging a GPIO or the CS line depending on how the chip was wired up. Or it
could use some specialized hardware to generate a pulse train to actually get
a known conversion rate.

For now though, oversampling is only implemented when using a SPI offload.
It works like this:

Channel		1   2       3 1   2       3 1   2       3
OSR		2   4       1 2   4       1 2   4       1
Trigger		| | | | | | | | | | | | | | | | | | | | |
ADC Conversion	+ + + + + + + + + + + + + + + + + + + + +
ADC data ready	   *       * *   *       * *   *       * *
IIO sample                   S0            S1            S2

In this case, there isn't a "sample" trigger that triggers a burst of
samples. Rather, there is only a "conversion" trigger that triggers
individual conversion. In other words, we would call this "continuous mode".
And it also shows that some chips allow individual channels to have
different oversampling ratios.

In this case, it would be nice to have an oversampling_frequency
attribute as well because it would exactly correspond to the conversion
rate. Currently each channel has a sampling_frequency attribute that
is oversampling_frequency / oversampling_ratio (same as RTQ6056).

---

So my conclusion here is that the new proposed oversampling_frequency
attribute has nothing to do with "burst mode" or "continuous mode" it
has the same meaning in both cases. It is effectively the rate for
individual ADC conversions.
Jorge Marques April 3, 2025, 8:35 a.m. UTC | #6
On Wed, Apr 02, 2025 at 03:27:51PM -0500, David Lechner wrote:
> On 4/2/25 8:47 AM, Jorge Marques wrote:
> > On Sun, Mar 30, 2025 at 06:53:53PM +0100, Jonathan Cameron wrote:
> >> On Sun, 30 Mar 2025 12:34:39 -0500
> >> David Lechner <dlechner@baylibre.com> wrote:
> >>
> >>> On 3/30/25 12:13 PM, Jonathan Cameron wrote:
> >>>> On Fri, 21 Mar 2025 15:50:02 +0100
> >>>> Jorge Marques <jorge.marques@analog.com> wrote:
> 
> Once you dig into it, the current situation is even more complicated than
> I expected. :-o
> 
> This reply has ended up being mostly a brain dump of my observations. I hope
> it isn't too confusing. Consider this more of a brainstorm on a future
> documentation page on sampling rates rather than commenting on what to do
> on this particular patch. :-)
> 
> ---
> 
> To make sure we are clear, I think we need to define some precise terminology,
> especially with regard to "sample rate" since that can be used to mean a
> lot of different things.
> 
> There is the "IIO sample rate" (could also call it the "effective sample rate")
> that is the rate that we push one complete set of data to an IIO buffer. In
> many cases, this would be the frequency of the hrtimer trigger that is configured
> as the trigger for the iio:deviceX.

Yes.

> 
> On the other end of the spectrum, we have the "conversion rate" which is the
> rate that individual ADC conversions can happen.

Yes.

> 
> What I had not seen before, but now I see in existing drivers, is that these
> may actually be completely independent. In other words, the hrtimer trigger
> only triggers reading the most recent set of conversions and conversions are
> driven by a completely separate trigger, generally some sort of clock in the
> ADC itself.
> 
> So I think we should expand the diagrams below to show more layers for the
> completely general case.
> 
> >>>>   
> >>>>> Some devices have an internal clock used to space out the conversion
> >>>>> trigger for the oversampling filter,
> >>>>> Consider an ADC with conversion and data ready pins topology:
> >>>>>
> >>>>>   Sampling trigger |       |       |       |       |
> >>>>>   ADC conversion   ++++    ++++    ++++    ++++    ++++
> >>>>>   ADC data ready      *       *       *       *       *
> >>>>>
> 
> For terminology, let's call this "burst mode" oversampling (maybe also
> referred to as "triggered mode" in some data sheets).
> 
> >>>>> With the oversampling frequency, conversions can be evenly space between
> >>>>> the sampling edge:  
> >>>>
> >>>> I'm not sure what this second example is providing.  Are you suggesting
> >>>> that if we don't provide oversampling frequency we should assume this
> >>>> pattern?  i.e. it is the default?
> >>>>   
> > 
> > The default is to do the n-conversions sequentially (n*t_conv),
> > "left-aligned" as in the diagram above.
> > The main application for oversampling is to average out the noise over a wider
> > bandwidth.
> > 
> > I looked into some of the drivers with oversampling and the supported devices
> > datasheets:
> > 
> > * ADS1298: Single field for sampling rate and oversampling,
> >            I assume the values are the maximum values that the
> > 	   oversampling time does not exceed the sampling period.
> > * RTQ6056: Field for oversampling and conversion time,
> >            maximum sampling period is roughly n*t_ovr.
> > * MCP3561: Field for oversampling and conversion time.
> >            maximum sampling period is roughly n*t_ovr.
> > * AD7380:  Field for oversampling and fixed conversion time,
> >            3 MSPS for the AD7380 and 4 MSPS for AD7381,
> >            maximum sampling period is n*t_ovr, e.g. f_samp=(6/4MSPS).
> > 
> > None will or claim to stretch over the sampling period the oversampling
> > conversions, but rather, do the n-conversions at oversampling rate,
> > providing the conversion as soon as it is ready and idling until the
> > next edge of the sampling frequency.
> > 
> >>>>>
> >>>>>   Sampling trigger |       |       |       |       |
> >>>>>   ADC conversion   + + + + + + + + + + + + + + + + + + + +
> >>>>>   ADC data ready         *       *       *       *       *
> >>>>> 
> 
> And let's call this one "continuous mode".
> 
> But as we will see, both of these are a bit ambiguous in their current
> form. The complete picture is a bit more nuanced.
> 
> ---
> 
> Let's take the RTQ6056 case (since I actually looked at that one before
> as inspiration for developing another driver).
> 
> The chip itself is programmable and can operate in either a burst/triggered
> mode or in a continuous mode. However, the way the IIO driver is implemented,
> it is configured in continuous mode to trigger ADC conversions. But uses an
> independent IIO trigger that triggers reading sample data. So from the point of
> view of the data in a buffered read, it looks like burst mode. But the value
> of the sampling_frequency attribute (the ADC device attribute, not the hrtimer
> trigger attribute) is for the hardware continuous mode.
> 
> Hardware:
> sampling_frequency   |       |       |       |       |
> ADC conversion       + + + + + + + + + + + + + + + + + + + +
> ADC data ready             *       *       *       *       *
> sample number              S0      S1      S2      S3      S4
> 
> IIO:
> hrtimer frequency               |                     |
> I2C read                         *                     *
> push to buffer                   S0                    S3
> 
> The IIO (hrtimer) trigger only reads the most recently available data, it
> doesn't trigger any conversion. The clocks are asynchronous.

That implementation sure works for low-speed converters where power-consuption
is not a concern.

> I think adding an oversampling_frequency attribute to this driver could make
> it easier to used/understand since oversampling_frequency would be exactly
> the "conversion rate". Compared to the current situation where the "conversion
> rate" is the sampling_frequency / oversampling_ratio.

I agree.
Applicable only for devices where sampling_frequency and
oversampling_frequency are detached.
Devices that provide a value for the fixed oversampling_frequency
frequency could have the attribute as read-only (e.g AD7380).

> 
> It is also interesting to consider that if someone decided to add SPI offload
> support to this driver, then there would be the possibility of using burst/
> trigggered mode or continuous mode and might want to support both even. In
> fact this is exactly the possibility we have with ad7606 that I mentioned in
> a previous reply. So we might even need an oversampling_mode attribute to allow
> selecting one or the other. But that is something to save for a ad7606 patch
> series.

For AD4052:

* iio raw read is the usual low-speed, CNV->DRDY->Read.
* iio buffer: uses offload, samp_freq is PWM node freq.
  - iio triggered buffer: not included on proposed series,
                          but doable as a fallback if offload
                          not available.
* iio events: monitor mode, device exits monitor mode on event
              (device specific behaviour, cannot be changed),
              the irq is propagated as iio event.

iio oversampling changes the iio raw and iio buffer readings behaviour
(takes longer to get a sample out).

> 
> ---
> 
> Another driver probably worth considering is ad4030. In this one, there is no
> internal clock to drive conversions. So, for oversampling, the sample rate is
> just "as fast as possible" (currently bit-banging a GPIO). So it doesn't actually
> have an oversampling frequency.
>
>
> If someone ever decided to hook it up to some hardware that could actually
> trigger a finite number of pulses at a specific rate, then this new attribute
> for oversampling_frequency would become useful. For this particular driver,
> the presence or absence of an oversampling_frequency attribute would have
> a meaning, but I don't think this generalizes to other ADCs.
> 

The hardware is the usual CNV trigger, so it is not really about hooking
up more hardware, but changing the behaviour
(takes n-CNV triggers to get a sample out).

The AD4052 also supports what you described, it is the "Averaging Mode" (p.31),
while the implemented uses the "Burst Averaging Mode" (p.32).
In the "Averaging Mode", effective sampling rate is
sampling_frequency / oversampling_ratio, while in
"Burst Averaging Mode" is sampling_frequency.

So, in "Burst Averaging Mode", the oversampling_frequency is *detached*
from sampling_frequency.

The driver could hide away the effective sampling frequency discrepancy
by reading the state and scaling sampling_frequency based on
oversampling_ratio, exactly like RTQ6056.

> ---
> 
> AD4695 is an interesting case to consider as well. When used without SPI offload
> support, we actually don't allow oversampling currently. If we did though, it
> be similar to the ad4030 in that we could either make it "as fast as possible"
> by banging a GPIO or the CS line depending on how the chip was wired up. Or it
> could use some specialized hardware to generate a pulse train to actually get
> a known conversion rate.
> 
> For now though, oversampling is only implemented when using a SPI offload.
> It works like this:
> 
> Channel		1   2       3 1   2       3 1   2       3
> OSR		2   4       1 2   4       1 2   4       1
> Trigger		| | | | | | | | | | | | | | | | | | | | |
> ADC Conversion	+ + + + + + + + + + + + + + + + + + + + +
> ADC data ready	   *       * *   *       * *   *       * *
> IIO sample                   S0            S1            S2
> 
> In this case, there isn't a "sample" trigger that triggers a burst of
> samples. Rather, there is only a "conversion" trigger that triggers
> individual conversion. In other words, we would call this "continuous mode".
> And it also shows that some chips allow individual channels to have
> different oversampling ratios.

I find this "continuous mode" name confusing, maybe stick with
"averaging mode", where each CNV pulse triggers a conversion and
"burst averaging mode", where a CNV pulse triggers a burst of
conversions.

> 
> In this case, it would be nice to have an oversampling_frequency
> attribute as well because it would exactly correspond to the conversion
> rate. Currently each channel has a sampling_frequency attribute that
> is oversampling_frequency / oversampling_ratio (same as RTQ6056).

I don't mind that, is the "hide away" I mentioned earlier.
I believe that, when oversampling is a driver feature,
oversampling_frequency is either:
 * The RW conversion frequency:
   - averaging mode:       oversampling_frequency, sampling_frequency,
                           and oversampling_ratio affect each other,
                           due to "hide away" logic.
                           One CNV one conversion.
   - burst averaging mode: sampling frequency and conversion frequency
                           are detached, doesn't need "hide away" logic.
                           One CNV one sample.
 * The RO conversion frequency with the role of maximum conversion frequency.

> 
> ---
> 
> So my conclusion here is that the new proposed oversampling_frequency
> attribute has nothing to do with "burst mode" or "continuous mode" it
> has the same meaning in both cases.

It depends if it is detached from sampling_frequency or not.
If the norm is to "hide away" with extra logic in "averaging mode",
they become interdependent.
If sampling frequency and conversion frequency are detached from each
other, oversampling_frequency for sure needs to exist.

>                                     It is effectively the rate for
> individual ADC conversions.
> 

Yes.
Jonathan Cameron April 6, 2025, 10:25 a.m. UTC | #7
On Thu, 3 Apr 2025 10:35:48 +0200
Jorge Marques <gastmaier@gmail.com> wrote:

> On Wed, Apr 02, 2025 at 03:27:51PM -0500, David Lechner wrote:
> > On 4/2/25 8:47 AM, Jorge Marques wrote:  
> > > On Sun, Mar 30, 2025 at 06:53:53PM +0100, Jonathan Cameron wrote:  
> > >> On Sun, 30 Mar 2025 12:34:39 -0500
> > >> David Lechner <dlechner@baylibre.com> wrote:
> > >>  
> > >>> On 3/30/25 12:13 PM, Jonathan Cameron wrote:  
> > >>>> On Fri, 21 Mar 2025 15:50:02 +0100
> > >>>> Jorge Marques <jorge.marques@analog.com> wrote:  
> > 
> > Once you dig into it, the current situation is even more complicated than
> > I expected. :-o
> > 
> > This reply has ended up being mostly a brain dump of my observations. I hope
> > it isn't too confusing. Consider this more of a brainstorm on a future
> > documentation page on sampling rates rather than commenting on what to do
> > on this particular patch. :-)

I think it's worth mentioning that the original oversampling attributes were
IIRC (note I could be wrong) on self clocking devices such as accelerometers,
at least sometimes with a hardware FIFO involved. In some cases even then
the conversions are bursted. In others I think (though can't find an example right
now) they are dumb enough that if you enable oversampling it clocks the same
as without. (the 'stretched case below).  A lot of the datasheets are very
vague!


> > 
> > ---
> > 
> > To make sure we are clear, I think we need to define some precise terminology,
> > especially with regard to "sample rate" since that can be used to mean a
> > lot of different things.
> > 
> > There is the "IIO sample rate" (could also call it the "effective sample rate")
> > that is the rate that we push one complete set of data to an IIO buffer. In
> > many cases, this would be the frequency of the hrtimer trigger that is configured
> > as the trigger for the iio:deviceX.  
> 
> Yes.

I'd be tempted focus more on the fully self clocked devices with a dataready signal.
Bringing in the different clock domain of a hrtimer trigger or similar adds
yet another layer of complexity.  You get into that below.

> 
> > 
> > On the other end of the spectrum, we have the "conversion rate" which is the
> > rate that individual ADC conversions can happen.  
> 
> Yes.
> 
> > 
> > What I had not seen before, but now I see in existing drivers, is that these
> > may actually be completely independent. In other words, the hrtimer trigger
> > only triggers reading the most recent set of conversions and conversions are
> > driven by a completely separate trigger, generally some sort of clock in the
> > ADC itself.

That is 'sometimes' the case.  Often on devices that provide a dataready trigger
as well (but were we might want to sample much slower, or where some designer
didn't wire the pin up).  Note the sampling frequency in question here is that
of the trigger. If the device is self clocked then it typically also provides
a sampling frequency attribute to describe what it is doing.

In other cases the hrtimer trigger is starting a conversion (or burst of conversions
as appropriate).  In those cases if the device provides a sampling frequency
it is a bound on how fast it can be driven.  That is effectively the same as
the previous case as if we are sampling latest value, there is no point in
going faster than the device sampling frequency (hence no different ABI).

> > 
> > So I think we should expand the diagrams below to show more layers for the
> > completely general case.
> >   
> > >>>>     
> > >>>>> Some devices have an internal clock used to space out the conversion
> > >>>>> trigger for the oversampling filter,
> > >>>>> Consider an ADC with conversion and data ready pins topology:
> > >>>>>
> > >>>>>   Sampling trigger |       |       |       |       |
> > >>>>>   ADC conversion   ++++    ++++    ++++    ++++    ++++
> > >>>>>   ADC data ready      *       *       *       *       *
> > >>>>>  
> > 
> > For terminology, let's call this "burst mode" oversampling (maybe also
> > referred to as "triggered mode" in some data sheets).
> >   
> > >>>>> With the oversampling frequency, conversions can be evenly space between
> > >>>>> the sampling edge:    
> > >>>>
> > >>>> I'm not sure what this second example is providing.  Are you suggesting
> > >>>> that if we don't provide oversampling frequency we should assume this
> > >>>> pattern?  i.e. it is the default?
> > >>>>     
> > > 
> > > The default is to do the n-conversions sequentially (n*t_conv),
> > > "left-aligned" as in the diagram above.

Hmm. What's the default if we don't provide information on the conversion sampling
frequency?  I don't mean what is most common or what does a particular device do,
just the default from an ABI point of view.  Either default is no information
or it is is 'evenly spread'.  

> > > The main application for oversampling is to average out the noise over a wider
> > > bandwidth.
> > > 
> > > I looked into some of the drivers with oversampling and the supported devices
> > > datasheets:
> > > 
> > > * ADS1298: Single field for sampling rate and oversampling,
> > >            I assume the values are the maximum values that the
> > > 	   oversampling time does not exceed the sampling period.
> > > * RTQ6056: Field for oversampling and conversion time,
> > >            maximum sampling period is roughly n*t_ovr.
> > > * MCP3561: Field for oversampling and conversion time.
> > >            maximum sampling period is roughly n*t_ovr.
> > > * AD7380:  Field for oversampling and fixed conversion time,
> > >            3 MSPS for the AD7380 and 4 MSPS for AD7381,
> > >            maximum sampling period is n*t_ovr, e.g. f_samp=(6/4MSPS).
> > > 
> > > None will or claim to stretch over the sampling period the oversampling
> > > conversions, but rather, do the n-conversions at oversampling rate,
> > > providing the conversion as soon as it is ready and idling until the
> > > next edge of the sampling frequency.

> > >   
> > >>>>>
> > >>>>>   Sampling trigger |       |       |       |       |
> > >>>>>   ADC conversion   + + + + + + + + + + + + + + + + + + + +
> > >>>>>   ADC data ready         *       *       *       *       *
> > >>>>>   
> > 
> > And let's call this one "continuous mode".
> > 
> > But as we will see, both of these are a bit ambiguous in their current
> > form. The complete picture is a bit more nuanced.
> > 
> > ---
> > 
> > Let's take the RTQ6056 case (since I actually looked at that one before
> > as inspiration for developing another driver).
> > 
> > The chip itself is programmable and can operate in either a burst/triggered
> > mode or in a continuous mode. However, the way the IIO driver is implemented,
> > it is configured in continuous mode to trigger ADC conversions. But uses an
> > independent IIO trigger that triggers reading sample data.

Hmm. To me it looks like that driver should provide at trigger given there
is a conversion ready interrupt.  I guess the author didn't care for their use cases.

Ideally it would do triggered capture if using an hrtimer trigger and
continuous mode only when using it's own trigger. That way data is captured
only when required. Would get fiddlier though if events were supported (which
they aren't currently)./

> > So from the point of
> > view of the data in a buffered read, it looks like burst mode. But the value
> > of the sampling_frequency attribute (the ADC device attribute, not the hrtimer
> > trigger attribute) is for the hardware continuous mode.
> > 
> > Hardware:
> > sampling_frequency   |       |       |       |       |
> > ADC conversion       + + + + + + + + + + + + + + + + + + + +
> > ADC data ready             *       *       *       *       *
> > sample number              S0      S1      S2      S3      S4
> > 
> > IIO:
> > hrtimer frequency               |                     |
> > I2C read                         *                     *
> > push to buffer                   S0                    S3
> > 
> > The IIO (hrtimer) trigger only reads the most recently available data, it
> > doesn't trigger any conversion. The clocks are asynchronous.  
> 
> That implementation sure works for low-speed converters where power-consuption
> is not a concern.

For this particular device it looks more like a stop gap to keep the driver simple.
Maybe there is some other reason fitting it into a more conventional model
doesn't work.

> 
> > I think adding an oversampling_frequency attribute to this driver could make
> > it easier to used/understand since oversampling_frequency would be exactly
> > the "conversion rate". Compared to the current situation where the "conversion
> > rate" is the sampling_frequency / oversampling_ratio.  
I hope sampling_frequency * oversampling_ratio or that is broken.

Sampling frequency is always meant to be the rate at which a device can provide
data (either exact or maximum if using another trigger).

> 
> I agree.
> Applicable only for devices where sampling_frequency and
> oversampling_frequency are detached.
> Devices that provide a value for the fixed oversampling_frequency
> frequency could have the attribute as read-only (e.g AD7380).

RO indeed makes sense where it is fixed.

> 
> > 
> > It is also interesting to consider that if someone decided to add SPI offload
> > support to this driver, then there would be the possibility of using burst/
> > trigggered mode or continuous mode and might want to support both even. In
> > fact this is exactly the possibility we have with ad7606 that I mentioned in
> > a previous reply. So we might even need an oversampling_mode attribute to allow
> > selecting one or the other. But that is something to save for a ad7606 patch
> > series.  

I'm on board with the reasoning for switching between them mentioned above based
on trigger but if you are doing SPI offload then either:
1) Drive the SPI offload from the alert signal and run the device in continuous mode or
2) Drive the SPI offload from an external signal and use triggered mode.

I'm not seeing a usecase for running continuous against an external signal when
we care about performance. That's kind of the worst of all possible worlds as
we are reading data we don't care about so wasting power, or getting weird
beating pattern issues against the internal clocking.


> 
> For AD4052:
> 
> * iio raw read is the usual low-speed, CNV->DRDY->Read.
> * iio buffer: uses offload, samp_freq is PWM node freq.
>   - iio triggered buffer: not included on proposed series,
>                           but doable as a fallback if offload
>                           not available.

I'd do that via the same path as raw read if anyone wants it.  In theory you could
time off the PWM if you routed that to an interrupt but that is nasty.

> * iio events: monitor mode, device exits monitor mode on event
>               (device specific behaviour, cannot be changed),
>               the irq is propagated as iio event.
> 
> iio oversampling changes the iio raw and iio buffer readings behaviour
> (takes longer to get a sample out).
> 
> > 
> > ---
> > 
> > Another driver probably worth considering is ad4030. In this one, there is no
> > internal clock to drive conversions. So, for oversampling, the sample rate is
> > just "as fast as possible" (currently bit-banging a GPIO). So it doesn't actually
> > have an oversampling frequency.
> >
> >
> > If someone ever decided to hook it up to some hardware that could actually
> > trigger a finite number of pulses at a specific rate, then this new attribute
> > for oversampling_frequency would become useful. For this particular driver,
> > the presence or absence of an oversampling_frequency attribute would have
> > a meaning, but I don't think this generalizes to other ADCs.
> >   
> 
> The hardware is the usual CNV trigger, so it is not really about hooking
> up more hardware, but changing the behaviour
> (takes n-CNV triggers to get a sample out).
> 
> The AD4052 also supports what you described, it is the "Averaging Mode" (p.31),
> while the implemented uses the "Burst Averaging Mode" (p.32).
> In the "Averaging Mode", effective sampling rate is
> sampling_frequency / oversampling_ratio, while in
> "Burst Averaging Mode" is sampling_frequency.
 
> So, in "Burst Averaging Mode", the oversampling_frequency is *detached*
> from sampling_frequency.
> 
> The driver could hide away the effective sampling frequency discrepancy
> by reading the state and scaling sampling_frequency based on
> oversampling_ratio, exactly like RTQ6056.

Agreed. It would need to do that.  Simple code that isn't messing with oversampling
ratio expects data at 'sampling_frequency' HZ. We need to maintain that.
> 
> > ---
> > 
> > AD4695 is an interesting case to consider as well. When used without SPI offload
> > support, we actually don't allow oversampling currently. If we did though, it
> > be similar to the ad4030 in that we could either make it "as fast as possible"
> > by banging a GPIO or the CS line depending on how the chip was wired up. Or it
> > could use some specialized hardware to generate a pulse train to actually get
> > a known conversion rate.
> > 
> > For now though, oversampling is only implemented when using a SPI offload.
> > It works like this:
> > 

Channel		1   2       3 1   2       3 1   2       3
OSR		2   4       1 2   4       1 2   4       1
Trigger		| | | | | | | | | | | | | | | | | | | | |
ADC Conversion	+ + + + + + + + + + + + + + + + + + + + +
ADC data ready	   *       * *   *       * *   *       * *
IIO sample                   S0            S1            S2

> > 
> > In this case, there isn't a "sample" trigger that triggers a burst of
> > samples. Rather, there is only a "conversion" trigger that triggers
> > individual conversion. In other words, we would call this "continuous mode".
> > And it also shows that some chips allow individual channels to have
> > different oversampling ratios.  
> 
> I find this "continuous mode" name confusing, maybe stick with
> "averaging mode", where each CNV pulse triggers a conversion and
> "burst averaging mode", where a CNV pulse triggers a burst of
> conversions.

Naming is tricky but agreed that 'continuous' means far to many things.

> 
> > 
> > In this case, it would be nice to have an oversampling_frequency
> > attribute as well because it would exactly correspond to the conversion
> > rate. Currently each channel has a sampling_frequency attribute that
> > is oversampling_frequency / oversampling_ratio (same as RTQ6056).  
> 
> I don't mind that, is the "hide away" I mentioned earlier.
> I believe that, when oversampling is a driver feature,
> oversampling_frequency is either:
>  * The RW conversion frequency:
>    - averaging mode:       oversampling_frequency, sampling_frequency,
>                            and oversampling_ratio affect each other,
>                            due to "hide away" logic.
>                            One CNV one conversion.

I'd expect oversampling_frequency to be the dependent one here (so RO)
In theory we could allow them all to be written but sometimes it is easier
to just set a precedence (which also has to be what was defined first)

>    - burst averaging mode: sampling frequency and conversion frequency
>                            are detached, doesn't need "hide away" logic.
>                            One CNV one sample.

For this one RW makes sense.

>  * The RO conversion frequency with the role of maximum conversion frequency.
> 
> > 
> > ---
> > 
> > So my conclusion here is that the new proposed oversampling_frequency
> > attribute has nothing to do with "burst mode" or "continuous mode" it
> > has the same meaning in both cases.  
> 
> It depends if it is detached from sampling_frequency or not.
> If the norm is to "hide away" with extra logic in "averaging mode",
> they become interdependent.
> If sampling frequency and conversion frequency are detached from each
> other, oversampling_frequency for sure needs to exist.
> 
> >                                     It is effectively the rate for
> > individual ADC conversions.
> >   
> 
> Yes.
Agreed. It is a useful thing to expose even when RO.  When it is
independent (subject to limits is fine) of sampling_frequency
and oversampling_ratio then it should be RW.  'Maybe' there will
be other cases were RW makes sense but I'm not yet seeing them.

Jonathan
Jorge Marques April 7, 2025, 4:28 p.m. UTC | #8
On Sun, Apr 06, 2025 at 11:25:51AM +0100, Jonathan Cameron wrote:
> On Thu, 3 Apr 2025 10:35:48 +0200
> Jorge Marques <gastmaier@gmail.com> wrote:
> 
> > On Wed, Apr 02, 2025 at 03:27:51PM -0500, David Lechner wrote:
> > > On 4/2/25 8:47 AM, Jorge Marques wrote:  
> > > > On Sun, Mar 30, 2025 at 06:53:53PM +0100, Jonathan Cameron wrote:  
> > > >> On Sun, 30 Mar 2025 12:34:39 -0500
> > > >> David Lechner <dlechner@baylibre.com> wrote:
> > > >>  
> > > >>> On 3/30/25 12:13 PM, Jonathan Cameron wrote:  
> > > >>>> On Fri, 21 Mar 2025 15:50:02 +0100
> > > >>>> Jorge Marques <jorge.marques@analog.com> wrote:  
> > > 
> > > Once you dig into it, the current situation is even more complicated than
> > > I expected. :-o
> > > 
> > > This reply has ended up being mostly a brain dump of my observations. I hope
> > > it isn't too confusing. Consider this more of a brainstorm on a future
> > > documentation page on sampling rates rather than commenting on what to do
> > > on this particular patch. :-)
> 
> I think it's worth mentioning that the original oversampling attributes were
> IIRC (note I could be wrong) on self clocking devices such as accelerometers,
> at least sometimes with a hardware FIFO involved. In some cases even then
> the conversions are bursted. In others I think (though can't find an example right
> now) they are dumb enough that if you enable oversampling it clocks the same
> as without. (the 'stretched case below).  A lot of the datasheets are very
> vague!
> 
> 
> > > 
> > > ---
> > > 
> > > To make sure we are clear, I think we need to define some precise terminology,
> > > especially with regard to "sample rate" since that can be used to mean a
> > > lot of different things.
> > > 
> > > There is the "IIO sample rate" (could also call it the "effective sample rate")
> > > that is the rate that we push one complete set of data to an IIO buffer. In
> > > many cases, this would be the frequency of the hrtimer trigger that is configured
> > > as the trigger for the iio:deviceX.  
> > 
> > Yes.
> 
> I'd be tempted focus more on the fully self clocked devices with a dataready signal.
> Bringing in the different clock domain of a hrtimer trigger or similar adds
> yet another layer of complexity.  You get into that below.
> 
> > 
> > > 
> > > On the other end of the spectrum, we have the "conversion rate" which is the
> > > rate that individual ADC conversions can happen.  
> > 
> > Yes.
> > 
> > > 
> > > What I had not seen before, but now I see in existing drivers, is that these
> > > may actually be completely independent. In other words, the hrtimer trigger
> > > only triggers reading the most recent set of conversions and conversions are
> > > driven by a completely separate trigger, generally some sort of clock in the
> > > ADC itself.
> 
> That is 'sometimes' the case.  Often on devices that provide a dataready trigger
> as well (but were we might want to sample much slower, or where some designer
> didn't wire the pin up).  Note the sampling frequency in question here is that
> of the trigger. If the device is self clocked then it typically also provides
> a sampling frequency attribute to describe what it is doing.
> 
> In other cases the hrtimer trigger is starting a conversion (or burst of conversions
> as appropriate).  In those cases if the device provides a sampling frequency
> it is a bound on how fast it can be driven.  That is effectively the same as
> the previous case as if we are sampling latest value, there is no point in
> going faster than the device sampling frequency (hence no different ABI).
> 
> > > 
> > > So I think we should expand the diagrams below to show more layers for the
> > > completely general case.
> > >   
> > > >>>>     
> > > >>>>> Some devices have an internal clock used to space out the conversion
> > > >>>>> trigger for the oversampling filter,
> > > >>>>> Consider an ADC with conversion and data ready pins topology:
> > > >>>>>
> > > >>>>>   Sampling trigger |       |       |       |       |
> > > >>>>>   ADC conversion   ++++    ++++    ++++    ++++    ++++
> > > >>>>>   ADC data ready      *       *       *       *       *
> > > >>>>>  
> > > 
> > > For terminology, let's call this "burst mode" oversampling (maybe also
> > > referred to as "triggered mode" in some data sheets).
> > >   
> > > >>>>> With the oversampling frequency, conversions can be evenly space between
> > > >>>>> the sampling edge:    
> > > >>>>
> > > >>>> I'm not sure what this second example is providing.  Are you suggesting
> > > >>>> that if we don't provide oversampling frequency we should assume this
> > > >>>> pattern?  i.e. it is the default?
> > > >>>>     
> > > > 
> > > > The default is to do the n-conversions sequentially (n*t_conv),
> > > > "left-aligned" as in the diagram above.
> 
> Hmm. What's the default if we don't provide information on the conversion sampling
> frequency?  I don't mean what is most common or what does a particular device do,
> just the default from an ABI point of view.  Either default is no information
> or it is is 'evenly spread'.  
> 

The default is no information for sure.

> > > > The main application for oversampling is to average out the noise over a wider
> > > > bandwidth.
> > > > 
> > > > I looked into some of the drivers with oversampling and the supported devices
> > > > datasheets:
> > > > 
> > > > * ADS1298: Single field for sampling rate and oversampling,
> > > >            I assume the values are the maximum values that the
> > > > 	   oversampling time does not exceed the sampling period.
> > > > * RTQ6056: Field for oversampling and conversion time,
> > > >            maximum sampling period is roughly n*t_ovr.
> > > > * MCP3561: Field for oversampling and conversion time.
> > > >            maximum sampling period is roughly n*t_ovr.
> > > > * AD7380:  Field for oversampling and fixed conversion time,
> > > >            3 MSPS for the AD7380 and 4 MSPS for AD7381,
> > > >            maximum sampling period is n*t_ovr, e.g. f_samp=(6/4MSPS).
> > > > 
> > > > None will or claim to stretch over the sampling period the oversampling
> > > > conversions, but rather, do the n-conversions at oversampling rate,
> > > > providing the conversion as soon as it is ready and idling until the
> > > > next edge of the sampling frequency.
> 
> > > >   
> > > >>>>>
> > > >>>>>   Sampling trigger |       |       |       |       |
> > > >>>>>   ADC conversion   + + + + + + + + + + + + + + + + + + + +
> > > >>>>>   ADC data ready         *       *       *       *       *
> > > >>>>>   
> > > 
> > > And let's call this one "continuous mode".
> > > 
> > > But as we will see, both of these are a bit ambiguous in their current
> > > form. The complete picture is a bit more nuanced.
> > > 
> > > ---
> > > 
> > > Let's take the RTQ6056 case (since I actually looked at that one before
> > > as inspiration for developing another driver).
> > > 
> > > The chip itself is programmable and can operate in either a burst/triggered
> > > mode or in a continuous mode. However, the way the IIO driver is implemented,
> > > it is configured in continuous mode to trigger ADC conversions. But uses an
> > > independent IIO trigger that triggers reading sample data.
> 
> Hmm. To me it looks like that driver should provide at trigger given there
> is a conversion ready interrupt.  I guess the author didn't care for their use cases.
> 
> Ideally it would do triggered capture if using an hrtimer trigger and
> continuous mode only when using it's own trigger. That way data is captured
> only when required. Would get fiddlier though if events were supported (which
> they aren't currently)./
> 
> > > So from the point of
> > > view of the data in a buffered read, it looks like burst mode. But the value
> > > of the sampling_frequency attribute (the ADC device attribute, not the hrtimer
> > > trigger attribute) is for the hardware continuous mode.
> > > 
> > > Hardware:
> > > sampling_frequency   |       |       |       |       |
> > > ADC conversion       + + + + + + + + + + + + + + + + + + + +
> > > ADC data ready             *       *       *       *       *
> > > sample number              S0      S1      S2      S3      S4
> > > 
> > > IIO:
> > > hrtimer frequency               |                     |
> > > I2C read                         *                     *
> > > push to buffer                   S0                    S3
> > > 
> > > The IIO (hrtimer) trigger only reads the most recently available data, it
> > > doesn't trigger any conversion. The clocks are asynchronous.  
> > 
> > That implementation sure works for low-speed converters where power-consuption
> > is not a concern.
> 
> For this particular device it looks more like a stop gap to keep the driver simple.
> Maybe there is some other reason fitting it into a more conventional model
> doesn't work.
> 
> > 
> > > I think adding an oversampling_frequency attribute to this driver could make
> > > it easier to used/understand since oversampling_frequency would be exactly
> > > the "conversion rate". Compared to the current situation where the "conversion
> > > rate" is the sampling_frequency / oversampling_ratio.  
> I hope sampling_frequency * oversampling_ratio or that is broken.
> 
> Sampling frequency is always meant to be the rate at which a device can provide
> data (either exact or maximum if using another trigger).
> 
> > 
> > I agree.
> > Applicable only for devices where sampling_frequency and
> > oversampling_frequency are detached.
> > Devices that provide a value for the fixed oversampling_frequency
> > frequency could have the attribute as read-only (e.g AD7380).
> 
> RO indeed makes sense where it is fixed.
> 
> > 
> > > 
> > > It is also interesting to consider that if someone decided to add SPI offload
> > > support to this driver, then there would be the possibility of using burst/
> > > trigggered mode or continuous mode and might want to support both even. In
> > > fact this is exactly the possibility we have with ad7606 that I mentioned in
> > > a previous reply. So we might even need an oversampling_mode attribute to allow
> > > selecting one or the other. But that is something to save for a ad7606 patch
> > > series.  
> 
> I'm on board with the reasoning for switching between them mentioned above based
> on trigger but if you are doing SPI offload then either:
> 1) Drive the SPI offload from the alert signal and run the device in continuous mode or
> 2) Drive the SPI offload from an external signal and use triggered mode.
> 
> I'm not seeing a usecase for running continuous against an external signal when
> we care about performance. That's kind of the worst of all possible worlds as
> we are reading data we don't care about so wasting power, or getting weird
> beating pattern issues against the internal clocking.
> 
> 
> > 
> > For AD4052:
> > 
> > * iio raw read is the usual low-speed, CNV->DRDY->Read.
> > * iio buffer: uses offload, samp_freq is PWM node freq.
> >   - iio triggered buffer: not included on proposed series,
> >                           but doable as a fallback if offload
> >                           not available.
> 
> I'd do that via the same path as raw read if anyone wants it.  In theory you could
> time off the PWM if you routed that to an interrupt but that is nasty.
> 
> > * iio events: monitor mode, device exits monitor mode on event
> >               (device specific behaviour, cannot be changed),
> >               the irq is propagated as iio event.
> > 
> > iio oversampling changes the iio raw and iio buffer readings behaviour
> > (takes longer to get a sample out).
> > 
> > > 
> > > ---
> > > 
> > > Another driver probably worth considering is ad4030. In this one, there is no
> > > internal clock to drive conversions. So, for oversampling, the sample rate is
> > > just "as fast as possible" (currently bit-banging a GPIO). So it doesn't actually
> > > have an oversampling frequency.
> > >
> > >
> > > If someone ever decided to hook it up to some hardware that could actually
> > > trigger a finite number of pulses at a specific rate, then this new attribute
> > > for oversampling_frequency would become useful. For this particular driver,
> > > the presence or absence of an oversampling_frequency attribute would have
> > > a meaning, but I don't think this generalizes to other ADCs.
> > >   
> > 
> > The hardware is the usual CNV trigger, so it is not really about hooking
> > up more hardware, but changing the behaviour
> > (takes n-CNV triggers to get a sample out).
> > 
> > The AD4052 also supports what you described, it is the "Averaging Mode" (p.31),
> > while the implemented uses the "Burst Averaging Mode" (p.32).
> > In the "Averaging Mode", effective sampling rate is
> > sampling_frequency / oversampling_ratio, while in
> > "Burst Averaging Mode" is sampling_frequency.
>  
> > So, in "Burst Averaging Mode", the oversampling_frequency is *detached*
> > from sampling_frequency.
> > 
> > The driver could hide away the effective sampling frequency discrepancy
> > by reading the state and scaling sampling_frequency based on
> > oversampling_ratio, exactly like RTQ6056.
> 
> Agreed. It would need to do that.  Simple code that isn't messing with oversampling
> ratio expects data at 'sampling_frequency' HZ. We need to maintain that.
> > 
> > > ---
> > > 
> > > AD4695 is an interesting case to consider as well. When used without SPI offload
> > > support, we actually don't allow oversampling currently. If we did though, it
> > > be similar to the ad4030 in that we could either make it "as fast as possible"
> > > by banging a GPIO or the CS line depending on how the chip was wired up. Or it
> > > could use some specialized hardware to generate a pulse train to actually get
> > > a known conversion rate.
> > > 
> > > For now though, oversampling is only implemented when using a SPI offload.
> > > It works like this:
> > > 
> 
> Channel		1   2       3 1   2       3 1   2       3
> OSR		2   4       1 2   4       1 2   4       1
> Trigger		| | | | | | | | | | | | | | | | | | | | |
> ADC Conversion	+ + + + + + + + + + + + + + + + + + + + +
> ADC data ready	   *       * *   *       * *   *       * *
> IIO sample                   S0            S1            S2
> 
> > > 
> > > In this case, there isn't a "sample" trigger that triggers a burst of
> > > samples. Rather, there is only a "conversion" trigger that triggers
> > > individual conversion. In other words, we would call this "continuous mode".
> > > And it also shows that some chips allow individual channels to have
> > > different oversampling ratios.  
> > 
> > I find this "continuous mode" name confusing, maybe stick with
> > "averaging mode", where each CNV pulse triggers a conversion and
> > "burst averaging mode", where a CNV pulse triggers a burst of
> > conversions.
> 
> Naming is tricky but agreed that 'continuous' means far to many things.
> 
> > 
> > > 
> > > In this case, it would be nice to have an oversampling_frequency
> > > attribute as well because it would exactly correspond to the conversion
> > > rate. Currently each channel has a sampling_frequency attribute that
> > > is oversampling_frequency / oversampling_ratio (same as RTQ6056).  
> > 
> > I don't mind that, is the "hide away" I mentioned earlier.
> > I believe that, when oversampling is a driver feature,
> > oversampling_frequency is either:
> >  * The RW conversion frequency:
> >    - averaging mode:       oversampling_frequency, sampling_frequency,
> >                            and oversampling_ratio affect each other,
> >                            due to "hide away" logic.
> >                            One CNV one conversion.
> 
> I'd expect oversampling_frequency to be the dependent one here (so RO)
> In theory we could allow them all to be written but sometimes it is easier
> to just set a precedence (which also has to be what was defined first)
> 
> >    - burst averaging mode: sampling frequency and conversion frequency
> >                            are detached, doesn't need "hide away" logic.
> >                            One CNV one sample.
> 
> For this one RW makes sense.
> 
> >  * The RO conversion frequency with the role of maximum conversion frequency.
> > 
> > > 
> > > ---
> > > 
> > > So my conclusion here is that the new proposed oversampling_frequency
> > > attribute has nothing to do with "burst mode" or "continuous mode" it
> > > has the same meaning in both cases.  
> > 
> > It depends if it is detached from sampling_frequency or not.
> > If the norm is to "hide away" with extra logic in "averaging mode",
> > they become interdependent.
> > If sampling frequency and conversion frequency are detached from each
> > other, oversampling_frequency for sure needs to exist.
> > 
> > >                                     It is effectively the rate for
> > > individual ADC conversions.
> > >   
> > 
> > Yes.
> Agreed. It is a useful thing to expose even when RO.  When it is
> independent (subject to limits is fine) of sampling_frequency
> and oversampling_ratio then it should be RW.  'Maybe' there will
> be other cases were RW makes sense but I'm not yet seeing them.
> 
> Jonathan
> 
> 

All things considered, I believe the appropriate ABI documentation is the following


  What:                /sys/bus/iio/devices/iio:deviceX/oversampling_frequency
  KernelVersion:       6.15
  Contact:     linux-iio@vger.kernel.org
  Description:
           Some devices have internal clocks for oversampling.
           Sets the resulting frequency to trigger a conversion used by the
           oversampling filter.
           If the device has a fixed internal clock or is computed based on
           the sampling frequency parameter, the parameter is read only.

If that's ok I'll resubmit the patch including only this commit,
and then focus on submitting AD4052 driver V2 series.

Jorge
Jonathan Cameron April 7, 2025, 6:40 p.m. UTC | #9
> 
> All things considered, I believe the appropriate ABI documentation is the following
> 
> 
>   What:                /sys/bus/iio/devices/iio:deviceX/oversampling_frequency
>   KernelVersion:       6.15
>   Contact:     linux-iio@vger.kernel.org
>   Description:
>            Some devices have internal clocks for oversampling.
>            Sets the resulting frequency to trigger a conversion used by the
>            oversampling filter.
>            If the device has a fixed internal clock or is computed based on
>            the sampling frequency parameter, the parameter is read only.
> 
> If that's ok I'll resubmit the patch including only this commit,
> and then focus on submitting AD4052 driver V2 series.

Looks good to me with one addition.  Mention the units (Hz)

Jonathan

> 
> Jorge
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 33c09c4ac60a4feec82308461643134f5ba84b66..2317bacf6a2884691a08725d6f01d18555a96227 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -139,6 +139,23 @@  Contact:	linux-iio@vger.kernel.org
 Description:
 		Hardware dependent values supported by the oversampling filter.
 
+What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency
+KernelVersion:	6.15
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Some devices have internal clocks for the ADC oversampling.
+		Sets the resulting sampling frequency to trigger a conversion
+		used by the oversampling filter.
+		Can be used to evenly space conversion between the sampling edge
+		on some devices.
+
+What:		/sys/bus/iio/devices/iio:deviceX/oversampling_frequency_available
+KernelVersion:	6.15
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Hardware dependent values supported by the oversampling
+		frequency.
+
 What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
 What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
 What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw