From af8609e96e269a423449cfb679977c150494c3a4 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 1 Aug 2024 18:36:28 +0700 Subject: fsdev improve ep bit manipulation --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 46 +++++++++++++-------------- src/portable/st/stm32_fsdev/fsdev_type.h | 17 +++++----- 2 files changed, 31 insertions(+), 32 deletions(-) (limited to 'src') diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index cff328418..d1c40deb6 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -353,11 +353,11 @@ static void handle_ctr_rx(uint32_t ep_id) { dcd_event_setup_received(0, (uint8_t*) setup_packet, true); // Reset EP to NAK (in case it had been stalling) - ep_reg = ep_add_status(ep_reg, TUSB_DIR_IN, EP_STAT_NAK); - ep_reg = ep_add_status(ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); + ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_NAK); + ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); - ep_reg = ep_add_dtog(ep_reg, TUSB_DIR_IN, 1); - ep_reg = ep_add_dtog(ep_reg, TUSB_DIR_OUT, 1); + ep_add_dtog(&ep_reg, TUSB_DIR_IN, 1); + ep_add_dtog(&ep_reg, TUSB_DIR_OUT, 1); } else { ep_reg &= USB_EPREG_MASK; // reversed all toggle } @@ -395,14 +395,14 @@ static void handle_ctr_rx(uint32_t ep_id) { // prepared for status packet btable_set_rx_bufsize(ep_id, BTABLE_BUF_RX, CFG_TUD_ENDPOINT0_SIZE); } - ep_reg = ep_add_status(ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); + ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); } else { // Set endpoint active again for receiving more data. Note that isochronous endpoints stay active always if (!is_iso) { uint16_t const cnt = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size); btable_set_rx_bufsize(ep_id, BTABLE_BUF_RX, cnt); } - ep_reg = ep_add_status(ep_reg, TUSB_DIR_OUT, EP_STAT_VALID); + ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_VALID); } } @@ -579,8 +579,8 @@ void edpt0_open(uint8_t rhport) { uint32_t ep_reg = FSDEV_REG->ep[0].reg & ~USB_EPREG_MASK; ep_reg |= USB_EP_CONTROL; - ep_reg = ep_add_status(ep_reg, TUSB_DIR_IN, EP_STAT_NAK); - ep_reg = ep_add_status(ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); + ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_NAK); + ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); // no need to explicitly set DTOG bits since we aren't masked DTOG bit // prepare for setup packet @@ -623,8 +623,8 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) { xfer->max_packet_size = packet_size; xfer->ep_idx = ep_idx; - ep_reg = ep_add_status(ep_reg, dir, EP_STAT_NAK); - ep_reg = ep_add_dtog(ep_reg, dir, 0); + ep_add_status(&ep_reg, dir, EP_STAT_NAK); + ep_add_dtog(&ep_reg, dir, 0); // reserve other direction toggle bits if (dir == TUSB_DIR_IN) { @@ -694,10 +694,10 @@ bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) uint32_t ep_reg = FSDEV_REG->ep[0].reg & ~USB_EPREG_MASK; ep_reg |= tu_edpt_number(ep_addr) | USB_EP_ISOCHRONOUS | USB_EP_CTR_RX | USB_EP_CTR_TX; - ep_reg = ep_add_status(ep_reg, TUSB_DIR_IN, EP_STAT_DISABLED); - ep_reg = ep_add_status(ep_reg, TUSB_DIR_OUT, EP_STAT_DISABLED); - ep_reg = ep_add_dtog(ep_reg, dir, 0); - ep_reg = ep_add_dtog(ep_reg, 1-dir, 1); + ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_DISABLED); + ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_DISABLED); + ep_add_dtog(&ep_reg, dir, 0); + ep_add_dtog(&ep_reg, 1-dir, 1); ep_write(ep_idx, ep_reg); @@ -707,7 +707,7 @@ bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) // Currently, single-buffered, and only 64 bytes at a time (max) static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) { uint16_t len = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size); - uint16_t ep_reg = ep_read(ep_ix) | EP_CTR_TXRX; + uint32_t ep_reg = ep_read(ep_ix) | EP_CTR_TXRX; bool const is_iso = ep_is_iso(ep_reg); uint8_t buf_id; @@ -727,8 +727,8 @@ static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) { xfer->queued_len = (uint16_t)(xfer->queued_len + len); ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_IN); - ep_reg = ep_add_status(ep_reg, TUSB_DIR_IN, EP_STAT_VALID); - ep_reg = ep_clear_ctr(ep_reg, TUSB_DIR_IN); + ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_VALID); + ep_clear_ctr(&ep_reg, TUSB_DIR_IN); dcd_int_disable(0); ep_write(ep_ix, ep_reg); @@ -748,9 +748,9 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_num, uint8_t dir) { dcd_transmit_packet(xfer, ep_idx); } else { uint32_t cnt = (uint32_t) tu_min16(xfer->total_len, xfer->max_packet_size); - uint16_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX; - ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); // keep CTR TX, clear CTR RX - ep_reg = ep_add_status(ep_reg, dir, EP_STAT_VALID); + uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX; // keep CTR TX, clear CTR RX + ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); + ep_add_status(&ep_reg, dir, EP_STAT_VALID); if (ep_is_iso(ep_reg)) { btable_set_rx_bufsize(ep_idx, 0, cnt); @@ -801,7 +801,7 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { uint32_t ep_reg = ep_read(ep_idx); ep_reg |= USB_EP_CTR_RX | USB_EP_CTR_TX; // reserve CTR bits ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); - ep_reg = ep_add_status(ep_reg, dir, EP_STAT_STALL); + ep_add_status(&ep_reg, dir, EP_STAT_STALL); ep_write(ep_idx, ep_reg); } @@ -818,9 +818,9 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir) | EP_DTOG_MASK(dir); if (!ep_is_iso(ep_reg)) { - ep_reg = ep_add_status(ep_reg, dir, EP_STAT_NAK); + ep_add_status(&ep_reg, dir, EP_STAT_NAK); } - ep_reg = ep_add_dtog(ep_reg, dir, 0); // Reset to DATA0 + ep_add_dtog(&ep_reg, dir, 0); // Reset to DATA0 ep_write(ep_idx, ep_reg); } diff --git a/src/portable/st/stm32_fsdev/fsdev_type.h b/src/portable/st/stm32_fsdev/fsdev_type.h index e4a0f3f28..a44d8b35b 100644 --- a/src/portable/st/stm32_fsdev/fsdev_type.h +++ b/src/portable/st/stm32_fsdev/fsdev_type.h @@ -75,10 +75,9 @@ enum { // Buffer Table is located in Packet Memory Area (PMA) and therefore its address access is forced to either // 16-bit or 32-bit depending on FSDEV_BUS_32BIT. +// 0: TX (IN), 1: RX (OUT) typedef union { - // 0: TX (IN), 1: RX (OUT) - - // strictly 16-bit access (could be 32-bit aligned) + // data is strictly 16-bit access (address could be 32-bit aligned) struct { volatile pma_aligned uint16_t addr; volatile pma_aligned uint16_t count; @@ -181,16 +180,16 @@ TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_read(uint32_t ep_id) { return FSDEV_REG->ep[ep_id].reg; } -TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_add_status(uint32_t reg, tusb_dir_t dir, ep_stat_t state) { - return reg ^ (state << (USB_EPTX_STAT_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); +TU_ATTR_ALWAYS_INLINE static inline void ep_add_status(uint32_t* reg, tusb_dir_t dir, ep_stat_t state) { + *reg ^= (state << (USB_EPTX_STAT_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); } -TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_add_dtog(uint32_t reg, tusb_dir_t dir, uint8_t state) { - return reg ^ (state << (USB_EP_DTOG_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); +TU_ATTR_ALWAYS_INLINE static inline void ep_add_dtog(uint32_t* reg, tusb_dir_t dir, uint8_t state) { + *reg ^= (state << (USB_EP_DTOG_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); } -TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_clear_ctr(uint32_t reg, tusb_dir_t dir) { - return reg & ~(1 << (USB_EP_CTR_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); +TU_ATTR_ALWAYS_INLINE static inline void ep_clear_ctr(uint32_t* reg, tusb_dir_t dir) { + *reg &= ~(1 << (USB_EP_CTR_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); } TU_ATTR_ALWAYS_INLINE static inline bool ep_is_iso(uint32_t reg) { -- cgit v1.3.1 From e180d915c64c98ce7349cf4696f23abd55a91e19 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 1 Aug 2024 23:08:12 +0700 Subject: read/write packet enhancement, merge 16-bit and 32-bit together --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 139 +++++++++----------------- src/portable/st/stm32_fsdev/fsdev_type.h | 24 +++-- 2 files changed, 60 insertions(+), 103 deletions(-) (limited to 'src') diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index d1c40deb6..2aa776dbb 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -825,121 +825,76 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { ep_write(ep_idx, ep_reg); } -#ifdef FSDEV_BUS_32BIT -static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, uint16_t wNBytes) { - const uint8_t *src8 = src; - volatile uint32_t *pma32 = (volatile uint32_t *)(USB_PMAADDR + dst); - - for (uint32_t n = wNBytes / 4; n > 0; --n) { - *pma32++ = tu_unaligned_read32(src8); - src8 += 4; - } - - uint16_t odd = wNBytes & 0x03; - if (odd) { - uint32_t wrVal = *src8; - odd--; - - if (odd) { - wrVal |= *++src8 << 8; - odd--; - - if (odd) { - wrVal |= *++src8 << 16; - } - } - - *pma32 = wrVal; - } +// Write to packet memory area (PMA) from user memory +// - Packet memory must be either strictly 16-bit or 32-bit depending on FSDEV_BUS_32BIT +// - Uses unaligned for RAM (since M0 cannot access unaligned address) +static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, uint16_t nbytes) { + enum { BUS_SIZE = sizeof(fsdev_bus_t) }; + uint32_t n_write = nbytes / BUS_SIZE; - return true; -} + fsdev_pma_buf_t* pma_buf = PMA_BUF_AT(dst); + const uint8_t *src8 = src; -static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, uint16_t wNBytes) { - uint8_t *dst8 = dst; - volatile uint32_t *src32 = (volatile uint32_t *)(USB_PMAADDR + src); + while (n_write--) { + #ifdef FSDEV_BUS_32BIT + pma_buf->value = tu_unaligned_read32(src8); + #else + pma_buf->value = tu_unaligned_read16(src8); + #endif - for (uint32_t n = wNBytes / 4; n > 0; --n) { - tu_unaligned_write32(dst8, *src32++); - dst8 += 4; + src8 += BUS_SIZE; + pma_buf++; } - uint16_t odd = wNBytes & 0x03; + // odd bytes e.g 1 for 16-bit or 1-3 for 32-bit + uint16_t odd = nbytes & (BUS_SIZE - 1); if (odd) { - uint32_t rdVal = *src32; - - *dst8 = tu_u32_byte0(rdVal); - odd--; - - if (odd) { - *++dst8 = tu_u32_byte1(rdVal); - odd--; - - if (odd) { - *++dst8 = tu_u32_byte2(rdVal); - } + fsdev_bus_t temp = 0; + for(uint16_t i = 0; i < odd; i++) { + temp |= *src8++ << (i * 8); } + pma_buf->value = temp; } return true; } -#else -// Packet buffer access can only be 8- or 16-bit. -/** - * @brief Copy a buffer from user memory area to packet memory area (PMA). - * This uses un-aligned for user memory and 16-bit access for packet memory. - * @param dst, byte address in PMA; must be 16-bit aligned - * @param src pointer to user memory area. - * @param wPMABufAddr address into PMA. - * @param nbytes no. of bytes to be copied. - * @retval None - */ -static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, uint16_t nbytes) { - uint32_t n16 = (uint32_t)nbytes >> 1U; - const uint8_t *src8 = src; - fsdev_pma16_t* pma16 = (fsdev_pma16_t*) (USB_PMAADDR + FSDEV_PMA_STRIDE * dst); - - while (n16--) { - pma16->u16 = tu_unaligned_read16(src8); - src8 += 2; - pma16++; - } +// Read from packet memory area (PMA) to user memory. +// - Packet memory must be either strictly 16-bit or 32-bit depending on FSDEV_BUS_32BIT +// - Uses unaligned for RAM (since M0 cannot access unaligned address) +static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, uint16_t nbytes) { + enum { BUS_SIZE = sizeof(fsdev_bus_t) }; + uint32_t n_write = nbytes / BUS_SIZE; - if (nbytes & 0x01) { - pma16->u16 = (uint16_t) *src8; - } + fsdev_pma_buf_t* pma_buf = PMA_BUF_AT(src); + uint8_t *dst8 = (uint8_t *)dst; - return true; -} + while (n_write--) { + fsdev_bus_t temp = pma_buf->value; -/** - * @brief Copy a buffer from packet memory area (PMA) to user memory area. - * Uses unaligned for system memory and 16-bit access of packet memory - * @param nbytes no. of bytes to be copied. - * @retval None - */ -static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, uint16_t nbytes) { - uint32_t n16 = (uint32_t)nbytes >> 1U; - fsdev_pma16_t* pma16 = (fsdev_pma16_t*) (USB_PMAADDR + FSDEV_PMA_STRIDE * src); - uint8_t *dst8 = (uint8_t *)dst; + #ifdef FSDEV_BUS_32BIT + tu_unaligned_write32(dst8, temp); + #else + tu_unaligned_write16(dst8, temp); + #endif - while (n16--) { - uint16_t temp16 = pma16->u16; - tu_unaligned_write16(dst8, temp16); - dst8 += 2; - pma16++; + dst8 += BUS_SIZE; + pma_buf++; } - if (nbytes & 0x01) { - *dst8++ = tu_u16_low(pma16->u16); + // odd bytes e.g 1 for 16-bit or 1-3 for 32-bit + uint16_t odd = nbytes & (BUS_SIZE - 1); + if (odd) { + fsdev_bus_t temp = pma_buf->value; + while (odd--) { + *dst8++ = (uint8_t) (temp & 0xfful); + temp >>= 8; + } } return true; } -#endif - /** * @brief Copy from FIFO to packet memory area (PMA). * Uses byte-access of system memory and 16-bit access of packet memory diff --git a/src/portable/st/stm32_fsdev/fsdev_type.h b/src/portable/st/stm32_fsdev/fsdev_type.h index a44d8b35b..cac3b4386 100644 --- a/src/portable/st/stm32_fsdev/fsdev_type.h +++ b/src/portable/st/stm32_fsdev/fsdev_type.h @@ -1,9 +1,8 @@ /* * The MIT License (MIT) * - * Copyright(c) 2016 STMicroelectronics * Copyright(c) N Conrad - * Copyright (c) 2024, hathach (tinyusb.org) + * Copyright(c) 2024, hathach (tinyusb.org) * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -23,6 +22,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. * + * This file is part of the TinyUSB stack. */ #ifndef TUSB_FSDEV_TYPE_H @@ -62,6 +62,13 @@ TU_VERIFY_STATIC(FSDEV_BTABLE_BASE % 8 == 0, "BTABLE base must be aligned to 8 b #define pma_aligned #endif +// The fsdev_bus_t type can be used for both register and PMA access necessities +#ifdef FSDEV_BUS_32BIT + typedef uint32_t fsdev_bus_t; +#else + typedef uint16_t fsdev_bus_t; +#endif + //--------------------------------------------------------------------+ // BTable Typedef //--------------------------------------------------------------------+ @@ -95,8 +102,10 @@ TU_VERIFY_STATIC(FSDEV_BTABLE_BASE + FSDEV_EP_COUNT*8 <= FSDEV_PMA_SIZE, "BTABLE #define FSDEV_BTABLE ((volatile fsdev_btable_t*) (USB_PMAADDR+FSDEV_BTABLE_BASE)) typedef struct { - volatile pma_aligned uint16_t u16; -} fsdev_pma16_t; + volatile pma_aligned fsdev_bus_t value; +} fsdev_pma_buf_t; + +#define PMA_BUF_AT(_addr) ((fsdev_pma_buf_t*) (USB_PMAADDR + FSDEV_PMA_STRIDE*(_addr))) //--------------------------------------------------------------------+ // Registers Typedef @@ -105,13 +114,6 @@ typedef struct { // volatile 32-bit aligned #define _va32 volatile TU_ATTR_ALIGNED(4) -// The fsdev_bus_t type can be used for both register and PMA access necessities -#ifdef FSDEV_BUS_32BIT -typedef uint32_t fsdev_bus_t; -#else -typedef uint16_t fsdev_bus_t; -#endif - typedef struct { struct { _va32 fsdev_bus_t reg; -- cgit v1.3.1 From 91e5a066c5b259c38d663d21e3485b2feecb779d Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 2 Aug 2024 17:12:28 +0700 Subject: more fsdev clean up hil test boards in parallel --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 8 -- src/portable/st/stm32_fsdev/fsdev_ch32.h | 18 +--- src/portable/st/stm32_fsdev/fsdev_stm32.h | 50 +++++------ src/portable/st/stm32_fsdev/fsdev_type.h | 29 ++++--- test/hil/hil_test.py | 120 +++++++++++++------------- 5 files changed, 102 insertions(+), 123 deletions(-) (limited to 'src') diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 2aa776dbb..cbd240ca1 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -114,8 +114,6 @@ #include "device/dcd.h" #if defined(TUP_USBIP_FSDEV_STM32) - // Undefine to reduce the dependence on HAL - #undef USE_HAL_DRIVER #include "fsdev_stm32.h" #elif defined(TUP_USBIP_FSDEV_CH32) #include "fsdev_ch32.h" @@ -125,12 +123,6 @@ #include "fsdev_type.h" -//--------------------------------------------------------------------+ -// Configuration -//--------------------------------------------------------------------+ - - - //--------------------------------------------------------------------+ // MACRO CONSTANT TYPEDEF //--------------------------------------------------------------------+ diff --git a/src/portable/st/stm32_fsdev/fsdev_ch32.h b/src/portable/st/stm32_fsdev/fsdev_ch32.h index 8c0961bb9..518197c47 100644 --- a/src/portable/st/stm32_fsdev/fsdev_ch32.h +++ b/src/portable/st/stm32_fsdev/fsdev_ch32.h @@ -54,22 +54,8 @@ #endif #define FSDEV_PMA_SIZE (512u) -#define FSDEV_REG_BASE 0x40005C00UL - -#define USB_BASE (APB1PERIPH_BASE + 0x00005C00UL) /*!< USB_IP Peripheral Registers base address */ -#define USB_PMAADDR (APB1PERIPH_BASE + 0x00006000UL) /*!< USB_IP Packet Memory Area base address */ -#define USB ((USB_TypeDef *)USB_BASE) - -/******************************************************************************/ -/* */ -/* USB Device General registers */ -/* */ -/******************************************************************************/ -#define USB_CNTR (USB_BASE + 0x40U) /*!< Control register */ -#define USB_ISTR (USB_BASE + 0x44U) /*!< Interrupt status register */ -#define USB_FNR (USB_BASE + 0x48U) /*!< Frame number register */ -#define USB_DADDR (USB_BASE + 0x4CU) /*!< Device address register */ -#define USB_BTABLE (USB_BASE + 0x50U) /*!< Buffer Table address register */ +#define FSDEV_REG_BASE (APB1PERIPH_BASE + 0x00005C00UL) +#define FSDEV_PMA_BASE (APB1PERIPH_BASE + 0x00006000UL) /**************************** ISTR interrupt events *************************/ #define USB_ISTR_CTR ((uint16_t)0x8000U) /*!< Correct TRansfer (clear-only bit) */ diff --git a/src/portable/st/stm32_fsdev/fsdev_stm32.h b/src/portable/st/stm32_fsdev/fsdev_stm32.h index bb2c72fd1..99fe8d55f 100644 --- a/src/portable/st/stm32_fsdev/fsdev_stm32.h +++ b/src/portable/st/stm32_fsdev/fsdev_stm32.h @@ -82,12 +82,9 @@ #elif CFG_TUSB_MCU == OPT_MCU_STM32G0 #include "stm32g0xx.h" - #define FSDEV_BUS_32BIT #define FSDEV_PMA_SIZE (2048u) - #undef USB_PMAADDR - #define USB_PMAADDR USB_DRD_PMAADDR - #define USB_TypeDef USB_DRD_TypeDef - #define EP0R CHEP0R + #define USB USB_DRD_FS + #define USB_EP_CTR_RX USB_EP_VTRX #define USB_EP_CTR_TX USB_EP_VTTX #define USB_EP_T_FIELD USB_CHEP_UTYPE @@ -100,7 +97,6 @@ #define USB_EPRX_DTOG2 USB_CHEP_RX_DTOG2 #define USB_EPRX_STAT USB_CH_RX_VALID #define USB_EPKIND_MASK USB_EP_KIND_MASK - #define USB USB_DRD_FS #define USB_CNTR_FRES USB_CNTR_USBRST #define USB_CNTR_RESUME USB_CNTR_L2RES #define USB_ISTR_EP_ID USB_ISTR_IDN @@ -110,17 +106,9 @@ #elif CFG_TUSB_MCU == OPT_MCU_STM32H5 #include "stm32h5xx.h" - #define FSDEV_BUS_32BIT - - #if !defined(USB_DRD_BASE) && defined(USB_DRD_FS_BASE) - #define USB_DRD_BASE USB_DRD_FS_BASE - #endif - #define FSDEV_PMA_SIZE (2048u) - #undef USB_PMAADDR - #define USB_PMAADDR USB_DRD_PMAADDR - #define USB_TypeDef USB_DRD_TypeDef - #define EP0R CHEP0R + #define USB USB_DRD_FS + #define USB_EP_CTR_RX USB_EP_VTRX #define USB_EP_CTR_TX USB_EP_VTTX #define USB_EP_T_FIELD USB_CHEP_UTYPE @@ -133,7 +121,6 @@ #define USB_EPRX_DTOG2 USB_CHEP_RX_DTOG2 #define USB_EPRX_STAT USB_CH_RX_VALID #define USB_EPKIND_MASK USB_EP_KIND_MASK - #define USB USB_DRD_FS #define USB_CNTR_FRES USB_CNTR_USBRST #define USB_CNTR_RESUME USB_CNTR_L2RES #define USB_ISTR_EP_ID USB_ISTR_IDN @@ -144,9 +131,8 @@ #elif CFG_TUSB_MCU == OPT_MCU_STM32WB #include "stm32wbxx.h" #define FSDEV_PMA_SIZE (1024u) - /* ST provided header has incorrect value */ - #undef USB_PMAADDR - #define USB_PMAADDR USB1_PMAADDR + /* ST provided header has incorrect value of USB_PMAADDR */ + #define FSDEV_PMA_BASE USB1_PMAADDR #elif CFG_TUSB_MCU == OPT_MCU_STM32L4 #include "stm32l4xx.h" @@ -162,13 +148,9 @@ #elif CFG_TUSB_MCU == OPT_MCU_STM32U5 #include "stm32u5xx.h" - #define FSDEV_BUS_32BIT - #define FSDEV_PMA_SIZE (2048u) - #undef USB_PMAADDR - #define USB_PMAADDR USB_DRD_PMAADDR - #define USB_TypeDef USB_DRD_TypeDef - #define EP0R CHEP0R + #define USB USB_DRD_FS + #define USB_EP_CTR_RX USB_EP_VTRX #define USB_EP_CTR_TX USB_EP_VTTX #define USB_EP_T_FIELD USB_CHEP_UTYPE @@ -181,7 +163,6 @@ #define USB_EPRX_DTOG2 USB_CHEP_RX_DTOG2 #define USB_EPRX_STAT USB_CH_RX_VALID #define USB_EPKIND_MASK USB_EP_KIND_MASK - #define USB USB_DRD_FS #define USB_CNTR_FRES USB_CNTR_USBRST #define USB_CNTR_RESUME USB_CNTR_L2RES #define USB_ISTR_EP_ID USB_ISTR_IDN @@ -194,6 +175,10 @@ // This includes U0 #endif +//--------------------------------------------------------------------+ +// Register and PMA Base Address +//--------------------------------------------------------------------+ +#ifndef FSDEV_REG_BASE #if defined(USB_BASE) #define FSDEV_REG_BASE USB_BASE #elif defined(USB_DRD_BASE) @@ -203,6 +188,17 @@ #else #error "FSDEV_REG_BASE not defined" #endif +#endif + +#ifndef FSDEV_PMA_BASE +#if defined(USB_PMAADDR) + #define FSDEV_PMA_BASE USB_PMAADDR +#elif defined(USB_DRD_PMAADDR) + #define FSDEV_PMA_BASE USB_DRD_PMAADDR +#else + #error "FSDEV_PMA_BASE not defined" +#endif +#endif // This checks if the device has "LPM" #if defined(USB_ISTR_L1REQ) diff --git a/src/portable/st/stm32_fsdev/fsdev_type.h b/src/portable/st/stm32_fsdev/fsdev_type.h index cac3b4386..bdff2c940 100644 --- a/src/portable/st/stm32_fsdev/fsdev_type.h +++ b/src/portable/st/stm32_fsdev/fsdev_type.h @@ -49,17 +49,18 @@ TU_VERIFY_STATIC(FSDEV_BTABLE_BASE % 8 == 0, "BTABLE base must be aligned to 8 b // For purposes of accessing the packet #if FSDEV_PMA_SIZE == 512 - #define FSDEV_PMA_STRIDE (2u) // 1x16 bit access scheme - #define pma_aligned TU_ATTR_ALIGNED(4) + // 1x16 bit / word access scheme + #define FSDEV_PMA_STRIDE 2 + #define pma_access_scheme TU_ATTR_ALIGNED(4) #elif FSDEV_PMA_SIZE == 1024 - #define FSDEV_PMA_STRIDE (1u) // 2x16 bit access scheme - #define pma_aligned + // 2x16 bit / word access scheme + #define FSDEV_PMA_STRIDE 1 + #define pma_access_scheme #elif FSDEV_PMA_SIZE == 2048 - #ifndef FSDEV_BUS_32BIT - #warning "FSDEV_PMA_SIZE is 2048, but FSDEV_BUS_32BIT is not defined" - #endif - #define FSDEV_PMA_STRIDE (1u) // 32 bit access scheme - #define pma_aligned + // 32 bit access scheme + #define FSDEV_BUS_32BIT + #define FSDEV_PMA_STRIDE 1 + #define pma_access_scheme #endif // The fsdev_bus_t type can be used for both register and PMA access necessities @@ -86,8 +87,8 @@ enum { typedef union { // data is strictly 16-bit access (address could be 32-bit aligned) struct { - volatile pma_aligned uint16_t addr; - volatile pma_aligned uint16_t count; + volatile pma_access_scheme uint16_t addr; + volatile pma_access_scheme uint16_t count; } ep16[FSDEV_EP_COUNT][2]; // strictly 32-bit access @@ -99,13 +100,13 @@ typedef union { TU_VERIFY_STATIC(sizeof(fsdev_btable_t) == FSDEV_EP_COUNT*8*FSDEV_PMA_STRIDE, "size is not correct"); TU_VERIFY_STATIC(FSDEV_BTABLE_BASE + FSDEV_EP_COUNT*8 <= FSDEV_PMA_SIZE, "BTABLE does not fit in PMA RAM"); -#define FSDEV_BTABLE ((volatile fsdev_btable_t*) (USB_PMAADDR+FSDEV_BTABLE_BASE)) +#define FSDEV_BTABLE ((volatile fsdev_btable_t*) (FSDEV_PMA_BASE + FSDEV_PMA_STRIDE*(FSDEV_BTABLE_BASE))) typedef struct { - volatile pma_aligned fsdev_bus_t value; + volatile pma_access_scheme fsdev_bus_t value; } fsdev_pma_buf_t; -#define PMA_BUF_AT(_addr) ((fsdev_pma_buf_t*) (USB_PMAADDR + FSDEV_PMA_STRIDE*(_addr))) +#define PMA_BUF_AT(_addr) ((fsdev_pma_buf_t*) (FSDEV_PMA_BASE + FSDEV_PMA_STRIDE*(_addr))) //--------------------------------------------------------------------+ // Registers Typedef diff --git a/test/hil/hil_test.py b/test/hil/hil_test.py index bdc48591e..6b8ba0675 100644 --- a/test/hil/hil_test.py +++ b/test/hil/hil_test.py @@ -34,6 +34,7 @@ import subprocess import json import glob import platform +from multiprocessing import Pool # for RPI double reset if platform.machine() == 'aarch64': @@ -130,10 +131,11 @@ def run_cmd(cmd): def flash_jlink(board, firmware): script = ['halt', 'r', f'loadfile {firmware}.elf', 'r', 'go', 'exit'] - with open('flash.jlink', 'w') as f: + f_jlink = f'{board["name"]}_{os.path.basename(firmware)}.jlink' + with open(f_jlink, 'w') as f: f.writelines(f'{s}\n' for s in script) - ret = run_cmd(f'JLinkExe -USB {board["flasher_sn"]} {board["flasher_args"]} -if swd -JTAGConf -1,-1 -speed auto -NoGui 1 -ExitOnError 1 -CommandFile flash.jlink') - os.remove('flash.jlink') + ret = run_cmd(f'JLinkExe -USB {board["flasher_sn"]} {board["flasher_args"]} -if swd -JTAGConf -1,-1 -speed auto -NoGui 1 -ExitOnError 1 -CommandFile {f_jlink}') + os.remove(f_jlink) return ret @@ -327,6 +329,61 @@ def test_hid_composite_freertos(id): # ------------------------------------------------------------- # Main # ------------------------------------------------------------- +# all possible tests: board_test is added last to disable board's usb +all_tests = [ + 'cdc_dual_ports', + 'cdc_msc', + #'cdc_msc_freertos', + 'dfu', + #'dfu_runtime', + 'hid_boot_interface', + 'board_test' +] + + +def test_board(item): + name = item['name'] + flasher = item['flasher'].lower() + + # default to all tests + if 'tests' in item: + test_list = item['tests'] + ['board_test'] + else: + test_list = list(all_tests) + + # remove skip_tests + if 'tests_skip' in item: + for skip in item['tests_skip']: + if skip in test_list: + test_list.remove(skip) + + for test in test_list: + fw_dir = f'cmake-build/cmake-build-{name}/device/{test}' + if not os.path.exists(fw_dir): + fw_dir = f'examples/cmake-build-{name}/device/{test}' + fw_name = f'{fw_dir}/{test}' + print(f'{name:20} {test:20} ... ', end='') + + if not os.path.exists(fw_dir): + print('Skip') + continue + + # flash firmware. It may fail randomly, retry a few times + for i in range(3): + ret = globals()[f'flash_{flasher}'](item, fw_name) + if ret.returncode == 0: + break + else: + print(f'Flashing failed, retry {i+1}') + time.sleep(1) + + assert ret.returncode == 0, 'Flash failed\n' + ret.stdout.decode() + + # run test + globals()[f'test_{test}'](item['uid']) + print('OK') + + def main(): """ Hardware test on specified boards @@ -345,66 +402,13 @@ def main(): with open(config_file) as f: config = json.load(f) - # all possible tests: board_test is added last to disable board's usb - all_tests = [ - 'cdc_dual_ports', - 'cdc_msc', - 'cdc_msc_freertos', - 'dfu', - 'dfu_runtime', - 'hid_boot_interface', - 'board_test' - ] - if len(boards) == 0: config_boards = config['boards'] else: config_boards = [e for e in config['boards'] if e['name'] in boards] - for item in config_boards: - name = item['name'] - print(f'Testing board:{name}') - flasher = item['flasher'].lower() - - # default to all tests - if 'tests' in item: - test_list = item['tests'] + ['board_test'] - else: - test_list = list(all_tests) - - # remove skip_tests - if 'tests_skip' in item: - for skip in item['tests_skip']: - if skip in test_list: - test_list.remove(skip) - - for test in test_list: - fw_dir = f'cmake-build/cmake-build-{name}/device/{test}' - if not os.path.exists(fw_dir): - fw_dir = f'examples/cmake-build-{name}/device/{test}' - fw_name = f'{fw_dir}/{test}' - print(f' {test} ... ', end='') - sys.stdout.flush() - - if not os.path.exists(fw_dir): - print('Skip') - continue - - # flash firmware. It may fail randomly, retry a few times - for i in range(3): - ret = globals()[f'flash_{flasher}'](item, fw_name) - if ret.returncode == 0: - break - else: - print(f'Flashing failed, retry {i+1}') - time.sleep(1) - - assert ret.returncode == 0, 'Flash failed\n' + ret.stdout.decode() - - # run test - globals()[f'test_{test}'](item['uid']) - - print('OK') + with Pool(processes=os.cpu_count()) as pool: + pool.map(test_board, config_boards) if __name__ == '__main__': -- cgit v1.3.1 From 315dae6a85cbb3df5b30429b28d8c74130f44db6 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 5 Aug 2024 17:43:27 +0700 Subject: finally fixed fsdev setup handling, which cause race condition for ch32v203 --- src/device/usbd.c | 1 - src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 59 ++++++++++++++------------- src/portable/st/stm32_fsdev/fsdev_type.h | 3 -- 3 files changed, 30 insertions(+), 33 deletions(-) (limited to 'src') diff --git a/src/device/usbd.c b/src/device/usbd.c index 6d27a3735..7089e9cf1 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1201,7 +1201,6 @@ TU_ATTR_FAST_FUNC void dcd_event_handler(dcd_event_t const* event, bool in_isr) break; case DCD_EVENT_SETUP_RECEIVED: - // TU_ASSERT(event->setup_received.bRequest != 0,); // for catching issue with ch32v203 and windows with -O0/-Og _usbd_queued_setup++; send = true; break; diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index cbd240ca1..43636198a 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -147,7 +147,6 @@ typedef struct { static xfer_ctl_t xfer_status[CFG_TUD_ENDPPOINT_MAX][2]; static ep_alloc_t ep_alloc_status[FSDEV_EP_COUNT]; - static uint8_t remoteWakeCountdown; // When wake is requested //--------------------------------------------------------------------+ @@ -171,6 +170,10 @@ static bool dcd_read_packet_memory_ff(tu_fifo_t *ff, uint16_t src, uint16_t wNBy static void edpt0_open(uint8_t rhport); +TU_ATTR_ALWAYS_INLINE static inline void edpt0_prepare_setup(void) { + btable_set_rx_bufsize(0, BTABLE_BUF_RX, 8); +} + //--------------------------------------------------------------------+ // Inline helper //--------------------------------------------------------------------+ @@ -340,21 +343,24 @@ static void handle_ctr_rx(uint32_t ep_id) { // Setup packet should always be 8 bytes. If not, ignore it, and try again. if (count == 8) { uint16_t rx_addr = btable_get_addr(ep_id, BTABLE_BUF_RX); - uint32_t setup_packet[2]; + uint8_t setup_packet[8] TU_ATTR_ALIGNED(4); + dcd_read_packet_memory(setup_packet, rx_addr, 8); dcd_event_setup_received(0, (uint8_t*) setup_packet, true); - // Reset EP to NAK (in case it had been stalling) + // Reset EP to NAK and set both toggle to 1 ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_NAK); ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); - ep_add_dtog(&ep_reg, TUSB_DIR_IN, 1); ep_add_dtog(&ep_reg, TUSB_DIR_OUT, 1); } else { + // Missed an setup packet !!! + TU_BREAKPOINT(); ep_reg &= USB_EPREG_MASK; // reversed all toggle + edpt0_prepare_setup(); } } else { - ep_reg &= USB_EPRX_STAT | USB_EPREG_MASK; // reversed all toggle except RX Status + ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_OUT); // reversed all toggle except RX Status bool const is_iso = ep_is_iso(ep_reg); xfer_ctl_t *xfer = xfer_ctl_ptr(ep_num, TUSB_DIR_OUT); @@ -374,20 +380,13 @@ static void handle_ctr_rx(uint32_t ep_id) { } else { dcd_read_packet_memory(xfer->buffer + xfer->queued_len, pma_addr, rx_count); } - xfer->queued_len = (uint16_t)(xfer->queued_len + rx_count); } if ((rx_count < xfer->max_packet_size) || (xfer->queued_len == xfer->total_len)) { - uint8_t const ep_addr = ep_num; // all bytes received or short packet - dcd_event_xfer_complete(0, ep_addr, xfer->queued_len, XFER_RESULT_SUCCESS, true); - - if (ep_num == 0) { - // prepared for status packet - btable_set_rx_bufsize(ep_id, BTABLE_BUF_RX, CFG_TUD_ENDPOINT0_SIZE); - } ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); + dcd_event_xfer_complete(0, ep_num, xfer->queued_len, XFER_RESULT_SUCCESS, true); } else { // Set endpoint active again for receiving more data. Note that isochronous endpoints stay active always if (!is_iso) { @@ -485,6 +484,8 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const *req uint8_t const dev_addr = (uint8_t)request->wValue; FSDEV_REG->DADDR = (USB_DADDR_EF | dev_addr); } + + edpt0_prepare_setup(); } /*** @@ -569,15 +570,13 @@ void edpt0_open(uint8_t rhport) { btable_set_addr(0, BTABLE_BUF_RX, pma_addr0); btable_set_addr(0, BTABLE_BUF_TX, pma_addr1); - uint32_t ep_reg = FSDEV_REG->ep[0].reg & ~USB_EPREG_MASK; + uint32_t ep_reg = ep_read(0) & ~USB_EPREG_MASK; // only get toggle bits ep_reg |= USB_EP_CONTROL; ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_NAK); ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); // no need to explicitly set DTOG bits since we aren't masked DTOG bit - // prepare for setup packet - btable_set_rx_bufsize(0, BTABLE_BUF_RX, CFG_TUD_ENDPOINT0_SIZE); - + edpt0_prepare_setup(); // prepare for setup packet ep_write(0, ep_reg); } @@ -590,7 +589,7 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) { uint8_t const ep_idx = dcd_ep_alloc(ep_addr, desc_ep->bmAttributes.xfer); TU_ASSERT(ep_idx < FSDEV_EP_COUNT); - uint32_t ep_reg = FSDEV_REG->ep[ep_idx].reg & ~USB_EPREG_MASK; + uint32_t ep_reg = ep_read(ep_idx) & ~USB_EPREG_MASK; ep_reg |= tu_edpt_number(ep_addr) | USB_EP_CTR_RX | USB_EP_CTR_TX; // Set type @@ -684,7 +683,7 @@ bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) xfer->max_packet_size = tu_edpt_packet_size(desc_ep); - uint32_t ep_reg = FSDEV_REG->ep[0].reg & ~USB_EPREG_MASK; + uint32_t ep_reg = ep_read(ep_idx) & ~USB_EPREG_MASK; ep_reg |= tu_edpt_number(ep_addr) | USB_EP_ISOCHRONOUS | USB_EP_CTR_RX | USB_EP_CTR_TX; ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_DISABLED); ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_DISABLED); @@ -699,7 +698,7 @@ bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) // Currently, single-buffered, and only 64 bytes at a time (max) static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) { uint16_t len = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size); - uint32_t ep_reg = ep_read(ep_ix) | EP_CTR_TXRX; + uint32_t ep_reg = ep_read(ep_ix) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR bits bool const is_iso = ep_is_iso(ep_reg); uint8_t buf_id; @@ -709,14 +708,17 @@ static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) { buf_id = BTABLE_BUF_TX; } uint16_t addr_ptr = (uint16_t) btable_get_addr(ep_ix, buf_id); - btable_set_count(ep_ix, buf_id, len); - if (xfer->ff) { - dcd_write_packet_memory_ff(xfer->ff, addr_ptr, len); - } else { - dcd_write_packet_memory(addr_ptr, &(xfer->buffer[xfer->queued_len]), len); + if (len) { + if (xfer->ff) { + dcd_write_packet_memory_ff(xfer->ff, addr_ptr, len); + } else { + dcd_write_packet_memory(addr_ptr, &(xfer->buffer[xfer->queued_len]), len); + } + xfer->queued_len = (uint16_t) (xfer->queued_len + len); } - xfer->queued_len = (uint16_t)(xfer->queued_len + len); + + btable_set_count(ep_ix, buf_id, len); ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_IN); ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_VALID); @@ -790,8 +792,7 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { xfer_ctl_t *xfer = xfer_ctl_ptr(ep_num, dir); uint8_t const ep_idx = xfer->ep_idx; - uint32_t ep_reg = ep_read(ep_idx); - ep_reg |= USB_EP_CTR_RX | USB_EP_CTR_TX; // reserve CTR bits + uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR bits ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); ep_add_status(&ep_reg, dir, EP_STAT_STALL); @@ -806,7 +807,7 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { xfer_ctl_t *xfer = xfer_ctl_ptr(ep_num, dir); uint8_t const ep_idx = xfer->ep_idx; - uint32_t ep_reg = ep_read(ep_idx) | EP_CTR_TXRX; + uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR bits ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir) | EP_DTOG_MASK(dir); if (!ep_is_iso(ep_reg)) { diff --git a/src/portable/st/stm32_fsdev/fsdev_type.h b/src/portable/st/stm32_fsdev/fsdev_type.h index bdff2c940..8432df4b0 100644 --- a/src/portable/st/stm32_fsdev/fsdev_type.h +++ b/src/portable/st/stm32_fsdev/fsdev_type.h @@ -156,9 +156,6 @@ TU_VERIFY_STATIC(sizeof(fsdev_regs_t) == 0x5C, "Size is not correct"); #define USB_EP_CTR_TX_Pos 7u #endif - -#define EP_CTR_TXRX (USB_EP_CTR_TX | USB_EP_CTR_RX) - typedef enum { EP_STAT_DISABLED = 0, EP_STAT_STALL = 1, -- cgit v1.3.1 From d680424f62b0d677158af7219b319a24f5497d1c Mon Sep 17 00:00:00 2001 From: hathach Date: Tue, 6 Aug 2024 22:18:25 +0700 Subject: improve dcd_int_handler() - skip DIR and use CTR TX/RX to handle complete transfer - clear CTR first, except for setup which we need to get data first - separate handle_ctr_setup() --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 238 ++++++++++++-------------- src/portable/st/stm32_fsdev/fsdev_type.h | 26 ++- 2 files changed, 132 insertions(+), 132 deletions(-) (limited to 'src') diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 43636198a..97529997d 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -4,7 +4,6 @@ * Copyright (c) 2019 Nathan Conrad * * Portions: - * Copyright (c) 2016 STMicroelectronics * Copyright (c) 2019 Ha Thach (tinyusb.org) * Copyright (c) 2022 Simon Küppers (skuep) * Copyright (c) 2022 HiFiPhile @@ -215,10 +214,11 @@ void dcd_init(uint8_t rhport) { // Reset endpoints to disabled for (uint32_t i = 0; i < FSDEV_EP_COUNT; i++) { // This doesn't clear all bits since some bits are "toggle", but does set the type to DISABLED. - ep_write(i, 0u); + ep_write(i, 0u, false); } FSDEV_REG->CNTR |= USB_CNTR_RESETM | USB_CNTR_ESOFM | USB_CNTR_CTRM | USB_CNTR_SUSPM | USB_CNTR_WKUPM; + //| USB_CNTR_ERRM | USB_CNTR_PMAOVRM; handle_bus_reset(rhport); // Enable pull-up if supported @@ -274,13 +274,10 @@ static void handle_bus_reset(uint8_t rhport) { // Handle CTR interrupt for the TX/IN direction static void handle_ctr_tx(uint32_t ep_id) { - uint32_t ep_reg = ep_read(ep_id) & USB_EPREG_MASK; - - // Verify the CTR bit is set. This was in the ST Micro code, but I'm not sure it's actually necessary? - TU_VERIFY(ep_reg & USB_EP_CTR_TX, ); + uint32_t ep_reg = ep_read(ep_id) | USB_EP_CTR_TX | USB_EP_CTR_RX; + ep_reg &= USB_EPREG_MASK; uint8_t const ep_num = ep_reg & USB_EPADDR_FIELD; - uint8_t ep_addr = (ep_reg & USB_EPADDR_FIELD) | TUSB_DIR_IN_MASK; xfer_ctl_t *xfer = xfer_ctl_ptr(ep_num, TUSB_DIR_IN); if (ep_is_iso(ep_reg)) { @@ -296,108 +293,72 @@ static void handle_ctr_tx(uint32_t ep_id) { } if (xfer->total_len != xfer->queued_len) { - dcd_transmit_packet(xfer, ep_id); // also clear CTR bit + dcd_transmit_packet(xfer, ep_id); } else { - dcd_event_xfer_complete(0, ep_addr, xfer->total_len, XFER_RESULT_SUCCESS, true); + dcd_event_xfer_complete(0, ep_num | TUSB_DIR_IN_MASK, xfer->queued_len, XFER_RESULT_SUCCESS, true); + } +} - // Clear CTR TX and reserved CTR RX - ep_reg = (ep_reg & ~USB_EP_CTR_TX) | USB_EP_CTR_RX; +static void handle_ctr_setup(uint32_t ep_id) { + uint16_t rx_count = btable_get_count(ep_id, BTABLE_BUF_RX); + uint16_t rx_addr = btable_get_addr(ep_id, BTABLE_BUF_RX); + uint8_t setup_packet[8] TU_ATTR_ALIGNED(4); - ep_write(ep_id, ep_reg); + dcd_read_packet_memory(setup_packet, rx_addr, rx_count); + + // Clear CTR RX if another setup packet arrived before this, it will be discarded + ep_write_clear_ctr(ep_id, TUSB_DIR_OUT); + + // Setup packet should always be 8 bytes. If not, we probably missed the packet + if (rx_count == 8) { + dcd_event_setup_received(0, (uint8_t*) setup_packet, true); + // Hardware should reset EP0 RX/TX to NAK and both toggle to 1 + } else { + // Missed setup packet !!! + TU_BREAKPOINT(); + edpt0_prepare_setup(); } } // Handle CTR interrupt for the RX/OUT direction static void handle_ctr_rx(uint32_t ep_id) { -#ifdef FSDEV_BUS_32BIT - /* https://www.st.com/resource/en/errata_sheet/es0561-stm32h503cbebkbrb-device-errata-stmicroelectronics.pdf - * From STM32H503 errata 2.15.1: Buffer description table update completes after CTR interrupt triggers - * Description: - * - During OUT transfers, the correct transfer interrupt (CTR) is triggered a little before the last USB SRAM accesses - * have completed. If the software responds quickly to the interrupt, the full buffer contents may not be correct. - * Workaround: - * - Software should ensure that a small delay is included before accessing the SRAM contents. This delay - * should be 800 ns in Full Speed mode and 6.4 μs in Low Speed mode - * - Since H5 can run up to 250Mhz -> 1 cycle = 4ns. Per errata, we need to wait 200 cycles. Though executing code - * also takes time, so we'll wait 60 cycles (count = 20). - * - Since Low Speed mode is not supported/popular, we will ignore it for now. - * - * Note: this errata also seems to apply to G0, U5, H5 etc. - */ - volatile uint32_t cycle_count = 20; // defined as PCD_RX_PMA_CNT in stm32 hal_driver - while (cycle_count > 0U) { - cycle_count--; // each count take 3 cycles (1 for sub, jump, and compare) - } -#endif - - uint32_t ep_reg = ep_read(ep_id); - - // Verify the CTR bit is set. This was in the ST Micro code, but I'm not sure it's actually necessary? - TU_VERIFY(ep_reg & USB_EP_CTR_RX, ); - ep_reg = (ep_reg & ~USB_EP_CTR_RX) | USB_EP_CTR_TX; // Clear CTR RX and reserved CTR TX + uint32_t ep_reg = ep_read(ep_id) | USB_EP_CTR_TX | USB_EP_CTR_RX; + ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_OUT); // will change RX Status, reserved other toggle bits uint8_t const ep_num = ep_reg & USB_EPADDR_FIELD; + bool const is_iso = ep_is_iso(ep_reg); + xfer_ctl_t* xfer = xfer_ctl_ptr(ep_num, TUSB_DIR_OUT); - if (ep_reg & USB_EP_SETUP) { - uint32_t count = btable_get_count(ep_id, BTABLE_BUF_RX); - // Setup packet should always be 8 bytes. If not, ignore it, and try again. - if (count == 8) { - uint16_t rx_addr = btable_get_addr(ep_id, BTABLE_BUF_RX); - uint8_t setup_packet[8] TU_ATTR_ALIGNED(4); - - dcd_read_packet_memory(setup_packet, rx_addr, 8); - dcd_event_setup_received(0, (uint8_t*) setup_packet, true); - - // Reset EP to NAK and set both toggle to 1 - ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_NAK); - ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); - ep_add_dtog(&ep_reg, TUSB_DIR_IN, 1); - ep_add_dtog(&ep_reg, TUSB_DIR_OUT, 1); - } else { - // Missed an setup packet !!! - TU_BREAKPOINT(); - ep_reg &= USB_EPREG_MASK; // reversed all toggle - edpt0_prepare_setup(); - } + uint8_t buf_id; + if (is_iso) { + buf_id = (ep_reg & USB_EP_DTOG_RX) ? 0 : 1; // ISO are double buffered } else { - ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_OUT); // reversed all toggle except RX Status - - bool const is_iso = ep_is_iso(ep_reg); - xfer_ctl_t *xfer = xfer_ctl_ptr(ep_num, TUSB_DIR_OUT); + buf_id = BTABLE_BUF_RX; + } + uint16_t const rx_count = btable_get_count(ep_id, buf_id); + uint16_t pma_addr = (uint16_t) btable_get_addr(ep_id, buf_id); - uint8_t buf_id; - if (is_iso) { - buf_id = (ep_reg & USB_EP_DTOG_RX) ? 0 : 1; // ISO are double buffered + if (rx_count != 0) { + if (xfer->ff) { + dcd_read_packet_memory_ff(xfer->ff, pma_addr, rx_count); } else { - buf_id = BTABLE_BUF_RX; - } - uint32_t rx_count = btable_get_count(ep_id, buf_id); - uint16_t pma_addr = (uint16_t) btable_get_addr(ep_id, buf_id); - - if (rx_count != 0) { - if (xfer->ff) { - dcd_read_packet_memory_ff(xfer->ff, pma_addr, rx_count); - } else { - dcd_read_packet_memory(xfer->buffer + xfer->queued_len, pma_addr, rx_count); - } - xfer->queued_len = (uint16_t)(xfer->queued_len + rx_count); + dcd_read_packet_memory(xfer->buffer + xfer->queued_len, pma_addr, rx_count); } + xfer->queued_len += rx_count; + } - if ((rx_count < xfer->max_packet_size) || (xfer->queued_len == xfer->total_len)) { - // all bytes received or short packet - ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); - dcd_event_xfer_complete(0, ep_num, xfer->queued_len, XFER_RESULT_SUCCESS, true); - } else { - // Set endpoint active again for receiving more data. Note that isochronous endpoints stay active always - if (!is_iso) { - uint16_t const cnt = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size); - btable_set_rx_bufsize(ep_id, BTABLE_BUF_RX, cnt); - } - ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_VALID); + if ((rx_count < xfer->max_packet_size) || (xfer->queued_len == xfer->total_len)) { + // all bytes received or short packet + dcd_event_xfer_complete(0, ep_num, xfer->queued_len, XFER_RESULT_SUCCESS, true); + } else { + // Set endpoint active again for receiving more data. Note that isochronous endpoints stay active always + if (!is_iso) { + uint16_t const cnt = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size); + btable_set_rx_bufsize(ep_id, BTABLE_BUF_RX, cnt); } + ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_VALID); + ep_write(ep_id, ep_reg, false); } - - ep_write(ep_id, ep_reg); } void dcd_int_handler(uint8_t rhport) { @@ -406,10 +367,6 @@ void dcd_int_handler(uint8_t rhport) { // | USB_ISTR_SUSP | USB_ISTR_SOF | USB_ISTR_ESOF; // unused IRQs: (USB_ISTR_PMAOVR | USB_ISTR_ERR | USB_ISTR_L1REQ ) - // The ST driver loops here on the CTR bit, but that loop has been moved into the - // dcd_ep_ctr_handler(), so less need to loop here. The other interrupts shouldn't - // be triggered repeatedly. - /* Put SOF flag at the beginning of ISR in case to get least amount of jitter if it is used for timing purposes */ if (int_status & USB_ISTR_SOF) { FSDEV_REG->ISTR = (fsdev_bus_t)~USB_ISTR_SOF; @@ -455,17 +412,51 @@ void dcd_int_handler(uint8_t rhport) { FSDEV_REG->ISTR = (fsdev_bus_t)~USB_ISTR_ESOF; } +// if (int_status & (USB_ISTR_ERR | USB_ISTR_PMAOVR)) { +// TU_BREAKPOINT(); +// } + // loop to handle all pending CTR interrupts - while (int_status & USB_ISTR_CTR) { - uint32_t const ep_id = int_status & USB_ISTR_EP_ID; + while (FSDEV_REG->ISTR & USB_ISTR_CTR) { + // skip DIR bit, and use CTR TX/RX instead, since there is chance we have both TX/RX completed in one interrupt + uint32_t const ep_id = FSDEV_REG->ISTR & USB_ISTR_EP_ID; + uint32_t const ep_reg = ep_read(ep_id); + + if (ep_reg & USB_EP_CTR_RX) { + #ifdef FSDEV_BUS_32BIT + /* https://www.st.com/resource/en/errata_sheet/es0561-stm32h503cbebkbrb-device-errata-stmicroelectronics.pdf + * https://www.st.com/resource/en/errata_sheet/es0587-stm32u535xx-and-stm32u545xx-device-errata-stmicroelectronics.pdf + * From H503/U535 errata: Buffer description table update completes after CTR interrupt triggers + * Description: + * - During OUT transfers, the correct transfer interrupt (CTR) is triggered a little before the last USB SRAM accesses + * have completed. If the software responds quickly to the interrupt, the full buffer contents may not be correct. + * Workaround: + * - Software should ensure that a small delay is included before accessing the SRAM contents. This delay + * should be 800 ns in Full Speed mode and 6.4 μs in Low Speed mode + * - Since H5 can run up to 250Mhz -> 1 cycle = 4ns. Per errata, we need to wait 200 cycles. Though executing code + * also takes time, so we'll wait 60 cycles (count = 20). + * - Since Low Speed mode is not supported/popular, we will ignore it for now. + * + * Note: this errata may also apply to G0, U5, H5 etc. + */ + volatile uint32_t cycle_count = 20; // defined as PCD_RX_PMA_CNT in stm32 hal_driver + while (cycle_count > 0U) { + cycle_count--; // each count take 3 cycles (1 for sub, jump, and compare) + } + #endif - if ((int_status & USB_ISTR_DIR) == 0U) { - handle_ctr_tx(ep_id); // TX/IN - } else { - handle_ctr_rx(ep_id); // RX/OUT or both (RX/TX !!) + if (ep_reg & USB_EP_SETUP) { + handle_ctr_setup(ep_id); + } else { + ep_write_clear_ctr(ep_id, TUSB_DIR_OUT); + handle_ctr_rx(ep_id); + } } - int_status = FSDEV_REG->ISTR; + if (ep_reg & USB_EP_CTR_TX) { + ep_write_clear_ctr(ep_id, TUSB_DIR_IN); + handle_ctr_tx(ep_id); + } } } @@ -577,7 +568,7 @@ void edpt0_open(uint8_t rhport) { // no need to explicitly set DTOG bits since we aren't masked DTOG bit edpt0_prepare_setup(); // prepare for setup packet - ep_write(0, ep_reg); + ep_write(0, ep_reg, false); } bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) { @@ -590,7 +581,7 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) { TU_ASSERT(ep_idx < FSDEV_EP_COUNT); uint32_t ep_reg = ep_read(ep_idx) & ~USB_EPREG_MASK; - ep_reg |= tu_edpt_number(ep_addr) | USB_EP_CTR_RX | USB_EP_CTR_TX; + ep_reg |= tu_edpt_number(ep_addr) | USB_EP_CTR_TX | USB_EP_CTR_RX; // Set type switch (desc_ep->bmAttributes.xfer) { @@ -624,18 +615,17 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) { ep_reg &= ~(USB_EPTX_STAT | USB_EP_DTOG_TX); } - ep_write(ep_idx, ep_reg); + ep_write(ep_idx, ep_reg, true); return true; } -void dcd_edpt_close_all(uint8_t rhport) -{ - (void)rhport; +void dcd_edpt_close_all(uint8_t rhport) { + dcd_int_disable(rhport); for (uint32_t i = 1; i < FSDEV_EP_COUNT; i++) { // Reset endpoint - ep_write(i, 0); + ep_write(i, 0, false); // Clear EP allocation status ep_alloc_status[i].ep_num = 0xFF; ep_alloc_status[i].ep_type = 0xFF; @@ -643,6 +633,8 @@ void dcd_edpt_close_all(uint8_t rhport) ep_alloc_status[i].allocated[1] = false; } + dcd_int_enable(rhport); + // Reset PMA allocation ep_buf_ptr = FSDEV_BTABLE_BASE + 8 * CFG_TUD_ENDPPOINT_MAX + 2 * CFG_TUD_ENDPOINT0_SIZE; } @@ -684,13 +676,13 @@ bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) xfer->max_packet_size = tu_edpt_packet_size(desc_ep); uint32_t ep_reg = ep_read(ep_idx) & ~USB_EPREG_MASK; - ep_reg |= tu_edpt_number(ep_addr) | USB_EP_ISOCHRONOUS | USB_EP_CTR_RX | USB_EP_CTR_TX; + ep_reg |= tu_edpt_number(ep_addr) | USB_EP_ISOCHRONOUS | USB_EP_CTR_TX | USB_EP_CTR_RX; ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_DISABLED); ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_DISABLED); ep_add_dtog(&ep_reg, dir, 0); ep_add_dtog(&ep_reg, 1-dir, 1); - ep_write(ep_idx, ep_reg); + ep_write(ep_idx, ep_reg, true); return true; } @@ -698,7 +690,9 @@ bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) // Currently, single-buffered, and only 64 bytes at a time (max) static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) { uint16_t len = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size); - uint32_t ep_reg = ep_read(ep_ix) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR bits + uint32_t ep_reg = ep_read(ep_ix) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR RX + ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_IN); // only change TX Status, reserve other toggle bits + bool const is_iso = ep_is_iso(ep_reg); uint8_t buf_id; @@ -715,21 +709,16 @@ static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) { } else { dcd_write_packet_memory(addr_ptr, &(xfer->buffer[xfer->queued_len]), len); } - xfer->queued_len = (uint16_t) (xfer->queued_len + len); + xfer->queued_len += len; } btable_set_count(ep_ix, buf_id, len); - - ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_IN); ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_VALID); - ep_clear_ctr(&ep_reg, TUSB_DIR_IN); - dcd_int_disable(0); - ep_write(ep_ix, ep_reg); if (is_iso) { xfer->iso_in_sending = true; } - dcd_int_enable(0); + ep_write(ep_ix, ep_reg, true); } static bool edpt_xfer(uint8_t rhport, uint8_t ep_num, uint8_t dir) { @@ -741,8 +730,8 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_num, uint8_t dir) { if (dir == TUSB_DIR_IN) { dcd_transmit_packet(xfer, ep_idx); } else { - uint32_t cnt = (uint32_t) tu_min16(xfer->total_len, xfer->max_packet_size); - uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX; // keep CTR TX, clear CTR RX + uint16_t cnt = tu_min16(xfer->total_len, xfer->max_packet_size); + uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX | USB_EP_CTR_RX; // keep CTR TX ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); ep_add_status(&ep_reg, dir, EP_STAT_VALID); @@ -753,7 +742,7 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_num, uint8_t dir) { btable_set_rx_bufsize(ep_idx, BTABLE_BUF_RX, cnt); } - ep_write(ep_idx, ep_reg); + ep_write(ep_idx, ep_reg, true); } return true; @@ -796,7 +785,7 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); ep_add_status(&ep_reg, dir, EP_STAT_STALL); - ep_write(ep_idx, ep_reg); + ep_write(ep_idx, ep_reg, true); } void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { @@ -814,8 +803,7 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { ep_add_status(&ep_reg, dir, EP_STAT_NAK); } ep_add_dtog(&ep_reg, dir, 0); // Reset to DATA0 - - ep_write(ep_idx, ep_reg); + ep_write(ep_idx, ep_reg, true); } // Write to packet memory area (PMA) from user memory diff --git a/src/portable/st/stm32_fsdev/fsdev_type.h b/src/portable/st/stm32_fsdev/fsdev_type.h index 8432df4b0..f29abd703 100644 --- a/src/portable/st/stm32_fsdev/fsdev_type.h +++ b/src/portable/st/stm32_fsdev/fsdev_type.h @@ -172,8 +172,24 @@ typedef enum { // - DTOG and STAT are write 1 to toggle //--------------------------------------------------------------------+ -TU_ATTR_ALWAYS_INLINE static inline void ep_write(uint32_t ep_id, uint32_t value) { +TU_ATTR_ALWAYS_INLINE static inline void ep_write(uint32_t ep_id, uint32_t value, bool need_exclusive) { + if (need_exclusive) { + dcd_int_disable(0); + } + FSDEV_REG->ep[ep_id].reg = (fsdev_bus_t) value; + + if (need_exclusive) { + dcd_int_enable(0); + } +} + +TU_ATTR_ALWAYS_INLINE static inline void ep_write_clear_ctr(uint32_t ep_id, tusb_dir_t dir) { + uint32_t reg = FSDEV_REG->ep[ep_id].reg; + reg |= USB_EP_CTR_TX | USB_EP_CTR_RX; + reg &= USB_EPREG_MASK; + reg &= ~(1 << (USB_EP_CTR_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); + ep_write(ep_id, reg, false); } TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_read(uint32_t ep_id) { @@ -188,10 +204,6 @@ TU_ATTR_ALWAYS_INLINE static inline void ep_add_dtog(uint32_t* reg, tusb_dir_t d *reg ^= (state << (USB_EP_DTOG_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); } -TU_ATTR_ALWAYS_INLINE static inline void ep_clear_ctr(uint32_t* reg, tusb_dir_t dir) { - *reg &= ~(1 << (USB_EP_CTR_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); -} - TU_ATTR_ALWAYS_INLINE static inline bool ep_is_iso(uint32_t reg) { return (reg & USB_EP_TYPE_MASK) == USB_EP_ISOCHRONOUS; } @@ -218,7 +230,7 @@ TU_ATTR_ALWAYS_INLINE static inline void btable_set_addr(uint32_t ep_id, uint8_t #endif } -TU_ATTR_ALWAYS_INLINE static inline uint32_t btable_get_count(uint32_t ep_id, uint8_t buf_id) { +TU_ATTR_ALWAYS_INLINE static inline uint16_t btable_get_count(uint32_t ep_id, uint8_t buf_id) { uint16_t count; #ifdef FSDEV_BUS_32BIT count = (FSDEV_BTABLE->ep32[ep_id][buf_id].count_addr >> 16); @@ -258,7 +270,7 @@ TU_ATTR_ALWAYS_INLINE static inline uint16_t pma_align_buffer_size(uint16_t size return (*num_block) * block_in_bytes; } -TU_ATTR_ALWAYS_INLINE static inline void btable_set_rx_bufsize(uint32_t ep_id, uint8_t buf_id, uint32_t wCount) { +TU_ATTR_ALWAYS_INLINE static inline void btable_set_rx_bufsize(uint32_t ep_id, uint8_t buf_id, uint16_t wCount) { uint8_t blsize, num_block; (void) pma_align_buffer_size(wCount, &blsize, &num_block); -- cgit v1.3.1 From 0860cd3b5e95a7366bc2c68eb7648e8479059172 Mon Sep 17 00:00:00 2001 From: hathach Date: Tue, 6 Aug 2024 22:20:24 +0700 Subject: minor rename --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 28 +++++++++++++-------------- src/portable/st/stm32_fsdev/fsdev_type.h | 4 ++-- 2 files changed, 16 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 97529997d..2075babe3 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -356,7 +356,7 @@ static void handle_ctr_rx(uint32_t ep_id) { uint16_t const cnt = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size); btable_set_rx_bufsize(ep_id, BTABLE_BUF_RX, cnt); } - ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_VALID); + ep_change_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_VALID); ep_write(ep_id, ep_reg, false); } } @@ -563,8 +563,8 @@ void edpt0_open(uint8_t rhport) { uint32_t ep_reg = ep_read(0) & ~USB_EPREG_MASK; // only get toggle bits ep_reg |= USB_EP_CONTROL; - ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_NAK); - ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); + ep_change_status(&ep_reg, TUSB_DIR_IN, EP_STAT_NAK); + ep_change_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_NAK); // no need to explicitly set DTOG bits since we aren't masked DTOG bit edpt0_prepare_setup(); // prepare for setup packet @@ -605,8 +605,8 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) { xfer->max_packet_size = packet_size; xfer->ep_idx = ep_idx; - ep_add_status(&ep_reg, dir, EP_STAT_NAK); - ep_add_dtog(&ep_reg, dir, 0); + ep_change_status(&ep_reg, dir, EP_STAT_NAK); + ep_change_dtog(&ep_reg, dir, 0); // reserve other direction toggle bits if (dir == TUSB_DIR_IN) { @@ -677,10 +677,10 @@ bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) uint32_t ep_reg = ep_read(ep_idx) & ~USB_EPREG_MASK; ep_reg |= tu_edpt_number(ep_addr) | USB_EP_ISOCHRONOUS | USB_EP_CTR_TX | USB_EP_CTR_RX; - ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_DISABLED); - ep_add_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_DISABLED); - ep_add_dtog(&ep_reg, dir, 0); - ep_add_dtog(&ep_reg, 1-dir, 1); + ep_change_status(&ep_reg, TUSB_DIR_IN, EP_STAT_DISABLED); + ep_change_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_DISABLED); + ep_change_dtog(&ep_reg, dir, 0); + ep_change_dtog(&ep_reg, 1 - dir, 1); ep_write(ep_idx, ep_reg, true); @@ -713,7 +713,7 @@ static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) { } btable_set_count(ep_ix, buf_id, len); - ep_add_status(&ep_reg, TUSB_DIR_IN, EP_STAT_VALID); + ep_change_status(&ep_reg, TUSB_DIR_IN, EP_STAT_VALID); if (is_iso) { xfer->iso_in_sending = true; @@ -733,7 +733,7 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_num, uint8_t dir) { uint16_t cnt = tu_min16(xfer->total_len, xfer->max_packet_size); uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX | USB_EP_CTR_RX; // keep CTR TX ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); - ep_add_status(&ep_reg, dir, EP_STAT_VALID); + ep_change_status(&ep_reg, dir, EP_STAT_VALID); if (ep_is_iso(ep_reg)) { btable_set_rx_bufsize(ep_idx, 0, cnt); @@ -783,7 +783,7 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR bits ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); - ep_add_status(&ep_reg, dir, EP_STAT_STALL); + ep_change_status(&ep_reg, dir, EP_STAT_STALL); ep_write(ep_idx, ep_reg, true); } @@ -800,9 +800,9 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir) | EP_DTOG_MASK(dir); if (!ep_is_iso(ep_reg)) { - ep_add_status(&ep_reg, dir, EP_STAT_NAK); + ep_change_status(&ep_reg, dir, EP_STAT_NAK); } - ep_add_dtog(&ep_reg, dir, 0); // Reset to DATA0 + ep_change_dtog(&ep_reg, dir, 0); // Reset to DATA0 ep_write(ep_idx, ep_reg, true); } diff --git a/src/portable/st/stm32_fsdev/fsdev_type.h b/src/portable/st/stm32_fsdev/fsdev_type.h index f29abd703..d5028c702 100644 --- a/src/portable/st/stm32_fsdev/fsdev_type.h +++ b/src/portable/st/stm32_fsdev/fsdev_type.h @@ -196,11 +196,11 @@ TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_read(uint32_t ep_id) { return FSDEV_REG->ep[ep_id].reg; } -TU_ATTR_ALWAYS_INLINE static inline void ep_add_status(uint32_t* reg, tusb_dir_t dir, ep_stat_t state) { +TU_ATTR_ALWAYS_INLINE static inline void ep_change_status(uint32_t* reg, tusb_dir_t dir, ep_stat_t state) { *reg ^= (state << (USB_EPTX_STAT_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); } -TU_ATTR_ALWAYS_INLINE static inline void ep_add_dtog(uint32_t* reg, tusb_dir_t dir, uint8_t state) { +TU_ATTR_ALWAYS_INLINE static inline void ep_change_dtog(uint32_t* reg, tusb_dir_t dir, uint8_t state) { *reg ^= (state << (USB_EP_DTOG_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); } -- cgit v1.3.1 From 3a2216306769f0af0ceb3dde7528dfb182fec8f5 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 7 Aug 2024 15:16:22 +0700 Subject: fix v203 race condition between rx bufsize and RX_STAT which cause PMAOVR fix set_rx_bufsize with invalid value for zero length packet --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 28 ++++++++++++++++----------- src/portable/st/stm32_fsdev/fsdev_type.h | 17 ++++++++++------ 2 files changed, 28 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 2075babe3..cd7410488 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -217,8 +217,8 @@ void dcd_init(uint8_t rhport) { ep_write(i, 0u, false); } - FSDEV_REG->CNTR |= USB_CNTR_RESETM | USB_CNTR_ESOFM | USB_CNTR_CTRM | USB_CNTR_SUSPM | USB_CNTR_WKUPM; - //| USB_CNTR_ERRM | USB_CNTR_PMAOVRM; + FSDEV_REG->CNTR |= USB_CNTR_RESETM | USB_CNTR_ESOFM | USB_CNTR_CTRM | + USB_CNTR_SUSPM | USB_CNTR_WKUPM | USB_CNTR_PMAOVRM; handle_bus_reset(rhport); // Enable pull-up if supported @@ -349,6 +349,10 @@ static void handle_ctr_rx(uint32_t ep_id) { if ((rx_count < xfer->max_packet_size) || (xfer->queued_len == xfer->total_len)) { // all bytes received or short packet + + // reset rx bufsize to mps to prevent race condition to cause PMAOVR (occurs with ch32v203 with msc write10) + btable_set_rx_bufsize(ep_id, BTABLE_BUF_RX, xfer->max_packet_size); + dcd_event_xfer_complete(0, ep_num, xfer->queued_len, XFER_RESULT_SUCCESS, true); } else { // Set endpoint active again for receiving more data. Note that isochronous endpoints stay active always @@ -412,9 +416,10 @@ void dcd_int_handler(uint8_t rhport) { FSDEV_REG->ISTR = (fsdev_bus_t)~USB_ISTR_ESOF; } -// if (int_status & (USB_ISTR_ERR | USB_ISTR_PMAOVR)) { -// TU_BREAKPOINT(); -// } + if (int_status & USB_ISTR_PMAOVR) { + TU_BREAKPOINT(); + FSDEV_REG->ISTR = (fsdev_bus_t)~USB_ISTR_PMAOVR; + } // loop to handle all pending CTR interrupts while (FSDEV_REG->ISTR & USB_ISTR_CTR) { @@ -446,7 +451,7 @@ void dcd_int_handler(uint8_t rhport) { #endif if (ep_reg & USB_EP_SETUP) { - handle_ctr_setup(ep_id); + handle_ctr_setup(ep_id); // CTR will be clear after copied setup packet } else { ep_write_clear_ctr(ep_id, TUSB_DIR_OUT); handle_ctr_rx(ep_id); @@ -690,7 +695,7 @@ bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) // Currently, single-buffered, and only 64 bytes at a time (max) static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) { uint16_t len = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size); - uint32_t ep_reg = ep_read(ep_ix) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR RX + uint32_t ep_reg = ep_read(ep_ix) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_IN); // only change TX Status, reserve other toggle bits bool const is_iso = ep_is_iso(ep_reg); @@ -730,10 +735,10 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_num, uint8_t dir) { if (dir == TUSB_DIR_IN) { dcd_transmit_packet(xfer, ep_idx); } else { - uint16_t cnt = tu_min16(xfer->total_len, xfer->max_packet_size); - uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX | USB_EP_CTR_RX; // keep CTR TX + uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); - ep_change_status(&ep_reg, dir, EP_STAT_VALID); + + uint16_t cnt = tu_min16(xfer->total_len, xfer->max_packet_size); if (ep_is_iso(ep_reg)) { btable_set_rx_bufsize(ep_idx, 0, cnt); @@ -742,7 +747,8 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_num, uint8_t dir) { btable_set_rx_bufsize(ep_idx, BTABLE_BUF_RX, cnt); } - ep_write(ep_idx, ep_reg, true); + ep_change_status(&ep_reg, dir, EP_STAT_VALID); + ep_write(ep_idx, ep_reg, false); } return true; diff --git a/src/portable/st/stm32_fsdev/fsdev_type.h b/src/portable/st/stm32_fsdev/fsdev_type.h index d5028c702..6c5ccae89 100644 --- a/src/portable/st/stm32_fsdev/fsdev_type.h +++ b/src/portable/st/stm32_fsdev/fsdev_type.h @@ -172,6 +172,10 @@ typedef enum { // - DTOG and STAT are write 1 to toggle //--------------------------------------------------------------------+ +TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_read(uint32_t ep_id) { + return FSDEV_REG->ep[ep_id].reg; +} + TU_ATTR_ALWAYS_INLINE static inline void ep_write(uint32_t ep_id, uint32_t value, bool need_exclusive) { if (need_exclusive) { dcd_int_disable(0); @@ -192,10 +196,6 @@ TU_ATTR_ALWAYS_INLINE static inline void ep_write_clear_ctr(uint32_t ep_id, tusb ep_write(ep_id, reg, false); } -TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_read(uint32_t ep_id) { - return FSDEV_REG->ep[ep_id].reg; -} - TU_ATTR_ALWAYS_INLINE static inline void ep_change_status(uint32_t* reg, tusb_dir_t dir, ep_stat_t state) { *reg ^= (state << (USB_EPTX_STAT_Pos + (dir == TUSB_DIR_IN ? 0 : 8))); } @@ -260,13 +260,13 @@ TU_ATTR_ALWAYS_INLINE static inline uint16_t pma_align_buffer_size(uint16_t size if (size > 62) { block_in_bytes = 32; *blsize = 1; + *num_block = tu_div_ceil(size, 32); } else { block_in_bytes = 2; *blsize = 0; + *num_block = tu_div_ceil(size, 2); } - *num_block = tu_div_ceil(size, block_in_bytes); - return (*num_block) * block_in_bytes; } @@ -276,6 +276,10 @@ TU_ATTR_ALWAYS_INLINE static inline void btable_set_rx_bufsize(uint32_t ep_id, u /* Encode into register. When BLSIZE==1, we need to subtract 1 block count */ uint16_t bl_nb = (blsize << 15) | ((num_block - blsize) << 10); + if (bl_nb == 0) { + // 0 is invalid, set up blsize to 1 + bl_nb = 1 << 15; + } #ifdef FSDEV_BUS_32BIT uint32_t count_addr = FSDEV_BTABLE->ep32[ep_id][buf_id].count_addr; @@ -284,6 +288,7 @@ TU_ATTR_ALWAYS_INLINE static inline void btable_set_rx_bufsize(uint32_t ep_id, u #else FSDEV_BTABLE->ep16[ep_id][buf_id].count = bl_nb; #endif + } #ifdef __cplusplus -- cgit v1.3.1 From 57c26fdc72dccee1da569e90f46b4060a97d47a7 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 8 Aug 2024 00:27:51 +0700 Subject: use EP_KIND for STATUS OUT to fix OUT packet is auto accepted after SETUP without usbd consent --- .idea/cmake.xml | 15 +++++------ src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 36 +++++++++++++++++---------- src/portable/st/stm32_fsdev/fsdev_type.h | 4 +-- 3 files changed, 33 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/.idea/cmake.xml b/.idea/cmake.xml index 67e793016..bf5696725 100644 --- a/.idea/cmake.xml +++ b/.idea/cmake.xml @@ -89,19 +89,20 @@ - - + + + - + - + - + @@ -133,8 +134,8 @@ - - + + diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index cd7410488..89697106d 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -147,6 +147,7 @@ typedef struct { static xfer_ctl_t xfer_status[CFG_TUD_ENDPPOINT_MAX][2]; static ep_alloc_t ep_alloc_status[FSDEV_EP_COUNT]; static uint8_t remoteWakeCountdown; // When wake is requested +static uint8_t _setup_dir; // for detecting status OUT //--------------------------------------------------------------------+ // Prototypes @@ -311,6 +312,7 @@ static void handle_ctr_setup(uint32_t ep_id) { // Setup packet should always be 8 bytes. If not, we probably missed the packet if (rx_count == 8) { + _setup_dir = (setup_packet[0] & 0x80 ? TUSB_DIR_IN : TUSB_DIR_OUT); dcd_event_setup_received(0, (uint8_t*) setup_packet, true); // Hardware should reset EP0 RX/TX to NAK and both toggle to 1 } else { @@ -323,8 +325,6 @@ static void handle_ctr_setup(uint32_t ep_id) { // Handle CTR interrupt for the RX/OUT direction static void handle_ctr_rx(uint32_t ep_id) { uint32_t ep_reg = ep_read(ep_id) | USB_EP_CTR_TX | USB_EP_CTR_RX; - ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_OUT); // will change RX Status, reserved other toggle bits - uint8_t const ep_num = ep_reg & USB_EPADDR_FIELD; bool const is_iso = ep_is_iso(ep_reg); xfer_ctl_t* xfer = xfer_ctl_ptr(ep_num, TUSB_DIR_OUT); @@ -345,6 +345,13 @@ static void handle_ctr_rx(uint32_t ep_id) { dcd_read_packet_memory(xfer->buffer + xfer->queued_len, pma_addr, rx_count); } xfer->queued_len += rx_count; + } else { + // ZLP Status OUT + if (ep_num == 0 && (ep_reg & USB_EP_KIND) ) { + ep_reg &= USB_EPREG_MASK; + ep_reg &= ~USB_EP_KIND; // clear kind bit + ep_write(ep_id, ep_reg, false); + } } if ((rx_count < xfer->max_packet_size) || (xfer->queued_len == xfer->total_len)) { @@ -360,6 +367,7 @@ static void handle_ctr_rx(uint32_t ep_id) { uint16_t const cnt = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size); btable_set_rx_bufsize(ep_id, BTABLE_BUF_RX, cnt); } + ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_OUT); // will change RX Status, reserved other toggle bits ep_change_status(&ep_reg, TUSB_DIR_OUT, EP_STAT_VALID); ep_write(ep_id, ep_reg, false); } @@ -367,9 +375,6 @@ static void handle_ctr_rx(uint32_t ep_id) { void dcd_int_handler(uint8_t rhport) { uint32_t int_status = FSDEV_REG->ISTR; - // const uint32_t handled_ints = USB_ISTR_CTR | USB_ISTR_RESET | USB_ISTR_WKUP - // | USB_ISTR_SUSP | USB_ISTR_SOF | USB_ISTR_ESOF; - // unused IRQs: (USB_ISTR_PMAOVR | USB_ISTR_ERR | USB_ISTR_L1REQ ) /* Put SOF flag at the beginning of ISR in case to get least amount of jitter if it is used for timing purposes */ if (int_status & USB_ISTR_SOF) { @@ -738,17 +743,22 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_num, uint8_t dir) { uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); - uint16_t cnt = tu_min16(xfer->total_len, xfer->max_packet_size); - - if (ep_is_iso(ep_reg)) { - btable_set_rx_bufsize(ep_idx, 0, cnt); - btable_set_rx_bufsize(ep_idx, 1, cnt); + if ( ep_num == 0 && dir == TUSB_DIR_OUT && _setup_dir == TUSB_DIR_IN ) { + // use EP0 OUT status stage, otherwise OUT packet right after SETUP will be accepted without + // the usbd (software) consent + ep_reg |= USB_EP_KIND; } else { - btable_set_rx_bufsize(ep_idx, BTABLE_BUF_RX, cnt); - } + uint16_t cnt = tu_min16(xfer->total_len, xfer->max_packet_size); + if (ep_is_iso(ep_reg)) { + btable_set_rx_bufsize(ep_idx, 0, cnt); + btable_set_rx_bufsize(ep_idx, 1, cnt); + } else { + btable_set_rx_bufsize(ep_idx, BTABLE_BUF_RX, cnt); + } + } ep_change_status(&ep_reg, dir, EP_STAT_VALID); - ep_write(ep_idx, ep_reg, false); + ep_write(ep_idx, ep_reg, true); } return true; diff --git a/src/portable/st/stm32_fsdev/fsdev_type.h b/src/portable/st/stm32_fsdev/fsdev_type.h index 6c5ccae89..7ebc32255 100644 --- a/src/portable/st/stm32_fsdev/fsdev_type.h +++ b/src/portable/st/stm32_fsdev/fsdev_type.h @@ -277,8 +277,8 @@ TU_ATTR_ALWAYS_INLINE static inline void btable_set_rx_bufsize(uint32_t ep_id, u /* Encode into register. When BLSIZE==1, we need to subtract 1 block count */ uint16_t bl_nb = (blsize << 15) | ((num_block - blsize) << 10); if (bl_nb == 0) { - // 0 is invalid, set up blsize to 1 - bl_nb = 1 << 15; + // zlp but 0 is invalid value, set num_block to 1 (2 bytes) + bl_nb = 1 << 10; } #ifdef FSDEV_BUS_32BIT -- cgit v1.3.1 From bd64625df271c461c3d6008f96c630c61218f451 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 8 Aug 2024 12:40:11 +0700 Subject: revert the use of EP_KIND. ch32v203 seems to unconditionally accept ZLP on EP0 OUT, which can incorrectly use queued_len of previous transfer. So reset total_len and queued_len to 0. --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 38 +++++++++------------------ 1 file changed, 13 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 89697106d..bbdfbaa09 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -147,7 +147,6 @@ typedef struct { static xfer_ctl_t xfer_status[CFG_TUD_ENDPPOINT_MAX][2]; static ep_alloc_t ep_alloc_status[FSDEV_EP_COUNT]; static uint8_t remoteWakeCountdown; // When wake is requested -static uint8_t _setup_dir; // for detecting status OUT //--------------------------------------------------------------------+ // Prototypes @@ -312,7 +311,6 @@ static void handle_ctr_setup(uint32_t ep_id) { // Setup packet should always be 8 bytes. If not, we probably missed the packet if (rx_count == 8) { - _setup_dir = (setup_packet[0] & 0x80 ? TUSB_DIR_IN : TUSB_DIR_OUT); dcd_event_setup_received(0, (uint8_t*) setup_packet, true); // Hardware should reset EP0 RX/TX to NAK and both toggle to 1 } else { @@ -345,22 +343,17 @@ static void handle_ctr_rx(uint32_t ep_id) { dcd_read_packet_memory(xfer->buffer + xfer->queued_len, pma_addr, rx_count); } xfer->queued_len += rx_count; - } else { - // ZLP Status OUT - if (ep_num == 0 && (ep_reg & USB_EP_KIND) ) { - ep_reg &= USB_EPREG_MASK; - ep_reg &= ~USB_EP_KIND; // clear kind bit - ep_write(ep_id, ep_reg, false); - } } - if ((rx_count < xfer->max_packet_size) || (xfer->queued_len == xfer->total_len)) { + if ((rx_count < xfer->max_packet_size) || (xfer->queued_len >= xfer->total_len)) { // all bytes received or short packet + dcd_event_xfer_complete(0, ep_num, xfer->queued_len, XFER_RESULT_SUCCESS, true); - // reset rx bufsize to mps to prevent race condition to cause PMAOVR (occurs with ch32v203 with msc write10) + // For ch32v203: reset rx bufsize to mps to prevent race condition to cause PMAOVR (occurs with msc write10) + // also ch32 seems to unconditionally accept ZLP on EP0 OUT, which can incorrectly use queued_len of previous + // transfer. So reset total_len and queued_len to 0. btable_set_rx_bufsize(ep_id, BTABLE_BUF_RX, xfer->max_packet_size); - - dcd_event_xfer_complete(0, ep_num, xfer->queued_len, XFER_RESULT_SUCCESS, true); + xfer->total_len = xfer->queued_len = 0; } else { // Set endpoint active again for receiving more data. Note that isochronous endpoints stay active always if (!is_iso) { @@ -743,20 +736,15 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_num, uint8_t dir) { uint32_t ep_reg = ep_read(ep_idx) | USB_EP_CTR_TX | USB_EP_CTR_RX; // reserve CTR ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); - if ( ep_num == 0 && dir == TUSB_DIR_OUT && _setup_dir == TUSB_DIR_IN ) { - // use EP0 OUT status stage, otherwise OUT packet right after SETUP will be accepted without - // the usbd (software) consent - ep_reg |= USB_EP_KIND; - } else { - uint16_t cnt = tu_min16(xfer->total_len, xfer->max_packet_size); + uint16_t cnt = tu_min16(xfer->total_len, xfer->max_packet_size); - if (ep_is_iso(ep_reg)) { - btable_set_rx_bufsize(ep_idx, 0, cnt); - btable_set_rx_bufsize(ep_idx, 1, cnt); - } else { - btable_set_rx_bufsize(ep_idx, BTABLE_BUF_RX, cnt); - } + if (ep_is_iso(ep_reg)) { + btable_set_rx_bufsize(ep_idx, 0, cnt); + btable_set_rx_bufsize(ep_idx, 1, cnt); + } else { + btable_set_rx_bufsize(ep_idx, BTABLE_BUF_RX, cnt); } + ep_change_status(&ep_reg, dir, EP_STAT_VALID); ep_write(ep_idx, ep_reg, true); } -- cgit v1.3.1 From 1ea38ebe13f07c4db99ae39f207e55815d3701b3 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 8 Aug 2024 15:43:11 +0700 Subject: refactor read/write pma from/to fifo --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 247 ++++++++++---------------- src/portable/st/stm32_fsdev/fsdev_type.h | 8 + 2 files changed, 98 insertions(+), 157 deletions(-) (limited to 'src') diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index bbdfbaa09..252969648 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -336,14 +336,12 @@ static void handle_ctr_rx(uint32_t ep_id) { uint16_t const rx_count = btable_get_count(ep_id, buf_id); uint16_t pma_addr = (uint16_t) btable_get_addr(ep_id, buf_id); - if (rx_count != 0) { - if (xfer->ff) { - dcd_read_packet_memory_ff(xfer->ff, pma_addr, rx_count); - } else { - dcd_read_packet_memory(xfer->buffer + xfer->queued_len, pma_addr, rx_count); - } - xfer->queued_len += rx_count; + if (xfer->ff) { + dcd_read_packet_memory_ff(xfer->ff, pma_addr, rx_count); + } else { + dcd_read_packet_memory(xfer->buffer + xfer->queued_len, pma_addr, rx_count); } + xfer->queued_len += rx_count; if ((rx_count < xfer->max_packet_size) || (xfer->queued_len >= xfer->total_len)) { // all bytes received or short packet @@ -706,14 +704,12 @@ static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) { } uint16_t addr_ptr = (uint16_t) btable_get_addr(ep_ix, buf_id); - if (len) { - if (xfer->ff) { - dcd_write_packet_memory_ff(xfer->ff, addr_ptr, len); - } else { - dcd_write_packet_memory(addr_ptr, &(xfer->buffer[xfer->queued_len]), len); - } - xfer->queued_len += len; + if (xfer->ff) { + dcd_write_packet_memory_ff(xfer->ff, addr_ptr, len); + } else { + dcd_write_packet_memory(addr_ptr, &(xfer->buffer[xfer->queued_len]), len); } + xfer->queued_len += len; btable_set_count(ep_ix, buf_id, len); ep_change_status(&ep_reg, TUSB_DIR_IN, EP_STAT_VALID); @@ -814,25 +810,20 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { // - Packet memory must be either strictly 16-bit or 32-bit depending on FSDEV_BUS_32BIT // - Uses unaligned for RAM (since M0 cannot access unaligned address) static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, uint16_t nbytes) { - enum { BUS_SIZE = sizeof(fsdev_bus_t) }; - uint32_t n_write = nbytes / BUS_SIZE; + if (nbytes == 0) return true; + uint32_t n_write = nbytes / FSDEV_BUS_SIZE; fsdev_pma_buf_t* pma_buf = PMA_BUF_AT(dst); const uint8_t *src8 = src; while (n_write--) { - #ifdef FSDEV_BUS_32BIT - pma_buf->value = tu_unaligned_read32(src8); - #else - pma_buf->value = tu_unaligned_read16(src8); - #endif - - src8 += BUS_SIZE; + pma_buf->value = fsdevbus_unaligned_read(src8); + src8 += FSDEV_BUS_SIZE; pma_buf++; } // odd bytes e.g 1 for 16-bit or 1-3 for 32-bit - uint16_t odd = nbytes & (BUS_SIZE - 1); + uint16_t odd = nbytes & (FSDEV_BUS_SIZE - 1); if (odd) { fsdev_bus_t temp = 0; for(uint16_t i = 0; i < odd; i++) { @@ -848,27 +839,20 @@ static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, ui // - Packet memory must be either strictly 16-bit or 32-bit depending on FSDEV_BUS_32BIT // - Uses unaligned for RAM (since M0 cannot access unaligned address) static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, uint16_t nbytes) { - enum { BUS_SIZE = sizeof(fsdev_bus_t) }; - uint32_t n_write = nbytes / BUS_SIZE; + if (nbytes == 0) return true; + uint32_t n_read = nbytes / FSDEV_BUS_SIZE; fsdev_pma_buf_t* pma_buf = PMA_BUF_AT(src); uint8_t *dst8 = (uint8_t *)dst; - while (n_write--) { - fsdev_bus_t temp = pma_buf->value; - - #ifdef FSDEV_BUS_32BIT - tu_unaligned_write32(dst8, temp); - #else - tu_unaligned_write16(dst8, temp); - #endif - - dst8 += BUS_SIZE; + while (n_read--) { + fsdevbus_unaligned_write(dst8, (fsdev_bus_t ) pma_buf->value); + dst8 += FSDEV_BUS_SIZE; pma_buf++; } // odd bytes e.g 1 for 16-bit or 1-3 for 32-bit - uint16_t odd = nbytes & (BUS_SIZE - 1); + uint16_t odd = nbytes & (FSDEV_BUS_SIZE - 1); if (odd) { fsdev_bus_t temp = pma_buf->value; while (odd--) { @@ -880,156 +864,105 @@ static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, uint16_t return true; } -/** - * @brief Copy from FIFO to packet memory area (PMA). - * Uses byte-access of system memory and 16-bit access of packet memory - * @param wNBytes no. of bytes to be copied. - * @retval None - */ +// Write to PMA from FIFO static bool dcd_write_packet_memory_ff(tu_fifo_t *ff, uint16_t dst, uint16_t wNBytes) { + if (wNBytes == 0) return true; + // Since we copy from a ring buffer FIFO, a wrap might occur making it necessary to conduct two copies tu_fifo_buffer_info_t info; tu_fifo_get_read_info(ff, &info); - uint16_t cnt_lin = TU_MIN(wNBytes, info.len_lin); - uint16_t cnt_wrap = TU_MIN(wNBytes - cnt_lin, info.len_wrap); + uint16_t cnt_lin = tu_min16(wNBytes, info.len_lin); + uint16_t cnt_wrap = tu_min16(wNBytes - cnt_lin, info.len_wrap); + uint16_t const cnt_total = cnt_lin + cnt_wrap; // We want to read from the FIFO and write it into the PMA, if LIN part is ODD and has WRAPPED part, - // last lin byte will be combined with wrapped part - // To ensure PMA is always access aligned (dst aligned to 16 or 32 bit) -#ifdef FSDEV_BUS_32BIT - if ((cnt_lin & 0x03) && cnt_wrap) { - // Copy first linear part - dcd_write_packet_memory(dst, info.ptr_lin, cnt_lin & ~0x03); - dst += cnt_lin & ~0x03; - - // Copy last linear bytes & first wrapped bytes to buffer - uint32_t i; - uint8_t tmp[4]; - for (i = 0; i < (cnt_lin & 0x03); i++) { - tmp[i] = ((uint8_t *)info.ptr_lin)[(cnt_lin & ~0x03) + i]; - } - uint32_t wCnt = cnt_wrap; - for (; i < 4 && wCnt > 0; i++, wCnt--) { - tmp[i] = *(uint8_t *)info.ptr_wrap; - info.ptr_wrap = (uint8_t *)info.ptr_wrap + 1; + // last lin byte will be combined with wrapped part To ensure PMA is always access aligned + uint16_t lin_even = cnt_lin & ~(FSDEV_BUS_SIZE - 1); + uint16_t lin_odd = cnt_lin & (FSDEV_BUS_SIZE - 1); + uint8_t const *src8 = (uint8_t const*) info.ptr_lin; + + // write even linear part + dcd_write_packet_memory(dst, src8, lin_even); + dst += lin_even; + src8 += lin_even; + + if (lin_odd == 0) { + src8 = (uint8_t const*) info.ptr_wrap; + } else { + // Combine last linear bytes + first wrapped bytes to form fsdev bus width data + fsdev_bus_t temp = 0; + uint16_t i; + for(i = 0; i < lin_odd; i++) { + temp |= *src8++ << (i * 8); } - // Write unaligned buffer - dcd_write_packet_memory(dst, &tmp, 4); - dst += 4; - - // Copy rest of wrapped byte - if (wCnt) { - dcd_write_packet_memory(dst, info.ptr_wrap, wCnt); - } - } -#else - if ((cnt_lin & 0x01) && cnt_wrap) { - // Copy first linear part - dcd_write_packet_memory(dst, info.ptr_lin, cnt_lin & ~0x01); - dst += cnt_lin & ~0x01; - - // Copy last linear byte & first wrapped byte - uint16_t tmp = ((uint8_t *)info.ptr_lin)[cnt_lin - 1] | ((uint16_t)(((uint8_t *)info.ptr_wrap)[0]) << 8U); - dcd_write_packet_memory(dst, &tmp, 2); - dst += 2; - - // Copy rest of wrapped byte - dcd_write_packet_memory(dst, ((uint8_t *)info.ptr_wrap) + 1, cnt_wrap - 1); - } -#endif - else { - // Copy linear part - dcd_write_packet_memory(dst, info.ptr_lin, cnt_lin); - dst += info.len_lin; - - if (info.len_wrap) { - // Copy wrapped byte - dcd_write_packet_memory(dst, info.ptr_wrap, cnt_wrap); + src8 = (uint8_t const*) info.ptr_wrap; + for(; i < FSDEV_BUS_SIZE && cnt_wrap > 0; i++, cnt_wrap--) { + temp |= *src8++ << (i * 8); } + + dcd_write_packet_memory(dst, &temp, FSDEV_BUS_SIZE); + dst += FSDEV_BUS_SIZE; } - tu_fifo_advance_read_pointer(ff, cnt_lin + cnt_wrap); + // write the rest of the wrapped part + dcd_write_packet_memory(dst, src8, cnt_wrap); + tu_fifo_advance_read_pointer(ff, cnt_total); return true; } -/** - * @brief Copy a buffer from user packet memory area (PMA) to FIFO. - * Uses byte-access of system memory and 16-bit access of packet memory - * @param wNBytes no. of bytes to be copied. - * @retval None - */ +// Read from PMA to FIFO static bool dcd_read_packet_memory_ff(tu_fifo_t *ff, uint16_t src, uint16_t wNBytes) { + if (wNBytes == 0) return true; + // Since we copy into a ring buffer FIFO, a wrap might occur making it necessary to conduct two copies // Check for first linear part tu_fifo_buffer_info_t info; tu_fifo_get_write_info(ff, &info); // We want to read from the FIFO - uint16_t cnt_lin = TU_MIN(wNBytes, info.len_lin); - uint16_t cnt_wrap = TU_MIN(wNBytes - cnt_lin, info.len_wrap); - - // We want to read from PMA and write it into the FIFO, if LIN part is ODD and has WRAPPED part, - // last lin byte will be combined with wrapped part - // To ensure PMA is always access aligned (src aligned to 16 or 32 bit) -#ifdef FSDEV_BUS_32BIT - if ((cnt_lin & 0x03) && cnt_wrap) { - // Copy first linear part - dcd_read_packet_memory(info.ptr_lin, src, cnt_lin & ~0x03); - src += cnt_lin & ~0x03; - - // Copy last linear bytes & first wrapped bytes - uint8_t tmp[4]; - dcd_read_packet_memory(tmp, src, 4); - src += 4; - - uint32_t i; - for (i = 0; i < (cnt_lin & 0x03); i++) { - ((uint8_t *)info.ptr_lin)[(cnt_lin & ~0x03) + i] = tmp[i]; - } - uint32_t wCnt = cnt_wrap; - for (; i < 4 && wCnt > 0; i++, wCnt--) { - *(uint8_t *)info.ptr_wrap = tmp[i]; - info.ptr_wrap = (uint8_t *)info.ptr_wrap + 1; - } + uint16_t cnt_lin = tu_min16(wNBytes, info.len_lin); + uint16_t cnt_wrap = tu_min16(wNBytes - cnt_lin, info.len_wrap); + uint16_t cnt_total = cnt_lin + cnt_wrap; - // Copy rest of wrapped byte - if (wCnt) { - dcd_read_packet_memory(info.ptr_wrap, src, wCnt); - } - } -#else - if ((cnt_lin & 0x01) && cnt_wrap) { - // Copy first linear part - dcd_read_packet_memory(info.ptr_lin, src, cnt_lin & ~0x01); - src += cnt_lin & ~0x01; + // We want to read from the FIFO and write it into the PMA, if LIN part is ODD and has WRAPPED part, + // last lin byte will be combined with wrapped part To ensure PMA is always access aligned - // Copy last linear byte & first wrapped byte - uint8_t tmp[2]; - dcd_read_packet_memory(tmp, src, 2); - src += 2; + uint16_t lin_even = cnt_lin & ~(FSDEV_BUS_SIZE - 1); + uint16_t lin_odd = cnt_lin & (FSDEV_BUS_SIZE - 1); + uint8_t *dst8 = (uint8_t *) info.ptr_lin; - ((uint8_t *)info.ptr_lin)[cnt_lin - 1] = tmp[0]; - ((uint8_t *)info.ptr_wrap)[0] = tmp[1]; + // read even linear part + dcd_read_packet_memory(dst8, src, lin_even); + dst8 += lin_even; + src += lin_even; - // Copy rest of wrapped byte - dcd_read_packet_memory(((uint8_t *)info.ptr_wrap) + 1, src, cnt_wrap - 1); - } -#endif - else { - // Copy linear part - dcd_read_packet_memory(info.ptr_lin, src, cnt_lin); - src += cnt_lin; - - if (info.len_wrap) { - // Copy wrapped byte - dcd_read_packet_memory(info.ptr_wrap, src, cnt_wrap); + if (lin_odd == 0) { + dst8 = (uint8_t *) info.ptr_wrap; + } else { + // Combine last linear bytes + first wrapped bytes to form fsdev bus width data + fsdev_bus_t temp; + dcd_read_packet_memory(&temp, src, FSDEV_BUS_SIZE); + src += FSDEV_BUS_SIZE; + + uint16_t i; + for (i = 0; i < lin_odd; i++) { + *dst8++ = (uint8_t) (temp & 0xfful); + temp >>= 8; + } + + dst8 = (uint8_t *) info.ptr_wrap; + for (; i < FSDEV_BUS_SIZE && cnt_wrap > 0; i++, cnt_wrap--) { + *dst8++ = (uint8_t) (temp & 0xfful); + temp >>= 8; } } - tu_fifo_advance_write_pointer(ff, cnt_lin + cnt_wrap); + // read the rest of the wrapped part + dcd_read_packet_memory(dst8, src, cnt_wrap); + tu_fifo_advance_write_pointer(ff, cnt_total); return true; } diff --git a/src/portable/st/stm32_fsdev/fsdev_type.h b/src/portable/st/stm32_fsdev/fsdev_type.h index 7ebc32255..26717fab0 100644 --- a/src/portable/st/stm32_fsdev/fsdev_type.h +++ b/src/portable/st/stm32_fsdev/fsdev_type.h @@ -66,10 +66,18 @@ TU_VERIFY_STATIC(FSDEV_BTABLE_BASE % 8 == 0, "BTABLE base must be aligned to 8 b // The fsdev_bus_t type can be used for both register and PMA access necessities #ifdef FSDEV_BUS_32BIT typedef uint32_t fsdev_bus_t; + #define fsdevbus_unaligned_read(_addr) tu_unaligned_read32(_addr) + #define fsdevbus_unaligned_write(_addr, _value) tu_unaligned_write32(_addr, _value) #else typedef uint16_t fsdev_bus_t; + #define fsdevbus_unaligned_read(_addr) tu_unaligned_read16(_addr) + #define fsdevbus_unaligned_write(_addr, _value) tu_unaligned_write16(_addr, _value) #endif +enum { + FSDEV_BUS_SIZE = sizeof(fsdev_bus_t), +}; + //--------------------------------------------------------------------+ // BTable Typedef //--------------------------------------------------------------------+ -- cgit v1.3.1