* Re: [PATCH 2/2] HID: lenovo: Use KEY_PERFORMANCE capability for ThinkPad X12 Tab Gen 2
From: sashiko-bot @ 2026-05-22 6:05 UTC (permalink / raw)
Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260522050632.8079-2-vishnuocv@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Low] The patch unconditionally adds the KEY_PERFORMANCE capability to all input nodes exposed by the device, causing capability pollution on unrelated interfaces.
Pre-existing issues:
- [High] The raw event handler emits keys like KEY_RFKILL, KEY_BOOKMARKS, and KEY_KBDILLUMTOGGLE, but their capabilities are never declared, causing the input subsystem to silently drop these events.
- [High] The raw event handler contains a loop that unconditionally fires key events on the first input node it encounters, ignoring capability routing and causing events to be dropped.
--
commit 1b40b44e417a26a21650fec78e6a2ea55f535004
Author: Vishnu Sankar <vishnuocv@gmail.com>
HID: lenovo: Use KEY_PERFORMANCE capability for ThinkPad X12 Tab Gen 2
This commit adds the KEY_PERFORMANCE capability for the ThinkPad X12 Tab Gen 2
and X13 Tab devices during input configuration. This allows userspace tools to
discover the key emitted via the raw event handler.
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 35b00619f0456..3492ba8d0fb02 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -1525,6 +1525,7 @@ static int lenovo_input_configured(struct hid_device *hdev,
> hi->input->propbit);
> }
> break;
> + case USB_DEVICE_ID_LENOVO_X12_TAB2:
> case USB_DEVICE_ID_LENOVO_X13_TAB:
> input_set_capability(hi->input, EV_KEY, KEY_PERFORMANCE);
[Severity: Low]
Does this unconditionally add the KEY_PERFORMANCE capability to all input
nodes exposed by the device?
Since lenovo_input_configured() is invoked for every hid_input interface,
this regression might advertise the capability on unrelated interfaces.
Could we filter this by checking the application usage or verifying it is
the primary keyboard node?
[Severity: High]
This isn't a bug introduced by this patch, but I noticed the raw event
handler also emits keys like KEY_RFKILL, KEY_PICKUP_PHONE, KEY_BOOKMARKS,
and KEY_KBDILLUMTOGGLE.
Should we also declare the capabilities for these keys here?
Since they are generated from raw reports rather than standard HID
descriptors, they appear to be missing from the capability bits, which would
cause the input subsystem to silently drop these events.
[Severity: High]
This is a pre-existing issue, but looking at how the raw events are handled
in lenovo_raw_event_TP_X12_tab(), is it possible the events are firing on
the wrong input node?
The handler loops over hdev->inputs but returns immediately after reporting
the key event on the first node it encounters. If the first device lacks the
specific key capabilities, the input core might discard the event.
Should the loop instead locate the specific input node that possesses the
corresponding key capability before dispatching?
> break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522050632.8079-1-vishnuocv@gmail.com?part=2
^ permalink raw reply
* Recall: [PATCH] HID: remove duplicate hid_warn_ratelimited definition
From: Liu Kai @ 2026-05-22 6:04 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Liu Kai would like to recall the message, "[PATCH] HID: remove duplicate hid_warn_ratelimited definition".
^ permalink raw reply
* [PATCH 2/2] HID: lenovo: Use KEY_PERFORMANCE capability for ThinkPad X12 Tab Gen 2
From: Vishnu @ 2026-05-22 5:06 UTC (permalink / raw)
To: jikos, bentiss, derekjohn.clark, mpearson-lenovo
Cc: vsankar, linux-input, linux-kernel, Vishnu Sankar
In-Reply-To: <20260522050632.8079-1-vishnuocv@gmail.com>
From: Vishnu Sankar <vishnuocv@gmail.com>
The X12 Tab Gen 2 emits KEY_PERFORMANCE via Fn+F8 through the raw
event handler but never declared the capability via
input_set_capability(). This prevents userspace tools from
discovering the key through evdev capability bits.
Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
---
drivers/hid/hid-lenovo.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 35b00619f045..3492ba8d0fb0 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -1525,6 +1525,7 @@ static int lenovo_input_configured(struct hid_device *hdev,
hi->input->propbit);
}
break;
+ case USB_DEVICE_ID_LENOVO_X12_TAB2:
case USB_DEVICE_ID_LENOVO_X13_TAB:
input_set_capability(hi->input, EV_KEY, KEY_PERFORMANCE);
break;
--
2.54.0
^ permalink raw reply related
* [PATCH 1/2] HID: lenovo: Add support for ThinkPad X13 Folio keyboard
From: Vishnu @ 2026-05-22 5:06 UTC (permalink / raw)
To: jikos, bentiss, derekjohn.clark, mpearson-lenovo
Cc: vsankar, linux-input, linux-kernel, Vishnu Sankar
From: Vishnu Sankar <vishnuocv@gmail.com>
Add USB ID support for the ThinkPad X13 detachable keyboard.
The Keyboard uses the same HID raw event protocol as the ThinkPad
X12 Gen 2. The functionality stays the same with X12 Gen 2 Keyboards.
Also declare KEY_PERFORMANCE capability in lenovo_input_configured()
for X13 detachable, allowing userspace to discover the key via evdev
capability bits.
Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-lenovo.c | 21 +++++++++++++++++----
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 4657d96fb083..064be2333a00 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -874,6 +874,7 @@
#define USB_DEVICE_ID_LENOVO_X1_TAB3 0x60b5
#define USB_DEVICE_ID_LENOVO_X12_TAB 0x60fe
#define USB_DEVICE_ID_LENOVO_X12_TAB2 0x61ae
+#define USB_DEVICE_ID_LENOVO_X13_TAB 0x62af
#define USB_DEVICE_ID_LENOVO_YOGABOOK9I 0x6161
#define USB_DEVICE_ID_LENOVO_OPTICAL_USB_MOUSE_600E 0x600e
#define USB_DEVICE_ID_LENOVO_PIXART_USB_MOUSE_608D 0x608d
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index a6b73e03c16b..35b00619f045 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -504,6 +504,7 @@ static int lenovo_input_mapping(struct hid_device *hdev,
usage, bit, max);
case USB_DEVICE_ID_LENOVO_X12_TAB:
case USB_DEVICE_ID_LENOVO_X12_TAB2:
+ case USB_DEVICE_ID_LENOVO_X13_TAB:
case USB_DEVICE_ID_LENOVO_X1_TAB:
case USB_DEVICE_ID_LENOVO_X1_TAB2:
case USB_DEVICE_ID_LENOVO_X1_TAB3:
@@ -620,6 +621,7 @@ static ssize_t attr_fn_lock_store(struct device *dev,
break;
case USB_DEVICE_ID_LENOVO_X12_TAB:
case USB_DEVICE_ID_LENOVO_X12_TAB2:
+ case USB_DEVICE_ID_LENOVO_X13_TAB:
case USB_DEVICE_ID_LENOVO_TP10UBKBD:
case USB_DEVICE_ID_LENOVO_X1_TAB:
case USB_DEVICE_ID_LENOVO_X1_TAB2:
@@ -791,9 +793,10 @@ static int lenovo_raw_event(struct hid_device *hdev,
* Lenovo TP X12 Tab KBD's Fn+XX is HID raw data defined. Report ID is 0x03
* e.g.: Raw data received for MIC mute is 0x00020003.
*/
- if (unlikely((hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB
- || hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB2)
- && size >= 3 && report->id == 0x03))
+ if (unlikely((hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB ||
+ hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB2 ||
+ hdev->product == USB_DEVICE_ID_LENOVO_X13_TAB) &&
+ size >= 3 && report->id == 0x03))
return lenovo_raw_event_TP_X12_tab(hdev, le32_to_cpu(*(__le32 *)data));
return 0;
@@ -877,6 +880,7 @@ static int lenovo_event(struct hid_device *hdev, struct hid_field *field,
return lenovo_event_cptkbd(hdev, field, usage, value);
case USB_DEVICE_ID_LENOVO_X12_TAB:
case USB_DEVICE_ID_LENOVO_X12_TAB2:
+ case USB_DEVICE_ID_LENOVO_X13_TAB:
case USB_DEVICE_ID_LENOVO_TP10UBKBD:
case USB_DEVICE_ID_LENOVO_X1_TAB:
case USB_DEVICE_ID_LENOVO_X1_TAB2:
@@ -1161,6 +1165,7 @@ static int lenovo_led_brightness_set(struct led_classdev *led_cdev,
break;
case USB_DEVICE_ID_LENOVO_X12_TAB:
case USB_DEVICE_ID_LENOVO_X12_TAB2:
+ case USB_DEVICE_ID_LENOVO_X13_TAB:
case USB_DEVICE_ID_LENOVO_TP10UBKBD:
case USB_DEVICE_ID_LENOVO_X1_TAB:
case USB_DEVICE_ID_LENOVO_X1_TAB2:
@@ -1355,7 +1360,8 @@ static int lenovo_probe_tp10ubkbd(struct hid_device *hdev)
*/
data->fn_lock = !(hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB ||
- hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB2);
+ hdev->product == USB_DEVICE_ID_LENOVO_X12_TAB2 ||
+ hdev->product == USB_DEVICE_ID_LENOVO_X13_TAB);
lenovo_led_set_tp10ubkbd(hdev, TP10UBKBD_FN_LOCK_LED, data->fn_lock);
@@ -1402,6 +1408,7 @@ static int lenovo_probe(struct hid_device *hdev,
break;
case USB_DEVICE_ID_LENOVO_X12_TAB:
case USB_DEVICE_ID_LENOVO_X12_TAB2:
+ case USB_DEVICE_ID_LENOVO_X13_TAB:
case USB_DEVICE_ID_LENOVO_TP10UBKBD:
case USB_DEVICE_ID_LENOVO_X1_TAB:
case USB_DEVICE_ID_LENOVO_X1_TAB2:
@@ -1490,6 +1497,7 @@ static void lenovo_remove(struct hid_device *hdev)
break;
case USB_DEVICE_ID_LENOVO_X12_TAB:
case USB_DEVICE_ID_LENOVO_X12_TAB2:
+ case USB_DEVICE_ID_LENOVO_X13_TAB:
case USB_DEVICE_ID_LENOVO_TP10UBKBD:
case USB_DEVICE_ID_LENOVO_X1_TAB:
case USB_DEVICE_ID_LENOVO_X1_TAB2:
@@ -1517,6 +1525,9 @@ static int lenovo_input_configured(struct hid_device *hdev,
hi->input->propbit);
}
break;
+ case USB_DEVICE_ID_LENOVO_X13_TAB:
+ input_set_capability(hi->input, EV_KEY, KEY_PERFORMANCE);
+ break;
}
return 0;
@@ -1551,6 +1562,8 @@ static const struct hid_device_id lenovo_devices[] = {
USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X12_TAB) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X12_TAB2) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+ USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X13_TAB) },
{ HID_DEVICE(BUS_I2C, HID_GROUP_GENERIC,
USB_VENDOR_ID_ITE, I2C_DEVICE_ID_ITE_LENOVO_YOGA_SLIM_7X_KEYBOARD) },
{ }
--
2.54.0
^ permalink raw reply related
* Re: [PATCH 02/10] [v3] input: gpio-keys: make legacy gpiolib optional
From: Matti Vaittinen @ 2026-05-22 4:55 UTC (permalink / raw)
To: Arnd Bergmann, linux-gpio
Cc: linux-kernel, Arnd Bergmann, Christian Lamparter, Johannes Berg,
Aaro Koskinen, Andreas Kemnade, Kevin Hilman, Roger Quadros,
Tony Lindgren, Thomas Bogendoerfer, John Paul Adrian Glaubitz,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Lee Jones, Pavel Machek, Florian Fainelli,
Jonas Gorski, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-wireless,
linux-omap, linux-arm-kernel, linux-mips, linux-sh, linux-input,
linux-leds, netdev
In-Reply-To: <20260520183815.2510387-3-arnd@kernel.org>
On 20/05/2026 21:38, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Most users of gpio-keys and gpio-keys-polled use modern gpiolib
> interfaces, but there are still number of ancient sh, arm32 and x86
> machines that have never been converted.
>
> Add an #ifdef block for the parts of the driver that are only used on
> those legacy machines.
>
> The two Rohm PMIC drivers use a gpio-keys device without an actual GPIO,
> passing an IRQ number instead. In order to keep this working both with
> and with CONFIG_GPIOLIB_LEGACY, change the gpio-keys driver to ignore
> the gpio number if an IRQ is passed.
>
> Link: https://lore.kernel.org/all/b3c94552-c104-42e3-be15-7e8362e8039e@gmail.com/
> Link: https://lore.kernel.org/all/afJXG4_rtaj3l2Dk@google.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v3: resend
> v2: skip the fake GPIO number passing from mfd
>
> The removal of the arm platforms using this is not yet going to happen
> for 7.2, and Dmitry's changes for the Rohm drivers have not yet
> made it into linux-next as of 2026-05-20, so for the moment I
> would still like to see this patch get merged, even if we are
> closing in on completely removing the legacy gpio support in
> the gpio_keys driver, so we can make CONFIG_GPIOLIB_LEGACY
> default-disabled sooner.
I am (still) all fine with this, even though I like Dmitry's set. I
suppose you already have a plan for merging this, but I still have to
ask - why the MFD changes aren't in own patch? I feel it would have
simplified merging, backporting, reviewing and reverting if needed.
Well, other than that:
Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>
Yours,
-- Matti
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH v8 3/4] HID: hid-msi: Add RGB control interface
From: sashiko-bot @ 2026-05-22 4:00 UTC (permalink / raw)
To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260522015518.1111290-4-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Explicitly unregistering a devm-managed LED class device causes a double unregister and list corruption upon device teardown.
- [High] Calling sleepable functions that allocate memory with GFP_KERNEL or take sleeping locks while holding a spinlock.
- [High] Race condition between sysfs/LED attributes and `claw_remove()` allows `rgb_queue` to be scheduled after cancellation.
- [High] Missing cancellation of the `rgb_queue` delayed work during system suspend can cause USB I/O to a suspended device.
--
commit 34d0916b058e394f024a63889da5c0fbe434c516
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add RGB control interface
Adds RGB control interface for MSI Claw devices. The MSI Claw uses a
fairly unique RGB interface with 9 total zones, supporting up to 8
sequential frames of RGB zone data. The driver mirrors the effects
available in Windows while keeping effect names consistent with Lenovo
drivers.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index b9901869075f6..0b875603dbb32 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> +static ssize_t effect_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
> + struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
> + int ret;
> +
> + scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
> + /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
> + if (!smp_load_acquire(&drvdata->rgb_registered))
> + return -ENODEV;
> + }
> +
> + ret = sysfs_match_string(claw_rgb_effect_text, buf);
> + if (ret < 0)
> + return ret;
> +
> + drvdata->rgb_effect = ret;
> + mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
[Severity: High]
Can mod_delayed_work() here race with claw_remove()?
If claw_remove() runs concurrently, it acquires the registration_lock, sets
rgb_registered to false, drops the lock, and calls
cancel_delayed_work_sync(&drvdata->rgb_queue).
It then calls device_remove_group(), which blocks until this sysfs store
completes. Once device_remove_group() returns, devres frees drvdata.
However, since this sysfs store function already checked rgb_registered and
dropped the lock, it will continue executing and call mod_delayed_work(),
re-arming the workqueue. Will this lead to the delayed work executing on
freed memory?
> +
> + return count;
> +}
[ ... ]
> +static void cfg_setup_fn(struct work_struct *work)
> +{
[ ... ]
> +try_gamepad:
> + scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
> + /* Pairs with smp_store_release from below */
> + if (!smp_load_acquire(&drvdata->gamepad_registered) && gamepad_ready) {
> + ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
[Severity: High]
Does devm_device_add_group() sleep?
It allocates memory using GFP_KERNEL inside devres_alloc() and acquires
the sleeping kernfs_mutex during sysfs group creation. Calling this inside a
spinlock block with interrupts disabled will trigger a scheduling while atomic
panic. Can this be called outside the spinlock?
> + if (ret) {
> + dev_err(&drvdata->hdev->dev,
> + "Failed to create gamepad attrs: %d\n", ret);
> + goto try_rgb;
> + }
> +
> + /* Pairs with smp_load_acquire in attribute show/store functions */
> + smp_store_release(&drvdata->gamepad_registered, true);
> + }
> }
> - scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
> - /* Pairs with smp_load_acquire in attribute show/store functions */
> - smp_store_release(&drvdata->gamepad_registered, true);
>
> - kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
> +try_rgb:
> + /* Add and enable RGB interface once we have the device state */
> + scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
> + /* Pairs with smp_store_release from below */
> + if (!smp_load_acquire(&drvdata->rgb_registered) && rgb_ready) {
> + ret = devm_led_classdev_multicolor_register(&drvdata->hdev->dev,
> + &drvdata->led_mc);
[Severity: High]
Does devm_led_classdev_multicolor_register() also sleep?
Similar to devm_device_add_group(), this registers a device which takes
sleeping locks and allocates memory with GFP_KERNEL. Should this be called
outside of the spinlock block?
> + if (ret) {
> + dev_err(&drvdata->hdev->dev,
> + "Failed to create led device: %d\n", ret);
> + goto update_kobjects;
> + }
> +
> + ret = device_add_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
> + if (ret) {
> + dev_err(&drvdata->hdev->dev,
> + "Failed to create RGB attrs: %d\n", ret);
> + led_classdev_unregister(&drvdata->led_mc.led_cdev);
[Severity: High]
Does this manual unregister cause a double-free on device teardown?
Since devm_led_classdev_multicolor_register() attaches a devres cleanup
action, manually calling led_classdev_unregister() here leaves the devres
action queued. When the driver is unbound, devres will execute
led_classdev_unregister() a second time on the same LED class device pointer.
Will this result in list corruption and use-after-free?
> + goto update_kobjects;
> + }
> +
> + /* Pairs with smp_load_acquire in attribute show/store functions */
> + smp_store_release(&drvdata->rgb_registered, true);
> + }
> + }
[ ... ]
> @@ -971,9 +1593,11 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
> spin_lock_init(&drvdata->cmd_lock);
> spin_lock_init(&drvdata->mode_lock);
> spin_lock_init(&drvdata->profile_lock);
> + spin_lock_init(&drvdata->frame_lock);
> init_completion(&drvdata->send_cmd_complete);
> INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
> INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
> + INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
[Severity: High]
Is the rgb_queue delayed work properly cancelled during system suspend?
In claw_suspend(), the driver cancels cfg_resume and cfg_setup, but it does
not appear to cancel the newly introduced rgb_queue delayed work. Because
rgb_queue is queued on system_wq (which is not freezable), could it execute
after the device has been suspended? This might lead to USB I/O errors and
possible hardware lockups.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522015518.1111290-1-derekjohn.clark@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH v8 2/4] HID: hid-msi: Add M-key mapping attributes
From: sashiko-bot @ 2026-05-22 3:27 UTC (permalink / raw)
To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260522015518.1111290-3-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Command synchronization cross-talk due to unconsumed generic ACKs from `SYNC_TO_ROM`
- [Medium] TOCTOU race in `claw_profile_event` unconditionally overwrites `profile_pending`
- [Low] Invalid anonymous struct declaration in `struct claw_mkey_report`
--
commit b8dd002e01cbcbf34fdd54df7543cbafb1207a47
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add M-key mapping attributes
This commit adds attributes that allow for remapping the M-keys with up to 5
values when in macro mode. It adds support for mappings from the gamepad,
keyboard, or mouse event devices.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 40b16253abbb3..b9901869075f6 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -87,25 +249,47 @@ struct claw_command_report {
> u8 report_id;
> u8 padding[2];
> u8 header_tail;
> u8 cmd;
> u8 data[59];
> } __packed;
>
> +struct claw_profile_report {
> + u8 profile;
> + __be16 read_addr;
> +} __packed;
> +
> +struct claw_mkey_report {
> + struct claw_profile_report;
[Severity: Low]
Does this act as a forward declaration rather than an embedded field? Under
standard C11 and GNU C, a struct with a tag cannot be an anonymous embedded
member without specific compiler extensions. Will this cause a compilation
error when accessing fields like mkeys->read_addr later?
> + u8 padding_0;
> + u8 padding_1;
> + u8 padding_2;
> + u8 codes[5];
> +} __packed;
[ ... ]
> @@ -137,6 +321,39 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
> return 0;
> }
>
> +static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
> +{
> + enum claw_profile_ack_pending profile;
> + struct claw_mkey_report *mkeys;
> + u8 *codes, key;
> + int i;
> +
> + scoped_guard(spinlock, &drvdata->profile_lock)
> + profile = drvdata->profile_pending;
> +
> + switch (profile) {
> + case CLAW_M1_PENDING:
> + case CLAW_M2_PENDING:
[ ... ]
> + default:
> + dev_dbg(&drvdata->hdev->dev,
> + "Got profile event without changes pending from command: %x\n",
> + cmd_rep->cmd);
> + return -EINVAL;
> + }
> + scoped_guard(spinlock, &drvdata->profile_lock)
> + drvdata->profile_pending = CLAW_NO_PENDING;
[Severity: Medium]
Could this unconditionally overwrite a new state if a timeout occurs? If a
command times out, the waiting thread might drop profile_mutex, allowing a
new thread to enter claw_mkey_show() and set profile_pending for a new
operation. If the delayed ACK from the first command arrives at that moment,
will this clear the new thread's state and cause its operation to fail?
> +
> + return 0;
> +}
[ ... ]
> @@ -442,6 +670,177 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
[ ... ]
> +static int claw_mkey_store(struct device *dev, const char *buf, u8 mkey)
> +{
[ ... ]
> +set_buttons:
> + scoped_guard(mutex, &drvdata->rom_mutex) {
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> + (u8 *)&report, sizeof(report), 25);
> + if (ret)
> + return ret;
> + /* MCU will not send ACK until the USB transaction completes. ACK is sent
> + * immediately after and will hit the stale state machine, before the next
> + * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
> + * for ACK that ill never come.
> + */
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
[Severity: High]
Can this lead to command synchronization cross-talk? Since timeout is 0,
the driver won't wait for the ACK. If a user rapidly issues another sysfs
write, the next operation could take rom_mutex and send its own command.
Because generic ACKs appear to lack command IDs, will claw_raw_event()
blindly consume the orphaned ACK from this command and prematurely complete
the new command's wait before the MCU has actually processed it?
> + }
> +
> + return ret;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522015518.1111290-1-derekjohn.clark@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v8 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: sashiko-bot @ 2026-05-22 2:33 UTC (permalink / raw)
To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260522015518.1111290-2-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Sysfs attributes remain active during and after device removal, leading to a Time-of-Check to Time-of-Use (TOCTOU) race condition and Use-After-Free of HID transport structures.
- [High] Calling the sleeping function `hid_hw_close()` while holding a spinlock in `claw_remove()`, causing a kernel panic.
- [Low] Mixing `goto` error handling with scope-based cleanups (`__free()` and `guard()`) in the same function.
--
commit 76e865e5c59094dfdc13a67a7954da1cc82124bc
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add MSI Claw configuration driver
This commit adds a new HID configuration driver for the MSI Claw handheld PC.
It introduces the initial driver structure, including sysfs attributes to
control the gamepad mode, M-key behavior, and device reset commands.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> new file mode 100644
> index 0000000000000..40b16253abbb3
> --- /dev/null
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> +static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
> + size_t len, unsigned int timeout)
> +{
> + unsigned char *dmabuf __free(kfree) = NULL;
> + u8 header[] = { CLAW_OUTPUT_REPORT_ID, 0, 0, 0x3c, index };
> + struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> + size_t header_size = ARRAY_SIZE(header);
> + int ret;
> +
> + if (header_size + len > CLAW_PACKET_SIZE)
> + return -EINVAL;
> +
> + /* We can't use a devm_alloc reusable buffer without side effects during suspend */
> + dmabuf = kzalloc(CLAW_PACKET_SIZE, GFP_KERNEL);
> + if (!dmabuf)
> + return -ENOMEM;
> +
> + memcpy(dmabuf, header, header_size);
> + if (data && len)
> + memcpy(dmabuf + header_size, data, len);
> +
> + guard(mutex)(&drvdata->cfg_mutex);
> + if (timeout) {
> + reinit_completion(&drvdata->send_cmd_complete);
> + scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
> + drvdata->waiting_cmd = index;
> + drvdata->waiting_for_ack = true;
> + drvdata->cmd_status = -ETIMEDOUT;
> + }
> + }
> +
> + dev_dbg(&hdev->dev, "Send data as raw output report: [%*ph]\n",
> + CLAW_PACKET_SIZE, dmabuf);
> +
> + ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
> + if (ret < 0)
> + goto err_clear_state;
[Severity: Low]
Is it okay to mix goto error labels with scope-based cleanups in the same
function?
According to the cleanup subsystem guidelines, functions should either fully
rely on scope-based cleanups or use manual cleanups entirely to avoid
confusing ownership semantics.
> +
> + ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
> + if (ret)
> + goto err_clear_state;
> +
> + if (timeout) {
> + ret = wait_for_completion_interruptible_timeout(&drvdata->send_cmd_complete,
> + msecs_to_jiffies(timeout));
> +
> + dev_dbg(&hdev->dev, "Remaining timeout: %u\n", ret);
> + ret = ret > 0 ? drvdata->cmd_status : ret ?: -EBUSY;
> + }
> +
> +err_clear_state:
> + if (timeout) {
> + guard(spinlock_irqsave)(&drvdata->cmd_lock);
> + drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
> + drvdata->waiting_for_ack = false;
> + }
> +
> + return ret;
> +}
[ ... ]
> +static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> + int i, ret = -EINVAL;
> + u8 data[2];
> +
> + scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
> + /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
> + if (!smp_load_acquire(&drvdata->gamepad_registered))
> + return -ENODEV;
> + }
[Severity: High]
Can this lead to a use-after-free during device disconnect?
If a sysfs thread calls gamepad_mode_store() and gets preempted right after
releasing registration_lock here, a concurrent device disconnect could
trigger msi_remove() which clears gamepad_registered.
msi_remove() will then call hid_hw_stop(), destroying the HID transport
structures. When the sysfs thread resumes and continues execution here:
> +
> + for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
> + if (claw_gamepad_mode_text[i] && sysfs_streq(buf, claw_gamepad_mode_text[i])) {
> + ret = i;
> + break;
> + }
> + }
> + if (ret < 0)
> + return ret;
> +
> + data[0] = ret;
> + scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
> + data[1] = drvdata->mkeys_function;
> +
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
It will call claw_hw_output_report() which delegates to hid_hw_output_report()
and accesses the now-destroyed transport structures.
Since devm_device_add_group() is used to register the sysfs attributes, the
sysfs group remains active until after msi_remove() finishes completely.
Should the sysfs group be removed synchronously before hid_hw_stop() is
called to prevent this race?
> + if (ret)
> + return ret;
> +
> + return count;
> +}
[ ... ]
> +static void claw_remove(struct hid_device *hdev)
> +{
> + struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> +
> + if (!drvdata)
> + return;
> +
> + cancel_delayed_work_sync(&drvdata->cfg_resume);
> + cancel_delayed_work_sync(&drvdata->cfg_setup);
> +
> + guard(spinlock_irqsave)(&drvdata->registration_lock);
> + /* Pairs with smp_load_acquire in attribute show/store functions */
> + smp_store_release(&drvdata->gamepad_registered, false);
> +
> + hid_hw_close(hdev);
[Severity: High]
Does this call a sleeping function while holding a spinlock?
The guard() macro acquires registration_lock and holds it until the end of
the function. While this spinlock is held and interrupts are disabled,
hid_hw_close() is called, which attempts to acquire the sleeping mutex
ll_open_lock.
I noticed this is fixed in a later patch in the series by changing guard()
to scoped_guard(), but doesn't this break bisectability by introducing a
kernel panic in this commit?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522015518.1111290-1-derekjohn.clark@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] HID: u2fzero: fix general protection fault in u2fzero_recv
From: Li Zao @ 2026-05-22 2:01 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <ag8UFDazSyHKR0d2@beelink>
> Couple of things:
> - sorry, but a couple of days later someone else send a patch to tackle
> the same problem at a better level IMO:
> https://lore.kernel.org/linux-input/20260424-u2fzero-probe-urb-unwind-v1-1-mhun512@gmail.com/
> So I'm going to take this one instead of yours. I would appreciate if
> you can confirm this fixes your current reproducer.
>
> - Your commit description is not great. Please have a look at
> Documentation/process/submitting-patches.rst The thing that made me
> notice this is the non imperative form (quoting
> submitting-patches.rst):
> """
> Describe your changes in imperative mood,
> e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do
> frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders
> to the codebase to change its behaviour.
> """
> The paragraph above is also hard to follow but valuable for
> understanding why it needs to be fixed.
>
> Again, in the end the problem is that is dev->urb is NULL, there are
> little chances the system is healthy and the device will work correctly,
> so it's best to bail out early in probe like the other patch I
> mentioned.
>
> Cheers,
> Benjamin
Hi Benjamin,
I have verified the patch against our reproducer and can confirm that
it indeed fixes the issue. And thanks for the guidance on the commit
description. That's quite useful.
BTW, would it be possible to add a "Reported-by" tag to the patch? I'm
not sure about the exact convention for this situation, since the bug
was found by our private fuzzing tool and no public report is available
on the web.
Thanks again for your time and guidance!
Best regards,
Haocheng
^ permalink raw reply
* [PATCH 2/2] Input: userio - Allow setting other id values
From: Vicki Pfau @ 2026-05-22 1:50 UTC (permalink / raw)
To: Dmitry Torokhov, Lyude Paul, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20260522015040.3953472-1-vi@endrift.com>
Previously, only the type value was settable. The proto value is used
internally for choosing the right drivers, so we should expose it. The
other values make sense to expose as well.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
Documentation/input/userio.rst | 23 +++++++++++++++++++++--
drivers/input/serio/userio.c | 30 ++++++++++++++++++++++++++++++
include/uapi/linux/userio.h | 5 ++++-
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/Documentation/input/userio.rst b/Documentation/input/userio.rst
index 415962152815..7aaaa629bde0 100644
--- a/Documentation/input/userio.rst
+++ b/Documentation/input/userio.rst
@@ -66,8 +66,27 @@ USERIO_CMD_SET_PORT_TYPE
~~~~~~~~~~~~~~~~~~~~~~~~
Sets the type of port we're emulating, where ``data`` is the port type being
-set. Can be any of the macros from <linux/serio.h>. For example: SERIO_8042
-would set the port type to be a normal PS/2 port.
+set. Can be any of the serio type macros from <linux/serio.h>. For example:
+SERIO_8042 would set the port type to be a normal PS/2 port.
+
+USERIO_CMD_SET_PORT_PROTO
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Sets the protocol of port we're emulating, where ``data`` is the protocol being
+set. Can be any of the serio proto macros from <linux/serio.h>. For example:
+SERIO_IFORCE would set the port type to be an I-Force serial joystick.
+
+USERIO_CMD_SET_PORT_ID
+~~~~~~~~~~~~~~~~~~~~~~
+
+Sets the ``id`` value on the identification of port we're emulating, where
+``data`` is the value being set.
+
+USERIO_CMD_SET_PORT_EXTRA
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Sets the ``extra`` value on the identification of port we're emulating, where
+``data`` is the value being set.
USERIO_CMD_SEND_INTERRUPT
~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
index abca8cb6aca5..8c19975c84bf 100644
--- a/drivers/input/serio/userio.c
+++ b/drivers/input/serio/userio.c
@@ -206,6 +206,36 @@ static int userio_execute_cmd(struct userio_device *userio,
userio->serio->id.type = cmd->data;
break;
+ case USERIO_CMD_SET_PORT_EXTRA:
+ if (userio->running) {
+ dev_warn(userio_misc.this_device,
+ "Can't change port extra on an already running userio instance\n");
+ return -EBUSY;
+ }
+
+ userio->serio->id.extra = cmd->data;
+ break;
+
+ case USERIO_CMD_SET_PORT_ID:
+ if (userio->running) {
+ dev_warn(userio_misc.this_device,
+ "Can't change port id on an already running userio instance\n");
+ return -EBUSY;
+ }
+
+ userio->serio->id.id = cmd->data;
+ break;
+
+ case USERIO_CMD_SET_PORT_PROTO:
+ if (userio->running) {
+ dev_warn(userio_misc.this_device,
+ "Can't change port proto on an already running userio instance\n");
+ return -EBUSY;
+ }
+
+ userio->serio->id.proto = cmd->data;
+ break;
+
case USERIO_CMD_SEND_INTERRUPT:
if (!userio->running) {
dev_warn(userio_misc.this_device,
diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
index 98fe7e9089c4..550c7465af1f 100644
--- a/include/uapi/linux/userio.h
+++ b/include/uapi/linux/userio.h
@@ -27,7 +27,10 @@
enum userio_cmd_type {
USERIO_CMD_REGISTER = 0,
USERIO_CMD_SET_PORT_TYPE = 1,
- USERIO_CMD_SEND_INTERRUPT = 2
+ USERIO_CMD_SEND_INTERRUPT = 2,
+ USERIO_CMD_SET_PORT_EXTRA = 3,
+ USERIO_CMD_SET_PORT_ID = 4,
+ USERIO_CMD_SET_PORT_PROTO = 5,
};
/*
--
2.54.0
^ permalink raw reply related
* [PATCH 1/2] Input: userio - Update maintainer name
From: Vicki Pfau @ 2026-05-22 1:50 UTC (permalink / raw)
To: Dmitry Torokhov, Lyude Paul, linux-input; +Cc: Vicki Pfau
She's been committing under the name Lyude Paul for a while
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
Documentation/input/userio.rst | 2 +-
MAINTAINERS | 2 +-
drivers/input/serio/userio.c | 4 ++--
include/uapi/linux/userio.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/input/userio.rst b/Documentation/input/userio.rst
index f780c77931fe..415962152815 100644
--- a/Documentation/input/userio.rst
+++ b/Documentation/input/userio.rst
@@ -5,7 +5,7 @@ The userio Protocol
===================
-:Copyright: |copy| 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+:Copyright: |copy| 2015 Lyude Paul <thatslyude@gmail.com>
Sponsored by Red Hat
diff --git a/MAINTAINERS b/MAINTAINERS
index 7cf5b55c5973..ceacb1c1d679 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28105,7 +28105,7 @@ F: sound/drivers/pcmtest.c
F: tools/testing/selftests/alsa/test-pcmtest-driver.c
VIRTUAL SERIO DEVICE DRIVER
-M: Stephen Chandler Paul <thatslyude@gmail.com>
+M: Lyude Paul <thatslyude@gmail.com>
S: Maintained
F: drivers/input/serio/userio.c
F: include/uapi/linux/userio.h
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
index 91cb7a177b2d..abca8cb6aca5 100644
--- a/drivers/input/serio/userio.c
+++ b/drivers/input/serio/userio.c
@@ -1,7 +1,7 @@
/*
* userio kernel serio device emulation module
* Copyright (C) 2015 Red Hat
- * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as published by
@@ -278,6 +278,6 @@ module_driver(userio_misc, misc_register, misc_deregister);
MODULE_ALIAS_MISCDEV(USERIO_MINOR);
MODULE_ALIAS("devname:" USERIO_NAME);
-MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
+MODULE_AUTHOR("Lyude Paul <thatslyude@gmail.com>");
MODULE_DESCRIPTION("Virtual Serio Device Support");
MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
index 74c9951d2cd0..98fe7e9089c4 100644
--- a/include/uapi/linux/userio.h
+++ b/include/uapi/linux/userio.h
@@ -2,7 +2,7 @@
/*
* userio: virtual serio device support
* Copyright (C) 2015 Red Hat
- * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
+ * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as published by the
--
2.54.0
^ permalink raw reply related
* [PATCH v8 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-05-22 1:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260522015518.1111290-1-derekjohn.clark@gmail.com>
Adds intensity adjustment for the left and right rumble motors.
Claude was used during the reverse-engineering data gathering for this
feature done by Zhouwang Huang. As the code had already been affected,
I used Claude to create the initial framing for the feature, then did
manual cleanup of the _show and _store functions afterwards to fix bugs
and keep the coding style consistent. Claude was also used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v7:
- Match on write address for rumble reports to prevent late ACK
from causing synchronization errors.
- Use spinlock for read/write profile_pending.
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Use struct for rumble reports.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Add spinlock_irqsave for read/write access on rumble_intensity
variables.
- Gate all attribute show/store functions with gamepad_registered.
v5:
- Remove mkey related changes.
v2:
- Use pending_profile and sync to rom mutexes.
---
drivers/hid/hid-msi.c | 200 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 200 insertions(+)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 0b875603dbb32..add790adfa462 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -79,6 +79,8 @@ enum claw_profile_ack_pending {
CLAW_M1_PENDING,
CLAW_M2_PENDING,
CLAW_RGB_PENDING,
+ CLAW_RUMBLE_LEFT_PENDING,
+ CLAW_RUMBLE_RIGHT_PENDING,
};
enum claw_key_index {
@@ -266,6 +268,11 @@ static const u16 button_mapping_addr_new[] = {
static const u16 rgb_addr_old = 0x01fa;
static const u16 rgb_addr_new = 0x024a;
+static const u16 rumble_addr[] = {
+ 0x0022, /* left */
+ 0x0023, /* right */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -308,6 +315,12 @@ struct claw_rgb_report {
struct rgb_frame zone_data;
} __packed;
+struct claw_rumble_report {
+ struct claw_profile_report;
+ u8 padding;
+ u8 intensity;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -332,9 +345,13 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
+ u8 rumble_intensity_right;
+ u8 rumble_intensity_left;
bool gamepad_registered;
+ spinlock_t rumble_lock; /* lock for rumble_intensity read/write */
spinlock_t mode_lock; /* Lock for mode data read/write */
const u16 *bmap_addr;
+ bool rumble_support;
bool bmap_support;
/* RGB Variables */
@@ -382,6 +399,7 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
{
enum claw_profile_ack_pending profile;
+ struct claw_rumble_report *rumble;
struct claw_mkey_report *mkeys;
struct claw_rgb_report *frame;
u16 rgb_addr, read_addr;
@@ -431,6 +449,20 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
}
break;
+ case CLAW_RUMBLE_LEFT_PENDING:
+ rumble = (struct claw_rumble_report *)cmd_rep->data;
+ if (be16_to_cpu(rumble->read_addr) != rumble_addr[0])
+ return -EINVAL;
+ scoped_guard(spinlock, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_left = rumble->intensity;
+ break;
+ case CLAW_RUMBLE_RIGHT_PENDING:
+ rumble = (struct claw_rumble_report *)cmd_rep->data;
+ if (be16_to_cpu(rumble->read_addr) != rumble_addr[1])
+ return -EINVAL;
+ scoped_guard(spinlock, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_right = rumble->intensity;
+ break;
default:
dev_dbg(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
@@ -930,6 +962,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
}
static DEVICE_ATTR_RO(button_mapping_options);
+static ssize_t rumble_intensity_left_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[0])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ report.intensity = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_left_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[0])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ val = drvdata->rumble_intensity_left;
+
+ return sysfs_emit(buf, "%u\n", val);
+}
+static DEVICE_ATTR_RW(rumble_intensity_left);
+
+static ssize_t rumble_intensity_right_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[1])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ report.intensity = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_right_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[1])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RUMBLE_RIGHT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ val = drvdata->rumble_intensity_right;
+
+ return sysfs_emit(buf, "%u\n", val);
+}
+static DEVICE_ATTR_RW(rumble_intensity_right);
+
+static ssize_t rumble_intensity_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0-100\n");
+}
+static DEVICE_ATTR_RO(rumble_intensity_range);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -950,6 +1138,12 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
attr == &dev_attr_reset.attr)
return attr->mode;
+ /* Hide rumble attrs if not supported */
+ if (attr == &dev_attr_rumble_intensity_left.attr ||
+ attr == &dev_attr_rumble_intensity_right.attr ||
+ attr == &dev_attr_rumble_intensity_range.attr)
+ return drvdata->rumble_support ? attr->mode : 0;
+
/* Hide button mapping attrs if it isn't supported */
return drvdata->bmap_support ? attr->mode : 0;
}
@@ -963,6 +1157,9 @@ static struct attribute *claw_gamepad_attrs[] = {
&dev_attr_mkeys_function.attr,
&dev_attr_mkeys_function_index.attr,
&dev_attr_reset.attr,
+ &dev_attr_rumble_intensity_left.attr,
+ &dev_attr_rumble_intensity_right.attr,
+ &dev_attr_rumble_intensity_range.attr,
NULL,
};
@@ -1534,6 +1731,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
drvdata->bmap_support = true;
if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
} else {
drvdata->bmap_addr = button_mapping_addr_old;
@@ -1545,6 +1743,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
return;
}
@@ -1594,6 +1793,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
spin_lock_init(&drvdata->mode_lock);
spin_lock_init(&drvdata->profile_lock);
spin_lock_init(&drvdata->frame_lock);
+ spin_lock_init(&drvdata->rumble_lock);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
--
2.53.0
^ permalink raw reply related
* [PATCH v8 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-05-22 1:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260522015518.1111290-1-derekjohn.clark@gmail.com>
Adds RGB control interface for MSI Claw devices. The MSI Claw uses a
fairly unique RGB interface. It has 9 total zones (4 per joystick ring
and 1 for the ABXY buttons), and supports up to 8 sequential frames of
RGB zone data. Each frame is written to a specific area of MCU memory by
the profile command, the value of which changes based on the firmware of
the device. Unlike other devices (such as the Legion Go or the OneXPlayer
devices), there are no hard coded effects built into the MCU. Instead,
the basic effects are provided as a series of frame data. I have
mirrored the effects available in Windows in this driver, while keeping
the effect names consistent with the Lenovo drivers for the effects that
are similar.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v8:
- Ensure led_classdev is unregistered if adding attribute group fails.
- Reorder remove actions to ensure no use-after free or rearming cleared
flags.
v7:
- Use smp_[store_release|load_acquire] pattern for checking
rgb_registered to avoid possible races during teardown.
- Add gating to cfg_setup_fn, allowing either gamepad settings or rgb
settings to populate if the other fails for any reason.
- Use spinlock when writing profile_pending.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with rgb_registered,
enabling use of devm_device_add_group.
v5:
- Move adding the RGB device into cfg_setup to prevent led core
attributes from being written to prior to setup completing.
- Ensure frame_lock is properly init.
- Change variable names in RGB functions from frame and zone to f and
z respectively to fit all scoped_guard actions in 100 columns.
v4:
- Fix frame_calc validity check to use >=.
- USe spinlock instead of mutex in raw_event and related attribute
_store function.
- Ensure delayed work is canceled in suspend & canceled before sysfs
attribute removal.
v3:
- Add mutex for read/write of rgb frame data.
- Remove setting rgb_frame_count when reading rgb profiles as it always
returns garbage data.
- Ensure rgb_speed is getting drvdata from a valid lookup (not hdev).
v2:
- Use pending_profile mutex
---
drivers/hid/hid-msi.c | 674 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 654 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index b9901869075f6..0b875603dbb32 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -21,6 +21,7 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/kobject.h>
+#include <linux/led-class-multicolor.h>
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -44,6 +45,10 @@
#define CLAW_KEYS_MAX 5
+#define CLAW_RGB_ZONES 9
+#define CLAW_RGB_MAX_FRAMES 8
+#define CLAW_RGB_FRAME_OFFSET 0x24
+
enum claw_command_index {
CLAW_COMMAND_TYPE_NONE = 0x00,
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
@@ -73,6 +78,7 @@ enum claw_profile_ack_pending {
CLAW_NO_PENDING,
CLAW_M1_PENDING,
CLAW_M2_PENDING,
+ CLAW_RGB_PENDING,
};
enum claw_key_index {
@@ -231,6 +237,22 @@ static const struct {
{ 0xff, "DISABLED" },
};
+enum claw_rgb_effect_index {
+ CLAW_RGB_EFFECT_MONOCOLOR,
+ CLAW_RGB_EFFECT_BREATHE,
+ CLAW_RGB_EFFECT_CHROMA,
+ CLAW_RGB_EFFECT_RAINBOW,
+ CLAW_RGB_EFFECT_FROSTFIRE,
+};
+
+static const char * const claw_rgb_effect_text[] = {
+ [CLAW_RGB_EFFECT_MONOCOLOR] = "monocolor",
+ [CLAW_RGB_EFFECT_BREATHE] = "breathe",
+ [CLAW_RGB_EFFECT_CHROMA] = "chroma",
+ [CLAW_RGB_EFFECT_RAINBOW] = "rainbow",
+ [CLAW_RGB_EFFECT_FROSTFIRE] = "frostfire",
+};
+
static const u16 button_mapping_addr_old[] = {
0x007a, /* M1 */
0x011f, /* M2 */
@@ -241,6 +263,9 @@ static const u16 button_mapping_addr_new[] = {
0x0164, /* M2 */
};
+static const u16 rgb_addr_old = 0x01fa;
+static const u16 rgb_addr_new = 0x024a;
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -262,6 +287,27 @@ struct claw_mkey_report {
u8 codes[5];
} __packed;
+struct rgb_zone {
+ u8 red;
+ u8 green;
+ u8 blue;
+};
+
+struct rgb_frame {
+ struct rgb_zone zone[CLAW_RGB_ZONES];
+};
+
+struct claw_rgb_report {
+ struct claw_profile_report;
+ u8 frame_bytes;
+ u8 padding;
+ u8 frame_count;
+ u8 state; /* Always 0x09 */
+ u8 speed;
+ u8 brightness;
+ struct rgb_frame zone_data;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -290,6 +336,18 @@ struct claw_drvdata {
spinlock_t mode_lock; /* Lock for mode data read/write */
const u16 *bmap_addr;
bool bmap_support;
+
+ /* RGB Variables */
+ struct rgb_frame rgb_frames[CLAW_RGB_MAX_FRAMES];
+ enum claw_rgb_effect_index rgb_effect;
+ struct led_classdev_mc led_mc;
+ struct delayed_work rgb_queue;
+ spinlock_t frame_lock; /* lock for rgb_frames read/write */
+ bool rgb_registered;
+ u8 rgb_frame_count;
+ bool rgb_enabled;
+ u8 rgb_speed;
+ u16 rgb_addr;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -325,8 +383,11 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
{
enum claw_profile_ack_pending profile;
struct claw_mkey_report *mkeys;
- u8 *codes, key;
- int i;
+ struct claw_rgb_report *frame;
+ u16 rgb_addr, read_addr;
+ u8 *codes, key, f_idx;
+ u16 frame_calc;
+ int i, ret = 0;
scoped_guard(spinlock, &drvdata->profile_lock)
profile = drvdata->profile_pending;
@@ -342,6 +403,34 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
for (i = 0; i < CLAW_KEYS_MAX; i++)
codes[i] = (mkeys->codes[i]);
break;
+ case CLAW_RGB_PENDING:
+ frame = (struct claw_rgb_report *)cmd_rep->data;
+ rgb_addr = drvdata->rgb_addr;
+ read_addr = be16_to_cpu(frame->read_addr);
+ frame_calc = (read_addr - rgb_addr) / CLAW_RGB_FRAME_OFFSET;
+ if (frame_calc >= CLAW_RGB_MAX_FRAMES) {
+ dev_err(&drvdata->hdev->dev, "Got unsupported frame index: %x\n",
+ frame_calc);
+ return -EINVAL;
+ }
+ f_idx = frame_calc;
+
+ scoped_guard(spinlock, &drvdata->frame_lock) {
+ memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
+ sizeof(struct rgb_frame));
+
+ /* Only use frame 0 for remaining variable assignment */
+ if (f_idx != 0)
+ break;
+
+ drvdata->rgb_speed = frame->speed;
+ drvdata->led_mc.led_cdev.brightness = frame->brightness;
+ drvdata->led_mc.subled_info[0].intensity = frame->zone_data.zone[0].red;
+ drvdata->led_mc.subled_info[1].intensity = frame->zone_data.zone[0].green;
+ drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
+ }
+
+ break;
default:
dev_dbg(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
@@ -351,7 +440,7 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
scoped_guard(spinlock, &drvdata->profile_lock)
drvdata->profile_pending = CLAW_NO_PENDING;
- return 0;
+ return ret;
}
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
@@ -882,32 +971,545 @@ static const struct attribute_group claw_gamepad_attr_group = {
.is_visible = claw_gamepad_attr_is_visible,
};
+/* Read RGB config from device */
+static int claw_read_rgb_config(struct hid_device *hdev)
+{
+ u8 data[4] = { 0x01, 0x00, 0x00, CLAW_RGB_FRAME_OFFSET };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u16 read_addr = drvdata->rgb_addr;
+ size_t len = ARRAY_SIZE(data);
+ int ret, i;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ /* Loop through all 8 pages of RGB data */
+ guard(mutex)(&drvdata->profile_mutex);
+ for (i = 0; i < CLAW_RGB_MAX_FRAMES; i++) {
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RGB_PENDING;
+ data[1] = (read_addr >> 8) & 0xff;
+ data[2] = read_addr & 0x00ff;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data, len, 25);
+ if (ret)
+ return ret;
+
+ read_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ return 0;
+}
+
+/* Send RGB configuration to device */
+static int claw_write_rgb_state(struct claw_drvdata *drvdata)
+{
+ struct claw_rgb_report report = { {0x01, 0}, CLAW_RGB_FRAME_OFFSET, 0x00,
+ drvdata->rgb_frame_count, 0x09, drvdata->rgb_speed,
+ drvdata->led_mc.led_cdev.brightness };
+ u16 write_addr = drvdata->rgb_addr;
+ int f, ret;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ if (!drvdata->rgb_frame_count)
+ return -EINVAL;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ /* Loop through (up to) 8 pages of RGB data */
+ for (f = 0; f < drvdata->rgb_frame_count; f++) {
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock)
+ report.zone_data = drvdata->rgb_frames[f];
+
+ /* Set the MCU address to write the frame data to */
+ report.read_addr = cpu_to_be16(write_addr);
+
+ /* Serialize the rgb_report and write it to MCU */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* Increment the write addr by the offset for the next frame */
+ write_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+
+ return ret;
+}
+
+/* Fill all zones with the same color */
+static void claw_frame_fill_solid(struct rgb_frame *frame, struct rgb_zone zone)
+{
+ int z;
+
+ for (z = 0; z < CLAW_RGB_ZONES; z++)
+ frame->zone[z] = zone;
+}
+
+/* Apply solid effect (1 frame, no color) */
+static int claw_apply_disabled(struct claw_drvdata *drvdata)
+{
+ struct rgb_zone off = { 0x00, 0x00, 0x00};
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], off);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply solid effect (1 frame, all zones same color) */
+static int claw_apply_monocolor(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply breathe effect (2 frames: color -> off) */
+static int claw_apply_breathe(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+ static const struct rgb_zone off = { 0, 0, 0 };
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 2;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ claw_frame_fill_solid(&drvdata->rgb_frames[1], off);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply chroma effect (6 frames: rainbow cycle, all zones sync) */
+static int claw_apply_chroma(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ {255, 255, 0}, /* yellow */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ {255, 0, 255}, /* magenta */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++)
+ claw_frame_fill_solid(&drvdata->rgb_frames[f], colors[f]);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply rainbow effect (4 frames: rotating colors around joysticks) */
+static int claw_apply_rainbow(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f, z;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++) {
+ for (z = 0; z < 4; z++) {
+ drvdata->rgb_frames[f].zone[z] = colors[(z + f) % 4];
+ drvdata->rgb_frames[f].zone[z + 4] = colors[(z + f) % 4];
+ }
+ drvdata->rgb_frames[f].zone[8] = colors[f];
+ }
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/*
+ * Apply frostfire effect (4 frames: fire vs ice rotating)
+ * Right joystick: fire red -> dark -> ice blue -> dark (clockwise)
+ * Left joystick: ice blue -> dark -> fire red -> dark (counter-clockwise)
+ * ABXY: fire red -> dark -> ice blue -> dark
+ */
+static int claw_apply_frostfire(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* fire red */
+ { 0, 0, 0}, /* dark */
+ { 0, 0, 255}, /* ice blue */
+ { 0, 0, 0}, /* dark */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f, z;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++) {
+ for (z = 0; z < 4; z++) {
+ drvdata->rgb_frames[f].zone[z] = colors[(z + f) % 4];
+ drvdata->rgb_frames[f].zone[z + 4] = colors[(z - f + 6) % 4];
+ }
+ drvdata->rgb_frames[f].zone[8] = colors[f];
+ }
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply current state to device */
+static int claw_apply_rgb_state(struct claw_drvdata *drvdata)
+{
+ if (!drvdata->rgb_enabled)
+ return claw_apply_disabled(drvdata);
+
+ switch (drvdata->rgb_effect) {
+ case CLAW_RGB_EFFECT_MONOCOLOR:
+ return claw_apply_monocolor(drvdata);
+ case CLAW_RGB_EFFECT_BREATHE:
+ return claw_apply_breathe(drvdata);
+ case CLAW_RGB_EFFECT_CHROMA:
+ return claw_apply_chroma(drvdata);
+ case CLAW_RGB_EFFECT_RAINBOW:
+ return claw_apply_rainbow(drvdata);
+ case CLAW_RGB_EFFECT_FROSTFIRE:
+ return claw_apply_frostfire(drvdata);
+ default:
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "No supported rgb_effect selected\n");
+ return -EINVAL;
+ }
+}
+
+static void claw_rgb_queue_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, rgb_queue);
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return;
+ }
+
+ ret = claw_apply_rgb_state(drvdata);
+ if (ret)
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "Failed to apply RGB state: %d\n", ret);
+}
+
+static ssize_t effect_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ ret = sysfs_match_string(claw_rgb_effect_text, buf);
+ if (ret < 0)
+ return ret;
+
+ drvdata->rgb_effect = ret;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t effect_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ if (drvdata->rgb_effect >= ARRAY_SIZE(claw_rgb_effect_text))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", claw_rgb_effect_text[drvdata->rgb_effect]);
+}
+
+static DEVICE_ATTR_RW(effect);
+
+static ssize_t effect_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_rgb_effect_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_rgb_effect_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(effect_index);
+
+static ssize_t enabled_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ bool val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ drvdata->rgb_enabled = val;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t enabled_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ return sysfs_emit(buf, "%s\n", drvdata->rgb_enabled ? "true" : "false");
+}
+static DEVICE_ATTR_RW(enabled);
+
+static ssize_t enabled_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "true false\n");
+}
+static DEVICE_ATTR_RO(enabled_index);
+
+static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ unsigned int val, speed;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 20)
+ return -EINVAL;
+
+ /* 0 is fastest, invert value for intuitive userspace speed */
+ speed = 20 - val;
+
+ drvdata->rgb_speed = speed;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t speed_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ u8 speed = 20 - drvdata->rgb_speed;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ return sysfs_emit(buf, "%u\n", speed);
+}
+static DEVICE_ATTR_RW(speed);
+
+static ssize_t speed_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0-20\n");
+}
+static DEVICE_ATTR_RO(speed_range);
+
+static void claw_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness _brightness)
+{
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return;
+ }
+
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+}
+
+static struct attribute *claw_rgb_attrs[] = {
+ &dev_attr_effect.attr,
+ &dev_attr_effect_index.attr,
+ &dev_attr_enabled.attr,
+ &dev_attr_enabled_index.attr,
+ &dev_attr_speed.attr,
+ &dev_attr_speed_range.attr,
+ NULL,
+};
+
+static const struct attribute_group claw_rgb_attr_group = {
+ .attrs = claw_rgb_attrs,
+};
+
+static struct mc_subled claw_rgb_subled_info[] = {
+ {
+ .color_index = LED_COLOR_ID_RED,
+ .channel = 0x1,
+ },
+ {
+ .color_index = LED_COLOR_ID_GREEN,
+ .channel = 0x2,
+ },
+ {
+ .color_index = LED_COLOR_ID_BLUE,
+ .channel = 0x3,
+ },
+};
+
static void cfg_setup_fn(struct work_struct *work)
{
struct delayed_work *dwork = container_of(work, struct delayed_work, work);
struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_setup);
+ bool gamepad_ready = false, rgb_ready = false;
int ret;
ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE,
NULL, 0, 25);
if (ret) {
dev_err(&drvdata->hdev->dev,
- "Failed to setup device, can't read gamepad mode: %d\n", ret);
- return;
+ "Failed to read gamepad mode: %d\n", ret);
+ goto prep_rgb;
}
+ gamepad_ready = true;
- /* Add sysfs attributes after we get the device state */
- ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+prep_rgb:
+ ret = claw_read_rgb_config(drvdata->hdev);
if (ret) {
dev_err(&drvdata->hdev->dev,
- "Failed to setup device, can't create gamepad attrs: %d\n", ret);
- return;
+ "Failed to read RGB config: %d\n", ret);
+ goto try_gamepad;
+ }
+ rgb_ready = true;
+
+ /* Add sysfs attributes after we get the device state */
+try_gamepad:
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from below */
+ if (!smp_load_acquire(&drvdata->gamepad_registered) && gamepad_ready) {
+ ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create gamepad attrs: %d\n", ret);
+ goto try_rgb;
+ }
+
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gamepad_registered, true);
+ }
}
- scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
- /* Pairs with smp_load_acquire in attribute show/store functions */
- smp_store_release(&drvdata->gamepad_registered, true);
- kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+try_rgb:
+ /* Add and enable RGB interface once we have the device state */
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from below */
+ if (!smp_load_acquire(&drvdata->rgb_registered) && rgb_ready) {
+ ret = devm_led_classdev_multicolor_register(&drvdata->hdev->dev,
+ &drvdata->led_mc);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create led device: %d\n", ret);
+ goto update_kobjects;
+ }
+
+ ret = device_add_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create RGB attrs: %d\n", ret);
+ led_classdev_unregister(&drvdata->led_mc.led_cdev);
+ goto update_kobjects;
+ }
+
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->rgb_registered, true);
+ }
+ }
+
+update_kobjects:
+ guard(spinlock_irqsave)(&drvdata->registration_lock);
+ /* Pairs with smp_store_release from above */
+ if (smp_load_acquire(&drvdata->gamepad_registered))
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+ /* Pairs with smp_store_release from above */
+ if (smp_load_acquire(&drvdata->rgb_registered))
+ kobject_uevent(&drvdata->led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
}
static void cfg_resume_fn(struct work_struct *work)
@@ -916,8 +1518,10 @@ static void cfg_resume_fn(struct work_struct *work)
struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_resume);
guard(spinlock_irqsave)(&drvdata->registration_lock);
- /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
- if (!smp_load_acquire(&drvdata->gamepad_registered))
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered) ||
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ !smp_load_acquire(&drvdata->rgb_registered))
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
}
@@ -928,18 +1532,24 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if (major == 0x01) {
drvdata->bmap_support = true;
- if (minor >= 0x66)
+ if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
- else
+ drvdata->rgb_addr = rgb_addr_new;
+ } else {
drvdata->bmap_addr = button_mapping_addr_old;
+ drvdata->rgb_addr = rgb_addr_old;
+ }
return;
}
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rgb_addr = rgb_addr_new;
return;
}
+
+ drvdata->rgb_addr = rgb_addr_old;
}
static int claw_probe(struct hid_device *hdev, u8 ep)
@@ -954,6 +1564,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
return -ENOMEM;
drvdata->gamepad_mode = CLAW_GAMEPAD_MODE_XINPUT;
+ drvdata->rgb_enabled = true;
drvdata->hdev = hdev;
drvdata->ep = ep;
@@ -964,6 +1575,17 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
if (!drvdata->bmap_support)
dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+ drvdata->led_mc.led_cdev.name = "msi_claw:rgb:joystick_rings";
+ drvdata->led_mc.led_cdev.brightness = 0x50;
+ drvdata->led_mc.led_cdev.max_brightness = 0x64;
+ drvdata->led_mc.led_cdev.color = LED_COLOR_ID_RGB;
+ drvdata->led_mc.led_cdev.brightness_set = claw_led_brightness_set;
+ drvdata->led_mc.num_colors = 3;
+ drvdata->led_mc.subled_info = devm_kmemdup(&hdev->dev, claw_rgb_subled_info,
+ sizeof(claw_rgb_subled_info), GFP_KERNEL);
+ if (!drvdata->led_mc.subled_info)
+ return -ENOMEM;
+
mutex_init(&drvdata->cfg_mutex);
mutex_init(&drvdata->profile_mutex);
mutex_init(&drvdata->rom_mutex);
@@ -971,9 +1593,11 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
spin_lock_init(&drvdata->cmd_lock);
spin_lock_init(&drvdata->mode_lock);
spin_lock_init(&drvdata->profile_lock);
+ spin_lock_init(&drvdata->frame_lock);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+ INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
/* For control interface: open the HID transport for sending commands. */
ret = hid_hw_open(hdev);
@@ -1029,6 +1653,7 @@ static int msi_probe(struct hid_device *hdev, const struct hid_device_id *id)
static void claw_remove(struct hid_device *hdev)
{
struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ bool rgb_registered;
if (!drvdata)
return;
@@ -1036,9 +1661,18 @@ static void claw_remove(struct hid_device *hdev)
cancel_delayed_work_sync(&drvdata->cfg_resume);
cancel_delayed_work_sync(&drvdata->cfg_setup);
- guard(spinlock_irqsave)(&drvdata->registration_lock);
- /* Pairs with smp_load_acquire in attribute show/store functions */
- smp_store_release(&drvdata->gamepad_registered, false);
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gamepad_registered, false);
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->rgb_registered, false);
+ }
+
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
+ if (rgb_registered)
+ device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
hid_hw_close(hdev);
}
--
2.53.0
^ permalink raw reply related
* [PATCH v8 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-05-22 1:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260522015518.1111290-1-derekjohn.clark@gmail.com>
Adds attributes that allow for remapping the M-keys with up to 5 values
when in macro mode. There are 2 mappable buttons on the rear of the
device, M1 on the right and M2 on the left. When mapped, the events will
fire from one of three event devices: gamepad buttons will fire from the
device handled by xpad, while keyboard and mouse events will fire from
respectively typed evdevs provided by the input core. Names of each
mapping have been kept as close to the event that will fire from the evdev
as possible, with context added to the ABS_ events on the direction of the
movement.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v8:
- Wrap all branches under single cmd_lock guard in claw_raw_event.
- Reject generic ACK in claw_raw_event if waiting_cmd is for another
branch.
v7:
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Add profile_lock for read/write profile_pending.
- Match on write address for mkey reports to prevent late ACK
from causing synchronization errors.
- Use struct for mkey reports.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered.
- Remove duplicated argv_free macro.
v5:
- Ensure adding "DISABLED" key to valid entries is done in the correct
patch.
- Re-enable sending an empty string to clear button mappings in
addition to setting DISABLED.
v4:
- Change dev_warn to dev_dbg in claw_profile_event.
- use __free with DEFINE_FREE macro for argv instead of manually
running argv_free, cleaining up scoped_guard goto.
v3:
- Use scoped_guard where necessary.
v2:
- Add mutex for SYNC_TO_ROM commands to ensure every SYNC is completed
before more data is written to the MCU volatile memory.
- Add mutex for profile_pending to ensure every profile action
response is serialized to the generating command.
---
drivers/hid/hid-msi.c | 448 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 446 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 40b16253abbb3..b9901869075f6 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -42,6 +42,8 @@
#define CLAW_DINPUT_CFG_INTF_IN 0x82
#define CLAW_XINPUT_CFG_INTF_IN 0x83
+#define CLAW_KEYS_MAX 5
+
enum claw_command_index {
CLAW_COMMAND_TYPE_NONE = 0x00,
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
@@ -67,6 +69,17 @@ static const char * const claw_gamepad_mode_text[] = {
[CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
};
+enum claw_profile_ack_pending {
+ CLAW_NO_PENDING,
+ CLAW_M1_PENDING,
+ CLAW_M2_PENDING,
+};
+
+enum claw_key_index {
+ CLAW_KEY_M1,
+ CLAW_KEY_M2,
+};
+
enum claw_mkeys_function_index {
CLAW_MKEY_FUNCTION_MACRO,
CLAW_MKEY_FUNCTION_DISABLED,
@@ -79,6 +92,155 @@ static const char * const claw_mkeys_function_text[] = {
[CLAW_MKEY_FUNCTION_COMBO] = "combination",
};
+static const struct {
+ u8 code;
+ const char *name;
+} claw_button_mapping_key_map[] = {
+ /* Gamepad buttons */
+ { 0x01, "ABS_HAT0Y_UP" },
+ { 0x02, "ABS_HAT0Y_DOWN" },
+ { 0x03, "ABS_HAT0X_LEFT" },
+ { 0x04, "ABS_HAT0X_RIGHT" },
+ { 0x05, "BTN_TL" },
+ { 0x06, "BTN_TR" },
+ { 0x07, "BTN_THUMBL" },
+ { 0x08, "BTN_THUMBR" },
+ { 0x09, "BTN_SOUTH" },
+ { 0x0a, "BTN_EAST" },
+ { 0x0b, "BTN_NORTH" },
+ { 0x0c, "BTN_WEST" },
+ { 0x0d, "BTN_MODE" },
+ { 0x0e, "BTN_SELECT" },
+ { 0x0f, "BTN_START" },
+ { 0x13, "BTN_TL2"},
+ { 0x14, "BTN_TR2"},
+ { 0x15, "ABS_Y_UP"},
+ { 0x16, "ABS_Y_DOWN"},
+ { 0x17, "ABS_X_LEFT"},
+ { 0x18, "ABS_X_RIGHT"},
+ { 0x19, "ABS_RY_UP"},
+ { 0x1a, "ABS_RY_DOWN"},
+ { 0x1b, "ABS_RX_LEFT"},
+ { 0x1c, "ABS_RX_RIGHT"},
+ /* Keyboard keys */
+ { 0x32, "KEY_ESC" },
+ { 0x33, "KEY_F1" },
+ { 0x34, "KEY_F2" },
+ { 0x35, "KEY_F3" },
+ { 0x36, "KEY_F4" },
+ { 0x37, "KEY_F5" },
+ { 0x38, "KEY_F6" },
+ { 0x39, "KEY_F7" },
+ { 0x3a, "KEY_F8" },
+ { 0x3b, "KEY_F9" },
+ { 0x3c, "KEY_F10" },
+ { 0x3d, "KEY_F11" },
+ { 0x3e, "KEY_F12" },
+ { 0x3f, "KEY_GRAVE" },
+ { 0x40, "KEY_1" },
+ { 0x41, "KEY_2" },
+ { 0x42, "KEY_3" },
+ { 0x43, "KEY_4" },
+ { 0x44, "KEY_5" },
+ { 0x45, "KEY_6" },
+ { 0x46, "KEY_7" },
+ { 0x47, "KEY_8" },
+ { 0x48, "KEY_9" },
+ { 0x49, "KEY_0" },
+ { 0x4a, "KEY_MINUS" },
+ { 0x4b, "KEY_EQUAL" },
+ { 0x4c, "KEY_BACKSPACE" },
+ { 0x4d, "KEY_TAB" },
+ { 0x4e, "KEY_Q" },
+ { 0x4f, "KEY_W" },
+ { 0x50, "KEY_E" },
+ { 0x51, "KEY_R" },
+ { 0x52, "KEY_T" },
+ { 0x53, "KEY_Y" },
+ { 0x54, "KEY_U" },
+ { 0x55, "KEY_I" },
+ { 0x56, "KEY_O" },
+ { 0x57, "KEY_P" },
+ { 0x58, "KEY_LEFTBRACE" },
+ { 0x59, "KEY_RIGHTBRACE" },
+ { 0x5a, "KEY_BACKSLASH" },
+ { 0x5b, "KEY_CAPSLOCK" },
+ { 0x5c, "KEY_A" },
+ { 0x5d, "KEY_S" },
+ { 0x5e, "KEY_D" },
+ { 0x5f, "KEY_F" },
+ { 0x60, "KEY_G" },
+ { 0x61, "KEY_H" },
+ { 0x62, "KEY_J" },
+ { 0x63, "KEY_K" },
+ { 0x64, "KEY_L" },
+ { 0x65, "KEY_SEMICOLON" },
+ { 0x66, "KEY_APOSTROPHE" },
+ { 0x67, "KEY_ENTER" },
+ { 0x68, "KEY_LEFTSHIFT" },
+ { 0x69, "KEY_Z" },
+ { 0x6a, "KEY_X" },
+ { 0x6b, "KEY_C" },
+ { 0x6c, "KEY_V" },
+ { 0x6d, "KEY_B" },
+ { 0x6e, "KEY_N" },
+ { 0x6f, "KEY_M" },
+ { 0x70, "KEY_COMMA" },
+ { 0x71, "KEY_DOT" },
+ { 0x72, "KEY_SLASH" },
+ { 0x73, "KEY_RIGHTSHIFT" },
+ { 0x74, "KEY_LEFTCTRL" },
+ { 0x75, "KEY_LEFTMETA" },
+ { 0x76, "KEY_LEFTALT" },
+ { 0x77, "KEY_SPACE" },
+ { 0x78, "KEY_RIGHTALT" },
+ { 0x79, "KEY_RIGHTCTRL" },
+ { 0x7a, "KEY_INSERT" },
+ { 0x7b, "KEY_HOME" },
+ { 0x7c, "KEY_PAGEUP" },
+ { 0x7d, "KEY_DELETE" },
+ { 0x7e, "KEY_END" },
+ { 0x7f, "KEY_PAGEDOWN" },
+ { 0x8a, "KEY_KPENTER" },
+ { 0x8b, "KEY_KP0" },
+ { 0x8c, "KEY_KP1" },
+ { 0x8d, "KEY_KP2" },
+ { 0x8e, "KEY_KP3" },
+ { 0x8f, "KEY_KP4" },
+ { 0x90, "KEY_KP5" },
+ { 0x91, "KEY_KP6" },
+ { 0x92, "KEY_KP7" },
+ { 0x93, "KEY_KP8" },
+ { 0x94, "KEY_KP9" },
+ { 0x95, "MD_PLAY" },
+ { 0x96, "MD_STOP" },
+ { 0x97, "MD_NEXT" },
+ { 0x98, "MD_PREV" },
+ { 0x99, "MD_VOL_UP" },
+ { 0x9a, "MD_VOL_DOWN" },
+ { 0x9b, "MD_VOL_MUTE" },
+ { 0x9c, "KEY_F23" },
+ /* Mouse events */
+ { 0xc8, "BTN_LEFT" },
+ { 0xc9, "BTN_MIDDLE" },
+ { 0xca, "BTN_RIGHT" },
+ { 0xcb, "BTN_SIDE" },
+ { 0xcc, "BTN_EXTRA" },
+ { 0xcd, "REL_WHEEL_UP" },
+ { 0xce, "REL_WHEEL_DOWN" },
+ { 0xff, "DISABLED" },
+};
+
+static const u16 button_mapping_addr_old[] = {
+ 0x007a, /* M1 */
+ 0x011f, /* M2 */
+};
+
+static const u16 button_mapping_addr_new[] = {
+ 0x00bb, /* M1 */
+ 0x0164, /* M2 */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -87,25 +249,47 @@ struct claw_command_report {
u8 data[59];
} __packed;
+struct claw_profile_report {
+ u8 profile;
+ __be16 read_addr;
+} __packed;
+
+struct claw_mkey_report {
+ struct claw_profile_report;
+ u8 padding_0;
+ u8 padding_1;
+ u8 padding_2;
+ u8 codes[5];
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
+ enum claw_profile_ack_pending profile_pending;
struct completion send_cmd_complete;
struct delayed_work cfg_resume;
struct delayed_work cfg_setup;
spinlock_t registration_lock; /* Lock for registration read/write */
+ struct mutex profile_mutex; /* mutex for profile_pending calls */
+ spinlock_t profile_lock; /* Lock for profile_pending read/write */
struct hid_device *hdev;
struct mutex cfg_mutex; /* mutex for synchronous data */
+ struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
bool waiting_for_ack;
spinlock_t cmd_lock; /* Lock for cmd data read/write */
u8 waiting_cmd;
int cmd_status;
+ u16 bcd_device;
u8 ep;
/* Gamepad Variables */
enum claw_mkeys_function_index mkeys_function;
enum claw_gamepad_mode_index gamepad_mode;
+ u8 m1_codes[CLAW_KEYS_MAX];
+ u8 m2_codes[CLAW_KEYS_MAX];
bool gamepad_registered;
spinlock_t mode_lock; /* Lock for mode data read/write */
+ const u16 *bmap_addr;
+ bool bmap_support;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -137,6 +321,39 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
return 0;
}
+static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
+{
+ enum claw_profile_ack_pending profile;
+ struct claw_mkey_report *mkeys;
+ u8 *codes, key;
+ int i;
+
+ scoped_guard(spinlock, &drvdata->profile_lock)
+ profile = drvdata->profile_pending;
+
+ switch (profile) {
+ case CLAW_M1_PENDING:
+ case CLAW_M2_PENDING:
+ key = (profile == CLAW_M1_PENDING) ? CLAW_KEY_M1 : CLAW_KEY_M2;
+ mkeys = (struct claw_mkey_report *)cmd_rep->data;
+ if (be16_to_cpu(mkeys->read_addr) != drvdata->bmap_addr[key])
+ return -EINVAL;
+ codes = (profile == CLAW_M1_PENDING) ? drvdata->m1_codes : drvdata->m2_codes;
+ for (i = 0; i < CLAW_KEYS_MAX; i++)
+ codes[i] = (mkeys->codes[i]);
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev,
+ "Got profile event without changes pending from command: %x\n",
+ cmd_rep->cmd);
+ return -EINVAL;
+ }
+ scoped_guard(spinlock, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_NO_PENDING;
+
+ return 0;
+}
+
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
u8 *data, int size)
{
@@ -165,9 +382,20 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
complete(&drvdata->send_cmd_complete);
}
+ break;
+ case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
+ ret = claw_profile_event(drvdata, cmd_rep);
+ if (drvdata->waiting_for_ack &&
+ drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_PROFILE) {
+ drvdata->cmd_status = ret;
+ drvdata->waiting_for_ack = false;
+ complete(&drvdata->send_cmd_complete);
+ }
+
break;
case CLAW_COMMAND_TYPE_ACK:
- if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE)
+ if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_PROFILE ||
+ drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE)
break;
if (drvdata->waiting_for_ack) {
drvdata->cmd_status = 0;
@@ -442,6 +670,177 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(reset);
+static int mkey_mapping_name_to_code(const char *name)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (!strcmp(name, claw_button_mapping_key_map[i].name))
+ return claw_button_mapping_key_map[i].code;
+ }
+
+ return -EINVAL;
+}
+
+static const char *mkey_mapping_code_to_name(u8 code)
+{
+ int i;
+
+ if (code == 0xff)
+ return NULL;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (claw_button_mapping_key_map[i].code == code)
+ return claw_button_mapping_key_map[i].name;
+ }
+
+ return NULL;
+}
+
+static int claw_mkey_store(struct device *dev, const char *buf, u8 mkey)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct claw_mkey_report report = { {0x01, cpu_to_be16(drvdata->bmap_addr[mkey])},
+ 0x07, 0x04, 0x00, {0xff, 0xff, 0xff, 0xff, 0xff} };
+ char **raw_keys __free(argv_free) = NULL;
+ int ret, key_count, i;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ raw_keys = argv_split(GFP_KERNEL, buf, &key_count);
+ if (!raw_keys)
+ return -ENOMEM;
+
+ if (key_count > CLAW_KEYS_MAX)
+ return -EINVAL;
+
+ if (key_count == 0)
+ goto set_buttons;
+
+ for (i = 0; i < key_count; i++) {
+ ret = mkey_mapping_name_to_code(raw_keys[i]);
+ if (ret < 0)
+ return ret;
+
+ report.codes[i] = ret;
+ }
+
+set_buttons:
+ scoped_guard(mutex, &drvdata->rom_mutex) {
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ }
+
+ return ret;
+}
+
+static int claw_mkey_show(struct device *dev, char *buf, enum claw_key_index m_key)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct claw_mkey_report report = { {0x01, cpu_to_be16(drvdata->bmap_addr[m_key])}, 0x07 };
+ int i, ret, count = 0;
+ const char *name;
+ u8 *codes;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = (m_key == CLAW_KEY_M1) ? CLAW_M1_PENDING
+ : CLAW_M2_PENDING;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < CLAW_KEYS_MAX; i++) {
+ name = mkey_mapping_code_to_name(codes[i]);
+ if (name)
+ count += sysfs_emit_at(buf, count, "%s ", name);
+ }
+
+ if (!count)
+ return sysfs_emit(buf, "(not set)\n");
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+
+static ssize_t button_m1_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_mkey_store(dev, buf, CLAW_KEY_M1);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m1_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_mkey_show(dev, buf, CLAW_KEY_M1);
+}
+static DEVICE_ATTR_RW(button_m1);
+
+static ssize_t button_m2_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_mkey_store(dev, buf, CLAW_KEY_M2);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m2_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_mkey_show(dev, buf, CLAW_KEY_M2);
+}
+static DEVICE_ATTR_RW(button_m2);
+
+static ssize_t button_mapping_options_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_button_mapping_key_map[i].name);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(button_mapping_options);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -454,10 +853,22 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
return 0;
}
- return attr->mode;
+ /* Always show attrs available on all firmware */
+ if (attr == &dev_attr_gamepad_mode.attr ||
+ attr == &dev_attr_gamepad_mode_index.attr ||
+ attr == &dev_attr_mkeys_function.attr ||
+ attr == &dev_attr_mkeys_function_index.attr ||
+ attr == &dev_attr_reset.attr)
+ return attr->mode;
+
+ /* Hide button mapping attrs if it isn't supported */
+ return drvdata->bmap_support ? attr->mode : 0;
}
static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_button_m1.attr,
+ &dev_attr_button_m2.attr,
+ &dev_attr_button_mapping_options.attr,
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
&dev_attr_mkeys_function.attr,
@@ -510,8 +921,31 @@ static void cfg_resume_fn(struct work_struct *work)
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
}
+static void claw_features_supported(struct claw_drvdata *drvdata)
+{
+ u8 major = (drvdata->bcd_device >> 8) & 0xff;
+ u8 minor = drvdata->bcd_device & 0xff;
+
+ if (major == 0x01) {
+ drvdata->bmap_support = true;
+ if (minor >= 0x66)
+ drvdata->bmap_addr = button_mapping_addr_new;
+ else
+ drvdata->bmap_addr = button_mapping_addr_old;
+ return;
+ }
+
+ if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
+ drvdata->bmap_support = true;
+ drvdata->bmap_addr = button_mapping_addr_new;
+ return;
+ }
+}
+
static int claw_probe(struct hid_device *hdev, u8 ep)
{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_device *udev = interface_to_usbdev(intf);
struct claw_drvdata *drvdata;
int ret;
@@ -523,10 +957,20 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
drvdata->hdev = hdev;
drvdata->ep = ep;
+ /* Determine feature level from firmware version */
+ drvdata->bcd_device = le16_to_cpu(udev->descriptor.bcdDevice);
+ claw_features_supported(drvdata);
+
+ if (!drvdata->bmap_support)
+ dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+
mutex_init(&drvdata->cfg_mutex);
+ mutex_init(&drvdata->profile_mutex);
+ mutex_init(&drvdata->rom_mutex);
spin_lock_init(&drvdata->registration_lock);
spin_lock_init(&drvdata->cmd_lock);
spin_lock_init(&drvdata->mode_lock);
+ spin_lock_init(&drvdata->profile_lock);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
--
2.53.0
^ permalink raw reply related
* [PATCH v8 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek J. Clark @ 2026-05-22 1:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260522015518.1111290-1-derekjohn.clark@gmail.com>
Adds configuration HID driver for the MSI Claw series of handheld PC's.
In this initial patch add the initial driver outline and attributes for
changing the gamepad mode, M-key behavior, and add a WO reset function.
Sending the SWITCH_MODE and RESET commands causes a USB disconnect in
the device. The completion will therefore never get hit and would trigger
an -EIO. To avoid showing the user an error for every write to these
attrs a bypass for the completion handling is introduced when timeout ==
0.
The initial version of this patch was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes. Finally, I refactored the drivers data
in/out flow and overall format to conform to kernel driver best
practices and style guides. Claude was used as an initial reviewer of
this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v8:
- Use spinlock when accessing gamepad_registered.
- Clear state machine on all errors in claw_hw_output_report.
- Wrap all branches under single cmd_lock guard in claw_raw_event.
- Reject generic ACK in claw_raw_event if waiting_cmd is for another
branch.
- Don't close hid devices that couldn't have been opened.
v7:
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Reorder reinit_completion in claw_hw_output_report to avoid race
with possible incoming ACKs.
- Reorder cancel_delayed_work_sync to ensure setup can't be re-armed
after cancel.
- Reset command state machine if hw_output_report has an error.
- Add comments to (hopefully) silence sashinko-bot warnings about the
use of endpoint matching and the impossible scenario of switching to
the alternate endpoint from userspace while the driver is bound.
- Don't use spinlock_irqsave when already in irq context.
v6:
- Add send/ack pattern to ensure synchronous acks.
- Use spinlock_irqsave instead of mutex for read/write MODE event
data.
- add select NEW_LEDS to kconfig.
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered,
enabling use of devm_device_add_group.
- Re-arm cfg_setup in resume if it was canceled in an early suspend.
- Don't set gamepad_mode on resume, MCU preserves state.
- Ensure all count variables are checked for > 0 characters before
setting buf - 1 to \n.
v5:
- Swap disabled & combination mkeys_function enum values.
- Ensure mode_mutex is properly init.
- Ensure claw_remove is calling hid_hw_close and not hid_hw_stop for
all paths.
v4:
- Add msi_suspend/claw_suspend.
- Reorder claw_remove to cancel all work before removing sysfs.
- Add mutex lock for removing sysfs attributes.
- Add mutex lock for MODE command data read/write.
v3:
- Ensure claw_hw_output_report is properly guarded.
- Reoder claw_probe to ensure all mutex, completion, and variable
assignments are in place prior to setting drvdata.
- Ensure gamepad_mode is set to a valid enum value in claw_probe.
v2:
- Rename driver to hid-msi from hid-msi-claw.
- Rename reusable/generic functions to msi_* from claw_*, retaining
claw specific functions.
- Add generic entrypoints for probe, remove, and raw event that route
to claw specific functions.
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 13 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 5 +
drivers/hid/hid-msi.c | 703 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 728 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6f6517bf4f970..8e2de98b768f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17965,6 +17965,12 @@ S: Odd Fixes
F: Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
F: drivers/net/ieee802154/mrf24j40.c
+MSI HID DRIVER
+M: Derek J. Clark <derekjohn.clark@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-msi.c
+
MSI EC DRIVER
M: Nikita Kravets <teackot@gmail.com>
L: platform-driver-x86@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 10c12d8e65579..7766676051a52 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -492,6 +492,19 @@ config HID_GT683R
Currently the following devices are know to be supported:
- MSI GT683R
+config HID_MSI
+ tristate "MSI Claw Gamepad Support"
+ depends on USB_HID
+ select NEW_LEDS
+ select LEDS_CLASS
+ select LEDS_CLASS_MULTICOLOR
+ help
+ Support for the MSI Claw RGB and controller configuration
+
+ Say Y here to include configuration interface support for the MSI Claw Line
+ of Handheld Console Controllers. Say M here to compile this driver as a
+ module. The module will be called hid-msi.
+
config HID_KEYTOUCH
tristate "Keytouch HID devices"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 07dfdb6a49c59..80925a17b059c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o
obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o
obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o
obj-$(CONFIG_HID_MONTEREY) += hid-monterey.o
+obj-$(CONFIG_HID_MSI) += hid-msi.o
obj-$(CONFIG_HID_MULTITOUCH) += hid-multitouch.o
obj-$(CONFIG_HID_NINTENDO) += hid-nintendo.o
obj-$(CONFIG_HID_NTI) += hid-nti.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 933b7943bdb50..94a9b89dc240a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1047,7 +1047,12 @@
#define USB_DEVICE_ID_MOZA_R16_R21_2 0x0010
#define USB_VENDOR_ID_MSI 0x1770
+#define USB_VENDOR_ID_MSI_2 0x0db0
#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
+#define USB_DEVICE_ID_MSI_CLAW_XINPUT 0x1901
+#define USB_DEVICE_ID_MSI_CLAW_DINPUT 0x1902
+#define USB_DEVICE_ID_MSI_CLAW_DESKTOP 0x1903
+#define USB_DEVICE_ID_MSI_CLAW_BIOS 0x1904
#define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
#define USB_DEVICE_ID_N_S_HARMONY 0xc359
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
new file mode 100644
index 0000000000000..40b16253abbb3
--- /dev/null
+++ b/drivers/hid/hid-msi.c
@@ -0,0 +1,703 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for MSI Claw Handheld PC gamepads.
+ *
+ * Provides configuration support for the MSI Claw series of handheld PC
+ * gamepads. Multiple iterations of the device firmware has led to some
+ * quirks for how certain attributes are handled. The original firmware
+ * did not support remapping of the M1 (right) and M2 (left) rear paddles.
+ * Additionally, the MCU RAM address for writing configuration data has
+ * changed twice. Checks are done during probe to enumerate these variances.
+ *
+ * Copyright (c) 2026 Zhouwang Huang <honjow311@gmail.com>
+ * Copyright (c) 2026 Denis Benato <denis.benato@linux.dev>
+ * Copyright (c) 2026 Valve Corporation
+ */
+
+#include <linux/array_size.h>
+#include <linux/cleanup.h>
+#include <linux/completion.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/kobject.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/spinlock.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define CLAW_OUTPUT_REPORT_ID 0x0f
+#define CLAW_INPUT_REPORT_ID 0x10
+
+#define CLAW_PACKET_SIZE 64
+
+#define CLAW_DINPUT_CFG_INTF_IN 0x82
+#define CLAW_XINPUT_CFG_INTF_IN 0x83
+
+enum claw_command_index {
+ CLAW_COMMAND_TYPE_NONE = 0x00,
+ CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
+ CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
+ CLAW_COMMAND_TYPE_ACK = 0x06,
+ CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA = 0x21,
+ CLAW_COMMAND_TYPE_SYNC_TO_ROM = 0x22,
+ CLAW_COMMAND_TYPE_SWITCH_MODE = 0x24,
+ CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE = 0x26,
+ CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK = 0x27,
+ CLAW_COMMAND_TYPE_RESET_DEVICE = 0x28,
+};
+
+enum claw_gamepad_mode_index {
+ CLAW_GAMEPAD_MODE_XINPUT = 0x01,
+ CLAW_GAMEPAD_MODE_DINPUT = 0x02,
+ CLAW_GAMEPAD_MODE_DESKTOP = 0x04,
+};
+
+static const char * const claw_gamepad_mode_text[] = {
+ [CLAW_GAMEPAD_MODE_XINPUT] = "xinput",
+ [CLAW_GAMEPAD_MODE_DINPUT] = "dinput",
+ [CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
+};
+
+enum claw_mkeys_function_index {
+ CLAW_MKEY_FUNCTION_MACRO,
+ CLAW_MKEY_FUNCTION_DISABLED,
+ CLAW_MKEY_FUNCTION_COMBO,
+};
+
+static const char * const claw_mkeys_function_text[] = {
+ [CLAW_MKEY_FUNCTION_MACRO] = "macro",
+ [CLAW_MKEY_FUNCTION_DISABLED] = "disabled",
+ [CLAW_MKEY_FUNCTION_COMBO] = "combination",
+};
+
+struct claw_command_report {
+ u8 report_id;
+ u8 padding[2];
+ u8 header_tail;
+ u8 cmd;
+ u8 data[59];
+} __packed;
+
+struct claw_drvdata {
+ /* MCU General Variables */
+ struct completion send_cmd_complete;
+ struct delayed_work cfg_resume;
+ struct delayed_work cfg_setup;
+ spinlock_t registration_lock; /* Lock for registration read/write */
+ struct hid_device *hdev;
+ struct mutex cfg_mutex; /* mutex for synchronous data */
+ bool waiting_for_ack;
+ spinlock_t cmd_lock; /* Lock for cmd data read/write */
+ u8 waiting_cmd;
+ int cmd_status;
+ u8 ep;
+
+ /* Gamepad Variables */
+ enum claw_mkeys_function_index mkeys_function;
+ enum claw_gamepad_mode_index gamepad_mode;
+ bool gamepad_registered;
+ spinlock_t mode_lock; /* Lock for mode data read/write */
+};
+
+static int get_endpoint_address(struct hid_device *hdev)
+{
+ struct usb_host_endpoint *ep;
+ struct usb_interface *intf;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ ep = intf->cur_altsetting->endpoint;
+ if (ep)
+ return ep->desc.bEndpointAddress;
+
+ return -ENODEV;
+}
+
+static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
+ struct claw_command_report *cmd_rep)
+{
+ if (cmd_rep->data[0] >= ARRAY_SIZE(claw_gamepad_mode_text) ||
+ !claw_gamepad_mode_text[cmd_rep->data[0]] ||
+ cmd_rep->data[1] >= ARRAY_SIZE(claw_mkeys_function_text))
+ return -EINVAL;
+
+ scoped_guard(spinlock, &drvdata->mode_lock) {
+ drvdata->gamepad_mode = cmd_rep->data[0];
+ drvdata->mkeys_function = cmd_rep->data[1];
+ }
+
+ return 0;
+}
+
+static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_command_report *cmd_rep;
+ int ret = 0;
+
+ if (size != CLAW_PACKET_SIZE)
+ return 0;
+
+ cmd_rep = (struct claw_command_report *)data;
+
+ if (cmd_rep->report_id != CLAW_INPUT_REPORT_ID || cmd_rep->header_tail != 0x3c)
+ return 0;
+
+ dev_dbg(&drvdata->hdev->dev, "Rx data as raw input report: [%*ph]\n",
+ CLAW_PACKET_SIZE, data);
+
+ guard(spinlock)(&drvdata->cmd_lock);
+ switch (cmd_rep->cmd) {
+ case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
+ ret = claw_gamepad_mode_event(drvdata, cmd_rep);
+ if (drvdata->waiting_for_ack &&
+ drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE) {
+ drvdata->cmd_status = ret;
+ drvdata->waiting_for_ack = false;
+ complete(&drvdata->send_cmd_complete);
+ }
+
+ break;
+ case CLAW_COMMAND_TYPE_ACK:
+ if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE)
+ break;
+ if (drvdata->waiting_for_ack) {
+ drvdata->cmd_status = 0;
+ drvdata->waiting_for_ack = false;
+ complete(&drvdata->send_cmd_complete);
+ }
+ dev_dbg(&drvdata->hdev->dev, "Waiting CMD: %x\n", drvdata->waiting_cmd);
+
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev, "Unknown command: %x\n", cmd_rep->cmd);
+ return 0;
+ }
+
+ return ret;
+}
+
+static int msi_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata || (drvdata->ep != CLAW_XINPUT_CFG_INTF_IN &&
+ drvdata->ep != CLAW_DINPUT_CFG_INTF_IN))
+ return 0;
+
+ return claw_raw_event(drvdata, report, data, size);
+}
+
+static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
+ size_t len, unsigned int timeout)
+{
+ unsigned char *dmabuf __free(kfree) = NULL;
+ u8 header[] = { CLAW_OUTPUT_REPORT_ID, 0, 0, 0x3c, index };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ size_t header_size = ARRAY_SIZE(header);
+ int ret;
+
+ if (header_size + len > CLAW_PACKET_SIZE)
+ return -EINVAL;
+
+ /* We can't use a devm_alloc reusable buffer without side effects during suspend */
+ dmabuf = kzalloc(CLAW_PACKET_SIZE, GFP_KERNEL);
+ if (!dmabuf)
+ return -ENOMEM;
+
+ memcpy(dmabuf, header, header_size);
+ if (data && len)
+ memcpy(dmabuf + header_size, data, len);
+
+ guard(mutex)(&drvdata->cfg_mutex);
+ if (timeout) {
+ reinit_completion(&drvdata->send_cmd_complete);
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
+ drvdata->waiting_cmd = index;
+ drvdata->waiting_for_ack = true;
+ drvdata->cmd_status = -ETIMEDOUT;
+ }
+ }
+
+ dev_dbg(&hdev->dev, "Send data as raw output report: [%*ph]\n",
+ CLAW_PACKET_SIZE, dmabuf);
+
+ ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
+ if (ret < 0)
+ goto err_clear_state;
+
+ ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
+ if (ret)
+ goto err_clear_state;
+
+ if (timeout) {
+ ret = wait_for_completion_interruptible_timeout(&drvdata->send_cmd_complete,
+ msecs_to_jiffies(timeout));
+
+ dev_dbg(&hdev->dev, "Remaining timeout: %u\n", ret);
+ ret = ret > 0 ? drvdata->cmd_status : ret ?: -EBUSY;
+ }
+
+err_clear_state:
+ if (timeout) {
+ guard(spinlock_irqsave)(&drvdata->cmd_lock);
+ drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
+ drvdata->waiting_for_ack = false;
+ }
+
+ return ret;
+}
+
+static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int i, ret = -EINVAL;
+ u8 data[2];
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (claw_gamepad_mode_text[i] && sysfs_streq(buf, claw_gamepad_mode_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ data[0] = ret;
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ data[1] = drvdata->mkeys_function;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t gamepad_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ i = drvdata->gamepad_mode;
+
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_gamepad_mode_text[i]);
+}
+static DEVICE_ATTR_RW(gamepad_mode);
+
+static ssize_t gamepad_mode_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ ssize_t count = 0;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ continue;
+ count += sysfs_emit_at(buf, count, "%s ", claw_gamepad_mode_text[i]);
+ }
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(gamepad_mode_index);
+
+static ssize_t mkeys_function_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int i, ret = -EINVAL;
+ u8 data[2];
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++) {
+ if (claw_mkeys_function_text[i] && sysfs_streq(buf, claw_mkeys_function_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ data[0] = drvdata->gamepad_mode;
+ data[1] = ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t mkeys_function_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ i = drvdata->mkeys_function;
+
+ if (i >= ARRAY_SIZE(claw_mkeys_function_text))
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_mkeys_function_text[i]);
+}
+static DEVICE_ATTR_RW(mkeys_function);
+
+static ssize_t mkeys_function_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_mkeys_function_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(mkeys_function_index);
+
+static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ bool val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ if (!val)
+ return -EINVAL;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_RESET_DEVICE, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_WO(reset);
+
+static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
+ int n)
+{
+ struct hid_device *hdev = to_hid_device(kobj_to_dev(kobj));
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata) {
+ dev_warn(&hdev->dev,
+ "Failed to get drvdata from kobj. Gamepad attributes are not available.\n");
+ return 0;
+ }
+
+ return attr->mode;
+}
+
+static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_gamepad_mode.attr,
+ &dev_attr_gamepad_mode_index.attr,
+ &dev_attr_mkeys_function.attr,
+ &dev_attr_mkeys_function_index.attr,
+ &dev_attr_reset.attr,
+ NULL,
+};
+
+static const struct attribute_group claw_gamepad_attr_group = {
+ .attrs = claw_gamepad_attrs,
+ .is_visible = claw_gamepad_attr_is_visible,
+};
+
+static void cfg_setup_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_setup);
+ int ret;
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE,
+ NULL, 0, 25);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't read gamepad mode: %d\n", ret);
+ return;
+ }
+
+ /* Add sysfs attributes after we get the device state */
+ ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create gamepad attrs: %d\n", ret);
+ return;
+ }
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gamepad_registered, true);
+
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+}
+
+static void cfg_resume_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_resume);
+
+ guard(spinlock_irqsave)(&drvdata->registration_lock);
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
+}
+
+static int claw_probe(struct hid_device *hdev, u8 ep)
+{
+ struct claw_drvdata *drvdata;
+ int ret;
+
+ drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ drvdata->gamepad_mode = CLAW_GAMEPAD_MODE_XINPUT;
+ drvdata->hdev = hdev;
+ drvdata->ep = ep;
+
+ mutex_init(&drvdata->cfg_mutex);
+ spin_lock_init(&drvdata->registration_lock);
+ spin_lock_init(&drvdata->cmd_lock);
+ spin_lock_init(&drvdata->mode_lock);
+ init_completion(&drvdata->send_cmd_complete);
+ INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
+ INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+
+ /* For control interface: open the HID transport for sending commands. */
+ ret = hid_hw_open(hdev);
+ if (ret)
+ return ret;
+
+ hid_set_drvdata(hdev, drvdata);
+ schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
+
+ return 0;
+}
+
+static int msi_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+ u8 ep;
+
+ if (!hid_is_usb(hdev)) {
+ ret = -ENODEV;
+ goto err_probe;
+ }
+
+ ret = hid_parse(hdev);
+ if (ret)
+ goto err_probe;
+
+ /* Set quirk to create separate input devices per HID application */
+ hdev->quirks |= HID_QUIRK_INPUT_PER_APP | HID_QUIRK_MULTI_INPUT;
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ goto err_probe;
+
+ /* For non-control interfaces (keyboard/mouse), allow userspace to grab the devices. */
+ ret = get_endpoint_address(hdev);
+ if (ret < 0)
+ goto err_stop_hw;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN) {
+ ret = claw_probe(hdev, ep);
+ if (ret)
+ goto err_stop_hw;
+ }
+
+ return 0;
+
+err_stop_hw:
+ hid_hw_stop(hdev);
+err_probe:
+ return dev_err_probe(&hdev->dev, ret, "Failed to init device\n");
+}
+
+static void claw_remove(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata)
+ return;
+
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+
+ guard(spinlock_irqsave)(&drvdata->registration_lock);
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gamepad_registered, false);
+
+ hid_hw_close(hdev);
+}
+
+static void msi_remove(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ /* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ goto hw_stop;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ claw_remove(hdev);
+
+hw_stop:
+ hid_hw_stop(hdev);
+}
+
+static int claw_resume(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata)
+ return -ENODEV;
+
+ /* MCU can take up to 500ms to be ready after resume */
+ schedule_delayed_work(&drvdata->cfg_resume, msecs_to_jiffies(500));
+ return 0;
+}
+
+static int msi_resume(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ /* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ return 0;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ return claw_resume(hdev);
+
+ return 0;
+}
+
+static int claw_suspend(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata)
+ return -ENODEV;
+
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+
+ return 0;
+}
+
+static int msi_suspend(struct hid_device *hdev, pm_message_t msg)
+{
+ int ret;
+ u8 ep;
+
+ /* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ return 0;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ return claw_suspend(hdev);
+
+ return 0;
+}
+
+static const struct hid_device_id msi_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_XINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DESKTOP) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_BIOS) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, msi_devices);
+
+static struct hid_driver msi_driver = {
+ .name = "hid-msi",
+ .id_table = msi_devices,
+ .raw_event = msi_raw_event,
+ .probe = msi_probe,
+ .remove = msi_remove,
+ .resume = msi_resume,
+ .suspend = pm_ptr(msi_suspend),
+};
+module_hid_driver(msi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Denis Benato <denis.benato@linux.dev>");
+MODULE_AUTHOR("Zhouwang Huang <honjow311@gmail.com>");
+MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>");
+MODULE_DESCRIPTION("HID driver for MSI Claw Handheld PC gamepads");
--
2.53.0
^ permalink raw reply related
* [PATCH v8 0/4] Add MSI Claw HID Configuration Driver
From: Derek J. Clark @ 2026-05-22 1:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
This series adds an HID Configuration driver for the MSI Claw line of
Handheld Gaming PC's. The MSI Claw HID interface provides multiple
features, such as the ability to switch between xinput, dinput, and a
desktop mode, RGB control, rumble intensity, and mapping of the rear "M"
keys. There are additional gamepad modes that are not included in this
driver as they appear to be used in assembly line testing or are
incomplete in the firmware. During my testing I found them to be unstable.
The initial version of this driver was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes and additional features. Finally, I
refactored the entire driver, fixed multiple bugs, and refined the overall
format to conform to kernel driver best practices and style guide.
Claude was used initially by Zhouwang Huang to quickly parse HID captures
during the reverse-engineering of some of the features. Since Claude had
already been used, as a test of its capabilities I had it implement the
rumble intensity attribute after I had already rewritten most of the
driver, which I then manually edited to fix some mistakes. I also used
Claude to review the driver and these patches for any mistakes and bugs.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v7: https://lore.kernel.org/linux-input/20260520013158.3633277-1-derekjohn.clark@gmail.com/
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered and rgb_registered to avoid possible races during
teardown.
- Reorder reinit_completion in claw_hw_output_report to avoid race
with possible incoming ACKs.
- Reorder cancel_delayed_work_sync to ensure setup can't be re-armed
after cancel.
- Reset command state machine if hw_output_report has an error.
- Add comments to (hopefully) silence sashinko-bot warnings about the
use of endpoint matching and the impossible scenario of switching to
the alternate endpoint from userspace while the driver is bound.
- Don't use spinlock_irqsave when already in irq context.
- Add profile_lock for read/write profile_pending.
- Use struct for mkey reports and rumble reports, following the
pattern established by rgb reports previously.
- Add gating to cfg_setup_fn, allowing either gamepad settings or rgb
settings to populate if the other fails for any reason.
- Match on write address for rumble and mkey reports to prevent late
ACK from causing synchronization errors.
v6: https://lore.kernel.org/linux-input/20260518222935.1802071-1-derekjohn.clark@gmail.com/
- Add send/ack pattern to ensure synchronous acks.
- Use spinlock_irqsave instead of mutex for read/write MODE event
data.
- add select NEW_LEDS to kconfig.
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered or
rgb_registered, enabling use of devm_device_add_group and ending
the need to hold a mutex during remove.
- Don't set gamepad_mode on resume, MCU preserves state.
- Ensure all count variables are checked for > 0 characters before
setting buf - 1 to \n.
- Re-arm cfg_setup in resume if it was canceled in an early suspend.
- Remove duplicated argv_free macro.
- Add spinlock_irqsave vice mutex for read/write access on attribute
variables.
v5: https://lore.kernel.org/linux-input/20260517013925.3120314-1-derekjohn.clark@gmail.com/
- Swap disabled & combination mkeys_function enum values.
- Fix bug introduced in v5 where claw_buttons_store would return
-EINVAL on all valid key entries.
- Ensure mode_mutex is properly init.
- Ensure claw_remove is calling hid_hw_close and not hid_hw_stop for
all paths.
- Ensure adding "DISABLED" key to valid entries is done in the correct
patch.
- Re-enable sending an empty string to clear button mappings in
addition to setting DISABLED.
- Move adding the RGB device into cfg_setup to prevent led core
attributes from being written to prior to setup completing.
- Ensure frame_lock is properly init.
- Change variable names in RGB functions from frame and zone to f and
z respectively to fit all scoped_guard actions in 100 columns.
v4: https://lore.kernel.org/linux-input/20260516042841.500299-1-derekjohn.clark@gmail.com/
- Add msi_suspend/claw_suspend.
- Reorder claw_remove to cancel all work before removing sysfs.
- Add mutex lock for removing sysfs attributes.
- Add mutex lock for MODE command data read/write.
- Change dev_warn to dev_dbg in claw_profile_event.
- use __free with DEFINE_FREE macro for argv instead of manually
running argv_free, cleaining up scoped_guard goto.
- Fix frame_calc validity check to use >=.
- Use spinlock instead of mutex in raw_event and related attribute
_store function.
- Ensure delayed work is canceled in suspend & canceled before sysfs
attribute removal.
v3: https://lore.kernel.org/linux-input/20260515033622.2095277-1-derekjohn.clark@gmail.com/
- Add mutex for read/write if rgb frame data.
- Ensure claw_hw_output_report is properly guarded.
- Remove setting rgb_frame_count when reading rgb profiles as it always
returns garbage data.
- Ensure rgb_speed is getting drvdata from a valid lookup (not hdev).
- Use scoped_guard where necessary.
- Reoder claw_probe to ensure all mutex, completion, and variable
assignments are in place prior to setting drvdata.
- Ensure gamepad_mode is set to a valid enum value in claw_probe.
v2: https://lore.kernel.org/linux-input/20260513231445.3213501-1-derekjohn.clark@gmail.com/
- Use mutexes to guard SYNC_TO_ROM calls and pending_profile calls.
- Rename driver to hid-msi and add generic entrypoints for
probe/resume/remove that call claw specific functions in order to
future proof the driver for other MSI HID interfaces.
- Fix various bugs and formatting issues.
v1: https://lore.kernel.org/linux-input/20260510043510.442807-1-derekjohn.clark@gmail.com/
Derek J. Clark (4):
HID: hid-msi: Add MSI Claw configuration driver
HID: hid-msi: Add M-key mapping attributes
HID: hid-msi: Add RGB control interface
HID: hid-msi: Add Rumble Intensity Attributes
MAINTAINERS | 6 +
drivers/hid/Kconfig | 13 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 5 +
drivers/hid/hid-msi.c | 1980 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 2005 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
--
2.53.0
^ permalink raw reply
* Re: [syzbot] [input?] possible deadlock in tasklet_action_common (2)
From: Hillf Danton @ 2026-05-21 22:35 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: syzbot, dmitry.torokhov, linux-input, Tetsuo Handa, linux-kernel,
syzkaller-bugs
In-Reply-To: <20260521143414.P7XZgsrN@linutronix.de>
On Thu, 21 May 2026 16:34:14 +0200 Sebastian Andrzej Siewior wrote:
>On 2026-05-21 09:52:32 [+0800], Hillf Danton wrote:
>> > Date: Wed, 20 May 2026 10:05:38 -0700 [thread overview]
>> > Hello,
>> >
>> > syzbot found the following issue on:
>> >
>> > HEAD commit: e98d21c170b0 Add linux-next specific files for 20260508
>> > git tree: linux-next
>> > console output: https://syzkaller.appspot.com/x/log.txt?x=1354bfce580000
>> > kernel config: https://syzkaller.appspot.com/x/.config?x=59b98218d9b2edf4
>> > dashboard link: https://syzkaller.appspot.com/bug?extid=b5d7ab56d43de3fd5aac
>> > compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
>> >
>> > Unfortunately, I don't have any reproducer for this issue yet.
>> >
>> > Downloadable assets:
>> > disk image: https://storage.googleapis.com/syzbot-assets/66f2a00ee290/disk-e98d21c1.raw.xz
>> > vmlinux: https://storage.googleapis.com/syzbot-assets/6b982257ce9e/vmlinux-e98d21c1.xz
>> > kernel image: https://storage.googleapis.com/syzbot-assets/a73fbea43e1a/bzImage-e98d21c1.xz
>> >
>> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
>> > Reported-by: syzbot+b5d7ab56d43de3fd5aac@syzkaller.appspotmail.com
>> >
>> > ======================================================
>> > WARNING: possible circular locking dependency detected
>> > syzkaller #0 Tainted: G L
>> > ------------------------------------------------------
>> > syz.2.4328/32663 is trying to acquire lock:
>> > ffff8880b8724168 (tasklet_sync_callback.cb_lock){+...}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:45 [inline]
>> > ffff8880b8724168 (tasklet_sync_callback.cb_lock){+...}-{3:3}, at: tasklet_lock_callback kernel/softirq.c:881 [inline]
>> > ffff8880b8724168 (tasklet_sync_callback.cb_lock){+...}-{3:3}, at: tasklet_action_common+0xc5/0x610 kernel/softirq.c:931
>> >
>> > but task is already holding lock:
>> > ffff888058598270 (&dev->event_lock#2){+.+.}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:45 [inline]
>> > ffff888058598270 (&dev->event_lock#2){+.+.}-{3:3}, at: class_spinlock_irqsave_constructor include/linux/spinlock.h:619 [inline]
>> > ffff888058598270 (&dev->event_lock#2){+.+.}-{3:3}, at: input_inject_event+0xa4/0x320 drivers/input/input.c:419
>> >
>> > which lock already depends on the new lock.
>> >
>> >
>> > the existing dependency chain (in reverse order) is:
>> >
>> > -> #1 (&dev->event_lock#2){+.+.}-{3:3}:
>> > rt_spin_lock+0x83/0x400 kernel/locking/spinlock_rt.c:56
>> > spin_lock include/linux/spinlock_rt.h:45 [inline]
>> > class_spinlock_irqsave_constructor include/linux/spinlock.h:619 [inline]
>> > input_inject_event+0xa4/0x320 drivers/input/input.c:419
>> > led_trigger_event+0x13b/0x220 drivers/leds/led-triggers.c:420
>> > kbd_propagate_led_state drivers/tty/vt/keyboard.c:1118 [inline]
>> > kbd_bh+0x1b4/0x2c0 drivers/tty/vt/keyboard.c:1297
>> > tasklet_action_common+0x31c/0x610 kernel/softirq.c:942
>> > handle_softirqs+0x1de/0x6d0 kernel/softirq.c:626
>> > __do_softirq kernel/softirq.c:660 [inline]
>> > run_ktimerd+0x69/0x100 kernel/softirq.c:1155
>> > smpboot_thread_fn+0x541/0xa50 kernel/smpboot.c:160
>> > kthread+0x388/0x470 kernel/kthread.c:436
>> > ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
>> > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
>
> This is obvious and looks fine.
>
>> Deadlock if the timer thread is preempted
>>
>> timer thread
>> tasklet_sync_callback.cb_lock
>>
>> // preempted
>> &dev->event_lock
>> do softirq
>> acquire tasklet_sync_callback.cb_lock
>>
>> acquire &dev->event_lock
>
> This shouldn't happen.
>
>> > -> #0 (tasklet_sync_callback.cb_lock){+...}-{3:3}:
>> > check_prev_add kernel/locking/lockdep.c:3167 [inline]
>> > check_prevs_add kernel/locking/lockdep.c:3286 [inline]
>> > validate_chain kernel/locking/lockdep.c:3910 [inline]
>> > __lock_acquire+0x15a5/0x2d10 kernel/locking/lockdep.c:5239
>> > lock_acquire+0x106/0x350 kernel/locking/lockdep.c:5870
>> > rt_spin_lock+0x83/0x400 kernel/locking/spinlock_rt.c:56
>> > spin_lock include/linux/spinlock_rt.h:45 [inline]
>> > tasklet_lock_callback kernel/softirq.c:881 [inline]
>> > tasklet_action_common+0xc5/0x610 kernel/softirq.c:931
>> > handle_softirqs+0x1de/0x6d0 kernel/softirq.c:626
>> > __do_softirq kernel/softirq.c:660 [inline]
>> > __local_bh_enable_ip+0x170/0x2b0 kernel/softirq.c:302
>> > local_bh_enable include/linux/bottom_half.h:33 [inline]
>> > __alloc_skb+0x1aa/0x7d0 net/core/skbuff.c:697
>> > alloc_skb include/linux/skbuff.h:1381 [inline]
>> > hidp_send_message+0xb5/0x230 net/bluetooth/hidp/core.c:111
>> > hidp_send_intr_message net/bluetooth/hidp/core.c:143 [inline]
>> > hidp_input_event+0x2a4/0x380 net/bluetooth/hidp/core.c:175
>> > input_event_dispose+0x80/0x6b0 drivers/input/input.c:322
>> > input_inject_event+0x1d7/0x320 drivers/input/input.c:424
>
> up to kbd_led_trigger_activate(), I can follow. In
> kbd_led_trigger_activate() it disables the keyboard tasklet and then
> does led_set_brightness(). Not sure how it ends up in
> input_inject_event().
>
input_inject_event() is invoked in the brightness_set callback [1] for
example.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/input/input-leds.c#n142
> Now, input_inject_event() does spin_lock_irqsave() and alloc_skb() does
> local_bh_disable()/ local_bh_enable(). On !RT this is not legal. On RT
Can you please specify why that is illegal on !RT?
> it is okay but then local_bh_enable() here should not invoke any
> softirqs because none were raised within the section (alloc_skb()).
On RT spinlock is replaced with mutex, and softirq can be raised in the
irq that could come any moment after spin_lock_irqsave().
> Not sure if this really did occur as such or lockdep comes with the
> shortest backtrace based on the lockclass and this is it.
>
>> > kbd_led_trigger_activate+0xbc/0x100 drivers/tty/vt/keyboard.c:1074
>> > led_trigger_set+0x53b/0x960 drivers/leds/led-triggers.c:220
>> > led_match_default_trigger drivers/leds/led-triggers.c:277 [inline]
>> > led_trigger_set_default+0x266/0x2a0 drivers/leds/led-triggers.c:300
>> > led_classdev_register_ext+0x787/0x9c0 drivers/leds/led-class.c:581
>> > led_classdev_register include/linux/leds.h:274 [inline]
>> > input_leds_connect+0x517/0x790 drivers/input/input-leds.c:145
>> > input_attach_handler drivers/input/input.c:1011 [inline]
>> > input_register_device+0xce0/0x1140 drivers/input/input.c:2395
>> > hidp_session_dev_add net/bluetooth/hidp/core.c:861 [inline]
>> > hidp_session_probe+0x1a5/0x8a0 net/bluetooth/hidp/core.c:1139
>> > l2cap_register_user+0xc2/0x1d0 net/bluetooth/l2cap_core.c:1725
>> > hidp_connection_add+0x158b/0x1a20 net/bluetooth/hidp/core.c:1411
>> > do_hidp_sock_ioctl net/bluetooth/hidp/sock.c:81 [inline]
>> > hidp_sock_ioctl+0x403/0x650 net/bluetooth/hidp/sock.c:128
>> > sock_do_ioctl+0x101/0x320 net/socket.c:1328
>> > sock_ioctl+0x5c9/0x7f0 net/socket.c:1449
>> > vfs_ioctl fs/ioctl.c:51 [inline]
>> > __do_sys_ioctl fs/ioctl.c:597 [inline]
>> > __se_sys_ioctl+0xff/0x170 fs/ioctl.c:583
>> > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>> > do_syscall_64+0x15f/0xf80 arch/x86/entry/syscall_64.c:94
>
>Sebastian
^ permalink raw reply
* Re: [PATCH v3] HID: Expose LattePanda IOTA UPS as a power_supply device
From: Andrew Maney @ 2026-05-21 20:22 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: jikos, linux-kernel, linux-input
In-Reply-To: <ag6qqUZKiTlfU4Rb@beelink>
On Thu, May 21, 2026 at 2:54 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> On May 20 2026, Andrew Maney wrote:
> > This driver exposes the DFRobot LattePanda IOTA UPS board as a standard
> > power_supply device, allowing desktop environments and power management
> > tools such as UPower and systemd-logind to display battery status,
> > remaining capacity, and charging status without any special
> > configuration. It also enables automatic suspend or shutdown on low
> > battery and power profile configuration via any tool that supports the
> > standard power_supply interface.
> >
> > The UPS presents itself as an Arduino Leonardo HID device running custom
> > firmware (VID 0x2341, PID 0x8036). It reports status and capacity via
> > HID reports 0x07 and 0x0C respectively.
>
> As sashiko detected, you are using a standard Arduino Leonardo and not
> making any specific detections. So I was thinking that maybe we should
> implement that feature as a HID-BPF program on top of the generic HID
> handling.
>
> However, the handling of battery supplies in the HID generic core is not
> entirely filling all of the requirements here, but that's something I
> wanted to do for a couple of month but I have been swamped with other
> projects.
>
> Anyway, I wanted to understand why this product was using a generic
> Leonardo PID, and: https://wiki.dfrobot.com/dfr1247#tech_specs
>
> "it leverages the standard HID-UPS protocol to be natively recognized by
> Windows as a battery-powered device"
>
> So. Instead of working on a custom driver, why not simply implement (or
> finish the generic implementation) of HID-UPS in hid-input.c?
>
> Cheers,
> Benjamin
>
Hi Benjamin,
Thank you for the feedback! I'll look into the HID-UPS implementation or a
HID-BPF program.
I should mention that the kernel does already detect the IOTA UPS via the
generic HID-UPS support, but the updates that it receives are so slow that
it becomes practically unusable for a device with smaller capacity batteries
like this. Changes can take several minutes or more to be detected. I
originally went with a driver that reads the HID reports directly because it
was providing updates far faster.
I implemented a check in driver v4 that verifies that the device's HID reports
are what's expected before binding to it. In theory, that should prevent the
driver from binding to other Arduino Leonardo devices, but I agree that fixing
the generic HID-UPS implementation would be a better long-term solution.
Which approach do you think would be more suitable for this, completing the
HID-UPS implementation or a HID-BPF program? I'm happy to work on either,
though as a first-time kernel contributor I'd appreciate any pointers
on where to
start.
Thanks,
Andrew
> >
> > The charge limit (80% or 100%) is configured via a physical DIP switch
> > on the UPS board and cannot be detected automatically. Userspace can
> > inform the driver of the configured limit via
> > charge_control_end_threshold.
> >
> > ---
> >
> > Changes in v3:
> > - Deferred power_supply registration to workqueue to avoid blocking probe
> > - Fixed kernel panic when instantiated via uhid by checking hid_is_usb()
> > before dereferencing USB-specific structures
> >
> > - Fixed ERR_PTR dereference in raw_event by only assigning ups->psu on
> > successful registration
> >
> > - Fixed data race on ups->charge_limit using spin_lock_irqsave()
> > - Removed TIME_TO_EMPTY_NOW and TIME_TO_FULL_NOW properties to avoid
> > spurious shutdowns
> >
> > - Changed plugged-in but not charging state from FULL to NOT_CHARGING
> > - Used devm_kasprintf() for a unique sysfs name in order to support
> > multiple devices
> >
> > - Added POWER_SUPPLY and HIDRAW dependencies to Kconfig
> > - Used %pe for more human-readable error messages
> >
> > Changes in v2:
> > - Rebased on top of the current tree
> > - Moved vendor and device IDs to drivers/hid/hid-ids.h
> > - Added Kconfig entry under HID bus support -> Special HID drivers
> > - Added build rule to drivers/hid/Makefile
> >
> > Signed-off-by: Andrew Maney <andrewmaney05@gmail.com>
> > ---
> > MAINTAINERS | 6 +
> > drivers/hid/Kconfig | 10 +
> > drivers/hid/Makefile | 1 +
> > drivers/hid/hid-ids.h | 3 +
> > drivers/hid/hid-lattepanda-iota-ups.c | 409 ++++++++++++++++++++++++++
> > 5 files changed, 429 insertions(+)
> > create mode 100644 drivers/hid/hid-lattepanda-iota-ups.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 10e825318..d80721c2c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11416,6 +11416,12 @@ F: include/uapi/linux/hid*
> > F: samples/hid/
> > F: tools/testing/selftests/hid/
> >
> > +HID LATTEPANDA IOTA UPS DRIVER
> > +M: Andrew Maney <andrewmaney05@gmail.com>
> > +L: linux-input@vger.kernel.org
> > +S: Maintained
> > +F: drivers/hid/hid-lattepanda-iota-ups.c
> > +
> > HID LOGITECH DRIVERS
> > R: Filipe Laíns <lains@riseup.net>
> > L: linux-input@vger.kernel.org
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index ff2f580b6..21ffc2fd0 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -510,6 +510,16 @@ config HID_KYSONA
> > Say Y here if you have a Kysona M600 mouse
> > and want to be able to read its battery capacity.
> >
> > +config HID_LATTEPANDA_IOTA_UPS
> > + tristate "LattePanda IOTA UPS"
> > + depends on USB_HID && USB_HIDDEV && X86 && POWER_SUPPLY
> > + help
> > + Support for the LattePanda IOTA UPS (DFRobot, VID 0x2341 PID 0x8036).
> > + Exposes the battery status and capacity via the power_supply interface.
> > +
> > + To compile as a module, choose M here: the module will be
> > + called hid-lattepanda-iota-ups.
> > +
> > config HID_UCLOGIC
> > tristate "UC-Logic"
> > depends on USB_HID
> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > index 0597fd6a4..d7ad3fc8f 100644
> > --- a/drivers/hid/Makefile
> > +++ b/drivers/hid/Makefile
> > @@ -74,6 +74,7 @@ obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o
> > obj-$(CONFIG_HID_KEYTOUCH) += hid-keytouch.o
> > obj-$(CONFIG_HID_KYE) += hid-kye.o
> > obj-$(CONFIG_HID_KYSONA) += hid-kysona.o
> > +obj-$(CONFIG_HID_LATTEPANDA_IOTA_UPS) += hid-lattepanda-iota-ups.o
> > obj-$(CONFIG_HID_LCPOWER) += hid-lcpower.o
> > obj-$(CONFIG_HID_LENOVO) += hid-lenovo.o
> > obj-$(CONFIG_HID_LENOVO_GO) += hid-lenovo-go.o
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index 4657d96fb..6ded2c943 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -859,6 +859,9 @@
> > #define USB_DEVICE_ID_LD_HYBRID 0x2090
> > #define USB_DEVICE_ID_LD_HEATCONTROL 0x20A0
> >
> > +#define USB_VENDOR_ID_LATTEPANDA_IOTA 0x2341
> > +#define USB_DEVICE_ID_LATTEPANDA_IOTA_UPS 0x8036
> > +
> > #define USB_VENDOR_ID_LENOVO 0x17ef
> > #define USB_DEVICE_ID_LENOVO_TPKBD 0x6009
> > #define USB_DEVICE_ID_LENOVO_CUSBKBD 0x6047
> > diff --git a/drivers/hid/hid-lattepanda-iota-ups.c b/drivers/hid/hid-lattepanda-iota-ups.c
> > new file mode 100644
> > index 000000000..f5d522695
> > --- /dev/null
> > +++ b/drivers/hid/hid-lattepanda-iota-ups.c
> > @@ -0,0 +1,409 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/power_supply.h>
> > +#include <linux/completion.h>
> > +#include <linux/workqueue.h>
> > +#include <linux/module.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/hid.h>
> > +#include <linux/usb.h>
> > +#include "hid-ids.h"
> > +
> > +#define REPORT_ID_CAPACITY 0x0C
> > +#define REPORT_ID_STATUS 0x07
> > +
> > +#define STATUS_DISCHARGING BIT(1)
> > +#define STATUS_PLUGGED_IN BIT(0)
> > +#define STATUS_CHARGING BIT(2)
> > +
> > +MODULE_AUTHOR("Andrew Maney");
> > +MODULE_DESCRIPTION("LattePanda IOTA UPS power supply driver");
> > +MODULE_LICENSE("GPL");
> > +
> > +struct iota_ups {
> > + struct power_supply_desc psu_desc;
> > + struct power_supply *psu;
> > + struct hid_device *hiddev;
> > + spinlock_t lock; /* Protects cached HID report values */
> > +
> > + /* Cached values that are updated from HID reports */
> > + bool plugged_in;
> > + char serial[64];
> > + int charge_limit;
> > + int psu_status;
> > + int capacity;
> > +
> > + /*
> > + * Wait for both status and capacity reports before registering
> > + * with the power_supply core, so initial values are correct and
> > + * not erroneous.
> > + */
> > + struct completion got_initial_data;
> > + struct work_struct register_work;
> > + bool got_capacity;
> > + bool data_ready;
> > + bool got_status;
> > +};
> > +
> > +static enum power_supply_property iota_ups_properties[] = {
> > + POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> > + POWER_SUPPLY_PROP_SERIAL_NUMBER,
> > + POWER_SUPPLY_PROP_MANUFACTURER,
> > + POWER_SUPPLY_PROP_MODEL_NAME,
> > + POWER_SUPPLY_PROP_TECHNOLOGY,
> > + POWER_SUPPLY_PROP_CAPACITY,
> > + POWER_SUPPLY_PROP_PRESENT,
> > + POWER_SUPPLY_PROP_ONLINE,
> > + POWER_SUPPLY_PROP_STATUS,
> > + POWER_SUPPLY_PROP_SCOPE,
> > +};
> > +
> > +static const struct hid_device_id iota_ups_devices[] = {
> > + { HID_USB_DEVICE(USB_VENDOR_ID_LATTEPANDA_IOTA,
> > + USB_DEVICE_ID_LATTEPANDA_IOTA_UPS) },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(hid, iota_ups_devices);
> > +
> > +static int iota_ups_get_property(struct power_supply *supply,
> > + enum power_supply_property psp,
> > + union power_supply_propval *val)
> > +{
> > + struct iota_ups *ups = power_supply_get_drvdata(supply);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&ups->lock, flags);
> > +
> > + switch (psp) {
> > + case POWER_SUPPLY_PROP_STATUS:
> > + val->intval = ups->psu_status;
> > + break;
> > +
> > + /* Remaining capacity as a percentage from 0 to 100 */
> > + case POWER_SUPPLY_PROP_CAPACITY:
> > + val->intval = ups->capacity;
> > + break;
> > +
> > + /* The UPS is always present if the driver is loaded */
> > + case POWER_SUPPLY_PROP_PRESENT:
> > + val->intval = 1;
> > + break;
> > +
> > + /* Whether mains power is connected */
> > + case POWER_SUPPLY_PROP_ONLINE:
> > + val->intval = ups->plugged_in ? 1 : 0;
> > + break;
> > +
> > + /*
> > + * The UPS board supplies power to the IOTA and any
> > + * peripherals connected to it, therefore its scope
> > + * is system-wide.
> > + */
> > + case POWER_SUPPLY_PROP_SCOPE:
> > + val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
> > + break;
> > +
> > + /* V1.0 only accepts 18650 Li-ion cells */
> > + case POWER_SUPPLY_PROP_TECHNOLOGY:
> > + val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> > + break;
> > +
> > + /* 80% or 100%, configured via a DIP switch on the UPS board */
> > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> > + val->intval = ups->charge_limit;
> > + break;
> > +
> > + case POWER_SUPPLY_PROP_MANUFACTURER:
> > + val->strval = "DFRobot";
> > + break;
> > +
> > + case POWER_SUPPLY_PROP_MODEL_NAME:
> > + val->strval = "LattePanda IOTA UPS";
> > + break;
> > +
> > + /* Retrieved from the USB descriptor */
> > + case POWER_SUPPLY_PROP_SERIAL_NUMBER:
> > + val->strval = ups->serial;
> > + break;
> > +
> > + default:
> > + spin_unlock_irqrestore(&ups->lock, flags);
> > + return -EINVAL;
> > + }
> > +
> > + spin_unlock_irqrestore(&ups->lock, flags);
> > + return 0;
> > +}
> > +
> > +static int iota_ups_set_property(struct power_supply *supply,
> > + enum power_supply_property psp,
> > + const union power_supply_propval *val)
> > +{
> > + struct iota_ups *ups = power_supply_get_drvdata(supply);
> > +
> > + if (psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD) {
> > + unsigned long flags;
> > +
> > + /*
> > + * V1.0 supports 80% and 100% charge limits only, which is
> > + * set via a DIP switch on the board. This property allows
> > + * userspace to inform the driver which limit is configured.
> > + */
> > + if (val->intval != 80 && val->intval != 100)
> > + return -EINVAL;
> > +
> > + spin_lock_irqsave(&ups->lock, flags);
> > + ups->charge_limit = val->intval;
> > + spin_unlock_irqrestore(&ups->lock, flags);
> > + return 0;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int iota_ups_property_is_writable(struct power_supply *supply,
> > + enum power_supply_property psp)
> > +{
> > + return psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD;
> > +}
> > +
> > +static int iota_ups_raw_event(struct hid_device *hdev,
> > + struct hid_report *report,
> > + u8 *data, int size)
> > +{
> > + struct iota_ups *ups = hid_get_drvdata(hdev);
> > + unsigned long flags;
> > + bool changed = false;
> > +
> > + /* All of the UPS's reports are at least 2 bytes */
> > + if (size < 2)
> > + return 0;
> > +
> > + spin_lock_irqsave(&ups->lock, flags);
> > +
> > + switch (data[0]) {
> > + case REPORT_ID_STATUS: {
> > + u8 status = data[1];
> > + int new_status;
> > + bool plugged_in = !!(status & STATUS_PLUGGED_IN);
> > +
> > + /*
> > + * The UPS status is determined as follows:
> > + * Battery full:
> > + * UPS is plugged in
> > + * Battery is at full capacity
> > + *
> > + * Battery charging:
> > + * UPS is plugged in
> > + * Battery is not at full capacity
> > + *
> > + * Battery discharging:
> > + * UPS is not plugged in
> > + *
> > + * Battery not charging:
> > + * UPS is plugged in
> > + * UPS has halted charging for some reason
> > + *
> > + * Unknown:
> > + * None of the above conditions are met
> > + */
> > + if (status & STATUS_CHARGING) {
> > + if (ups->capacity >= ups->charge_limit)
> > + new_status = POWER_SUPPLY_STATUS_FULL;
> > + else
> > + new_status = POWER_SUPPLY_STATUS_CHARGING;
> > +
> > + } else if (status & STATUS_DISCHARGING) {
> > + new_status = POWER_SUPPLY_STATUS_DISCHARGING;
> > +
> > + } else if (plugged_in) {
> > + new_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > +
> > + } else {
> > + new_status = POWER_SUPPLY_STATUS_UNKNOWN;
> > + }
> > +
> > + if (new_status != ups->psu_status ||
> > + plugged_in != ups->plugged_in) {
> > + ups->plugged_in = plugged_in;
> > + ups->psu_status = new_status;
> > + changed = true;
> > + }
> > +
> > + ups->got_status = true;
> > + break;
> > + }
> > +
> > + case REPORT_ID_CAPACITY: {
> > + int new_cap = clamp((int)data[1], 0, 100);
> > +
> > + if (new_cap != ups->capacity) {
> > + ups->capacity = new_cap;
> > + changed = true;
> > + }
> > +
> > + ups->got_capacity = true;
> > + break;
> > + }
> > + }
> > +
> > + /*
> > + * Signal that the UPS is ready to be registered because we have
> > + * received both capacity and status reports.
> > + */
> > + if (!ups->data_ready && ups->got_status && ups->got_capacity) {
> > + ups->data_ready = true;
> > + complete(&ups->got_initial_data);
> > + }
> > +
> > + spin_unlock_irqrestore(&ups->lock, flags);
> > +
> > + /*
> > + * Notify the power_supply core outside the spinlock to avoid
> > + * a deadlock; power_supply_changed() may call back into
> > + * get_property() which acquires the same lock.
> > + */
> > + if (changed && ups->psu)
> > + power_supply_changed(ups->psu);
> > +
> > + return 0;
> > +}
> > +
> > +static void iota_ups_register_work(struct work_struct *work)
> > +{
> > + struct iota_ups *ups = container_of(work, struct iota_ups, register_work);
> > + struct power_supply_config psu_config = {};
> > + struct power_supply *psu;
> > +
> > + /*
> > + * Wait for both status and capacity reports before registering.
> > + * The device sends reports every ~1 second, so 3 seconds is safe.
> > + * We wait here in order to prevent registration in an unknown
> > + * state, since this could cause emergency shutdowns or other
> > + * undesired effects.
> > + */
> > + wait_for_completion_timeout(&ups->got_initial_data,
> > + msecs_to_jiffies(3000));
> > +
> > + /* Configure the UPS's power supply properties */
> > + ups->psu_desc.name = devm_kasprintf(&ups->hiddev->dev, GFP_KERNEL,
> > + "lattepanda-iota-ups.%s",
> > + dev_name(&ups->hiddev->dev));
> > +
> > + if (!ups->psu_desc.name) {
> > + hid_err(ups->hiddev, "failed to allocate power supply name\n");
> > + return;
> > + }
> > +
> > + ups->psu_desc.property_is_writeable = iota_ups_property_is_writable;
> > + ups->psu_desc.num_properties = ARRAY_SIZE(iota_ups_properties);
> > + ups->psu_desc.get_property = iota_ups_get_property;
> > + ups->psu_desc.set_property = iota_ups_set_property;
> > + ups->psu_desc.properties = iota_ups_properties;
> > + ups->psu_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> > + psu_config.drv_data = ups;
> > +
> > + /* Register the UPS as a power_supply device */
> > + psu = devm_power_supply_register(&ups->hiddev->dev, &ups->psu_desc, &psu_config);
> > + if (IS_ERR(psu)) {
> > + hid_err(ups->hiddev, "power supply registration failed: %pe\n", psu);
> > + return;
> > + }
> > +
> > + /*
> > + * Finally, notify the power_supply core so userspace reads the correct
> > + * initial state immediately after registration.
> > + */
> > + ups->psu = psu;
> > + power_supply_changed(ups->psu);
> > + hid_info(ups->hiddev, "LattePanda IOTA UPS registered as a power_supply device\n");
> > +}
> > +
> > +static int iota_ups_probe(struct hid_device *hdev,
> > + const struct hid_device_id *id)
> > +{
> > + struct iota_ups *ups;
> > + int ret;
> > +
> > + ups = devm_kzalloc(&hdev->dev, sizeof(*ups), GFP_KERNEL);
> > + if (!ups)
> > + return -ENOMEM;
> > +
> > + ups->hiddev = hdev;
> > + ups->psu_status = POWER_SUPPLY_STATUS_UNKNOWN;
> > +
> > + /* 50% is a safe default if wait_for_completion_timeout() times out. */
> > + ups->capacity = 50;
> > +
> > + /*
> > + * Default to 100% to prevent unexpected shutdowns.
> > + * Userspace can update this via charge_control_end_threshold.
> > + */
> > + ups->charge_limit = 100;
> > +
> > + init_completion(&ups->got_initial_data);
> > + spin_lock_init(&ups->lock);
> > + hid_set_drvdata(hdev, ups);
> > +
> > + /*
> > + * Retrieve the UPS's serial number from the USB descriptor. If the device is not
> > + * a USB device, we can use the unique device identifier as the serial number.
> > + */
> > + if (hid_is_usb(hdev)) {
> > + struct usb_device *udev = to_usb_device(hdev->dev.parent->parent);
> > +
> > + if (udev->serial)
> > + strscpy(ups->serial, udev->serial, sizeof(ups->serial));
> > + else
> > + strscpy(ups->serial, "Unknown", sizeof(ups->serial));
> > + } else {
> > + if (*hdev->uniq)
> > + strscpy(ups->serial, hdev->uniq, sizeof(ups->serial));
> > + else
> > + strscpy(ups->serial, "Unknown", sizeof(ups->serial));
> > + }
> > +
> > + ret = hid_parse(hdev);
> > + if (ret) {
> > + hid_err(hdev, "HID parse failed: %pe\n", ERR_PTR(ret));
> > + return ret;
> > + }
> > +
> > + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> > + if (ret) {
> > + hid_err(hdev, "HID hw start failed: %pe\n", ERR_PTR(ret));
> > + return ret;
> > + }
> > +
> > + ret = hid_hw_open(hdev);
> > + if (ret) {
> > + hid_err(hdev, "HID hw open failed: %pe\n", ERR_PTR(ret));
> > + goto err_stop;
> > + }
> > +
> > + /* Probe for the UPS in a worker queue so we don't halt the enumeration thread */
> > + INIT_WORK(&ups->register_work, iota_ups_register_work);
> > + schedule_work(&ups->register_work);
> > + return 0;
> > +
> > +err_stop:
> > + hid_hw_stop(hdev);
> > + return ret;
> > +}
> > +
> > +static void iota_ups_remove(struct hid_device *hdev)
> > +{
> > + struct iota_ups *ups = hid_get_drvdata(hdev);
> > +
> > + cancel_work_sync(&ups->register_work);
> > + hid_hw_close(hdev);
> > + hid_hw_stop(hdev);
> > +}
> > +
> > +static struct hid_driver iota_ups_driver = {
> > + .name = "lattepanda-iota-ups",
> > + .id_table = iota_ups_devices,
> > + .probe = iota_ups_probe,
> > + .remove = iota_ups_remove,
> > + .raw_event = iota_ups_raw_event,
> > +};
> > +module_hid_driver(iota_ups_driver);
> > --
> > 2.54.0
> >
> >
^ permalink raw reply
* Re: [PATCH 1/1] HID: wacom: Fix OOB write in wacom_hid_set_device_mode()
From: Lee Jones @ 2026-05-21 16:22 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Ping Cheng, Ping Cheng, Jason Gerecke, Jiri Kosina, linux-input,
linux-kernel, stable
In-Reply-To: <ag8ozWBDSDckicSS@beelink>
On Thu, 21 May 2026, Benjamin Tissoires wrote:
> On May 19 2026, Lee Jones wrote:
> > On Wed, 13 May 2026, Ping Cheng wrote:
> >
> > > On Wed, May 13, 2026 at 1:05 AM Lee Jones <lee@kernel.org> wrote:
> > > >
> > > > wacom_hid_set_device_mode() currently assumes that the HID_DG_INPUTMODE
> > > > usage is always located in the first field (field[0]) of the feature report.
> > > > However, a device can specify HID_DG_INPUTMODE in a different field.
> > > >
> > > > If HID_DG_INPUTMODE is in a field other than the first one and the first
> > > > field has a report_count smaller than the usage_index of HID_DG_INPUTMODE,
> > > > this leads to an out-of-bounds write to r->field[0]->value.
> > > >
> > > > Fix this by storing the field index of HID_DG_INPUTMODE in 'struct
> > > > hid_data' during feature mapping. In wacom_hid_set_device_mode(), use
> > > > this stored field index to access the correct field and add bounds
> > > > checks to ensure both the field index and the value index are within
> > > > valid ranges before writing.
> > > >
> > > > Cc: stable@vger.kernel.org
> > > > Fixes: 5ae6e89f7409 ("HID: wacom: implement the finger part of the HID generic handling")
> > > > Signed-off-by: Lee Jones <lee@kernel.org>
> > >
> > > Patch looks sensible to me. Thank you for your effort, Lee!
> > >
> > > Tested-by: Ping Cheng <ping.cheng@wacom.com>
> > > Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
> >
> > Thank you Ping, I appreciate your review.
> >
> > HID folks - any movement on this please?
> >
>
> I wanted to apply it today, but the patch conflicts with our current
> for-7.1/upstream-fixes.
>
> Could you rebase on top of this branch so we can take this without me
> messing with your patch?
Sure. Leave it with me. Probably be early next week.
--
Lee Jones
^ permalink raw reply
* Re: [PATCH] HID: multitouch: Fix stale MT slots when contact count drops to zero
From: Benjamin Tissoires @ 2026-05-21 15:52 UTC (permalink / raw)
To: Dave Carey; +Cc: linux-input, jikos
In-Reply-To: <CALPvROT0GE24qUOpjZdEd6FiSfMFVWjbwmS=9CiP7NiF+ZQGSA@mail.gmail.com>
On Thu, May 21, 2026 at 5:03 PM Dave Carey <carvsdriver@gmail.com> wrote:
>
> Benjamin - would you like me to make that update and test? I don't see an immediate problem with it otherwise.
Yes, please test and send as a v2.
Cheers,
Benjamin
>
> On Thu, May 21, 2026 at 9:17 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>>
>> On May 15 2026, Dave Carey wrote:
>> > The INGENIC 17EF:6161 touchscreen (Lenovo Yoga Book 9 14IAH10) reports
>> > HID_DG_CONTACTCOUNT=0 in the frame immediately following the last finger
>> > lift rather than omitting the frame entirely. In mt_touch_report() the
>> > existing code only updates num_expected when contact_count is non-zero,
>> > so a zero contact count on the first packet of a new frame leaves
>> > num_expected at its previous value (e.g. 2 for a two-finger gesture).
>> > The sync check "num_received >= num_expected" then evaluates "0 >= 2"
>> > and never fires, preventing INPUT_MT_DROP_UNUSED from releasing the
>> > stale slots. Those slots remain active in the kernel MT layer until the
>> > next touch, at which point they are released in a batch alongside the
>> > new contact — causing the userspace event consumer to miss the intervening
>> > finger-up sequence and corrupt its gesture session state.
>> >
>> > Fix by resetting num_expected to 0 when contact_count is zero and
>> > num_received is still 0 (i.e., this is the first and only packet of the
>> > frame, not a continuation packet in a multi-packet sequence). With
>> > num_expected=0 the sync check "0 >= 0" fires immediately, calling
>> > input_mt_sync_frame() which drops the stale slots via INPUT_MT_DROP_UNUSED.
>> >
>> > The num_received==0 guard is critical: continuation packets in a
>> > multi-packet frame arrive after at least one contact has already been
>> > processed (num_received>0), so they are correctly excluded from this
>> > path and the existing multi-packet logic is unaffected.
>>
>> I've just tested the change, and the testsuite seem to still be happy
>> and I can't find a flaw in the design.
>> However: (inlined comment)
>>
>> >
>> > Signed-off-by: Dave Carey <carvsdriver@gmail.com>
>> > Tested-by: Dave Carey <carvsdriver@gmail.com>
>> > ---
>> > This follows commit 108ac841 ("HID: multitouch: Fix Yoga Book 9 14IAH10
>> > touchscreen misclassification"), applied to hid/for-next on 2026-05-12.
>> > That commit established MT_CLS_YOGABOOK9I.
>> >
>> > This fix is independent of the companion ghost-contacts patch ("HID:
>> > multitouch: Honor ContactCount for Yoga Book 9 to suppress ghost contacts",
>> > sent 2026-05-14), which adds MT_QUIRK_CONTACT_CNT_ACCURATE to the same
>> > class. Both apply cleanly on top of 108ac841.
>> >
>> > Tested on Lenovo Yoga Book 9 14IAH10 (83KJ).
>> >
>> > drivers/hid/hid-multitouch.c | 12 ++++++++++++
>> > 1 file changed, 12 insertions(+)
>> >
>> > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> > index ec04dba..e91ba89 100644
>> > --- a/drivers/hid/hid-multitouch.c
>> > +++ b/drivers/hid/hid-multitouch.c
>> > @@ -1336,6 +1336,18 @@ static void mt_touch_report(struct hid_device *hid,
>> > /* A non 0 contact count always indicates a first packet */
>> > else if (contact_count)
>> > app->num_expected = contact_count;
>> > + /*
>> > + * contact_count == 0 on the first packet of a new frame means
>> > + * all contacts have lifted (the firmware sends an explicit zero
>> > + * to signal all-up rather than simply omitting the frame).
>> > + * Reset num_expected so that the sync check below fires and
>> > + * INPUT_MT_DROP_UNUSED releases any stale slots. This is safe
>> > + * for multi-packet continuation frames because those arrive with
>> > + * num_received > 0 (at least one contact was already processed
>> > + * from the preceding first-packet in the same frame).
>> > + */
>> > + else if (app->num_received == 0)
>> > + app->num_expected = 0;
>>
>> this succession of if/else if makes the code harder to understand,
>> especially with long blocks of comments.
>>
>> How about this instead (wouldn't mind a second pair of eyes):
>>
>> /*
>> * Includes multi-packet support where subsequent
>> * packets are sent with zero contactcount.
>> */
>> if (contact_count > 0)
>> app->num_expected = contact_count;
>> else if (!contact_count && app->num_received == 0 && app->prev_scantime != scantime) {
>> /*
>> * New multi-report frame:
>> *
>> * For Win8 PTPs the first packet (td->num_received == 0) may
>> * have a contactcount of 0 if there only is a button event.
>> *
>> * Some other devices use a sentinel frame with 0 to release all contacts
>> */
>> app->num_expected = 0;
>> }
>>
>> At least the test-suite is happy with this change.
>>
>> Cheers,
>> Benjamin
>>
>> > }
>> > app->prev_scantime = scantime;
>> >
>> > --
>> > 2.54.0
>> >
>> >
^ permalink raw reply
* Re: [PATCH 1/1] HID: wacom: Fix OOB write in wacom_hid_set_device_mode()
From: Benjamin Tissoires @ 2026-05-21 15:47 UTC (permalink / raw)
To: Lee Jones
Cc: Ping Cheng, Ping Cheng, Jason Gerecke, Jiri Kosina, linux-input,
linux-kernel, stable
In-Reply-To: <20260519111354.GT305027@google.com>
On May 19 2026, Lee Jones wrote:
> On Wed, 13 May 2026, Ping Cheng wrote:
>
> > On Wed, May 13, 2026 at 1:05 AM Lee Jones <lee@kernel.org> wrote:
> > >
> > > wacom_hid_set_device_mode() currently assumes that the HID_DG_INPUTMODE
> > > usage is always located in the first field (field[0]) of the feature report.
> > > However, a device can specify HID_DG_INPUTMODE in a different field.
> > >
> > > If HID_DG_INPUTMODE is in a field other than the first one and the first
> > > field has a report_count smaller than the usage_index of HID_DG_INPUTMODE,
> > > this leads to an out-of-bounds write to r->field[0]->value.
> > >
> > > Fix this by storing the field index of HID_DG_INPUTMODE in 'struct
> > > hid_data' during feature mapping. In wacom_hid_set_device_mode(), use
> > > this stored field index to access the correct field and add bounds
> > > checks to ensure both the field index and the value index are within
> > > valid ranges before writing.
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: 5ae6e89f7409 ("HID: wacom: implement the finger part of the HID generic handling")
> > > Signed-off-by: Lee Jones <lee@kernel.org>
> >
> > Patch looks sensible to me. Thank you for your effort, Lee!
> >
> > Tested-by: Ping Cheng <ping.cheng@wacom.com>
> > Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
>
> Thank you Ping, I appreciate your review.
>
> HID folks - any movement on this please?
>
I wanted to apply it today, but the patch conflicts with our current
for-7.1/upstream-fixes.
Could you rebase on top of this branch so we can take this without me
messing with your patch?
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] HID: bpf: Add Huion Inspiroy Frego M button quirk
From: Benjamin Tissoires @ 2026-05-21 15:35 UTC (permalink / raw)
To: jikos, Nikhil Chatterjee; +Cc: linux-input
In-Reply-To: <20260426031819.808555-1-nikhilc1527@gmail.com>
On Sat, 25 Apr 2026 20:18:19 -0700, Nikhil Chatterjee wrote:
> The Huion Inspiroy Frego M pen report descriptor exposes the second
> side button as Secondary Tip Switch instead of Secondary Barrel Switch.
> This makes userspace see the control as the wrong pen button.
>
> Add a HID-BPF report descriptor fixup for the Bluetooth 256c:8251
> device and USB 256c:2012 L610 variant. The fixup matches the expected
> pen descriptor and rewrites the offending usage from Secondary Tip
> Switch to Secondary Barrel Switch.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.2/bpf), thanks!
[1/1] HID: bpf: Add Huion Inspiroy Frego M button quirk
https://git.kernel.org/hid/hid/c/857e71cb0a53
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 3/6] selftests: Remove unused LLD variable
From: Benjamin Tissoires @ 2026-05-21 15:30 UTC (permalink / raw)
To: James Clark
Cc: Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Jiri Kosina, Shuah Khan, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Kumar Kartikeya Dwivedi,
Song Liu, Yonghong Song, ihor.solodrai, linux-kernel, llvm,
linux-input, linux-kselftest, bpf, linux-perf-users
In-Reply-To: <20260518-james-perf-llvm-version-v2-3-f12cc4f031d8@linaro.org>
On May 18 2026, James Clark wrote:
> This file was mostly copied from selftests/bpf/Makefile, but the LLD
> variable is not used here. Also, this copied block didn't get the same
> fixes as the original one did later.
>
> Remove it to avoid confusion and so future fixes don't have to be in two
> places.
>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.2/bpf), thanks!
[3/6] selftests: Remove unused LLD variable
https://git.kernel.org/hid/hid/c/08918b98b02b
Cheers,
Benjamin
> tools/testing/selftests/hid/Makefile | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
> index 50ec9e0406ab..96071b4800e8 100644
> --- a/tools/testing/selftests/hid/Makefile
> +++ b/tools/testing/selftests/hid/Makefile
> @@ -105,13 +105,6 @@ $(MAKE_DIRS):
> $(call msg,MKDIR,,$@)
> $(Q)mkdir -p $@
>
> -# LLVM's ld.lld doesn't support all the architectures, so use it only on x86
> -ifeq ($(SRCARCH),x86)
> -LLD := lld
> -else
> -LLD := ld
> -endif
> -
> DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
>
> TEST_GEN_PROGS_EXTENDED += $(DEFAULT_BPFTOOL)
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH v2 0/2] HID: lenovo-go: clean up USB assumption
From: Benjamin Tissoires @ 2026-05-21 15:23 UTC (permalink / raw)
To: Derek J . Clark, Mark Pearson, Jiri Kosina, Louis Clinckx
Cc: linux-input, linux-kernel
In-Reply-To: <cover.1778701376.git.clinckx.louis@gmail.com>
On Fri, 15 May 2026 14:57:38 +0000, Louis Clinckx wrote:
> These drivers' id_tables only match HID_USB_DEVICE() entries and the
> code freely assumes a USB transport (to_usb_interface() on hdev->dev.parent,
> endpoint access in raw_event). Make the assumption explicit at probe and
> drop a NULL check that the compiler-level definition of to_usb_interface()
> makes unreachable.
>
> No functional change for the supported (USB) devices.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.1/upstream-fixes), thanks!
[1/2] HID: lenovo-go: reject non-USB transports in probe
https://git.kernel.org/hid/hid/c/2ee7e632405b
[2/2] HID: lenovo-go: drop dead NULL check on to_usb_interface()
https://git.kernel.org/hid/hid/c/da7f96a68c39
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: [PATCH] HID: lenovo: Fix buffer over-read and unaligned access in X12 Tab raw_event handler
From: Benjamin Tissoires @ 2026-05-21 15:23 UTC (permalink / raw)
To: sashiko-bot, derekjohn.clark, Kean
Cc: linux-input, mpearson-lenovo, jikos, linux-kernel
In-Reply-To: <20260514125838.3307386-1-rh_king@163.com>
On Thu, 14 May 2026 20:58:38 +0800, Kean wrote:
> In lenovo_raw_event(), the X12 Tab keyboard handler reads a 4-byte
> little-endian value from the raw HID report buffer but:
>
> 1. The size guard is size >= 3, while the access reads 4 bytes.
> A malformed 3-byte report with ID 0x03 would over-read the
> buffer by one byte.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.1/upstream-fixes), thanks!
[1/1] HID: lenovo: Fix buffer over-read and unaligned access in X12 Tab raw_event handler
https://git.kernel.org/hid/hid/c/c7ee0b73c8c4
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ 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