musb_hdrc: Cannot stall an endpoint 0 control transfer from a data stage cal lback...

Gadget drivers are supposed to be able to cause EP0 protocol stalls by
issuing an appropriate request from the callback issued when the DATA
stage completes ... not only from the setup() callback or from some
thread that decides how to handle the request.

This fix is based on a patch from Geoffrey Tam <geoffrey@evertz.com>,
and addresses that by updating the endpoint state AFTER the callback
is issued, providing the correct IRQ-acking CSR to the halt() so it
can just mask in the SEND_STALL bit, and ensuring that only the CSR
is still written only once even on this new code path.

Also includes a few small cleanups:  avoid "this" variable name, and
pack device bitfields more efficiently (wasting less space).

Allegedly helps file_storage on Davinci.

diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index dc56425fd1d911d3370b0f125c6d321ab24d5c85..332d39dca181cee35221436693715c26ce49aed6 100644 (file)
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -388,15 +388,15 @@ struct musb {
 
        bool                    is_host;
 
+       int                     a_wait_bcon;    /* VBUS timeout in msecs */
+       unsigned long           idle_timeout;   /* Next timeout in jiffies */
+
        /* active means connected and not suspended */
        unsigned                is_active:1;
 
        unsigned is_multipoint:1;
        unsigned ignore_disconnect:1;   /* during bus resets */
 
-       int                     a_wait_bcon;    /* VBUS timeout in msecs */
-       unsigned long           idle_timeout;   /* Next timeout in jiffies */
-
 #ifdef C_MP_TX
        unsigned bulk_split:1;
 #define        can_bulk_split(musb,type) \
@@ -431,10 +431,10 @@ struct musb {
        unsigned                test_mode:1;
        unsigned                softconnect:1;
 
-       enum musb_g_ep0_state   ep0_state;
        u8                      address;
        u8                      test_mode_nr;
        u16                     ackpend;                /* ep0 */
+       enum musb_g_ep0_state   ep0_state;
        struct usb_gadget       g;                      /* the gadget */
        struct usb_gadget_driver *gadget_driver;        /* its driver */
 #endif

diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c
index 51cb737fc68986d822147ca788be03c6f1c398ae..0319ced121eb86c3f4a4b7a0c0613507b13c4d1e 100644 (file)
--- a/drivers/usb/musb/musb_gadget_ep0.c
+++ b/drivers/usb/musb/musb_gadget_ep0.c
@@ -196,8 +196,8 @@ service_in_request(struct musb *musb, const struct usb_ctrlrequest *ctrlrequest)
  */
 static void musb_g_ep0_giveback(struct musb *musb, struct usb_request *req)
 {
-       musb->ep0_state = MUSB_EP0_STAGE_SETUP;
        musb_g_giveback(&musb->endpoints[0].ep_in, req, 0);
+       musb->ep0_state = MUSB_EP0_STAGE_SETUP;
 }
 
 /*
@@ -433,13 +433,13 @@ stall:
 /* we have an ep0out data packet
  * Context:  caller holds controller lock
  */
-static void ep0_rxstate(struct musb *this)
+static void ep0_rxstate(struct musb *musb)
 {
-       void __iomem            *regs = this->control_ep->regs;
+       void __iomem            *regs = musb->control_ep->regs;
        struct usb_request      *req;
        u16                     tmp;
 
-       req = next_ep0_request(this);
+       req = next_ep0_request(musb);
 
        /* read packet and ack; or stall because of gadget driver bug:
         * should have provided the rx buffer before setup() returned.
@@ -454,25 +454,29 @@ static void ep0_rxstate(struct musb *this)
                        req->status = -EOVERFLOW;
                        tmp = len;
                }
-               musb_read_fifo(&this->endpoints[0], tmp, buf);
+               musb_read_fifo(&musb->endpoints[0], tmp, buf);
                req->actual += tmp;
                tmp = MUSB_CSR0_P_SVDRXPKTRDY;
                if (tmp < 64 || req->actual == req->length) {
-                       this->ep0_state = MUSB_EP0_STAGE_STATUSIN;
+                       musb->ep0_state = MUSB_EP0_STAGE_STATUSIN;
                        tmp |= MUSB_CSR0_P_DATAEND;
                } else
                        req = NULL;
        } else
                tmp = MUSB_CSR0_P_SVDRXPKTRDY | MUSB_CSR0_P_SENDSTALL;
-       musb_writew(regs, MUSB_CSR0, tmp);
 
 
-       /* NOTE:  we "should" hold off reporting DATAEND and going to
-        * STATUSIN until after the completion handler decides whether
-        * to issue a stall instead, since this hardware can do that.
+       /* Completion handler may choose to stall, e.g. because the
+        * message just received holds invalid data.
         */
-       if (req)
-               musb_g_ep0_giveback(this, req);
+       if (req) {
+               musb->ackpend = tmp;
+               musb_g_ep0_giveback(musb, req);
+               if (!musb->ackpend)
+                       return;
+               musb->ackpend = 0;
+       }
+       musb_writew(regs, MUSB_CSR0, tmp);
 }
 
 /*
@@ -510,16 +514,21 @@ static void ep0_txstate(struct musb *musb)
        } else
                request = NULL;
 
-       /* send it out, triggering a "txpktrdy cleared" irq */
-       musb_writew(regs, MUSB_CSR0, csr);
-
        /* report completions as soon as the fifo's loaded; there's no
         * win in waiting till this last packet gets acked.  (other than
         * very precise fault reporting, needed by USB TMC; possible with
         * this hardware, but not usable from portable gadget drivers.)
         */
-       if (request)
+       if (request) {
+               musb->ackpend = csr;
                musb_g_ep0_giveback(musb, request);
+               if (!musb->ackpend)
+                       return;
+               musb->ackpend = 0;
+       }
+
+       /* send it out, triggering a "txpktrdy cleared" irq */
+       musb_writew(regs, MUSB_CSR0, csr);
 }
 
 /*
@@ -917,6 +926,7 @@ static int musb_g_ep0_halt(struct usb_ep *e, int value)
        musb = ep->musb;
        base = musb->mregs;
        regs = musb->control_ep->regs;
+       status = 0;
 
        spin_lock_irqsave(&musb->lock, flags);
 
@@ -925,17 +935,30 @@ static int musb_g_ep0_halt(struct usb_ep *e, int value)
                goto cleanup;
        }
 
+       musb_ep_select(base, 0);
+       csr = musb->ackpend;
+
        switch (musb->ep0_state) {
+
+       /* Stalls are usually issued after parsing SETUP packet, either
+        * directly in irq context from setup() or else later.
+        */
        case MUSB_EP0_STAGE_TX:         /* control-IN data */
        case MUSB_EP0_STAGE_ACKWAIT:    /* STALL for zero-length data */
        case MUSB_EP0_STAGE_RX:         /* control-OUT data */
-               status = 0;
-
-               musb_ep_select(base, 0);
                csr = musb_readw(regs, MUSB_CSR0);
+               /* FALLTHROUGH */
+
+       /* It's also OK to issue stalls during callbacks when a non-empty
+        * DATA stage buffer has been read (or even written).
+        */
+       case MUSB_EP0_STAGE_STATUSIN:   /* control-OUT status */
+       case MUSB_EP0_STAGE_STATUSOUT:  /* control-IN status */
+
                csr |= MUSB_CSR0_P_SENDSTALL;
                musb_writew(regs, MUSB_CSR0, csr);
                musb->ep0_state = MUSB_EP0_STAGE_SETUP;
+               musb->ackpend = 0;
                break;
        default:
                DBG(1, "ep0 can't halt in state %d\n", musb->ep0_state);