Re: [PATCH 2/2] HID: intel-thc-hid: quicki2c: quiesce IRQs during teardown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hid: intel-thc: 1s timeout on HID teardown commands due to early IRQ disable
--

commit 074bce22ab8075bef92df21dcd016a16b939a785
Author: Myeonghun Pak <mhun512@gmail.com>

HID: intel-thc-hid: quicki2c: quiesce IRQs during teardown

This commit disables and synchronizes the IRQ, drains recovery work, and
masks THC interrupts before freeing the IRQ and releasing HID/DMA resources.
It tracks IRQ/DMA initialization for conditional probe cleanup and disables
the IRQ during shutdown as well.

> diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> index 7a913f717efaa..ac073be894817 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
[ ... ]
> @@ -788,9 +798,12 @@ static void quicki2c_remove(struct pci_dev *pdev)
>  	if (!qcdev)
>  		return;
>  
> -	quicki2c_hid_remove(qcdev);
> -
> +	WRITE_ONCE(qcdev->recovery_disabled, true);
> +	disable_irq(pdev->irq);
>  	quicki2c_dev_deinit(qcdev);
> +	devm_free_irq(&pdev->dev, pdev->irq, qcdev);
> +
> +	quicki2c_hid_remove(qcdev);

[Severity: Medium]
Could moving the IRQ disable/free and device deinit before
quicki2c_hid_remove() break any HID raw_requests issued during high-level
driver teardown?

High-level HID drivers (like hid-multitouch) often send final commands
(e.g., turning off LEDs or disabling features) during their remove
callback, which is triggered by hid_destroy_device() inside
quicki2c_hid_remove().

Since the patch moves disable_irq() and devm_free_irq() before
quicki2c_hid_remove(), thc_dma_write() in intel-thc-dma.c will block
for 1 second waiting for an IRQ completion that will never arrive:

	ret = wait_event_interruptible_timeout(dev->write_complete_wait, dev->write_done, 1 * HZ);
	if (ret <= 0 || !dev->write_done) {
		dev_err_once(dev->dev, "timeout for waiting TxDMA completion\n");
		ret = -ETIMEDOUT;
		goto end;
	}

This might unconditionally introduce a 1-second delay per command
during device unbind or system shutdown if the bound high-level HID driver
issues any hardware commands in its remove callback.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011220531.2073892-1-mhun512@gmail.com?part=2