[PATCH v2 0/3] Re-add filter_type/filter_mode

[PATCH v2 0/3] Re-add filter_type/filter_mode

[Marcelo Schmitt]

Better document sysfs ABI for ADC digital filter configuration.

Change log v1 -> v2
- Split into 3 patches.
- Re-added sysfs-bus-iio-adc-ad4130 to keep filter_mode separate from filter_type.
- Removed in_voltage-voltage_filter_mode_available from sysfs-bus-iio.
- Added deprecation notes in sysfs-bus-iio-adc-ad4130 asking to use filter_type
  for new drivers and referencing sysfs-bus-iio.
- Improved commit message with explanation about use of filter type attributes.
- Dropped AD4130 specific filter_type_available options from sysfs-bus-iio.

Link to v1: https://lore.kernel.org/linux-iio/b2132bd3ca1d64cdd8d5afab1f1f33c574718b50.1732901318.git.marcelo.schmitt@analog.com/

Additional specific comments in patches 1 and 3.

Marcelo Schmitt (3):
  Documentation: ABI: IIO: Re-add sysfs-bus-iio-adc-ad4130
  Documentation: ABI: IIO: Add filter_type documentation
  Documentation: ABI: IIO: Drop AD4130 specific filter_type_available
    options

 Documentation/ABI/testing/sysfs-bus-iio       | 29 ++++------
 .../ABI/testing/sysfs-bus-iio-adc-ad4130      | 55 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 3 files changed, 68 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130


base-commit: 9b75dd1b7d6b98699a104c6b1eec0c8817e5fd4b
-- 
2.45.2

[PATCH v2 1/3] Documentation: ABI: IIO: Re-add sysfs-bus-iio-adc-ad4130

[Marcelo Schmitt]

The ad4130 driver exports in_voltageY-voltageZ_filter_mode and
in_voltage-voltage_filter_mode_available attributes to user space. A
previous patch merged the documentation for those attributes with the
documentation for filter_type/filter_type_available into sysfs-bus-iio.
Filter mode and filter type refer to the same feature which is the digital
filter applied over ADC samples. However, since datasheets use the term
`filter type` and ad4130 driver is the only one using filter_mode,
deprecate the filter_mode ABI in favor of filter_type and keep the docs
separate to avoid confusion and intricate attribute descriptions.

The digital filter helps shape the noise rejection bandwidth and is
supposed to be adjusted according to the signal of interest. Depending on
the particular ADC design, the digital filter configuration may have an
impact on the output data rate (ODR) (which is equivalent to the sampling
frequency for SAR ADCs - `sampling_frequency` ABI), 3dB cutoff frequency of
the filter (`_low_pass_3db_frequency` attributes), or settling time.

Digital filters are a common feature among sigma-delta ADCs and
IIO drivers set digital filters according to different attributes.
ad4130 and ad7779 filter options are supported through dedicated
filter_mode/filter_type attributes in sysfs.
ad7768-1 and ad7173 set the sinc filters according to the
sampling_frequency attribute. No filter_type attribute.
ad7124 sets the digital filter according to a filter_low_pass_3db_frequency
attribute so it doesn't export filter type attributes to user space.

Fixes: 01bb12922b60 ("Documentation: ABI: added filter mode doc in sysfs-bus-iio")
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Second and third paragraphs of the body could maybe go into device specific docs
as suggested in some previous thread but those drivers don't have docs in
Documentation/iio/ and I've been more interested in how filter_type is supported
accross IIO to be able to support it appropriatly in new drivers.

 Documentation/ABI/testing/sysfs-bus-iio       |  1 -
 .../ABI/testing/sysfs-bus-iio-adc-ad4130      | 55 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 3 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index d97c35057a03..1faac5b2ad84 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2277,7 +2277,6 @@ Description:
 		representing the sensor unique ID number.
 
 What:		/sys/bus/iio/devices/iio:deviceX/filter_type_available
