Merge pull request #336 from pigrew/edpt_close

> If you notice my chain of events above, the bulk transfer was started BEFORE the SET_INTERFACE call. The USB device hardware swaps the order of them being delivered. On STM32, it gives priority to the lower-numbered EP index.

It shouldn't be a matter, control is 2+ stage, before sending the setup. Host should stop all communication to the endpoint that It wants to close.

4 files changed, 155 insertions, 28 deletions

diff --git a/src/device/dcd.h b/src/device/dcd.h
index 08b217fa9..6554be2c8 100644
--- a/src/device/dcd.h
+++ b/src/device/dcd.h
@@ -123,6 +123,10 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * re
 // Configure endpoint's registers according to descriptor
 bool dcd_edpt_open        (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc);
 
+// Close an endpoint.
+// Since it is weak, caller must TU_ASSERT this function's existence before calling it.
+void dcd_edpt_close        (uint8_t rhport, uint8_t ep_addr) TU_ATTR_WEAK;
+
 // Submit a transfer, When complete dcd_event_xfer_complete() is invoked to notify the stack
 bool dcd_edpt_xfer        (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes);
 
diff --git a/src/device/usbd.c b/src/device/usbd.c
index 29845ff36..21d520bde 100644
--- a/src/device/usbd.c
+++ b/src/device/usbd.c
@@ -1011,4 +1011,20 @@ bool usbd_edpt_stalled(uint8_t rhport, uint8_t ep_addr)
   return _usbd_dev.ep_status[epnum][dir].stalled;
 }
 
+/**
+ * usbd_edpt_close will disable an endpoint.
+ * 
+ * In progress transfers on this EP may be delivered after this call.
+ * 
+ */
+void usbd_edpt_close(uint8_t rhport, uint8_t ep_addr)
+{
+  TU_ASSERT(dcd_edpt_close, /**/);
+  TU_LOG2("  CLOSING Endpoint: 0x%02X\r\n", ep_addr);
+
+  dcd_edpt_close(rhport, ep_addr);
+
+  return;
+}
+
 #endif
diff --git a/src/device/usbd_pvt.h b/src/device/usbd_pvt.h
index b9947044b..26b9700e8 100644
--- a/src/device/usbd_pvt.h
+++ b/src/device/usbd_pvt.h
@@ -38,6 +38,7 @@
 //--------------------------------------------------------------------+
 
 //bool usbd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc);
+void usbd_edpt_close(uint8_t rhport, uint8_t ep_addr);
 
 // Submit a usb transfer
 bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes);
diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c
index 2939e5ae2..14ffcbe40 100644
--- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c
+++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c
@@ -63,19 +63,16 @@
  * Current driver limitations (i.e., a list of features for you to add):
  * - STALL handled, but not tested.
  *   - Does it work? No clue.
- * - All EP BTABLE buffers are created as max 64 bytes.
- *   - Smaller can be requested, but it has to be an even number.
+ * - All EP BTABLE buffers are created based on max packet size of first EP opened with that address.
  * - No isochronous endpoints
  * - Endpoint index is the ID of the endpoint
  *   - This means that priority is given to endpoints with lower ID numbers
  *   - Code is mixing up EP IX with EP ID. Everywhere.
- * - No way to close endpoints; Can a device be reconfigured without a reset?
  * - Packet buffer memory is copied in the interrupt.
  *   - This is better for performance, but means interrupts are disabled for longer
  *   - DMA may be the best choice, but it could also be pushed to the USBD task.
  * - No double-buffering
  * - No DMA
- * - No provision to control the D+ pull-up using GPIO on devices without an internal pull-up.
  * - Minimal error handling
  *   - Perhaps error interrupts should be reported to the stack, or cause a device reset?
  * - Assumes a single USB peripheral; I think that no hardware has multiple so this is fine.
@@ -131,15 +128,16 @@
  * Configuration
  *****************************************************/
 
-// HW supports max of 8 endpoints, but this can be reduced to save RAM
+// HW supports max of 8 bidirectional endpoints, but this can be reduced to save RAM
+// (8u here would mean 8 IN and 8 OUT)
 #ifndef MAX_EP_COUNT
