2024-02-13 08:19:41

by Gradinariu, Ramona

[permalink] [raw]
Subject: [PATCH v4 2/3] docs: iio: add documentation for device buffers

Add documentation for IIO device buffers describing buffer
attributes and how data is structured in buffers using
scan elements.

Signed-off-by: Ramona Gradinariu <[email protected]>
---
changes in v4:
- documented multiple buffer support
- reworked scan elements section
- added reference to ABI docs
Documentation/iio/iio_devbuf.rst | 125 +++++++++++++++++++++++++++++++
Documentation/iio/index.rst | 1 +
2 files changed, 126 insertions(+)
create mode 100644 Documentation/iio/iio_devbuf.rst

diff --git a/Documentation/iio/iio_devbuf.rst b/Documentation/iio/iio_devbuf.rst
new file mode 100644
index 000000000000..e99143efb4d7
--- /dev/null
+++ b/Documentation/iio/iio_devbuf.rst
@@ -0,0 +1,125 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============================
+Industrial IIO device buffers
+=============================
+
+1. Overview
+===========
+
+The Industrial I/O core offers a way for continuous data capture based on a
+trigger source. Multiple data channels can be read at once from
+/dev/iio:deviceX character device node, thus reducing the CPU load.
+
+Devices with buffer support feature an additional sub-folder in the
+/sys/bus/iio/devices/deviceX/ folder hierarchy, called bufferY, where Y defaults
+to 0, for devices with a single buffer.
+
+2. Buffer attributes
+====================
+
+An IIO buffer has an associated attributes directory under
+/sys/bus/iio/iio:deviceX/bufferY/. The attributes are described below.
+
+Length
+------
+
+Read / Write attribute which states the total number of data samples (capacity)
+that can be stored by the buffer.
+
+Enable
+------
+
+Read / Write attribute which starts / stops the buffer capture. This file should
+be written last, after length and selection of scan elements.
+
+Watermark
+---------
+
+Read / Write positive integer attribute specifying the maximum number of scan
+elements to wait for.
+
+Poll will block until the watermark is reached.
+
+Blocking read will wait until the minimum between the requested read amount or
+the low water mark is available.
+
+Non-blocking read will retrieve the available samples from the buffer even if
+there are less samples then watermark level. This allows the application to
+block on poll with a timeout and read the available samples after the timeout
+expires and thus have a maximum delay guarantee.
+
+Data available
+--------------
+
+Read-only attribute indicating the bytes of data available in the buffer. In the
+case of an output buffer, this indicates the amount of empty space available to
+write data to. In the case of an input buffer, this indicates the amount of data
+available for reading.
+
+Scan elements
+-------------
+
+The meta information associated with a channel reading placed in a buffer is
+called a scan element. The scan elements are configurable per buffer, thus they
+are exposed to userspace applications via the /sys/bus/iio/iio:deviceX/bufferY/
+directory. The scan elements attributes are presented below.
+
+**_en**
+
+Read/ Write attribute used for enabling a channel. If and only if its value
+is non zero, then a triggered capture will contain data samples for this
+channel.
+
+**_index**
+
+Read-only positive integer attribute specifying the position of the channel in
+the buffer. Note these are not dependent on what is enabled and may not be
+contiguous. Thus for user-space to establish the full layout these must be used
+in conjunction with all _en attributes to establish which channels are present,
+and the relevant _type attributes to establish the data storage format.
+
+**_type**
+
+Read-only attribute containing the description of the scan element data storage
+within the buffer and hence the form in which it is read from user space. Format
+is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
+
+- **be** or **le** specifies big or little endian.
+- **s** or **u**, specifies if signed (2's complement) or unsigned.
+- **bits**, is the number of valid data bits.
+- **storagebits**, is the number of bits (after padding) that it occupies in the
+ buffer.
+- **repeat**, specifies the number of bits/storagebits repetitions. When the
+ repeat element is 0 or 1, then the repeat value is omitted.
+- **shift**, if specified, is the shift that needs to be applied prior to
+ masking out unused bits.
+
+For example, a driver for a 3-axis accelerometer with 12 bit resolution where
+data is stored in two 8-bits registers as follows:
+
+.. code-block:: bash
+
+ 7 6 5 4 3 2 1 0
+ +---+---+---+---+---+---+---+---+
+ |D3 |D2 |D1 |D0 | X | X | X | X | (LOW byte, address 0x06)
+ +---+---+---+---+---+---+---+---+
+
+ 7 6 5 4 3 2 1 0
+ +---+---+---+---+---+---+---+---+
+ |D11|D10|D9 |D8 |D7 |D6 |D5 |D4 | (HIGH byte, address 0x07)
+ +---+---+---+---+---+---+---+---+
+
+will have the following scan element type for each axis:
+
+.. code-block:: bash
+
+ $ cat /sys/bus/iio/devices/iio:device0/buffer0/in_accel_y_type
+ le:s12/16>>4
+
+A user space application will interpret data samples read from the buffer as two
+byte little endian signed data, that needs a 4 bits right shift before masking
+out the 12 valid bits of data.
+
+Please see Documentation/ABI/testing/sysfs-bus-iio for a complete description of
+the attributes.
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index db341b45397f..206a0aff5ca1 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -8,6 +8,7 @@ Industrial I/O
:maxdepth: 1

iio_configfs
+ iio_devbuf

Industrial I/O Kernel Drivers
=============================
--
2.34.1



2024-02-14 00:01:24

by David Lechner

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] docs: iio: add documentation for device buffers

