From 48e1f6d8990e450783f4887e41ce58fe0e463953 Mon Sep 17 00:00:00 2001 From: Valentin Milea Date: Sat, 4 Dec 2021 16:04:48 +0200 Subject: [PATCH 1/3] Handle the closing of endpoints on RP2040 --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 28 ++++++++++++++++---- src/portable/raspberrypi/rp2040/rp2040_usb.c | 4 ++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index e084478e0..8be16ad22 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -83,7 +83,7 @@ static void _hw_endpoint_alloc(struct hw_endpoint *ep, uint8_t transfer_type) assert(((uintptr_t )next_buffer_ptr & 0b111111u) == 0); uint dpram_offset = hw_data_offset(ep->hw_data_buf); - assert(hw_data_offset(next_buffer_ptr) <= USB_DPRAM_MAX); + hard_assert(hw_data_offset(next_buffer_ptr) <= USB_DPRAM_MAX); pico_info(" Alloced %d bytes at offset 0x%x (0x%p)\r\n", size, dpram_offset, ep->hw_data_buf); @@ -93,7 +93,6 @@ static void _hw_endpoint_alloc(struct hw_endpoint *ep, uint8_t transfer_type) *ep->endpoint_control = reg; } -#if 0 // todo unused static void _hw_endpoint_close(struct hw_endpoint *ep) { // Clear hardware registers and then zero the struct @@ -103,6 +102,22 @@ static void _hw_endpoint_close(struct hw_endpoint *ep) *ep->buffer_control = 0; // Clear any endpoint state memset(ep, 0, sizeof(struct hw_endpoint)); + + // Reclaim buffer space if all endpoints are closed + bool reclaim_buffers = true; + for ( uint8_t i = 1; i < USB_MAX_ENDPOINTS; i++ ) + { + if (hw_endpoint_get_by_num(i, TUSB_DIR_OUT)->hw_data_buf != NULL || hw_endpoint_get_by_num(i, TUSB_DIR_IN)->hw_data_buf != NULL) + { + reclaim_buffers = false; + break; + } + } + if (reclaim_buffers) + { + pico_info(" reclaim buffer space\n"); + next_buffer_ptr = &usb_dpram->epx_data[0]; + } } static void hw_endpoint_close(uint8_t ep_addr) @@ -110,7 +125,6 @@ static void hw_endpoint_close(uint8_t ep_addr) struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); _hw_endpoint_close(ep); } -#endif static void hw_endpoint_init(uint8_t ep_addr, uint16_t wMaxPacketSize, uint8_t transfer_type) { @@ -224,6 +238,8 @@ static void reset_non_control_endpoints(void) // clear non-control hw endpoints tu_memclr(hw_endpoints[1], sizeof(hw_endpoints) - 2*sizeof(hw_endpoint_t)); + + pico_info(" reclaim buffer space\n"); next_buffer_ptr = &usb_dpram->epx_data[0]; } @@ -479,10 +495,12 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) { (void) rhport; - (void) ep_addr; - // usbd.c says: In progress transfers on this EP may be delivered after this call pico_trace("dcd_edpt_close %02x\n", ep_addr); + + // usbd.c says: In progress transfers on this EP may be delivered after this call. + // If the endpoint is no longer active when the transfer event is delivered, it will be ignored. + hw_endpoint_close(ep_addr); } void dcd_int_handler(uint8_t rhport) diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index c9e2f6b26..0a943b676 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -294,7 +294,9 @@ bool hw_endpoint_xfer_continue(struct hw_endpoint *ep) // Part way through a transfer if (!ep->active) { - panic("Can't continue xfer on inactive ep %d %s", tu_edpt_number(ep->ep_addr), ep_dir_string); + pico_info("Ignore xfer on inactive ep %d %s", tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); + _hw_endpoint_lock_update(ep, -1); + return false; } // Update EP struct from hardware state From 36e69b86bf0f95916a66a35874da15fb420296e1 Mon Sep 17 00:00:00 2001 From: Valentin Milea Date: Tue, 7 Dec 2021 15:35:30 +0200 Subject: [PATCH 2/3] Remove buffer reclaim logs --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 8be16ad22..3a3bef6a5 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -115,7 +115,6 @@ static void _hw_endpoint_close(struct hw_endpoint *ep) } if (reclaim_buffers) { - pico_info(" reclaim buffer space\n"); next_buffer_ptr = &usb_dpram->epx_data[0]; } } @@ -239,7 +238,7 @@ static void reset_non_control_endpoints(void) // clear non-control hw endpoints tu_memclr(hw_endpoints[1], sizeof(hw_endpoints) - 2*sizeof(hw_endpoint_t)); - pico_info(" reclaim buffer space\n"); + // reclaim buffer space next_buffer_ptr = &usb_dpram->epx_data[0]; } From ae970ba2e2cc83881c52e66cd79fad5947e5e59e Mon Sep 17 00:00:00 2001 From: Valentin Milea Date: Wed, 8 Dec 2021 12:33:30 +0200 Subject: [PATCH 3/3] Handle xfer events before closing EP --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 17 ++++++++--------- src/portable/raspberrypi/rp2040/rp2040_usb.c | 4 +--- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 3a3bef6a5..42add3167 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -247,6 +247,14 @@ static void dcd_rp2040_irq(void) uint32_t const status = usb_hw->ints; uint32_t handled = 0; + // xfer events are handled before setup req. So if a transfer completes immediately + // before closing the EP, the events will be delivered in same order. + if (status & USB_INTS_BUFF_STATUS_BITS) + { + handled |= USB_INTS_BUFF_STATUS_BITS; + hw_handle_buff_status(); + } + if (status & USB_INTS_SETUP_REQ_BITS) { handled |= USB_INTS_SETUP_REQ_BITS; @@ -260,12 +268,6 @@ static void dcd_rp2040_irq(void) usb_hw_clear->sie_status = USB_SIE_STATUS_SETUP_REC_BITS; } - if (status & USB_INTS_BUFF_STATUS_BITS) - { - handled |= USB_INTS_BUFF_STATUS_BITS; - hw_handle_buff_status(); - } - #if FORCE_VBUS_DETECT == 0 // Since we force VBUS detect On, device will always think it is connected and // couldn't distinguish between disconnect and suspend @@ -496,9 +498,6 @@ void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) (void) rhport; pico_trace("dcd_edpt_close %02x\n", ep_addr); - - // usbd.c says: In progress transfers on this EP may be delivered after this call. - // If the endpoint is no longer active when the transfer event is delivered, it will be ignored. hw_endpoint_close(ep_addr); } diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index 0a943b676..9d833e65f 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -294,9 +294,7 @@ bool hw_endpoint_xfer_continue(struct hw_endpoint *ep) // Part way through a transfer if (!ep->active) { - pico_info("Ignore xfer on inactive ep %d %s", tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); - _hw_endpoint_lock_update(ep, -1); - return false; + panic("Can't continue xfer on inactive ep %d %s", tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); } // Update EP struct from hardware state