USB: pxa27x_udc: single-thread setup requests

Since the PXA 27x UDC automatically ACK's some control
packets such as SET_INTERFACE, the gadgets may not get a
chance to process the request before another control packet
is received. The Linux gadgets do not expect to receive
setup callbacks out of order. The file storage gadget only
saves the "highest" priority request.

The PXA27x UDC driver must make sure it only sends one up at
a time, allowing the gadget to make changes before
continuing. In theory, the host would be NACK'd while the
gadget processes the change but the UDC has already ACK'd
the request. If another request is sent by the host that is
not automatically ACK'd by the UDC, then the throttling
happens properly to regain sync.

The observed case was the file_storage gadget timing out on
a BulkReset request because the SET_INTERFACE was being
processed by the gadget. Since SET_INTERFACE is higher
priority than BulkReset, the BulkReset was dropped.  This
was exacerbated by turning on the debug which delayed the
fsg signal processing thread.

This also fixes the "should never get in
WAIT_ACK_SET_CONF_INTERF state here!!!" warning.

Reported-by: Vernon Sauder <vernoninhand@gmail.com>
Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
Acked-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
index 51790b0..1937d8c 100644

diff --git a/drivers/usb/gadget/pxa27x_udc.c b/drivers/usb/gadget/pxa27x_udc.c
index 51790b06b8cab6840cce9a7d91b576c49048edc4..1937d8c7b433201dc0d31474f99014fc25ce2f04 100644 (file)
--- a/drivers/usb/gadget/pxa27x_udc.c
+++ b/drivers/usb/gadget/pxa27x_udc.c
@@ -472,6 +472,23 @@ static inline void udc_clear_mask_UDCCR(struct pxa_udc *udc, int mask)
                        (udccr & UDCCR_MASK_BITS) & ~(mask & UDCCR_MASK_BITS));
 }
 
