Skip to content

Commit

Permalink
USB: pxa27x_udc: single-thread setup requests
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Robert Jarzmik authored and Greg Kroah-Hartman committed Jun 16, 2009
1 parent 9f5351b commit 367815e
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 15 deletions.
50 changes: 35 additions & 15 deletions drivers/usb/gadget/pxa27x_udc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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" : "",
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -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)
Expand All @@ -1937,7 +1955,7 @@ static void handle_ep0_ctrl_req(struct pxa_udc *udc,
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;
}
Expand Down Expand Up @@ -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);
}

Expand All @@ -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)
Expand All @@ -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:
/*
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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);
}

/*
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 2 additions & 0 deletions drivers/usb/gadget/pxa27x_udc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down

0 comments on commit 367815e

Please sign in to comment.