* [PATCH 01/12] Documentation: Correction in HID output_report callback description.
From: Jingyuan Liang @ 2026-03-03 6:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
In-Reply-To: <20260303-send-upstream-v1-0-1515ba218f3d@chromium.org>
From: Jarrett Schultz <jaschultz@microsoft.com>
Originally output_report callback was described as must-be asynchronous,
but that is not the case in some implementations, namely i2c-hid.
Correct the documentation to say that it may be asynchronous.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
Documentation/hid/hid-transport.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/hid/hid-transport.rst b/Documentation/hid/hid-transport.rst
index 6f1692da296c..2008cf432af1 100644
--- a/Documentation/hid/hid-transport.rst
+++ b/Documentation/hid/hid-transport.rst
@@ -327,8 +327,8 @@ The available HID callbacks are:
Send raw output report via intr channel. Used by some HID device drivers
which require high throughput for outgoing requests on the intr channel. This
- must not cause SET_REPORT calls! This must be implemented as asynchronous
- output report on the intr channel!
+ must not cause SET_REPORT calls! This call might be asynchronous, so the
+ caller should not expect an immediate response!
::
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related
* [PATCH 00/12] Add spi-hid transport driver
From: Jingyuan Liang @ 2026-03-03 6:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov, Angela Czubak
This series picks up the spi-hid driver work originally started by
Microsoft. The patch breakdown has been modified and the implementation has
been refactored to address upstream feedback and testing issues. We are
submitting this as a new series while keeping the original sign-off
chain to reflect the history.
Same as the original series, there is a change to HID documentation, some
HID core changes to support a SPI device, the SPI HID transport driver,
and HID over SPI Device Tree binding. We have added the HID over SPI ACPI
support, power management, panel follower, and quirks for Ilitek touch
controllers.
Original authors: Jarrett Schultz <jaschultz@microsoft.com>,
Dmitry Antipov <dmanti@microsoft.com>
Link: https://lore.kernel.org/r/86b63b7b-afda-d7f4-7bfa-175085d5a8ef@gmail.com
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
Angela Czubak (2):
HID: spi-hid: add transport driver skeleton for HID over SPI bus
HID: spi_hid: add ACPI support for SPI over HID
Jarrett Schultz (3):
Documentation: Correction in HID output_report callback description.
HID: Add BUS_SPI support and define HID_SPI_DEVICE macro
HID: spi_hid: add device tree support for SPI over HID
Jingyuan Liang (7):
HID: spi-hid: add spi-hid driver HID layer
HID: spi-hid: add HID SPI protocol implementation
HID: spi_hid: add spi_hid traces
dt-bindings: input: Document hid-over-spi DT schema
HID: spi-hid: add power management implementation
HID: spi-hid: add panel follower support
HID: spi-hid: add quirkis to support mode switch for Ilitek touch
.../devicetree/bindings/input/hid-over-spi.yaml | 153 ++
Documentation/hid/hid-transport.rst | 4 +-
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/hid-core.c | 3 +
drivers/hid/spi-hid/Kconfig | 45 +
drivers/hid/spi-hid/Makefile | 11 +
drivers/hid/spi-hid/spi-hid-acpi.c | 254 ++++
drivers/hid/spi-hid/spi-hid-core.c | 1493 ++++++++++++++++++++
drivers/hid/spi-hid/spi-hid-core.h | 94 ++
drivers/hid/spi-hid/spi-hid-of.c | 244 ++++
drivers/hid/spi-hid/spi-hid.h | 51 +
include/linux/hid.h | 2 +
include/trace/events/spi_hid.h | 156 ++
14 files changed, 2512 insertions(+), 2 deletions(-)
---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20260212-send-upstream-75f6fd9ed92e
Best regards,
--
Jingyuan Liang <jingyliang@chromium.org>
^ permalink raw reply
* Re: [PATCH 3/4] iio: buffer: cache largest scan element size
From: David Lechner @ 2026-03-02 21:58 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Nuno Sá, Andy Shevchenko,
Lars Möllendorf, Lars-Peter Clausen, Greg Kroah-Hartman,
Jonathan Cameron, Lixu Zhang, linux-input, linux-iio,
linux-kernel
In-Reply-To: <20260302204755.462d6b63@jic23-huawei>
On 3/2/26 2:47 PM, Jonathan Cameron wrote:
> On Sun, 01 Mar 2026 14:24:52 -0600
> David Lechner <dlechner@baylibre.com> wrote:
>
>> Cache the largest scan element size of elements enabled in a scan
>> buffer. This will be used later to ensure proper alignment of the
>> timestamp element in the scan buffer.
> Why can't we just stash the start of the timestamp?
This is exactly what Nuno suggested. ;-)
^ permalink raw reply
* Re: [PATCH v2 0/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment
From: Jonathan Cameron @ 2026-03-02 20:54 UTC (permalink / raw)
To: David Lechner
Cc: Nuno Sá, Andy Shevchenko, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada, linux-iio, linux-kernel,
Jonathan Cameron, linux-input, Lixu Zhang
In-Reply-To: <d22e03b2-2093-4d33-8ec5-484f64f2e8cd@baylibre.com>
On Mon, 2 Mar 2026 09:52:58 -0600
David Lechner <dlechner@baylibre.com> wrote:
> On 3/2/26 6:32 AM, Nuno Sá wrote:
> > On Mon, 2026-03-02 at 10:58 +0200, Andy Shevchenko wrote:
> >> On Sat, Feb 28, 2026 at 02:02:21PM -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_QUATERNION(), to declare the
> >>> array with proper alignment. This is meant to follow the pattern of
> >>> the similar IIO_DECLARE_BUFFER_WITH_TS() macro.
> >>
> >> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> >>
> >> But this is in conflict with the other hack-patch in another series.
> >> I'm a bit lost what patch 2 fixes here and that hack-patch fixes
> >> in the same driver. Shouldn't survive only one?
> >
> > +1.
> >
> > I see this is older so should we only look at the other series?
> >
> > - Nuno Sá
>
> They are fixing two separate bugs.
>
> This bug (which is a recent regression) is that the size of the struct is
> wrong. It is supposed to be 32 bytes, but a recent change make it only 24.
> This causes iio_push_to_buffers_with_ts() to write over data past the end
> of the struct.
>
> The other bug (which has been a bug for 6 years) is that the the timestamp
> is in the wrong place. That patch also has an effect of making the struct
> the right size, but that is only a side-effect.
>
> So yes, I should have mentioned that in the cover letter that this series
> should probably be applied first since it is the worse bug. And it looks
> like I'll be doing a v2 of the other series anyway, so I can properly rebase
> it and declare the dependency.
>
>
I've applied these two to the fixes-togreg branch with them marked for stable
inclusion. For the first one I added a comment to the tags block to say
it's stable as a precusor for the second.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH 4/4] iio: buffer: fix timestamp alignment when quaternion in scan
From: Jonathan Cameron @ 2026-03-02 20:49 UTC (permalink / raw)
To: David Lechner
Cc: Nuno Sá, Jiri Kosina, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman, Jonathan Cameron, Lixu Zhang, linux-input,
linux-iio, linux-kernel
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
^ permalink raw reply
* Re: [PATCH 3/4] iio: buffer: cache largest scan element size
From: Jonathan Cameron @ 2026-03-02 20:47 UTC (permalink / raw)
To: David Lechner
Cc: Jiri Kosina, Srinivas Pandruvada, Nuno Sá, Andy Shevchenko,
Lars Möllendorf, Lars-Peter Clausen, Greg Kroah-Hartman,
Jonathan Cameron, Lixu Zhang, linux-input, linux-iio,
linux-kernel
In-Reply-To: <20260301-iio-fix-timestamp-alignment-v1-3-1a54980bfb90@baylibre.com>
On Sun, 01 Mar 2026 14:24:52 -0600
David Lechner <dlechner@baylibre.com> wrote:
> Cache the largest scan element size of elements enabled in a scan
> buffer. This will be used later to ensure proper alignment of the
> timestamp element in the scan buffer.
Why can't we just stash the start of the timestamp? It's computed
a couple of lines up (or at least it's supposed to be computed there!)
if (timestamp) {
length = iio_storage_bytes_for_timestamp(indio_dev);
if (length < 0)
return length;
bytes = ALIGN(bytes, length);
Or is that wrong currently? If it is we need to fix that logic.
J
>
> 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>
> ---
> 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 71dfc81cb9e5..83e9392f949f 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -765,7 +765,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 *largest_element_size)
> {
> unsigned int bytes = 0;
> int length, i, largest = 0;
> @@ -793,6 +794,9 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
>
> *scan_bytes = ALIGN(bytes, largest);
>
> + if (largest_element_size)
> + *largest_element_size = largest;
> +
> return 0;
> }
>
> @@ -848,7 +852,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;
>
> @@ -892,6 +896,7 @@ struct iio_device_config {
> unsigned int watermark;
> const unsigned long *scan_mask;
> unsigned int scan_bytes;
> + unsigned int largest_scan_element_size;
> bool scan_timestamp;
> };
>
> @@ -997,7 +1002,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->largest_scan_element_size);
> if (ret)
> return ret;
>
> @@ -1155,6 +1161,8 @@ 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, largest_scan_element_size) =
> + config->largest_scan_element_size;
> 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 a9ecff191bd9..85bcb5f8ae15 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
> + * @largest_scan_element_size: [INTERN] cache of the largest scan element size
> + * among the channels selected in the scan mask
> * @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 largest_scan_element_size;
>
> const unsigned long *available_scan_masks;
> unsigned int __private masklength;
>
^ permalink raw reply
* Re: [PATCH 1/4] iio: orientation: hid-sensor-rotation: add timestamp hack to not break userspace
From: Jonathan Cameron @ 2026-03-02 20:39 UTC (permalink / raw)
To: David Lechner
Cc: Andy Shevchenko, Jiri Kosina, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman, Jonathan Cameron, Lixu Zhang, linux-input,
linux-iio, linux-kernel
In-Reply-To: <8f9dc90d-9dba-4ed4-8cca-41012027aa10@baylibre.com>
On Mon, 2 Mar 2026 09:18:56 -0600
David Lechner <dlechner@baylibre.com> wrote:
> On 3/2/26 2:50 AM, Andy Shevchenko wrote:
> > On Sun, Mar 01, 2026 at 02:24:50PM -0600, David Lechner 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.
> >
> > ...
> >
> >> + /*
> >> + * HACK: There are two copies of the same timestamp in case of
> >
> > Usually we use FIXME in such cases. HACK is something which goes with
> > "do not apply".
> >
> > Does it mean it will stay forever?
>
> Yes, it will have to stay forever because we can't break userspace.
>
> My intention here was for HACK to mean "do not copy to another driver".
>
>
> If we think that no one is depending on timestamps being in the wrong
> offset, we could omit this change and apply only the rest of the series.
> And then only apply this patch if anyone complains. I am just trying to
> play it safe here and not risk breaking things.
I think it makes sense, but maybe label it something like
ABI regression avoidance:
Good idea to just keep the wrongly aligned one as it removes the
risk and should do no harm as correct userspace should ignore the
second copy as it's in the padding.
Jonathan
^ permalink raw reply
* [BUG] HP Pavilion x360: Built-in Keyboard and Touchpad disabled permanently after screen removal (Stuck in Tablet Mode)
From: Ngon @ 2026-03-02 20:32 UTC (permalink / raw)
To: dmitry.torokhov, linux-input
Hi Dmitry and Linux-Input developers,
I am reporting a hardware state issue on the HP Pavilion x360
Convertible 14-cd0xxx that causes the built-in keyboard and
touchpad to become completely non-functional under Linux.
The Situation:
I have physically removed the original laptop screen (using the
device as a headless mini-PC). Because the lid/hall sensors and
rotation sensors are located in the screen assembly, the Embedded
Controller (EC) or BIOS seems to default to a permanent
"Tablet Mode" or "Lid Closed" state.
Observations:
1. Running libinput debug-events clearly shows:
event20 SWITCH_TOGGLE switch tablet-mode state 1
2. The keyboard (AT Raw Set 2) and touchpad (SynPS/2 Synaptics) are
detected by the kernel but do not emit any events
(verified via libinput debug-events).
3. On Windows, the keyboard still works despite driver warnings,
likely due to a proprietary HP override.
4. On Linux, common kernel parameters like i8042.nopnp,
i8042.dumbkbd, or button.lid_init_state=open do not restore
functionality. Blacklisting intel_vbtn and hp_wmi also does not
force the system back to "Laptop Mode."
Question:
Is there a way to implement a kernel quirk to force-ignore the
Tablet Mode switch for this specific model, or a way to bypass
the EC's hard-lock on the i8042 bus when sensors are missing?
I am available to test any patches or provide further DSDT/log details.
Best regards,
kemzsitink
^ permalink raw reply
* [PATCH] Input: gpio-keys - do not print gpio number if using gpiod
From: Andrew Davis @ 2026-03-02 16:59 UTC (permalink / raw)
To: Dmitry Torokhov, Thomas Gleixner; +Cc: linux-input, linux-kernel, Andrew Davis
The value in button->gpio is not valid when in this branch which is for
the case of gpiod. Do not print this out as it will always be 0 here.
While here, there is no reason to assign irq to error, use irq directly.
Lastly, dev_err_probe() returns the given error value, so we can return
directly the result of dev_err_probe() making this a single line.
Signed-off-by: Andrew Davis <afd@ti.com>
---
drivers/input/keyboard/gpio_keys.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index e196174856796..a6ac693d8fce5 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -580,13 +580,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
bdata->irq = button->irq;
} else {
irq = gpiod_to_irq(bdata->gpiod);
- if (irq < 0) {
- error = irq;
- dev_err_probe(dev, error,
- "Unable to get irq number for GPIO %d\n",
- button->gpio);
- return error;
- }
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "Unable to get irq from GPIO\n");
bdata->irq = irq;
}
--
2.39.2
^ permalink raw reply related
* [hid:for-7.1/lenovo 14/16] drivers/hid/hid-lenovo-go-s.c:1173: multiple definition of `rgb_enabled'; drivers/hid/hid-lenovo-go.o:drivers/hid/hid-lenovo-go.c:2122: first defined here
From: kernel test robot @ 2026-03-02 16:38 UTC (permalink / raw)
To: Derek J. Clark; +Cc: oe-kbuild-all, linux-input, Jiri Kosina, Mark Pearson
tree: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-7.1/lenovo
head: d2c424e80caf8237bda4c94bc2e25398967243f9
commit: 550752e2c153663c3a374b048535654073007c90 [14/16] HID: hid-lenovo-go-s: Add RGB LED control interface
config: loongarch-randconfig-r053-20260301 (https://download.01.org/0day-ci/archive/20260303/202603030044.qd0XVpQ5-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260303/202603030044.qd0XVpQ5-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603030044.qd0XVpQ5-lkp@intel.com/
All errors (new ones prefixed by >>):
loongarch64-linux-ld: drivers/hid/hid-lenovo-go-s.o: in function `.LANCHOR4':
>> drivers/hid/hid-lenovo-go-s.c:1173: multiple definition of `rgb_enabled'; drivers/hid/hid-lenovo-go.o:drivers/hid/hid-lenovo-go.c:2122: first defined here
loongarch64-linux-ld: drivers/hid/hid-lenovo-go-s.o: in function `.LANCHOR4':
drivers/hid/hid-lenovo-go-s.c:1136: multiple definition of `touchpad_enabled'; drivers/hid/hid-lenovo-go.o:drivers/hid/hid-lenovo-go.c:2083: first defined here
loongarch64-linux-ld: drivers/hid/hid-lenovo-go-s.o: in function `.LANCHOR4':
drivers/hid/hid-lenovo-go-s.c:1054: multiple definition of `gamepad_mode'; drivers/hid/hid-lenovo-go.o:drivers/hid/hid-lenovo-go.c:1792: first defined here
loongarch64-linux-ld: drivers/hid/hid-lenovo-go-s.o: in function `.LANCHOR2':
drivers/hid/hid-lenovo-go-s.c:58: multiple definition of `drvdata'; drivers/hid/hid-lenovo-go.o:drivers/hid/hid-lenovo-go.c:93: first defined here
vim +1173 drivers/hid/hid-lenovo-go-s.c
1171
1172 /* RGB */
> 1173 struct gos_cfg_attr rgb_enabled = { FEATURE_RGB_ENABLE };
1174 LEGOS_DEVICE_ATTR_RW(rgb_enabled, "enabled", index, gamepad);
1175 static DEVICE_ATTR_RO_NAMED(rgb_enabled_index, "enabled_index");
1176
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [BUG] cros_ec: Chromebook side volume keys not generating volume events on UEFI firmware
From: Kokutou Mikan @ 2026-03-02 16:22 UTC (permalink / raw)
To: chrome-platform@lists.linux.dev
Cc: linux-input@vger.kernel.org, bleung@chromium.org,
groeck@chromium.org
[-- Attachment #1.1: Type: text/plain, Size: 2674 bytes --]
Hi maintainers,
I am reporting a ChromeOS EC input issue on a Chromebook platform.
Hardware / firmware:
- Vendor/Product: Google Redrix (HP Elite Dragonfly Chromebook)
- Firmware: MrChromebox-2512.1
Software:
- Ubuntu 25.10
- Tested kernels:
- 6.17.0-14-generic
- 6.18.14-061814-generic
- Session: KDE Wayland
Problem:
- Side volume buttons do not generate Linux input events.
- `evtest` on `cros_ec_buttons` (`/dev/input/event9`) shows no key events while pressing side buttons.
- `evtest --grab /dev/input/event2` also shows no side-button events (normal keys are captured as expected).
Expected:
- Side `Volume Up` / `Volume Down` should produce `KEY_VOLUMEUP` / `KEY_VOLUMEDOWN`.
Observed low-level behavior:
1) EC sees button presses:
- Polling `ectool mkbpget buttons` while pressing side buttons shows state transitions:
- `0x0000` (idle)
- `0x0002` (volume up)
- `0x0004` (volume down)
2) Input node exists and advertises volume keys:
- `cros_ec_buttons` is present in `/proc/bus/input/devices`
- KEY bitmap includes `KEY_VOLUMEDOWN`/`KEY_VOLUMEUP`/`KEY_POWER`
3) Driver reset does not help:
- Unbind/bind of `GOOG0007:00` from `cros-ec-keyb` re-creates the input node
- No side-button events appear afterward
4) Re-tested on 6.18:
- Disabled userspace bridge service to avoid masking native behavior
- `evtest /dev/input/event9` still shows no side-button key events
- `ectool mkbpget buttons` still shows side-button state transitions
So behavior is unchanged between 6.17 and 6.18 on this machine.
Modules/config:
- Loaded: `cros_ec`, `cros_ec_lpcs`, `cros_ec_spi`, `cros_ec_keyb`, `matrix_keymap`
- Kernel config:
- `CONFIG_KEYBOARD_CROS_EC=m`
- `CONFIG_CROS_EC=m`
- `CONFIG_CROS_EC_LPC=m`
- `CONFIG_CROS_EC_SPI=m`
- `CONFIG_CROS_EC_PROTO=m`
Working theory:
- This looks like an event propagation issue in the path:
`MKBP (EC) -> cros_ec_keyb/cros_ec_buttons -> input subsystem`
- EC-level button state is changing, but no corresponding input events are emitted.
Current workaround:
- A userspace bridge polls `ectool mkbpget buttons` and controls volume via `wpctl`.
- This workaround works, but it should not be required.
Questions:
1. Is there a known quirk needed for Redrix / MrChromebox firmware in `cros_ec_keyb`?
2. Is there a known MKBP event handling path where button state changes can be visible via host command but not pushed as input events?
I have attached the artifact bundle: cros-side-volume-artifacts-20260301-202113.tar.gz (includes dmesg, evtest traces, /proc/bus/input/devices, and ectool polling logs).
Thanks.
Mikan
3/3/26
[-- Attachment #1.2: Type: text/html, Size: 3470 bytes --]
[-- Attachment #2: bundle.tar.gz --]
[-- Type: application/gzip, Size: 7049 bytes --]
[-- Attachment #3: side_volume_bridge.tar.gz --]
[-- Type: application/gzip, Size: 5096 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] iio: buffer: cache largest scan element size
From: Nuno Sá @ 2026-03-02 16:18 UTC (permalink / raw)
To: David Lechner, Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Nuno Sá, Andy Shevchenko, Lars Möllendorf,
Lars-Peter Clausen, Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, linux-input, linux-iio,
linux-kernel
In-Reply-To: <d9b14aaa-5c03-4769-b7e5-0a6b3dcbe1f6@baylibre.com>
On Mon, 2026-03-02 at 09:35 -0600, David Lechner wrote:
> On 3/2/26 6:16 AM, Nuno Sá wrote:
> > On Sun, 2026-03-01 at 14:24 -0600, David Lechner wrote:
> > > Cache the largest scan element size of elements enabled 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>
> > > ---
> > > 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 71dfc81cb9e5..83e9392f949f 100644
> > > --- a/drivers/iio/industrialio-buffer.c
> > > +++ b/drivers/iio/industrialio-buffer.c
> > > @@ -765,7 +765,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 *largest_element_size)
> > > {
> > > unsigned int bytes = 0;
> > > int length, i, largest = 0;
> > > @@ -793,6 +794,9 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
> > >
> > > *scan_bytes = ALIGN(bytes, largest);
> > >
> > > + if (largest_element_size)
> > > + *largest_element_size = largest;
> >
> > I might be missing something but it seems we now have two paths:
> >
> > 1. Go with 32 bytes
> > 2. Go with 24 bytes (natural alignment)
>
> Hmm... You are right, but we are safe for now because the only repeat is
> a quaternion, which has repeat of 4, so we don't have a case with e.g.
> 24 bytes right now.
>
> I can do a follow-up to future-proof iio_storage_bytes_for_si() to
> handle repeat = 3 (or any other less likely value).
>
> Without the repeat, the size is based on .storagebits, and .storagebits /
> BITS_PER_BYTE is always a power of 2, so no problem there.
>
> >
> > ABI was not clear so I'm not sure if we do want to enforce/treat repeated values as one single
> > element
>
> It has always been this way and we had some recent breakage because I didn't
> know about this. This patch series is a direct result of that [1] because we
> identified more bugs when analyzing it.
Hmm true. If we shrink the buffer, userspace expecting timestamps at bit 24 will not work.
- Nuno Sá
>
> [1]: https://lore.kernel.org/linux-iio/bug-221077-217253@https.bugzilla.kernel.org%2F/
>
> > If so, nothing to change. But if not, we could re-think the approach and save some bytes.
>
> It's too late and breaks usespace as we have seen.
>
> > Marginal savings though so If having the smaller buffer is not straight enough I would be ok
> > with
> > the simplicity tradeoff.
> >
> > - Nuno Sá
> >
^ permalink raw reply
* Re: [PATCH 4/4] iio: buffer: fix timestamp alignment when quaternion in scan
From: Andy Shevchenko @ 2026-03-02 16:03 UTC (permalink / raw)
To: David Lechner
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman, Jonathan Cameron, Lixu Zhang, linux-input,
linux-iio, linux-kernel
In-Reply-To: <d7809acc-01e8-4dd7-8c6a-a7e8386a9e6b@baylibre.com>
On Mon, Mar 02, 2026 at 09:39:30AM -0600, David Lechner wrote:
> On 3/2/26 2:47 AM, Andy Shevchenko wrote:
> > On Sun, Mar 01, 2026 at 02:24:53PM -0600, David Lechner wrote:
...
> >> - ((int64_t *)data)[ts_offset] = timestamp;
> >
> >> + *(int64_t *)(data + ts_offset) = timestamp;
> >
> > What's the point in modifying this? The comment you added suffice for
> > the original line.
>
> ts_offset is now in bytes rather than number of int64_t elements.
> So ((int64_t *)data)[ts_offset] would now 8x past the end of the array.
So semantic of the variable changed w/o name changing... OK.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 2/2] HID: input: Add HID_BATTERY_QUIRK_DYNAMIC for Elan touchscreens
From: Sebastian Reichel @ 2026-03-02 15:55 UTC (permalink / raw)
To: Hans de Goede
Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input,
ggrundik
In-Reply-To: <06d2504d-2b66-4ae7-94ec-8c062a8c15dc@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 8197 bytes --]
Hi,
On Mon, Mar 02, 2026 at 10:19:59AM +0100, Hans de Goede wrote:
> Hi Dmitry,
>
> On 2-Mar-26 1:44 AM, Dmitry Torokhov wrote:
> > Hi Hans,
> >
> > On Sat, Feb 28, 2026 at 03:52:58PM +0100, Hans de Goede wrote:
> >> Elan touchscreens have a HID-battery device for the stylus which is always
> >> there even if there is no stylus.
> >>
> >> This is causing upower to report an empty battery for the stylus and some
> >> desktop-environments will show a notification about this, which is quite
> >> annoying.
> >>
> >> Because of this the HID-battery is being ignored on all Elan I2c and USB
> >> touchscreens, but this causes there to be no battery reporting for
> >> the stylus at all.
> >>
> >> This adds a new HID_BATTERY_QUIRK_DYNAMIC and uses these for the Elan
> >> touchscreens.
> >>
> >> This new quirks causes the present value of the battery to start at 0,
> >> which will make userspace ignore it and only sets present to 1 after
> >> receiving a battery input report which only happens when the stylus
> >> gets in range.
> >
> > My understanding was that "present" attribute is to be used for
> > removable batteries (i.e. when a battery can be extracted from either
> > from the system or from a peripheral) and we want to notify userspace of
> > that fact).
>
> In this case we have a touchscreen and a stylus with a battery embedded
> in the stylus and we're setting present to 0 when the stylus (and thus
> the battery) is not there. I realize this is a bit different but it
> is close enough IMHO.
>
> Also keep in mind that most people simply do not have the stylus at all,
> these styluses are typically an optional accessory and some models where
> the touchscreen supports this the vendor is not even selling a stylus to
> go with the 2-in-1 laptop (but you may use a compatible stylus from other
> vendors).
>
> TL;DR; the user not having a stylus at all is the most common use case here.
>
> > For example, if a laptop can have an additional battery UI
> > can show if it is installed or not, and it should not simply ignore such
> > power supplies.
>
> As the screenshots of the KDE control-panel in the bug for this:
> https://bugzilla.kernel.org/show_bug.cgi?id=221118
>
> Show it is not being ignored, but its charge-level is ignored,
> with the battery being shown as not present. Which is pretty much
> what we want.
>
> > On the other hand the "Unknown" operating status signals that we
> > actually do not know the state of the battery, and fits better in our
> > situation.
>
> Reporting "Unknown" in the status attribute is not something which
> drivers normally do. It is there in the ABI and upower seems to somewhat
> handle it, but upower only handles it in that it forwards it to upower
> DBUS API users it does not reset history, etc. as it does for present=0.
>
> The problem with status=Unknown is that its meaning is not really well
> defined. Unknown status mostly means that it is unknown if the battery is
> charging / discharging / full for some reason. But if there still is a low
> capacity reading then userspace might still warn about the low capacity in
> this case, since the battery is still almost empty and presumably not
> charging.
>
> Also when the stylus is not there, the battery state is not unknown,
> the entire stylus, including the battery is simply not there, so
> reporting present=0 seems more appropriate and its meaning is well
> defined and present=0 causes upower to not read/report any of the
> other attributes.
>
> And as mentioned most users will not have the stylus I think having
> a non-present "ELAN Touchscreen Stylus battery" being reported makes
> some sense to end users, where as having the same battery being
> reported "Unknown" status and at 0% charge (Unknown status batteries
> do have a charge) is more confusing for users.
>
> > Anyway, I think we need Sebastian's input here.
>
> Ack, Sebastian your input would be appreciated here ?
The rationale and the patch LGTM. The other option would be to not
register the battery device in the first place and do that once the
first report arrived.
Greetings,
-- Sebastian
>
> Regards,
>
> Hans
>
>
>
> >
> >>
> >> Reported-by: ggrundik@gmail.com
> >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221118
> >> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> >> ---
> >> drivers/hid/hid-input.c | 14 +++++++++++---
> >> include/linux/hid.h | 1 +
> >> 2 files changed, 12 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> >> index 67ca1e88ce13..8fc20df99b97 100644
> >> --- a/drivers/hid/hid-input.c
> >> +++ b/drivers/hid/hid-input.c
> >> @@ -354,6 +354,7 @@ static enum power_supply_property hidinput_battery_props[] = {
> >> #define HID_BATTERY_QUIRK_FEATURE (1 << 1) /* ask for feature report */
> >> #define HID_BATTERY_QUIRK_IGNORE (1 << 2) /* completely ignore the battery */
> >> #define HID_BATTERY_QUIRK_AVOID_QUERY (1 << 3) /* do not query the battery */
> >> +#define HID_BATTERY_QUIRK_DYNAMIC (1 << 4) /* report present only after life signs */
> >>
> >> static const struct hid_device_id hid_battery_quirks[] = {
> >> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
> >> @@ -398,8 +399,8 @@ static const struct hid_device_id hid_battery_quirks[] = {
> >> * Elan HID touchscreens seem to all report a non present battery,
> >> * set HID_BATTERY_QUIRK_IGNORE for all Elan I2C and USB HID devices.
> >> */
> >> - { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_IGNORE },
> >> - { HID_USB_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_IGNORE },
> >> + { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_DYNAMIC },
> >> + { HID_USB_DEVICE(USB_VENDOR_ID_ELAN, HID_ANY_ID), HID_BATTERY_QUIRK_DYNAMIC },
> >> {}
> >> };
> >>
> >> @@ -456,11 +457,14 @@ static int hidinput_get_battery_property(struct power_supply *psy,
> >> int ret = 0;
> >>
> >> switch (prop) {
> >> - case POWER_SUPPLY_PROP_PRESENT:
> >> case POWER_SUPPLY_PROP_ONLINE:
> >> val->intval = 1;
> >> break;
> >>
> >> + case POWER_SUPPLY_PROP_PRESENT:
> >> + val->intval = dev->battery_present;
> >> + break;
> >> +
> >> case POWER_SUPPLY_PROP_CAPACITY:
> >> if (dev->battery_status != HID_BATTERY_REPORTED &&
> >> !dev->battery_avoid_query) {
> >> @@ -573,6 +577,8 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> >> if (quirks & HID_BATTERY_QUIRK_AVOID_QUERY)
> >> dev->battery_avoid_query = true;
> >>
> >> + dev->battery_present = (quirks & HID_BATTERY_QUIRK_DYNAMIC) ? false : true;
> >> +
> >> dev->battery = power_supply_register(&dev->dev, psy_desc, &psy_cfg);
> >> if (IS_ERR(dev->battery)) {
> >> error = PTR_ERR(dev->battery);
> >> @@ -628,6 +634,7 @@ static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> >> return;
> >>
> >> if (hidinput_update_battery_charge_status(dev, usage, value)) {
> >> + dev->battery_present = true;
> >> power_supply_changed(dev->battery);
> >> return;
> >> }
> >> @@ -643,6 +650,7 @@ static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> >> if (dev->battery_status != HID_BATTERY_REPORTED ||
> >> capacity != dev->battery_capacity ||
> >> ktime_after(ktime_get_coarse(), dev->battery_ratelimit_time)) {
> >> + dev->battery_present = true;
> >> dev->battery_capacity = capacity;
> >> dev->battery_status = HID_BATTERY_REPORTED;
> >> dev->battery_ratelimit_time =
> >> diff --git a/include/linux/hid.h b/include/linux/hid.h
> >> index dce862cafbbd..d9b54f0e8671 100644
> >> --- a/include/linux/hid.h
> >> +++ b/include/linux/hid.h
> >> @@ -682,6 +682,7 @@ struct hid_device {
> >> __s32 battery_charge_status;
> >> enum hid_battery_status battery_status;
> >> bool battery_avoid_query;
> >> + bool battery_present;
> >> ktime_t battery_ratelimit_time;
> >> #endif
> >>
> >
> > Thanks.
> >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 0/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment
From: David Lechner @ 2026-03-02 15:52 UTC (permalink / raw)
To: Nuno Sá, Andy Shevchenko
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Jiri Kosina,
Srinivas Pandruvada, linux-iio, linux-kernel, Jonathan Cameron,
linux-input, Lixu Zhang
In-Reply-To: <6158f495b13f2a8cdc09f37e296166ccf0393c78.camel@gmail.com>
On 3/2/26 6:32 AM, Nuno Sá wrote:
> On Mon, 2026-03-02 at 10:58 +0200, Andy Shevchenko wrote:
>> On Sat, Feb 28, 2026 at 02:02:21PM -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_QUATERNION(), to declare the
>>> array with proper alignment. This is meant to follow the pattern of
>>> the similar IIO_DECLARE_BUFFER_WITH_TS() macro.
>>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>>
>> But this is in conflict with the other hack-patch in another series.
>> I'm a bit lost what patch 2 fixes here and that hack-patch fixes
>> in the same driver. Shouldn't survive only one?
>
> +1.
>
> I see this is older so should we only look at the other series?
>
> - Nuno Sá
They are fixing two separate bugs.
This bug (which is a recent regression) is that the size of the struct is
wrong. It is supposed to be 32 bytes, but a recent change make it only 24.
This causes iio_push_to_buffers_with_ts() to write over data past the end
of the struct.
The other bug (which has been a bug for 6 years) is that the the timestamp
is in the wrong place. That patch also has an effect of making the struct
the right size, but that is only a side-effect.
So yes, I should have mentioned that in the cover letter that this series
should probably be applied first since it is the worse bug. And it looks
like I'll be doing a v2 of the other series anyway, so I can properly rebase
it and declare the dependency.
^ permalink raw reply
* Re: [PATCH 4/4] iio: buffer: fix timestamp alignment when quaternion in scan
From: David Lechner @ 2026-03-02 15:42 UTC (permalink / raw)
To: Nuno Sá, Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Nuno Sá, Andy Shevchenko, Lars Möllendorf,
Lars-Peter Clausen, Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, linux-input, linux-iio,
linux-kernel
In-Reply-To: <f481f4ae4d9545bbb186e51a1204ca2d65b74a26.camel@gmail.com>
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. :-)
^ permalink raw reply
* Re: [PATCH 4/4] iio: buffer: fix timestamp alignment when quaternion in scan
From: David Lechner @ 2026-03-02 15:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman, Jonathan Cameron, Lixu Zhang, linux-input,
linux-iio, linux-kernel
In-Reply-To: <aaVOrvyzgDKNsB0S@ashevche-desk.local>
On 3/2/26 2:47 AM, Andy Shevchenko wrote:
> On Sun, Mar 01, 2026 at 02:24:53PM -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.
>
> ...
>
>> - ((int64_t *)data)[ts_offset] = timestamp;
>
>> + *(int64_t *)(data + ts_offset) = timestamp;
>
> What's the point in modifying this? The comment you added suffice for
> the original line.
>
ts_offset is now in bytes rather than number of int64_t elements.
So ((int64_t *)data)[ts_offset] would now 8x past the end of the array.
^ permalink raw reply
* Re: [PATCH 3/4] iio: buffer: cache largest scan element size
From: David Lechner @ 2026-03-02 15:35 UTC (permalink / raw)
To: Nuno Sá, Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Nuno Sá, Andy Shevchenko, Lars Möllendorf,
Lars-Peter Clausen, Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, linux-input, linux-iio,
linux-kernel
In-Reply-To: <091936ced74a1eb051dfda009e27bdb8f32426b2.camel@gmail.com>
On 3/2/26 6:16 AM, Nuno Sá wrote:
> On Sun, 2026-03-01 at 14:24 -0600, David Lechner wrote:
>> Cache the largest scan element size of elements enabled 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>
>> ---
>> 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 71dfc81cb9e5..83e9392f949f 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -765,7 +765,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 *largest_element_size)
>> {
>> unsigned int bytes = 0;
>> int length, i, largest = 0;
>> @@ -793,6 +794,9 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
>>
>> *scan_bytes = ALIGN(bytes, largest);
>>
>> + if (largest_element_size)
>> + *largest_element_size = largest;
>
> I might be missing something but it seems we now have two paths:
>
> 1. Go with 32 bytes
> 2. Go with 24 bytes (natural alignment)
Hmm... You are right, but we are safe for now because the only repeat is
a quaternion, which has repeat of 4, so we don't have a case with e.g.
24 bytes right now.
I can do a follow-up to future-proof iio_storage_bytes_for_si() to
handle repeat = 3 (or any other less likely value).
Without the repeat, the size is based on .storagebits, and .storagebits /
BITS_PER_BYTE is always a power of 2, so no problem there.
>
> ABI was not clear so I'm not sure if we do want to enforce/treat repeated values as one single
> element
It has always been this way and we had some recent breakage because I didn't
know about this. This patch series is a direct result of that [1] because we
identified more bugs when analyzing it.
[1]: https://lore.kernel.org/linux-iio/bug-221077-217253@https.bugzilla.kernel.org%2F/
> If so, nothing to change. But if not, we could re-think the approach and save some bytes.
It's too late and breaks usespace as we have seen.
> Marginal savings though so If having the smaller buffer is not straight enough I would be ok with
> the simplicity tradeoff.
>
> - Nuno Sá
>
^ permalink raw reply
* Re: [PATCH 1/4] iio: orientation: hid-sensor-rotation: add timestamp hack to not break userspace
From: David Lechner @ 2026-03-02 15:18 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman, Jonathan Cameron, Lixu Zhang, linux-input,
linux-iio, linux-kernel
In-Reply-To: <aaVPQsvz02S8GtsX@ashevche-desk.local>
On 3/2/26 2:50 AM, Andy Shevchenko wrote:
> On Sun, Mar 01, 2026 at 02:24:50PM -0600, David Lechner 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.
>
> ...
>
>> + /*
>> + * HACK: There are two copies of the same timestamp in case of
>
> Usually we use FIXME in such cases. HACK is something which goes with
> "do not apply".
>
> Does it mean it will stay forever?
Yes, it will have to stay forever because we can't break userspace.
My intention here was for HACK to mean "do not copy to another driver".
If we think that no one is depending on timestamps being in the wrong
offset, we could omit this change and apply only the rest of the series.
And then only apply this patch if anyone complains. I am just trying to
play it safe here and not risk breaking things.
^ permalink raw reply
* Re: [PATCH 2/2] HID: multitouch: Check to ensure report responses match the request
From: Benjamin Tissoires @ 2026-03-02 14:31 UTC (permalink / raw)
To: Lee Jones; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20260227163031.1166560-2-lee@kernel.org>
On Feb 27 2026, Lee Jones wrote:
> It is possible for a malicious (or clumsy) device to respond to a
> specific report's feature request using a completely different report
> ID. This can cause confusion in the HID core resulting in nasty
> side-effects such as OOB writes.
>
> Add a check to ensure that the report ID in the response, matches the
> one that was requested. If it doesn't, omit reporting the raw event and
> return early.
>
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
> drivers/hid/hid-multitouch.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index b1c3ef129058..834c1a334887 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -499,12 +499,19 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
> dev_warn(&hdev->dev, "failed to fetch feature %d\n",
> report->id);
> } else {
> + /* The report ID in the request and the response should match */
> + if (report->id != buf[0]) {
> + hid_err(hdev, "Returned feature report did not match the request\n");
> + goto free;
> + }
> +
AFAICT, hid-vivaldi-common.c is doing the same calls to
hid_report_raw_event() in vivaldi_feature_mapping() and would suffer
from the same bug.
It would be nice if we could have the matching fix.
Cheers,
Benjamin
> ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
> size, 0);
> if (ret)
> dev_warn(&hdev->dev, "failed to report feature\n");
> }
>
> +free:
> kfree(buf);
> }
>
> --
> 2.53.0.473.g4a7958ca14-goog
>
^ permalink raw reply
* Re: [PATCH 1/2] HID: core: Mitigate potential OOB by removing bogus memset()
From: Benjamin Tissoires @ 2026-03-02 14:29 UTC (permalink / raw)
To: Lee Jones; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20260227163031.1166560-1-lee@kernel.org>
On Feb 27 2026, Lee Jones wrote:
> The memset() in hid_report_raw_event() has the good intention of
> clearing out bogus data by zeroing the area from the end of the incoming
> data string to the assumed end of the buffer. However, as we have
> recently seen, the size of the report buffer isn't always correct which
> can culminate in OOB writes.
>
> The current suggestion from one of the HID maintainers is to remove the
> attempt completely. The subsequent handling should be able to simply
> use the data size provided to prevent any potential overruns.
>
> Suggested-by Benjamin Tissoires <bentiss@kernel.org>
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
> drivers/hid/hid-core.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index a5b3a8ca2fcb..1d51042e4b1f 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2056,12 +2056,6 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
> else if (rsize > max_buffer_size)
> rsize = max_buffer_size;
>
> - if (csize < rsize) {
> - dbg_hid("report %d is too short, (%d < %d)\n", report->id,
> - csize, rsize);
> - memset(cdata + csize, 0, rsize - csize);
> - }
> -
Simply removing this check is not enough.
later we have a call to `hid_process_report(hid, report, cdata,
interrupt);`` which loses the size information and which will make an
OOB read while calling hid_input_fetch_field().
I think we should drop here the processing with a warning (maybe rate
limnited), and hope for the best.
Cheers,
Benjamin
> if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_report_event)
> hid->hiddev_report_event(hid, report);
> if (hid->claimed & HID_CLAIMED_HIDRAW) {
> --
> 2.53.0.473.g4a7958ca14-goog
>
^ permalink raw reply
* Re: [PATCH v2 0/2] iio: orientation: hid-sensor-rotation: fix quaternion alignment
From: Nuno Sá @ 2026-03-02 12:32 UTC (permalink / raw)
To: Andy Shevchenko, David Lechner
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Jiri Kosina,
Srinivas Pandruvada, linux-iio, linux-kernel, Jonathan Cameron,
linux-input, Lixu Zhang
In-Reply-To: <aaVRIOejIgnOdAfY@ashevche-desk.local>
On Mon, 2026-03-02 at 10:58 +0200, Andy Shevchenko wrote:
> On Sat, Feb 28, 2026 at 02:02:21PM -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_QUATERNION(), to declare the
> > array with proper alignment. This is meant to follow the pattern of
> > the similar IIO_DECLARE_BUFFER_WITH_TS() macro.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> But this is in conflict with the other hack-patch in another series.
> I'm a bit lost what patch 2 fixes here and that hack-patch fixes
> in the same driver. Shouldn't survive only one?
+1.
I see this is older so should we only look at the other series?
- Nuno Sá
^ permalink raw reply
* Re: [PATCH 3/4] iio: buffer: cache largest scan element size
From: Nuno Sá @ 2026-03-02 12:16 UTC (permalink / raw)
To: David Lechner, Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Nuno Sá, Andy Shevchenko, Lars Möllendorf,
Lars-Peter Clausen, Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, linux-input, linux-iio,
linux-kernel
In-Reply-To: <20260301-iio-fix-timestamp-alignment-v1-3-1a54980bfb90@baylibre.com>
On Sun, 2026-03-01 at 14:24 -0600, David Lechner wrote:
> Cache the largest scan element size of elements enabled 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>
> ---
> 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 71dfc81cb9e5..83e9392f949f 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -765,7 +765,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 *largest_element_size)
> {
> unsigned int bytes = 0;
> int length, i, largest = 0;
> @@ -793,6 +794,9 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
>
> *scan_bytes = ALIGN(bytes, largest);
>
> + if (largest_element_size)
> + *largest_element_size = largest;
I might be missing something but it seems we now have two paths:
1. Go with 32 bytes
2. Go with 24 bytes (natural alignment)
ABI was not clear so I'm not sure if we do want to enforce/treat repeated values as one single
element? If so, nothing to change. But if not, we could re-think the approach and save some bytes.
Marginal savings though so If having the smaller buffer is not straight enough I would be ok with
the simplicity tradeoff.
- Nuno Sá
> +
> return 0;
> }
>
> @@ -848,7 +852,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;
>
> @@ -892,6 +896,7 @@ struct iio_device_config {
> unsigned int watermark;
> const unsigned long *scan_mask;
> unsigned int scan_bytes;
> + unsigned int largest_scan_element_size;
> bool scan_timestamp;
> };
>
> @@ -997,7 +1002,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->largest_scan_element_size);
> if (ret)
> return ret;
>
> @@ -1155,6 +1161,8 @@ 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, largest_scan_element_size) =
> + config->largest_scan_element_size;
> 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 a9ecff191bd9..85bcb5f8ae15 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
> + * @largest_scan_element_size: [INTERN] cache of the largest scan element size
> + * among the channels selected in the scan mask
> * @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 largest_scan_element_size;
>
> const unsigned long *available_scan_masks;
> unsigned int __private masklength;
^ permalink raw reply
* Re: [PATCH 4/4] iio: buffer: fix timestamp alignment when quaternion in scan
From: Nuno Sá @ 2026-03-02 12:04 UTC (permalink / raw)
To: David Lechner, Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Nuno Sá, Andy Shevchenko, Lars Möllendorf,
Lars-Peter Clausen, Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, linux-input, linux-iio,
linux-kernel
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á
>
^ permalink raw reply
* Re: [PATCH 07/10] Input: stmfts - add optional reset GPIO support
From: Krzysztof Kozlowski @ 2026-03-02 11:17 UTC (permalink / raw)
To: Konrad Dybcio, david, Dmitry Torokhov, Maxime Coquelin,
Alexandre Torgue, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bjorn Andersson, Konrad Dybcio
Cc: Petr Hodina, linux-input, linux-stm32, linux-arm-kernel,
linux-kernel, devicetree, linux-arm-msm, phone-devel
In-Reply-To: <5d3694c6-57e7-4943-8dbb-41334086e8ec@oss.qualcomm.com>
On 02/03/2026 12:04, Konrad Dybcio wrote:
> On 3/1/26 6:51 PM, David Heidelberg via B4 Relay wrote:
>> From: Petr Hodina <petr.hodina@protonmail.com>
>>
>> Add support for an optional "reset-gpios" property. If present, the
>> driver drives the reset line high at probe time and releases it during
>> power-on, after the regulators have been enabled.
>>
>> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
>> Co-developed-by: David Heidelberg <david@ixit.cz>
>> Signed-off-by: David Heidelberg <david@ixit.cz>
>> ---
>
> [...]
>
>> + sdata->reset_gpio = devm_gpiod_get_optional(dev, "reset",
>> + GPIOD_OUT_HIGH);
>
> Are you really sure the reset is active-high?
This is not choice of ACTIVE HIGH here. It's initialization to logical
level to keep device in reset state.
Best regards,
Krzysztof
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox