diff mbox series

[1/1] Documentation: ABI: IIO: Re-add filter_type/filter_mode

Message ID b2132bd3ca1d64cdd8d5afab1f1f33c574718b50.1732901318.git.marcelo.schmitt@analog.com (mailing list archive)
State Changes Requested
Headers show
Series [1/1] Documentation: ABI: IIO: Re-add filter_type/filter_mode | expand

Commit Message

Marcelo Schmitt Dec. 2, 2024, 6:22 p.m. UTC 
The ad4130 driver exports in_voltageY-voltageZ_filter_mode and
in_voltage-voltage_filter_mode_available attributes to user space.
The ad7779 driver exports filter_type and filter_type_available.
Add (back again) documentation for filter_type/filter_mode attributes.

Fixes: 01bb12922b60 ("Documentation: ABI: added filter mode doc in sysfs-bus-iio")
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Digressing a bit away from the specific ABI used by ad4130 and ad7779,
the sinc3/4/5 filters are called `filter_mode` in ad4130 driver while other
drivers (ad7779, ad7124, ad7768-1) call sinc3/4/5 filters a `filter_type`.
Datasheets use the term `filter type`.

Depending on the particular ADC chip/design, the sinc3/4/5 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.

ad7768-1 sets sinc3/4/5 according to the sampling_frequency attribute. No
filter_type attribute.

ad7173 sets the filter_type according to sampling_frequency too, though it
looks like support for only one filter type is implemented.

ad7124 sets sinc3/4/5 filters according to a filter_low_pass_3db_frequency
attribute so it doesn't export filter type attributes to user space.
Missing `in_voltageY-voltageZ_filter_low_pass_3db_frequency` documentation?
follow up patch?

 Documentation/ABI/testing/sysfs-bus-iio | 14 ++++++++++++++
 1 file changed, 14 insertions(+)


base-commit: 9dd2270ca0b38ee16094817f4a53e7ba78e31567
prerequisite-patch-id: 3370db9ec1e67ba97b55607f445ff37c60929668
prerequisite-patch-id: d686dd309e1d3d39d038613f514e58ff5893ae42
prerequisite-patch-id: c832285d7bcc22433f2314a144566aa9437fd5da
prerequisite-patch-id: 3f758a121e36edd43789e80379ff81beeb2d75ce
prerequisite-patch-id: 0ef36ec4d6cf23f08bdb3bc4399ced2561a2a69b
prerequisite-patch-id: c8e7f0e10a2630bd0029ee160f8dfc3f742378ba
prerequisite-patch-id: 5e85d52a87f2a833893eeeac5d1651bda46d0931
prerequisite-patch-id: cd75aba06cf77f8cd398dc7d0c33d94e1277d1f3
prerequisite-patch-id: b813c25db823f1b02d0e9005188d41c0d89eb291
prerequisite-patch-id: 024ac23a16e45e802b70afe9bc464d1caeb41fcc
prerequisite-patch-id: bc084c859bfa93c5764e656bbcbfd4d14e031299
prerequisite-patch-id: 51ebb591fbbb3535899332ce1b106a3f8d6497da
prerequisite-patch-id: 2b396d1069227fee1c5a7bcf33bc63a56681441b
prerequisite-patch-id: c00b841cea6e331e19fb1f31beae831572bce4f5
prerequisite-patch-id: 4fe5fcebfeb745a83a7054390a304a1e250d74d1
prerequisite-patch-id: 461cce4f81f88bbec71580c0743b8970a504899c
prerequisite-patch-id: 59cf79cfa5f091815f578aad884ba0e3f9ae2175
prerequisite-patch-id: d48f6e531e64ee7797890e9f36f849f881884f1a
prerequisite-patch-id: 5f48c69023ecae6b3de595c9a209d1c4d65b5ba2
prerequisite-patch-id: 73e2fc3be282880231105142342b47b00b23ab6d
prerequisite-patch-id: d71deacf6bb4e90e8059a12a94ade36866729fa0
prerequisite-patch-id: 6173a25ddf92a3d1446923d9e87b15642b761034
prerequisite-patch-id: 5b248ee02cc148eeea4f01c435e701b74bc07c60
prerequisite-patch-id: 9fa4f11d62ba0e1ef9f3ca08ef1ee5c1f0711038
prerequisite-patch-id: 6cf99f094cfa8d984a1c1cab8813d1078ee48f05
prerequisite-patch-id: b42c4bf1ce430dccca920f942c6040f641c8307e

Comments

