public inbox for linux-input@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
Cc: "Nuno Sá" <noname.nuno@gmail.com>,
	"Jiri Kosina" <jikos@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>,
	"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, 2 Mar 2026 20:49:58 +0000	[thread overview]
Message-ID: <20260302204958.246ce312@jic23-huawei> (raw)
In-Reply-To: <a7078fba-af96-4d77-8b10-35a31cb364ac@baylibre.com>

On Mon, 2 Mar 2026 09:42:03 -0600
David Lechner <dlechner@baylibre.com> wrote:

> 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@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á  
> >>  
> 
> Clever. :-)
Ah. I should have read on!

J

      reply	other threads:[~2026-03-02 20:50 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á
2026-03-02 15:42     ` David Lechner
2026-03-02 20:49       ` Jonathan Cameron [this message]

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=20260302204958.246ce312@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=gregkh@linuxfoundation.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=noname.nuno@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox