Hi,
On Wed, Sep 24, 2014 at 01:08:15PM -0500, Felipe Balbi wrote:
> On Wed, Sep 24, 2014 at 01:53:31PM -0400, Alan Stern wrote:
> > On Wed, 24 Sep 2014, Felipe Balbi wrote:
> >
> > > > I'll capture usbmon and send here shortly.
> > >
> > > here it is... Interesting part starts at line 73 (114 on this email)
> > > where the data transport received EPIPE (due to Stall). This time
> > > however, I was eventually able to talk to the device and managed to
> > > issue quite a few writes to it.
> >
> > Here's where the unexpected stuff begins:
> >
> > > ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 06000000 c0000000 8000061a 003f00c0 00000000 00000000 000000
> > > ed2541c0 1237463431 C Bo:003:01 0 31 >
> > > ec1a8540 1237463873 S Bi:003:01 -115 192 <
> > > ec1a8540 1237464053 C Bi:003:01 -32 0
> > > ed2541c0 1237464158 S Co:003:00 s 02 01 0000 0081 0000 0
> > > ed2541c0 1237464359 C Co:003:00 0 0
> > > ed2541c0 1237468607 S Bi:003:01 -115 13 <
> > > ed2541c0 1237468802 C Bi:003:01 -75 0
> >
> > This is the first MODE SENSE command. The gadget should send as much
> > data as it can before halting the bulk-IN endpoint. Instead, the
> > endpoint was halted first.
> >
> > Then, after the host cleared the halt, the gadget apparently sent the
> > data that _should_ have been sent previously. The host was expecting
> > to receive the CSW at this point, so there was an overflow error.
> > That's what caused the host to perform a reset.
> >
> > Evidently this UDC implements the set_halt method incorrectly.
> > According to the kerneldoc for usb_ep_set_halt:
> >
> > * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any
> > * transfer requests are still queued, or if the controller hardware
> > * (usually a FIFO) still holds bytes that the host hasn't collected.
>
> damn old bugs :-) I'll fix that up and Cc stable.
alright fixed. Below you can see a combined diff which I'll still split
into patches so they can be applied properly.
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 66f6256..834f524 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -210,6 +210,14 @@
#define DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(n) (((n) & (0x0f << 13)) >> 13)
#define DWC3_MAX_HIBER_SCRATCHBUFS 15
+/* Global FIFO Space Register */
+#define DWC3_GDBGFIFOSPACE_TXFIFO (0 << 5)
+#define DWC3_GDBGFIFOSPACE_RXFIFO (1 << 5)
+#define DWC3_GDBGFIFOSPACE_TXREQ_Q (2 << 5)
+#define DWC3_GDBGFIFOSPACE_RXREQ_Q (3 << 5)
+
+#define DWC3_GDBGFIFOSPACE_SPACE_AVAIL(num) (((num) & 0xffff0000) >> 16)
+
/* Device Configuration Register */
#define DWC3_DCFG_DEVADDR(addr) ((addr) << 3)
#define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index de53361..5e89913 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -145,6 +145,75 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum dwc3_link_state state)
}
/**
+ * dwc3_gadget_ep_fifo_space - returns currently available space on FIFO
+ * @dwc: pointer to our context struct
+ * @dep: the endpoint to fetch FIFO space
+ *
+ * This function will return the currently available FIFO space
+ */
+static int dwc3_gadget_ep_fifo_space(struct dwc3 *dwc, struct dwc3_ep *dep)
+{
+ u32 current_fifo;
+ u32 reg;
+
+ if (dep->direction)
+ reg = DWC3_GDBGFIFOSPACE_TXFIFO;
+ else
+ reg = DWC3_GDBGFIFOSPACE_RXFIFO;
+
+ /* remove direction bit */
+ reg |= dep->number >> 1;
+
+ dwc3_writel(dwc->regs, DWC3_GDBGFIFOSPACE, reg);
+ reg = dwc3_readl(dwc->regs, DWC3_GDBGFIFOSPACE);
+ current_fifo = DWC3_GDBGFIFOSPACE_SPACE_AVAIL(reg);
+
+ return current_fifo;
+}
+
+/**
+ * dwc3_gadget_ep_fifo_size - returns allocated FIFO size
+ * @dwc: pointer to our context struct
+ * @dep: TX endpoint to fetch allocated FIFO size
+ *
+ * This function will read the correct TX FIFO register and
+ * return the FIFO size
+ */
+static int dwc3_gadget_ep_fifo_size(struct dwc3 *dwc, struct dwc3_ep *dep)
+{
+ if (WARN_ON(!dep->direction))
+ return 0;
+
+ return dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(dep->number)) & 0xffff;
+}
+
+/**
+ * dwc3_gadget_ep_fifo_empty - returns true when FIFO is empty
+ * @dwc: pointer to our context struct
+ * @dep: endpoint to request FIFO space
+ *
+ * This function will return a TRUE when FIFO is empty and FALSE
+ * otherwise.
+ */
+static int dwc3_gadget_ep_fifo_empty(struct dwc3 *dwc, struct dwc3_ep *dep)
+{
+ u32 allocated_fifo;
+ u32 current_fifo;
+
+ /* should not be called for RX endpoints */
+ if (WARN_ON(!dep->direction))
+ return false;
+
+ current_fifo = dwc3_gadget_ep_fifo_space(dwc, dep);
+ allocated_fifo = dwc3_gadget_ep_fifo_size(dwc, dep);
+
+ dev_vdbg(dwc->dev, "%s: FIFO space %u/%u\n", dep->name,
+ current_fifo, allocated_fifo);
+
+ return current_fifo == allocated_fifo;
+}
+
+/**
* dwc3_gadget_resize_tx_fifos - reallocate fifo spaces for current use-case
* @dwc: pointer to our context structure
*
@@ -1216,6 +1285,20 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value)
memset(¶ms, 0x00, sizeof(params));
if (value) {
+ if (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;
+ }
+
+ if (dep->direction && !dwc3_gadget_ep_fifo_empty(dwc, dep)) {
+ dev_dbg(dwc->dev, "%s: FIFO not empty, cannot halt\n",
+ dep->name);
+ return -EAGAIN;
+ }
+
ret = dwc3_send_gadget_ep_cmd(dwc, dep->number,
DWC3_DEPCMD_SETSTALL, ¶ms);
if (ret)
--
balbi
Attachment:
signature.asc
Description: Digital signature
