[PATCH v2 0/2] HID: intel-thc-hid: quiesce IRQs during teardown

[PATCH v2 0/2] HID: intel-thc-hid: quiesce IRQs during teardown

[Myeonghun Pak]

Rebase the original QuickSPI and QuickI2C teardown patches on mainline
b3dfbde62cf1981c4638939c159dfd087b7a72e9. The source additions and deletions
are unchanged from the original submission.

No hardware testing.

Changes in v2:
- Rebase on latest mainline.

 .../intel-thc-hid/intel-quicki2c/pci-quicki2c.c    | 28 +++++++++++++++++-----
 .../intel-thc-hid/intel-quickspi/pci-quickspi.c    | 28 +++++++++++++++++-----
 2 files changed, 44 insertions(+), 12 deletions(-)

base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
-- 
2.53.0

[PATCH v2 1/2] HID: intel-thc-hid: quickspi: quiesce IRQs during teardown

[Myeonghun Pak]

The managed IRQ can access HID/DMA buffers or queue recovery work while
remove or probe unwind tears down those resources.

Disable and synchronize the IRQ, drain recovery work, then mask THC
interrupts before freeing the IRQ and releasing HID/DMA resources.
Track IRQ/DMA initialization for conditional probe cleanup.

Disable the IRQ before shutdown deinit as well. Keep IRQ disabling and
release outside the shared hibernation helper.

This issue was found by our static analysis tool under development.

Fixes: 4138f21115ae ("HID: intel-thc-hid: intel-quickspi: Complete THC QuickSPI driver")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
No hardware testing.

Changes in v2:
- Rebase on mainline b3dfbde62cf1. No source changes from v1.

 .../intel-thc-hid/intel-quickspi/pci-quickspi.c    | 28 +++++++++++++++++-----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
