[PATCH 0/3] HID: Intel-thc-hid: Refine error recovery flow

[PATCH 0/3] HID: Intel-thc-hid: Refine error recovery flow

[Even Xu]

This series refines the fatal error recovery flow for the Intel THC
(Touch Host Controller) subsystem, covering both the QuickI2C and
QuickSPI drivers.

Currently, when a fatal DMA error is detected in the IRQ thread handler,
the recovery is performed inline: the interrupt handler calls
try_recover() directly, which unconfigures and reconfigures the DMA
engine.

This approach has several problems:
1. Recovery runs in the IRQ thread context, which is not ideal for
   potentially slow reset operations.
2. The interrupt is re-enabled before recovery completes, risking an
   interrupt storm if DMA errors persist.
3. The DMA reset logic is open-coded in each protocol driver, leading
   to duplication and divergence over time.

This patch series addresses all of the above:

By adding a new thc_rxdma_reset() API to the THC core layer, QuickI2C
and QuickSPI drivers can call it respectively to refine the recovery
callback.

The synchronous try_recover() call in the IRQ thread is replaced with
schedule_work(), deferring recovery to a workqueue.  Within the work
function:
- The interrupt line is disabled before any DMA manipulation.
- thc_rxdma_reset() is used instead of the open-coded sequence.
- On failure the device is marked DISABLED and the interrupt remains
  off, preventing an interrupt storm.

Even Xu (3):
  HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
  HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback
  HID: Intel-thc-hid: Intel-quickspi: Refine recover callback

 .../intel-quicki2c/pci-quicki2c.c             | 31 +++++++---------
 .../intel-quicki2c/quicki2c-dev.h             |  2 +
 .../intel-quickspi/pci-quickspi.c             | 37 +++++++------------
 .../intel-quickspi/quickspi-dev.h             |  2 +
 .../intel-thc-hid/intel-thc/intel-thc-dma.c   | 37 +++++++++++++++++++
 .../intel-thc-hid/intel-thc/intel-thc-dma.h   |  1 +
 6 files changed, 68 insertions(+), 42 deletions(-)

-- 
2.43.0

[PATCH 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA

[Even Xu]

Add a helper function thc_rxdam_reset() to do read DMA reset, it can be
called when fatal DMA error happens.

Signed-off-by: Even Xu <even.xu@intel.com>
---
 .../intel-thc-hid/intel-thc/intel-thc-dma.c   | 37 +++++++++++++++++++
 .../intel-thc-hid/intel-thc/intel-thc-dma.h   |  1 +
 2 files changed, 38 insertions(+)

diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
index 6ee675e0a738..d771c85dff28 100644
--- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
+++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
@@ -532,6 +532,43 @@ void thc_dma_unconfigure(struct thc_device *dev)
 }
 EXPORT_SYMBOL_NS_GPL(thc_dma_unconfigure, "INTEL_THC");
 
