Re: [PATCH v1 03/15] Documentation: ABI: testing: ad7768-1: Add device specific ABI documentation.

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 12 Jan 2025 12:20:47 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sat, 11 Jan 2025 20:22:36 -0300
> Jonathan Santos <jonath4nns@gmail.com> wrote:
> 
> > On 01/07, David Lechner wrote:  
> > > On 1/7/25 9:24 AM, Jonathan Santos wrote:    
> > > > Add ABI documentation specific to the ad7768-1 device, detailing
> > > > the decimation_rate attribute for better clarity and usability.
> > > > 
> > > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > > > ---
> > > >  .../ABI/testing/sysfs-bus-iio-adc-ad7768-1          | 13 +++++++++++++
> > > >  1 file changed, 13 insertions(+)
> > > >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad7768-1
> > > > 
> > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7768-1 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7768-1
> > > > new file mode 100644
> > > > index 000000000000..065247f07cfb
> > > > --- /dev/null
> > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad7768-1
> > > > @@ -0,0 +1,13 @@
> > > > +What:		/sys/bus/iio/devices/iio:deviceX/decimation_rate_available
> > > > +KernelVersion:
> > > > +Contact:	linux-iio@vger.kernel.org
> > > > +Description:
> > > > +		Reading returns a range of possible decimation rate values.
> > > > +
> > > > +What:		/sys/bus/iio/devices/iio:deviceX/decimation_rate
> > > > +KernelVersion:
> > > > +Contact:	linux-iio@vger.kernel.org
> > > > +Description:
> > > > +		Sets up the decimation rate for the digital filter. This can
> > > > +		directly impact in the final sampling frequency. Reading returns
> > > > +		the decimation rate. Writing sets the decimation rate.    
> > > 
> > > If this only affects the filter, I would suggest to add `filter_` to the
> > > beginning of the attribute names.
> > > 
> > > Also, an explanation of how to interpret the numbers would be helpful. It looks
> > > like a unitless number that acts a sort of a multiplier or divider, but that
> > > part isn't so clear to me. 
> > > 
> > > Or...
> > > 
> > > Since the decimation rate affects the -3dB point of the filters we could use
> > > the standard IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY instead of introducing
> > > a new attribute.    
> > 
> > Well, here the -3dB cutoff depends on the ODR, which is determined by both the MCLK
> > divider and decimation rate.
> > 
> > Wideband: -3dB at 0.433 × ODR
> > Sinc5: -3dB at 0.204 × ODR
> > Sinc3: -3dB at 0.2617 × ODR
> > 
> > If we use _filter_low_pass_3db_frequency to control the decimation and _sampling_frequency
> > to control the MCLK divider, wouldn’t it be confusing for one to always affect the other?
> > A different ODR would result in a different cutoff, and vice versa.  
> 
> We should definitely not have a filter control changing sampling frequency (which tends to
> be a more common control for users to fiddle with).  However the other way around is
> fine.  So for a given _sampling_frequency present via
> in_xx_filter_low_pass_3db_frequency_available the list of
> possible 3db frequencies and use them to configure the decimation.
> 
> > 
> > Would something like <type>[_name]_oversampling_ratio make more sense? Let me know what you think  
> 
> I'd rather not if we can avoid that new ABI, but it is is better than a new term
> like decimation_rate.
Reading the code I realised I'd misunderstood this question.
Yes for controlling decimation via oversampling.  Decimation (to me at least)
means 'ignoring' data but seems here it means averaging it.

Controls should be in order of preference
sampling_frequency (try to keep this constant as others chagnes - if you have to because of range issues,
  tweak it) This is the rate we get data at after filter.
oversampling_ratio (again try to keep constant) - controls samples taken per output one.
_3db_frequency - anything left to control on the filter, or just a RO output of what it currently means.

Jonathan



> 
> Jonathan
> 
> >   
> 
>