-#  define MAX_EP_COUNT 8u
+#  define MAX_EP_COUNT 8U
 #endif
 
 // If sharing with CAN, one can set this to be non-zero to give CAN space where it wants it
 // Both of these MUST be a multiple of 2, and are in byte units.
 #ifndef DCD_STM32_BTABLE_BASE
-#  define DCD_STM32_BTABLE_BASE 0u
+#  define DCD_STM32_BTABLE_BASE 0U
 #endif
 
 #ifndef DCD_STM32_BTABLE_LENGTH
@@ -163,7 +161,9 @@ typedef struct
   uint8_t * buffer;
   uint16_t total_len;
   uint16_t queued_len;
-  uint16_t max_packet_size;
+  uint16_t pma_ptr;
+  uint8_t max_packet_size;
+  uint8_t pma_alloc_size;
 } xfer_ctl_t;
 
 static xfer_ctl_t xfer_status[MAX_EP_COUNT][2];
@@ -177,15 +177,19 @@ static TU_ATTR_ALIGNED(4) uint32_t _setup_packet[6];
 
 static uint8_t remoteWakeCountdown; // When wake is requested
 
-// EP Buffers assigned from end of memory location, to minimize their chance of crashing
 // into the stack.
-static uint16_t ep_buf_ptr;
 static void dcd_handle_bus_reset(void);
-static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, size_t wNBytes);
-static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wNBytes);
 static void dcd_transmit_packet(xfer_ctl_t * xfer, uint16_t ep_ix);
 static void dcd_ep_ctr_handler(void);
 
+// PMA allocation/access 
+static uint8_t open_ep_count;
+static uint16_t ep_buf_ptr; ///< Points to first free memory location
+static void dcd_pma_alloc_reset(void);
+static uint16_t dcd_pma_alloc(uint8_t ep_addr, size_t length);
+static void dcd_pma_free(uint8_t ep_addr);
+static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, size_t wNBytes);
+static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wNBytes);
 
 // Using a function due to better type checks
 // This seems better than having to do type casts everywhere else
@@ -219,7 +223,7 @@ void dcd_init (uint8_t rhport)
     asm("NOP");
   }
   USB->CNTR = 0; // Enable USB
-
+  
   USB->BTABLE = DCD_STM32_BTABLE_BASE;
 
   reg16_clear_bits(&USB->ISTR, USB_ISTR_ALL_EVENTS); // Clear pending interrupts
@@ -231,12 +235,6 @@ void dcd_init (uint8_t rhport)
     pcd_set_endpoint(USB,i,0u);
   }
 
-  // Initialize the BTABLE for EP0 at this point (though setting up the EP0R is unneeded)
-  // This is actually not necessary, but helps debugging to start with a blank RAM area
-  for(uint32_t i=0;i<(DCD_STM32_BTABLE_LENGTH>>1); i++)
-  {
-    pma[PMA_STRIDE*(DCD_STM32_BTABLE_BASE + i)] = 0u;
-  }
   USB->CNTR |= USB_CNTR_RESETM | USB_CNTR_SOFM | USB_CNTR_ESOFM | USB_CNTR_CTRM | USB_CNTR_SUSPM | USB_CNTR_WKUPM;
   dcd_handle_bus_reset();
   
@@ -368,7 +366,7 @@ static void dcd_handle_bus_reset(void)
     pcd_set_endpoint(USB,i,0u);
   }
 
-  ep_buf_ptr = DCD_STM32_BTABLE_BASE + 8*MAX_EP_COUNT; // 8 bytes per endpoint (two TX and two RX words, each)
+  dcd_pma_alloc_reset();
   dcd_edpt_open (0, &ep0OUT_desc);
   dcd_edpt_open (0, &ep0IN_desc);
 
@@ -592,6 +590,85 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * re
   }
 }
 
