Hi,
On Tue, Jun 05, 2012 at 07:19:08PM +0530, Pratyush Anand wrote:
> It might happen that dwc3_remove_requests is called when req_queued list
> is not empty but end transfer command was issued. It will occur if
> dwc3_remove_requests is called again before interrupt for end transfer
> command is received.
>
> This situation can be reproduced when cable is physically disconnected after
> some isoc in transfer (testusb case 16).
>
> Signed-off-by: Pratyush Anand <pratyush.anand@xxxxxx>
> ---
> drivers/usb/dwc3/core.h | 2 ++
> drivers/usb/dwc3/gadget.c | 5 ++++-
> 2 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 1210dae..5c6cd18 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -357,6 +357,7 @@ struct dwc3_event_buffer {
> * @name: a human readable name e.g. ep1out-bulk
> * @direction: true for TX, false for RX
> * @stream_capable: true when streams are enabled
> + * @end_xfer_issued: true when EndTransfer command has been issued
> */
> struct dwc3_ep {
> struct usb_ep endpoint;
> @@ -394,6 +395,7 @@ struct dwc3_ep {
>
> unsigned direction:1;
> unsigned stream_capable:1;
> + unsigned end_xfer_issued:1;
> };
>
> enum dwc3_phy {
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 322b62e..fe2f4f9 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -530,7 +530,7 @@ static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep)
> {
> struct dwc3_request *req;
>
> - if (!list_empty(&dep->req_queued))
> + if (!list_empty(&dep->req_queued) && !dep->end_xfer_issued)
I would rather not wait for ENDXFER IRQ, I have a patch for that, can
you test it ? (attached).
--
balbi
From c6bc3b1ee363f08cddaf92e959ef956a632abd64 Mon Sep 17 00:00:00 2001 From: Felipe Balbi <balbi@xxxxxx> Date: Fri, 17 Feb 2012 11:58:14 +0200 Subject: [PATCH 1/2] usb: dwc3: gadget: drop useless code We never set CMDIOC bit for Start Transfer command, so that code will never be used. Signed-off-by: Felipe Balbi <balbi@xxxxxx> --- drivers/usb/dwc3/gadget.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 854f3a3..056ace8 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1765,9 +1765,6 @@ static void dwc3_ep_cmd_compl(struct dwc3_ep *dep, case DWC3_DEPCMD_ENDTRANSFER: dwc3_process_ep_cmd_complete(dep, event); break; - case DWC3_DEPCMD_STARTTRANSFER: - dep->res_trans_idx = param & 0x7f; - break; default: printk(KERN_ERR "%s() unknown /unexpected type: %d\n", __func__, cmd_type); -- 1.7.10.3
From b5a7535502c08873ce1c732ee70ff56ab5a19947 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <balbi@xxxxxx>
Date: Fri, 17 Feb 2012 12:10:04 +0200
Subject: [PATCH 2/2] usb: dwc3: gadget: don't wait for ep cmd IRQ
That IRQ is causing way too much trouble. We have
a different handling which was agreed with IP
provider and has been tested with FPGA and OMAP5.
Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
drivers/usb/dwc3/gadget.c | 66 +++++++++++++++++++--------------------------
1 file changed, 27 insertions(+), 39 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 056ace8..0f19978 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -555,9 +555,34 @@ static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep)
{
struct dwc3_request *req;
- if (!list_empty(&dep->req_queued))
+ if (!list_empty(&dep->req_queued)) {
dwc3_stop_active_transfer(dwc, dep->number);
+ /*
+ * NOTICE: We are violating what the Databook says about the
+ * EndTransfer command. Ideally we would _always_ wait for the
+ * EndTransfer Command Completion IRQ, but that's causing too
+ * much trouble synchronizing between us and gadget driver.
+ *
+ * We have discussed this with the IP Provider and it was
+ * suggested to giveback all requests here, but give HW some
+ * extra time to synchronize with the interconnect. We're using
+ * an arbitraty 100us delay for that.
+ *
+ * Note also that a similar handling was tested by Synopsys
+ * (thanks a lot Paul) and nothing bad has come out of it.
+ * In short, what we're doing is:
+ *
+ * - Issue EndTransfer WITH CMDIOC bit set
+ * - Wait 100us
+ * - giveback all requests to gadget driver
+ */
+ udelay(100);
+
+ req = next_request(&dep->req_queued);
+ dwc3_gadget_giveback(dep, req, -ESHUTDOWN);
+ }
+
while (!list_empty(&dep->request_list)) {
req = next_request(&dep->request_list);
@@ -1735,43 +1760,6 @@ static void dwc3_endpoint_transfer_complete(struct dwc3 *dwc,
}
}
-static void dwc3_process_ep_cmd_complete(struct dwc3_ep *dep,
- const struct dwc3_event_depevt *event)
-{
- struct dwc3 *dwc = dep->dwc;
- struct dwc3_event_depevt mod_ev = *event;
-
- /*
- * We were asked to remove one request. It is possible that this
- * request and a few others were started together and have the same
- * transfer index. Since we stopped the complete endpoint we don't
- * know how many requests were already completed (and not yet)
- * reported and how could be done (later). We purge them all until
- * the end of the list.
- */
- mod_ev.status = DEPEVT_STATUS_LST;
- dwc3_cleanup_done_reqs(dwc, dep, &mod_ev, -ESHUTDOWN);
- dep->flags &= ~DWC3_EP_BUSY;
- /* pending requests are ignored and are queued on XferNotReady */
-}
-
-static void dwc3_ep_cmd_compl(struct dwc3_ep *dep,
- const struct dwc3_event_depevt *event)
-{
- u32 param = event->parameters;
- u32 cmd_type = (param >> 8) & ((1 << 5) - 1);
-
- switch (cmd_type) {
- case DWC3_DEPCMD_ENDTRANSFER:
- dwc3_process_ep_cmd_complete(dep, event);
- break;
- default:
- printk(KERN_ERR "%s() unknown /unexpected type: %d\n",
- __func__, cmd_type);
- break;
- };
-}
-
static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
const struct dwc3_event_depevt *event)
{
@@ -1853,7 +1841,7 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
dev_dbg(dwc->dev, "%s FIFO Overrun\n", dep->name);
break;
case DWC3_DEPEVT_EPCMDCMPLT:
- dwc3_ep_cmd_compl(dep, event);
+ dev_vdbg(dwc->dev, "Endpoint Command Complete\n");
break;
}
}
--
1.7.10.3
Attachment:
signature.asc
Description: Digital signature
