| Message ID | f78c3dee381b23c17787f1e2bc9c5667741d407b.1736201898.git.Jonathan.Santos@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: ad7768-1: Add features, improvements, and fixes | expand |
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.
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. Would something like <type>[_name]_oversampling_ratio make more sense? Let me know what you think
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 >
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 > > > > >
On 01/12, Jonathan Cameron wrote: > 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. > 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 > > > > > > > > > >
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.
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