SAM4E USP: Re-vamp read request queue handling. Add logic to handle RX overrun errors

1 files changed, 140 insertions, 112 deletions

diff --git a/nuttx/arch/arm/src/sam34/sam_udp.c b/nuttx/arch/arm/src/sam34/sam_udp.c
index acb0657e2..948020a8e 100644
--- a/nuttx/arch/arm/src/sam34/sam_udp.c
+++ b/nuttx/arch/arm/src/sam34/sam_udp.c
@@ -159,7 +159,7 @@
 #define SAM_TRACEERR_IRQREGISTRATION      0x0018
 #define SAM_TRACEERR_NOTCONFIGURED        0x0019
 #define SAM_TRACEERR_REQABORTED           0x001a
-#define SAM_TRACEERR_RXDATABK0ERR         0x001b
+#define SAM_TRACEERR_RXDATABKERR          0x001b
 #define SAM_TRACEERR_TXCOMPERR            0x001c
 #define SAM_TRACEERR_UNSUPPEPTYPE         0x001d
 
@@ -230,7 +230,7 @@ enum sam_epstate_e
   UDP_EPSTATE_STALLED,      /* Endpoint is stalled */
   UDP_EPSTATE_IDLE,         /* Endpoint is idle (i.e. ready for transmission) */
   UDP_EPSTATE_SENDING,      /* Endpoint is sending data */
-  UDP_EPSTATE_RECEIVING,    /* Endpoint is receiving data */
+  UDP_EPSTATE_RXSTOPPED,    /* OUT dndpoint is stopped waiting for a read request */
                             /* --- Endpoint 0 Only --- */
   UDP_EPSTATE_EP0DATAOUT,   /* Endpoint 0 is receiving SETUP OUT data */
   UDP_EPSTATE_EP0STATUSIN,  /* Endpoint 0 is sending SETUP status */
@@ -303,7 +303,7 @@ struct sam_ep_s
   uint8_t              halted:1;     /* true: Endpoint feature halted */
   uint8_t              zlpneeded:1;  /* Zero length packet needed at end of transfer */
   uint8_t              zlpsent:1;    /* Zero length packet has been sent */
-  uint8_t              wqbusy:1;     /* Write request queue is busy (recursion avoidance kludge) */
+  uint8_t              txbusy:1;     /* Write request queue is busy (recursion avoidance kludge) */
 };
 
 struct sam_usbdev_s
@@ -381,7 +381,8 @@ static void   sam_req_wrsetup(struct sam_usbdev_s *priv,
 static int    sam_req_write(struct sam_usbdev_s *priv,
                 struct sam_ep_s *privep);
 static int    sam_req_read(struct sam_usbdev_s *priv,
-                struct sam_ep_s *privep, uint16_t recvsize);
+                struct sam_ep_s *privep, uint16_t recvsize,
+                int bank);
 static void   sam_req_cancel(struct sam_ep_s *privep, int16_t status);
 
 /* Interrupt level processing ***********************************************/
@@ -532,7 +533,7 @@ const struct trace_msg_t g_usb_trace_strings_deverror[] =
   TRACE_STR(SAM_TRACEERR_IRQREGISTRATION),
   TRACE_STR(SAM_TRACEERR_NOTCONFIGURED),
   TRACE_STR(SAM_TRACEERR_REQABORTED),
-  TRACE_STR(SAM_TRACEERR_RXDATABK0ERR),
+  TRACE_STR(SAM_TRACEERR_RXDATABKERR),
   TRACE_STR(SAM_TRACEERR_TXCOMPERR),
   TRACE_STR(SAM_TRACEERR_UNSUPPEPTYPE),
   TRACE_STR_END
@@ -1050,7 +1051,7 @@ static int sam_req_write(struct sam_usbdev_s *priv, struct sam_ep_s *privep)
 
       if (privep->epstate == UDP_EPSTATE_IDLE)
         {
-          /* Return the write request to the class driver.  Set the wqbusy
+          /* Return the write request to the class driver.  Set the txbusy
            * bit to prevent being called recursively from any new submission
            * generated by returning the write request.
            */
@@ -1058,9 +1059,9 @@ static int sam_req_write(struct sam_usbdev_s *priv, struct sam_ep_s *privep)
           usbtrace(TRACE_COMPLETE(epno), privreq->req.xfrd);
           DEBUGASSERT(privreq->req.len == privreq->req.xfrd);
 
-          privep->wqbusy = true;
+          privep->txbusy = true;
           sam_req_complete(privep, OK);
-          privep->wqbusy = false;
+          privep->txbusy = false;
         }
     }
 
@@ -1073,12 +1074,12 @@ static int sam_req_write(struct sam_usbdev_s *priv, struct sam_ep_s *privep)
  * Description:
  *   Complete the last read request by transferring the data from the RX FIFO
  *   to the request buffer, return the completed read request to the class
- *   implementation, and try to started the next queued read request.
+ *   implementation, and try to start the next queued read request.
  *
- *   This function is called in one of three contexts:  (1) When the endpoint
- *   is IDLE and a new read request is submitted (with interrupts disabled),
- *   (2) from interrupt handling when the current FIFO transfer completes,
- *   or (3) when resuming a stalled OUT or control endpoint.
+ *   This function is called in one of two contexts:  The normal case is (1)
+ *   from interrupt handling when the current RX FIFO transfer completes.
+ *   But there is also a special case (2) when the OUT endpoint is stopped
+ *   because there are no available read requests.
  *
  *   Calling rules:
  *
@@ -1093,10 +1094,13 @@ static int sam_req_write(struct sam_usbdev_s *priv, struct sam_ep_s *privep)
  *     When the transfer completes, the 'recvsize' is the number of bytes
  *     waiting in the FIFO to be read.
  *
+ *     bank indicates the bit in the CSR register that must be cleared
+ *     after the data has been read from the RX FIFO
+ *
  ****************************************************************************/
 
 static int sam_req_read(struct sam_usbdev_s *priv, struct sam_ep_s *privep,
-                        uint16_t recvsize)
+                        uint16_t recvsize, int bank)
 {
   struct sam_req_s *privreq;
   volatile const uint32_t *fifo;
@@ -1107,19 +1111,30 @@ static int sam_req_read(struct sam_usbdev_s *priv, struct sam_ep_s *privep,
 
   DEBUGASSERT(priv && privep && privep->epstate == UDP_EPSTATE_IDLE);
 
-  /* Loop in case we need to handle multiple read requests */
+  /* Check the request from the head of the endpoint request queue */
 
-  while (privep->epstate == UDP_EPSTATE_IDLE)
+  epno = USB_EPNO(privep->ep.eplog);
+  do
     {
-      /* Check the request from the head of the endpoint request queue */
+      /* Peek at the next read request in the requeust queue */
 
-      epno    = USB_EPNO(privep->ep.eplog);
       privreq = sam_rqpeek(&privep->reqq);
       if (!privreq)
         {
-          /* No packet to receive data */
+          /* No read request to receive data */
 
           usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_EPOUTQEMPTY), epno);
+
+          /* Disable further interrupts from this endpoint.  The RXDATABK0/1
+           * interrupt will pend until either another read request is received
+           * from the class driver or until the endpoint is reset because of
+           * no response.  Set a flag so that we know that we are in this
+           * perverse state and can re-enable endpoint interrupts when the
+           * next read request is received.
+           */
+
+          sam_putreg(UDP_INT_EP(epno), SAM_UDP_IDR);
+          privep->epstate = UDP_EPSTATE_RXSTOPPED;
           return -ENOENT;
         }
 
@@ -1132,70 +1147,58 @@ static int sam_req_read(struct sam_usbdev_s *priv, struct sam_ep_s *privep,
         {
           usbtrace(TRACE_DEVERROR(SAM_TRACEERR_EPOUTNULLPACKET), 0);
           sam_req_complete(privep, OK);
-          recvsize = 0;
-          continue;
+          privreq = NULL;
         }
+    }
+  while (privreq == NULL);
 
-      usbtrace(TRACE_READ(USB_EPNO(privep->ep.eplog)), recvsize);
-
-      /* Get the number of bytes that can be received.  This is the size
-       * of the user-provided request buffer, minus the number of bytes
-       * already transferred to the user-buffer.
-       */
-
-      remaining = privreq->req.len - privreq->req.xfrd;
+  usbtrace(TRACE_READ(USB_EPNO(privep->ep.eplog)), recvsize);
 
-      /* Read the smaller of the number of bytes available in FIFO and the
-       * size remaining in the request buffer provided by the caller.
-       */
+  /* Get the number of bytes that can be received.  This is the size
+   * of the user-provided request buffer, minus the number of bytes
+   * already transferred to the user-buffer.
+   */
 
-      readlen  = MIN(remaining, recvsize);
-      recvsize = 0;
+  remaining = privreq->req.len - privreq->req.xfrd;
 
-      /* Get the source and destination transfer addresses */
+  /* Read the smaller of the number of bytes available in FIFO and the
+   * size remaining in the request buffer provided by the caller.
+   */
 
-      fifo = (volatile const uint32_t *)SAM_UDPEP_FDR(epno);
-      dest = privreq->req.buf + privreq->req.xfrd;
+  readlen  = MIN(remaining, recvsize);
+  recvsize = 0;
 
-      /* Update the total number of bytes transferred */
+  /* Get the source and destination transfer addresses */
 
-      privreq->req.xfrd += readlen;
-      privreq->inflight  = 0;
+  fifo = (volatile const uint32_t *)SAM_UDPEP_FDR(epno);
+  dest = privreq->req.buf + privreq->req.xfrd;
 
-      /* Retrieve packet from the endpoint FIFO */
+  /* Update the total number of bytes transferred */
 
-      for (; readlen > 0; readlen--)
-        {
-          *dest++ = (uint8_t)(*fifo);
-        }
+  privreq->req.xfrd += readlen;
+  privreq->inflight  = 0;
 
-      /* If nothing has yet be transferred into the read request, then
-       * indicate that we are in the RECEIVING state.
-       */
+  /* Retrieve packet from the endpoint FIFO */
 
-      if (privreq->req.xfrd == 0)
-        {
-          /* Set the RECEIVING state */
-
-          privep->epstate = UDP_EPSTATE_RECEIVING;
-        }
+  for (; readlen > 0; readlen--)
+    {
+      *dest++ = (uint8_t)(*fifo);
+    }
 
-      /* We will not try to accumulate packet data here.  If anything
-       * has been received, we will complete the transfer immediately and
-       * give the data to the class driver.  The idea is that we will let the
-       * receiving be in-charge if incoming buffer.
-       */
+  /* We get here when an RXDATABK0/1 interrupt occurs.  That interrupt
+   * cannot be cleared until all of the data has been taken from the RX
+   * FIFO.  But we can
+   */
 
-      else
-        {
-          /* Return the read request to the class driver. */
+  sam_csr_clrbits(epno, bank ? UDPEP_CSR_RXDATABK1 : UDPEP_CSR_RXDATABK0);
 
-          usbtrace(TRACE_COMPLETE(epno), privreq->req.xfrd);
-          privep->epstate = UDP_EPSTATE_IDLE;
-          sam_req_complete(privep, OK);
-        }
-    }
+  /* Complete the transfer immediately and give the data to the class
+   * driver.  The idea is that we will let the receiving be in-charge of
+   * re-assembling data fragments.
+   */
 
