The PIC32 USB driver (finally) works the the Mass Storage Class

git-svn-id: https://nuttx.svn.sourceforge.net/svnroot/nuttx/trunk@4496 7fd9a85b-ad96-42d3-883c-3090e2eb8679

nuttx/ChangeLog

@@ -2577,5 +2577,5 @@
 	  CAN2 clocking must be enabled.
 	* arch/mips/src/pic32mx/picm32mx-usbdev.c:  Several stall-related fixes so that
 	  the USB device driver can used the the mass storage class (which does a LOT
-	  of stalling as part of its normal protocol).  I suspect that there are still
-	  outstanding issues with the USB driver and stalling.
+	  of stalling as part of its normal protocol).  The PIC32 USB Mass Storage
+	  device now seems functional (but has not been tested exhaustively).

nuttx/TODO

@@ -35,7 +35,7 @@ nuttx/
   (3)  AVR (arch/avr)
   (0)  Intel x86 (arch/x86)
   (4)  8051 / MCS51 (arch/8051/)
-  (1)  MIPS/PIC32 (arch/mips)
+  (0)  MIPS/PIC32 (arch/mips)
   (1)  Hitachi/Renesas SH-1 (arch/sh/src/sh1)
   (4)  Renesas M16C/26 (arch/sh/src/m16c)
  (10)  z80/z8/ez80 (arch/z80/)
@@ -1256,15 +1256,6 @@ o 8051 / MCS51 (arch/8051/)
 o MIPS (arch/mips)
   ^^^^^^^^^^^^^^^^
 
-  Title:       PIC32MX USB MASS STORAGE
-  Description: A USB device-side driver has been written for the PIC3MX and
-               is partially tested.  It does not, however, seem to work with the
-               mass storage device.  I believe that these are timing-related
-               errors in how endpoint stalls are handled since the mass storage
-               protocol depends on stalls to indicate the end-of-data.
-  Status:      Open
-  Priority:    Medium
-
 o Hitachi/Renesas SH-1 (arch/sh/src/sh1)
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

nuttx/arch/mips/src/pic32mx/pic32mx-usbdev.c

@@ -87,6 +87,14 @@
 #  undef CONFIG_PIC32MX_USBDEV_BDTDEBUG
 #endif
 
+/* Disable this logic because it is buggy.  It works most of the time but
+ * has some lurking issues that keep this higher performance solution from
+ * being usable.
+ */
+
+#undef CONFIG_USBDEV_NOWRITEAHEAD
+#define CONFIG_USBDEV_NOWRITEAHEAD 1
+
 /* Interrupts ***************************************************************/
 /* Initial interrupt sets */
 
@@ -353,7 +361,11 @@ union wb_u
 struct pic32mx_req_s
 {
   struct usbdev_req_s   req;                 /* Standard USB request */
+#ifdef CONFIG_USBDEV_NOWRITEAHEAD
+  uint16_t              inflight[1];         /* The number of bytes "in-flight" */
+#else
   uint16_t              inflight[2];         /* The number of bytes "in-flight" */
+#endif
   struct pic32mx_req_s *flink;               /* Supports a singly linked list */
 };
 
