[PATCH v2 0/5] iio: buffer: fix timestamp alignment (in rare case)

[PATCH v2 0/5] iio: buffer: fix timestamp alignment (in rare case)

[David Lechner]

In [1], it was pointed out that the iio_push_to_buffers_with_timestamp()
function is not putting the timestamp at the correct offset in the scan
buffer in rare cases where the largest scan element size is larger than
sizeof(int64_t).

[1]: https://lore.kernel.org/linux-iio/20260215162351.79f40b32@jic23-huawei/

This only affected one driver, namely hid-sensor-rotation since it is
the only driver that meets the condition. To fix things up, first we
fix the hid-sensor-rotation driver in a way that preserves compatibility
with the broken timestamp alignment. Then we are free to fix the core
IIO code without affecting any users.

The first patch depends on [2] which is now in iio/fixes-togreg. It
should be OK to apply the first patch there and let the rest of the
patches go through iio/togreg (the later patches are just preventing
future bugs).

[2]: https://lore.kernel.org/linux-iio/20260228-iio-fix-repeat-alignment-v2-0-d58bfaa2920d@baylibre.com/

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
Changes in v2:
- Don't say "HACK" in comments.
- Cache timestamp offset instead of largest scan element size.
- New patch to ensure size/alignment is always power of 2 bytes.
- Link to v1: https://lore.kernel.org/r/20260301-iio-fix-timestamp-alignment-v1-0-1a54980bfb90@baylibre.com

---
David Lechner (5):
      iio: orientation: hid-sensor-rotation: add timestamp hack to not break userspace
      iio: buffer: check return value of iio_compute_scan_bytes()
      iio: buffer: cache timestamp offset in scan buffer
      iio: buffer: ensure repeat alignment is a power of two
      iio: buffer: fix timestamp alignment when quaternion in scan

 drivers/iio/industrialio-buffer.c             | 46 ++++++++++++++++++++-------
 drivers/iio/orientation/hid-sensor-rotation.c | 22 +++++++++++--
 include/linux/iio/buffer.h                    | 12 +++++--
 include/linux/iio/iio.h                       |  3 ++
 4 files changed, 66 insertions(+), 17 deletions(-)
---
base-commit: 6f25a6105c41a7d6b12986dbe80ded396a5667f8
change-id: 20260228-iio-fix-timestamp-alignment-89ade1af458b
prerequisite-message-id: <20260228-iio-fix-repeat-alignment-v2-0-d58bfaa2920d@baylibre.com>
prerequisite-patch-id: e155a526d57c5759a2fcfbfca7f544cb419addfd
prerequisite-patch-id: 6c69eaad0dd2ae69bd2745e7d387f739fc1a9ba0

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

[PATCH v2 1/5] iio: orientation: hid-sensor-rotation: add timestamp hack to not break userspace

[David Lechner]

Add a hack to push two timestamps in the hid-sensor-rotation scan data
to avoid breaking userspace applications that depend on the timestamp
being at the incorrect location in the scan data due to unintentional
misalignment in older kernels.

