Re: [PATCH] usb: dwc3: Clear DWC3_EVENT_PENDING when count is 0

Linyu Yuan <quic_linyyuan@xxxxxxxxxxx> · Fri, 6 Jan 2023 11:13:07 +0800

On 1/5/2023 5:54 PM, 정재훈 wrote:

-----Original Message-----
From: Linyu Yuan [mailto:quic_linyyuan@xxxxxxxxxxx]
Sent: Thursday, January 5, 2023 12:35 PM
To: JaeHun Jung; Felipe Balbi; Greg Kroah-Hartman; Thinh Nguyen
Cc: open list:USB XHCI DRIVER; open list; Seungchull Suh; Daehwan Jung
Subject: Re: [PATCH] usb: dwc3: Clear DWC3_EVENT_PENDING when count is 0


On 1/5/2023 11:29 AM, Linyu Yuan wrote:
On 1/2/2023 1:08 PM, JaeHun Jung wrote:
Sometimes very rarely, The count is 0 and the DWC3 flag is set has
status.
It must not have these status. Because, It can make happen interrupt
storming status.
could you help explain without clear the flag, how interrupt storming
happen ?

as your change didn't touch any hardware register, i don't know how it
fix storming.

H/W interrupts are still occur on IP.

I guess we should fix it from IP layer.


but when checking DWC3_EVENT_PENDING flag, it will auto clear in dwc3 
thread irq handler.

there is one possible root cause is it cleared only after irq enabled in 
dwc3_process_event_buf(),

we should move unmask irq operation at end of this function.


But. ISR doesn't handle it. Because of this
"if (evt->flags & DWC3_EVENT_PENDING)"