+  usbtrace(TRACE_COMPLETE(epno), privreq->req.xfrd);
+  sam_req_complete(privep, OK);
   return OK;
 }
 
@@ -1846,24 +1849,31 @@ static void sam_ep_bankinterrupt(struct sam_usbdev_s *priv,
 {
   uint32_t eptype;
   uint16_t pktsize;
+  uint8_t epno;
 
   /* Get the endpoint type */
 
   eptype = csr & UDPEP_CSR_EPTYPE_MASK;
+  epno   = USB_EPNO(privep->ep.eplog);
 
-  /* Are we receiving data for a read request? */
+  /* Are we receiving data for a read request?  EP0 does not receive data
+   * using read requests.
+   */
 
-  if (privep->epstate == UDP_EPSTATE_RECEIVING)
+  if (privep->epstate == UDP_EPSTATE_IDLE && epno != 0)
     {
       /* Yes, get the size of the packet that we just received */
 
       pktsize = (uint16_t)
         ((csr & UDPEP_CSR_RXBYTECNT_MASK) >> UDPEP_CSR_RXBYTECNT_SHIFT);
 
-      /* And continue processing the read request */
+      /* And continue processing the read request.  sam_req_read will
+       * clear the  RXDATABK1 interrupt once that data has been
+       * transferred from the FIFO.
+       */
 
       privep->epstate = UDP_EPSTATE_IDLE;
-      sam_req_read(priv, privep, pktsize);
+      (void)sam_req_read(priv, privep, pktsize, bank);
     }
 
   /* Did we just receive the data associated with an OUT SETUP command? */
@@ -1872,6 +1882,8 @@ static void sam_ep_bankinterrupt(struct sam_usbdev_s *priv,
     {
       uint16_t len;
 
+      DEBUGASSERT(epno == EP0 && bank == 0);
+
       /* Yes.. back to the IDLE state */
 
       privep->epstate = UDP_EPSTATE_IDLE;
@@ -1890,23 +1902,46 @@ static void sam_ep_bankinterrupt(struct sam_usbdev_s *priv,
 
           sam_ep0_read(priv->ep0out, len);
 
+          /* Clear the RX Data Bank 0 interrupt (should not be bank 1!). */
+
+          sam_csr_clrbits(EP0, UDPEP_CSR_RXDATABK0);
+
           /* And handle the EP0 SETUP now. */
 
           sam_ep0_setup(priv);
         }
       else
         {
+          /* Clear the RX Data Bank 0 interrupt (should not be bank 1!).
+           * Then stall.
+           */
+
           usbtrace(TRACE_DEVERROR(SAM_TRACEERR_EP0SETUPOUTSIZE), pktsize);
+          sam_csr_clrbits(EP0, UDPEP_CSR_RXDATABK0);
           (void)sam_ep_stall(privep);
         }
     }
 
-  /* Check if ACK received on a Control EP */
+  /* Check for a EP0 STATUS packet returned by the host at the end of a
+   * SETUP status phase
+   */
 
-  else if (eptype != UDPEP_CSR_EPTYPE_CTRL ||
-          (csr & UDPEP_CSR_RXBYTECNT_MASK) != 0)
+  else if (eptype == UDPEP_CSR_EPTYPE_CTRL &&
+          (csr & UDPEP_CSR_RXBYTECNT_MASK) == 0)
     {
-      usbtrace(TRACE_DEVERROR(SAM_TRACEERR_RXDATABK0ERR), privep->epstate);
+      DEBUGASSERT(epno == EP0 && bank == 0);
+
+      /* Clear the RX Data Bank 0 interrupt */
+
+      sam_csr_clrbits(EP0, UDPEP_CSR_RXDATABK0);
+    }
+
+  /* Otherwise there is a problem.  Complain an clear the interrupt */
+
+  else
+    {
+      usbtrace(TRACE_DEVERROR(SAM_TRACEERR_RXDATABKERR), privep->epstate);
+      sam_csr_clrbits(epno, bank ? UDPEP_CSR_RXDATABK1 : UDPEP_CSR_RXDATABK0);
     }
 }
 
@@ -1990,13 +2025,12 @@ static void sam_ep_interrupt(struct sam_usbdev_s *priv, int epno)
     {
       usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_RXDATABK0), (uint16_t)csr);
 
-      /* Handle data received on Bank 0 */
+      /* Handle data received on Bank 0.  sam_ep_bankinterrupt will
+       * clear the  RXDATABK0 interrupt once that data has been
+       * transferred from the FIFO.
+       */
 
       sam_ep_bankinterrupt(priv, privep, csr, 0);
-
-      /* Acknowledge the RX Data Bank 0 interrupt */
-
-      sam_csr_clrbits(epno, UDPEP_CSR_RXDATABK0);
     }
 
   /* OUT packet received in data bank 1 */
