Re: [PATCH v6 7/7] iio: imu: st_lsm6dsx: Add support for rotation sensor

[David Lechner <dlechner@baylibre.com>]

On 2/25/26 4:18 AM, Francesco Lavra wrote:
> Some IMU chips in the LSM6DSX family have sensor fusion features that
> combine data from the accelerometer and gyroscope. One of these features
> generates rotation vector data and makes it available in the hardware
> FIFO as a quaternion (more specifically, the X, Y and Z components of the
> quaternion vector, expressed as 16-bit half-precision floating-point
> numbers).
> 
> Add support for a new sensor instance that allows receiving sensor fusion
> data, by defining a new struct st_lsm6dsx_sf_settings (which contains
> chip-specific details for the sensor fusion functionality), and adding this
> struct as a new field in struct st_lsm6dsx_settings. In st_lsm6dsx_core.c,
> populate this new struct for the LSM6DSV and LSM6DSV16X chips, and add the
> logic to initialize an additional IIO device if this struct is populated
> for the hardware type being probed.
> Note: a new IIO device is being defined (as opposed to adding channels to
> an existing device) because the rate at which sensor fusion data is
> generated may not match the data rate from any of the existing devices.
> 
> Tested on LSMDSV16X.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  Documentation/iio/index.rst                   |   1 +
>  Documentation/iio/st_lsm6dsx.rst              |  44 ++++
>  drivers/iio/imu/st_lsm6dsx/Makefile           |   2 +-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  26 +-
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |   9 +-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  |  58 +++++
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c    | 235 ++++++++++++++++++
>  7 files changed, 368 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/iio/st_lsm6dsx.rst
>  create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c

It makes things easier to review if we put the documentation changes in a
separate patch from the code changes.

> 
> diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
> index ba3e609c6a13..4e2ebe64f97f 100644
> --- a/Documentation/iio/index.rst
> +++ b/Documentation/iio/index.rst
> @@ -39,3 +39,4 @@ Industrial I/O Kernel Drivers
>     bno055
>     ep93xx_adc
>     opt4060
> +   st_lsm6dsx
> diff --git a/Documentation/iio/st_lsm6dsx.rst b/Documentation/iio/st_lsm6dsx.rst
> new file mode 100644
> index 000000000000..7f11a0b7e5c0
> --- /dev/null
> +++ b/Documentation/iio/st_lsm6dsx.rst
> @@ -0,0 +1,44 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +=================
> +ST LSM6DSX driver
> +=================
> +
> +Device driver for STMicroelectronics LSM6DSx, LSM9DS1, ASM330, and ISM330 series
> +of 6-axis inertial measurement units. The core module is called ``st_lsm6dsx``,
> +and the transport-specific modules are called ``st_lsm6dsx_i2c``,
> +``st_lsm6dsx_spi``, and ``st_lsm6dsx_i3c``.
> +
> +IIO devices
> +===========
> +
> +This driver instantiates multiple IIO devices:
> +
> +* accelerometer (IIO_ACCEL channel)
> +* gyroscope (IIO_ANGL_VEL channel)
> +* (optionally) magnetometer (IIO_MAGN channel), if the device has a secondary
> +  I2C interface connected to a slave sensor device (sensor hub functionality)
> +* (optionally) sensor fusion (IIO_CUSTOM channel), which combines acceleration
> +  and angular velocity data
> +
> +Sensor Fusion
> +-------------
> +
> +Some chips supported by this driver implement an internal algorithm that takes
> +input data from the accelerometer and gyroscope, and calculates the device
> +orientation in 3D space, which is then made available in the hardware FIFO.
> +Orientation is expressed in the form of a 4-dimensional quaternion vector, whose
> +components are typically represented in an array as ``[x, y, z, w]``.
> +The sensor device outputs the ``[x, y, z]`` components of the quaternion,
> +expressed as half-precision (16-bit) floating-point numbers.
> +
> +The ``z`` component is not output by the device, but its value can be derived

Should be ``w``.

> +from the rest of the data, due to the following constraints:
> +
> +* the quaternion vector is normalized, i.e. :math:`x^2 + y^2 + z^2 + w^2 = 1`
> +* the rotation angle :math:`\theta` remains within
> +  :math:`[-180^\circ, 180^\circ]`, i.e. the ``w`` component is non-negative:
> +  :math:`w = \cos(\theta/2) \geq 0`
> +
> +These constraints allow the ``w`` value to be calculated from the other
> +components: :math:`w = \sqrt{1 - (x^2 + y^2 + z^2)}`.


...

> +
>  /**
>   * struct st_lsm6dsx_ext_dev_settings - i2c controller slave settings
>   * @i2c_addr: I2c slave address list.
> @@ -388,6 +398,7 @@ struct st_lsm6dsx_settings {
>  	struct st_lsm6dsx_hw_ts_settings ts_settings;
>  	struct st_lsm6dsx_shub_settings shub_settings;
>  	struct st_lsm6dsx_event_settings event_settings;
> +	struct st_lsm6dsx_sf_settings sf_settings;

Can we spell out "fusion"? I don't think this is a common abbreviation.

>  };
>  

...

> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c
> new file mode 100644
> index 000000000000..7033aaeba13e
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c
> @@ -0,0 +1,235 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*

Copyright?

> + * STMicroelectronics st_lsm6dsx IMU sensor fusion
> + */
> +

...

> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(st_lsm6dsx_sf_sampling_freq_avail);
> +static struct attribute *st_lsm6dsx_sf_attributes[] = {
> +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> +	NULL
> +};

Can we not use the .read_avail callback for this instead of a manual
attribute?

> +
> +static const struct attribute_group st_lsm6dsx_sf_attribute_group = {
> +	.attrs = st_lsm6dsx_sf_attributes,
> +};
> +
> +static const struct iio_info st_lsm6dsx_sf_info = {
> +	.attrs = &st_lsm6dsx_sf_attribute_group,
> +	.read_raw = st_lsm6dsx_sf_read_raw,
> +	.write_raw = st_lsm6dsx_sf_write_raw,
> +	.hwfifo_set_watermark = st_lsm6dsx_set_watermark,
> +};