On Tue, Feb 13, 2024 at 2:19 AM Ramona Gradinariu
<[email protected]> wrote:
>
> Add documentation for IIO device buffers describing buffer
> attributes and how data is structured in buffers using
> scan elements.

Really nice to see this being added to the docs.

>
> Signed-off-by: Ramona Gradinariu <[email protected]>
> ---
> changes in v4:
> - documented multiple buffer support
> - reworked scan elements section
> - added reference to ABI docs
> Documentation/iio/iio_devbuf.rst | 125 +++++++++++++++++++++++++++++++
> Documentation/iio/index.rst | 1 +
> 2 files changed, 126 insertions(+)
> create mode 100644 Documentation/iio/iio_devbuf.rst
>
> diff --git a/Documentation/iio/iio_devbuf.rst b/Documentation/iio/iio_devbuf.rst
> new file mode 100644
> index 000000000000..e99143efb4d7
> --- /dev/null
> +++ b/Documentation/iio/iio_devbuf.rst
> @@ -0,0 +1,125 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=============================
> +Industrial IIO device buffers
> +=============================
> +
> +1. Overview
> +===========
> +
> +The Industrial I/O core offers a way for continuous data capture based on a
> +trigger source. Multiple data channels can be read at once from
> +/dev/iio:deviceX character device node, thus reducing the CPU load.

It could be nice to use inline code format style (double-backtick,
e.g. ``/dev/iio:deviceX``) on paths like this throughout the document.

> +
> +Devices with buffer support feature an additional sub-folder in the
> +/sys/bus/iio/devices/deviceX/ folder hierarchy, called bufferY, where Y defaults

Should this be `iio:deviceX` instead of `deviceX` to match the sysfs docs?

> +to 0, for devices with a single buffer.

Is /sys/bus/iio/devices/deviceX/buffer (without the Y) for backwards
compatibility?

> +
> +2. Buffer attributes
> +====================
> +
> +An IIO buffer has an associated attributes directory under
> +/sys/bus/iio/iio:deviceX/bufferY/. The attributes are described below.
> +
> +Length
> +------

Could be nice to give the actual attribute name ``length`` here to
avoid possible case-sensitivity confusion with the section header.
Same applies to other attributes.

> +
> +Read / Write attribute which states the total number of data samples (capacity)
> +that can be stored by the buffer.
> +
> +Enable
> +------
> +
> +Read / Write attribute which starts / stops the buffer capture. This file should
> +be written last, after length and selection of scan elements.