+static void dcd_pma_alloc_reset(void)
+{
+  ep_buf_ptr = DCD_STM32_BTABLE_BASE + 8*MAX_EP_COUNT; // 8 bytes per endpoint (two TX and two RX words, each)
+  //TU_LOG2("dcd_pma_alloc_reset()\r\n");
+  for(uint32_t i=0; i<MAX_EP_COUNT; i++)
+  {
+    xfer_ctl_ptr(i,TUSB_DIR_OUT)->pma_alloc_size = 0U;
+    xfer_ctl_ptr(i,TUSB_DIR_IN)->pma_alloc_size = 0U;
+    xfer_ctl_ptr(i,TUSB_DIR_OUT)->pma_ptr = 0U;
+    xfer_ctl_ptr(i,TUSB_DIR_IN)->pma_ptr = 0U;
+  }
+}
+
+/***
+ * Allocate a section of PMA
+ * 
+ * If the EP number has already been allocated, and the new allocation
+ * is larger than the old allocation, then this will fail with a TU_ASSERT.
+ * (This is done to simplify the code. More complicated algorithms could be used)
+ * 
+ * During failure, TU_ASSERT is used. If this happens, rework/reallocate memory manually.
+ */
+static uint16_t dcd_pma_alloc(uint8_t ep_addr, size_t length)
+{
+  uint8_t const epnum = tu_edpt_number(ep_addr);
+  uint8_t const dir   = tu_edpt_dir(ep_addr);
+  xfer_ctl_t* epXferCtl = xfer_ctl_ptr(epnum,dir);
+
+  if(epXferCtl->pma_alloc_size != 0U)
+  {
+    //TU_LOG2("dcd_pma_alloc(%x,%x)=%x (cached)\r\n",ep_addr,length,epXferCtl->pma_ptr);
+    // Previously allocated
+    TU_ASSERT(length <= epXferCtl->pma_alloc_size, 0xFFFF);  // Verify no larger than previous alloc
+    return epXferCtl->pma_ptr;
+  }
+  
+  uint16_t addr = ep_buf_ptr; 
+  ep_buf_ptr = (uint16_t)(ep_buf_ptr + length); // increment buffer pointer
+  
+  // Verify no overflow
+  TU_ASSERT(ep_buf_ptr <= PMA_LENGTH, 0xFFFF);
+  
+  epXferCtl->pma_ptr = addr;
+  epXferCtl->pma_alloc_size = length;
+  //TU_LOG2("dcd_pma_alloc(%x,%x)=%x\r\n",ep_addr,length,addr);
+
+  return addr;
+}
+
+/***
+ * Free a block of PMA space
+ */
+static void dcd_pma_free(uint8_t ep_addr)
+{
+  uint8_t const epnum = tu_edpt_number(ep_addr);
+  uint8_t const dir   = tu_edpt_dir(ep_addr);
+
+  // Presently, this should never be called for EP0 IN/OUT
+  TU_ASSERT(open_ep_count > 2, /**/);
+  TU_ASSERT(xfer_ctl_ptr(epnum,dir)->max_packet_size != 0, /**/);
+  open_ep_count--;
+
+  // If count is 2, only EP0 should be open, so allocations can be mostly reset.
+
+  if(open_ep_count == 2)
+  {
+    ep_buf_ptr = DCD_STM32_BTABLE_BASE + 8*MAX_EP_COUNT + 2*CFG_TUD_ENDPOINT0_SIZE; // 8 bytes per endpoint (two TX and two RX words, each), and EP0
+
+    // Skip EP0
+    for(uint32_t i=1; i<MAX_EP_COUNT; i++)
+    {
+      xfer_ctl_ptr(i,TUSB_DIR_OUT)->pma_alloc_size = 0U;
+      xfer_ctl_ptr(i,TUSB_DIR_IN)->pma_alloc_size = 0U;
+      xfer_ctl_ptr(i,TUSB_DIR_OUT)->pma_ptr = 0U;
+      xfer_ctl_ptr(i,TUSB_DIR_IN)->pma_ptr = 0U;
+    }
+  }
+}
+
 // The STM32F0 doesn't seem to like |= or &= to manipulate the EP#R registers,
 // so I'm using the #define from HAL here, instead.
 
