diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index 7c5df03d0..48efd5a46 100644 --- a/nuttx/ChangeLog +++ b/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). diff --git a/nuttx/TODO b/nuttx/TODO index 7ceb72698..d20309ae1 100644 --- a/nuttx/TODO +++ b/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) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/nuttx/arch/mips/src/pic32mx/pic32mx-usbdev.c b/nuttx/arch/mips/src/pic32mx/pic32mx-usbdev.c index e5d5adfad..9998ca442 100644 --- a/nuttx/arch/mips/src/pic32mx/pic32mx-usbdev.c +++ b/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 */ diff --git a/nuttx/configs/pcblogic-pic32mx/nsh/defconfig b/nuttx/configs/pcblogic-pic32mx/nsh/defconfig index 877c347a0..f47b75889 100644 --- a/nuttx/configs/pcblogic-pic32mx/nsh/defconfig +++ b/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 diff --git a/nuttx/configs/pcblogic-pic32mx/ostest/defconfig b/nuttx/configs/pcblogic-pic32mx/ostest/defconfig index 0e30a7fe8..a8fc2c630 100644 --- a/nuttx/configs/pcblogic-pic32mx/ostest/defconfig +++ b/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 diff --git a/nuttx/configs/pic32-starterkit/README.txt b/nuttx/configs/pic32-starterkit/README.txt index 7ab730ba8..fb693f54e 100644 --- a/nuttx/configs/pic32-starterkit/README.txt +++ b/nuttx/configs/pic32-starterkit/README.txt @@ -1167,10 +1167,8 @@ Where 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. + diff --git a/nuttx/configs/pic32-starterkit/nsh/defconfig b/nuttx/configs/pic32-starterkit/nsh/defconfig index 6746fc3b5..2c39be262 100644 --- a/nuttx/configs/pic32-starterkit/nsh/defconfig +++ b/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 diff --git a/nuttx/configs/pic32-starterkit/nsh2/defconfig b/nuttx/configs/pic32-starterkit/nsh2/defconfig index 342a7d349..d2734561f 100644 --- a/nuttx/configs/pic32-starterkit/nsh2/defconfig +++ b/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 diff --git a/nuttx/configs/pic32-starterkit/ostest/defconfig b/nuttx/configs/pic32-starterkit/ostest/defconfig index 19a8376c2..c1a33b13e 100644 --- a/nuttx/configs/pic32-starterkit/ostest/defconfig +++ b/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 diff --git a/nuttx/configs/sure-pic32mx/nsh/defconfig b/nuttx/configs/sure-pic32mx/nsh/defconfig index d87c5680a..209dac322 100644 --- a/nuttx/configs/sure-pic32mx/nsh/defconfig +++ b/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 diff --git a/nuttx/configs/sure-pic32mx/ostest/defconfig b/nuttx/configs/sure-pic32mx/ostest/defconfig index a214a4f83..b674cf6fa 100644 --- a/nuttx/configs/sure-pic32mx/ostest/defconfig +++ b/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 diff --git a/nuttx/configs/sure-pic32mx/usbnsh/defconfig b/nuttx/configs/sure-pic32mx/usbnsh/defconfig index ab7b6636c..07c4a199e 100644 --- a/nuttx/configs/sure-pic32mx/usbnsh/defconfig +++ b/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 diff --git a/nuttx/drivers/usbdev/usbmsc_scsi.c b/nuttx/drivers/usbdev/usbmsc_scsi.c index 54fc22795..fa443497f 100644 --- a/nuttx/drivers/usbdev/usbmsc_scsi.c +++ b/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;