David Lechner Dec. 2, 2024, 11:05 p.m. UTC | #1 
On 12/2/24 12:22 PM, Marcelo Schmitt wrote:
> The ad4130 driver exports in_voltageY-voltageZ_filter_mode and
> in_voltage-voltage_filter_mode_available attributes to user space.
> The ad7779 driver exports filter_type and filter_type_available.
> Add (back again) documentation for filter_type/filter_mode attributes.
> 
> Fixes: 01bb12922b60 ("Documentation: ABI: added filter mode doc in sysfs-bus-iio")
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> Digressing a bit away from the specific ABI used by ad4130 and ad7779,
> the sinc3/4/5 filters are called `filter_mode` in ad4130 driver while other
> drivers (ad7779, ad7124, ad7768-1) call sinc3/4/5 filters a `filter_type`.
> Datasheets use the term `filter type`.
> 
> Depending on the particular ADC chip/design, the sinc3/4/5 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.
> 
> ad7768-1 sets sinc3/4/5 according to the sampling_frequency attribute. No
> filter_type attribute.
> 
> ad7173 sets the filter_type according to sampling_frequency too, though it
> looks like support for only one filter type is implemented.
> 
> ad7124 sets sinc3/4/5 filters according to a filter_low_pass_3db_frequency
> attribute so it doesn't export filter type attributes to user space.
> Missing `in_voltageY-voltageZ_filter_low_pass_3db_frequency` documentation?
> follow up patch?

cc: Guillaume and Uwe since they are working on these last two drivers
currently. Maybe something they could address?

> 
>  Documentation/ABI/testing/sysfs-bus-iio | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index f83bd6829285..704c9033cb5b 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -2268,6 +2268,20 @@ Description:
>  		An example format is 16-bytes, 2-digits-per-byte, HEX-string
>  		representing the sensor unique ID number.
>  
> +What:		/sys/bus/iio/devices/iio:deviceX/filter_type
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY-voltageZ_filter_mode
> +voltageY_filter_type_available
> +KernelVersion:	6.1
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Set the filter mode of the 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.

I think it can be safely assumed that changing any IIO attribute can
cause any other to change, so we probably don't need to mention the
sampling frequency interaction here, especially since it doesn't apply
to every possible user of these attributes.

Some other useful things to add instead:
* Mention that the values are the same as the ones listed in the
  "..._available" attribute docs.
* We should deprecate one of the names and recommend the other for
  future drivers to use. Since "type" is used more than once and
  "mode" only once, it seems natural to keep using "type" going
  forward.

> +
>  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
>
Marcelo Schmitt Dec. 3, 2024, 12:19 p.m. UTC | #2 
On 12/02, David Lechner wrote:
> On 12/2/24 12:22 PM, Marcelo Schmitt wrote:
> > The ad4130 driver exports in_voltageY-voltageZ_filter_mode and
> > in_voltage-voltage_filter_mode_available attributes to user space.
> > The ad7779 driver exports filter_type and filter_type_available.
> > Add (back again) documentation for filter_type/filter_mode attributes.
> > 
> > Fixes: 01bb12922b60 ("Documentation: ABI: added filter mode doc in sysfs-bus-iio")
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> > Digressing a bit away from the specific ABI used by ad4130 and ad7779,
> > the sinc3/4/5 filters are called `filter_mode` in ad4130 driver while other
> > drivers (ad7779, ad7124, ad7768-1) call sinc3/4/5 filters a `filter_type`.
> > Datasheets use the term `filter type`.
> > 
> > Depending on the particular ADC chip/design, the sinc3/4/5 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.
> > 
> > ad7768-1 sets sinc3/4/5 according to the sampling_frequency attribute. No
> > filter_type attribute.
> > 
> > ad7173 sets the filter_type according to sampling_frequency too, though it
> > looks like support for only one filter type is implemented.
> > 
> > ad7124 sets sinc3/4/5 filters according to a filter_low_pass_3db_frequency
> > attribute so it doesn't export filter type attributes to user space.
> > Missing `in_voltageY-voltageZ_filter_low_pass_3db_frequency` documentation?
> > follow up patch?
> 
> cc: Guillaume and Uwe since they are working on these last two drivers
> currently. Maybe something they could address?
> 
> > 
> >  Documentation/ABI/testing/sysfs-bus-iio | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index f83bd6829285..704c9033cb5b 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -2268,6 +2268,20 @@ Description:
> >  		An example format is 16-bytes, 2-digits-per-byte, HEX-string
> >  		representing the sensor unique ID number.
> >  
> > +What:		/sys/bus/iio/devices/iio:deviceX/filter_type
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY-voltageZ_filter_mode
> > +voltageY_filter_type_available
> > +KernelVersion:	6.1
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Set the filter mode of the 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.
> 
> I think it can be safely assumed that changing any IIO attribute can
> cause any other to change, so we probably don't need to mention the
> sampling frequency interaction here, especially since it doesn't apply
> to every possible user of these attributes.

Besides these filter attributes, the _offset attribute was allowed to change
after a change in _scale for a different driver so I'm also thinking IIO
attribute changes are allowed to cause updates to other device attributes.
The description above is roughly the same that was removed in 01bb12922b60.
Can think of something more accurate if that would be appreciated.

