[PATCH v7 0/6] imu: st_lsm6dsx: Add support for rotation sensor

[PATCH v7 0/6] 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 IIO ABI has been amended to allow the use of floating-point data and
a "partial quaternion" channel modifier in IIO buffers.
Tested on LSM6DSV16X.

Changes from v6 [6]:
- dropped patches 1 and 2 (already picked up by Jonathan)
- added floating-point support to iio-tools (Andy)
- replaced `sign` field in struct iio_scan_type with union instead of
  `format` field (David, Jonathan)
- changed [s|u|f] to [f|s|u] in ABI and driver documentation files (Andy)
- removed IIO_CUSTOM channel type in favor of IIO_MOD_PARTIAL_QUATERNION
  modifier (David, Andy)
- replaced "sf" with "fusion" in type, variable and function names (David)
- added copyright statement in st_lsm6dsx_fusion.c (David)
- added `ret` variable in st_lsm6dsx_fusion_probe() for better readability
  (Andy)
- modified data structure holding the list of available frequencies in
  order to replace device attribute with .read_avail callback (David)

Changes from v5 [5]:
- cleaned up FIFO data representation in st_lsm6dsx_read_tagged_fifo() to
  avoid casting (Andy, Jonathan)
- renamed 'sign' field to `format` in struct iio_scan_type
- added support for IEEE 754 floating-point format in buffer scan elements
- added custom data type in the ABI (IIO_CUSTOM in enum iio_chan_type)
  (Jonathan)
- added driver document in Documentation/iio/ (Jonathan)

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

[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/
[5] https://lore.kernel.org/linux-iio/20260122162335.2020006-1-flavra@baylibre.com/T/
[6] https://lore.kernel.org/linux-iio/20260225100421.2366864-1-flavra@baylibre.com/T/

Francesco Lavra (6):
  iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO
  iio: Replace 'sign' field with union in struct iio_scan_type
  iio: tools: Add support for floating-point numbers in buffer scan
    elements
  iio: ABI: Add support for floating-point numbers in buffer scan
    elements
  iio: ABI: Add partial quaternion modifier
  iio: imu: st_lsm6dsx: Add support for rotation sensor

 Documentation/ABI/testing/sysfs-bus-iio       |  46 ++--
 Documentation/driver-api/iio/buffers.rst      |   7 +-
 Documentation/iio/iio_devbuf.rst              |   3 +-
 drivers/iio/imu/st_lsm6dsx/Makefile           |   2 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  28 ++-
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  32 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  |  58 +++++
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c    | 234 ++++++++++++++++++
 drivers/iio/industrialio-core.c               |   1 +
 include/linux/iio/iio.h                       |   7 +-
 include/uapi/linux/iio/types.h                |   1 +
 tools/iio/iio_event_monitor.c                 |   1 +
 tools/iio/iio_generic_buffer.c                |  56 ++++-
 tools/iio/iio_utils.c                         |   8 +-
 tools/iio/iio_utils.h                         |   4 +-
 15 files changed, 438 insertions(+), 50 deletions(-)
 create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c

-- 
2.39.5

[PATCH v7 1/6] 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. To avoid casting to __le16 * when checking sample values, clean
up the type representation for data read from the FIFO.

Fixes: 960506ed2c69 ("iio: imu: st_lsm6dsx: enable drdy-mask if available")
Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 +++++++++++--------
 1 file changed, 14 insertions(+), 9 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..a6ee2da5a06c 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -365,8 +365,6 @@ static inline int st_lsm6dsx_read_block(struct st_lsm6dsx_hw *hw, u8 addr,
 	return 0;
 }
 
-#define ST_LSM6DSX_IIO_BUFF_SIZE	(ALIGN(ST_LSM6DSX_SAMPLE_SIZE, \
-					       sizeof(s64)) + sizeof(s64))
 /**
  * st_lsm6dsx_read_fifo() - hw FIFO read routine
  * @hw: Pointer to instance of struct st_lsm6dsx_hw.
@@ -539,14 +537,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;
 
 	/*
@@ -609,7 +607,13 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
 	 * must be passed a buffer that is aligned to 8 bytes so
 	 * as to allow insertion of a naturally aligned timestamp.
 	 */
-	u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8);
+	struct {
+		union {
+			__le16 data[3];
+			__le32 fifo_ts;
+		};
+		aligned_s64 timestamp;
+	} iio_buff = { };
 	u8 tag;
 	bool reset_ts = false;
 	int i, err, read_len;
