Separate SAMA5 OHCI interrupt handling into separate functions

2 files changed, 209 insertions, 150 deletions

diff --git a/nuttx/arch/arm/src/sama5/sam_ohci.c b/nuttx/arch/arm/src/sama5/sam_ohci.c
index d466f7648..3c275010f 100644
--- a/nuttx/arch/arm/src/sama5/sam_ohci.c
+++ b/nuttx/arch/arm/src/sama5/sam_ohci.c
@@ -345,6 +345,8 @@ static int sam_ctrltd(struct sam_ohci_s *priv, uint32_t dirpid,
 
 /* Interrupt handling **********************************************************/
 
+static void sam_rhsc_interrupt(struct sam_ohci_s *priv);
+static void sam_wdh_interrupt(struct sam_ohci_s *priv);
 static int sam_ohci_interrupt(int irq, FAR void *context);
 
 /* USB host controller operations **********************************************/
@@ -1188,7 +1190,8 @@ static inline int sam_remisoced(struct sam_ohci_s *priv,
 
 static int sam_enqueuetd(struct sam_ohci_s *priv,
                          struct sam_ed_s *ed, uint32_t dirpid,
-                         uint32_t toggle, volatile uint8_t *buffer, size_t buflen)
+                         uint32_t toggle, volatile uint8_t *buffer,
+                         size_t buflen)
 {
   struct sam_gtd_s *td;
   int ret = -ENOMEM;
@@ -1339,221 +1342,272 @@ static int sam_ctrltd(struct sam_ohci_s *priv, uint32_t dirpid, uint8_t *buffer,
 }
 
 /*******************************************************************************
- * Name: sam_ohci_interrupt
+ * Name: sam_rhsc_interrupt
  *
  * Description:
- *   USB interrupt handler
+ *   OHCI root hub status change interrupt handler
  *
  *******************************************************************************/
 
-static int sam_ohci_interrupt(int irq, FAR void *context)
+static void sam_rhsc_interrupt(struct sam_ohci_s *priv)
 {
-  struct sam_ohci_s *priv = &g_usbhost;
-  uint32_t intst;
-  uint32_t pending;
-  uint32_t regval;
+  struct sam_rhport_s *rhport;
   uint32_t regaddr;
+  uint32_t rhportst;
   int rhpndx;
 
-  /* Read Interrupt Status and mask out interrupts that are not enabled. */
-
-  intst  = sam_getreg(SAM_USBHOST_INTST);
-  regval = sam_getreg(SAM_USBHOST_INTEN);
-  ullvdbg("INST: %08x INTEN: %08x\n", intst, regval);
+  /* Handle root hub status change on each root port */
 
-  pending = intst & regval;
-  if (pending != 0)
+  for (rhpndx = 0; rhpndx < SAM_USBHOST_NRHPORT; rhpndx++)
     {
-      /* Root hub status change interrupt */
+      rhport   = &priv->rhport[rhpndx];
 
-      if ((pending & OHCI_INT_RHSC) != 0)
+      regaddr  = SAM_USBHOST_RHPORTST(rhpndx+1);
+      rhportst = sam_getreg(regaddr);
+
+      ullvdbg("RHPORTST%d: %08x\n", rhpndx + 1, rhportst);
+
+      if ((rhportst & OHCI_RHPORTST_CSC) != 0)
         {
-          /* Hand root hub status change on each root port */
+          uint32_t rhstatus = sam_getreg(SAM_USBHOST_RHSTATUS);
+          ullvdbg("Connect Status Change, RHSTATUS: %08x\n", rhstatus);
 
-          ullvdbg("Root Hub Status Change\n");
-          for (rhpndx = 0; rhpndx < SAM_USBHOST_NRHPORT; rhpndx++)
+          /* If DRWE is set, Connect Status Change indicates a remote
+           * wake-up event
+           */
+
+          if (rhstatus & OHCI_RHSTATUS_DRWE)
             {
-              struct sam_rhport_s *rhport = &priv->rhport[rhpndx];
-              uint32_t rhportst;
+              ullvdbg("DRWE: Remote wake-up\n");
+            }
 
-              regaddr  = SAM_USBHOST_RHPORTST(rhpndx+1);
-              rhportst = sam_getreg(regaddr);
+          /* Otherwise... Not a remote wake-up event */
 
-              ullvdbg("RHPORTST%d: %08x\n", rhpndx + 1, rhportst);
+          else
+            {
+              /* Check current connect status */
 
-              if ((rhportst & OHCI_RHPORTST_CSC) != 0)
+              if ((rhportst & OHCI_RHPORTST_CCS) != 0)
                 {
-                  uint32_t rhstatus = sam_getreg(SAM_USBHOST_RHSTATUS);
-                  ullvdbg("Connect Status Change, RHSTATUS: %08x\n", rhstatus);
-
-                  /* If DRWE is set, Connect Status Change indicates a remote
-                   * wake-up event
-                   */
+                  /* Connected ... Did we just become connected? */
 
-                  if (rhstatus & OHCI_RHSTATUS_DRWE)
+                  if (!rhport->connected)
                     {
-                      ullvdbg("DRWE: Remote wake-up\n");
-                    }
+                      /* Yes.. connected. */
+
+                      rhport->connected = true;
 
-                  /* Otherwise... Not a remote wake-up event */
+                      ullvdbg("RHPort%d connected, rhswait: %d\n",
+                              rhpndx + 1, priv->rhswait);
 
+                      /* Notify any waiters */
+
+                      if (priv->rhswait)
+                        {
+                          sam_givesem(&priv->rhssem);
+                          priv->rhswait = false;
+                        }
+                    }
                   else
                     {
-                      /* Check current connect status */
+                      ulldbg("Spurious status change (connected)\n");
+                    }
 
-                      if ((rhportst & OHCI_RHPORTST_CCS) != 0)
-                        {
-                          /* Connected ... Did we just become connected? */
+                  /* The LSDA (Low speed device attached) bit is valid
+                   * when CCS == 1.
+                   */
 
-                          if (!rhport->connected)
-                            {
-                              /* Yes.. connected. */
+                  rhport->lowspeed = (rhportst & OHCI_RHPORTST_LSDA) != 0;
+                  ullvdbg("Speed: %s\n", rhport->lowspeed ? "LOW" : "FULL");
+                }
 
-                              rhport->connected = true;
+              /* Check if we are now disconnected */
 
-                              ullvdbg("RHPort%d connected, rhswait: %d\n",
-                                      rhpndx + 1, priv->rhswait);
+              else if (rhport->connected)
+                {
+                  /* Yes.. disconnect the device */
 
-                              /* Notify any waiters */
+                  ullvdbg("RHport%d disconnected\n", rhpndx+1);
+                  rhport->connected = false;
+                  rhport->lowspeed  = false;
 
-                              if (priv->rhswait)
-                                {
-                                  sam_givesem(&priv->rhssem);
-                                  priv->rhswait = false;
-                                }
-                            }
-                          else
-                            {
-                              ulldbg("Spurious status change (connected)\n");
-                            }
+                  /* Are we bound to a class instance? */
 
-                          /* The LSDA (Low speed device attached) bit is valid
-                           * when CCS == 1.
-                           */
+                  if (rhport->class)
+                    {
+                      /* Yes.. Disconnect the class */
 
-                          rhport->lowspeed =
-                            (rhportst & OHCI_RHPORTST_LSDA) != 0;
+                      CLASS_DISCONNECTED(rhport->class);
+                      rhport->class = NULL;
+                    }
 
-                          ullvdbg("Speed: %s\n",
-                                  rhport->lowspeed ? "LOW" : "FULL");
-                        }
+                  /* Notify any waiters for the Root Hub Status change
+                   * event.
+                   */
 
-                      /* Check if we are now disconnected */
+                  if (priv->rhswait)
+                    {
+                      sam_givesem(&priv->rhssem);
+                      priv->rhswait = false;
+                    }
+                }
+              else
+                {
+                   ulldbg("Spurious status change (disconnected)\n");
+                }
+            }
 
-                      else if (rhport->connected)
-                        {
-                          /* Yes.. disconnect the device */
+          /* Clear the status change interrupt */
 
-                          ullvdbg("RHport%d disconnected\n", rhpndx+1);
-                          rhport->connected = false;
-                          rhport->lowspeed  = false;
+          sam_putreg(OHCI_RHPORTST_CSC, regaddr);
+        }
 
-                          /* Are we bound to a class instance? */
+      /* Check for port reset status change */
 
-                          if (rhport->class)
-                            {
-                              /* Yes.. Disconnect the class */
+      if ((rhportst & OHCI_RHPORTST_PRSC) != 0)
+        {
+          /* Release the RH port from reset */
 
-                              CLASS_DISCONNECTED(rhport->class);
-                              rhport->class = NULL;
-                            }
+          sam_putreg(OHCI_RHPORTST_PRSC, regaddr);
+        }
+    }
+}
 
-                        /* Notify any waiters for the Root Hub Status change
-                         * event.
-                         */
+/*******************************************************************************
+ * Name: sam_wdh_interrupt
+ *
+ * Description:
+ *   OHCI write done head interrupt handler
+ *
+ *******************************************************************************/
 
-                          if (priv->rhswait)
-                            {
-                              sam_givesem(&priv->rhssem);
-                              priv->rhswait = false;
-                            }
-                        }
-                      else
-                        {
-                           ulldbg("Spurious status change (disconnected)\n");
-                        }
-                    }
+static void sam_wdh_interrupt(struct sam_ohci_s *priv)
+{
+  struct sam_gtd_s *td;
+  struct sam_gtd_s *next;
 
-                  /* Clear the status change interrupt */
+  /* The host controller just wrote the list of finished TDs into the HCCA
+   * done head.  This may include multiple packets that were transferred
+   * in the preceding frame.
+   *
+   * Remove the TD(s) from the Writeback Done Head in the HCCA and return
+   * them to the free list.  Note that this is safe because the hardware
+   * will not modify the writeback done head again until the WDH bit is
+   * cleared in the interrupt status register.
+   */
 
-                  sam_putreg(OHCI_RHPORTST_CSC, regaddr);
-                }
+  td = (struct sam_gtd_s *)g_hcca.donehead;
+  g_hcca.donehead = 0;
 
