We uncovered a limitation of this core WRT to the
Link Layer Compliance Suite's TD7.06.
On that test, host will start a GetDescriptor(DEVICE)
standard request, but it will do so only on the
SETUP phase, meaning there will *NOT* be any DATA or
STATUS phases.
The idea of the test is to verify robustness of the
IP WRT framing errors, so the test will send a
sequence of different SETUP_DPs each with a different
framing error and the Suite expects us to be able to
receive all SETUP_DPs with no timeouts.
This core, has the ability to tell us which phase the
host is expecting before we start it. Whenever we
receive a TP or DP when no transfers are cached on
the internal IP's caches, the IP will generate a
XferNotReady event with status informing us (in case
of physical ep0/ep1) if it's related to DATA or STATUS
phases - SETUP phase is expected to be prestarted.
Because we're always waiting for XferNotReady
events for DATA and STATUS phases, we will never
be able to know that the Host wants to start another
SETUP phase instead, which will render us "not
compliant" with TD7.06.
In order to "fix" the problem we must not rely
on XferNotReady events for the DATA phase and try
to always pre-start DATA transfers on physical
endpoints 0 and 1. If host goes back to SETUP phase
from DATA phase we will receive a XferComplete for
that phase with TRB's status set to SETUP_PENDING,
which is only useful for printing a debugging log as
the core expects us to still go through to the STATUS
phase, initiate a CONTROL_STATUS TRB just so it
completes right away and, only then, we go back to
the pending SETUP phase.
SNPS has decided to modify the programming model of
the core so that on-demand DATA phases will not be
supported anymore. Note that this limitation does not
affect 2-stage transfers, meaning that if TD7.06 would
start a 2-stage transfer instead of a 3-stage transfer,
we would receive a "fake" XferNotReady(STATUS) which
would complete right after being initiated with
SETUP_PENDING status.
Other endpoints are also not affected, so we can still
use on-demand transfers on Bulk/Isoc/Interrupt endpoints.
Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
drivers/usb/dwc3/ep0.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 39abc58..69d5741 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -174,6 +174,49 @@ static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep,
return 0;
}
+ /*
+ * Unfortunately we have uncovered a limitation wrt the Data Phase.
+ *
+ * Section 9.4 says we can wait for the XferNotReady(DATA) event to
+ * come before issueing Start Transfer command, but if we do, we will
+ * miss situations where the host starts another SETUP phase instead of
+ * the DATA phase. Such cases happen at least on TD.7.6 of the Link
+ * Layer Compliance Suite.
+ *
+ * The problem surfaces due to the fact that in case of back-to-back
+ * SETUP packets there will be no XferNotReady(DATA) generated and we
+ * will be stuck waiting for XferNotReady(DATA) forever.
+ *
+ * By looking at tables 9-13 and 9-14 of the Databook, we can see that
+ * it tells us to start Data Phase right away. It also mentions that if
+ * we receive a SETUP phase instead of the DATA phase, core will issue
+ * XferComplete for the DATA phase, before actually initiating it in
+ * the wire, with the TRB's status set to "SETUP_PENDING". Such status
+ * can only be used to print some debugging logs, as the core expects
+ * us to go through to the STATUS phase and start a CONTROL_STATUS TRB,
+ * just so it completes right away, without transferring anything and,
+ * only then, we can go back to the SETUP phase.
+ *
+ * Because of this scenario, SNPS decided to change the programming
+ * model of control transfers and support on-demand transfers only for
+ * the STATUS phase. To fix the issue we have now, we will always wait
+ * for gadget driver to queue the DATA phase's struct usb_request, then
+ * start it right away.
+ *
+ * If we're actually in a 2-stage transfer, we will wait for
+ * XferNotReady(STATUS).
+ */
+ if (dwc->three_stage_setup) {
+ unsigned direction;
+
+ direction = dwc->ep0_expect_in;
+ dwc->ep0state = EP0_DATA_PHASE;
+
+ __dwc3_ep0_do_control_data(dwc, dwc->eps[direction], req);
+
+ dep->flags &= ~DWC3_EP0_DIR_IN;
+ }
+
return 0;
}
@@ -707,6 +750,7 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
struct dwc3_trb *trb;
struct dwc3_ep *ep0;
u32 transferred;
+ u32 status;
u32 length;
u8 epnum;
@@ -719,6 +763,17 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
ur = &r->request;
trb = dwc->ep0_trb;
+
+ status = DWC3_TRB_SIZE_TRBSTS(trb->size);
+ if (status == DWC3_TRBSTS_SETUP_PENDING) {
+ dev_dbg(dwc->dev, "Setup Pending received\n");
+
+ if (r)
+ dwc3_gadget_giveback(ep0, r, -ECONNRESET);
+
+ return;
+ }
+
length = trb->size & DWC3_TRB_SIZE_MASK;
if (dwc->ep0_bounced) {
@@ -755,8 +810,11 @@ static void dwc3_ep0_complete_status(struct dwc3 *dwc,
{
struct dwc3_request *r;
struct dwc3_ep *dep;
+ struct dwc3_trb *trb;
+ u32 status;
dep = dwc->eps[0];
+ trb = dwc->ep0_trb;
if (!list_empty(&dep->request_list)) {
r = next_request(&dep->request_list);
@@ -776,6 +834,10 @@ static void dwc3_ep0_complete_status(struct dwc3 *dwc,
}
}
+ status = DWC3_TRB_SIZE_TRBSTS(trb->size);
+ if (status == DWC3_TRBSTS_SETUP_PENDING)
+ dev_dbg(dwc->dev, "Setup Pending received\n");
+
dwc->ep0state = EP0_SETUP_PHASE;
dwc3_ep0_out_start(dwc);
}
--
1.7.11
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
