From f3a6e564ee9dfe3587d8d9dce051eae76bdb9d0c Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 11 Aug 2021 20:06:57 +0700 Subject: [PATCH 1/8] rp2040 enable suspend and resume interrupt --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 74 ++++++++++++-------- src/portable/raspberrypi/rp2040/hcd_rp2040.c | 7 +- src/portable/raspberrypi/rp2040/rp2040_usb.c | 6 +- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index f78a932f1..f39cfab3e 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -37,6 +37,14 @@ #include "device/dcd.h" +// Current implementation force vbus detection as always present, causing device think it is always plugged into host. +// Therefore it cannot detect disconnect event, mistaken it as suspend. +// Note: won't work if change to 0 (for now) +// +// Note: Line state when disconnected is very sensitive, in actual testing, +// it can toggles between J-state (idle) and SE1 if cable still connected to rp2040 (not connected to host) +#define FORCE_VBUS_DETECT 1 + /*------------------------------------------------------------------*/ /* Low level controller *------------------------------------------------------------------*/ @@ -52,14 +60,14 @@ static struct hw_endpoint hw_endpoints[USB_MAX_ENDPOINTS][2] = {0}; static inline struct hw_endpoint *hw_endpoint_get_by_num(uint8_t num, tusb_dir_t dir) { - return &hw_endpoints[num][dir]; + return &hw_endpoints[num][dir]; } static struct hw_endpoint *hw_endpoint_get_by_addr(uint8_t ep_addr) { - uint8_t num = tu_edpt_number(ep_addr); - tusb_dir_t dir = tu_edpt_dir(ep_addr); - return hw_endpoint_get_by_num(num, dir); + uint8_t num = tu_edpt_number(ep_addr); + tusb_dir_t dir = tu_edpt_dir(ep_addr); + return hw_endpoint_get_by_num(num, dir); } static void _hw_endpoint_alloc(struct hw_endpoint *ep) @@ -290,7 +298,9 @@ static void dcd_rp2040_irq(void) hw_handle_buff_status(); } - // SE0 for 2 us or more, usually together with Bus Reset +#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 if (status & USB_INTS_DEV_CONN_DIS_BITS) { handled |= USB_INTS_DEV_CONN_DIS_BITS; @@ -306,6 +316,7 @@ static void dcd_rp2040_irq(void) usb_hw_clear->sie_status = USB_SIE_STATUS_CONNECTED_BITS; } +#endif // SE0 for 2.5 us or more if (status & USB_INTS_BUS_RESET_BITS) @@ -322,7 +333,7 @@ static void dcd_rp2040_irq(void) #endif } -#if 0 +#if 1 // TODO Enable SUSPEND & RESUME interrupt and test later on with/without VBUS detection /* Note from pico datasheet 4.1.2.6.4 (v1.2) @@ -364,36 +375,43 @@ static void dcd_rp2040_irq(void) /*------------------------------------------------------------------*/ /* Controller API *------------------------------------------------------------------*/ + void dcd_init (uint8_t rhport) { - pico_trace("dcd_init %d\n", rhport); - assert(rhport == 0); + pico_trace("dcd_init %d\n", rhport); + assert(rhport == 0); - // Reset hardware to default state - rp2040_usb_init(); + // Reset hardware to default state + rp2040_usb_init(); - irq_set_exclusive_handler(USBCTRL_IRQ, dcd_rp2040_irq); - memset(hw_endpoints, 0, sizeof(hw_endpoints)); - next_buffer_ptr = &usb_dpram->epx_data[0]; +#if FORCE_VBUS_DETECT + // Force VBUS detect so the device thinks it is plugged into a host + usb_hw->pwr = USB_USB_PWR_VBUS_DETECT_BITS | USB_USB_PWR_VBUS_DETECT_OVERRIDE_EN_BITS; +#endif - // EP0 always exists so init it now - // EP0 OUT - hw_endpoint_init(0x0, 64, TUSB_XFER_CONTROL); - // EP0 IN - hw_endpoint_init(0x80, 64, TUSB_XFER_CONTROL); + irq_set_exclusive_handler(USBCTRL_IRQ, dcd_rp2040_irq); + memset(hw_endpoints, 0, sizeof(hw_endpoints)); + next_buffer_ptr = &usb_dpram->epx_data[0]; - // Initializes the USB peripheral for device mode and enables it. - // Don't need to enable the pull up here. Force VBUS - usb_hw->main_ctrl = USB_MAIN_CTRL_CONTROLLER_EN_BITS; + // EP0 always exists so init it now + // EP0 OUT + hw_endpoint_init(0x0, 64, TUSB_XFER_CONTROL); + // EP0 IN + hw_endpoint_init(0x80, 64, TUSB_XFER_CONTROL); - // Enable individual controller IRQS here. Processor interrupt enable will be used - // for the global interrupt enable... - // TODO Enable SUSPEND & RESUME interrupt - usb_hw->sie_ctrl = USB_SIE_CTRL_EP0_INT_1BUF_BITS; - usb_hw->inte = USB_INTS_BUFF_STATUS_BITS | USB_INTS_BUS_RESET_BITS | USB_INTS_SETUP_REQ_BITS | - USB_INTS_DEV_CONN_DIS_BITS /* | USB_INTS_DEV_SUSPEND_BITS | USB_INTS_DEV_RESUME_FROM_HOST_BITS */ ; + // Initializes the USB peripheral for device mode and enables it. + // Don't need to enable the pull up here. Force VBUS + usb_hw->main_ctrl = USB_MAIN_CTRL_CONTROLLER_EN_BITS; - dcd_connect(rhport); + // Enable individual controller IRQS here. Processor interrupt enable will be used + // for the global interrupt enable... + // Note: Force VBUS detect cause disconnection not detectable + usb_hw->sie_ctrl = USB_SIE_CTRL_EP0_INT_1BUF_BITS; + usb_hw->inte = USB_INTS_BUFF_STATUS_BITS | USB_INTS_BUS_RESET_BITS | USB_INTS_SETUP_REQ_BITS | + USB_INTS_DEV_SUSPEND_BITS | USB_INTS_DEV_RESUME_FROM_HOST_BITS | + (FORCE_VBUS_DETECT ? 0 : USB_INTS_DEV_CONN_DIS_BITS); + + dcd_connect(rhport); } void dcd_int_enable(uint8_t rhport) diff --git a/src/portable/raspberrypi/rp2040/hcd_rp2040.c b/src/portable/raspberrypi/rp2040/hcd_rp2040.c index 40a0805c6..2c007ddd3 100644 --- a/src/portable/raspberrypi/rp2040/hcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/hcd_rp2040.c @@ -61,8 +61,8 @@ static struct hw_endpoint ep_pool[1 + PICO_USB_HOST_INTERRUPT_ENDPOINTS]; // Flags we set by default in sie_ctrl (we add other bits on top) enum { - SIE_CTRL_BASE = USB_SIE_CTRL_SOF_EN_BITS | USB_SIE_CTRL_KEEP_ALIVE_EN_BITS | - USB_SIE_CTRL_PULLDOWN_EN_BITS | USB_SIE_CTRL_EP0_INT_1BUF_BITS + SIE_CTRL_BASE = USB_SIE_CTRL_SOF_EN_BITS | USB_SIE_CTRL_KEEP_ALIVE_EN_BITS | + USB_SIE_CTRL_PULLDOWN_EN_BITS | USB_SIE_CTRL_EP0_INT_1BUF_BITS }; static struct hw_endpoint *get_dev_ep(uint8_t dev_addr, uint8_t ep_addr) @@ -360,6 +360,9 @@ bool hcd_init(uint8_t rhport) // Reset any previous state rp2040_usb_init(); + // Force VBUS detect to always present, for now we assume vbus is always provided (without using VBUS En) + usb_hw->pwr = USB_USB_PWR_VBUS_DETECT_BITS | USB_USB_PWR_VBUS_DETECT_OVERRIDE_EN_BITS; + irq_set_exclusive_handler(USBCTRL_IRQ, hcd_rp2040_irq); // clear epx and interrupt eps diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index dc12e6d82..43554d28b 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -62,11 +62,7 @@ void rp2040_usb_init(void) memset(usb_dpram, 0, sizeof(*usb_dpram)); // Mux the controller to the onboard usb phy - usb_hw->muxing = USB_USB_MUXING_TO_PHY_BITS | USB_USB_MUXING_SOFTCON_BITS; - - // Force VBUS detect so the device thinks it is plugged into a host - // TODO support VBUs detect - usb_hw->pwr = USB_USB_PWR_VBUS_DETECT_BITS | USB_USB_PWR_VBUS_DETECT_OVERRIDE_EN_BITS; + usb_hw->muxing = USB_USB_MUXING_TO_PHY_BITS | USB_USB_MUXING_SOFTCON_BITS; } void hw_endpoint_reset_transfer(struct hw_endpoint *ep) From 979af6c2a859a1e76930d429f37abff6a224599a Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 11 Aug 2021 20:29:39 +0700 Subject: [PATCH 2/8] clean up endpoint set/clear stall --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 78 +++++++++----------- src/portable/raspberrypi/rp2040/rp2040_usb.h | 2 +- 2 files changed, 35 insertions(+), 45 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index f39cfab3e..6b1e79af4 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -230,7 +230,6 @@ static void reset_ep0(void) { struct hw_endpoint *ep = hw_endpoint_get_by_addr(addrs[i]); ep->next_pid = 1u; - ep->stalled = 0; } } @@ -241,39 +240,9 @@ static void ep0_0len_status(void) hw_endpoint_xfer(0x80, NULL, 0); } -static void _hw_endpoint_stall(struct hw_endpoint *ep) +static void bus_reset(void) { - assert(!ep->stalled); - if (tu_edpt_number(ep->ep_addr) == 0) - { - // A stall on EP0 has to be armed so it can be cleared on the next setup packet - usb_hw_set->ep_stall_arm = (tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN) ? USB_EP_STALL_ARM_EP0_IN_BITS : USB_EP_STALL_ARM_EP0_OUT_BITS; - } - _hw_endpoint_buffer_control_set_mask32(ep, USB_BUF_CTRL_STALL); - ep->stalled = true; -} -static void hw_endpoint_stall(uint8_t ep_addr) -{ - struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); - _hw_endpoint_stall(ep); -} - -static void _hw_endpoint_clear_stall(struct hw_endpoint *ep) -{ - if (tu_edpt_number(ep->ep_addr) == 0) - { - // Probably already been cleared but no harm - usb_hw_clear->ep_stall_arm = (tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN) ? USB_EP_STALL_ARM_EP0_IN_BITS : USB_EP_STALL_ARM_EP0_OUT_BITS; - } - _hw_endpoint_buffer_control_clear_mask32(ep, USB_BUF_CTRL_STALL); - ep->stalled = false; -} - -static void hw_endpoint_clear_stall(uint8_t ep_addr) -{ - struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); - _hw_endpoint_clear_stall(ep); } static void dcd_rp2040_irq(void) @@ -322,9 +291,13 @@ static void dcd_rp2040_irq(void) if (status & USB_INTS_BUS_RESET_BITS) { pico_trace("BUS RESET\n"); - usb_hw->dev_addr_ctrl = 0; + handled |= USB_INTS_BUS_RESET_BITS; + + usb_hw->dev_addr_ctrl = 0; + bus_reset(); dcd_event_bus_reset(0, TUSB_SPEED_FULL, true); + usb_hw_clear->sie_status = USB_SIE_STATUS_BUS_RESET_BITS; #if TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX @@ -492,20 +465,37 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t t return true; } -void dcd_edpt_stall (uint8_t rhport, uint8_t ep_addr) +void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { - pico_trace("dcd_edpt_stall %02x\n", ep_addr); - assert(rhport == 0); - hw_endpoint_stall(ep_addr); + pico_trace("dcd_edpt_stall %02x\n", ep_addr); + assert(rhport == 0); + + if ( tu_edpt_number(ep_addr) == 0 ) + { + // A stall on EP0 has to be armed so it can be cleared on the next setup packet + usb_hw_set->ep_stall_arm = (tu_edpt_dir(ep_addr) == TUSB_DIR_IN) ? USB_EP_STALL_ARM_EP0_IN_BITS : USB_EP_STALL_ARM_EP0_OUT_BITS; + } + + struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); + + // TODO check with double buffered + _hw_endpoint_buffer_control_set_mask32(ep, USB_BUF_CTRL_STALL); } -void dcd_edpt_clear_stall (uint8_t rhport, uint8_t ep_addr) +void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { - pico_trace("dcd_edpt_clear_stall %02x\n", ep_addr); - assert(rhport == 0); - hw_endpoint_clear_stall(ep_addr); -} + pico_trace("dcd_edpt_clear_stall %02x\n", ep_addr); + assert(rhport == 0); + if (tu_edpt_number(ep_addr)) + { + struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); + + // clear stall also reset toggle to DATA0 + // TODO check with double buffered + _hw_endpoint_buffer_control_clear_mask32(ep, USB_BUF_CTRL_STALL | USB_BUF_CTRL_DATA1_PID); + } +} void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) { @@ -518,8 +508,8 @@ void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr) void dcd_int_handler(uint8_t rhport) { - (void) rhport; - dcd_rp2040_irq(); + (void) rhport; + dcd_rp2040_irq(); } #endif diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.h b/src/portable/raspberrypi/rp2040/rp2040_usb.h index 514152cd9..5570a731a 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.h +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.h @@ -42,7 +42,7 @@ struct hw_endpoint // Buffer pointer in usb dpram uint8_t *hw_data_buf; - // Have we been stalled + // Have we been stalled TODO remove later bool stalled; // Current transfer information From a2baf9427d62ecb49b8d0e58a7ddb12698ddf814 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 11 Aug 2021 20:36:23 +0700 Subject: [PATCH 3/8] more dcd clean up --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 6b1e79af4..0dabdd1d7 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -233,13 +233,6 @@ static void reset_ep0(void) } } -static void ep0_0len_status(void) -{ - // Send 0len complete response on EP0 IN - reset_ep0(); - hw_endpoint_xfer(0x80, NULL, 0); -} - static void bus_reset(void) { @@ -405,7 +398,9 @@ void dcd_set_address (uint8_t rhport, uint8_t dev_addr) assert(rhport == 0); // Can't set device address in hardware until status xfer has complete - ep0_0len_status(); + // Send 0len complete response on EP0 IN + reset_ep0(); + hw_endpoint_xfer(0x80, NULL, 0); } void dcd_remote_wakeup(uint8_t rhport) From 88d4cb402d5db6cb88ffa0aecb78fccfe3ab0afd Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 12 Aug 2021 00:11:04 +0700 Subject: [PATCH 4/8] simplify hw_endpoint_init() --- src/device/dcd.h | 1 + src/portable/raspberrypi/rp2040/dcd_rp2040.c | 65 +++++++++----------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/device/dcd.h b/src/device/dcd.h index 8bfad9b72..66767c1fe 100644 --- a/src/device/dcd.h +++ b/src/device/dcd.h @@ -152,6 +152,7 @@ bool dcd_edpt_xfer_fifo (uint8_t rhport, uint8_t ep_addr, tu_fifo_t * ff, void dcd_edpt_stall (uint8_t rhport, uint8_t ep_addr); // clear stall, data toggle is also reset to DATA0 +// This API never calls with control endpoints, since it is auto cleared when receiving setup packet void dcd_edpt_clear_stall (uint8_t rhport, uint8_t ep_addr); //--------------------------------------------------------------------+ diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 0dabdd1d7..ca083d500 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -101,8 +101,29 @@ static void _hw_endpoint_alloc(struct hw_endpoint *ep) *ep->endpoint_control = reg; } -static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint16_t wMaxPacketSize, uint8_t transfer_type) +#if 0 // todo unused +static void _hw_endpoint_close(struct hw_endpoint *ep) { + // Clear hardware registers and then zero the struct + // Clears endpoint enable + *ep->endpoint_control = 0; + // Clears buffer available, etc + *ep->buffer_control = 0; + // Clear any endpoint state + memset(ep, 0, sizeof(struct hw_endpoint)); +} + +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) +{ + struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); + const uint8_t num = tu_edpt_number(ep_addr); const tusb_dir_t dir = tu_edpt_dir(ep_addr); @@ -112,7 +133,7 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint16_t ep->rx = (dir == TUSB_DIR_OUT); // Response to a setup packet on EP0 starts with pid of 1 - ep->next_pid = num == 0 ? 1u : 0u; + ep->next_pid = (num == 0 ? 1u : 0u); ep->wMaxPacketSize = wMaxPacketSize; ep->transfer_type = transfer_type; @@ -152,7 +173,7 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint16_t } // Now if it hasn't already been done - //alloc a buffer and fill in endpoint control register + // alloc a buffer and fill in endpoint control register // TODO device may change configuration (dynamic), should clear and reallocate if(!(ep->configured)) { @@ -163,31 +184,6 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint16_t ep->configured = true; } -#if 0 // todo unused -static void _hw_endpoint_close(struct hw_endpoint *ep) -{ - // Clear hardware registers and then zero the struct - // Clears endpoint enable - *ep->endpoint_control = 0; - // Clears buffer available, etc - *ep->buffer_control = 0; - // Clear any endpoint state - memset(ep, 0, sizeof(struct hw_endpoint)); -} - -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 bmAttributes) -{ - struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); - _hw_endpoint_init(ep, ep_addr, wMaxPacketSize, bmAttributes); -} - static void hw_endpoint_xfer(uint8_t ep_addr, uint8_t *buffer, uint16_t total_bytes) { struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); @@ -290,7 +286,6 @@ static void dcd_rp2040_irq(void) usb_hw->dev_addr_ctrl = 0; bus_reset(); dcd_event_bus_reset(0, TUSB_SPEED_FULL, true); - usb_hw_clear->sie_status = USB_SIE_STATUS_BUS_RESET_BITS; #if TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX @@ -394,13 +389,13 @@ void dcd_int_disable(uint8_t rhport) void dcd_set_address (uint8_t rhport, uint8_t dev_addr) { - pico_trace("dcd_set_address %d %d\n", rhport, dev_addr); - assert(rhport == 0); + pico_trace("dcd_set_address %d %d\n", rhport, dev_addr); + assert(rhport == 0); - // Can't set device address in hardware until status xfer has complete - // Send 0len complete response on EP0 IN - reset_ep0(); - hw_endpoint_xfer(0x80, NULL, 0); + // Can't set device address in hardware until status xfer has complete + // Send 0len complete response on EP0 IN + reset_ep0(); + hw_endpoint_xfer(0x80, NULL, 0); } void dcd_remote_wakeup(uint8_t rhport) From 4f2999bc0488cf6bb9823ea315e0960888d1d593 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 12 Aug 2021 00:12:15 +0700 Subject: [PATCH 5/8] white space --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 96 ++++++++++---------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index ca083d500..24c6c010e 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -70,13 +70,13 @@ static struct hw_endpoint *hw_endpoint_get_by_addr(uint8_t ep_addr) return hw_endpoint_get_by_num(num, dir); } -static void _hw_endpoint_alloc(struct hw_endpoint *ep) +static void _hw_endpoint_alloc(struct hw_endpoint *ep, uint8_t transfer_type) { // size must be multiple of 64 uint16_t size = tu_div_ceil(ep->wMaxPacketSize, 64) * 64u; // double buffered for Control and Bulk endpoint - if ( ep->transfer_type == TUSB_XFER_CONTROL || ep->transfer_type == TUSB_XFER_BULK) + if ( transfer_type == TUSB_XFER_CONTROL || transfer_type == TUSB_XFER_BULK ) { size *= 2u; } @@ -96,7 +96,7 @@ static void _hw_endpoint_alloc(struct hw_endpoint *ep) ep_dir_string[tu_edpt_dir(ep->ep_addr)]); // Fill in endpoint control register with buffer offset - uint32_t const reg = EP_CTRL_ENABLE_BITS | (ep->transfer_type << EP_CTRL_BUFFER_TYPE_LSB) | dpram_offset; + uint32_t const reg = EP_CTRL_ENABLE_BITS | (transfer_type << EP_CTRL_BUFFER_TYPE_LSB) | dpram_offset; *ep->endpoint_control = reg; } @@ -122,66 +122,66 @@ static void hw_endpoint_close(uint8_t ep_addr) static void hw_endpoint_init(uint8_t ep_addr, uint16_t wMaxPacketSize, uint8_t transfer_type) { - struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); + struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr); - const uint8_t num = tu_edpt_number(ep_addr); - const tusb_dir_t dir = tu_edpt_dir(ep_addr); + const uint8_t num = tu_edpt_number(ep_addr); + const tusb_dir_t dir = tu_edpt_dir(ep_addr); - ep->ep_addr = ep_addr; + ep->ep_addr = ep_addr; - // For device, IN is a tx transfer and OUT is an rx transfer - ep->rx = (dir == TUSB_DIR_OUT); + // For device, IN is a tx transfer and OUT is an rx transfer + ep->rx = (dir == TUSB_DIR_OUT); - // Response to a setup packet on EP0 starts with pid of 1 - ep->next_pid = (num == 0 ? 1u : 0u); + // Response to a setup packet on EP0 starts with pid of 1 + ep->next_pid = (num == 0 ? 1u : 0u); - ep->wMaxPacketSize = wMaxPacketSize; - ep->transfer_type = transfer_type; + ep->wMaxPacketSize = wMaxPacketSize; + ep->transfer_type = transfer_type; - // Every endpoint has a buffer control register in dpram - if (dir == TUSB_DIR_IN) + // Every endpoint has a buffer control register in dpram + if ( dir == TUSB_DIR_IN ) + { + ep->buffer_control = &usb_dpram->ep_buf_ctrl[num].in; + } + else + { + ep->buffer_control = &usb_dpram->ep_buf_ctrl[num].out; + } + + // Clear existing buffer control state + *ep->buffer_control = 0; + + if ( num == 0 ) + { + // EP0 has no endpoint control register because + // the buffer offsets are fixed + ep->endpoint_control = NULL; + + // Buffer offset is fixed + ep->hw_data_buf = (uint8_t*) &usb_dpram->ep0_buf_a[0]; + } + else + { + // Set the endpoint control register (starts at EP1, hence num-1) + if ( dir == TUSB_DIR_IN ) { - ep->buffer_control = &usb_dpram->ep_buf_ctrl[num].in; + ep->endpoint_control = &usb_dpram->ep_ctrl[num - 1].in; } else { - ep->buffer_control = &usb_dpram->ep_buf_ctrl[num].out; + ep->endpoint_control = &usb_dpram->ep_ctrl[num - 1].out; } - // Clear existing buffer control state - *ep->buffer_control = 0; - - if (num == 0) + // Now if it hasn't already been done + // alloc a buffer and fill in endpoint control register + // TODO device may change configuration (dynamic), should clear and reallocate + if ( !(ep->configured) ) { - // EP0 has no endpoint control register because - // the buffer offsets are fixed - ep->endpoint_control = NULL; - - // Buffer offset is fixed - ep->hw_data_buf = (uint8_t*)&usb_dpram->ep0_buf_a[0]; + _hw_endpoint_alloc(ep, transfer_type); } - else - { - // Set the endpoint control register (starts at EP1, hence num-1) - if (dir == TUSB_DIR_IN) - { - ep->endpoint_control = &usb_dpram->ep_ctrl[num-1].in; - } - else - { - ep->endpoint_control = &usb_dpram->ep_ctrl[num-1].out; - } + } - // Now if it hasn't already been done - // alloc a buffer and fill in endpoint control register - // TODO device may change configuration (dynamic), should clear and reallocate - if(!(ep->configured)) - { - _hw_endpoint_alloc(ep); - } - } - - ep->configured = true; + ep->configured = true; } static void hw_endpoint_xfer(uint8_t ep_addr, uint8_t *buffer, uint16_t total_bytes) From 4ad47d9e264a7119f5429a045a185ad0d448d998 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 12 Aug 2021 15:40:26 +0700 Subject: [PATCH 6/8] bus_reset will reset all endpoints allow for dynamic configuration as well as state-less enumeration --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 50 +++++++------------- 1 file changed, 17 insertions(+), 33 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 24c6c010e..63d8278da 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -40,9 +40,6 @@ // Current implementation force vbus detection as always present, causing device think it is always plugged into host. // Therefore it cannot detect disconnect event, mistaken it as suspend. // Note: won't work if change to 0 (for now) -// -// Note: Line state when disconnected is very sensitive, in actual testing, -// it can toggles between J-state (idle) and SE1 if cable still connected to rp2040 (not connected to host) #define FORCE_VBUS_DETECT 1 /*------------------------------------------------------------------*/ @@ -56,7 +53,7 @@ static uint8_t *next_buffer_ptr; // USB_MAX_ENDPOINTS Endpoints, direction TUSB_DIR_OUT for out and TUSB_DIR_IN for in. -static struct hw_endpoint hw_endpoints[USB_MAX_ENDPOINTS][2] = {0}; +static struct hw_endpoint hw_endpoints[USB_MAX_ENDPOINTS][2]; static inline struct hw_endpoint *hw_endpoint_get_by_num(uint8_t num, tusb_dir_t dir) { @@ -75,8 +72,8 @@ static void _hw_endpoint_alloc(struct hw_endpoint *ep, uint8_t transfer_type) // size must be multiple of 64 uint16_t size = tu_div_ceil(ep->wMaxPacketSize, 64) * 64u; - // double buffered for Control and Bulk endpoint - if ( transfer_type == TUSB_XFER_CONTROL || transfer_type == TUSB_XFER_BULK ) + // double buffered Bulk endpoint + if ( transfer_type == TUSB_XFER_BULK ) { size *= 2u; } @@ -88,12 +85,7 @@ static void _hw_endpoint_alloc(struct hw_endpoint *ep, uint8_t transfer_type) uint dpram_offset = hw_data_offset(ep->hw_data_buf); assert(hw_data_offset(next_buffer_ptr) <= USB_DPRAM_MAX); - pico_info("Alloced %d bytes at offset 0x%x (0x%p) for ep %d %s\n", - size, - dpram_offset, - ep->hw_data_buf, - tu_edpt_number(ep->ep_addr), - ep_dir_string[tu_edpt_dir(ep->ep_addr)]); + pico_info(" Alloced %d bytes at offset 0x%x (0x%p)\r\n", size, dpram_offset, ep->hw_data_buf); // Fill in endpoint control register with buffer offset uint32_t const reg = EP_CTRL_ENABLE_BITS | (transfer_type << EP_CTRL_BUFFER_TYPE_LSB) | dpram_offset; @@ -157,7 +149,7 @@ static void hw_endpoint_init(uint8_t ep_addr, uint16_t wMaxPacketSize, uint8_t t // the buffer offsets are fixed ep->endpoint_control = NULL; - // Buffer offset is fixed + // Buffer offset is fixed (also double buffered) ep->hw_data_buf = (uint8_t*) &usb_dpram->ep0_buf_a[0]; } else @@ -172,16 +164,9 @@ static void hw_endpoint_init(uint8_t ep_addr, uint16_t wMaxPacketSize, uint8_t t ep->endpoint_control = &usb_dpram->ep_ctrl[num - 1].out; } - // Now if it hasn't already been done // alloc a buffer and fill in endpoint control register - // TODO device may change configuration (dynamic), should clear and reallocate - if ( !(ep->configured) ) - { - _hw_endpoint_alloc(ep, transfer_type); - } + _hw_endpoint_alloc(ep, transfer_type); } - - ep->configured = true; } static void hw_endpoint_xfer(uint8_t ep_addr, uint8_t *buffer, uint16_t total_bytes) @@ -229,14 +214,19 @@ static void reset_ep0(void) } } -static void bus_reset(void) +static void reset_all_endpoints(void) { + memset(hw_endpoints, 0, sizeof(hw_endpoints)); + next_buffer_ptr = &usb_dpram->epx_data[0]; + // Init Control endpoint out & in + hw_endpoint_init(0x0, 64, TUSB_XFER_CONTROL); + hw_endpoint_init(0x80, 64, TUSB_XFER_CONTROL); } static void dcd_rp2040_irq(void) { - uint32_t status = usb_hw->ints; + uint32_t const status = usb_hw->ints; uint32_t handled = 0; if (status & USB_INTS_SETUP_REQ_BITS) @@ -276,7 +266,7 @@ static void dcd_rp2040_irq(void) } #endif - // SE0 for 2.5 us or more + // SE0 for 2.5 us or more (will last at least 10ms) if (status & USB_INTS_BUS_RESET_BITS) { pico_trace("BUS RESET\n"); @@ -284,7 +274,7 @@ static void dcd_rp2040_irq(void) handled |= USB_INTS_BUS_RESET_BITS; usb_hw->dev_addr_ctrl = 0; - bus_reset(); + reset_all_endpoints(); dcd_event_bus_reset(0, TUSB_SPEED_FULL, true); usb_hw_clear->sie_status = USB_SIE_STATUS_BUS_RESET_BITS; @@ -351,14 +341,9 @@ void dcd_init (uint8_t rhport) #endif irq_set_exclusive_handler(USBCTRL_IRQ, dcd_rp2040_irq); - memset(hw_endpoints, 0, sizeof(hw_endpoints)); - next_buffer_ptr = &usb_dpram->epx_data[0]; - // EP0 always exists so init it now - // EP0 OUT - hw_endpoint_init(0x0, 64, TUSB_XFER_CONTROL); - // EP0 IN - hw_endpoint_init(0x80, 64, TUSB_XFER_CONTROL); + // reset endpoints + reset_all_endpoints(); // Initializes the USB peripheral for device mode and enables it. // Don't need to enable the pull up here. Force VBUS @@ -442,7 +427,6 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * re bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * desc_edpt) { - pico_info("dcd_edpt_open %02x\n", desc_edpt->bEndpointAddress); assert(rhport == 0); hw_endpoint_init(desc_edpt->bEndpointAddress, desc_edpt->wMaxPacketSize.size, desc_edpt->bmAttributes.xfer); return true; From 17ef9f4843250e8c6b6b1fde53ed6a54a4f20470 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 12 Aug 2021 15:46:33 +0700 Subject: [PATCH 7/8] add ready check for edpt claim --- src/device/usbd.c | 5 +++++ src/portable/raspberrypi/rp2040/dcd_rp2040.c | 4 ---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index a30eab217..bef7725b5 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1191,6 +1191,8 @@ bool usbd_edpt_claim(uint8_t rhport, uint8_t ep_addr) { (void) rhport; + TU_VERIFY(tud_ready()); + uint8_t const epnum = tu_edpt_number(ep_addr); uint8_t const dir = tu_edpt_dir(ep_addr); @@ -1244,6 +1246,9 @@ bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t uint8_t const epnum = tu_edpt_number(ep_addr); uint8_t const dir = tu_edpt_dir(ep_addr); + // TODO skip ready() check for now since enumeration also use this API + // TU_VERIFY(tud_ready()); + TU_LOG2(" Queue EP %02X with %u bytes ...\r\n", ep_addr, total_bytes); // Attempt to transfer on a busy endpoint, sound like an race condition ! diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 63d8278da..49284e92e 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -284,9 +284,6 @@ static void dcd_rp2040_irq(void) #endif } -#if 1 - // TODO Enable SUSPEND & RESUME interrupt and test later on with/without VBUS detection - /* Note from pico datasheet 4.1.2.6.4 (v1.2) * If you enable the suspend interrupt, it is likely you will see a suspend interrupt when * the device is first connected but the bus is idle. The bus can be idle for a few ms before @@ -308,7 +305,6 @@ static void dcd_rp2040_irq(void) dcd_event_bus_signal(0, DCD_EVENT_RESUME, true); usb_hw_clear->sie_status = USB_SIE_STATUS_RESUME_BITS; } -#endif if (status ^ handled) { From d52b981c3af06233891e827edcd484ece244a5a8 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 12 Aug 2021 17:07:39 +0700 Subject: [PATCH 8/8] revert ready() check in claim (do it later in separated PR) --- src/device/usbd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index bef7725b5..a15959710 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1191,7 +1191,8 @@ bool usbd_edpt_claim(uint8_t rhport, uint8_t ep_addr) { (void) rhport; - TU_VERIFY(tud_ready()); + // TODO add this check later, also make sure we don't starve an out endpoint while suspending + // TU_VERIFY(tud_ready()); uint8_t const epnum = tu_edpt_number(ep_addr); uint8_t const dir = tu_edpt_dir(ep_addr);