* Re: [PATCH v3] HID: hid-msi: Send a uevent when the MCU changes the mode by itself
2026-11-15 3:23 ` [PATCH v3] " Ana Veto
@ 2026-11-15 3:31 ` sashiko-bot
2026-11-15 14:21 ` [PATCH v4] " Ana Veto
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-11-15 3:31 UTC (permalink / raw)
To: Ana Veto; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] HID: hid-msi: missed uevent and stale state in
claw_switch_mode() race
--
commit cd52e9bad8fc6caa521f438abf30e16af93b21b9
Author: Ana Veto <green.koala2979@fastmail.com>
HID: hid-msi: Send a uevent when the MCU changes the mode by itself
This commit schedules a work item to emit a KOBJ_CHANGE uevent when an
incoming GAMEPAD_MODE_ACK report changes the stored gamepad mode or M-key
function, and updates the stored state after a SWITCH_MODE output report
unless a report arrived during the transfer.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 23d731f700c90..dcbab90f32060 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -387,17 +389,40 @@ static int get_endpoint_address(struct hid_device *hdev)
> return -ENODEV;
> }
>
> +static void claw_mode_event_fn(struct work_struct *work)
> +{
> + struct claw_drvdata *drvdata = container_of(work, struct claw_drvdata, mode_event);
> +
> + kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
> +}
> +
> static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
> struct claw_command_report *cmd_rep)
> {
> + bool changed;
> +
> if (cmd_rep->data[0] >= ARRAY_SIZE(claw_gamepad_mode_text) ||
> !claw_gamepad_mode_text[cmd_rep->data[0]] ||
> cmd_rep->data[1] >= ARRAY_SIZE(claw_mkeys_function_text))
> return -EINVAL;
>
> scoped_guard(spinlock_irqsave, &drvdata->mode_lock) {
> + changed = drvdata->gamepad_mode != cmd_rep->data[0] ||
> + drvdata->mkeys_function != cmd_rep->data[1];
> drvdata->gamepad_mode = cmd_rep->data[0];
> drvdata->mkeys_function = cmd_rep->data[1];
> + drvdata->mode_reports++;
> + }
> +
> + /*
> + * The MCU changes the mode or M-key function by itself, e.g. while
> + * View+LB is held. Userspace learns of changes only through uevents.
> + */
> + if (changed) {
> + guard(spinlock_irqsave)(&drvdata->registration_lock);
> + /* Pairs with smp_store_release from cfg_setup_fn and claw_remove */
> + if (smp_load_acquire(&drvdata->gp_registered))
> + schedule_work(&drvdata->mode_event);
> }
>
> return 0;
> @@ -656,11 +681,14 @@ static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
> static int claw_switch_mode(struct hid_device *hdev, enum claw_mode_field field, u8 val)
> {
> struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> + unsigned int reports;
> u8 data[2];
> + int ret;
>
> guard(mutex)(&drvdata->cfg_mutex);
>
> scoped_guard(spinlock_irqsave, &drvdata->mode_lock) {
> + reports = drvdata->mode_reports;
> switch (field) {
> case CLAW_FIELD_GAMEPAD_MODE:
> data[0] = val;
> @@ -673,8 +701,23 @@ static int claw_switch_mode(struct hid_device *hdev, enum claw_mode_field field,
> }
> }
>
> - return __claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
> - ARRAY_SIZE(data), 0);
> + ret = __claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
> + ARRAY_SIZE(data), 0);
> + if (ret)
> + return ret;
> +
> + /*
> + * The driver's own change is no change to report. A report that came
> + * in during the transfer may already show a newer state; keep it.
> + */
> + scoped_guard(spinlock_irqsave, &drvdata->mode_lock) {
> + if (drvdata->mode_reports == reports) {
> + drvdata->gamepad_mode = data[0];
> + drvdata->mkeys_function = data[1];
> + }
> + }
> +
> + return 0;
> }
[Severity: Medium]
Can a GAMEPAD_MODE_ACK report that arrives during claw_switch_mode()
suppress drvdata->mode_event when it carries the same state that was stored
in drvdata before the write?
When a report sets drvdata->mkeys_function to CLAW_MKEY_FUNCTION_DISABLED,
userspace reacts to the uevent by calling mkeys_function_store() ->
claw_switch_mode() to write CLAW_MKEY_FUNCTION_MACRO. Because
claw_switch_mode() leaves drvdata->mkeys_function at the old value while
__claw_hw_output_report() runs, a second report carrying
CLAW_MKEY_FUNCTION_DISABLED during the transfer sees no change from the
pre-write state:
CPU0 CPU1
claw_switch_mode()
/* mkeys_function is DISABLED */
reports = drvdata->mode_reports;
data[1] = CLAW_MKEY_FUNCTION_MACRO;
__claw_hw_output_report(...)
claw_gamepad_mode_event()
/* cmd_rep->data[1] is DISABLED */
changed = false;
drvdata->mode_reports++;
/* mode_event not scheduled */
/* mode_reports != reports */
/* skips updating drvdata */ <-- no uevent emitted
If that report was sent after the MCU processed SWITCH_MODE (for example,
View+LB overriding M-keys back to disabled), the controller stays disabled
with no KOBJ_CHANGE uevent emitted; if the report was already in flight
before the MCU processed SWITCH_MODE, the MCU switches to macro while
drvdata->mkeys_function remains DISABLED, so the next unsolicited report
reverting to disabled will also see changed == false.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261115032314.7094-1-green.koala2979@fastmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v4] HID: hid-msi: Send a uevent when the MCU changes the mode by itself
2026-11-15 3:23 ` [PATCH v3] " Ana Veto
2026-11-15 3:31 ` sashiko-bot
@ 2026-11-15 14:21 ` Ana Veto
1 sibling, 0 replies; 4+ messages in thread
From: Ana Veto @ 2026-11-15 14:21 UTC (permalink / raw)
To: derekjohn.clark, Jiri Kosina, Benjamin Tissoires
Cc: Ana Veto, linux-input, linux-kernel
The MCU of the MSI Claw changes the M-key function by itself once a
second while View+LB is held, and the controller can end up in desktop
mode, where the sticks drive the mouse. The MCU reports the current
mode and M-key function in a GAMEPAD_MODE_ACK report, unsolicited after
such a change and in reply to a query, and the driver stores both, but
userspace is not told. udev rules that keep the controller usable run
only on add, change and bind (for example InputPlumber's rules that
switch desktop mode back to xinput and set the M-keys to macro), so the
controller stays in desktop mode or with M1/M2 disabled until it is
probed again.
Emit KOBJ_CHANGE from a work item whenever a report changes the stored
mode or M-key function. Store the new values after a successful
SWITCH_MODE write as well, so that the driver's own writes raise no
event. A report that comes in during the write was sent either before
the MCU applied the write or after the MCU changed the state again, so
neither it nor the written values can be trusted: read the state back
and emit the event, so that userspace checks the state.
Tested on an MSI Claw 8 EX AI+ (0db0:1901) with a v7.3.3-based kernel and
InputPlumber's rules. The controller was switched to desktop mode by a
SWITCH_MODE report written to hidraw, bypassing the driver. A read of
gamepad_mode then returned desktop and raised a change uevent, and the
rule wrote xinput 66 ms later. After another such switch, View+LB made
the MCU report mode 4 by itself, and the rule wrote xinput within
0.3 s. While View+LB was held in xinput mode, the MCU reported the
M-keys disabled once a second, and the M-key rule set macro again each
time. A mode report with the state from before a SWITCH_MODE write was
also made to arrive during the write, by a query written to hidraw while
the driver waited to send it: the driver read the state back and raised
an event, and the next change of the M-key function by the MCU raised
one as well.
Fixes: a9517958e828 ("HID: hid-msi: Add MSI Claw configuration driver")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Ana Veto <green.koala2979@fastmail.com>
---
Notes:
Based on hid.git for-next. This changes lines near "HID: hid-msi: Drain orphan MCU ACKs by issuing a query" (v2, posted 2026-11-14); the two patches apply in either order.
v4: if a GAMEPAD_MODE_ACK comes in during the SWITCH_MODE write, the driver now reads the state back and emits the uevent. In v3 such a report raised no event when it carried the state from before the write, and the stored state could stay wrong, so a later change by the MCU could go unreported as well (Sashiko review: https://lore.kernel.org/r/sashiko-outbox-215202@kernel.org). Tested on the Claw with the report forced into the write: with v3, no event in 2 of 2 runs; with v4, an event in 4 of 4.
v3: https://lore.kernel.org/r/20261115032314.7094-1-green.koala2979@fastmail.com
v2: https://lore.kernel.org/r/20261114221440.12580-1-green.koala2979@fastmail.com
drivers/hid/hid-msi.c | 65 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 63 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 3225d3b..6f14647 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -350,6 +350,8 @@ struct claw_drvdata {
/* Gamepad Variables */
enum claw_mkeys_function_index mkeys_function;
enum claw_gamepad_mode_index gamepad_mode;
+ struct work_struct mode_event; /* uevent when the MCU changes the mode */
+ unsigned int mode_reports; /* Mode reports stored, guarded by mode_lock */
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
u8 rumble_intensity_right;
@@ -387,19 +389,46 @@ static int get_endpoint_address(struct hid_device *hdev)
return -ENODEV;
}
+static void claw_mode_event_fn(struct work_struct *work)
+{
+ struct claw_drvdata *drvdata = container_of(work, struct claw_drvdata, mode_event);
+
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+}
+
+static void claw_queue_mode_event(struct claw_drvdata *drvdata)
+{
+ guard(spinlock_irqsave)(&drvdata->registration_lock);
+ /* Pairs with smp_store_release from cfg_setup_fn and claw_remove */
+ if (smp_load_acquire(&drvdata->gp_registered))
+ schedule_work(&drvdata->mode_event);
+}
+
static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
struct claw_command_report *cmd_rep)
{
+ bool changed;
+
if (cmd_rep->data[0] >= ARRAY_SIZE(claw_gamepad_mode_text) ||
!claw_gamepad_mode_text[cmd_rep->data[0]] ||
cmd_rep->data[1] >= ARRAY_SIZE(claw_mkeys_function_text))
return -EINVAL;
scoped_guard(spinlock_irqsave, &drvdata->mode_lock) {
+ changed = drvdata->gamepad_mode != cmd_rep->data[0] ||
+ drvdata->mkeys_function != cmd_rep->data[1];
drvdata->gamepad_mode = cmd_rep->data[0];
drvdata->mkeys_function = cmd_rep->data[1];
+ drvdata->mode_reports++;
}
+ /*
+ * The MCU changes the mode or M-key function by itself, e.g. while
+ * View+LB is held. Userspace learns of changes only through uevents.
+ */
+ if (changed)
+ claw_queue_mode_event(drvdata);
+
return 0;
}
@@ -656,11 +685,15 @@ static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
static int claw_switch_mode(struct hid_device *hdev, enum claw_mode_field field, u8 val)
{
struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ unsigned int reports;
+ bool raced;
u8 data[2];
+ int ret;
guard(mutex)(&drvdata->cfg_mutex);
scoped_guard(spinlock_irqsave, &drvdata->mode_lock) {
+ reports = drvdata->mode_reports;
switch (field) {
case CLAW_FIELD_GAMEPAD_MODE:
data[0] = val;
@@ -673,8 +706,32 @@ static int claw_switch_mode(struct hid_device *hdev, enum claw_mode_field field,
}
}
- return __claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
- ARRAY_SIZE(data), 0);
+ ret = __claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
+ ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ /* The driver's own change is no change to report */
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock) {
+ raced = drvdata->mode_reports != reports;
+ if (!raced) {
+ drvdata->gamepad_mode = data[0];
+ drvdata->mkeys_function = data[1];
+ }
+ }
+
+ /*
+ * A report came in during the transfer. The MCU sent it either before
+ * it applied the write or after it changed the state again by itself,
+ * so the stored state may be wrong and a change may have gone
+ * unreported. Read the state back and let userspace check it.
+ */
+ if (raced) {
+ __claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 25);
+ claw_queue_mode_event(drvdata);
+ }
+
+ return 0;
}
static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
@@ -1871,6 +1928,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
+ INIT_WORK(&drvdata->mode_event, &claw_mode_event_fn);
/* For control interface: open the HID transport for sending commands. */
ret = hid_hw_open(hdev);
@@ -1946,6 +2004,9 @@ static void claw_remove(struct hid_device *hdev)
smp_store_release(&drvdata->rgb_registered, false);
}
+ /* gp_registered is false now, so no new mode event can be queued */
+ cancel_work_sync(&drvdata->mode_event);
+
if (gp_registered)
device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
--
2.56.0
^ permalink raw reply related [flat|nested] 4+ messages in thread