This is an automated email from the ASF dual-hosted git repository.

xiaoxiang781216 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new fdbd3001191 drivers/usbhost: Fix concurrent HID devices on MAX3421E 
and HID reports
fdbd3001191 is described below

commit fdbd3001191d4b3522255125f36b61f7632e4893
Author: Alan Carvalho de Assis <[email protected]>
AuthorDate: Fri Oct 23 15:21:43 2026 -0300

    drivers/usbhost: Fix concurrent HID devices on MAX3421E and HID reports
    
    Fixes found while using a wireless keyboard + touchpad receiver
    (1611:2800), whose keyboard and mouse interfaces must work at the
    same time.
    
    MAX3421E:
    - Release the host lock while waiting for the next polling interval
      of an interrupt endpoint.  An idle endpoint held it until the 5s NAK
      timeout, blocking the transfers of the other interfaces.
    - Account for the data received before a NAK.  A full packet (e.g. an
      8-byte keyboard report) is followed by another IN token; when it was
      NAKed the received data was discarded, so no key was ever reported.
    - Return when the transfer completes.  A short packet did not end the
      transfer and the next report overwrote the buffer while the returned
      length kept growing.
    - Do not retry control transfers that STALL: it is the final answer of
      the device and the retries took 25s and flooded the console.
    - Bound the waits for bus reset, bus sample and SOF events, and fail
      the initialization when the oscillator does not start.  Without the
      12MHz clock these events never come and the board bring-up hung
      forever, so NSH never started.
    
    HID keyboard:
    - If the device stalls GET_REPORT, read the reports from the interrupt
      IN endpoint instead.
    - Send SET_PROTOCOL to the keyboard interface instead of interface 0.
    - Fix the nxstyle issues of the file.
    
    HID mouse:
    - Some devices ignore SET_PROTOCOL(boot) and send report protocol
      reports with a report ID.  Read the report descriptor; if the mouse
      report has a report ID and 8-bit X/Y, select the report protocol,
      skip the report ID and ignore the other reports of the interface.
    - Do not log a NAK (no data) as an error.
    
    Signed-off-by: Alan C. Assis <[email protected]>
    Assisted-by: Claude Code Opus 5.5
---
 drivers/usbhost/usbhost_hidkbd.c   | 309 ++++++++++++++++++++-----------------
 drivers/usbhost/usbhost_hidmouse.c | 198 ++++++++++++++++++++++--
 drivers/usbhost/usbhost_max3421e.c | 182 +++++++++++++++++-----
 3 files changed, 502 insertions(+), 187 deletions(-)

diff --git a/drivers/usbhost/usbhost_hidkbd.c b/drivers/usbhost/usbhost_hidkbd.c
index 5b9837e4345..0d13225f75f 100644
--- a/drivers/usbhost/usbhost_hidkbd.c
+++ b/drivers/usbhost/usbhost_hidkbd.c
@@ -212,6 +212,7 @@ struct usbhost_state_s
   size_t                  ctrllen;      /* Size of the allocated control 
buffer */
   bool                    caps_lock;    /* Private caps lock status */
   bool                    sync_led;     /* For LED update */
+  bool                    intin;        /* GET_REPORT stalled, use EP IN */
   sem_t                   syncsem;      /* Semaphore to wait for a poll thread 
*/
 
 #ifdef CONFIG_HIDKBD_NOGETREPORT
