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

[Xu, Even]

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.3874015-1
> > > -
> > > even.xu@xxxxxxxxx/T/
> > > https://patchew.org/linux/20260729050145.2272597-1-even.xu@xxxxxxxxx
> > > /
> > >
> > >  .../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 */