If miss isoc event happens, the current code just set
the req status to -EXDEV and giveback the req to the usb
gadget driver, and then stop the active transfer with the
cmd DWC3_DEPCMD_ENDTRANSFER and wait for a XferNotReady
event to restart a transfer again. However, for isoc
ep in transfer, it cause to lost the isoc data of the
req.
This patch moves the miss isoc req to pending_list in
order to restart transfer immediately instead of give
back the req to the usb gadget driver.
Signed-off-by: William Wu <[email protected]>
---
drivers/usb/dwc3/gadget.c | 47 +++++++++++++++++++++++++++++++++++++++
drivers/usb/dwc3/gadget.h | 16 +++++++++++++
2 files changed, 63 insertions(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 5fd067151fbf..ef295746b241 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3454,6 +3454,7 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
const struct dwc3_event_depevt *event,
struct dwc3_request *req, int status)
{
+ struct dwc3 *dwc = dep->dwc;
int request_status;
int ret;
@@ -3475,6 +3476,28 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
req->needs_extra_trb = false;
}
+ /*
+ * If MISS ISOC happens, we need to move the req from started_list
+ * to cancelled_list, then unmap the req and clear the HWO of trb.
+ * Later in the dwc3_gadget_endpoint_trbs_complete(), it will move
+ * the req from the cancelled_list to the pending_list, and restart
+ * the req for isoc transfer.
+ */
+ if (status == -EXDEV && usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
+ req->remaining = 0;
+ req->needs_extra_trb = false;
+ dwc3_gadget_move_cancelled_request(req, DWC3_REQUEST_STATUS_DEQUEUED);
+ if (req->trb) {
+ usb_gadget_unmap_request_by_dev(dwc->sysdev,
+ &req->request,
+ req->direction);
+ req->trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
+ req->trb = NULL;
+ }
+ ret = 0;
+ goto out;
+ }
+
/*
* The event status only reflects the status of the TRB with IOC set.
* For the requests that don't set interrupt on completion, the driver
@@ -3564,6 +3587,7 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
const struct dwc3_event_depevt *event, int status)
{
struct dwc3 *dwc = dep->dwc;
+ struct dwc3_request *req, *tmp;
bool no_started_trb = true;
dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
@@ -3574,6 +3598,29 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
if (!dep->endpoint.desc)
return no_started_trb;
+ /*
+ * If MISS ISOC happens, we need to do the following three steps
+ * to restart the reqs in the cancelled_list and pending_list
+ * in order.
+ * Step1. Move all the reqs from pending_list to the tail of
+ * cancelled_list.
+ * Step2. Move all the reqs from cancelled_list to the tail
+ * of pending_list.
+ * Step3. Stop and restart an isoc transfer.
+ */
+ if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && status == -EXDEV &&
+ !list_empty(&dep->cancelled_list) &&
+ !list_empty(&dep->pending_list)) {
+ list_for_each_entry_safe(req, tmp, &dep->pending_list, list)
+ dwc3_gadget_move_cancelled_request(req, DWC3_REQUEST_STATUS_DEQUEUED);
+ }
+
+ if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && status == -EXDEV &&
+ !list_empty(&dep->cancelled_list)) {
+ list_for_each_entry_safe(req, tmp, &dep->cancelled_list, list)
+ dwc3_gadget_move_queued_request(req);
+ }
+
if (usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
list_empty(&dep->started_list) &&
(list_empty(&dep->pending_list) || status == -EXDEV))
diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
index 55a56cf67d73..242426b67798 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -104,6 +104,22 @@ static inline void dwc3_gadget_move_cancelled_request(struct dwc3_request *req,
list_move_tail(&req->list, &dep->cancelled_list);
}
+/**
+ * dwc3_gadget_move_queued_request - move @req to the pending_list
+ * @req: the request to be moved
+ *
+ * Caller should take care of locking. This function will move @req from its
+ * current list to the endpoint's pending_list.
+ *
+ */
+static inline void dwc3_gadget_move_queued_request(struct dwc3_request *req)
+{
+ struct dwc3_ep *dep = req->dep;
+
+ req->status = DWC3_REQUEST_STATUS_QUEUED;
+ list_move_tail(&req->list, &dep->pending_list);
+}
+
void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
int status);
--
2.17.1
On Thu, Jul 13, 2023, William Wu wrote:
> If miss isoc event happens, the current code just set
> the req status to -EXDEV and giveback the req to the usb
> gadget driver, and then stop the active transfer with the
> cmd DWC3_DEPCMD_ENDTRANSFER and wait for a XferNotReady
> event to restart a transfer again. However, for isoc
> ep in transfer, it cause to lost the isoc data of the
> req.
That's intentional and expected behavior.
>
> This patch moves the miss isoc req to pending_list in
> order to restart transfer immediately instead of give
> back the req to the usb gadget driver.
No. Now you're breaking the usage of isoc endpoint and also re-ordering
the transfer sequence.
>
> Signed-off-by: William Wu <[email protected]>
Regardless of direction, isoc data is time sensitive. If an isoc request
(or part of it) does not go out at a scheduled interval, then it's
dropped.
You should look into why it's being dropped (whether it's due to
software or hardware latency) and see if you can help it.
If your application does not care about the timing, then perhaps it
should use a different endpoint type.
If your application protocol requires the use of isoc endpoint and
somehow also demands no data drop tolerance, then you can workaround
that from the gadget driver. However I doubt this is the case.
BR,
Thinh