usb: dwc3: gadget: check if dep->frame_number is still valid
PATCH 4/4 usb: dwc3: gadget: check if dep->frame_number is still valid
Hi,
Quoted 55 lines · Thinh Nguyen
Quoted 51 lines
Quoted 41 lines
Quoted 28 lines
drivers/usb/dwc3/gadget.c +10
-0@@ -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); }drivers/usb/dwc3/gadget.c static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep, · line 2336
Quoted 7 lines
drivers/usb/dwc3/gadget.c +1
-0@@ -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_timestampmay not correspond to theframe_numberfrom 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.Quoted 3 lines
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 howDWC3_ALIGN_FRAME()is implemented.Quoted 3 lines
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.
Quoted 7 lines
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_TRANSFERcommand with a newframe_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)