-              /* Check for port reset status change */
+  /* Process each TD in the write done list */
 
-              if ((rhportst & OHCI_RHPORTST_PRSC) != 0)
-                {
-                  /* Release the RH port from reset */
+  for (; td; td = next)
+    {
+      /* Get the ED in which this TD was enqueued */
 
-                  sam_putreg(OHCI_RHPORTST_PRSC, regaddr);
-                }
-            }
-        }
+      struct sam_ed_s *ed = td->ed;
+      DEBUGASSERT(ed != NULL);
 
-      /* Writeback Done Head interrupt */
+      /* Save the condition code from the (single) TD status/control
+       * word.
+       */
 
-      if ((pending & OHCI_INT_WDH) != 0)
+      ed->tdstatus = (td->hw.ctrl & GTD_STATUS_CC_MASK) >> GTD_STATUS_CC_SHIFT;
+
+#ifdef CONFIG_DEBUG_USB
+      if (ed->tdstatus != TD_CC_NOERROR)
         {
-          struct sam_gtd_s *td;
-          struct sam_gtd_s *next;
+          /* The transfer failed for some reason... dump some diagnostic info. */
 
-          /* The host controller just wrote the list of finished TDs into the HCCA
-           * done head.  This may include multiple packets that were transferred
-           * in the preceding frame.
-           *
-           * Remove the TD(s) from the Writeback Done Head in the HCCA and return
-           * them to the free list.  Note that this is safe because the hardware
-           * will not modify the writeback done head again until the WDH bit is
-           * cleared in the interrupt status register.
-           */
+          ulldbg("ERROR: ED xfrtype: %d TD CTRL: %08x/CC: %d\n",
+                 ed->xfrtype, td->hw.ctrl, ed->tdstatus);
+        }
+#endif
 
