* Re: [PATCH v2] platform/x86/lenovo: add Yoga Book 9 keyboard dock driver
From: sashiko-bot @ 2026-05-17 15:25 UTC (permalink / raw)
To: Dave Carey; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260517150224.50191-1-carvsdriver@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] NULL Pointer Dereference on sysfs read for QUERY device and DMI check bypass
- [High] Use-After-Free due to unreferenced QUERY WMI device pointer
- [Medium] Race condition exposing incorrect initial switch state to userspace
--
commit f26ec3cd843093c54646b94f3331c501f619e886
Author: Dave Carey <carvsdriver@gmail.com>
platform/x86/lenovo: add Yoga Book 9 keyboard dock driver
This commit adds a new driver, lenovo-yb9-kbdock, to support the detachable
keyboard dock on the Lenovo Yoga Book 9 14IAH10. It queries the BKBD state
via WMI to report SW_TABLET_MODE input events and exposes the raw position
to userspace via sysfs.
> diff --git a/drivers/platform/x86/lenovo/yb9-kbdock.c b/drivers/platform/x86/lenovo/yb9-kbdock.c
> new file mode 100644
> index 0000000000000..ba0579f7981a3
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo/yb9-kbdock.c
[ ... ]
> +static int yb9_kbdock_probe(struct wmi_device *wdev, const void *ctx)
> +{
> + enum yb9_guid_type type = (enum yb9_guid_type)(uintptr_t)ctx;
> + struct yb9_kbdock_priv *priv;
> + struct input_dev *input_dev;
> + struct wmi_device *qwdev;
> + int err;
> +
> + if (type == YB9_GUID_QUERY) {
> + mutex_lock(&yb9_query_lock);
> + yb9_query_wdev = wdev;
> + mutex_unlock(&yb9_query_lock);
> + return 0;
> + }
The wmi_driver struct sets .dev_groups to yb9_kbdock_groups, which means the
keyboard_position sysfs attribute is created for all devices binding to this
driver, including the query WMI device.
If userspace reads this sysfs attribute on the query device, won't
keyboard_position_show() receive a NULL pointer from dev_get_drvdata()
and dereference it when accessing priv->bkbd?
Also, because this returns before dmi_check_system() is called, does this
allow the driver to bind to the query GUID on unsupported hardware?
> +
> + if (!dmi_check_system(yb9_kbdock_dmi_table))
> + return -ENODEV;
[ ... ]
> + input_set_capability(input_dev, EV_SW, SW_TABLET_MODE);
> +
> + err = input_register_device(input_dev);
> + if (err) {
> + dev_err(&wdev->dev, "failed to register input device: %d\n", err);
> + return err;
> + }
> +
> + priv->query_wdev = qwdev;
Is a device link or reference count via get_device() needed here?
If the query WMI device is unbound or its parent ACPI device is hot-removed,
could priv->query_wdev become a dangling pointer that gets used in
yb9_kbdock_notify() when a WMI event fires?
> + priv->input_dev = input_dev;
> + dev_set_drvdata(&wdev->dev, priv);
> +
> + yb9_kbdock_update(wdev);
Could this expose an incorrect initial switch state to userspace?
Since input_register_device() makes the device visible before
yb9_kbdock_update() queries the hardware state, userspace might
read the default laptop mode (0) even if the keyboard is detached.
Can the initial hardware state be queried and reported before registering
the input device?
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260517150224.50191-1-carvsdriver@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 3/4] HID: wacom: validate report length for DTUS handler
From: sashiko-bot @ 2026-05-17 15:19 UTC (permalink / raw)
To: Jinmo Yang; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260517135215.2220117-4-jinmo44.yang@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The global `if (len < 7)` check in `wacom_wac_irq()` for DTUS/DTUSX devices drops valid short reports, breaking the DTUSPAD (pad buttons) functionality.
- [High] Incomplete fix leaving other device types vulnerable to slab-out-of-bounds reads.
- [High] NULL pointer dereference in `wacom_dtus_irq` for DTUSX devices.
--
commit bbe438798c1e7006b14d3fc0a92585f47e3776e2
Author: Jinmo Yang <jinmo44.yang@gmail.com>
HID: wacom: validate report length for DTUS handler
This commit adds a minimum length check in wacom_wac_irq() for DTUS and DTUSX
devices before dispatching to wacom_dtus_irq() to prevent out-of-bounds reads.
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 873d58a6d5a3b..269e8318f79af 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -3479,6 +3479,8 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
>
> case DTUS:
> case DTUSX:
> + if (len < 7)
> + return;
Does this global length check drop valid short reports for pad buttons?
Looking at wacom_dtus_irq(), it processes two different reports:
WACOM_REPORT_DTUS which accesses up to data[6], and WACOM_REPORT_DTUSPAD
which only accesses data[0] and data[1].
If the pad report is sent as a 2-byte transfer, this check will drop it before
it reaches the handler, breaking the ExpressKeys functionality.
Should the validation first check the report ID (ensuring len >= 1) and then
enforce the minimum length specific to the report type?
Also, while this patch adds a check for DTUS/DTUSX, does it leave other device
types vulnerable to similar out-of-bounds reads?
Other branches in wacom_wac_irq() such as PENPARTNER (which accesses up to
data[6]) and GRAPHIRE (which accesses up to data[7]) do not appear to have
length checks before calling their respective handlers.
Lastly, looking at wacom_dtus_irq(), it handles WACOM_REPORT_DTUSPAD by using
wacom->pad_input:
drivers/hid/wacom_wac.c:wacom_dtus_irq() {
...
} else if (data[0] == WACOM_REPORT_DTUSPAD) {
input = wacom->pad_input;
input_report_key(input, BTN_0, (data[1] & 0x01));
...
}
Since DTUSX does not receive the WACOM_DEVICETYPE_PAD capability flag during
initialization, wacom->pad_input remains NULL.
If a spoofed DTUSX device sends a WACOM_REPORT_DTUSPAD report, does this cause
a NULL pointer dereference when calling input_report_key()?
> sync = wacom_dtus_irq(wacom_wac);
> break;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260517135215.2220117-1-jinmo44.yang@gmail.com?part=3
^ permalink raw reply
* [PATCH v2] platform/x86/lenovo: add Yoga Book 9 keyboard dock driver
From: Dave Carey @ 2026-05-17 15:02 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: hdegoede, pithenrich2d, mpearson-lenovo, derekjohn.clark, W_Armin,
platform-driver-x86, linux-input, linux-kernel, Dave Carey
In-Reply-To: <9459f535-d140-a431-3f76-a5d8623f3e2d@linux.intel.com>
The Lenovo Yoga Book 9 14IAH10 (83KJ) ships with a detachable Bluetooth
keyboard that magnetically attaches to the bottom screen in one of two
positions.
The Embedded Controller tracks the attachment state in a 2-bit field called
BKBD (byte 0x23 of EC RAM, bits 4-5) and signals changes via EC query _QB0,
which calls Notify(WM10, 0xEB) on the WM10 ACPI WMI device (_UID "GMZN").
The WMI event GUID 806BD2A2-177B-481D-BFB5-3BA0BB4A2285 fires on every
state change. The current state is readable at any time via WMI data block
E7F300FA-21CD-4003-ADAC-2696135982E6 (WQAF), which returns an 8-byte buffer
whose upper four bytes hold the BKBD value. BKBD encodes the attachment
state as: 0 = keyboard detached, 1 = docked on the top half of the bottom
screen, 2 = docked on the bottom half, 3 = reserved (treated as an error by
this driver).
Add a new driver, lenovo-yb9-kbdock, that queries the BKBD state on probe
and on each WMI event notification. Both docked positions indicate a
physical keyboard is present and report SW_TABLET_MODE=0 (laptop mode);
keyboard detached reports SW_TABLET_MODE=1 (tablet mode). The raw BKBD
value is additionally exposed via the read-only sysfs attribute
"keyboard_position" for userspace that needs to distinguish the two docked
positions (e.g. to choose a different UI layout).
Tested on Lenovo Yoga Book 9 14IAH10 (83KJ): all three BKBD states
(detached, top-half dock, bottom-half dock) reported correctly;
SW_TABLET_MODE transitions verified with evtest.
Signed-off-by: Dave Carey <carvsdriver@gmail.com>
---
drivers/platform/x86/lenovo/Kconfig | 14 ++++
drivers/platform/x86/lenovo/Makefile | 1 +
drivers/platform/x86/lenovo/yb9-kbdock.c | 233 +++++++++++++++++++++
3 files changed, 248 insertions(+)
create mode 100644 drivers/platform/x86/lenovo/yb9-kbdock.c
--- a/drivers/platform/x86/lenovo/Kconfig
+++ b/drivers/platform/x86/lenovo/Kconfig
@@ -43,6 +43,20 @@
To compile this driver as a module, choose M here: the module
will be called lenovo-wmi-camera.
+config LENOVO_YB9_KBDOCK
+ tristate "Lenovo Yoga Book 9 keyboard dock detection"
+ depends on ACPI_WMI
+ depends on DMI
+ depends on INPUT
+ help
+ Say Y here to enable keyboard dock detection on the Lenovo Yoga Book 9
+ 14IAH10. The detachable Bluetooth keyboard magnetically attaches to
+ either screen; this driver reports SW_TABLET_MODE input events based
+ on the attachment state and exposes the raw position in sysfs.
+
+ To compile this driver as a module, choose M here: the module will be
+ called lenovo-yb9-kbdock.
+
config LENOVO_YMC
tristate "Lenovo Yoga Tablet Mode Control"
depends on ACPI_WMI
--- a/drivers/platform/x86/lenovo/Makefile
+++ b/drivers/platform/x86/lenovo/Makefile
@@ -8,6 +8,7 @@
obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o
lenovo-target-$(CONFIG_LENOVO_WMI_HOTKEY_UTILITIES) += wmi-hotkey-utilities.o
+lenovo-target-$(CONFIG_LENOVO_YB9_KBDOCK) += yb9-kbdock.o
lenovo-target-$(CONFIG_LENOVO_YMC) += ymc.o
lenovo-target-$(CONFIG_YOGABOOK) += yogabook.o
lenovo-target-$(CONFIG_YT2_1380) += yoga-tab2-pro-1380-fastcharger.o
--- a/drivers/platform/x86/lenovo/yb9-kbdock.c
+++ b/drivers/platform/x86/lenovo/yb9-kbdock.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Lenovo Yoga Book 9 keyboard-dock detection
+ *
+ * Reports SW_TABLET_MODE based on keyboard attachment state and exposes the
+ * raw dock position via sysfs.
+ *
+ * Copyright (C) 2026 Dave Carey <carvsdriver@gmail.com>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/acpi.h>
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/dmi.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/wmi.h>
+
+/*
+ * WM10 ACPI device (_UID "GMZN") exposes two relevant WMI GUIDs:
+ * YB9_KBDOCK_EVENT_GUID — notify ID 0xEB fires on attachment state change.
+ * YB9_KBDOCK_QUERY_GUID — object "AF" (WQAF), returns an 8-byte buffer
+ * whose upper four bytes hold the BKBD value.
+ *
+ * BKBD encoding:
+ * 0 (BKBD_DETACHED) — keyboard detached → SW_TABLET_MODE = 1
+ * 1 (BKBD_TOP_HALF) — docked, top half → SW_TABLET_MODE = 0
+ * 2 (BKBD_BOTTOM_HALF) — docked, bottom half → SW_TABLET_MODE = 0
+ * 3 — reserved; treated as an error
+ */
+#define YB9_KBDOCK_EVENT_GUID "806BD2A2-177B-481D-BFB5-3BA0BB4A2285"
+#define YB9_KBDOCK_QUERY_GUID "E7F300FA-21CD-4003-ADAC-2696135982E6"
+#define YB9_KBDOCK_QUERY_INSTANCE 0
+
+#define BKBD_DETACHED 0
+#define BKBD_TOP_HALF 1
+#define BKBD_BOTTOM_HALF 2
+#define BKBD_MASK GENMASK(1, 0)
+
+/* Distinguish the two GUIDs via the id_table context field. */
+enum yb9_guid_type { YB9_GUID_EVENT, YB9_GUID_QUERY };
+
+/*
+ * Both GUIDs are children of the same ACPI device (WM10). Store the query
+ * WMI device globally so the event-device probe and notify path can reach it
+ * via wmidev_block_query(). Protected by yb9_query_lock during probe/remove.
+ */
+static struct wmi_device *yb9_query_wdev;
+static DEFINE_MUTEX(yb9_query_lock);
+
+struct yb9_kbdock_priv {
+ struct wmi_device *query_wdev;
+ struct input_dev *input_dev;
+ unsigned int bkbd;
+};
+
+/* Returns 0–2 on success, -errno on error. */
+static int yb9_kbdock_query(struct wmi_device *event_wdev,
+ struct wmi_device *query_wdev)
+{
+ u32 bkbd;
+
+ union acpi_object *obj __free(kfree) =
+ wmidev_block_query(query_wdev, YB9_KBDOCK_QUERY_INSTANCE);
+ if (!obj) {
+ dev_warn(&event_wdev->dev, "WQAF query returned NULL\n");
+ return -EIO;
+ }
+
+ /*
+ * WQAF returns an 8-byte buffer: bytes [0..3] = LFID (0x00060000),
+ * bytes [4..7] = BKBD value. Guard against short buffers.
+ */
+ if (obj->type == ACPI_TYPE_BUFFER && obj->buffer.length >= 8)
+ memcpy(&bkbd, obj->buffer.pointer + 4, sizeof(bkbd));
+ else if (obj->type == ACPI_TYPE_INTEGER)
+ bkbd = obj->integer.value;
+ else {
+ dev_warn(&event_wdev->dev,
+ "WQAF: unexpected result type %d len %u\n",
+ obj->type, obj->type == ACPI_TYPE_BUFFER ? obj->buffer.length : 0);
+ return -EIO;
+ }
+
+ bkbd = FIELD_GET(BKBD_MASK, bkbd);
+ if (bkbd == 3) {
+ dev_warn(&event_wdev->dev, "BKBD value 3 is reserved\n");
+ return -EINVAL;
+ }
+
+ return bkbd;
+}
+
+static void yb9_kbdock_update(struct wmi_device *wdev)
+{
+ struct yb9_kbdock_priv *priv = dev_get_drvdata(&wdev->dev);
+ int tablet_mode;
+ int bkbd;
+
+ bkbd = yb9_kbdock_query(wdev, priv->query_wdev);
+ if (bkbd < 0)
+ return;
+
+ priv->bkbd = bkbd;
+ tablet_mode = (bkbd == BKBD_DETACHED) ? 1 : 0;
+
+ input_report_switch(priv->input_dev, SW_TABLET_MODE, tablet_mode);
+ input_sync(priv->input_dev);
+
+ dev_dbg(&wdev->dev, "BKBD=%u tablet_mode=%d\n", bkbd, tablet_mode);
+}
+
+static void yb9_kbdock_notify(struct wmi_device *wdev, union acpi_object *data)
+{
+ yb9_kbdock_update(wdev);
+}
+
+static ssize_t keyboard_position_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ static const char * const names[] = {
+ "detached", "top-half", "bottom-half",
+ };
+ struct yb9_kbdock_priv *priv = dev_get_drvdata(dev);
+ unsigned int bkbd = priv->bkbd;
+
+ if (WARN_ON_ONCE(bkbd >= ARRAY_SIZE(names)))
+ return -EINVAL;
+ return sysfs_emit(buf, "%u (%s)\n", bkbd, names[bkbd]);
+}
+static DEVICE_ATTR_RO(keyboard_position);
+
+static struct attribute *yb9_kbdock_attrs[] = {
+ &dev_attr_keyboard_position.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(yb9_kbdock);
+
+static const struct dmi_system_id yb9_kbdock_dmi_table[] = {
+ {
+ /* Lenovo Yoga Book 9 14IAH10 */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "83KJ"),
+ },
+ },
+ { }
+};
+
+static int yb9_kbdock_probe(struct wmi_device *wdev, const void *ctx)
+{
+ enum yb9_guid_type type = (enum yb9_guid_type)(uintptr_t)ctx;
+ struct yb9_kbdock_priv *priv;
+ struct input_dev *input_dev;
+ struct wmi_device *qwdev;
+ int err;
+
+ if (type == YB9_GUID_QUERY) {
+ mutex_lock(&yb9_query_lock);
+ yb9_query_wdev = wdev;
+ mutex_unlock(&yb9_query_lock);
+ return 0;
+ }
+
+ if (!dmi_check_system(yb9_kbdock_dmi_table))
+ return -ENODEV;
+
+ mutex_lock(&yb9_query_lock);
+ qwdev = yb9_query_wdev;
+ mutex_unlock(&yb9_query_lock);
+ if (!qwdev)
+ return -EPROBE_DEFER;
+
+ priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ input_dev = devm_input_allocate_device(&wdev->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ input_dev->name = "Lenovo Yoga Book 9 keyboard dock switch";
+ input_dev->phys = YB9_KBDOCK_EVENT_GUID "/input0";
+ input_dev->id.bustype = BUS_HOST;
+ input_dev->dev.parent = &wdev->dev;
+ input_set_capability(input_dev, EV_SW, SW_TABLET_MODE);
+
+ err = input_register_device(input_dev);
+ if (err) {
+ dev_err(&wdev->dev, "failed to register input device: %d\n", err);
+ return err;
+ }
+
+ priv->query_wdev = qwdev;
+ priv->input_dev = input_dev;
+ dev_set_drvdata(&wdev->dev, priv);
+
+ yb9_kbdock_update(wdev);
+ return 0;
+}
+
+static void yb9_kbdock_remove(struct wmi_device *wdev)
+{
+ mutex_lock(&yb9_query_lock);
+ if (wdev == yb9_query_wdev)
+ yb9_query_wdev = NULL;
+ mutex_unlock(&yb9_query_lock);
+}
+
+static const struct wmi_device_id yb9_kbdock_wmi_id_table[] = {
+ { .guid_string = YB9_KBDOCK_EVENT_GUID, .context = (void *)YB9_GUID_EVENT },
+ { .guid_string = YB9_KBDOCK_QUERY_GUID, .context = (void *)YB9_GUID_QUERY },
+ { }
+};
+MODULE_DEVICE_TABLE(wmi, yb9_kbdock_wmi_id_table);
+
+static struct wmi_driver yb9_kbdock_driver = {
+ .driver = {
+ .name = "lenovo-yb9-kbdock",
+ .dev_groups = yb9_kbdock_groups,
+ },
+ .id_table = yb9_kbdock_wmi_id_table,
+ .probe = yb9_kbdock_probe,
+ .remove = yb9_kbdock_remove,
+ .notify = yb9_kbdock_notify,
+};
+module_wmi_driver(yb9_kbdock_driver);
+
+MODULE_AUTHOR("Dave Carey <carvsdriver@gmail.com>");
+MODULE_DESCRIPTION("Lenovo Yoga Book 9 keyboard dock detection");
+MODULE_LICENSE("GPL");
2.47.0
^ permalink raw reply
* Re: [PATCH] platform/x86/lenovo: Add Yoga Book 9 keyboard dock detection driver
From: Dave Carey @ 2026-05-17 15:01 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: hdegoede, pithenrich2d, mpearson-lenovo, derekjohn.clark, W_Armin,
platform-driver-x86, linux-input, linux-kernel, Dave Carey
In-Reply-To: <9459f535-d140-a431-3f76-a5d8623f3e2d@linux.intel.com>
On Tue, 28 Apr 2026 17:39:17 +0300, Ilpo Järvinen wrote:
> On Sat, 25 Apr 2026, Dave Carey wrote:
Thank you for the review. All points addressed in v2 below.
> Please put this in depth explanation in own paragraph.
> This seems mostly duplicate of what was said previously.
> These two can be combined with the in depth explanation paragraph.
> Please write this without bullet points.
Commit message rewritten as prose. The hardware description, WMI
mechanism, and BKBD encoding are now combined into a single explanatory
paragraph. The functional summary (what the driver does) follows in a
second paragraph without bullet points.
> I don't think the functional description on this level is warranted in
> the top comment.
Top-of-file block comment trimmed to hardware context only. The
functional description (query on probe, SW_TABLET_MODE mapping, sysfs
attribute) has been removed from there; the comment now covers only the
two WMI GUIDs and the BKBD encoding table, which are non-obvious from
the code alone.
> This interface has been deprecated.
Done. v2 uses wmidev_block_query() with two arguments, returning
union acpi_object * directly. This required registering both the event
and query GUIDs in the id_table with context pointers (enum
yb9_guid_type) so the query wdev is reachable from probe and the notify
path. The event-device probe defers with -EPROBE_DEFER until the query
device arrives.
> Please use __free() instead of duplicating kfree()s.
> When converting to __free(), don't use ... = NULL; pattern, instead place
> the variable declaration mid-function as instructed in cleanup.h.
Done. The obj declaration now uses __free(kfree) placed mid-function
after the early-exit checks, per the cleanup.h guidance. Added
linux/cleanup.h to the includes.
> Can this literal be named with a define? Should it use FIELD_GET()?
> (don't forget the header if you start to use FIELD_GET())
Done. BKBD_MASK is now GENMASK(1, 0) and both extraction sites use
FIELD_GET(BKBD_MASK, bkbd). Added linux/bitfield.h to the includes.
> Unnecessary cast. And your types are a major mess between int and
> unsigned types.
Fixed. The query function returns int throughout (0-2 on success,
-errno on error); the cast is gone. Local variables in yb9_kbdock_update
are consistently int.
> Please use reverse-xmas tree order.
Fixed in yb9_kbdock_update(): tablet_mode is now declared before bkbd.
> WARN_ON_ONCE() instead of just WARN_ON() in the sysfs show function.
Done.
> You didn't document unknown but return it (maybe it should return some
> -Exx code instead?).
The "unknown" fourth entry has been removed entirely. BKBD value 3 is
now caught in yb9_kbdock_query() and returned as -EINVAL so it never
reaches the sysfs show function or priv->bkbd. The show function uses
WARN_ON_ONCE(bkbd >= ARRAY_SIZE(names)) and returns -EINVAL as
suggested — this can only fire if there is a driver bug.
> Missing include.
Added linux/bitfield.h (FIELD_GET) and linux/cleanup.h (__free).
> Normally these [DMI table] appear towards the end of the file.
Moved to just before yb9_kbdock_probe().
> Put the last two lines to one line.
Done.
> I wonder if this is similar physically to what is being added here:
> https://lore.kernel.org/all/20260419102724.91451-1-pithenrich2d@gmail.com/
> If yes, we may have to take another look at how to create the interface
> for this.
Pit Henrich's patch targets the ThinkPad X1 Fold 16 Gen 1 — also a
Lenovo device with a magnetically-attached keyboard that docks to the
display and changes between tablet and laptop mode, so the concept is
physically similar.
However, the two drivers differ in several meaningful ways:
* Different hardware families and kernel paths: the X1 Fold patch
extends thinkpad_acpi using an ACPI method (\\_SB.DEVD.GDST) and
the existing TP_HKEY_EV_TABLET_CHANGED hotkey path. The Yoga Book 9
driver is a standalone WMI driver using two separate WMI GUIDs.
* Different state cardinality: the X1 Fold has a binary
keyboard_attached_on_screen attribute because the keyboard is either
present or not. The Yoga Book 9 needs three states — detached,
docked on the top half of the bottom screen, or docked on the bottom
half — because the two docked positions select different screen
layouts that userspace needs to distinguish. A binary attribute
would lose that information.
* SW_TABLET_MODE: both drivers emit SW_TABLET_MODE=1 when the
keyboard is absent and SW_TABLET_MODE=0 when docked, consistent
with existing drivers in this subsystem.
Given these differences the sysfs attribute semantics do not clash, but
if a preferred naming convention is being established for keyboard-dock
attributes across these devices I am happy to align with whatever is
decided for the X1 Fold patch.
Dave
^ permalink raw reply
* Re: [PATCH 2/4] HID: wacom: validate report length for DTU handler
From: sashiko-bot @ 2026-05-17 14:47 UTC (permalink / raw)
To: Jinmo Yang; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260517135215.2220117-3-jinmo44.yang@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete fix for slab-out-of-bounds read vulnerability. The patch correctly validates the buffer length for the DTU device handler, but leaves numerous other device handlers in the same function completely unpatched and vulnerable to the identical attack vector.
--
commit 873407ec5964c67c8e302293e3be453a56be7e51
Author: Jinmo Yang <jinmo44.yang@gmail.com>
HID: wacom: validate report length for DTU handler
wacom_dtu_irq() accesses fixed offsets up to data[7] in the raw HID
report buffer without validating the buffer length. This commit adds
a minimum length check in wacom_wac_irq() before dispatching to
wacom_dtu_irq() to prevent slab-out-of-bounds reads.
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 6d06842b64080..873d58a6d5a3b 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -3472,6 +3472,8 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
> break;
>
> case DTU:
> + if (len < 8)
> + return;
> sync = wacom_dtu_irq(wacom_wac);
> break;
Could this same regression occur in the other device handlers called
from wacom_wac_irq()?
For example, cases like PENPARTNER, GRAPHIRE, and INTUOS also dispatch
to their respective handlers without checking len:
switch (wacom_wac->features.type) {
case PENPARTNER:
sync = wacom_penpartner_irq(wacom_wac);
break;
[ ... ]
case GRAPHIRE:
[ ... ]
sync = wacom_graphire_irq(wacom_wac);
break;
[ ... ]
case INTUOS:
[ ... ]
sync = wacom_intuos_irq(wacom_wac);
break;
Since these sub-handlers do not receive the len parameter, an attacker
spoofing the VID/PID of a PENPARTNER or INTUOS device might be able to
send a short report and trigger a similar out-of-bounds read regression.
Should length validations be added for the remaining device types to
ensure all handlers are protected?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260517135215.2220117-1-jinmo44.yang@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 1/4] HID: wacom: validate report length for PL and PTU handlers
From: sashiko-bot @ 2026-05-17 14:22 UTC (permalink / raw)
To: Jinmo Yang; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260517135215.2220117-2-jinmo44.yang@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete Fix / Unhandled OOB Reads for Multiple Device Types
--
commit 4ac677c30d7590ff24c2b6674eafbe8d0fff6898
Author: Jinmo Yang <jinmo44.yang@gmail.com>
HID: wacom: validate report length for PL and PTU handlers
This commit adds minimum length checks before dispatching HID reports to
the PL and PTU handlers. This prevents slab-out-of-bounds reads when a
malicious device sends an artificially small report.
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..6d06842b64080 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -3453,6 +3453,8 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
> break;
>
> case PL:
> + if (len < 8)
> + return;
> sync = wacom_pl_irq(wacom_wac);
> break;
>
[ ... ]
> @@ -3464,6 +3466,8 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
> break;
>
> case PTU:
> + if (len < 8)
> + return;
> sync = wacom_ptu_irq(wacom_wac);
> break;
>
While this successfully prevents out-of-bounds reads for PL and PTU devices,
do other handlers called from wacom_wac_irq() remain vulnerable to the same
issue?
For example, wacom_graphire_irq() unconditionally accesses data[8] for
WACOM_MO:
wacom_wac_irq()
wacom_graphire_irq()
case WACOM_MO:
prox = (data[7] & 0xf8) || data[8];
Similarly, wacom_remote_irq() ignores its len argument and accesses up to
data[12], and wacom_intuos_pro2_bt_battery() can access data[284].
If a device spoofing these other types sends an undersized report, could it
bypass the checks added here and still trigger a slab-out-of-bounds read?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260517135215.2220117-1-jinmo44.yang@gmail.com?part=1
^ permalink raw reply
* [PATCH 4/4] HID: wacom: validate report length for 24HDT and 27QHDT handlers
From: Jinmo Yang @ 2026-05-17 13:52 UTC (permalink / raw)
To: linux-input; +Cc: jikos, benjamin.tissoires, linux-kernel, stable, Jinmo Yang
In-Reply-To: <20260517135215.2220117-1-jinmo44.yang@gmail.com>
wacom_24hdt_irq() accesses data[61] for WACOM_24HDT and data[63] for
WACOM_27QHDT in the raw HID report buffer without validating the buffer
length. This sub-function is called from wacom_wac_irq() which receives
the length parameter but does not pass it to the handler.
A malicious USB device can declare a small HID report in its descriptor
and send a matching short report that passes the HID core size check
(csize >= rsize), but the driver assumes a full-size hardware report
layout, leading to slab-out-of-bounds reads.
Add minimum length checks in wacom_wac_irq() before dispatching to
wacom_24hdt_irq() for both device types.
Fixes: b1e4279e4ef5 ("Input: wacom - add touch sensor support for Cintiq 24HD touch")
Cc: stable@vger.kernel.org
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_wac.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 269e8318f..2fd1c4e80 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3509,7 +3509,14 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
break;
case WACOM_24HDT:
+ if (len < 62)
+ return;
+ sync = wacom_24hdt_irq(wacom_wac);
+ break;
+
case WACOM_27QHDT:
+ if (len < 64)
+ return;
sync = wacom_24hdt_irq(wacom_wac);
break;
--
2.53.0
^ permalink raw reply related
* [PATCH 3/4] HID: wacom: validate report length for DTUS handler
From: Jinmo Yang @ 2026-05-17 13:52 UTC (permalink / raw)
To: linux-input; +Cc: jikos, benjamin.tissoires, linux-kernel, stable, Jinmo Yang
In-Reply-To: <20260517135215.2220117-1-jinmo44.yang@gmail.com>
wacom_dtus_irq() accesses fixed offsets up to data[6] in the raw HID
report buffer without validating the buffer length. This sub-function
is called from wacom_wac_irq() which receives the length parameter but
does not pass it to the handler.
A malicious USB device can declare a small HID report in its descriptor
and send a matching short report that passes the HID core size check
(csize >= rsize), but the driver assumes a full-size hardware report
layout, leading to slab-out-of-bounds reads.
Add a minimum length check in wacom_wac_irq() before dispatching to
wacom_dtus_irq().
Fixes: 497ab1f290a2 ("Input: wacom - add support for DTU-1031")
Cc: stable@vger.kernel.org
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_wac.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 873d58a6d..269e8318f 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3479,6 +3479,8 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
case DTUS:
case DTUSX:
+ if (len < 7)
+ return;
sync = wacom_dtus_irq(wacom_wac);
break;
--
2.53.0
^ permalink raw reply related
* [PATCH 2/4] HID: wacom: validate report length for DTU handler
From: Jinmo Yang @ 2026-05-17 13:52 UTC (permalink / raw)
To: linux-input; +Cc: jikos, benjamin.tissoires, linux-kernel, stable, Jinmo Yang
In-Reply-To: <20260517135215.2220117-1-jinmo44.yang@gmail.com>
wacom_dtu_irq() accesses fixed offsets up to data[7] in the raw HID
report buffer without validating the buffer length. This sub-function
is called from wacom_wac_irq() which receives the length parameter but
does not pass it to the handler.
A malicious USB device can declare a small HID report in its descriptor
and send a matching short report that passes the HID core size check
(csize >= rsize), but the driver assumes a full-size hardware report
layout, leading to slab-out-of-bounds reads.
Add a minimum length check in wacom_wac_irq() before dispatching to
wacom_dtu_irq().
Fixes: c8f2edc56acf ("Input: wacom - add support for DTU2231 and DTU1631")
Cc: stable@vger.kernel.org
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_wac.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 6d06842b6..873d58a6d 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3472,6 +3472,8 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
break;
case DTU:
+ if (len < 8)
+ return;
sync = wacom_dtu_irq(wacom_wac);
break;
--
2.53.0
^ permalink raw reply related
* [PATCH 1/4] HID: wacom: validate report length for PL and PTU handlers
From: Jinmo Yang @ 2026-05-17 13:52 UTC (permalink / raw)
To: linux-input; +Cc: jikos, benjamin.tissoires, linux-kernel, stable, Jinmo Yang
In-Reply-To: <20260517135215.2220117-1-jinmo44.yang@gmail.com>
wacom_pl_irq() and wacom_ptu_irq() access fixed offsets up to data[7]
in the raw HID report buffer without validating the buffer length.
These sub-functions are called from wacom_wac_irq() which receives the
length parameter but does not pass it to the handlers.
A malicious USB device can declare a small HID report in its descriptor
and send a matching short report that passes the HID core size check
(csize >= rsize), but the driver assumes a full-size hardware report
layout, leading to slab-out-of-bounds reads.
Add minimum length checks in wacom_wac_irq() before dispatching to
wacom_pl_irq() and wacom_ptu_irq().
Fixes: 4104d13fe019 ("Input: move USB tablets under drivers/input/tablet")
Cc: stable@vger.kernel.org
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_wac.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85..6d06842b6 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3453,6 +3453,8 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
break;
case PL:
+ if (len < 8)
+ return;
sync = wacom_pl_irq(wacom_wac);
break;
@@ -3464,6 +3466,8 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
break;
case PTU:
+ if (len < 8)
+ return;
sync = wacom_ptu_irq(wacom_wac);
break;
--
2.53.0
^ permalink raw reply related
* [PATCH 0/4] HID: wacom: add report length validation in irq handlers
From: Jinmo Yang @ 2026-05-17 13:52 UTC (permalink / raw)
To: linux-input; +Cc: jikos, benjamin.tissoires, linux-kernel, stable, Jinmo Yang
Several wacom IRQ handler sub-functions access fixed offsets in the raw
HID report buffer without validating the buffer length. wacom_wac_irq()
receives the length from wacom_raw_event() but does not validate it
before dispatching to the sub-functions, which do not receive the length
parameter.
A malicious USB device can declare a small HID report in its descriptor
and send a matching short report that passes the HID core size check
(csize >= rsize), but the driver assumes a full-size hardware report
layout, leading to slab-out-of-bounds reads.
Note: this is not mitigated by the recent HID core bounds checking
series which validates actual_size >= declared_size. An attacker
controls both the descriptor (declared size) and the sent data (actual
size), so the core check passes. Driver-level validation against the
expected hardware report layout is still necessary.
Tested with KASAN on Linux 7.1-rc3 (slab-out-of-bounds confirmed) and
verified kernel panic on a production device via uhid.
Jinmo Yang (4):
HID: wacom: validate report length for PL and PTU handlers
HID: wacom: validate report length for DTU handler
HID: wacom: validate report length for DTUS handler
HID: wacom: validate report length for 24HDT and 27QHDT handlers
drivers/hid/wacom_wac.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
--
2.53.0
^ permalink raw reply
* Re: [PATCH] HID: picolcd: prevent NULL pointer dereference in picolcd_send_and_wait()
From: sashiko-bot @ 2026-05-17 12:51 UTC (permalink / raw)
To: Georgiy Osokin; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260517120639.38003-1-g.osokin@auroraos.dev>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Pre-existing sleeping in atomic context due to calling `hid_hw_request()` while holding a spinlock.
- [Medium] Unnecessary fix for an impossible scenario introducing a latent API vulnerability.
--
commit 26db02d94328ec65757bbf69ad49450f8c2b4f46
Author: Georgiy Osokin <g.osokin@auroraos.dev>
HID: picolcd: prevent NULL pointer dereference in picolcd_send_and_wait()
This commit changes the loop counter k to an unsigned int in
picolcd_send_and_wait(). The stated goal is to prevent an integer overflow
and a resulting out-of-bounds access.
> In picolcd_send_and_wait(), an integer overflow of the signed loop counter
> 'k' can theoretically lead to a NULL pointer dereference of 'raw_data'.
> If the loop executes more than INT_MAX times, 'k' becomes negative,
> making the condition 'k < size' true even when 'size' is 0.
Is this scenario actually possible? The HID core explicitly limits
report->maxfield to 256 (HID_MAX_FIELDS) and report->field[i]->report_count
to 12288 (HID_MAX_USAGES).
The maximum possible value for the loop counter k is 3,145,728, which
would not overflow a 32-bit signed integer.
> diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
> index 2cc01e1bc1a84..d73e97c8b853e 100644
> --- a/drivers/hid/hid-picolcd_core.c
> +++ b/drivers/hid/hid-picolcd_core.c
> @@ -72,7 +72,8 @@ struct picolcd_pending *picolcd_send_and_wait(struct hid_device *hdev,
> struct picolcd_pending *work;
> struct hid_report *report = picolcd_out_report(report_id, hdev);
> unsigned long flags;
> - int i, j, k;
> + int i, j;
> + unsigned int k;
>
> if (!report || !data)
> return NULL;
[ ... ]
Does changing k to an unsigned int introduce a latent vulnerability here?
drivers/hid/hid-picolcd_core.c:picolcd_send_and_wait() {
...
for (i = k = 0; i < report->maxfield; i++)
for (j = 0; j < report->field[i]->report_count; j++) {
hid_set_field(report->field[i], j, k < size ? raw_data[k] : 0);
...
}
Because the size parameter passed to picolcd_send_and_wait() is still a
signed int, the condition k < size causes an implicit signed-to-unsigned
conversion.
If a caller were to pass a negative size, it would be promoted to a large
unsigned value, bypassing the loop bounds check and causing an out-of-bounds
read of raw_data.
While looking at this function, does the subsequent code sleep in atomic
context?
drivers/hid/hid-picolcd_core.c:picolcd_send_and_wait() {
...
spin_lock_irqsave(&data->lock, flags);
...
hid_hw_request(data->hdev, report, HID_REQ_SET_REPORT);
...
}
If the HID device is instantiated via the user-space HID subsystem (uhid),
hdev->ll_driver->request is not implemented.
This causes hid_hw_request() to fall back to __hid_request(), which
unconditionally calls hid_alloc_report_buf() with GFP_KERNEL. Allocating
memory with GFP_KERNEL while holding a spinlock violates atomic context rules.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260517120639.38003-1-g.osokin@auroraos.dev?part=1
^ permalink raw reply
* [PATCH] HID: picolcd: prevent NULL pointer dereference in picolcd_send_and_wait()
From: Georgiy Osokin @ 2026-05-17 12:06 UTC (permalink / raw)
To: bonbons, jikos, bentiss
Cc: linux-input, linux-kernel, lvc-project, s.shtylyov
In picolcd_send_and_wait(), an integer overflow of the signed loop counter
'k' can theoretically lead to a NULL pointer dereference of 'raw_data'.
If the loop executes more than INT_MAX times, 'k' becomes negative,
making the condition 'k < size' true even when 'size' is 0.
Change the type of 'k' to 'unsigned int' to prevent the overflow and
eliminate the out-of-bounds access.
Found by Linux Verification Center (linuxtesting.org) with the Svace static
analysis tool.
Fixes: fabdbf2 ("HID: picoLCD: split driver code")
Signed-off-by: Georgiy Osokin <g.osokin@auroraos.dev>
---
drivers/hid/hid-picolcd_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
index 2cc01e1bc1a8..d73e97c8b853 100644
--- a/drivers/hid/hid-picolcd_core.c
+++ b/drivers/hid/hid-picolcd_core.c
@@ -72,7 +72,8 @@ struct picolcd_pending *picolcd_send_and_wait(struct hid_device *hdev,
struct picolcd_pending *work;
struct hid_report *report = picolcd_out_report(report_id, hdev);
unsigned long flags;
- int i, j, k;
+ int i, j;
+ unsigned int k;
if (!report || !data)
return NULL;
--
2.50.1
^ permalink raw reply related
* [PATCH] HID: core: Fix size_t specifier in hid_report_raw_event()
From: Nathan Chancellor @ 2026-05-17 4:51 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Greg Kroah-Hartman, Johan Hovold, linux-input, linux-kernel,
stable, Miguel Ojeda, Nathan Chancellor
When building for 32-bit platforms, for which 'size_t' is
'unsigned int', there are warnings around using the incorrect format
specifier to print bsize in hid_report_raw_event():
drivers/hid/hid-core.c:2054:29: error: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
2053 | hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
| ~~~
| %zu
2054 | report->id, csize, bsize);
| ^~~~~
drivers/hid/hid-core.c:2076:29: error: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
2075 | hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
| ~~~
| %zu
2076 | report->id, rsize, bsize);
| ^~~~~
Use the proper 'size_t' format specifier, '%zu', to clear up the
warnings.
Cc: stable@vger.kernel.org
Fixes: 2c85c61d1332 ("HID: pass the buffer size to hid_report_raw_event")
Reported-by: Miguel Ojeda <ojeda@kernel.org>
Closes: https://lore.kernel.org/20260516020430.110135-1-ojeda@kernel.org/
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/hid/hid-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b3596851c719..41a79e43c82b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2050,7 +2050,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
return 0;
if (unlikely(bsize < csize)) {
- hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
+ hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %zu)\n",
report->id, csize, bsize);
return -EINVAL;
}
@@ -2072,7 +2072,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
rsize = max_buffer_size;
if (bsize < rsize) {
- hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
+ hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
report->id, rsize, bsize);
return -EINVAL;
}
---
base-commit: 64ffa2e5e02ff54b23221d0282155f37283fabea
change-id: 20260517-hid-core-fix-size_t-specifier-14daf3b496b6
Best regards,
--
Cheers,
Nathan
^ permalink raw reply related
* Re: [PATCH 6.18 000/188] 6.18.32-rc1 review
From: Nathan Chancellor @ 2026-05-17 4:37 UTC (permalink / raw)
To: Greg KH
Cc: Miguel Ojeda, achill, akpm, broonie, conor, f.fainelli, hargar,
jonathanh, linux-kernel, linux, lkft-triage, patches, patches,
pavel, rwarsow, shuah, sr, stable, sudipm.mukherjee, torvalds,
Jiri Kosina, Benjamin Tissoires, linux-input, Johan Hovold
In-Reply-To: <2026051659-facing-superior-e50a@gregkh>
On Sat, May 16, 2026 at 12:11:56PM +0200, Greg KH wrote:
> On Sat, May 16, 2026 at 04:04:30AM +0200, Miguel Ojeda wrote:
> > On Fri, 15 May 2026 17:46:57 +0200 Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > >
> > > This is the start of the stable review cycle for the 6.18.32 release.
> > > There are 188 patches in this series, all will be posted as a response
> > > to this one. If anyone has any issues with these being applied, please
> > > let me know.
> > >
> > > Responses should be made by Sun, 17 May 2026 15:46:37 +0000.
> > > Anything received after that time might be too late.
> >
> > Boot-tested under QEMU for Rust x86_64, arm64 and riscv64; built-tested
> > for loongarch64:
> >
> > Tested-by: Miguel Ojeda <ojeda@kernel.org>
> >
> > Via arm32 I see:
> >
> > drivers/hid/hid-core.c:2050:29: error: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
> > 2049 | hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
> > | ~~~
> > | %zu
> > 2050 | report->id, csize, bsize);
> > | ^~~~~
> >
> > It is also reproducible in mainline, though. Cc'ing a few folks...
>
> bsize is size_t, so that should be %zu, right?
Indeed. Does not look like anyone has sent a patch yet so I will send
one shortly since this is breaking i386_defconfig in my tests since
-Werror is enabled by default there.
--
Cheers,
Nathan
^ permalink raw reply
* i2c_hid_acpi: ELAN2514 touchscreen causes IRQ flood on HP Omnibook X 14 (Lunar Lake)
From: Dennis Foster @ 2026-05-17 4:33 UTC (permalink / raw)
To: linux-input; +Cc: linux-i2c, benjamintissoires, jikos
Hi,
I'm seeing a continuous IRQ flood from the ELAN2514 touchscreen on an HP Omnibook X 14 (Intel Core Ultra 5 226V, Lunar Lake). With the driver bound, IRQ 20 fires non-stop and keeps one CPU core at ~30% load with the machine otherwise idle.
Kernel: 7.0.8-arch1-1
Machine: HP Omnibook X 14, Intel Core Ultra 5 226V
Touchscreen: ELAN2514, 04F3:43F4, ACPI \_SB_.PC00.I2C4.TPL1
Controller: i2c_designware.2 on IRQ 20 (IR-IO-APIC 20-fasteoi)
At boot, i2c_hid_acpi logs this once during probe (rate-limited):
[ 5.042325] i2c_hid_acpi i2c-ELAN2514:00: i2c_hid_get_input: IRQ triggered but there's no data
The device registers successfully after that, but the interrupt line never deasserts. The counter on CPU5 climbs without bound:
20: 0 0 0 0 0 19537 0 0 IR-IO-APIC 20-fasteoi i2c_designware.2
Also worth noting - the device registers an unusual set of input nodes, including several labeled UNKNOWN:
ELAN2514:00 04F3:43F4 Touchscreen
ELAN2514:00 04F3:43F4 Stylus
ELAN2514:00 04F3:43F4 Keyboard
ELAN2514:00 04F3:43F4 Mouse
ELAN2514:00 04F3:43F4 UNKNOWN (x5)
As a workaround I'm unbinding the driver via udev immediately after probe completes:
ACTION=="bind", DRIVER=="i2c_hid_acpi", KERNEL=="i2c-ELAN2514:00", \
RUN+="/bin/sh -c 'echo i2c-ELAN2514:00 > /sys/bus/i2c/drivers/i2c_hid_acpi/unbind'"
This stops the flood but obviously leaves the touchscreen non-functional.
--
Dennis
^ permalink raw reply
* Re: [PATCH v5 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: sashiko-bot @ 2026-05-17 3:50 UTC (permalink / raw)
To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260517013925.3120314-5-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardware state machine desynchronization due to abandoned interruptible waits and lack of request tracking.
- [Medium] Data race on shared state variables `rumble_intensity_left`, `rumble_intensity_right`, and `profile_pending`.
--
commit 9659fef77f1187c96b3944136a0f1b514f35ccfc
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add Rumble Intensity Attributes
This commit adds intensity adjustment for the left and right rumble motors
to the MSI claw driver. It implements sysfs show and store attributes and
handles the hardware profile events for the rumble intensity states.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 4443f53b54cb1..6658d7f2b2ddc 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -403,6 +413,12 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
> drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
> }
>
> + break;
> + case CLAW_RUMBLE_LEFT_PENDING:
> + drvdata->rumble_intensity_left = cmd_rep->data[4];
> + break;
> + case CLAW_RUMBLE_RIGHT_PENDING:
> + drvdata->rumble_intensity_right = cmd_rep->data[4];
> break;
> default:
Is there a data race on rumble_intensity_left and profile_pending here?
The store functions update rumble_intensity_left while holding rom_mutex,
and the show functions set profile_pending while holding profile_mutex.
However, it appears claw_profile_event() can be triggered concurrently by
in-flight hardware responses in the HID event handler, writing to
rumble_intensity_left and clearing profile_pending without holding either
mutex.
[ ... ]
> @@ -822,6 +838,126 @@ static ssize_t button_mapping_options_show(struct device *dev,
> }
[ ... ]
> +static ssize_t rumble_intensity_left_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + u8 data[4] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01 };
> + struct hid_device *hdev = to_hid_device(dev);
> + struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> + int ret;
> +
> + guard(mutex)(&drvdata->profile_mutex);
> + drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
> + ARRAY_SIZE(data), 8);
Can this sequence lead to desynchronization of the hardware state machine?
Since claw_hw_output_report() uses an interruptible wait with a short
timeout (8ms), if the wait times out or is interrupted by a signal, the
hardware will likely continue processing the command.
If a subsequent sysfs operation (like rumble_intensity_right_show) then
sets profile_pending to CLAW_RUMBLE_RIGHT_PENDING, could a delayed ACK
from the previously aborted left rumble request arrive?
Because claw_profile_event() relies solely on the global profile_pending
state without validating payload addresses or transaction IDs, it seems
this delayed ACK would mistakenly apply the left rumble response to the
right rumble variable.
Additionally, could a delayed generic ACK from a timed-out SYNC_TO_ROM
prematurely complete the wait here, causing the show() operation to return
immediately with stale data while leaving profile_pending dangling?
> + if (ret) {
> + drvdata->profile_pending = CLAW_NO_PENDING;
> + return ret;
> + }
> +
> + return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_left);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260517013925.3120314-1-derekjohn.clark@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH v5 3/4] HID: hid-msi: Add RGB control interface
From: sashiko-bot @ 2026-05-17 3:24 UTC (permalink / raw)
To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260517013925.3120314-4-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Holding `cfg_mutex` while removing sysfs attribute groups in `claw_remove()` deadlocks with concurrent sysfs operations.
- [High] Teardown race condition allows `rgb_queue` to be re-queued after cancellation, leading to a Use-After-Free.
- [High] Unconditionally canceling the `rgb_queue` delayed work causes a crash if the setup worker has not executed.
- [Medium] Spurious timeouts due to an 8ms wait limit can break command-response synchronization.
- [Medium] Error handling paths in device initialization log errors using a NULL device pointer.
- [Medium] RGB work can be incorrectly scheduled while the device is suspended.
- [Low] Using a `break` statement inside a `scoped_guard` block creates confusing control flow.
--
commit 4a725c6bd4a5cd260e20d40567af5406130e6b41
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add RGB control interface
Adds RGB control interface for MSI Claw devices, supporting 9 zones
and up to 8 sequential frames of RGB data written to MCU memory.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 991d5a25d3de0..4443f53b54cb1 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -311,15 +375,45 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
> for (i = 0; i < CLAW_KEYS_MAX; i++)
> codes[i] = (cmd_rep->data[6 + i]);
> break;
> + case CLAW_RGB_PENDING:
> + frame = (struct 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->led_mc.led_cdev.dev, "Got unsupported frame index: %x\n",
> + frame_calc);
Since this function is called during the initial setup in
claw_read_rgb_config() before devm_led_classdev_multicolor_register() has
run, is led_cdev.dev potentially NULL here? Will it print an obscured
"(NULL device *)" log message instead of the device name?
Could we use &drvdata->hdev->dev instead?
> + drvdata->profile_pending = CLAW_NO_PENDING;
> + return -EINVAL;
> + }
> + f_idx = frame_calc;
> +
> + scoped_guard(spinlock_irqsave, &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;
This isn't a bug, but using a break inside a scoped_guard escapes the
internal hidden for loop used to implement the macro. It works correctly
here because the lock is dropped and the next line is the switch statement's
own break, but it creates confusing control flow.
Should we use an if (f_idx == 0) { ... } block instead?
> +
> + 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;
[ ... ]
> @@ -769,6 +863,420 @@ 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 < 8; i++) {
> + 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, 8);
Does an 8ms timeout pose a risk of spurious timeouts on systems where HZ=100?
With HZ=100, msecs_to_jiffies(8) evaluates to 1 jiffy. If the system timer
ticks immediately after the wait begins,
wait_for_completion_interruptible_timeout() can return 0 in less than 1ms.
If this happens, does it break command synchronization since the hardware
will eventually ACK the first command?
[ ... ]
> +static void cfg_setup_fn(struct work_struct *work)
> {
> struct delayed_work *dwork = container_of(work, struct delayed_work, work);
> @@ -782,6 +1290,13 @@ static void cfg_setup_fn(struct work_struct *work)
> return;
> }
>
> + ret = claw_read_rgb_config(drvdata->hdev);
> + if (ret) {
> + dev_err(drvdata->led_mc.led_cdev.dev,
> + "Failed to setup device, can't read RGB config: %d\n", ret);
Is it possible led_cdev.dev is NULL here since
devm_led_classdev_multicolor_register() has not been called yet?
> + return;
> + }
> +
> /* Add sysfs attributes after we get the device state */
> ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
> if (ret) {
> @@ -791,7 +1306,25 @@ static void cfg_setup_fn(struct work_struct *work)
> }
> drvdata->gamepad_registered = true;
>
> + /* Add and enable RGB interface once we have the device state */
> + INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
If INIT_DELAYED_WORK is called asynchronously 500ms after probe, what
happens if the device is unplugged or suspended before this timer expires?
Will the unconditional cancel_delayed_work_sync(&drvdata->rgb_queue) in
claw_remove() or claw_suspend() be called on a zero-initialized work
structure, causing undefined behavior and a kernel crash?
[ ... ]
> @@ -918,10 +1475,15 @@ static void claw_remove(struct hid_device *hdev)
> return;
> }
>
> + /* Block writes to brightness/multi_intensity during teardown */
> + drvdata->led_mc.led_cdev.brightness_set = NULL;
> cancel_delayed_work_sync(&drvdata->cfg_setup);
> cancel_delayed_work_sync(&drvdata->cfg_resume);
> + cancel_delayed_work_sync(&drvdata->rgb_queue);
Can rgb_queue be re-queued after this point, leading to a Use-After-Free?
Since cancel_delayed_work_sync() is called before the custom sysfs
attributes are removed via device_remove_group() and before the devm-managed
LED class device is unregistered, a user could write to an RGB sysfs
attribute (like effect or speed) concurrently with or immediately after the
work is canceled. The sysfs callback executes mod_delayed_work() and
re-queues the delayed work.
When claw_remove() finishes, devres frees the drvdata memory. Could the
re-queued work execute later on the freed memory?
>
> guard(mutex)(&drvdata->cfg_mutex);
> + if (drvdata->rgb_registered)
> + device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
> if (drvdata->gamepad_registered)
> device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
Can holding cfg_mutex across these device_remove_group() calls cause an
ABBA deadlock?
device_remove_group() blocks until all active sysfs operations for the
group finish. However, if a sysfs callback (like mkeys_function_store() for
the gamepad group) is executing concurrently, it will call
claw_hw_output_report() which attempts to acquire cfg_mutex.
If this occurs, the sysfs callback will block waiting for cfg_mutex, and
claw_remove() will block waiting for the sysfs callback to complete,
hanging device teardown.
[ ... ]
> @@ -982,6 +1544,7 @@ static int claw_suspend(struct hid_device *hdev)
>
> cancel_delayed_work_sync(&drvdata->cfg_setup);
> cancel_delayed_work_sync(&drvdata->cfg_resume);
> + cancel_delayed_work_sync(&drvdata->rgb_queue);
Can the rgb_queue work be incorrectly re-scheduled while the device is
suspended?
Since the LED class device and custom sysfs attributes remain active, if an
active LED trigger or a userspace sysfs write occurs during suspend, could
mod_delayed_work() re-queue the work and cause the driver to attempt USB
communication with a suspended device?
>
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260517013925.3120314-1-derekjohn.clark@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH v5 2/4] HID: hid-msi: Add M-key mapping attributes
From: sashiko-bot @ 2026-05-17 2:56 UTC (permalink / raw)
To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260517013925.3120314-3-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ABBA deadlock between cfg_mutex and sysfs active references during device removal.
- [High] Race conditions and lockless access to profile_pending cause TOCTOU bugs and mismatched command responses.
- [Low] Redundant local redefinition of core cleanup wrapper for argv_free.
--
commit 1f541fdc7955ce5da7d37b9d253518245001f68a
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add M-key mapping attributes
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.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 3f809dc70a4cc..991d5a25d3de0 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -128,6 +298,30 @@ 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)
> +{
> + u8 *codes;
> + int i;
> +
> + switch (drvdata->profile_pending) {
> + case CLAW_M1_PENDING:
> + case CLAW_M2_PENDING:
> + codes = (drvdata->profile_pending == CLAW_M1_PENDING) ?
> + drvdata->m1_codes : drvdata->m2_codes;
Could this cause a race condition since drvdata->profile_pending is read twice
locklessly?
If the state changes between the switch statement and the ternary operator,
could M1's profile response be incorrectly written to M2's buffer?
Is it possible to use an IRQ-safe spinlock here to protect profile_pending
and the profile buffers, given they are shared between process context and
the HID raw event context?
> + for (i = 0; i < CLAW_KEYS_MAX; i++)
> + codes[i] = (cmd_rep->data[6 + i]);
> + break;
> + default:
> + dev_dbg(&drvdata->hdev->dev,
> + "Got profile event without changes pending from command: %x\n",
> + cmd_rep->cmd);
> + return -EINVAL;
> + }
> + drvdata->profile_pending = CLAW_NO_PENDING;
> +
> + return 0;
> +}
[ ... ]
> @@ -149,6 +343,9 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
> case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
> ret = claw_gamepad_mode_event(drvdata, cmd_rep);
> break;
> + case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
> + ret = claw_profile_event(drvdata, cmd_rep);
> + break;
> case CLAW_COMMAND_TYPE_ACK:
> break;
> default:
Since claw_raw_event() unconditionally calls
complete(&drvdata->send_cmd_complete) for any recognized ACK type after this
switch statement, could it wake up claw_buttons_show() prematurely if a
delayed or generic ACK arrives while it is waiting for READ_PROFILE_ACK?
If claw_buttons_show() wakes up prematurely, could it read uninitialized or
stale data from the codes array and leave profile_pending in a dirty state
that corrupts subsequent reads when the real ACK finally arrives?
[ ... ]
> @@ -373,6 +570,164 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_WO(reset);
>
> +static int button_mapping_name_to_code(const char *name)
> +{
[ ... ]
> +DEFINE_FREE(argv, char **, if (_T) argv_free(_T))
Is it necessary to locally redefine this cleanup wrapper?
The core kernel header include/linux/string.h already provides the exact same
functionality via
DEFINE_FREE(argv_free, char **, if (!IS_ERR_OR_NULL(_T)) argv_free(_T)).
Could this simply be declared as char **raw_keys __free(argv_free) = NULL;
to avoid duplicate code?
> +
> +static int claw_buttons_store(struct device *dev, const char *buf, u8 mkey_idx)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> + u8 data[] = { 0x01, (drvdata->bmap_addr[mkey_idx] >> 8) & 0xff,
> + drvdata->bmap_addr[mkey_idx] & 0xff, 0x07,
> + 0x04, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff };
> + char **raw_keys __free(argv) = NULL;
> + size_t len = ARRAY_SIZE(data);
> + int ret, key_count, i;
[ ... ]
> +set_buttons:
> + scoped_guard(mutex, &drvdata->rom_mutex) {
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> + data, len, 8);
Could this cause an ABBA deadlock during device removal?
During removal, claw_remove() acquires drvdata->cfg_mutex and then calls
device_remove_group() to remove the sysfs attributes. This blocks waiting
for kernfs to drain any active references to the sysfs files.
Meanwhile, this sysfs store method (and the show methods) acquire the exact
same drvdata->cfg_mutex internally via claw_hw_output_report().
If a sysfs attribute is being accessed concurrently when device removal occurs,
will the sysfs method block waiting for cfg_mutex while claw_remove() blocks
waiting for the sysfs method to drop its active reference?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260517013925.3120314-1-derekjohn.clark@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v5 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: sashiko-bot @ 2026-05-17 2:11 UTC (permalink / raw)
To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260517013925.3120314-2-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] ABBA deadlock in `claw_remove` during sysfs group removal.
- [High] Dynamic endpoint querying in `msi_remove` can bypass cleanup on alternate setting changes, leading to a Use-After-Free.
- [High] Unbalanced `hid_hw_close` call in `claw_remove` causes reference count underflow if `drvdata` is NULL.
- [High] Late ACKs from timed-out commands incorrectly fulfill the completion wait of subsequent synchronous commands.
- [Medium] The 8ms timeout for hardware output reports is too short and will randomly fail due to jiffy timer granularity.
- [Medium] System suspend during initial probe permanently prevents sysfs attribute registration.
- [Medium] Kconfig dependency mismatch for LEDS_CLASS causes unmet dependency warnings.
- [Medium] cfg_resume_fn unconditionally forces a destructive USB disconnect on every system resume.
- [Medium] Unprotected data race on `drvdata->gamepad_mode` and `drvdata->mkeys_function` between interrupt and process contexts.
--
commit 518cb80b6362b58a1246f31d9fcaf94c7bfd77ee
Author: Derek J. Clark <derekjohn.clark@gmail.com>
Subject: HID: hid-msi: Add MSI Claw configuration driver
This commit adds a configuration driver for the MSI Claw series of handhelds.
It supports gamepad mode switching, M-key behavior, and a device reset via
sysfs attributes.
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index ff2f580b660ba..ec412bba1e1ed 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -485,6 +485,18 @@ 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 LEDS_CLASS
> + select LEDS_CLASS_MULTICOLOR
Is it possible this will cause an unmet dependency Kconfig warning if NEW_LEDS
is not enabled?
LEDS_CLASS depends on NEW_LEDS, so selecting it without inheriting the
dependency can cause build issues on some configurations.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> new file mode 100644
> index 0000000000000..3f809dc70a4cc
> --- /dev/null
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> +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;
> +
> + drvdata->gamepad_mode = cmd_rep->data[0];
> + drvdata->mkeys_function = cmd_rep->data[1];
Is there a potential data race here?
These fields are written from the raw event callback (which runs in interrupt
context), but they are read in process context by functions like cfg_resume_fn
and gamepad_mode_show without acquiring a shared lock.
> +
> + return 0;
> +}
> +
> +static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
> + u8 *data, int size)
> +{
[ ... ]
> + switch (cmd_rep->cmd) {
> + case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
> + ret = claw_gamepad_mode_event(drvdata, cmd_rep);
> + break;
> + case CLAW_COMMAND_TYPE_ACK:
> + break;
> + default:
> + dev_dbg(&drvdata->hdev->dev, "Unknown command: %x\n", cmd_rep->cmd);
> + return 0;
> + }
> +
> + complete(&drvdata->send_cmd_complete);
If a previous synchronous command timed out, but the device later sends an ACK,
will this unconditionally complete the completion?
When the next command calls reinit_completion(), an immediate late ACK from the
previous command might falsely complete the new command's wait, causing the
driver to read stale state.
> +
> + return ret;
> +}
[ ... ]
> +static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
> + size_t len, unsigned int timeout)
> +{
[ ... ]
> + if (timeout) {
> + ret = wait_for_completion_interruptible_timeout(&drvdata->send_cmd_complete,
> + msecs_to_jiffies(timeout));
Could this timeout prematurely?
When the timeout is very short (like 8ms as used in gamepad_mode_show),
msecs_to_jiffies(8) evaluates to 1 jiffy on HZ=100 kernels.
A 1-jiffy timeout can expire almost instantly if the system jiffy tick occurs
immediately after wait_for_completion_interruptible_timeout() is called,
leading to a spurious -EBUSY.
> +
> + dev_dbg(&hdev->dev, "Remaining timeout: %u\n", ret);
> + if (ret >= 0) /* preserve errors */
> + ret = ret == 0 ? -EBUSY : 0; /* timeout occurred : time remained */
> + }
> +
> + return ret;
> +}
[ ... ]
> +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);
> + u8 data[2] = { drvdata->gamepad_mode, drvdata->mkeys_function };
> + int ret;
> +
> + ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
> + ARRAY_SIZE(data), 0);
The commit message mentions that sending the SWITCH_MODE command causes a USB
disconnect in the device.
Will this unconditional SWITCH_MODE call on every system resume forcibly
disconnect and re-enumerate the gamepad, potentially breaking active
applications holding an open file descriptor?
> + if (ret)
> + dev_err(&drvdata->hdev->dev, "Failed to set gamepad mode settings: %d\n", ret);
> +}
[ ... ]
> +static void claw_remove(struct hid_device *hdev)
> +{
> + struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> +
> + if (!drvdata) {
> + hid_hw_close(hdev);
If drvdata is NULL (meaning claw_probe was never called or failed), will
calling hid_hw_close() underflow the hdev->ll_open_count, since hid_hw_open()
was never successfully called?
> + return;
> + }
> +
> + cancel_delayed_work_sync(&drvdata->cfg_setup);
> + cancel_delayed_work_sync(&drvdata->cfg_resume);
> +
> + guard(mutex)(&drvdata->cfg_mutex);
> + if (drvdata->gamepad_registered)
> + device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
Will holding the cfg_mutex while removing the sysfs group cause an ABBA
deadlock?
The sysfs core's removal blocks until active references to attributes are
drained. If a user concurrently accesses an attribute, the sysfs callback will
try to acquire the cfg_mutex (via claw_hw_output_report).
The driver would hold the mutex waiting for the sysfs reference to drop, while
the callback holds the sysfs reference waiting for the mutex.
> +
> + hid_hw_close(hdev);
> +}
> +
> +static void msi_remove(struct hid_device *hdev)
> +{
> + int ret;
> + u8 ep;
> +
> + 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);
If userspace dynamically changes the USB alternate setting via
USBDEVFS_SETINTERFACE, the endpoint check here could fail and bypass
claw_remove().
If claw_remove() is bypassed, the delayed works (cfg_setup and cfg_resume)
are never cancelled. Could this result in a use-after-free when those works
eventually execute and access the freed drvdata?
> +
> +hw_stop:
> + hid_hw_stop(hdev);
> +}
[ ... ]
> +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_setup);
If the system suspends before the cfg_setup delayed work fires, the work is
synchronously cancelled here.
Since claw_resume schedules cfg_resume but does not reschedule cfg_setup,
will drvdata->gamepad_registered remain false forever, permanently
preventing sysfs attribute registration?
> + cancel_delayed_work_sync(&drvdata->cfg_resume);
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260517013925.3120314-1-derekjohn.clark@gmail.com?part=1
^ permalink raw reply
* [PATCH v5 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-05-17 1:39 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: <20260517013925.3120314-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>
---
v5:
- Remove mkey related changes.
v2:
- Use pending_profile and sync to rom mutexes.
---
drivers/hid/hid-msi.c | 149 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 148 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 4443f53b54cb1..d9951bffc8019 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];
@@ -316,8 +323,11 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
- bool gamepad_registered;
+ u8 rumble_intensity_right;
+ u8 rumble_intensity_left;
+ bool gamepad_registered;
const u16 *bmap_addr;
+ bool rumble_support;
bool bmap_support;
/* RGB Variables */
@@ -403,6 +413,12 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
}
+ break;
+ case CLAW_RUMBLE_LEFT_PENDING:
+ drvdata->rumble_intensity_left = cmd_rep->data[4];
+ break;
+ case CLAW_RUMBLE_RIGHT_PENDING:
+ drvdata->rumble_intensity_right = cmd_rep->data[4];
break;
default:
dev_dbg(&drvdata->hdev->dev,
@@ -822,6 +838,126 @@ 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)
+{
+ u8 data[] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01, 0x00 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ data[4] = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, ARRAY_SIZE(data), 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ drvdata->rumble_intensity_left = val;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_left_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 data[4] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
+ ARRAY_SIZE(data), 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+
+ return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_left);
+}
+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)
+{
+ u8 data[] = { 0x01, (rumble_addr[1] >> 8) & 0xff, rumble_addr[1] & 0xff, 0x01, 0x00 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ data[4] = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, ARRAY_SIZE(data), 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ drvdata->rumble_intensity_right = val;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_right_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 data[4] = { 0x01, (rumble_addr[1] >> 8) & 0xff, rumble_addr[1] & 0xff, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = CLAW_RUMBLE_RIGHT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
+ ARRAY_SIZE(data), 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+
+ return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_right);
+}
+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)
{
@@ -842,6 +978,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;
}
@@ -855,6 +997,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,
};
@@ -1353,6 +1498,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;
@@ -1364,6 +1510,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;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v5 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-05-17 1:39 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: <20260517013925.3120314-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>
---
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 | 573 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 568 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 991d5a25d3de0..4443f53b54cb1 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -21,10 +21,13 @@
#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>
#include <linux/pm.h>
+#include <linux/spinlock.h>
+#include <linux/spinlock_types.h>
#include <linux/sysfs.h>
#include <linux/types.h>
#include <linux/unaligned.h>
@@ -43,6 +46,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_READ_PROFILE = 0x04,
CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
@@ -71,6 +78,7 @@ enum claw_profile_ack_pending {
CLAW_NO_PENDING,
CLAW_M1_PENDING,
CLAW_M2_PENDING,
+ CLAW_RGB_PENDING,
};
enum claw_key_index {
@@ -229,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 */
@@ -239,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];
@@ -247,6 +274,28 @@ struct claw_command_report {
u8 data[59];
} __packed;
+struct rgb_zone {
+ u8 red;
+ u8 green;
+ u8 blue;
+};
+
+struct rgb_frame {
+ struct rgb_zone zone[CLAW_RGB_ZONES];
+};
+
+struct rgb_report {
+ u8 profile;
+ __be16 read_addr;
+ 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;
@@ -258,6 +307,7 @@ struct claw_drvdata {
struct mutex mode_mutex; /* mutex for mode calls */
struct mutex cfg_mutex; /* mutex for synchronous data */
struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
+ spinlock_t frame_lock; /* lock for read/write rgb_frames */
u16 bcd_device;
u8 ep;
@@ -269,6 +319,17 @@ struct claw_drvdata {
bool gamepad_registered;
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;
+ 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)
@@ -300,8 +361,11 @@ 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)
{
- u8 *codes;
- int i;
+ struct rgb_report *frame;
+ u16 rgb_addr, read_addr;
+ u8 *codes, f_idx;
+ u16 frame_calc;
+ int i, ret = 0;
switch (drvdata->profile_pending) {
case CLAW_M1_PENDING:
@@ -311,15 +375,45 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
for (i = 0; i < CLAW_KEYS_MAX; i++)
codes[i] = (cmd_rep->data[6 + i]);
break;
+ case CLAW_RGB_PENDING:
+ frame = (struct 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->led_mc.led_cdev.dev, "Got unsupported frame index: %x\n",
+ frame_calc);
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return -EINVAL;
+ }
+ f_idx = frame_calc;
+
+ scoped_guard(spinlock_irqsave, &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",
cmd_rep->cmd);
return -EINVAL;
}
+
drvdata->profile_pending = CLAW_NO_PENDING;
- return 0;
+ return ret;
}
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
@@ -769,6 +863,420 @@ 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 < 8; i++) {
+ 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, 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ 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 rgb_report report = { 0x01, 0x0000, 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;
+ size_t len = sizeof(report);
+ 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, len, 8);
+ if (ret)
+ return ret;
+
+ /* Increment the write addr by the offset for the next frame */
+ write_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+
+ 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;
+
+ if (!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;
+
+ 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);
+
+ 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;
+
+ 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);
+
+ 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;
+
+ 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;
+
+ 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);
+
+ 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);
@@ -782,6 +1290,13 @@ static void cfg_setup_fn(struct work_struct *work)
return;
}
+ ret = claw_read_rgb_config(drvdata->hdev);
+ if (ret) {
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "Failed to setup device, can't read RGB config: %d\n", ret);
+ return;
+ }
+
/* Add sysfs attributes after we get the device state */
ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
if (ret) {
@@ -791,7 +1306,25 @@ static void cfg_setup_fn(struct work_struct *work)
}
drvdata->gamepad_registered = true;
+ /* Add and enable RGB interface once we have the device state */
+ INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
+ ret = devm_led_classdev_multicolor_register(&drvdata->hdev->dev, &drvdata->led_mc);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create led device: %d\n", ret);
+ return;
+ }
+
+ ret = device_add_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create led attributes: %d\n", ret);
+ return;
+ }
+ drvdata->rgb_registered = true;
+
kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+ kobject_uevent(&drvdata->led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
}
static void cfg_resume_fn(struct work_struct *work)
@@ -801,6 +1334,10 @@ static void cfg_resume_fn(struct work_struct *work)
u8 data[2] = { drvdata->gamepad_mode, drvdata->mkeys_function };
int ret;
+ ret = claw_read_rgb_config(drvdata->hdev);
+ if (ret)
+ dev_err(&drvdata->hdev->dev, "Failed to read RGB config: %d\n", ret);
+
ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
ARRAY_SIZE(data), 0);
if (ret)
@@ -814,18 +1351,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)
@@ -840,6 +1383,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;
@@ -850,10 +1394,22 @@ 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->mode_mutex);
mutex_init(&drvdata->cfg_mutex);
mutex_init(&drvdata->profile_mutex);
mutex_init(&drvdata->rom_mutex);
+ 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);
@@ -864,6 +1420,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
return ret;
hid_set_drvdata(hdev, drvdata);
+
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
return 0;
@@ -918,10 +1475,15 @@ static void claw_remove(struct hid_device *hdev)
return;
}
+ /* Block writes to brightness/multi_intensity during teardown */
+ drvdata->led_mc.led_cdev.brightness_set = NULL;
cancel_delayed_work_sync(&drvdata->cfg_setup);
cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
guard(mutex)(&drvdata->cfg_mutex);
+ if (drvdata->rgb_registered)
+ device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
if (drvdata->gamepad_registered)
device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
@@ -982,6 +1544,7 @@ static int claw_suspend(struct hid_device *hdev)
cancel_delayed_work_sync(&drvdata->cfg_setup);
cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v5 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-05-17 1:39 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: <20260517013925.3120314-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>
---
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 | 401 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 400 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 3f809dc70a4cc..991d5a25d3de0 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -41,6 +41,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_READ_PROFILE = 0x04,
CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
@@ -65,6 +67,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,
@@ -77,6 +90,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_LEFT_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,18 +249,26 @@ struct claw_command_report {
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;
+ struct mutex profile_mutex; /* mutex for profile_pending calls */
struct hid_device *hdev;
struct mutex mode_mutex; /* mutex for mode calls */
struct mutex cfg_mutex; /* mutex for synchronous data */
+ struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
+ 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;
+ const u16 *bmap_addr;
+ bool bmap_support;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -128,6 +298,30 @@ 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)
+{
+ u8 *codes;
+ int i;
+
+ switch (drvdata->profile_pending) {
+ case CLAW_M1_PENDING:
+ case CLAW_M2_PENDING:
+ codes = (drvdata->profile_pending == CLAW_M1_PENDING) ?
+ drvdata->m1_codes : drvdata->m2_codes;
+ for (i = 0; i < CLAW_KEYS_MAX; i++)
+ codes[i] = (cmd_rep->data[6 + i]);
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev,
+ "Got profile event without changes pending from command: %x\n",
+ cmd_rep->cmd);
+ return -EINVAL;
+ }
+ 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)
{
@@ -149,6 +343,9 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
ret = claw_gamepad_mode_event(drvdata, cmd_rep);
break;
+ case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
+ ret = claw_profile_event(drvdata, cmd_rep);
+ break;
case CLAW_COMMAND_TYPE_ACK:
break;
default:
@@ -373,6 +570,164 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(reset);
+static int button_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 *button_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;
+}
+
+DEFINE_FREE(argv, char **, if (_T) argv_free(_T))
+
+static int claw_buttons_store(struct device *dev, const char *buf, u8 mkey_idx)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[] = { 0x01, (drvdata->bmap_addr[mkey_idx] >> 8) & 0xff,
+ drvdata->bmap_addr[mkey_idx] & 0xff, 0x07,
+ 0x04, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff };
+ char **raw_keys __free(argv) = NULL;
+ size_t len = ARRAY_SIZE(data);
+ int ret, key_count, i;
+
+ 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 = button_mapping_name_to_code(raw_keys[i]);
+ if (ret < 0)
+ return ret;
+
+ data[6 + i] = ret;
+ }
+
+set_buttons:
+ scoped_guard(mutex, &drvdata->rom_mutex) {
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, len, 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ }
+
+ return ret;
+}
+
+static int claw_buttons_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);
+ u8 data[] = { 0x01, (drvdata->bmap_addr[m_key] >> 8) & 0xff,
+ drvdata->bmap_addr[m_key] & 0xff, 0x07 };
+ size_t len = ARRAY_SIZE(data);
+ int i, ret, count = 0;
+ const char *name;
+ u8 *codes;
+
+ codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ 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, data, len, 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+ for (i = 0; i < CLAW_KEYS_MAX; i++) {
+ name = button_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_buttons_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_buttons_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_buttons_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_buttons_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);
+
+ 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)
{
@@ -385,10 +740,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,
@@ -440,8 +807,31 @@ static void cfg_resume_fn(struct work_struct *work)
dev_err(&drvdata->hdev->dev, "Failed to set gamepad mode settings: %d\n", ret);
}
+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;
@@ -453,8 +843,17 @@ 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->mode_mutex);
mutex_init(&drvdata->cfg_mutex);
+ mutex_init(&drvdata->profile_mutex);
+ mutex_init(&drvdata->rom_mutex);
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 v5 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek J. Clark @ 2026-05-17 1:39 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: <20260517013925.3120314-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>
---
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 | 12 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 5 +
drivers/hid/hid-msi.c | 630 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 654 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..af146691bd481 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -492,6 +492,18 @@ 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 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..3f809dc70a4cc
--- /dev/null
+++ b/drivers/hid/hid-msi.c
@@ -0,0 +1,630 @@
+// 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/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_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;
+ struct hid_device *hdev;
+ struct mutex mode_mutex; /* mutex for mode calls */
+ struct mutex cfg_mutex; /* mutex for synchronous data */
+ u8 ep;
+
+ /* Gamepad Variables */
+ enum claw_mkeys_function_index mkeys_function;
+ enum claw_gamepad_mode_index gamepad_mode;
+ bool gamepad_registered;
+};
+
+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;
+
+ 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);
+
+ switch (cmd_rep->cmd) {
+ case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
+ ret = claw_gamepad_mode_event(drvdata, cmd_rep);
+ break;
+ case CLAW_COMMAND_TYPE_ACK:
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev, "Unknown command: %x\n", cmd_rep->cmd);
+ return 0;
+ }
+
+ complete(&drvdata->send_cmd_complete);
+
+ 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);
+
+ 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)
+ return ret;
+
+ ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
+ if (ret)
+ return ret;
+
+ 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);
+ if (ret >= 0) /* preserve errors */
+ ret = ret == 0 ? -EBUSY : 0; /* timeout occurred : time remained */
+ }
+
+ 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];
+
+ 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;
+
+ guard(mutex)(&drvdata->mode_mutex);
+ data[0] = ret;
+ 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;
+
+ guard(mutex)(&drvdata->mode_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ 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]);
+ }
+
+ 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];
+
+ 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;
+
+ guard(mutex)(&drvdata->mode_mutex);
+ 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;
+
+ guard(mutex)(&drvdata->mode_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ 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]);
+
+ 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);
+ bool val;
+ int ret;
+
+ 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, 8);
+ 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 = 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;
+ }
+ 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);
+ u8 data[2] = { drvdata->gamepad_mode, drvdata->mkeys_function };
+ int ret;
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
+ ARRAY_SIZE(data), 0);
+ if (ret)
+ dev_err(&drvdata->hdev->dev, "Failed to set gamepad mode settings: %d\n", ret);
+}
+
+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->mode_mutex);
+ mutex_init(&drvdata->cfg_mutex);
+ 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) {
+ hid_hw_close(hdev);
+ return;
+ }
+
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+
+ guard(mutex)(&drvdata->cfg_mutex);
+ if (drvdata->gamepad_registered)
+ device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
+
+ hid_hw_close(hdev);
+}
+
+static void msi_remove(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ 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;
+
+ 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_setup);
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+
+ return 0;
+}
+
+static int msi_suspend(struct hid_device *hdev, pm_message_t msg)
+{
+ int ret;
+ u8 ep;
+
+ 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 v5 0/4] Add MSI Claw HID Configuration Driver
From: Derek J. Clark @ 2026-05-17 1:39 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>
---
v5:
- 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 | 12 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 6 +
drivers/hid/hid-msi.c | 1728 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 1753 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
--
2.53.0
^ 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