index bf49d8ce5f07bed092339e46c444538d78cffe26..acb4d2716ea7450d2852a900c3f0acc1d7570421 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -600,6 +600,8 @@ static int quickspi_probe(struct pci_dev *pdev,
 			  const struct pci_device_id *id)
 {
 	struct quickspi_device *qsdev;
+	bool dma_initialized = false;
+	bool irq_requested = false;
 	void __iomem *mem_addr;
 	int ret;
 
@@ -652,6 +654,7 @@ static int quickspi_probe(struct pci_dev *pdev,
 					qsdev);
 	if (ret)
 		goto dev_deinit;
+	irq_requested = true;
 
 	ret = reset_tic(qsdev);
 	if (ret) {
@@ -670,17 +673,18 @@ static int quickspi_probe(struct pci_dev *pdev,
 		dev_err(&pdev->dev, "Setup THC DMA failed, ret= %d\n", ret);
 		goto dev_deinit;
 	}
+	dma_initialized = true;
 
 	ret = quickspi_get_report_descriptor(qsdev);
 	if (ret) {
 		dev_err(&pdev->dev, "Get report descriptor failed, ret = %d\n", ret);
-		goto dma_deinit;
+		goto dev_deinit;
 	}
 
 	ret = quickspi_hid_probe(qsdev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register HID device, ret = %d\n", ret);
-		goto dma_deinit;
+		goto dev_deinit;
 	}
 
 	qsdev->state = QUICKSPI_ENABLED;
@@ -695,10 +699,16 @@ static int quickspi_probe(struct pci_dev *pdev,
 
 	return 0;
 
-dma_deinit:
-	quickspi_dma_deinit(qsdev);
 dev_deinit:
+	if (irq_requested) {
+		WRITE_ONCE(qsdev->recovery_disabled, true);
+		disable_irq(pdev->irq);
+	}
 	quickspi_dev_deinit(qsdev);
+	if (irq_requested)
+		devm_free_irq(&pdev->dev, pdev->irq, qsdev);
+	if (dma_initialized)
+		quickspi_dma_deinit(qsdev);
 disable_pci_device:
 	pci_clear_master(pdev);
 
@@ -721,9 +731,12 @@ static void quickspi_remove(struct pci_dev *pdev)
 	if (!qsdev)
 		return;
 
-	quickspi_hid_remove(qsdev);
-
+	WRITE_ONCE(qsdev->recovery_disabled, true);
+	disable_irq(pdev->irq);
 	quickspi_dev_deinit(qsdev);
+	devm_free_irq(&pdev->dev, pdev->irq, qsdev);
+
+	quickspi_hid_remove(qsdev);
 
 	quickspi_dma_deinit(qsdev);
 
@@ -750,6 +763,9 @@ static void quickspi_shutdown(struct pci_dev *pdev)
 	if (!qsdev)
 		return;
 
+	WRITE_ONCE(qsdev->recovery_disabled, true);
+	disable_irq(pdev->irq);
+
 	quickspi_dev_deinit(qsdev);
 
 	/* Must stop DMA before reboot to avoid DMA entering into unknown state */

base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
-- 
2.53.0

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

[Myeonghun Pak]

The managed IRQ can access HID/DMA buffers or queue recovery work while
remove or probe unwind tears down those resources.

Disable and synchronize the IRQ, drain recovery work, then mask THC
interrupts before freeing the IRQ and releasing HID/DMA resources.
Track IRQ/DMA initialization for conditional probe cleanup.

Disable the IRQ before shutdown deinit as well. Keep IRQ disabling and
release outside the shared hibernation helper.

This issue was found by our static analysis tool under development.

Fixes: 66b59bfce6d9 ("HID: intel-thc-hid: intel-quicki2c: Complete THC QuickI2C driver")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
No hardware testing.

Changes in v2:
- Rebase on mainline b3dfbde62cf1. No source changes from v1.

 .../intel-thc-hid/intel-quicki2c/pci-quicki2c.c    | 28 +++++++++++++++++-----
 1 file changed, 22 insertions(+), 6 deletions(-)

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 29319b6227f3f6b14f3206d15b544b050b9bad1c..a3e98e0faff83ba90efcbfd244ed641688718342 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
@@ -652,6 +652,8 @@ static int quicki2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	const struct quicki2c_ddata *ddata = (const struct quicki2c_ddata *)id->driver_data;
 	struct quicki2c_device *qcdev;
+	bool dma_initialized = false;
+	bool irq_requested = false;
 	void __iomem *mem_addr;
 	int ret;
 
@@ -704,6 +706,7 @@ static int quicki2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 					qcdev);
 	if (ret)
 		goto dev_deinit;
+	irq_requested = true;
 
 	ret = quicki2c_get_device_descriptor(qcdev);
 	if (ret) {
@@ -722,6 +725,7 @@ static int quicki2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		dev_err(&pdev->dev, "Setup THC DMA failed, ret= %d\n", ret);
 		goto dev_deinit;
 	}
+	dma_initialized = true;
 
 	ret = thc_interrupt_quiesce(qcdev->thc_hw, false);
 	if (ret)
@@ -742,13 +746,13 @@ static int quicki2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	ret = quicki2c_get_report_descriptor(qcdev);
 	if (ret) {
 		dev_err(&pdev->dev, "Get report descriptor failed, ret = %d\n", ret);
-		goto dma_deinit;
+		goto dev_deinit;
 	}
 
 	ret = quicki2c_hid_probe(qcdev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register HID device, ret = %d\n", ret);
-		goto dma_deinit;
+		goto dev_deinit;
 	}
 
 	qcdev->state = QUICKI2C_ENABLED;
@@ -763,10 +767,16 @@ static int quicki2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	return 0;
 
-dma_deinit:
-	quicki2c_dma_deinit(qcdev);
 dev_deinit:
+	if (irq_requested) {
+		WRITE_ONCE(qcdev->recovery_disabled, true);
+		disable_irq(pdev->irq);
+	}
 	quicki2c_dev_deinit(qcdev);
+	if (irq_requested)
+		devm_free_irq(&pdev->dev, pdev->irq, qcdev);
+	if (dma_initialized)
+		quicki2c_dma_deinit(qcdev);
 disable_pci_device:
 	pci_clear_master(pdev);
 
@@ -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);
 
 	quicki2c_dma_deinit(qcdev);
 
@@ -815,6 +828,9 @@ static void quicki2c_shutdown(struct pci_dev *pdev)
 	if (!qcdev)
 		return;
 
+	WRITE_ONCE(qcdev->recovery_disabled, true);
+	disable_irq(pdev->irq);
+
 	quicki2c_dev_deinit(qcdev);
 
 	/* Must stop DMA before reboot to avoid DMA entering into unknown state */

base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
-- 
2.53.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] HID: intel-thc-hid: quicki2c: premature IRQ/dev teardown in
  quicki2c_remove()
--

commit 91b9650f43556a025a9bb311fa9a30624bcb700a
Author: Myeonghun Pak <mhun512@gmail.com>

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

This commit disables and frees the device IRQ, cancels recovery work, and
quiesces THC interrupts during probe error unwind, removal, and shutdown.

> 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 29319b6227f3f..a3e98e0faff83 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);
>  
>  	quicki2c_dma_deinit(qcdev);

[Severity: High]
Does disabling and freeing pdev->irq and calling quicki2c_dev_deinit()
before quicki2c_hid_remove() break synchronous HID report transfers issued
while unregistering qcdev->hid_dev?

