[PATCH v2 0/4] usb: dwc3: gadget: Improve isoc starting mechanism

[PATCH v2 0/4] usb: dwc3: gadget: Improve isoc starting mechanism

[Thinh Nguyen]

Currently we use the "retry" method to issue START_TRANSFER command multiple
times, each with a next interval. There's been report that 5 number of retries
may not be enough. See https://lkml.org/lkml/2019/11/11/535

We could increase the number of retries. However, that also may not be enough
depending on different system latency. It is not often that the latency is
higher than 5 isoc intervals. If it goes beyond 5 intervals, let's restart the
whole process again.


Changes in v2:
 - Include patch "usb: dwc3: gadget: WARN on no-resource status" to this series

Thinh Nguyen (4):
  usb: dwc3: gadget: Properly handle failed kick_transfer
  usb: dwc3: gadget: Store resource index of start cmd
  usb: dwc3: gadget: Issue END_TRANSFER to retry isoc transfer
  usb: dwc3: gadget: WARN on no-resource status

 drivers/usb/dwc3/gadget.c | 71 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 59 insertions(+), 12 deletions(-)

-- 
2.11.0

[PATCH v2 1/4] usb: dwc3: gadget: Properly handle failed kick_transfer

[Thinh Nguyen]

If dwc3 fails to issue START_TRANSFER/UPDATE_TRANSFER command, then we
should properly end an active transfer and give back all the started
requests. However if it's for an isoc endpoint, the failure maybe due to
bus-expiry status. In this case, don't give back the requests and wait
for the next retry.

Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 6d2b3de455cc..b02832ad9e72 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1220,6 +1220,8 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
 	}
 }
 
+static void dwc3_gadget_ep_cleanup_cancelled_requests(struct dwc3_ep *dep);
+
 static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)
 {
 	struct dwc3_gadget_ep_cmd_params params;
@@ -1259,14 +1261,20 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)
 
 	ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
 	if (ret < 0) {
-		/*
-		 * FIXME we need to iterate over the list of requests
-		 * here and stop, unmap, free and del each of the linked
-		 * requests instead of what we do now.
-		 */
-		if (req->trb)
-			memset(req->trb, 0, sizeof(struct dwc3_trb));
-		dwc3_gadget_del_and_unmap_request(dep, req, ret);
+		struct dwc3_request *tmp;
+
+		if (ret == -EAGAIN)
+			return ret;
+
+		dwc3_stop_active_transfer(dep, true, true);
+
+		list_for_each_entry_safe(req, tmp, &dep->started_list, list)
+			dwc3_gadget_move_cancelled_request(req);
+
+		/* If ep isn't started, then there's no end transfer pending */
+		if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING))
+			dwc3_gadget_ep_cleanup_cancelled_requests(dep);
+
 		return ret;
 	}
 
-- 
2.11.0

[PATCH v2 2/4] usb: dwc3: gadget: Store resource index of start cmd

[Thinh Nguyen]

As long as the START_TRANSFER command completes, it provides the
resource index of the endpoint. Use this when we need to issue
END_TRANSFER command to an isoc endpoint to retry with a new
XferNotReady event.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index b02832ad9e72..7989fe663300 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -387,9 +387,12 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd,
 
 	trace_dwc3_gadget_ep_cmd(dep, cmd, params, cmd_status);
 
