From 3a2216306769f0af0ceb3dde7528dfb182fec8f5 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 7 Aug 2024 15:16:22 +0700 Subject: [PATCH] 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(-) 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