Could be useful here to mention that writing a non-zero value here to
enable the buffer may result in an error, such as EINVAL, e.g. if an
invalid configuration was selected, like choosing a combination of
scan elements that don't match one of the valid scan masks.

> +
> +Watermark
> +---------
> +
> +Read / Write positive integer attribute specifying the maximum number of scan
> +elements to wait for.
> +
> +Poll will block until the watermark is reached.
> +
> +Blocking read will wait until the minimum between the requested read amount or
> +the low water mark is available.
> +
> +Non-blocking read will retrieve the available samples from the buffer even if
> +there are less samples then watermark level. This allows the application to
> +block on poll with a timeout and read the available samples after the timeout
> +expires and thus have a maximum delay guarantee.
> +
> +Data available
> +--------------
> +
> +Read-only attribute indicating the bytes of data available in the buffer In the
> +case of an output buffer, this indicates the amount of empty space available to
> +write data to. In the case of an input buffer, this indicates the amount of data
> +available for reading.
> +
> +Scan elements
> +-------------
> +
> +The meta information associated with a channel reading placed in a buffer is

Maybe say "data" instead of "reading" here since it could be writing
instead for DACs.

> +called a scan element. The scan elements are configurable per buffer, thus they
> +are exposed to userspace applications via the /sys/bus/iio/iio:deviceX/bufferY/

Giving the directory again seems redundant here.

> +directory. The scan elements attributes are presented below.
> +
> +**_en**
> +
> +Read/ Write attribute used for enabling a channel. If and only if its value
> +is non zero, then a triggered capture will contain data samples for this
> +channel.
> +
> +**_index**
> +
> +Read-only positive integer attribute specifying the position of the channel in

Isn't 0 a valid scan index? So non-negative? Or unsigned?

> +the buffer. Note these are not dependent on what is enabled and may not be
> +contiguous. Thus for user-space to establish the full layout these must be used
> +in conjunction with all _en attributes to establish which channels are present,
> +and the relevant _type attributes to establish the data storage format.
> +

It would also be nice to get an example on the binary layout for
something that has multiple channels enabled. In particular with the
data alignment, e.g. when you have a 16-bit word followed by a 64-bit
word.


> +**_type**
> +
> +Read-only attribute containing the description of the scan element data storage
> +within the buffer and hence the form in which it is read from user space Format
> +is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
> +
> +- **be** or **le** specifies big or little endian.
> +- **s** or **u**, specifies if signed (2's complement) or unsigned.
> +- **bits**, is the number of valid data bits.
> +- **storagebits**, is the number of bits (after padding) that it occupies in the
> + buffer.
> +- **repeat**, specifies the number of bits/storagebits repetitions. When the
> + repeat element is 0 or 1, then the repeat value is omitted.
> +- **shift**, if specified, is the shift that needs to be applied prior to
> + masking out unused bits.
> +
> +For example, a driver for a 3-axis accelerometer with 12 bit resolution where
> +data is stored in two 8-bits registers as follows:
> +
> +.. code-block:: bash

Doesn't look like this should use "bash" styling.

> +
> + 7 6 5 4 3 2 1 0
> + +---+---+---+---+---+---+---+---+
> + |D3 |D2 |D1 |D0 | X | X | X | X | (LOW byte, address 0x06)
> + +---+---+---+---+---+---+---+---+
> +
> + 7 6 5 4 3 2 1 0
> + +---+---+---+---+---+---+---+---+
> + |D11|D10|D9 |D8 |D7 |D6 |D5 |D4 | (HIGH byte, address 0x07)
> + +---+---+---+---+---+---+---+---+
> +
> +will have the following scan element type for each axis:
> +
> +.. code-block:: bash
> +
> + $ cat /sys/bus/iio/devices/iio:device0/buffer0/in_accel_y_type
> + le:s12/16>>4
> +
> +A user space application will interpret data samples read from the buffer as two
> +byte little endian signed data, that needs a 4 bits right shift before masking
> +out the 12 valid bits of data.

Is it always assumed that scan data is `raw` and needs to be
multiplied by `scale` for that channel to convert it to SI (or IIO
standard) units?

> +
> +Please see Documentation/ABI/testing/sysfs-bus-iio for a complete description of
> +the attributes.

Is it also worth mentioning
``Documentation/ABI/testing/sysfs-bus-iio-dma-buffer`` here?

> diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
> index db341b45397f..206a0aff5ca1 100644
> --- a/Documentation/iio/index.rst
> +++ b/Documentation/iio/index.rst
> @@ -8,6 +8,7 @@ Industrial I/O
> :maxdepth: 1
>
> iio_configfs
> + iio_devbuf
>
> Industrial I/O Kernel Drivers
> =============================
> --
> 2.34.1
>
>

2024-02-14 01:13:27

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] docs: iio: add documentation for device buffers