While hid_destroy_device() runs, bound HID drivers and open hidraw clients
still invoke quicki2c_hid_raw_request() or quicki2c_hid_output_report().
For example, when an input device is open during removal, hid-multitouch
calls hid_hw_close() -> mt_on_hid_hw_close() -> mt_set_modes(), which sends
HID_REQ_SET_REPORT and HID_REQ_GET_REPORT via thc_dma_write() and
thc_swdma_read().

Because pdev->irq is already disabled and freed and THC interrupts are
masked, wait_event_interruptible_timeout() on dev->write_complete_wait and
dev->swdma_complete_wait can never be woken, stalling each transfer for 1
second until it fails with -ETIMEDOUT.

On timeout, thc_swdma_read() also skips thc_swdma_read_completion(), and
thc_dma_unconfigure() only clears THC_M_PRT_READ_DMA_CNTRL_START on
THC_RXDMA1 and THC_RXDMA2, leaving THC_SWDMA or THC_TXDMA started when
thc_dma_release() unmaps and frees the DMA buffers:

drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:thc_swdma_read() {
	...
	ret = thc_swdma_read_start(dev, write_buff, write_len, prd_tbl_len);
	if (ret)
		goto end;

	ret = wait_event_interruptible_timeout(dev->swdma_complete_wait, dev->swdma_done, 1 * HZ);
	if (ret <= 0 || !dev->swdma_done) {
		dev_err_once(dev->dev, "timeout for waiting SWDMA completion\n");
		ret = -ETIMEDOUT;
		goto end;
	}
	...
	ret = thc_swdma_read_completion(dev);
	...
}

Conversely, if an in-flight hidraw GET_REPORT receives its THC_SWDMA_INT
wake-up just before disable_irq(), thc_swdma_read_completion() runs without
serialization against quicki2c_dev_deinit() and restarts THC_RXDMA2 and
unquiesces device interrupts after quicki2c_dev_deinit() has completed.

Calling quicki2c_hid_remove() before disabling the IRQ and running
quicki2c_dev_deinit() avoids starting or completing HID transfers after the
interrupt and hardware have been torn down.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-thc-rebase-v2-0-mhun512@gmail.com?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] HID: intel-thc-hid: quickspi: broken HID teardown in
  quickspi_remove()
--

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

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

This commit disables and frees the device IRQ and stops recovery work
before tearing down HID and DMA resources during probe error unwind,
driver removal, and shutdown.

> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> index bf49d8ce5f07b..acb4d2716ea74 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c

[ ... ]

> @@ -721,9 +731,12 @@ static void quickspi_remove(struct pci_dev *pdev)
>  	if (!qsdev)
>  		return;
>  
> -	quickspi_hid_remove(qsdev);
> -
> +	WRITE_ONCE(qsdev->recovery_disabled, true);
> +	disable_irq(pdev->irq);
>  	quickspi_dev_deinit(qsdev);
> +	devm_free_irq(&pdev->dev, pdev->irq, qsdev);
> +
> +	quickspi_hid_remove(qsdev);
>  
>  	quickspi_dma_deinit(qsdev);

[Severity: Medium]
Does moving quickspi_hid_remove() after disable_irq(),
quickspi_dev_deinit(), and devm_free_irq() break synchronous HID requests
issued during hid_destroy_device()?

When a hid-multitouch input device is open during unbind,
hid_destroy_device() closes it synchronously via mt_on_hid_hw_close() ->
mt_set_modes(), which calls quickspi_hid_raw_request() ->
quickspi_set_report() -> write_cmd_to_txdma() -> thc_dma_write(). Because
neither quickspi_hid_raw_request() nor thc_dma_write() checks qsdev->state
and THC_TXDMA remains enabled until quickspi_dma_deinit(), thc_dma_write()
starts hardware TxDMA and stalls until timeout waiting for a THC_TXDMA_INT
completion interrupt that can no longer fire.

In addition, when thc_dma_write() times out or is interrupted by a signal,
its error path calls thc_interrupt_quiesce(dev, false) if dev->perf_limit
is set (re-enabling external touch interrupts after quickspi_dev_deinit()),
and quickspi_dma_deinit() then unmaps and frees the TxDMA scatterlist and
PRD table while hardware TxDMA may still be active.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-thc-rebase-v2-0-mhun512@gmail.com?part=1

RE: [PATCH v2 1/2] HID: intel-thc-hid: quickspi: quiesce IRQs during teardown

[Xu, Even]