-What:		/sys/bus/iio/devices/iio:deviceX/in_voltage-voltage_filter_mode_available
 KernelVersion:	6.1
 Contact:	linux-iio@vger.kernel.org
 Description:
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
new file mode 100644
index 000000000000..bd827186ad1f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
@@ -0,0 +1,55 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltage-voltage_filter_mode_available
+KernelVersion:  6.2
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Reading returns a list with the possible filter modes.
+
+		  * "sinc4"       - Sinc 4. Excellent noise performance. Long
+                    1st conversion time. No natural 50/60Hz rejection.
+
+		  * "sinc4+sinc1" - Sinc4 + averaging by 8. Low 1st conversion
+		    time.
+
+		  * "sinc3"	      - Sinc3. Moderate 1st conversion time.
+		    Good noise performance.
+
+		  * "sinc3+rej60" - Sinc3 + 60Hz rejection. At a sampling
+		    frequency of 50Hz, achieves simultaneous 50Hz and 60Hz
+		    rejection.
+
+		  * "sinc3+sinc1" - Sinc3 + averaging by 8. Low 1st conversion
+		    time. Best used with a sampling frequency of at least
+		    216.19Hz.
+
+		  * "sinc3+pf1"   - Sinc3 + Post Filter 1. 53dB rejection @
+		    50Hz, 58dB rejection @ 60Hz.
+
+		  * "sinc3+pf2"   - Sinc3 + Post Filter 2. 70dB rejection @
+		    50Hz, 70dB rejection @ 60Hz.
+
+		  * "sinc3+pf3"   - Sinc3 + Post Filter 3. 99dB rejection @
+		    50Hz, 103dB rejection @ 60Hz.
+
+		  * "sinc3+pf4"   - Sinc3 + Post Filter 4. 103dB rejection @
+		    50Hz, 109dB rejection @ 60Hz.
+
+		This interface is deprecated; please use filter_type_available
+		documented in Documentation/ABI/testing/sysfs-bus-iio to provide
+		filter options for new drivers.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY-voltageZ_filter_mode
+KernelVersion:  6.2
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Set the filter mode of the differential channel. When the filter
+		mode changes, the in_voltageY-voltageZ_sampling_frequency and
+		in_voltageY-voltageZ_sampling_frequency_available attributes
+		might also change to accommodate the new filter mode.
+		If the current sampling frequency is out of range for the new
+		filter mode, the sampling frequency will be changed to the
+		closest valid one.
+
+		This interface is deprecated; please use
+		in_voltageY-voltageZ_filter_type documented in
+		Documentation/ABI/testing/sysfs-bus-iio to provide
+		filter options for new drivers.
diff --git a/MAINTAINERS b/MAINTAINERS
index 98a3c1e46311..28dbbf6d6efa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1279,6 +1279,7 @@ M:	Cosmin Tanislav <cosmin.tanislav@analog.com>
 L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
 F:	drivers/iio/adc/ad4130.c
 
-- 
2.45.2

[PATCH v2 2/3] Documentation: ABI: IIO: Add filter_type documentation

[Marcelo Schmitt]

A previous patch added documentation for filter_type_available attributes.
However, the description for the value attribute (filter_type) was missing.
Add documentation for filter_type sysfs ABI.

Fixes: 01bb12922b60 ("Documentation: ABI: added filter mode doc in sysfs-bus-iio")
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 1faac5b2ad84..1ddf3d7dd2b3 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2299,6 +2299,13 @@ Description:
 		* "sinc3+pf3" - Sinc3 + device specific Post Filter 3.
 		* "sinc3+pf4" - Sinc3 + device specific Post Filter 4.
 
+What:		/sys/bus/iio/devices/iio:deviceX/filter_type
+KernelVersion:	6.1
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Specifies which filter type apply to the channel. The possible
+		values are given by the filter_type_available attribute.
+
 What:		/sys/.../events/in_proximity_thresh_either_runningperiod
 KernelVersion:	6.6
 Contact:	linux-iio@vger.kernel.org
-- 
2.45.2

[PATCH v2 3/3] Documentation: ABI: IIO: Drop AD4130 specific filter_type_available options

[Marcelo Schmitt]

