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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z4W4nS8a5A2PZ4wV@JSANTO12-L01.ad.analog.com>
Date: Mon, 13 Jan 2025 22:06:37 -0300
From: Jonathan Santos <jonath4nns@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: 58ea1899-05be-4743-911b-77a56f08c347@...libre.com,
	David Lechner <dlechner@...libre.com>,
	Jonathan Santos <Jonathan.Santos@...log.com>,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, lars@...afoo.de,
	Michael.Hennerich@...log.com, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, marcelo.schmitt1@...il.com
Subject: Re: [PATCH v1 03/15] Documentation: ABI: testing: ad7768-1: Add
 device specific ABI documentation.

On 01/12, Jonathan Cameron wrote:
> On Sun, 12 Jan 2025 12:20:47 +0000
> Jonathan Cameron <jic23@...nel.org> wrote:
> 
> > On Sat, 11 Jan 2025 20:22:36 -0300
> > Jonathan Santos <jonath4nns@...il.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@...log.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@...r.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@...r.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.
> 

Yes, for this device the decimation is indeed averaging the data.

> 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
> 

Great, then. Decimation will be controlled by oversampling, frequency 
modulator controlled by sampling frequency and _3db_frequency as RO 
attribute, since there are no parameters left.

> 
> 
> > 
> > Jonathan
> > 
> > >   
> > 
> > 
>