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

From: Xu, Even

Date: Tue Nov 10 2026 - 00:29:21 EST


Hi, Myeonghun,

Got it, will review your new patches and feedback!

Best Regards,
Even Xu

> -----Original Message-----
> From: Myeonghun Pak <mhun512@xxxxxxxxx>
> Sent: Tuesday, November 10, 2026 12:43 PM
> To: Xu, Even <even.xu@xxxxxxxxx>
> Cc: Sun, Xinpeng <xinpeng.sun@xxxxxxxxx>; Jiri Kosina <jikos@xxxxxxxxxx>;
> Benjamin Tissoires <bentiss@xxxxxxxxxx>; Srinivas Pandruvada
> <srinivas.pandruvada@xxxxxxxxxxxxxxx>; Mark Pearson <mpearson-
> lenovo@xxxxxxxxx>; linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> Ijae Kim <ae878000@xxxxxxxxx>
> Subject: Re: [PATCH 1/2] HID: intel-thc-hid: quickspi: quiesce IRQs during
> teardown
>
> Hi Even,
>
> Thanks for testing. I've rebased both patches onto the latest mainline with no
> other code changes.
>
> Best regards,
> Myeonghun
>
> 2026년 10월 26일 (월) 오후 10:05, Xu, Even <even.xu@xxxxxxxxx>님이 작성:
> >
> > Hi, Myeonghun,
> >
> > Validation passed, but I suppose you need rebase your patches on top of latest
> mainline kernel.
> >
> > Best Regards,
> > Even Xu
> >
> > > -----Original Message-----
> > > From: Xu, Even
> > > Sent: Thursday, October 15, 2026 9:53 AM
> > > To: 'Myeonghun Pak' <mhun512@xxxxxxxxx>
> > > Cc: Sun, Xinpeng <Xinpeng.Sun@xxxxxxxxx>; Jiri Kosina
> > > <jikos@xxxxxxxxxx>; Benjamin Tissoires <bentiss@xxxxxxxxxx>;
> > > Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>; Mark
> > > Pearson <mpearson- lenovo@xxxxxxxxx>; linux-input@xxxxxxxxxxxxxxx;
> > > linux-kernel@xxxxxxxxxxxxxxx; Ijae Kim <ae878000@xxxxxxxxx>
> > > Subject: RE: [PATCH 1/2] HID: intel-thc-hid: quickspi: quiesce IRQs
> > > during teardown
> > >
> > > Hi, Myeonghun,
> > >
> > > Then let me test it, it may take several days.
> > > Will feedback once test finished.
> > >
> > > Best Regards,
> > > Even Xu
> > >
> > > > -----Original Message-----
> > > > From: Myeonghun Pak <mhun512@xxxxxxxxx>
> > > > Sent: Thursday, October 15, 2026 8:36 AM
> > > > To: Xu, Even <even.xu@xxxxxxxxx>
> > > > Cc: Sun, Xinpeng <xinpeng.sun@xxxxxxxxx>; Jiri Kosina
> > > > <jikos@xxxxxxxxxx>; Benjamin Tissoires <bentiss@xxxxxxxxxx>;
> > > > Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>; Mark
> > > > Pearson
> > > > <mpearson- lenovo@xxxxxxxxx>; linux-input@xxxxxxxxxxxxxxx;
> > > > linux-kernel@xxxxxxxxxxxxxxx; Ijae Kim <ae878000@xxxxxxxxx>
> > > > Subject: Re: [PATCH 1/2] HID: intel-thc-hid: quickspi: quiesce
> > > > IRQs during teardown
> > > >
> > > > Hi Even,
> > > >
> > > > Thanks for reviewing the patch.
> > > >
> > > > No, I haven't tested it on actual hardware. I have only
> > > > compile-tested it with GCC and W=1 on x86_64.
> > > >
> > > > Best regards,
> > > > Myeonghun
> > > >
> > > > 2026년 10월 12일 (월) 오후 8:52, Xu, Even <even.xu@xxxxxxxxx>님이
> 작성:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Myeonghun Pak <mhun512@xxxxxxxxx>
> > > > > > Sent: Monday, October 12, 2026 6:06 AM
> > > > > > To: Xu, Even <even.xu@xxxxxxxxx>; Sun, Xinpeng
> > > > > > <xinpeng.sun@xxxxxxxxx>; Jiri Kosina <jikos@xxxxxxxxxx>;
> > > > > > Benjamin Tissoires <bentiss@xxxxxxxxxx>
> > > > > > Cc: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>;
> > > > > > Mark Pearson <mpearson-lenovo@xxxxxxxxx>;
> > > > > > linux-input@xxxxxxxxxxxxxxx;
> > > > > > linux- kernel@xxxxxxxxxxxxxxx; Ijae Kim <ae878000@xxxxxxxxx>
> > > > > > Subject: [PATCH 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 static analysis.
> > > > >
> > > > > Thanks for your patch!
> > > > >
> > > > > I think this patch is still necessary. It can prevent some
> > > > > extremely unlikely but
> > > > problematic cases, such as an interrupt already being queued in
> > > > the thread before the THC interrupt is disabled. Have you tested
> > > > this code on actual
> > > hardware?
> > > > >
> > > > > Best Regards,
> > > > > Even Xu
> > > > >
> > > > > >
> > > > > > Fixes: 4138f21115ae ("HID: intel-thc-hid: intel-quickspi:
> > > > > > Complete THC QuickSPI
> > > > > > driver")
> > > > > > Assisted-by: LLM
> > > > > > Co-developed-by: Ijae Kim <ae878000@xxxxxxxxx>
> > > > > > Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
> > > > > > Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
> > > > > > ---
> > > > > > Compile-tested with GCC, W=1 (x86_64). No runtime testing.
> > > > > >
> > > > > > Related recovery-work review and series:
> > > > > > https://lore-kernel.gnuweeb.org/linux-input/20260717035609.387
> > > > > > 4015
> > > > > > -1
> > > > > > -
> > > > > > even.xu@xxxxxxxxx/T/
> > > > > > https://patchew.org/linux/20260729050145.2272597-1-even.xu@int
> > > > > > el.c
> > > > > > om
> > > > > > /
> > > > > >
> > > > > > .../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 89226f5ce45e..0bd22f285c78 100644
> > > > > > --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> > > > > > +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> > > > > > @@ -597,6 +597,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;
> > > > > >
> > > > > > @@ -649,6 +651,7 @@ static int quickspi_probe(struct pci_dev *pdev,
> > > > > > qsdev);
> > > > > > if (ret)
> > > > > > goto dev_deinit;
> > > > > > + irq_requested = true;
> > > > > >
> > > > > > ret = reset_tic(qsdev);
> > > > > > if (ret) {
> > > > > > @@ -667,17 +670,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; @@ -692,10 +696,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);
> > > > > >
> > > > > > @@ -718,9 +728,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);
> > > > > >
> > > > > > @@ -747,6 +760,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 */