-----Original Message-----
From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
Sent: Friday, July 3, 2026 4:13 PM
To: Xu, Even <even.xu@intel.com>
Cc: linux-input@vger.kernel.org; dmitry.torokhov@gmail.com
Subject: Re: [PATCH v2 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read
DMA

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

New issues:
- [High] thc_rxdma_reset() fails to re-initialize the RxDMA PRD tables, leading to
hardware reading shrunken buffer sizes on restart.
- [High] thc_rxdma_reset() unsafely clears PRD base addresses for all DMA
channels while TX or SW DMA engines may still be running.
- [Medium] thc_rxdma_reset() ignores the return value of
thc_interrupt_quiesce(dev, false) on the success path.

Pre-existing issues:
- [High] Incorrect polling condition and execution order in thc_interrupt_quiesce
during unquiesce (int_quiesce = false).
--

commit b0beafd1bc7a02f7a2277c5a6d69c17c58724f01
Author: Even Xu [off-list ref]

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

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

quoted

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

@@ -561,6 +561,57 @@ static int thc_wait_for_dma_pause(struct

thc_device *dev, enum thc_dma_channel c

[ ... ]

quoted

+int thc_rxdma_reset(struct thc_device *dev) {

[ ... ]

quoted

+	ret = thc_wait_for_dma_pause(dev, THC_RXDMA2);
+	if (ret) {
+		dev_err(dev->dev, "Wait for RxDMA2 pause failed during RxDMA

reset\n");

quoted

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

[Severity: High]
Could this cause memory corruption or IOMMU faults if TX or SW DMA engines
are still running?

thc_rxdma_reset() explicitly pauses RxDMA1 and RxDMA2 but does not pause
TXDMA or SWDMA. It then calls thc_dma_unconfigure(), which loops over all
DMA channels (MAX_THC_DMA_CHANNEL) and blindly overwrites their PRD
base addresses to 0x0.

If TXDMA was left running (for instance, if a prior thc_dma_write() timed out
without clearing the TXDMA start bit), setting its PRD base address to 0 while it is
actively fetching could cause the hardware to fetch descriptors from physical
address 0.

quoted

+
+	ret = thc_dma_configure(dev);

This isn't a problem, all DMA operations are protected by mutex "thc_bus_lock", that guarantees when do this rxdma_reset, TxDMA and SWDMA are not working.

[Severity: High]
Does this leave dirty 'len' descriptors in the PRD memory layout?

thc_dma_configure() restarts the DMA engine pointers to 0, but it does not re-
initialize the PRD memory layout. The hardware updates the 'len' field of PRD
entries to indicate the actual length of received packets.

Restarting the DMA from index 0 without restoring 'len' to the maximum buffer
capacity could cause the hardware to use previously received smaller packet sizes
as the maximum capacity for new transfers, resulting in truncated packets or
DMA errors.

quoted

+	if (ret) {
+		dev_err(dev->dev, "Re-config DMA failed during RxDMA

reset\n");

quoted

+		goto end;
+	}
+
+	thc_interrupt_quiesce(dev, false);

This is expected, once DMA buffers were allocated, in order to avoid memory fragment, this DMA buffers will be reused.
That means the memory address and the max memory length are fixed.
Need pay attention is, this length is just a max memory length, the real length for every DMA data will be stored in every PRD entry and the length will be updated every time once DMA finished. 

[Severity: Medium]
Is it safe to ignore the return value of thc_interrupt_quiesce() here?

Discarding this return value on the success path could silently hide a hardware
timeout or regmap read failure, potentially leaving interrupts disabled while the
function reports success.

[Severity: High]
This is a pre-existing issue, but is the polling condition and execution order in
thc_interrupt_quiesce() correct when unquiescing?

When thc_interrupt_quiesce() is called with int_quiesce = false, it polls the
HW_STS bit expecting it to be 1 BEFORE it clears the QUIESCE_EN bit:

drivers/hid/intel-thc-hid/intel-thc/intel-thc-dev.c:thc_interrupt_quiesce() {
    ...
    ret = regmap_read_poll_timeout(dev->thc_regmap,
THC_M_PRT_CONTROL_OFFSET, ctrl,
                                   ctrl &
THC_M_PRT_CONTROL_THC_DEVINT_QUIESCE_HW_STS,
                                   THC_REGMAP_POLLING_INTERVAL_US,
THC_QUIESCE_EN_TIMEOUT_US);
    ...
    /* Unquiesce device interrupt - Clear the quiesce bit */
    if (!int_quiesce)
        regmap_write_bits(dev->thc_regmap, THC_M_PRT_CONTROL_OFFSET,
                          THC_M_PRT_CONTROL_THC_DEVINT_QUIESCE_EN, 0); }

Since the device is already quiesced, HW_STS is already 1, causing the poll to
return immediately. It then clears the QUIESCE_EN bit and returns without ever
waiting for HW_STS to become 0 to acknowledge the unquiesce. This could cause
the driver to proceed before interrupts are actually re-enabled.

That's what this function call works. Called with int_quiesce = false means quiesce the interrupt, if already quiesced, HW_STS should be 0, so clear this bit directly is correct behavior.

quoted

+
+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/20260703075858.2780398-1-
even.xu@intel.com?part=1