@@ -2004,14 +2038,14 @@ static void sam_ep_interrupt(struct sam_usbdev_s *priv, int epno)
   else if ((csr & UDPEP_CSR_RXDATABK1) != 0)
     {
       usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_RXDATABK1), (uint16_t)csr);
+      DEBUGASSERT(SAM_UDP_NBANKS(epno) > 1);
 
-      /* Handle data received on Bank 1 */
+      /* Handle data received on Bank 1.  sam_ep_bankinterrupt will
+       * clear the  RXDATABK1 interrupt once that data has been
+       * transferred from the FIFO.
+       */
 
       sam_ep_bankinterrupt(priv, privep, csr, 1);
-
-      /* Acknowledge the RX Data Bank 1 interrupt */
-
-      sam_csr_clrbits(epno, UDPEP_CSR_RXDATABK1);
     }
 
   /* STALL sent */
@@ -2024,7 +2058,7 @@ static void sam_ep_interrupt(struct sam_usbdev_s *priv, int epno)
 
       usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_STALLSNT), (uint16_t)csr);
 
-      /* Acknowledge the interrupt */
+      /* Clear the STALLSENT interrupt */
 
       sam_csr_clrbits(epno, UDPEP_CSR_STALLSENT);
 
@@ -2059,10 +2093,9 @@ static void sam_ep_interrupt(struct sam_usbdev_s *priv, int epno)
 
       usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_RXSETUP), (uint16_t)csr);
 
