[PATCH v2 0/3] iio: Improce kernel docs

[PATCH v2 0/3] iio: Improce kernel docs

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 2013 bytes --]

IIO has very nice facilities for efficiently providing data from a
device to user (and probably also vice-versa - but I've not used that
direction). Getting started with IIO may not be so simple though - some
of the concepts like triggers and buffers are quite unique.

This series tries to make it easier for a newcomer to write his/her first
IIO driver by adding some documentation to used enums. Series does not
provide extensive documentation but just documents those few entries I
have become familiar with - but it still aims to be a starting point for
others to add missing bits and pieces.

This series is marked as v2 because the patch 1 was previously sent as a
stan-alone RFC to collect the missing channel units. RFC can be seen
here:
https://lore.kernel.org/all/10a855f9adc1d710150b7f647500c3c6a769f9ca.1677243698.git.mazziesaccount@gmail.com/

Patches 2 and 3 were added as a result of discussion followed by the
RFC.

Revision history:
RFCv1 => v2:
    - added patches 2 and 3
    - added missing channel type docs provided by Jonathan
    - added @in front of member names and fix typos pointed by Andy
    - dropped TODOs as Jonathan clarified the units

---

Matti Vaittinen (3):
  iio: Add some kerneldoc for channel types
  iio: add documentation for iio_chan_info_enum
  doc: Make sysfs-bus-iio doc more exact

 Documentation/ABI/testing/sysfs-bus-iio |  11 +-
 include/linux/iio/types.h               |  46 +++++++-
 include/uapi/linux/iio/types.h          | 134 ++++++++++++++++++++++++
 3 files changed, 185 insertions(+), 6 deletions(-)


base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
-- 
2.39.2


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 1/3] iio: Add some kerneldoc for channel types

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 6551 bytes --]

For occasional contributor like me navigating the IIO channel types and
modifiers may be a daunting task. One may have hard time finding out
what type of channel should be used for device data and what units the
data should be converted.

There is a great documentation for the sysfs interfaces though. What is
missing is mapping of the channel types and modifiers to the sysfs
documentation (and entries in documentation).

Give a hand to a driver writer by providing some documentation and by
pointing to the sysfs document from the kernel doc of respective enums.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Changelog RFCv1 => v2:
- add missing channel type docs provided by Jonathan
- add @in front of member names and fix typos pointed by Andy
- drop TODOs as Jonathan clarified the units

Initial discussion about these docs can be found from:
https://lore.kernel.org/all/0e0d45b7-e582-82b2-9bac-1f70f9dad9f7@gmail.com/
---
 include/uapi/linux/iio/types.h | 134 +++++++++++++++++++++++++++++++++
 1 file changed, 134 insertions(+)

diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index c79f2f046a0b..78f4cfdc5e45 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -11,6 +11,124 @@
 #ifndef _UAPI_IIO_TYPES_H_
 #define _UAPI_IIO_TYPES_H_
 
