All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nuno Sá" <noname.nuno@gmail.com>
To: "David Lechner" <dlechner@baylibre.com>,
	"Jiri Kosina" <jikos@kernel.org>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"Srinivas Pandruvada" <srinivas.pandruvada@linux.intel.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Lars Möllendorf" <lars.moellendorf@plating.de>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Lixu Zhang	 <lixu.zhang@intel.com>,
	linux-input@vger.kernel.org, linux-iio@vger.kernel.org,
		linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4] iio: buffer: fix timestamp alignment when quaternion in scan
Date: Mon, 02 Mar 2026 12:04:00 +0000	[thread overview]
Message-ID: <f481f4ae4d9545bbb186e51a1204ca2d65b74a26.camel@gmail.com> (raw)
In-Reply-To: <20260301-iio-fix-timestamp-alignment-v1-4-1a54980bfb90@baylibre.com>

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@baylibre.com>
> ---
> 
> 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á
> 

  parent reply	other threads:[~2026-03-02 12:03 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-01 20:24 [PATCH 0/4] iio: buffer: fix timestamp alignment (in rare case) David Lechner
2026-03-01 20:24 ` [PATCH 1/4] iio: orientation: hid-sensor-rotation: add timestamp hack to not break userspace David Lechner
2026-03-02  8:50   ` Andy Shevchenko
2026-03-02 15:18     ` David Lechner
2026-03-02 20:39       ` Jonathan Cameron
2026-03-01 20:24 ` [PATCH 2/4] iio: buffer: check return value of iio_compute_scan_bytes() David Lechner
2026-03-01 20:24 ` [PATCH 3/4] iio: buffer: cache largest scan element size David Lechner
2026-03-02 12:16   ` Nuno Sá
2026-03-02 15:35     ` David Lechner
2026-03-02 16:18       ` Nuno Sá
2026-03-02 20:47   ` Jonathan Cameron
2026-03-02 21:58     ` David Lechner
2026-03-01 20:24 ` [PATCH 4/4] iio: buffer: fix timestamp alignment when quaternion in scan David Lechner
2026-03-02  8:47   ` Andy Shevchenko
2026-03-02 15:39     ` David Lechner
2026-03-02 16:03       ` Andy Shevchenko
2026-03-02 12:04   ` Nuno Sá [this message]
2026-03-02 15:42     ` David Lechner
2026-03-02 20:49       ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f481f4ae4d9545bbb186e51a1204ca2d65b74a26.camel@gmail.com \
    --to=noname.nuno@gmail.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jic23@kernel.org \
    --cc=jikos@kernel.org \
    --cc=lars.moellendorf@plating.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lixu.zhang@intel.com \
    --cc=nuno.sa@analog.com \
    --cc=srinivas.pandruvada@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.