From 0eeaccaf46c28444cc0830895559652cf5638c60 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Sun, 12 Apr 2020 21:07:17 -0400 Subject: [PATCH 1/6] Skeleton, and initial stm32fsdev implementation (that leaks memory) --- docs/porting.md | 7 +++ src/device/dcd.h | 6 +- src/device/usbd.c | 61 +++++++++++++++++++ src/device/usbd.h | 2 + src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 37 ++++++++++- 5 files changed, 111 insertions(+), 2 deletions(-) diff --git a/docs/porting.md b/docs/porting.md index ccafa799..de300c08 100644 --- a/docs/porting.md +++ b/docs/porting.md @@ -119,6 +119,13 @@ Opening an endpoint is done for all non-control endpoints once the host picks a Also make sure to enable endpoint specific interrupts. +##### dcd_edpt_close + +Close an endpoint. After calling this, the device should not respond to any packets directed towards this endpoint. Implementation is optional, and is to be called from USBD in a non-interrupt context. +This function is used for implementing alternate settings. + +When called, this function need to abort any transfers in progress through this endpoint, before returning. + ##### dcd_edpt_xfer `dcd_edpt_xfer` is responsible for configuring the peripheral to send or receive data from the host. "xfer" is short for "transfer". **This is one of the core methods you must implement for TinyUSB to work (one other is the interrupt handler).** Data from the host is the OUT direction and data to the host is IN. It is used for all endpoints including the control endpoint 0. Make sure to handle the zero-length packet STATUS packet on endpoint 0 correctly. It may be a special transaction to the peripheral. diff --git a/src/device/dcd.h b/src/device/dcd.h index 487ddb3b..f4853022 100644 --- a/src/device/dcd.h +++ b/src/device/dcd.h @@ -66,7 +66,7 @@ typedef struct TU_ATTR_ALIGNED(4) // USBD_EVT_XFER_COMPLETE struct { - uint8_t ep_addr; + uint8_t ep_addr; ///< 0xFF signifies that the transfer was aborted. uint8_t result; uint32_t len; }xfer_complete; @@ -123,6 +123,10 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * re // Configure endpoint's registers according to descriptor bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc); +// Close an endpoint. +// Since it is weak, caller must TU_ASSERT this function's existence before calling it. +void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) TU_ATTR_WEAK; + // Submit a transfer, When complete dcd_event_xfer_complete() is invoked to notify the stack bool dcd_edpt_xfer (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes); diff --git a/src/device/usbd.c b/src/device/usbd.c index 397a681e..e4653256 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -420,6 +420,11 @@ void tud_task (void) uint8_t const ep_addr = event.xfer_complete.ep_addr; uint8_t const epnum = tu_edpt_number(ep_addr); uint8_t const ep_dir = tu_edpt_dir(ep_addr); + + if(ep_addr == 0xFF) // aborted transfer + { + break; + } TU_LOG2(" Endpoint: 0x%02X, Bytes: %u\r\n", ep_addr, (unsigned int) event.xfer_complete.len); @@ -1036,4 +1041,60 @@ bool usbd_edpt_stalled(uint8_t rhport, uint8_t ep_addr) return _usbd_dev.ep_status[epnum][dir].stalled; } +/** + * Remove queued xfer complete messages from event queue, + * for a particular ep. + */ +static void usbd_abort_transfers(uint8_t rhport, uint8_t ep_addr) +{ + dcd_event_t ev_sentinal = + { + .event_id = DCD_EVENT_COUNT, ///< This is an invalid event ID. + }; + dcd_event_t event; + uint8_t const epnum = tu_edpt_number(ep_addr); + uint8_t const ep_dir = tu_edpt_dir(ep_addr); + + dcd_int_disable(rhport); + // Queue sentinal element + TU_ASSERT(osal_queue_send(_usbd_q, &ev_sentinal, true), /**/); + + TU_ASSERT(osal_queue_receive(_usbd_q, &event), /**/); + + while(event.event_id != DCD_EVENT_COUNT) + { + if((event.rhport == rhport) && (event.event_id == DCD_EVENT_XFER_COMPLETE) + && (event.xfer_complete.ep_addr == ep_addr)) + { + _usbd_dev.ep_status[epnum][ep_dir].busy = false; + event.xfer_complete.ep_addr = 0xFF; // Mark transfer as invalid + } + TU_ASSERT(osal_queue_send(_usbd_q, &event, true), /**/); + TU_ASSERT(osal_queue_receive(_usbd_q, &event), /**/); + } + + dcd_int_enable(rhport); +} + +/** + * tud_edpt_close will disable an endpoint, and clear all pending transfers + * through the particular endpoint. + * + * It must be called from the usb task (i.e. from the control request + * handler while handling SET_ALTERNATE). + */ +void tud_edpt_close(uint8_t rhport, uint8_t ep_addr) +{ + + TU_ASSERT(dcd_edpt_close, /**/); + TU_LOG2(" CLOSING Endpoint: 0x%02X\r\n", ep_addr); + + dcd_edpt_close(rhport, ep_addr); + + /* Now, in progress transfers have to be expunged */ + usbd_abort_transfers(rhport, ep_addr); + + return; +} + #endif diff --git a/src/device/usbd.h b/src/device/usbd.h index 817af20e..2447c208 100644 --- a/src/device/usbd.h +++ b/src/device/usbd.h @@ -79,6 +79,8 @@ static inline bool tud_connect(void) return true; } +void tud_edpt_close(uint8_t rhport, uint8_t ep_addr); + // Carry out Data and Status stage of control transfer // - If len = 0, it is equivalent to sending status only // - If len > wLength : it will be truncated diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index f962a4be..fa044e2d 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -69,7 +69,6 @@ * - Endpoint index is the ID of the endpoint * - This means that priority is given to endpoints with lower ID numbers * - Code is mixing up EP IX with EP ID. Everywhere. - * - No way to close endpoints; Can a device be reconfigured without a reset? * - Packet buffer memory is copied in the interrupt. * - This is better for performance, but means interrupts are disabled for longer * - DMA may be the best choice, but it could also be pushed to the USBD task. @@ -623,6 +622,7 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc if(dir == TUSB_DIR_IN) { + // FIXME: use pma_alloc to allocate memory dynamically *pcd_ep_tx_address_ptr(USB, epnum) = ep_buf_ptr; pcd_set_ep_tx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size); pcd_clear_tx_dtog(USB, epnum); @@ -630,6 +630,7 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc } else { + // FIXME: use pma_alloc to allocate memory dynamically *pcd_ep_rx_address_ptr(USB, epnum) = ep_buf_ptr; pcd_set_ep_rx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size); pcd_clear_rx_dtog(USB, epnum); @@ -642,6 +643,40 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc return true; } +/** + * Close an endpoint. + * + * This function should be called with interrupts enabled, though + * this implementation should be valid with them disabled, too. + * This also clears transfers in progress, should there be any. + */ +void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) +{ + (void)rhport; + uint32_t const epnum = tu_edpt_number(ep_addr); + uint32_t const dir = tu_edpt_dir(ep_addr); + +#ifndef NDEBUG + TU_ASSERT(epnum < MAX_EP_COUNT, /**/); +#endif + + //uint16_t memptr; + + if(dir == TUSB_DIR_IN) + { + pcd_set_ep_tx_status(USB,epnum,USB_EP_TX_DIS); + //memptr = *pcd_ep_tx_address_ptr(USB, epnum); + } + else + { + pcd_set_ep_rx_status(USB, epnum, USB_EP_RX_DIS); + //memptr = *pcd_ep_rx_address_ptr(USB, epnum); + } + + // FIXME: Free memory + // pma_free(memptr); +} + // Currently, single-buffered, and only 64 bytes at a time (max) static void dcd_transmit_packet(xfer_ctl_t * xfer, uint16_t ep_ix) From f8e7487355c1d48517211a2316fbbc8566a1532d Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Mon, 13 Apr 2020 09:25:16 -0400 Subject: [PATCH 2/6] edpt_close: Updated based on feedback. --- docs/porting.md | 7 ++- src/device/usbd.c | 10 ++-- src/device/usbd.h | 2 - src/device/usbd_pvt.h | 1 + src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 60 +++++++++++++------ 5 files changed, 52 insertions(+), 28 deletions(-) diff --git a/docs/porting.md b/docs/porting.md index de300c08..9561dfaf 100644 --- a/docs/porting.md +++ b/docs/porting.md @@ -121,10 +121,11 @@ Also make sure to enable endpoint specific interrupts. ##### dcd_edpt_close -Close an endpoint. After calling this, the device should not respond to any packets directed towards this endpoint. Implementation is optional, and is to be called from USBD in a non-interrupt context. -This function is used for implementing alternate settings. +Close an endpoint. his function is used for implementing alternate settings. -When called, this function need to abort any transfers in progress through this endpoint, before returning. +After calling this, the device should not respond to any packets directed towards this endpoint. When called, this function must abort any transfers in progress through this endpoint, before returning. + +Implementation is optional. Must be called from the USB task. Interrupts could be disabled or enabled during the call. ##### dcd_edpt_xfer diff --git a/src/device/usbd.c b/src/device/usbd.c index e4653256..71f5454e 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1044,6 +1044,8 @@ bool usbd_edpt_stalled(uint8_t rhport, uint8_t ep_addr) /** * Remove queued xfer complete messages from event queue, * for a particular ep. + * + * Must be called with interrupts enabled. */ static void usbd_abort_transfers(uint8_t rhport, uint8_t ep_addr) { @@ -1077,13 +1079,13 @@ static void usbd_abort_transfers(uint8_t rhport, uint8_t ep_addr) } /** - * tud_edpt_close will disable an endpoint, and clear all pending transfers + * usbd_edpt_close will disable an endpoint, and clear all pending transfers * through the particular endpoint. * - * It must be called from the usb task (i.e. from the control request - * handler while handling SET_ALTERNATE). + * It must be called from the usb task (e.g. from the control request + * handler while handling SET_ALTERNATE), with interrupts enabled. */ -void tud_edpt_close(uint8_t rhport, uint8_t ep_addr) +void usbd_edpt_close(uint8_t rhport, uint8_t ep_addr) { TU_ASSERT(dcd_edpt_close, /**/); diff --git a/src/device/usbd.h b/src/device/usbd.h index 2447c208..817af20e 100644 --- a/src/device/usbd.h +++ b/src/device/usbd.h @@ -79,8 +79,6 @@ static inline bool tud_connect(void) return true; } -void tud_edpt_close(uint8_t rhport, uint8_t ep_addr); - // Carry out Data and Status stage of control transfer // - If len = 0, it is equivalent to sending status only // - If len > wLength : it will be truncated diff --git a/src/device/usbd_pvt.h b/src/device/usbd_pvt.h index b9947044..26b9700e 100644 --- a/src/device/usbd_pvt.h +++ b/src/device/usbd_pvt.h @@ -38,6 +38,7 @@ //--------------------------------------------------------------------+ //bool usbd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc); +void usbd_edpt_close(uint8_t rhport, uint8_t ep_addr); // Submit a usb transfer bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes); diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index fa044e2d..13e54443 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -63,8 +63,10 @@ * Current driver limitations (i.e., a list of features for you to add): * - STALL handled, but not tested. * - Does it work? No clue. - * - All EP BTABLE buffers are created as max 64 bytes. - * - Smaller can be requested, but it has to be an even number. + * - All EP BTABLE buffers are created as fixed 64 bytes. + * - Smaller can be requested: + * - Must be a multiple of 8 bytes + * - All EP must have identical buffer size. * - No isochronous endpoints * - Endpoint index is the ID of the endpoint * - This means that priority is given to endpoints with lower ID numbers @@ -130,21 +132,29 @@ * Configuration *****************************************************/ -// HW supports max of 8 endpoints, but this can be reduced to save RAM +// HW supports max of 8 bidirectional endpoints, but this can be reduced to save RAM +// (8u here would mean 8 IN and 8 OUT) #ifndef MAX_EP_COUNT -# define MAX_EP_COUNT 8u +# if (PMA_LENGTH == 512U) +# define MAX_EP_COUNT 4U +# else +# define MAX_EP_COUNT 8U +# endif #endif // If sharing with CAN, one can set this to be non-zero to give CAN space where it wants it // Both of these MUST be a multiple of 2, and are in byte units. #ifndef DCD_STM32_BTABLE_BASE -# define DCD_STM32_BTABLE_BASE 0u +# define DCD_STM32_BTABLE_BASE 0U #endif #ifndef DCD_STM32_BTABLE_LENGTH # define DCD_STM32_BTABLE_LENGTH (PMA_LENGTH - DCD_STM32_BTABLE_BASE) #endif +// Below is used by the static PMA allocator +#define DCD_STM32_PMA_ALLOC_SIZE 64U + /*************************************************** * Checks, structs, defines, function definitions, etc. */ @@ -156,6 +166,11 @@ TU_VERIFY_STATIC(((DCD_STM32_BTABLE_BASE) + (DCD_STM32_BTABLE_LENGTH))<=(PMA_LEN TU_VERIFY_STATIC(((DCD_STM32_BTABLE_BASE) % 8) == 0, "BTABLE base must be aligned to 8 bytes"); +// With static allocation of 64 bytes per endpoint, ensure there is enough packet buffer space +TU_VERIFY_STATIC(((MAX_EP_COUNT*2u*DCD_STM32_PMA_ALLOC_SIZE) <= DCD_STM32_BTABLE_LENGTH), "Packed buffer not long enough for count of endpoints"); + +TU_VERIFY_STATIC((DCD_STM32_PMA_ALLOC_SIZE % 8) == 0, "Packet buffer allocation must be a multiple of 8 bytes"); + // One of these for every EP IN & OUT, uses a bit of RAM.... typedef struct { @@ -178,7 +193,6 @@ static uint8_t remoteWakeCountdown; // When wake is requested // EP Buffers assigned from end of memory location, to minimize their chance of crashing // into the stack. -static uint16_t ep_buf_ptr; static void dcd_handle_bus_reset(void); static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, size_t wNBytes); static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wNBytes); @@ -355,7 +369,6 @@ static void dcd_handle_bus_reset(void) pcd_set_endpoint(USB,i,0u); } - ep_buf_ptr = DCD_STM32_BTABLE_BASE + 8*MAX_EP_COUNT; // 8 bytes per endpoint (two TX and two RX words, each) dcd_edpt_open (0, &ep0OUT_desc); dcd_edpt_open (0, &ep0IN_desc); @@ -577,6 +590,22 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * re } } +/*** + * Allocate a section of PMA + * Currently statitically allocates 64 bytes for each EP. + * + * stm32fsdev have 512 or 1024 bytes of packet RAM, and support 16 EP (8 in and 8 out), + * Static checks above have verified that there is enough packet memory space, so + * this should NEVER fail. + */ +static uint16_t dcd_pma_alloc(uint8_t ep_addr) +{ + uint8_t const epnum = tu_edpt_number(ep_addr); + uint8_t const dir = tu_edpt_dir(ep_addr); + + return ((2*epnum + dir) * DCD_STM32_PMA_ALLOC_SIZE); +} + // The STM32F0 doesn't seem to like |= or &= to manipulate the EP#R registers, // so I'm using the #define from HAL here, instead. @@ -590,6 +619,7 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc TU_ASSERT(p_endpoint_desc->bmAttributes.xfer != TUSB_XFER_ISOCHRONOUS); TU_ASSERT(epnum < MAX_EP_COUNT); + TU_ASSERT(epMaxPktSize <= DCD_STM32_PMA_ALLOC_SIZE); // Set type switch(p_endpoint_desc->bmAttributes.xfer) { @@ -623,7 +653,7 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc if(dir == TUSB_DIR_IN) { // FIXME: use pma_alloc to allocate memory dynamically - *pcd_ep_tx_address_ptr(USB, epnum) = ep_buf_ptr; + *pcd_ep_tx_address_ptr(USB, epnum) = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress); pcd_set_ep_tx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size); pcd_clear_tx_dtog(USB, epnum); pcd_set_ep_tx_status(USB,epnum,USB_EP_TX_NAK); @@ -631,14 +661,13 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc else { // FIXME: use pma_alloc to allocate memory dynamically - *pcd_ep_rx_address_ptr(USB, epnum) = ep_buf_ptr; + *pcd_ep_rx_address_ptr(USB, epnum) = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress); pcd_set_ep_rx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size); pcd_clear_rx_dtog(USB, epnum); pcd_set_ep_rx_status(USB, epnum, USB_EP_RX_NAK); } xfer_ctl_ptr(epnum, dir)->max_packet_size = epMaxPktSize; - ep_buf_ptr = (uint16_t)(ep_buf_ptr + p_endpoint_desc->wMaxPacketSize.size); // increment buffer pointer return true; } @@ -646,8 +675,8 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc /** * Close an endpoint. * - * This function should be called with interrupts enabled, though - * this implementation should be valid with them disabled, too. + * This function may be called with interrupts enabled or disabled. + * * This also clears transfers in progress, should there be any. */ void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) @@ -660,21 +689,14 @@ void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) TU_ASSERT(epnum < MAX_EP_COUNT, /**/); #endif - //uint16_t memptr; - if(dir == TUSB_DIR_IN) { pcd_set_ep_tx_status(USB,epnum,USB_EP_TX_DIS); - //memptr = *pcd_ep_tx_address_ptr(USB, epnum); } else { pcd_set_ep_rx_status(USB, epnum, USB_EP_RX_DIS); - //memptr = *pcd_ep_rx_address_ptr(USB, epnum); } - - // FIXME: Free memory - // pma_free(memptr); } // Currently, single-buffered, and only 64 bytes at a time (max) From de208b31cf648c03670f34ff615a0f7ac6b566f3 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Mon, 13 Apr 2020 11:05:34 -0400 Subject: [PATCH 3/6] edpt_close: Remove item from queue instead of modifying it. --- src/device/dcd.h | 2 +- src/device/usbd.c | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/device/dcd.h b/src/device/dcd.h index f4853022..99e62335 100644 --- a/src/device/dcd.h +++ b/src/device/dcd.h @@ -66,7 +66,7 @@ typedef struct TU_ATTR_ALIGNED(4) // USBD_EVT_XFER_COMPLETE struct { - uint8_t ep_addr; ///< 0xFF signifies that the transfer was aborted. + uint8_t ep_addr; uint8_t result; uint32_t len; }xfer_complete; diff --git a/src/device/usbd.c b/src/device/usbd.c index 71f5454e..386401d9 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -420,11 +420,6 @@ void tud_task (void) uint8_t const ep_addr = event.xfer_complete.ep_addr; uint8_t const epnum = tu_edpt_number(ep_addr); uint8_t const ep_dir = tu_edpt_dir(ep_addr); - - if(ep_addr == 0xFF) // aborted transfer - { - break; - } TU_LOG2(" Endpoint: 0x%02X, Bytes: %u\r\n", ep_addr, (unsigned int) event.xfer_complete.len); @@ -1069,9 +1064,11 @@ static void usbd_abort_transfers(uint8_t rhport, uint8_t ep_addr) && (event.xfer_complete.ep_addr == ep_addr)) { _usbd_dev.ep_status[epnum][ep_dir].busy = false; - event.xfer_complete.ep_addr = 0xFF; // Mark transfer as invalid } - TU_ASSERT(osal_queue_send(_usbd_q, &event, true), /**/); + else + { + TU_ASSERT(osal_queue_send(_usbd_q, &event, true), /**/); + } TU_ASSERT(osal_queue_receive(_usbd_q, &event), /**/); } @@ -1087,7 +1084,6 @@ static void usbd_abort_transfers(uint8_t rhport, uint8_t ep_addr) */ void usbd_edpt_close(uint8_t rhport, uint8_t ep_addr) { - TU_ASSERT(dcd_edpt_close, /**/); TU_LOG2(" CLOSING Endpoint: 0x%02X\r\n", ep_addr); From bbc59f1ceb73131b5fc2158cdd3483b61408d59d Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Mon, 13 Apr 2020 13:11:45 -0400 Subject: [PATCH 4/6] stm32fsdev: add static assert for PMA size bigger than EP0 size. --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 13e54443..defdfe05 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -136,9 +136,9 @@ // (8u here would mean 8 IN and 8 OUT) #ifndef MAX_EP_COUNT # if (PMA_LENGTH == 512U) -# define MAX_EP_COUNT 4U +# define MAX_EP_COUNT 3U # else -# define MAX_EP_COUNT 8U +# define MAX_EP_COUNT 7U # endif #endif @@ -153,7 +153,9 @@ #endif // Below is used by the static PMA allocator -#define DCD_STM32_PMA_ALLOC_SIZE 64U +#ifndef DCD_STM32_PMA_ALLOC_SIZE +# define DCD_STM32_PMA_ALLOC_SIZE 64U +#endif /*************************************************** * Checks, structs, defines, function definitions, etc. @@ -167,10 +169,13 @@ TU_VERIFY_STATIC(((DCD_STM32_BTABLE_BASE) + (DCD_STM32_BTABLE_LENGTH))<=(PMA_LEN TU_VERIFY_STATIC(((DCD_STM32_BTABLE_BASE) % 8) == 0, "BTABLE base must be aligned to 8 bytes"); // With static allocation of 64 bytes per endpoint, ensure there is enough packet buffer space -TU_VERIFY_STATIC(((MAX_EP_COUNT*2u*DCD_STM32_PMA_ALLOC_SIZE) <= DCD_STM32_BTABLE_LENGTH), "Packed buffer not long enough for count of endpoints"); +// 8 bytes are required for control data for each EP. +TU_VERIFY_STATIC(((MAX_EP_COUNT*2u*DCD_STM32_PMA_ALLOC_SIZE + 8*MAX_EP_COUNT) <= DCD_STM32_BTABLE_LENGTH), "Packed buffer not long enough for count of endpoints"); TU_VERIFY_STATIC((DCD_STM32_PMA_ALLOC_SIZE % 8) == 0, "Packet buffer allocation must be a multiple of 8 bytes"); +TU_VERIFY_STATIC(CFG_TUD_ENDPOINT0_SIZE <= DCD_STM32_PMA_ALLOC_SIZE,"EP0 size is more than PMA allocation size"); + // One of these for every EP IN & OUT, uses a bit of RAM.... typedef struct { @@ -598,12 +603,18 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * re * Static checks above have verified that there is enough packet memory space, so * this should NEVER fail. */ -static uint16_t dcd_pma_alloc(uint8_t ep_addr) +static uint16_t dcd_pma_alloc(uint8_t ep_addr, size_t length) { uint8_t const epnum = tu_edpt_number(ep_addr); uint8_t const dir = tu_edpt_dir(ep_addr); + + (void)length; - return ((2*epnum + dir) * DCD_STM32_PMA_ALLOC_SIZE); + TU_ASSERT(length <= DCD_STM32_PMA_ALLOC_SIZE); + + uint16_t addr = DCD_STM32_BTABLE_BASE + 8*MAX_EP_COUNT; // Each EP needs 8 bytes to store control data + addr += ((2*epnum + dir) * DCD_STM32_PMA_ALLOC_SIZE); + return addr; } // The STM32F0 doesn't seem to like |= or &= to manipulate the EP#R registers, @@ -619,7 +630,6 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc TU_ASSERT(p_endpoint_desc->bmAttributes.xfer != TUSB_XFER_ISOCHRONOUS); TU_ASSERT(epnum < MAX_EP_COUNT); - TU_ASSERT(epMaxPktSize <= DCD_STM32_PMA_ALLOC_SIZE); // Set type switch(p_endpoint_desc->bmAttributes.xfer) { @@ -652,16 +662,14 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc if(dir == TUSB_DIR_IN) { - // FIXME: use pma_alloc to allocate memory dynamically - *pcd_ep_tx_address_ptr(USB, epnum) = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress); + *pcd_ep_tx_address_ptr(USB, epnum) = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress, p_endpoint_desc->wMaxPacketSize.size); pcd_set_ep_tx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size); pcd_clear_tx_dtog(USB, epnum); pcd_set_ep_tx_status(USB,epnum,USB_EP_TX_NAK); } else { - // FIXME: use pma_alloc to allocate memory dynamically - *pcd_ep_rx_address_ptr(USB, epnum) = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress); + *pcd_ep_rx_address_ptr(USB, epnum) = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress, p_endpoint_desc->wMaxPacketSize.size); pcd_set_ep_rx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size); pcd_clear_rx_dtog(USB, epnum); pcd_set_ep_rx_status(USB, epnum, USB_EP_RX_NAK); From b0270f499b7f0cc2f5589a274105cd4cac87f1a7 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Tue, 14 Apr 2020 10:22:37 -0400 Subject: [PATCH 5/6] stm32fsdev: dynamic allocation of PMA. --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 147 +++++++++++------- 1 file changed, 94 insertions(+), 53 deletions(-) diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index e1f459b6..14ffcbe4 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -63,10 +63,7 @@ * Current driver limitations (i.e., a list of features for you to add): * - STALL handled, but not tested. * - Does it work? No clue. - * - All EP BTABLE buffers are created as fixed 64 bytes. - * - Smaller can be requested: - * - Must be a multiple of 8 bytes - * - All EP must have identical buffer size. + * - All EP BTABLE buffers are created based on max packet size of first EP opened with that address. * - No isochronous endpoints * - Endpoint index is the ID of the endpoint * - This means that priority is given to endpoints with lower ID numbers @@ -76,7 +73,6 @@ * - DMA may be the best choice, but it could also be pushed to the USBD task. * - No double-buffering * - No DMA - * - No provision to control the D+ pull-up using GPIO on devices without an internal pull-up. * - Minimal error handling * - Perhaps error interrupts should be reported to the stack, or cause a device reset? * - Assumes a single USB peripheral; I think that no hardware has multiple so this is fine. @@ -135,11 +131,7 @@ // HW supports max of 8 bidirectional endpoints, but this can be reduced to save RAM // (8u here would mean 8 IN and 8 OUT) #ifndef MAX_EP_COUNT -# if (PMA_LENGTH == 512U) -# define MAX_EP_COUNT 3U -# else -# define MAX_EP_COUNT 7U -# endif +# define MAX_EP_COUNT 8U #endif // If sharing with CAN, one can set this to be non-zero to give CAN space where it wants it @@ -152,11 +144,6 @@ # define DCD_STM32_BTABLE_LENGTH (PMA_LENGTH - DCD_STM32_BTABLE_BASE) #endif -// Below is used by the static PMA allocator -#ifndef DCD_STM32_PMA_ALLOC_SIZE -# define DCD_STM32_PMA_ALLOC_SIZE 64U -#endif - /*************************************************** * Checks, structs, defines, function definitions, etc. */ @@ -168,21 +155,15 @@ TU_VERIFY_STATIC(((DCD_STM32_BTABLE_BASE) + (DCD_STM32_BTABLE_LENGTH))<=(PMA_LEN TU_VERIFY_STATIC(((DCD_STM32_BTABLE_BASE) % 8) == 0, "BTABLE base must be aligned to 8 bytes"); -// With static allocation of 64 bytes per endpoint, ensure there is enough packet buffer space -// 8 bytes are required for control data for each EP. -TU_VERIFY_STATIC(((MAX_EP_COUNT*2u*DCD_STM32_PMA_ALLOC_SIZE + 8*MAX_EP_COUNT) <= DCD_STM32_BTABLE_LENGTH), "Packed buffer not long enough for count of endpoints"); - -TU_VERIFY_STATIC((DCD_STM32_PMA_ALLOC_SIZE % 8) == 0, "Packet buffer allocation must be a multiple of 8 bytes"); - -TU_VERIFY_STATIC(CFG_TUD_ENDPOINT0_SIZE <= DCD_STM32_PMA_ALLOC_SIZE,"EP0 size is more than PMA allocation size"); - // One of these for every EP IN & OUT, uses a bit of RAM.... typedef struct { uint8_t * buffer; uint16_t total_len; uint16_t queued_len; - uint16_t max_packet_size; + uint16_t pma_ptr; + uint8_t max_packet_size; + uint8_t pma_alloc_size; } xfer_ctl_t; static xfer_ctl_t xfer_status[MAX_EP_COUNT][2]; @@ -196,14 +177,19 @@ static TU_ATTR_ALIGNED(4) uint32_t _setup_packet[6]; static uint8_t remoteWakeCountdown; // When wake is requested -// EP Buffers assigned from end of memory location, to minimize their chance of crashing // into the stack. static void dcd_handle_bus_reset(void); -static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, size_t wNBytes); -static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wNBytes); static void dcd_transmit_packet(xfer_ctl_t * xfer, uint16_t ep_ix); static void dcd_ep_ctr_handler(void); +// PMA allocation/access +static uint8_t open_ep_count; +static uint16_t ep_buf_ptr; ///< Points to first free memory location +static void dcd_pma_alloc_reset(void); +static uint16_t dcd_pma_alloc(uint8_t ep_addr, size_t length); +static void dcd_pma_free(uint8_t ep_addr); +static bool dcd_write_packet_memory(uint16_t dst, const void *__restrict src, size_t wNBytes); +static bool dcd_read_packet_memory(void *__restrict dst, uint16_t src, size_t wNBytes); // Using a function due to better type checks // This seems better than having to do type casts everywhere else @@ -237,7 +223,7 @@ void dcd_init (uint8_t rhport) asm("NOP"); } USB->CNTR = 0; // Enable USB - + USB->BTABLE = DCD_STM32_BTABLE_BASE; reg16_clear_bits(&USB->ISTR, USB_ISTR_ALL_EVENTS); // Clear pending interrupts @@ -249,12 +235,6 @@ void dcd_init (uint8_t rhport) pcd_set_endpoint(USB,i,0u); } - // Initialize the BTABLE for EP0 at this point (though setting up the EP0R is unneeded) - // This is actually not necessary, but helps debugging to start with a blank RAM area - for(uint32_t i=0;i<(DCD_STM32_BTABLE_LENGTH>>1); i++) - { - pma[PMA_STRIDE*(DCD_STM32_BTABLE_BASE + i)] = 0u; - } USB->CNTR |= USB_CNTR_RESETM | USB_CNTR_SOFM | USB_CNTR_ESOFM | USB_CNTR_CTRM | USB_CNTR_SUSPM | USB_CNTR_WKUPM; dcd_handle_bus_reset(); @@ -386,6 +366,7 @@ static void dcd_handle_bus_reset(void) pcd_set_endpoint(USB,i,0u); } + dcd_pma_alloc_reset(); dcd_edpt_open (0, &ep0OUT_desc); dcd_edpt_open (0, &ep0IN_desc); @@ -609,28 +590,85 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * re } } +static void dcd_pma_alloc_reset(void) +{ + ep_buf_ptr = DCD_STM32_BTABLE_BASE + 8*MAX_EP_COUNT; // 8 bytes per endpoint (two TX and two RX words, each) + //TU_LOG2("dcd_pma_alloc_reset()\r\n"); + for(uint32_t i=0; ipma_alloc_size = 0U; + xfer_ctl_ptr(i,TUSB_DIR_IN)->pma_alloc_size = 0U; + xfer_ctl_ptr(i,TUSB_DIR_OUT)->pma_ptr = 0U; + xfer_ctl_ptr(i,TUSB_DIR_IN)->pma_ptr = 0U; + } +} + /*** * Allocate a section of PMA - * Currently statitically allocates 64 bytes for each EP. * - * stm32fsdev have 512 or 1024 bytes of packet RAM, and support 16 EP (8 in and 8 out), - * Static checks above have verified that there is enough packet memory space, so - * this should NEVER fail. + * If the EP number has already been allocated, and the new allocation + * is larger than the old allocation, then this will fail with a TU_ASSERT. + * (This is done to simplify the code. More complicated algorithms could be used) + * + * During failure, TU_ASSERT is used. If this happens, rework/reallocate memory manually. */ static uint16_t dcd_pma_alloc(uint8_t ep_addr, size_t length) { uint8_t const epnum = tu_edpt_number(ep_addr); uint8_t const dir = tu_edpt_dir(ep_addr); + xfer_ctl_t* epXferCtl = xfer_ctl_ptr(epnum,dir); - (void)length; + if(epXferCtl->pma_alloc_size != 0U) + { + //TU_LOG2("dcd_pma_alloc(%x,%x)=%x (cached)\r\n",ep_addr,length,epXferCtl->pma_ptr); + // Previously allocated + TU_ASSERT(length <= epXferCtl->pma_alloc_size, 0xFFFF); // Verify no larger than previous alloc + return epXferCtl->pma_ptr; + } - TU_ASSERT(length <= DCD_STM32_PMA_ALLOC_SIZE); + uint16_t addr = ep_buf_ptr; + ep_buf_ptr = (uint16_t)(ep_buf_ptr + length); // increment buffer pointer + + // Verify no overflow + TU_ASSERT(ep_buf_ptr <= PMA_LENGTH, 0xFFFF); + + epXferCtl->pma_ptr = addr; + epXferCtl->pma_alloc_size = length; + //TU_LOG2("dcd_pma_alloc(%x,%x)=%x\r\n",ep_addr,length,addr); - uint16_t addr = DCD_STM32_BTABLE_BASE + 8*MAX_EP_COUNT; // Each EP needs 8 bytes to store control data - addr += ((2*epnum + dir) * DCD_STM32_PMA_ALLOC_SIZE); return addr; } +/*** + * Free a block of PMA space + */ +static void dcd_pma_free(uint8_t ep_addr) +{ + uint8_t const epnum = tu_edpt_number(ep_addr); + uint8_t const dir = tu_edpt_dir(ep_addr); + + // Presently, this should never be called for EP0 IN/OUT + TU_ASSERT(open_ep_count > 2, /**/); + TU_ASSERT(xfer_ctl_ptr(epnum,dir)->max_packet_size != 0, /**/); + open_ep_count--; + + // If count is 2, only EP0 should be open, so allocations can be mostly reset. + + if(open_ep_count == 2) + { + ep_buf_ptr = DCD_STM32_BTABLE_BASE + 8*MAX_EP_COUNT + 2*CFG_TUD_ENDPOINT0_SIZE; // 8 bytes per endpoint (two TX and two RX words, each), and EP0 + + // Skip EP0 + for(uint32_t i=1; ipma_alloc_size = 0U; + xfer_ctl_ptr(i,TUSB_DIR_IN)->pma_alloc_size = 0U; + xfer_ctl_ptr(i,TUSB_DIR_OUT)->pma_ptr = 0U; + xfer_ctl_ptr(i,TUSB_DIR_IN)->pma_ptr = 0U; + } + } +} + // The STM32F0 doesn't seem to like |= or &= to manipulate the EP#R registers, // so I'm using the #define from HAL here, instead. @@ -640,28 +678,30 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc uint8_t const epnum = tu_edpt_number(p_endpoint_desc->bEndpointAddress); uint8_t const dir = tu_edpt_dir(p_endpoint_desc->bEndpointAddress); const uint16_t epMaxPktSize = p_endpoint_desc->wMaxPacketSize.size; + uint16_t pma_addr; + uint32_t wType; + // Isochronous not supported (yet), and some other driver assumptions. - TU_ASSERT(p_endpoint_desc->bmAttributes.xfer != TUSB_XFER_ISOCHRONOUS); TU_ASSERT(epnum < MAX_EP_COUNT); // Set type switch(p_endpoint_desc->bmAttributes.xfer) { case TUSB_XFER_CONTROL: - pcd_set_eptype(USB, epnum, USB_EP_CONTROL); + wType = USB_EP_CONTROL; break; #if (0) case TUSB_XFER_ISOCHRONOUS: // FIXME: Not yet supported - pcd_set_eptype(USB, epnum, USB_EP_ISOCHRONOUS); break; + wType = USB_EP_ISOCHRONOUS; break; #endif case TUSB_XFER_BULK: - pcd_set_eptype(USB, epnum, USB_EP_BULK); + wType = USB_EP_CONTROL; break; case TUSB_XFER_INTERRUPT: - pcd_set_eptype(USB, epnum, USB_EP_INTERRUPT); + wType = USB_EP_INTERRUPT; break; default: @@ -669,21 +709,24 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc return false; } + pcd_set_eptype(USB, epnum, wType); pcd_set_ep_address(USB, epnum, epnum); // Be normal, for now, instead of only accepting zero-byte packets (on control endpoint) // or being double-buffered (bulk endpoints) pcd_clear_ep_kind(USB,0); + pma_addr = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress, p_endpoint_desc->wMaxPacketSize.size); + if(dir == TUSB_DIR_IN) { - *pcd_ep_tx_address_ptr(USB, epnum) = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress, p_endpoint_desc->wMaxPacketSize.size); + *pcd_ep_tx_address_ptr(USB, epnum) = pma_addr; pcd_set_ep_tx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size); pcd_clear_tx_dtog(USB, epnum); pcd_set_ep_tx_status(USB,epnum,USB_EP_TX_NAK); } else { - *pcd_ep_rx_address_ptr(USB, epnum) = dcd_pma_alloc(p_endpoint_desc->bEndpointAddress, p_endpoint_desc->wMaxPacketSize.size); + *pcd_ep_rx_address_ptr(USB, epnum) = pma_addr; pcd_set_ep_rx_cnt(USB, epnum, p_endpoint_desc->wMaxPacketSize.size); pcd_clear_rx_dtog(USB, epnum); pcd_set_ep_rx_status(USB, epnum, USB_EP_RX_NAK); @@ -707,10 +750,6 @@ void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) uint32_t const epnum = tu_edpt_number(ep_addr); uint32_t const dir = tu_edpt_dir(ep_addr); -#ifndef NDEBUG - TU_ASSERT(epnum < MAX_EP_COUNT, /**/); -#endif - if(dir == TUSB_DIR_IN) { pcd_set_ep_tx_status(USB,epnum,USB_EP_TX_DIS); @@ -719,6 +758,8 @@ void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) { pcd_set_ep_rx_status(USB, epnum, USB_EP_RX_DIS); } + + dcd_pma_free(ep_addr); } // Currently, single-buffered, and only 64 bytes at a time (max) From 2994d100cd6198a4c90dce667f52fde07fe4a073 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Thu, 16 Apr 2020 09:57:56 -0400 Subject: [PATCH 6/6] Remove transfer queue filtering. May need to be revisited later. --- src/device/usbd.c | 47 ++--------------------------------------------- 1 file changed, 2 insertions(+), 45 deletions(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index 75c0a372..55a9cceb 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1064,50 +1064,10 @@ bool usbd_edpt_stalled(uint8_t rhport, uint8_t ep_addr) } /** - * Remove queued xfer complete messages from event queue, - * for a particular ep. + * usbd_edpt_close will disable an endpoint. * - * Must be called with interrupts enabled. - */ -static void usbd_abort_transfers(uint8_t rhport, uint8_t ep_addr) -{ - dcd_event_t ev_sentinal = - { - .event_id = DCD_EVENT_COUNT, ///< This is an invalid event ID. - }; - dcd_event_t event; - uint8_t const epnum = tu_edpt_number(ep_addr); - uint8_t const ep_dir = tu_edpt_dir(ep_addr); - - dcd_int_disable(rhport); - // Queue sentinal element - TU_ASSERT(osal_queue_send(_usbd_q, &ev_sentinal, true), /**/); - - TU_ASSERT(osal_queue_receive(_usbd_q, &event), /**/); - - while(event.event_id != DCD_EVENT_COUNT) - { - if((event.rhport == rhport) && (event.event_id == DCD_EVENT_XFER_COMPLETE) - && (event.xfer_complete.ep_addr == ep_addr)) - { - _usbd_dev.ep_status[epnum][ep_dir].busy = false; - } - else - { - TU_ASSERT(osal_queue_send(_usbd_q, &event, true), /**/); - } - TU_ASSERT(osal_queue_receive(_usbd_q, &event), /**/); - } - - dcd_int_enable(rhport); -} - -/** - * usbd_edpt_close will disable an endpoint, and clear all pending transfers - * through the particular endpoint. + * In progress transfers on this EP may be delivered after this call. * - * It must be called from the usb task (e.g. from the control request - * handler while handling SET_ALTERNATE), with interrupts enabled. */ void usbd_edpt_close(uint8_t rhport, uint8_t ep_addr) { @@ -1116,9 +1076,6 @@ void usbd_edpt_close(uint8_t rhport, uint8_t ep_addr) dcd_edpt_close(rhport, ep_addr); - /* Now, in progress transfers have to be expunged */ - usbd_abort_transfers(rhport, ep_addr); - return; }