A few bug fixes, remove the unstested device notices, note supported boards in READMEs, and implement PMA

access stride (used on MCU's with 512 byte USB buffers).

README.md

@@ -55,7 +55,7 @@ The stack supports the following MCUs
 - **Nordic:** nRF52840
 - **NXP:** LPC11Uxx, LPC13xx, LPC175x_6x, LPC177x_8x, LPC18xx, LPC40xx, LPC43xx, LPC51Uxx
 - **MicroChip:** SAMD21, SAMD51 (device only)
-- **ST:** STM32F4, STM32H7 (device only)
+- **ST:** STM32F070xB, STM32F4, STM32H7 (device only)
 
 [Here is the list of supported Boards](docs/boards.md)
 

docs/boards.md

@@ -39,7 +39,7 @@ This code base already had supported for a handful of following boards
 - [Adafruit Metro M4 Express](https://www.adafruit.com/product/3382)
 
 ### ST STM32
-
+- [STM32F070RB Nucleo](https://www.st.com/en/evaluation-tools/nucleo-f070rb.html)
 - [STM32F407g Discovery](https://www.st.com/en/evaluation-tools/stm32f4discovery.html)
 - [STM32F411e Discovery](https://www.st.com/en/evaluation-tools/32f411ediscovery.html)
 - [STM32F412g Discovery](https://www.st.com/en/evaluation-tools/32f412gdiscovery.html)

src/common/tusb_common.h

@@ -89,7 +89,7 @@ static inline uint32_t tu_u32(uint8_t b1, uint8_t b2, uint8_t b3, uint8_t b4)
 
 static inline uint16_t tu_u16(uint8_t high, uint8_t low)
 {
-  return (((uint16_t) high) << 8) + low;
+  return (uint16_t)((((uint16_t) high) << 8) + low);
 }
 
 static inline uint8_t tu_u16_high(uint16_t u16) { return (uint8_t) (((uint16_t) (u16 >> 8)) & 0x00ff); }

src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c

@@ -29,11 +29,17 @@
  */
 
 /**********************************************
- * This driver should work with minimal for the ST Micro "USB A" peripheral. This
- *  covers:
+ * This driver has been tested with the following MCUs:
+ * 
+ * 
+ * STM32F070RB
+ *
+ *
+ * It also should work with minimal changes for any ST MCU with an "USB A" peripheral. This
+ * covers:
  *
  * F04x, F072, F078, 070x6/B      1024 byte buffer
- * F102, F103                      512 byte buffer; no internal D+ pull-up
+ * F102, F103                      512 byte buffer; no internal D+ pull-up (maybe many more changes?)
  * F302xB/C, F303xB/C, F373        512 byte buffer; no internal D+ pull-up
  * F302x6/8, F302xD/E2, F303xD/E  1024 byte buffer; no internal D+ pull-up
  * L0x2, L0x3                     1024 byte buffer
@@ -70,6 +76,11 @@
  * - Minimal error handling
  *   - Perhaps error interrupts sholud 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.
+ * - Add a callback for enabling/disabling the D+ PU on devices without an internal PU.
+ * - F3 models use three separate interrupts. I think we could only use the LP interrupt for
+ *     everything?  However, the interrupts are configurable so the DisableInt and EnableInt
+ *     below functions could be adjusting the wrong interrupts (if they had been reconfigured)
+ * - LPM is not used correctly, or at all?
  *
  * USB documentation and Reference implementations
  * - STM32 Reference manuals
@@ -92,7 +103,7 @@
 
 #include "tusb_option.h"
 
-#if TUSB_OPT_DEVICE_ENABLED && CFG_TUSB_MCU == OPT_MCU_STM32_FSDEV
+#if (TUSB_OPT_DEVICE_ENABLED) && ((CFG_TUSB_MCU) == (OPT_MCU_STM32_FSDEV))
 
 // In order to reduce the dependance on HAL, we undefine this.
 // Some definitions are copied to our private include file.
@@ -137,6 +148,7 @@
 // per STM32F3 reference manual
 #error BTABLE must be aligned to 8 bytes
 #endif
+
 // Max size of a USB FS packet is 64...
 #define MAX_PACKET_SIZE 64
 
@@ -204,17 +216,21 @@ void dcd_init (uint8_t rhport)
 
   // 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(uint16_t i=0;i<(PMA_LENGTH>>1); i++)
+  for(uint16_t i=0;i<(DCD_STM32_BTABLE_LENGTH>>1); i++)
   {
-    ((uint16_t*)USB_PMAADDR)[DCD_STM32_BTABLE_BASE + i] = 0u;
+    pma[PMA_STRIDE*(DCD_STM32_BTABLE_BASE + i)] = 0u;
   }
   USB->CNTR |= USB_CNTR_RESETM | USB_CNTR_SOFM | USB_CNTR_CTRM | USB_CNTR_SUSPM | USB_CNTR_WKUPM;
   dcd_handle_bus_reset();
+
   // And finally enable pull-up, which may trigger the RESET IRQ if the host is connected.
   // (if this MCU has an internal pullup)
 #if defined(USB_BCDR_DPPU)
   USB->BCDR |= USB_BCDR_DPPU;
+#else
+  // FIXME: callback to the user to ask them to twiddle a GPIO to disable/enable D+???
 #endif
+
 }
 
 // Enable device interrupt
@@ -225,14 +241,12 @@ void dcd_int_enable (uint8_t rhport)
   NVIC_SetPriority(USB_IRQn, 0);
   NVIC_EnableIRQ(USB_IRQn);
 #elif defined(STM32F3)
-#warning need to check these since the F3 can have its USB interrupts remapped.
   NVIC_SetPriority(USB_HP_CAN_TX_IRQn, 0);
   NVIC_SetPriority(USB_LP_CAN_RX0_IRQn, 0);
   NVIC_SetPriority(USBWakeUp_IRQn, 0);
   NVIC_EnableIRQ(USB_HP_CAN_TX_IRQn);
   NVIC_EnableIRQ(USB_LP_CAN_RX0_IRQn);
   NVIC_EnableIRQ(USBWakeUp_IRQn);
-
 #endif
 }
 
@@ -243,7 +257,6 @@ void dcd_int_disable(uint8_t rhport)
 #if defined(STM32F0)
   NVIC_DisableIRQ(USB_IRQn);
 #elif defined(STM32F3)
-#warning need to check these since the F3 can have its USB interrupts remapped.
   NVIC_DisableIRQ(USB_HP_CAN_TX_IRQn);
   NVIC_DisableIRQ(USB_LP_CAN_RX0_IRQn);
   NVIC_DisableIRQ(USBWakeUp_IRQn);
@@ -311,7 +324,7 @@ static void dcd_handle_bus_reset(void)
     EPREG(0) = 0u;
   }
 
-  ep_buf_ptr = 8*MAX_EP_COUNT; // 8 bytes per endpoint (two TX and two RX words, each)
+  ep_buf_ptr = DCD_STM32_BTABLE_BASE + 8*MAX_EP_COUNT; // 8 bytes per endpoint (two TX and two RX words, each)
   dcd_edpt_open (0, &ep0OUT_desc);
   dcd_edpt_open (0, &ep0IN_desc);
   newDADDR = 0;
@@ -386,7 +399,7 @@ static uint16_t dcd_ep_ctr_handler(void)
           /* Get SETUP Packet*/
           count = PCD_GET_EP_RX_CNT(USB, EPindex);
           //TU_ASSERT_ERR(count == 8);
-          dcd_read_packet_memory(userMemBuf, *PCD_EP_RX_ADDRESS(USB,EPindex), 8);
+          dcd_read_packet_memory(userMemBuf, *PCD_EP_RX_ADDRESS_PTR(USB,EPindex), 8);
           /* SETUP bit kept frozen while CTR_RX = 1*/
           dcd_event_setup_received(0, (uint8_t*)userMemBuf, true);
           PCD_CLEAR_RX_EP_CTR(USB, EPindex);
@@ -401,7 +414,7 @@ static uint16_t dcd_ep_ctr_handler(void)
 
           if (count != 0U)
           {
-            dcd_read_packet_memory(xfer->buffer, *PCD_EP_RX_ADDRESS(USB,EPindex), count);
+            dcd_read_packet_memory(xfer->buffer, *PCD_EP_RX_ADDRESS_PTR(USB,EPindex), count);
             xfer->queued_len = (uint16_t)(xfer->queued_len + count);
           }
 
@@ -440,7 +453,7 @@ static uint16_t dcd_ep_ctr_handler(void)
         if (count != 0U)
         {
           dcd_read_packet_memory(&(xfer->buffer[xfer->queued_len]),
-              *PCD_EP_RX_ADDRESS(USB,EPindex), count);
+              *PCD_EP_RX_ADDRESS_PTR(USB,EPindex), count);
         }
 
         /*multi-packet on the NON control OUT endpoint */
@@ -568,14 +581,14 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc
 
   if(dir == TUSB_DIR_IN)
   {
-    *PCD_EP_TX_ADDRESS(USB, epnum) = ep_buf_ptr;
+    *PCD_EP_TX_ADDRESS_PTR(USB, epnum) = ep_buf_ptr;
     PCD_SET_EP_RX_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(USB, epnum) = ep_buf_ptr;
+    *PCD_EP_RX_ADDRESS_PTR(USB, epnum) = ep_buf_ptr;
     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);
@@ -596,11 +609,11 @@ static void dcd_transmit_packet(xfer_ctl_t * xfer, uint16_t ep_ix)
   {
     len = 64u;
   }
-  dcd_write_packet_memory(*PCD_EP_TX_ADDRESS(USB,ep_ix), &(xfer->buffer[xfer->queued_len]), len);
+  dcd_write_packet_memory(*PCD_EP_TX_ADDRESS_PTR(USB,ep_ix), &(xfer->buffer[xfer->queued_len]), len);
   xfer->queued_len = (uint16_t)(xfer->queued_len + len);
 
   PCD_SET_EP_TX_CNT(USB,ep_ix,len);
-  PCD_SET_EP_TX_STATUS(USB, ep_ix, USB_EP_TX_VALID)
+  PCD_SET_EP_TX_STATUS(USB, ep_ix, USB_EP_TX_VALID);
 }
 
 bool dcd_edpt_xfer (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes)
@@ -687,8 +700,8 @@ void dcd_edpt_clear_stall (uint8_t rhport, uint8_t ep_addr)
   * @brief Copy a buffer from user memory area to packet memory area (PMA).
   *        This uses byte-access for user memory (so support non-aligned buffers)
   *        and 16-bit access for packet memory.
-  * @param   dst, but not necessary in system-memory addressing
-  * @param   pbUsrBuf pointer to user memory area.
+  * @param   dst, byte address in PMA; must be 16-bit aligned
+  * @param   src pointer to user memory area.
   * @param   wPMABufAddr address into PMA.
   * @param   wNBytes no. of bytes to be copied.
   * @retval None
@@ -699,19 +712,27 @@ static void dcd_write_packet_memory(uint16_t dst, const void *__restrict src, si
   uint32_t i;
   uint16_t temp1, temp2;
   const uint8_t * srcVal;
+
+#ifdef DEBUG
+  if(((dst%2) != 0) ||
+      (dst < DCD_STM32_BTABLE_BASE) ||
+      dst >= (DCD_STM32_BTABLE_BASE + DCD_STM32_BTABLE_LENGTH))
+    while(1) TU_BREAKPOINT();
+#endif
   // The GCC optimizer will combine access to 32-bit sizes if we let it. Force
   // it volatile so that it won't do that.
   __IO uint16_t *pdwVal;
 
   srcVal = src;
-  pdwVal = (__IO uint16_t*)( ((uint8_t*)USB) + 0x400U + dst );
+  pdwVal = &pma[PMA_STRIDE*(dst>>1)];
 
   for (i = n; i != 0; i--)
   {
     temp1 = (uint16_t) *srcVal;
     srcVal++;
     temp2 = temp1 | ((uint16_t)((uint16_t) ((*srcVal) << 8U))) ;
-    *pdwVal++ = temp2;
+    *pdwVal = temp2;
+    pdwVal += PMA_STRIDE;
     srcVal++;
   }
 }
@@ -731,19 +752,28 @@ static void dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wN
   __IO const uint16_t *pdwVal;
   uint32_t temp;
 
-  pdwVal = (__IO uint16_t*)( ((uint8_t*)USB) + 0x400U + src );
+#ifdef DEBUG
+  if((src%2) != 0 ||
+      (src < DCD_STM32_BTABLE_BASE) ||
+      src >= (DCD_STM32_BTABLE_BASE + DCD_STM32_BTABLE_LENGTH))
+    while(1) TU_BREAKPOINT();
+#endif
+
+  pdwVal = &pma[PMA_STRIDE*(src>>1)];
   uint8_t *dstVal = (uint8_t*)dst;
 
   for (i = n; i != 0U; i--)
   {
-    temp = *pdwVal++;
+    temp = *pdwVal;
+    pdwVal += PMA_STRIDE;
     *dstVal++ = ((temp >> 0) & 0xFF);
     *dstVal++ = ((temp >> 8) & 0xFF);
   }
 
   if (wNBytes % 2)
   {
-    temp = *pdwVal++;
+    temp = *pdwVal;
+    pdwVal += PMA_STRIDE;
     *dstVal++ = ((temp >> 0) & 0xFF);
   }
 }

src/portable/st/stm32_fsdev/dcd_stm32_fsdev_pvt_st.h

@@ -34,24 +34,64 @@
 
 // This file contains source copied from ST's HAL, and thus should have their copyright statement.
 
-
 // PMA_LENGTH is PMA buffer size in bytes.
+// On 512-byte devices, access with a stride of two words (use every other 16-bit address)
+// On 1024-byte devices, access with a stride of one word (use every 16-bit address)
 
-#if defined(STM32F070xB) | defined(STM32F070x6)
+#ifndef PORTABLE_ST_STM32F0_DCD_STM32F0_FSDEV_PVT_ST_H_
+#define PORTABLE_ST_STM32F0_DCD_STM32F0_FSDEV_PVT_ST_H_
+
+#if defined(STM32F042x6) | \
+    defined(STM32F070x6) | defined(STM32F070xB) | \
+    defined(STM32F072xB) | \
+    defined(STM32F078xx)
 #include "stm32f0xx.h"
 #define PMA_LENGTH 1024
-#elif defined(STM32F303xB) | defined(STM32F303xC)
-#warning STM32F3 platform is untested.
+// F0x2 models are crystal-less
+// All have internal D+ pull-up
+// 070RB:    2 x 16 bits/word memory     LPM Support, BCD Support
+// PMA dedicated to USB (no sharing with CAN)
+#elif defined(STM32F102x6) | defined(STM32F102x6) | \
+      defined(STM32F103x6) | defined(STM32F103xB) | \
+      defined(STM32F103xE) | defined(STM32F103xB)
+#include "stm32f1xx.h"
+#define PMA_LENGTH 512u
+// NO internal Pull-ups
+//         *B, and *C:    2 x 16 bits/word
+#error The F102/F103 driver is expected not to work, but it might? Try it?
+
+#elif defined(STM32F302xB) | defined(STM32F302xC) | \
+      defined(STM32F303xB) | defined(STM32F303xC) | \ //good
+      defined(STM32F373xC)
+#include "stm32f3xx.h"
+#define PMA_LENGTH 512u
+// NO internal Pull-ups
+//         *B, and *C:    1 x 16 bits/word
+// PMA dedicated to USB (no sharing with CAN)
+#elif defined(STM32F302x6) | defined(STM32F302x8) | \
+      defined(STM32F302xD) | defined(STM32F302xE) | \
+      defined(STM32F303xD) | defined(STM32F303xE) | \ //good
 #include "stm32f3xx.h"
-#define PMA_LENGTH 512
+#define PMA_LENGTH 1024u
+// NO internal Pull-ups
+// *6, *8, *D, and *E:    2 x 16 bits/word     LPM Support
+// When CAN clock is enabled, USB can use first 768 bytes ONLY.
 #else
-#error You are using an untested or unimplemented STM32 variant
+#error You are using an untested or unimplemented STM32 variant. Please update the driver.
+// This includes for L0x2, L0x3, L1, L4x2 and L4x3
 #endif
 
+// For purposes of accessing the packet
+#if ((PMA_LENGTH) == 512u)
+#  define PMA_STRIDE (2u)
+#elif ((PMA_LENGTH) == 1024u)
+#  define PMA_STRIDE (1u)
+#endif
 
-
-#ifndef PORTABLE_ST_STM32F0_DCD_STM32F0_FSDEV_PVT_ST_H_
-#define PORTABLE_ST_STM32F0_DCD_STM32F0_FSDEV_PVT_ST_H_
+// And for type-safety create a new macro for the volatile address of PMAADDR
+// The compiler should warn us if we cast it to a non-volatile type?
+// Volatile is also needed to prevent the optimizer from changing access to 32-bit (as 32-bit access is forbidden)
+static __IO uint16_t * const pma = (__IO uint16_t*)USB_PMAADDR;
 
 /* SetENDPOINT */
 #define PCD_SET_ENDPOINT(USBx, bEpNum,wRegValue)  (*((__IO uint16_t *)(((uint32_t)(&(USBx)->EP0R + (bEpNum) * 2U))))= (uint16_t)(wRegValue))
@@ -77,8 +117,8 @@
   * @param  bEpNum Endpoint Number.
   * @retval Counter value
   */
-#define PCD_GET_EP_TX_CNT(USBx, bEpNum)((uint16_t)(*PCD_EP_TX_CNT((USBx), (bEpNum))) & 0x3ffU)
-#define PCD_GET_EP_RX_CNT(USBx, bEpNum)((uint16_t)(*PCD_EP_RX_CNT((USBx), (bEpNum))) & 0x3ffU)
+#define PCD_GET_EP_TX_CNT(USBx, bEpNum)((uint16_t)(*PCD_EP_TX_CNT_PTR((USBx), (bEpNum))) & 0x3ffU)
+#define PCD_GET_EP_RX_CNT(USBx, bEpNum)((uint16_t)(*PCD_EP_RX_CNT_PTR((USBx), (bEpNum))) & 0x3ffU)
 
 /**
   * @brief  Sets counter of rx buffer with no. of blocks.
@@ -131,16 +171,18 @@
 #define PCD_SET_EP_ADDRESS(USBx, bEpNum,bAddr) PCD_SET_ENDPOINT((USBx), (bEpNum),\
     USB_EP_CTR_RX|USB_EP_CTR_TX|(((uint32_t)(PCD_GET_ENDPOINT((USBx), (bEpNum)))) & USB_EPREG_MASK) | (bAddr))
 
+#define PCD_BTABLE_WORD_PTR(USBx,x) (&(pma[PMA_STRIDE*((((USBx)->BTABLE)>>1) + x)]))
 
-#define PCD_EP_TX_ADDRESS(USBx, bEpNum) ((__IO uint16_t *)((uint32_t)((((USBx)->BTABLE+(bEpNum)*8u)+     ((uint32_t)(USBx) + 0x400U)))))
-#define PCD_EP_TX_CNT(USBx, bEpNum) ((__IO uint16_t *)((uint32_t)((((USBx)->BTABLE+(bEpNum)*8u+2u)+  ((uint32_t)(USBx) + 0x400U)))))
+// Pointers to the PMA table entries (using the ARM address space)
+#define PCD_EP_TX_ADDRESS_PTR(USBx, bEpNum) (PCD_BTABLE_WORD_PTR(USBx,(bEpNum)*4u + 0u))
+#define PCD_EP_TX_CNT_PTR(USBx, bEpNum)     (PCD_BTABLE_WORD_PTR(USBx,(bEpNum)*4u + 1u))
 
-#define PCD_EP_RX_ADDRESS(USBx, bEpNum) ((__IO uint16_t *)((uint32_t)((((USBx)->BTABLE+(bEpNum)*8u+4u)+ ((uint32_t)(USBx) + 0x400U)))))
-#define PCD_EP_RX_CNT(USBx, bEpNum) ((__IO uint16_t *)((uint32_t)((((USBx)->BTABLE+(bEpNum)*8u+6u)+  ((uint32_t)(USBx) + 0x400U)))))
+#define PCD_EP_RX_ADDRESS_PTR(USBx, bEpNum) (PCD_BTABLE_WORD_PTR(USBx,(bEpNum)*4u + 2u))
+#define PCD_EP_RX_CNT_PTR(USBx, bEpNum)     (PCD_BTABLE_WORD_PTR(USBx,(bEpNum)*4u + 3u))
 
-#define PCD_SET_EP_TX_CNT(USBx, bEpNum,wCount) (*PCD_EP_TX_CNT((USBx), (bEpNum)) = (wCount))
+#define PCD_SET_EP_TX_CNT(USBx, bEpNum,wCount) (*PCD_EP_TX_CNT_PTR((USBx), (bEpNum)) = (wCount))
 #define PCD_SET_EP_RX_CNT(USBx, bEpNum,wCount) do {\
-    __IO uint16_t *pdwReg =PCD_EP_RX_CNT((USBx),(bEpNum)); \
+    __IO uint16_t *pdwReg =PCD_EP_RX_CNT_PTR((USBx),(bEpNum)); \
     PCD_SET_EP_CNT_RX_REG((pdwReg), (wCount))\
   } while(0)
 
@@ -232,8 +274,9 @@
 
 #define EPREG(n) (((__IO uint16_t*)USB_BASE)[n*2])
 
+// This checks if the device has "LPM"
 #if defined(USB_ISTR_L1REQ)
-#define USB_ISTR_L1REQ_FORCED USB_ISTR_L1REQ
+#define USB_ISTR_L1REQ_FORCED (USB_ISTR_L1REQ)
 #else
 #define USB_ISTR_L1REQ_FORCED ((uint16_t)0x0000U)
 #endif