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

[sashiko-bot@kernel.org]

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