This is event values.
(struct dwc3_event_buffer *) ev_buf = 0xFFFFFF883DBF1180 = kernel_size_le_lo32+0xFFFFFF883B391180 -> (
         (void *) buf = 0xFFFFFFC00DBDD000 = end+0x337D000 -> ,
         (void *) cache = 0xFFFFFF8839F54080 = kernel_size_le_lo32+0xFFFFFF88376F4080 -> ,
         (unsigned int) length = 0x1000,
         (unsigned int) lpos = 0x0,
         (unsigned int) count = 0x0,
         (unsigned int) flags = 0x00100001,
         (dma_addr_t) dma = 0x00000008BD7D7000,

*((struct dwc3_event_type *) 0xFFFFFF8839F54080) = (
     is_devspec = 1,
     type = 0,
     reserved8_31 = 774)

*((struct dwc3_event_devt  *) 0xFFFFFF8839F54080) = (
     one_bit = 1,
     device_event = 0,
     type = 6, << DWC3_DEVICE_EVENT_SUSPEND
     reserved15_12 = 0,
     event_info = 3,
     reserved31_25 = 0)

Occurred interrupts are "DWC3_DEVICE_EVENT_SUSPEND".
If when "count has 0" and DWC3_EVENT_PENDING are set at the same time than
  ISR and bottom thread couldn't handle next interrupt.
We guessing connected with "dwc3_gadget_process_pending_events()" function.
But, We could not find reproduce way.


storming from software layer ?

So, It have to clean up DWC3_EVENT_PENDING flags set when count is 0.
It means "There are no interrupts to handle.".

(struct dwc3_event_buffer *) ev_buf = 0xFFFFFF883DBF1180 (
     (void *) buf = 0xFFFFFFC00DBDD000 = end+0x337D000,
     (void *) cache = 0xFFFFFF8839F54080,
     (unsigned int) length = 0x1000,
     (unsigned int) lpos = 0x0,
     (unsigned int) count = 0x0,
     (unsigned int) flags = 0x00000001,
     (dma_addr_t) dma = 0x00000008BD7D7000,
     (struct dwc3 *) dwc = 0xFFFFFF8839CBC880,
     (u64) android_kabi_reserved1 = 0x0),

(time = 47557628930999, irq = 165, fn = dwc3_interrupt, latency = 0,
en = 1), (time = 47557628931268, irq = 165, fn = dwc3_interrupt,
latency = 0, en = 3), (time = 47557628932383, irq = 165, fn =
dwc3_interrupt, latency = 0, en = 1), (time = 47557628932652, irq =
165, fn = dwc3_interrupt, latency = 0, en = 3), (time =
47557628933768, irq = 165, fn = dwc3_interrupt, latency = 0, en = 1),
(time = 47557628934037, irq = 165, fn = dwc3_interrupt, latency = 0,
en = 3), (time = 47557628935152, irq = 165, fn = dwc3_interrupt,
latency = 0, en = 1), (time = 47557628935460, irq = 165, fn =
dwc3_interrupt, latency = 0, en = 3), (time = 47557628936575, irq =
165, fn = dwc3_interrupt, latency = 0, en = 1), (time =
47557628936845, irq = 165, fn = dwc3_interrupt, latency = 0, en = 3),
(time = 47557628937960, irq = 165, fn = dwc3_interrupt, latency = 0,
en = 1), (time = 47557628938229, irq = 165, fn = dwc3_interrupt,
latency = 0, en = 3), (time = 47557628939345, irq = 165, fn =
dwc3_interrupt, latency = 0, en = 1), (time = 47557628939652, irq =
165, fn = dwc3_interrupt, latency = 0, en = 3), (time =
47557628940768, irq = 165, fn = dwc3_interrupt, latency = 0, en = 1),
(time = 47557628941037, irq = 165, fn = dwc3_interrupt, latency = 0,
en = 3), (time = 47557628942152, irq = 165, fn = dwc3_interrupt,
latency = 0, en = 1), (time = 47557628942422, irq = 165, fn =
dwc3_interrupt, latency = 0, en = 3), (time = 47557628943537, irq =
165, fn = dwc3_interrupt, latency = 0, en = 1), (time =
47557628943806, irq = 165, fn = dwc3_interrupt, latency = 0, en = 3),
(time = 47557628944922, irq = 165, fn = dwc3_interrupt, latency = 0,
en = 1), (time = 47557628945229, irq = 165, fn = dwc3_interrupt,
latency = 0, en = 3), (time = 47557628946345, irq = 165, fn =
dwc3_interrupt, latency = 0, en = 1), (time = 47557628946614, irq =
165, fn = dwc3_interrupt, latency = 0, en = 3), (time =
47557628947729, irq = 165, fn = dwc3_interrupt, latency = 0, en = 1),
(time = 47557628947999, irq = 165, fn = dwc3_interrupt, latency = 0,
en = 3),

Signed-off-by: JaeHun Jung <jh0801.jung@xxxxxxxxxxx>
---
   drivers/usb/dwc3/gadget.c | 5 ++++-
   1 file changed, 4 insertions(+), 1 deletion(-) diff --git
a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index
789976567f9f..5d2d5a9b9915 100644

--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -4355,8 +4355,11 @@ static irqreturn_t dwc3_check_event_buf(struct
dwc3_event_buffer *evt)
        * irq event handler completes before caching new event to
prevent
        * losing events.
        */
-    if (evt->flags & DWC3_EVENT_PENDING)
+    if (evt->flags & DWC3_EVENT_PENDING) {
+        if (!evt->count)
+            evt->flags &= ~DWC3_EVENT_PENDING;
           return IRQ_HANDLED;
+    }
         count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
       count &= DWC3_GEVNTCOUNT_MASK;
how about below change ?

         count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
         count &= DWC3_GEVNTCOUNT_MASK;
-       if (!count)
+       if (!count) {
+               evt->flags &= ~DWC3_EVENT_PENDING;
                 return IRQ_NONE;
+       }
ignore my suggestion, add Thinh to comment.
         evt->count = count;
         evt->flags |= DWC3_EVENT_PENDING;