-	if (ret == 0 && DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) {
-		dep->flags |= DWC3_EP_TRANSFER_STARTED;
-		dwc3_gadget_ep_get_transfer_index(dep);
+	if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) {
+		if (ret == 0)
+			dep->flags |= DWC3_EP_TRANSFER_STARTED;
+
+		if (ret != -ETIMEDOUT)
+			dwc3_gadget_ep_get_transfer_index(dep);
 	}
 
 	if (saved_config) {
-- 
2.11.0

[PATCH v2 3/4] usb: dwc3: gadget: Issue END_TRANSFER to retry isoc transfer

[Thinh Nguyen]

After a number of unsuccessful start isoc attempts due to bus-expiry
status, issue END_TRANSFER command and retry on the next XferNotReady
event.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 7989fe663300..ee87b7b383f6 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1413,7 +1413,8 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
 	int ret;
 	int i;
 
-	if (list_empty(&dep->pending_list)) {
+	if (list_empty(&dep->pending_list) &&
+	    list_empty(&dep->started_list)) {
 		dep->flags |= DWC3_EP_PENDING_REQUEST;
 		return -EAGAIN;
 	}
@@ -1436,6 +1437,27 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
 			break;
 	}
 
+	/*
+	 * After a number of unsuccessful start attempts due to bus-expiry
+	 * status, issue END_TRANSFER command and retry on the next XferNotReady
+	 * event.
+	 */
+	if (ret == -EAGAIN) {
+		struct dwc3_gadget_ep_cmd_params params;
+		u32 cmd;
+
+		cmd = DWC3_DEPCMD_ENDTRANSFER |
+			DWC3_DEPCMD_CMDIOC |
+			DWC3_DEPCMD_PARAM(dep->resource_index);
+
+		dep->resource_index = 0;
+		memset(&params, 0, sizeof(params));
+
+		ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
+		if (!ret)
+			dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
+	}
+
 	return ret;
 }
 
@@ -2645,6 +2667,18 @@ static void dwc3_gadget_endpoint_transfer_not_ready(struct dwc3_ep *dep,
 		const struct dwc3_event_depevt *event)
 {
 	dwc3_gadget_endpoint_frame_from_event(dep, event);
+
+	/*
+	 * The XferNotReady event is generated only once before the endpoint
+	 * starts. It will be generated again when END_TRANSFER command is
+	 * issued. For some controller versions, the XferNotReady event may be
+	 * generated while the END_TRANSFER command is still in process. Ignore
+	 * it and wait for the next XferNotReady event after the command is
+	 * completed.
+	 */
+	if (dep->flags & DWC3_EP_END_TRANSFER_PENDING)
+		return;
+
 	(void) __dwc3_gadget_start_isoc(dep);
 }
 
-- 
2.11.0

[PATCH v2 4/4] usb: dwc3: gadget: WARN on no-resource status

[Thinh Nguyen]

If the driver issued START_TRANSFER and received a no-resource status,
then generally there are a few reasons for this:

1) The driver did not allocate resource for the endpoint during
power-on-reset initialization.

2) The transfer resource was reset. At this moment, we don't do this in
the driver, but it occurs when the driver issues START_CONFIG cmd to ep0
with resource index=2.

3) The driver issues the START_TRANSFER command to an already started
endpoint. Usually, this is because the END_TRANSFER command hasn't
completed yet.

Print out a warning to help debug this issue in the driver.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index ee87b7b383f6..1a4fc03742aa 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -356,6 +356,8 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd,
 				ret = 0;
 				break;
 			case DEPEVT_TRANSFER_NO_RESOURCE:
+				dev_WARN(dwc->dev, "No resource for %s\n",
+					 dep->name);
 				ret = -EINVAL;
 				break;
 			case DEPEVT_TRANSFER_BUS_EXPIRY:
-- 
2.11.0

Re: [PATCH v2 0/4] usb: dwc3: gadget: Improve isoc starting mechanism

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:

> Currently we use the "retry" method to issue START_TRANSFER command multiple
> times, each with a next interval. There's been report that 5 number of retries
> may not be enough. See https://lkml.org/lkml/2019/11/11/535
>
> We could increase the number of retries. However, that also may not be enough
> depending on different system latency. It is not often that the latency is
> higher than 5 isoc intervals. If it goes beyond 5 intervals, let's restart the
> whole process again.
>
>
> Changes in v2:
>  - Include patch "usb: dwc3: gadget: WARN on no-resource status" to this series
>
> Thinh Nguyen (4):
>   usb: dwc3: gadget: Properly handle failed kick_transfer
>   usb: dwc3: gadget: Store resource index of start cmd
>   usb: dwc3: gadget: Issue END_TRANSFER to retry isoc transfer
>   usb: dwc3: gadget: WARN on no-resource status

all in testing/next now

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]