* [hid:for-7.1/lenovo 10/16] drivers/hid/hid-lenovo-go-s.c:164:73: sparse: sparse: Using plain integer as NULL pointer
From: kernel test robot @ 2026-03-07 20:40 UTC (permalink / raw)
To: Derek J. Clark
Cc: oe-kbuild-all, linux-input, Jiri Kosina, Mark Pearson,
Mario Limonciello
tree: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-7.1/lenovo
head: d2c424e80caf8237bda4c94bc2e25398967243f9
commit: 4325fdab5dbbfd467df6797e018c9dd0e5c3ae75 [10/16] HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver
config: arc-randconfig-r113-20260307 (https://download.01.org/0day-ci/archive/20260308/202603080451.lcXQ6ejg-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 13.4.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260308/202603080451.lcXQ6ejg-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/202603080451.lcXQ6ejg-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/hid/hid-lenovo-go-s.c:164:73: sparse: sparse: Using plain integer as NULL pointer
vim +164 drivers/hid/hid-lenovo-go-s.c
159
160 static void cfg_setup(struct work_struct *work)
161 {
162 int ret;
163
> 164 ret = mcu_property_out(drvdata.hdev, GET_VERSION, FEATURE_NONE, 0, 0);
165 if (ret) {
166 dev_err(&drvdata.hdev->dev, "Failed to retrieve MCU Version: %i\n", ret);
167 return;
168 }
169 }
170
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2] usbhid: tolerate intermittent errors
From: Alan Stern @ 2026-03-07 22:52 UTC (permalink / raw)
To: Liam Mitchell
Cc: Jiri Kosina, Benjamin Tissoires, Oliver Neukum, linux-usb,
linux-input, linux-kernel
In-Reply-To: <20260307-usbhid-eproto-v2-1-e5a8abce4652@gmail.com>
On Sat, Mar 07, 2026 at 07:57:09PM +0100, Liam Mitchell wrote:
> Modifies usbhid error handling to tolerate intermittent protocol
> errors, avoiding URB resubmission delay and device reset.
>
> ---
> Protocol errors like EPROTO can occur randomly, sometimes frequently and are often not fixed by a device reset.
>
> The current error handling will only resubmit the URB after at least 13ms delay and may reset the USB device if another error occurs 1-1.5s later, regardless of error type or count.
>
> These delays and device resets increase the chance that input events will be missed and that users see symptoms like missed or sticky keyboard keys.
>
> This patch allows one protocol error per 500ms to be tolerated and have the URB re-submitted immediately.
>
> 500ms was chosen to be well above the error rate of a malfunctioning
> device but low enough to be useful for users with devices noisier than
> mine.
>
> Signed-off-by: Liam Mitchell <mitchell.liam@gmail.com>
> Link: https://lore.kernel.org/linux-input/CAOQ1CL6Q+4GNy=kgisLzs0UBXFT3b02PG8t-0rPuW-Wf6NhQ6g@mail.gmail.com/
> ---
Liam, take a look at
https://bugzilla.kernel.org/show_bug.cgi?id=221184
On Roman's system, these protocol errors occur fairly regularly,
apparently caused by high system load.
Do you think a better approach might be to reduce the 13-ms delay to
just 1 or 2 ms, and perform a reset only there has been no successful
communication for about one second? This might perhaps be _too_ lenient
sometimes, but I don't think such situations will arise in practice.
The reason for having at least a small delay is to avoid getting into a
tight resubmit/error loop in cases where the device has been unplugged.
Alan Stern
^ permalink raw reply
* [hid:for-7.1/lenovo 12/16] drivers/hid/hid-lenovo-go-s.c:583:21: sparse: sparse: symbol 'gamepad_poll_rate' was not declared. Should it be static?
From: kernel test robot @ 2026-03-07 23:36 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: 14651777fd67507d19574cd7e7835c16e6174853 [12/16] HID: hid-lenovo-go-s: Add Feature Status Attributes
config: arc-randconfig-r113-20260307 (https://download.01.org/0day-ci/archive/20260308/202603080756.jqm0j0md-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 13.4.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260308/202603080756.jqm0j0md-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/202603080756.jqm0j0md-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
drivers/hid/hid-lenovo-go-s.c:406:72: sparse: sparse: Using plain integer as NULL pointer
>> drivers/hid/hid-lenovo-go-s.c:583:21: sparse: sparse: symbol 'gamepad_poll_rate' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go-s.c:609:21: sparse: sparse: symbol 'imu_sensor_enabled' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go-s.c:645:21: sparse: sparse: symbol 'mouse_wheel_step' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:690:72: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:697:73: sparse: sparse: Using plain integer as NULL pointer
>> drivers/hid/hid-lenovo-go-s.c:407:21: sparse: sparse: unsigned value that used to be signed checked against zero?
drivers/hid/hid-lenovo-go-s.c:406:33: sparse: signed value source
vim +/gamepad_poll_rate +583 drivers/hid/hid-lenovo-go-s.c
398
399 static ssize_t gamepad_property_show(struct device *dev,
400 struct device_attribute *attr, char *buf,
401 enum feature_status_index index)
402 {
403 size_t count = 0;
404 u8 i;
405
406 count = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, index, 0, 0);
> 407 if (count < 0)
408 return count;
409
410 switch (index) {
411 case FEATURE_GAMEPAD_MODE:
412 i = drvdata.gp_mode;
413 if (i >= ARRAY_SIZE(gamepad_mode_text))
414 return -EINVAL;
415 count = sysfs_emit(buf, "%s\n", gamepad_mode_text[i]);
416 break;
417 case FEATURE_AUTO_SLEEP_TIME:
418 count = sysfs_emit(buf, "%u\n", drvdata.gp_auto_sleep_time);
419 break;
420 case FEATURE_IMU_ENABLE:
421 i = drvdata.imu_sensor_en;
422 if (i >= ARRAY_SIZE(feature_enabled_text))
423 return -EINVAL;
424 count = sysfs_emit(buf, "%s\n", feature_enabled_text[i]);
425 break;
426 case FEATURE_IMU_BYPASS:
427 i = drvdata.imu_bypass_en;
428 if (i >= ARRAY_SIZE(feature_enabled_text))
429 return -EINVAL;
430 count = sysfs_emit(buf, "%s\n", feature_enabled_text[i]);
431 break;
432 case FEATURE_RGB_ENABLE:
433 i = drvdata.rgb_en;
434 if (i >= ARRAY_SIZE(feature_enabled_text))
435 return -EINVAL;
436 count = sysfs_emit(buf, "%s\n", feature_enabled_text[i]);
437 break;
438 case FEATURE_TOUCHPAD_ENABLE:
439 i = drvdata.tp_en;
440 if (i >= ARRAY_SIZE(feature_enabled_text))
441 return -EINVAL;
442 count = sysfs_emit(buf, "%s\n", feature_enabled_text[i]);
443 break;
444 case FEATURE_OS_MODE:
445 i = drvdata.os_mode;
446 if (i >= ARRAY_SIZE(os_type_text))
447 return -EINVAL;
448 count = sysfs_emit(buf, "%s\n", os_type_text[i]);
449 break;
450 case FEATURE_POLL_RATE:
451 i = drvdata.gp_poll_rate;
452 if (i >= ARRAY_SIZE(poll_rate_text))
453 return -EINVAL;
454 count = sysfs_emit(buf, "%s\n", poll_rate_text[i]);
455 break;
456 case FEATURE_DPAD_MODE:
457 i = drvdata.gp_dpad_mode;
458 if (i >= ARRAY_SIZE(dpad_mode_text))
459 return -EINVAL;
460 count = sysfs_emit(buf, "%s\n", dpad_mode_text[i]);
461 break;
462 case FEATURE_MOUSE_WHEEL_STEP:
463 i = drvdata.mouse_step;
464 if (i < 1 || i > 127)
465 return -EINVAL;
466 count = sysfs_emit(buf, "%u\n", i);
467 break;
468 default:
469 return -EINVAL;
470 }
471
472 return count;
473 }
474
475 static ssize_t gamepad_property_options(struct device *dev,
476 struct device_attribute *attr,
477 char *buf,
478 enum feature_status_index index)
479 {
480 size_t count = 0;
481 unsigned int i;
482
483 switch (index) {
484 case FEATURE_GAMEPAD_MODE:
485 for (i = 0; i < ARRAY_SIZE(gamepad_mode_text); i++) {
486 count += sysfs_emit_at(buf, count, "%s ",
487 gamepad_mode_text[i]);
488 }
489 break;
490 case FEATURE_AUTO_SLEEP_TIME:
491 return sysfs_emit(buf, "0-255\n");
492 case FEATURE_IMU_ENABLE:
493 for (i = 0; i < ARRAY_SIZE(feature_enabled_text); i++) {
494 count += sysfs_emit_at(buf, count, "%s ",
495 feature_enabled_text[i]);
496 }
497 break;
498 case FEATURE_IMU_BYPASS:
499 case FEATURE_RGB_ENABLE:
500 case FEATURE_TOUCHPAD_ENABLE:
501 for (i = 0; i < ARRAY_SIZE(feature_enabled_text); i++) {
502 count += sysfs_emit_at(buf, count, "%s ",
503 feature_enabled_text[i]);
504 }
505 break;
506 case FEATURE_OS_MODE:
507 for (i = 0; i < ARRAY_SIZE(os_type_text); i++) {
508 count += sysfs_emit_at(buf, count, "%s ",
509 os_type_text[i]);
510 }
511 break;
512 case FEATURE_POLL_RATE:
513 for (i = 0; i < ARRAY_SIZE(poll_rate_text); i++) {
514 count += sysfs_emit_at(buf, count, "%s ",
515 poll_rate_text[i]);
516 }
517 break;
518 case FEATURE_DPAD_MODE:
519 for (i = 0; i < ARRAY_SIZE(dpad_mode_text); i++) {
520 count += sysfs_emit_at(buf, count, "%s ",
521 dpad_mode_text[i]);
522 }
523 break;
524 case FEATURE_MOUSE_WHEEL_STEP:
525 return sysfs_emit(buf, "1-127\n");
526 default:
527 return count;
528 }
529
530 if (count)
531 buf[count - 1] = '\n';
532
533 return count;
534 }
535
536 static ssize_t mcu_id_show(struct device *dev, struct device_attribute *attr,
537 char *buf)
538 {
539 return sysfs_emit(buf, "%*phN\n", 12, &drvdata.mcu_id);
540 }
541
542 #define LEGOS_DEVICE_ATTR_RW(_name, _attrname, _rtype, _group) \
543 static ssize_t _name##_store(struct device *dev, \
544 struct device_attribute *attr, \
545 const char *buf, size_t count) \
546 { \
547 return _group##_property_store(dev, attr, buf, count, \
548 _name.index); \
549 } \
550 static ssize_t _name##_show(struct device *dev, \
551 struct device_attribute *attr, char *buf) \
552 { \
553 return _group##_property_show(dev, attr, buf, _name.index); \
554 } \
555 static ssize_t _name##_##_rtype##_show( \
556 struct device *dev, struct device_attribute *attr, char *buf) \
557 { \
558 return _group##_property_options(dev, attr, buf, _name.index); \
559 } \
560 static DEVICE_ATTR_RW_NAMED(_name, _attrname)
561
562 #define LEGOS_DEVICE_ATTR_RO(_name, _attrname, _group) \
563 static ssize_t _name##_show(struct device *dev, \
564 struct device_attribute *attr, char *buf) \
565 { \
566 return _group##_property_show(dev, attr, buf, _name.index); \
567 } \
568 static DEVICE_ATTR_RO_NAMED(_name, _attrname)
569
570 /* Gamepad */
571 struct gos_cfg_attr auto_sleep_time = { FEATURE_AUTO_SLEEP_TIME };
572 LEGOS_DEVICE_ATTR_RW(auto_sleep_time, "auto_sleep_time", range, gamepad);
573 static DEVICE_ATTR_RO(auto_sleep_time_range);
574
575 struct gos_cfg_attr dpad_mode = { FEATURE_DPAD_MODE };
576 LEGOS_DEVICE_ATTR_RW(dpad_mode, "dpad_mode", index, gamepad);
577 static DEVICE_ATTR_RO(dpad_mode_index);
578
579 struct gos_cfg_attr gamepad_mode = { FEATURE_GAMEPAD_MODE };
580 LEGOS_DEVICE_ATTR_RW(gamepad_mode, "mode", index, gamepad);
581 static DEVICE_ATTR_RO_NAMED(gamepad_mode_index, "mode_index");
582
> 583 struct gos_cfg_attr gamepad_poll_rate = { FEATURE_POLL_RATE };
584 LEGOS_DEVICE_ATTR_RW(gamepad_poll_rate, "poll_rate", index, gamepad);
585 static DEVICE_ATTR_RO_NAMED(gamepad_poll_rate_index, "poll_rate_index");
586
587 static struct attribute *legos_gamepad_attrs[] = {
588 &dev_attr_auto_sleep_time.attr,
589 &dev_attr_auto_sleep_time_range.attr,
590 &dev_attr_dpad_mode.attr,
591 &dev_attr_dpad_mode_index.attr,
592 &dev_attr_gamepad_mode.attr,
593 &dev_attr_gamepad_mode_index.attr,
594 &dev_attr_gamepad_poll_rate.attr,
595 &dev_attr_gamepad_poll_rate_index.attr,
596 NULL,
597 };
598
599 static const struct attribute_group gamepad_attr_group = {
600 .name = "gamepad",
601 .attrs = legos_gamepad_attrs,
602 };
603
604 /* IMU */
605 struct gos_cfg_attr imu_bypass_enabled = { FEATURE_IMU_BYPASS };
606 LEGOS_DEVICE_ATTR_RW(imu_bypass_enabled, "bypass_enabled", index, gamepad);
607 static DEVICE_ATTR_RO_NAMED(imu_bypass_enabled_index, "bypass_enabled_index");
608
> 609 struct gos_cfg_attr imu_sensor_enabled = { FEATURE_IMU_ENABLE };
610 LEGOS_DEVICE_ATTR_RW(imu_sensor_enabled, "sensor_enabled", index, gamepad);
611 static DEVICE_ATTR_RO_NAMED(imu_sensor_enabled_index, "sensor_enabled_index");
612
613 static struct attribute *legos_imu_attrs[] = {
614 &dev_attr_imu_bypass_enabled.attr,
615 &dev_attr_imu_bypass_enabled_index.attr,
616 &dev_attr_imu_sensor_enabled.attr,
617 &dev_attr_imu_sensor_enabled_index.attr,
618 NULL,
619 };
620
621 static const struct attribute_group imu_attr_group = {
622 .name = "imu",
623 .attrs = legos_imu_attrs,
624 };
625
626 /* MCU */
627 static DEVICE_ATTR_RO(mcu_id);
628
629 struct gos_cfg_attr os_mode = { FEATURE_OS_MODE };
630 LEGOS_DEVICE_ATTR_RW(os_mode, "os_mode", index, gamepad);
631 static DEVICE_ATTR_RO(os_mode_index);
632
633 static struct attribute *legos_mcu_attrs[] = {
634 &dev_attr_mcu_id.attr,
635 &dev_attr_os_mode.attr,
636 &dev_attr_os_mode_index.attr,
637 NULL,
638 };
639
640 static const struct attribute_group mcu_attr_group = {
641 .attrs = legos_mcu_attrs,
642 };
643
644 /* Mouse */
> 645 struct gos_cfg_attr mouse_wheel_step = { FEATURE_MOUSE_WHEEL_STEP };
646 LEGOS_DEVICE_ATTR_RW(mouse_wheel_step, "step", range, gamepad);
647 static DEVICE_ATTR_RO_NAMED(mouse_wheel_step_range, "step_range");
648
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 1/2] iio: add IIO_DECLARE_QUATERNION() macro
From: David Lechner @ 2026-03-08 0:35 UTC (permalink / raw)
To: Francesco Lavra
Cc: linux-iio, linux-kernel, Jonathan Cameron, linux-input,
Jonathan Cameron, Nuno Sá, Andy Shevchenko, Jiri Kosina,
Srinivas Pandruvada
In-Reply-To: <20260228-iio-fix-repeat-alignment-v2-1-d58bfaa2920d@baylibre.com>
On 2/28/26 2:02 PM, David Lechner wrote:
> Add a new IIO_DECLARE_QUATERNION() macro that is used to declare the
> field in an IIO buffer struct that contains a quaternion vector.
>
> Quaternions are currently the only IIO data type that uses the .repeat
> feature of struct iio_scan_type. This has an implicit rule that the
> element in the buffer must be aligned to the entire size of the repeated
> element. This macro will make that requirement explicit. Since this is
> the only user, we just call the macro IIO_DECLARE_QUATERNION() instead
> of something more generic.
>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> include/linux/iio/iio.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index a9ecff191bd9..2c91b7659ce9 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -931,6 +931,18 @@ static inline void *iio_device_get_drvdata(const struct iio_dev *indio_dev)
> #define IIO_DECLARE_DMA_BUFFER_WITH_TS(type, name, count) \
> __IIO_DECLARE_BUFFER_WITH_TS(type, name, count) __aligned(IIO_DMA_MINALIGN)
>
> +/**
> + * IIO_DECLARE_QUATERNION() - Declare a quaternion element
> + * @type: element type of the individual vectors
> + * @name: identifier name
> + *
> + * Quaternions are a vector composed of 4 elements (W, X, Y, Z). Use this macro
> + * to declare a quaternion element in a struct to ensure proper alignment in
> + * an IIO buffer.
> + */
> +#define IIO_DECLARE_QUATERNION(type, name) \
> + type name[4] __aligned(sizeof(type) * 4)
> +
> struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv);
>
> /* The information at the returned address is guaranteed to be cacheline aligned */
>
Hi Francesco,
I should have cc'ed you on this one. We'll want to add another macro
like this for IIO_DECLARE_QUATERNION_AXIS(), I imagine. (This has been
applied to the fixes-togreg branch since it is a dependency to a fix.)
What to do on that for the alignment though is an open question since
we've never had a repeat of 3 before. The question is if it is OK to
have an alignment that isn't a power of two bytes. For your case, since
the data is 3 x 16-bit, it would be 3 * 2 = 6 bytes.
^ permalink raw reply
* [PATCH v2 0/5] iio: buffer: fix timestamp alignment (in rare case)
From: David Lechner @ 2026-03-08 1:44 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, Francesco Lavra, linux-input,
linux-iio, linux-kernel, 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>
^ permalink raw reply
* [PATCH v2 1/5] iio: orientation: hid-sensor-rotation: add timestamp hack to not break userspace
From: David Lechner @ 2026-03-08 1:44 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, Francesco Lavra, linux-input,
linux-iio, linux-kernel, David Lechner
In-Reply-To: <20260307-iio-fix-timestamp-alignment-v2-0-d1d48fbadbbf@baylibre.com>
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
^ permalink raw reply related
* [PATCH v2 2/5] iio: buffer: check return value of iio_compute_scan_bytes()
From: David Lechner @ 2026-03-08 1:44 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, Francesco Lavra, linux-input,
linux-iio, linux-kernel, David Lechner
In-Reply-To: <20260307-iio-fix-timestamp-alignment-v2-0-d1d48fbadbbf@baylibre.com>
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
^ permalink raw reply related
* [PATCH v2 3/5] iio: buffer: cache timestamp offset in scan buffer
From: David Lechner @ 2026-03-08 1:44 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, Francesco Lavra, linux-input,
linux-iio, linux-kernel, David Lechner
In-Reply-To: <20260307-iio-fix-timestamp-alignment-v2-0-d1d48fbadbbf@baylibre.com>
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
^ permalink raw reply related
* [PATCH v2 4/5] iio: buffer: ensure repeat alignment is a power of two
From: David Lechner @ 2026-03-08 1:44 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, Francesco Lavra, linux-input,
linux-iio, linux-kernel, David Lechner
In-Reply-To: <20260307-iio-fix-timestamp-alignment-v2-0-d1d48fbadbbf@baylibre.com>
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
^ permalink raw reply related
* [PATCH v2 5/5] iio: buffer: fix timestamp alignment when quaternion in scan
From: David Lechner @ 2026-03-08 1:44 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, Nuno Sá,
Andy Shevchenko, Lars Möllendorf, Lars-Peter Clausen,
Greg Kroah-Hartman
Cc: Jonathan Cameron, Lixu Zhang, Francesco Lavra, linux-input,
linux-iio, linux-kernel, David Lechner
In-Reply-To: <20260307-iio-fix-timestamp-alignment-v2-0-d1d48fbadbbf@baylibre.com>
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
^ permalink raw reply related
* [hid:for-7.1/lenovo 13/16] drivers/hid/hid-lenovo-go-s.c:795:21: sparse: sparse: symbol 'touchpad_linux_mode' was not declared. Should it be static?
From: kernel test robot @ 2026-03-08 2:37 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: f3ac4e11aaf3cd334d7f2cb205851bd157a2535f [13/16] HID: hid-lenovo-go-s: Add Touchpad Mode Attributes
config: arc-randconfig-r113-20260307 (https://download.01.org/0day-ci/archive/20260308/202603081041.UgxXYvsF-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 13.4.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260308/202603081041.UgxXYvsF-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/202603081041.UgxXYvsF-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
drivers/hid/hid-lenovo-go-s.c:447:72: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:619:67: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:713:21: sparse: sparse: symbol 'gamepad_poll_rate' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:739:21: sparse: sparse: symbol 'imu_sensor_enabled' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:775:21: sparse: sparse: symbol 'mouse_wheel_step' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go-s.c:795:21: sparse: sparse: symbol 'touchpad_linux_mode' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go-s.c:799:21: sparse: sparse: symbol 'touchpad_windows_mode' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:832:72: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:839:73: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:448:21: sparse: sparse: unsigned value that used to be signed checked against zero?
drivers/hid/hid-lenovo-go-s.c:447:33: sparse: signed value source
vim +/touchpad_linux_mode +795 drivers/hid/hid-lenovo-go-s.c
794
> 795 struct gos_cfg_attr touchpad_linux_mode = { CFG_LINUX_MODE };
796 LEGOS_DEVICE_ATTR_RW(touchpad_linux_mode, "linux_mode", index, touchpad);
797 static DEVICE_ATTR_RO_NAMED(touchpad_linux_mode_index, "linux_mode_index");
798
> 799 struct gos_cfg_attr touchpad_windows_mode = { CFG_WINDOWS_MODE };
800 LEGOS_DEVICE_ATTR_RW(touchpad_windows_mode, "windows_mode", index, touchpad);
801 static DEVICE_ATTR_RO_NAMED(touchpad_windows_mode_index, "windows_mode_index");
802
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [hid:for-7.1/lenovo 14/16] drivers/hid/hid-lenovo-go-s.c:1204:18: sparse: sparse: symbol 'gos_rgb_subled_info' was not declared. Should it be static?
From: kernel test robot @ 2026-03-08 5:36 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: arc-randconfig-r113-20260307 (https://download.01.org/0day-ci/archive/20260308/202603081333.9m9vOhOR-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 13.4.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260308/202603081333.9m9vOhOR-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/202603081333.9m9vOhOR-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
drivers/hid/hid-lenovo-go-s.c:522:72: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:694:67: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:765:63: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:909:71: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:964:74: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:1058:21: sparse: sparse: symbol 'gamepad_poll_rate' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1084:21: sparse: sparse: symbol 'imu_sensor_enabled' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1120:21: sparse: sparse: symbol 'mouse_wheel_step' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1140:21: sparse: sparse: symbol 'touchpad_linux_mode' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1144:21: sparse: sparse: symbol 'touchpad_windows_mode' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go-s.c:1204:18: sparse: sparse: symbol 'gos_rgb_subled_info' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go-s.c:1225:24: sparse: sparse: symbol 'gos_cdev_rgb' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1241:72: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:1248:73: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:523:21: sparse: sparse: unsigned value that used to be signed checked against zero?
drivers/hid/hid-lenovo-go-s.c:522:33: sparse: signed value source
vim +/gos_rgb_subled_info +1204 drivers/hid/hid-lenovo-go-s.c
1203
> 1204 struct mc_subled gos_rgb_subled_info[] = {
1205 {
1206 .color_index = LED_COLOR_ID_RED,
1207 .brightness = 0x50,
1208 .intensity = 0x24,
1209 .channel = 0x1,
1210 },
1211 {
1212 .color_index = LED_COLOR_ID_GREEN,
1213 .brightness = 0x50,
1214 .intensity = 0x22,
1215 .channel = 0x2,
1216 },
1217 {
1218 .color_index = LED_COLOR_ID_BLUE,
1219 .brightness = 0x50,
1220 .intensity = 0x99,
1221 .channel = 0x3,
1222 },
1223 };
1224
> 1225 struct led_classdev_mc gos_cdev_rgb = {
1226 .led_cdev = {
1227 .name = "go_s:rgb:joystick_rings",
1228 .brightness = 0x50,
1229 .max_brightness = 0x64,
1230 .brightness_set = hid_gos_brightness_set,
1231 },
1232 .num_colors = ARRAY_SIZE(gos_rgb_subled_info),
1233 .subled_info = gos_rgb_subled_info,
1234 };
1235
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: Add support for HID++ Multi-Platform feature (0x4531)
From: dev exalt @ 2026-03-08 8:08 UTC (permalink / raw)
To: jikos, bentiss
Cc: lains, hadess, linux-input, linux-kernel, sari.kreitem, hbarnor
In-Reply-To: <CAJaUH_8A70=_Cb8yCWqJxbjpW-BnK958fExnC1kSgyhVaydbUw@mail.gmail.com>
Dear maintainers,
We would like to kindly follow up on this patch sent on Dec 15, 2025,
as we haven't received feedback yet.
Patch link:
https://lore.kernel.org/linux-input/20251215125319.33261-1-exalt.dev.team@gmail.com/T/#u
We understand maintainers are busy, so just sending a gentle reminder
in case this slipped.
Thank you for your time and review.
Best regards,
Baraa Atta (Dev Exalt) <exalt.dev.team@gmail.com>
On Sun, Mar 8, 2026 at 10:01 AM dev exalt <exalt.dev.team@gmail.com> wrote:
>
> Dear maintainers,
>
>
> We would like to kindly follow up on this patch sent on Dec 15, 2025, as we haven't received feedback yet.
>
> Patch link:
> https://lore.kernel.org/linux-input/20251215125319.33261-1-exalt.dev.team@gmail.com/T/#u
>
> We understand maintainers are busy, so just sending a gentle reminder in case this slipped.
>
> Thank you for your time and review.
>
>
> Best regards,
>
> Baraa Atta (Dev Exalt) <exalt.dev.team@gmail.com>
>
>
> On Mon, Dec 15, 2025 at 2:53 PM DevExalt <exalt.dev.team@gmail.com> wrote:
>>
>> From: "Baraa Atta (Dev Exalt)" <exalt.dev.team@gmail.com>
>>
>> Add support in the Logitech HID++ driver for the HID++ Multi-Platform
>> feature (0x4531), which enables HID++ devices to adjust their behavior
>> based on the host operating system (Linux, ChromeOS, Android).
>>
>> This patch:
>> * Adds device IDs for MX Keys S (046d:b378) and Casa Keys (046d:b371).
>> * Introduces the module parameter "hidpp_platform" to allow selecting a
>> target platform.
>> * Detects whether a device implements feature 0x4531.
>> * Validates that the requested platform is supported by the device.
>> * Applies the platform index when valid, otherwise leaves the device
>> unchanged.
>> * Keeps default behavior when "hidpp_platform" is unset or invalid.
>>
>> Supported values for hidpp_platform:
>> Android, Linux, Chrome
>>
>> TEST=Pair MX Keys S and Casa Keys over Bluetooth and verify:
>> * Feature 0x4531 is detected.
>> * Valid platform values are accepted and applied.
>> * Invalid platform values result in no update.
>> * Devices without 0x4531 retain default behavior.
>> * Platform-specific key behavior is observed once applied.
>>
>> Signed-off-by: Baraa Atta (Dev Exalt) <exalt.dev.team@gmail.com>
>> ---
>> drivers/hid/hid-ids.h | 2 +
>> drivers/hid/hid-logitech-hidpp.c | 280 +++++++++++++++++++++++++++++++
>> drivers/hid/hid-quirks.c | 2 +
>> 3 files changed, 284 insertions(+)
>>
>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> index d31711f1aaec..12de1194d7fa 100644
>> --- a/drivers/hid/hid-ids.h
>> +++ b/drivers/hid/hid-ids.h
>> @@ -866,6 +866,8 @@
>> #define USB_DEVICE_ID_LOGITECH_T651 0xb00c
>> #define USB_DEVICE_ID_LOGITECH_DINOVO_EDGE_KBD 0xb309
>> #define USB_DEVICE_ID_LOGITECH_CASA_TOUCHPAD 0xbb00
>> +#define USB_DEVICE_ID_LOGITECH_CASA_KEYS_KEYBOARD 0xb371
>> +#define USB_DEVICE_ID_LOGITECH_MX_KEYS_S_KEYBOARD 0xb378
>> #define USB_DEVICE_ID_LOGITECH_C007 0xc007
>> #define USB_DEVICE_ID_LOGITECH_C077 0xc077
>> #define USB_DEVICE_ID_LOGITECH_RECEIVER 0xc101
>> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
>> index d5011a5d0890..e94daed31981 100644
>> --- a/drivers/hid/hid-logitech-hidpp.c
>> +++ b/drivers/hid/hid-logitech-hidpp.c
>> @@ -4373,6 +4373,280 @@ static bool hidpp_application_equals(struct hid_device *hdev,
>> return report && report->application == application;
>> }
>>
>> +/* -------------------------------------------------------------------------- */
>> +/* 0x4531: Multi-Platform Support */
>> +/* -------------------------------------------------------------------------- */
>> +
>> +/*
>> + * Some Logitech devices expose the HID++ feature 0x4531 (Multi-Platform) allowing
>> + * the host to specify which operating system platform to use on the device. Changing device's
>> + * platform may alter the behavior of the device to match the specified platform.
>> + */
>> +
>> +static char *hidpp_platform;
>> +module_param(hidpp_platform, charp, 0644);
>> +MODULE_PARM_DESC(hidpp_platform, "Select host platform type for Logitech HID++ Multi-Platform feature "
>> + "0x4531, valid values: (linux|chrome|android). If unset, no "
>> + "change is applied.");
>> +
>> +#define HIDPP_MULTIPLATFORM_FEAT_ID 0x4531
>> +#define HIDPP_MULTIPLATFORM_GET_FEATURE_INFO 0x0F
>> +#define HIDPP_MULTIPLATFORM_GET_PLATFORM_DESCRIPTOR 0x1F
>> +#define HIDPP_MULTIPLATFORM_SET_CURRENT_PLATFORM 0x3F
>> +
>> +#define HIDPP_MULTIPLATFORM_PLATFORM_MASK_LINUX BIT(10)
>> +#define HIDPP_MULTIPLATFORM_PLATFORM_MASK_CHROME BIT(11)
>> +#define HIDPP_MULTIPLATFORM_PLATFORM_MASK_ANDROID BIT(12)
>> +
>> +struct hidpp_platform_desc {
>> + u8 plat_idx;
>> + u8 desc_idx;
>> + u16 plat_mask;
>> +};
>> +
>> +/**
>> + * hidpp_multiplatform_mask_from_str() - Convert platform name to an HID++ platform mask
>> + * @pname: Platform name string
>> + *
>> + * Converts a platform name string to its corresponding HID++ platform mask based on
>> + * the Multi-Platform feature specification.
>> + *
>> + * Return: Platform mask corresponding to @pname on success,
>> + * or 0 if @pname is NULL or unsupported.
>> + */
>> +static u16 hidpp_multiplatform_mask_from_str(const char *pname)
>> +{
>> + if (!pname)
>> + return 0;
>> +
>> + if (!strcasecmp(pname, "linux"))
>> + return HIDPP_MULTIPLATFORM_PLATFORM_MASK_LINUX;
>> + if (!strcasecmp(pname, "chrome"))
>> + return HIDPP_MULTIPLATFORM_PLATFORM_MASK_CHROME;
>> + if (!strcasecmp(pname, "android"))
>> + return HIDPP_MULTIPLATFORM_PLATFORM_MASK_ANDROID;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * hidpp_multiplatform_get_num_pdesc() - Retrieve number of platform descriptors
>> + * @hidpp: Pointer to the hidpp_device instance
>> + * @feat_index: Feature index of the Multi-Platform feature
>> + * @num_desc: Pointer to store the number of platform descriptors
>> + *
>> + * Retrieves the number of platform descriptors supported by the device through
>> + * the Multi-Platform feature and stores it in @num_desc.
>> + *
>> + * Return: 0 on success, or non-zero on failure.
>> + */
>> +static int hidpp_multiplatform_get_num_pdesc(struct hidpp_device *hidpp,
>> + u8 feat_index, u8 *num_desc)
>> +{
>> + int ret;
>> + struct hidpp_report response;
>> + struct hid_device *hdev = hidpp->hid_dev;
>> +
>> + ret = hidpp_send_fap_command_sync(hidpp, feat_index,
>> + HIDPP_MULTIPLATFORM_GET_FEATURE_INFO,
>> + NULL, 0, &response);
>> + if (ret) {
>> + hid_warn(hdev, "Multiplatform: GET_FEATURE_INFO failed (err=%d)", ret);
>> + return ret;
>> + }
>> +
>> + *num_desc = response.fap.params[3];
>> + hid_dbg(hdev, "Multiplatform: Device supports %d platform descriptors", *num_desc);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * hidpp_multiplatform_get_platform_desc() - Retrieve a platform descriptor entry
>> + * @hidpp: Pointer to the hidpp_device instance
>> + * @feat_index: Feature index of the Multi-Platform feature
>> + * @platform_idx: Index of the platform descriptor to retrieve
>> + * @pdesc: Pointer to store the retrieved platform descriptor
>> + *
>> + * Retrieves a single platform descriptor identified by @platform_idx from the
>> + * device and stores the parsed descriptor fields in @pdesc.
>> + *
>> + * Return: 0 on success, or non-zero on failure.
>> + */
>> +static int hidpp_multiplatform_get_platform_desc(struct hidpp_device *hidpp, u8 feat_index,
>> + u8 platform_idx, struct hidpp_platform_desc *pdesc)
>> +{
>> + int ret;
>> + struct hidpp_report response;
>> + u8 params[1] = { platform_idx };
>> + struct hid_device *hdev = hidpp->hid_dev;
>> +
>> + ret = hidpp_send_fap_command_sync(hidpp, feat_index,
>> + HIDPP_MULTIPLATFORM_GET_PLATFORM_DESCRIPTOR,
>> + params, sizeof(params), &response);
>> +
>> + if (ret) {
>> + hid_warn(hdev,
>> + "Multiplatform: GET_PLATFORM_DESCRIPTOR failed for index %d (err=%d)",
>> + platform_idx, ret);
>> + return ret;
>> + }
>> +
>> + pdesc->plat_idx = response.fap.params[0];
>> + pdesc->desc_idx = response.fap.params[1];
>> + pdesc->plat_mask = get_unaligned_be16(&response.fap.params[2]);
>> +
>> + hid_dbg(hdev,
>> + "Multiplatform: descriptor %d: plat_idx=%d, desc_idx=%d, plat_mask=0x%04x",
>> + platform_idx, pdesc->plat_idx, pdesc->desc_idx, pdesc->plat_mask);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * hidpp_multiplatform_get_platform_index() - Find platform index for a mask
>> + * @hidpp: Pointer to the hidpp_device instance
>> + * @feat_index: Feature index of the Multi-Platform feature
>> + * @plat_mask: Platform mask to search for
>> + * @plat_index: Pointer to store the matched platform index
>> + *
>> + * Iterates through all platform descriptors exposed by the device via the
>> + * Multi-Platform feature, retrieving each descriptor and comparing its
>> + * platform mask to @plat_mask. A descriptor matches if its mask overlaps with
>> + * the requested @plat_mask (i.e. (pdesc.plat_mask & plat_mask) is non-zero).
>> + *
>> + * When a matching descriptor is found, its platform index (plat_idx) is
>> + * written to @plat_index and the function returns success.
>> + *
>> + * If no descriptor matches, -ENOENT is returned.
>> + *
>> + * Return: 0 on success; -ENOENT if no matching descriptor exists;
>> + * or non-zero on failure.
>> + */
>> +static int hidpp_multiplatform_get_platform_index(struct hidpp_device *hidpp,
>> + u8 feat_index, u16 plat_mask,
>> + u8 *plat_index)
>> +{
>> + int i;
>> + int ret;
>> + u8 num_desc;
>> + struct hidpp_platform_desc pdesc;
>> + struct hid_device *hdev = hidpp->hid_dev;
>> +
>> + ret = hidpp_multiplatform_get_num_pdesc(hidpp, feat_index, &num_desc);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; i < num_desc; i++) {
>> + ret = hidpp_multiplatform_get_platform_desc(hidpp, feat_index, i, &pdesc);
>> + if (ret)
>> + return ret;
>> +
>> + if (pdesc.plat_mask & plat_mask) {
>> + *plat_index = pdesc.plat_idx;
>> + hid_dbg(hdev,
>> + "Multiplatform: Selected platform index %d for platform '%s'",
>> + *plat_index, hidpp_platform);
>> + return 0;
>> + }
>> + }
>> +
>> + hid_dbg(hdev,
>> + "Multiplatform: No matching platform descriptor found for platform '%s'",
>> + hidpp_platform);
>> + return -ENOENT;
>> +}
>> +
>> +/**
>> + * hidpp_multiplatform_update_device_platform() - Update the device platform
>> + * @hidpp: Pointer to the hidpp_device instance
>> + * @feat_index: Feature index of the Multi-Platform feature
>> + * @plat_index: Platform index to set on the device
>> + *
>> + * Sends the HID++ Multi-Platform 'SET_CURRENT_PLATFORM' command to the device to
>> + * update its platform index to @plat_index.
>> + *
>> + * Return: 0 on success, or non-zero on failure.
>> + */
>> +static int hidpp_multiplatform_update_device_platform(struct hidpp_device *hidpp,
>> + u8 feat_index, u8 plat_index)
>> +{
>> + int ret;
>> + struct hidpp_report response;
>> + /* Byte 0 (hostIndex): 0xFF selects the current host. */
>> + u8 params[2] = { 0xFF, plat_index };
>> +
>> + ret = hidpp_send_fap_command_sync(hidpp, feat_index,
>> + HIDPP_MULTIPLATFORM_SET_CURRENT_PLATFORM,
>> + params, sizeof(params), &response);
>> +
>> + if (ret)
>> + hid_warn(hidpp->hid_dev,
>> + "Multiplatform: SET_CURRENT_PLATFORM failed for index %d (err=%d)",
>> + plat_index, ret);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * hidpp_multiplatform_init() - Apply the HID++ Multi-Platform (0x4531) feature
>> + * @hidpp: Pointer to the hidpp_device instance
>> + *
>> + * Initializes the Multi-Platform feature by selecting the device platform
>> + * corresponding to the module parameter @hidpp_platform, if provided.
>> + *
>> + * The function performs the following steps:
>> + * 1. Convert the @hidpp_platform string into a platform mask.
>> + * 2. Check whether the device supports the Multi-Platform feature (0x4531).
>> + * 3. Look up the device's platform index whose mask matches the host
>> + * platform mask.
>> + * 4. Apply that platform index to the device via 'SET_CURRENT_PLATFORM'.
>> + *
>> + * If the module parameter is unset or invalid, or the device does not support
>> + * the feature, or no matching platform descriptor is found, the function exits
>> + * silently without modifying the device state.
>> + *
>> + * On success, the device's platform configuration is updated.
>> + */
>> +static void hidpp_multiplatform_init(struct hidpp_device *hidpp)
>> +{
>> + int ret;
>> + u8 feat_index;
>> + u8 plat_index;
>> + u16 host_plat_mask;
>> + struct hid_device *hdev = hidpp->hid_dev;
>> +
>> + if (!hidpp_platform)
>> + return;
>> +
>> + host_plat_mask = hidpp_multiplatform_mask_from_str(hidpp_platform);
>> + if (!host_plat_mask) {
>> + hid_warn(hdev,
>> + "Multiplatform: Invalid or unsupported platform name '%s'",
>> + hidpp_platform);
>> + return;
>> + }
>> +
>> + ret = hidpp_root_get_feature(hidpp, HIDPP_MULTIPLATFORM_FEAT_ID, &feat_index);
>> + if (ret) {
>> + hid_warn(hdev,
>> + "Multiplatform: Failed to get the HID++ multiplatform feature 0x4531");
>> + return;
>> + }
>> +
>> + ret = hidpp_multiplatform_get_platform_index(hidpp, feat_index, host_plat_mask,
>> + &plat_index);
>> + if (ret)
>> + return;
>> +
>> + ret = hidpp_multiplatform_update_device_platform(hidpp, feat_index, plat_index);
>> + if (ret)
>> + return;
>> +
>> + hid_info(hdev,
>> + "Multiplatform: Device platform successfully set to '%s'", hidpp_platform);
>> +}
>> +
>> static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> {
>> struct hidpp_device *hidpp;
>> @@ -4467,6 +4741,8 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
>> connect_mask &= ~HID_CONNECT_HIDINPUT;
>>
>> + hidpp_multiplatform_init(hidpp);
>> +
>> /* Now export the actual inputs and hidraw nodes to the world */
>> hid_device_io_stop(hdev);
>> ret = hid_connect(hdev, connect_mask);
>> @@ -4664,6 +4940,10 @@ static const struct hid_device_id hidpp_devices[] = {
>> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb034) },
>> { /* MX Anywhere 3SB mouse over Bluetooth */
>> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb038) },
>> + { /* Casa Keys keyboard over Bluetooth */
>> + HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_CASA_KEYS_KEYBOARD) },
>> + { /* MX Keys S keyboard over Bluetooth */
>> + HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MX_KEYS_S_KEYBOARD) },
>> {}
>> };
>>
>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>> index c89a015686c0..99ca04b61bda 100644
>> --- a/drivers/hid/hid-quirks.c
>> +++ b/drivers/hid/hid-quirks.c
>> @@ -520,6 +520,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
>> #endif
>> #if IS_ENABLED(CONFIG_HID_LOGITECH_HIDPP)
>> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL) },
>> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_CASA_KEYS_KEYBOARD) },
>> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MX_KEYS_S_KEYBOARD) },
>> #endif
>> #if IS_ENABLED(CONFIG_HID_MAGICMOUSE)
>> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGICMOUSE) },
>> --
>> 2.34.1
>>
^ permalink raw reply
* [hid:for-7.1/lenovo 15/16] drivers/hid/hid-lenovo-go-s.c:1175:21: sparse: sparse: symbol 'imu_manufacturer' was not declared. Should it be static?
From: kernel test robot @ 2026-03-08 8:56 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: 041eadd5f2d207dd1d286747d137a7d896dd7d5c [15/16] HID: hid-lenovo-go-s: Add IMU and Touchpad RO Attributes
config: arc-randconfig-r113-20260307 (https://download.01.org/0day-ci/archive/20260308/202603081611.J61k8tIx-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 13.4.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260308/202603081611.J61k8tIx-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/202603081611.J61k8tIx-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
drivers/hid/hid-lenovo-go-s.c:582:72: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:754:67: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:856:63: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:1000:71: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:1055:74: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:1149:21: sparse: sparse: symbol 'gamepad_poll_rate' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go-s.c:1175:21: sparse: sparse: symbol 'imu_manufacturer' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1178:21: sparse: sparse: symbol 'imu_sensor_enabled' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1215:21: sparse: sparse: symbol 'mouse_wheel_step' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1235:21: sparse: sparse: symbol 'touchpad_linux_mode' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go-s.c:1239:21: sparse: sparse: symbol 'touchpad_manufacturer' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go-s.c:1242:21: sparse: sparse: symbol 'touchpad_version' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1245:21: sparse: sparse: symbol 'touchpad_windows_mode' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1307:18: sparse: sparse: symbol 'gos_rgb_subled_info' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1328:24: sparse: sparse: symbol 'gos_cdev_rgb' was not declared. Should it be static?
drivers/hid/hid-lenovo-go-s.c:1344:72: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:1351:73: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:1357:72: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:1364:72: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:1371:73: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go-s.c:583:21: sparse: sparse: unsigned value that used to be signed checked against zero?
drivers/hid/hid-lenovo-go-s.c:582:33: sparse: signed value source
vim +/imu_manufacturer +1175 drivers/hid/hid-lenovo-go-s.c
1174
> 1175 struct gos_cfg_attr imu_manufacturer = { TEST_IMU_MFR };
1176 LEGOS_DEVICE_ATTR_RO(imu_manufacturer, "manufacturer", test);
1177
1178 struct gos_cfg_attr imu_sensor_enabled = { FEATURE_IMU_ENABLE };
1179 LEGOS_DEVICE_ATTR_RW(imu_sensor_enabled, "sensor_enabled", index, gamepad);
1180 static DEVICE_ATTR_RO_NAMED(imu_sensor_enabled_index, "sensor_enabled_index");
1181
1182 static struct attribute *legos_imu_attrs[] = {
1183 &dev_attr_imu_bypass_enabled.attr,
1184 &dev_attr_imu_bypass_enabled_index.attr,
1185 &dev_attr_imu_manufacturer.attr,
1186 &dev_attr_imu_sensor_enabled.attr,
1187 &dev_attr_imu_sensor_enabled_index.attr,
1188 NULL,
1189 };
1190
1191 static const struct attribute_group imu_attr_group = {
1192 .name = "imu",
1193 .attrs = legos_imu_attrs,
1194 };
1195
1196 /* MCU */
1197 static DEVICE_ATTR_RO(mcu_id);
1198
1199 struct gos_cfg_attr os_mode = { FEATURE_OS_MODE };
1200 LEGOS_DEVICE_ATTR_RW(os_mode, "os_mode", index, gamepad);
1201 static DEVICE_ATTR_RO(os_mode_index);
1202
1203 static struct attribute *legos_mcu_attrs[] = {
1204 &dev_attr_mcu_id.attr,
1205 &dev_attr_os_mode.attr,
1206 &dev_attr_os_mode_index.attr,
1207 NULL,
1208 };
1209
1210 static const struct attribute_group mcu_attr_group = {
1211 .attrs = legos_mcu_attrs,
1212 };
1213
1214 /* Mouse */
1215 struct gos_cfg_attr mouse_wheel_step = { FEATURE_MOUSE_WHEEL_STEP };
1216 LEGOS_DEVICE_ATTR_RW(mouse_wheel_step, "step", range, gamepad);
1217 static DEVICE_ATTR_RO_NAMED(mouse_wheel_step_range, "step_range");
1218
1219 static struct attribute *legos_mouse_attrs[] = {
1220 &dev_attr_mouse_wheel_step.attr,
1221 &dev_attr_mouse_wheel_step_range.attr,
1222 NULL,
1223 };
1224
1225 static const struct attribute_group mouse_attr_group = {
1226 .name = "mouse",
1227 .attrs = legos_mouse_attrs,
1228 };
1229
1230 /* Touchpad */
1231 struct gos_cfg_attr touchpad_enabled = { FEATURE_TOUCHPAD_ENABLE };
1232 LEGOS_DEVICE_ATTR_RW(touchpad_enabled, "enabled", index, gamepad);
1233 static DEVICE_ATTR_RO_NAMED(touchpad_enabled_index, "enabled_index");
1234
1235 struct gos_cfg_attr touchpad_linux_mode = { CFG_LINUX_MODE };
1236 LEGOS_DEVICE_ATTR_RW(touchpad_linux_mode, "linux_mode", index, touchpad);
1237 static DEVICE_ATTR_RO_NAMED(touchpad_linux_mode_index, "linux_mode_index");
1238
> 1239 struct gos_cfg_attr touchpad_manufacturer = { TEST_TP_MFR };
1240 LEGOS_DEVICE_ATTR_RO(touchpad_manufacturer, "manufacturer", test);
1241
> 1242 struct gos_cfg_attr touchpad_version = { TEST_TP_VER };
1243 LEGOS_DEVICE_ATTR_RO(touchpad_version, "version", test);
1244
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: Input: Add Wacom W9000-series penabled touchscreens
From: Krzysztof Kozlowski @ 2026-03-08 9:15 UTC (permalink / raw)
To: Hendrik Noack
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Ferass El Hafidi, linux-input, devicetree, linux-kernel
In-Reply-To: <20260307181557.66927-2-hendrik-noack@gmx.de>
On Sat, Mar 07, 2026 at 07:15:32PM +0100, Hendrik Noack wrote:
> Add bindings for Wacom W9002 and two Wacom W9007 variants which can be
> found in tablets.
>
> Co-developed-by: Ferass El Hafidi <funderscore@postmarketos.org>
> Signed-off-by: Ferass El Hafidi <funderscore@postmarketos.org>
> Signed-off-by: Hendrik Noack <hendrik-noack@gmx.de>
> ---
You received review and instruction what to do. Did you read it?
Your way of organizing your work makes it difficult for us. Look, try
yourself:
b4 diff '20260307181557.66927-2-hendrik-noack@gmx.de'
Checking for older revisions
Grabbing search results from lore.kernel.org
Added from v3: 2 patches
---
Analyzing 16 messages in the thread
Preparing fake-am for v3: dt-bindings: Input: Add Wacom W9000-series penabled touchscreens
ERROR: v3 series incomplete; unable to create a fake-am range
---
Could not create fake-am range for lower series v3
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2] usbhid: tolerate intermittent errors
From: Liam Mitchell @ 2026-03-08 13:48 UTC (permalink / raw)
To: Alan Stern
Cc: Jiri Kosina, Benjamin Tissoires, Oliver Neukum, linux-usb,
linux-input, linux-kernel
In-Reply-To: <6cbc6c70-8252-43d0-8701-e1613ddc769f@rowland.harvard.edu>
On Sat, 7 Mar 2026 at 23:53, Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Sat, Mar 07, 2026 at 07:57:09PM +0100, Liam Mitchell wrote:
> > Modifies usbhid error handling to tolerate intermittent protocol
> > errors, avoiding URB resubmission delay and device reset.
> >
> > ---
> > Protocol errors like EPROTO can occur randomly, sometimes frequently and are often not fixed by a device reset.
> >
> > The current error handling will only resubmit the URB after at least 13ms delay and may reset the USB device if another error occurs 1-1.5s later, regardless of error type or count.
> >
> > These delays and device resets increase the chance that input events will be missed and that users see symptoms like missed or sticky keyboard keys.
> >
> > This patch allows one protocol error per 500ms to be tolerated and have the URB re-submitted immediately.
> >
> > 500ms was chosen to be well above the error rate of a malfunctioning
> > device but low enough to be useful for users with devices noisier than
> > mine.
> >
> > Signed-off-by: Liam Mitchell <mitchell.liam@gmail.com>
> > Link: https://lore.kernel.org/linux-input/CAOQ1CL6Q+4GNy=kgisLzs0UBXFT3b02PG8t-0rPuW-Wf6NhQ6g@mail.gmail.com/
> > ---
>
> Liam, take a look at
>
> https://bugzilla.kernel.org/show_bug.cgi?id=221184
>
> On Roman's system, these protocol errors occur fairly regularly,
> apparently caused by high system load.
Thanks Alan, I commented there.
> Do you think a better approach might be to reduce the 13-ms delay to
> just 1 or 2 ms, and perform a reset only there has been no successful
> communication for about one second? This might perhaps be _too_ lenient
> sometimes, but I don't think such situations will arise in practice.
I would prefer to have at least the first error resubmit immediately.
I want to reduce device downtime and missed events. From that
perspective, I want to assume the error is intermittent unless we see
evidence otherwise.
The current reset logic "reset only if there has been no successful
communication for one second" is problematic because there is no sign
of successful communication if the user isn't pressing keys or moving
the mouse. Two EPROTO errors 1.4 seconds apart will trigger device
reset and 100-200ms of downtime when ideally URBs would be immediately
resubmitted with only a few ms of downtime.
Can we infer from not receiving errors that we have successful
communication? That might change the equation. If we don't receive
errors for say 10x the polling interval, can we assume it is working?
Ideally the reset is only triggered when we are very sure the device
is not working and needs it.
> The reason for having at least a small delay is to avoid getting into a
> tight resubmit/error loop in cases where the device has been unplugged.
>
> Alan Stern
This patch would only allow one immediate resubmission per window
(500ms). How costly is a URB submission? I was assuming they are
relatively cheap and even one per 100ms wouldn't cause problems.
Liam
^ permalink raw reply
* Re: [PATCH v2] usbhid: tolerate intermittent errors
From: Alan Stern @ 2026-03-08 15:19 UTC (permalink / raw)
To: Liam Mitchell
Cc: Jiri Kosina, Benjamin Tissoires, Oliver Neukum, linux-usb,
linux-input, linux-kernel
In-Reply-To: <CAOQ1CL6GcPUDw+wriKtqq05ywkuhjyi-ujp7WwFpSWgY1fV1zg@mail.gmail.com>
On Sun, Mar 08, 2026 at 02:48:42PM +0100, Liam Mitchell wrote:
> On Sat, 7 Mar 2026 at 23:53, Alan Stern <stern@rowland.harvard.edu> wrote:
> > Do you think a better approach might be to reduce the 13-ms delay to
> > just 1 or 2 ms, and perform a reset only there has been no successful
> > communication for about one second? This might perhaps be _too_ lenient
> > sometimes, but I don't think such situations will arise in practice.
>
> I would prefer to have at least the first error resubmit immediately.
> I want to reduce device downtime and missed events. From that
> perspective, I want to assume the error is intermittent unless we see
> evidence otherwise.
Okay; a single immediate resubmission won't cause any problems.
> The current reset logic "reset only if there has been no successful
> communication for one second" is problematic because there is no sign
> of successful communication if the user isn't pressing keys or moving
> the mouse. Two EPROTO errors 1.4 seconds apart will trigger device
> reset and 100-200ms of downtime when ideally URBs would be immediately
> resubmitted with only a few ms of downtime.
>
> Can we infer from not receiving errors that we have successful
> communication? That might change the equation. If we don't receive
> errors for say 10x the polling interval, can we assume it is working?
Pretty much, yes. If the communication is not working at all (for
example, if the device was unplugged) then an interrupt URB will fail
within three polling intervals. 10 intervals seems like a reasonable
limit.
> Ideally the reset is only triggered when we are very sure the device
> is not working and needs it.
Agreed. I don't know how frequently the bad states that HID devices get
into can be fixed by a reset, but I suspect it's not very frequent at
all.
> > The reason for having at least a small delay is to avoid getting into a
> > tight resubmit/error loop in cases where the device has been unplugged.
> >
> > Alan Stern
>
> This patch would only allow one immediate resubmission per window
> (500ms). How costly is a URB submission? I was assuming they are
> relatively cheap and even one per 100ms wouldn't cause problems.
This problem mainly shows up in syzbot testing. Submission isn't all
that expensive, but in the virtual environment used by syzbot, failure
occurs during or shortly after submission. If resubmission is then
immediate after failure, the whole thing becomes an unending tight loop
executing mostly in atomic context, which ties up a CPU long enough to
trigger a warning about a possible kernel hang.
Alan Stern
^ permalink raw reply
* [hid:for-7.1/lenovo 2/16] drivers/hid/hid-lenovo-go.c:484:20: sparse: sparse: symbol 'version_product_mcu' was not declared. Should it be static?
From: kernel test robot @ 2026-03-08 16:01 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: 3bb54f568ecc35be7675eef5303a47e14aba54bc [2/16] HID: hid-lenovo-go: Add Lenovo Legion Go Series HID Driver
config: loongarch-randconfig-r131-20260308 (https://download.01.org/0day-ci/archive/20260308/202603082311.tPfUsIMR-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260308/202603082311.tPfUsIMR-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/202603082311.tPfUsIMR-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/hid/hid-lenovo-go.c:484:20: sparse: sparse: symbol 'version_product_mcu' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:487:20: sparse: sparse: symbol 'version_protocol_mcu' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:490:20: sparse: sparse: symbol 'version_firmware_mcu' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:493:20: sparse: sparse: symbol 'version_hardware_mcu' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:496:20: sparse: sparse: symbol 'version_gen_mcu' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:513:20: sparse: sparse: symbol 'version_product_tx_dongle' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:516:20: sparse: sparse: symbol 'version_protocol_tx_dongle' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:519:20: sparse: sparse: symbol 'version_firmware_tx_dongle' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:522:20: sparse: sparse: symbol 'version_hardware_tx_dongle' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:525:20: sparse: sparse: symbol 'version_gen_tx_dongle' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:543:20: sparse: sparse: symbol 'version_product_left' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:546:20: sparse: sparse: symbol 'version_protocol_left' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:549:20: sparse: sparse: symbol 'version_firmware_left' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:552:20: sparse: sparse: symbol 'version_hardware_left' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:555:20: sparse: sparse: symbol 'version_gen_left' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:573:20: sparse: sparse: symbol 'version_product_right' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:576:20: sparse: sparse: symbol 'version_protocol_right' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:579:20: sparse: sparse: symbol 'version_firmware_right' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:582:20: sparse: sparse: symbol 'version_hardware_right' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:585:20: sparse: sparse: symbol 'version_gen_right' was not declared. Should it be static?
>> drivers/hid/hid-lenovo-go.c:624:58: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:632:59: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:640:59: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:648:59: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:656:62: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:665:60: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:673:61: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:681:61: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:689:61: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:697:64: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:706:66: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:714:67: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:722:67: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:730:67: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:738:70: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:747:67: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:755:68: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:763:68: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:771:68: sparse: sparse: Using plain integer as NULL pointer
drivers/hid/hid-lenovo-go.c:779:71: sparse: sparse: Using plain integer as NULL pointer
vim +/version_product_mcu +484 drivers/hid/hid-lenovo-go.c
444
445 #define LEGO_DEVICE_ATTR_RW(_name, _attrname, _dtype, _rtype, _group) \
446 static ssize_t _name##_store(struct device *dev, \
447 struct device_attribute *attr, \
448 const char *buf, size_t count) \
449 { \
450 return _group##_store(dev, attr, buf, count, _name.index, \
451 _dtype); \
452 } \
453 static ssize_t _name##_show(struct device *dev, \
454 struct device_attribute *attr, char *buf) \
455 { \
456 return _group##_show(dev, attr, buf, _name.index, _dtype); \
457 } \
458 static ssize_t _name##_##_rtype##_show( \
459 struct device *dev, struct device_attribute *attr, char *buf) \
460 { \
461 return _group##_options(dev, attr, buf, _name.index); \
462 } \
463 static DEVICE_ATTR_RW_NAMED(_name, _attrname)
464
465 #define LEGO_DEVICE_ATTR_WO(_name, _attrname, _dtype, _group) \
466 static ssize_t _name##_store(struct device *dev, \
467 struct device_attribute *attr, \
468 const char *buf, size_t count) \
469 { \
470 return _group##_store(dev, attr, buf, count, _name.index, \
471 _dtype); \
472 } \
473 static DEVICE_ATTR_WO_NAMED(_name, _attrname)
474
475 #define LEGO_DEVICE_ATTR_RO(_name, _attrname, _dtype, _group) \
476 static ssize_t _name##_show(struct device *dev, \
477 struct device_attribute *attr, char *buf) \
478 { \
479 return _group##_show(dev, attr, buf, _name.index, _dtype); \
480 } \
481 static DEVICE_ATTR_RO_NAMED(_name, _attrname)
482
483 /* Gamepad - MCU */
> 484 struct go_cfg_attr version_product_mcu = { PRODUCT_VERSION };
485 LEGO_DEVICE_ATTR_RO(version_product_mcu, "product_version", USB_MCU, version);
486
> 487 struct go_cfg_attr version_protocol_mcu = { PROTOCOL_VERSION };
488 LEGO_DEVICE_ATTR_RO(version_protocol_mcu, "protocol_version", USB_MCU, version);
489
> 490 struct go_cfg_attr version_firmware_mcu = { FIRMWARE_VERSION };
491 LEGO_DEVICE_ATTR_RO(version_firmware_mcu, "firmware_version", USB_MCU, version);
492
> 493 struct go_cfg_attr version_hardware_mcu = { HARDWARE_VERSION };
494 LEGO_DEVICE_ATTR_RO(version_hardware_mcu, "hardware_version", USB_MCU, version);
495
> 496 struct go_cfg_attr version_gen_mcu = { HARDWARE_GENERATION };
497 LEGO_DEVICE_ATTR_RO(version_gen_mcu, "hardware_generation", USB_MCU, version);
498
499 static struct attribute *mcu_attrs[] = {
500 &dev_attr_version_firmware_mcu.attr,
501 &dev_attr_version_gen_mcu.attr,
502 &dev_attr_version_hardware_mcu.attr,
503 &dev_attr_version_product_mcu.attr,
504 &dev_attr_version_protocol_mcu.attr,
505 NULL,
506 };
507
508 static const struct attribute_group mcu_attr_group = {
509 .attrs = mcu_attrs,
510 };
511
512 /* Gamepad - TX Dongle */
> 513 struct go_cfg_attr version_product_tx_dongle = { PRODUCT_VERSION };
514 LEGO_DEVICE_ATTR_RO(version_product_tx_dongle, "product_version", TX_DONGLE, version);
515
> 516 struct go_cfg_attr version_protocol_tx_dongle = { PROTOCOL_VERSION };
517 LEGO_DEVICE_ATTR_RO(version_protocol_tx_dongle, "protocol_version", TX_DONGLE, version);
518
> 519 struct go_cfg_attr version_firmware_tx_dongle = { FIRMWARE_VERSION };
520 LEGO_DEVICE_ATTR_RO(version_firmware_tx_dongle, "firmware_version", TX_DONGLE, version);
521
> 522 struct go_cfg_attr version_hardware_tx_dongle = { HARDWARE_VERSION };
523 LEGO_DEVICE_ATTR_RO(version_hardware_tx_dongle, "hardware_version", TX_DONGLE, version);
524
> 525 struct go_cfg_attr version_gen_tx_dongle = { HARDWARE_GENERATION };
526 LEGO_DEVICE_ATTR_RO(version_gen_tx_dongle, "hardware_generation", TX_DONGLE, version);
527
528 static struct attribute *tx_dongle_attrs[] = {
529 &dev_attr_version_hardware_tx_dongle.attr,
530 &dev_attr_version_firmware_tx_dongle.attr,
531 &dev_attr_version_gen_tx_dongle.attr,
532 &dev_attr_version_product_tx_dongle.attr,
533 &dev_attr_version_protocol_tx_dongle.attr,
534 NULL,
535 };
536
537 static const struct attribute_group tx_dongle_attr_group = {
538 .name = "tx_dongle",
539 .attrs = tx_dongle_attrs,
540 };
541
542 /* Gamepad - Left */
> 543 struct go_cfg_attr version_product_left = { PRODUCT_VERSION };
544 LEGO_DEVICE_ATTR_RO(version_product_left, "product_version", LEFT_CONTROLLER, version);
545
> 546 struct go_cfg_attr version_protocol_left = { PROTOCOL_VERSION };
547 LEGO_DEVICE_ATTR_RO(version_protocol_left, "protocol_version", LEFT_CONTROLLER, version);
548
> 549 struct go_cfg_attr version_firmware_left = { FIRMWARE_VERSION };
550 LEGO_DEVICE_ATTR_RO(version_firmware_left, "firmware_version", LEFT_CONTROLLER, version);
551
> 552 struct go_cfg_attr version_hardware_left = { HARDWARE_VERSION };
553 LEGO_DEVICE_ATTR_RO(version_hardware_left, "hardware_version", LEFT_CONTROLLER, version);
554
> 555 struct go_cfg_attr version_gen_left = { HARDWARE_GENERATION };
556 LEGO_DEVICE_ATTR_RO(version_gen_left, "hardware_generation", LEFT_CONTROLLER, version);
557
558 static struct attribute *left_gamepad_attrs[] = {
559 &dev_attr_version_hardware_left.attr,
560 &dev_attr_version_firmware_left.attr,
561 &dev_attr_version_gen_left.attr,
562 &dev_attr_version_product_left.attr,
563 &dev_attr_version_protocol_left.attr,
564 NULL,
565 };
566
567 static const struct attribute_group left_gamepad_attr_group = {
568 .name = "left_handle",
569 .attrs = left_gamepad_attrs,
570 };
571
572 /* Gamepad - Right */
> 573 struct go_cfg_attr version_product_right = { PRODUCT_VERSION };
574 LEGO_DEVICE_ATTR_RO(version_product_right, "product_version", RIGHT_CONTROLLER, version);
575
> 576 struct go_cfg_attr version_protocol_right = { PROTOCOL_VERSION };
577 LEGO_DEVICE_ATTR_RO(version_protocol_right, "protocol_version", RIGHT_CONTROLLER, version);
578
> 579 struct go_cfg_attr version_firmware_right = { FIRMWARE_VERSION };
580 LEGO_DEVICE_ATTR_RO(version_firmware_right, "firmware_version", RIGHT_CONTROLLER, version);
581
> 582 struct go_cfg_attr version_hardware_right = { HARDWARE_VERSION };
583 LEGO_DEVICE_ATTR_RO(version_hardware_right, "hardware_version", RIGHT_CONTROLLER, version);
584
> 585 struct go_cfg_attr version_gen_right = { HARDWARE_GENERATION };
586 LEGO_DEVICE_ATTR_RO(version_gen_right, "hardware_generation", RIGHT_CONTROLLER, version);
587
588 static struct attribute *right_gamepad_attrs[] = {
589 &dev_attr_version_hardware_right.attr,
590 &dev_attr_version_firmware_right.attr,
591 &dev_attr_version_gen_right.attr,
592 &dev_attr_version_product_right.attr,
593 &dev_attr_version_protocol_right.attr,
594 NULL,
595 };
596
597 static const struct attribute_group right_gamepad_attr_group = {
598 .name = "right_handle",
599 .attrs = right_gamepad_attrs,
600 };
601
602 /* Touchpad */
603 static struct attribute *touchpad_attrs[] = {
604 NULL,
605 };
606
607 static const struct attribute_group touchpad_attr_group = {
608 .name = "touchpad",
609 .attrs = touchpad_attrs,
610 };
611
612 static const struct attribute_group *top_level_attr_groups[] = {
613 &mcu_attr_group, &tx_dongle_attr_group,
614 &left_gamepad_attr_group, &right_gamepad_attr_group,
615 &touchpad_attr_group, NULL,
616 };
617
618 static void cfg_setup(struct work_struct *work)
619 {
620 int ret;
621
622 /* MCU Version Attrs */
623 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
> 624 PRODUCT_VERSION, USB_MCU, 0, 0);
625 if (ret < 0) {
626 dev_err(&drvdata.hdev->dev,
627 "Failed to retrieve USB_MCU Product Version: %i\n", ret);
628 return;
629 }
630
631 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
632 PROTOCOL_VERSION, USB_MCU, 0, 0);
633 if (ret < 0) {
634 dev_err(&drvdata.hdev->dev,
635 "Failed to retrieve USB_MCU Protocol Version: %i\n", ret);
636 return;
637 }
638
639 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
640 FIRMWARE_VERSION, USB_MCU, 0, 0);
641 if (ret < 0) {
642 dev_err(&drvdata.hdev->dev,
643 "Failed to retrieve USB_MCU Firmware Version: %i\n", ret);
644 return;
645 }
646
647 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
648 HARDWARE_VERSION, USB_MCU, 0, 0);
649 if (ret < 0) {
650 dev_err(&drvdata.hdev->dev,
651 "Failed to retrieve USB_MCU Hardware Version: %i\n", ret);
652 return;
653 }
654
655 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
656 HARDWARE_GENERATION, USB_MCU, 0, 0);
657 if (ret < 0) {
658 dev_err(&drvdata.hdev->dev,
659 "Failed to retrieve USB_MCU Hardware Generation: %i\n", ret);
660 return;
661 }
662
663 /* TX Dongle Version Attrs */
664 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
665 PRODUCT_VERSION, TX_DONGLE, 0, 0);
666 if (ret < 0) {
667 dev_err(&drvdata.hdev->dev,
668 "Failed to retrieve TX_DONGLE Product Version: %i\n", ret);
669 return;
670 }
671
672 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
673 PROTOCOL_VERSION, TX_DONGLE, 0, 0);
674 if (ret < 0) {
675 dev_err(&drvdata.hdev->dev,
676 "Failed to retrieve TX_DONGLE Protocol Version: %i\n", ret);
677 return;
678 }
679
680 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
681 FIRMWARE_VERSION, TX_DONGLE, 0, 0);
682 if (ret < 0) {
683 dev_err(&drvdata.hdev->dev,
684 "Failed to retrieve TX_DONGLE Firmware Version: %i\n", ret);
685 return;
686 }
687
688 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
689 HARDWARE_VERSION, TX_DONGLE, 0, 0);
690 if (ret < 0) {
691 dev_err(&drvdata.hdev->dev,
692 "Failed to retrieve TX_DONGLE Hardware Version: %i\n", ret);
693 return;
694 }
695
696 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
697 HARDWARE_GENERATION, TX_DONGLE, 0, 0);
698 if (ret < 0) {
699 dev_err(&drvdata.hdev->dev,
700 "Failed to retrieve TX_DONGLE Hardware Generation: %i\n", ret);
701 return;
702 }
703
704 /* Left Handle Version Attrs */
705 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
706 PRODUCT_VERSION, LEFT_CONTROLLER, 0, 0);
707 if (ret < 0) {
708 dev_err(&drvdata.hdev->dev,
709 "Failed to retrieve LEFT_CONTROLLER Product Version: %i\n", ret);
710 return;
711 }
712
713 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
714 PROTOCOL_VERSION, LEFT_CONTROLLER, 0, 0);
715 if (ret < 0) {
716 dev_err(&drvdata.hdev->dev,
717 "Failed to retrieve LEFT_CONTROLLER Protocol Version: %i\n", ret);
718 return;
719 }
720
721 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
722 FIRMWARE_VERSION, LEFT_CONTROLLER, 0, 0);
723 if (ret < 0) {
724 dev_err(&drvdata.hdev->dev,
725 "Failed to retrieve LEFT_CONTROLLER Firmware Version: %i\n", ret);
726 return;
727 }
728
729 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
730 HARDWARE_VERSION, LEFT_CONTROLLER, 0, 0);
731 if (ret < 0) {
732 dev_err(&drvdata.hdev->dev,
733 "Failed to retrieve LEFT_CONTROLLER Hardware Version: %i\n", ret);
734 return;
735 }
736
737 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
738 HARDWARE_GENERATION, LEFT_CONTROLLER, 0, 0);
739 if (ret < 0) {
740 dev_err(&drvdata.hdev->dev,
741 "Failed to retrieve LEFT_CONTROLLER Hardware Generation: %i\n", ret);
742 return;
743 }
744
745 /* Right Handle Version Attrs */
746 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
747 PRODUCT_VERSION, RIGHT_CONTROLLER, 0, 0);
748 if (ret < 0) {
749 dev_err(&drvdata.hdev->dev,
750 "Failed to retrieve RIGHT_CONTROLLER Product Version: %i\n", ret);
751 return;
752 }
753
754 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
755 PROTOCOL_VERSION, RIGHT_CONTROLLER, 0, 0);
756 if (ret < 0) {
757 dev_err(&drvdata.hdev->dev,
758 "Failed to retrieve RIGHT_CONTROLLER Protocol Version: %i\n", ret);
759 return;
760 }
761
762 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
763 FIRMWARE_VERSION, RIGHT_CONTROLLER, 0, 0);
764 if (ret < 0) {
765 dev_err(&drvdata.hdev->dev,
766 "Failed to retrieve RIGHT_CONTROLLER Firmware Version: %i\n", ret);
767 return;
768 }
769
770 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
771 HARDWARE_VERSION, RIGHT_CONTROLLER, 0, 0);
772 if (ret < 0) {
773 dev_err(&drvdata.hdev->dev,
774 "Failed to retrieve RIGHT_CONTROLLER Hardware Version: %i\n", ret);
775 return;
776 }
777
778 ret = mcu_property_out(drvdata.hdev, MCU_CONFIG_DATA, GET_VERSION_DATA,
779 HARDWARE_GENERATION, RIGHT_CONTROLLER, 0, 0);
780 if (ret < 0) {
781 dev_err(&drvdata.hdev->dev,
782 "Failed to retrieve RIGHT_CONTROLLER Hardware Generation: %i\n", ret);
783 return;
784 }
785 }
786
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v4 0/2] Input: st1232 - add system wakeup support
From: phucduc.bui @ 2026-03-09 0:03 UTC (permalink / raw)
To: krzk+dt, geert+renesas
Cc: krzk, krzysztof.kozlowski, conor+dt, devicetree, dmitry.torokhov,
hechtb, javier.carrasco, jeff, phucduc.bui, linux-input,
linux-kernel, linux-renesas-soc, magnus.damm, robh, wsa+renesas
From: bui duc phuc <phucduc.bui@gmail.com>
This patch series adds support for using the Sitronix ST1232
touchscreen as a wakeup source on the Armadillo800EVA board.
Patch 1 documents the generic wakeup-source property in the
Devicetree binding for the ST1232 touchscreen controller.
Patch 2 enables the wakeup-source property in the ST1232
touchscreen node for the Armadillo800EVA board, allowing touch
events to wake the system from suspend.
Verified functionality
* The "power/wakeup" sysfs attribute is present for the device.
* The system resumes correctly from 'mem' and 'freeze' states when the
touchscreen is touched.
Additional test information
Demo video showing wakeup from suspend:
https://youtu.be/POJhbguiA7A
Kernel config and boot logs:
https://gist.github.com/BuiDucPhuc/ac7d5d732658ca293af4323ad04accca
Changes in v4:
*Drop patch 3 as the I2C core already performs the initialization,
registration, and management of the wakeup interrupt, making the
implementation in the driver redundant.
The original intention of patch 3 was to expose active_count,
event_count, and wakeup_count to user space. However, this is not
necessary since the R8A7740 SoC has some specific characteristics
in its wakeup interrupt handling.
Moreover, modifying this driver could potentially affect other SoCs
sharing the same driver, so the patch is removed.
*Going back to v1 design.
*Update the cover letter
Changes in v3:
* Patch 3: Removed debug dev_info() log messages for a cleaner
production-ready implementation.
* No changes to Patch 1 and Patch 2.
* Link :
https://lore.kernel.org/all/20260306111912.58388-1-phucduc.bui@gmail.com/
Changes in v2
* Drop description for wakeup-source property as suggested by
Krzysztof Kozlowski.
* Updated commit messages for clarity.
* Added driver-side wakeup handling in st1232.c.
* Link :
https://lore.kernel.org/all/20260306104025.43970-1-phucduc.bui@gmail.com/
v1
*Link:
https://lore.kernel.org/all/20260305113512.227269-1-phucduc.bui@gmail.com/
This series depends on the following patch which has been
submitted but not yet merged:
drm: shmobile: Fix blank screen after resume when LCDC is stopped
Link: https://lore.kernel.org/all/20260226054035.30330-1-phucduc.bui@gmail.com/
bui duc phuc (2):
dt-bindings: input: touchscreen: sitronix,st1232: Add wakeup-source
arm: dts: renesas: r8a7740-armadillo800eva: Add wakeup-source to
st1232
.../bindings/input/touchscreen/sitronix,st1232.yaml | 4 ++++
arch/arm/boot/dts/renesas/r8a7740-armadillo800eva.dts | 1 +
2 files changed, 5 insertions(+)
--
2.43.0
^ permalink raw reply
* [PATCH v4 1/2] dt-bindings: input: touchscreen: sitronix,st1232: Add wakeup-source
From: phucduc.bui @ 2026-03-09 0:03 UTC (permalink / raw)
To: krzk+dt, geert+renesas
Cc: krzk, krzysztof.kozlowski, conor+dt, devicetree, dmitry.torokhov,
hechtb, javier.carrasco, jeff, phucduc.bui, linux-input,
linux-kernel, linux-renesas-soc, magnus.damm, robh, wsa+renesas
In-Reply-To: <20260309000319.74880-1-phucduc.bui@gmail.com>
From: bui duc phuc <phucduc.bui@gmail.com>
Document the 'wakeup-source' property for Sitronix ST1232 touchscreen
controllers to allow the device to wake the system from suspend.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
.../bindings/input/touchscreen/sitronix,st1232.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
index 978afaa4fcef..fe1fa217d842 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
@@ -32,6 +32,9 @@ properties:
description: A phandle to the reset GPIO
maxItems: 1
+ wakeup-source:
+ type: boolean
+
required:
- compatible
- reg
@@ -51,6 +54,7 @@ examples:
reg = <0x55>;
interrupts = <2 0>;
gpios = <&gpio1 166 0>;
+ wakeup-source;
touch-overlay {
segment-0 {
--
2.43.0
^ permalink raw reply related
* [PATCH v4 2/2] arm: dts: renesas: r8a7740-armadillo800eva: Add wakeup-source to st1232
From: phucduc.bui @ 2026-03-09 0:03 UTC (permalink / raw)
To: krzk+dt, geert+renesas
Cc: krzk, krzysztof.kozlowski, conor+dt, devicetree, dmitry.torokhov,
hechtb, javier.carrasco, jeff, phucduc.bui, linux-input,
linux-kernel, linux-renesas-soc, magnus.damm, robh, wsa+renesas
In-Reply-To: <20260309000319.74880-1-phucduc.bui@gmail.com>
From: bui duc phuc <phucduc.bui@gmail.com>
Add the wakeup-source property to the ST1232 touchscreen node
in the device tree so that the touchscreen interrupt can wake
the system from suspend when the panel is touched.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
arch/arm/boot/dts/renesas/r8a7740-armadillo800eva.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/renesas/r8a7740-armadillo800eva.dts b/arch/arm/boot/dts/renesas/r8a7740-armadillo800eva.dts
index 04d24b6d8056..d47a6cc3e756 100644
--- a/arch/arm/boot/dts/renesas/r8a7740-armadillo800eva.dts
+++ b/arch/arm/boot/dts/renesas/r8a7740-armadillo800eva.dts
@@ -228,6 +228,7 @@ touchscreen@55 {
pinctrl-0 = <&st1232_pins>;
pinctrl-names = "default";
gpios = <&pfc 166 GPIO_ACTIVE_LOW>;
+ wakeup-source;
};
};
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
From: Dmitry Torokhov @ 2026-03-09 5:44 UTC (permalink / raw)
To: Val Packett
Cc: Jingyuan Liang, Jiri Kosina, Benjamin Tissoires, Jonathan Corbet,
Mark Brown, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
linux-doc, linux-kernel, linux-spi, linux-trace-kernel,
devicetree, hbarnor, Dmitry Antipov, Jarrett Schultz
In-Reply-To: <1cc6de61-8b56-492e-ab78-e3aa448f58ad@packett.cool>
On Sat, Mar 07, 2026 at 04:25:44AM -0300, Val Packett wrote:
>
> On 3/3/26 3:13 AM, Jingyuan Liang wrote:
> > Documentation describes the required and optional properties for
> > implementing Device Tree for a Microsoft G6 Touch Digitizer that
> > supports HID over SPI Protocol 1.0 specification.
> > […]
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - microsoft,g6-touch-digitizer
> > + - const: hid-over-spi
> > + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> > […]
> > +required:
> > + - compatible
> > + - interrupts
> > + - reset-gpios
>
> Why is reset required? Is it so implausible on some device implementing the
> spec there wouldn't be a reset gpio?
No, because it is mandated by the spec:
"HID SPI peripheral must provide a dedicated reset line, driven by the
HOST, which, when toggled (pulled LOW for at least 10ms, normally HIGH),
will have the effect of resetting the device. If a HID SPI peripheral is
enumerated via ACPI, the device ASL configuration must expose an ACPI
FLDR (_RST) method to control this line."
The spec also states that the host must initiate reset during
initialization of the device.
>
> > + - vdd-supply
> Linux makes up a dummy regulator if DT doesn't provide one, so can
> regulators even be required?
There is still a supply line to the chip even if it is not exposed to
the OS control. So as far as chip is concerned the supply is required.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] Input: mpr121: Drop redundant wakeup handling
From: phucduc.bui @ 2026-03-09 7:14 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: phucduc.bui, linux-input, linux-kernel
From: bui duc phuc <phucduc.bui@gmail.com>
The driver currently calls device_init_wakeup() and manually toggles
IRQ wake in suspend and resume paths. This is unnecessary since the
I2C core already handles wakeup configuration when the device is
described in Device Tree with the "wakeup-source" property.
Note: Compile-tested only, not verified on hardware.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
drivers/input/keyboard/mpr121_touchkey.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
index bd1a944ded46..47edc161ec77 100644
--- a/drivers/input/keyboard/mpr121_touchkey.c
+++ b/drivers/input/keyboard/mpr121_touchkey.c
@@ -295,8 +295,6 @@ static int mpr_touchkey_probe(struct i2c_client *client)
return error;
i2c_set_clientdata(client, mpr121);
- device_init_wakeup(dev,
- device_property_read_bool(dev, "wakeup-source"));
return 0;
}
@@ -305,9 +303,6 @@ static int mpr_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
- if (device_may_wakeup(&client->dev))
- enable_irq_wake(client->irq);
-
i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR, 0x00);
return 0;
@@ -318,9 +313,6 @@ static int mpr_resume(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct mpr121_touchkey *mpr121 = i2c_get_clientdata(client);
- if (device_may_wakeup(&client->dev))
- disable_irq_wake(client->irq);
-
i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR,
mpr121->keycount);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v7 1/3] input: trackpoint - Enable doubletap by default on capable devices
From: Ilpo Järvinen @ 2026-03-09 8:01 UTC (permalink / raw)
To: Vishnu Sankar
Cc: Mark Pearson, dmitry.torokhov, hmh, Hans de Goede, corbet,
derekjohn.clark, linux-input, LKML, ibm-acpi-devel, linux-doc,
platform-driver-x86, vsankar
In-Reply-To: <20260209063355.491189-2-vishnuocv@gmail.com>
On Mon, 9 Feb 2026, Vishnu Sankar wrote:
> Enable doubletap functionality by default on TrackPoint devices that
> support it. The feature is detected using firmware ID pattern matching
> (PNP: LEN03xxx) with a deny list of incompatible devices.
>
> This provides immediate doubletap functionality without requiring
> userspace configuration. The hardware is enabled during device
> detection, while event filtering continues to be handled by the
> thinkpad_acpi driver as before.
>
> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> Changes in v7:
> - Removed unwanted comments
> - Removed psmouse_info ()
>
> Changes in v6:
> - No Changes
>
> Changes in v5:
> - Renamed function to trackpoint_is_dt_capable()
> - Simplified string comparison without sscanf()
> - Removed wrapper function as suggested
> - Fixed missing period in comment
>
> Changes in v4:
> - Simplified approach: removed all sysfs attributes and user interface
> - Enable doubletap by default during device detection
> - Removed global variables and complex attribute infrastructure
> - Uses minimal firmware ID detection with deny list
> - Follows KISS principle as suggested by reviewers
>
> Changes in v3:
> - No changes
>
> Changes in v2:
> - Improve commit messages
> - Sysfs attributes moved to trackpoint.c
> - Removed unnecessary comments
> - Removed unnecessary debug messages
> - Using strstarts() instead of strcmp()
> - is_trackpoint_dt_capable() modified
> - Removed _BIT suffix and used BIT() define
> - Reverse the trackpoint_doubletap_status() logic to return error first
> - Removed export functions as a result of the design change
> - Changed trackpoint_dev->psmouse to parent_psmouse
> - The path of trackpoint.h is not changed
> ---
> drivers/input/mouse/trackpoint.c | 45 ++++++++++++++++++++++++++++++++
> drivers/input/mouse/trackpoint.h | 5 ++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index 5f6643b69a2c..e12d76350252 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -393,6 +393,45 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
> return 0;
> }
>
> +/* List of known incapable device PNP IDs */
> +static const char * const dt_incompatible_devices[] = {
> + "LEN0304",
> + "LEN0306",
> + "LEN0317",
> + "LEN031A",
> + "LEN031B",
> + "LEN031C",
> + "LEN031D",
> +};
> +
> +/*
> + * Checks if it's a doubletap capable device.
> + * The PNP ID format is "PNP: LEN030d PNP0f13".
> + */
> +static bool trackpoint_is_dt_capable(const char *pnp_id)
> +{
> + size_t i;
> +
> + if (!pnp_id)
> + return false;
> +
> + /* Must start with "PNP: LEN03" */
> + if (!strstarts(pnp_id, "PNP: LEN03"))
Missing include.
> + return false;
> +
> + /* Ensure enough length before comparing */
> + if (strlen(pnp_id) < 12)
> + return false;
> +
> + /* Check deny-list */
> + for (i = 0; i < ARRAY_SIZE(dt_incompatible_devices); i++) {
Missing include for ARRAY_SIZE().
> + if (!strncmp(pnp_id + 5,
> + dt_incompatible_devices[i], 7))
Fits to one line.
> + return false;
> + }
> + return true;
> +}
> +
> int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> {
> struct ps2dev *ps2dev = &psmouse->ps2dev;
> @@ -470,6 +509,12 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> psmouse->vendor, firmware_id,
> (button_info & 0xf0) >> 4, button_info & 0x0f);
>
> + if (trackpoint_is_dt_capable(ps2dev->serio->firmware_id)) {
> + error = trackpoint_write(ps2dev, TP_DOUBLETAP, TP_DOUBLETAP_ENABLE);
> + if (error)
> + psmouse_warn(psmouse, "Failed to enable doubletap: %d\n", error);
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/input/mouse/trackpoint.h b/drivers/input/mouse/trackpoint.h
> index eb5412904fe0..3e03cdb39449 100644
> --- a/drivers/input/mouse/trackpoint.h
> +++ b/drivers/input/mouse/trackpoint.h
> @@ -69,6 +69,8 @@
> /* (how hard it is to drag */
> /* with Z-axis pressed) */
>
> +#define TP_DOUBLETAP 0x58 /* TrackPoint doubletap register */
> +
> #define TP_MINDRAG 0x59 /* Minimum amount of force needed */
> /* to trigger dragging */
>
> @@ -110,6 +112,9 @@
> external device will be forced to 1 */
> #define TP_MASK_EXT_TAG 0x04
>
> +/* Doubletap register values */
> +#define TP_DOUBLETAP_ENABLE 0xFF /* Enable value */
> +#define TP_DOUBLETAP_DISABLE 0xFE /* Disable value */
>
> /* Power on Self Test Results */
> #define TP_POR_SUCCESS 0x3B
>
--
i.
^ permalink raw reply
* Re: [PATCH v7 3/3] Documentation: thinkpad-acpi - Document doubletap_enable attribute
From: Ilpo Järvinen @ 2026-03-09 8:03 UTC (permalink / raw)
To: Vishnu Sankar
Cc: Mark Pearson, dmitry.torokhov, hmh, Hans de Goede, corbet,
derekjohn.clark, linux-input, LKML, ibm-acpi-devel, linux-doc,
platform-driver-x86, vsankar
In-Reply-To: <20260209063355.491189-4-vishnuocv@gmail.com>
On Mon, 9 Feb 2026, Vishnu Sankar wrote:
> Document the doubletap_enable sysfs attribute for ThinkPad ACPI driver.
>
> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> ---
> + * 1 - doubletap events are processed (default)
> + * 0 - doubletap events are filtered out (ignored)
There's something odd in space vs tab here.
--
i.
^ 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