On 2/13/24 00:17, Ramona Gradinariu wrote:
> Add documentation for IIO device buffers describing buffer
> attributes and how data is structured in buffers using
> scan elements.
>
> Signed-off-by: Ramona Gradinariu <[email protected]>
> ---
> changes in v4:
> - documented multiple buffer support
> - reworked scan elements section
> - added reference to ABI docs
> Documentation/iio/iio_devbuf.rst | 125 +++++++++++++++++++++++++++++++
> Documentation/iio/index.rst | 1 +
> 2 files changed, 126 insertions(+)
> create mode 100644 Documentation/iio/iio_devbuf.rst
>
> diff --git a/Documentation/iio/iio_devbuf.rst b/Documentation/iio/iio_devbuf.rst
> new file mode 100644
> index 000000000000..e99143efb4d7
> --- /dev/null
> +++ b/Documentation/iio/iio_devbuf.rst
> @@ -0,0 +1,125 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=============================
> +Industrial IIO device buffers
> +=============================
> +
> +1. Overview
> +===========
> +
> +The Industrial I/O core offers a way for continuous data capture based on a
> +trigger source. Multiple data channels can be read at once from
> +/dev/iio:deviceX character device node, thus reducing the CPU load.
> +
> +Devices with buffer support feature an additional sub-folder in the

folder or directory?

> +/sys/bus/iio/devices/deviceX/ folder hierarchy, called bufferY, where Y defaults

folder or directory?

> +to 0, for devices with a single buffer.
> +
> +2. Buffer attributes
> +====================
> +
> +An IIO buffer has an associated attributes directory under

directory or folder?

Just be consistent, please.

> +/sys/bus/iio/iio:deviceX/bufferY/. The attributes are described below.
> +

What are the corresponding attribute names?

> +Length
> +------
> +
> +Read / Write attribute which states the total number of data samples (capacity)
> +that can be stored by the buffer.
> +
> +Enable
> +------
> +
> +Read / Write attribute which starts / stops the buffer capture. This file should
> +be written last, after length and selection of scan elements.
> +
> +Watermark
> +---------
> +
> +Read / Write positive integer attribute specifying the maximum number of scan
> +elements to wait for.
> +
> +Poll will block until the watermark is reached.
> +
> +Blocking read will wait until the minimum between the requested read amount or
> +the low water mark is available.

watermark
> +
> +Non-blocking read will retrieve the available samples from the buffer even if
> +there are less samples then watermark level. This allows the application to

than the

> +block on poll with a timeout and read the available samples after the timeout
> +expires and thus have a maximum delay guarantee.
> +
> +Data available
> +--------------
> +
> +Read-only attribute indicating the bytes of data available in the buffer. In the
> +case of an output buffer, this indicates the amount of empty space available to
> +write data to. In the case of an input buffer, this indicates the amount of data
> +available for reading.
> +
> +Scan elements
> +-------------
> +
> +The meta information associated with a channel reading placed in a buffer is

That line gives me -ENOPARSE. Can it be improved?

