Message-ID: <hf5dwxs62oof3gom43c6rkdsq3gky6eplxej627t46ktt5blfr@kpmjpxku4inc>
Date: Wed, 2 Apr 2025 15:47:55 +0200
From: Jorge Marques <gastmaier@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: David Lechner <dlechner@...libre.com>, 
	Jorge Marques <jorge.marques@...log.com>, Lars-Peter Clausen <lars@...afoo.de>, 
	Michael.Hennerich@...log.com, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] Documentation: ABI: add oversampling frequency in
 sysfs-bus-iio

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@...libre.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@...log.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@...log.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@...r.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@...r.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@...r.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
> > >>  
> > >   
> > 
> 

Powered by blists - more mailing lists