Hi Alan, > From: Alan Stern, Sent: Tuesday, August 27, 2019 12:58 AM > > On Mon, 26 Aug 2019, Yoshihiro Shimoda wrote: > > > Hi Alan, > > > > > From: Alan Stern, Sent: Saturday, August 24, 2019 12:33 AM > > > > > > On Fri, 23 Aug 2019, Yoshihiro Shimoda wrote: <snip> > > > > The ohci_shutdown() should hold the spin lock while disabling > > > > the interruption and changing the rh_state flag. Note that > > > > io_watchdog_func() also calls the ohci_shutdown() and it > > > > already held the spin lock, so that the patch makes a new > > > > function as _ohci_shutdown(). > > > > > > I don't understand this description. It sounds like the OHCI > > > controller generates an interrupt request, and then ohci_shutdown() > > > disables the interrupt request before the handler can run. When the > > > handler does run, it sees that no interrupts are enabled and so it > > > returns IRQ_NOTMINE, leading to the error shown above. > > > > > > How will holding the spinlock fix this problem? > > > > I'm sorry for lacking description. I should have described the following > > descriptions instead of that. What do you think? > > > > -- > > ohci_shutdown() disables all the interrupt and rh_state is set to > > OHCI_RH_HALTED. In other hand, ohci_irq() is possible to enable > > OHCI_INTR_SF and OHCI_INTR_MIE on ohci_irq(). Note that OHCI_INTR_SF > > is possible to be set by start_ed_unlink() which is called: > > ohci_irq() > > -> process_done_list() > > -> takeback_td() > > -> start_ed_unlink() > > > > So, ohci_irq() has the following condition, the issue happens by > > &ohci->regs->intrenable = OHCI_INTR_MIE | OHCI_INTR_SF and > > ohci->rh_state = OHCI_RH_HALTED: > > > > /* interrupt for some other device? */ > > if (ints == 0 || unlikely(ohci->rh_state == OHCI_RH_HALTED)) > > return IRQ_NOTMINE; > > > > To fix the issue, ohci_shutdown() holds the spin lock while disabling > > the interruption and changing the rh_state flag to prevent reenable > > the OHCI_INTR_MIE unexpectedly. Note that io_watchdog_func() also calls > > the ohci_shutdown() and it already held the spin lock, so that the patch > > makes a new function as _ohci_shutdown(). > > Okay, that's a lot better. Please resubmit the patch with the new > description. Thank you for your review! I have submitted v2 patch as the following: https://patchwork.kernel.org/patch/11115947/ Best regards, Yoshihiro Shimoda
