Hi,
John Youn <John.Youn@xxxxxxxxxxxx> writes:
>>>>> The device hangs at the end of the log.
>>>
>>> I repeated the test with USB 3.0 IP version 2.90a and USB 3.1 IP version
>>> 1.60, and I found the same result where the TH is occasionally called
>>> twice back-to-back.
>>>
>>>>
>>>> Right, found this in the logs. So the only thing that I can think of
>>>> here, is that we need to flush posted writes. Does this help?
>>>>
>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>> index 79e7a3480d51..6f06738273f2 100644
>>>> --- a/drivers/usb/dwc3/gadget.c
>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>> @@ -3099,6 +3099,10 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
>>>>
>>>> dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);
>>>>
>>>> + /* flush posted writes */
>>>> + dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
>>>> + dwc3_readl(dwc->regs, DWC3_GEVNTSIZ(0));
>>>> +
>>>> return IRQ_WAKE_THREAD;
>>>> }
>>>>
>>>>
>>>
>>> With the change above, I don't see the TH get called twice in in a row.
>>> However, I also tested with replacing the code above with a udelay(4),
>>> from reading the kernel trace, to take in account the latency of reading
>>> the registers. I also don't see TH get called twice in a row.
>>>
>>> The extra latency from reading the registers above can mask this issue.
>>
>> okay, so it is a problem with posted writes. Can you send above as a
>> formal patch? That's something we can backport to every stable
>> kernel. And the real reason here _IS_ caused by posted writes. There's
>> no spurious interrupt happening.
>>
>
> Hi Felipe,
>
> I only mentioned spurious interrupts as something the driver needs to
> account for anyways. They are not the cause of this issue. And I don't
> think the PCIe posted writes are either becuase of what I said before
> about the interrupts being packets and the latency in assertion and
> detection.
>
> But anyways, after thorough testing we don't see this in our system
> anymore with the read-back (probably due to the 4 us delay that's
> introduced). We can submit a patch for this for stable.
please do :-)
> However I don't think this is a sufficient fix for mainline, and dwc3
> should be able to handle multiple interrupts before BH, whatever the
> reason for it.
>
> How do you want to handle mainline? Either Thinh's original patch,
> create a queue for the events, or some other way? We can work on this
> fix too if you want.
Can't we just check for the PENDING flag? something like:
@@ -3052,6 +3052,9 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
u32 count;
u32 reg;
+ if (evt->flags & DWC3_EVENT_PENDING)
+ return IRQ_HANDLED;
+
if (pm_runtime_suspended(dwc->dev)) {
pm_runtime_get(dwc->dev);
disable_irq_nosync(dwc->irq_gadget);
--
balbi
Attachment:
signature.asc
Description: PGP signature
