[PATCH v2 1/2] iio: add IIO_DECLARE_QUATERNION() macro

David Lechner posted 2 patches 1 month ago
[PATCH v2 1/2] iio: add IIO_DECLARE_QUATERNION() macro
Posted by David Lechner 1 month ago
Add a new IIO_DECLARE_QUATERNION() macro that is used to declare the
field in an IIO buffer struct that contains a quaternion vector.

Quaternions are currently the only IIO data type that uses the .repeat
feature of struct iio_scan_type. This has an implicit rule that the
element in the buffer must be aligned to the entire size of the repeated
element. This macro will make that requirement explicit. Since this is
the only user, we just call the macro IIO_DECLARE_QUATERNION() instead
of something more generic.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 include/linux/iio/iio.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index a9ecff191bd9..2c91b7659ce9 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -931,6 +931,18 @@ static inline void *iio_device_get_drvdata(const struct iio_dev *indio_dev)
 #define IIO_DECLARE_DMA_BUFFER_WITH_TS(type, name, count) \
 	__IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(IIO_DMA_MINALIGN)
 
+/**
+ * IIO_DECLARE_QUATERNION() - Declare a quaternion element
+ * @type: element type of the individual vectors
+ * @name: identifier name
+ *
+ * Quaternions are a vector composed of 4 elements (W, X, Y, Z). Use this macro
+ * to declare a quaternion element in a struct to ensure proper alignment in
+ * an IIO buffer.
+ */
+#define IIO_DECLARE_QUATERNION(type, name) \
+	type name[4] __aligned(sizeof(type) * 4)
+
 struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv);
 
 /* The information at the returned address is guaranteed to be cacheline aligned */

-- 
2.43.0
Re: [PATCH v2 1/2] iio: add IIO_DECLARE_QUATERNION() macro
Posted by David Lechner 3 weeks, 4 days ago
On 2/28/26 2:02 PM, David Lechner wrote:
> Add a new IIO_DECLARE_QUATERNION() macro that is used to declare the
> field in an IIO buffer struct that contains a quaternion vector.
> 
> Quaternions are currently the only IIO data type that uses the .repeat
> feature of struct iio_scan_type. This has an implicit rule that the
> element in the buffer must be aligned to the entire size of the repeated
> element. This macro will make that requirement explicit. Since this is
> the only user, we just call the macro IIO_DECLARE_QUATERNION() instead
> of something more generic.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  include/linux/iio/iio.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index a9ecff191bd9..2c91b7659ce9 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -931,6 +931,18 @@ static inline void *iio_device_get_drvdata(const struct iio_dev *indio_dev)
>  #define IIO_DECLARE_DMA_BUFFER_WITH_TS(type, name, count) \
>  	__IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(IIO_DMA_MINALIGN)
>  
> +/**
> + * IIO_DECLARE_QUATERNION() - Declare a quaternion element
> + * @type: element type of the individual vectors
> + * @name: identifier name
> + *
> + * Quaternions are a vector composed of 4 elements (W, X, Y, Z). Use this macro
> + * to declare a quaternion element in a struct to ensure proper alignment in
> + * an IIO buffer.
> + */
> +#define IIO_DECLARE_QUATERNION(type, name) \
> +	type name[4] __aligned(sizeof(type) * 4)
> +
>  struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv);
>  
>  /* The information at the returned address is guaranteed to be cacheline aligned */
> 

Hi Francesco,

I should have cc'ed you on this one. We'll want to add another macro
like this for IIO_DECLARE_QUATERNION_AXIS(), I imagine. (This has been
applied to the fixes-togreg branch since it is a dependency to a fix.)

What to do on that for the alignment though is an open question since
we've never had a repeat of 3 before. The question is if it is OK to
have an alignment that isn't a power of two bytes. For your case, since
the data is 3 x 16-bit, it would be 3 * 2 = 6 bytes.
Re: [PATCH v2 1/2] iio: add IIO_DECLARE_QUATERNION() macro
Posted by Francesco Lavra 2 weeks, 3 days ago
On Sat, 2026-03-07 at 18:35 -0600, David Lechner wrote:
> On 2/28/26 2:02 PM, David Lechner wrote:
> > Add a new IIO_DECLARE_QUATERNION() macro that is used to declare the
> > field in an IIO buffer struct that contains a quaternion vector.
> > 
> > Quaternions are currently the only IIO data type that uses the .repeat
> > feature of struct iio_scan_type. This has an implicit rule that the
> > element in the buffer must be aligned to the entire size of the
> > repeated
> > element. This macro will make that requirement explicit. Since this is
> > the only user, we just call the macro IIO_DECLARE_QUATERNION() instead
> > of something more generic.

...


> Hi Francesco,
> 
> I should have cc'ed you on this one. We'll want to add another macro
> like this for IIO_DECLARE_QUATERNION_AXIS(), I imagine. (This has been
> applied to the fixes-togreg branch since it is a dependency to a fix.)
> 
> What to do on that for the alignment though is an open question since
> we've never had a repeat of 3 before. The question is if it is OK to
> have an alignment that isn't a power of two bytes. For your case, since
> the data is 3 x 16-bit, it would be 3 * 2 = 6 bytes.

