[PATCH v5 0/4] imu: st_lsm6dsx: Add support for rotation sensor

[PATCH v5 0/4] imu: st_lsm6dsx: Add support for rotation sensor

[Francesco Lavra]

This series adds support for the rotation sensor functionality present in
some chips from the ST LSM6DSX IMU family.
The second commit is a fix to a previous commit of mine [*] which made it
to 6.19-rcX; the fix technically changes userspace, but this should be OK
as long as it goes in during this release cycle.
Tested on LSM6DSV16X.

Changes from v4 [4]:
- changed data parameter in st_lsm6dsx_push_tagged_data() to __le16 *
  (Andy)

Changes from v3 [3]:
- added patch 3 (Andy)
- removed unneeded checks for negative return values in st_lsm6dsx_fusion.c
  (Andy)
- replaced st_lsm6dsx_sf_set_page function with
  st_lsm6dsx_sf_page_enable/disable (Andy)
- used reversed xmas tree ordering for local variables (Andy)
- added parentheses to MILLI / MICRO in st_lsm6dsx_sf_write_raw (Andy)
- added check for string truncation in st_lsm6dsx_sf_probe (Andy)

Changes from v2 [2]:
- amended description of patch 2 to point out that there are no supported
  gyro events (Jonathan)
- removed superfluous parentheses in st_lsm6dsx_fifo_setup (Lorenzo)
- added Lorenzo's acked-by tag to patch 3
- added missing checks of st_lsm6dsx_sf_set_page() return value (Jonathan)
- added comment in st_lsm6dsx_sf_write_raw (Jonathan)

Changes from v1 [1]:
- swapped patches 1 and 2 (Jonathan)
- miscellaneous stylistic changes (Andy)
- fixed usage of MICRO and MILLI constants in st_lsm6dsx_sf_read_raw and
  st_lsm6dsx_sf_write_raw (Andy)
- replaced scnprintf() with sysfs_emit_at() in
  st_lsm6dsx_sf_sampling_freq_avail (Andy)
- replaced scnprintf() with snprintf() in st_lsm6dsx_sf_probe (Andy)
- clarified in a comment in st_lsm6dsx_set_fifo_odr() that only internal
  sensors have a FIFO ODR configuration register (Jonathan)
- modified patch 3 description to explain justification for the extra IIO
  device (Jonathan)
- moved page lock from st_lsm6dsx_sf_set_page() to the callers (Jonathan)
- s/magnetometer/gyroscope/ in patch 2 description

[*] https://lore.kernel.org/linux-iio/20251017164255.1251060-3-flavra@baylibre.com/
[1] https://lore.kernel.org/linux-iio/20260109181528.154127-1-flavra@baylibre.com/T/
[2] https://lore.kernel.org/linux-iio/20260115122431.1014630-1-flavra@baylibre.com/T/
[3] https://lore.kernel.org/linux-iio/20260119100449.1559624-1-flavra@baylibre.com/T/
[4] https://lore.kernel.org/linux-iio/20260121112758.1831077-1-flavra@baylibre.com/T/

Francesco Lavra (4):
  iio: imu: st_lsm6dsx: Set FIFO ODR for accelerometer and gyroscope
    only
  iio: imu: st_lsm6dsx: Set buffer sampling frequency for accelerometer
    only
  iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO
  iio: imu: st_lsm6dsx: Add support for rotation sensor

 drivers/iio/imu/st_lsm6dsx/Makefile           |   2 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  26 +-
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  38 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  |  58 +++++
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c    | 235 ++++++++++++++++++
 5 files changed, 348 insertions(+), 11 deletions(-)
 create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c

-- 
2.39.5

[PATCH v5 1/4] iio: imu: st_lsm6dsx: Set FIFO ODR for accelerometer and gyroscope only

[Francesco Lavra]

The st_lsm6dsx_set_fifo_odr() function, which is called when enabling and
disabling the hardware FIFO, checks the contents of the hw->settings->batch
array at index sensor->id, and then sets the current ODR value in sensor
registers that depend on whether the register address is set in the above
array element. This logic is valid for internal sensors only, i.e. the
accelerometer and gyroscope; however, since commit c91c1c844ebd ("iio: imu:
st_lsm6dsx: add i2c embedded controller support"), this function is called
also when configuring the hardware FIFO for external sensors (i.e. sensors
accessed through the sensor hub functionality), which can result in
unrelated device registers being written.

Add a check to the beginning of st_lsm6dsx_set_fifo_odr() so that it does
not touch any registers unless it is called for internal sensors.

Fixes: c91c1c844ebd ("iio: imu: st_lsm6dsx: add i2c embedded controller support")
Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 55d877745575..1ee2fc5f5f1f 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -225,6 +225,10 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
 	const struct st_lsm6dsx_reg *batch_reg;
 	u8 data;
 
+	/* Only internal sensors have a FIFO ODR configuration register. */
+	if (sensor->id >= ARRAY_SIZE(hw->settings->batch))
+		return 0;
+
 	batch_reg = &hw->settings->batch[sensor->id];
 	if (batch_reg->addr) {
 		int val;
-- 
2.39.5

[PATCH v5 2/4] iio: imu: st_lsm6dsx: Set buffer sampling frequency for accelerometer only

[Francesco Lavra]

The st_lsm6dsx_hwfifo_odr_store() function, which is called when userspace
writes the buffer sampling frequency sysfs attribute, calls
st_lsm6dsx_check_odr(), which accesses the odr_table array at index
`sensor->id`; since this array is only 2 entries long, an access for any
sensor type other than accelerometer or gyroscope is an out-of-bounds
access.

The motivation for being able to set a buffer frequency different from the
sensor sampling frequency is to support use cases that need accurate event
detection (which requires a high sampling frequency) while retrieving
sensor data at low frequency. Since all the supported event types are
generated from acceleration data only, do not create the buffer sampling
frequency attribute for sensor types other than the accelerometer.

Fixes: 6b648a36c200 ("iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate")
Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 1ee2fc5f5f1f..5b28a3ffcc3d 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -862,12 +862,21 @@ int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
 	int i, ret;
 
 	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
+		const struct iio_dev_attr **attrs;
+
 		if (!hw->iio_devs[i])
 			continue;
 
+		/*
+		 * For the accelerometer, allow setting FIFO sampling frequency
+		 * values different from the sensor sampling frequency, which
+		 * may be needed to keep FIFO data rate low while sampling
+		 * acceleration data at high rates for accurate event detection.
+		 */
+		attrs = i == ST_LSM6DSX_ID_ACC ? st_lsm6dsx_buffer_attrs : NULL;
 		ret = devm_iio_kfifo_buffer_setup_ext(hw->dev, hw->iio_devs[i],
 						      &st_lsm6dsx_buffer_ops,
-						      st_lsm6dsx_buffer_attrs);
+						      attrs);
 		if (ret)
 			return ret;
 	}
-- 
2.39.5

[PATCH v5 3/4] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO

[Francesco Lavra]

The DRDY_MASK feature implemented in sensor chips marks gyroscope and
accelerometer invalid samples (i.e. samples that have been acquired during
the settling time of sensor filters) with the special values 0x7FFFh,
0x7FFE, and 0x7FFD.
The driver checks FIFO samples against these special values in order to
discard invalid samples; however, it does the check regardless of the type
of samples being processed, whereas this feature is specific to gyroscope
and accelerometer data. This could cause valid samples to be discarded.