> -----Original Message-----
> From: Myeonghun Pak <mhun512@gmail.com>
> Sent: Tuesday, November 10, 2026 12:47 PM
> To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>; Jiri
> Kosina <jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>
> Cc: Myeonghun Pak <mhun512@gmail.com>; Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson-
> lenovo@squebb.ca>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org;
> Ijae Kim <ae878000@gmail.com>
> Subject: [PATCH v2 1/2] HID: intel-thc-hid: quickspi: quiesce IRQs during
> teardown
> 
> The managed IRQ can access HID/DMA buffers or queue recovery work while
> remove or probe unwind tears down those resources.
> 
> Disable and synchronize the IRQ, drain recovery work, then mask THC interrupts
> before freeing the IRQ and releasing HID/DMA resources.
> Track IRQ/DMA initialization for conditional probe cleanup.
> 
> Disable the IRQ before shutdown deinit as well. Keep IRQ disabling and release
> outside the shared hibernation helper.
> 
> This issue was found by our static analysis tool under development.
> 
> Fixes: 4138f21115ae ("HID: intel-thc-hid: intel-quickspi: Complete THC QuickSPI
> driver")
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
> No hardware testing.
> 
> Changes in v2:
> - Rebase on mainline b3dfbde62cf1. No source changes from v1.
> 
>  .../intel-thc-hid/intel-quickspi/pci-quickspi.c    | 28 +++++++++++++++++-----
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> index
> bf49d8ce5f07bed092339e46c444538d78cffe26..acb4d2716ea7450d2852a900c3f
> 0acc1d7570421 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -600,6 +600,8 @@ static int quickspi_probe(struct pci_dev *pdev,
>  			  const struct pci_device_id *id)
>  {
>  	struct quickspi_device *qsdev;
> +	bool dma_initialized = false;
> +	bool irq_requested = false;
>  	void __iomem *mem_addr;
>  	int ret;
> 
> @@ -652,6 +654,7 @@ static int quickspi_probe(struct pci_dev *pdev,
>  					qsdev);
>  	if (ret)
>  		goto dev_deinit;
> +	irq_requested = true;
> 
>  	ret = reset_tic(qsdev);
>  	if (ret) {
> @@ -670,17 +673,18 @@ static int quickspi_probe(struct pci_dev *pdev,
>  		dev_err(&pdev->dev, "Setup THC DMA failed, ret= %d\n", ret);
>  		goto dev_deinit;
>  	}
> +	dma_initialized = true;
> 
>  	ret = quickspi_get_report_descriptor(qsdev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Get report descriptor failed, ret = %d\n",
> ret);
> -		goto dma_deinit;
> +		goto dev_deinit;
>  	}
> 
>  	ret = quickspi_hid_probe(qsdev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to register HID device, ret = %d\n",
> ret);
> -		goto dma_deinit;
> +		goto dev_deinit;
>  	}
> 
>  	qsdev->state = QUICKSPI_ENABLED;
> @@ -695,10 +699,16 @@ static int quickspi_probe(struct pci_dev *pdev,
> 
>  	return 0;
> 
> -dma_deinit:
> -	quickspi_dma_deinit(qsdev);
>  dev_deinit:
> +	if (irq_requested) {
> +		WRITE_ONCE(qsdev->recovery_disabled, true);
> +		disable_irq(pdev->irq);
> +	}
Add a blank line here.

>  	quickspi_dev_deinit(qsdev);
Blank line.

> +	if (irq_requested)
> +		devm_free_irq(&pdev->dev, pdev->irq, qsdev);
Here.

> +	if (dma_initialized)
> +		quickspi_dma_deinit(qsdev);
Here too.
And do the same thing for Quickspi driver.

Thanks!

Best Regards,
Even Xu

>  disable_pci_device:
>  	pci_clear_master(pdev);
> 
> @@ -721,9 +731,12 @@ static void quickspi_remove(struct pci_dev *pdev)
>  	if (!qsdev)
>  		return;
> 
> -	quickspi_hid_remove(qsdev);
> -
> +	WRITE_ONCE(qsdev->recovery_disabled, true);
> +	disable_irq(pdev->irq);
>  	quickspi_dev_deinit(qsdev);
> +	devm_free_irq(&pdev->dev, pdev->irq, qsdev);
> +
> +	quickspi_hid_remove(qsdev);
> 
>  	quickspi_dma_deinit(qsdev);
> 
> @@ -750,6 +763,9 @@ static void quickspi_shutdown(struct pci_dev *pdev)
>  	if (!qsdev)
>  		return;
> 
> +	WRITE_ONCE(qsdev->recovery_disabled, true);
> +	disable_irq(pdev->irq);
> +
>  	quickspi_dev_deinit(qsdev);
> 
>  	/* Must stop DMA before reboot to avoid DMA entering into unknown
> state */
> 
> base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
> --
> 2.53.0