From d7558e8a0f12c685f2690c985e5f75df0777065b Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 28 Nov 2019 13:39:29 +0700 Subject: [PATCH] use dcd_edpt0_status_complete() to set address without blocking for samd21/samd51/stm32_fsdev --- src/device/usbd.c | 25 ++++++-------- src/device/usbd_control.c | 12 +++++-- src/portable/microchip/samd21/dcd_samd21.c | 25 ++++++++++---- src/portable/microchip/samd51/dcd_samd51.c | 23 ++++++++++--- src/portable/microchip/samg/dcd_samg.c | 5 ++- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 34 +++++++++++-------- src/portable/st/synopsys/dcd_synopsys.c | 1 - 7 files changed, 82 insertions(+), 43 deletions(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index 01ec0e5f..497281c2 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -202,9 +202,10 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const static bool process_set_config(uint8_t rhport, uint8_t cfg_num); static bool process_get_descriptor(uint8_t rhport, tusb_control_request_t const * p_request); -void usbd_control_reset (uint8_t rhport); -bool usbd_control_xfer_cb (uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t xferred_bytes); +void usbd_control_reset(void); +void usbd_control_set_request(tusb_control_request_t const *request); void usbd_control_set_complete_callback( bool (*fp) (uint8_t, tusb_control_request_t const * ) ); +bool usbd_control_xfer_cb (uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t xferred_bytes); //--------------------------------------------------------------------+ @@ -321,7 +322,7 @@ static void usbd_reset(uint8_t rhport) memset(_usbd_dev.itf2drv, DRVID_INVALID, sizeof(_usbd_dev.itf2drv)); // invalid mapping memset(_usbd_dev.ep2drv , DRVID_INVALID, sizeof(_usbd_dev.ep2drv )); // invalid mapping - usbd_control_reset(rhport); + usbd_control_reset(); for (uint8_t i = 0; i < USBD_CLASS_DRIVER_COUNT; i++) { @@ -376,7 +377,7 @@ void tud_task (void) case DCD_EVENT_SETUP_RECEIVED: TU_LOG2(" "); - TU_LOG1_MEM(&event.setup_received, 1, 8); + TU_LOG2_MEM(&event.setup_received, 1, 8); // Mark as connected after receiving 1st setup packet. // But it is easier to set it every time instead of wasting time to check then set @@ -385,7 +386,7 @@ void tud_task (void) // Process control request if ( !process_control_request(event.rhport, &event.setup_received) ) { - TU_LOG1(" Stall EP0\r\n"); + TU_LOG2(" Stall EP0\r\n"); // Failed -> stall both control endpoint IN and OUT dcd_edpt_stall(event.rhport, 0); dcd_edpt_stall(event.rhport, 0 | TUSB_DIR_IN_MASK); @@ -499,16 +500,12 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const switch ( p_request->bRequest ) { case TUSB_REQ_SET_ADDRESS: - // Depending on mcu, status phase could be sent either before or after changing device address - // Therefore DCD must include zero-length status response + // Depending on mcu, status phase could be sent either before or after changing device address, + // or even require stack to not response with status at all + // Therefore DCD must take full responsibility to response and include zlp status packet if needed. + usbd_control_set_request(p_request); // set request since DCD has no access to tud_control_status() API dcd_set_address(rhport, (uint8_t) p_request->wValue); - -// FIXME remove STATUS response from dcd_set_address(), -#if CFG_TUSB_MCU == OPT_MCU_SAMG // skip status for nrf5x mcu - tud_control_status(rhport, p_request); -#else - return true; // skip status -#endif + // skip tud_control_status() break; case TUSB_REQ_GET_CONFIGURATION: diff --git a/src/device/usbd_control.c b/src/device/usbd_control.c index 8b9b8f1c..e6d1caf4 100644 --- a/src/device/usbd_control.c +++ b/src/device/usbd_control.c @@ -119,9 +119,8 @@ bool tud_control_xfer(uint8_t rhport, tusb_control_request_t const * request, vo // USBD API //--------------------------------------------------------------------+ -void usbd_control_reset (uint8_t rhport) +void usbd_control_reset(void) { - (void) rhport; tu_varclr(&_ctrl_xfer); } @@ -131,6 +130,15 @@ void usbd_control_set_complete_callback( bool (*fp) (uint8_t, tusb_control_reque _ctrl_xfer.complete_cb = fp; } +// useful for dcd_set_address where DCD is responsible for status response +void usbd_control_set_request(tusb_control_request_t const *request) +{ + _ctrl_xfer.request = (*request); + _ctrl_xfer.buffer = NULL; + _ctrl_xfer.total_xferred = 0; + _ctrl_xfer.data_len = 0; +} + // callback when a transaction complete on // - DATA stage of control endpoint or // - Status stage diff --git a/src/portable/microchip/samd21/dcd_samd21.c b/src/portable/microchip/samd21/dcd_samd21.c index 231abc7d..3ffaabf0 100644 --- a/src/portable/microchip/samd21/dcd_samd21.c +++ b/src/portable/microchip/samd21/dcd_samd21.c @@ -98,13 +98,13 @@ void dcd_int_disable(uint8_t rhport) void dcd_set_address (uint8_t rhport, uint8_t dev_addr) { - // Response with status first before changing device address - dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0); + (void) dev_addr; - // Wait for EP0 to finish before switching the address. - while (USB->DEVICE.DeviceEndpoint[0].EPSTATUS.bit.BK1RDY == 1) {} + // Response with zlp status + dcd_edpt_xfer(rhport, 0x80, NULL, 0); - USB->DEVICE.DADD.reg = USB_DEVICE_DADD_DADD(dev_addr) | USB_DEVICE_DADD_ADDEN; + // DCD can only set address after status for this request is complete + // do it at dcd_edpt0_status_complete() // Enable SUSPEND interrupt since the bus signal D+/D- are stable now. USB->DEVICE.INTFLAG.reg = USB_DEVICE_INTENCLR_SUSPEND; // clear pending @@ -116,7 +116,6 @@ void dcd_set_config (uint8_t rhport, uint8_t config_num) (void) rhport; (void) config_num; // Nothing to do - } void dcd_remote_wakeup(uint8_t rhport) @@ -130,6 +129,20 @@ void dcd_remote_wakeup(uint8_t rhport) /* DCD Endpoint port *------------------------------------------------------------------*/ +// Invoked when a control transfer's status stage is complete. +// May help DCD to prepare for next control transfer, this API is optional. +void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * request) +{ + (void) rhport; + + if (request->bRequest == TUSB_REQ_SET_ADDRESS) + { + uint8_t const dev_addr = (uint8_t) request->wValue; + USB->DEVICE.DADD.reg = USB_DEVICE_DADD_DADD(dev_addr) | USB_DEVICE_DADD_ADDEN; + } +} + + bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * desc_edpt) { (void) rhport; diff --git a/src/portable/microchip/samd51/dcd_samd51.c b/src/portable/microchip/samd51/dcd_samd51.c index f58e7cbc..1af24770 100644 --- a/src/portable/microchip/samd51/dcd_samd51.c +++ b/src/portable/microchip/samd51/dcd_samd51.c @@ -104,13 +104,13 @@ void dcd_int_disable(uint8_t rhport) void dcd_set_address (uint8_t rhport, uint8_t dev_addr) { - // Response with status first before changing device address - dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0); + (void) dev_addr; - // Wait for EP0 to finish before switching the address. - while (USB->DEVICE.DeviceEndpoint[0].EPSTATUS.bit.BK1RDY == 1) {} + // Response with zlp status + dcd_edpt_xfer(rhport, 0x80, NULL, 0); - USB->DEVICE.DADD.reg = USB_DEVICE_DADD_DADD(dev_addr) | USB_DEVICE_DADD_ADDEN; + // DCD can only set address after status for this request is complete + // do it at dcd_edpt0_status_complete() // Enable SUSPEND interrupt since the bus signal D+/D- are stable now. USB->DEVICE.INTFLAG.reg = USB_DEVICE_INTENCLR_SUSPEND; // clear pending @@ -135,6 +135,19 @@ void dcd_remote_wakeup(uint8_t rhport) /* DCD Endpoint port *------------------------------------------------------------------*/ +// Invoked when a control transfer's status stage is complete. +// May help DCD to prepare for next control transfer, this API is optional. +void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * request) +{ + (void) rhport; + + if (request->bRequest == TUSB_REQ_SET_ADDRESS) + { + uint8_t const dev_addr = (uint8_t) request->wValue; + USB->DEVICE.DADD.reg = USB_DEVICE_DADD_DADD(dev_addr) | USB_DEVICE_DADD_ADDEN; + } +} + bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * desc_edpt) { (void) rhport; diff --git a/src/portable/microchip/samg/dcd_samg.c b/src/portable/microchip/samg/dcd_samg.c index c38621e0..122dc04b 100644 --- a/src/portable/microchip/samg/dcd_samg.c +++ b/src/portable/microchip/samg/dcd_samg.c @@ -120,7 +120,10 @@ void dcd_set_address (uint8_t rhport, uint8_t dev_addr) (void) rhport; (void) dev_addr; - // SAMG can only set address after status for this request is complete + // Response with zlp status + dcd_edpt_xfer(rhport, 0x80, NULL, 0); + + // DCD can only set address after status for this request is complete. // do it at dcd_edpt0_status_complete() } diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 7ba9481b..f52e113b 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -172,7 +172,6 @@ static inline xfer_ctl_t* xfer_ctl_ptr(uint32_t epnum, uint32_t dir) static TU_ATTR_ALIGNED(4) uint32_t _setup_packet[6]; -static uint8_t newDADDR; // Used to set the new device address during the CTR IRQ handler static uint8_t remoteWakeCountdown; // When wake is requested // EP Buffers assigned from end of memory location, to minimize their chance of crashing @@ -297,14 +296,11 @@ void dcd_set_address(uint8_t rhport, uint8_t dev_addr) { (void)rhport; - // FIXME use dcd_edpt0_status_complete() - // We cannot immediatly change it; it must be queued to change after the STATUS packet is sent. - // (CTR handler will actually change the address once it sees that the transmission is complete) - newDADDR = dev_addr; - // Respond with status dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0); + // DCD can only set address after status for this request is complete. + // do it at dcd_edpt0_status_complete() } // Receive Set Config request @@ -361,7 +357,7 @@ static void dcd_handle_bus_reset(void) 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); - newDADDR = 0u; + USB->DADDR = USB_DADDR_EF; // Set enable flag, and leaving the device address as zero. } @@ -397,13 +393,7 @@ static uint16_t dcd_ep_ctr_handler(void) if((xfer->total_len == xfer->queued_len)) { dcd_event_xfer_complete(0u, (uint8_t)(0x80 + EPindex), xfer->total_len, XFER_RESULT_SUCCESS, true); - if((newDADDR != 0) && ( xfer->total_len == 0U)) - { - // Delayed setting of the DADDR after the 0-len DATA packet acking the request is sent. - reg16_clear_bits(&USB->DADDR, USB_DADDR_ADD); - USB->DADDR = (uint16_t)(USB->DADDR | newDADDR); // leave the enable bit set - newDADDR = 0; - } + if(xfer->total_len == 0) // Probably a status message? { pcd_clear_rx_dtog(USB,EPindex); @@ -601,6 +591,22 @@ static void dcd_fs_irqHandler(void) { // Endpoint API //--------------------------------------------------------------------+ +// Invoked when a control transfer's status stage is complete. +// May help DCD to prepare for next control transfer, this API is optional. +void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * request) +{ + (void) rhport; + + if (request->bRequest == TUSB_REQ_SET_ADDRESS) + { + uint8_t const dev_addr = (uint8_t) request->wValue; + + // Setting new address after the whole request is complete + reg16_clear_bits(&USB->DADDR, USB_DADDR_ADD); + USB->DADDR = (uint16_t)(USB->DADDR | dev_addr); // leave the enable bit set + } +} + // The STM32F0 doesn't seem to like |= or &= to manipulate the EP#R registers, // so I'm using the #define from HAL here, instead. diff --git a/src/portable/st/synopsys/dcd_synopsys.c b/src/portable/st/synopsys/dcd_synopsys.c index 7929b691..e02580e4 100644 --- a/src/portable/st/synopsys/dcd_synopsys.c +++ b/src/portable/st/synopsys/dcd_synopsys.c @@ -236,7 +236,6 @@ void dcd_set_address (uint8_t rhport, uint8_t dev_addr) (void) rhport; USB_OTG_DeviceTypeDef * dev = DEVICE_BASE; - dev->DCFG |= (dev_addr << USB_OTG_DCFG_DAD_Pos) & USB_OTG_DCFG_DAD_Msk; // Response with status after changing device address