Hi,

On Fri, Sep 26, 2014 at 11:15:03AM -0500, Felipe Balbi wrote:
> On Fri, Sep 26, 2014 at 12:06:00PM -0400, Alan Stern wrote:
> > > Any hints will be appreciated.
> > 
> > We could implement READ CAPACITY(16) if necessary.  But since we don't 
> > implement READ(16) or WRITE(16), it seems rather pointless.
> > 
> > Are you certain you are interpreting the USBCV output correctly?
> 
> heh, yes I am. That READ CAPACITY(16) is just a note, not the error
> itself. It seems like one of the CSWs (it sends a lot of commands on
> this test) was sent with zero bytes. I'll go fire up the sniffer and
> debug this further.

alright, found it. It was caused by another bug on my set_stall fix.
Now, hopefully, it's all fixed. Passing USB20CV chapter 9 and MSC tests,
also working against Linux and Mac OS X.

Here's the final patch, I'll send it officialy once the merge window has
closed:

8<--------------------------------------------------------------------

From 4c6282e877ebef6db6bde6f661acf4ea37b3920a Mon Sep 17 00:00:00 2001
From: Felipe Balbi <ba...@ti.com>
Date: Wed, 24 Sep 2014 14:19:52 -0500
Subject: [PATCH] usb: dwc3: gadget: fix set_halt() bug with pending transfers

According to our Gadget Framework API documentation,
->set_halt() *must* return -EAGAIN if we have pending
transfers (on either direction) or FIFO isn't empty (on
TX endpoints).

Fix this bug so that the mass storage gadget can be used
without stall=0 parameter.

This patch should be backported to all kernels since v3.2.

Cc: <sta...@vger.kernel.org> # v3.2+
Suggested-by: Alan Stern <st...@rowland.harvard.edu>
Signed-off-by: Felipe Balbi <ba...@ti.com>
---
 drivers/usb/dwc3/ep0.c    |  4 ++--
 drivers/usb/dwc3/gadget.c | 16 ++++++++++++----
 drivers/usb/dwc3/gadget.h |  2 +-
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 36f6158..ae6b575 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -256,7 +256,7 @@ static void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)
 
        /* stall is always issued on EP0 */
        dep = dwc->eps[0];
-       __dwc3_gadget_ep_set_halt(dep, 1);
+       __dwc3_gadget_ep_set_halt(dep, 1, false);
        dep->flags = DWC3_EP_ENABLED;
        dwc->delayed_status = false;
 
@@ -480,7 +480,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
                                return -EINVAL;
                        if (set == 0 && (dep->flags & DWC3_EP_WEDGE))
                                break;
-                       ret = __dwc3_gadget_ep_set_halt(dep, set);
+                       ret = __dwc3_gadget_ep_set_halt(dep, set, true);
                        if (ret)
                                return -EINVAL;
                        break;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index de53361..68497b3 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -581,7 +581,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
 
        /* make sure HW endpoint isn't stalled */
        if (dep->flags & DWC3_EP_STALL)
-               __dwc3_gadget_ep_set_halt(dep, 0);
+               __dwc3_gadget_ep_set_halt(dep, 0, false);
 
        reg = dwc3_readl(dwc->regs, DWC3_DALEPENA);
        reg &= ~DWC3_DALEPENA_EP(dep->number);
@@ -1202,7 +1202,7 @@ out0:
        return ret;
 }
 
-int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value)
+int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol)
 {
        struct dwc3_gadget_ep_cmd_params        params;
        struct dwc3                             *dwc = dep->dwc;
@@ -1216,6 +1216,14 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int 
value)
        memset(&params, 0x00, sizeof(params));
 
        if (value) {
+               if (!protocol && ((dep->direction && dep->flags & DWC3_EP_BUSY) 
||
+                               (!list_empty(&dep->req_queued) ||
+                                !list_empty(&dep->request_list)))) {
+                       dev_dbg(dwc->dev, "%s: pending request, cannot halt\n",
+                                       dep->name);
+                       return -EAGAIN;
+               }
+
                ret = dwc3_send_gadget_ep_cmd(dwc, dep->number,
                        DWC3_DEPCMD_SETSTALL, &params);
                if (ret)
@@ -1246,7 +1254,7 @@ static int dwc3_gadget_ep_set_halt(struct usb_ep *ep, int 
value)
        int                             ret;
 
        spin_lock_irqsave(&dwc->lock, flags);
-       ret = __dwc3_gadget_ep_set_halt(dep, value);
+       ret = __dwc3_gadget_ep_set_halt(dep, value, false);
        spin_unlock_irqrestore(&dwc->lock, flags);
 
        return ret;
@@ -1265,7 +1273,7 @@ static int dwc3_gadget_ep_set_wedge(struct usb_ep *ep)
        if (dep->number == 0 || dep->number == 1)
                ret = __dwc3_gadget_ep0_set_halt(ep, 1);
        else
-               ret = __dwc3_gadget_ep_set_halt(dep, 1);
+               ret = __dwc3_gadget_ep_set_halt(dep, 1, false);
        spin_unlock_irqrestore(&dwc->lock, flags);
 
        return ret;
diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
index f889008..18ae3ea 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -86,7 +86,7 @@ int __dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value);
 int dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value);
 int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct usb_request *request,
                gfp_t gfp_flags);
-int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value);
+int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol);
 
 /**
  * dwc3_gadget_ep_get_transfer_index - Gets transfer index from HW
-- 
2.1.0.GIT


-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to