@@ -601,28 +678,30 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc
   uint8_t const epnum = tu_edpt_number(p_endpoint_desc->bEndpointAddress);
   uint8_t const dir   = tu_edpt_dir(p_endpoint_desc->bEndpointAddress);
   const uint16_t epMaxPktSize = p_endpoint_desc->wMaxPacketSize.size;
+  uint16_t pma_addr;
+  uint32_t wType;
+  
   // Isochronous not supported (yet), and some other driver assumptions.
-
   TU_ASSERT(p_endpoint_desc->bmAttributes.xfer != TUSB_XFER_ISOCHRONOUS);
   TU_ASSERT(epnum < MAX_EP_COUNT);
 
   // Set type
   switch(p_endpoint_desc->bmAttributes.xfer) {
   case TUSB_XFER_CONTROL:
-    pcd_set_eptype(USB, epnum, USB_EP_CONTROL);
+    wType = USB_EP_CONTROL;
     break;
 #if (0)
   case TUSB_XFER_ISOCHRONOUS: // FIXME: Not yet supported
-    pcd_set_eptype(USB, epnum, USB_EP_ISOCHRONOUS); break;
+    wType = USB_EP_ISOCHRONOUS;
     break;
 #endif
 
   case TUSB_XFER_BULK:
-    pcd_set_eptype(USB, epnum, USB_EP_BULK);
+    wType = USB_EP_CONTROL;
     break;
 
   case TUSB_XFER_INTERRUPT:
-    pcd_set_eptype(USB, epnum, USB_EP_INTERRUPT);
+    wType = USB_EP_INTERRUPT;
     break;
 
   default:
@@ -630,32 +709,59 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc
     return false;
   }
 
+  pcd_set_eptype(USB, epnum, wType);
   pcd_set_ep_address(USB, epnum, epnum);
   // Be normal, for now, instead of only accepting zero-byte packets (on control endpoint)
   // or being double-buffered (bulk endpoints)
   pcd_clear_ep_kind(USB,0);
 
+  pma_addr = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress, p_endpoint_desc->wMaxPacketSize.size);
+
   if(dir == TUSB_DIR_IN)
   {
-    *pcd_ep_tx_address_ptr(USB, epnum) = ep_buf_ptr;
+    *pcd_ep_tx_address_ptr(USB, epnum) = pma_addr;
     pcd_set_ep_tx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size);
     pcd_clear_tx_dtog(USB, epnum);
     pcd_set_ep_tx_status(USB,epnum,USB_EP_TX_NAK);
   }
   else
   {
-    *pcd_ep_rx_address_ptr(USB, epnum) = ep_buf_ptr;
+    *pcd_ep_rx_address_ptr(USB, epnum) = pma_addr;
     pcd_set_ep_rx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size);
     pcd_clear_rx_dtog(USB, epnum);
     pcd_set_ep_rx_status(USB, epnum, USB_EP_RX_NAK);
   }
 
   xfer_ctl_ptr(epnum, dir)->max_packet_size = epMaxPktSize;
-  ep_buf_ptr = (uint16_t)(ep_buf_ptr + p_endpoint_desc->wMaxPacketSize.size); // increment buffer pointer
 
   return true;
 }
 
+/**
+ * Close an endpoint.
+ * 
+ * This function may be called with interrupts enabled or disabled.
+ * 
+ * This also clears transfers in progress, should there be any.
+ */
+void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr)
+{
+  (void)rhport;
+  uint32_t const epnum = tu_edpt_number(ep_addr);
+  uint32_t const dir   = tu_edpt_dir(ep_addr);
+  
+  if(dir == TUSB_DIR_IN)
+  {
+    pcd_set_ep_tx_status(USB,epnum,USB_EP_TX_DIS);
+  }
+  else
+  {
+    pcd_set_ep_rx_status(USB, epnum, USB_EP_RX_DIS);
+  }
+
+  dcd_pma_free(ep_addr);
+}
+
 // Currently, single-buffered, and only 64 bytes at a time (max)
 
 static void dcd_transmit_packet(xfer_ctl_t * xfer, uint16_t ep_ix)