Fix the above check so that it takes into account the type of samples being
processed. In st_lsm6dsx_push_tagged_data(), change the type of the data
parameter to __le16 *, to reflect the fact that this function is called
with an aligned data argument and avoid casting to __le16 * when checking
sample values.

Fixes: 960506ed2c69 ("iio: imu: st_lsm6dsx: enable drdy-mask if available")
Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 5b28a3ffcc3d..ded9a96076e6 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -539,14 +539,14 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
 #define ST_LSM6DSX_INVALID_SAMPLE	0x7ffd
 static int
 st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
-			    u8 *data, s64 ts)
+			    __le16 *data, s64 ts)
 {
-	s16 val = le16_to_cpu(*(__le16 *)data);
 	struct st_lsm6dsx_sensor *sensor;
 	struct iio_dev *iio_dev;
 
 	/* invalid sample during bootstrap phase */
-	if (val >= ST_LSM6DSX_INVALID_SAMPLE)
+	if ((tag == ST_LSM6DSX_GYRO_TAG || tag == ST_LSM6DSX_ACC_TAG) &&
+	    (s16)le16_to_cpup(data) >= ST_LSM6DSX_INVALID_SAMPLE)
 		return -EINVAL;
 
 	/*
@@ -670,7 +670,8 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
 					reset_ts = true;
 				ts *= hw->ts_gain;
 			} else {
-				st_lsm6dsx_push_tagged_data(hw, tag, iio_buff,
+				st_lsm6dsx_push_tagged_data(hw, tag,
+							    (__le16 *)iio_buff,
 							    ts);
 			}
 		}
-- 
2.39.5

[PATCH v5 4/4] iio: imu: st_lsm6dsx: Add support for rotation sensor

[Francesco Lavra]

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>
---
 drivers/iio/imu/st_lsm6dsx/Makefile           |   2 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  26 +-
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  16 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  |  58 +++++
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c    | 235 ++++++++++++++++++
 5 files changed, 330 insertions(+), 7 deletions(-)
 create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c

diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
index 57cbcd67d64f..19a488254de3 100644
--- a/drivers/iio/imu/st_lsm6dsx/Makefile
+++ b/drivers/iio/imu/st_lsm6dsx/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
-		st_lsm6dsx_shub.o
+		st_lsm6dsx_shub.o st_lsm6dsx_fusion.o
 
 obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
 obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 07b1773c87bd..4173f670f7af 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -294,6 +294,7 @@ enum st_lsm6dsx_sensor_id {
 	ST_LSM6DSX_ID_EXT0,
 	ST_LSM6DSX_ID_EXT1,
 	ST_LSM6DSX_ID_EXT2,
+	ST_LSM6DSX_ID_SF,
 	ST_LSM6DSX_ID_MAX
 };
 
@@ -301,6 +302,15 @@ enum st_lsm6dsx_ext_sensor_id {
 	ST_LSM6DSX_ID_MAGN,
 };
 
+struct st_lsm6dsx_sf_settings {
+	const struct iio_chan_spec *chan;
+	int chan_len;
+	struct st_lsm6dsx_odr_table_entry odr_table;
+	struct st_lsm6dsx_reg fifo_enable;
+	struct st_lsm6dsx_reg page_mux;
+	struct st_lsm6dsx_reg enable;
+};
+
 /**
  * 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;
 };
 
 enum st_lsm6dsx_fifo_mode {
@@ -510,6 +521,9 @@ int st_lsm6dsx_check_odr(struct st_lsm6dsx_sensor *sensor, u32 odr, u8 *val);
 int st_lsm6dsx_shub_probe(struct st_lsm6dsx_hw *hw, const char *name);
 int st_lsm6dsx_shub_set_enable(struct st_lsm6dsx_sensor *sensor, bool enable);
 int st_lsm6dsx_shub_read_output(struct st_lsm6dsx_hw *hw, u8 *data, int len);
+int st_lsm6dsx_sf_probe(struct st_lsm6dsx_hw *hw, const char *name);
+int st_lsm6dsx_sf_set_enable(struct st_lsm6dsx_sensor *sensor, bool enable);
+int st_lsm6dsx_sf_set_odr(struct st_lsm6dsx_sensor *sensor, bool enable);
 int st_lsm6dsx_set_page(struct st_lsm6dsx_hw *hw, bool enable);
 
 static inline int
@@ -564,12 +578,14 @@ st_lsm6dsx_get_mount_matrix(const struct iio_dev *iio_dev,
 static inline int
 st_lsm6dsx_device_set_enable(struct st_lsm6dsx_sensor *sensor, bool enable)
 {
-	if (sensor->id == ST_LSM6DSX_ID_EXT0 ||
-	    sensor->id == ST_LSM6DSX_ID_EXT1 ||
-	    sensor->id == ST_LSM6DSX_ID_EXT2)
+	switch (sensor->id) {
+	case ST_LSM6DSX_ID_EXT0 ... ST_LSM6DSX_ID_EXT2:
 		return st_lsm6dsx_shub_set_enable(sensor, enable);
-
-	return st_lsm6dsx_sensor_set_enable(sensor, enable);
+	case ST_LSM6DSX_ID_SF:
+		return st_lsm6dsx_sf_set_enable(sensor, enable);
+	default:
+		return st_lsm6dsx_sensor_set_enable(sensor, enable);
+	}
 }
 
 static const
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index ded9a96076e6..3b4fa57bf461 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -88,6 +88,7 @@ enum st_lsm6dsx_fifo_tag {
 	ST_LSM6DSX_EXT0_TAG = 0x0f,
 	ST_LSM6DSX_EXT1_TAG = 0x10,
 	ST_LSM6DSX_EXT2_TAG = 0x11,
+	ST_LSM6DSX_ROT_TAG = 0x13,
 };
 
 static const
@@ -226,8 +227,11 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
 	u8 data;
 
 	/* Only internal sensors have a FIFO ODR configuration register. */
-	if (sensor->id >= ARRAY_SIZE(hw->settings->batch))
+	if (sensor->id >= ARRAY_SIZE(hw->settings->batch)) {
+		if (sensor->id == ST_LSM6DSX_ID_SF)
+			return st_lsm6dsx_sf_set_odr(sensor, enable);
 		return 0;
+	}
 
 	batch_reg = &hw->settings->batch[sensor->id];
 	if (batch_reg->addr) {
@@ -580,6 +584,16 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
 	case ST_LSM6DSX_EXT2_TAG:
 		iio_dev = hw->iio_devs[ST_LSM6DSX_ID_EXT2];
 		break;
+	case ST_LSM6DSX_ROT_TAG:
+		/*
+		 * The sensor reports only the {X, Y, Z} elements of the
+		 * quaternion vector; set the W value to 0 (it can be derived
+		 * from the {X, Y, Z} values due to the property that the vector
+		 * is normalized).
+		 */
+		data[ST_LSM6DSX_SAMPLE_SIZE / ST_LSM6DSX_CHAN_SIZE] = 0;
+		iio_dev = hw->iio_devs[ST_LSM6DSX_ID_SF];
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index dc0ae0e488ce..c21163a06a71 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -94,6 +94,24 @@
 
 #define ST_LSM6DSX_REG_WHOAMI_ADDR		0x0f
 
+/* Raw values from the IMU are 16-bit half-precision floating-point numbers. */
+#define ST_LSM6DSX_CHANNEL_ROT						\
+{									\
+	.type = IIO_ROT,						\
+	.modified = 1,							\
+	.channel2 = IIO_MOD_QUATERNION,					\
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.scan_index = 0,						\
+	.scan_type = {							\
+		.sign = 'u',						\
+		.realbits = 16,						\
+		.storagebits = 16,					\
+		.endianness = IIO_LE,					\
+		.repeat = 4,						\
+	},								\
+	.ext_info = st_lsm6dsx_ext_info,				\
+}
+
 static const struct iio_event_spec st_lsm6dsx_ev_motion[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -153,6 +171,11 @@ static const struct iio_chan_spec st_lsm6ds0_gyro_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
+static const struct iio_chan_spec st_lsm6dsx_sf_channels[] = {
+	ST_LSM6DSX_CHANNEL_ROT,
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
 static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 	{
 		.reset = {
@@ -1492,6 +1515,35 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				},
 			},
 		},
+		.sf_settings = {
+			.chan = st_lsm6dsx_sf_channels,
+			.chan_len = ARRAY_SIZE(st_lsm6dsx_sf_channels),
+			.odr_table = {
+				.reg = {
+					.addr = 0x5e,
+					.mask = GENMASK(5, 3),
+				},
+				.odr_avl[0] = {  15000, 0x00 },
+				.odr_avl[1] = {  30000, 0x01 },
+				.odr_avl[2] = {  60000, 0x02 },
+				.odr_avl[3] = { 120000, 0x03 },
+				.odr_avl[4] = { 240000, 0x04 },
+				.odr_avl[5] = { 480000, 0x05 },
+				.odr_len = 6,
+			},
+			.fifo_enable = {
+				.addr = 0x44,
+				.mask = BIT(1),
+			},
+			.page_mux = {
+				.addr = 0x01,
+				.mask = BIT(7),
+			},
+			.enable = {
+				.addr = 0x04,
+				.mask = BIT(1),
+			},
+		},
 	},
 	{
 		.reset = {
@@ -2899,6 +2951,12 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 			return err;
 	}
 
+	if (hw->settings->sf_settings.chan) {
+		err = st_lsm6dsx_sf_probe(hw, name);
+		if (err)
+			return err;
+	}
+
 	if (hw->irq > 0) {
 		err = st_lsm6dsx_irq_setup(hw);
 		if (err < 0)
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
+/*
+ * STMicroelectronics st_lsm6dsx IMU sensor fusion
+ */
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/sprintf.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include "st_lsm6dsx.h"
+
+static int
+st_lsm6dsx_sf_get_odr_val(const struct st_lsm6dsx_sf_settings *settings,
+			  u32 odr, u8 *val)
+{
+	int i;
+
+	for (i = 0; i < settings->odr_table.odr_len; i++) {
+		if (settings->odr_table.odr_avl[i].milli_hz == odr)
+			break;
+	}
+	if (i == settings->odr_table.odr_len)
+		return -EINVAL;
+
+	*val = settings->odr_table.odr_avl[i].val;
+	return 0;
+}
+
+/**
+ * st_lsm6dsx_sf_page_enable - Enable access to sensor fusion configuration
+ * registers.
+ * @hw: Sensor hardware instance.
+ *
+ * Return: 0 on success, negative value on error.
+ */
+static int st_lsm6dsx_sf_page_enable(struct st_lsm6dsx_hw *hw)
+{
+	const struct st_lsm6dsx_reg *mux = &hw->settings->sf_settings.page_mux;
+
+	return regmap_set_bits(hw->regmap, mux->addr, mux->mask);
+}
+
+/**
+ * st_lsm6dsx_sf_page_disable - Disable access to sensor fusion configuration
+ * registers.
+ * @hw: Sensor hardware instance.
+ *
+ * Return: 0 on success, negative value on error.
+ */
+static int st_lsm6dsx_sf_page_disable(struct st_lsm6dsx_hw *hw)
+{
+	const struct st_lsm6dsx_reg *mux = &hw->settings->sf_settings.page_mux;
+
+	return regmap_clear_bits(hw->regmap, mux->addr, mux->mask);
+}
+
+int st_lsm6dsx_sf_set_enable(struct st_lsm6dsx_sensor *sensor, bool enable)
+{
+	struct st_lsm6dsx_hw *hw = sensor->hw;
+	const struct st_lsm6dsx_reg *en_reg;
+	int err;
+
+	guard(mutex)(&hw->page_lock);
+
+	en_reg = &hw->settings->sf_settings.enable;
+	err = st_lsm6dsx_sf_page_enable(hw);
+	if (err)
+		return err;
+
+	err = regmap_assign_bits(hw->regmap, en_reg->addr, en_reg->mask, enable);
+	if (err) {
+		st_lsm6dsx_sf_page_disable(hw);
+		return err;
+	}
+
+	return st_lsm6dsx_sf_page_disable(hw);
+}
+
+int st_lsm6dsx_sf_set_odr(struct st_lsm6dsx_sensor *sensor, bool enable)
+{
+	const struct st_lsm6dsx_sf_settings *settings;
+	struct st_lsm6dsx_hw *hw = sensor->hw;
+	u8 data;
+	int err;
+
+	guard(mutex)(&hw->page_lock);
+
+	err = st_lsm6dsx_sf_page_enable(hw);
+	if (err)
+		return err;
+
+	settings = &hw->settings->sf_settings;
+	if (enable) {
+		const struct st_lsm6dsx_reg *reg = &settings->odr_table.reg;
+		u8 odr_val;
+
+		st_lsm6dsx_sf_get_odr_val(settings, sensor->hwfifo_odr_mHz,
+					  &odr_val);
+		data = ST_LSM6DSX_SHIFT_VAL(odr_val, reg->mask);
+		err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
+					 data);
+		if (err)
+			goto out;
+	}
+
+	err = regmap_assign_bits(hw->regmap, settings->fifo_enable.addr,
+				 settings->fifo_enable.mask, enable);
+	if (err)
+		goto out;
+
+	return st_lsm6dsx_sf_page_disable(hw);
+
+out:
+	st_lsm6dsx_sf_page_disable(hw);
+
+	return err;
+}
+
+static int st_lsm6dsx_sf_read_raw(struct iio_dev *iio_dev,
+				  struct iio_chan_spec const *ch,
+				  int *val, int *val2, long mask)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = sensor->hwfifo_odr_mHz / MILLI;
+		*val2 = (sensor->hwfifo_odr_mHz % MILLI) * (MICRO / MILLI);
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int st_lsm6dsx_sf_write_raw(struct iio_dev *iio_dev,
+				   struct iio_chan_spec const *chan,
+				   int val, int val2, long mask)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
+	const struct st_lsm6dsx_sf_settings *settings;
+	int err;
+
+	settings = &sensor->hw->settings->sf_settings;
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		u32 odr_mHz = val * MILLI + val2 * (MILLI / MICRO);
+		u8 odr_val;
+
+		/* check that the requested frequency is supported */
+		err = st_lsm6dsx_sf_get_odr_val(settings, odr_mHz, &odr_val);
+		if (err)
+			return err;
+
+		sensor->hwfifo_odr_mHz = odr_mHz;
+		return 0;
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t st_lsm6dsx_sf_sampling_freq_avail(struct device *dev,
+						 struct device_attribute *attr,
+						 char *buf)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_to_iio_dev(dev));
+	const struct st_lsm6dsx_sf_settings *settings;
+	int len = 0;
+
+	settings = &sensor->hw->settings->sf_settings;
+	for (unsigned int i = 0; i < settings->odr_table.odr_len; i++) {
+		u32 val = settings->odr_table.odr_avl[i].milli_hz;
+
+		len += sysfs_emit_at(buf, len, "%lu.%03lu ", val / MILLI,
+				     val % MILLI);
+	}
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
+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
+};
+
+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,
+};
+
+int st_lsm6dsx_sf_probe(struct st_lsm6dsx_hw *hw, const char *name)
+{
+	const struct st_lsm6dsx_sf_settings *settings;
+	struct st_lsm6dsx_sensor *sensor;
+	struct iio_dev *iio_dev;
+
+	iio_dev = devm_iio_device_alloc(hw->dev, sizeof(*sensor));
+	if (!iio_dev)
+		return -ENOMEM;
+
+	settings = &hw->settings->sf_settings;
+	sensor = iio_priv(iio_dev);
+	sensor->id = ST_LSM6DSX_ID_SF;
+	sensor->hw = hw;
+	sensor->hwfifo_odr_mHz = settings->odr_table.odr_avl[0].milli_hz;
+	sensor->watermark = 1;
+	iio_dev->modes = INDIO_DIRECT_MODE;
+	iio_dev->info = &st_lsm6dsx_sf_info;
+	iio_dev->channels = settings->chan;
+	iio_dev->num_channels = settings->chan_len;
+	if (snprintf(sensor->name, sizeof(sensor->name), "%s_sf", name) >=
+	    sizeof(sensor->name))
+		return -E2BIG;
+	iio_dev->name = sensor->name;
+
+	/*
+	 *  Put the IIO device pointer in the iio_devs array so that the caller
+	 *  can set up a buffer and register this IIO device.
+	 */
+	hw->iio_devs[ST_LSM6DSX_ID_SF] = iio_dev;
+
+	return 0;
+}
-- 
2.39.5

