Baolin Wang <baolin.w...@linaro.org> writes:
>>>>>> >> 8<------------------------------------------------------------------------
>>>>>> >> From f3fa94f3171709f787a30e3c5ce69a668960b66e Mon Sep 17 00:00:00 2001
>>>>>> >> From: Felipe Balbi <felipe.ba...@linux.intel.com>
>>>>>> >> Date: Thu, 13 Oct 2016 14:09:47 +0300
>>>>>> >> Subject: [PATCH v2] usb: dwc3: gadget: wait for End Transfer to 
>>>>>> >> complete
>>>>>> >>
>>>>>> >> Instead of just delaying for 100us, we should
>>>>>> >> actually wait for End Transfer Command Complete
>>>>>> >> interrupt before moving on. Note that this should
>>>>>> >> only be done if we're dealing with one of the core
>>>>>> >> revisions that actually require the interrupt before
>>>>>> >> moving on.
>>>>>> >>
>>>>>> >> Reported-by: Baolin Wang <baolin.w...@linaro.org>
>>>>>> >> Signed-off-by: Felipe Balbi <felipe.ba...@linux.intel.com>
>>>>>> >
>>>>>> > From my testing, there are still some problems caused by the nested
>>>>>> > lock, at lease I found 2 functions will issue the usb_ep_disable()
>>>>>> > function with spinlock:
>>>>>> Thanks for testing. Seems like I really need to revisit locking in the
>>>>>> entire gadget API. In either case, we need to have a larger discussion
>>>>>> about this situation.
>>>>>> IMO, usb_ep_disable() and usb_ep_enable() should only be callable from
>>>>>> process context, but that has never been a requirement before. Still,
>>>>>> it's not far-fetched to assume that a controller (such as dwc3, but
>>>>>> probably others) might sleep while cancelling a transfer because they
>>>>>> need to wait for an Interrupt.
>>>>>> In fact, we know of two controllers that need this: dwc3 and dwc3.1.
>>>>> It's not clear that this should be the deciding factor.  That is, does
>>>>> usb_ep_disable() need to wait until all the endpoint's outstanding
>>>>> transfers have been completed before it returns?  I don't think it
>>>>> does.
>>>> not all, no. And, frankly, this is really only a problem if we're
>>>> unregistering a gadget while there are still pending transfers.
>>>> The original problem statement is that we unregister a gadget, issue End
>>>> Transfer, but CmdCompletion for the EndTransfer comes way too
>>>> late. Baolin, can you clarify again what happens when the IRQ comes
>>>> late?
>>> Sure. The problem I met was, when we change the USB function with
>>> configfs frequently, sometimes it will hang the system to crash. The
>>> reason is,  we will start end transfer command when disable the
>>> endpoint, but sometimes the end transfer command complete event comes
>>> after we have freed the gadget irq (since the xHCI will share the same
>>> interrupt number with gadget, thus when free the gadget irq, it will
>>> not shutdown this gadget irq line), it will trigger the interrupt line
>>> all the time.
>> okay, thanks for describing it again. Another option would be to only
>> wait_for_completion() before calling free_irq() is any endpoint has
>> END_TRANSFER_PENDING flag set. Something like below:
> Yeah, that is what my original patch did, but you did one better
> patch, I will test it too.

That's true. I'll give you proper credit in the commit log since you
originated the idea.


Attachment: signature.asc
Description: PGP signature

Reply via email to