+/**
+ * iio_chan_type - Type of data transferred via IIO channel.
+ *
+ * The 'main' type of data transferred via channel. Please note that most
+ * devices also need to specify a more accurate 'sub category'. See the
+ * enum iio_modifier for this. (For example, IIO_ACCEL channel often needs to
+ * specify the direction. IIO_CONCENTRATION specifies the type of substance
+ * it measures etc).
+ *
+ * These reflect the units of the measurement via processed or unit after
+ * application of scale and offset. See the enum iio_chan_info_enum for
+ * scale and offset.
+ *
+ * Please find the detailed documentation for reported values from the
+ * Documentation/ABI/testing/sysfs-bus-iio.
+ *
+ * @IIO_ACCEL:		Acceleration, m/s^2
+ *			Doc keyword: in_accel_x_raw
+ *
+ * @IIO_ACTIVITY:	Activity state. For example a pedometer signaling
+ *			jogging, walking or staying still.
+ *			Doc keyword: in_activity_still_thresh_rising_en
+ *
+ * @IIO_ALTVOLTAGE:	Peak to peak voltage, millivolts
+ *
+ * @IIO_ANGL:		Angle of rotation, radians.
+ *			Doc keyword: in_angl_raw
+ *
+ * @IIO_ANGL_VEL:	Angular velocity, rad/s
+ *			Doc keyword: in_anglvel_x_raw
+ *
+ * @IIO_CAPACITANCE:	Capacitance, nanofarads.
+ *			Doc keyword: in_capacitanceY_raw
+ *
+ * @IIO_CCT:		Correlated color temperature, Kelvins
+ *
+ * @IIO_CURRENT:	Current, milliamps
+ *			Doc keyword: in_currentY_raw
+ *
+ * @IIO_CONCENTRATION:	Reading of a substance, percents. Used for example by
+ *			devices measuring amount of CO2, O2, ethanol...
+ *			Doc keyword: in_concentration_raw
+ *
+ * @IIO_COUNT:		Deprecated, please use counter subsystem.
+ *
+ * @IIO_DISTANCE:	Distance in meters. Typically used to report measured
+ *			distance to an object or the distance covered by the
+ *			user
+ *			Doc keyword: in_distance_input
+ *
+ * @IIO_ELECTRICALCONDUCTIVITY: electric conductivity, siemens per meter
+ *			Doc keyword: in_electricalconductivity_raw
+ *
+ * @IIO_ENERGY:		Energy in Joules. Typically reported by a device
+ *			measuring energy burnt by the user.
+ *			Doc keyword: in_energy_input
+ *
+ * @IIO_GRAVITY:	Gravity, m/s^2
+ *			Doc keyword: in_gravity_x_raw
+ *
+ * @IIO_HUMIDITYRELATIVE: Relative humidity, percents
+ *			Doc keyword: in_humidityrelative_raw
+ *
+ * @IIO_INCLI:		Inclination, degrees
+ *			Doc keyword: in_incli_x_raw
+ *
+ * @IIO_INDEX:		Deprecated, please use Counter subsystem
+ *
+ * @IIO_INTENSITY:	Unitless intensity.
+ *			Doc keyword: in_intensityY_raw
+ *
+ * @IIO_LIGHT:		Visible light intensity, lux
+ *			Doc keyword: in_illuminance_raw
+ *
+ * @IIO_MAGN:		Magnetic field, Gauss.
+ *			Doc keyword: in_magn_x_raw
+ *
+ * @IIO_MASSCONCENTRATION: Mass concentration, ug / m3
+ *			Doc keyword: in_massconcentration_pm1_input
+ *
+ * @IIO_PH:		pH reading, negative base-10 logarithm of hydrodium
+ *			ions in a litre of water
+ *			Doc keyword: in_ph_raw
+ *
+ * @IIO_PHASE:		Phase difference, radians
+ *			Doc keyword: in_phaseY_raw
+ *
+ * @IIO_POSITIONRELATIVE: Relative position.
+ *			Doc keyword: in_positionrelative_x_raw
+ *
+ * @IIO_POWER:		Power, milliwatts
+ *			Doc keyword: in_powerY_raw
+ *
+ * @IIO_PRESSURE:	Pressure, kilopascal
+ *			Doc keyword: in_pressureY_raw
+ *
+ * @IIO_RESISTANCE:	Resistance, ohms
+ *			Doc keyword: in_resistance_raw
+ *
+ * @IIO_ROT:		Euler angles, deg
+ *			Doc keyword: in_rot_yaw_raw
+ *
+ * @IIO_STEPS:		Steps taken by the user
+ *			Doc keyword: in_steps_input
+ *
+ * @IIO_TEMP:		Temperature, milli degrees Celsius
+ *			Doc keyword: in_temp_raw
+ *
+ * @IIO_UVINDEX:	UV light intensity index
+ *			Doc keyword: in_uvindex_input
+ *
+ * @IIO_VELOCITY:	Current speed (norm or magnitude of the velocity
+ *			vector), m/s
+ *			Doc keyword: in_velocity_sqrt(x^2+y^2+z^2)_input
+ *
+ * @IIO_VOLTAGE:	Voltage, millivolts
+ *			Doc keyword: in_voltageY_raw
+ */
 enum iio_chan_type {
 	IIO_VOLTAGE,
 	IIO_CURRENT,
@@ -49,6 +167,22 @@ enum iio_chan_type {
 	IIO_MASSCONCENTRATION,
 };
 
+/**
+ * iio_modifier - accurate class for channel data
+ *
+ * @IIO_MOD_<X,Y,Z>:	Value represents <X,Y,Z>-axis data.
+ *			Typically used by channels of type:
+ *			IIO_ACCEL, IIO_TEMP, IIO_GRAVITY, IIO_POSITIONRELATIVE,
+ *			IIO_ANGL_VEL, IIO_INCLI, IIO_MAGN
+ * @IIO_MOD_LIGHT_BOTH:	Value contains visible and infrared light components
+ * @IIO_MOD_LIGHT_IR:	Value represents infrared radiation
+ * @IIO_MOD_LIGHT_<RED, GREEN, BLUE>:
+ *			Value represents visible <red, green, blue>  light
+ * @IIO_MOD_LIGHT_CLEAR:	Value represents all visible light frequencies
+ *
+ * Please find the detailed documentation for reported values from the
+ * Documentation/ABI/testing/sysfs-bus-iio.
+ */
 enum iio_modifier {
 	IIO_NO_MOD,
 	IIO_MOD_X,
-- 
2.39.2


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/3] iio: Add some kerneldoc for channel types

[Jonathan Cameron]

On Sat, 25 Feb 2023 15:54:30 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> For occasional contributor like me navigating the IIO channel types and
> modifiers may be a daunting task. One may have hard time finding out
> what type of channel should be used for device data and what units the
> data should be converted.
> 
> There is a great documentation for the sysfs interfaces though. What is
> missing is mapping of the channel types and modifiers to the sysfs
> documentation (and entries in documentation).
> 
> Give a hand to a driver writer by providing some documentation and by
> pointing to the sysfs document from the kernel doc of respective enums.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

If following seems a little over 'picky' I blame the bit of my day
job that involves reviewing far to many specifications...

Some of what follows probably applies to the ABI docs as well.


> ---
> Changelog RFCv1 => v2:
> - add missing channel type docs provided by Jonathan
> - add @in front of member names and fix typos pointed by Andy
> - drop TODOs as Jonathan clarified the units
> 
> Initial discussion about these docs can be found from:
> https://lore.kernel.org/all/0e0d45b7-e582-82b2-9bac-1f70f9dad9f7@gmail.com/
> ---
>  include/uapi/linux/iio/types.h | 134 +++++++++++++++++++++++++++++++++
>  1 file changed, 134 insertions(+)
> 
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index c79f2f046a0b..78f4cfdc5e45 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -11,6 +11,124 @@
>  #ifndef _UAPI_IIO_TYPES_H_
>  #define _UAPI_IIO_TYPES_H_
>  
> +/**
> + * iio_chan_type - Type of data transferred via IIO channel.
I worry 'Type of data' could end up with people thinking '16 bit 2's complment'

Perhaps something more general would work?

- What is being measured / output by the channel.

> + *
> + * The 'main' type of data transferred via channel. Please note that most

type is fine here because it's more broken down than the short description.

most -> many

I can't be bothered to count actual devices and we have a lot of ADCs
and DACs that don't.  Just relaxing that to "many" avoids this being
or becoming inaccurate.

> + * devices also need to specify a more accurate 'sub category'. See the
> + * enum iio_modifier for this. (For example, IIO_ACCEL channel often needs to
> + * specify the direction. IIO_CONCENTRATION specifies the type of substance
> + * it measures etc).
> + *
> + * These reflect the units of the measurement via processed or unit after
> + * application of scale and offset. See the enum iio_chan_info_enum for
> + * scale and offset.
> + *
> + * Please find the detailed documentation for reported values from the

in Documentation/...

> + * Documentation/ABI/testing/sysfs-bus-iio.

Unusual to document an enum in a different order to the enum.
I'm in two minds on whether this is a good idea here.

> + *
> + * @IIO_ACCEL:		Acceleration, m/s^2
> + *			Doc keyword: in_accel_x_raw
> + *
> + * @IIO_ACTIVITY:	Activity state. For example a pedometer signaling
> + *			jogging, walking or staying still.
> + *			Doc keyword: in_activity_still_thresh_rising_en

Use in_activity_still_input
rather than the event control related to that here.

> + *
> + * @IIO_ALTVOLTAGE:	Peak to peak voltage, millivolts

Reference out_altvoltageY_raw

(the fun and complex game of DDS devices)

> + *
> + * @IIO_ANGL:		Angle of rotation, radians.
> + *			Doc keyword: in_angl_raw
> + *
> + * @IIO_ANGL_VEL:	Angular velocity, rad/s
> + *			Doc keyword: in_anglvel_x_raw
> + *
> + * @IIO_CAPACITANCE:	Capacitance, nanofarads.
> + *			Doc keyword: in_capacitanceY_raw
> + *
> + * @IIO_CCT:		Correlated color temperature, Kelvins
> + *
> + * @IIO_CURRENT:	Current, milliamps
> + *			Doc keyword: in_currentY_raw
> + *
> + * @IIO_CONCENTRATION:	Reading of a substance, percents. Used for example by

percentage

> + *			devices measuring amount of CO2, O2, ethanol...
concentration of CO2 ...

> + *			Doc keyword: in_concentration_raw
> + *
> + * @IIO_COUNT:		Deprecated, please use counter subsystem.
> + *
> + * @IIO_DISTANCE:	Distance in meters. Typically used to report measured
> + *			distance to an object or the distance covered by the
> + *			user
> + *			Doc keyword: in_distance_input
> + *
> + * @IIO_ELECTRICALCONDUCTIVITY: electric conductivity, siemens per meter
> + *			Doc keyword: in_electricalconductivity_raw
> + *
> + * @IIO_ENERGY:		Energy in Joules. Typically reported by a device
> + *			measuring energy burnt by the user.
> + *			Doc keyword: in_energy_input
> + *
> + * @IIO_GRAVITY:	Gravity, m/s^2
This needs more to differentiate from IIO_ACCEL.  Hmm. Something like
Acceleration due to gravity alone, independent of other accelerations due
to device movement.  (Which way is down!)

I'd forgotten this one even existed ;)

> + *			Doc keyword: in_gravity_x_raw
> + *
> + * @IIO_HUMIDITYRELATIVE: Relative humidity, percents
> + *			Doc keyword: in_humidityrelative_raw
> + *
> + * @IIO_INCLI:		Inclination, degrees
> + *			Doc keyword: in_incli_x_raw

(nothing to change here, but gah - an oddity in the ABI as all other angles
are radians)...

> + *
> + * @IIO_INDEX:		Deprecated, please use Counter subsystem
> + *
> + * @IIO_INTENSITY:	Unitless intensity.

This could do with an example.  "Used for measurements of light where
the exact meaning is dependent on the sensitivity curve."

> + *			Doc keyword: in_intensityY_raw
> + *
> + * @IIO_LIGHT:		Visible light intensity, lux

Not an intensity as depends on approx human eye sensitivity.
Raiding wikipedia - perhaps something like:

"Wave length weighted measure of light correlated with human
brightness perception." 

> + *			Doc keyword: in_illuminance_raw
> + *
> + * @IIO_MAGN:		Magnetic field, Gauss.
> + *			Doc keyword: in_magn_x_raw
> + *
> + * @IIO_MASSCONCENTRATION: Mass concentration, ug / m3
> + *			Doc keyword: in_massconcentration_pm1_input
> + *
> + * @IIO_PH:		pH reading, negative base-10 logarithm of hydrodium
> + *			ions in a litre of water
> + *			Doc keyword: in_ph_raw
> + *
> + * @IIO_PHASE:		Phase difference, radians
> + *			Doc keyword: in_phaseY_raw
> + *
> + * @IIO_POSITIONRELATIVE: Relative position.
Hohum. Another odd unit (however careful we are with this, some oddities
have made it in) Milipercent of 'pad' size.

> + *			Doc keyword: in_positionrelative_x_raw
> + *
> + * @IIO_POWER:		Power, milliwatts
> + *			Doc keyword: in_powerY_raw
> + *
> + * @IIO_PRESSURE:	Pressure, kilopascal
> + *			Doc keyword: in_pressureY_raw
> + *
> + * @IIO_RESISTANCE:	Resistance, ohms
> + *			Doc keyword: in_resistance_raw
> + *
> + * @IIO_ROT:		Euler angles, deg
> + *			Doc keyword: in_rot_yaw_raw
> + *
> + * @IIO_STEPS:		Steps taken by the user
> + *			Doc keyword: in_steps_input
> + *
> + * @IIO_TEMP:		Temperature, milli degrees Celsius
> + *			Doc keyword: in_temp_raw
> + *
> + * @IIO_UVINDEX:	UV light intensity index
> + *			Doc keyword: in_uvindex_input
> + *
> + * @IIO_VELOCITY:	Current speed (norm or magnitude of the velocity
> + *			vector), m/s
> + *			Doc keyword: in_velocity_sqrt(x^2+y^2+z^2)_input
> + *
> + * @IIO_VOLTAGE:	Voltage, millivolts
> + *			Doc keyword: in_voltageY_raw
> + */
>  enum iio_chan_type {
>  	IIO_VOLTAGE,
>  	IIO_CURRENT,
> @@ -49,6 +167,22 @@ enum iio_chan_type {
>  	IIO_MASSCONCENTRATION,
>  };
>  
> +/**
> + * iio_modifier - accurate class for channel data
accurate class -> sub class or direction for channel data
> + *
> + * @IIO_MOD_<X,Y,Z>:	Value represents <X,Y,Z>-axis data.
> + *			Typically used by channels of type:
> + *			IIO_ACCEL, IIO_TEMP, IIO_GRAVITY, IIO_POSITIONRELATIVE,
> + *			IIO_ANGL_VEL, IIO_INCLI, IIO_MAGN
> + * @IIO_MOD_LIGHT_BOTH:	Value contains visible and infrared light components
> + * @IIO_MOD_LIGHT_IR:	Value represents infrared radiation
> + * @IIO_MOD_LIGHT_<RED, GREEN, BLUE>:
> + *			Value represents visible <red, green, blue>  light
> + * @IIO_MOD_LIGHT_CLEAR:	Value represents all visible light frequencies

I guess a partial list is better than nothing here.  We can try and fill many
of the others in a future patch set.

> + *
> + * Please find the detailed documentation for reported values from the
> + * Documentation/ABI/testing/sysfs-bus-iio.
> + */
>  enum iio_modifier {
>  	IIO_NO_MOD,
>  	IIO_MOD_X,

[PATCH v2 2/3] iio: add documentation for iio_chan_info_enum

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 3466 bytes --]

Values in the iio_chan_info_enum are crucial for understanding the
characteristics of an IIO channel and the data delivered via IIO channel.
Give a hand to developers who do their first set of IIO drivers.

Add some documentation to these channel specifiers.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Please note that I did only add documentation for entries I am familiar
with. I did still add doc placeholders for all of the enum entries to
ease seeing which entries could still be documented. Hopefully this
encourages people to add missing pieces of documentation.
---
 include/linux/iio/types.h | 46 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 82faa98c719a..c8e3288ca24b 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -35,7 +35,51 @@ enum iio_available_type {
 	IIO_AVAIL_LIST,
 	IIO_AVAIL_RANGE,
 };
-
+/**
+ * enum iio_chan_info_enum - Information related to a IIO channel
+ *
+ * Many IIO channels have extra properties. Typically these properties can be
+ * read / written by user using the read_raw or write_raw callbacks in the
+ * struct iio_info.
+ *
+ * @IIO_CHAN_INFO_RAW:		Raw channel data as provided by device. Scale
+ *				and offset are often required to convert these
+ *				values to meaningful units.
+ * @IIO_CHAN_INFO_PROCESSED:	Processed data. Typically driver performs
+ *				computations to convert device data to more
+ *				meaningfull processed values.
+ * @IIO_CHAN_INFO_SCALE:	Scale to be applied to data in order to convert
+ *				it to units mandated by the channel type.
+ * @IIO_CHAN_INFO_OFFSET:	Offset to be applied to data in order to convert
+ *				it to units mandated by the channel type.
+ * @IIO_CHAN_INFO_CALIBSCALE:
+ * @IIO_CHAN_INFO_CALIBBIAS:
+ * @IIO_CHAN_INFO_PEAK:		Peak value (TODO: Since measurement start?)
+ * @IIO_CHAN_INFO_PEAK_SCALE:	Scale to be applied to the peak value in order
+ *				to convert it to units mandated by the channel
+ *				type.
+ * @IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW:
+ * @IIO_CHAN_INFO_AVERAGE_RAW:	Average of raw values (TODO: Since measurement
+ *				start or just for some undefined time?)
+ * @IIO_CHAN_INFO_SAMP_FREQ:	Sampling frequency for device.
+ * @IIO_CHAN_INFO_FREQUENCY:
+ * @IIO_CHAN_INFO_PHASE:
+ * @IIO_CHAN_INFO_HARDWAREGAIN:	Amplification applied by the hardware.
+ * @IIO_CHAN_INFO_HYSTERESIS:
+ * @IIO_CHAN_INFO_HYSTERESIS_RELATIVE:
+ * @IIO_CHAN_INFO_INT_TIME:	Integration time. Time during which the data is
+ *				accumulated by the device.
+ * @IIO_CHAN_INFO_ENABLE:
+ * @IIO_CHAN_INFO_CALIBHEIGHT:
+ * @IIO_CHAN_INFO_CALIBWEIGHT:
+ * @IIO_CHAN_INFO_DEBOUNCE_COUNT:
+ * @IIO_CHAN_INFO_DEBOUNCE_TIME:
+ * @IIO_CHAN_INFO_CALIBEMISSIVITY:
+ * @IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ * @IIO_CHAN_INFO_THERMOCOUPLE_TYPE:
+ * @IIO_CHAN_INFO_CALIBAMBIENT:
+ * @IIO_CHAN_INFO_ZEROPOINT:
+ */
 enum iio_chan_info_enum {
 	IIO_CHAN_INFO_RAW = 0,
 	IIO_CHAN_INFO_PROCESSED,
-- 
2.39.2


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/3] iio: add documentation for iio_chan_info_enum

[Jonathan Cameron]

On Sat, 25 Feb 2023 15:55:25 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Values in the iio_chan_info_enum are crucial for understanding the
> characteristics of an IIO channel and the data delivered via IIO channel.
> Give a hand to developers who do their first set of IIO drivers.
> 
> Add some documentation to these channel specifiers.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> Please note that I did only add documentation for entries I am familiar
> with. I did still add doc placeholders for all of the enum entries to
> ease seeing which entries could still be documented. Hopefully this
> encourages people to add missing pieces of documentation.

Good to hear the optimism :) 

I'll add it to my activities for boring journeys (with good internet
as probably need datasheets).  Note I'm reviewing this on a train
(having ignored it for a few weeks ;)

