Hi,
Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>>>> If dwc3 fails to issue START_TRANSFER/UPDATE_TRANSFER command, then we
>>>> should properly end an active transfer and give back all the started
>>>> requests. However if it's for an isoc endpoint, the failure maybe due to
>>>> bus-expiry status. In this case, don't give back the requests and wait
>>>> for the next retry.
>>>>
>>>> Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
>>>> Signed-off-by: Thinh Nguyen <thinhn@xxxxxxxxxxxx>
>>> could you give some details regarding when does this happen?
>>>
>> So, here are the scenarios in which dwc3_send_gadget_ep_cmd() may return
>> a negative errno:
>>
>> * -EAGAIN: Isoc bus-expiry status
>> As you already know, this occurs when we try to schedule isoc too
>> late. If we're going to retry the request, don't unmap it.
> right
>
>> * -EINVAL: No resource due to issuing START_TRANSFER to an already
>> started endpoint
>> This happens generally because of SW programming error
> Sounds like this should be fixed separately and, probably, we should add
> a WARN() so we catch these situations. Have you reproduced this
> particular case?
There are a couple of examples of no resource issue that I recall:
1) When we removed the DWC3_EP_END_TRANSFER_PENDING flag, we wasn't able
to check if the endpoint had ended. So, if the function driver queues a
new request while END_TRANSFER command is in progress, it'd trigger
START_TRANSFER to an already started endpoint
2) For this new method of retrying for isoc, when we issue END_TRANSFER
command, for some controller versions, the controller would generate
XferNotReady event while the END_TRANSFER command is in progress plus
another after it completed. That means we'd start on the same endpoint
twice and trigger a no-resource error. (We'd run into this scenario
because END_TRANSFER completion cleared the started flag).
We added the checks to prevent this issue from happening, so this
scenario should not happen again.
If we want to add a WARN(), I think we should add that inside of
dwc3_send_gadget_ep_cmd() function, as a separate patch. We can also
just look at the tracepoint for "no resource" status.
BR,
Thinh