Hi David. Differently from the hid-sensor driver (where the `scan` field in
struct dev_rot_state is used exclusively for quaternion data), the lsm6dsx
driver uses a data structure (see the `iio_buff` variable in
st_lsm6dsx_buffer.c:st_lsm6dsx_read_tagged_fifo()) that is shared between
all the different data types that can be read from the hardware FIFO
(accel, gyro, quaternion, external sensor data), so changing this structure
to a quaternion-specific type is not ideal. So I think for the time being
there wouldn't be any users of an IIO_DECLARE_QUATERNION_AXIS() macro.

As for the alignment, according to your patch at [1], when the repeat
number is not a power of two, it is (will be) rounded up to the next power
of two (and this is consistent with what the lsm6dsx driver expects), so
the alignment will be 8 bytes.

[1]
https://lore.kernel.org/linux-iio/20260307-iio-fix-timestamp-alignment-v2-4-d1d48fbadbbf@baylibre.com/
Re: [PATCH v2 1/2] iio: add IIO_DECLARE_QUATERNION() macro
Posted by David Lechner 2 weeks, 3 days ago
On 3/16/26 2:03 PM, Francesco Lavra wrote:
> On Sat, 2026-03-07 at 18:35 -0600, David Lechner wrote:
>> On 2/28/26 2:02 PM, David Lechner wrote:
>>> Add a new IIO_DECLARE_QUATERNION() macro that is used to declare the
>>> field in an IIO buffer struct that contains a quaternion vector.
>>>
>>> Quaternions are currently the only IIO data type that uses the .repeat
>>> feature of struct iio_scan_type. This has an implicit rule that the
>>> element in the buffer must be aligned to the entire size of the
>>> repeated
>>> element. This macro will make that requirement explicit. Since this is
>>> the only user, we just call the macro IIO_DECLARE_QUATERNION() instead
>>> of something more generic.
> 
> ...
> 
> 
>> Hi Francesco,
>>
>> I should have cc'ed you on this one. We'll want to add another macro
>> like this for IIO_DECLARE_QUATERNION_AXIS(), I imagine. (This has been
>> applied to the fixes-togreg branch since it is a dependency to a fix.)
>>
>> What to do on that for the alignment though is an open question since
>> we've never had a repeat of 3 before. The question is if it is OK to
>> have an alignment that isn't a power of two bytes. For your case, since
>> the data is 3 x 16-bit, it would be 3 * 2 = 6 bytes.
> 
> Hi David. Differently from the hid-sensor driver (where the `scan` field in
> struct dev_rot_state is used exclusively for quaternion data), the lsm6dsx
> driver uses a data structure (see the `iio_buff` variable in
> st_lsm6dsx_buffer.c:st_lsm6dsx_read_tagged_fifo()) that is shared between
> all the different data types that can be read from the hardware FIFO
> (accel, gyro, quaternion, external sensor data), so changing this structure
> to a quaternion-specific type is not ideal. So I think for the time being
> there wouldn't be any users of an IIO_DECLARE_QUATERNION_AXIS() macro.

Makes sense.

> 
> As for the alignment, according to your patch at [1], when the repeat
> number is not a power of two, it is (will be) rounded up to the next power
> of two (and this is consistent with what the lsm6dsx driver expects), so
> the alignment will be 8 bytes.

I think you are referring to the 8-byte alignment for the timestamp?

Patch [1] should make a difference when the timestamp is not enabled
in a buffered read though.

When the timestamp is enabled, the buffer is going to be 16 bytes per
sample no matter what because of the 8-byte alignment of the timestamp.

But if the timestamp is not enabled, then for 16-bit storagebits and
repeat of 3, before the patch, the buffer would only be 6 bytes per
sample, but after the patch would be 8 bytes per sample. This doesn't
make a difference in the driver itself, but does make a difference to
userspace that is reading the buffer.

> 
> [1]
> https://lore.kernel.org/linux-iio/20260307-iio-fix-timestamp-alignment-v2-4-d1d48fbadbbf@baylibre.com/
Re: [PATCH v2 1/2] iio: add IIO_DECLARE_QUATERNION() macro
Posted by Francesco Lavra 2 weeks, 2 days ago
On Mon, 2026-03-16 at 14:56 -0500, David Lechner wrote:
...
> > As for the alignment, according to your patch at [1], when the repeat
> > number is not a power of two, it is (will be) rounded up to the next
> > power
> > of two (and this is consistent with what the lsm6dsx driver expects),
> > so
> > the alignment will be 8 bytes.
> 
> I think you are referring to the 8-byte alignment for the timestamp?
> 
> Patch [1] should make a difference when the timestamp is not enabled
> in a buffered read though.
> 
> When the timestamp is enabled, the buffer is going to be 16 bytes per
> sample no matter what because of the 8-byte alignment of the timestamp.
> 
> But if the timestamp is not enabled, then for 16-bit storagebits and
> repeat of 3, before the patch, the buffer would only be 6 bytes per
> sample, but after the patch would be 8 bytes per sample. This doesn't
> make a difference in the driver itself, but does make a difference to
> userspace that is reading the buffer.

I was referring to alignment in general, regardless of the timestamp being
enabled. From my understanding, the computed storage size is also used for
alignment. Anyway, yes, after [1] the storage size for the quaternion scan
element will be 8 bytes, with the last 2 bytes of each sample zeroed out.

> 
> > [1]
> > https://lore.kernel.org/linux-iio/20260307-iio-fix-timestamp-alignment-v2-4-d1d48fbadbbf@baylibre.com/