-      /* If a request transfer was pending, complete it. */
+      /* If a write request transfer was pending, complete it. */
 
-      if (privep->epstate == UDP_EPSTATE_RECEIVING ||
-          privep->epstate == UDP_EPSTATE_SENDING)
+      if (privep->epstate == UDP_EPSTATE_SENDING)
         {
           sam_req_complete(privep, -EPROTO);
         }
@@ -2475,7 +2508,7 @@ static void sam_ep_reset(struct sam_usbdev_s *priv, uint8_t epno)
   privep->halted    = false;
   privep->zlpneeded = false;
   privep->zlpsent   = false;
-  privep->wqbusy    = false;
+  privep->txbusy    = false;
 }
 
 /****************************************************************************
@@ -2531,8 +2564,8 @@ static int sam_ep_stall(struct sam_ep_s *privep)
       epno = USB_EPNO(privep->ep.eplog);
       usbtrace(TRACE_EPSTALL, epno);
 
-      /* If this is an IN endpoint (or endpoint 0), then cancel all
-       * of the pending write requests.
+      /* If this is an IN endpoint (or endpoint 0), then cancel any
+       * write requests in progress.
        */
 
       if (epno == 0 || USB_ISEPIN(privep->ep.eplog))
@@ -2540,15 +2573,6 @@ static int sam_ep_stall(struct sam_ep_s *privep)
           sam_req_cancel(privep, -EPERM);
         }
 
-      /* Otherwise, it is an OUT endpoint.  Complete any read request
-       * currently in progress (they will get requeued immediately).
-       */
-
-      else if (privep->epstate == UDP_EPSTATE_RECEIVING)
-        {
-          sam_req_complete(privep, -EPERM);
-        }
-
       /* Put endpoint into stalled state */
 
       privep->epstate = UDP_EPSTATE_STALLED;
@@ -2620,12 +2644,6 @@ static int sam_ep_resume(struct sam_ep_s *privep)
 
           (void)sam_req_write(priv, privep);
         }
-      else
-        {
-          /* OUT endpoint.  Restart any queued read requests. */
-
-          (void)sam_req_read(priv, privep, 0);
-        }
     }
 
   irqrestore(flags);
@@ -3117,7 +3135,7 @@ static int sam_ep_submit(struct usbdev_ep_s *ep, struct usbdev_req_s *req)
            * processing in progress, then transfer the data now.
            */
 
-          if (privep->epstate == UDP_EPSTATE_IDLE && !privep->wqbusy)
+          if (privep->epstate == UDP_EPSTATE_IDLE && !privep->txbusy)
             {
               ret = sam_req_write(priv, privep);
             }
@@ -3133,11 +3151,21 @@ static int sam_ep_submit(struct usbdev_ep_s *ep, struct usbdev_req_s *req)
       sam_req_enqueue(&privep->reqq, privreq);
       usbtrace(TRACE_OUTREQQUEUED(epno), req->len);
 
-      /* If the OUT endpoint IDLE, then setup the read */
+      /* Check if we have stopped RX receipt due to lack of read
+       * read requests.  If that is the case for this endpoint, then
+       * re-enable endpoint interrupts now.
+       */
 
-      if (privep->epstate == UDP_EPSTATE_IDLE)
+      if (privep->epstate == UDP_EPSTATE_RXSTOPPED)
         {
-          ret = sam_req_read(priv, privep, 0);
+          /* Un-stop the OUT endpoint be re-enabling endpoint interrupts.
+           * There should be a pending RXDATABK0/1 interrupt or, if a long
+           * time has elapsed since the endpoint was stopped, an ENDBUSRES
+           * interrupt.
+           */
+
+          privep->epstate = UDP_EPSTATE_IDLE;
+          sam_putreg(UDP_INT_EP(epno), SAM_UDP_IER);
         }
     }
 
@@ -3565,7 +3593,7 @@ static void sam_reset(struct sam_usbdev_s *priv)
       privep->halted    = false;
       privep->zlpneeded = false;
       privep->zlpsent   = false;
-      privep->wqbusy    = false;
+      privep->txbusy    = false;
     }
 
   /* Re-configure the USB controller in its initial, unconnected state */