When this driver was written, the timestamp was in the correct location
because of the way iio_compute_scan_bytes() was implemented at the time.
(Samples were 24 bytes each.) Then commit 883f61653069 ("iio: buffer:
align the size of scan bytes to size of the largest element") changed
the computed scan_bytes to be a different size (32 bytes), which caused
iio_push_to_buffers_with_timestamp() to place the timestamp at an
incorrect offset.

There have been long periods of time (6 years each) where the timestamp
was in either location, so to not break either case, we open-code the
timestamps to be pushed to both locations in the scan data.

Reported-by: Jonathan Cameron <jic23@kernel.org>
Closes: https://lore.kernel.org/linux-iio/20260215162351.79f40b32@jic23-huawei/
Fixes: 883f61653069 ("iio: buffer: align the size of scan bytes to size of the largest element")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---

v2 changes:
- Rebased on fix that also touches the scan struct.
- Improved comments.

I found that I could emulate this thanks to /dev/uhid. And thanks to AI
code generators, I was able to reasonably quickly make a script that
worked for emulating "HID-SENSOR-20008a". See v1 message for test script
source code.

[v1]: https://lore.kernel.org/linux-iio/20260301-iio-fix-timestamp-alignment-v1-1-1a54980bfb90@baylibre.com/

I set up the buffer like this:

cd /sys/bus/iio/devices/iio:device1/buffer0
echo 1 > in_rot_quaternion_en
echo 1 > in_timestamp_en
echo 1 > enable

Before this series is applied, we can see that the timestamp (group of 8
ending in "98 18") is at offset of 24 in the 32-byte data.

hd /dev/iio\:device1

00000000  6a 18 00 00 ac f3 ff ff  83 2d 00 00 02 d3 ff ff  |j........-......|
00000010  00 00 00 00 00 00 00 00  5a 17 a0 2a 73 cb 98 18  |........Z..*s...|

00000020  ad 17 00 00 6a f4 ff ff  35 2b 00 00 ca d0 ff ff  |....j...5+......|
00000030  00 00 00 00 00 00 00 00  2a a6 bb 30 73 cb 98 18  |........*..0s...|

00000040  92 1e 00 00 50 ec ff ff  ea c1 ff ff 78 f0 ff ff  |....P.......x...|
00000050  00 00 00 00 00 00 00 00  8f 3b a7 39 77 cb 98 18  |.........;.9w...|

After the first patch, we can see that the timestamp is now repeated at
both the correct and previous incorrect offsets (24 and 32). (Normally,
the last 8 bytes would be all 00 for padding.)

00000000  dd e0 ff ff 0e e0 ff ff  75 07 00 00 90 3f 00 00  |........u....?..|
00000010  f4 38 82 d0 3a cc 98 18  f4 38 82 d0 3a cc 98 18  |.8..:....8..:...|

00000020  a0 e0 ff ff 1d e0 ff ff  a0 0a 00 00 1c 3f 00 00  |.............?..|
00000030  3a 29 9f d6 3a cc 98 18  3a 29 9f d6 3a cc 98 18  |:)..:...:)..:...|

00000040  a9 e1 ff ff 1e 14 00 00  6c c1 ff ff 98 f2 ff ff  |........l.......|
00000050  39 21 77 11 55 cc 98 18  39 21 77 11 55 cc 98 18  |9!w.U...9!w.U...|
---
 drivers/iio/orientation/hid-sensor-rotation.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index 6806481873be..5a5e6e4fbe34 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -20,7 +20,12 @@ struct dev_rot_state {
 	struct hid_sensor_hub_attribute_info quaternion;
 	struct {
 		IIO_DECLARE_QUATERNION(s32, sampled_vals);
-		aligned_s64 timestamp;
+		/*
+		 * ABI regression avoidance: There are two copies of the same
+		 * timestamp in case of userspace depending on broken alignment
+		 * from older kernels.
+		 */
+		aligned_s64 timestamp[2];
 	} scan;
 	int scale_pre_decml;
 	int scale_post_decml;
@@ -154,8 +159,19 @@ static int dev_rot_proc_event(struct hid_sensor_hub_device *hsdev,
 		if (!rot_state->timestamp)
 			rot_state->timestamp = iio_get_time_ns(indio_dev);
 
-		iio_push_to_buffers_with_timestamp(indio_dev, &rot_state->scan,
-						   rot_state->timestamp);
+		/*
+		 * ABI regression avoidance: IIO previously had an incorrect
+		 * implementation of iio_push_to_buffers_with_timestamp() that
+		 * put the timestamp in the last 8 bytes of the buffer, which
+		 * was incorrect according to the IIO ABI. To avoid breaking
+		 * userspace that may be depending on this broken behavior, we
+		 * put the timestamp in both the correct place [0] and the old
+		 * incorrect place [1].
+		 */
+		rot_state->scan.timestamp[0] = rot_state->timestamp;
+		rot_state->scan.timestamp[1] = rot_state->timestamp;
+
+		iio_push_to_buffers(indio_dev, &rot_state->scan);
 
 		rot_state->timestamp = 0;
 	}

-- 
2.43.0

[PATCH v2 2/5] iio: buffer: check return value of iio_compute_scan_bytes()

[David Lechner]

Check return value of iio_compute_scan_bytes() as it can return an
error.

The result is moved to an output parameter while we are touching this
as we will need to add a second output parameter in a later change.

The return type of iio_buffer_update_bytes_per_datum() also had to be
changed to propagate the error.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/industrialio-buffer.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index f15a180dc49e..4e413b4bb073 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -762,7 +762,8 @@ static int iio_storage_bytes_for_timestamp(struct iio_dev *indio_dev)
 }
 
 static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
