From 293b5d7723729239940f7048b6c8baf552722cfc Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sat, 22 Mar 2014 17:30:17 -0600 Subject: [PATCH] SAM4E-EK UDP: prevent some bad recursive behavior --- arch/arm/src/sam34/sam_udp.c | 64 +++++++++++++++++++----------------- configs/sam4e-ek/README.txt | 12 ++++--- 2 files changed, 42 insertions(+), 34 deletions(-) diff --git a/arch/arm/src/sam34/sam_udp.c b/arch/arm/src/sam34/sam_udp.c index e4b95a0993..13480e9c04 100644 --- a/arch/arm/src/sam34/sam_udp.c +++ b/arch/arm/src/sam34/sam_udp.c @@ -303,6 +303,7 @@ struct sam_ep_s uint8_t halted:1; /* true: Endpoint feature halted */ uint8_t zlpneeded:1; /* Zero length packet needed at end of transfer */ uint8_t zlpsent:1; /* Zero length packet has been sent */ + uint8_t wqbusy:1; /* Write request queue is busy (recursion avoidance kludge) */ }; struct sam_usbdev_s @@ -851,7 +852,7 @@ static void sam_req_wrsetup(struct sam_usbdev_s *priv, /* Get the number of bytes remaining to be sent. */ - DEBUGASSERT(privreq->req.xfrd <= privreq->req.len); + DEBUGASSERT(privreq->req.xfrd < privreq->req.len); nbytes = privreq->req.len - privreq->req.xfrd; /* Either send the maxpacketsize or all of the remaining data in @@ -868,24 +869,19 @@ static void sam_req_wrsetup(struct sam_usbdev_s *priv, privreq->inflight = nbytes; usbtrace(TRACE_WRITE(USB_EPNO(privep->ep.eplog)), nbytes); - /* Check if we are sending a zero length packet */ + /* The new buffer pointer is the start of the buffer plus the number of + * bytes successfully transferred plus the number of bytes previously + * "in-flight". + */ - if (nbytes > 0) + buf = privreq->req.buf + privreq->req.xfrd; + + /* Write packet in the FIFO buffer */ + + fifo = (volatile uint32_t *)SAM_UDPEP_FDR(epno); + for (; nbytes; nbytes--) { - /* The new buffer pointer is the start of the buffer plus the number of - * bytes successfully transferred plus the number of bytes previously - * "in-flight". - */ - - buf = privreq->req.buf + privreq->req.xfrd; - - /* Write packet in the FIFO buffer */ - - fifo = (volatile uint32_t *)SAM_UDPEP_FDR(epno); - for (; nbytes; nbytes--) - { - *fifo = (uint32_t)(*buf++); - } + *fifo = (uint32_t)(*buf++); } /* Indicate that there we are in the sending state (even if this is a @@ -1044,20 +1040,27 @@ static int sam_req_write(struct sam_usbdev_s *priv, struct sam_ep_s *privep) } /* If all of the bytes were sent (including any final zero length - * packet) then we are finished with the request buffer), then we can - * return the request buffer to the class driver. The transfer may - * not finished yet, however. There may still be bytes in flight. - * The transfer is truly finished when we are called again and the - * request buffer is empty. + * packet) then we are finished with the request buffer and we can + * return the request buffer to the class driver. The state will + * remain IDLE only if nothing else was put in flight. + * + * Note that we will then loop to check to check the next queued + * write request. */ - if (privreq->req.len >= privreq->req.xfrd && - privep->epstate == UDP_EPSTATE_IDLE) + if (privep->epstate == UDP_EPSTATE_IDLE) { - /* Return the write request to the class driver */ + /* Return the write request to the class driver. Set the wqbusy + * bit to prevent being called recursively from any new submission + * generated by returning the write request. + */ usbtrace(TRACE_COMPLETE(epno), privreq->req.xfrd); + DEBUGASSERT(privreq->req.len == privreq->req.xfrd); + + privep->wqbusy = true; sam_req_complete(privep, OK); + privep->wqbusy = false; } } @@ -2472,6 +2475,7 @@ static void sam_ep_reset(struct sam_usbdev_s *priv, uint8_t epno) privep->halted = false; privep->zlpneeded = false; privep->zlpsent = false; + privep->wqbusy = false; } /**************************************************************************** @@ -3078,9 +3082,6 @@ static int sam_ep_submit(struct usbdev_ep_s *ep, struct usbdev_req_s *req) epno = USB_EPNO(ep->eplog); req->result = -EINPROGRESS; req->xfrd = 0; - privreq->inflight = 0; - privep->zlpneeded = false; - privep->zlpsent = false; flags = irqsave(); /* Handle IN (device-to-host) requests. NOTE: If the class device is @@ -3111,9 +3112,11 @@ static int sam_ep_submit(struct usbdev_ep_s *ep, struct usbdev_req_s *req) sam_req_enqueue(&privep->reqq, privreq); usbtrace(TRACE_INREQQUEUED(epno), req->len); - /* If the IN endpoint is IDLE, then transfer the data now */ + /* If the IN endpoint is IDLE and there is not write queue + * processing in progress, then transfer the data now. + */ - if (privep->epstate == UDP_EPSTATE_IDLE) + if (privep->epstate == UDP_EPSTATE_IDLE && !privep->wqbusy) { ret = sam_req_write(priv, privep); } @@ -3561,6 +3564,7 @@ static void sam_reset(struct sam_usbdev_s *priv) privep->halted = false; privep->zlpneeded = false; privep->zlpsent = false; + privep->wqbusy = false; } /* Re-configure the USB controller in its initial, unconnected state */ diff --git a/configs/sam4e-ek/README.txt b/configs/sam4e-ek/README.txt index b35c38f665..68b250c0ee 100644 --- a/configs/sam4e-ek/README.txt +++ b/configs/sam4e-ek/README.txt @@ -276,10 +276,11 @@ Loading Code into SRAM with J-Link J-Link> setpc
J-Link> ... start debugging ... - STATUS: As of this writing, I have no been successful writing to FLASH - using the GDB server. I think that this is because of issues with GPNVM1 - settings and flash lock bits. In any event, the GDB server works great for - debugging after writing the program to FLASH using SAM-BA. + STATUS: As of this writing, I have not been successful writing to FLASH + using the GDB server; the write succeeds with no complaints, but the contents + of the FLASH memory remain unchanged. This may be because of issues with + GPNVM1 settings and flash lock bits? In any event, the GDB server works + great for debugging after writing the program to FLASH using SAM-BA. Writing to FLASH using SAM-BA ============================= @@ -1153,6 +1154,9 @@ Configurations This is another NSH example. If differs from the 'nsh' configuration in that this configurations uses a USB serial device for console I/O. + STATUS: + 2014-3-22: Partially functional, but not yet reliable. + NOTES: 1. See the NOTES in the description of the nsh configuration. Those