Re: [PATCH v2] usb: dwc3: Prevent indefinite sleep in _dwc3_set_mode during suspend/resume

From: Minas Harutyunyan
Date: Mon Mar 19 2018 - 07:36:30 EST


Hi,

On 3/19/2018 12:55 PM, Felipe Balbi wrote:
>
> Hi,
>
> Minas Harutyunyan <Minas.Harutyunyan@xxxxxxxxxxxx> writes:
>>>>>>> Thanks for picking this for -next.
>>>>>>> Is it better to have this in v4.16-rc fixes?
>>>>>>> and also stable? v4.12+
>>>>>>
>>>>>> Well, there was no "Fixes: foobar" or "Cc: stable" lines in the commit
>>>>>> log ;-)
>>>>>>
>>>>>> The best we can do now, is wait for -rc1 and manually send the commit to
>>>>>> stable.
>>>>>>
>>>>>
>>>>> That's fine. Thanks.
>>>>>
>>>>
>>>> Same issue seen in dwc3_gadget_ep_dequeue() function where also used
>>>> wait_event_lock_irq() - as result infinite loop.
>>>
>>> how did this happen? During rmmod dwc3? Or, perhaps, after you unloaded
>>> a gadget driver?
>>>
>> No, not during rmmod's.
>> We using our internal USB testing tool. Test case; ISOC OUT, transfer
>> size N frames. When host starts ISOC OUT traffic then the dwc3 based on
>> "Transfer not ready" event in frame F starts transfers staring from
>> frame F+4 (for bInterval=1) as result 4 requests, which already queued
>> on device side, remain incomplete. Function driver on some timeout
>> trying dequeue these 4 requests (without disabling EP) to complete test.
>> For IN ISOC's these requests completed on MISSED ISOC event, but for
>> ISOC OUT required call dequeue on some timeout.
>
> okay
>
>>>> Actually to fix this issue I updated condition of wait function
>>>> from:
>>>> !(dep->flags & DWC3_EP_END_TRANSFER_PENDING)
>>>> to:
>>>> !(dep->flags & DWC3_EP_END_TRANSFER_PENDING & DWC3_EP_ENABLED)
>>>
>>> you're not fixing anything. You're, essentially, removing the entire
>>> end transfer pending logic.
>> yes, you are right, but how to overcome this infinite loop? Replace
>> wait_event_lock_irq() by wait_event_interruptible_lock_irq_timeout()?
>
> The best way here would be to figure why we're missing command complete
> IRQ in those cases. According to documentation, we *should* receive that
> interrupt, so why is it missing?
>

Additional info on test. Core configuration is HS only mode, test speed
HS, core version v2.90a. Maybe it will help to understand cause of issue.
BTW, currently to pass above describe ISOC OUT test we just commented
wait_event_lock_irq() in dwc3_gadget_ep_dequeue() function and
successfully received request completion in function driver.
Thanks,
Minas