| Message ID | 20181108065743.20863-4-felipe.balbi@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] usb: dwc3: gadget: return errors from __dwc3_gadget_start_isoc() | expand |
Hi Felipe, On 11/7/2018 10:58 PM, Felipe Balbi wrote: > Gadget driver may take an unbounded amount of time to queue requests > after XferNotReady. This is important for isochronous endpoints which > need to be started for a specific (micro-)frame. > > Before kicking the transfer, let's check how much time has elapsed > since dep->frame_number was updated and make sure we start the request > to the next valid interval. > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> > --- > drivers/usb/dwc3/core.h | 5 +++++ > drivers/usb/dwc3/gadget.c | 11 +++++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 131028501752..306a2dd75ed5 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -651,6 +651,7 @@ struct dwc3_event_buffer { > * @number: endpoint number (1 - 15) > * @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK > * @resource_index: Resource transfer index > + * @frame_timestamp: timestamp of most recent frame number > * @frame_number: set to the frame number we want this transfer to start (ISOC) > * @interval: the interval on which the ISOC transfer is started > * @name: a human readable name e.g. ep1out-bulk > @@ -697,7 +698,11 @@ struct dwc3_ep { > u8 number; > u8 type; > u8 resource_index; > + > + u64 frame_timestamp; > u32 frame_number; > +#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff > + > u32 interval; > > char name[20]; > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index d8c7ad0c22e8..00fe01a01977 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1268,12 +1268,22 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc) > > static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) > { > + u64 current_timestamp; > + u64 diff_timestamp; > + u32 elapsed_frames; > + > if (list_empty(&dep->pending_list)) { > dep->flags |= DWC3_EP_PENDING_REQUEST; > return -EAGAIN; > } > > + current_timestamp = ktime_get_ns(); > + diff_timestamp = current_timestamp - dep->frame_timestamp; > + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); > + > + dep->frame_number += elapsed_frames; > dep->frame_number = DWC3_ALIGN_FRAME(dep); > + > return __dwc3_gadget_kick_transfer(dep); > } > > @@ -2320,6 +2330,7 @@ static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep, > const struct dwc3_event_depevt *event) > { > dep->frame_number = event->parameters; > + dep->frame_timestamp = ktime_get_ns(); > } > > static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, This may not be enough. The dep->frame_timestamp may not correspond to the frame_number from XferNotReady event. When there's system latency (which is possible when this failure happens), the time the driver handle the event may be a few uframes passed the time the controller's XferInProgress uframe parameter. Rather than starting the isoc transfer immediately on the next interval. How about starting the transfer with some minimum buffer uframes just like before? (e.g. frame_number + max(4, interval)) Thinh
Hi, Thinh Nguyen <thinh.nguyen@synopsys.com> writes: > On 11/7/2018 10:58 PM, Felipe Balbi wrote: >> Gadget driver may take an unbounded amount of time to queue requests >> after XferNotReady. This is important for isochronous endpoints which >> need to be started for a specific (micro-)frame. >> >> Before kicking the transfer, let's check how much time has elapsed >> since dep->frame_number was updated and make sure we start the request >> to the next valid interval. >> >> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> >> --- >> drivers/usb/dwc3/core.h | 5 +++++ >> drivers/usb/dwc3/gadget.c | 11 +++++++++++ >> 2 files changed, 16 insertions(+) >> >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >> index 131028501752..306a2dd75ed5 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -651,6 +651,7 @@ struct dwc3_event_buffer { >> * @number: endpoint number (1 - 15) >> * @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK >> * @resource_index: Resource transfer index >> + * @frame_timestamp: timestamp of most recent frame number >> * @frame_number: set to the frame number we want this transfer to start (ISOC) >> * @interval: the interval on which the ISOC transfer is started >> * @name: a human readable name e.g. ep1out-bulk >> @@ -697,7 +698,11 @@ struct dwc3_ep { >> u8 number; >> u8 type; >> u8 resource_index; >> + >> + u64 frame_timestamp; >> u32 frame_number; >> +#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff >> + >> u32 interval; >> >> char name[20]; >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index d8c7ad0c22e8..00fe01a01977 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -1268,12 +1268,22 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc) >> >> static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) >> { >> + u64 current_timestamp; >> + u64 diff_timestamp; >> + u32 elapsed_frames; >> + >> if (list_empty(&dep->pending_list)) { >> dep->flags |= DWC3_EP_PENDING_REQUEST; >> return -EAGAIN; >> } >> >> + current_timestamp = ktime_get_ns(); >> + diff_timestamp = current_timestamp - dep->frame_timestamp; >> + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >> + >> + dep->frame_number += elapsed_frames; >> dep->frame_number = DWC3_ALIGN_FRAME(dep); >> + >> return __dwc3_gadget_kick_transfer(dep); >> } >> >> @@ -2320,6 +2330,7 @@ static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep, >> const struct dwc3_event_depevt *event) >> { >> dep->frame_number = event->parameters; >> + dep->frame_timestamp = ktime_get_ns(); >> } >> >> static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, > > This may not be enough. The dep->frame_timestamp may not correspond to > the frame_number from XferNotReady event. When there's system latency > (which is possible when this failure happens), the time the driver > handle the event may be a few uframes passed the time the controller's > XferInProgress uframe parameter. > > Rather than starting the isoc transfer immediately on the next interval. > How about starting the transfer with some minimum buffer uframes just > like before? (e.g. frame_number + max(4, interval)) The problem with this is cases with interval of 1ms. This will result in a 4ms delay. I really want to start transfer as soon as possible and the timestamp trick seems to be the best idea so far, without resorting to 4 intervals delay. We do, however, have the possibility that this will start 2 intervals in the future because of the usage of DIV_ROUND_UP_ULL() and because of how DWC3_ALIGN_FRAME() is implemented. I understand what you're saying, though, but it seems like we don't have to avoid that case completely. We can only make it less likely.
Hi Felipe, On 11/8/2018 11:11 PM, Felipe Balbi wrote: > Hi, > > Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >> On 11/7/2018 10:58 PM, Felipe Balbi wrote: >>> Gadget driver may take an unbounded amount of time to queue requests >>> after XferNotReady. This is important for isochronous endpoints which >>> need to be started for a specific (micro-)frame. >>> >>> Before kicking the transfer, let's check how much time has elapsed >>> since dep->frame_number was updated and make sure we start the request >>> to the next valid interval. >>> >>> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> >>> --- >>> drivers/usb/dwc3/core.h | 5 +++++ >>> drivers/usb/dwc3/gadget.c | 11 +++++++++++ >>> 2 files changed, 16 insertions(+) >>> >>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >>> index 131028501752..306a2dd75ed5 100644 >>> --- a/drivers/usb/dwc3/core.h >>> +++ b/drivers/usb/dwc3/core.h >>> @@ -651,6 +651,7 @@ struct dwc3_event_buffer { >>> * @number: endpoint number (1 - 15) >>> * @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK >>> * @resource_index: Resource transfer index >>> + * @frame_timestamp: timestamp of most recent frame number >>> * @frame_number: set to the frame number we want this transfer to start (ISOC) >>> * @interval: the interval on which the ISOC transfer is started >>> * @name: a human readable name e.g. ep1out-bulk >>> @@ -697,7 +698,11 @@ struct dwc3_ep { >>> u8 number; >>> u8 type; >>> u8 resource_index; >>> + >>> + u64 frame_timestamp; >>> u32 frame_number; >>> +#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff >>> + >>> u32 interval; >>> >>> char name[20]; >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index d8c7ad0c22e8..00fe01a01977 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -1268,12 +1268,22 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc) >>> >>> static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) >>> { >>> + u64 current_timestamp; >>> + u64 diff_timestamp; >>> + u32 elapsed_frames; >>> + >>> if (list_empty(&dep->pending_list)) { >>> dep->flags |= DWC3_EP_PENDING_REQUEST; >>> return -EAGAIN; >>> } >>> >>> + current_timestamp = ktime_get_ns(); >>> + diff_timestamp = current_timestamp - dep->frame_timestamp; >>> + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >>> + >>> + dep->frame_number += elapsed_frames; >>> dep->frame_number = DWC3_ALIGN_FRAME(dep); >>> + >>> return __dwc3_gadget_kick_transfer(dep); >>> } >>> >>> @@ -2320,6 +2330,7 @@ static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep, >>> const struct dwc3_event_depevt *event) >>> { >>> dep->frame_number = event->parameters; >>> + dep->frame_timestamp = ktime_get_ns(); >>> } >>> >>> static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, >> This may not be enough. The dep->frame_timestamp may not correspond to >> the frame_number from XferNotReady event. When there's system latency >> (which is possible when this failure happens), the time the driver >> handle the event may be a few uframes passed the time the controller's >> XferInProgress uframe parameter. >> >> Rather than starting the isoc transfer immediately on the next interval. >> How about starting the transfer with some minimum buffer uframes just >> like before? (e.g. frame_number + max(4, interval)) > The problem with this is cases with interval of 1ms. This will result in > a 4ms delay. I really want to start transfer as soon as possible and the > timestamp trick seems to be the best idea so far, without resorting to > 4 intervals delay. We do, however, have the possibility that this will > start 2 intervals in the future because of the usage of > DIV_ROUND_UP_ULL() and because of how DWC3_ALIGN_FRAME() is implemented. > > I understand what you're saying, though, but it seems like we don't > have to avoid that case completely. We can only make it less likely. > In the case of interval of 1ms, it will start on the next interval. frame_number + max(4, interval) will start at least 4 uframes in the future. In any case, what about immediately retry the START_TRANSFER command with a new frame_number + (interval*retry) should it fail with bus-expiry? You can set the number of retries to maybe 5 times. This should remove the need to do time stamping. Thinh
Hi, Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>> index d8c7ad0c22e8..00fe01a01977 100644 >>>> --- a/drivers/usb/dwc3/gadget.c >>>> +++ b/drivers/usb/dwc3/gadget.c >>>> @@ -1268,12 +1268,22 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc) >>>> >>>> static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) >>>> { >>>> + u64 current_timestamp; >>>> + u64 diff_timestamp; >>>> + u32 elapsed_frames; >>>> + >>>> if (list_empty(&dep->pending_list)) { >>>> dep->flags |= DWC3_EP_PENDING_REQUEST; >>>> return -EAGAIN; >>>> } >>>> >>>> + current_timestamp = ktime_get_ns(); >>>> + diff_timestamp = current_timestamp - dep->frame_timestamp; >>>> + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >>>> + >>>> + dep->frame_number += elapsed_frames; >>>> dep->frame_number = DWC3_ALIGN_FRAME(dep); >>>> + >>>> return __dwc3_gadget_kick_transfer(dep); >>>> } >>>> >>>> @@ -2320,6 +2330,7 @@ static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep, >>>> const struct dwc3_event_depevt *event) >>>> { >>>> dep->frame_number = event->parameters; >>>> + dep->frame_timestamp = ktime_get_ns(); >>>> } >>>> >>>> static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, >>> This may not be enough. The dep->frame_timestamp may not correspond to >>> the frame_number from XferNotReady event. When there's system latency >>> (which is possible when this failure happens), the time the driver >>> handle the event may be a few uframes passed the time the controller's >>> XferInProgress uframe parameter. >>> >>> Rather than starting the isoc transfer immediately on the next interval. >>> How about starting the transfer with some minimum buffer uframes just >>> like before? (e.g. frame_number + max(4, interval)) >> The problem with this is cases with interval of 1ms. This will result in >> a 4ms delay. I really want to start transfer as soon as possible and the >> timestamp trick seems to be the best idea so far, without resorting to >> 4 intervals delay. We do, however, have the possibility that this will >> start 2 intervals in the future because of the usage of >> DIV_ROUND_UP_ULL() and because of how DWC3_ALIGN_FRAME() is implemented. >> >> I understand what you're saying, though, but it seems like we don't >> have to avoid that case completely. We can only make it less likely. >> > > In the case of interval of 1ms, it will start on the next interval. > frame_number + max(4, interval) will start at least 4 uframes in the future. > > In any case, what about immediately retry the START_TRANSFER command > with a new frame_number + (interval*retry) should it fail with > bus-expiry? You can set the number of retries to maybe 5 times. This > should remove the need to do time stamping. That seems like a good idea. Something like below? (on top of $subject) modified 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 modified drivers/usb/dwc3/gadget.c @@ -1271,20 +1271,29 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) u64 current_timestamp; u64 diff_timestamp; u32 elapsed_frames; + int retries; + int delta = 1 + int ret; if (list_empty(&dep->pending_list)) { dep->flags |= DWC3_EP_PENDING_REQUEST; return -EAGAIN; } - current_timestamp = ktime_get_ns(); - diff_timestamp = current_timestamp - dep->frame_timestamp; - elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) { + current_timestamp = ktime_get_ns(); + diff_timestamp = current_timestamp - dep->frame_timestamp; + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); - dep->frame_number += elapsed_frames; - dep->frame_number = DWC3_ALIGN_FRAME(dep); + dep->frame_number += elapsed_frames + (delta * i); + dep->frame_number = DWC3_ALIGN_FRAME(dep); - return __dwc3_gadget_kick_transfer(dep); + 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)
Hi again, Felipe Balbi <felipe.balbi@linux.intel.com> writes: > In the case of interval of 1ms, it will start on the next interval. >> frame_number + max(4, interval) will start at least 4 uframes in the future. >> >> In any case, what about immediately retry the START_TRANSFER command >> with a new frame_number + (interval*retry) should it fail with >> bus-expiry? You can set the number of retries to maybe 5 times. This >> should remove the need to do time stamping. > > That seems like a good idea. Something like below? (on top of $subject) > > modified 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 > modified drivers/usb/dwc3/gadget.c > @@ -1271,20 +1271,29 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) > u64 current_timestamp; > u64 diff_timestamp; > u32 elapsed_frames; > + int retries; > + int delta = 1 > + int ret; > > if (list_empty(&dep->pending_list)) { > dep->flags |= DWC3_EP_PENDING_REQUEST; > return -EAGAIN; > } > > - current_timestamp = ktime_get_ns(); > - diff_timestamp = current_timestamp - dep->frame_timestamp; > - elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); > + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) { > + current_timestamp = ktime_get_ns(); > + diff_timestamp = current_timestamp - dep->frame_timestamp; > + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); > > - dep->frame_number += elapsed_frames; > - dep->frame_number = DWC3_ALIGN_FRAME(dep); > + dep->frame_number += elapsed_frames + (delta * i); the other possibility is that we can call DWC3_ALIGN_FRAME() n times since that will put the transfer on the following interval. That would look like so: modified 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 modified 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)) /** @@ -1271,20 +1271,28 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) u64 current_timestamp; u64 diff_timestamp; u32 elapsed_frames; + int retries; + int ret; if (list_empty(&dep->pending_list)) { dep->flags |= DWC3_EP_PENDING_REQUEST; return -EAGAIN; } - current_timestamp = ktime_get_ns(); - diff_timestamp = current_timestamp - dep->frame_timestamp; - elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) { + current_timestamp = ktime_get_ns(); + diff_timestamp = current_timestamp - dep->frame_timestamp; + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); - dep->frame_number += elapsed_frames; - dep->frame_number = DWC3_ALIGN_FRAME(dep); + dep->frame_number += elapsed_frames; + dep->frame_number = DWC3_ALIGN_FRAME(dep, i); - return __dwc3_gadget_kick_transfer(dep); + 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)
Hi Felipe, On 11/9/2018 3:04 AM, Felipe Balbi wrote: > Hi again, > > Felipe Balbi <felipe.balbi@linux.intel.com> writes: >> In the case of interval of 1ms, it will start on the next interval. >>> frame_number + max(4, interval) will start at least 4 uframes in the future. >>> >>> In any case, what about immediately retry the START_TRANSFER command >>> with a new frame_number + (interval*retry) should it fail with >>> bus-expiry? You can set the number of retries to maybe 5 times. This >>> should remove the need to do time stamping. >> That seems like a good idea. Something like below? (on top of $subject) >> >> modified 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 >> modified drivers/usb/dwc3/gadget.c >> @@ -1271,20 +1271,29 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) >> u64 current_timestamp; >> u64 diff_timestamp; >> u32 elapsed_frames; >> + int retries; >> + int delta = 1 >> + int ret; >> >> if (list_empty(&dep->pending_list)) { >> dep->flags |= DWC3_EP_PENDING_REQUEST; >> return -EAGAIN; >> } >> >> - current_timestamp = ktime_get_ns(); >> - diff_timestamp = current_timestamp - dep->frame_timestamp; >> - elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >> + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) { >> + current_timestamp = ktime_get_ns(); >> + diff_timestamp = current_timestamp - dep->frame_timestamp; >> + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >> >> - dep->frame_number += elapsed_frames; >> - dep->frame_number = DWC3_ALIGN_FRAME(dep); >> + dep->frame_number += elapsed_frames + (delta * i); > the other possibility is that we can call DWC3_ALIGN_FRAME() n times > since that will put the transfer on the following interval. That would > look like so: > > modified 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 > modified 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)) > > /** > @@ -1271,20 +1271,28 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) > u64 current_timestamp; > u64 diff_timestamp; > u32 elapsed_frames; > + int retries; > + int ret; > > if (list_empty(&dep->pending_list)) { > dep->flags |= DWC3_EP_PENDING_REQUEST; > return -EAGAIN; > } > > - current_timestamp = ktime_get_ns(); > - diff_timestamp = current_timestamp - dep->frame_timestamp; > - elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); > + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) { > + current_timestamp = ktime_get_ns(); > + diff_timestamp = current_timestamp - dep->frame_timestamp; > + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); > > - dep->frame_number += elapsed_frames; > - dep->frame_number = DWC3_ALIGN_FRAME(dep); > + dep->frame_number += elapsed_frames; > + dep->frame_number = DWC3_ALIGN_FRAME(dep, i); > > - return __dwc3_gadget_kick_transfer(dep); > + 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) > > I like the second method. But do we need to keep track of the frame_timestamp? I don't think it accurately reflects the timestamp of XferNotReady frame number. As for the number of retries, we should adjust it according to the service interval. For example, for larger service interval such as 16, then we don't need to try more than once. To calculate the max number of retries, we can do this check (where interval is from 1 to 16): if (interval >= (16 - (MAX_NUM_RETRIES >> 1)) num_retries = 1 << (16 - interval); Please check my math.. 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? BR, Thinh
Hi, Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >> Felipe Balbi <felipe.balbi@linux.intel.com> writes: >>> In the case of interval of 1ms, it will start on the next interval. >>>> frame_number + max(4, interval) will start at least 4 uframes in the future. >>>> >>>> In any case, what about immediately retry the START_TRANSFER command >>>> with a new frame_number + (interval*retry) should it fail with >>>> bus-expiry? You can set the number of retries to maybe 5 times. This >>>> should remove the need to do time stamping. >>> That seems like a good idea. Something like below? (on top of $subject) >>> >>> modified 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 >>> modified drivers/usb/dwc3/gadget.c >>> @@ -1271,20 +1271,29 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) >>> u64 current_timestamp; >>> u64 diff_timestamp; >>> u32 elapsed_frames; >>> + int retries; >>> + int delta = 1 >>> + int ret; >>> >>> if (list_empty(&dep->pending_list)) { >>> dep->flags |= DWC3_EP_PENDING_REQUEST; >>> return -EAGAIN; >>> } >>> >>> - current_timestamp = ktime_get_ns(); >>> - diff_timestamp = current_timestamp - dep->frame_timestamp; >>> - elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >>> + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) { >>> + current_timestamp = ktime_get_ns(); >>> + diff_timestamp = current_timestamp - dep->frame_timestamp; >>> + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >>> >>> - dep->frame_number += elapsed_frames; >>> - dep->frame_number = DWC3_ALIGN_FRAME(dep); >>> + dep->frame_number += elapsed_frames + (delta * i); >> the other possibility is that we can call DWC3_ALIGN_FRAME() n times >> since that will put the transfer on the following interval. That would >> look like so: >> >> modified 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 >> modified 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)) >> >> /** >> @@ -1271,20 +1271,28 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) >> u64 current_timestamp; >> u64 diff_timestamp; >> u32 elapsed_frames; >> + int retries; >> + int ret; >> >> if (list_empty(&dep->pending_list)) { >> dep->flags |= DWC3_EP_PENDING_REQUEST; >> return -EAGAIN; >> } >> >> - current_timestamp = ktime_get_ns(); >> - diff_timestamp = current_timestamp - dep->frame_timestamp; >> - elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >> + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) { >> + current_timestamp = ktime_get_ns(); >> + diff_timestamp = current_timestamp - dep->frame_timestamp; >> + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >> >> - dep->frame_number += elapsed_frames; >> - dep->frame_number = DWC3_ALIGN_FRAME(dep); >> + dep->frame_number += elapsed_frames; >> + dep->frame_number = DWC3_ALIGN_FRAME(dep, i); >> >> - return __dwc3_gadget_kick_transfer(dep); >> + 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) >> >> > I like the second method. But do we need to keep track of the > frame_timestamp? I don't think it accurately reflects the timestamp of no we don't. I've since removed it from my local tree. > XferNotReady frame number. > As for the number of retries, we should adjust it according to the > service interval. For example, for larger service interval such as 16, > then we don't need to try more than once. To calculate the max number of > retries, we can do this check (where interval is from 1 to 16): > > if (interval >= (16 - (MAX_NUM_RETRIES >> 1)) > num_retries = 1 << (16 - interval); > > Please check my math.. > > 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); + + 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.
On 11/14/2018 12:42 AM, Felipe Balbi wrote: > Hi, > > Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >>> Felipe Balbi <felipe.balbi@linux.intel.com> writes: >>>> In the case of interval of 1ms, it will start on the next interval. >>>>> frame_number + max(4, interval) will start at least 4 uframes in the future. >>>>> >>>>> In any case, what about immediately retry the START_TRANSFER command >>>>> with a new frame_number + (interval*retry) should it fail with >>>>> bus-expiry? You can set the number of retries to maybe 5 times. This >>>>> should remove the need to do time stamping. >>>> That seems like a good idea. Something like below? (on top of $subject) >>>> >>>> modified 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 >>>> modified drivers/usb/dwc3/gadget.c >>>> @@ -1271,20 +1271,29 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) >>>> u64 current_timestamp; >>>> u64 diff_timestamp; >>>> u32 elapsed_frames; >>>> + int retries; >>>> + int delta = 1 >>>> + int ret; >>>> >>>> if (list_empty(&dep->pending_list)) { >>>> dep->flags |= DWC3_EP_PENDING_REQUEST; >>>> return -EAGAIN; >>>> } >>>> >>>> - current_timestamp = ktime_get_ns(); >>>> - diff_timestamp = current_timestamp - dep->frame_timestamp; >>>> - elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >>>> + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) { >>>> + current_timestamp = ktime_get_ns(); >>>> + diff_timestamp = current_timestamp - dep->frame_timestamp; >>>> + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >>>> >>>> - dep->frame_number += elapsed_frames; >>>> - dep->frame_number = DWC3_ALIGN_FRAME(dep); >>>> + dep->frame_number += elapsed_frames + (delta * i); >>> the other possibility is that we can call DWC3_ALIGN_FRAME() n times >>> since that will put the transfer on the following interval. That would >>> look like so: >>> >>> modified 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 >>> modified 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)) >>> >>> /** >>> @@ -1271,20 +1271,28 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) >>> u64 current_timestamp; >>> u64 diff_timestamp; >>> u32 elapsed_frames; >>> + int retries; >>> + int ret; >>> >>> if (list_empty(&dep->pending_list)) { >>> dep->flags |= DWC3_EP_PENDING_REQUEST; >>> return -EAGAIN; >>> } >>> >>> - current_timestamp = ktime_get_ns(); >>> - diff_timestamp = current_timestamp - dep->frame_timestamp; >>> - elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >>> + for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) { >>> + current_timestamp = ktime_get_ns(); >>> + diff_timestamp = current_timestamp - dep->frame_timestamp; >>> + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); >>> >>> - dep->frame_number += elapsed_frames; >>> - dep->frame_number = DWC3_ALIGN_FRAME(dep); >>> + dep->frame_number += elapsed_frames; >>> + dep->frame_number = DWC3_ALIGN_FRAME(dep, i); >>> >>> - return __dwc3_gadget_kick_transfer(dep); >>> + 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) >>> >>> >> I like the second method. But do we need to keep track of the >> frame_timestamp? I don't think it accurately reflects the timestamp of > no we don't. I've since removed it from my local tree. > >> XferNotReady frame number. >> As for the number of retries, we should adjust it according to the >> service interval. For example, for larger service interval such as 16, >> then we don't need to try more than once. To calculate the max number of >> retries, we can do this check (where interval is from 1 to 16): >> >> if (interval >= (16 - (MAX_NUM_RETRIES >> 1)) >> num_retries = 1 << (16 - interval); >> >> Please check my math.. >> >> 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); > + > + 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. Thinh
Hi, Thinh Nguyen <thinh.nguyen@synopsys.com> 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.
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 131028501752..306a2dd75ed5 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -651,6 +651,7 @@ struct dwc3_event_buffer { * @number: endpoint number (1 - 15) * @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK * @resource_index: Resource transfer index + * @frame_timestamp: timestamp of most recent frame number * @frame_number: set to the frame number we want this transfer to start (ISOC) * @interval: the interval on which the ISOC transfer is started * @name: a human readable name e.g. ep1out-bulk @@ -697,7 +698,11 @@ struct dwc3_ep { u8 number; u8 type; u8 resource_index; + + u64 frame_timestamp; u32 frame_number; +#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff + u32 interval; char name[20]; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index d8c7ad0c22e8..00fe01a01977 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1268,12 +1268,22 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc) static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) { + u64 current_timestamp; + u64 diff_timestamp; + u32 elapsed_frames; + if (list_empty(&dep->pending_list)) { dep->flags |= DWC3_EP_PENDING_REQUEST; return -EAGAIN; } + current_timestamp = ktime_get_ns(); + diff_timestamp = current_timestamp - dep->frame_timestamp; + elapsed_frames = DIV_ROUND_UP_ULL(diff_timestamp, 125000); + + dep->frame_number += elapsed_frames; dep->frame_number = DWC3_ALIGN_FRAME(dep); + return __dwc3_gadget_kick_transfer(dep); } @@ -2320,6 +2330,7 @@ static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep, const struct dwc3_event_depevt *event) { dep->frame_number = event->parameters; + dep->frame_timestamp = ktime_get_ns(); } static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
Gadget driver may take an unbounded amount of time to queue requests after XferNotReady. This is important for isochronous endpoints which need to be started for a specific (micro-)frame. Before kicking the transfer, let's check how much time has elapsed since dep->frame_number was updated and make sure we start the request to the next valid interval. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> --- drivers/usb/dwc3/core.h | 5 +++++ drivers/usb/dwc3/gadget.c | 11 +++++++++++ 2 files changed, 16 insertions(+)