> +called a scan element. The scan elements are configurable per buffer, thus they
> +are exposed to userspace applications via the /sys/bus/iio/iio:deviceX/bufferY/
> +directory. The scan elements attributes are presented below.
> +
> +**_en**
> +
> +Read/ Write attribute used for enabling a channel. If and only if its value
> +is non zero, then a triggered capture will contain data samples for this

non-zero,

> +channel.
> +
> +**_index**
> +
> +Read-only positive integer attribute specifying the position of the channel in
> +the buffer. Note these are not dependent on what is enabled and may not be
> +contiguous. Thus for user-space to establish the full layout these must be used

userspace
as above.

> +in conjunction with all _en attributes to establish which channels are present,
> +and the relevant _type attributes to establish the data storage format.
> +
> +**_type**
> +
> +Read-only attribute containing the description of the scan element data storage
> +within the buffer and hence the form in which it is read from user space. Format
> +is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
> +
> +- **be** or **le** specifies big or little endian.
> +- **s** or **u**, specifies if signed (2's complement) or unsigned.

no comma ^

> +- **bits**, is the number of valid data bits.

no comma ^

> +- **storagebits**, is the number of bits (after padding) that it occupies in the

no comma ^

> + buffer.
> +- **repeat**, specifies the number of bits/storagebits repetitions. When the

no comma ^

> + repeat element is 0 or 1, then the repeat value is omitted.
> +- **shift**, if specified, is the shift that needs to be applied prior to

no comma ^

> + masking out unused bits.
> +
> +For example, a driver for a 3-axis accelerometer with 12 bit resolution where

12-bit

> +data is stored in two 8-bits registers as follows:

8-bit is as follows:

> +
> +.. code-block:: bash
> +
> + 7 6 5 4 3 2 1 0
> + +---+---+---+---+---+---+---+---+
> + |D3 |D2 |D1 |D0 | X | X | X | X | (LOW byte, address 0x06)
> + +---+---+---+---+---+---+---+---+
> +
> + 7 6 5 4 3 2 1 0
> + +---+---+---+---+---+---+---+---+
> + |D11|D10|D9 |D8 |D7 |D6 |D5 |D4 | (HIGH byte, address 0x07)
> + +---+---+---+---+---+---+---+---+
> +
> +will have the following scan element type for each axis:
> +
> +.. code-block:: bash
> +
> + $ cat /sys/bus/iio/devices/iio:device0/buffer0/in_accel_y_type
> + le:s12/16>>4
> +
> +A user space application will interpret data samples read from the buffer as two

userspace
for consistency.
as two-

> +byte little endian signed data, that needs a 4 bits right shift before masking

little-endian

> +out the 12 valid bits of data.
> +
> +Please see Documentation/ABI/testing/sysfs-bus-iio for a complete description of
> +the attributes.
> diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
> index db341b45397f..206a0aff5ca1 100644
> --- a/Documentation/iio/index.rst
> +++ b/Documentation/iio/index.rst
> @@ -8,6 +8,7 @@ Industrial I/O
> :maxdepth: 1
>
> iio_configfs
> + iio_devbuf
>
> Industrial I/O Kernel Drivers
> =============================
> --
> 2.34.1
>
>

--
#Randy

2024-02-16 11:56:04

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] docs: iio: add documentation for device buffers


A few follow up comments on your David's review
Anything I deleted didn't need a comment as all made sense to me!

>
> > +to 0, for devices with a single buffer.
>
> Is /sys/bus/iio/devices/deviceX/buffer (without the Y) for backwards
> compatibility?

Yes. For these docs I'd not mention it. New software should be aware of multiple
buffers being possible and not use it. Same is true of the scan_elements directory.
If we really want to mention it, say buffer/ and scan_elements are for backwards
compatibility and should not be used in new userspace software.

They aren't going anywhere, but better people start from a multibuffer world!

>
> > +
> > +Read / Write attribute which states the total number of data samples (capacity)
> > +that can be stored by the buffer.
> > +
> > +Enable
> > +------
> > +
> > +Read / Write attribute which starts / stops the buffer capture. This file should
> > +be written last, after length and selection of scan elements.
>
> Could be useful here to mention that writing a non-zero value here to
> enable the buffer may result in an error, such as EINVAL, e.g. if an
> invalid configuration was selected, like choosing a combination of
> scan elements that don't match one of the valid scan masks.

