diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 1681d6412..96579c75b 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -36,7 +36,7 @@ * * * It also should work with minimal changes for any ST MCU with an "USB A"/"PCD"/"HCD" peripheral. This - * covers: + * covers: * * F04x, F072, F078, 070x6/B 1024 byte buffer * F102, F103 512 byte buffer; no internal D+ pull-up (maybe many more changes?) @@ -46,9 +46,13 @@ * L1 512 byte buffer * L4x2, L4x3 1024 byte buffer * + * To use this driver, you must: + * - Enable USB clock; Perhaps use __HAL_RCC_USB_CLK_ENABLE(); + * - (Optionally configure GPIO HAL to tell it the USB driver is using the USB pins) + * - call tusb_init(); + * - periodically call tusb_task(); + * * Assumptions of the driver: - * - dcd_fs_irqHandler() is called by the USB interrupt handler - * - USB clock enabled before usb_init() is called; Perhaps use __HAL_RCC_USB_CLK_ENABLE(); * - You are not using CAN (it must share the packet buffer) * - APB clock is >= 10 MHz * - On some boards, series resistors are required, but not on others. @@ -59,7 +63,6 @@ * Current driver limitations (i.e., a list of features for you to add): * - STALL handled, but not tested. * - Does it work? No clue. - * - Only tested on F070RB; other models will have an #error during compilation * - All EP BTABLE buffers are created as max 64 bytes. * - Smaller can be requested, but it has to be an even number. * - No isochronous endpoints @@ -74,7 +77,7 @@ * - 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 sholud be reported to the stack, or cause a device reset? + * - 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. * - 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 @@ -144,18 +147,12 @@ * Checks, structs, defines, function definitions, etc. */ -#if ((MAX_EP_COUNT) > 8) -# error Only 8 endpoints supported on the hardware -#endif +TU_VERIFY_STATIC((MAX_EP_COUNT) <= STFSDEV_EP_COUNT,"Only 8 endpoints supported on the hardware"); -#if (((DCD_STM32_BTABLE_BASE) + (DCD_STM32_BTABLE_LENGTH))>(PMA_LENGTH)) -# error BTABLE does not fit in PMA RAM -#endif +TU_VERIFY_STATIC(((DCD_STM32_BTABLE_BASE) + (DCD_STM32_BTABLE_LENGTH))<=(PMA_LENGTH), + "BTABLE does not fit in PMA RAM"); -#if (((DCD_STM32_BTABLE_BASE) % 8) != 0) -// per STM32F3 reference manual -#error BTABLE must be aligned to 8 bytes -#endif +TU_VERIFY_STATIC(((DCD_STM32_BTABLE_BASE) % 8) == 0, "BTABLE base must be aligned to 8 bytes"); // Max size of a USB FS packet is 64... #define MAX_PACKET_SIZE 64 @@ -180,11 +177,18 @@ static uint8_t newDADDR; // Used to set the new device address during the CTR IR // into the stack. static uint16_t ep_buf_ptr; static void dcd_handle_bus_reset(void); -static void dcd_write_packet_memory(uint16_t dst, const void *__restrict src, size_t wNBytes); -static void dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wNBytes); +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 uint16_t dcd_ep_ctr_handler(void); + +// Using a function due to better type checks +// This seems better than having to do type casts everywhere else +static inline void reg16_clear_bits(__IO uint16_t *reg, uint16_t mask) { + *reg = (uint16_t)(*reg & ~mask); +} + void dcd_init (uint8_t rhport) { (void)rhport; @@ -204,7 +208,7 @@ void dcd_init (uint8_t rhport) { asm("NOP"); } - USB->CNTR &= ~(USB_CNTR_PDWN);// Remove powerdown + reg16_clear_bits(&USB->CNTR, USB_CNTR_PDWN);// Remove powerdown // Wait startup time, for F042 and F070, this is <= 1 us. for(uint32_t i = 0; i<200; i++) // should be a few us { @@ -214,17 +218,18 @@ void dcd_init (uint8_t rhport) USB->BTABLE = DCD_STM32_BTABLE_BASE; - USB->ISTR &= ~(USB_ISTR_ALL_EVENTS); // Clear pending interrupts + reg16_clear_bits(&USB->ISTR, USB_ISTR_ALL_EVENTS); // Clear pending interrupts - // Clear all EPREG - for(uint16_t i=0; i<8; i++) + // Reset endpoints to disabled + for(uint32_t i=0; i>1); i++) + for(uint32_t i=0;i<(DCD_STM32_BTABLE_LENGTH>>1); i++) { pma[PMA_STRIDE*(DCD_STM32_BTABLE_BASE + i)] = 0u; } @@ -327,15 +332,15 @@ static void dcd_handle_bus_reset(void) USB->DADDR = 0u; // disable USB peripheral by clearing the EF flag // Clear all EPREG (or maybe this is automatic? I'm not sure) - for(uint16_t i=0; i<8; i++) + for(uint32_t i=0; iDADDR = USB_DADDR_EF; // Set enable flag, and leaving the device address as zero. PCD_SET_EP_RX_STATUS(USB, 0, USB_EP_RX_VALID); // And start accepting SETUP on EP0 } @@ -375,8 +380,8 @@ static uint16_t dcd_ep_ctr_handler(void) if((newDADDR != 0) && ( xfer->total_len == 0U)) { // Delayed setting of the DADDR after the 0-len DATA packet acking the request is sent. - USB->DADDR &= ~USB_DADDR_ADD; - USB->DADDR |= newDADDR; + reg16_clear_bits(&USB->DADDR, USB_DADDR_ADD); + USB->DADDR |= (uint16_t)newDADDR; // leave the enable bit set newDADDR = 0; } if(xfer->total_len == 0) // Probably a status message? @@ -427,14 +432,14 @@ static uint16_t dcd_ep_ctr_handler(void) } /* Process Control Data OUT status Packet*/ - if(EPindex == 0 && xfer->total_len == 0) + if(EPindex == 0u && xfer->total_len == 0u) { PCD_CLEAR_EP_KIND(USB,0); // Good, so allow non-zero length packets now. } dcd_event_xfer_complete(0, EPindex, xfer->total_len, XFER_RESULT_SUCCESS, true); PCD_SET_EP_RX_CNT(USB, EPindex, CFG_TUD_ENDPOINT0_SIZE); - if(EPindex == 0 && xfer->total_len == 0) + if(EPindex == 0u && xfer->total_len == 0u) { PCD_SET_EP_RX_STATUS(USB, EPindex, USB_EP_RX_VALID);// Await next SETUP } @@ -507,7 +512,7 @@ static uint16_t dcd_ep_ctr_handler(void) return 0; } -void dcd_fs_irqHandler(void) { +static void dcd_fs_irqHandler(void) { uint16_t int_status = USB->ISTR; // unused IRQs: (USB_ISTR_PMAOVR | USB_ISTR_ERR | USB_ISTR_WKUP | USB_ISTR_SUSP | USB_ISTR_ESOF | USB_ISTR_L1REQ ) @@ -517,20 +522,20 @@ void dcd_fs_irqHandler(void) { /* servicing of the endpoint correct transfer interrupt */ /* clear of the CTR flag into the sub */ dcd_ep_ctr_handler(); - USB->ISTR &= ~USB_ISTR_CTR; + reg16_clear_bits(&USB->ISTR, USB_ISTR_CTR); } if(int_status & USB_ISTR_RESET) { // USBRST is start of reset. - USB->ISTR &= ~USB_ISTR_RESET; + reg16_clear_bits(&USB->ISTR, USB_ISTR_RESET); dcd_handle_bus_reset(); dcd_event_bus_signal(0, DCD_EVENT_BUS_RESET, true); } if (int_status & USB_ISTR_WKUP) { - USB->CNTR &= ~USB_CNTR_LPMODE; - USB->CNTR &= ~USB_CNTR_FSUSP; - USB->ISTR &= ~USB_ISTR_WKUP; + reg16_clear_bits(&USB->CNTR, USB_CNTR_LPMODE); + reg16_clear_bits(&USB->CNTR, USB_CNTR_FSUSP); + reg16_clear_bits(&USB->ISTR, USB_ISTR_WKUP); } if (int_status & USB_ISTR_SUSP) @@ -540,11 +545,11 @@ void dcd_fs_irqHandler(void) { USB->CNTR |= USB_CNTR_LPMODE; /* clear of the ISTR bit must be done after setting of CNTR_FSUSP */ - USB->ISTR &= ~USB_ISTR_SUSP; + reg16_clear_bits(&USB->ISTR, USB_ISTR_SUSP); } if(int_status & USB_ISTR_SOF) { - USB->ISTR &= ~USB_ISTR_SOF; + reg16_clear_bits(&USB->ISTR, USB_ISTR_SOF); dcd_event_bus_signal(0, DCD_EVENT_SOF, true); } } @@ -714,7 +719,7 @@ void dcd_edpt_clear_stall (uint8_t rhport, uint8_t ep_addr) * @param wNBytes no. of bytes to be copied. * @retval None */ -static void dcd_write_packet_memory(uint16_t dst, const void *__restrict src, size_t wNBytes) +static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, size_t wNBytes) { uint32_t n = ((uint32_t)((uint32_t)wNBytes + 1U)) >> 1U; uint32_t i; @@ -722,11 +727,12 @@ static void dcd_write_packet_memory(uint16_t dst, const void *__restrict src, si 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(); +# if (DCD_STM32_BTABLE_BASE > 0u) + TU_ASSERT(dst >= DCD_STM32_BTABLE_BASE); +# endif + TU_ASSERT(((dst%2) == 0) && (dst + wNBytes) <= (DCD_STM32_BTABLE_BASE + DCD_STM32_BTABLE_LENGTH)); #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; @@ -743,6 +749,7 @@ static void dcd_write_packet_memory(uint16_t dst, const void *__restrict src, si pdwVal += PMA_STRIDE; srcVal++; } + return true; } /** @@ -751,7 +758,7 @@ static void dcd_write_packet_memory(uint16_t dst, const void *__restrict src, si * @param wNBytes no. of bytes to be copied. * @retval None */ -static void dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wNBytes) +static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wNBytes) { uint32_t n = (uint32_t)wNBytes >> 1U; uint32_t i; @@ -761,12 +768,13 @@ static void dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wN uint32_t temp; #ifdef DEBUG - if((src%2) != 0 || - (src < DCD_STM32_BTABLE_BASE) || - src >= (DCD_STM32_BTABLE_BASE + DCD_STM32_BTABLE_LENGTH)) - while(1) TU_BREAKPOINT(); +# if (DCD_STM32_BTABLE_BASE > 0u) + TU_ASSERT(src >= DCD_STM32_BTABLE_BASE); +# endif + TU_ASSERT(((src%2) == 0) && (src + wNBytes) <= (DCD_STM32_BTABLE_BASE + DCD_STM32_BTABLE_LENGTH)); #endif + pdwVal = &pma[PMA_STRIDE*(src>>1)]; uint8_t *dstVal = (uint8_t*)dst; @@ -784,6 +792,7 @@ static void dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wN pdwVal += PMA_STRIDE; *dstVal++ = ((temp >> 0) & 0xFF); } + return true; } diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev_pvt_st.h b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev_pvt_st.h index 4f46e81c4..586d94abf 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev_pvt_st.h +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev_pvt_st.h @@ -271,9 +271,6 @@ static __IO uint16_t * const pma = (__IO uint16_t*)USB_PMAADDR; #define PCD_CLEAR_EP_KIND(USBx, bEpNum) (PCD_SET_ENDPOINT((USBx), (bEpNum), \ (USB_EP_CTR_RX|USB_EP_CTR_TX|((((uint32_t)(PCD_GET_ENDPOINT((USBx), (bEpNum)))) & USB_EPKIND_MASK))))) - -#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) @@ -284,5 +281,7 @@ static __IO uint16_t * const pma = (__IO uint16_t*)USB_PMAADDR; #define USB_ISTR_ALL_EVENTS (USB_ISTR_PMAOVR | USB_ISTR_ERR | USB_ISTR_WKUP | USB_ISTR_SUSP | \ USB_ISTR_RESET | USB_ISTR_SOF | USB_ISTR_ESOF | USB_ISTR_L1REQ_FORCED ) +// Number of endpoints in hardware +#define STFSDEV_EP_COUNT (8) #endif /* PORTABLE_ST_STM32F0_DCD_STM32F0_FSDEV_PVT_ST_H_ */