Re: [PATCH v1] usb: dwc3: gadget: Prevent irq storm when TH re-executes

[Thinh Nguyen]

On Sat, Feb 01, 2025, Badhri Jagan Sridharan wrote:
> On Fri, Jan 31, 2025 at 4:09 PM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
> >
> > Hmm... Can you try this instead:
> 
> Thanks, this seems to be working !
> I also sent out the following as there isn't a way to enable interrupt
> moderation through the device tree node:
> https://urldefense.com/v3/__https://lore.kernel.org/all/20250202035100.31235-1-badhri@xxxxxxxxxx/__;!!A4F2R9G_pg!e6BA5h7M1-HZjrH2-bLVF0YbmzStu9ASv1lkZudrnyX2RFDlDUnrlFCgMdbwXcbTrMsVUQvgnpRmYxmcV-w$ 
> https://urldefense.com/v3/__https://lore.kernel.org/all/20250202035100.31235-2-badhri@xxxxxxxxxx/__;!!A4F2R9G_pg!e6BA5h7M1-HZjrH2-bLVF0YbmzStu9ASv1lkZudrnyX2RFDlDUnrlFCgMdbwXcbTrMsVUQvgnpRm9hdUtjE$ 
> 
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 0fe92c0fb520..c1b5a3742ab4 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -5737,14 +5737,20 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3_event_buffer *evt)
> >         dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0),
> >                     DWC3_GEVNTSIZ_SIZE(evt->length));
> >
> > +       /*
> > +        * Keep the clearing of DWC3_EVENT_PENDING after the interrupt unmask
> > +        * but before the clearing of DWC3_GEVNTCOUNT_EHB.
> > +        */
> > +       evt->flags &= ~DWC3_EVENT_PENDING;
> > +
> > +       /* Ensure the flag is updated before clearing DWC3_GEVNTCOUNT_EHB */
> > +       wmb();
> 
> I still have one more question though :)
> Wondering why not move this code about the DWC3_GEVNTSIZ write where
> the interrupt is actually unmasked that way this would also work for
> systems which dont have interrupt moderation enabled right ?
> 

It's mainly for PCI devices. PCI devices sends assert/de-assert messages
to represent the level interrupt for PCI legacy interrupt. The de-assert
interrupt message may not be received right after masking the interrupt,
such as due to system latency. This may trigger a scheduling of the
TH again. If there's new event received during the TH, this will cause a
race and dwc3 driver may overwrite the cached events (a window between
clearing the flag and unmasking of interrupt).

We may have a potential issue for platform with devices with shared
interrupt too (though I haven't seen report of this issue before).

BR,
Thinh