@@ -892,9 +904,14 @@ static void pic32mx_wrcomplete(struct pic32mx_usbdev_s *priv,
   bdtin = privep->bdtin;
   epno   = USB_EPNO(privep->ep.eplog);
 
+#ifdef CONFIG_USBDEV_NOWRITEAHEAD
+  ullvdbg("EP%d: len=%d xfrd=%d inflight=%d\n",
+          epno, privreq->req.len, privreq->req.xfrd, privreq->inflight[0]);
+#else
   ullvdbg("EP%d: len=%d xfrd=%d inflight={%d, %d}\n",
           epno, privreq->req.len, privreq->req.xfrd,
           privreq->inflight[0], privreq->inflight[1]);
+#endif
   bdtdbg("EP%d BDT IN [%p] {%08x, %08x}\n",
          epno, bdtin, bdtin->status, bdtin->addr);
 
@@ -922,8 +939,12 @@ static void pic32mx_wrcomplete(struct pic32mx_usbdev_s *priv,
   /* Update the number of bytes transferred. */
    
   privreq->req.xfrd   += privreq->inflight[0];
+#ifdef CONFIG_USBDEV_NOWRITEAHEAD
+  privreq->inflight[0] = 0;
+#else
   privreq->inflight[0] = privreq->inflight[1];
   privreq->inflight[1] = 0;
+#endif
   bytesleft            = privreq->req.len - privreq->req.xfrd;
 
   /* If all of the bytes were sent (bytesleft == 0) and no NULL packet is
@@ -1001,8 +1022,9 @@ static void pic32mx_rqrestart(int argc, uint32_t arg1, ...)
 
               privreq->req.xfrd    = 0;
               privreq->inflight[0] = 0;
+#ifdef CONFIG_USBDEV_NOWRITEAHEAD
               privreq->inflight[1] = 0;
-
+#endif
               (void)pic32mx_wrrequest(priv, privep);
             }
         }
@@ -1078,6 +1100,13 @@ static int pic32mx_wrstart(struct pic32mx_usbdev_s *priv,
 
   if (bdt->status || bdt->addr)
     {
+#ifdef CONFIG_USBDEV_NOWRITEAHEAD
+      /* The current BDT is not available and write ahead is disabled.  There
+       * is nothing we can do now.  Return -EBUSY to indicate this condition.
+       */
+
+      return -EBUSY;
+#else
       /* The current BDT is not available, check the other BDT */
 
       volatile struct usbotg_bdtentry_s *otherbdt;
@@ -1102,6 +1131,7 @@ static int pic32mx_wrstart(struct pic32mx_usbdev_s *priv,
 
       bdt   = otherbdt;
       index = 1;
+#endif
     }
 
   /* A BDT is available.  Which request should we be operating on?  The last
@@ -1124,8 +1154,9 @@ static int pic32mx_wrstart(struct pic32mx_usbdev_s *priv,
        * because we know that there is a BDT availalbe.
        */
 
+#ifdef CONFIG_USBDEV_NOWRITEAHEAD
       DEBUGASSERT(privreq->inflight[1] == 0);
-
+#endif
       /* Has the transfer been initiated for all of the bytes? */
 
       if (bytesleft > privreq->inflight[0])
@@ -1246,6 +1277,7 @@ static int pic32mx_wrrequest(struct pic32mx_usbdev_s *priv, struct pic32mx_ep_s
    */
 
   ret = pic32mx_wrstart(priv, privep);
+#ifndef CONFIG_USBDEV_NOWRITEAHEAD
   if (ret == OK)
     {
       /* Note:  We need to return the error condition only if nothing was
@@ -1254,6 +1286,7 @@ static int pic32mx_wrrequest(struct pic32mx_usbdev_s *priv, struct pic32mx_ep_s
 
       (void)pic32mx_wrstart(priv, privep);
     }
+#endif
 
   /* We return OK to indicate that a write request is still in progress */
 

nuttx/configs/pcblogic-pic32mx/nsh/defconfig

@@ -731,12 +731,12 @@ CONFIG_PL2303_TXBUFSIZE=512
 #
 CONFIG_USBMSC=n
 CONFIG_USBMSC_EP0MAXPACKET=64
-CONFIG_USBMSC_EPBULKOUT=2
-CONFIG_USBMSC_EPBULKIN=5
-CONFIG_USBMSC_NRDREQS=2
+CONFIG_USBMSC_EPBULKOUT=1
+CONFIG_USBMSC_EPBULKIN=2
+CONFIG_USBMSC_NRDREQS=8
 CONFIG_USBMSC_NWRREQS=2
 CONFIG_USBMSC_BULKINREQLEN=256
-CONFIG_USBMSC_BULKOUTREQLEN=256
+CONFIG_USBMSC_BULKOUTREQLEN=64
 CONFIG_USBMSC_VENDORID=0x584e
 CONFIG_USBMSC_VENDORSTR="NuttX"
 CONFIG_USBMSC_PRODUCTID=0x5342

nuttx/configs/pcblogic-pic32mx/ostest/defconfig

@@ -672,12 +672,12 @@ CONFIG_PL2303_TXBUFSIZE=512
 #
 CONFIG_USBMSC=n
 CONFIG_USBMSC_EP0MAXPACKET=64
-CONFIG_USBMSC_EPBULKOUT=2
-CONFIG_USBMSC_EPBULKIN=5
-CONFIG_USBMSC_NRDREQS=2
+CONFIG_USBMSC_EPBULKOUT=1
+CONFIG_USBMSC_EPBULKIN=2
+CONFIG_USBMSC_NRDREQS=8
 CONFIG_USBMSC_NWRREQS=2
 CONFIG_USBMSC_BULKINREQLEN=256
-CONFIG_USBMSC_BULKOUTREQLEN=256
+CONFIG_USBMSC_BULKOUTREQLEN=64
 CONFIG_USBMSC_VENDORID=0x584e
 CONFIG_USBMSC_VENDORSTR="NuttX"
 CONFIG_USBMSC_PRODUCTID=0x5342

nuttx/configs/pic32-starterkit/README.txt

@@ -1167,10 +1167,8 @@ Where <subdir> is one of the following:
 
       nsh> msconn
 
-    NOTE:  This modification is experimental and does not yet
-    work properly!  I can only occasionally get Windows to mount
-    the RAM disk.  I think there are still a few lurking bugs in
-    USB device driver -- probably because the required MSC stall
-    handling. However, this configuration is worth remembering
-    for future USB MSC testing.
+    NOTE:  This modification should be considered experimental.  IN the
+    little testing I have done with it, it appears functional.  But the
+    logic has not been stressed and there could still be lurking issues.
+ 
 

nuttx/configs/pic32-starterkit/nsh/defconfig

@@ -985,10 +985,10 @@ CONFIG_USBMSC=n
 CONFIG_USBMSC_EP0MAXPACKET=64
 CONFIG_USBMSC_EPBULKOUT=1
 CONFIG_USBMSC_EPBULKIN=2
-CONFIG_USBMSC_NRDREQS=2
+CONFIG_USBMSC_NRDREQS=8
 CONFIG_USBMSC_NWRREQS=2
 CONFIG_USBMSC_BULKINREQLEN=256
-CONFIG_USBMSC_BULKOUTREQLEN=256
+CONFIG_USBMSC_BULKOUTREQLEN=64
 CONFIG_USBMSC_VENDORID=0x584e
 CONFIG_USBMSC_VENDORSTR="NuttX"
 CONFIG_USBMSC_PRODUCTID=0x5342

nuttx/configs/pic32-starterkit/nsh2/defconfig

@@ -985,10 +985,10 @@ CONFIG_USBMSC=n
 CONFIG_USBMSC_EP0MAXPACKET=64
 CONFIG_USBMSC_EPBULKOUT=1
 CONFIG_USBMSC_EPBULKIN=2
-CONFIG_USBMSC_NRDREQS=2
+CONFIG_USBMSC_NRDREQS=8
 CONFIG_USBMSC_NWRREQS=2
 CONFIG_USBMSC_BULKINREQLEN=256
-CONFIG_USBMSC_BULKOUTREQLEN=256
+CONFIG_USBMSC_BULKOUTREQLEN=64
 CONFIG_USBMSC_VENDORID=0x584e
 CONFIG_USBMSC_VENDORSTR="NuttX"
 CONFIG_USBMSC_PRODUCTID=0x5342

nuttx/configs/pic32-starterkit/ostest/defconfig

@@ -987,10 +987,10 @@ CONFIG_USBMSC=n
 CONFIG_USBMSC_EP0MAXPACKET=64
 CONFIG_USBMSC_EPBULKOUT=1
 CONFIG_USBMSC_EPBULKIN=2
-CONFIG_USBMSC_NRDREQS=2
+CONFIG_USBMSC_NRDREQS=8
 CONFIG_USBMSC_NWRREQS=2
 CONFIG_USBMSC_BULKINREQLEN=256
-CONFIG_USBMSC_BULKOUTREQLEN=256
+CONFIG_USBMSC_BULKOUTREQLEN=64
 CONFIG_USBMSC_VENDORID=0x584e
 CONFIG_USBMSC_VENDORSTR="NuttX"
 CONFIG_USBMSC_PRODUCTID=0x5342

nuttx/configs/sure-pic32mx/nsh/defconfig

@@ -814,12 +814,12 @@ CONFIG_CDCACM_CONSOLE=n
 #
 CONFIG_USBMSC=n
 CONFIG_USBMSC_EP0MAXPACKET=64
-CONFIG_USBMSC_EPBULKOUT=2
-CONFIG_USBMSC_EPBULKIN=5
-CONFIG_USBMSC_NRDREQS=2
+CONFIG_USBMSC_EPBULKOUT=1
+CONFIG_USBMSC_EPBULKIN=2
+CONFIG_USBMSC_NRDREQS=8
 CONFIG_USBMSC_NWRREQS=2
 CONFIG_USBMSC_BULKINREQLEN=256
-CONFIG_USBMSC_BULKOUTREQLEN=256
+CONFIG_USBMSC_BULKOUTREQLEN=64
 CONFIG_USBMSC_VENDORID=0x584e
 CONFIG_USBMSC_VENDORSTR="NuttX"
 CONFIG_USBMSC_PRODUCTID=0x5342

nuttx/configs/sure-pic32mx/ostest/defconfig

@@ -672,12 +672,12 @@ CONFIG_PL2303_TXBUFSIZE=512
 #
 CONFIG_USBMSC=n
 CONFIG_USBMSC_EP0MAXPACKET=64
-CONFIG_USBMSC_EPBULKOUT=2
-CONFIG_USBMSC_EPBULKIN=5
-CONFIG_USBMSC_NRDREQS=2
+CONFIG_USBMSC_EPBULKOUT=1
+CONFIG_USBMSC_EPBULKIN=2
+CONFIG_USBMSC_NRDREQS=8
 CONFIG_USBMSC_NWRREQS=2
 CONFIG_USBMSC_BULKINREQLEN=256
-CONFIG_USBMSC_BULKOUTREQLEN=256
+CONFIG_USBMSC_BULKOUTREQLEN=64
 CONFIG_USBMSC_VENDORID=0x584e
 CONFIG_USBMSC_VENDORSTR="NuttX"
 CONFIG_USBMSC_PRODUCTID=0x5342

nuttx/configs/sure-pic32mx/usbnsh/defconfig

@@ -812,12 +812,12 @@ CONFIG_CDCACM_CONSOLE=y
 #
 CONFIG_USBMSC=n
 CONFIG_USBMSC_EP0MAXPACKET=64
-CONFIG_USBMSC_EPBULKOUT=2
-CONFIG_USBMSC_EPBULKIN=5
-CONFIG_USBMSC_NRDREQS=2
+CONFIG_USBMSC_EPBULKOUT=1
+CONFIG_USBMSC_EPBULKIN=2
+CONFIG_USBMSC_NRDREQS=8
 CONFIG_USBMSC_NWRREQS=2
 CONFIG_USBMSC_BULKINREQLEN=256
-CONFIG_USBMSC_BULKOUTREQLEN=256
+CONFIG_USBMSC_BULKOUTREQLEN=64
 CONFIG_USBMSC_VENDORID=0x584e
 CONFIG_USBMSC_VENDORSTR="NuttX"
 CONFIG_USBMSC_PRODUCTID=0x5342

nuttx/drivers/usbdev/usbmsc_scsi.c

@@ -2172,25 +2172,25 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
        * to the block driver OR all of the request data has been transferred.
        */
 
-     while (priv->nreqbytes > 0 && priv->u.xfrlen > 0)
-       {
-         /* Copy the data received in the read request into the sector I/O buffer */
+      while (priv->nreqbytes > 0 && priv->u.xfrlen > 0)
+        {
+          /* Copy the data received in the read request into the sector I/O buffer */
 
-         src  = &req->buf[xfrd - priv->nreqbytes];
-         dest = &priv->iobuffer[priv->nsectbytes];
+          src  = &req->buf[xfrd - priv->nreqbytes];
+          dest = &priv->iobuffer[priv->nsectbytes];
 
-         nbytes = MIN(lun->sectorsize - priv->nsectbytes, priv->nreqbytes);
+          nbytes = MIN(lun->sectorsize - priv->nsectbytes, priv->nreqbytes);
 
-         /* Copy the data from the sector buffer to the USB request and update counts */
+          /* Copy the data from the sector buffer to the USB request and update counts */
 
-         memcpy(dest, src, nbytes);
-         priv->nsectbytes += nbytes;
-         priv->nreqbytes  -= nbytes;
+          memcpy(dest, src, nbytes);
+          priv->nsectbytes += nbytes;
+          priv->nreqbytes  -= nbytes;
 
-         /* Is the I/O buffer full? */
+          /* Is the I/O buffer full? */
 
-         if (priv->nsectbytes >= lun->sectorsize)
-           {
+          if (priv->nsectbytes >= lun->sectorsize)
+            {
               /* Yes.. Write the next sector */
 
               nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer, priv->sector, 1);
@@ -2202,17 +2202,17 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
                   goto errout;
                 }
 
-             priv->nsectbytes = 0;
-             priv->residue   -= lun->sectorsize;
-             priv->u.xfrlen--;
-             priv->sector++;
-           }
-       }
+              priv->nsectbytes = 0;
+              priv->residue   -= lun->sectorsize;
+              priv->u.xfrlen--;
+              priv->sector++;
+            }
+        }
 
-     /* In either case, we are finished with this read request and can return it
-      * to the endpoint.  Then we will go back to the top of the top and attempt
-      * to get the next read request.
-      */
+      /* In either case, we are finished with this read request and can return it
+       * to the endpoint.  Then we will go back to the top of the top and attempt
+       * to get the next read request.
+       */
 
       req->len      = CONFIG_USBMSC_BULKOUTREQLEN;
       req->priv     = privreq;