On Mon, Jan 27, 2025 at 6:44 PM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
>
> On Fri, Jan 24, 2025, Badhri Jagan Sridharan wrote:
> > While commit d325a1de49d6 ("usb: dwc3: gadget: Prevent losing events
> > in event cache") makes sure that top half(TH) does not end up overwriting
> > the cached events before processing them when the TH gets invoked more
> > than one time, returning IRQ_HANDLED results in occasional irq storm
> > where the TH hogs the CPU. The irq storm can be prevented if
> > IRQ_WAKE_THREAD is returned.
> >
> > ftrace event stub during dwc3 irq storm:
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000866: irq_handler_exit: irq=14 ret=handled
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000872: irq_handler_entry: irq=504 name=dwc3
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000874: irq_handler_exit: irq=504 ret=handled
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000881: irq_handler_entry: irq=504 name=dwc3
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000883: irq_handler_exit: irq=504 ret=handled
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000889: irq_handler_entry: irq=504 name=dwc3
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000892: irq_handler_exit: irq=504 ret=handled
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000898: irq_handler_entry: irq=504 name=dwc3
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000901: irq_handler_exit: irq=504 ret=handled
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000907: irq_handler_entry: irq=504 name=dwc3
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000909: irq_handler_exit: irq=504 ret=handled
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000915: irq_handler_entry: irq=504 name=dwc3
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000918: irq_handler_exit: irq=504 ret=handled
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000924: irq_handler_entry: irq=504 name=dwc3
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000927: irq_handler_exit: irq=504 ret=handled
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000933: irq_handler_entry: irq=504 name=dwc3
> > irq/504_dwc3-1111 ( 1111) [000] .... 70.000935: irq_handler_exit: irq=504 ret=handled
> > ....
> >
> > Cc: stable@xxxxxxxxxx
> > Fixes: d325a1de49d6 ("usb: dwc3: gadget: Prevent losing events in event cache")
>
> I don't think this should be Cc to stable, at least not the way it is
> right now.
>
> > Signed-off-by: Badhri Jagan Sridharan <badhri@xxxxxxxxxx>
> > ---
> > drivers/usb/dwc3/gadget.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index d27af65eb08a..376ab75adc4e 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -4519,7 +4519,7 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
> > * losing events.
> > */
> > if (evt->flags & DWC3_EVENT_PENDING)
> > - return IRQ_HANDLED;
> > + return IRQ_WAKE_THREAD;
>
> This looks like a workaround to the issue we have. Have you tried to
> enable imod instead? It's the feature to help avoid these kind of issue.
Hi Thinh,
Thanks for the feedback ! Yes, we have been experimenting with the
interrupt moderation interval as well.
Have follow up questions though, please bear with me !
1. Given that when DWC3_EVENT_PENDING is still set,
dwc3_check_event_buf() is still waiting for the previous cached events
to be processed by the dwc3_thread_interrupt(), what's the reasoning
behind returning IRQ_HANDLED here ? Shouldn't we be returning
IRQ_WAKE_THREAD anyways ?
2. When interrupt moderation is enabled, does DEVICE_IMODC start to
decrement as soon as the interrupt is masked (where I expect that the
interrupt line gets de-asserted by the controller) in
dwc3_check_event_buf() ?
/* Mask interrupt */
dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0),
DWC3_GEVNTSIZ_INTMASK | DWC3_GEVNTSIZ_SIZE(evt->length));
Regards,
Badhri
>
> Thanks,
> Thinh
>
> >
> > count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
> > count &= DWC3_GEVNTCOUNT_MASK;
> >
> > base-commit: 70cd0576aa39c55aabd227851cba0c601e811fb6
> > --
> > 2.48.1.262.g85cc9f2d1e-goog
> >