The only driver exporting filter_type_available attribute is ad7779 and it
only supports sinc3 and sinc5 filters. Remove options that are not
presented through filter_type_available attribute.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Maybe this is not worth it (or desirable?) since the options may come back in
the future if new drivers happen to need them.

 Documentation/ABI/testing/sysfs-bus-iio | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 1ddf3d7dd2b3..946c519f8687 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2280,24 +2280,13 @@ What:		/sys/bus/iio/devices/iio:deviceX/filter_type_available
 KernelVersion:	6.1
 Contact:	linux-iio@vger.kernel.org
 Description:
-		Reading returns a list with the possible filter modes. Options
-		for the attribute:
+		Lists all available filter types. Possible options for the
+		attribute:
 
-		* "sinc3" - The digital sinc3 filter. Moderate 1st
-		  conversion time. Good noise performance.
-		* "sinc4" - Sinc 4. Excellent noise performance. Long
-		  1st conversion time.
+		* "sinc3" - The digital sinc3 filter. Moderate 1st conversion
+		  time. Good noise performance.
 		* "sinc5" - The digital sinc5 filter. Excellent noise
-		  performance
-		* "sinc4+sinc1" - Sinc4 + averaging by 8. Low 1st conversion
-		  time.
-		* "sinc3+rej60" - Sinc3 + 60Hz rejection.
-		* "sinc3+sinc1" - Sinc3 + averaging by 8. Low 1st conversion
-		  time.
-		* "sinc3+pf1" - Sinc3 + device specific Post Filter 1.
-		* "sinc3+pf2" - Sinc3 + device specific Post Filter 2.
-		* "sinc3+pf3" - Sinc3 + device specific Post Filter 3.
-		* "sinc3+pf4" - Sinc3 + device specific Post Filter 4.
+		  performance.
 
 What:		/sys/bus/iio/devices/iio:deviceX/filter_type
 KernelVersion:	6.1
-- 
2.45.2

Re: [PATCH v2 3/3] Documentation: ABI: IIO: Drop AD4130 specific filter_type_available options

[David Lechner]

On 1/7/25 9:13 AM, Marcelo Schmitt wrote:
> The only driver exporting filter_type_available attribute is ad7779 and it
> only supports sinc3 and sinc5 filters. Remove options that are not
> presented through filter_type_available attribute.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> Maybe this is not worth it (or desirable?) since the options may come back in
> the future if new drivers happen to need them.

I would be tempted to add filter_type{,_available} attributes to the ad4130
driver that mirrors the existing filter_mode{,_available} attributes. We can't
remove filter_mode since that would break things, but having filter_type in
addition to that would mean than userspace tools could standardize on
filter_type and not have to make a special exception for the different naming
on ad4130.

Then we wouldn't need to delete the extra items here and we wouldn't have to
repeat the docs for sysfs-bus-iio-adc-ad4130. Those docs can just have the
deprecation paragraph and mention that it returns values identical to the
filter_type attributes for backwards compatibility.

Re: [PATCH v2 3/3] Documentation: ABI: IIO: Drop AD4130 specific filter_type_available options

[Jonathan Cameron]

On Tue, 7 Jan 2025 11:53:44 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 1/7/25 9:13 AM, Marcelo Schmitt wrote:
> > The only driver exporting filter_type_available attribute is ad7779 and it
> > only supports sinc3 and sinc5 filters. Remove options that are not
> > presented through filter_type_available attribute.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> > Maybe this is not worth it (or desirable?) since the options may come back in
> > the future if new drivers happen to need them.  
> 
> I would be tempted to add filter_type{,_available} attributes to the ad4130
> driver that mirrors the existing filter_mode{,_available} attributes. We can't
> remove filter_mode since that would break things, but having filter_type in
> addition to that would mean than userspace tools could standardize on
> filter_type and not have to make a special exception for the different naming
> on ad4130.
> 
> Then we wouldn't need to delete the extra items here and we wouldn't have to
> repeat the docs for sysfs-bus-iio-adc-ad4130. Those docs can just have the
> deprecation paragraph and mention that it returns values identical to the
> filter_type attributes for backwards compatibility.

Good idea.  Given we always allow any attribute to change any other, having
duplicates for ABI backwards compatibility is fine.

