Hi,
Thinh Nguyen <thinh.nguyen@xxxxxxxxxxxx> writes:
>>> If it fails for over 5 times in a row, then we should probably wait for
>>> the next XferNotReady to get the frame_number again. Also 5 is an
>>> arbitrary number I came up without any testing, we can probably decide
>>> on a better number. What do you think?
>> I have this now:
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 131028501752..3390fa46ea30 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -37,6 +37,7 @@
>> #define DWC3_EP0_SETUP_SIZE 512
>> #define DWC3_ENDPOINTS_NUM 32
>> #define DWC3_XHCI_RESOURCES_NUM 2
>> +#define DWC3_ISOC_MAX_RETRIES 5
>>
>> #define DWC3_SCRATCHBUF_SIZE 4096 /* each buffer is assumed to be 4KiB */
>> #define DWC3_EVENT_BUFFERS_SIZE 4096
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index d8c7ad0c22e8..1590516735cb 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -27,7 +27,7 @@
>> #include "gadget.h"
>> #include "io.h"
>>
>> -#define DWC3_ALIGN_FRAME(d) (((d)->frame_number + (d)->interval) \
>> +#define DWC3_ALIGN_FRAME(d, n) (((d)->frame_number + ((d)->interval * (n))) \
>> & ~((d)->interval - 1))
>>
>> /**
>> @@ -1268,13 +1268,24 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
>>
>> static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
>> {
>> + int retries;
>> + int ret;
>> + int i;
>> +
>> if (list_empty(&dep->pending_list)) {
>> dep->flags |= DWC3_EP_PENDING_REQUEST;
>> return -EAGAIN;
>> }
>>
>> - dep->frame_number = DWC3_ALIGN_FRAME(dep);
>> - return __dwc3_gadget_kick_transfer(dep);
>> + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) {
>> + dep->frame_number = DWC3_ALIGN_FRAME(dep, i);
>
> Shouldn't this be like this?
> dep->frame_number = DWC3_ALIGN_FRAME(dep, i + 1);
good catch, I'll fix.
>> +
>> + ret = __dwc3_gadget_kick_transfer(dep);
>> + if (ret != -EAGAIN)
>> + break;
>> + }
>> +
>> + return ret;
>> }
>>
>> static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)
>>
>> This means we will increment in full intervals, up to 5 intervals in the
>> future. If it still fails, then there's not much we can do.
>>
> I agree. Also, depending on the application requirement, we may want to
> giveback/cleanup request(s) every time we fail so that the data won't be
> pushed back/delayed for too long.
> We could experiment and decide on the maximum delay time (base on the
> service interval length and number of retries) to be considered
> acceptable to start giveback stale requests.
That's something we can consider in the future. I'd say that the gadget
driver should, then, tell us how much slack to give.
--
balbi
Attachment:
signature.asc
Description: PGP signature