Re: [PATCH v5 3/4] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO

[Jonathan Cameron]

On Thu, 22 Jan 2026 17:23:34 +0100
Francesco Lavra <flavra@baylibre.com> wrote:

> The DRDY_MASK feature implemented in sensor chips marks gyroscope and
> accelerometer invalid samples (i.e. samples that have been acquired during
> the settling time of sensor filters) with the special values 0x7FFFh,
> 0x7FFE, and 0x7FFD.
> The driver checks FIFO samples against these special values in order to
> discard invalid samples; however, it does the check regardless of the type
> of samples being processed, whereas this feature is specific to gyroscope
> and accelerometer data. This could cause valid samples to be discarded.
> 
> Fix the above check so that it takes into account the type of samples being
> processed. In st_lsm6dsx_push_tagged_data(), change the type of the data
> parameter to __le16 *, to reflect the fact that this function is called
> with an aligned data argument and avoid casting to __le16 * when checking
> sample values.
Hi Francesco,

I'm going to guess Andy meant all the way up rather than pushing the cast
upwards.  I think this can be done in a fashion that cleans up the type
representation in general.
> 
> Fixes: 960506ed2c69 ("iio: imu: st_lsm6dsx: enable drdy-mask if available")
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>


> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index 5b28a3ffcc3d..ded9a96076e6 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -539,14 +539,14 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>  #define ST_LSM6DSX_INVALID_SAMPLE	0x7ffd
>  static int
>  st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
> -			    u8 *data, s64 ts)
> +			    __le16 *data, s64 ts)
>  {
> -	s16 val = le16_to_cpu(*(__le16 *)data);
>  	struct st_lsm6dsx_sensor *sensor;
>  	struct iio_dev *iio_dev;
>  
>  	/* invalid sample during bootstrap phase */
> -	if (val >= ST_LSM6DSX_INVALID_SAMPLE)
> +	if ((tag == ST_LSM6DSX_GYRO_TAG || tag == ST_LSM6DSX_ACC_TAG) &&
> +	    (s16)le16_to_cpup(data) >= ST_LSM6DSX_INVALID_SAMPLE)
>  		return -EINVAL;
>  
>  	/*
> @@ -670,7 +670,8 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
>  					reset_ts = true;
>  				ts *= hw->ts_gain;
>  			} else {
> -				st_lsm6dsx_push_tagged_data(hw, tag, iio_buff,
> +				st_lsm6dsx_push_tagged_data(hw, tag,
> +							    (__le16 *)iio_buff,

This is ugly but at least it's near the declaration so improvement on previous.

However, I smell a cleaner solution.  If I read the buffer definition correctly
it could be replaced with

struct {
	__le16 data[3];
	aligned_s64 timestamp;
} iio_buff = { };

//note the zeroing because the code never writes the hole which is
bad...  Also that the { } is guaranteed to fill the hole with the build options
the kernel uses (there is a selftest for this).


Which will let you pass iio_buf->data to this call.


>  							    ts);
>  			}
>  		}

Re: [PATCH v5 4/4] iio: imu: st_lsm6dsx: Add support for rotation sensor

[Jonathan Cameron]

On Thu, 22 Jan 2026 17:23:35 +0100
Francesco Lavra <flavra@baylibre.com> 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>

> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index ded9a96076e6..3b4fa57bf461 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c

> @@ -580,6 +584,16 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
>  	case ST_LSM6DSX_EXT2_TAG:
>  		iio_dev = hw->iio_devs[ST_LSM6DSX_ID_EXT2];
>  		break;
> +	case ST_LSM6DSX_ROT_TAG:
> +		/*
> +		 * The sensor reports only the {X, Y, Z} elements of the
> +		 * quaternion vector; set the W value to 0 (it can be derived
> +		 * from the {X, Y, Z} values due to the property that the vector
> +		 * is normalized).

I'd missed this before.  This is going to really confuse user space.
I don't think we can just return it with a 0 in that last entry.
At the very least we need an ABI doc update to reflect this oddity.

I don't think that is enough though. This isn't a quaternion, but
rather something we can derive one from. Annoying though it is,
we can't realistically fix it up in kernel, so we are probably talking
a new MOD_TYPE.

Also it's been a long time since I did much with quaternions,
but isn't the sign of w ambiguous if we are relying on only X, Y and Z?
A bit of googling + AI suggests flipping it inverts the direction of
rotation around a given axis. Feels like there is a constraint missing
in this description.

Jonathan

Re: [PATCH v5 3/4] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO

[Andy Shevchenko]

On Thu, Jan 22, 2026 at 08:07:57PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Jan 2026 17:23:34 +0100
> Francesco Lavra <flavra@baylibre.com> wrote:
> 
> > The DRDY_MASK feature implemented in sensor chips marks gyroscope and
> > accelerometer invalid samples (i.e. samples that have been acquired during
> > the settling time of sensor filters) with the special values 0x7FFFh,
> > 0x7FFE, and 0x7FFD.
> > The driver checks FIFO samples against these special values in order to
> > discard invalid samples; however, it does the check regardless of the type
> > of samples being processed, whereas this feature is specific to gyroscope
> > and accelerometer data. This could cause valid samples to be discarded.
> > 
> > Fix the above check so that it takes into account the type of samples being
> > processed. In st_lsm6dsx_push_tagged_data(), change the type of the data
> > parameter to __le16 *, to reflect the fact that this function is called
> > with an aligned data argument and avoid casting to __le16 * when checking
> > sample values.

> I'm going to guess Andy meant all the way up rather than pushing the cast
> upwards.  I think this can be done in a fashion that cleans up the type
> representation in general.

Indeed. This version is not so better than the previous.

...

> > -				st_lsm6dsx_push_tagged_data(hw, tag, iio_buff,
> > +				st_lsm6dsx_push_tagged_data(hw, tag,
> > +							    (__le16 *)iio_buff,
> 
> This is ugly but at least it's near the declaration so improvement on previous.
> 
> However, I smell a cleaner solution.  If I read the buffer definition correctly
> it could be replaced with
> 
> struct {
> 	__le16 data[3];
> 	aligned_s64 timestamp;
> } iio_buff = { };
> 
> //note the zeroing because the code never writes the hole which is
> bad...  Also that the { } is guaranteed to fill the hole with the build options
> the kernel uses (there is a selftest for this).
> 
> Which will let you pass iio_buf->data to this call.

Yes, this looks way better and allows to get rid of all assumptions and
castings.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 4/4] iio: imu: st_lsm6dsx: Add support for rotation sensor

[Francesco Lavra]

On Thu, 2026-01-22 at 20:29 +0000, Jonathan Cameron wrote:
> On Thu, 22 Jan 2026 17:23:35 +0100
> Francesco Lavra <flavra@baylibre.com> 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>
> 
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > index ded9a96076e6..3b4fa57bf461 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> 
> > @@ -580,6 +584,16 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw
> > *hw, u8 tag,
> >         case ST_LSM6DSX_EXT2_TAG:
> >                 iio_dev = hw->iio_devs[ST_LSM6DSX_ID_EXT2];
> >                 break;
> > +       case ST_LSM6DSX_ROT_TAG:
> > +               /*
> > +                * The sensor reports only the {X, Y, Z} elements of
> > the
> > +                * quaternion vector; set the W value to 0 (it can be
> > derived
> > +                * from the {X, Y, Z} values due to the property that
> > the vector
> > +                * is normalized).
> 
> I'd missed this before.  This is going to really confuse user space.
> I don't think we can just return it with a 0 in that last entry.
> At the very least we need an ABI doc update to reflect this oddity.
> 
> I don't think that is enough though. This isn't a quaternion, but
> rather something we can derive one from. Annoying though it is,
> we can't realistically fix it up in kernel, so we are probably talking
> a new MOD_TYPE.

Quaternion data read from the sensor is expressed in floating-point format,
and as such needs to be interpreted by userspace in a "non-standard" way
(note there is no scale in the channel info, and this is intentional
because we are not dealing with integers) regardless of whether the W value
is present or must be derived.
Isn't the absence of the scale info enough to let userspace know that this
data is non-standard? Only applications that know how to deal specifically
with this sensor device can make sense of the data (and these applications
know that the quaternion vector is normalized and the W value must be
derived from X, Y, Z).

> Also it's been a long time since I did much with quaternions,
> but isn't the sign of w ambiguous if we are relying on only X, Y and Z?
> A bit of googling + AI suggests flipping it inverts the direction of
> rotation around a given axis. Feels like there is a constraint missing
> in this description.

Flipping the sign of W doesn't just invert the direction of rotation, it
basically applies an offset of -360 degrees; if a value w0 indicates a
rotation by an angle theta0, the value -w0 indicates a rotation by (theta0
- 360), which is basically the same as rotating by theta0. So knowing the
{X, Y, Z} values is enough to have a non-ambiguous orientation.

Re: [PATCH v5 4/4] iio: imu: st_lsm6dsx: Add support for rotation sensor

[Jonathan Cameron]

On Fri, 23 Jan 2026 12:03:29 +0100
Francesco Lavra <flavra@baylibre.com> wrote:

> On Thu, 2026-01-22 at 20:29 +0000, Jonathan Cameron wrote:
> > On Thu, 22 Jan 2026 17:23:35 +0100
> > Francesco Lavra <flavra@baylibre.com> 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>  
> >   
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > index ded9a96076e6..3b4fa57bf461 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c  
> >   
> > > @@ -580,6 +584,16 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw
> > > *hw, u8 tag,
> > >         case ST_LSM6DSX_EXT2_TAG:
> > >                 iio_dev = hw->iio_devs[ST_LSM6DSX_ID_EXT2];
> > >                 break;
> > > +       case ST_LSM6DSX_ROT_TAG:
> > > +               /*
> > > +                * The sensor reports only the {X, Y, Z} elements of
> > > the
> > > +                * quaternion vector; set the W value to 0 (it can be
> > > derived
> > > +                * from the {X, Y, Z} values due to the property that
> > > the vector
> > > +                * is normalized).  
> > 
> > I'd missed this before.  This is going to really confuse user space.
> > I don't think we can just return it with a 0 in that last entry.
> > At the very least we need an ABI doc update to reflect this oddity.
> > 
> > I don't think that is enough though. This isn't a quaternion, but
> > rather something we can derive one from. Annoying though it is,
> > we can't realistically fix it up in kernel, so we are probably talking
> > a new MOD_TYPE.  
> 
> Quaternion data read from the sensor is expressed in floating-point format,
> and as such needs to be interpreted by userspace in a "non-standard" way
> (note there is no scale in the channel info, and this is intentional
> because we are not dealing with integers) regardless of whether the W value
> is present or must be derived.
> Isn't the absence of the scale info enough to let userspace know that this
> data is non-standard?

Given both scale and offset are optional with defaults of 1.0 and 0 if
not there, likely code won't notice that both are missing and under
the ABI that would just make _raw == _scale which is odd but not specifically
excluded.

> Only applications that know how to deal specifically
> with this sensor device can make sense of the data (and these applications
> know that the quaternion vector is normalized and the W value must be
> derived from X, Y, Z).

This is the sort of feature that I'm reluctant to support. The only thing
that we have let in (because it was truely obscure) that looks like this
is pulse oximeters - the stuff in drivers/iio/health. It's a complex
many reading maths thing to go from the data to the actual thing being
measured.  The purpose of unified interfaces is being able to use them
across different sensors. Here we can't.  Hence this need some careful
thought.

> 
> > Also it's been a long time since I did much with quaternions,
> > but isn't the sign of w ambiguous if we are relying on only X, Y and Z?
> > A bit of googling + AI suggests flipping it inverts the direction of
> > rotation around a given axis. Feels like there is a constraint missing
> > in this description.  
> 
> Flipping the sign of W doesn't just invert the direction of rotation, it
> basically applies an offset of -360 degrees; if a value w0 indicates a
> rotation by an angle theta0, the value -w0 indicates a rotation by (theta0
> - 360), which is basically the same as rotating by theta0. So knowing the
> {X, Y, Z} values is enough to have a non-ambiguous orientation.

Ok. Taking a while to remember this stuff, but I'm fairly sure it isn't quite
that.
Inverting w is the difference between theta and (360 - theta) not (theta - 360)
given the 360 doesn't matter as you say, it's a clockwise vs anticlockwise
rotation.

Lets take a vector to rotate (say representing up on a screen represnted
as pure quaternion  v= (0 1 0 0). Apply rotation quaternion to rotate that about the
Y axis by 90 degrees in one direction (I'm too lazy to figure out which but doesn't
matter!)
q = (cos(theta/2), 0, sin(theta/2)j, 0)
  = (sqrt(2)/2, 0, sqrt(2)/2, 0)

Apply rotation is q v q' 

So multiplying it out 
(sqrt(2)/2, 0, (sqrt(2)/2)j, 0) (0, 1i, 0 0) (sqrt(2)/2, 0, -(sqrt(2)/2)j, 0)
Given it's all multiples of (Sqrt(2)/2) Lets call that A
= (A, 0, Aj, 0)(0, 1j, 0, 0)(A, 0, -Aj, 0)
= (0, Ai, 0, -Ak) (A, 0, -Aj, 0)
= (0, (A*A - (-A)*(-A))i, 0, (A *(-A) + (-A) * A)j)
= (0, 0, 0, -1) or down on the z axis

Same again, but now flip the W value

(-A, 0, Aj, 0)(0, 1j, 0, 0)(-A, 0, -Aj, 0)
= (0, (-A)i, 0, -(A)k)(-A, 0, -Aj, 0)
= (0, ((-A)*(-A) -  (-A)*(-A))j, 0, (-A)*(-A) + (-A)*(-A)
= (0, 0, 0, 1) or up on the z axis.

Anyhow, that is moot if we don't figure out what to do about the fact
we are forcing data into a representation a long way from what
user space accepts.

Jonathan









> 

Re: [PATCH v5 4/4] iio: imu: st_lsm6dsx: Add support for rotation sensor

[David Lechner]

On 1/22/26 2:29 PM, Jonathan Cameron wrote:
> On Thu, 22 Jan 2026 17:23:35 +0100
> Francesco Lavra <flavra@baylibre.com> 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>
> 
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
>> index ded9a96076e6..3b4fa57bf461 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> 
>> @@ -580,6 +584,16 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
>>  	case ST_LSM6DSX_EXT2_TAG:
>>  		iio_dev = hw->iio_devs[ST_LSM6DSX_ID_EXT2];
>>  		break;
>> +	case ST_LSM6DSX_ROT_TAG:
>> +		/*
>> +		 * The sensor reports only the {X, Y, Z} elements of the
>> +		 * quaternion vector; set the W value to 0 (it can be derived
>> +		 * from the {X, Y, Z} values due to the property that the vector
>> +		 * is normalized).
> 
> I'd missed this before.  This is going to really confuse user space.
> I don't think we can just return it with a 0 in that last entry.
> At the very least we need an ABI doc update to reflect this oddity.
> 
> I don't think that is enough though. This isn't a quaternion, but
> rather something we can derive one from. Annoying though it is,
> we can't realistically fix it up in kernel, so we are probably talking
> a new MOD_TYPE.

Do we also need to add something to struct iio_scan_type to indicate
that the raw value is floating point as opposed to integer?

> 
> Also it's been a long time since I did much with quaternions,
> but isn't the sign of w ambiguous if we are relying on only X, Y and Z?
> A bit of googling + AI suggests flipping it inverts the direction of
> rotation around a given axis. Feels like there is a constraint missing
> in this description.
> 
> Jonathan

Re: [PATCH v5 4/4] iio: imu: st_lsm6dsx: Add support for rotation sensor

[Francesco Lavra]

On Fri, 2026-01-23 at 17:48 +0000, Jonathan Cameron wrote:
> On Fri, 23 Jan 2026 12:03:29 +0100
> Francesco Lavra <flavra@baylibre.com> wrote:
> 
> > On Thu, 2026-01-22 at 20:29 +0000, Jonathan Cameron wrote:
> > > On Thu, 22 Jan 2026 17:23:35 +0100
> > > Francesco Lavra <flavra@baylibre.com> 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>  
> > >   
> > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > index ded9a96076e6..3b4fa57bf461 100644
> > > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c  
> > >   
> > > > @@ -580,6 +584,16 @@ st_lsm6dsx_push_tagged_data(struct
> > > > st_lsm6dsx_hw
> > > > *hw, u8 tag,
> > > >         case ST_LSM6DSX_EXT2_TAG:
> > > >                 iio_dev = hw->iio_devs[ST_LSM6DSX_ID_EXT2];
> > > >                 break;
> > > > +       case ST_LSM6DSX_ROT_TAG:
> > > > +               /*
> > > > +                * The sensor reports only the {X, Y, Z} elements
> > > > of
> > > > the
> > > > +                * quaternion vector; set the W value to 0 (it can
> > > > be
> > > > derived
> > > > +                * from the {X, Y, Z} values due to the property
> > > > that
> > > > the vector
> > > > +                * is normalized).  
> > > 
> > > I'd missed this before.  This is going to really confuse user space.
> > > I don't think we can just return it with a 0 in that last entry.
> > > At the very least we need an ABI doc update to reflect this oddity.
> > > 
> > > I don't think that is enough though. This isn't a quaternion, but
> > > rather something we can derive one from. Annoying though it is,
> > > we can't realistically fix it up in kernel, so we are probably
> > > talking
> > > a new MOD_TYPE.  
> > 
> > Quaternion data read from the sensor is expressed in floating-point
> > format,
> > and as such needs to be interpreted by userspace in a "non-standard"
> > way
> > (note there is no scale in the channel info, and this is intentional
> > because we are not dealing with integers) regardless of whether the W
> > value
> > is present or must be derived.
> > Isn't the absence of the scale info enough to let userspace know that
> > this
> > data is non-standard?
> 
> Given both scale and offset are optional with defaults of 1.0 and 0 if
> not there, likely code won't notice that both are missing and under
> the ABI that would just make _raw == _scale which is odd but not
> specifically
> excluded.
> 
> > Only applications that know how to deal specifically
> > with this sensor device can make sense of the data (and these
> > applications
> > know that the quaternion vector is normalized and the W value must be
> > derived from X, Y, Z).
> 
> This is the sort of feature that I'm reluctant to support. The only thing
> that we have let in (because it was truely obscure) that looks like this
> is pulse oximeters - the stuff in drivers/iio/health. It's a complex
> many reading maths thing to go from the data to the actual thing being
> measured.  The purpose of unified interfaces is being able to use them
> across different sensors. Here we can't.  Hence this need some careful
> thought.

I see. So there are two things that need to be fixed:

1. the floating point format: as David suggested in the other post, the
specification of the scan type could be amended to allow floating point
formats; how about adding a new option to the ABI for the sign character
(which could perhaps be renamed to `format` in struct iio_scan_type)?
Currently we have 's' for signed and 'u' for unsigned: could we add 'f' for
IEEE 754 floats? Then the 'bits' part can have values in the {16,32,64}
set, to indicate half-, single- or double-precision numbers, respectively;
but could also not specify what numbers of bits are allowed in the ABI, and
just refer to the IEEE 754 standard for the available formats.

2. the absence of the W value in the quaternion vector; possible solutions
to this issue could be:
- adding in_rot_{x,y,z}_* (and perhaps in_rot_w_*) to the ABI to represent
the individual components of the quaternion as separate channels; if the w
channel is not present, its sample values could be derived from the other
channels under the assumptions that the vector is normalized and the
rotation angle is in the [-180, 180] range
- adding something like in_rot_reduced_quaternion_*, which would be the
same as in_rot_quaternion_* with the exclusion of the W component, which
could be derived as above

> > 
> > > Also it's been a long time since I did much with quaternions,
> > > but isn't the sign of w ambiguous if we are relying on only X, Y and
> > > Z?
> > > A bit of googling + AI suggests flipping it inverts the direction of
> > > rotation around a given axis. Feels like there is a constraint
> > > missing
> > > in this description.  
> > 
> > Flipping the sign of W doesn't just invert the direction of rotation,
> > it
> > basically applies an offset of -360 degrees; if a value w0 indicates a
> > rotation by an angle theta0, the value -w0 indicates a rotation by
> > (theta0
> > - 360), which is basically the same as rotating by theta0. So knowing
> > the
> > {X, Y, Z} values is enough to have a non-ambiguous orientation.
> 
> Ok. Taking a while to remember this stuff, but I'm fairly sure it isn't
> quite
> that.
> Inverting w is the difference between theta and (360 - theta) not (theta
> - 360)
> given the 360 doesn't matter as you say, it's a clockwise vs
> anticlockwise
> rotation.
> 
> Lets take a vector to rotate (say representing up on a screen represnted
> as pure quaternion  v= (0 1 0 0). Apply rotation quaternion to rotate
> that about the
> Y axis by 90 degrees in one direction (I'm too lazy to figure out which
> but doesn't
> matter!)
> q = (cos(theta/2), 0, sin(theta/2)j, 0)
>   = (sqrt(2)/2, 0, sqrt(2)/2, 0)
> 
> Apply rotation is q v q' 
> 
> So multiplying it out 
> (sqrt(2)/2, 0, (sqrt(2)/2)j, 0) (0, 1i, 0 0) (sqrt(2)/2, 0, -
> (sqrt(2)/2)j, 0)
> Given it's all multiples of (Sqrt(2)/2) Lets call that A
> = (A, 0, Aj, 0)(0, 1j, 0, 0)(A, 0, -Aj, 0)
> = (0, Ai, 0, -Ak) (A, 0, -Aj, 0)
> = (0, (A*A - (-A)*(-A))i, 0, (A *(-A) + (-A) * A)j)
> = (0, 0, 0, -1) or down on the z axis
> 
> Same again, but now flip the W value
> 
> (-A, 0, Aj, 0)(0, 1j, 0, 0)(-A, 0, -Aj, 0)
> = (0, (-A)i, 0, -(A)k)(-A, 0, -Aj, 0)
> = (0, ((-A)*(-A) -  (-A)*(-A))j, 0, (-A)*(-A) + (-A)*(-A)
> = (0, 0, 0, 1) or up on the z axis.

OK, I got the math wrong, flipping the W value does indeed invert the
direction of rotation as you said. As alluded to above, the missing
constraint is that the amount of rotation is within the [-180, 180] range:
this ensures that cos(theta/2) is always non-negative and removes the
ambiguity, while still allowing all possible orientations in 3D space to be
represented.

> Anyhow, that is moot if we don't figure out what to do about the fact
> we are forcing data into a representation a long way from what
> user space accepts.
> 
> Jonathan

Re: [PATCH v5 4/4] iio: imu: st_lsm6dsx: Add support for rotation sensor

[Jonathan Cameron]

On Mon, 26 Jan 2026 12:15:11 +0100
Francesco Lavra <flavra@baylibre.com> wrote:

> On Fri, 2026-01-23 at 17:48 +0000, Jonathan Cameron wrote:
> > On Fri, 23 Jan 2026 12:03:29 +0100
> > Francesco Lavra <flavra@baylibre.com> wrote:
> >   
> > > On Thu, 2026-01-22 at 20:29 +0000, Jonathan Cameron wrote:  
> > > > On Thu, 22 Jan 2026 17:23:35 +0100
> > > > Francesco Lavra <flavra@baylibre.com> 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>    
> > > >     
> > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > > index ded9a96076e6..3b4fa57bf461 100644
> > > > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    
> > > >     
> > > > > @@ -580,6 +584,16 @@ st_lsm6dsx_push_tagged_data(struct
> > > > > st_lsm6dsx_hw
> > > > > *hw, u8 tag,
> > > > >         case ST_LSM6DSX_EXT2_TAG:
> > > > >                 iio_dev = hw->iio_devs[ST_LSM6DSX_ID_EXT2];
> > > > >                 break;
> > > > > +       case ST_LSM6DSX_ROT_TAG:
> > > > > +               /*
> > > > > +                * The sensor reports only the {X, Y, Z} elements
> > > > > of
> > > > > the
> > > > > +                * quaternion vector; set the W value to 0 (it can
> > > > > be
> > > > > derived
> > > > > +                * from the {X, Y, Z} values due to the property
> > > > > that
> > > > > the vector
> > > > > +                * is normalized).    
> > > > 
> > > > I'd missed this before.  This is going to really confuse user space.
> > > > I don't think we can just return it with a 0 in that last entry.
> > > > At the very least we need an ABI doc update to reflect this oddity.
> > > > 
> > > > I don't think that is enough though. This isn't a quaternion, but
> > > > rather something we can derive one from. Annoying though it is,
> > > > we can't realistically fix it up in kernel, so we are probably
> > > > talking
> > > > a new MOD_TYPE.    
> > > 
> > > Quaternion data read from the sensor is expressed in floating-point
> > > format,
> > > and as such needs to be interpreted by userspace in a "non-standard"
> > > way
> > > (note there is no scale in the channel info, and this is intentional
> > > because we are not dealing with integers) regardless of whether the W
> > > value
> > > is present or must be derived.
> > > Isn't the absence of the scale info enough to let userspace know that
> > > this
> > > data is non-standard?  
> > 
> > Given both scale and offset are optional with defaults of 1.0 and 0 if
> > not there, likely code won't notice that both are missing and under
> > the ABI that would just make _raw == _scale which is odd but not
> > specifically
> > excluded.
> >   
> > > Only applications that know how to deal specifically
> > > with this sensor device can make sense of the data (and these
> > > applications
> > > know that the quaternion vector is normalized and the W value must be
> > > derived from X, Y, Z).  
> > 
> > This is the sort of feature that I'm reluctant to support. The only thing
> > that we have let in (because it was truely obscure) that looks like this
> > is pulse oximeters - the stuff in drivers/iio/health. It's a complex
> > many reading maths thing to go from the data to the actual thing being
> > measured.  The purpose of unified interfaces is being able to use them
> > across different sensors. Here we can't.  Hence this need some careful
> > thought.  
> 

Took me a while to reply because I wanted to think about this a little.

> I see. So there are two things that need to be fixed:
> 
> 1. the floating point format: as David suggested in the other post, the
> specification of the scan type could be amended to allow floating point
> formats; how about adding a new option to the ABI for the sign character
> (which could perhaps be renamed to `format` in struct iio_scan_type)?
> Currently we have 's' for signed and 'u' for unsigned: could we add 'f' for
> IEEE 754 floats?
> Then the 'bits' part can have values in the {16,32,64}
> set, to indicate half-, single- or double-precision numbers, respectively;
> but could also not specify what numbers of bits are allowed in the ABI, and
> just refer to the IEEE 754 standard for the available formats.

I'd not thought of that approach. At first thought seems like a good idea
given as you note we already have the precision.  Document it clearly as
being IEEE 754 as I suspect if we open the door to floats we'll get some
others given there are far too many formats out there and we care about
small ones for sensors where it gets even messier!  So subject to seeing
the code and ABI docs this sounds good to me.

> 
> 2. the absence of the W value in the quaternion vector; possible solutions
> to this issue could be:
> - adding in_rot_{x,y,z}_* (and perhaps in_rot_w_*) to the ABI to represent
> the individual components of the quaternion as separate channels; if the w
> channel is not present, its sample values could be derived from the other
> channels under the assumptions that the vector is normalized and the
> rotation angle is in the [-180, 180] range
> - adding something like in_rot_reduced_quaternion_*, which would be the
> same as in_rot_quaternion_* with the exclusion of the W component, which
> could be derived as above

I've been mulling this over. Given quaternion components on their own are
more or less meaningless and there isn't a clean way to describe a quaternion
with missing bits, I don't think breaking it out into components makes sense
(that's why we added the multi part channels in the first place).

I think maybe we should just allow for a custom channel type and it's
up to custom userspace to deal with it. that lets us just output this as
a 3 part value for a channel of type IIO_CUSTOM.

Expectation is anyone using the custom type must provide a per driver
document to explain what it is actually is, preferably with links to userspace
code to interpret it etc.

Getting those documents reviewed would be the gate on this getting abused
in upstream drivers for things that really should be normal types.
So when adding the type, add a comment on that being required as well.

> 
> > >   
> > > > Also it's been a long time since I did much with quaternions,
> > > > but isn't the sign of w ambiguous if we are relying on only X, Y and
> > > > Z?
> > > > A bit of googling + AI suggests flipping it inverts the direction of
> > > > rotation around a given axis. Feels like there is a constraint
> > > > missing
> > > > in this description.    
> > > 
> > > Flipping the sign of W doesn't just invert the direction of rotation,
> > > it
> > > basically applies an offset of -360 degrees; if a value w0 indicates a
> > > rotation by an angle theta0, the value -w0 indicates a rotation by
> > > (theta0
> > > - 360), which is basically the same as rotating by theta0. So knowing
> > > the
> > > {X, Y, Z} values is enough to have a non-ambiguous orientation.  
> > 
> > Ok. Taking a while to remember this stuff, but I'm fairly sure it isn't
> > quite
> > that.
> > Inverting w is the difference between theta and (360 - theta) not (theta
> > - 360)
> > given the 360 doesn't matter as you say, it's a clockwise vs
> > anticlockwise
> > rotation.
> > 
> > Lets take a vector to rotate (say representing up on a screen represnted
> > as pure quaternion  v= (0 1 0 0). Apply rotation quaternion to rotate
> > that about the
> > Y axis by 90 degrees in one direction (I'm too lazy to figure out which
> > but doesn't
> > matter!)
> > q = (cos(theta/2), 0, sin(theta/2)j, 0)
> >   = (sqrt(2)/2, 0, sqrt(2)/2, 0)
> > 
> > Apply rotation is q v q' 
> > 
> > So multiplying it out 
> > (sqrt(2)/2, 0, (sqrt(2)/2)j, 0) (0, 1i, 0 0) (sqrt(2)/2, 0, -
> > (sqrt(2)/2)j, 0)
> > Given it's all multiples of (Sqrt(2)/2) Lets call that A
> > = (A, 0, Aj, 0)(0, 1j, 0, 0)(A, 0, -Aj, 0)
> > = (0, Ai, 0, -Ak) (A, 0, -Aj, 0)
> > = (0, (A*A - (-A)*(-A))i, 0, (A *(-A) + (-A) * A)j)
> > = (0, 0, 0, -1) or down on the z axis
> > 
> > Same again, but now flip the W value
> > 
> > (-A, 0, Aj, 0)(0, 1j, 0, 0)(-A, 0, -Aj, 0)
> > = (0, (-A)i, 0, -(A)k)(-A, 0, -Aj, 0)
> > = (0, ((-A)*(-A) -  (-A)*(-A))j, 0, (-A)*(-A) + (-A)*(-A)
> > = (0, 0, 0, 1) or up on the z axis.  
> 
> OK, I got the math wrong, flipping the W value does indeed invert the
> direction of rotation as you said. As alluded to above, the missing
> constraint is that the amount of rotation is within the [-180, 180] range:
> this ensures that cos(theta/2) is always non-negative and removes the
> ambiguity, while still allowing all possible orientations in 3D space to be
> represented.

Ok. We'll need to have that in the per driver doc.
Thanks for chasing that down.

Jonathan


> 
> > Anyhow, that is moot if we don't figure out what to do about the fact
> > we are forcing data into a representation a long way from what
> > user space accepts.
> > 
> > Jonathan  

Re: [PATCH v5 2/4] iio: imu: st_lsm6dsx: Set buffer sampling frequency for accelerometer only

[Francesco Lavra]

On Thu, 2026-01-22 at 17:23 +0100, Francesco Lavra wrote:
> The st_lsm6dsx_hwfifo_odr_store() function, which is called when
> userspace
> writes the buffer sampling frequency sysfs attribute, calls
> st_lsm6dsx_check_odr(), which accesses the odr_table array at index
> `sensor->id`; since this array is only 2 entries long, an access for any
> sensor type other than accelerometer or gyroscope is an out-of-bounds
> access.
> 
> The motivation for being able to set a buffer frequency different from
> the
> sensor sampling frequency is to support use cases that need accurate
> event
> detection (which requires a high sampling frequency) while retrieving
> sensor data at low frequency. Since all the supported event types are
> generated from acceleration data only, do not create the buffer sampling
> frequency attribute for sensor types other than the accelerometer.
> 
> Fixes: 6b648a36c200 ("iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO
> batch data rate")

This patch didn't make it to 6.19. Would it still be OK to merge this in
the next release cycle? It technically changes userspace, but I think this
change can be put in the "no one will notice" category.

Re: [PATCH v5 2/4] iio: imu: st_lsm6dsx: Set buffer sampling frequency for accelerometer only

[Jonathan Cameron]

On Mon, 09 Feb 2026 09:02:43 +0100
Francesco Lavra <flavra@baylibre.com> wrote:

> On Thu, 2026-01-22 at 17:23 +0100, Francesco Lavra wrote:
> > The st_lsm6dsx_hwfifo_odr_store() function, which is called when
> > userspace
> > writes the buffer sampling frequency sysfs attribute, calls
> > st_lsm6dsx_check_odr(), which accesses the odr_table array at index
> > `sensor->id`; since this array is only 2 entries long, an access for any
> > sensor type other than accelerometer or gyroscope is an out-of-bounds
> > access.
> > 
> > The motivation for being able to set a buffer frequency different from
> > the
> > sensor sampling frequency is to support use cases that need accurate
> > event
> > detection (which requires a high sampling frequency) while retrieving
> > sensor data at low frequency. Since all the supported event types are
> > generated from acceleration data only, do not create the buffer sampling
> > frequency attribute for sensor types other than the accelerometer.
> > 
> > Fixes: 6b648a36c200 ("iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO
> > batch data rate")  
> 
> This patch didn't make it to 6.19. Would it still be OK to merge this in
> the next release cycle? It technically changes userspace, but I think this
> change can be put in the "no one will notice" category.
> 

I think it should be fine in 7.0 (though I've been wrong before!)

Thanks,

Jonathan