[PATCH] HID: hid-msi: Drain orphan MCU ACKs by issuing a query

[PATCH] HID: hid-msi: Drain orphan MCU ACKs by issuing a query

[Kenhou Iijima]

The Claw MCU re-sends an unhandled ACK roughly every 2 seconds until it
receives another command. When an ACK arrives with waiting_cmd set to
CLAW_COMMAND_TYPE_NONE, such as after a command is written through
hidraw or once orphan_ack_pending has been cleared without a subsequent
command, the driver currently logs "Got unexpected ACK from MCU,
ignoring" on every re-transmission and never clears the MCU's pending
ACK state.

Schedule a work item that issues a harmless READ_GAMEPAD_MODE query when
an unexpected ACK arrives. Receiving any command causes the MCU to stop
re-sending the pending ACK, eliminating the repeated events and log
spam.

Fixes: a9517958e828 ("HID: hid-msi: Add MSI Claw configuration driver")
Signed-off-by: Kenhou Iijima <kenhou@kenhou.net>
---
Tested on an MSI Claw 8 AI+ A2VM (0db0:1901, bcdDevice 2.30).

 drivers/hid/hid-msi.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 23d731f700..e52923fcc8 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -332,6 +332,7 @@ struct claw_drvdata {
 	enum claw_profile_ack_pending profile_pending;
 	struct completion orphan_ack_complete;
 	struct completion send_cmd_complete;
+	struct work_struct orphan_ack_work;
 	struct delayed_work cfg_resume;
 	struct delayed_work cfg_setup;
 	spinlock_t registration_lock; /* Lock for registration read/write */
@@ -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;
 		}
 
@@ -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);
+}
+
 static int claw_switch_mode(struct hid_device *hdev, enum claw_mode_field field, u8 val)
 {
 	struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -1868,6 +1888,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
 	spin_lock_init(&drvdata->rumble_lock);
 	init_completion(&drvdata->orphan_ack_complete);
 	init_completion(&drvdata->send_cmd_complete);
+	INIT_WORK(&drvdata->orphan_ack_work, &claw_orphan_ack_drain);
 	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);
@@ -1934,6 +1955,7 @@ static void claw_remove(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);
 
 	scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
 		/* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
@@ -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;
-- 
2.56.0

Re: [PATCH] HID: hid-msi: Drain orphan MCU ACKs by issuing a query

[sashiko-bot]

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 <kenhou@kenhou.net>

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.

> 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

[ ... ]

> @@ -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.

> @@ -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);
> +}

[ ... ]

> @@ -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

[PATCH v2] HID: hid-msi: Drain orphan MCU ACKs by issuing a query

[Kenhou Iijima]

On the MSI Claw 8 AI+ (0db0:1901, firmware 2.30), when a timeout-0
command such as SYNC_TO_ROM (0x22) or SWITCH_MODE (0x24) is issued
(either by the driver or from userspace via hidraw), the controller MCU
sends an asynchronous 0x06 ACK. Unlike regular command responses, the MCU
firmware keeps re-sending this 0x06 ACK approximately every 2 seconds
indefinitely until it receives another feature command (such as
READ_GAMEPAD_MODE).

When issued from userspace via hidraw while hid-msi is bound, the
driver never arms orphan_ack_pending, so every 0x06 ACK arrives with
waiting_cmd == CLAW_COMMAND_TYPE_NONE. When issued in-kernel with
timeout == 0, the initial ACK clears orphan_ack_pending if it arrives
within the 25 ms orphan_ack_complete window (whereas a SYNC_TO_ROM flash
commit ACK arrives ~560 ms later, after a subsequent command or its
timeout clears orphan_ack_pending), and all subsequent 2-second retries
likewise arrive with waiting_cmd != CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA.
Because nothing sends a follow-up command when the device is otherwise
idle, the MCU stays stuck in its ACK-retry state:

1. An endless stream of 0x06 packets is delivered over the USB interrupt
   IN endpoint every 2 seconds until the next command is sent.
2. Only WRITE_PROFILE_DATA (0x21) expects a generic 0x06 ACK; read
   commands wait for their own ACK command types. Without guarding
   CLAW_COMMAND_TYPE_ACK against other waiting commands, a stray or
   delayed 0x06 ACK arriving while READ_PROFILE or READ_GAMEPAD_MODE is
   in flight completes send_cmd_complete early with an empty response
   buffer, corrupting the read result.

Fix this by:
- Scheduling a work item (orphan_ack_work) that issues a harmless
  CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE query whenever an unhandled 0x06
  ACK arrives (whether caught by orphan_ack_pending or when waiting_cmd
  is not CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA), which clears the MCU's
  pending ACK state so it stops re-transmitting 0x06 every 2 seconds.
- Completing send_cmd_complete on CLAW_COMMAND_TYPE_ACK only when
  waiting_cmd is CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA, and draining any
  other 0x06 ACK via orphan_ack_work instead.
- Cancelling orphan_ack_work via disable_work_sync() in claw_remove()
  and msi_suspend(), and re-enabling it via enable_work() in
  msi_resume(). Set .reset_resume = pm_ptr(msi_resume) as well, because
  usbhid defines .reset_resume without falling back to
  hdev->driver->resume in hid_driver_reset_resume(), so reset-resume
  paths also need msi_resume() to re-enable orphan_ack_work.

Fixes: a9517958e828 ("HID: hid-msi: Add MSI Claw configuration driver")
Signed-off-by: Kenhou Iijima <kenhou@kenhou.net>
---
Changes in v2:
- Guard CLAW_COMMAND_TYPE_ACK so send_cmd_complete is only completed
  when waiting_cmd == CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA, avoiding
  early completion of in-flight reads; schedule orphan_ack_work (with a
  dev_dbg log) for any other 0x06 ACK (whether caught by
  orphan_ack_pending or arriving later) instead of waiting for a 2 s
  retry.
- Use disable_work_sync() in claw_remove() and claw_suspend() paired
  with enable_work() in claw_resume(), and wire .reset_resume to
  msi_resume so urbin interrupts cannot re-queue work across suspend,
  reset-resume, or removal.
- Document why only WRITE_PROFILE_DATA expects 0x06 and why drain
  errors (e.g. during RESET_DEVICE or disconnect) are ignored.
- Link to v1: https://lore.kernel.org/linux-input/20261114101045.21986-1-kenhou@kenhou.net/

 drivers/hid/hid-msi.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 23d731f700..054babadda 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -332,6 +332,7 @@ struct claw_drvdata {
 	enum claw_profile_ack_pending profile_pending;
 	struct completion orphan_ack_complete;
 	struct completion send_cmd_complete;
+	struct work_struct orphan_ack_work;
 	struct delayed_work cfg_resume;
 	struct delayed_work cfg_setup;
 	spinlock_t registration_lock; /* Lock for registration read/write */
