Re: [PATCH 4/4] iio: buffer: fix timestamp alignment when quaternion in scan

[David Lechner]

On 3/2/26 6:04 AM, Nuno Sá wrote:
> On Sun, 2026-03-01 at 14:24 -0600, David Lechner wrote:
>> 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.
>>
>> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
>> ---
>>
>> 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..ac19b39bdbe4 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 = indio_dev->scan_bytes -
>> +			ACCESS_PRIVATE(indio_dev, largest_scan_element_size);
> 
> Given that we're adding a new private member, maybe we could just directly cache the ts_offset
> in iio_compute_scan_bytes()? Would make the code a bit easier to follow IMHO
> 
> - Nuno Sá
>>

Clever. :-)