On 4/11/2018 1:21 AM, Felipe Balbi wrote:
> Hi,
> Felipe Balbi <felipe.ba...@linux.intel.com> writes:
>>>> Without XferNotReady, we won't have a reliable way to know the uFrame
>>>> number. Read the Isochronous programming sequence from your databook.
>>> Right. We need XferNotReady to know when to start isoc transfer. But if
>>> there are still queued requests, DWC3 can just wait to see if any of
>>> them will succeed to continue with the transfer just as how DWC3 is
>>> handling it now.
>> That's not what the databook says though. And that's also not intention
>> of how the code is written as of now either. The way the code is written
>> is the following:
>> queue() -> XferNotReady -> start_isoc() -> if (missed) do_nothing() ->
>> queue() -> end_transfer.
>> That's not really waiting for the queue to be consumed, it's just
>> delaying end transfer until we get another queue(). IOW, it just
>> *happens* to give the controller time to go through the list of started
>> requests.
>>> If we end and restart the transfer right away, then we may lose more
>>> isoc data than necessary (due to isoc scheduling at least 4 uFrame
>>> ahead of time). Is there something you see that doesn't work with the
>>> current implementation?
>> Not _really_, I'm just trying to make the code easier to read and, I
>> think, I've achieved that. Now, if we need to delay end transfer in the
>> case where we have more requests in the controller's queue, that's easy
>> enough to implement:
>> @@ -2371,7 +2371,8 @@ static void 
>> dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
>>      if (event->status & DEPEVT_STATUS_BUSERR)
>>              status = -ECONNRESET;
>> -    if (event->status & DEPEVT_STATUS_MISSED_ISOC) {
>> +    if (event->status & DEPEVT_STATUS_MISSED_ISOC &&
>> +                    list_empty(&dep->started_list) {
>>              status = -EXDEV;

Maybe we should return the -EXDEV status every time there's a missed isoc.

>>              stop = true;
>>      }
>> I'm not sure this is a good idea though. Once we miss an interval, don't
>> we need to know the next frame when transfer needs to be scheduled?
>> Meaning we would need XferNotReady to properly schedule the new
>> transfer?
> thinking about this a little more. This extra list_empty() check is not
> wrong at all :-) I've amended this series with the 3 patches below. I'll
> resend the series once I've given more time for people to test. Patches
> have been updated to the branch on kernel.org as well, btw.

Great! :)
Thanks for all the new updates. I'll test it out when I have a chance.


To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to