Be careful to not refer to matching. Could be a subset. I'd refer to
"an unsupported combination of channels" or something like that.


> > +directory. The scan elements attributes are presented below.
> > +
> > +**_en**
> > +
> > +Read/ Write attribute used for enabling a channel. If and only if its value
> > +is non zero, then a triggered capture will contain data samples for this
> > +channel.
> > +
> > +**_index**
> > +
> > +Read-only positive integer attribute specifying the position of the channel in
>
> Isn't 0 a valid scan index? So non-negative? Or unsigned?

Yes - unsigned would be my preference.

>
> > +the buffer. Note these are not dependent on what is enabled and may not be
> > +contiguous. Thus for user-space to establish the full layout these must be used
> > +in conjunction with all _en attributes to establish which channels are present,
> > +and the relevant _type attributes to establish the data storage format.
> > +
>
> It would also be nice to get an example on the binary layout for
> something that has multiple channels enabled. In particular with the
> data alignment, e.g. when you have a 16-bit word followed by a 64-bit
> word.
>

Agreed - the padding is sometimes not what people expect.

>
> > +**_type**
> > +
> > +Read-only attribute containing the description of the scan element data storage
> > +within the buffer and hence the form in which it is read from user space. Format
> > +is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
> > +
> > +- **be** or **le** specifies big or little endian.
> > +- **s** or **u**, specifies if signed (2's complement) or unsigned.
> > +- **bits**, is the number of valid data bits.
> > +- **storagebits**, is the number of bits (after padding) that it occupies in the
> > + buffer.
> > +- **repeat**, specifies the number of bits/storagebits repetitions. When the
> > + repeat element is 0 or 1, then the repeat value is omitted.
> > +- **shift**, if specified, is the shift that needs to be applied prior to
> > + masking out unused bits.
> > +
> > +For example, a driver for a 3-axis accelerometer with 12 bit resolution where
> > +data is stored in two 8-bits registers as follows:
> > +
> > +.. code-block:: bash
>
> Doesn't look like this should use "bash" styling.
>
> > +
> > + 7 6 5 4 3 2 1 0
> > + +---+---+---+---+---+---+---+---+
> > + |D3 |D2 |D1 |D0 | X | X | X | X | (LOW byte, address 0x06)
> > + +---+---+---+---+---+---+---+---+
> > +
> > + 7 6 5 4 3 2 1 0
> > + +---+---+---+---+---+---+---+---+
> > + |D11|D10|D9 |D8 |D7 |D6 |D5 |D4 | (HIGH byte, address 0x07)
> > + +---+---+---+---+---+---+---+---+
> > +
> > +will have the following scan element type for each axis:
> > +
> > +.. code-block:: bash
> > +
> > + $ cat /sys/bus/iio/devices/iio:device0/buffer0/in_accel_y_type
> > + le:s12/16>>4
> > +
> > +A user space application will interpret data samples read from the buffer as two
> > +byte little endian signed data, that needs a 4 bits right shift before masking
> > +out the 12 valid bits of data.
>
> Is it always assumed that scan data is `raw` and needs to be
> multiplied by `scale` for that channel to convert it to SI (or IIO
> standard) units?

Definitely by far the most common case but there are a few exceptions where
there isn't a _raw attribute but only an _input one where the assumption is
processed data. Tricky to mention that here without adding complexity.
Maybe just add some weasel words to hint there are corners not covered by
this doc.

>
> > +
> > +Please see Documentation/ABI/testing/sysfs-bus-iio for a complete description of
> > +the attributes.
>
> Is it also worth mentioning
> ``Documentation/ABI/testing/sysfs-bus-iio-dma-buffer`` here?

I'd not do that until we have a section for these docs on dma buffers which
are different in a bunch of ways. Would just be a potential source of
confusion.

Jonathan