+/**
+ * ep_write_UDCCSR - set bits in UDCCSR
+ * @udc: udc device
+ * @mask: bits to set in UDCCR
+ *
+ * Sets bits in UDCCSR (UDCCSR0 and UDCCSR*).
+ *
+ * A specific case is applied to ep0 : the ACM bit is always set to 1, for
+ * SET_INTERFACE and SET_CONFIGURATION.
+ */
+static inline void ep_write_UDCCSR(struct pxa_ep *ep, int mask)
+{
+       if (is_ep0(ep))
+               mask |= UDCCSR0_ACM;
+       udc_ep_writel(ep, UDCCSR, mask);
+}
+
 /**
  * ep_count_bytes_remain - get how many bytes in udc endpoint
  * @ep: udc endpoint
@@ -860,7 +877,7 @@ static int read_packet(struct pxa_ep *ep, struct pxa27x_request *req)
                *buf++ = udc_ep_readl(ep, UDCDR);
        req->req.actual += count;
 
-       udc_ep_writel(ep, UDCCSR, UDCCSR_PC);
+       ep_write_UDCCSR(ep, UDCCSR_PC);
 
        return count;
 }
@@ -968,12 +985,12 @@ static int write_fifo(struct pxa_ep *ep, struct pxa27x_request *req)
                if (udccsr & UDCCSR_PC) {
                        ep_vdbg(ep, "Clearing Transmit Complete, udccsr=%x\n",
                                udccsr);
-                       udc_ep_writel(ep, UDCCSR, UDCCSR_PC);
+                       ep_write_UDCCSR(ep, UDCCSR_PC);
                }
                if (udccsr & UDCCSR_TRN) {
                        ep_vdbg(ep, "Clearing Underrun on, udccsr=%x\n",
                                udccsr);
-                       udc_ep_writel(ep, UDCCSR, UDCCSR_TRN);
+                       ep_write_UDCCSR(ep, UDCCSR_TRN);
                }
 
                count = write_packet(ep, req, max);
@@ -995,7 +1012,7 @@ static int write_fifo(struct pxa_ep *ep, struct pxa27x_request *req)
                }
 
                if (is_short)
-                       udc_ep_writel(ep, UDCCSR, UDCCSR_SP);
+                       ep_write_UDCCSR(ep, UDCCSR_SP);
 
                /* requests complete when all IN data is in the FIFO */
                if (is_last) {
@@ -1028,7 +1045,7 @@ static int read_ep0_fifo(struct pxa_ep *ep, struct pxa27x_request *req)
 
        while (epout_has_pkt(ep)) {
                count = read_packet(ep, req);
-               udc_ep_writel(ep, UDCCSR, UDCCSR0_OPC);
+               ep_write_UDCCSR(ep, UDCCSR0_OPC);
                inc_ep_stats_bytes(ep, count, !USB_DIR_IN);
 
                is_short = (count < ep->fifo_size);
@@ -1073,7 +1090,7 @@ static int write_ep0_fifo(struct pxa_ep *ep, struct pxa27x_request *req)
 
        /* Sends either a short packet or a 0 length packet */
        if (unlikely(is_short))
-               udc_ep_writel(ep, UDCCSR, UDCCSR0_IPR);
+               ep_write_UDCCSR(ep, UDCCSR0_IPR);
 
        ep_dbg(ep, "in %d bytes%s%s, %d left, req=%p, udccsr0=0x%03x\n",
                count, is_short ? "/S" : "", is_last ? "/L" : "",
@@ -1276,7 +1293,7 @@ static int pxa_ep_set_halt(struct usb_ep *_ep, int value)
 
        /* FST, FEF bits are the same for control and non control endpoints */
        rc = 0;
-       udc_ep_writel(ep, UDCCSR, UDCCSR_FST | UDCCSR_FEF);
+       ep_write_UDCCSR(ep, UDCCSR_FST | UDCCSR_FEF);
        if (is_ep0(ep))
                set_ep0state(ep->dev, STALL);
 
@@ -1342,7 +1359,7 @@ static void pxa_ep_fifo_flush(struct usb_ep *_ep)
                        udc_ep_readl(ep, UDCDR);
        } else {
                /* most IN status is the same, but ISO can't stall */
-               udc_ep_writel(ep, UDCCSR,
+               ep_write_UDCCSR(ep,
                                UDCCSR_PC | UDCCSR_FEF | UDCCSR_TRN
                                | (EPXFERTYPE_is_ISO(ep) ? 0 : UDCCSR_SST));
        }
@@ -1727,6 +1744,7 @@ static void udc_enable(struct pxa_udc *udc)
        memset(&udc->stats, 0, sizeof(udc->stats));
 
        udc_set_mask_UDCCR(udc, UDCCR_UDE);
+       ep_write_UDCCSR(&udc->pxa_ep[0], UDCCSR0_ACM);
        udelay(2);
        if (udc_readl(udc, UDCCR) & UDCCR_EMCE)
                dev_err(udc->dev, "Configuration errors, udc disabled\n");
@@ -1899,7 +1917,7 @@ static void handle_ep0_ctrl_req(struct pxa_udc *udc,
         * packet. Generalize to pxa27x CPUs.
         */
        if (epout_has_pkt(ep) && (ep_count_bytes_remain(ep) == 0))
-               udc_ep_writel(ep, UDCCSR, UDCCSR0_OPC);
+               ep_write_UDCCSR(ep, UDCCSR0_OPC);
 
        /* read SETUP packet */
        for (i = 0; i < 2; i++) {
@@ -1927,7 +1945,7 @@ static void handle_ep0_ctrl_req(struct pxa_udc *udc,
                set_ep0state(udc, OUT_DATA_STAGE);
 
        /* Tell UDC to enter Data Stage */
-       udc_ep_writel(ep, UDCCSR, UDCCSR0_SA | UDCCSR0_OPC);
+       ep_write_UDCCSR(ep, UDCCSR0_SA | UDCCSR0_OPC);
 
        i = udc->driver->setup(&udc->gadget, &u.r);
        if (i < 0)
@@ -1937,7 +1955,7 @@ out:
 stall:
        ep_dbg(ep, "protocol STALL, udccsr0=%03x err %d\n",
                udc_ep_readl(ep, UDCCSR), i);
-       udc_ep_writel(ep, UDCCSR, UDCCSR0_FST | UDCCSR0_FTF);
+       ep_write_UDCCSR(ep, UDCCSR0_FST | UDCCSR0_FTF);
        set_ep0state(udc, STALL);
        goto out;
 }
@@ -2008,7 +2026,7 @@ static void handle_ep0(struct pxa_udc *udc, int fifo_irq, int opc_irq)
        if (udccsr0 & UDCCSR0_SST) {
                ep_dbg(ep, "clearing stall status\n");
                nuke(ep, -EPIPE);
-               udc_ep_writel(ep, UDCCSR, UDCCSR0_SST);
+               ep_write_UDCCSR(ep, UDCCSR0_SST);
                ep0_idle(udc);
        }
 
@@ -2033,7 +2051,7 @@ static void handle_ep0(struct pxa_udc *udc, int fifo_irq, int opc_irq)
                break;
        case IN_DATA_STAGE:                     /* GET_DESCRIPTOR */
                if (epout_has_pkt(ep))
-                       udc_ep_writel(ep, UDCCSR, UDCCSR0_OPC);
+                       ep_write_UDCCSR(ep, UDCCSR0_OPC);
                if (req && !ep_is_full(ep))
                        completed = write_ep0_fifo(ep, req);
                if (completed)
@@ -2046,7 +2064,7 @@ static void handle_ep0(struct pxa_udc *udc, int fifo_irq, int opc_irq)
                        ep0_end_out_req(ep, req);
                break;
        case STALL:
-               udc_ep_writel(ep, UDCCSR, UDCCSR0_FST);
+               ep_write_UDCCSR(ep, UDCCSR0_FST);
                break;
        case IN_STATUS_STAGE:
                /*
@@ -2141,6 +2159,7 @@ static void pxa27x_change_configuration(struct pxa_udc *udc, int config)
 
        set_ep0state(udc, WAIT_ACK_SET_CONF_INTERF);
        udc->driver->setup(&udc->gadget, &req);
+       ep_write_UDCCSR(&udc->pxa_ep[0], UDCCSR0_AREN);
 }
 
 /**
@@ -2169,6 +2188,7 @@ static void pxa27x_change_interface(struct pxa_udc *udc, int iface, int alt)
 
        set_ep0state(udc, WAIT_ACK_SET_CONF_INTERF);
        udc->driver->setup(&udc->gadget, &req);
+       ep_write_UDCCSR(&udc->pxa_ep[0], UDCCSR0_AREN);
 }
 
 /*
@@ -2290,7 +2310,7 @@ static void irq_udc_reset(struct pxa_udc *udc)
        memset(&udc->stats, 0, sizeof udc->stats);
 
        nuke(ep, -EPROTO);
-       udc_ep_writel(ep, UDCCSR, UDCCSR0_FTF | UDCCSR0_OPC);
+       ep_write_UDCCSR(ep, UDCCSR0_FTF | UDCCSR0_OPC);
        ep0_idle(udc);
 }
 

diff --git a/drivers/usb/gadget/pxa27x_udc.h b/drivers/usb/gadget/pxa27x_udc.h
index db58125331dad5d3788d5b3774a34f1637c893df..e25225e26586698ea1ac7ceb5e042b38c4245574 100644 (file)
--- a/drivers/usb/gadget/pxa27x_udc.h
+++ b/drivers/usb/gadget/pxa27x_udc.h
@@ -130,6 +130,8 @@
 #define UP2OCR_HXOE    (1 << 17)       /* Transceiver Output Enable */
 #define UP2OCR_SEOS    (1 << 24)       /* Single-Ended Output Select */
 
+#define UDCCSR0_ACM    (1 << 9)        /* Ack Control Mode */
+#define UDCCSR0_AREN   (1 << 8)        /* Ack Response Enable */
 #define UDCCSR0_SA     (1 << 7)        /* Setup Active */
 #define UDCCSR0_RNE    (1 << 6)        /* Receive FIFO Not Empty */
 #define UDCCSR0_FST    (1 << 5)        /* Force Stall */