Jonathan, let me know if you prefer to re-add ABI doc as it was or if we
can re-add an updated version of it.

> 
> Some other useful things to add instead:
> * Mention that the values are the same as the ones listed in the
>   "..._available" attribute docs.
Sure, will do if going to update the ABI description.

> * We should deprecate one of the names and recommend the other for
>   future drivers to use. Since "type" is used more than once and
>   "mode" only once, it seems natural to keep using "type" going
>   forward.
Agree.

> 
> > +
> >  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
> >
Jonathan Cameron Dec. 8, 2024, 6:04 p.m. UTC | #3 
On Tue, 3 Dec 2024 09:19:30 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 12/02, David Lechner wrote:
> > On 12/2/24 12:22 PM, Marcelo Schmitt wrote:  
> > > The ad4130 driver exports in_voltageY-voltageZ_filter_mode and
> > > in_voltage-voltage_filter_mode_available attributes to user space.
> > > The ad7779 driver exports filter_type and filter_type_available.
> > > Add (back again) documentation for filter_type/filter_mode attributes.
> > > 
> > > Fixes: 01bb12922b60 ("Documentation: ABI: added filter mode doc in sysfs-bus-iio")
> > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > ---
> > > Digressing a bit away from the specific ABI used by ad4130 and ad7779,
> > > the sinc3/4/5 filters are called `filter_mode` in ad4130 driver while other
> > > drivers (ad7779, ad7124, ad7768-1) call sinc3/4/5 filters a `filter_type`.
> > > Datasheets use the term `filter type`.
> > > 
> > > Depending on the particular ADC chip/design, the sinc3/4/5 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.
> > > 
> > > ad7768-1 sets sinc3/4/5 according to the sampling_frequency attribute. No
> > > filter_type attribute.
> > > 
> > > ad7173 sets the filter_type according to sampling_frequency too, though it
> > > looks like support for only one filter type is implemented.
> > > 
> > > ad7124 sets sinc3/4/5 filters according to a filter_low_pass_3db_frequency
> > > attribute so it doesn't export filter type attributes to user space.
> > > Missing `in_voltageY-voltageZ_filter_low_pass_3db_frequency` documentation?
> > > follow up patch?  
> > 
> > cc: Guillaume and Uwe since they are working on these last two drivers
> > currently. Maybe something they could address?
> >   
> > > 
> > >  Documentation/ABI/testing/sysfs-bus-iio | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > > index f83bd6829285..704c9033cb5b 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > > @@ -2268,6 +2268,20 @@ Description:
> > >  		An example format is 16-bytes, 2-digits-per-byte, HEX-string
> > >  		representing the sensor unique ID number.
> > >  
> > > +What:		/sys/bus/iio/devices/iio:deviceX/filter_type
> > > +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY-voltageZ_filter_mode
> > > +voltageY_filter_type_available
> > > +KernelVersion:	6.1
> > > +Contact:	linux-iio@vger.kernel.org
> > > +Description:
> > > +		Set the filter mode of the 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.  
> > 
> > I think it can be safely assumed that changing any IIO attribute can
> > cause any other to change, so we probably don't need to mention the
> > sampling frequency interaction here, especially since it doesn't apply
> > to every possible user of these attributes.  
> 
> Besides these filter attributes, the _offset attribute was allowed to change
> after a change in _scale for a different driver so I'm also thinking IIO
> attribute changes are allowed to cause updates to other device attributes.

They are.  There is no practical way to design an ABI where that doesn't
happen unfortunately. We try to keep the changes as intuitive as possible
(so don't change unless we have to, find nearest value etc) where it is
reasonably easy to do.


> The description above is roughly the same that was removed in 01bb12922b60.
> Can think of something more accurate if that would be appreciated.
> 
> Jonathan, let me know if you prefer to re-add ABI doc as it was or if we
> can re-add an updated version of it.
I'm fine with updating in the same patch.

> 
> > 
> > Some other useful things to add instead:
> > * Mention that the values are the same as the ones listed in the
> >   "..._available" attribute docs.  
> Sure, will do if going to update the ABI description.
> 
> > * We should deprecate one of the names and recommend the other for
> >   future drivers to use. Since "type" is used more than once and
> >   "mode" only once, it seems natural to keep using "type" going
> >   forward.  
> Agree.

If mode is only used once, perhaps move it to a driver specific file?
Fine to cross reference to the main file from there.

Jonathan

> 
> >   
> > > +
> > >  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
> > >
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index f83bd6829285..704c9033cb5b 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2268,6 +2268,20 @@  Description:
 		An example format is 16-bytes, 2-digits-per-byte, HEX-string
 		representing the sensor unique ID number.
 
+What:		/sys/bus/iio/devices/iio:deviceX/filter_type
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY-voltageZ_filter_mode
+voltageY_filter_type_available
+KernelVersion:	6.1
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Set the filter mode of the 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.
+
 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