Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] HID: hid-msi: race in claw_suspend() lets orphan_ack_work run
after suspend
- [Medium] HID: hid-msi: orphan ACK in claw_raw_event() completes read
commands early
--
commit d1c0d0f5c28e7bbec3449fd155157b8c3178c746
Author: Kenhou Iijima [off-list ref]
HID: hid-msi: Drain orphan MCU ACKs by issuing a query
This commit schedules a work item to issue a READ_GAMEPAD_MODE query when an
unexpected ACK arrives from the MSI Claw MCU so the MCU stops re-sending the
pending ACK.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 23d731f700c90..e52923fcc8260 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -532,7 +533,12 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
}
if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_NONE) {
- dev_warn(&drvdata->hdev->dev, "Got unexpected ACK from MCU, ignoring\n");
+ /*
+ * The MCU re-sends an unhandled ACK roughly every 2 s
+ * until it receives another command. Drain it by
+ * scheduling a harmless query so the ACK storm stops.
+ */
+ schedule_work(&drvdata->orphan_ack_work);
break;
}
[Severity: Medium]
Can an orphan CLAW_COMMAND_TYPE_ACK arriving while a read command is in flight
complete drvdata->send_cmd_complete early?
__claw_hw_output_report() sets drvdata->waiting_cmd to
CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE or CLAW_COMMAND_TYPE_READ_PROFILE before
transmitting the request. If a re-transmitted CLAW_COMMAND_TYPE_ACK arrives
before the read response (CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK or
CLAW_COMMAND_TYPE_READ_PROFILE_ACK), waiting_cmd is not
CLAW_COMMAND_TYPE_NONE, so claw_raw_event() falls through, sets
drvdata->cmd_status = 0, and completes drvdata->send_cmd_complete. Callers
such as gamepad_mode_show() and claw_mkey_show() then return stale or
uninitialized data while leaving drvdata->profile_pending set.
In addition, when the initial ACK for a timeout-0 command arrives with
drvdata->orphan_ack_pending set, the earlier branch clears orphan_ack_pending
and breaks without scheduling orphan_ack_work, leaving the MCU's pending ACK
state undrained so it still re-transmits 2 seconds later.
quoted hunk ↗ jump to hunk
@@ -653,6 +659,20 @@ static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
return __claw_hw_output_report(hdev, index, data, len, timeout);
}
+static void claw_orphan_ack_drain(struct work_struct *work)
+{
+ struct claw_drvdata *drvdata = container_of(work, struct claw_drvdata,
+ orphan_ack_work);
+
+ /*
+ * A query is enough to make the MCU stop re-sending the pending ACK.
+ * claw_hw_output_report() also waits for any orphan ACK before
+ * sending, so this cannot interleave with a real command sequence.
+ */
+ claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE,
+ NULL, 0, 25);
+}
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2014,6 +2036,7 @@ static int claw_suspend(struct hid_device *hdev)
cancel_delayed_work_sync(&drvdata->cfg_resume);
cancel_delayed_work_sync(&drvdata->cfg_setup);
+ cancel_work_sync(&drvdata->orphan_ack_work);
cancel_delayed_work_sync(&drvdata->rgb_queue);
return 0;
[Severity: Medium]
Can claw_raw_event() re-schedule drvdata->orphan_ack_work after claw_suspend()
cancels it?
In drivers/hid/usbhid/hid-core.c, hid_suspend() runs hid_driver_suspend()
before hid_cease_io() stops usbhid->urbin, and claw_suspend() also cancels
orphan_ack_work before cancel_delayed_work_sync(&drvdata->rgb_queue) (where
claw_rgb_queue_fn() sends CLAW_COMMAND_TYPE_SYNC_TO_ROM with timeout 0):
CPU0 CPU1
hid_suspend()
...
claw_suspend()
cancel_work_sync(&drvdata->orphan_ack_work)
claw_raw_event()
schedule_work(&drvdata->orphan_ack_work)
cancel_delayed_work_sync(&drvdata->rgb_queue)
...
hid_cease_io(usbhid)
claw_orphan_ack_drain()
claw_hw_output_report()
...
usb_interrupt_msg() <-- I/O while suspended
Because nothing prevents claw_raw_event() from queueing orphan_ack_work during
suspend, claw_orphan_ack_drain() can run and issue synchronous USB output
transfers against a suspending or already-suspended device after usbhid->urbin
has been stopped.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261114101045.21986-1-kenhou@kenhou.net?part=1