@@ -528,11 +529,20 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
 		if (drvdata->orphan_ack_pending) {
 			drvdata->orphan_ack_pending = false;
 			complete(&drvdata->orphan_ack_complete);
-			break;
 		}
 
-		if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_NONE) {
-			dev_warn(&drvdata->hdev->dev, "Got unexpected ACK from MCU, ignoring\n");
+		if (drvdata->waiting_cmd != CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA) {
+			/*
+			 * Only WRITE_PROFILE_DATA waits for a generic 0x06 ACK;
+			 * reads wait for their own ACK command type. Any other
+			 * 0x06 ACK is an unhandled orphan ACK that the MCU will
+			 * re-send roughly every 2 s until it receives another
+			 * command, so schedule a harmless query to drain it.
+			 */
+			dev_dbg(&drvdata->hdev->dev,
+				"Draining orphan ACK (waiting CMD: %x)\n",
+				drvdata->waiting_cmd);
+			schedule_work(&drvdata->orphan_ack_work);
 			break;
 		}
 
@@ -653,6 +663,22 @@ 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.
+	 * Best-effort drain: errors (e.g. during RESET_DEVICE or disconnect)
+	 * are harmless and intentionally ignored.
+	 */
+	claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE,
+			      NULL, 0, 25);
+}
+
 static int claw_switch_mode(struct hid_device *hdev, enum claw_mode_field field, u8 val)
 {
 	struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -1868,6 +1894,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
 	spin_lock_init(&drvdata->rumble_lock);
 	init_completion(&drvdata->orphan_ack_complete);
 	init_completion(&drvdata->send_cmd_complete);
+	INIT_WORK(&drvdata->orphan_ack_work, &claw_orphan_ack_drain);
 	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);
@@ -1954,6 +1981,7 @@ static void claw_remove(struct hid_device *hdev)
 		led_classdev_multicolor_unregister(&drvdata->led_mc);
 	}
 	cancel_delayed_work_sync(&drvdata->rgb_queue);
+	disable_work_sync(&drvdata->orphan_ack_work);
 
 	hid_hw_close(hdev);
 }
@@ -1983,6 +2011,8 @@ static int claw_resume(struct hid_device *hdev)
 	if (!drvdata)
 		return -ENODEV;
 
+	enable_work(&drvdata->orphan_ack_work);
+
 	/* MCU can take up to 500ms to be ready after resume */
 	schedule_delayed_work(&drvdata->cfg_resume, msecs_to_jiffies(500));
 	return 0;
@@ -2015,6 +2045,7 @@ static int claw_suspend(struct hid_device *hdev)
 	cancel_delayed_work_sync(&drvdata->cfg_resume);
 	cancel_delayed_work_sync(&drvdata->cfg_setup);
 	cancel_delayed_work_sync(&drvdata->rgb_queue);
+	disable_work_sync(&drvdata->orphan_ack_work);
 
 	return 0;
 }
@@ -2052,6 +2083,7 @@ static struct hid_driver msi_driver = {
 	.probe		= msi_probe,
 	.remove		= msi_remove,
 	.resume		= pm_ptr(msi_resume),
+	.reset_resume	= pm_ptr(msi_resume),
 	.suspend	= pm_ptr(msi_suspend),
 };
 module_hid_driver(msi_driver);
-- 
2.56.0