> -----Original Message-----
> From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Sent: Tuesday, October 17, 2023 3:24 AM
> To: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Li, Meng <Meng.Li@xxxxxxxxxxxxx>; Ingo Molnar <mingo@xxxxxxxxxx>;
> USB mailing list <linux-usb@xxxxxxxxxxxxxxx>; Sebastian Andrzej Siewior
> <bigeasy@xxxxxxxxxxxxx>; linux-rt-users <linux-rt-users@xxxxxxxxxxxxxxx>
> Subject: Re: USB: add check to detect host controller hardware removal
>
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and
> know the content is safe.
>
> On Mon, Oct 16, 2023 at 12:56:24PM -0400, Steven Rostedt wrote:
> > On Fri, 13 Oct 2023 13:17:52 -0400
> > Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > > --- a/drivers/usb/core/hcd-pci.c
> > > +++ b/drivers/usb/core/hcd-pci.c
> > > @@ -292,6 +292,14 @@ void usb_hcd_pci_remove(struct pci_dev *dev)
> > > if (!hcd)
> > > return;
> > >
> > > + /* Fake an interrupt request in order to give the driver a chance
> > > + * to test whether the controller hardware has been removed (e.g.,
> > > + * cardbus physical eject).
> > > + */
> > > + local_irq_disable();
> > > + usb_hcd_irq(0, hcd);
> > > + local_irq_enable();
> > > +
> > > usb_remove_hcd(hcd);
> > > if (hcd->driver->flags & HCD_MEMORY) {
> > > iounmap(hcd->regs);
> > >
> > > The local_irq_disable() is there so that the irq handler will be
> > > invoked in the state that it expects (i.e., with interrupts disabled).
> > > Apparently Meng's RT kernel doesn't like it when the handler then
> > > calls spin_lock(); I don't know why.
> >
> > Because in RT, spin_lock()s are actually mutexes.
> >
> > In RT, interrupt handlers do not even run with interrupts disabled
> > (they run as threads), so the assumption that they run with interrupts
> > disabled on RT is incorrect. One hack would simply be:
> >
> > if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> > local_irq_disable();
> > usb_hcd_irq(0, hcd);
> > if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> > local_irq_enable();
> >
> > But that's rather ugly. We use to have that as a wrapper of just:
> >
> > local_irq_disable_nort();
> >
> > but I don't know if we removed that or not.
> >
> > Sebastian?
>
> Thanks for the information. I guess a simple approach would be to add the
> wrapper back in, since it's not present in the current kernel.
>
I did some debugs on my side.
Firstly, the local_irq_disable_nort() function had been removed from latest RT kernel.
Second, because of creating xhci-pci.c, the commit c548795abe0d("USB: add check to detect host controller hardware removal") is no longer useful.
Because the function usb_remove_hcd() is invoked from xhci_pci_remove() of file xhci-pci.c in advance.
I am trying to fix this issue with getting register status directly without local_irq_disable().
Thanks,
Limeng
> Alan Stern
