Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 29 Apr 2024 08:23:48 -0700 | Subject | Re: [PATCH v2] usb: ohci: Prevent missed ohci interrupts | From | Guenter Roeck <> |
| |
On 4/29/24 07:01, David Laight wrote: > From: Guenter Roeck >> Sent: 29 April 2024 14:34 >> >> On 4/28/24 23:49, Gerd Hoffmann wrote: >>> Hi, >>> >>>>>> + /* repeat until all enabled interrupts are handled */ >>>>>> + if (ohci->rh_state != OHCI_RH_HALTED) { >>>>>> + ints = ohci_readl(ohci, ®s->intrstatus); >>>>>> + if (ints & ohci_readl(ohci, ®s->intrenable)) >>>>> >>>>> Doesn't the driver know which interrupts are enabled? >>>>> So it should be able to avoid doing two (likely) slow io reads? >>>>> (PCIe reads are pretty much guaranteed to be high latency.) >>>> >>>> No, the driver does not cache intrenable. >>> >>> Does the driver ever change intrenable after initialization? >>> >> >> $ git grep -e intrenable -e intrdisable drivers/usb/host/*ohci*c | grep ohci_writel >> drivers/usb/host/ohci-hcd.c: ohci_writel(ohci, (u32) ~0, &ohci->regs->intrdisable); >> drivers/usb/host/ohci-hcd.c: ohci_writel (ohci, OHCI_INTR_OC, &ohci->regs->intrenable); >> drivers/usb/host/ohci-hcd.c: ohci_writel (ohci, OHCI_INTR_MIE, &ohci->regs->intrdisable); >> drivers/usb/host/ohci-hcd.c: ohci_writel (ohci, mask, &ohci->regs->intrenable); >> drivers/usb/host/ohci-hcd.c: ohci_writel (ohci, OHCI_INTR_UE, ®s->intrdisable); >> drivers/usb/host/ohci-hcd.c: ohci_writel(ohci, OHCI_INTR_RHSC, ®s->intrdisable); >> drivers/usb/host/ohci-hcd.c: ohci_writel (ohci, OHCI_INTR_SF, ®s->intrdisable); >> drivers/usb/host/ohci-hcd.c: ohci_writel (ohci, OHCI_INTR_MIE, ®s->intrenable); >> drivers/usb/host/ohci-hcd.c: ohci_writel (ohci, OHCI_INTR_MIE, &ohci->regs->intrdisable); >> drivers/usb/host/ohci-hcd.c: ohci_writel(ohci, OHCI_INTR_MIE, &ohci->regs->intrdisable); >> drivers/usb/host/ohci-hcd.c: ohci_writel(ohci, OHCI_INTR_MIE, &ohci->regs->intrenable); >> drivers/usb/host/ohci-hub.c: ohci_writel(ohci, OHCI_INTR_SF, &ohci->regs->intrdisable); >> drivers/usb/host/ohci-hub.c: ohci_writel (ohci, OHCI_INTR_INIT, &ohci->regs->intrenable); >> drivers/usb/host/ohci-hub.c: ohci_writel (ohci, OHCI_INTR_SF, &ohci->regs->intrenable); >> drivers/usb/host/ohci-hub.c: ohci_writel(ohci, rhsc_enable, &ohci->regs->intrenable); >> drivers/usb/host/ohci-hub.c: ohci_writel(ohci, OHCI_INTR_RHSC, &ohci->regs->intrenable); >> drivers/usb/host/ohci-q.c: ohci_writel (ohci, OHCI_INTR_SF, &ohci->regs->intrenable); > > At least the hardware has separate enable/disable registers > so the driver isn't doing RMW cycles. > > I'd guess that the normal condition is that no interrupts are pending. > So it can be held to one (slow) read by checking: > if (ints && (ints & ohci_readl(ohci, ®s->intrenable)))
Guess that can't hurt. I'll send v3.
Thanks, Guenter
| ![\](/images/icornerr.gif) |