> ---
>  include/linux/iio/types.h | 46 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index 82faa98c719a..c8e3288ca24b 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -35,7 +35,51 @@ enum iio_available_type {
>  	IIO_AVAIL_LIST,
>  	IIO_AVAIL_RANGE,
>  };
> -
> +/**
> + * enum iio_chan_info_enum - Information related to a IIO channel
> + *
> + * Many IIO channels have extra properties. Typically these properties can be
"extra" glosses over the fact that some of these almost always exist.
E.g. raw.

IIO channels have a range of properties that may be read from userspace
(via sysfs attributes) or from other drivers using the in kernel IIO consumer
interfaces.  These properties are read / written using the read_raw...


> + * read / written by user using the read_raw or write_raw callbacks in the
> + * struct iio_info.
> + *
> + * @IIO_CHAN_INFO_RAW:		Raw channel data as provided by device. Scale
> + *				and offset are often required to convert these
> + *				values to meaningful units.

to base units as defined in the IIO ABI (link)

> + * @IIO_CHAN_INFO_PROCESSED:	Processed data. Typically driver performs
> + *				computations to convert device data to more
> + *				meaningfull processed values.

Typically a driver performs computations to convert device data to the 
base units defined in the IIO ABI (link)

> + * @IIO_CHAN_INFO_SCALE:	Scale to be applied to data in order to convert
> + *				it to units mandated by the channel type.
> + * @IIO_CHAN_INFO_OFFSET:	Offset to be applied to data in order to convert
> + *				it to units mandated by the channel type.

Add ordering info.  "Applied before scale."

> + * @IIO_CHAN_INFO_CALIBSCALE:
> + * @IIO_CHAN_INFO_CALIBBIAS:
> + * @IIO_CHAN_INFO_PEAK:		Peak value (TODO: Since measurement start?)

IIRC not that consistent. Some devices have it from device reset (so start), some
do it on a short time scale (thing of a voltage channel measuring a sine wave -
instantaneous reading is the current voltage, peak can be the peak in the cycle).
Others again do it on an 'event detection basis'. 
Sometimes constructive ambiguity can be handy in documentation ;)

> + * @IIO_CHAN_INFO_PEAK_SCALE:	Scale to be applied to the peak value in order
> + *				to convert it to units mandated by the channel
> + *				type.
> + * @IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW:
> + * @IIO_CHAN_INFO_AVERAGE_RAW:	Average of raw values (TODO: Since measurement
> + *				start or just for some undefined time?)

Again, not that tightly defined (IIRC).  Average of raw values over a device specific time period.

> + * @IIO_CHAN_INFO_SAMP_FREQ:	Sampling frequency for device.
> + * @IIO_CHAN_INFO_FREQUENCY:
> + * @IIO_CHAN_INFO_PHASE:
> + * @IIO_CHAN_INFO_HARDWAREGAIN:	Amplification applied by the hardware.
Given how often this is done wrong I'd love to call out something like:
"SCALE should be used for control if the HARDWAREGAIN directly affects the
 channel RAW measurement". Examples of HARDWAREGAIN include amplification of
 the light signal in a time of flight sensor."


> + * @IIO_CHAN_INFO_HYSTERESIS:
> + * @IIO_CHAN_INFO_HYSTERESIS_RELATIVE:
> + * @IIO_CHAN_INFO_INT_TIME:	Integration time. Time during which the data is
> + *				accumulated by the device.

Unit? (seconds I think).

> + * @IIO_CHAN_INFO_ENABLE:
> + * @IIO_CHAN_INFO_CALIBHEIGHT:
> + * @IIO_CHAN_INFO_CALIBWEIGHT:
> + * @IIO_CHAN_INFO_DEBOUNCE_COUNT:
> + * @IIO_CHAN_INFO_DEBOUNCE_TIME:
> + * @IIO_CHAN_INFO_CALIBEMISSIVITY:
> + * @IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + * @IIO_CHAN_INFO_THERMOCOUPLE_TYPE:
> + * @IIO_CHAN_INFO_CALIBAMBIENT:
> + * @IIO_CHAN_INFO_ZEROPOINT:
> + */
>  enum iio_chan_info_enum {
>  	IIO_CHAN_INFO_RAW = 0,
>  	IIO_CHAN_INFO_PROCESSED,

Re: [PATCH v2 2/3] iio: add documentation for iio_chan_info_enum

[Vaittinen, Matti]

On 4/8/23 13:30, Jonathan Cameron wrote:
> On Sat, 25 Feb 2023 15:55:25 +0200
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> Values in the iio_chan_info_enum are crucial for understanding the
>> characteristics of an IIO channel and the data delivered via IIO channel.
>> Give a hand to developers who do their first set of IIO drivers.
>>
>> Add some documentation to these channel specifiers.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> ---
>> Please note that I did only add documentation for entries I am familiar
>> with. I did still add doc placeholders for all of the enum entries to
>> ease seeing which entries could still be documented. Hopefully this
>> encourages people to add missing pieces of documentation.
> 
> Good to hear the optimism :)
> 
> I'll add it to my activities for boring journeys (with good internet
> as probably need datasheets).  Note I'm reviewing this on a train
> (having ignored it for a few weeks ;)
> 
>> ---
>>   include/linux/iio/types.h | 46 ++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
>> index 82faa98c719a..c8e3288ca24b 100644
>> --- a/include/linux/iio/types.h
>> +++ b/include/linux/iio/types.h
>> @@ -35,7 +35,51 @@ enum iio_available_type {
>>   	IIO_AVAIL_LIST,
>>   	IIO_AVAIL_RANGE,
>>   };
>> -
>> +/**
>> + * enum iio_chan_info_enum - Information related to a IIO channel
>> + *
>> + * Many IIO channels have extra properties. Typically these properties can be
> "extra" glosses over the fact that some of these almost always exist.
> E.g. raw.
> 
> IIO channels have a range of properties that may be read from userspace
> (via sysfs attributes) or from other drivers using the in kernel IIO consumer
> interfaces.  These properties are read / written using the read_raw...
> 
> 
>> + * read / written by user using the read_raw or write_raw callbacks in the
>> + * struct iio_info.
>> + *
>> + * @IIO_CHAN_INFO_RAW:		Raw channel data as provided by device. Scale
>> + *				and offset are often required to convert these
>> + *				values to meaningful units.
> 
> to base units as defined in the IIO ABI (link)

This is just great. I like adding the link to ABI doc here! Thanks!

>> + * @IIO_CHAN_INFO_HARDWAREGAIN:	Amplification applied by the hardware.
> Given how often this is done wrong I'd love to call out something like:
> "SCALE should be used for control if the HARDWAREGAIN directly affects the
>   channel RAW measurement". Examples of HARDWAREGAIN include amplification of
>   the light signal in a time of flight sensor."

So, let's try to tell people that HARDWAREGAIN is typically not the 
thing they are interested in :) I think this is exactly the way these 
docs should help both the driver authors as well as the poor sod 
reviewing all the driver patches ;)

>> + * @IIO_CHAN_INFO_HYSTERESIS:
>> + * @IIO_CHAN_INFO_HYSTERESIS_RELATIVE:
>> + * @IIO_CHAN_INFO_INT_TIME:	Integration time. Time during which the data is
>> + *				accumulated by the device.
> 
> Unit? (seconds I think).

Really...? Can you please double check this?

I believe the BU27034 uses micro seconds. I thought that was correct 
approach - but if it is not then we can probably still change it w/o 
breaking userland...


Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Re: [PATCH v2 2/3] iio: add documentation for iio_chan_info_enum

[Jonathan Cameron]

> >> + * @IIO_CHAN_INFO_HYSTERESIS:
> >> + * @IIO_CHAN_INFO_HYSTERESIS_RELATIVE:
> >> + * @IIO_CHAN_INFO_INT_TIME:	Integration time. Time during which the data is
> >> + *				accumulated by the device.  
> > 
> > Unit? (seconds I think).  
> 
> Really...? Can you please double check this?
> 
> I believe the BU27034 uses micro seconds. I thought that was correct 
> approach - but if it is not then we can probably still change it w/o 
> breaking userland...

It's seconds in the ABI docs.  Thankfully time to fix the
driver.  No huge rush as long as we do it in time to slip in before
release.

In the early days of IIO we got most of our scaling from hwmon.
It became clear after a while that those weren't appropriate because
they tended to not have enough precision.  Hence things shifted (with
a few exceptions) to the SI unit without mili etc. I think the time
related ones all came after that change of approach.

Thanks,

Jonathan


> 
> 
> Yours,
> 	-- Matti
> 

Re: [PATCH v2 2/3] iio: add documentation for iio_chan_info_enum

[Matti Vaittinen]

On 4/8/23 13:30, Jonathan Cameron wrote:
> On Sat, 25 Feb 2023 15:55:25 +0200
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> + * @IIO_CHAN_INFO_INT_TIME:	Integration time. Time during which the data is
>> + *				accumulated by the device.
> 
> Unit? (seconds I think).

Holy moly. Thanks for bringing this up now. I just checked this and the 
API doc indeed says clearly and loud the unit is in seconds. This means 
the BU27034 driver as well as the gain-time-scale helper does this 
wrong. I hope you can postpone sending them upstream until this gets 
fixed. I'll try to cook incremental patch on top of the iio/togreg - but 
I am not sure if I can do it today as I need to run after an hour... 
Sorry and thanks!

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Re: [PATCH v2 2/3] iio: add documentation for iio_chan_info_enum

[Jonathan Cameron]

On Wed, 12 Apr 2023 11:52:05 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 4/8/23 13:30, Jonathan Cameron wrote:
> > On Sat, 25 Feb 2023 15:55:25 +0200
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >   
> >> + * @IIO_CHAN_INFO_INT_TIME:	Integration time. Time during which the data is
> >> + *				accumulated by the device.  
> > 
> > Unit? (seconds I think).  
> 
> Holy moly. Thanks for bringing this up now. I just checked this and the 
> API doc indeed says clearly and loud the unit is in seconds. This means 
> the BU27034 driver as well as the gain-time-scale helper does this 
> wrong. I hope you can postpone sending them upstream until this gets 
> fixed. I'll try to cook incremental patch on top of the iio/togreg - but 
> I am not sure if I can do it today as I need to run after an hour... 
> Sorry and thanks!

It's a fix so we have time.  This one we'd take even if the driver
had made it to a release, but then we'd manage to annoy people.

Jonathan

> 
> Yours,
> 	-- Matti
> 

[PATCH v2 3/3] doc: Make sysfs-bus-iio doc more exact

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 2711 bytes --]

A few IIC channel descriptions explained used units as:
data is in foo "that can be processed into an" [unit] value. The "can be
processed into" is quite broad statement as it does not really explain
what this processing means. This makes units pretty much useless.

After discussion with Jonathan, it seems the units for these channels
should also be well-defined as for all other channels. The processing
means the standard scale and offset application that is used throughout
the IIO. Let's make it more obvious by stating that the units are [unit]
after scale ane offset are applied.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 6ba34c0d9789..b435c6f065ae 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1807,8 +1807,8 @@ What:		/sys/bus/iio/devices/iio:deviceX/out_resistanceX_raw
 KernelVersion:	4.3
 Contact:	linux-iio@vger.kernel.org
 Description:
-		Raw (unscaled no offset etc.) resistance reading that can be processed
-		into an ohm value.
+		Raw (unscaled no offset etc.) resistance reading.
+		Units after application of scale and offset are ohms.
 
 What:		/sys/bus/iio/devices/iio:deviceX/heater_enable
 KernelVersion:	4.1.0
@@ -1894,8 +1894,9 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_electricalconductivity_raw
 KernelVersion:	4.8
 Contact:	linux-iio@vger.kernel.org
 Description:
-		Raw (unscaled no offset etc.) electric conductivity reading that
-		can be processed to siemens per meter.
+		Raw (unscaled no offset etc.) electric conductivity reading.
+		Units after application of scale and offset are siemens per
+		meter.
 
 What:		/sys/bus/iio/devices/iio:deviceX/in_countY_raw
 KernelVersion:	4.10
@@ -1952,7 +1953,7 @@ KernelVersion:	4.18
 Contact:	linux-iio@vger.kernel.org
 Description:
 		Raw (unscaled) phase difference reading from channel Y
-		that can be processed to radians.
+		Units after application of scale and offset are radians.
 
 What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm1_input
 What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm1_input
-- 
2.39.2


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 3/3] doc: Make sysfs-bus-iio doc more exact

[Jonathan Cameron]

On Sat, 25 Feb 2023 15:56:16 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> A few IIC channel descriptions explained used units as:
IIO?
> data is in foo "that can be processed into an" [unit] value. The "can be
> processed into" is quite broad statement as it does not really explain
> what this processing means. This makes units pretty much useless.
> 
> After discussion with Jonathan, it seems the units for these channels
> should also be well-defined as for all other channels. The processing
> means the standard scale and offset application that is used throughout
> the IIO. Let's make it more obvious by stating that the units are [unit]
> after scale ane offset are applied.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Excellent. Thanks for doing this. I'll pick this patch up now.

Applied to the togreg branch of iio.git and pushed out as testing
(in this case to be mostly ignored by 0-day ;)

Jonathan

> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 6ba34c0d9789..b435c6f065ae 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1807,8 +1807,8 @@ What:		/sys/bus/iio/devices/iio:deviceX/out_resistanceX_raw
>  KernelVersion:	4.3
>  Contact:	linux-iio@vger.kernel.org
>  Description:
> -		Raw (unscaled no offset etc.) resistance reading that can be processed
> -		into an ohm value.
> +		Raw (unscaled no offset etc.) resistance reading.
> +		Units after application of scale and offset are ohms.
>  
>  What:		/sys/bus/iio/devices/iio:deviceX/heater_enable
>  KernelVersion:	4.1.0
> @@ -1894,8 +1894,9 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_electricalconductivity_raw
>  KernelVersion:	4.8
>  Contact:	linux-iio@vger.kernel.org
>  Description:
> -		Raw (unscaled no offset etc.) electric conductivity reading that
> -		can be processed to siemens per meter.
> +		Raw (unscaled no offset etc.) electric conductivity reading.
> +		Units after application of scale and offset are siemens per
> +		meter.
>  
>  What:		/sys/bus/iio/devices/iio:deviceX/in_countY_raw
>  KernelVersion:	4.10
> @@ -1952,7 +1953,7 @@ KernelVersion:	4.18
>  Contact:	linux-iio@vger.kernel.org
>  Description:
>  		Raw (unscaled) phase difference reading from channel Y
> -		that can be processed to radians.
> +		Units after application of scale and offset are radians.
>  
>  What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm1_input
>  What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm1_input

Re: [PATCH v2 3/3] doc: Make sysfs-bus-iio doc more exact

[Andy Shevchenko]

On Sat, Apr 8, 2023 at 1:18 PM Jonathan Cameron <jic23@kernel.org> wrote:
 On Sat, 25 Feb 2023 15:56:16 +0200
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:

...

> Applied to the togreg branch of iio.git and pushed out as testing
> (in this case to be mostly ignored by 0-day ;)

One fix is needed?

...

> >  Description:
> >               Raw (unscaled) phase difference reading from channel Y
> > -             that can be processed to radians.
> > +             Units after application of scale and offset are radians.

The previous chunks and confusion in reading the above suggest that
period after Y must be added.

> >  What:                /sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm1_input
> >  What:                /sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm1_input

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/3] doc: Make sysfs-bus-iio doc more exact

[Jonathan Cameron]

On Sun, 9 Apr 2023 12:47:58 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Apr 8, 2023 at 1:18 PM Jonathan Cameron <jic23@kernel.org> wrote:
>  On Sat, 25 Feb 2023 15:56:16 +0200
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:  
> 
> ...
> 
> > Applied to the togreg branch of iio.git and pushed out as testing
> > (in this case to be mostly ignored by 0-day ;)  
> 
> One fix is needed?
> 
> ...
> 
> > >  Description:
> > >               Raw (unscaled) phase difference reading from channel Y
> > > -             that can be processed to radians.
> > > +             Units after application of scale and offset are radians.  
> 
> The previous chunks and confusion in reading the above suggest that
> period after Y must be added.
Fixed.  Thanks

> 
> > >  What:                /sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm1_input
> > >  What:                /sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm1_input  
> 

Re: [PATCH v2 0/3] iio: Improce kernel docs

[Jonathan Cameron]

On Sat, 25 Feb 2023 15:53:22 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> IIO has very nice facilities for efficiently providing data from a
> device to user (and probably also vice-versa - but I've not used that
> direction). Getting started with IIO may not be so simple though - some
> of the concepts like triggers and buffers are quite unique.
> 
> This series tries to make it easier for a newcomer to write his/her first
> IIO driver by adding some documentation to used enums. Series does not
> provide extensive documentation but just documents those few entries I
> have become familiar with - but it still aims to be a starting point for
> others to add missing bits and pieces.
> 
> This series is marked as v2 because the patch 1 was previously sent as a
> stan-alone RFC to collect the missing channel units. RFC can be seen
> here:
> https://lore.kernel.org/all/10a855f9adc1d710150b7f647500c3c6a769f9ca.1677243698.git.mazziesaccount@gmail.com/

Not sure I'll get to these today, so just a quick: typo in cover letter typo
in case I forget.  Improve


> 
> Patches 2 and 3 were added as a result of discussion followed by the
> RFC.
> 
> Revision history:
> RFCv1 => v2:
>     - added patches 2 and 3
>     - added missing channel type docs provided by Jonathan
>     - added @in front of member names and fix typos pointed by Andy
>     - dropped TODOs as Jonathan clarified the units
> 
> ---
> 
> Matti Vaittinen (3):
>   iio: Add some kerneldoc for channel types
>   iio: add documentation for iio_chan_info_enum
>   doc: Make sysfs-bus-iio doc more exact
> 
>  Documentation/ABI/testing/sysfs-bus-iio |  11 +-
>  include/linux/iio/types.h               |  46 +++++++-
>  include/uapi/linux/iio/types.h          | 134 ++++++++++++++++++++++++
>  3 files changed, 185 insertions(+), 6 deletions(-)
> 
> 
> base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c