-				  const unsigned long *mask, bool timestamp)
+				  const unsigned long *mask, bool timestamp,
+				  unsigned int *scan_bytes)
 {
 	unsigned int bytes = 0;
 	int length, i, largest = 0;
@@ -788,8 +789,9 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
 		largest = max(largest, length);
 	}
 
-	bytes = ALIGN(bytes, largest);
-	return bytes;
+	*scan_bytes = ALIGN(bytes, largest);
+
+	return 0;
 }
 
 static void iio_buffer_activate(struct iio_dev *indio_dev,
@@ -834,18 +836,23 @@ static int iio_buffer_disable(struct iio_buffer *buffer,
 	return buffer->access->disable(buffer, indio_dev);
 }
 
-static void iio_buffer_update_bytes_per_datum(struct iio_dev *indio_dev,
-					      struct iio_buffer *buffer)
+static int iio_buffer_update_bytes_per_datum(struct iio_dev *indio_dev,
+					     struct iio_buffer *buffer)
 {
 	unsigned int bytes;
+	int ret;
 
 	if (!buffer->access->set_bytes_per_datum)
-		return;
+		return 0;
 
-	bytes = iio_compute_scan_bytes(indio_dev, buffer->scan_mask,
-				       buffer->scan_timestamp);
+	ret = iio_compute_scan_bytes(indio_dev, buffer->scan_mask,
+				     buffer->scan_timestamp, &bytes);
+	if (ret)
+		return ret;
 
 	buffer->access->set_bytes_per_datum(buffer, bytes);
+
+	return 0;
 }
 
 static int iio_buffer_request_update(struct iio_dev *indio_dev,
@@ -853,7 +860,10 @@ static int iio_buffer_request_update(struct iio_dev *indio_dev,
 {
 	int ret;
 
-	iio_buffer_update_bytes_per_datum(indio_dev, buffer);
+	ret = iio_buffer_update_bytes_per_datum(indio_dev, buffer);
+	if (ret)
+		return ret;
+
 	if (buffer->access->request_update) {
 		ret = buffer->access->request_update(buffer);
 		if (ret) {
@@ -896,6 +906,7 @@ static int iio_verify_update(struct iio_dev *indio_dev,
 	struct iio_buffer *buffer;
 	bool scan_timestamp;
 	unsigned int modes;
+	int ret;
 
 	if (insert_buffer &&
 	    bitmap_empty(insert_buffer->scan_mask, masklength)) {
@@ -983,8 +994,11 @@ static int iio_verify_update(struct iio_dev *indio_dev,
 		scan_mask = compound_mask;
 	}
 
-	config->scan_bytes = iio_compute_scan_bytes(indio_dev,
-						    scan_mask, scan_timestamp);
+	ret = iio_compute_scan_bytes(indio_dev, scan_mask, scan_timestamp,
+				     &config->scan_bytes);
+	if (ret)
+		return ret;
+
 	config->scan_mask = scan_mask;
 	config->scan_timestamp = scan_timestamp;
 

-- 
2.43.0

[PATCH v2 3/5] iio: buffer: cache timestamp offset in scan buffer

[David Lechner]

Cache the offset (in bytes) for the timestamp element in a scan buffer.
This will be used later to ensure proper alignment of the timestamp
element in the scan buffer.

The new field could not be placed in struct iio_dev_opaque because we
will need to access it in a static inline function later, so we make it
__private instead. It is only intended to be used by core IIO code.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---

v2 changes:
* Cache the timestamp offset instead of the largest scan element size.
---
 drivers/iio/industrialio-buffer.c | 14 +++++++++++---
 include/linux/iio/iio.h           |  3 +++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 4e413b4bb073..ecfe0c9740e2 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -763,7 +763,8 @@ static int iio_storage_bytes_for_timestamp(struct iio_dev *indio_dev)
 
 static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
 				  const unsigned long *mask, bool timestamp,
-				  unsigned int *scan_bytes)
+				  unsigned int *scan_bytes,
+				  unsigned int *timestamp_offset)
 {
 	unsigned int bytes = 0;
 	int length, i, largest = 0;
@@ -785,6 +786,10 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
 			return length;
 
 		bytes = ALIGN(bytes, length);
+
+		if (timestamp_offset)
+			*timestamp_offset = bytes;
+
 		bytes += length;
 		largest = max(largest, length);
 	}
@@ -846,7 +851,7 @@ static int iio_buffer_update_bytes_per_datum(struct iio_dev *indio_dev,
 		return 0;
 
 	ret = iio_compute_scan_bytes(indio_dev, buffer->scan_mask,
-				     buffer->scan_timestamp, &bytes);
+				     buffer->scan_timestamp, &bytes, NULL);
 	if (ret)
 		return ret;
 
@@ -890,6 +895,7 @@ struct iio_device_config {
 	unsigned int watermark;
 	const unsigned long *scan_mask;
 	unsigned int scan_bytes;
+	unsigned int scan_timestamp_offset;
 	bool scan_timestamp;
 };
 
@@ -995,7 +1001,8 @@ static int iio_verify_update(struct iio_dev *indio_dev,
 	}
 
 	ret = iio_compute_scan_bytes(indio_dev, scan_mask, scan_timestamp,
-				     &config->scan_bytes);
+				     &config->scan_bytes,
+				     &config->scan_timestamp_offset);
 	if (ret)
 		return ret;
 
@@ -1153,6 +1160,7 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
 	indio_dev->active_scan_mask = config->scan_mask;
 	ACCESS_PRIVATE(indio_dev, scan_timestamp) = config->scan_timestamp;
 	indio_dev->scan_bytes = config->scan_bytes;
+	ACCESS_PRIVATE(indio_dev, scan_timestamp_offset) = config->scan_timestamp_offset;
 	iio_dev_opaque->currentmode = config->mode;
 
 	iio_update_demux(indio_dev);
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 2c91b7659ce9..ecbaeecbe0ac 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -584,6 +584,8 @@ struct iio_buffer_setup_ops {
  *			and owner
  * @buffer:		[DRIVER] any buffer present
  * @scan_bytes:		[INTERN] num bytes captured to be fed to buffer demux
+ * @scan_timestamp_offset: [INTERN] cache of the offset (in bytes) for the
+ *			   timestamp in the scan buffer
  * @available_scan_masks: [DRIVER] optional array of allowed bitmasks. Sort the
  *			   array in order of preference, the most preferred
  *			   masks first.
@@ -610,6 +612,7 @@ struct iio_dev {
 
 	struct iio_buffer		*buffer;
 	int				scan_bytes;
+	unsigned int			__private scan_timestamp_offset;
 
 	const unsigned long		*available_scan_masks;
 	unsigned int			__private masklength;

-- 
2.43.0

[PATCH v2 4/5] iio: buffer: ensure repeat alignment is a power of two

[David Lechner]

Use roundup_pow_of_two() in the calculation of iio_storage_bytes_for_si()
when scan_type->repeat > 1 to ensure that the size is a power of two.
storagebits is always going to be a power of two bytes, so we only need
to apply this to the repeat factor. The storage size is also used for
alignment, and we want to ensure that all alignments are a power of two.

The only repeat in use in the kernel currently is for quaternions, which
have a repeat of 4, so this does not change the result for existing
users.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---

v2 changes: new patch

In v1, Nuno made the point that if the size isn't a power of two, then
the alignment won't be a power of two either. And this could cause
unexpected problems regarding alignment in general.

This will affect the work Francesco is doing with IIO_MOD_QUATERNION_AXIS
which will have a repeat of 3, so this is a good time to think about this.
---
 drivers/iio/industrialio-buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index ecfe0c9740e2..c38da24561c0 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -748,7 +748,7 @@ static int iio_storage_bytes_for_si(struct iio_dev *indio_dev,
 	bytes = scan_type->storagebits / 8;
 
 	if (scan_type->repeat > 1)
-		bytes *= scan_type->repeat;
+		bytes *= roundup_pow_of_two(scan_type->repeat);
 
 	return bytes;
 }

-- 
2.43.0

[PATCH v2 5/5] iio: buffer: fix timestamp alignment when quaternion in scan

[David Lechner]

Fix timestamp alignment when a scan buffer contains an element larger
than sizeof(int64_t). Currently s32 quaternions are the only such
element, and the one driver that has this (hid-sensor-rotation) has a
workaround in place already so this change does not affect it.

Previously, we assumed that the timestamp would always be 8-byte aligned
relative to the end of the scan buffer, but in the case of a scan buffer
a 16-byte quaternion vector, scan_bytes == 32, but the timestamp needs
to be placed at offset 16, not 24.

ts_offset is now a value in bytes so we have to change how the array
access is done.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---

v2 changes:
* Use cached timestamp offset instead of largest scan element size.
* Mention change of ts_offset semantics in commit message.

To test this, I used hid-sensor-rotation minus the first patch in the
series so that we can see that the timestamp actually moved to the
correct location.

Before this patch, the timestamp (8 bytes ending with "98 18") is in the
wrong location.

00000000  6a 18 00 00 ac f3 ff ff  83 2d 00 00 02 d3 ff ff  |j........-......|
00000010  00 00 00 00 00 00 00 00  5a 17 a0 2a 73 cb 98 18  |........Z..*s...|

00000020  ad 17 00 00 6a f4 ff ff  35 2b 00 00 ca d0 ff ff  |....j...5+......|
00000030  00 00 00 00 00 00 00 00  2a a6 bb 30 73 cb 98 18  |........*..0s...|

00000040  92 1e 00 00 50 ec ff ff  ea c1 ff ff 78 f0 ff ff  |....P.......x...|
00000050  00 00 00 00 00 00 00 00  8f 3b a7 39 77 cb 98 18  |.........;.9w...|

After this patch, timestamp is now in the correct location.

00000000  55 0f 00 00 dd 1f 00 00  af 0b 00 00 ec 3e 00 00  |U............>..|
00000010  c7 17 68 42 6d d0 98 18  00 00 00 00 00 00 00 00  |..hBm...........|

00000020  57 0e 00 00 c8 1f 00 00  d1 0e 00 00 42 3e 00 00  |W...........B>..|
00000030  56 a2 87 48 6d d0 98 18  00 00 00 00 00 00 00 00  |V..Hm...........|

00000040  a3 e2 ff ff d3 1b 00 00  0b c9 ff ff cc 20 00 00  |............. ..|
00000050  27 59 4d b3 72 d0 98 18  00 00 00 00 00 00 00 00  |'YM.r...........|

I also tested this with a different driver not affected by this bug to
make sure that the timestamp is still in the correct location for all
other drivers.
---
 include/linux/iio/buffer.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index d37f82678f71..8fd0d57d238f 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -34,8 +34,16 @@ static inline int iio_push_to_buffers_with_timestamp(struct iio_dev *indio_dev,
 	void *data, int64_t timestamp)
 {
 	if (ACCESS_PRIVATE(indio_dev, scan_timestamp)) {
-		size_t ts_offset = indio_dev->scan_bytes / sizeof(int64_t) - 1;
-		((int64_t *)data)[ts_offset] = timestamp;
+		size_t ts_offset = ACCESS_PRIVATE(indio_dev, scan_timestamp_offset);
+
+		/*
+		 * The size of indio_dev->scan_bytes is always aligned to the
+		 * largest scan element's alignment (see iio_compute_scan_bytes()).
+		 * So there may be padding after the timestamp. ts_offset contains
+		 * the offset in bytes that was already computed for correctly
+		 * aligning the timestamp.
+		 */
+		*(int64_t *)(data + ts_offset) = timestamp;
 	}
 
 	return iio_push_to_buffers(indio_dev, data);

-- 
2.43.0

Re: [PATCH v2 0/5] iio: buffer: fix timestamp alignment (in rare case)

[Nuno Sá]

On Sat, 2026-03-07 at 19:44 -0600, David Lechner wrote:
> In [1], it was pointed out that the iio_push_to_buffers_with_timestamp()
> function is not putting the timestamp at the correct offset in the scan
> buffer in rare cases where the largest scan element size is larger than
> sizeof(int64_t).
> 
> [1]: https://lore.kernel.org/linux-iio/20260215162351.79f40b32@jic23-huawei/
> 
> This only affected one driver, namely hid-sensor-rotation since it is
> the only driver that meets the condition. To fix things up, first we
> fix the hid-sensor-rotation driver in a way that preserves compatibility
> with the broken timestamp alignment. Then we are free to fix the core
> IIO code without affecting any users.
> 
> The first patch depends on [2] which is now in iio/fixes-togreg. It
> should be OK to apply the first patch there and let the rest of the
> patches go through iio/togreg (the later patches are just preventing
> future bugs).
> 
> [2]:
> https://lore.kernel.org/linux-iio/20260228-iio-fix-repeat-alignment-v2-0-d58bfaa2920d@baylibre.com/
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

LGTM,

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> Changes in v2:
> - Don't say "HACK" in comments.
> - Cache timestamp offset instead of largest scan element size.
> - New patch to ensure size/alignment is always power of 2 bytes.
> - Link to v1:
> https://lore.kernel.org/r/20260301-iio-fix-timestamp-alignment-v1-0-1a54980bfb90@baylibre.com
> 
> ---
> David Lechner (5):
>       iio: orientation: hid-sensor-rotation: add timestamp hack to not break userspace
>       iio: buffer: check return value of iio_compute_scan_bytes()
>       iio: buffer: cache timestamp offset in scan buffer
>       iio: buffer: ensure repeat alignment is a power of two
>       iio: buffer: fix timestamp alignment when quaternion in scan
> 
>  drivers/iio/industrialio-buffer.c             | 46 ++++++++++++++++++++-------
>  drivers/iio/orientation/hid-sensor-rotation.c | 22 +++++++++++--
>  include/linux/iio/buffer.h                    | 12 +++++--
>  include/linux/iio/iio.h                       |  3 ++
>  4 files changed, 66 insertions(+), 17 deletions(-)
> ---
> base-commit: 6f25a6105c41a7d6b12986dbe80ded396a5667f8
> change-id: 20260228-iio-fix-timestamp-alignment-89ade1af458b
> prerequisite-message-id: <20260228-iio-fix-repeat-alignment-v2-0-d58bfaa2920d@baylibre.com>
> prerequisite-patch-id: e155a526d57c5759a2fcfbfca7f544cb419addfd
> prerequisite-patch-id: 6c69eaad0dd2ae69bd2745e7d387f739fc1a9ba0
> 
> Best regards,
> --  
> David Lechner <dlechner@baylibre.com>

Re: [PATCH v2 1/5] iio: orientation: hid-sensor-rotation: add timestamp hack to not break userspace

[Jonathan Cameron]

On Sat, 07 Mar 2026 19:44:09 -0600
David Lechner <dlechner@baylibre.com> wrote:

> Add a hack to push two timestamps in the hid-sensor-rotation scan data
> to avoid breaking userspace applications that depend on the timestamp
> being at the incorrect location in the scan data due to unintentional
> misalignment in older kernels.
> 
> When this driver was written, the timestamp was in the correct location
> because of the way iio_compute_scan_bytes() was implemented at the time.
> (Samples were 24 bytes each.) Then commit 883f61653069 ("iio: buffer:
> align the size of scan bytes to size of the largest element") changed
> the computed scan_bytes to be a different size (32 bytes), which caused
> iio_push_to_buffers_with_timestamp() to place the timestamp at an
> incorrect offset.
> 
> There have been long periods of time (6 years each) where the timestamp
> was in either location, so to not break either case, we open-code the
> timestamps to be pushed to both locations in the scan data.
> 
> Reported-by: Jonathan Cameron <jic23@kernel.org>
> Closes: https://lore.kernel.org/linux-iio/20260215162351.79f40b32@jic23-huawei/
> Fixes: 883f61653069 ("iio: buffer: align the size of scan bytes to size of the largest element")
> Signed-off-by: David Lechner <dlechner@baylibre.com>
As suggested, I've applied this one to fixes-togreg and the rest will have
to wait for now.  Interestingly this cycle has more fixes coming through
than any I remember!  Hence I've sent a 2nd fixes pull and this will
be queued up for the 3rd in a week or so.  It's possible the rest
might have to wait for next cycle though given round trip times etc.

Let's see.

Thanks for sorting this out and for the test scripts etc!


Jonathan

> ---
> 
> v2 changes:
> - Rebased on fix that also touches the scan struct.
> - Improved comments.
> 
> I found that I could emulate this thanks to /dev/uhid. And thanks to AI
> code generators, I was able to reasonably quickly make a script that
> worked for emulating "HID-SENSOR-20008a". See v1 message for test script
> source code.
> 
> [v1]: https://lore.kernel.org/linux-iio/20260301-iio-fix-timestamp-alignment-v1-1-1a54980bfb90@baylibre.com/
> 
> I set up the buffer like this:
> 
> cd /sys/bus/iio/devices/iio:device1/buffer0
> echo 1 > in_rot_quaternion_en
> echo 1 > in_timestamp_en
> echo 1 > enable
> 
> Before this series is applied, we can see that the timestamp (group of 8
> ending in "98 18") is at offset of 24 in the 32-byte data.
> 
> hd /dev/iio\:device1
> 
> 00000000  6a 18 00 00 ac f3 ff ff  83 2d 00 00 02 d3 ff ff  |j........-......|
> 00000010  00 00 00 00 00 00 00 00  5a 17 a0 2a 73 cb 98 18  |........Z..*s...|
> 
> 00000020  ad 17 00 00 6a f4 ff ff  35 2b 00 00 ca d0 ff ff  |....j...5+......|
> 00000030  00 00 00 00 00 00 00 00  2a a6 bb 30 73 cb 98 18  |........*..0s...|
> 
> 00000040  92 1e 00 00 50 ec ff ff  ea c1 ff ff 78 f0 ff ff  |....P.......x...|
> 00000050  00 00 00 00 00 00 00 00  8f 3b a7 39 77 cb 98 18  |.........;.9w...|
> 
> After the first patch, we can see that the timestamp is now repeated at
> both the correct and previous incorrect offsets (24 and 32). (Normally,
> the last 8 bytes would be all 00 for padding.)
> 
> 00000000  dd e0 ff ff 0e e0 ff ff  75 07 00 00 90 3f 00 00  |........u....?..|
> 00000010  f4 38 82 d0 3a cc 98 18  f4 38 82 d0 3a cc 98 18  |.8..:....8..:...|
> 
> 00000020  a0 e0 ff ff 1d e0 ff ff  a0 0a 00 00 1c 3f 00 00  |.............?..|
> 00000030  3a 29 9f d6 3a cc 98 18  3a 29 9f d6 3a cc 98 18  |:)..:...:)..:...|
> 
> 00000040  a9 e1 ff ff 1e 14 00 00  6c c1 ff ff 98 f2 ff ff  |........l.......|
> 00000050  39 21 77 11 55 cc 98 18  39 21 77 11 55 cc 98 18  |9!w.U...9!w.U...|
> ---
>  drivers/iio/orientation/hid-sensor-rotation.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
> index 6806481873be..5a5e6e4fbe34 100644
> --- a/drivers/iio/orientation/hid-sensor-rotation.c
> +++ b/drivers/iio/orientation/hid-sensor-rotation.c
> @@ -20,7 +20,12 @@ struct dev_rot_state {
>  	struct hid_sensor_hub_attribute_info quaternion;
>  	struct {
>  		IIO_DECLARE_QUATERNION(s32, sampled_vals);
> -		aligned_s64 timestamp;
> +		/*
> +		 * ABI regression avoidance: There are two copies of the same
> +		 * timestamp in case of userspace depending on broken alignment
> +		 * from older kernels.
> +		 */
> +		aligned_s64 timestamp[2];
>  	} scan;
>  	int scale_pre_decml;
>  	int scale_post_decml;
> @@ -154,8 +159,19 @@ static int dev_rot_proc_event(struct hid_sensor_hub_device *hsdev,
>  		if (!rot_state->timestamp)
>  			rot_state->timestamp = iio_get_time_ns(indio_dev);
>  
> -		iio_push_to_buffers_with_timestamp(indio_dev, &rot_state->scan,
> -						   rot_state->timestamp);
> +		/*
> +		 * ABI regression avoidance: IIO previously had an incorrect
> +		 * implementation of iio_push_to_buffers_with_timestamp() that
> +		 * put the timestamp in the last 8 bytes of the buffer, which
> +		 * was incorrect according to the IIO ABI. To avoid breaking
> +		 * userspace that may be depending on this broken behavior, we
> +		 * put the timestamp in both the correct place [0] and the old
> +		 * incorrect place [1].
> +		 */
> +		rot_state->scan.timestamp[0] = rot_state->timestamp;
> +		rot_state->scan.timestamp[1] = rot_state->timestamp;
> +
> +		iio_push_to_buffers(indio_dev, &rot_state->scan);
>  
>  		rot_state->timestamp = 0;
>  	}
>