-          td = (struct sam_gtd_s *)g_hcca.donehead;
-          g_hcca.donehead = 0;
+      /* Return the TD to the free list */
 
-          /* Process each TD in the write done list */
+      next = (struct sam_gtd_s *)td->hw.nexttd;
+      sam_tdfree(td);
 
-          for (; td; td = next)
-            {
-              /* Get the ED in which this TD was enqueued */
+      /* And wake up the thread waiting for the WDH event */
 
-              struct sam_ed_s *ed = td->ed;
-              DEBUGASSERT(ed != NULL);
+      if (ed->wdhwait)
+        {
+          sam_givesem(&ed->wdhsem);
+          ed->wdhwait = false;
+        }
+    }
+}
 
-              /* Save the condition code from the (single) TD status/control
-               * word.
-               */
+/*******************************************************************************
+ * Name: sam_ohci_interrupt
+ *
+ * Description:
+ *   OHCI interrupt handler
+ *
+ *******************************************************************************/
 
-              ed->tdstatus = (td->hw.ctrl & GTD_STATUS_CC_MASK) >> GTD_STATUS_CC_SHIFT;
+static int sam_ohci_interrupt(int irq, FAR void *context)
+{
+  struct sam_ohci_s *priv = &g_usbhost;
+  uint32_t intst;
+  uint32_t pending;
+  uint32_t regval;
 
-#ifdef CONFIG_DEBUG_USB
-              if (ed->tdstatus != TD_CC_NOERROR)
-                {
-                  /* The transfer failed for some reason... dump some diagnostic info. */
+  /* Read Interrupt Status and mask out interrupts that are not enabled. */
 
-                  ulldbg("ERROR: ED xfrtype: %d TD CTRL: %08x/CC: %d\n",
-                         ed->xfrtype, td->hw.ctrl, ed->tdstatus);
-                }
-#endif
+  intst  = sam_getreg(SAM_USBHOST_INTST);
+  regval = sam_getreg(SAM_USBHOST_INTEN);
+  ullvdbg("INST: %08x INTEN: %08x\n", intst, regval);
 
-              /* Return the TD to the free list */
+  pending = intst & regval;
+  if (pending != 0)
+    {
+      /* Root hub status change interrupt */
 
-              next = (struct sam_gtd_s *)td->hw.nexttd;
-              sam_tdfree(td);
+      if ((pending & OHCI_INT_RHSC) != 0)
+        {
+          /* Handle root hub status change on each root port */
 
-              /* And wake up the thread waiting for the WDH event */
+          ullvdbg("Root Hub Status Change\n");
+          sam_rhsc_interrupt(priv);
+        }
 
-              if (ed->wdhwait)
-                {
-                  sam_givesem(&ed->wdhsem);
-                  ed->wdhwait = false;
-                }
-            }
+      /* Writeback Done Head interrupt */
+
+      if ((pending & OHCI_INT_WDH) != 0)
+        {
+          /* The host controller just wrote the list of finished TDs into the HCCA
+           * done head.  This may include multiple packets that were transferred
+           * in the preceding frame.
+           */
+
+          ullvdbg("Writeback Done Head interrupt\n");
+          sam_wdh_interrupt(priv);
         }
 
 #ifdef CONFIG_DEBUG_USB
       if ((pending & SAM_DEBUG_INTS) != 0)
         {
-          ulldbg("ERROR: Unhandled interrupts INTST: %08x\n", intst);
+          if ((pending & OHCI_INT_UE) != 0)
+            {
+              /* An unrecoverable error occurred.  Unrecoverable errors
+               * are usually the consequence of bad descriptor contents
+               * or DMA errors.
+               *
+               * Treat this like a normal write done head interrupt.  We
+               * just want to see if there is any status information writen
+               * to the descriptors (and the normal write done head
+               * interrupt will not be occurring).
+               */
+
+              ulldbg("ERROR: Unrecoverable error. INTST: %08x\n", intst);
+              sam_wdh_interrupt(priv);
+            }
+          else
+            {
+              ulldbg("ERROR: Unhandled interrupts INTST: %08x\n", intst);
+            }
         }
 #endif
 
@@ -1727,7 +1781,8 @@ static int sam_enumerate(FAR struct usbhost_driver_s *drvr, int rhpndx)
  *   drvr - The USB host driver instance obtained as a parameter from the call to
  *      the class create() method.
  *   funcaddr - The USB address of the function containing the endpoint that EP0
- *     controls
+ *     controls.  A funcaddr of zero will be received if no address is yet assigned
+ *     to the device.
  *   maxpacketsize - The maximum number of bytes that can be sent to or
  *    received from the endpoint in a single data packet
  *
@@ -1746,8 +1801,10 @@ static int sam_ep0configure(FAR struct usbhost_driver_s *drvr, uint8_t funcaddr,
   struct sam_ohci_s *priv = (struct sam_ohci_s *)drvr;
   struct sam_rhport_s *rhport;
 
-  DEBUGASSERT(priv && funcaddr > 0 && funcaddr <= SAM_USBHOST_NRHPORT &&
+  DEBUGASSERT(priv &&
+              funcaddr >= 0 && funcaddr <= SAM_USBHOST_NRHPORT &&
               maxpacketsize < 2048);
+
   rhport = &priv->rhport[funcaddr - 1];
 
   /* We must have exclusive access to EP0 and the control list */
diff --git a/nuttx/drivers/usbhost/usbhost_enumerate.c b/nuttx/drivers/usbhost/usbhost_enumerate.c
index 26b93bf36..7bcf1840e 100644
--- a/nuttx/drivers/usbhost/usbhost_enumerate.c
+++ b/nuttx/drivers/usbhost/usbhost_enumerate.c
@@ -401,6 +401,7 @@ int usbhost_enumerate(FAR struct usbhost_driver_s *drvr, uint8_t funcaddr,
       udbg("ERROR: SETADDRESS DRVR_CTRLOUT returned %d\n", ret);
       goto errout;
     }
+
   usleep(2*1000);
 
   /* Modify control pipe with the provided USB device address */
@@ -514,5 +515,6 @@ errout:
     {
       DRVR_FREE(drvr, (uint8_t*)ctrlreq);
     }
+
   return ret;
 }