@@ -648,7 +652,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
 
 		for (i = 0; i < pattern_len;
 		     i += ST_LSM6DSX_TAGGED_SAMPLE_SIZE) {
-			memcpy(iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
+			memcpy(&iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
 			       ST_LSM6DSX_SAMPLE_SIZE);
 
 			tag = hw->buff[i] >> 3;
@@ -659,7 +663,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
 				 * B0 = ts[7:0], B1 = ts[15:8], B2 = ts[23:16],
 				 * B3 = ts[31:24]
 				 */
-				ts = le32_to_cpu(*((__le32 *)iio_buff));
+				ts = le32_to_cpu(iio_buff.fifo_ts);
 				/*
 				 * check if hw timestamp engine is going to
 				 * reset (the sensor generates an interrupt
@@ -670,7 +674,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,
+							    iio_buff.data,
 							    ts);
 			}
 		}
-- 
2.39.5

Re: [PATCH v7 1/6] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO

[Jonathan Cameron]

On Wed,  4 Mar 2026 09:06:00 +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. To avoid casting to __le16 * when checking sample values, clean
> up the type representation for data read from the FIFO.
> 
> Fixes: 960506ed2c69 ("iio: imu: st_lsm6dsx: enable drdy-mask if available")
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
Looks fine to me, but looking for a Lorenzo tag ideally given it's not
a particularly trivial fix!

Jonathan
> ---
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 +++++++++++--------
>  1 file changed, 14 insertions(+), 9 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..a6ee2da5a06c 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -365,8 +365,6 @@ static inline int st_lsm6dsx_read_block(struct st_lsm6dsx_hw *hw, u8 addr,
>  	return 0;
>  }
>  
> -#define ST_LSM6DSX_IIO_BUFF_SIZE	(ALIGN(ST_LSM6DSX_SAMPLE_SIZE, \
> -					       sizeof(s64)) + sizeof(s64))
>  /**
>   * st_lsm6dsx_read_fifo() - hw FIFO read routine
>   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
> @@ -539,14 +537,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;
>  
>  	/*
> @@ -609,7 +607,13 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
>  	 * must be passed a buffer that is aligned to 8 bytes so
>  	 * as to allow insertion of a naturally aligned timestamp.
>  	 */
> -	u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8);
> +	struct {
> +		union {
> +			__le16 data[3];
> +			__le32 fifo_ts;
> +		};
> +		aligned_s64 timestamp;
> +	} iio_buff = { };
>  	u8 tag;
>  	bool reset_ts = false;
>  	int i, err, read_len;
> @@ -648,7 +652,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
>  
>  		for (i = 0; i < pattern_len;
>  		     i += ST_LSM6DSX_TAGGED_SAMPLE_SIZE) {
> -			memcpy(iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
> +			memcpy(&iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
>  			       ST_LSM6DSX_SAMPLE_SIZE);
>  
>  			tag = hw->buff[i] >> 3;
> @@ -659,7 +663,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
>  				 * B0 = ts[7:0], B1 = ts[15:8], B2 = ts[23:16],
>  				 * B3 = ts[31:24]
>  				 */
> -				ts = le32_to_cpu(*((__le32 *)iio_buff));
> +				ts = le32_to_cpu(iio_buff.fifo_ts);
>  				/*
>  				 * check if hw timestamp engine is going to
>  				 * reset (the sensor generates an interrupt
> @@ -670,7 +674,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,
> +							    iio_buff.data,
>  							    ts);
>  			}
>  		}

Re: [PATCH v7 1/6] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO

[Lorenzo Bianconi]

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

> On Wed,  4 Mar 2026 09:06:00 +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. To avoid casting to __le16 * when checking sample values, clean
> > up the type representation for data read from the FIFO.
> > 
> > Fixes: 960506ed2c69 ("iio: imu: st_lsm6dsx: enable drdy-mask if available")
> > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> Looks fine to me, but looking for a Lorenzo tag ideally given it's not
> a particularly trivial fix!
> 
> Jonathan

Hi Francesco,

thx for fixing this, I think the patch is fine, just a couple of nits inline if
you need to repost.

Regards,
Lorenzo

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> > ---
> >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 +++++++++++--------
> >  1 file changed, 14 insertions(+), 9 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..a6ee2da5a06c 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > @@ -365,8 +365,6 @@ static inline int st_lsm6dsx_read_block(struct st_lsm6dsx_hw *hw, u8 addr,
> >  	return 0;
> >  }
> >  
> > -#define ST_LSM6DSX_IIO_BUFF_SIZE	(ALIGN(ST_LSM6DSX_SAMPLE_SIZE, \
> > -					       sizeof(s64)) + sizeof(s64))
> >  /**
> >   * st_lsm6dsx_read_fifo() - hw FIFO read routine
> >   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
> > @@ -539,14 +537,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;

what about moving this check to a dedicated routine? Like
st_lsm6dsx_check_data() or similar?

> >  
> >  	/*
> > @@ -609,7 +607,13 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
> >  	 * must be passed a buffer that is aligned to 8 bytes so
> >  	 * as to allow insertion of a naturally aligned timestamp.
> >  	 */
> > -	u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8);
> > +	struct {
> > +		union {
> > +			__le16 data[3];
> > +			__le32 fifo_ts;
> > +		};
> > +		aligned_s64 timestamp;
> > +	} iio_buff = { };

you can get rid of space between brackets.

> >  	u8 tag;
> >  	bool reset_ts = false;
> >  	int i, err, read_len;
> > @@ -648,7 +652,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
> >  
> >  		for (i = 0; i < pattern_len;
> >  		     i += ST_LSM6DSX_TAGGED_SAMPLE_SIZE) {
> > -			memcpy(iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
> > +			memcpy(&iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
> >  			       ST_LSM6DSX_SAMPLE_SIZE);
> >  
> >  			tag = hw->buff[i] >> 3;
> > @@ -659,7 +663,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
> >  				 * B0 = ts[7:0], B1 = ts[15:8], B2 = ts[23:16],
> >  				 * B3 = ts[31:24]
> >  				 */
> > -				ts = le32_to_cpu(*((__le32 *)iio_buff));
> > +				ts = le32_to_cpu(iio_buff.fifo_ts);
> >  				/*
> >  				 * check if hw timestamp engine is going to
> >  				 * reset (the sensor generates an interrupt
> > @@ -670,7 +674,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,
> > +							    iio_buff.data,
> >  							    ts);
> >  			}
> >  		}
> 

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

Re: [PATCH v7 1/6] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO

[Jonathan Cameron]

On Sat, 7 Mar 2026 16:23:33 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> > On Wed,  4 Mar 2026 09:06:00 +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. To avoid casting to __le16 * when checking sample values, clean
> > > up the type representation for data read from the FIFO.
> > > 
> > > Fixes: 960506ed2c69 ("iio: imu: st_lsm6dsx: enable drdy-mask if available")
> > > Signed-off-by: Francesco Lavra <flavra@baylibre.com>  
> > Looks fine to me, but looking for a Lorenzo tag ideally given it's not
> > a particularly trivial fix!
> > 
> > Jonathan  
> 
> Hi Francesco,
> 
> thx for fixing this, I think the patch is fine, just a couple of nits inline if
> you need to repost.
> 
> Regards,
> Lorenzo
> 
> Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> > > ---
> > >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 +++++++++++--------
> > >  1 file changed, 14 insertions(+), 9 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..a6ee2da5a06c 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > @@ -365,8 +365,6 @@ static inline int st_lsm6dsx_read_block(struct st_lsm6dsx_hw *hw, u8 addr,
> > >  	return 0;
> > >  }
> > >  
> > > -#define ST_LSM6DSX_IIO_BUFF_SIZE	(ALIGN(ST_LSM6DSX_SAMPLE_SIZE, \
> > > -					       sizeof(s64)) + sizeof(s64))
> > >  /**
> > >   * st_lsm6dsx_read_fifo() - hw FIFO read routine
> > >   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
> > > @@ -539,14 +537,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;  
> 
> what about moving this check to a dedicated routine? Like
> st_lsm6dsx_check_data() or similar?

Make sense. Let's do that as part of this fix.

> 
> > >  
> > >  	/*
> > > @@ -609,7 +607,13 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
> > >  	 * must be passed a buffer that is aligned to 8 bytes so
> > >  	 * as to allow insertion of a naturally aligned timestamp.
> > >  	 */
> > > -	u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8);
> > > +	struct {
> > > +		union {
> > > +			__le16 data[3];
> > > +			__le32 fifo_ts;
> > > +		};
> > > +		aligned_s64 timestamp;
> > > +	} iio_buff = { };  
> 
> you can get rid of space between brackets.

Prefer not on this.  I'm trying to standardize on having the space as it looks
more normal in some other usecases than no space.  I had to a pick a style and
this is the one I went with a year or so back.

> 
> > >  	u8 tag;
> > >  	bool reset_ts = false;
> > >  	int i, err, read_len;
> > > @@ -648,7 +652,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
> > >  
> > >  		for (i = 0; i < pattern_len;
> > >  		     i += ST_LSM6DSX_TAGGED_SAMPLE_SIZE) {
> > > -			memcpy(iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
> > > +			memcpy(&iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
> > >  			       ST_LSM6DSX_SAMPLE_SIZE);
> > >  
> > >  			tag = hw->buff[i] >> 3;
> > > @@ -659,7 +663,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
> > >  				 * B0 = ts[7:0], B1 = ts[15:8], B2 = ts[23:16],
> > >  				 * B3 = ts[31:24]
> > >  				 */
> > > -				ts = le32_to_cpu(*((__le32 *)iio_buff));
> > > +				ts = le32_to_cpu(iio_buff.fifo_ts);
> > >  				/*
> > >  				 * check if hw timestamp engine is going to
> > >  				 * reset (the sensor generates an interrupt
> > > @@ -670,7 +674,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,
> > > +							    iio_buff.data,
> > >  							    ts);
> > >  			}
> > >  		}  
> >   

Re: [PATCH v7 1/6] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO

[Lorenzo Bianconi]

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

> On Sat, 7 Mar 2026 16:23:33 +0100
> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> 
> > > On Wed,  4 Mar 2026 09:06:00 +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. To avoid casting to __le16 * when checking sample values, clean
> > > > up the type representation for data read from the FIFO.
> > > > 
> > > > Fixes: 960506ed2c69 ("iio: imu: st_lsm6dsx: enable drdy-mask if available")
> > > > Signed-off-by: Francesco Lavra <flavra@baylibre.com>  
> > > Looks fine to me, but looking for a Lorenzo tag ideally given it's not
> > > a particularly trivial fix!
> > > 
> > > Jonathan  
> > 
> > Hi Francesco,
> > 
> > thx for fixing this, I think the patch is fine, just a couple of nits inline if
> > you need to repost.
> > 
> > Regards,
> > Lorenzo
> > 
> > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > 
> > > > ---
> > > >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 +++++++++++--------
> > > >  1 file changed, 14 insertions(+), 9 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..a6ee2da5a06c 100644
> > > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > @@ -365,8 +365,6 @@ static inline int st_lsm6dsx_read_block(struct st_lsm6dsx_hw *hw, u8 addr,
> > > >  	return 0;
> > > >  }
> > > >  
> > > > -#define ST_LSM6DSX_IIO_BUFF_SIZE	(ALIGN(ST_LSM6DSX_SAMPLE_SIZE, \
> > > > -					       sizeof(s64)) + sizeof(s64))
> > > >  /**
> > > >   * st_lsm6dsx_read_fifo() - hw FIFO read routine
> > > >   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
> > > > @@ -539,14 +537,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;  
> > 
> > what about moving this check to a dedicated routine? Like
> > st_lsm6dsx_check_data() or similar?
> 
> Make sense. Let's do that as part of this fix.
> 
> > 
> > > >  
> > > >  	/*
> > > > @@ -609,7 +607,13 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
> > > >  	 * must be passed a buffer that is aligned to 8 bytes so
> > > >  	 * as to allow insertion of a naturally aligned timestamp.
> > > >  	 */
> > > > -	u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8);
> > > > +	struct {
> > > > +		union {
> > > > +			__le16 data[3];
> > > > +			__le32 fifo_ts;
> > > > +		};
> > > > +		aligned_s64 timestamp;
> > > > +	} iio_buff = { };  
> > 
> > you can get rid of space between brackets.
> 
> Prefer not on this.  I'm trying to standardize on having the space as it looks
> more normal in some other usecases than no space.  I had to a pick a style and
> this is the one I went with a year or so back.

ack, that's fine in this case (I missed that).

Regards,
Lorenzo

> 
> > 
> > > >  	u8 tag;
> > > >  	bool reset_ts = false;
> > > >  	int i, err, read_len;
> > > > @@ -648,7 +652,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
> > > >  
> > > >  		for (i = 0; i < pattern_len;
> > > >  		     i += ST_LSM6DSX_TAGGED_SAMPLE_SIZE) {
> > > > -			memcpy(iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
> > > > +			memcpy(&iio_buff, &hw->buff[i + ST_LSM6DSX_TAG_SIZE],
> > > >  			       ST_LSM6DSX_SAMPLE_SIZE);
> > > >  
> > > >  			tag = hw->buff[i] >> 3;
> > > > @@ -659,7 +663,7 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
> > > >  				 * B0 = ts[7:0], B1 = ts[15:8], B2 = ts[23:16],
> > > >  				 * B3 = ts[31:24]
> > > >  				 */
> > > > -				ts = le32_to_cpu(*((__le32 *)iio_buff));
> > > > +				ts = le32_to_cpu(iio_buff.fifo_ts);
> > > >  				/*
> > > >  				 * check if hw timestamp engine is going to
> > > >  				 * reset (the sensor generates an interrupt
> > > > @@ -670,7 +674,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,
> > > > +							    iio_buff.data,
> > > >  							    ts);
> > > >  			}
> > > >  		}  
> > >   
> 

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

[PATCH v7 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

[Francesco Lavra]

This field is used to differentiate between signed and unsigned integers.
A following commit will extend its use in order to add support for non-
integer scan elements; therefore, replace it with a union that contains a
more generic 'format' field. This union will be dropped when all drivers
are changed to use the format field.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 Documentation/driver-api/iio/buffers.rst | 4 ++--
 include/linux/iio/iio.h                  | 7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/driver-api/iio/buffers.rst b/Documentation/driver-api/iio/buffers.rst
index 63f364e862d1..f36e6d00173f 100644
--- a/Documentation/driver-api/iio/buffers.rst
+++ b/Documentation/driver-api/iio/buffers.rst
@@ -78,7 +78,7 @@ fields in iio_chan_spec definition::
    /* other members */
            int scan_index
            struct {
-                   char sign;
+                   char format;
                    u8 realbits;
                    u8 storagebits;
                    u8 shift;
@@ -98,7 +98,7 @@ following channel definition::
 		   /* other stuff here */
 		   .scan_index = 0,
 		   .scan_type = {
-		           .sign = 's',
+		           .format = 's',
 			   .realbits = 12,
 			   .storagebits = 16,
 			   .shift = 4,
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index a9ecff191bd9..61f1dfc14e02 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -178,7 +178,7 @@ struct iio_event_spec {
 
 /**
  * struct iio_scan_type - specification for channel data format in buffer
- * @sign:		's' or 'u' to specify signed or unsigned
+ * @format:		(signed or unsigned) integer, or floating point
  * @realbits:		Number of valid bits of data
  * @storagebits:	Realbits + padding
  * @shift:		Shift right by this before masking out realbits.
@@ -189,7 +189,10 @@ struct iio_event_spec {
  * @endianness:		little or big endian
  */
 struct iio_scan_type {
-	char	sign;
+	union {
+		char sign;
+		char format;
+	};
 	u8	realbits;
 	u8	storagebits;
 	u8	shift;
-- 
2.39.5

Re: [PATCH v7 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

[David Lechner]

On 3/4/26 2:06 AM, Francesco Lavra wrote:
> This field is used to differentiate between signed and unsigned integers.
> A following commit will extend its use in order to add support for non-
> integer scan elements; therefore, replace it with a union that contains a
> more generic 'format' field. This union will be dropped when all drivers
> are changed to use the format field.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  Documentation/driver-api/iio/buffers.rst | 4 ++--
>  include/linux/iio/iio.h                  | 7 +++++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/driver-api/iio/buffers.rst b/Documentation/driver-api/iio/buffers.rst
> index 63f364e862d1..f36e6d00173f 100644
> --- a/Documentation/driver-api/iio/buffers.rst
> +++ b/Documentation/driver-api/iio/buffers.rst
> @@ -78,7 +78,7 @@ fields in iio_chan_spec definition::
>     /* other members */
>             int scan_index
>             struct {
> -                   char sign;
> +                   char format;
>                     u8 realbits;
>                     u8 storagebits;
>                     u8 shift;
> @@ -98,7 +98,7 @@ following channel definition::
>  		   /* other stuff here */
>  		   .scan_index = 0,
>  		   .scan_type = {
> -		           .sign = 's',
> +		           .format = 's',
>  			   .realbits = 12,
>  			   .storagebits = 16,
>  			   .shift = 4,
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index a9ecff191bd9..61f1dfc14e02 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -178,7 +178,7 @@ struct iio_event_spec {
>  
>  /**
>   * struct iio_scan_type - specification for channel data format in buffer
> - * @sign:		's' or 'u' to specify signed or unsigned
> + * @format:		(signed or unsigned) integer, or floating point

We should keep the list of valid values here.

>   * @realbits:		Number of valid bits of data
>   * @storagebits:	Realbits + padding
>   * @shift:		Shift right by this before masking out realbits.
> @@ -189,7 +189,10 @@ struct iio_event_spec {
>   * @endianness:		little or big endian
>   */
>  struct iio_scan_type {
> -	char	sign;
> +	union {
> +		char sign;
> +		char format;

Could add some comments here to say that format should be used
in new code and sign will be removed eventually.

> +	};
>  	u8	realbits;
>  	u8	storagebits;
>  	u8	shift;

Re: [PATCH v7 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

[Jonathan Cameron]

On Wed, 4 Mar 2026 16:55:42 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 3/4/26 2:06 AM, Francesco Lavra wrote:
> > This field is used to differentiate between signed and unsigned integers.
> > A following commit will extend its use in order to add support for non-
> > integer scan elements; therefore, replace it with a union that contains a
> > more generic 'format' field. This union will be dropped when all drivers
> > are changed to use the format field.
> > 
> > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> > ---
> >  Documentation/driver-api/iio/buffers.rst | 4 ++--
> >  include/linux/iio/iio.h                  | 7 +++++--
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/iio/buffers.rst b/Documentation/driver-api/iio/buffers.rst
> > index 63f364e862d1..f36e6d00173f 100644
> > --- a/Documentation/driver-api/iio/buffers.rst
> > +++ b/Documentation/driver-api/iio/buffers.rst
> > @@ -78,7 +78,7 @@ fields in iio_chan_spec definition::
> >     /* other members */
> >             int scan_index
> >             struct {
> > -                   char sign;
> > +                   char format;
> >                     u8 realbits;
> >                     u8 storagebits;
> >                     u8 shift;
> > @@ -98,7 +98,7 @@ following channel definition::
> >  		   /* other stuff here */
> >  		   .scan_index = 0,
> >  		   .scan_type = {
> > -		           .sign = 's',
> > +		           .format = 's',
> >  			   .realbits = 12,
> >  			   .storagebits = 16,
> >  			   .shift = 4,
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index a9ecff191bd9..61f1dfc14e02 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -178,7 +178,7 @@ struct iio_event_spec {
> >  
> >  /**
> >   * struct iio_scan_type - specification for channel data format in buffer
> > - * @sign:		's' or 'u' to specify signed or unsigned

I think this is going to trigger kernel-doc warnings.  So I'd keep the docs
but mention it's deprecated in favour of format.

> > + * @format:		(signed or unsigned) integer, or floating point  
> 
> We should keep the list of valid values here.
> 
> >   * @realbits:		Number of valid bits of data
> >   * @storagebits:	Realbits + padding
> >   * @shift:		Shift right by this before masking out realbits.
> > @@ -189,7 +189,10 @@ struct iio_event_spec {
> >   * @endianness:		little or big endian
> >   */
> >  struct iio_scan_type {
> > -	char	sign;
> > +	union {
> > +		char sign;
> > +		char format;  
> 
> Could add some comments here to say that format should be used
> in new code and sign will be removed eventually.
> 
> > +	};
> >  	u8	realbits;
> >  	u8	storagebits;
> >  	u8	shift;  
> 

Re: [PATCH v7 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

[Jonathan Cameron]

On Wed,  4 Mar 2026 09:06:40 +0100
Francesco Lavra <flavra@baylibre.com> wrote:

> This field is used to differentiate between signed and unsigned integers.
> A following commit will extend its use in order to add support for non-
> integer scan elements; therefore, replace it with a union that contains a
> more generic 'format' field. This union will be dropped when all drivers
> are changed to use the format field.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  Documentation/driver-api/iio/buffers.rst | 4 ++--
>  include/linux/iio/iio.h                  | 7 +++++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/driver-api/iio/buffers.rst b/Documentation/driver-api/iio/buffers.rst
> index 63f364e862d1..f36e6d00173f 100644
> --- a/Documentation/driver-api/iio/buffers.rst
> +++ b/Documentation/driver-api/iio/buffers.rst
> @@ -78,7 +78,7 @@ fields in iio_chan_spec definition::
>     /* other members */
>             int scan_index
>             struct {
> -                   char sign;
> +                   char format;
>                     u8 realbits;
>                     u8 storagebits;
>                     u8 shift;
> @@ -98,7 +98,7 @@ following channel definition::
>  		   /* other stuff here */
>  		   .scan_index = 0,
>  		   .scan_type = {
> -		           .sign = 's',
> +		           .format = 's',
This made me wonder if we should use this opportunity to restrict
the flexibility of what can go in .format via some defines?

#define IIO_SCAN_FORMAT_SIGNED_INT 's'
#define IIO_SCAN_FORMAT_UNSIGNED_INT 'u'
#define IIO_SCAN_FORMAT_FLOAT 'f'

or something like that.  Given the aim is to convert all current
instances we can move to the macros as part of switching from
sign to format.


>  			   .realbits = 12,
>  			   .storagebits = 16,
>  			   .shift = 4,

[PATCH v7 3/6] iio: tools: Add support for floating-point numbers in buffer scan elements

[Francesco Lavra]

A subsequent commit will add floating-point support to the ABI; enhance
the iio_generic_buffer tool to be able to parse this new data format.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 tools/iio/iio_generic_buffer.c | 56 +++++++++++++++++++++++++++++-----
 tools/iio/iio_utils.c          |  8 ++---
 tools/iio/iio_utils.h          |  4 +--
 3 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index bc82bb6a7a2a..39c13b9ad20b 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -89,7 +89,7 @@ static void print1byte(uint8_t input, struct iio_channel_info *info)
 	 */
 	input >>= info->shift;
 	input &= info->mask;
-	if (info->is_signed) {
+	if (info->format == 's') {
 		int8_t val = (int8_t)(input << (8 - info->bits_used)) >>
 			     (8 - info->bits_used);
 		printf("%05f ", ((float)val + info->offset) * info->scale);
@@ -112,12 +112,26 @@ static void print2byte(uint16_t input, struct iio_channel_info *info)
 	 */
 	input >>= info->shift;
 	input &= info->mask;
-	if (info->is_signed) {
+	switch (info->format) {
+	case 's': {
 		int16_t val = (int16_t)(input << (16 - info->bits_used)) >>
 			      (16 - info->bits_used);
 		printf("%05f ", ((float)val + info->offset) * info->scale);
-	} else {
+		break;
+	}
+	case 'u':
 		printf("%05f ", ((float)input + info->offset) * info->scale);
+		break;
+	case 'f': {
+		union {
+			uint16_t u;
+			__fp16 f;
+		} converter;
+
+		converter.u = input;
+		printf("%05f ", ((float)converter.f + info->offset) * info->scale);
+		break;
+	}
 	}
 }
 
@@ -135,12 +149,26 @@ static void print4byte(uint32_t input, struct iio_channel_info *info)
 	 */
 	input >>= info->shift;
 	input &= info->mask;
-	if (info->is_signed) {
+	switch (info->format) {
+	case 's': {
 		int32_t val = (int32_t)(input << (32 - info->bits_used)) >>
 			      (32 - info->bits_used);
 		printf("%05f ", ((float)val + info->offset) * info->scale);
-	} else {
+		break;
+	}
+	case 'u':
 		printf("%05f ", ((float)input + info->offset) * info->scale);
+		break;
+	case 'f': {
+		union {
+			uint32_t u;
+			float f;
+		} converter;
+
+		converter.u = input;
+		printf("%05f ", (converter.f + info->offset) * info->scale);
+		break;
+	}
 	}
 }
 
@@ -158,7 +186,8 @@ static void print8byte(uint64_t input, struct iio_channel_info *info)
 	 */
 	input >>= info->shift;
 	input &= info->mask;
-	if (info->is_signed) {
+	switch (info->format) {
+	case 's': {
 		int64_t val = (int64_t)(input << (64 - info->bits_used)) >>
 			      (64 - info->bits_used);
 		/* special case for timestamp */
@@ -167,8 +196,21 @@ static void print8byte(uint64_t input, struct iio_channel_info *info)
 		else
 			printf("%05f ",
 			       ((float)val + info->offset) * info->scale);
-	} else {
+		break;
+	}
+	case 'u':
 		printf("%05f ", ((float)input + info->offset) * info->scale);
+		break;
+	case 'f': {
+		union {
+			uint64_t u;
+			double f;
+		} converter;
+
+		converter.u = input;
+		printf("%05f ", (converter.f + info->offset) * info->scale);
+		break;
+	}
 	}
 }
 
diff --git a/tools/iio/iio_utils.c b/tools/iio/iio_utils.c
index c5c5082cb24e..0643a943b2ad 100644
--- a/tools/iio/iio_utils.c
+++ b/tools/iio/iio_utils.c
@@ -70,7 +70,7 @@ int iioutils_break_up_name(const char *full_name, char **generic_name)
 
 /**
  * iioutils_get_type() - find and process _type attribute data
- * @is_signed: output whether channel is signed
+ * @format: output channel format
  * @bytes: output how many bytes the channel storage occupies
  * @bits_used: output number of valid bits of data
  * @shift: output amount of bits to shift right data before applying bit mask
@@ -83,7 +83,7 @@ int iioutils_break_up_name(const char *full_name, char **generic_name)
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-static int iioutils_get_type(unsigned int *is_signed, unsigned int *bytes,
+static int iioutils_get_type(unsigned int *format, unsigned int *bytes,
 			     unsigned int *bits_used, unsigned int *shift,
 			     uint64_t *mask, unsigned int *be,
 			     const char *device_dir, int buffer_idx,
@@ -162,7 +162,7 @@ static int iioutils_get_type(unsigned int *is_signed, unsigned int *bytes,
 			else
 				*mask = (1ULL << *bits_used) - 1ULL;
 
-			*is_signed = (signchar == 's');
+			*format = signchar;
 			if (fclose(sysfsfp)) {
 				ret = -errno;
 				fprintf(stderr, "Failed to close %s\n",
@@ -487,7 +487,7 @@ int build_channel_array(const char *device_dir, int buffer_idx,
 			if ((ret < 0) && (ret != -ENOENT))
 				goto error_cleanup_array;
 
-			ret = iioutils_get_type(&current->is_signed,
+			ret = iioutils_get_type(&current->format,
 						&current->bytes,
 						&current->bits_used,
 						&current->shift,
diff --git a/tools/iio/iio_utils.h b/tools/iio/iio_utils.h
index 663c94a6c705..e09116af194b 100644
--- a/tools/iio/iio_utils.h
+++ b/tools/iio/iio_utils.h
@@ -32,7 +32,7 @@ extern const char *iio_dir;
  * @shift: amount of bits to shift right data before applying bit mask
  * @mask: a bit mask for the raw output
  * @be: flag if data is big endian
- * @is_signed: is the raw value stored signed
+ * @format: format of the raw value
  * @location: data offset for this channel inside the buffer (in bytes)
  **/
 struct iio_channel_info {
@@ -46,7 +46,7 @@ struct iio_channel_info {
 	unsigned shift;
 	uint64_t mask;
 	unsigned be;
-	unsigned is_signed;
+	unsigned format;
 	unsigned location;
 };
 
-- 
2.39.5

Re: [PATCH v7 3/6] iio: tools: Add support for floating-point numbers in buffer scan elements

[David Lechner]

On 3/4/26 2:06 AM, Francesco Lavra wrote:
> A subsequent commit will add floating-point support to the ABI; enhance
> the iio_generic_buffer tool to be able to parse this new data format.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  tools/iio/iio_generic_buffer.c | 56 +++++++++++++++++++++++++++++-----
>  tools/iio/iio_utils.c          |  8 ++---
>  tools/iio/iio_utils.h          |  4 +--
>  3 files changed, 55 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index bc82bb6a7a2a..39c13b9ad20b 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -89,7 +89,7 @@ static void print1byte(uint8_t input, struct iio_channel_info *info)
>  	 */
>  	input >>= info->shift;
>  	input &= info->mask;
> -	if (info->is_signed) {
> +	if (info->format == 's') {
>  		int8_t val = (int8_t)(input << (8 - info->bits_used)) >>
>  			     (8 - info->bits_used);
>  		printf("%05f ", ((float)val + info->offset) * info->scale);
> @@ -112,12 +112,26 @@ static void print2byte(uint16_t input, struct iio_channel_info *info)
>  	 */
>  	input >>= info->shift;
>  	input &= info->mask;
> -	if (info->is_signed) {
> +	switch (info->format) {
> +	case 's': {
>  		int16_t val = (int16_t)(input << (16 - info->bits_used)) >>
>  			      (16 - info->bits_used);
>  		printf("%05f ", ((float)val + info->offset) * info->scale);
> -	} else {
> +		break;
> +	}
> +	case 'u':
>  		printf("%05f ", ((float)input + info->offset) * info->scale);
> +		break;
> +	case 'f': {
> +		union {
> +			uint16_t u;
> +			__fp16 f;

Do we need to check for compiler support here since this data type might not
be supported on all targets?

Also wondering if _Float16 would be better than __fp16.

> +		} converter;
> +
> +		converter.u = input;
> +		printf("%05f ", ((float)converter.f + info->offset) * info->scale);
> +		break;
> +	}
>  	}
>  }
>  

...

> @@ -46,7 +46,7 @@ struct iio_channel_info {
>  	unsigned shift;
>  	uint64_t mask;
>  	unsigned be;
> -	unsigned is_signed;
> +	unsigned format;

char would be more natural.

>  	unsigned location;
>  };
>  

[PATCH v7 4/6] iio: ABI: Add support for floating-point numbers in buffer scan elements

[Francesco Lavra]

In the data storage description of a scan element, the first character
after the colon can have the values 's' and 'u' to specify signed and
unsigned integers, respectively.
Add 'f' as an allowed value to specify floating-point numbers formatted
according to the IEEE 754 standard.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 Documentation/ABI/testing/sysfs-bus-iio  | 33 +++++++++++++-----------
 Documentation/driver-api/iio/buffers.rst |  3 ++-
 Documentation/iio/iio_devbuf.rst         |  3 ++-
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 5f87dcee78f7..bd6c3305dd2b 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1510,21 +1510,24 @@ Contact:	linux-iio@vger.kernel.org
 Description:
 		Description of the scan element data storage within the buffer
 		and hence the form in which it is read from user-space.
-		Form is [be|le]:[s|u]bits/storagebits[>>shift].
-		be or le specifies big or little endian. s or u specifies if
-		signed (2's complement) or unsigned. bits is the number of bits
-		of data and storagebits is the space (after padding) that it
-		occupies in the buffer. shift if specified, is the shift that
-		needs to be applied prior to masking out unused bits. Some
-		devices put their data in the middle of the transferred elements
-		with additional information on both sides.  Note that some
-		devices will have additional information in the unused bits
-		so to get a clean value, the bits value must be used to mask
-		the buffer output value appropriately.  The storagebits value
-		also specifies the data alignment.  So s48/64>>2 will be a
-		signed 48 bit integer stored in a 64 bit location aligned to
-		a 64 bit boundary. To obtain the clean value, shift right 2
-		and apply a mask to zero the top 16 bits of the result.
+		Form is [be|le]:[f|s|u]bits/storagebits[>>shift].
+		be or le specifies big or little endian. f means floating-point
+		(IEEE 754 binary format), s means signed (2's complement), u means
+		unsigned. bits is the number of bits of data and storagebits is the
+		space (after padding) that it occupies in the buffer; when using a
+		floating-point format, bits must be one of the width values defined
+		in the IEEE 754 standard for binary interchange formats (e.g. 16
+		indicates the binary16 format for half-precision numbers). shift,
+		if specified, is the shift that needs to be applied prior to
+		masking out unused bits. Some devices put their data in the middle
+		of the transferred elements with additional information on both
+		sides. Note that some devices will have additional information in
+		the unused bits, so to get a clean value the bits value must be
+		used to mask the buffer output value appropriately. The storagebits
+		value also specifies the data alignment. So s48/64>>2 will be a
+		signed 48 bit integer stored in a 64 bit location aligned to a 64
+		bit boundary. To obtain the clean value, shift right 2 and apply a
+		mask to zero the top 16 bits of the result.
 		For other storage combinations this attribute will be extended
 		appropriately.
 
diff --git a/Documentation/driver-api/iio/buffers.rst b/Documentation/driver-api/iio/buffers.rst
index f36e6d00173f..2fc9c2951a9d 100644
--- a/Documentation/driver-api/iio/buffers.rst
+++ b/Documentation/driver-api/iio/buffers.rst
@@ -37,9 +37,10 @@ directory contains attributes of the following form:
 * :file:`index`, the scan_index of the channel.
 * :file:`type`, 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] .
+  Format is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift] .
 
   * *be* or *le*, specifies big or little endian.
+  * *f*, specifies if floating-point.
   * *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
diff --git a/Documentation/iio/iio_devbuf.rst b/Documentation/iio/iio_devbuf.rst
index dca1f0200b0d..e91730fa3cea 100644
--- a/Documentation/iio/iio_devbuf.rst
+++ b/Documentation/iio/iio_devbuf.rst
@@ -83,9 +83,10 @@ and the relevant _type attributes to establish the data storage format.
 
 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 userspace. Format
-is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
+is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift], where:
 
 - **be** or **le** specifies big or little-endian.
+- **f** specifies if floating-point.
 - **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
-- 
2.39.5

Re: [PATCH v7 4/6] iio: ABI: Add support for floating-point numbers in buffer scan elements

[David Lechner]

On 3/4/26 2:06 AM, Francesco Lavra wrote:
> In the data storage description of a scan element, the first character
> after the colon can have the values 's' and 'u' to specify signed and
> unsigned integers, respectively.
> Add 'f' as an allowed value to specify floating-point numbers formatted
> according to the IEEE 754 standard.
> 
...

> diff --git a/Documentation/driver-api/iio/buffers.rst b/Documentation/driver-api/iio/buffers.rst
> index f36e6d00173f..2fc9c2951a9d 100644
> --- a/Documentation/driver-api/iio/buffers.rst
> +++ b/Documentation/driver-api/iio/buffers.rst
> @@ -37,9 +37,10 @@ directory contains attributes of the following form:
>  * :file:`index`, the scan_index of the channel.
>  * :file:`type`, 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] .
> +  Format is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift] .
>  
>    * *be* or *le*, specifies big or little endian.
> +  * *f*, specifies if floating-point.
>    * *s* or *u*, specifies if signed (2's complement) or unsigned.

I would keep all of the format options on one bullet point.

>    * *bits*, is the number of valid data bits.
>    * *storagebits*, is the number of bits (after padding) that it occupies in the
> diff --git a/Documentation/iio/iio_devbuf.rst b/Documentation/iio/iio_devbuf.rst
> index dca1f0200b0d..e91730fa3cea 100644
> --- a/Documentation/iio/iio_devbuf.rst
> +++ b/Documentation/iio/iio_devbuf.rst
> @@ -83,9 +83,10 @@ and the relevant _type attributes to establish the data storage format.
>  
>  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 userspace. Format
> -is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
> +is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift], where:
>  
>  - **be** or **le** specifies big or little-endian.
> +- **f** specifies if floating-point.
>  - **s** or **u** specifies if signed (2's complement) or unsigned.

same here

>  - **bits** is the number of valid data bits.
>  - **storagebits** is the number of bits (after padding) that it occupies in the

Re: [PATCH v7 4/6] iio: ABI: Add support for floating-point numbers in buffer scan elements

[Francesco Lavra]

On Wed, 2026-03-04 at 16:45 -0600, David Lechner wrote:
> On 3/4/26 2:06 AM, Francesco Lavra wrote:
> > In the data storage description of a scan element, the first character
> > after the colon can have the values 's' and 'u' to specify signed and
> > unsigned integers, respectively.
> > Add 'f' as an allowed value to specify floating-point numbers formatted
> > according to the IEEE 754 standard.
> > 
> ...
> 
> > diff --git a/Documentation/driver-api/iio/buffers.rst
> > b/Documentation/driver-api/iio/buffers.rst
> > index f36e6d00173f..2fc9c2951a9d 100644
> > --- a/Documentation/driver-api/iio/buffers.rst
> > +++ b/Documentation/driver-api/iio/buffers.rst
> > @@ -37,9 +37,10 @@ directory contains attributes of the following form:
> >  * :file:`index`, the scan_index of the channel.
> >  * :file:`type`, 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] .
> > +  Format is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift] .
> >  
> >    * *be* or *le*, specifies big or little endian.
> > +  * *f*, specifies if floating-point.
> >    * *s* or *u*, specifies if signed (2's complement) or unsigned.
> 
> I would keep all of the format options on one bullet point.

That's what I did initially, but Andy suggested doing differently [1].


> >    * *bits*, is the number of valid data bits.
> >    * *storagebits*, is the number of bits (after padding) that it
> > occupies in the
> > diff --git a/Documentation/iio/iio_devbuf.rst
> > b/Documentation/iio/iio_devbuf.rst
> > index dca1f0200b0d..e91730fa3cea 100644
> > --- a/Documentation/iio/iio_devbuf.rst
> > +++ b/Documentation/iio/iio_devbuf.rst
> > @@ -83,9 +83,10 @@ and the relevant _type attributes to establish the
> > data storage format.
> >  
> >  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
> > userspace. Format
> > -is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
> > +is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift], where:
> >  
> >  - **be** or **le** specifies big or little-endian.
> > +- **f** specifies if floating-point.
> >  - **s** or **u** specifies if signed (2's complement) or unsigned.
> 
> same here

[1] https://lore.kernel.org/linux-iio/aZ7dCdLs5xcJ4UGW@smile.fi.intel.com/
> 

Re: [PATCH v7 4/6] iio: ABI: Add support for floating-point numbers in buffer scan elements

[Andy Shevchenko]

On Thu, Mar 5, 2026 at 11:09 AM Francesco Lavra <flavra@baylibre.com> wrote:
> On Wed, 2026-03-04 at 16:45 -0600, David Lechner wrote:
> > On 3/4/26 2:06 AM, Francesco Lavra wrote:
> > > In the data storage description of a scan element, the first character
> > > after the colon can have the values 's' and 'u' to specify signed and
> > > unsigned integers, respectively.
> > > Add 'f' as an allowed value to specify floating-point numbers formatted
> > > according to the IEEE 754 standard.

...

> > > -  Format is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift] .
> > > +  Format is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift] .
> > >
> > >    * *be* or *le*, specifies big or little endian.
> > > +  * *f*, specifies if floating-point.
> > >    * *s* or *u*, specifies if signed (2's complement) or unsigned.
> >
> > I would keep all of the format options on one bullet point.
>
> That's what I did initially, but Andy suggested doing differently [1].

And still I think it's better to not mix them. The floating in the
same sentence is confusing (along with 2's complement mention and
sign).

...

> > > -is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
> > > +is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift], where:
> > >
> > >  - **be** or **le** specifies big or little-endian.
> > > +- **f** specifies if floating-point.
> > >  - **s** or **u** specifies if signed (2's complement) or unsigned.
> >
> > same here
>
> [1] https://lore.kernel.org/linux-iio/aZ7dCdLs5xcJ4UGW@smile.fi.intel.com/

Same here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 4/6] iio: ABI: Add support for floating-point numbers in buffer scan elements

[David Lechner]

On 3/5/26 3:23 AM, Andy Shevchenko wrote:
> On Thu, Mar 5, 2026 at 11:09 AM Francesco Lavra <flavra@baylibre.com> wrote:
>> On Wed, 2026-03-04 at 16:45 -0600, David Lechner wrote:
>>> On 3/4/26 2:06 AM, Francesco Lavra wrote:
>>>> In the data storage description of a scan element, the first character
>>>> after the colon can have the values 's' and 'u' to specify signed and
>>>> unsigned integers, respectively.
>>>> Add 'f' as an allowed value to specify floating-point numbers formatted
>>>> according to the IEEE 754 standard.
> 
> ...
> 
>>>> -  Format is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift] .
>>>> +  Format is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift] .
>>>>
>>>>    * *be* or *le*, specifies big or little endian.
>>>> +  * *f*, specifies if floating-point.
>>>>    * *s* or *u*, specifies if signed (2's complement) or unsigned.
>>>
>>> I would keep all of the format options on one bullet point.
>>
>> That's what I did initially, but Andy suggested doing differently [1].
> 
> And still I think it's better to not mix them. The floating in the
> same sentence is confusing (along with 2's complement mention and
> sign).

Then I would split up all 3. It is strange to mix some and not
all.

> 
> ...
> 
>>>> -is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
>>>> +is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift], where:
>>>>
>>>>  - **be** or **le** specifies big or little-endian.
>>>> +- **f** specifies if floating-point.
>>>>  - **s** or **u** specifies if signed (2's complement) or unsigned.
>>>
>>> same here
>>
>> [1] https://lore.kernel.org/linux-iio/aZ7dCdLs5xcJ4UGW@smile.fi.intel.com/
> 
> Same here.
> 

Re: [PATCH v7 4/6] iio: ABI: Add support for floating-point numbers in buffer scan elements

[Andy Shevchenko]

On Thu, Mar 05, 2026 at 08:37:48AM -0600, David Lechner wrote:
> On 3/5/26 3:23 AM, Andy Shevchenko wrote:
> > On Thu, Mar 5, 2026 at 11:09 AM Francesco Lavra <flavra@baylibre.com> wrote:
> >> On Wed, 2026-03-04 at 16:45 -0600, David Lechner wrote:
> >>> On 3/4/26 2:06 AM, Francesco Lavra wrote:
> >>>> In the data storage description of a scan element, the first character
> >>>> after the colon can have the values 's' and 'u' to specify signed and
> >>>> unsigned integers, respectively.
> >>>> Add 'f' as an allowed value to specify floating-point numbers formatted
> >>>> according to the IEEE 754 standard.

...

> >>>> -  Format is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift] .
> >>>> +  Format is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift] .
> >>>>
> >>>>    * *be* or *le*, specifies big or little endian.
> >>>> +  * *f*, specifies if floating-point.
> >>>>    * *s* or *u*, specifies if signed (2's complement) or unsigned.
> >>>
> >>> I would keep all of the format options on one bullet point.
> >>
> >> That's what I did initially, but Andy suggested doing differently [1].
> > 
> > And still I think it's better to not mix them. The floating in the
> > same sentence is confusing (along with 2's complement mention and
> > sign).
> 
> Then I would split up all 3. It is strange to mix some and not
> all.

I don't find it 'strange'. The integer are grouped together, floats do not
belong to that group.

...

> >>>> -is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
> >>>> +is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift], where:
> >>>>
> >>>>  - **be** or **le** specifies big or little-endian.
> >>>> +- **f** specifies if floating-point.
> >>>>  - **s** or **u** specifies if signed (2's complement) or unsigned.
> >>>
> >>> same here
> >>
> >> [1] https://lore.kernel.org/linux-iio/aZ7dCdLs5xcJ4UGW@smile.fi.intel.com/
> > 
> > Same here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 4/6] iio: ABI: Add support for floating-point numbers in buffer scan elements

[Jonathan Cameron]

On Fri, 6 Mar 2026 14:09:51 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Mar 05, 2026 at 08:37:48AM -0600, David Lechner wrote:
> > On 3/5/26 3:23 AM, Andy Shevchenko wrote:  
> > > On Thu, Mar 5, 2026 at 11:09 AM Francesco Lavra <flavra@baylibre.com> wrote:  
> > >> On Wed, 2026-03-04 at 16:45 -0600, David Lechner wrote:  
> > >>> On 3/4/26 2:06 AM, Francesco Lavra wrote:  
> > >>>> In the data storage description of a scan element, the first character
> > >>>> after the colon can have the values 's' and 'u' to specify signed and
> > >>>> unsigned integers, respectively.
> > >>>> Add 'f' as an allowed value to specify floating-point numbers formatted
> > >>>> according to the IEEE 754 standard.  
> 
> ...
> 
> > >>>> -  Format is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift] .
> > >>>> +  Format is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift] .
> > >>>>
> > >>>>    * *be* or *le*, specifies big or little endian.
> > >>>> +  * *f*, specifies if floating-point.
> > >>>>    * *s* or *u*, specifies if signed (2's complement) or unsigned.  
> > >>>
> > >>> I would keep all of the format options on one bullet point.  
> > >>
> > >> That's what I did initially, but Andy suggested doing differently [1].  
> > > 
> > > And still I think it's better to not mix them. The floating in the
> > > same sentence is confusing (along with 2's complement mention and
> > > sign).  
> > 
> > Then I would split up all 3. It is strange to mix some and not
> > all.  
> 
> I don't find it 'strange'. The integer are grouped together, floats do not
> belong to that group.
Maybe two paragaraphs in one bullet point?
	* *f*, specifies if floating-point.
	  *s* or *u*, specifies if signed (2's complement) or unsigned.  

Though then we'll definitely need to check it didn't break the formatting
in the docs generated from these files.

For me any of the above are fine.

	
> 
> ...
> 
> > >>>> -is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
> > >>>> +is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift], where:
> > >>>>
> > >>>>  - **be** or **le** specifies big or little-endian.
> > >>>> +- **f** specifies if floating-point.
> > >>>>  - **s** or **u** specifies if signed (2's complement) or unsigned.  
> > >>>
> > >>> same here  
> > >>
> > >> [1] https://lore.kernel.org/linux-iio/aZ7dCdLs5xcJ4UGW@smile.fi.intel.com/  
> > > 
> > > Same here.  
> 

Re: [PATCH v7 4/6] iio: ABI: Add support for floating-point numbers in buffer scan elements

[Francesco Lavra]

On Sat, 2026-03-07 at 12:51 +0000, Jonathan Cameron wrote:
> On Fri, 6 Mar 2026 14:09:51 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
> > On Thu, Mar 05, 2026 at 08:37:48AM -0600, David Lechner wrote:
> > > On 3/5/26 3:23 AM, Andy Shevchenko wrote:  
> > > > On Thu, Mar 5, 2026 at 11:09 AM Francesco Lavra
> > > > <flavra@baylibre.com> wrote:  
> > > > > On Wed, 2026-03-04 at 16:45 -0600, David Lechner wrote:  
> > > > > > On 3/4/26 2:06 AM, Francesco Lavra wrote:  
> > > > > > > In the data storage description of a scan element, the first
> > > > > > > character
> > > > > > > after the colon can have the values 's' and 'u' to specify
> > > > > > > signed and
> > > > > > > unsigned integers, respectively.
> > > > > > > Add 'f' as an allowed value to specify floating-point numbers
> > > > > > > formatted
> > > > > > > according to the IEEE 754 standard.  
> > 
> > ...
> > 
> > > > > > > -  Format is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift]
> > > > > > > .
> > > > > > > +  Format is
> > > > > > > [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift] .
> > > > > > > 
> > > > > > >    * *be* or *le*, specifies big or little endian.
> > > > > > > +  * *f*, specifies if floating-point.
> > > > > > >    * *s* or *u*, specifies if signed (2's complement) or
> > > > > > > unsigned.  
> > > > > > 
> > > > > > I would keep all of the format options on one bullet point.  
> > > > > 
> > > > > That's what I did initially, but Andy suggested doing differently
> > > > > [1].  
> > > > 
> > > > And still I think it's better to not mix them. The floating in the
> > > > same sentence is confusing (along with 2's complement mention and
> > > > sign).  
> > > 
> > > Then I would split up all 3. It is strange to mix some and not
> > > all.  
> > 
> > I don't find it 'strange'. The integer are grouped together, floats do
> > not
> > belong to that group.
> Maybe two paragaraphs in one bullet point?
>         * *f*, specifies if floating-point.
>           *s* or *u*, specifies if signed (2's complement) or unsigned.  
> 
> Though then we'll definitely need to check it didn't break the formatting
> in the docs generated from these files.

This does break the formatting in the HTML docs, where the two paragraphs
end up in the same line.

> For me any of the above are fine.

I will keep it as is.

> > ...
> > 
> > > > > > > -is [be|le]:[s|u]bits/storagebits[Xrepeat][>>shift], where:
> > > > > > > +is [be|le]:[f|s|u]bits/storagebits[Xrepeat][>>shift], where:
> > > > > > > 
> > > > > > >  - **be** or **le** specifies big or little-endian.
> > > > > > > +- **f** specifies if floating-point.
> > > > > > >  - **s** or **u** specifies if signed (2's complement) or
> > > > > > > unsigned.  
> > > > > > 
> > > > > > same here  
> > > > > 
> > > > > [1]
> > > > > https://lore.kernel.org/linux-iio/aZ7dCdLs5xcJ4UGW@smile.fi.intel.com/
> > > > >  
> > > > 
> > > > Same here.

Same here.

[PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Francesco Lavra]

This modifier applies to the IIO_ROT channel type, and indicates a data
representation that specifies the {x, y, z} components of the normalized
quaternion vector.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 13 +++++++++++++
 drivers/iio/industrialio-core.c         |  1 +
 include/uapi/linux/iio/types.h          |  1 +
 tools/iio/iio_event_monitor.c           |  1 +
 4 files changed, 16 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index bd6c3305dd2b..54e38ebb044d 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1755,6 +1755,19 @@ Description:
 		measurement from channel Y. Units after application of scale and
 		offset are milliamps.
 
+What:		/sys/bus/iio/devices/iio:deviceX/in_rot_partial_quaternion_raw
+KernelVersion:	7.1
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Raw value of {x, y, z} components of the quaternion vector. These
+		components represent the axis about which a rotation occurs, and are
+		subject to the following costraints:
+		- the quaternion vector is normalized, i.e. x^2 + y^2 + z^2 + w^2 = 1
+		- the rotation angle is within the [-180, 180] range, i.e. the w
+		  component (which represents the amount of rotation) is non-negative
+		These constraints allow the w value to be calculated from the other
+		components: w = sqrt(1 - (x^2 + y^2 + z^2)).
+
 What:		/sys/.../iio:deviceX/in_energy_en
 What:		/sys/.../iio:deviceX/in_distance_en
 What:		/sys/.../iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_en
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 22eefd048ba9..792fbeb0dfa8 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -125,6 +125,7 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_LIGHT_UVB] = "uvb",
 	[IIO_MOD_LIGHT_DUV] = "duv",
 	[IIO_MOD_QUATERNION] = "quaternion",
+	[IIO_MOD_PARTIAL_QUATERNION] = "partial_quaternion",
 	[IIO_MOD_TEMP_AMBIENT] = "ambient",
 	[IIO_MOD_TEMP_OBJECT] = "object",
 	[IIO_MOD_NORTH_MAGN] = "from_north_magnetic",
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 6d269b844271..c80ef7c1ed12 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -77,6 +77,7 @@ enum iio_modifier {
 	IIO_MOD_LIGHT_GREEN,
 	IIO_MOD_LIGHT_BLUE,
 	IIO_MOD_QUATERNION,
+	IIO_MOD_PARTIAL_QUATERNION,
 	IIO_MOD_TEMP_AMBIENT,
 	IIO_MOD_TEMP_OBJECT,
 	IIO_MOD_NORTH_MAGN,
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index 03ca33869ce8..db0fb75806ab 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -113,6 +113,7 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_LIGHT_UVB] = "uvb",
 	[IIO_MOD_LIGHT_DUV] = "duv",
 	[IIO_MOD_QUATERNION] = "quaternion",
+	[IIO_MOD_PARTIAL_QUATERNION] = "partial_quaternion",
 	[IIO_MOD_TEMP_AMBIENT] = "ambient",
 	[IIO_MOD_TEMP_OBJECT] = "object",
 	[IIO_MOD_NORTH_MAGN] = "from_north_magnetic",
-- 
2.39.5

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Andy Shevchenko]

On Wed, Mar 04, 2026 at 09:07:05AM +0100, Francesco Lavra wrote:
> This modifier applies to the IIO_ROT channel type, and indicates a data
> representation that specifies the {x, y, z} components of the normalized
> quaternion vector.

...

> +What:		/sys/bus/iio/devices/iio:deviceX/in_rot_partial_quaternion_raw

> +		Raw value of {x, y, z} components of the quaternion vector. These
> +		components represent the axis about which a rotation occurs, and are
> +		subject to the following costraints:
> +		- the quaternion vector is normalized, i.e. x^2 + y^2 + z^2 + w^2 = 1
> +		- the rotation angle is within the [-180, 180] range, i.e. the w
> +		  component (which represents the amount of rotation) is non-negative
> +		These constraints allow the w value to be calculated from the other
> +		components: w = sqrt(1 - (x^2 + y^2 + z^2)).

Just to double check if we really do not have a special mathematical term for
that already. If we do, I prefer to have that over odd "partial quaternion".

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Francesco Lavra]

On Wed, 2026-03-04 at 13:51 +0200, Andy Shevchenko wrote:
> On Wed, Mar 04, 2026 at 09:07:05AM +0100, Francesco Lavra wrote:
> > This modifier applies to the IIO_ROT channel type, and indicates a data
> > representation that specifies the {x, y, z} components of the
> > normalized
> > quaternion vector.
> 
> ...
> 
> > +What:          /sys/bus/iio/devices/iio:deviceX/in_rot_partial_quatern
> > ion_raw
> 
> > +               Raw value of {x, y, z} components of the quaternion
> > vector. These
> > +               components represent the axis about which a rotation
> > occurs, and are
> > +               subject to the following costraints:
> > +               - the quaternion vector is normalized, i.e. x^2 + y^2 +
> > z^2 + w^2 = 1
> > +               - the rotation angle is within the [-180, 180] range,
> > i.e. the w
> > +                 component (which represents the amount of rotation)
> > is non-negative
> > +               These constraints allow the w value to be calculated
> > from the other
> > +               components: w = sqrt(1 - (x^2 + y^2 + z^2)).
> 
> Just to double check if we really do not have a special mathematical term
> for
> that already. If we do, I prefer to have that over odd "partial
> quaternion".

A quaternion is often represented as w + xi + yj + zk, i.e. it's composed
of a real coefficient (w) and 3 imaginary coefficients (x, y, z). With this
notation, the (x, y, z) components are referred to as the imaginary part of
the quaternion.
Alternatively, a quaternion is represented as the combination of a scalar
value w and a 3D vector value (x, y, z). With this notation, the (x, y, z)
components are referred to as the vector part of the quaternion; this can
be confusing, since the quaternion as a whole is often considered as a 4D
vector.

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[David Lechner]

On 3/4/26 8:21 AM, Francesco Lavra wrote:
> On Wed, 2026-03-04 at 13:51 +0200, Andy Shevchenko wrote:
>> On Wed, Mar 04, 2026 at 09:07:05AM +0100, Francesco Lavra wrote:
>>> This modifier applies to the IIO_ROT channel type, and indicates a data
>>> representation that specifies the {x, y, z} components of the
>>> normalized
>>> quaternion vector.
>>
>> ...
>>
>>> +What:          /sys/bus/iio/devices/iio:deviceX/in_rot_partial_quatern
>>> ion_raw
>>
>>> +               Raw value of {x, y, z} components of the quaternion
>>> vector. These
>>> +               components represent the axis about which a rotation
>>> occurs, and are
>>> +               subject to the following costraints:
>>> +               - the quaternion vector is normalized, i.e. x^2 + y^2 +
>>> z^2 + w^2 = 1
>>> +               - the rotation angle is within the [-180, 180] range,
>>> i.e. the w
>>> +                 component (which represents the amount of rotation)
>>> is non-negative
>>> +               These constraints allow the w value to be calculated
>>> from the other
>>> +               components: w = sqrt(1 - (x^2 + y^2 + z^2)).
>>
>> Just to double check if we really do not have a special mathematical term
>> for
>> that already. If we do, I prefer to have that over odd "partial
>> quaternion".
> 
> A quaternion is often represented as w + xi + yj + zk, i.e. it's composed
> of a real coefficient (w) and 3 imaginary coefficients (x, y, z). With this
> notation, the (x, y, z) components are referred to as the imaginary part of
> the quaternion.
> Alternatively, a quaternion is represented as the combination of a scalar
> value w and a 3D vector value (x, y, z). With this notation, the (x, y, z)
> components are referred to as the vector part of the quaternion; this can
> be confusing, since the quaternion as a whole is often considered as a 4D
> vector.

I'm surprised there isn't a common name for this. When I went looking, the
thing that came up most often is that Doom 3 used this in their MD5 file
format. So maybe IIO_MOD_DOOM3_QUATERNION? (joking)

I do think we could come up with something better than "partial" though.

My first thought is IIO_MOD_3D_QUATERNION since it is a 3-dimentional number,
but I could see that being confusing to some since quaternions are generally
used for rotations in 3-D space.

Maybe something like IIO_MOD_3VALUE_QUATERNION or IIO_MOD_COMPRESSED_QUATERNION
or just IIO_MOD_3QUATERNION?

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Francesco Lavra]

On Wed, 2026-03-04 at 16:42 -0600, David Lechner wrote:
> On 3/4/26 8:21 AM, Francesco Lavra wrote:
> > On Wed, 2026-03-04 at 13:51 +0200, Andy Shevchenko wrote:
> > > On Wed, Mar 04, 2026 at 09:07:05AM +0100, Francesco Lavra wrote:
> > > > This modifier applies to the IIO_ROT channel type, and indicates a
> > > > data
> > > > representation that specifies the {x, y, z} components of the
> > > > normalized
> > > > quaternion vector.
> > > 
> > > ...
> > > 
> > > > +What:          /sys/bus/iio/devices/iio:deviceX/in_rot_partial_qua
> > > > tern
> > > > ion_raw
> > > 
> > > > +               Raw value of {x, y, z} components of the quaternion
> > > > vector. These
> > > > +               components represent the axis about which a
> > > > rotation
> > > > occurs, and are
> > > > +               subject to the following costraints:
> > > > +               - the quaternion vector is normalized, i.e. x^2 +
> > > > y^2 +
> > > > z^2 + w^2 = 1
> > > > +               - the rotation angle is within the [-180, 180]
> > > > range,
> > > > i.e. the w
> > > > +                 component (which represents the amount of
> > > > rotation)
> > > > is non-negative
> > > > +               These constraints allow the w value to be
> > > > calculated
> > > > from the other
> > > > +               components: w = sqrt(1 - (x^2 + y^2 + z^2)).
> > > 
> > > Just to double check if we really do not have a special mathematical
> > > term
> > > for
> > > that already. If we do, I prefer to have that over odd "partial
> > > quaternion".
> > 
> > A quaternion is often represented as w + xi + yj + zk, i.e. it's
> > composed
> > of a real coefficient (w) and 3 imaginary coefficients (x, y, z). With
> > this
> > notation, the (x, y, z) components are referred to as the imaginary
> > part of
> > the quaternion.
> > Alternatively, a quaternion is represented as the combination of a
> > scalar
> > value w and a 3D vector value (x, y, z). With this notation, the (x, y,
> > z)
> > components are referred to as the vector part of the quaternion; this
> > can
> > be confusing, since the quaternion as a whole is often considered as a
> > 4D
> > vector.
> 
> I'm surprised there isn't a common name for this. When I went looking,
> the
> thing that came up most often is that Doom 3 used this in their MD5 file
> format. So maybe IIO_MOD_DOOM3_QUATERNION? (joking)
> 
> I do think we could come up with something better than "partial" though.
> 
> My first thought is IIO_MOD_3D_QUATERNION since it is a 3-dimentional
> number,
> but I could see that being confusing to some since quaternions are
> generally
> used for rotations in 3-D space.
> 
> Maybe something like IIO_MOD_3VALUE_QUATERNION or
> IIO_MOD_COMPRESSED_QUATERNION
> or just IIO_MOD_3QUATERNION?

Or, since this represents the axis of rotation, perhaps something like
IIO_MOD_QUATERNION_AXIS?

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[David Lechner]

On 3/5/26 2:50 AM, Francesco Lavra wrote:
> On Wed, 2026-03-04 at 16:42 -0600, David Lechner wrote:
>> On 3/4/26 8:21 AM, Francesco Lavra wrote:
>>> On Wed, 2026-03-04 at 13:51 +0200, Andy Shevchenko wrote:
>>>> On Wed, Mar 04, 2026 at 09:07:05AM +0100, Francesco Lavra wrote:
>>>>> This modifier applies to the IIO_ROT channel type, and indicates a
>>>>> data
>>>>> representation that specifies the {x, y, z} components of the
>>>>> normalized
>>>>> quaternion vector.
>>>>
>>>> ...
>>>>
>>>>> +What:          /sys/bus/iio/devices/iio:deviceX/in_rot_partial_qua
>>>>> tern
>>>>> ion_raw
>>>>
>>>>> +               Raw value of {x, y, z} components of the quaternion
>>>>> vector. These
>>>>> +               components represent the axis about which a
>>>>> rotation
>>>>> occurs, and are
>>>>> +               subject to the following costraints:
>>>>> +               - the quaternion vector is normalized, i.e. x^2 +
>>>>> y^2 +
>>>>> z^2 + w^2 = 1
>>>>> +               - the rotation angle is within the [-180, 180]
>>>>> range,
>>>>> i.e. the w
>>>>> +                 component (which represents the amount of
>>>>> rotation)
>>>>> is non-negative
>>>>> +               These constraints allow the w value to be
>>>>> calculated
>>>>> from the other
>>>>> +               components: w = sqrt(1 - (x^2 + y^2 + z^2)).
>>>>
>>>> Just to double check if we really do not have a special mathematical
>>>> term
>>>> for
>>>> that already. If we do, I prefer to have that over odd "partial
>>>> quaternion".
>>>
>>> A quaternion is often represented as w + xi + yj + zk, i.e. it's
>>> composed
>>> of a real coefficient (w) and 3 imaginary coefficients (x, y, z). With
>>> this
>>> notation, the (x, y, z) components are referred to as the imaginary
>>> part of
>>> the quaternion.
>>> Alternatively, a quaternion is represented as the combination of a
>>> scalar
>>> value w and a 3D vector value (x, y, z). With this notation, the (x, y,
>>> z)
>>> components are referred to as the vector part of the quaternion; this
>>> can
>>> be confusing, since the quaternion as a whole is often considered as a
>>> 4D
>>> vector.
>>
>> I'm surprised there isn't a common name for this. When I went looking,
>> the
>> thing that came up most often is that Doom 3 used this in their MD5 file
>> format. So maybe IIO_MOD_DOOM3_QUATERNION? (joking)
>>
>> I do think we could come up with something better than "partial" though.
>>
>> My first thought is IIO_MOD_3D_QUATERNION since it is a 3-dimentional
>> number,
>> but I could see that being confusing to some since quaternions are
>> generally
>> used for rotations in 3-D space.
>>
>> Maybe something like IIO_MOD_3VALUE_QUATERNION or
>> IIO_MOD_COMPRESSED_QUATERNION
>> or just IIO_MOD_3QUATERNION?
> 
> Or, since this represents the axis of rotation, perhaps something like
> IIO_MOD_QUATERNION_AXIS?

Sounds OK to me.

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Andy Shevchenko]

On Thu, Mar 05, 2026 at 08:40:26AM -0600, David Lechner wrote:
> On 3/5/26 2:50 AM, Francesco Lavra wrote:
> > On Wed, 2026-03-04 at 16:42 -0600, David Lechner wrote:
> >> On 3/4/26 8:21 AM, Francesco Lavra wrote:
> >>> On Wed, 2026-03-04 at 13:51 +0200, Andy Shevchenko wrote:
> >>>> On Wed, Mar 04, 2026 at 09:07:05AM +0100, Francesco Lavra wrote:

...

> >>>>> +What:          /sys/bus/iio/devices/iio:deviceX/in_rot_partial_qua
> >>>>> tern
> >>>>> ion_raw
> >>>>
> >>>>> +               Raw value of {x, y, z} components of the quaternion
> >>>>> vector. These
> >>>>> +               components represent the axis about which a
> >>>>> rotation
> >>>>> occurs, and are
> >>>>> +               subject to the following costraints:
> >>>>> +               - the quaternion vector is normalized, i.e. x^2 +
> >>>>> y^2 +
> >>>>> z^2 + w^2 = 1
> >>>>> +               - the rotation angle is within the [-180, 180]
> >>>>> range,
> >>>>> i.e. the w
> >>>>> +                 component (which represents the amount of
> >>>>> rotation)
> >>>>> is non-negative
> >>>>> +               These constraints allow the w value to be
> >>>>> calculated
> >>>>> from the other
> >>>>> +               components: w = sqrt(1 - (x^2 + y^2 + z^2)).
> >>>>
> >>>> Just to double check if we really do not have a special mathematical
> >>>> term
> >>>> for
> >>>> that already. If we do, I prefer to have that over odd "partial
> >>>> quaternion".
> >>>
> >>> A quaternion is often represented as w + xi + yj + zk, i.e. it's
> >>> composed
> >>> of a real coefficient (w) and 3 imaginary coefficients (x, y, z). With
> >>> this
> >>> notation, the (x, y, z) components are referred to as the imaginary
> >>> part of
> >>> the quaternion.
> >>> Alternatively, a quaternion is represented as the combination of a
> >>> scalar
> >>> value w and a 3D vector value (x, y, z). With this notation, the (x, y,
> >>> z)
> >>> components are referred to as the vector part of the quaternion; this
> >>> can
> >>> be confusing, since the quaternion as a whole is often considered as a
> >>> 4D
> >>> vector.
> >>
> >> I'm surprised there isn't a common name for this. When I went looking,
> >> the
> >> thing that came up most often is that Doom 3 used this in their MD5 file
> >> format. So maybe IIO_MOD_DOOM3_QUATERNION? (joking)
> >>
> >> I do think we could come up with something better than "partial" though.
> >>
> >> My first thought is IIO_MOD_3D_QUATERNION since it is a 3-dimentional
> >> number,
> >> but I could see that being confusing to some since quaternions are
> >> generally
> >> used for rotations in 3-D space.
> >>
> >> Maybe something like IIO_MOD_3VALUE_QUATERNION or
> >> IIO_MOD_COMPRESSED_QUATERNION
> >> or just IIO_MOD_3QUATERNION?
> > 
> > Or, since this represents the axis of rotation, perhaps something like
> > IIO_MOD_QUATERNION_AXIS?
> 
> Sounds OK to me.

Agree, this is at least better than opaque "partial".

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[kernel test robot]

Hi Francesco,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20260303]
[also build test WARNING on v7.0-rc2]
[cannot apply to jic23-iio/togreg linus/master v7.0-rc2 v7.0-rc1 v6.19]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Francesco-Lavra/iio-imu-st_lsm6dsx-Fix-check-for-invalid-samples-from-FIFO/20260304-161927
base:   next-20260303
patch link:    https://lore.kernel.org/r/20260304080706.2844472-1-flavra%40baylibre.com
patch subject: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
docutils: docutils (Docutils 0.21.2, Python 3.13.5, on linux)
reproduce: (https://download.01.org/0day-ci/archive/20260304/202603042011.WHve1GRB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603042011.WHve1GRB-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Warning: tools/docs/documentation-file-ref-check references a file that doesn't exist: Documentation/devicetree/dt-object-internal.txt
   Warning: tools/docs/documentation-file-ref-check references a file that doesn't exist: m,^Documentation/scheduler/sched-pelt
   Warning: tools/docs/documentation-file-ref-check references a file that doesn't exist: m,(Documentation/translations/[
   Using alabaster theme
   Documentation/ABI/testing/sysfs-bus-iio:1758: ERROR: Unexpected indentation. [docutils]
>> Documentation/ABI/testing/sysfs-bus-iio:1758: WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]
   Documentation/core-api/kref:328: ./include/linux/kref.h:72: WARNING: Invalid C declaration: Expected end of definition. [error at 96]
   int kref_put_mutex (struct kref *kref, void (*release)(struct kref *kref), struct mutex *mutex) __cond_acquires(true# mutex)
   ------------------------------------------------------------------------------------------------^
   Documentation/core-api/kref:328: ./include/linux/kref.h:94: WARNING: Invalid C declaration: Expected end of definition. [error at 92]
   int kref_put_lock (struct kref *kref, void (*release)(struct kref *kref), spinlock_t *lock) __cond_acquires(true# lock)


vim +1758 Documentation/ABI/testing/sysfs-bus-iio

> 1758	What:		/sys/bus/iio/devices/iio:deviceX/in_rot_partial_quaternion_raw
  1759	KernelVersion:	7.1
  1760	Contact:	linux-iio@vger.kernel.org
  1761	Description:
  1762			Raw value of {x, y, z} components of the quaternion vector. These
  1763			components represent the axis about which a rotation occurs, and are
  1764			subject to the following costraints:
  1765			- the quaternion vector is normalized, i.e. x^2 + y^2 + z^2 + w^2 = 1
  1766			- the rotation angle is within the [-180, 180] range, i.e. the w
  1767			  component (which represents the amount of rotation) is non-negative
  1768			These constraints allow the w value to be calculated from the other
  1769			components: w = sqrt(1 - (x^2 + y^2 + z^2)).
  1770	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[David Lechner]

On 3/4/26 2:07 AM, Francesco Lavra wrote:
> This modifier applies to the IIO_ROT channel type, and indicates a data
> representation that specifies the {x, y, z} components of the normalized
> quaternion vector.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 13 +++++++++++++
>  drivers/iio/industrialio-core.c         |  1 +
>  include/uapi/linux/iio/types.h          |  1 +
>  tools/iio/iio_event_monitor.c           |  1 +
>  4 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index bd6c3305dd2b..54e38ebb044d 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1755,6 +1755,19 @@ Description:
>  		measurement from channel Y. Units after application of scale and
>  		offset are milliamps.
>  
> +What:		/sys/bus/iio/devices/iio:deviceX/in_rot_partial_quaternion_raw
> +KernelVersion:	7.1
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Raw value of {x, y, z} components of the quaternion vector. These
> +		components represent the axis about which a rotation occurs, and are
> +		subject to the following costraints:

s/costraints/constraints/

> +		- the quaternion vector is normalized, i.e. x^2 + y^2 + z^2 + w^2 = 1

Isn't this usually written with w first?

> +		- the rotation angle is within the [-180, 180] range, i.e. the w

Best to say the angle units. IIO standard unit for angle is radians.

> +		  component (which represents the amount of rotation) is non-negative
> +		These constraints allow the w value to be calculated from the other
> +		components: w = sqrt(1 - (x^2 + y^2 + z^2)).
> +
>  What:		/sys/.../iio:deviceX/in_energy_en
>  What:		/sys/.../iio:deviceX/in_distance_en
>  What:		/sys/.../iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_en
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 22eefd048ba9..792fbeb0dfa8 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -125,6 +125,7 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_LIGHT_UVB] = "uvb",
>  	[IIO_MOD_LIGHT_DUV] = "duv",
>  	[IIO_MOD_QUATERNION] = "quaternion",
> +	[IIO_MOD_PARTIAL_QUATERNION] = "partial_quaternion",

We've found that using `_` in components of the attribute name makes it
difficult to parse since you have to know all component names to know
where the break between components is. Each time we add something with
an `_` means all parsers need to be updated to handle the new name.

In other components, we've just squashed the two words together without
any space or punctuation to get around this problem. I find that rather hard
to read though.

I wonder if we could propose something different going forward to do a better
job being consistent and readable. For example, use a `-` at the word break
in components that have more than one word in the name.

>  	[IIO_MOD_TEMP_AMBIENT] = "ambient",
>  	[IIO_MOD_TEMP_OBJECT] = "object",
>  	[IIO_MOD_NORTH_MAGN] = "from_north_magnetic",
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 6d269b844271..c80ef7c1ed12 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -77,6 +77,7 @@ enum iio_modifier {
>  	IIO_MOD_LIGHT_GREEN,
>  	IIO_MOD_LIGHT_BLUE,
>  	IIO_MOD_QUATERNION,
> +	IIO_MOD_PARTIAL_QUATERNION,

This is userspace API. We can't change the values of the enum
members after this, so new members have to be added at the end.

>  	IIO_MOD_TEMP_AMBIENT,
>  	IIO_MOD_TEMP_OBJECT,
>  	IIO_MOD_NORTH_MAGN,
> diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
> index 03ca33869ce8..db0fb75806ab 100644
> --- a/tools/iio/iio_event_monitor.c
> +++ b/tools/iio/iio_event_monitor.c
> @@ -113,6 +113,7 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_LIGHT_UVB] = "uvb",
>  	[IIO_MOD_LIGHT_DUV] = "duv",
>  	[IIO_MOD_QUATERNION] = "quaternion",
> +	[IIO_MOD_PARTIAL_QUATERNION] = "partial_quaternion",
>  	[IIO_MOD_TEMP_AMBIENT] = "ambient",
>  	[IIO_MOD_TEMP_OBJECT] = "object",
>  	[IIO_MOD_NORTH_MAGN] = "from_north_magnetic",

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Andy Shevchenko]

On Wed, Mar 04, 2026 at 04:35:39PM -0600, David Lechner wrote:
> On 3/4/26 2:07 AM, Francesco Lavra wrote:

...

> > +		- the quaternion vector is normalized, i.e. x^2 + y^2 + z^2 + w^2 = 1
> 
> Isn't this usually written with w first?
> 
> > +		- the rotation angle is within the [-180, 180] range, i.e. the w
> 
> Best to say the angle units. IIO standard unit for angle is radians.
> 
> > +		  component (which represents the amount of rotation) is non-negative

Based on these, maybe the name should be

  normalised_360_quaternion

(or variations of this)? Actually "partial" quaternion some articles use
for something different as far as I can say.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Jonathan Cameron]

On Thu, 5 Mar 2026 09:04:40 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Mar 04, 2026 at 04:35:39PM -0600, David Lechner wrote:
> > On 3/4/26 2:07 AM, Francesco Lavra wrote:  
> 
> ...
> 
> > > +		- the quaternion vector is normalized, i.e. x^2 + y^2 + z^2 + w^2 = 1  
> > 
> > Isn't this usually written with w first?
> >   
> > > +		- the rotation angle is within the [-180, 180] range, i.e. the w  
> > 
> > Best to say the angle units. IIO standard unit for angle is radians.
> >   
> > > +		  component (which represents the amount of rotation) is non-negative  
> 
> Based on these, maybe the name should be
> 
>   normalised_360_quaternion
> 
> (or variations of this)? Actually "partial" quaternion some articles use
> for something different as far as I can say.

The "axis" naming for the define is probably as good as we can do for
something that is mathematically dubious in my view. There are two many
ways it could be interpreted and I'd be surprised if this does get used
in other devices, and they all pick the same way of filling in the
missing constraint.

Mind you I never much like quaternion's (part of my PhD was on Clifford /
Geometric algebra's and exponentials of bivectors are so much cleaner :)

Jonathan

> 

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Jonathan Cameron]

On Wed, 4 Mar 2026 16:35:39 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 3/4/26 2:07 AM, Francesco Lavra wrote:
> > This modifier applies to the IIO_ROT channel type, and indicates a data
> > representation that specifies the {x, y, z} components of the normalized
> > quaternion vector.
> > 
> > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-iio | 13 +++++++++++++
> >  drivers/iio/industrialio-core.c         |  1 +
> >  include/uapi/linux/iio/types.h          |  1 +
> >  tools/iio/iio_event_monitor.c           |  1 +
> >  4 files changed, 16 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index bd6c3305dd2b..54e38ebb044d 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -1755,6 +1755,19 @@ Description:
> >  		measurement from channel Y. Units after application of scale and
> >  		offset are milliamps.
> >  
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_rot_partial_quaternion_raw
> > +KernelVersion:	7.1
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Raw value of {x, y, z} components of the quaternion vector. These
> > +		components represent the axis about which a rotation occurs, and are
> > +		subject to the following costraints:  
> 
> s/costraints/constraints/
> 
> > +		- the quaternion vector is normalized, i.e. x^2 + y^2 + z^2 + w^2 = 1  
> 
> Isn't this usually written with w first?
> 
> > +		- the rotation angle is within the [-180, 180] range, i.e. the w  
> 
> Best to say the angle units. IIO standard unit for angle is radians.
> 
> > +		  component (which represents the amount of rotation) is non-negative
> > +		These constraints allow the w value to be calculated from the other
> > +		components: w = sqrt(1 - (x^2 + y^2 + z^2)).
> > +
> >  What:		/sys/.../iio:deviceX/in_energy_en
> >  What:		/sys/.../iio:deviceX/in_distance_en
> >  What:		/sys/.../iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_en
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 22eefd048ba9..792fbeb0dfa8 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -125,6 +125,7 @@ static const char * const iio_modifier_names[] = {
> >  	[IIO_MOD_LIGHT_UVB] = "uvb",
> >  	[IIO_MOD_LIGHT_DUV] = "duv",
> >  	[IIO_MOD_QUATERNION] = "quaternion",
> > +	[IIO_MOD_PARTIAL_QUATERNION] = "partial_quaternion",  
> 
> We've found that using `_` in components of the attribute name makes it
> difficult to parse since you have to know all component names to know
> where the break between components is. Each time we add something with
> an `_` means all parsers need to be updated to handle the new name.
> 
> In other components, we've just squashed the two words together without
> any space or punctuation to get around this problem. I find that rather hard
> to read though.
> 
> I wonder if we could propose something different going forward to do a better
> job being consistent and readable. For example, use a `-` at the word break
> in components that have more than one word in the name.
Whilst I can see the argument, I'm not keen on more punctuation in the filenames.
May well trip up someone's parser.  For now squashing the spaces is probably
the best option.

This made me spot a major issue though. We can only add to end of these
enums, not the middle!

> 
> >  	[IIO_MOD_TEMP_AMBIENT] = "ambient",
> >  	[IIO_MOD_TEMP_OBJECT] = "object",
> >  	[IIO_MOD_NORTH_MAGN] = "from_north_magnetic",

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Andy Shevchenko]

On Sat, Mar 07, 2026 at 01:03:12PM +0000, Jonathan Cameron wrote:
> On Wed, 4 Mar 2026 16:35:39 -0600
> David Lechner <dlechner@baylibre.com> wrote:
> > On 3/4/26 2:07 AM, Francesco Lavra wrote:

...

> Whilst I can see the argument, I'm not keen on more punctuation in the filenames.
> May well trip up someone's parser.  For now squashing the spaces is probably
> the best option.
> 
> This made me spot a major issue though. We can only add to end of these
> enums, not the middle!

So, if not yet, we need to have a patch that adds a big fat note/warning about
this on top of the enum(s).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Jonathan Cameron]

On Sun, 8 Mar 2026 22:27:26 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Sat, Mar 07, 2026 at 01:03:12PM +0000, Jonathan Cameron wrote:
> > On Wed, 4 Mar 2026 16:35:39 -0600
> > David Lechner <dlechner@baylibre.com> wrote:  
> > > On 3/4/26 2:07 AM, Francesco Lavra wrote:  
> 
> ...
> 
> > Whilst I can see the argument, I'm not keen on more punctuation in the filenames.
> > May well trip up someone's parser.  For now squashing the spaces is probably
> > the best option.
> > 
> > This made me spot a major issue though. We can only add to end of these
> > enums, not the middle!  
> 
> So, if not yet, we need to have a patch that adds a big fat note/warning about
> this on top of the enum(s).

To me that would basically say 'oh look this is a uapi header, you can't
change anything'. I guess we have a few proof points that people don't
actually realise that so maybe you are right.

Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Jonathan Cameron]

> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 6d269b844271..c80ef7c1ed12 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -77,6 +77,7 @@ enum iio_modifier {
>  	IIO_MOD_LIGHT_GREEN,
>  	IIO_MOD_LIGHT_BLUE,
>  	IIO_MOD_QUATERNION,
> +	IIO_MOD_PARTIAL_QUATERNION,
I nearly missed this.  This is a userspace header for a reason. The ABI is
fixed so we can't change the enumeration. We can only add to the end.

Good thing the discussion on the _ came up and made me wonder why the name
above was in near one with underscores!



>  	IIO_MOD_TEMP_AMBIENT,
>  	IIO_MOD_TEMP_OBJECT,
>  	IIO_MOD_NORTH_MAGN,
All userspace code using these would now be broken.

[PATCH v7 6/6] 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_fusion_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       |  28 ++-
 .../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    | 234 ++++++++++++++++++
 5 files changed, 324 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..767aadfe7061 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_FUSION,
 	ST_LSM6DSX_ID_MAX
 };
 
@@ -301,6 +302,17 @@ enum st_lsm6dsx_ext_sensor_id {
 	ST_LSM6DSX_ID_MAGN,
 };
 
+struct st_lsm6dsx_fusion_settings {
+	const struct iio_chan_spec *chan;
+	int chan_len;
+	struct st_lsm6dsx_reg odr_reg;
+	int odr_hz[ST_LSM6DSX_ODR_LIST_SIZE];
+	int odr_len;
+	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 +400,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_fusion_settings fusion_settings;
 };
 
 enum st_lsm6dsx_fifo_mode {
@@ -510,6 +523,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_fusion_probe(struct st_lsm6dsx_hw *hw, const char *name);
+int st_lsm6dsx_fusion_set_enable(struct st_lsm6dsx_sensor *sensor, bool enable);
+int st_lsm6dsx_fusion_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 +580,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_FUSION:
+		return st_lsm6dsx_fusion_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 a6ee2da5a06c..67f2ba0ff642 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_FUSION)
+			return st_lsm6dsx_fusion_set_odr(sensor, enable);
 		return 0;
+	}
 
 	batch_reg = &hw->settings->batch[sensor->id];
 	if (batch_reg->addr) {
@@ -578,6 +582,9 @@ 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:
+		iio_dev = hw->iio_devs[ST_LSM6DSX_ID_FUSION];
+		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 450cb5b47346..763ac9dfe5b1 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -94,6 +94,26 @@
 
 #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_PARTIAL_QUATERNION,				\
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.info_mask_shared_by_all_available =				\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),				\
+	.scan_index = 0,						\
+	.scan_type = {							\
+		.format = 'f',						\
+		.realbits = 16,						\
+		.storagebits = 16,					\
+		.endianness = IIO_LE,					\
+		.repeat = 3,						\
+	},								\
+	.ext_info = st_lsm6dsx_ext_info,				\
+}
+
 static const struct iio_event_spec st_lsm6dsx_ev_motion[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -153,6 +173,11 @@ static const struct iio_chan_spec st_lsm6ds0_gyro_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
+static const struct iio_chan_spec st_lsm6dsx_fusion_channels[] = {
+	ST_LSM6DSX_CHANNEL_ROT,
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
 static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 	{
 		.reset = {
@@ -1492,6 +1517,33 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				},
 			},
 		},
+		.fusion_settings = {
+			.chan = st_lsm6dsx_fusion_channels,
+			.chan_len = ARRAY_SIZE(st_lsm6dsx_fusion_channels),
+			.odr_reg = {
+				.addr = 0x5e,
+				.mask = GENMASK(5, 3),
+			},
+			.odr_hz[0] = 15,
+			.odr_hz[1] = 30,
+			.odr_hz[2] = 60,
+			.odr_hz[3] = 120,
+			.odr_hz[4] = 240,
+			.odr_hz[5] = 480,
+			.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->fusion_settings.chan) {
+		err = st_lsm6dsx_fusion_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..38a99ca9bd7b
--- /dev/null
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * STMicroelectronics st_lsm6dsx IMU sensor fusion
+ *
+ * Copyright 2026 BayLibre, SAS
+ */
+
+#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_fusion_get_odr_val(const struct st_lsm6dsx_fusion_settings *settings,
+			      u32 odr_mHz, u8 *val)
+{
+	int odr_hz = odr_mHz / MILLI;
+	int i;
+
+	for (i = 0; i < settings->odr_len; i++) {
+		if (settings->odr_hz[i] == odr_hz)
+			break;
+	}
+	if (i == settings->odr_len)
+		return -EINVAL;
+
+	*val = i;
+	return 0;
+}
+
+/**
+ * st_lsm6dsx_fusion_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_fusion_page_enable(struct st_lsm6dsx_hw *hw)
+{
+	const struct st_lsm6dsx_reg *mux;
+
+	mux = &hw->settings->fusion_settings.page_mux;
+
+	return regmap_set_bits(hw->regmap, mux->addr, mux->mask);
+}
+
+/**
+ * st_lsm6dsx_fusion_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_fusion_page_disable(struct st_lsm6dsx_hw *hw)
+{
+	const struct st_lsm6dsx_reg *mux;
+
+	mux = &hw->settings->fusion_settings.page_mux;
+
+	return regmap_clear_bits(hw->regmap, mux->addr, mux->mask);
+}
+
+int st_lsm6dsx_fusion_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->fusion_settings.enable;
+	err = st_lsm6dsx_fusion_page_enable(hw);
+	if (err)
+		return err;
+
+	err = regmap_assign_bits(hw->regmap, en_reg->addr, en_reg->mask, enable);
+	if (err) {
+		st_lsm6dsx_fusion_page_disable(hw);
+		return err;
+	}
+
+	return st_lsm6dsx_fusion_page_disable(hw);
+}
+
+int st_lsm6dsx_fusion_set_odr(struct st_lsm6dsx_sensor *sensor, bool enable)
+{
+	const struct st_lsm6dsx_fusion_settings *settings;
+	struct st_lsm6dsx_hw *hw = sensor->hw;
+	u8 data;
+	int err;
+
+	guard(mutex)(&hw->page_lock);
+
+	err = st_lsm6dsx_fusion_page_enable(hw);
+	if (err)
+		return err;
+
+	settings = &hw->settings->fusion_settings;
+	if (enable) {
+		const struct st_lsm6dsx_reg *reg = &settings->odr_reg;
+		u8 odr_val;
+
+		st_lsm6dsx_fusion_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_fusion_page_disable(hw);
+
+out:
+	st_lsm6dsx_fusion_page_disable(hw);
+
+	return err;
+}
+
+static int st_lsm6dsx_fusion_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_fusion_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_fusion_settings *settings;
+	int err;
+
+	settings = &sensor->hw->settings->fusion_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_fusion_get_odr_val(settings, odr_mHz,
+						    &odr_val);
+		if (err)
+			return err;
+
+		sensor->hwfifo_odr_mHz = odr_mHz;
+		return 0;
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int st_lsm6dsx_fusion_read_avail(struct iio_dev *indio_dev,
+					struct iio_chan_spec const *chan,
+					const int **vals, int *type,
+					int *length, long mask)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(indio_dev);
+	const struct st_lsm6dsx_fusion_settings *settings;
+
+	settings = &sensor->hw->settings->fusion_settings;
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = settings->odr_hz;
+		*type = IIO_VAL_INT;
+		*length = settings->odr_len;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info st_lsm6dsx_fusion_info = {
+	.read_raw = st_lsm6dsx_fusion_read_raw,
+	.read_avail = st_lsm6dsx_fusion_read_avail,
+	.write_raw = st_lsm6dsx_fusion_write_raw,
+	.hwfifo_set_watermark = st_lsm6dsx_set_watermark,
+};
+
+int st_lsm6dsx_fusion_probe(struct st_lsm6dsx_hw *hw, const char *name)
+{
+	const struct st_lsm6dsx_fusion_settings *settings;
+	struct st_lsm6dsx_sensor *sensor;
+	struct iio_dev *iio_dev;
+	int ret;
+
+	iio_dev = devm_iio_device_alloc(hw->dev, sizeof(*sensor));
+	if (!iio_dev)
+		return -ENOMEM;
+
+	settings = &hw->settings->fusion_settings;
+	sensor = iio_priv(iio_dev);
+	sensor->id = ST_LSM6DSX_ID_FUSION;
+	sensor->hw = hw;
+	sensor->hwfifo_odr_mHz = settings->odr_hz[0] * MILLI;
+	sensor->watermark = 1;
+	iio_dev->modes = INDIO_DIRECT_MODE;
+	iio_dev->info = &st_lsm6dsx_fusion_info;
+	iio_dev->channels = settings->chan;
+	iio_dev->num_channels = settings->chan_len;
+	ret = snprintf(sensor->name, sizeof(sensor->name), "%s_fusion", name);
+	if (ret >= 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_FUSION] = iio_dev;
+
+	return 0;
+}
-- 
2.39.5

Re: [PATCH v7 6/6] iio: imu: st_lsm6dsx: Add support for rotation sensor

[Andy Shevchenko]

On Wed, Mar 04, 2026 at 09:07:11AM +0100, 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_fusion_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.

...

> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_fusion.c

+ cleanup.h
+ errno.h

> +#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>

...

> +	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_fusion_get_odr_val(settings, odr_mHz,
> +						    &odr_val);

In cases like this I would leave it on a single line. Here I counted only 81
characters.

> +		if (err)
> +			return err;
> +
> +		sensor->hwfifo_odr_mHz = odr_mHz;
> +		return 0;
> +	}

-- 
With Best Regards,
Andy Shevchenko