+/**
+ * thc_rxdma_reset - Reset all read DMA engines
+ *
+ * @dev: The pointer of THC private device context
+ *
+ * This is a helper function to reset RxDMA configure. It's typically used
+ * for RxDMA recovery when fatal error happens.
+ */
+int thc_rxdma_reset(struct thc_device *dev)
+{
+	int ret;
+
+	if (mutex_lock_interruptible(&dev->thc_bus_lock))
+		return -EINTR;
+
+	ret = thc_interrupt_quiesce(dev, true);
+	if (ret) {
+		dev_err_once(dev->dev, "Quiesce interrupt failed during RxDMA reset\n");
+		goto end;
+	}
+
+	thc_dma_unconfigure(dev);
+
+	ret = thc_dma_configure(dev);
+	if (ret) {
+		dev_err_once(dev->dev, "Re-config DMA failed during RxDMA reset\n");
+		goto end;
+	}
+
+	thc_interrupt_quiesce(dev, false);
+
+end:
+	mutex_unlock(&dev->thc_bus_lock);
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(thc_rxdma_reset, "INTEL_THC");
+
 static int thc_wait_for_dma_pause(struct thc_device *dev, enum thc_dma_channel channel)
 {
 	u32 ctrl_reg, sts_reg, sts;
diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.h b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.h
index 541d33995baf..715423453a9d 100644
--- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.h
+++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.h
@@ -145,6 +145,7 @@ int  thc_dma_allocate(struct thc_device *dev);
 int  thc_dma_configure(struct thc_device *dev);
 void thc_dma_unconfigure(struct thc_device *dev);
 void thc_dma_release(struct thc_device *dev);
+int  thc_rxdma_reset(struct thc_device *dev);
 int  thc_rxdma_read(struct thc_device *dev, enum thc_dma_channel dma_channel,
 		    void *read_buff, size_t *read_len, int *read_finished);
 int  thc_swdma_read(struct thc_device *dev, void *write_buff, size_t write_len,
-- 
2.43.0

Re: [PATCH 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Using dev_err_once() hides subsequent failures during fatal error recovery.

Pre-existing issues:
- [High] Zeroing the Physical Region Descriptor (PRD) base addresses before pausing the DMA engines can cause IOMMU faults or memory corruption.
--

commit fb285a619300634b9f208053996047e32855d49b
Author: Even Xu <even.xu@intel.com>
Date:   Wed Jul 1 02:04:30 2026 +0000

HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA

Add a helper function thc_rxdam_reset() to do read DMA reset, it can be
called when fatal DMA error happens.

> diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> index 6ee675e0a7384..d771c85dff28b 100644
> --- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> +++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c

[ ... ]

> @@ -532,6 +532,43 @@ void thc_dma_unconfigure(struct thc_device *dev)
>  }
>  EXPORT_SYMBOL_NS_GPL(thc_dma_unconfigure, "INTEL_THC");
>  
> +/**
> + * thc_rxdma_reset - Reset all read DMA engines
> + *
> + * @dev: The pointer of THC private device context
> + *
> + * This is a helper function to reset RxDMA configure. It's typically used
> + * for RxDMA recovery when fatal error happens.
> + */
> +int thc_rxdma_reset(struct thc_device *dev)
> +{
> +	int ret;
> +
> +	if (mutex_lock_interruptible(&dev->thc_bus_lock))
> +		return -EINTR;
> +
> +	ret = thc_interrupt_quiesce(dev, true);
> +	if (ret) {
> +		dev_err_once(dev->dev, "Quiesce interrupt failed during RxDMA reset\n");

[Severity: Medium]
Since thc_rxdma_reset() is called as part of a recurring fatal error
recovery flow, will dev_err_once() hide subsequent failures?

If a device successfully recovers the first time but encounters an
unrecoverable failure during a subsequent fatal error later in its uptime,
the failure will be silently ignored because dev_err_once() only prints
once per boot.

Should dev_err_ratelimited() or dev_err() be used instead?

> +		goto end;
> +	}
> +
> +	thc_dma_unconfigure(dev);

[Severity: High]
This is a pre-existing issue, but does calling thc_dma_unconfigure()
here risk IOMMU faults or memory corruption?

When thc_rxdma_reset() invokes thc_dma_unconfigure() during fatal error
recovery, the hardware DMA engine might still be actively fetching or in
an unpredictable state. 

Looking at thc_dma_unconfigure(), it zeroes out the PRD base addresses
before clearing the DMA START bits:

thc_dma_unconfigure() {
    for (chan = 0; chan < MAX_THC_DMA_CHANNEL; chan++) {
        dma_set_prd_base_addr(dev, 0, &dev->dma_ctx->dma_config[chan]);
        ...
    }
    regmap_write_bits(dev->thc_regmap, THC_M_PRT_READ_DMA_CNTRL_1_OFFSET,
                      THC_M_PRT_READ_DMA_CNTRL_START, 0);
}

If the hardware DMA engine is active and continues fetching descriptors
before the START bit is cleared, will it attempt to fetch from physical
address 0x0?

Should it wait for the ACTIVE status to clear (e.g., using
thc_wait_for_dma_pause()) before clearing the base addresses?

> +
> +	ret = thc_dma_configure(dev);
> +	if (ret) {
> +		dev_err_once(dev->dev, "Re-config DMA failed during RxDMA reset\n");
> +		goto end;
> +	}
> +
> +	thc_interrupt_quiesce(dev, false);
> +
> +end:
> +	mutex_unlock(&dev->thc_bus_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(thc_rxdma_reset, "INTEL_THC");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701020432.3774514-1-even.xu@intel.com?part=1

[PATCH 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback

[Even Xu]

Refine recover flow:
1. Use workqueue to handle recover flow instead of processing in irq
   handler.
2. Call thc_rxdma_reset() API to simplify the recover operation.
3. Disable interrupt during whole recover flow.
4. If recover fails, disable interrupt to avoid interrupt storm.

Signed-off-by: Even Xu <even.xu@intel.com>
---
 .../intel-quicki2c/pci-quicki2c.c             | 31 ++++++++-----------
 .../intel-quicki2c/quicki2c-dev.h             |  2 ++
 2 files changed, 15 insertions(+), 18 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 46d3e9a01999..411d7a0ab90e 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
@@ -245,28 +245,23 @@ static irqreturn_t quicki2c_irq_quick_handler(int irq, void *dev_id)
 }
 
 /**
- * try_recover - Try to recovery THC and Device
- * @qcdev: Pointer to quicki2c_device structure
+ * try_recover - Recover callback to recover THC
+ * @work: pointer to work_struct
  *
  * This function is an error handler, called when fatal error happens.
- * It try to reset touch device and re-configure THC to recovery
- * communication between touch device and THC.
- *
- * Return: 0 if successful or error code on failure
+ * It try to reset Touch Device and re-configure THC to recover
+ * transferring between Device and THC.
  */
-static int try_recover(struct quicki2c_device *qcdev)
+static void try_recover(struct work_struct *work)
 {
-	int ret;
+	struct quicki2c_device *qcdev = container_of(work, struct quicki2c_device, recover_work);
 
-	thc_dma_unconfigure(qcdev->thc_hw);
-
-	ret = thc_dma_configure(qcdev->thc_hw);
-	if (ret) {
-		dev_err(qcdev->dev, "Reconfig DMA failed\n");
-		return ret;
-	}
+	thc_interrupt_enable(qcdev->thc_hw, false);
 
-	return 0;
+	if (thc_rxdma_reset(qcdev->thc_hw))
+		qcdev->state = QUICKI2C_DISABLED;
+	else
+		thc_interrupt_enable(qcdev->thc_hw, true);
 }
 
 static int handle_input_report(struct quicki2c_device *qcdev)
@@ -346,8 +341,7 @@ static irqreturn_t quicki2c_irq_thread_handler(int irq, void *dev_id)
 	thc_interrupt_enable(qcdev->thc_hw, true);
 
 	if (err_recover)
-		if (try_recover(qcdev))
-			qcdev->state = QUICKI2C_DISABLED;
+		schedule_work(&qcdev->recover_work);
 
 	pm_runtime_put_autosuspend(qcdev->dev);
 
@@ -386,6 +380,7 @@ static struct quicki2c_device *quicki2c_dev_init(struct pci_dev *pdev, void __io
 	qcdev->ddata = ddata;
 
 	init_waitqueue_head(&qcdev->reset_ack_wq);
+	INIT_WORK(&qcdev->recover_work, try_recover);
 
 	/* THC hardware init */
 	qcdev->thc_hw = thc_dev_init(qcdev->dev, qcdev->mem_addr);
diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
index 61dbdece59a1..aedf85291e60 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
@@ -222,6 +222,8 @@ struct quicki2c_device {
 	wait_queue_head_t reset_ack_wq;
 	bool reset_ack;
 
+	struct work_struct recover_work;
+
 	u32 i2c_max_frame_size_enable;
 	u32 i2c_max_frame_size;
 	u32 i2c_int_delay_enable;
-- 
2.43.0

Re: [PATCH 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing cancellation of the scheduled asynchronous recovery work during device removal and shutdown leads to a Use-After-Free.
- [High] Unconditionally re-enabling hardware interrupts before the asynchronous recovery completes causes an immediate interrupt storm.
- [High] The deferred `try_recover` work function accesses hardware registers directly without acquiring a Runtime PM reference.
--

commit 8f5c7d640eb5d690c93bdf294d89a5d9087edae5
Author: Even Xu <even.xu@intel.com>

HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback

This commit refines the recover flow by moving the hardware recovery operation
to an asynchronous workqueue. It replaces the inline recover operation with a
call to schedule_work() and simplifies the recover callback using
thc_rxdma_reset().

> 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 46d3e9a019995..411d7a0ab90ef 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
[ ... ]
> @@ -245,28 +245,23 @@ static irqreturn_t quicki2c_irq_quick_handler(int irq, void *dev_id)
>  }
>  
> -static int try_recover(struct quicki2c_device *qcdev)
> +static void try_recover(struct work_struct *work)
>  {
> +	struct quicki2c_device *qcdev = container_of(work, struct quicki2c_device, recover_work);
>  
> +	thc_interrupt_enable(qcdev->thc_hw, false);
>  

[Severity: High]
Is it safe to access the hardware registers in this work function without
holding a runtime PM reference?

Since the IRQ handler calls pm_runtime_put_autosuspend() right after
scheduling the work, could the device transition to a suspended state before
try_recover() executes, resulting in invalid MMIO accesses when trying to
manipulate the THC registers?

> -	thc_dma_unconfigure(qcdev->thc_hw);
> -
> -	ret = thc_dma_configure(qcdev->thc_hw);
> -	if (ret) {
> -		dev_err(qcdev->dev, "Reconfig DMA failed\n");
> -		return ret;
> -	}
> +	if (thc_rxdma_reset(qcdev->thc_hw))
> +		qcdev->state = QUICKI2C_DISABLED;
> +	else
> +		thc_interrupt_enable(qcdev->thc_hw, true);
>  }
[ ... ]
> @@ -346,8 +341,7 @@ static irqreturn_t quicki2c_irq_thread_handler(int irq, void *dev_id)
>  	thc_interrupt_enable(qcdev->thc_hw, true);
>  

[Severity: High]
Does re-enabling the interrupt here before the recovery work executes cause
an interrupt storm?

Because the actual hardware error state isn't cleared until thc_rxdma_reset()
runs asynchronously in the workqueue, unmasking the interrupt here might
cause the hardware to instantly re-assert the interrupt. Could this lead to
an infinite loop between the hard IRQ and threaded IRQ handlers, thereby
starving the system workqueue from ever executing try_recover()?

>  	if (err_recover)
> -		if (try_recover(qcdev))
> -			qcdev->state = QUICKI2C_DISABLED;
> +		schedule_work(&qcdev->recover_work);
>  
>  	pm_runtime_put_autosuspend(qcdev->dev);
[ ... ]
> @@ -386,6 +380,7 @@ static struct quicki2c_device *quicki2c_dev_init(struct pci_dev *pdev, void __io
>  	qcdev->ddata = ddata;
>  
>  	init_waitqueue_head(&qcdev->reset_ack_wq);
> +	INIT_WORK(&qcdev->recover_work, try_recover);
>  

[Severity: High]
When the device is removed, could the scheduled work run after the device
context is freed?

Looking at quicki2c_remove(), there is no call to cancel_work_sync() before
tearing down the device. Since qcdev is allocated with devres, might
this lead to a use-after-free if the recovery work is still queued on the
system workqueue when the removal path finishes?

>  	/* THC hardware init */
>  	qcdev->thc_hw = thc_dev_init(qcdev->dev, qcdev->mem_addr);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701020432.3774514-1-even.xu@intel.com?part=2

[PATCH 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback

[Even Xu]

Refine recover flow:
1. Use workqueue to handle recover flow instead of processing in irq
   handler.
2. Call thc_rxdma_reset() API to simplify the recover operation.
3. Disable interrupt during whole recover flow.
4. If recover fails, disable interrupt to avoid interrupt storm.

Signed-off-by: Even Xu <even.xu@intel.com>
---
 .../intel-quickspi/pci-quickspi.c             | 37 +++++++------------
 .../intel-quickspi/quickspi-dev.h             |  2 +
 2 files changed, 15 insertions(+), 24 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 f669235f1883..1e63b183b3b1 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -252,34 +252,23 @@ static irqreturn_t quickspi_irq_quick_handler(int irq, void *dev_id)
 }
 
 /**
- * try_recover - Try to recovery THC and Device
- * @qsdev: pointer to quickspi device
+ * try_recover - Recover callback to recover THC
+ * @work: pointer to work_struct
  *
- * This function is a error handler, called when fatal error happens.
- * It try to reset Touch Device and re-configure THC to recovery
+ * This function is an error handler, called when fatal error happens.
+ * It try to reset Touch Device and re-configure THC to recover
  * transferring between Device and THC.
- *
- * Return: 0 if successful or error code on failed.
  */
-static int try_recover(struct quickspi_device *qsdev)
+static void try_recover(struct work_struct *work)
 {
-	int ret;
+	struct quickspi_device *qsdev = container_of(work, struct quickspi_device, recover_work);
 
-	ret = reset_tic(qsdev);
-	if (ret) {
-		dev_err(qsdev->dev, "Reset touch device failed, ret = %d\n", ret);
-		return ret;
-	}
-
-	thc_dma_unconfigure(qsdev->thc_hw);
-
-	ret = thc_dma_configure(qsdev->thc_hw);
-	if (ret) {
-		dev_err(qsdev->dev, "Re-configure THC DMA failed, ret = %d\n", ret);
-		return ret;
-	}
+	thc_interrupt_enable(qsdev->thc_hw, false);
 
-	return 0;
+	if (thc_rxdma_reset(qsdev->thc_hw))
+		qsdev->state = QUICKSPI_DISABLED;
+	else
+		thc_interrupt_enable(qsdev->thc_hw, true);
 }
 
 /**
@@ -340,8 +329,7 @@ static irqreturn_t quickspi_irq_thread_handler(int irq, void *dev_id)
 	thc_interrupt_enable(qsdev->thc_hw, true);
 
 	if (err_recover)
-		if (try_recover(qsdev))
-			qsdev->state = QUICKSPI_DISABLED;
+		schedule_work(&qsdev->recover_work);
 
 	pm_runtime_put_autosuspend(qsdev->dev);
 
@@ -385,6 +373,7 @@ static struct quickspi_device *quickspi_dev_init(struct pci_dev *pdev, void __io
 	init_waitqueue_head(&qsdev->report_desc_got_wq);
 	init_waitqueue_head(&qsdev->get_report_cmpl_wq);
 	init_waitqueue_head(&qsdev->set_report_cmpl_wq);
+	INIT_WORK(&qsdev->recover_work, try_recover);
 
 	/* thc hw init */
 	qsdev->thc_hw = thc_dev_init(qsdev->dev, qsdev->mem_addr);
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
index bf5e18f5a5f4..b0c1219866a4 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
@@ -173,6 +173,8 @@ struct quickspi_device {
 
 	wait_queue_head_t set_report_cmpl_wq;
 	bool set_report_cmpl;
+
+	struct work_struct recover_work;
 };
 
 #endif /* _QUICKSPI_DEV_H_ */
-- 
2.43.0

Re: [PATCH 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] Use-after-free of qsdev during device removal due to uncanceled async work
- [High] Missing runtime PM protection during asynchronous hardware recovery
- [High] Unconditional interrupt unmasking causes severe interrupt storm
- [High] Data race on DMA configuration between IRQ thread and async recovery

Pre-existing issues:
- [High] Invalid runtime PM teardown causes unpowered hardware accesses during removal
--

commit b4ff70b4c528a251392ae3368aa0d2f80233ab2e
Author: Even Xu <even.xu@intel.com>
Subject: HID: Intel-thc-hid: Intel-quickspi: Refine recover callback

This commit refines the recovery flow by using a workqueue to handle
recovery instead of processing it in the IRQ handler. It calls
thc_rxdma_reset() to simplify operations and disables interrupts during
the recovery process.

> 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 4ae2e1718b30c..a2428db5f9414 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -252,34 +252,23 @@ static irqreturn_t quickspi_irq_quick_handler(int irq, void *dev_id)
[ ... ]
> +static void try_recover(struct work_struct *work)
> +{
> +	struct quickspi_device *qsdev = container_of(work, struct quickspi_device, recover_work);
> +
> +	thc_interrupt_enable(qsdev->thc_hw, false);
> +

[Severity: High]
Does this asynchronously scheduled work need to acquire its own runtime PM
reference?

The IRQ thread drops its PM reference with pm_runtime_put_autosuspend()
immediately after scheduling this work. If the autosuspend delay expires
before this workqueue executes, the device may enter a suspended state.

Will calling thc_interrupt_enable() and thc_rxdma_reset() here cause a system
crash or silent failure by accessing unpowered hardware registers?

> +	if (thc_rxdma_reset(qsdev->thc_hw))

[Severity: High]
Is there a potential data race here with the IRQ thread?

The try_recover() function calls thc_rxdma_reset(), which holds the
thc_bus_lock while unconfiguring and reconfiguring the DMA hardware.
However, if a concurrent interrupt occurs, quickspi_irq_thread_handler()
calls thc_rxdma_read() without holding thc_bus_lock.

Could this allow the IRQ thread to read from DMA rings while they are
actively being torn down and rebuilt by the recovery work on another CPU?

> +		qsdev->state = QUICKSPI_DISABLED;
> +	else
> +		thc_interrupt_enable(qsdev->thc_hw, true);
>  }
[ ... ]
> @@ -340,8 +329,7 @@ static irqreturn_t quickspi_irq_thread_handler(int irq, void *dev_id)
>  	thc_interrupt_enable(qsdev->thc_hw, true);
>  
>  	if (err_recover)
> -		if (try_recover(qsdev))
> -			qsdev->state = QUICKSPI_DISABLED;
> +		schedule_work(&qsdev->recover_work);

[Severity: High]
Could this unconditional interrupt unmasking cause an interrupt storm?

When the hardware signals a fatal error, the handler unmasks the interrupt
here before returning, but the hardware reset is deferred to the asynchronous
workqueue.

Since the hardware error condition hasn't been cleared yet, won't the
device immediately re-assert the interrupt and trap the CPU in an endless
hardirq-to-thread loop before the workqueue ever has a chance to execute?

>  
>  	pm_runtime_put_autosuspend(qsdev->dev);
>  
> @@ -385,6 +373,7 @@ static struct quickspi_device *quickspi_dev_init(struct pci_dev *pdev, void __io
>  	init_waitqueue_head(&qsdev->report_desc_got_wq);
>  	init_waitqueue_head(&qsdev->get_report_cmpl_wq);
>  	init_waitqueue_head(&qsdev->set_report_cmpl_wq);
> +	INIT_WORK(&qsdev->recover_work, try_recover);

[Severity: High]
Does this newly added work item need to be explicitly canceled during device
removal?

In the driver removal path in quickspi_remove(), there is no call to
cancel_work_sync().

Since qsdev is allocated with devm_kzalloc(), it will be freed shortly after
quickspi_remove() returns. If a fatal error interrupt schedules this work just
before or during driver unbind, could the workqueue execute after qsdev is
freed, causing a use-after-free when try_recover() dereferences it?

[Severity: High]
This is a pre-existing issue, but does quickspi_remove() properly wake the
device before accessing its hardware registers?

Currently, quickspi_remove() uses pm_runtime_get_noresume():

quickspi_remove() {
    ...
    pm_runtime_get_noresume(qsdev->dev);
    quickspi_dev_deinit(qsdev);
    ...
}

Calling pm_runtime_get_noresume() only increments the usage counter, it does
not resume the device. If the device was already auto-suspended,
quickspi_dev_deinit() will write to unpowered registers.

Could this lead to dropped writes or a PCIe bus error?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701020432.3774514-1-even.xu@intel.com?part=3