Re: [PATCH v3 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 c5d058390d4d45710eca02a89498e5e65474bafb
Author: Myeonghun Pak <mhun512@gmail.com>

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

This commit disables and frees the device IRQ and drains recovery work
during probe failure, driver removal, and shutdown before releasing HID
and DMA resources.

> 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..3aa0ae81053e8 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 +735,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 disable_irq(), quickspi_dev_deinit(), and devm_free_irq()
ahead of quickspi_hid_remove() break HID teardown?

When a hid-multitouch input device is open during unbind,
quickspi_hid_remove() -> hid_destroy_device() -> ... -> hid_hw_close() ->
mt_on_hid_hw_close() -> mt_set_modes() synchronously calls
quickspi_hid_raw_request() -> quickspi_set_report() ->
write_cmd_to_txdma() -> thc_dma_write(). Because quickspi_hid_raw_request()
does not check qsdev->state and THC_TXDMA remains enabled until
quickspi_dma_deinit(), thc_dma_write() starts hardware TxDMA and blocks in
wait_event_interruptible_timeout(dev->write_complete_wait, ...) waiting
for a THC_TXDMA_INT completion interrupt that can no longer arrive. When
thc_dma_write() times out or is interrupted by a signal,
quickspi_dma_deinit() then unmaps and frees the TxDMA scatterlist and PRD
table without clearing THC_M_PRT_WRITE_DMA_CNTRL_THC_WRDMA_START.

Calling quickspi_hid_remove() before disabling the IRQ and deinitializing
the controller would avoid starting new transfers after interrupts are
disabled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112T063704Z-thc-v3-0-mhun512@gmail.com?part=1