@@ -706,6 +707,7 @@ static int usbhost_allocdevno(FAR struct usbhost_state_s 
*priv)
   for (devno = 0; devno < 26; devno++)
     {
       uint32_t bitno = 1 << devno;
+
       if ((g_devinuse & bitno) == 0)
         {
           g_devinuse |= bitno;
@@ -726,6 +728,7 @@ static void usbhost_freedevno(FAR struct usbhost_state_s 
*priv)
   if (devno >= 0 && devno < 26)
     {
       irqstate_t flags = spin_lock_irqsave(&g_lock);
+
       g_devinuse &= ~(1 << devno);
       spin_unlock_irqrestore(&g_lock, flags);
     }
@@ -1354,22 +1357,50 @@ static int usbhost_kbdpoll(int argc, FAR char *argv[])
 #ifdef CONFIG_HIDKBD_NOGETREPORT
       nxmutex_unlock(&priv->lock);
 #else
-      /* Send HID report request */
+      if (!priv->intin)
+        {
+          /* Send HID report request */
+
+          ret = usbhost_send_request(priv, USB_REQ_DIR_IN,
+                                     USBHID_REQUEST_GETREPORT,
+                                     USBHID_REPORTTYPE_INPUT << 8,
+                                     priv->ifno,
+                                     sizeof(struct usbhid_kbdreport_s),
+                                     priv->tbuffer);
+
+          /* Some devices (e.g. wireless receivers) stall GET_REPORT.  Then
+           * read the reports from the interrupt IN endpoint instead.
+           */
 
-      ret = usbhost_send_request(priv, USB_REQ_DIR_IN,
-                                 USBHID_REQUEST_GETREPORT,
-                                 USBHID_REPORTTYPE_INPUT << 8,
-                                 priv->ifno,
-                                 sizeof(struct usbhid_kbdreport_s),
-                                 priv->tbuffer);
+          if (ret == -EPERM && priv->epin != NULL)
+            {
+              uwarn("WARNING: GET_REPORT stalled, using interrupt IN EP\n");
+              priv->intin = true;
+            }
+        }
 
       nxmutex_unlock(&priv->lock);
 
+      if (priv->intin)
+        {
+          /* Wait for the next report.  -EAGAIN means no new report. */
+
+          ssize_t nbytes = DRVR_TRANSFER(priv->usbclass.hport->drvr,
+                                         priv->epin, priv->tbuffer,
+                                         priv->tbuflen);
+
+          ret = nbytes < 0 ? nbytes : OK;
+        }
+
       /* Check for errors -- Bail if an excessive number of consecutive
        * errors are encountered.
        */
 
-      if (ret < 0)
+      if (ret == -EAGAIN)
+        {
+          /* No new report from the interrupt IN endpoint */
+        }
+      else if (ret < 0)
         {
           nerrors++;
           uerr("ERROR: GETREPORT/INPUT, DRVR_CTRLIN returned: %d/%d\n",
@@ -1571,138 +1602,138 @@ static inline int usbhost_cfgdesc(FAR struct 
usbhost_state_s *priv,
       desc = (FAR struct usb_desc_s *)configdesc;
       switch (desc->type)
         {
-        /* Interface descriptor. We really should get the number of endpoints
-         * from this descriptor too.
-         */
-
-        case USB_DESC_TYPE_INTERFACE:
-          {
-            FAR struct usb_ifdesc_s *ifdesc =
-              (FAR struct usb_ifdesc_s *)configdesc;
-
-            uinfo("Interface descriptor\n");
-            DEBUGASSERT(remaining >= USB_SIZEOF_IFDESC);
-
-            /* Did we already find what we needed
-             * from a preceding interface?
-             */
-
-            if ((found & USBHOST_RQDFOUND) == USBHOST_RQDFOUND)
-              {
-                /* Yes.. then break out of the loop and use the preceding
-                 * interface.
-                 */
-
-                done       = true;
-              }
-            else
-              {
-                /* Otherwise, save the interface number and discard any
-                 * endpoints previously found
-                 */
-
-                priv->ifno = ifdesc->ifno;
-                found      = USBHOST_IFFOUND;
-              }
-          }
-          break;
-
-        /* HID descriptor */
-
-        case USBHID_DESCTYPE_HID:
+          /* Interface descriptor. We really should get the number of
+           * endpoints from this descriptor too.
+           */
+
+          case USB_DESC_TYPE_INTERFACE:
+            {
+              FAR struct usb_ifdesc_s *ifdesc =
+                (FAR struct usb_ifdesc_s *)configdesc;
+
+              uinfo("Interface descriptor\n");
+              DEBUGASSERT(remaining >= USB_SIZEOF_IFDESC);
+
+              /* Did we already find what we needed
+               * from a preceding interface?
+               */
+
+              if ((found & USBHOST_RQDFOUND) == USBHOST_RQDFOUND)
+                {
+                  /* Yes.. then break out of the loop and use the preceding
+                   * interface.
+                   */
+
+                  done       = true;
+                }
+              else
+                {
+                  /* Otherwise, save the interface number and discard any
+                   * endpoints previously found
+                   */
+
+                  priv->ifno = ifdesc->ifno;
+                  found      = USBHOST_IFFOUND;
+                }
+            }
+            break;
+
+          /* HID descriptor */
+
+          case USBHID_DESCTYPE_HID:
             uinfo("HID descriptor\n");
             break;
 
-        /* Endpoint descriptor.  We expect one or two interrupt endpoints,
-         * a required IN endpoint and an optional OUT endpoint.
-         */
-
-        case USB_DESC_TYPE_ENDPOINT:
-          {
-            FAR struct usb_epdesc_s *epdesc =
-              (FAR struct usb_epdesc_s *)configdesc;
-
-            uinfo("Endpoint descriptor\n");
-            DEBUGASSERT(remaining >= USB_SIZEOF_EPDESC);
-
-            /* Check for an interrupt endpoint. */
-
-            if ((epdesc->attr & USB_EP_ATTR_XFERTYPE_MASK) ==
-                USB_EP_ATTR_XFER_INT)
-              {
-                /* Yes.. it is a interrupt endpoint.  IN or OUT? */
-
-                if (USB_ISEPOUT(epdesc->addr))
-                  {
-                    /* It is an interrupt OUT endpoint.  There not be more
-                     * than one interrupt OUT endpoint.
-                     */
-
-                    if ((found & USBHOST_EPOUTFOUND) != 0)
-                      {
-                        /* Oops.. more than one endpoint.  We don't know what
-                         * to do with this.
-                         */
-
-                        return -EINVAL;
-                      }
-
-                    found |= USBHOST_EPOUTFOUND;
-
-                    /* Save the interrupt OUT endpoint information */
-
-                    epoutdesc.hport        = hport;
-                    epoutdesc.addr         = epdesc->addr &
-                                             USB_EP_ADDR_NUMBER_MASK;
-                    epoutdesc.in           = false;
-                    epoutdesc.xfrtype      = USB_EP_ATTR_XFER_INT;
-                    epoutdesc.interval     = epdesc->interval;
-                    epoutdesc.mxpacketsize =
-                      usbhost_getle16(epdesc->mxpacketsize);
-
-                    uinfo("Interrupt OUT EP addr:%d mxpacketsize:%d\n",
-                          epoutdesc.addr, epoutdesc.mxpacketsize);
-                  }
-                else
-                  {
-                    /* It is an interrupt IN endpoint.  There should be only
-                     * one interrupt IN endpoint.
-                     */
-
-                    if ((found & USBHOST_EPINFOUND) != 0)
-                      {
-                        /* Oops.. more than one endpoint.  We don't know what
-                         * to do with this.
-                         */
-
-                        return -EINVAL;
-                      }
-
-                    found |= USBHOST_EPINFOUND;
-
-                    /* Save the interrupt IN endpoint information */
-
-                    epindesc.hport        = hport;
-                    epindesc.addr         = epdesc->addr &
-                                            USB_EP_ADDR_NUMBER_MASK;
-                    epindesc.in           = 1;
-                    epindesc.xfrtype      = USB_EP_ATTR_XFER_INT;
-                    epindesc.interval     = epdesc->interval;
-                    epindesc.mxpacketsize =
-                      usbhost_getle16(epdesc->mxpacketsize);
-
-                    uinfo("Interrupt IN EP addr:%d mxpacketsize:%d\n",
-                          epindesc.addr, epindesc.mxpacketsize);
-                  }
-              }
-          }
-          break;
-
-        /* Other descriptors are just ignored for now */
-
-        default:
-          uinfo("Other descriptor: %d\n", desc->type);
-          break;
+          /* Endpoint descriptor.  We expect one or two interrupt endpoints,
+           * a required IN endpoint and an optional OUT endpoint.
+           */
+
+          case USB_DESC_TYPE_ENDPOINT:
+            {
+              FAR struct usb_epdesc_s *epdesc =
+                (FAR struct usb_epdesc_s *)configdesc;
+
+              uinfo("Endpoint descriptor\n");
+              DEBUGASSERT(remaining >= USB_SIZEOF_EPDESC);
+
+              /* Check for an interrupt endpoint. */
+
+              if ((epdesc->attr & USB_EP_ATTR_XFERTYPE_MASK) ==
+                  USB_EP_ATTR_XFER_INT)
+                {
+                  /* Yes.. it is a interrupt endpoint.  IN or OUT? */
+
+                  if (USB_ISEPOUT(epdesc->addr))
+                    {
+                      /* It is an interrupt OUT endpoint.  There not be more
+                       * than one interrupt OUT endpoint.
+                       */
+
+                      if ((found & USBHOST_EPOUTFOUND) != 0)
+                        {
+                          /* Oops.. more than one endpoint.  We don't know
+                           * what to do with this.
+                           */
+
+                          return -EINVAL;
+                        }
+
+                      found |= USBHOST_EPOUTFOUND;
+
+                      /* Save the interrupt OUT endpoint information */
+
+                      epoutdesc.hport        = hport;
+                      epoutdesc.addr         = epdesc->addr &
+                                               USB_EP_ADDR_NUMBER_MASK;
+                      epoutdesc.in           = false;
+                      epoutdesc.xfrtype      = USB_EP_ATTR_XFER_INT;
+                      epoutdesc.interval     = epdesc->interval;
+                      epoutdesc.mxpacketsize =
+                        usbhost_getle16(epdesc->mxpacketsize);
+
+                      uinfo("Interrupt OUT EP addr:%d mxpacketsize:%d\n",
+                            epoutdesc.addr, epoutdesc.mxpacketsize);
+                    }
+                  else
+                    {
+                      /* It is an interrupt IN endpoint.  There should be
+                       * only one interrupt IN endpoint.
+                       */
+
+                      if ((found & USBHOST_EPINFOUND) != 0)
+                        {
+                          /* Oops.. more than one endpoint.  We don't know
+                           * what to do with this.
+                           */
+
+                          return -EINVAL;
+                        }
+
+                      found |= USBHOST_EPINFOUND;
+
+                      /* Save the interrupt IN endpoint information */
+
+                      epindesc.hport        = hport;
+                      epindesc.addr         = epdesc->addr &
+                                              USB_EP_ADDR_NUMBER_MASK;
+                      epindesc.in           = 1;
+                      epindesc.xfrtype      = USB_EP_ATTR_XFER_INT;
+                      epindesc.interval     = epdesc->interval;
+                      epindesc.mxpacketsize =
+                        usbhost_getle16(epdesc->mxpacketsize);
+
+                      uinfo("Interrupt IN EP addr:%d mxpacketsize:%d\n",
+                            epindesc.addr, epindesc.mxpacketsize);
+                    }
+                }
+            }
+            break;
+
+          /* Other descriptors are just ignored for now */
+
+          default:
+            uinfo("Other descriptor: %d\n", desc->type);
+            break;
         }
 
       /* What we found everything that we are going to find? */
@@ -1791,8 +1822,10 @@ static inline int usbhost_devinit(FAR struct 
usbhost_state_s *priv)
   int ret;
 #ifdef CONFIG_HIDKBD_NOGETREPORT
   FAR struct usbhost_hubport_s *hport;
+
   hport = priv->usbclass.hport;
 #endif
+
   /* Set aside a transfer buffer for exclusive
    * use by the keyboard class driver
    */
@@ -1821,7 +1854,7 @@ static inline int usbhost_devinit(FAR struct 
usbhost_state_s *priv)
   /* Choose the boot protocol */
 
   usbhost_send_request(priv, USB_REQ_DIR_OUT, USBHID_REQUEST_SETPROTOCOL,
-                       0, 0, 0, NULL);
+                       0, priv->ifno, 0, NULL);
 
   /* Initialize the Caps Lock LED */
 
diff --git a/drivers/usbhost/usbhost_hidmouse.c 
b/drivers/usbhost/usbhost_hidmouse.c
index 64a7c1f0443..f2876056022 100644
--- a/drivers/usbhost/usbhost_hidmouse.c
+++ b/drivers/usbhost/usbhost_hidmouse.c
@@ -254,6 +254,7 @@ struct usbhost_state_s
   volatile bool           valid;        /* TRUE: New sample data is available 
*/
   uint8_t                 devno;        /* Minor number in the /dev/mouse[n] 
device */
   uint8_t                 ifno;         /* Mouse interface number */
+  uint8_t                 reportid;     /* Mouse report ID (0: boot protocol) 
*/
   uint8_t                 nwaiters;     /* Number of threads waiting for mouse 
data */
 #ifdef CONFIG_HIDMOUSE_TSCIF
   uint8_t                 id;           /* Current "touch" point ID */
@@ -315,6 +316,8 @@ static bool usbhost_touchscreen(FAR struct usbhost_state_s 
*priv,
 #endif
 static bool usbhost_threshold(FAR struct usbhost_state_s *priv);
 static int usbhost_mouse_poll(int argc, FAR char *argv[]);
+static void usbhost_reportdesc(FAR struct usbhost_state_s *priv,
+                               FAR const uint8_t *desc, int len);
 static int usbhost_sample(FAR struct usbhost_state_s *priv,
                           FAR struct mouse_sample_s *sample);
 static int usbhost_waitsample(FAR struct usbhost_state_s *priv,
@@ -931,6 +934,138 @@ static bool usbhost_threshold(FAR struct usbhost_state_s 
*priv)
 #endif
 }
 
+/****************************************************************************
+ * Name: usbhost_reportdesc
+ *
+ * Description:
+ *   Parse the HID report descriptor and find the report ID of the mouse
+ *   application collection.  The report ID is used only if the X and Y
+ *   fields are 8 bits wide, i.e. the report has the boot protocol layout
+ *   after the report ID.
+ *
+ ****************************************************************************/
+
+static void usbhost_reportdesc(FAR struct usbhost_state_s *priv,
+                               FAR const uint8_t *desc, int len)
+{
+  uint32_t value;
+  uint8_t usagepage = 0;
+  uint8_t usage = 0;
+  uint8_t rptsize = 0;
+  uint8_t xsize = 0;
+  uint8_t reportid = 0;
+  bool inmouse = false;
+  bool hasx = false;
+  int depth = 0;
+  int mousedepth = 0;
+  int size;
+  int i;
+  int j;
+
+  for (i = 0; i < len; i += 1 + size)
+    {
+      /* Long items are not used by mice, just skip them */
+
+      if (desc[i] == 0xfe)
+        {
+          size = i + 1 < len ? desc[i + 1] + 2 : len;
+          continue;
+        }
+
+      size = desc[i] & 0x03;
+      if (size == 3)
+        {
+          size = 4;
+        }
+
+      if (i + size >= len)
+        {
+          break;
+        }
+
+      for (value = 0, j = 0; j < size; j++)
+        {
+          value |= (uint32_t)desc[i + 1 + j] << (8 * j);
+        }
+
+      switch (desc[i] & 0xfc)
+        {
+          case 0x04: /* Usage Page */
+            usagepage = value;
+            break;
+
+          case 0x74: /* Report Size */
+            rptsize = value;
+            break;
+
+          case 0x84: /* Report ID */
+            if (inmouse && reportid == 0)
+              {
+                reportid = value;
+              }
+            break;
+
+          case 0x08: /* Usage */
+            usage = value;
+            if (usagepage == 0x01 && value == 0x30) /* X */
+              {
+                hasx = true;
+              }
+            break;
+
+          case 0xa0: /* Collection */
+            depth++;
+            if (!inmouse && value == 0x01 && usagepage == 0x01 &&
+                usage == 0x02) /* Application, Mouse */
+              {
+                inmouse    = true;
+                mousedepth = depth;
+              }
+
+            usage = 0;
+            break;
+
+          case 0xc0: /* End Collection */
+            if (inmouse && depth == mousedepth)
+              {
+                inmouse = false;
+              }
+
+            depth--;
+            break;
+
+          case 0x80: /* Input */
+            if (inmouse && hasx && xsize == 0)
+              {
+                xsize = rptsize;
+              }
+
+            hasx  = false;
+            usage = 0;
+            break;
+
+          case 0x90: /* Output */
+          case 0xb0: /* Feature */
+            hasx  = false;
+            usage = 0;
+            break;
+
+          default:
+            break;
+        }
+    }
+
+  if (reportid != 0 && xsize != 8)
+    {
+      uwarn("WARNING: Mouse report %d has %d bit X/Y, using boot protocol\n",
+            reportid, xsize);
+      reportid = 0;
+    }
+
+  uinfo("Mouse report ID: %d\n", reportid);
+  priv->reportid = reportid;
+}
+
 /****************************************************************************
  * Name: usbhost_mouse_poll
  *
@@ -1006,11 +1141,11 @@ static int usbhost_mouse_poll(int argc, FAR char 
*argv[])
            * long time).
            */
 
-          uerr("ERROR: DRVR_TRANSFER returned: %d/%u\n",
-               (int)nbytes, nerrors);
-
           if (nbytes != -EAGAIN)
             {
+              uerr("ERROR: DRVR_TRANSFER returned: %d/%u\n",
+                   (int)nbytes, nerrors);
+
               if (++nerrors > 200)
                 {
                   uerr("  Too many errors... aborting: %d\n", nerrors);
@@ -1028,9 +1163,27 @@ static int usbhost_mouse_poll(int argc, FAR char *argv[])
 
           nerrors = 0;
 
+          /* In report protocol the report starts with the report ID:  skip
+           * it and ignore the other reports of the interface.
+           */
+
+          rpt = (FAR struct usbhid_mousereport_s *)priv->tbuffer;
+          if (priv->reportid != 0)
+            {
+              if (nbytes < 4 || priv->tbuffer[0] != priv->reportid)
+                {
+                  rpt = NULL;
+                }
+              else
+                {
+                  rpt = (FAR struct usbhid_mousereport_s *)
+                        &priv->tbuffer[1];
+                }
+            }
+
           /* Ignore the mouse data if no task has opened the driver. */
 
-          if (priv->open)
+          if (priv->open && rpt != NULL)
             {
               /* Get exclusive access to the mouse state data */
 
@@ -1042,10 +1195,6 @@ static int usbhost_mouse_poll(int argc, FAR char *argv[])
                   break;
                 }
 
-              /* Get the HID mouse report */
-
-              rpt = (FAR struct usbhid_mousereport_s *)priv->tbuffer;
-
               /* Get the updated mouse position */
 
               usbhost_position(priv, rpt);
@@ -1586,17 +1735,41 @@ static inline int usbhost_devinit(FAR struct 
usbhost_state_s *priv)
       return ret;
     }
 
+  /* Some devices ignore SET_PROTOCOL(boot) and keep sending report
+   * protocol reports, which start with a report ID.  Read the report
+   * descriptor to find the report ID of the mouse (the transfer buffer is
+   * not in use yet, so it holds the request and the descriptor).
+   */
+
+  ctrlreq           = (FAR struct usb_ctrlreq_s *)priv->tbuffer;
+  ctrlreq->type     = USB_REQ_DIR_IN | USB_REQ_RECIPIENT_INTERFACE;
+  ctrlreq->req      = USB_REQ_GETDESCRIPTOR;
+  ctrlreq->value[0] = 0;
+  ctrlreq->value[1] = USBHID_DESCTYPE_REPORT;
+  ctrlreq->index[0] = priv->ifno;
+  ctrlreq->index[1] = 0;
+  ctrlreq->len[0]   = priv->tbuflen - USB_SIZEOF_CTRLREQ;
+  ctrlreq->len[1]   = 0;
+
+  ret = DRVR_CTRLIN(priv->usbclass.hport->drvr, priv->usbclass.hport->ep0,
+                    ctrlreq, priv->tbuffer + USB_SIZEOF_CTRLREQ);
+  if (ret >= 0)
+    {
+      usbhost_reportdesc(priv, priv->tbuffer + USB_SIZEOF_CTRLREQ,
+                         priv->tbuflen - USB_SIZEOF_CTRLREQ);
+    }
+
   /* The reports are decoded using the fixed boot protocol format, but a HID
    * device starts in report protocol after configuration.  Select the boot
-   * protocol (the transfer buffer is not in use yet, so it holds the
-   * request).
+   * protocol, or the report protocol if the mouse report has a report ID
+   * and the boot protocol layout.
    */
 
   ctrlreq           = (FAR struct usb_ctrlreq_s *)priv->tbuffer;
   ctrlreq->type     = USB_REQ_DIR_OUT | USB_REQ_TYPE_CLASS |
                       USB_REQ_RECIPIENT_INTERFACE;
   ctrlreq->req      = USBHID_REQUEST_SETPROTOCOL;
-  ctrlreq->value[0] = 0;               /* 0 = Boot protocol */
+  ctrlreq->value[0] = priv->reportid != 0 ;  /* 1 = Report */
   ctrlreq->value[1] = 0;
   ctrlreq->index[0] = priv->ifno;
   ctrlreq->index[1] = 0;
@@ -1607,7 +1780,8 @@ static inline int usbhost_devinit(FAR struct 
usbhost_state_s *priv)
                      ctrlreq, NULL);
   if (ret < 0)
     {
-      uwarn("WARNING: SET_PROTOCOL(boot) failed: %d\n", ret);
+      uwarn("WARNING: SET_PROTOCOL(%d) failed: %d\n",
+            ctrlreq->value[0], ret);
     }
 
   /* Increment the reference count.  This will prevent usbhost_destroy() from
diff --git a/drivers/usbhost/usbhost_max3421e.c 
b/drivers/usbhost/usbhost_max3421e.c
index 022c4428f56..fabf5ad6c3a 100644
--- a/drivers/usbhost/usbhost_max3421e.c
+++ b/drivers/usbhost/usbhost_max3421e.c
@@ -115,6 +115,10 @@
 #define MAX3421E_DATANAK_DELAY       SEC2TICK(5) /* 5 seconds in system ticks 
*/
 #define MAX3421E_RETRY_COUNT         5           /* Number of tries before 
giving up */
 
+/* Maximum time to wait for a MAX3421E event (in system ticks) */
+
+#define MAX3421E_INT_TIMEOUT         MSEC2TICK(500)
+
 /* Debug ********************************************************************/
 
 #define TR_FMT1 false
@@ -511,7 +515,7 @@ static inline uint8_t max3421e_int_status(
               FAR struct max3421e_usbhost_s *priv);
 static inline void max3421e_int_clear(FAR struct max3421e_usbhost_s *priv,
               uint8_t irqset);
-static void max3421e_int_wait(FAR struct max3421e_usbhost_s *priv,
+static int max3421e_int_wait(FAR struct max3421e_usbhost_s *priv,
               uint8_t irqset, unsigned int usec);
 
 /* USB host controller operations *******************************************/
@@ -564,7 +568,7 @@ static void max3421e_disconnect(FAR struct usbhost_driver_s 
*drvr,
 
 /* Initialization ***********************************************************/
 
-static void max3421e_busreset(FAR struct max3421e_usbhost_s *priv);
+static int max3421e_busreset(FAR struct max3421e_usbhost_s *priv);
 static int  max3421e_startsof(FAR struct max3421e_usbhost_s *priv);
 
 static inline int max3421e_sw_initialize(FAR struct max3421e_usbhost_s *priv,
@@ -2689,6 +2693,13 @@ static ssize_t max3421e_in_transfer(FAR struct 
max3421e_usbhost_s *priv,
 
       ret = max3421e_chan_wait(priv, chan);
 
+      /* Account for the data received before the transfer ended.  A full
+       * packet is followed by another IN token, so the data of a transfer
+       * that ends with a NAK is valid too.
+       */
+
+      xfrd += priv->xfrd;
+
       /* EAGAIN indicates that the device NAKed the transfer. */
 
       if (ret < 0)
@@ -2789,7 +2800,30 @@ static ssize_t max3421e_in_transfer(FAR struct 
max3421e_usbhost_s *priv,
                    * amount and destroy system throughput?
                    */
 
-                  if (delay > CONFIG_USEC_PER_TICK)
+                  if (chan->eptype == USB_EP_ATTR_XFER_INT)
+                    {
+                      /* Release the host while waiting for the next
+                       * polling interval.  Otherwise an idle interrupt
+                       * endpoint would block the transfers of the other
+                       * endpoints (e.g. the keyboard and the mouse of a
+                       * composite device) until the NAK timeout.
+                       */
+
+                      nxrmutex_unlock(&priv->lock);
+                      nxsched_usleep(delay);
+                      ret = nxrmutex_lock(&priv->lock);
+                      if (ret < 0)
+                        {
+                          return (ssize_t)ret;
+                        }
+
+                      /* Other transfers may have used the shared state */
+
+                      priv->buffer = buffer;
+                      priv->buflen = buflen;
+                      priv->xfrd   = 0;
+                    }
+                  else if (delay > CONFIG_USEC_PER_TICK)
                     {
                       nxsched_usleep(delay - CONFIG_USEC_PER_TICK);
                     }
@@ -2808,13 +2842,11 @@ static ssize_t max3421e_in_transfer(FAR struct 
max3421e_usbhost_s *priv,
         }
       else
         {
-          /* Successfully received another chunk of data... add that to the
-           * running total.  Then continue reading until we read 'buflen'
-           * bytes of data or until the devices NAKs (implying a short
-           * packet).
+          /* The transfer completed:  either all of the requested data was
+           * received or a short packet ended the transfer.
            */
 
-          xfrd += priv->xfrd;
+          break;
         }
     }
 
@@ -3031,7 +3063,13 @@ static int max3421e_connected(FAR struct 
max3421e_usbhost_s *priv)
 
   /* Stop SOF generation and reset the bus */
 
-  max3421e_busreset(priv);
+  ret = max3421e_busreset(priv);
+  if (ret < 0)
+    {
+      usbhost_trace1(MAX3421E_TRACE1_DEVDISCONN7, -ret);
+      return ret;
+    }
+
   nxsched_sleep(1);
 
   /* Check for low- or full-speed and restart SOF generation. */
@@ -3073,30 +3111,62 @@ static void max3421e_disconnected(FAR struct 
max3421e_usbhost_s *priv)
 }
 
 /****************************************************************************
- * Name: max3421e_device_present
+ * Name: max3421e_bussample
  *
  * Description:
- *   Sample the bus and return true if a device is attached (J or K state)
- *   without disturbing the bus with a reset.
+ *   Sample the J/K state of the bus into the HRSL register.
+ *
+ * Returned Value:
+ *   OK on success; -ETIMEDOUT if the sample was not taken.
  *
  * Assumptions:
  *   The SPI bus is locked.
  *
  ****************************************************************************/
 
-static bool max3421e_device_present(FAR struct max3421e_usbhost_s *priv)
+static int max3421e_bussample(FAR struct max3421e_usbhost_s *priv)
 {
-  uint8_t regval;
+  clock_t start;
 
   max3421e_modifyreg(priv, MAX3421E_USBHOST_HCTL, 0,
                      USBHOST_HCTL_BUSSAMPLE);
 
+  start = clock_systime_ticks();
   while ((max3421e_getreg(priv, MAX3421E_USBHOST_HCTL) &
           USBHOST_HCTL_BUSSAMPLE) == 0)
     {
+      if (clock_systime_ticks() - start > MAX3421E_INT_TIMEOUT)
+        {
+          return -ETIMEDOUT;
+        }
+
       nxsched_usleep(5);
     }
 
+  return OK;
+}
+
+/****************************************************************************
+ * Name: max3421e_device_present
+ *
+ * Description:
+ *   Sample the bus and return true if a device is attached (J or K state)
+ *   without disturbing the bus with a reset.
+ *
+ * Assumptions:
+ *   The SPI bus is locked.
+ *
+ ****************************************************************************/
+
+static bool max3421e_device_present(FAR struct max3421e_usbhost_s *priv)
+{
+  uint8_t regval;
+
+  if (max3421e_bussample(priv) < 0)
+    {
+      return false;
+    }
+
   regval = max3421e_getreg(priv, MAX3421E_USBHOST_HRSL);
   regval &= (USBHOST_HRSL_KSTATUS | USBHOST_HRSL_JSTATUS);
 
@@ -3349,24 +3419,30 @@ static inline void max3421e_int_clear(FAR struct 
max3421e_usbhost_s *priv,
   max3421e_putreg(priv, MAX3421E_USBHOST_HIRQ, irqset);
 }
 
-/* Wait until any interrupt from a set of interrupts occurs */
+/* Wait until any interrupt from a set of interrupts occurs.  These events
+ * never come if the MAX3421E oscillator is not running, so give up after
+ * MAX3421E_INT_TIMEOUT.
+ */
 
-static void max3421e_int_wait(FAR struct max3421e_usbhost_s *priv,
-                              uint8_t irqset, unsigned int usec)
+static int max3421e_int_wait(FAR struct max3421e_usbhost_s *priv,
+                             uint8_t irqset, unsigned int usec)
 {
-  uint8_t regval;
+  clock_t start = clock_systime_ticks();
 
-  do
+  while ((max3421e_getreg(priv, MAX3421E_USBHOST_HIRQ) & irqset) == 0)
     {
-      regval  =  max3421e_getreg(priv, MAX3421E_USBHOST_HIRQ);
-      regval &= irqset;
+      if (clock_systime_ticks() - start > MAX3421E_INT_TIMEOUT)
+        {
+          return -ETIMEDOUT;
+        }
 
-      if (regval == 0 && usec > 0)
+      if (usec > 0)
         {
           nxsched_usleep(usec);
         }
     }
-  while (regval == 0);
+
+  return OK;
 }
 
 /****************************************************************************
@@ -3535,7 +3611,13 @@ static int max3421e_getspeed(FAR struct 
max3421e_usbhost_s *priv,
 
   /* Stop SOF generation and reset the host port */
 
-  max3421e_busreset(priv);
+  ret = max3421e_busreset(priv);
+  if (ret < 0)
+    {
+      usbhost_trace1(MAX3421E_TRACE1_DEVDISCONN8, -ret);
+      return ret;
+    }
+
   nxsched_sleep(1);
 
   /* Get the current device speed */
@@ -4135,6 +4217,14 @@ static int max3421e_ctrlin(FAR struct usbhost_driver_s 
*drvr,
               usbhost_trace1(MAX3421E_TRACE1_SENDSTATUS_FAIL, -ret);
             }
 
+          /* A STALL is the final answer of the device, do not retry */
+
+          if (ret == -EPERM)
+            {
+              nxrmutex_unlock(&priv->lock);
+              return ret;
+            }
+
           /* Get the elapsed time (in frames) */
 
           elapsed = clock_systime_ticks() - start;
@@ -4237,6 +4327,14 @@ static int max3421e_ctrlout(FAR struct usbhost_driver_s 
*drvr,
               usbhost_trace1(MAX3421E_TRACE1_RECVSTATUS_FAIL, -ret);
             }
 
+          /* A STALL is the final answer of the device, do not retry */
+
+          if (ret == -EPERM)
+            {
+              nxrmutex_unlock(&priv->lock);
+              return ret;
+            }
+
           /* Get the elapsed time (in frames) */
 
           elapsed = clock_systime_ticks() - start;
@@ -4594,12 +4692,14 @@ static void max3421e_disconnect(FAR struct 
usbhost_driver_s *drvr,
  *   priv -- USB host driver private data structure.
  *
  * Returned Value:
- *   None
+ *   OK on success; -ETIMEDOUT if the reset did not complete.
  *
  ****************************************************************************/
 
-static void max3421e_busreset(FAR struct max3421e_usbhost_s *priv)
+static int max3421e_busreset(FAR struct max3421e_usbhost_s *priv)
 {
+  int ret;
+
   /* Disable the SOF generator */
 
   max3421e_modifyreg(priv, MAX3421E_USBHOST_MODE, USBHOST_MODE_SOFKAENAB, 0);
@@ -4611,11 +4711,12 @@ static void max3421e_busreset(FAR struct 
max3421e_usbhost_s *priv)
   /* Perform the reset */
 
   max3421e_modifyreg(priv, MAX3421E_USBHOST_HCTL, 0, USBHOST_HCTL_BUSRST);
-  max3421e_int_wait(priv, USBHOST_HIRQ_BUSEVENTIRQ, 250);
+  ret = max3421e_int_wait(priv, USBHOST_HIRQ_BUSEVENTIRQ, 250);
 
   /* The reset (SE0) is seen as a connection change; discard it */
 
   max3421e_int_clear(priv, USBHOST_HIRQ_CONNIRQ);
+  return ret;
 }
 
 /****************************************************************************
@@ -4628,7 +4729,8 @@ static void max3421e_busreset(FAR struct 
max3421e_usbhost_s *priv)
  *   priv -- USB host driver private data structure.
  *
  * Returned Value:
- *   OK if successfully connect; -ENODEV if not connected.
+ *   OK if successfully connect; -ENODEV if not connected; -ETIMEDOUT if
+ *   the MAX3421E did not respond.
  *
  ****************************************************************************/
 
@@ -4638,6 +4740,7 @@ static int max3421e_startsof(FAR struct 
max3421e_usbhost_s *priv)
   uint8_t setbits;
   uint8_t regval;
   bool lowspeed;
+  int ret;
 
   /* Check if we are already in low- or full-speed mode */
 
@@ -4646,13 +4749,10 @@ static int max3421e_startsof(FAR struct 
max3421e_usbhost_s *priv)
 
   /* Enable SAMPLEBUS */
 
-  max3421e_modifyreg(priv, MAX3421E_USBHOST_HCTL, 0,
-                     USBHOST_HCTL_BUSSAMPLE);
-
-  while ((max3421e_getreg(priv, MAX3421E_USBHOST_HCTL) &
-          USBHOST_HCTL_BUSSAMPLE) == 0)
+  ret = max3421e_bussample(priv);
+  if (ret < 0)
     {
-      nxsched_usleep(5);
+      return ret;
     }
 
   /* Check for low- or full-speed and start SOF (actually already started
@@ -4725,7 +4825,12 @@ static int max3421e_startsof(FAR struct 
max3421e_usbhost_s *priv)
 
   /* Wait for the first SOF received and 20ms has passed */
 
-  max3421e_int_wait(priv, USBHOST_HIRQ_FRAMEIRQ, 0);
+  ret = max3421e_int_wait(priv, USBHOST_HIRQ_FRAMEIRQ, 0);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
   nxsched_usleep(20 * 1000);
   return OK;
 }
@@ -4906,8 +5011,11 @@ static inline int max3421e_hw_initialize(FAR struct 
max3421e_usbhost_s *priv)
     {
       if (i >= 100)
         {
-          uwarn("WARNING: Timeout waiting for OSCOKIRQ\n");
-          break;
+          /* Without the oscillator the USB engine does not run */
+
+          uerr("ERROR: Timeout waiting for OSCOKIRQ\n");
+          max3421e_unlock(priv);
+          return -ENODEV;
         }
 
       nxsched_usleep(1000);

Reply via email to