diff --git a/arch/arm/src/sam34/sam_serial.c b/arch/arm/src/sam34/sam_serial.c index 46df27954a1..36043e9deb8 100644 --- a/arch/arm/src/sam34/sam_serial.c +++ b/arch/arm/src/sam34/sam_serial.c @@ -692,8 +692,8 @@ static void up_disableallints(struct up_dev_s *priv, uint32_t *imr) static int up_setup(struct uart_dev_s *dev) { - struct up_dev_s *priv = (struct up_dev_s *)dev->priv; #ifndef CONFIG_SUPPRESS_UART_CONFIG + struct up_dev_s *priv = (struct up_dev_s *)dev->priv; uint32_t regval; uint32_t imr; diff --git a/arch/arm/src/sam34/sam_udp.c b/arch/arm/src/sam34/sam_udp.c index 6fe76f4f57b..ed2016ea518 100644 --- a/arch/arm/src/sam34/sam_udp.c +++ b/arch/arm/src/sam34/sam_udp.c @@ -305,6 +305,7 @@ struct sam_ep_s uint8_t zlpneeded:1; /* Zero length packet needed at end of transfer */ uint8_t zlpsent:1; /* Zero length packet has been sent */ uint8_t txbusy:1; /* Write request queue is busy (recursion avoidance kludge) */ + uint8_t lastbank:1; /* Last bank we read data from */ }; struct sam_usbdev_s @@ -1188,9 +1189,14 @@ static int sam_req_read(struct sam_usbdev_s *priv, struct sam_ep_s *privep, /* 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 + * FIFO. + * + * Also, we need to remember which bank we read last so the interrupt handler + * can determine the correct bank read sequence for future reads. */ + privep->lastbank = bank; + sam_csr_clrbits(epno, bank ? UDPEP_CSR_RXDATABK1 : UDPEP_CSR_RXDATABK0); /* Complete the transfer immediately and give the data to the class @@ -1873,7 +1879,6 @@ static void sam_ep_bankinterrupt(struct sam_usbdev_s *priv, * transferred from the FIFO. */ - privep->epstate = UDP_EPSTATE_IDLE; (void)sam_req_read(priv, privep, pktsize, bank); } @@ -2020,34 +2025,75 @@ static void sam_ep_interrupt(struct sam_usbdev_s *priv, int epno) } } - /* OUT packet received in data bank 0 */ - if ((csr & UDPEP_CSR_RXDATABK0) != 0) + /* OUT packet received. + * + * OUT packets are received in two banks. The hardware does not provide + * information about which bank has been filled last. Therefore we need to + * keep track about which bank we read last to figure out which bank(s) we + * need to read next. + * + * When we get here either none, one or both banks can be filled with data. + * Depending on which bank we read last and which bank(s) contain data we + * need to correctly sequence the FIFO reads: + * + * case lastbank bk0 bk1 read sequence + * 1. 0 0 0 No data to read + * 2. 0 1 0 Only read bank 0 + * 3. 0 0 1 Only read bank 1 + * 4. 0 1 1 Read bank 1, then read bank 0 + * + * 5. 1 0 0 No data to read + * 6. 1 1 0 Only read bank 0 + * 7. 1 0 1 Only read bank 1 (should not happen) + * 8. 1 1 1 Read bank 0, then read bank 1 + * + * lastbank will be updated in sam_req_read() after the FIFO has been read + * and clear RXDATABKx. + */ + + bool bk0 = (csr & UDPEP_CSR_RXDATABK0) != 0; + bool bk1 = (csr & UDPEP_CSR_RXDATABK1) != 0; + + /* 2. and 6. - Only read bank 0 */ + if (bk0 && !bk1) { usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_RXDATABK0), (uint16_t)csr); - - /* 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); } - - /* OUT packet received in data bank 1 */ - - else if ((csr & UDPEP_CSR_RXDATABK1) != 0) + /* 3. and 7. - Only read bank 1*/ + else if (!bk0 && bk1) { +#ifdef CONFIG_DEBUG + if (privep->lastbank == 1) + { + ulldbg("Unexpected USB RX case.\n"); + } +#endif usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_RXDATABK1), (uint16_t)csr); - DEBUGASSERT(SAM_UDP_NBANKS(epno) > 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); } + else if (bk0 && bk1) + { + /* 4. - Read bank 1, then read bank 0 */ + if (privep->lastbank == 0) + { + usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_RXDATABK1), (uint16_t)csr); + sam_ep_bankinterrupt(priv, privep, csr, 1); + + usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_RXDATABK0), (uint16_t)csr); + sam_ep_bankinterrupt(priv, privep, csr, 0); + } + /* 8. - Read bank 0, then read bank 1 */ + else + { + usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_RXDATABK0), (uint16_t)csr); + sam_ep_bankinterrupt(priv, privep, csr, 0); + + usbtrace(TRACE_INTDECODE(SAM_TRACEINTID_RXDATABK1), (uint16_t)csr); + sam_ep_bankinterrupt(priv, privep, csr, 1); + } + } /* STALL sent */ @@ -2510,6 +2556,7 @@ static void sam_ep_reset(struct sam_usbdev_s *priv, uint8_t epno) privep->zlpneeded = false; privep->zlpsent = false; privep->txbusy = false; + privep->lastbank = 1; } /**************************************************************************** diff --git a/drivers/usbdev/usbdev_strings.c b/drivers/usbdev/usbdev_strings.c index 9dc9afdc407..d8c35189f0a 100644 --- a/drivers/usbdev/usbdev_strings.c +++ b/drivers/usbdev/usbdev_strings.c @@ -177,7 +177,7 @@ const struct trace_msg_t g_usb_trace_strings_clserror[] = TRACE_STR(USBMSC_TRACEERR_CMDREADREADFAIL), TRACE_STR(USBMSC_TRACEERR_CMDREADSUBMIT), TRACE_STR(USBMSC_TRACEERR_CMDREADWRRQEMPTY), - TRACE_STR(USBMSC_TRACEERR_CMDSTATUSRDREQLISTEMPTY), + TRACE_STR(USBMSC_TRACEERR_CMDSTATUSWRREQLISTEMPTY), TRACE_STR(USBMSC_TRACEERR_CMDUNEVIOLATION), TRACE_STR(USBMSC_TRACEERR_CMDWRITERDSUBMIT), TRACE_STR(USBMSC_TRACEERR_CMDWRITERDRQEMPTY), diff --git a/drivers/usbdev/usbmsc_scsi.c b/drivers/usbdev/usbmsc_scsi.c index fd9e55cb6b4..d088312ca94 100644 --- a/drivers/usbdev/usbmsc_scsi.c +++ b/drivers/usbdev/usbmsc_scsi.c @@ -2263,7 +2263,9 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv) * data to be written. */ + irqstate_t flags = enter_critical_section(); privreq = (FAR struct usbmsc_req_s *)sq_remfirst(&priv->rdreqlist); + leave_critical_section(flags); /* If there no request data available, then just return an error. * This will cause us to remain in the CMDWRITE state. When a filled request is @@ -2537,7 +2539,7 @@ static int usbmsc_cmdstatusstate(FAR struct usbmsc_dev_s *priv) if (!privreq) { - usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDSTATUSRDREQLISTEMPTY), 0); + usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDSTATUSWRREQLISTEMPTY), 0); return -ENOMEM; } @@ -2639,8 +2641,8 @@ int usbmsc_scsi_main(int argc, char *argv[]) uinfo("Waiting to be signalled\n"); usbmsc_scsi_lock(priv); priv->thstate = USBMSC_STATE_STARTED; - while ((priv->theventset & USBMSC_EVENT_READY) != 0 && - (priv->theventset & USBMSC_EVENT_TERMINATEREQUEST) != 0) + while ((priv->theventset & USBMSC_EVENT_READY) == 0 && + (priv->theventset & USBMSC_EVENT_TERMINATEREQUEST) == 0) { usbmsc_scsi_wait(priv); } diff --git a/include/nuttx/usb/usbdev_trace.h b/include/nuttx/usb/usbdev_trace.h index 4ef4e170e11..55915caee83 100644 --- a/include/nuttx/usb/usbdev_trace.h +++ b/include/nuttx/usb/usbdev_trace.h @@ -295,7 +295,7 @@ #define USBMSC_TRACEERR_CMDREADREADFAIL 0x0094 #define USBMSC_TRACEERR_CMDREADSUBMIT 0x0095 #define USBMSC_TRACEERR_CMDREADWRRQEMPTY 0x0096 -#define USBMSC_TRACEERR_CMDSTATUSRDREQLISTEMPTY 0x0097 +#define USBMSC_TRACEERR_CMDSTATUSWRREQLISTEMPTY 0x0097 #define USBMSC_TRACEERR_CMDUNEVIOLATION 0x0098 #define USBMSC_TRACEERR_CMDWRITERDSUBMIT 0x0099 #define USBMSC_TRACEERR_CMDWRITERDRQEMPTY 0x009a