[PATCH 0/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment

[PATCH 0/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment

[David Lechner]

The main point of this series is to fix a regression reported in
hid-sensor-rotation where the alignment of the quaternion field in the
data was inadvertently changed from 16 bytes to 8 bytes. This is an
unusually case (one of only 2 in the kernel) where the .repeat field of
struct iio_scan_type is used and we have such a requirement. (The other
case uses u16 instead of u32, so it wasn't affected.)

To make the reason for the alignment more explicit to future readers,
we introduce a new macro, IIO_DECLARE_REPEATED_ELEMENT, to declare the
array with proper allignment. This is meant to follow the pattern of
the similar IIO_DECLARE_BUFFER_WITH_TS() macro.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
David Lechner (2):
      iio: add IIO_DECLARE_REPEATED_ELEMENT() macro
      iio: orientation: hid-sensor-rotation: fix quaternion alignment

 drivers/iio/orientation/hid-sensor-rotation.c |  2 +-
 include/linux/iio/iio.h                       | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)
---
base-commit: 0f11bb7985ceef2aeeb5c45c3c7bfff3f5a16e03
change-id: 20260214-iio-fix-repeat-alignment-575b2c009e25

Best regards,
-- 
David Lechner <dlechner@baylibre.com>

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

[David Lechner]

Add a new IIO_DECLARE_REPEATED_ELEMENT() macro that is used to declare
the field in an IIO buffer struct that contains a repeated element.

There are only a few iio drivers that actually make use of 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.

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

diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 872ebdf0dd77..28b708166b9b 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -816,6 +816,19 @@ 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_REPEATED_ELEMENT() - Declare a repeated element
+ * @type: element type of the repeated element
+ * @name: identifier name of the repeated element
+ * @repeat: number of times the element is repeated
+ *
+ * For special cases with repeated elements, like IIO_MOT_QUATERNION, a multi-
+ * word element is treated as a single element of a larger size in the buffer.
+ * As such, it requires alignment to the size of the entire repeated element.
+ */
+#define IIO_DECLARE_REPEATED_ELEMENT(type, name, repeat) \
+	type name[repeat] __aligned(sizeof(type) * repeat)
+
 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

[PATCH 2/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment

[David Lechner]

Restore the alignment of sampled_vals to 16 bytes by using
IIO_DECLARE_REPEATED_ELEMENT(). This field contains a quaternion value
which scan_type.repeat = 4 and storagebits = 32. So the alignment must
be 16 bytes to match the assumptions of iio_storage_bytes_for_si() and
also to not break userspace.

Reported-by: Lixu Zhang <lixu.zhang@intel.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221077
Fixes: b31a74075cb4 ("iio: orientation: hid-sensor-rotation: remove unnecessary alignment")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/orientation/hid-sensor-rotation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index e759f91a710a..9df955ecfbac 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -19,7 +19,7 @@ struct dev_rot_state {
 	struct hid_sensor_common common_attributes;
 	struct hid_sensor_hub_attribute_info quaternion;
 	struct {
-		s32 sampled_vals[4];
+		IIO_DECLARE_REPEATED_ELEMENT(s32, sampled_vals, 4);
 		aligned_s64 timestamp;
 	} scan;
 	int scale_pre_decml;

-- 
2.43.0

Re: [PATCH 2/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment

[Andy Shevchenko]

On Sat, Feb 14, 2026 at 03:00:21PM -0600, David Lechner wrote:
> Restore the alignment of sampled_vals to 16 bytes by using
> IIO_DECLARE_REPEATED_ELEMENT(). This field contains a quaternion value
> which scan_type.repeat = 4 and storagebits = 32. So the alignment must
> be 16 bytes to match the assumptions of iio_storage_bytes_for_si() and
> also to not break userspace.

...

>  	struct {
> -		s32 sampled_vals[4];
> +		IIO_DECLARE_REPEATED_ELEMENT(s32, sampled_vals, 4);

This becomes
		IIO_DECLARE_QUATERNION(s32, sampled_vals);

>  		aligned_s64 timestamp;
>  	} scan;

In the similar way we can address drivers/iio/imu/bno055/bno055.c.

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH 2/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment

[Zhang, Lixu]

>-----Original Message-----
>From: David Lechner <dlechner@baylibre.com>
>Sent: Sunday, February 15, 2026 5:00 AM
>To: Jonathan Cameron <jic23@kernel.org>; Nuno Sá <nuno.sa@analog.com>;
>Andy Shevchenko <andy@kernel.org>; Jiri Kosina <jikos@kernel.org>; Srinivas
>Pandruvada <srinivas.pandruvada@linux.intel.com>
>Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Jonathan Cameron
><Jonathan.Cameron@huawei.com>; linux-input@vger.kernel.org; Lechner,
>David <dlechner@baylibre.com>; Zhang, Lixu <lixu.zhang@intel.com>
>Subject: [PATCH 2/2] iio: orientation: hid-sensor-rotation: fix quaternion
>alignment
>
>Restore the alignment of sampled_vals to 16 bytes by using
>IIO_DECLARE_REPEATED_ELEMENT(). This field contains a quaternion value
>which scan_type.repeat = 4 and storagebits = 32. So the alignment must be 16
>bytes to match the assumptions of iio_storage_bytes_for_si() and also to not
>break userspace.
>
>Reported-by: Lixu Zhang <lixu.zhang@intel.com>
>Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221077
>Fixes: b31a74075cb4 ("iio: orientation: hid-sensor-rotation: remove
>unnecessary alignment")
>Signed-off-by: David Lechner <dlechner@baylibre.com>

Tested-by: Lixu Zhang <lixu.zhang@intel.com>

Thanks,
-Lixu

Re: [PATCH 0/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment

[Andy Shevchenko]

On Sat, Feb 14, 2026 at 03:00:19PM -0600, David Lechner wrote:
> The main point of this series is to fix a regression reported in
> hid-sensor-rotation where the alignment of the quaternion field in the
> data was inadvertently changed from 16 bytes to 8 bytes. This is an
> unusually case (one of only 2 in the kernel) where the .repeat field of
> struct iio_scan_type is used and we have such a requirement. (The other
> case uses u16 instead of u32, so it wasn't affected.)
> 
> To make the reason for the alignment more explicit to future readers,
> we introduce a new macro, IIO_DECLARE_REPEATED_ELEMENT, to declare the
> array with proper allignment. This is meant to follow the pattern of
> the similar IIO_DECLARE_BUFFER_WITH_TS() macro.

In both cases it's quaternion, maybe be more explicit and define
IIO_DECLARE_QUATERNION() ?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment

[David Lechner]

On 2/16/26 1:44 AM, Andy Shevchenko wrote:
> On Sat, Feb 14, 2026 at 03:00:19PM -0600, David Lechner wrote:
>> The main point of this series is to fix a regression reported in
>> hid-sensor-rotation where the alignment of the quaternion field in the
>> data was inadvertently changed from 16 bytes to 8 bytes. This is an
>> unusually case (one of only 2 in the kernel) where the .repeat field of
>> struct iio_scan_type is used and we have such a requirement. (The other
>> case uses u16 instead of u32, so it wasn't affected.)
>>
>> To make the reason for the alignment more explicit to future readers,
>> we introduce a new macro, IIO_DECLARE_REPEATED_ELEMENT, to declare the
>> array with proper allignment. This is meant to follow the pattern of
>> the similar IIO_DECLARE_BUFFER_WITH_TS() macro.
> 
> In both cases it's quaternion, maybe be more explicit and define
> IIO_DECLARE_QUATERNION() ?
> 

It is really the fact that the scan_type has .repeat > 1 that requires
this, so I was trying to make a name that shows that link.

But right now, quaternion is the only thing that has .repeat > 1, so
I guess it would be OK either way. We'll see if Jonathan has an
opinion on the naming.

Re: [PATCH 0/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment

[Andy Shevchenko]

On Mon, Feb 16, 2026 at 09:25:58AM -0600, David Lechner wrote:
> On 2/16/26 1:44 AM, Andy Shevchenko wrote:
> > On Sat, Feb 14, 2026 at 03:00:19PM -0600, David Lechner wrote:
> >> The main point of this series is to fix a regression reported in
> >> hid-sensor-rotation where the alignment of the quaternion field in the
> >> data was inadvertently changed from 16 bytes to 8 bytes. This is an
> >> unusually case (one of only 2 in the kernel) where the .repeat field of
> >> struct iio_scan_type is used and we have such a requirement. (The other
> >> case uses u16 instead of u32, so it wasn't affected.)
> >>
> >> To make the reason for the alignment more explicit to future readers,
> >> we introduce a new macro, IIO_DECLARE_REPEATED_ELEMENT, to declare the
> >> array with proper allignment. This is meant to follow the pattern of
> >> the similar IIO_DECLARE_BUFFER_WITH_TS() macro.
> > 
> > In both cases it's quaternion, maybe be more explicit and define
> > IIO_DECLARE_QUATERNION() ?
> 
> It is really the fact that the scan_type has .repeat > 1 that requires
> this, so I was trying to make a name that shows that link.
> 
> But right now, quaternion is the only thing that has .repeat > 1, so
> I guess it would be OK either way. We'll see if Jonathan has an
> opinion on the naming.

I think we should solve the problems when they appear. Naming explicitly
for quaternion makes it easier to get from the core reading without having
a variable name to repeat that. Magic 4  might not always be a quaternion.

Do we have "repeat" to be used in other cases, btw?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment

[Jonathan Cameron]

On Tue, 17 Feb 2026 10:10:53 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Feb 16, 2026 at 09:25:58AM -0600, David Lechner wrote:
> > On 2/16/26 1:44 AM, Andy Shevchenko wrote:  
> > > On Sat, Feb 14, 2026 at 03:00:19PM -0600, David Lechner wrote:  
> > >> The main point of this series is to fix a regression reported in
> > >> hid-sensor-rotation where the alignment of the quaternion field in the
> > >> data was inadvertently changed from 16 bytes to 8 bytes. This is an
> > >> unusually case (one of only 2 in the kernel) where the .repeat field of
> > >> struct iio_scan_type is used and we have such a requirement. (The other
> > >> case uses u16 instead of u32, so it wasn't affected.)
> > >>
> > >> To make the reason for the alignment more explicit to future readers,
> > >> we introduce a new macro, IIO_DECLARE_REPEATED_ELEMENT, to declare the
> > >> array with proper allignment. This is meant to follow the pattern of
> > >> the similar IIO_DECLARE_BUFFER_WITH_TS() macro.  
> > > 
> > > In both cases it's quaternion, maybe be more explicit and define
> > > IIO_DECLARE_QUATERNION() ?  

I like this.  It's special and this shouts that nicely.

> > 
> > It is really the fact that the scan_type has .repeat > 1 that requires
> > this, so I was trying to make a name that shows that link.
> > 
> > But right now, quaternion is the only thing that has .repeat > 1, so
> > I guess it would be OK either way. We'll see if Jonathan has an
> > opinion on the naming.  
> 
> I think we should solve the problems when they appear. Naming explicitly
> for quaternion makes it easier to get from the core reading without having
> a variable name to repeat that. Magic 4  might not always be a quaternion.
> 
> Do we have "repeat" to be used in other cases, btw?
> 
Don't think so.

Note there is a second older bug here.
The timestamp is landing at the wrong location :(  See discussion of
original bug.  I suspect that applies to the bno055 as well (that avoids
the bug we are fixing in this patch because helpfully the quaternion is
a total of 8 bytes. 

Short story - it is just another channel, so should be naturally aligned
at first valid location after the previous channel. That's bytes 32-39 not
55-63 which is where iio_push_to_buffers_with_timestamp() puts it.


Jonathan