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

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.

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

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);