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

[Jonathan Cameron <jic23@kernel.org>]

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.

Jonathan

>