Thanks,

Jonathan

Re: [PATCH v2 0/3] Re-add filter_type/filter_mode

[David Lechner]

On 1/7/25 9:11 AM, Marcelo Schmitt wrote:
> Better document sysfs ABI for ADC digital filter configuration.
> 
> Change log v1 -> v2
> - Split into 3 patches.
> - Re-added sysfs-bus-iio-adc-ad4130 to keep filter_mode separate from filter_type.

If ad7779 is the only driver with filter_type{,available} attributes while
ad4130 and admv8818 both have filter_mode{,available} attributes, shouldn't we
be deprecating filter_type and promoting filter_mode?

(Looks like I might have got that backwards in my previous reply. :-/)

Re: [PATCH v2 0/3] Re-add filter_type/filter_mode

[Marcelo Schmitt]

On 01/07, David Lechner wrote:
> On 1/7/25 9:11 AM, Marcelo Schmitt wrote:
> > Better document sysfs ABI for ADC digital filter configuration.
> > 
> > Change log v1 -> v2
> > - Split into 3 patches.
> > - Re-added sysfs-bus-iio-adc-ad4130 to keep filter_mode separate from filter_type.
> 
> If ad7779 is the only driver with filter_type{,available} attributes while
> ad4130 and admv8818 both have filter_mode{,available} attributes, shouldn't we
> be deprecating filter_type and promoting filter_mode?

Oh, I had only grepped the adc dir and I didn't noticed admv8818 also had
filter_mode.
Hmm, don't know, though still biased towards filter_type.

- The admv8818 filter_mode_available ABI exports different values (auto,
  manual, bypass).
- AD7768-1, ADAQ7768-1, AD7768/AD7768-4, AD7124-4, and AD4170 datasheets
  refer to sinc filters as filter types (though not all of those designs are
  supported in IIO or have filter attributes).
- The AD4130 datasheets also refers to the digital filter as filter type in
  some occasions.

But some datasheets seem to use filter mode and filter type interchangeably
so I don't know. The admv8818 use case is valid and is both low and high-pass
filtering while the ADCs sinc digital filters act like low-pass filter.
Maybe even provide a different name for ADC digital filtering?
digital_filter_type (or digital_filter_mode)?.

Thanks for pointing that out.

Re: [PATCH v2 0/3] Re-add filter_type/filter_mode

[David Lechner]

On 1/7/25 11:44 AM, Marcelo Schmitt wrote:
> On 01/07, David Lechner wrote:
>> On 1/7/25 9:11 AM, Marcelo Schmitt wrote:
>>> Better document sysfs ABI for ADC digital filter configuration.
>>>
>>> Change log v1 -> v2
>>> - Split into 3 patches.
>>> - Re-added sysfs-bus-iio-adc-ad4130 to keep filter_mode separate from filter_type.
>>
>> If ad7779 is the only driver with filter_type{,available} attributes while
>> ad4130 and admv8818 both have filter_mode{,available} attributes, shouldn't we
>> be deprecating filter_type and promoting filter_mode?
> 
> Oh, I had only grepped the adc dir and I didn't noticed admv8818 also had
> filter_mode.
> Hmm, don't know, though still biased towards filter_type.
> 
> - The admv8818 filter_mode_available ABI exports different values (auto,
>   manual, bypass).

Ah, I didn't look that far, so yes, I agree this is a good reason to standardize
on filter_type instead.

> - AD7768-1, ADAQ7768-1, AD7768/AD7768-4, AD7124-4, and AD4170 datasheets
>   refer to sinc filters as filter types (though not all of those designs are
>   supported in IIO or have filter attributes).
> - The AD4130 datasheets also refers to the digital filter as filter type in
>   some occasions.
> 
> But some datasheets seem to use filter mode and filter type interchangeably
> so I don't know. The admv8818 use case is valid and is both low and high-pass
> filtering while the ADCs sinc digital filters act like low-pass filter.
> Maybe even provide a different name for ADC digital filtering?
> digital_filter_type (or digital_filter_mode)?.
> 
> Thanks for pointing that out.