From a2f4786865e85f9cfe7f58c86fbb9355bbd2d701 Mon Sep 17 00:00:00 2001 From: Zixun LI Date: Mon, 27 Jul 2026 14:39:02 +0200 Subject: portable/chipidea: configure LPC USB0 AHB bursts --- src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h | 14 ++++++++++++++ src/portable/chipidea/ci_hs/dcd_ci_hs.c | 4 ++++ src/portable/chipidea/ci_hs/hcd_ci_hs.c | 4 ++++ 3 files changed, 22 insertions(+) (limited to 'src/portable') diff --git a/src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h b/src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h index f2061bd7a..dec3a34b1 100644 --- a/src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h +++ b/src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h @@ -34,4 +34,18 @@ static const ci_hs_controller_t _ci_controller[] = #define CI_HCD_INT_ENABLE(_p) NVIC_EnableIRQ ((IRQn_Type)_ci_controller[_p].irqnum) #define CI_HCD_INT_DISABLE(_p) NVIC_DisableIRQ((IRQn_Type)_ci_controller[_p].irqnum) +enum { + CI_HS_LPC18_43_SBUSCFG_OFFSET = 0x90u, + CI_HS_LPC18_43_AHBBRST_INCR16_UNSPEC = 0x07u, +}; + +TU_ATTR_ALWAYS_INLINE static inline void ci_hs_lpc18_43_set_ahb_burst(uint8_t rhport) { + // USB0 SBUSCFG is at offset 0x90. NXP recommends AHBBRST=0x7: + // INCR16 with non-multiple transfers decomposed into smaller unspecified bursts. + if (rhport == 0) { + volatile uint32_t *sbuscfg = (volatile uint32_t *)(_ci_controller[rhport].reg_base + CI_HS_LPC18_43_SBUSCFG_OFFSET); + *sbuscfg = CI_HS_LPC18_43_AHBBRST_INCR16_UNSPEC; + } +} + #endif diff --git a/src/portable/chipidea/ci_hs/dcd_ci_hs.c b/src/portable/chipidea/ci_hs/dcd_ci_hs.c index fa98d6882..32c701bfa 100644 --- a/src/portable/chipidea/ci_hs/dcd_ci_hs.c +++ b/src/portable/chipidea/ci_hs/dcd_ci_hs.c @@ -237,6 +237,10 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { usbmode |= USBMODE_CM_DEVICE; dcd_reg->USBMODE = usbmode; + #if TU_CHECK_MCU(OPT_MCU_LPC18XX, OPT_MCU_LPC43XX) + ci_hs_lpc18_43_set_ahb_burst(rhport); + #endif + #ifdef CFG_TUD_CI_HS_VBUS_CHARGE dcd_reg->OTGSC = OTGSC_VBUS_CHARGE | OTGSC_OTG_TERMINATION; #else diff --git a/src/portable/chipidea/ci_hs/hcd_ci_hs.c b/src/portable/chipidea/ci_hs/hcd_ci_hs.c index 3cb69acfa..c94ce810f 100644 --- a/src/portable/chipidea/ci_hs/hcd_ci_hs.c +++ b/src/portable/chipidea/ci_hs/hcd_ci_hs.c @@ -82,6 +82,10 @@ bool hcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { hcd_reg->USBMODE = USBMODE_CM_HOST; #endif + #if TU_CHECK_MCU(OPT_MCU_LPC18XX, OPT_MCU_LPC43XX) + ci_hs_lpc18_43_set_ahb_burst(rhport); + #endif + #if !TUH_OPT_HIGH_SPEED hcd_reg->PORTSC1 |= PORTSC1_FORCE_FULL_SPEED; #endif -- cgit v1.3.1 From f0a8a1483bd4e89ab3adf40d8c61777a5ddadc7f Mon Sep 17 00:00:00 2001 From: Zixun LI Date: Mon, 27 Jul 2026 14:58:13 +0200 Subject: portable/chipidea: configure i.MX RT AHB bursts --- src/portable/chipidea/ci_hs/ci_hs_imxrt.h | 10 ++++++++++ src/portable/chipidea/ci_hs/dcd_ci_hs.c | 4 +++- src/portable/chipidea/ci_hs/hcd_ci_hs.c | 4 +++- 3 files changed, 16 insertions(+), 2 deletions(-) (limited to 'src/portable') diff --git a/src/portable/chipidea/ci_hs/ci_hs_imxrt.h b/src/portable/chipidea/ci_hs/ci_hs_imxrt.h index f0f918fe2..601e4d1c9 100644 --- a/src/portable/chipidea/ci_hs/ci_hs_imxrt.h +++ b/src/portable/chipidea/ci_hs/ci_hs_imxrt.h @@ -36,6 +36,16 @@ static const ci_hs_controller_t _ci_controller[] = #define CI_HS_REG(_port) ((ci_hs_regs_t*) _ci_controller[_port].reg_base) +enum { + // INCR16/8/4 followed by an unspecified-length burst for the remainder. + CI_HS_IMXRT_AHBBRST_INCR16_UNSPEC = 0x07u, +}; + +TU_ATTR_ALWAYS_INLINE static inline void ci_hs_imxrt_set_ahb_burst(uint8_t rhport) { + USB_Type *usb = (USB_Type *)_ci_controller[rhport].reg_base; + usb->SBUSCFG = USB_SBUSCFG_AHBBRST(CI_HS_IMXRT_AHBBRST_INCR16_UNSPEC); +} + //------------- DCD -------------// #define CI_DCD_INT_ENABLE(_p) NVIC_EnableIRQ ((IRQn_Type)_ci_controller[_p].irqnum) #define CI_DCD_INT_DISABLE(_p) NVIC_DisableIRQ((IRQn_Type)_ci_controller[_p].irqnum) diff --git a/src/portable/chipidea/ci_hs/dcd_ci_hs.c b/src/portable/chipidea/ci_hs/dcd_ci_hs.c index 32c701bfa..62d75b4d3 100644 --- a/src/portable/chipidea/ci_hs/dcd_ci_hs.c +++ b/src/portable/chipidea/ci_hs/dcd_ci_hs.c @@ -237,7 +237,9 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { usbmode |= USBMODE_CM_DEVICE; dcd_reg->USBMODE = usbmode; - #if TU_CHECK_MCU(OPT_MCU_LPC18XX, OPT_MCU_LPC43XX) + #if CFG_TUSB_MCU == OPT_MCU_MIMXRT1XXX + ci_hs_imxrt_set_ahb_burst(rhport); + #elif TU_CHECK_MCU(OPT_MCU_LPC18XX, OPT_MCU_LPC43XX) ci_hs_lpc18_43_set_ahb_burst(rhport); #endif diff --git a/src/portable/chipidea/ci_hs/hcd_ci_hs.c b/src/portable/chipidea/ci_hs/hcd_ci_hs.c index c94ce810f..0fc8e4d70 100644 --- a/src/portable/chipidea/ci_hs/hcd_ci_hs.c +++ b/src/portable/chipidea/ci_hs/hcd_ci_hs.c @@ -82,7 +82,9 @@ bool hcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { hcd_reg->USBMODE = USBMODE_CM_HOST; #endif - #if TU_CHECK_MCU(OPT_MCU_LPC18XX, OPT_MCU_LPC43XX) + #if CFG_TUSB_MCU == OPT_MCU_MIMXRT1XXX + ci_hs_imxrt_set_ahb_burst(rhport); + #elif TU_CHECK_MCU(OPT_MCU_LPC18XX, OPT_MCU_LPC43XX) ci_hs_lpc18_43_set_ahb_burst(rhport); #endif -- cgit v1.3.1 From a20cf74e6a62f5b833baacfe1198648b237b3e7f Mon Sep 17 00:00:00 2001 From: Zixun LI Date: Tue, 28 Jul 2026 21:15:38 +0200 Subject: portable/dwc2: rewind DMA on ISO IN retry --- src/portable/synopsys/dwc2/dcd_dwc2.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/dcd_dwc2.c b/src/portable/synopsys/dwc2/dcd_dwc2.c index 86aa54510..b2f1a93a4 100644 --- a/src/portable/synopsys/dwc2/dcd_dwc2.c +++ b/src/portable/synopsys/dwc2/dcd_dwc2.c @@ -1143,7 +1143,12 @@ static void handle_incomplete_iso_in(uint8_t rhport) { xfer_ctl_t *xfer = XFER_CTL_BASE(epnum, TUSB_DIR_IN); if (xfer->iso_retry > 0) { xfer->iso_retry--; - // Restart ISO transfe: re-write TSIZ and CTL + // Restart ISO transfer: re-write DMA address, TSIZ, and CTL + #if CFG_TUD_DWC2_DMA_ENABLE + if (dma_device_enabled(dwc2)) { + epin->diepdma = (uintptr_t) xfer->buffer; + } + #endif dwc2_ep_tsize_t deptsiz = {.value = 0}; deptsiz.xfer_size = xfer->total_len; deptsiz.packet_count = tu_div_ceil(xfer->total_len, xfer->max_size); -- cgit v1.3.1 From f617a208c16ec720b4b5da93b32cbcb8914fcaf7 Mon Sep 17 00:00:00 2001 From: HiFiPhile Date: Tue, 11 Aug 2026 14:41:00 +0200 Subject: fix(dwc2): correct host FIFO allocation --- src/portable/synopsys/dwc2/hcd_dwc2.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 089b839ae..e8a3d6623 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -336,13 +336,13 @@ TU_ATTR_ALWAYS_INLINE static inline uint8_t cal_next_pid(uint8_t pid, uint8_t pa static void dfifo_host_init(uint8_t rhport, bool is_hs_phy) { const dwc2_controller_t* dwc2_controller = &_dwc2_controller[rhport]; dwc2_regs_t* dwc2 = DWC2_REG(rhport); - const dwc2_ghwcfg2_t ghwcfg2 = {.value = dwc2->ghwcfg2}; + const uint8_t channel_count = dwc2_channel_count(dwc2); // Scatter/Gather DMA mode is not yet supported. Buffer DMA only need 1 words per channel const bool is_dma = dma_host_enabled(dwc2); uint16_t dfifo_top = dwc2_controller->otg_dfifo_depth; if (is_dma) { - dfifo_top -= ghwcfg2.num_host_ch; + dfifo_top -= channel_count; } // fixed allocation for now, improve later: @@ -358,13 +358,12 @@ static void dfifo_host_init(uint8_t rhport, bool is_hs_phy) { } uint16_t nptxfsiz = 2 * nptx_largest; - uint16_t rxfsiz = 2 * (ptx_largest + 2) + ghwcfg2.num_host_ch; + uint16_t rxfsiz = 2 * (ptx_largest + 2) + channel_count; TU_ASSERT(dfifo_top >= (nptxfsiz + rxfsiz),); uint16_t ptxfsiz = dfifo_top - (nptxfsiz + rxfsiz); dwc2->gdfifocfg = (dfifo_top << GDFIFOCFG_EPINFOBASE_SHIFT) | dfifo_top; - dfifo_top -= rxfsiz; dwc2->grxfsiz = rxfsiz; dfifo_top -= nptxfsiz; -- cgit v1.3.1 From 73e787ae418df76b79ac0fd31bb0674a72e68c88 Mon Sep 17 00:00:00 2001 From: Ryzee119 Date: Wed, 12 Aug 2026 20:40:15 +0930 Subject: ohci: fix double allocation of dummy TDs in gtd_find_free --- src/portable/ohci/ohci.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src/portable') diff --git a/src/portable/ohci/ohci.c b/src/portable/ohci/ohci.c index e2c5956b3..a294c5f5f 100644 --- a/src/portable/ohci/ohci.c +++ b/src/portable/ohci/ohci.c @@ -400,6 +400,7 @@ static void ed_list_remove_by_addr(ohci_ed_t * p_head, uint8_t dev_addr) { static ohci_gtd_t* gtd_find_free(void) { for (uint8_t i = 0; i < GTD_MAX; i++) { if (!ohci_data.gtd_pool[i].used) { + ohci_data.gtd_pool[i].used = 1; return &ohci_data.gtd_pool[i]; } } -- cgit v1.3.1 From 8ccd0d549798c66d484e5a4b4c57edf49e8bb097 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 13 Aug 2026 14:35:01 +0700 Subject: portable/chipidea: name SBUSCFG in ci_hs_regs_t, unify AHB burst hook Replace the duplicated per-MCU dispatch in dcd_init/hcd_init and the two helper flavors (USB_Type access on iMX RT, raw offset 0x90 on LPC18/43) with one SBUSCFG register field plus a per-header CI_HS_SET_AHB_BURST() hook, compiled only where defined. The LPC USB0-only policy is now visible at the macro definition. --- src/portable/chipidea/ci_hs/ci_hs_imxrt.h | 11 ++--------- src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h | 17 ++++------------- src/portable/chipidea/ci_hs/ci_hs_type.h | 9 ++++++++- src/portable/chipidea/ci_hs/dcd_ci_hs.c | 6 ++---- src/portable/chipidea/ci_hs/hcd_ci_hs.c | 6 ++---- 5 files changed, 18 insertions(+), 31 deletions(-) (limited to 'src/portable') diff --git a/src/portable/chipidea/ci_hs/ci_hs_imxrt.h b/src/portable/chipidea/ci_hs/ci_hs_imxrt.h index 601e4d1c9..8f0d6083e 100644 --- a/src/portable/chipidea/ci_hs/ci_hs_imxrt.h +++ b/src/portable/chipidea/ci_hs/ci_hs_imxrt.h @@ -36,15 +36,8 @@ static const ci_hs_controller_t _ci_controller[] = #define CI_HS_REG(_port) ((ci_hs_regs_t*) _ci_controller[_port].reg_base) -enum { - // INCR16/8/4 followed by an unspecified-length burst for the remainder. - CI_HS_IMXRT_AHBBRST_INCR16_UNSPEC = 0x07u, -}; - -TU_ATTR_ALWAYS_INLINE static inline void ci_hs_imxrt_set_ahb_burst(uint8_t rhport) { - USB_Type *usb = (USB_Type *)_ci_controller[rhport].reg_base; - usb->SBUSCFG = USB_SBUSCFG_AHBBRST(CI_HS_IMXRT_AHBBRST_INCR16_UNSPEC); -} +// NXP recommends AHBBRST = INCR16 (remainder as unspecified-length bursts) +#define CI_HS_SET_AHB_BURST(_p) (CI_HS_REG(_p)->SBUSCFG = SBUSCFG_AHBBRST_INCR16_UNSPEC) //------------- DCD -------------// #define CI_DCD_INT_ENABLE(_p) NVIC_EnableIRQ ((IRQn_Type)_ci_controller[_p].irqnum) diff --git a/src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h b/src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h index dec3a34b1..c7dc7e69f 100644 --- a/src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h +++ b/src/portable/chipidea/ci_hs/ci_hs_lpc18_43.h @@ -34,18 +34,9 @@ static const ci_hs_controller_t _ci_controller[] = #define CI_HCD_INT_ENABLE(_p) NVIC_EnableIRQ ((IRQn_Type)_ci_controller[_p].irqnum) #define CI_HCD_INT_DISABLE(_p) NVIC_DisableIRQ((IRQn_Type)_ci_controller[_p].irqnum) -enum { - CI_HS_LPC18_43_SBUSCFG_OFFSET = 0x90u, - CI_HS_LPC18_43_AHBBRST_INCR16_UNSPEC = 0x07u, -}; - -TU_ATTR_ALWAYS_INLINE static inline void ci_hs_lpc18_43_set_ahb_burst(uint8_t rhport) { - // USB0 SBUSCFG is at offset 0x90. NXP recommends AHBBRST=0x7: - // INCR16 with non-multiple transfers decomposed into smaller unspecified bursts. - if (rhport == 0) { - volatile uint32_t *sbuscfg = (volatile uint32_t *)(_ci_controller[rhport].reg_base + CI_HS_LPC18_43_SBUSCFG_OFFSET); - *sbuscfg = CI_HS_LPC18_43_AHBBRST_INCR16_UNSPEC; - } -} +// USB0 (high-speed) only: NXP recommends AHBBRST = INCR16 (remainder as +// unspecified-length bursts) +#define CI_HS_SET_AHB_BURST(_p) \ + do { if ((_p) == 0) { CI_HS_REG(_p)->SBUSCFG = SBUSCFG_AHBBRST_INCR16_UNSPEC; } } while (0) #endif diff --git a/src/portable/chipidea/ci_hs/ci_hs_type.h b/src/portable/chipidea/ci_hs/ci_hs_type.h index 70817a6e3..b209c7545 100644 --- a/src/portable/chipidea/ci_hs/ci_hs_type.h +++ b/src/portable/chipidea/ci_hs/ci_hs_type.h @@ -71,11 +71,18 @@ enum { USBMODE_VBUS_POWER_SELECT = TU_BIT(5), // Need to be enabled for LPC18XX/43XX in host mode }; +// SBUSCFG +enum { + SBUSCFG_AHBBRST_INCR16_UNSPEC = 7, // INCR16 burst, remainder as unspecified-length bursts +}; + // Device Registers typedef struct { //------------- ID + HW Parameter Registers-------------// - volatile uint32_t TU_RESERVED[64]; ///< For iMX RT10xx, but not used by LPC18XX/LPC43XX + volatile uint32_t TU_RESERVED[36]; ///< ID/HW parameter registers, not used by this driver + volatile uint32_t SBUSCFG; ///< System Bus Interface Configuration (not present on every MCU) + volatile uint32_t TU_RESERVED[27]; //------------- Capability Registers-------------// volatile uint8_t CAPLENGTH; ///< Capability Registers Length diff --git a/src/portable/chipidea/ci_hs/dcd_ci_hs.c b/src/portable/chipidea/ci_hs/dcd_ci_hs.c index 62d75b4d3..8c08c6bd5 100644 --- a/src/portable/chipidea/ci_hs/dcd_ci_hs.c +++ b/src/portable/chipidea/ci_hs/dcd_ci_hs.c @@ -237,10 +237,8 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { usbmode |= USBMODE_CM_DEVICE; dcd_reg->USBMODE = usbmode; - #if CFG_TUSB_MCU == OPT_MCU_MIMXRT1XXX - ci_hs_imxrt_set_ahb_burst(rhport); - #elif TU_CHECK_MCU(OPT_MCU_LPC18XX, OPT_MCU_LPC43XX) - ci_hs_lpc18_43_set_ahb_burst(rhport); + #ifdef CI_HS_SET_AHB_BURST + CI_HS_SET_AHB_BURST(rhport); #endif #ifdef CFG_TUD_CI_HS_VBUS_CHARGE diff --git a/src/portable/chipidea/ci_hs/hcd_ci_hs.c b/src/portable/chipidea/ci_hs/hcd_ci_hs.c index 0fc8e4d70..0f24f5bb6 100644 --- a/src/portable/chipidea/ci_hs/hcd_ci_hs.c +++ b/src/portable/chipidea/ci_hs/hcd_ci_hs.c @@ -82,10 +82,8 @@ bool hcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { hcd_reg->USBMODE = USBMODE_CM_HOST; #endif - #if CFG_TUSB_MCU == OPT_MCU_MIMXRT1XXX - ci_hs_imxrt_set_ahb_burst(rhport); - #elif TU_CHECK_MCU(OPT_MCU_LPC18XX, OPT_MCU_LPC43XX) - ci_hs_lpc18_43_set_ahb_burst(rhport); + #ifdef CI_HS_SET_AHB_BURST + CI_HS_SET_AHB_BURST(rhport); #endif #if !TUH_OPT_HIGH_SPEED -- cgit v1.3.1 From 18bb2d650404432d0c6c61c26e98c7074f46ec6b Mon Sep 17 00:00:00 2001 From: Ryzee119 Date: Wed, 12 Aug 2026 21:49:00 +0930 Subject: ohci: reclaim orphaned TDs on device disconnect --- src/portable/ohci/ohci.c | 77 ++++++++++++++++++++++++++++++++++++++++++++---- src/portable/ohci/ohci.h | 3 +- 2 files changed, 73 insertions(+), 7 deletions(-) (limited to 'src/portable') diff --git a/src/portable/ohci/ohci.c b/src/portable/ohci/ohci.c index a294c5f5f..7ede1ac72 100644 --- a/src/portable/ohci/ohci.c +++ b/src/portable/ohci/ohci.c @@ -378,7 +378,7 @@ static void ed_list_remove_by_addr(ohci_ed_t * p_head, uint8_t dev_addr) { ohci_ed_t* p_prev = p_head; while (p_prev->next) { - ohci_ed_t* ed = (ohci_ed_t*)_virt_addr((void*)p_prev->next); + ohci_ed_t* ed = hcd_dcache_uncached((ohci_ed_t*)_virt_addr((void*)p_prev->next)); if (ed->w0.dev_addr == dev_addr) { // Prevent Host Controller from processing this ED while we remove it @@ -387,12 +387,24 @@ static void ed_list_remove_by_addr(ohci_ed_t * p_head, uint8_t dev_addr) { // unlink ed, will also move up p_prev p_prev->next = ed->next; - // point the removed ED's next pointer to list head to make sure HC can always safely move away from this ED - ed->next = (uint32_t)_phys_addr(p_head); - ed->w0.used = 0; - ed->w0.skip = 0; + // Control endpoints (EP number 0) are statically allocated with the device which are only reused + // after connection of another device long after HC has finished with them now, these can be freed immediately. + if (ed->w0.ep_number != 0) { + ed->w0.is_reclaiming = 1; + + // 5.2.7.1.2 Removing. Disable list processing for bulk + if (p_head == p_ed_head[TUSB_XFER_BULK]) { + OHCI_REG->control &= ~OHCI_CONTROL_LIST_BULK_ENABLE_MASK; + } + + // Temporarily enable SOF IRQ. ED and TD Memory will be reclaimed in the SOF IRQ. + OHCI_REG->interrupt_enable = OHCI_INT_SOF_MASK; + } else { + ed->w0.used = 0; + ed->w0.skip = 0; + } } else { - p_prev = (ohci_ed_t*)_virt_addr((void*)p_prev->next); + p_prev = ed; } } } @@ -653,6 +665,59 @@ void hcd_int_handler(uint8_t hostid, bool in_isr) { // Disable MIE as per OHCI spec 5.3 OHCI_REG->interrupt_disable = OHCI_INT_MASTER_ENABLE_MASK; + // Start of frame (SOF) + if (int_status & OHCI_INT_SOF_MASK) { + OHCI_REG->interrupt_disable = OHCI_INT_SOF_MASK; + + bool re_enable_lists = false; + + for (size_t i = 0; i < ED_MAX; i++) { + ohci_ed_t* ed = hcd_dcache_uncached(&ohci_data.ed_pool[i]); + if (ed->w0.used && ed->w0.is_reclaiming) { + TU_ASSERT(ed->w0.skip == 1, ); + TU_ASSERT(ed->w0.ep_number != 0, ); + + // Reclaim orphaned TDs + uint32_t td_addr = ed->td_head.address & ~0x0F; + while (td_addr) { + if (!ed->w0.is_iso) { + ohci_gtd_t *gtd = (ohci_gtd_t*)_virt_addr((void*)(uintptr_t)td_addr); + gtd->used = 0; + } else { + // TODO: Free ITD once implemented + } + + if (td_addr == ed->td_tail) { + break; + } + td_addr = ((ohci_td_item_t*)_virt_addr((void*)(uintptr_t)td_addr))->next; + } + + ed->w0.is_reclaiming = 0; + ed->w0.used = 0; + ed->w0.skip = 0; + + re_enable_lists = true; + } + } + + if (re_enable_lists) { + // 5.2.7.1.2 Removing + // Reset current ED pointers and re-enable lists + // Once the next frame has started, the HcControlCurrentED or HcBulkCurrentED register should be adjusted so + // that it does not point to the Endpoint Descriptor being removed (for simplicity you may just write + // a zero to the register); + if (!(OHCI_REG->control & OHCI_CONTROL_LIST_CONTROL_ENABLE_MASK)) { + OHCI_REG->control_current_ed = 0; + OHCI_REG->control |= OHCI_CONTROL_LIST_CONTROL_ENABLE_MASK; + } + if (!(OHCI_REG->control & OHCI_CONTROL_LIST_BULK_ENABLE_MASK)) { + OHCI_REG->bulk_current_ed = 0; + OHCI_REG->control |= OHCI_CONTROL_LIST_BULK_ENABLE_MASK; + } + } + } + // Frame number overflow if (int_status & OHCI_INT_FRAME_OVERFLOW_MASK) { ohci_data.frame_number_hi++; diff --git a/src/portable/ohci/ohci.h b/src/portable/ohci/ohci.h index 84ae04b0f..c66954502 100644 --- a/src/portable/ohci/ohci.h +++ b/src/portable/ohci/ohci.h @@ -107,7 +107,8 @@ typedef union { // HCD: make use of 5 reserved bits uint32_t used : 1; uint32_t is_interrupt_xfer : 1; - uint32_t : 3; + uint32_t is_reclaiming : 1; + uint32_t : 2; }; uint32_t value; } ohci_ed_word0_t; -- cgit v1.3.1 From 2fda873fa5f6ef0c893f4f138b5c54e49c24e0a9 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 17 Aug 2026 01:01:54 +0700 Subject: dcd(ci_hs): rework bus reset handling and bound the register waits A bus reset was detected only from the port change that ends it, which is late: the manual asks the DCD to clear the endpoint semaphores, cancel every prime and free the dTDs while the reset is still being driven. Enable the reset interrupt and do all of that there, in the manual's order (IMXRT1060RM 42.5.6.2.1, p.2394), including the two steps that were missing - confirming the port is still being reset, and freeing the dTDs. A failed check means the cleanup arrived late and the controller may be in an undefined state, so the manual's remedy is carried out rather than noted: a controller reset, followed by the full re-initialisation it then requires, since the reset detaches the device. The port change that ends the reset is left with what the manual gives it, the negotiated speed, which the new BUS_RESET_END event carries. A port change is classified by the interrupt that preceded it: a suspend raises no port change of its own, the resume that ends it does. Every unbounded register spin is now bounded. They waited on bits the hardware clears within a frame, but each could hang an interrupt handler outright on a controller that had stopped responding. The endpoint flush follows all three steps of IMXRT1060RM 42.5.6.6.5 (p.2413), repeating a flush the controller refuses while a packet is in progress - previously reported as success. EP0 setup handling is hardened alongside: the payload is copied out of the queue head through the volatile qualifier before ENDPTSETUPSTAT is cleared, since that clear releases the setup lockout and a back-to-back setup can overwrite the buffer immediately after, and C orders volatile accesses only against each other, so a plain memcpy may legally be sunk past the store. There is deliberately no unplug detection. IMXRT1060RM 42.7.31 (p.2470) states a zero Current Connect Status means the device "did not attach successfully or was forcibly disconnected by the software writing a zero to the Run bit ... It does not state the device being disconnected or suspended", so a cable pull raises no port change at all; VBUS via OTGSC is the manual's disconnect indicator and is board dependent. Verified on mimxrt1064_evk: 30 forced bus resets each re-enumerating at high speed with no descriptor errors, plus repeated full usbtest batteries at 30/30 across the series. --- src/portable/chipidea/ci_hs/ci_hs_type.h | 8 + src/portable/chipidea/ci_hs/dcd_ci_hs.c | 247 +++++++++++++++++++++---------- 2 files changed, 180 insertions(+), 75 deletions(-) (limited to 'src/portable') diff --git a/src/portable/chipidea/ci_hs/ci_hs_type.h b/src/portable/chipidea/ci_hs/ci_hs_type.h index b209c7545..5baa14821 100644 --- a/src/portable/chipidea/ci_hs/ci_hs_type.h +++ b/src/portable/chipidea/ci_hs/ci_hs_type.h @@ -36,10 +36,18 @@ enum { PORTSC1_CURRENT_CONNECT_STATUS = TU_BIT(0), PORTSC1_FORCE_PORT_RESUME = TU_BIT(6), PORTSC1_SUSPEND = TU_BIT(7), + PORTSC1_PORT_RESET = TU_BIT(8), // read-only in device mode: a reset is being driven PORTSC1_FORCE_FULL_SPEED = TU_BIT(24), PORTSC1_PORT_SPEED = TU_BIT(26) | TU_BIT(27) }; +// PORTSC1 PSPD field values, once shifted down by PORTSC1_PORT_SPEED_POS. 3 is undefined. +enum { + PORTSC1_PORT_SPEED_FULL = 0, + PORTSC1_PORT_SPEED_LOW = 1, + PORTSC1_PORT_SPEED_HIGH = 2, +}; + // OTGSC enum { OTGSC_VBUS_DISCHARGE = TU_BIT(0), diff --git a/src/portable/chipidea/ci_hs/dcd_ci_hs.c b/src/portable/chipidea/ci_hs/dcd_ci_hs.c index 8c08c6bd5..6ab28e0be 100644 --- a/src/portable/chipidea/ci_hs/dcd_ci_hs.c +++ b/src/portable/chipidea/ci_hs/dcd_ci_hs.c @@ -154,6 +154,14 @@ TU_VERIFY_STATIC(sizeof(dcd_qhd_t) == 64, "size is not correct"); #define QTD_NEXT_INVALID 0x01 +// Bounded spin for register waits. The longest legitimate wait is a flush held off by a packet +// already in progress: ~50 us for a full-speed 64-byte packet, a low thousands of dependent +// register reads, so healthy hardware never approaches this bound. Exceeding it means the +// controller has stopped responding, and the spin then only serves to keep an ISR (or an +// IRQ-masked caller) from hanging outright - the 3 ms reset-cleanup window of IMXRT1060RM 42.5.6.2.1 (p.2394) +// is already unreachable in that state, and the manual's remedy there is a controller reset. +#define CI_HS_BUSY_SPIN 10000u + typedef struct { // Must be at 2K alignment // Each endpoint with direction (IN/OUT) occupies a queue head @@ -164,6 +172,17 @@ typedef struct { CFG_TUD_MEM_SECTION TU_ATTR_ALIGNED(2048) static dcd_data_t _dcd_data; +// What the next Port Change Detect will be. Each one is preceded by the interrupt that causes it: +// a reset interrupt for the end of a bus reset - where the speed first becomes final - or a +// suspend interrupt for the resume that ends the suspend. A suspend itself raises no port change, +// which is why there is no such value here. Indexed by rhport, which is 0 or 1 on every ci_hs +// variant (NOT the controller count: mcx/rw61x map rhport 1 to controller 0). +enum { + PORT_CHANGE_REASON_RESET = 0, + PORT_CHANGE_REASON_RESUME = 1, +}; +static volatile uint8_t _port_change_reason[2]; + //--------------------------------------------------------------------+ // Prototypes and Helper Functions //--------------------------------------------------------------------+ @@ -172,12 +191,37 @@ TU_ATTR_ALWAYS_INLINE static inline uint8_t ci_ep_count(const ci_hs_regs_t *dcd_ return dcd_reg->DCCPARAMS & DCCPARAMS_DEN_MASK; } +static bool controller_reset(uint8_t rhport); + //--------------------------------------------------------------------+ // Controller API //--------------------------------------------------------------------+ -/// follows LPC43xx User Manual 23.10.3 -static void bus_reset(uint8_t rhport) { +// Flush endpoint buffers, following IMXRT1060RM 42.5.6.6.5 Flushing/De-priming an Endpoint +// (p.2413): write ENDPTFLUSH, wait for the controller +// to acknowledge, then confirm ENDPTSTAT went to zero. The controller refuses the flush when a +// packet is in progress, and the manual requires the procedure be repeated until it takes. +// Callers proceed regardless of the result; the bound only prevents an ISR-context hang on dead +// hardware. +static bool flush_endpoints(ci_hs_regs_t *dcd_reg, uint32_t mask) { + uint32_t guard = CI_HS_BUSY_SPIN; + do { + dcd_reg->ENDPTFLUSH = mask; + while (dcd_reg->ENDPTFLUSH & mask) { + if (!guard--) { + return false; + } + } + } while ((dcd_reg->ENDPTSTAT & mask) && guard--); + + return !(dcd_reg->ENDPTSTAT & mask); +} + +/// Everything the manual asks of the DCD when a reset is detected, in its order: clear the setup +/// and completion semaphores, cancel every prime, check the reset is still being driven, and free +/// the dTDs. All of it belongs inside the reset window (IMXRT1060RM 42.5.6.2.1, p.2394); nothing +/// is left for the port change that ends the reset, which only reports the negotiated speed. +static void bus_reset_begin(uint8_t rhport) { ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); // The reset value for all endpoint types is the control endpoint. If one endpoint @@ -193,17 +237,24 @@ static void bus_reset(uint8_t rhport) { //------------- Clear All Registers -------------// dcd_reg->ENDPTNAK = dcd_reg->ENDPTNAK; dcd_reg->ENDPTNAKEN = 0; - dcd_reg->USBSTS = dcd_reg->USBSTS; dcd_reg->ENDPTSETUPSTAT = dcd_reg->ENDPTSETUPSTAT; dcd_reg->ENDPTCOMPLETE = dcd_reg->ENDPTCOMPLETE; - while (dcd_reg->ENDPTPRIME) {} - dcd_reg->ENDPTFLUSH = 0xFFFFFFFF; - while (dcd_reg->ENDPTFLUSH) {} - - // read reset bit in portsc + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTPRIME && guard--) {} + dcd_reg->ENDPTFLUSH = 0xFFFFFFFFUL; + + // All of the above must land while the reset is still being driven - it lasts at least 3 ms. + // Arriving late leaves the controller in an undefined state, and the manual's remedy is to + // hardware-reset it. That clears Run/Stop, so the device detaches and the host will drive a + // fresh reset and enumeration - which is why nothing below this point is worth doing here. + if (!(dcd_reg->PORTSC1 & PORTSC1_PORT_RESET)) { + TU_LOG1("ci_hs: reset cleanup ran past the end of the reset, resetting controller\r\n"); + controller_reset(rhport); + return; // the controller detached; the host's next reset redoes everything below + } - //------------- Queue Head & Queue TD -------------// + //------------- Free all allocated dTDs: the controller will not execute them again -------------// tu_memclr(&_dcd_data, sizeof(dcd_data_t)); //------------- Set up Control Endpoints (0 OUT, 1 IN) -------------// @@ -216,21 +267,19 @@ static void bus_reset(uint8_t rhport) { dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); } -bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { - (void)rh_init; - tu_memclr(&_dcd_data, sizeof(dcd_data_t)); - +/// Reset the controller and bring it back up in device mode. Also the manual's remedy when the +/// reset cleanup misses its window: the controller reset clears Run/Stop and detaches the device, +/// so it must be re-initialised completely afterwards (IMXRT1060RM 42.5.6.2.1, p.2394). +static bool controller_reset(uint8_t rhport) { ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); - TU_ASSERT(ci_ep_count(dcd_reg) <= TUP_DCD_ENDPOINT_MAX); - - #if TU_CHECK_MCU(OPT_MCU_HPM) - usb_phy_init((USB_Type *)dcd_reg, false); - #endif + tu_memclr(&_dcd_data, sizeof(dcd_data_t)); // Reset controller dcd_reg->USBCMD |= USBCMD_RESET; - while (dcd_reg->USBCMD & USBCMD_RESET) {} + uint32_t guard = CI_HS_BUSY_SPIN; + while ((dcd_reg->USBCMD & USBCMD_RESET) && guard--) {} + TU_VERIFY(!(dcd_reg->USBCMD & USBCMD_RESET)); // reached from the ISR too, so never halt here // Set mode to device, must be set immediately after reset uint32_t usbmode = dcd_reg->USBMODE & ~USBMOD_CM_MASK; @@ -257,9 +306,11 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESET; + dcd_reg->ENDPTLISTADDR = (uint32_t)_dcd_data.qhd; // Endpoint List Address has to be 2K alignment dcd_reg->USBSTS = dcd_reg->USBSTS; - dcd_reg->USBINTR = INTR_USB | INTR_ERROR | INTR_PORT_CHANGE | INTR_SUSPEND; + dcd_reg->USBINTR = INTR_USB | INTR_ERROR | INTR_PORT_CHANGE | INTR_RESET | INTR_SUSPEND; uint32_t usbcmd = dcd_reg->USBCMD; usbcmd &= ~USBCMD_INTR_THRESHOLD_MASK; // Interrupt Threshold Interval = 0 @@ -270,8 +321,22 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { return true; } +bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { + (void)rh_init; + ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); + + TU_ASSERT(ci_ep_count(dcd_reg) <= TUP_DCD_ENDPOINT_MAX); + + #if TU_CHECK_MCU(OPT_MCU_HPM) + usb_phy_init((USB_Type *)dcd_reg, false); + #endif + + return controller_reset(rhport); +} + bool dcd_deinit(uint8_t rhport) { ci_hs_regs_t* dcd_reg = CI_HS_REG(rhport); + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESET; // disable all interrupt dcd_reg->USBINTR = 0; @@ -280,9 +345,9 @@ bool dcd_deinit(uint8_t rhport) { dcd_reg->USBCMD &= ~USBCMD_RUN_STOP; // flush all endpoints - while (dcd_reg->ENDPTPRIME) {} - dcd_reg->ENDPTFLUSH = 0xFFFFFFFF; - while (dcd_reg->ENDPTFLUSH) {} + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTPRIME && guard--) {} + flush_endpoints(dcd_reg, 0xFFFFFFFF); return true; } @@ -296,11 +361,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, false); - - ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); - dcd_reg->DEVICEADDR = (dev_addr << 25) | TU_BIT(24); + // Response with status first before changing device address. A refused prime means a new + // setup superseded this transfer; staging an address whose ACK will never arrive would + // leave the device answering on it, so only arm the address when the status went out. + if (dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0, false)) { + ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); + dcd_reg->DEVICEADDR = (dev_addr << 25) | TU_BIT(24); + } } void dcd_remote_wakeup(uint8_t rhport) { @@ -468,9 +535,7 @@ bool dcd_edpt_iso_activate(uint8_t rhport, const tusb_desc_endpoint_t *desc_ep) // dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); // Flush EP - const uint32_t flush_mask = TU_BIT(epnum + (dir ? 16 : 0)); - dcd_reg->ENDPTFLUSH = flush_mask; - while (dcd_reg->ENDPTFLUSH & flush_mask) {} + flush_endpoints(dcd_reg, TU_BIT(epnum + (dir ? 16 : 0))); // disable to change max packet size ep_ctrl_clear(endptctrl, dir, ENDPTCTRL_ENABLE); @@ -496,7 +561,7 @@ void dcd_edpt_close_all(uint8_t rhport) { } } -static void qhd_start_xfer(uint8_t rhport, uint8_t epnum, uint8_t dir) { +static bool qhd_start_xfer(uint8_t rhport, uint8_t epnum, uint8_t dir) { ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); dcd_qhd_t *p_qhd = &_dcd_data.qhd[epnum][dir]; dcd_qtd_t *p_qtd = &_dcd_data.qtd[epnum][dir]; @@ -509,13 +574,22 @@ static void qhd_start_xfer(uint8_t rhport, uint8_t epnum, uint8_t dir) { dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); if (epnum == 0) { - // follows UM 24.10.8.1.1 Setup packet handling using setup lockout mechanism - // wait until ENDPTSETUPSTAT before priming data/status in response TODO add time out - while (dcd_reg->ENDPTSETUPSTAT & TU_BIT(0)) {} + // Setup lockout (IMXRT1060RM 42.5.6.4.2.1 Setup Phase, p.2403): never prime EP0 while a new + // SETUP is pending. The ISR + // normally consumes ENDPTSETUPSTAT quickly; if the guard trips, fail the transfer so usbd + // releases the endpoint (a pending SETUP supersedes this response anyway; without one, usbd + // stalls EP0 and the host recovers with a fresh control transfer). + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTSETUPSTAT & TU_BIT(0)) { + if (!guard--) { + return false; + } + } } // start transfer dcd_reg->ENDPTPRIME = TU_BIT(epnum + (dir ? 16 : 0)); + return true; } bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_t total_bytes, bool is_isr) { @@ -531,9 +605,7 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_t to // Start qhd transfer p_qhd->ff = NULL; - qhd_start_xfer(rhport, epnum, dir); - - return true; + return qhd_start_xfer(rhport, epnum, dir); } #if !CFG_TUD_MEM_DCACHE_ENABLE @@ -584,9 +656,7 @@ bool dcd_edpt_xfer_fifo(uint8_t rhport, uint8_t ep_addr, tu_fifo_t *ff, uint16_t // Start qhd transfer p_qhd->ff = ff; - qhd_start_xfer(rhport, epnum, dir); - - return true; + return qhd_start_xfer(rhport, epnum, dir); } #endif @@ -634,43 +704,43 @@ void dcd_int_handler(uint8_t rhport) { return; } - // Set if the port controller enters the full or high-speed operational state. - // either from Bus Reset or Suspended state - if (int_status & INTR_PORT_CHANGE) { - // TU_LOG2("PortChange %08lx\r\n", dcd_reg->PORTSC1); - - // Reset interrupt is not enabled, we manually check if Port Change is due - // to connection / disconnection - if (dcd_reg->USBSTS & INTR_RESET) { - dcd_reg->USBSTS = INTR_RESET; - - if (dcd_reg->PORTSC1 & PORTSC1_CURRENT_CONNECT_STATUS) { - const uint32_t speed = (dcd_reg->PORTSC1 & PORTSC1_PORT_SPEED) >> PORTSC1_PORT_SPEED_POS; - bus_reset(rhport); - dcd_event_bus_reset(rhport, (tusb_speed_t)speed, true); - } else { - dcd_event_bus_signal(rhport, DCD_EVENT_UNPLUGGED, true); - } - } else { - // Triggered by resuming from suspended state - if (!(dcd_reg->PORTSC1 & PORTSC1_SUSPEND)) { - dcd_event_bus_signal(rhport, DCD_EVENT_RESUME, true); - } - } - } + const uint8_t pci_reason = _port_change_reason[rhport]; // save current pci_reason if (int_status & INTR_SUSPEND) { - // TU_LOG2("Suspend %08lx\r\n", dcd_reg->PORTSC1); + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESUME; // next PCI is resume + dcd_event_bus_signal(rhport, DCD_EVENT_SUSPEND, true); + } - if (dcd_reg->PORTSC1 & PORTSC1_SUSPEND) { - // Note: Host may delay more than 3 ms before and/or after bus reset before doing enumeration. - // Skip suspend event if we are not addressed - if ((dcd_reg->DEVICEADDR >> 25) & 0x0f) { - dcd_event_bus_signal(rhport, DCD_EVENT_SUSPEND, true); - } + // USB Reset Received: register cleanup runs here within the reset window (IMXRT1060RM 42.5.6.2.1, p.2394) + // and BUS_RESET_START fires now; BUS_RESET_END, with the final speed, is triggered later by PCI. + if (int_status & INTR_RESET) { + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESET; + bus_reset_begin(rhport); + dcd_event_bus_signal(rhport, DCD_EVENT_BUS_RESET_START, true); + } + + // Port entered the full/high-speed operational state: the end of a bus reset, or a resume. + if (int_status & INTR_PORT_CHANGE) { + if (pci_reason == PORT_CHANGE_REASON_RESUME) { + dcd_event_bus_signal(rhport, DCD_EVENT_RESUME, true); + } else { + // the undefined encoding falls back to full speed + const uint32_t pspd = (dcd_reg->PORTSC1 & PORTSC1_PORT_SPEED) >> PORTSC1_PORT_SPEED_POS; + const tusb_speed_t speed = (pspd == PORTSC1_PORT_SPEED_LOW) ? TUSB_SPEED_LOW : + (pspd == PORTSC1_PORT_SPEED_HIGH) ? TUSB_SPEED_HIGH : TUSB_SPEED_FULL; + dcd_event_bus_reset(rhport, speed, true); + // This reset is over, so the next port change is a resume. Leaving it at RESET instead would + // dispatch every later resume as another end-of-reset, clearing the queue heads mid-session. + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESUME; } } + // No unplug detection yet, by the manual rather than by omission: IMXRT1060RM 42.7.31 (p.2470) says a zero + // Current Connect Status means the device "did not attach successfully or was forcibly + // disconnected by the software writing a zero to the Run bit ... It does not state the device + // being disconnected or suspended", so a cable pull raises no port change at all. VBUS via + // OTGSC BSV is the manual's disconnect indicator, and it is board dependent. + if (int_status & INTR_USB) { // Make sure we read the latest version of _dcd_data. dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); @@ -678,7 +748,7 @@ void dcd_int_handler(uint8_t rhport) { const uint32_t edpt_complete = dcd_reg->ENDPTCOMPLETE; dcd_reg->ENDPTCOMPLETE = edpt_complete; // acknowledge - // 23.10.12.3 Failed QTD also get ENDPTCOMPLETE set + // 42.5.6.6.4 Transfer Completion (p.2413): a failed dTD also sets ENDPTCOMPLETE // nothing to do, we will submit xfer as error to usbd // if (int_status & INTR_ERROR) { } @@ -694,12 +764,39 @@ void dcd_int_handler(uint8_t rhport) { } // Set up Received - // 23.10.10.2 Operational model for setup transfers + // 42.5.6.4.2 Control Endpoint Operation Model (p.2403) // Must be after normal transfer complete since it is possible to have both previous control status + new setup // in the same frame and we should handle previous status first. if (dcd_reg->ENDPTSETUPSTAT) { + // 42.5.6.4.2.1 Setup Phase (p.2403) steps 1-2: duplicate the setup payload BEFORE clearing + // ENDPTSETUPSTAT - + // the clear releases the setup lockout and a back-to-back SETUP (usbtest case 10) can + // overwrite the queue-head buffer immediately after. The copy is read through the volatile + // qualifier rather than memcpy'd because C orders volatile accesses only against each + // other: a plain copy may legally be sunk past the lockout-releasing store below. + union { + tusb_control_request_t request; + uint8_t byte[8]; + } setup; + const volatile uint8_t *setup_src = (const volatile uint8_t *)&_dcd_data.qhd[0][0].setup_request; + for (uint8_t i = 0; i < sizeof(setup.request); i++) { + setup.byte[i] = setup_src[i]; + } dcd_reg->ENDPTSETUPSTAT = dcd_reg->ENDPTSETUPSTAT; - dcd_event_setup_received(rhport, (uint8_t *)(uintptr_t)&_dcd_data.qhd[0][0].setup_request, true); + + // Retire a status/handshake phase left primed by the previous control sequence + // (IMXRT1060RM 42.5.6.4.2.1, p.2403), which would otherwise retire the response the task is about to + // prime for this setup. Skipped when EP0 has nothing primed or priming, since the manual + // does not want the flush wait in an interrupt handler when it has nothing to do. + // One volatile read per statement: C leaves their order unspecified within a single + // expression, which IAR rejects outright (Pa082). + const uint32_t ep0_mask = TU_BIT(0) | TU_BIT(16); + const uint32_t ep0_stat = dcd_reg->ENDPTSTAT; + const uint32_t ep0_prime = dcd_reg->ENDPTPRIME; + if ((ep0_stat | ep0_prime) & ep0_mask) { + flush_endpoints(dcd_reg, ep0_mask); + } + dcd_event_setup_received(rhport, setup.byte, true); } } -- cgit v1.3.1 From 5baf5925c8b6a033de85e3b5537ea879de75e3da Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 17 Aug 2026 01:02:22 +0700 Subject: dcd(ip3511): fix DEVCMDSTAT write-1-to-clear handling and EP0 setup races DEVCMDSTAT mixes read/write fields with write-1-to-clear latches, so a blind read-modify-write writes a pending latch back as a one and silently clears it - a setup consumed that way strands EP0. Mask the latches on every update. The setup path follows the manual's order: acknowledge the latch, then read the payload. The EP0 IN interrupt is cleared along with EP0 OUT, as the control endpoint flowchart requires - a control IN completion latched before the setup must not reach usbd after it, where it would be applied to the request the setup just started and arm its status stage early. The payload is copied a byte at a time out of a buffer now declared volatile: the controller DMAs a new setup packet into it as soon as the latch is cleared, and C orders volatile accesses only against each other, so gcc sinks a plain memcpy below the guard read that follows at -O2 and -O3 - leaving only -Os, the level CI builds, correct. --- src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c | 106 +++++++++++++++++++++------ 1 file changed, 85 insertions(+), 21 deletions(-) (limited to 'src/portable') diff --git a/src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c b/src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c index d5b03e4b1..42f6750b1 100644 --- a/src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c +++ b/src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c @@ -87,6 +87,10 @@ enum { DEVCMDSTAT_SUSPEND_CHANGE_MASK = TU_BIT(25), DEVCMDSTAT_RESET_CHANGE_MASK = TU_BIT(26), DEVCMDSTAT_VBUS_DEBOUNCED_MASK = TU_BIT(28), + + // write-1-to-clear latches + DEVCMDSTAT_W1C_MASK = DEVCMDSTAT_SETUP_RECEIVED_MASK | DEVCMDSTAT_CONNECT_CHANGE_MASK | + DEVCMDSTAT_SUSPEND_CHANGE_MASK | DEVCMDSTAT_RESET_CHANGE_MASK, }; enum { @@ -171,7 +175,9 @@ typedef struct ep_cmd_sts_t ep[2*MAX_EP_PAIRS][2]; xfer_dma_t dma[2*MAX_EP_PAIRS]; - TU_ATTR_ALIGNED(64) uint8_t setup_packet[8]; + // volatile: the controller DMAs a new setup packet into this buffer as soon as the SETUP + // latch is cleared, so reads of it must stay ordered against the register accesses around them + TU_ATTR_ALIGNED(64) volatile uint8_t setup_packet[8]; }dcd_data_t; // EP list must be 256-byte aligned @@ -180,8 +186,12 @@ typedef struct // Use CFG_TUD_MEM_SECTION to place it accordingly. CFG_TUD_MEM_SECTION TU_ATTR_ALIGNED(256) static dcd_data_t _dcd; -// Dummy buffer to fix ZLPs overwriting the buffer (probably an USB/DMA controller bug) -// TODO find way to save memory +// Dummy buffer to fix ZLPs overwriting the buffer: Errata LPC55S6x USB.5 / LPC55S2x USB.4 - the +// HS device controller always DMA-writes OUT data in 8-byte units, so up to 7 bytes land past the +// received length. This redirects the ZLP case; the general short-OUT case is unhandled here +// (TinyUSB's own endpoint buffers are sized/aligned so the spill stays inside them, but a tight +// caller buffer can be overrun by up to 7 bytes - the SDK's documented workaround is a bounce +// buffer). TODO find way to save memory CFG_TUD_MEM_SECTION TU_ATTR_ALIGNED(64) static uint8_t dummy[8]; //--------------------------------------------------------------------+ @@ -221,7 +231,7 @@ static const dcd_controller_t _dcd_controller[] = { // INTERNAL OBJECT & FUNCTION DECLARATION //--------------------------------------------------------------------+ -TU_ATTR_ALWAYS_INLINE static inline uint16_t get_buf_offset(void const * buffer) { +TU_ATTR_ALWAYS_INLINE static inline uint16_t get_buf_offset(void const volatile * buffer) { uint32_t addr = (uint32_t) buffer; TU_ASSERT( (addr & 0x3f) == 0, 0 ); return ( (addr >> 6) & 0xFFFFUL ) ; @@ -247,6 +257,16 @@ TU_ATTR_ALWAYS_INLINE static inline bool rhport_is_highspeed(uint8_t rhport) { return _dcd_controller[rhport].is_highspeed; } + +// DEVCMDSTAT mixes RW fields with write-1-to-clear latches (SETUP + the 3 change bits): a blind +// RMW writes a pending latch back as 1 and silently clears it (a SETUP eaten this way strands +// EP0). Mask the latches on every update; pass one in set_mask only to clear it. +TU_ATTR_ALWAYS_INLINE static inline void devcmdstat_update(dcd_registers_t* dcd_reg, + uint32_t clear_mask, uint32_t set_mask) { + const uint32_t v = dcd_reg->DEVCMDSTAT & ~(DEVCMDSTAT_W1C_MASK | clear_mask); + dcd_reg->DEVCMDSTAT = v | set_mask; +} + //--------------------------------------------------------------------+ // CONTROLLER API //--------------------------------------------------------------------+ @@ -284,8 +304,10 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) { dcd_reg->DATABUFSTART = tu_align((uint32_t) &_dcd, TU_BIT(22)); // 22-bit alignment dcd_reg->INTSTAT = dcd_reg->INTSTAT; // clear all pending interrupt dcd_reg->INTEN = INT_DEVICE_STATUS_MASK; - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_DEVICE_ENABLE_MASK | DEVCMDSTAT_DEVICE_CONNECT_MASK | - DEVCMDSTAT_RESET_CHANGE_MASK | DEVCMDSTAT_CONNECT_CHANGE_MASK | DEVCMDSTAT_SUSPEND_CHANGE_MASK; + // deliberately clear every latch (incl. a SETUP left by a bootloader/warm start) for a + // deterministic init state + devcmdstat_update(dcd_reg, 0, DEVCMDSTAT_DEVICE_ENABLE_MASK | DEVCMDSTAT_DEVICE_CONNECT_MASK | + DEVCMDSTAT_W1C_MASK); NVIC_ClearPendingIRQ(_dcd_controller[rhport].irqnum); @@ -309,8 +331,7 @@ 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, false); - dcd_reg->DEVCMDSTAT &= ~DEVCMDSTAT_DEVICE_ADDR_MASK; - dcd_reg->DEVCMDSTAT |= dev_addr; + devcmdstat_update(dcd_reg, DEVCMDSTAT_DEVICE_ADDR_MASK, dev_addr); } void dcd_remote_wakeup(uint8_t rhport) @@ -321,13 +342,13 @@ void dcd_remote_wakeup(uint8_t rhport) void dcd_connect(uint8_t rhport) { dcd_registers_t* dcd_reg = _dcd_controller[rhport].regs; - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_DEVICE_CONNECT_MASK; + devcmdstat_update(dcd_reg, 0, DEVCMDSTAT_DEVICE_CONNECT_MASK); } void dcd_disconnect(uint8_t rhport) { dcd_registers_t* dcd_reg = _dcd_controller[rhport].regs; - dcd_reg->DEVCMDSTAT &= ~DEVCMDSTAT_DEVICE_CONNECT_MASK; + devcmdstat_update(dcd_reg, DEVCMDSTAT_DEVICE_CONNECT_MASK, 0); } void dcd_sof_enable(uint8_t rhport, bool en) @@ -380,9 +401,17 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) uint8_t const ep_id = ep_addr2id(ep_addr); + // Preserve rf_tv: for non-control endpoints it is a TYPE bit, not the toggle value (UM11126: + // T=1 + RF 1/0 = interrupt/iso). Zeroing it here turned HS periodic interrupt endpoints into + // isochronous - no handshake on OUT, dead IN (usbtest cases 25/26 on lpc55 HS port). + // TODO implement the Errata LPC546xx USB.13 work-around (same semantics in UM11126): with RF/TV preserved at 1, TR + // loads the toggle from TV, so an HS interrupt endpoint restarts on DATA1 after clear-halt and + // the host discards one packet as a retransmission. The documented workaround needs an + // interrupt-on-NAK state machine (park as generic TR=1/TV=0, wait for a NAKed token to latch + // toggle 0 via EPTOGGLE, restore the type) - deferred; one lost packet beats the fully broken + // endpoint the old rf_tv clear caused. _dcd.ep[ep_id][0].cmd_sts.stall = 0; _dcd.ep[ep_id][0].cmd_sts.toggle_reset = 1; - _dcd.ep[ep_id][0].cmd_sts.rf_tv = 0; } bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc) @@ -432,7 +461,7 @@ void dcd_edpt_close_all (uint8_t rhport) { for (uint8_t ep_id = 0; ep_id < 2*_dcd_controller[rhport].ep_pairs; ++ep_id) { - _dcd.ep[ep_id][0].cmd_sts.active = _dcd.ep[ep_id][0].cmd_sts.active = 0; // TODO proper way is to EPSKIP then wait ep[][].active then write ep[][].disable (see table 778 in LPC55S69 Use Manual) + _dcd.ep[ep_id][0].cmd_sts.active = _dcd.ep[ep_id][1].cmd_sts.active = 0; // TODO proper way is to EPSKIP then wait ep[][].active then write ep[][].disable (see table 778 in LPC55S69 Use Manual) _dcd.ep[ep_id][0].cmd_sts.disable = _dcd.ep[ep_id][1].cmd_sts.disable = 1; } } @@ -538,7 +567,7 @@ static void bus_reset(uint8_t rhport) dcd_reg->EPSKIP = 0xFFFFFFFF; dcd_reg->INTSTAT = dcd_reg->INTSTAT; // clear all pending interrupt - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_SETUP_RECEIVED_MASK; // clear setup received interrupt + devcmdstat_update(dcd_reg, 0, DEVCMDSTAT_SETUP_RECEIVED_MASK); // clear setup received interrupt dcd_reg->INTEN = INT_DEVICE_STATUS_MASK | TU_BIT(0) | TU_BIT(1); // enable device status & control endpoints } @@ -597,18 +626,25 @@ void dcd_int_handler(uint8_t rhport) { dcd_registers_t* dcd_reg = _dcd_controller[rhport].regs; - uint32_t const cmd_stat = dcd_reg->DEVCMDSTAT; - uint32_t int_status = dcd_reg->INTSTAT; - int_status &= dcd_reg->INTEN; + int_status &= dcd_reg->INTEN; dcd_reg->INTSTAT = int_status; // Acknowledge handled interrupt if (int_status == 0) return; + // Snapshot after the INTSTAT ack: latch bits persist (RWC) so nothing is lost, while the reverse + // order could consume INTSTAT bit0 for a SETUP not yet visible in the snapshot - stranding the + // SETUP (INTSTAT is edge-latched) and feeding bit0 to process_xfer_isr as a bogus completion. + uint32_t const cmd_stat = dcd_reg->DEVCMDSTAT; + //------------- Device Status -------------// if ( int_status & INT_DEVICE_STATUS_MASK ) { - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_RESET_CHANGE_MASK | DEVCMDSTAT_CONNECT_CHANGE_MASK | DEVCMDSTAT_SUSPEND_CHANGE_MASK; + // clear only the change latches observed in the snapshot: one latched by hardware between the + // snapshot and this write would be acknowledged unseen (its DEV_INT re-latches and dispatches + // next pass instead) + devcmdstat_update(dcd_reg, 0, cmd_stat & + (DEVCMDSTAT_RESET_CHANGE_MASK | DEVCMDSTAT_CONNECT_CHANGE_MASK | DEVCMDSTAT_SUSPEND_CHANGE_MASK)); if ( cmd_stat & DEVCMDSTAT_RESET_CHANGE_MASK) // bus reset { @@ -653,15 +689,43 @@ void dcd_int_handler(uint8_t rhport) _dcd.ep[0][0].cmd_sts.active = _dcd.ep[1][0].cmd_sts.active = 0; _dcd.ep[0][0].cmd_sts.stall = _dcd.ep[1][0].cmd_sts.stall = 0; - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_SETUP_RECEIVED_MASK; + // UM flow: ack the latch FIRST, then read the payload. This IP has no setup lockout, so a + // back-to-back SETUP can overwrite _dcd.setup_packet at any time - but with the latch already + // released, any such overwrite re-latches SETUP_RECEIVED and is redelivered (worst case a + // superseded duplicate, absorbed by usbd's queued-setup counter). The reverse order can + // consume the newer SETUP's latch unseen and lose it. + devcmdstat_update(dcd_reg, 0, DEVCMDSTAT_SETUP_RECEIVED_MASK); + + // UM11126 Fig 163 (control EP0 flowchart) requires clearing the EP0IN interrupt here: a + // control IN completion latched before this SETUP must not reach usbd after it, where it + // would be applied to the new request and arm its status stage early. EP0OUT goes with it - + // bit0 is set by SETUP reception too, and left set it would replay next pass as a phantom + // completion. Neither can discard live work: the SETUP latch NAKs all EP0 traffic until the + // update above, and both EP0 Active bits were cleared a few lines up. + dcd_reg->INTSTAT = TU_BIT(0) | TU_BIT(1); + + // Copied a byte at a time rather than with memcpy: C orders volatile accesses only against + // each other, so a non-volatile copy of this buffer may be sunk below the guard read that + // follows - gcc does exactly that at -O2 and -O3, leaving only -Os correct. + uint8_t setup_copy[8]; + for (uint8_t i = 0; i < sizeof(setup_copy); i++) { + setup_copy[i] = _dcd.setup_packet[i]; + } - dcd_event_setup_received(rhport, _dcd.setup_packet, true); + // a SETUP that raced in after the acks (its bit0 consumed above) makes this copy suspect: + // its latch is visible again, so re-raise the endpoint interrupt and let the next pass + // deliver the newer payload rather than passing up bytes that may be torn between the two + if (dcd_reg->DEVCMDSTAT & DEVCMDSTAT_SETUP_RECEIVED_MASK) { + dcd_reg->INTSETSTAT = TU_BIT(0); + } else { + dcd_event_setup_received(rhport, setup_copy, true); + } // keep waiting for next setup prepare_setup_packet(rhport); - // clear bit0 - int_status = tu_bit_clear(int_status, 0); + // drop both EP0 bits: acked above, and neither belongs to the request this SETUP starts + int_status &= ~(TU_BIT(0) | TU_BIT(1)); } // Endpoint transfer complete interrupt -- cgit v1.3.1 From f065f280284a6f1b5b4c4d2849db4bff4a3ce70c Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Wed, 19 Aug 2026 05:06:04 +0200 Subject: ohci: defer descriptor reclaim until next frame --- src/portable/ohci/ohci.c | 11 ++++++++--- src/portable/ohci/ohci.h | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) (limited to 'src/portable') diff --git a/src/portable/ohci/ohci.c b/src/portable/ohci/ohci.c index 7ede1ac72..2a174e46c 100644 --- a/src/portable/ohci/ohci.c +++ b/src/portable/ohci/ohci.c @@ -390,6 +390,9 @@ static void ed_list_remove_by_addr(ohci_ed_t * p_head, uint8_t dev_addr) { // Control endpoints (EP number 0) are statically allocated with the device which are only reused // after connection of another device long after HC has finished with them now, these can be freed immediately. if (ed->w0.ep_number != 0) { + // Wait until the next frame before reclaiming the ED and its TDs. Set the deadline before + // publishing is_reclaiming so a pending SOF IRQ cannot use an older deadline for this ED. + ohci_data.reclaim_frame = (uint16_t)(OHCI_REG->frame_number + 1); ed->w0.is_reclaiming = 1; // 5.2.7.1.2 Removing. Disable list processing for bulk @@ -397,7 +400,8 @@ static void ed_list_remove_by_addr(ohci_ed_t * p_head, uint8_t dev_addr) { OHCI_REG->control &= ~OHCI_CONTROL_LIST_BULK_ENABLE_MASK; } - // Temporarily enable SOF IRQ. ED and TD Memory will be reclaimed in the SOF IRQ. + // Temporarily enable SOF IRQ. Clear any pending SOF first to wait for the next frame. + OHCI_REG->interrupt_status = OHCI_INT_SOF_MASK; OHCI_REG->interrupt_enable = OHCI_INT_SOF_MASK; } else { ed->w0.used = 0; @@ -665,8 +669,9 @@ void hcd_int_handler(uint8_t hostid, bool in_isr) { // Disable MIE as per OHCI spec 5.3 OHCI_REG->interrupt_disable = OHCI_INT_MASTER_ENABLE_MASK; - // Start of frame (SOF) - if (int_status & OHCI_INT_SOF_MASK) { + // Start of frame (SOF). Signed subtraction handles frame number rollover and delayed interrupts. + if ((int_status & OHCI_INT_SOF_MASK) && + ((int16_t)((uint16_t)OHCI_REG->frame_number - ohci_data.reclaim_frame) >= 0)) { OHCI_REG->interrupt_disable = OHCI_INT_SOF_MASK; bool re_enable_lists = false; diff --git a/src/portable/ohci/ohci.h b/src/portable/ohci/ohci.h index c66954502..e28c6404f 100644 --- a/src/portable/ohci/ohci.h +++ b/src/portable/ohci/ohci.h @@ -183,6 +183,7 @@ typedef struct TU_ATTR_ALIGNED(256) { gtd_extra_data_t gtd_extra[GTD_MAX]; volatile uint16_t frame_number_hi; + volatile uint16_t reclaim_frame; } ohci_data_t; //--------------------------------------------------------------------+ -- cgit v1.3.1 From 75a01f561438c16677b48f3a59fda80a2b096ad8 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 19 Aug 2026 12:29:45 +0700 Subject: dcd(ci_hs): stage the device address before priming the status stage IMXRT1060RM 42.7.23 and UM10503 Table 478 both ask for the DEVICEADDR write with USBADRA=1 to happen after the SET_ADDRESS data phase and before the prime of the status stage, so the controller loads USBADR from its holding register when the status stage is ACKed. The driver did it the other way round, leaving a window between the ENDPTPRIME store and the DEVICEADDR store: an IN answered inside that window ACKs with USBADRA still 0, so the holding register is never consulted and the device keeps answering on address 0 while the host has moved to the new one. Instruction timing alone cannot open that window, but dcd_set_address() runs in task context, so any interrupt landing between the two stores stretches it past a microframe. Hardware discards a staged address on a SETUP or OUT to endpoint 0 and zeroes USBADR on a bus reset, which covers a superseded SET_ADDRESS. What it cannot cover is a SETUP latched before this write and still unconsumed after the full CI_HS_BUSY_SPIN spin, which refuses the prime: condition 2 already fired for that earlier SETUP, so the stage would survive and load USBADR on the next EP0 IN ACK of an unrelated transfer. USB 2.0 9.4.6 is explicit that "the USB device does not change its device address until after the Status stage of this request is completed successfully", so the refused-prime path restores the previous USBADR rather than leaving a stage armed. Restoring the previous value rather than writing zero keeps 9.4.6's Address-state row correct, where a device already at a non-zero address must stay there; on Linux that write is always a no-op, since hub_set_address only issues SET_ADDRESS from USB_STATE_DEFAULT. Cast dev_addr before the shift: it is uint8_t, promoted to int, so an address of 64 or more reached the sign bit of a 32-bit int. No errata applies: IMXRT1060CE_A Rev 1.3 lists only ERR050101 and ERR010661 for USB, IMXRT1060CE_B Rev 1.1 only ERR010661. Validated on mimxrt1064_evk: 18/19 device+host tests, 6x usbtest 30/30, and a 100-iteration forced re-enumeration A/B that is clean on both this change and its parent (0/100 each). All 19 ci_hs boards build; unit tests 63/63; PVS drops one diagnostic (the sign-bit shift) and adds none. --- src/portable/chipidea/ci_hs/ci_hs_type.h | 8 ++++++++ src/portable/chipidea/ci_hs/dcd_ci_hs.c | 17 +++++++++++------ 2 files changed, 19 insertions(+), 6 deletions(-) (limited to 'src/portable') diff --git a/src/portable/chipidea/ci_hs/ci_hs_type.h b/src/portable/chipidea/ci_hs/ci_hs_type.h index 5baa14821..b3ef3b6af 100644 --- a/src/portable/chipidea/ci_hs/ci_hs_type.h +++ b/src/portable/chipidea/ci_hs/ci_hs_type.h @@ -29,6 +29,14 @@ enum { USBCMD_INTR_THRESHOLD_MASK = 0x00FF0000u, // Interrupt Threshold bit 23:16 }; +// DEVICEADDR +#define DEVICEADDR_USBADR_POS 25 + +enum { + DEVICEADDR_USBADRA = TU_BIT(24), ///< Device Address Advance: stage USBADR until the next EP0 IN is ACKed + DEVICEADDR_USBADR_MASK = 0xFE000000u, ///< Device Address bit 31:25 +}; + // PORTSC1 #define PORTSC1_PORT_SPEED_POS 26 diff --git a/src/portable/chipidea/ci_hs/dcd_ci_hs.c b/src/portable/chipidea/ci_hs/dcd_ci_hs.c index 6ab28e0be..f1c333280 100644 --- a/src/portable/chipidea/ci_hs/dcd_ci_hs.c +++ b/src/portable/chipidea/ci_hs/dcd_ci_hs.c @@ -361,12 +361,17 @@ 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. A refused prime means a new - // setup superseded this transfer; staging an address whose ACK will never arrive would - // leave the device answering on it, so only arm the address when the status went out. - if (dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0, false)) { - ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); - dcd_reg->DEVICEADDR = (dev_addr << 25) | TU_BIT(24); + ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); + const uint32_t prev = dcd_reg->DEVICEADDR & DEVICEADDR_USBADR_MASK; + + // IMXRT1060RM 42.7.23 / UM10503 Table 478: stage the address before priming the status stage so + // hardware loads USBADR at the status ACK. Priming first races that ACK against this write. + dcd_reg->DEVICEADDR = ((uint32_t)dev_addr << DEVICEADDR_USBADR_POS) | DEVICEADDR_USBADRA; + + if (!dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0, false)) { + // USB 2.0 9.4.6: the address changes only after the status stage completes successfully. The + // status never went out, so drop the stage - USBADRA=0 takes effect instantly. + dcd_reg->DEVICEADDR = prev; } } -- cgit v1.3.1 From f9ba5d48e0d1926488c78be6d385137f99b277b7 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Tue, 25 Aug 2026 09:26:37 +0200 Subject: fix(dwc2): keep isochronous transfers on DATA0 Isochronous endpoints do not use the normal data-toggle sequence. Avoid saving or advancing HCTSIZ PID state on completion and retry so later transfers cannot be submitted with an invalid toggled PID. Signed-off-by: HiFiPHile --- src/portable/synopsys/dwc2/hcd_dwc2.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index e8a3d6623..12918a9f7 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -587,7 +587,10 @@ static void channel_xfer_out_wrapup(dwc2_regs_t* dwc2, uint8_t ch_id) { hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; - edpt->next_pid = hctsiz.pid; // save PID + const dwc2_channel_char_t hcchar = {.value = channel->hcchar}; + if (hcchar.ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { + edpt->next_pid = hctsiz.pid; // save PID + } /* Since hctsiz.xfersize field reflects the number of bytes transferred via the AHB, not the USB) * For IN: we can use hctsiz.xfersize as remaining bytes. @@ -596,7 +599,6 @@ static void channel_xfer_out_wrapup(dwc2_regs_t* dwc2, uint8_t ch_id) { * transfer was halted before its normal completion. */ const uint16_t remain_packets = hctsiz.packet_count; - const dwc2_channel_char_t hcchar = {.value = channel->hcchar}; const uint16_t total_packets = cal_packet_count(edpt->buflen, hcchar.ep_size); const uint16_t actual_bytes = (total_packets - remain_packets) * hcchar.ep_size; @@ -635,10 +637,11 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { channel->hctsiz = hctsiz.value; edpt->next_do_ping = 0; - // pre-calculate next PID based on packet count, adjusted in transfer complete interrupt if short packet + // Single-transaction isochronous endpoints always use DATA0. Pre-calculate the next PID for other endpoints, + // adjusted in the transfer-complete interrupt if a short packet is received. if (hcchar_bm->ep_num == 0) { edpt->next_pid = HCTSIZ_PID_DATA1; // control data and status stage always start with DATA1 - } else { + } else if (hcchar_bm->ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { edpt->next_pid = cal_next_pid(edpt->next_pid, packet_count); } @@ -806,7 +809,9 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci } else { // otherwise, de-allocate channel, enable SOF set frame counter for later transfer const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; - edpt->next_pid = hctsiz.pid; // save PID + if (hcchar.ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { + edpt->next_pid = hctsiz.pid; // save PID + } edpt->uframe_countdown = edpt->uframe_interval - ucount; // enable SOF interrupt if not already enabled if (0 == (dwc2->gintmsk & GINTMSK_SOFM)) { @@ -931,7 +936,8 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h // } if (hcint & HCINT_XFER_COMPLETE) { - if (edpt->hcchar_bm.ep_num != 0) { + if (edpt->hcchar_bm.ep_num != 0 && + edpt->hcchar_bm.ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { edpt->next_pid = hctsiz.pid; // save pid (already toggled) } -- cgit v1.3.1 From 191b443d3a4f68176a637ae5b3159ef10f478485 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Tue, 25 Aug 2026 09:26:37 +0200 Subject: fix(dwc2): complete periodic IN channels immediately The core has already halted a periodic IN channel when every packet completes. Report the transfer at that point instead of requesting another halt interrupt, allowing the next service interval to be queued without delay. Signed-off-by: HiFiPHile --- src/portable/synopsys/dwc2/hcd_dwc2.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 12918a9f7..7fbefff86 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -950,7 +950,13 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h xfer->result = XFER_RESULT_SUCCESS; } - channel_disable(dwc2, channel); + if (channel_is_periodic(channel->hcchar) && remain_packets == 0) { + // The core has already halted a completed periodic IN channel. Complete + // it now so the next interval can be submitted without another halt IRQ. + is_done = true; + } else { + channel_disable(dwc2, channel); + } } else if (hcint & (HCINT_XACT_ERR | HCINT_BABBLE_ERR | HCINT_STALL)) { if (hcint & HCINT_STALL) { xfer->result = XFER_RESULT_STALLED; -- cgit v1.3.1 From 04a6ec80d94e13c70087c44dcf095d842302bafa Mon Sep 17 00:00:00 2001 From: runelauridsen Date: Tue, 1 Sep 2026 10:10:33 +0200 Subject: Fix DWC2 DMA data toggle mismatch in IN-transfers --- src/portable/synopsys/dwc2/hcd_dwc2.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 089b839ae..761627730 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -1129,6 +1129,10 @@ static bool handle_channel_in_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci channel_send_in_token(dwc2, channel); } } else if (hcint & (HCINT_XFER_COMPLETE | HCINT_STALL | HCINT_BABBLE_ERR)) { + if (edpt->hcchar_bm.ep_num != 0 && (hcint & HCINT_XFER_COMPLETE)) { + edpt->next_pid = hctsiz.pid; // save pid (already toggled) + } + const uint16_t remain_bytes = (uint16_t) hctsiz.xfer_size; const uint16_t remain_packets = hctsiz.packet_count; const uint16_t actual_len = edpt->buflen - remain_bytes; -- cgit v1.3.1 From 278c0531a07e4fe8112ef91d0d2dbba8ef0a40b3 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Tue, 25 Aug 2026 09:26:38 +0200 Subject: fix(dwc2): preserve periodic transfer phase Anchor resubmitted periodic transfers to the endpoint service interval and defer early submissions through SOF. This prevents callback latency from shifting the cadence or causing intervals to be skipped, while keeping pending transfers abortable. Signed-off-by: HiFiPHile --- src/portable/synopsys/dwc2/hcd_dwc2.c | 88 ++++++++++++++++++++++++++++------- 1 file changed, 72 insertions(+), 16 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 7fbefff86..05efacbee 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -37,7 +37,8 @@ enum { }; enum { - HCD_XFER_PERIOD_SPLIT_NYET_MAX = 3 + HCD_XFER_PERIOD_SPLIT_NYET_MAX = 3, + HCD_FRAME_NUMBER_MASK = 0x3fff }; //-------------------------------------------------------------------- @@ -56,18 +57,21 @@ typedef struct { }; struct TU_ATTR_PACKED { - uint32_t uframe_interval : 18; // micro-frame interval + uint32_t uframe_interval : 19; // micro-frame interval uint32_t speed : 2; uint32_t next_pid : 2; // PID for next transfer uint32_t next_do_ping : 1; // Do PING for next transfer if possible (highspeed OUT) uint32_t closing : 1; // endpoint is closing - // uint32_t : 8; + uint32_t periodic_phase : 1; // periodic transfer phase is established + uint32_t xfer_pending : 1; // periodic transfer waiting for its service interval + // uint32_t : 5; }; - uint32_t uframe_countdown; // micro-frame count down to transfer for periodic, only need 18-bit + uint32_t uframe_countdown; // micro-frame count down to transfer for periodic, only need 19-bit uint8_t* buffer; uint16_t buflen; + uint16_t periodic_frame; // frame/microframe number of the last scheduled periodic transaction } hcd_endpoint_t; // Additional info for each channel when it is active @@ -547,7 +551,7 @@ bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, const tusb_desc_endpoint_t* edpt->next_pid = HCTSIZ_PID_DATA0; switch (desc_ep->bmAttributes.xfer) { case TUSB_XFER_ISOCHRONOUS: - edpt->uframe_interval = 1 << (desc_ep->bInterval - 1); + edpt->uframe_interval = 1u << (desc_ep->bInterval - 1); if (bus_info.speed == TUSB_SPEED_FULL) { edpt->uframe_interval <<= 3; } @@ -555,7 +559,7 @@ bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, const tusb_desc_endpoint_t* case TUSB_XFER_INTERRUPT: if (bus_info.speed == TUSB_SPEED_HIGH) { - edpt->uframe_interval = 1 << (desc_ep->bInterval - 1); + edpt->uframe_interval = 1u << (desc_ep->bInterval - 1); } else { edpt->uframe_interval = desc_ep->bInterval << 3; } @@ -701,7 +705,44 @@ static bool edpt_xfer_kickoff(dwc2_regs_t* dwc2, uint8_t ep_id) { xfer->ep_id = ep_id; xfer->result = XFER_RESULT_INVALID; - return channel_xfer_start(dwc2, ch_id); + const bool result = channel_xfer_start(dwc2, ch_id); + if (result && channel_is_periodic(_hcd_data.edpt[ep_id].hcchar)) { + hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; + edpt->periodic_frame = (uint16_t) ((dwc2->hfnum + 1u) & HCD_FRAME_NUMBER_MASK); + edpt->periodic_phase = 1; + edpt->xfer_pending = 0; + } + return result; +} + +static uint32_t periodic_xfer_countdown(dwc2_regs_t* dwc2, hcd_endpoint_t const* edpt) { + const uint32_t ucount = (hprt_speed_get(dwc2) == TUSB_SPEED_HIGH) ? 1u : 8u; + const uint16_t frame = (uint16_t) (dwc2->hfnum & HCD_FRAME_NUMBER_MASK); + const uint16_t elapsed_frames = (uint16_t) (frame - edpt->periodic_frame) & HCD_FRAME_NUMBER_MASK; + const uint32_t elapsed_uframes = (uint32_t) elapsed_frames * ucount; + + if (elapsed_uframes < edpt->uframe_interval) { + return edpt->uframe_interval - elapsed_uframes - ucount; + } + + // The service opportunity was missed. Keep the established phase and use + // the next interval rather than starting a new interval from this request. + return edpt->uframe_interval - (elapsed_uframes % edpt->uframe_interval) - ucount; +} + +static void periodic_xfer_defer(dwc2_regs_t* dwc2, hcd_endpoint_t* edpt, uint32_t uframe_countdown) { + const uint32_t gahbcfg = dwc2->gahbcfg; + dwc2->gahbcfg = gahbcfg & ~GAHBCFG_GINT; + + edpt->uframe_countdown = uframe_countdown; + edpt->xfer_pending = 1; + + if (0 == (dwc2->gintmsk & GINTMSK_SOFM)) { + dwc2->gintsts = GINTSTS_SOF; + dwc2->gintmsk |= GINTMSK_SOFM; + } + + dwc2->gahbcfg = gahbcfg; } bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * buffer, uint16_t buflen) { @@ -722,6 +763,17 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * edpt->hcchar_bm.ep_dir = ep_dir; } + if (channel_is_periodic(edpt->hcchar) && edpt->periodic_phase) { + const uint32_t ucount = (hprt_speed_get(dwc2) == TUSB_SPEED_HIGH) ? 1u : 8u; + if (edpt->uframe_interval > ucount) { + const uint32_t countdown = periodic_xfer_countdown(dwc2, edpt); + if (countdown > 0) { + periodic_xfer_defer(dwc2, edpt, countdown); + return true; + } + } + } + return edpt_xfer_kickoff(dwc2, ep_id); } @@ -733,6 +785,13 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { const uint8_t ep_dir = tu_edpt_dir(ep_addr); const uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir); TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); + hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; + + if (edpt->xfer_pending) { + edpt->xfer_pending = 0; + edpt->uframe_countdown = 0; + return true; + } // hcd_int_disable(rhport); @@ -812,12 +871,7 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci if (hcchar.ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { edpt->next_pid = hctsiz.pid; // save PID } - edpt->uframe_countdown = edpt->uframe_interval - ucount; - // enable SOF interrupt if not already enabled - if (0 == (dwc2->gintmsk & GINTMSK_SOFM)) { - dwc2->gintsts = GINTSTS_SOF; - dwc2->gintmsk |= GINTMSK_SOFM; - } + periodic_xfer_defer(dwc2, edpt, periodic_xfer_countdown(dwc2, edpt)); // already halted, de-allocate channel (called from DMA isr) channel_dealloc(dwc2, ch_id); } @@ -1384,15 +1438,17 @@ static bool handle_sof_irq(uint8_t rhport, bool in_isr) { for(uint8_t ep_id = 0; ep_id < CFG_TUH_DWC2_ENDPOINT_MAX; ep_id++) { hcd_endpoint_t *edpt = &_hcd_data.edpt[ep_id]; if (edpt->closing == 0) { - if (edpt->hcchar_bm.enable && channel_is_periodic(edpt->hcchar) && edpt->uframe_countdown > 0) { - edpt->uframe_countdown -= tu_min32(ucount, edpt->uframe_countdown); + if (edpt->hcchar_bm.enable && channel_is_periodic(edpt->hcchar) && edpt->xfer_pending) { + if (edpt->uframe_countdown > 0) { + edpt->uframe_countdown -= tu_min32(ucount, edpt->uframe_countdown); + } if (edpt->uframe_countdown == 0) { if (!edpt_xfer_kickoff(dwc2, ep_id)) { edpt->uframe_countdown = ucount; // failed to start, try again next frame } } - more_isr = true; + more_isr = more_isr || edpt->xfer_pending; } } } -- cgit v1.3.1 From 13971b58df52bc29de97632295fcae43aeade5c9 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Thu, 27 Aug 2026 14:49:47 +0200 Subject: fix(dwc2): drain host RX status before channel IRQ Popping an IN transfer-completion entry from GRXSTSP asserts HCINT.XferCompl. Drain the receive FIFO first, then read the live masked global status so the newly asserted channel completion is handled without waiting for another interrupt. --- src/portable/synopsys/dwc2/hcd_dwc2.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 05efacbee..ca16eec70 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -1574,21 +1574,6 @@ void hcd_int_handler(uint8_t rhport, bool in_isr) { handle_hprt_irq(rhport, in_isr); } - if (gintsts & GINTSTS_HCINT) { - // Host Channel interrupt: source is cleared in HCINT register - // must be handled after TX FIFO empty - handle_channel_irq(rhport, in_isr); - } - - if (gintsts & GINTSTS_DISCINT) { - // Device disconnected - dwc2->gintsts = GINTSTS_DISCINT; - - if (0 == (dwc2->hprt & HPRT_CONN_STATUS)) { - hcd_event_device_remove(rhport, in_isr); - } - } - #if CFG_TUH_DWC2_SLAVE_ENABLE // RxFIFO non-empty interrupt handling if (gintsts & GINTSTS_RXFLVL) { @@ -1620,6 +1605,21 @@ void hcd_int_handler(uint8_t rhport, bool in_isr) { } } #endif + + // Draining the RxFIFO completion status can assert HCINT.XferCompl. Read + // the live status here so the completion is handled in this ISR invocation. + if ((dwc2->gintsts & dwc2->gintmsk) & GINTSTS_HCINT) { + handle_channel_irq(rhport, in_isr); + } + + if (gintsts & GINTSTS_DISCINT) { + // Device disconnected + dwc2->gintsts = GINTSTS_DISCINT; + + if (0 == (dwc2->hprt & HPRT_CONN_STATUS)) { + hcd_event_device_remove(rhport, in_isr); + } + } } #endif -- cgit v1.3.1 From ee92fa7607c8eacac0ea44ffa09ece2d4e67a6f0 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Thu, 27 Aug 2026 15:35:56 +0200 Subject: fix(dwc2): serialize deferred transfer abort Protect periodic deferral cancellation from the SOF interrupt. Re-enable the host interrupt before disabling an active channel because slave-mode channel disable may wait for request-queue space. --- src/portable/synopsys/dwc2/hcd_dwc2.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index ca16eec70..45add6c7f 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -787,14 +787,20 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; - if (edpt->xfer_pending) { + hcd_int_disable(rhport); + + const bool xfer_pending = edpt->xfer_pending; + if (xfer_pending) { edpt->xfer_pending = 0; edpt->uframe_countdown = 0; - return true; } + hcd_int_enable(rhport); - // hcd_int_disable(rhport); + if (xfer_pending) { + return true; + } + // Channel disable may wait for request-queue space in slave mode. // Find enabled channeled and disable it, channel will be de-allocated in the interrupt handler const uint8_t ch_id = channel_find_enabled(dwc2, dev_addr, ep_num, ep_dir); if (ch_id < 16) { @@ -802,8 +808,6 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { channel_disable(dwc2, channel); } - // hcd_int_enable(rhport); - return true; } -- cgit v1.3.1 From c023de98f44a6c0563e8addc2be3864887aa1d41 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Tue, 1 Sep 2026 03:46:15 +0200 Subject: fix(dwc2): preserve simultaneous slave channel halt Slave-mode channel handlers process one interrupt cause per pass, but the dispatcher acknowledged every HCINT bit before invoking them. When ChHltd arrived together with another cause, the handler consumed the other cause and the halt was lost. A subsequent disable could then leave CHENA|CHDIS asserted with HCINT and HAINT clear, so the submitted periodic transfer never completed. When a slave channel reports ChHltd with another cause, acknowledge only the non-halt causes and leave ChHltd pending for the next channel-IRQ pass. DMA handlers retain their existing combined-cause behavior. The uninstrumented negative capture reproduced the lost terminal state with HCCHAR=0xe044881c, HCTSIZ=0x0008001c, HCINT=0, and XFER_RESULT_INVALID. --- src/portable/synopsys/dwc2/hcd_dwc2.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 45add6c7f..9f6fc1469 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -1389,7 +1389,10 @@ static void handle_channel_irq(uint8_t rhport, bool in_isr) { dwc2_channel_char_t hcchar = {.value = channel->hcchar}; const uint32_t hcint = channel->hcint; - channel->hcint = hcint; // clear interrupt + // Slave handlers process one cause per pass. If ChHltd arrived with + // another cause, leave it pending so the next pass retires the halt. + const uint32_t hcint_clear = (!is_dma && (hcint & ~HCINT_HALTED)) ? (hcint & ~HCINT_HALTED) : hcint; + channel->hcint = hcint_clear; bool is_done = false; if (is_dma) { -- cgit v1.3.1 From dea4d268d909d3063703bf9b431cabf6223479e1 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Tue, 1 Sep 2026 03:47:15 +0200 Subject: fix(dwc2): let DMA periodic channels halt naturally DWC2 buffer/external DMA mode automatically halts a periodic channel at its next service boundary. Programming HCCHAR.CHDIS|CHENA for a non-split periodic channel is explicitly disallowed by the controller programming guide, yet channel_disable() skipped that write only for split periodic transfers. Return without programming channel disable for every periodic DMA channel. Non-periodic DMA and slave-mode channels retain the existing explicit-disable path. The previous path reproduced after 420 seconds in O2/DMA with a closing capture transfer left INVALID while HCCHAR retained CHENA|CHDIS and HCINT was clear. --- src/portable/synopsys/dwc2/hcd_dwc2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 9f6fc1469..d10f28f6a 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -191,7 +191,7 @@ TU_ATTR_ALWAYS_INLINE static inline bool channel_disable(const dwc2_regs_t* dwc2 // the worst case), the controller generates a channel halted and disables the channel automatically. // - For split enabled channels (both non-periodic and periodic), channel disable must not be programmed randomly. // However, channel disable can be programmed for specific scenarios such as NAK and FrmOvrn. - if (is_period && (channel->hcsplt & HCSPLT_SPLITEN)) { + if (is_period) { return true; } } else { -- cgit v1.3.1 From 393694d842b454ecadd32760851be899a91d69af Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Tue, 1 Sep 2026 03:59:39 +0200 Subject: fix(dwc2): fail missed isochronous frames A frame-overrun interrupt means the selected periodic service interval has already been missed. Retrying an isochronous transfer after that point cannot deliver the original packet and can leave the class waiting indefinitely for a terminal result. Enable frame-overrun interrupts for slave periodic channels. Complete isochronous IN and OUT overruns as XFER_RESULT_FAILED in both slave and DMA modes, accounting for bytes already written on OUT. Preserve the existing retry behavior for non-isochronous DMA transfers. This reports the missed packet honestly through the normal HCD completion path: no fabricated success and no class-level abort workaround. --- src/portable/synopsys/dwc2/hcd_dwc2.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index d10f28f6a..55a15b326 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -666,6 +666,9 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { } } else { uint32_t hcintmsk = HCINT_NAK | HCINT_XACT_ERR | HCINT_STALL | HCINT_XFER_COMPLETE | HCINT_DATATOGGLE_ERR; + if (is_period) { + hcintmsk |= HCINT_FARME_OVERRUN; + } if (hcchar_bm->ep_dir == TUSB_DIR_IN) { hcintmsk |= HCINT_BABBLE_ERR | HCINT_DATATOGGLE_ERR | HCINT_ACK; } else { @@ -1015,6 +1018,11 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h } else { channel_disable(dwc2, channel); } + } else if (hcint & HCINT_FARME_OVERRUN) { + if (edpt->hcchar_bm.ep_type == HCCHAR_EPTYPE_ISOCHRONOUS) { + xfer->result = XFER_RESULT_FAILED; + } + channel_disable(dwc2, channel); } else if (hcint & (HCINT_XACT_ERR | HCINT_BABBLE_ERR | HCINT_STALL)) { if (hcint & HCINT_STALL) { xfer->result = XFER_RESULT_STALLED; @@ -1108,6 +1116,12 @@ static bool handle_channel_out_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t } else if (hcint & HCINT_STALL) { xfer->result = XFER_RESULT_STALLED; channel_disable(dwc2, channel); + } else if (hcint & HCINT_FARME_OVERRUN) { + channel_xfer_out_wrapup(dwc2, ch_id); + if (edpt->hcchar_bm.ep_type == HCCHAR_EPTYPE_ISOCHRONOUS) { + xfer->result = XFER_RESULT_FAILED; + } + channel_disable(dwc2, channel); } else if (hcint & HCINT_NYET) { xfer->err_count = 0; if (hcsplt.split_en == 1u) { @@ -1273,8 +1287,12 @@ static bool handle_channel_in_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci channel_xfer_in_retry(dwc2, ch_id, hcint); } } else if (hcint & HCINT_FARME_OVERRUN) { - // retry start-split in next binterval - channel_xfer_in_retry(dwc2, ch_id, hcint); + if (hcchar.ep_type == HCCHAR_EPTYPE_ISOCHRONOUS) { + xfer->result = XFER_RESULT_FAILED; + is_done = true; + } else { + channel_xfer_in_retry(dwc2, ch_id, hcint); + } } if (xfer->closing == 1) { @@ -1341,6 +1359,14 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc } } } + } else if (hcint & HCINT_FARME_OVERRUN) { + channel_xfer_out_wrapup(dwc2, ch_id); + if (edpt->hcchar_bm.ep_type == HCCHAR_EPTYPE_ISOCHRONOUS) { + xfer->result = XFER_RESULT_FAILED; + is_done = true; + } else { + channel_xfer_start(dwc2, ch_id); + } } else if (hcint & HCINT_NYET) { if (hcsplt.split_en && hcsplt.split_compl) { // split not yet mean hub has no data, retry complete split -- cgit v1.3.1 From 3a81af3de30ee999b543af4e52408f3a6b9b5fdf Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Tue, 1 Sep 2026 04:02:54 +0200 Subject: fix(dwc2): enable periodic channels in the selected frame Periodic IN and DMA-backed transfers selected ODDFRM before waiting for request-queue space. A DWC2 interrupt could also run between reading HFNUM and writing HCCHAR.CHENA, allowing the selected frame to pass while the transfer still appeared active. Wait for request-queue capacity with controller interrupts enabled, then mask only GAHBCFG.GINT while sampling HFNUM and enabling a new periodic channel. Record the periodic phase from that same HFNUM sample so a boundary after channel enable cannot shift later interval calculations. The bounded critical section contains no queue wait, callback, disable, or allocation loop. Retries that already selected their frame bypass the new selection step. Also clear a retained HCCHAR.CHDIS before every channel enable. A halted channel can otherwise be re-enabled as CHENA|CHDIS and wait for a terminal interrupt that never arrives. Hardware traces captured periodic IN selections at frames 0x3303 and 0x3266 but activation only after 0x330c and 0x3273, respectively. --- src/portable/synopsys/dwc2/hcd_dwc2.c | 85 ++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 31 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 55a15b326..31ea2ae20 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -204,13 +204,37 @@ TU_ATTR_ALWAYS_INLINE static inline bool channel_disable(const dwc2_regs_t* dwc2 return true; } -// attempt to send IN token to receive data -TU_ATTR_ALWAYS_INLINE static inline bool channel_send_in_token(const dwc2_regs_t* dwc2, dwc2_channel_t* channel) { +// Enable a channel, selecting the following frame for a new periodic transfer. +// Return that frame from the same HFNUM sample used for ODDFRM selection. +// Clear CHDIS explicitly: a halted channel may retain it in HCCHAR. +TU_ATTR_ALWAYS_INLINE static inline uint16_t channel_enable(dwc2_regs_t* dwc2, dwc2_channel_t* channel, + bool next_periodic_frame) { + uint32_t hcchar = channel->hcchar & ~HCCHAR_CHDIS; + uint16_t periodic_frame = 0; + if (next_periodic_frame) { + // Prevent the USB interrupt from consuming the selected frame before + // HCCHAR.CHENA is written. Queue-space waits happen before this helper. + const uint32_t gahbcfg = dwc2->gahbcfg; + dwc2->gahbcfg = gahbcfg & ~GAHBCFG_GINT; + const uint32_t hfnum = dwc2->hfnum; + hcchar = (hcchar & ~HCCHAR_ODDFRM) | (((hfnum & 1u) ^ 1u) << HCCHAR_ODDFRM_Pos); + channel->hcchar = hcchar | HCCHAR_CHENA; + periodic_frame = (uint16_t) ((hfnum + 1u) & HCD_FRAME_NUMBER_MASK); + dwc2->gahbcfg = gahbcfg; + } else { + channel->hcchar = hcchar | HCCHAR_CHENA; + } + return periodic_frame; +} + +// Attempt to send an IN token to receive data. For a new periodic transfer, +// select its frame only after request-queue space is available. +TU_ATTR_ALWAYS_INLINE static inline uint16_t channel_send_in_token(dwc2_regs_t* dwc2, dwc2_channel_t* channel, + bool next_periodic_frame) { while (0 == req_queue_avail(dwc2, channel_is_periodic(channel->hcchar))) { // blocking wait for request queue available } - channel->hcchar |= HCCHAR_CHENA; - return true; + return channel_enable(dwc2, channel, next_periodic_frame); } // Find currently enabled channel. Note: EP0 is bidirectional @@ -612,20 +636,17 @@ static void channel_xfer_out_wrapup(dwc2_regs_t* dwc2, uint8_t ch_id) { edpt->buflen -= actual_bytes; } -static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { +static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id, bool new_periodic_xfer) { hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; dwc2_channel_char_t* hcchar_bm = &edpt->hcchar_bm; dwc2_channel_t* channel = &dwc2->channel[ch_id]; bool const is_period = channel_is_periodic(edpt->hcchar); - + uint16_t periodic_frame = 0; // clear previous state xfer->fifo_bytes = 0; // hchar: restore but don't enable yet - if (is_period) { - hcchar_bm->odd_frame = 1 - (dwc2->hfnum & 1); // transfer on next frame - } channel->hcchar = (edpt->hcchar & ~HCCHAR_CHENA); // hctsiz: zero length packet still count as 1 @@ -659,10 +680,10 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { channel->hcdma = (uint32_t) edpt->buffer; if (hcchar_bm->ep_dir == TUSB_DIR_IN) { - channel_send_in_token(dwc2, channel); + periodic_frame = channel_send_in_token(dwc2, channel, is_period); } else { hcd_dcache_clean(edpt->buffer, edpt->buflen); - channel->hcchar |= HCCHAR_CHENA; + periodic_frame = channel_enable(dwc2, channel, is_period); } } else { uint32_t hcintmsk = HCINT_NAK | HCINT_XACT_ERR | HCINT_STALL | HCINT_XFER_COMPLETE | HCINT_DATATOGGLE_ERR; @@ -686,9 +707,9 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { // IN Token. If we got NAK, we have to re-enable the channel again in the interrupt. Due to the way usbh stack only // call hcd_edpt_xfer() once, we will need to manage de-allocate/re-allocate IN channel dynamically. if (hcchar_bm->ep_dir == TUSB_DIR_IN) { - channel_send_in_token(dwc2, channel); + periodic_frame = channel_send_in_token(dwc2, channel, is_period); } else { - channel->hcchar |= HCCHAR_CHENA; + periodic_frame = channel_enable(dwc2, channel, is_period); if (edpt->buflen > 0) { // To prevent conflict with other channel, we will enable periodic/non-periodic FIFO empty interrupt accordingly // And write packet in the interrupt handler @@ -697,6 +718,10 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { } } + if (is_period && new_periodic_xfer) { + edpt->periodic_frame = periodic_frame; + } + return true; } @@ -707,11 +732,9 @@ static bool edpt_xfer_kickoff(dwc2_regs_t* dwc2, uint8_t ep_id) { hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; xfer->ep_id = ep_id; xfer->result = XFER_RESULT_INVALID; - - const bool result = channel_xfer_start(dwc2, ch_id); - if (result && channel_is_periodic(_hcd_data.edpt[ep_id].hcchar)) { - hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; - edpt->periodic_frame = (uint16_t) ((dwc2->hfnum + 1u) & HCD_FRAME_NUMBER_MASK); + hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; + const bool result = channel_xfer_start(dwc2, ch_id, true); + if (result && channel_is_periodic(edpt->hcchar)) { edpt->periodic_phase = 1; edpt->xfer_pending = 0; } @@ -858,7 +881,7 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci if (xfer->period_split_nyet_count < HCD_XFER_PERIOD_SPLIT_NYET_MAX) { hcchar.odd_frame = 1 - (dwc2->hfnum & 1); // transfer on next frame channel->hcchar = hcchar.value; - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); return; } else { // too many NYET, de-allocate channel with below code @@ -871,7 +894,7 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci // retry on next frame if bInterval is 1 hcchar.odd_frame = 1 - (dwc2->hfnum & 1); channel->hcchar = hcchar.value; - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } else { // otherwise, de-allocate channel, enable SOF set frame counter for later transfer const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; @@ -884,7 +907,7 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci } } else { // for control/bulk: retry immediately - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } } @@ -1059,7 +1082,7 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h channel->hcintmsk |= HCINT_NYET; hcsplt.split_compl = 1; channel->hcsplt = hcsplt.value; - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } else { // do nothing for complete split with DATA, this will trigger XferComplete and handled there } @@ -1070,7 +1093,7 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h // still more packet to receive, also reset to start split hcsplt.split_compl = 0; channel->hcsplt = hcsplt.value; - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } } } else if (hcint & HCINT_HALTED) { @@ -1157,7 +1180,7 @@ static bool handle_channel_out_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t is_done = true; } else { // Got here due to NAK or NYET - TU_ASSERT(channel_xfer_start(dwc2, ch_id)); + TU_ASSERT(channel_xfer_start(dwc2, ch_id, false)); } } else if (hcint & HCINT_ACK) { xfer->err_count = 0; @@ -1209,7 +1232,7 @@ static bool handle_channel_in_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci if (xfer->closing) { is_done = true; } else { - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } } else if (hcint & (HCINT_XFER_COMPLETE | HCINT_STALL | HCINT_BABBLE_ERR)) { const uint16_t remain_bytes = (uint16_t) hctsiz.xfer_size; @@ -1270,7 +1293,7 @@ static bool handle_channel_in_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci hcchar.odd_frame = 1 - (dwc2->hfnum & 1); // transfer on next frame channel->hcchar = hcchar.value; } - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } } else if (hcint & (HCINT_NAK | HCINT_DATATOGGLE_ERR)) { xfer->err_count = 0; @@ -1321,7 +1344,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc if (xfer->closing) { is_done = true; } else { - channel_xfer_start(dwc2, ch_id); + channel_xfer_start(dwc2, ch_id, false); } } else if (hcint & (HCINT_XFER_COMPLETE | HCINT_STALL)) { is_done = true; @@ -1339,7 +1362,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc xfer->err_count = 0; // clean up transfer so far and start again channel_xfer_out_wrapup(dwc2, ch_id); - channel_xfer_start(dwc2, ch_id); + channel_xfer_start(dwc2, ch_id, false); } else { xfer->err_count++; if (xfer->err_count >= HCD_XFER_ERROR_MAX) { @@ -1355,7 +1378,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc xfer->retry_disabled = 1; channel_disable(dwc2, channel); } else { - channel_xfer_start(dwc2, ch_id); + channel_xfer_start(dwc2, ch_id, false); } } } @@ -1365,7 +1388,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc xfer->result = XFER_RESULT_FAILED; is_done = true; } else { - channel_xfer_start(dwc2, ch_id); + channel_xfer_start(dwc2, ch_id, false); } } else if (hcint & HCINT_NYET) { if (hcsplt.split_en && hcsplt.split_compl) { @@ -1387,7 +1410,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc // Non-split OUT NAK is core-handled (5.1.2.2), so this is split-only. xfer->err_count = 0; channel_xfer_out_wrapup(dwc2, ch_id); - channel_xfer_start(dwc2, ch_id); + channel_xfer_start(dwc2, ch_id, false); } if (xfer->closing == 1) { -- cgit v1.3.1 From 4fb18bdc2bed03f7baa66bd4a550e5e6623d09f8 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Tue, 1 Sep 2026 04:04:01 +0200 Subject: fix(dwc2): queue initial slave OUT packet immediately In slave mode, channel_xfer_start() enabled an OUT channel but left every FIFO write to a later PTXFEMP interrupt. DWC2 creates the request-queue entry only when the packet's final FIFO word is written, so unrelated interrupt work could consume the selected service frame before the transfer was actually queued. Factor FIFO writes into a capacity-checked helper and write the initial packet while the channel-enable operation is still protected from DWC2 interrupts. Keep FIFO-empty interrupts only for data that does not fit immediately. The protected section never waits for FIFO or request-queue space. When initial periodic OUT submission is too close to the frame boundary, release the unused channel and defer the still-pending endpoint to the next SOF. Internal retries bypass this initial boundary guard. A hardware trace showed HCCHAR enabled for frame 0x0378 while the packet's final FIFO word was delayed until frame 0x03ae. The complete five-commit fix set passed 600 seconds in every O0/O2 and slave/DMA mode. Signed-off-by: HiFiPHile --- src/portable/synopsys/dwc2/hcd_dwc2.c | 174 +++++++++++++++++++++++----------- 1 file changed, 117 insertions(+), 57 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 31ea2ae20..5bb5a979b 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -26,6 +26,12 @@ #endif #define DWC2_CHANNEL_COUNT_MAX 16u // absolute max channel count + + // Conservative time budget for enabling a slave-mode periodic OUT channel and writing its first packet before the + // current (micro)frame ends. HFNUM.FrRem is measured in PHY clocks; 1024 clocks are 17.1 us at 60 MHz, 21.3 us at + // 48 MHz, or 34.1 us at 30 MHz. Defer to SOF when less time remains. + #define DWC2_PERIODIC_OUT_MIN_FRREM 1024u + TU_VERIFY_STATIC(CFG_TUH_DWC2_ENDPOINT_MAX <= 255, "currently only use 8-bit for index"); enum { @@ -636,12 +642,21 @@ static void channel_xfer_out_wrapup(dwc2_regs_t* dwc2, uint8_t ch_id) { edpt->buflen -= actual_bytes; } -static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id, bool new_periodic_xfer) { +#if CFG_TUH_DWC2_SLAVE_ENABLE +static bool channel_txfifo_write(dwc2_regs_t* dwc2, uint8_t ch_id, bool is_periodic); +#endif +static void periodic_xfer_defer(dwc2_regs_t* dwc2, hcd_endpoint_t* edpt, uint32_t uframe_countdown); + +static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id, bool defer_periodic_out) { hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; dwc2_channel_char_t* hcchar_bm = &edpt->hcchar_bm; dwc2_channel_t* channel = &dwc2->channel[ch_id]; bool const is_period = channel_is_periodic(edpt->hcchar); +#if CFG_TUH_DWC2_SLAVE_ENABLE + const uint8_t saved_pid = edpt->next_pid; + const uint8_t saved_do_ping = edpt->next_do_ping; +#endif uint16_t periodic_frame = 0; // clear previous state xfer->fifo_bytes = 0; @@ -685,8 +700,11 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id, bool new_period hcd_dcache_clean(edpt->buffer, edpt->buflen); periodic_frame = channel_enable(dwc2, channel, is_period); } - } else { - uint32_t hcintmsk = HCINT_NAK | HCINT_XACT_ERR | HCINT_STALL | HCINT_XFER_COMPLETE | HCINT_DATATOGGLE_ERR; + } +#if CFG_TUH_DWC2_SLAVE_ENABLE + else { + uint32_t hcintmsk = HCINT_NAK | HCINT_XACT_ERR | HCINT_STALL | + HCINT_XFER_COMPLETE | HCINT_DATATOGGLE_ERR; if (is_period) { hcintmsk |= HCINT_FARME_OVERRUN; } @@ -709,16 +727,32 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id, bool new_period if (hcchar_bm->ep_dir == TUSB_DIR_IN) { periodic_frame = channel_send_in_token(dwc2, channel, is_period); } else { + // The final FIFO word creates the OUT request. Keep CHENA and that write + // atomic with respect to this controller's ISR. + // This region never waits for FIFO or queue space. + const uint32_t gahbcfg = dwc2->gahbcfg; + dwc2->gahbcfg = gahbcfg & ~GAHBCFG_GINT; + if (defer_periodic_out && is_period) { + const dwc2_hfnum_t hfnum = {.value = dwc2->hfnum}; + if (hfnum.remainning < DWC2_PERIODIC_OUT_MIN_FRREM) { + edpt->next_pid = saved_pid; + edpt->next_do_ping = saved_do_ping; + dwc2->gahbcfg = gahbcfg; + return false; + } + } periodic_frame = channel_enable(dwc2, channel, is_period); - if (edpt->buflen > 0) { - // To prevent conflict with other channel, we will enable periodic/non-periodic FIFO empty interrupt accordingly - // And write packet in the interrupt handler + if (edpt->buflen > 0 && channel_txfifo_write(dwc2, ch_id, is_period)) { + // The FIFO-empty interrupt handles only work that did not fit in the + // initial synchronous write. dwc2->gintmsk |= (is_period ? GINTSTS_PTX_FIFO_EMPTY : GINTSTS_NPTX_FIFO_EMPTY); } + dwc2->gahbcfg = gahbcfg; } } +#endif - if (is_period && new_periodic_xfer) { + if (is_period && defer_periodic_out) { edpt->periodic_frame = periodic_frame; } @@ -734,7 +768,12 @@ static bool edpt_xfer_kickoff(dwc2_regs_t* dwc2, uint8_t ep_id) { xfer->result = XFER_RESULT_INVALID; hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; const bool result = channel_xfer_start(dwc2, ch_id, true); - if (result && channel_is_periodic(edpt->hcchar)) { + if (!result) { + channel_dealloc(dwc2, ch_id); + periodic_xfer_defer(dwc2, edpt, 0); + return true; + } + if (channel_is_periodic(_hcd_data.edpt[ep_id].hcchar)) { edpt->periodic_phase = 1; edpt->xfer_pending = 0; } @@ -789,9 +828,18 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * edpt->hcchar_bm.ep_dir = ep_dir; } - if (channel_is_periodic(edpt->hcchar) && edpt->periodic_phase) { + if (channel_is_periodic(edpt->hcchar)) { const uint32_t ucount = (hprt_speed_get(dwc2) == TUSB_SPEED_HIGH) ? 1u : 8u; - if (edpt->uframe_interval > ucount) { +#if CFG_TUH_DWC2_SLAVE_ENABLE + // Establish a slower slave-mode OUT schedule from SOF. bInterval=1 must be queued immediately to avoid + // losing every other service opportunity. + if (!dma_host_enabled(dwc2) && ep_dir == TUSB_DIR_OUT && !edpt->periodic_phase && + edpt->uframe_interval > ucount) { + periodic_xfer_defer(dwc2, edpt, 0); + return true; + } +#endif + if (edpt->periodic_phase && edpt->uframe_interval > ucount) { const uint32_t countdown = periodic_xfer_countdown(dwc2, edpt); if (countdown > 0) { periodic_xfer_defer(dwc2, edpt, countdown); @@ -971,38 +1019,50 @@ static void handle_rxflvl_irq(uint8_t rhport) { } } -// return true if there is still pending data and need more ISR +// Return true if data remains for a later FIFO-empty interrupt. +static bool channel_txfifo_write(dwc2_regs_t* dwc2, uint8_t ch_id, bool is_periodic) { + hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; + dwc2_channel_t* channel = &dwc2->channel[ch_id]; + const dwc2_channel_char_t hcchar = {.value = channel->hcchar}; + TU_ASSERT(xfer->ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); + hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; + const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; + const uint16_t remain_packets = hctsiz.packet_count; + + for (uint16_t i = 0; i < remain_packets; i++) { + const uint16_t remain_bytes = edpt->buflen - xfer->fifo_bytes; + const uint16_t xact_bytes = tu_min16(remain_bytes, hcchar.ep_size); + + // The packet's last FIFO word creates its request-queue entry. + // HNPTXSTS differs by one request-queue bit, which is outside these fields. + const dwc2_hptxsts_t txsts = {.value = (is_periodic ? dwc2->hptxsts : dwc2->hnptxsts)}; + if ((xact_bytes > (txsts.fifo_available << 2)) || (txsts.req_queue_available == 0)) { + return true; + } + + tu_hwfifo_write(dwc2->fifo[ch_id], edpt->buffer + xfer->fifo_bytes, xact_bytes, NULL); + xfer->fifo_bytes += xact_bytes; + } + + return false; +} + +// Return true if at least one matching channel needs another interrupt. static bool handle_txfifo_empty(dwc2_regs_t* dwc2, bool is_periodic) { const uint8_t max_channel = dwc2_channel_count(dwc2); for (uint8_t ch_id = 0; ch_id < max_channel; ch_id++) { + hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; dwc2_channel_t* channel = &dwc2->channel[ch_id]; const dwc2_channel_char_t hcchar = {.value = channel->hcchar}; - // skip writing to FIFO if channel is expecting halted. - if (0 == (channel->hcintmsk & HCINT_HALTED) && (hcchar.ep_dir == TUSB_DIR_OUT)) { - hcd_xfer_t *xfer = &_hcd_data.xfer[ch_id]; - TU_ASSERT(xfer->ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); - hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; - const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; - const uint16_t remain_packets = hctsiz.packet_count; - for (uint16_t i = 0; i < remain_packets; i++) { - const uint16_t remain_bytes = edpt->buflen - xfer->fifo_bytes; - const uint16_t xact_bytes = tu_min16(remain_bytes, hcchar.ep_size); - - // skip if there is not enough space in FIFO and RequestQueue. - // Packet's last word written to FIFO will trigger a request queue - // Use period txsts for both p/np to get request queue space available (1-bit difference, it is small enough) - const dwc2_hptxsts_t txsts = {.value = (is_periodic ? dwc2->hptxsts : dwc2->hnptxsts)}; - if ((xact_bytes > (txsts.fifo_available << 2)) || (txsts.req_queue_available == 0)) { - return true; - } - - tu_hwfifo_write(dwc2->fifo[ch_id], edpt->buffer + xfer->fifo_bytes, xact_bytes, NULL); - xfer->fifo_bytes += xact_bytes; + if (xfer->allocated && channel_is_periodic(hcchar.value) == is_periodic && + 0 == (channel->hcintmsk & HCINT_HALTED) && hcchar.ep_dir == TUSB_DIR_OUT) { + if (channel_txfifo_write(dwc2, ch_id, is_periodic)) { + return true; } } } - return false; // no channel has pending data + return false; } static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hcint) { @@ -1358,30 +1418,30 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc } channel->hcintmsk &= ~HCINT_ACK; } else if (hcint & HCINT_XACT_ERR) { - if (hcint & (HCINT_NAK | HCINT_NYET | HCINT_ACK)) { - xfer->err_count = 0; - // clean up transfer so far and start again - channel_xfer_out_wrapup(dwc2, ch_id); - channel_xfer_start(dwc2, ch_id, false); - } else { - xfer->err_count++; - if (xfer->err_count >= HCD_XFER_ERROR_MAX) { - xfer->result = XFER_RESULT_FAILED; - is_done = true; - } else { - // Rewind, then retry the start-split. Non-periodic SPLIT throttles via channel_disable + re-arm on - // the halt (immediate re-fire exhausts the retry budget; the disable gives the hub TT a recovery - // gap, like slave). Periodic split is excluded: channel_disable() is a no-op for it, so the halt - // never fires and the channel would wedge. Non-split re-inits immediately (Programming Guide 5.1.2.3). - channel_xfer_out_wrapup(dwc2, ch_id); - if (hcsplt.split_en && !channel_is_periodic(channel->hcchar)) { - xfer->retry_disabled = 1; - channel_disable(dwc2, channel); - } else { - channel_xfer_start(dwc2, ch_id, false); - } - } - } + if (hcint & (HCINT_NAK | HCINT_NYET | HCINT_ACK)) { + xfer->err_count = 0; + // clean up transfer so far and start again + channel_xfer_out_wrapup(dwc2, ch_id); + channel_xfer_start(dwc2, ch_id, false); + } else { + xfer->err_count++; + if (xfer->err_count >= HCD_XFER_ERROR_MAX) { + xfer->result = XFER_RESULT_FAILED; + is_done = true; + } else { + // Rewind, then retry the start-split. Non-periodic SPLIT throttles via channel_disable + re-arm on + // the halt (immediate re-fire exhausts the retry budget; the disable gives the hub TT a recovery + // gap, like slave). Periodic split is excluded: channel_disable() is a no-op for it, so the halt + // never fires and the channel would wedge. Non-split re-inits immediately (Programming Guide 5.1.2.3). + channel_xfer_out_wrapup(dwc2, ch_id); + if (hcsplt.split_en && !channel_is_periodic(channel->hcchar)) { + xfer->retry_disabled = 1; + channel_disable(dwc2, channel); + } else { + channel_xfer_start(dwc2, ch_id, false); + } + } + } } else if (hcint & HCINT_FARME_OVERRUN) { channel_xfer_out_wrapup(dwc2, ch_id); if (edpt->hcchar_bm.ep_type == HCCHAR_EPTYPE_ISOCHRONOUS) { -- cgit v1.3.1 From 3a1dc0dc1f57d45ed75aa9e176425901270e982e Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Tue, 1 Sep 2026 07:06:47 +0200 Subject: DWC2 host: clean up channels on disconnect A root-port disconnect invalidates every active transfer. Retire channel and FIFO interrupt sources plus host-channel state in the disconnect ISR using the Linux DWC2 cleanup model instead of reinitializing the core and PHY, which can sleep on STM32 HS PHYs. Flush posted slave requests, request halts for enabled channels, clear channel interrupt and software ownership, and keep endpoint records closing until USBH processes the remove event. Reject transfer submissions to closing endpoints, preserve fast-replug notification, and re-enable the global host-channel interrupt when a new channel is initialized. --- src/portable/synopsys/dwc2/hcd_dwc2.c | 98 +++++++++++++++++++++++++++++------ 1 file changed, 81 insertions(+), 17 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 5bb5a979b..b6e8ecb03 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -210,6 +210,55 @@ TU_ATTR_ALWAYS_INLINE static inline bool channel_disable(const dwc2_regs_t* dwc2 return true; } +// Retire all active host channels on root-port disconnect without waiting for +// Channel Halted interrupts. +// stop new channel/FIFO interrupts, flush queued slave requests, request a +// halt for enabled channels, then clear their interrupt and software state. +static void channel_cleanup_on_disconnect(dwc2_regs_t *dwc2) { + const uint32_t xfer_ints = GINTSTS_NPTX_FIFO_EMPTY | GINTSTS_PTX_FIFO_EMPTY | GINTSTS_HCINT; + dwc2->gintmsk &= ~xfer_ints; + dwc2->gintsts = xfer_ints; + dwc2->haintmsk = 0; + + const uint8_t max_channel = dwc2_channel_count(dwc2); + #if CFG_TUH_DWC2_SLAVE_ENABLE + if (!dma_host_enabled(dwc2)) { + // With CHENA clear, CHDIS flushes a posted request without consuming + // request-queue space. Clear EPDIR as required for this flush operation. + for (uint8_t ch_id = 0; ch_id < max_channel; ch_id++) { + if (_hcd_data.xfer[ch_id].allocated) { + dwc2_channel_t *channel = &dwc2->channel[ch_id]; + const uint32_t hcchar = channel->hcchar; + if (hcchar & HCCHAR_CHENA) { + channel->hcchar = (hcchar & ~(HCCHAR_CHENA | HCCHAR_EPDIR)) | HCCHAR_CHDIS; + } + } + } + } + #endif + + for (uint8_t ch_id = 0; ch_id < max_channel; ch_id++) { + if (_hcd_data.xfer[ch_id].allocated) { + dwc2_channel_t *channel = &dwc2->channel[ch_id]; + const uint32_t hcchar = channel->hcchar; + if (hcchar & HCCHAR_CHENA) { + channel->hcchar = hcchar | HCCHAR_CHDIS; + } + channel->hcintmsk = 0; + channel->hcint = 0xFFFFFFFFU; + } + } + + tu_memclr(_hcd_data.xfer, sizeof(_hcd_data.xfer)); + for (uint8_t ep_id = 0; ep_id < CFG_TUH_DWC2_ENDPOINT_MAX; ep_id++) { + hcd_endpoint_t *edpt = &_hcd_data.edpt[ep_id]; + if (edpt->hcchar_bm.enable) { + edpt->closing = 1; + edpt->xfer_pending = 0; + } + } +} + // Enable a channel, selecting the following frame for a new periodic transfer. // Return that frame from the same HFNUM sample used for ODDFRM selection. // Clear CHDIS explicitly: a halted channel may retain it in HCCHAR. @@ -296,11 +345,13 @@ static void edpt_close(dwc2_regs_t *dwc2, uint8_t ep_id) { // Find an endpoint that is opened previously with hcd_edpt_open() // Note: EP0 is bidirectional -TU_ATTR_ALWAYS_INLINE static inline uint8_t edpt_find_opened(uint8_t dev_addr, uint8_t ep_num, uint8_t ep_dir) { +TU_ATTR_ALWAYS_INLINE static inline uint8_t edpt_find_opened(uint8_t dev_addr, uint8_t ep_num, uint8_t ep_dir, + bool include_closing) { for (uint8_t i = 0; i < (uint8_t)CFG_TUH_DWC2_ENDPOINT_MAX; i++) { const hcd_endpoint_t *edpt = &_hcd_data.edpt[i]; const dwc2_channel_char_t hcchar_bm = edpt->hcchar_bm; - if (hcchar_bm.enable && hcchar_bm.dev_addr == dev_addr && hcchar_bm.ep_num == ep_num && + if (hcchar_bm.enable && (include_closing || !edpt->closing) && hcchar_bm.dev_addr == dev_addr && + hcchar_bm.ep_num == ep_num && (ep_num == 0 || hcchar_bm.ep_dir == ep_dir)) { return i; } @@ -606,7 +657,7 @@ bool hcd_edpt_close(uint8_t rhport, uint8_t daddr, uint8_t ep_addr) { dwc2_regs_t *dwc2 = DWC2_REG(rhport); const uint8_t ep_num = tu_edpt_number(ep_addr); const uint8_t ep_dir = tu_edpt_dir(ep_addr); - const uint8_t ep_id = edpt_find_opened(daddr, ep_num, ep_dir); + const uint8_t ep_id = edpt_find_opened(daddr, ep_num, ep_dir, true); TU_ASSERT(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); edpt_close(dwc2, ep_id); @@ -687,6 +738,7 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id, bool defer_peri channel->hcsplt = edpt->hcsplt; channel->hcint = 0xFFFFFFFFU; // clear all channel interrupts + dwc2->gintmsk |= GINTSTS_HCINT; if (dma_host_enabled(dwc2)) { channel->hcintmsk = HCINT_HALTED; @@ -815,8 +867,8 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * const uint8_t ep_num = tu_edpt_number(ep_addr); const uint8_t ep_dir = tu_edpt_dir(ep_addr); - uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir); - TU_ASSERT(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); + uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir, false); + TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); hcd_endpoint_t *edpt = &_hcd_data.edpt[ep_id]; TU_VERIFY(edpt->closing == 0); // skip if endpoint is closing @@ -857,7 +909,7 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { dwc2_regs_t* dwc2 = DWC2_REG(rhport); const uint8_t ep_num = tu_edpt_number(ep_addr); const uint8_t ep_dir = tu_edpt_dir(ep_addr); - const uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir); + const uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir, false); TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; @@ -887,8 +939,8 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { // Submit a special transfer to send 8-byte Setup Packet, when complete hcd_event_xfer_complete() must be invoked bool hcd_setup_send(uint8_t rhport, uint8_t dev_addr, const uint8_t setup_packet[8]) { - uint8_t ep_id = edpt_find_opened(dev_addr, 0, TUSB_DIR_OUT); - TU_ASSERT(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); // no opened endpoint + uint8_t ep_id = edpt_find_opened(dev_addr, 0, TUSB_DIR_OUT, false); + TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); // endpoint can close asynchronously on disconnect hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; edpt->next_pid = HCTSIZ_PID_SETUP; @@ -900,7 +952,7 @@ bool hcd_edpt_clear_stall(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { (void) rhport; const uint8_t ep_num = tu_edpt_number(ep_addr); const uint8_t ep_dir = tu_edpt_dir(ep_addr); - const uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir); + const uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir, false); TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; @@ -990,6 +1042,13 @@ static void handle_rxflvl_irq(uint8_t rhport) { // In packet received, pop this entry --> ACK interrupt const uint16_t byte_count = grxstsp.byte_count; hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; + if (!xfer->allocated) { + // Discard data for a channel retired by disconnect. + for (uint16_t count = 0; count < byte_count; count += sizeof(uint32_t)) { + (void) dwc2->fifo[0][0]; + } + break; + } TU_ASSERT(xfer->ep_id < CFG_TUH_DWC2_ENDPOINT_MAX,); hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; @@ -1684,6 +1743,19 @@ void hcd_int_handler(uint8_t rhport, bool in_isr) { } } + if (gintsts & GINTSTS_DISCINT) { + dwc2->gintsts = GINTSTS_DISCINT; + channel_cleanup_on_disconnect(dwc2); + hcd_event_device_remove(rhport, in_isr); + + // A fast replug can be visible without a pending connect-detect interrupt. + const uint32_t hprt = dwc2->hprt; + if (!(hprt & HPRT_CONN_DETECT) && (hprt & HPRT_CONN_STATUS)) { + hcd_event_device_attach(rhport, in_isr); + } + return; + } + if (gintsts & GINTSTS_HPRTINT) { // Host port interrupt: source is cleared in HPRT register // TU_LOG1_HEX(dwc2->hprt); @@ -1728,14 +1800,6 @@ void hcd_int_handler(uint8_t rhport, bool in_isr) { handle_channel_irq(rhport, in_isr); } - if (gintsts & GINTSTS_DISCINT) { - // Device disconnected - dwc2->gintsts = GINTSTS_DISCINT; - - if (0 == (dwc2->hprt & HPRT_CONN_STATUS)) { - hcd_event_device_remove(rhport, in_isr); - } - } } #endif -- cgit v1.3.1 From 14060950b487f931d959699c42b0875c96ca6a01 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Wed, 2 Sep 2026 03:54:17 +0200 Subject: fix(dwc2): defer periodic DMA abort cleanup until halt Periodic DMA channels use their natural service-boundary halt instead of a software CHDIS request. Keep the endpoint busy after an abort so a replacement transfer cannot reuse its state or buffer while the channel remains active. When HCINT.HALTED arrives, release the channel without reporting completion for the aborted transfer. If endpoint closure is also pending, release the endpoint from the same halt path. --- src/portable/synopsys/dwc2/hcd_dwc2.c | 42 ++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 67fdd0270..2055b320b 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -68,9 +68,10 @@ typedef struct { uint32_t next_pid : 2; // PID for next transfer uint32_t next_do_ping : 1; // Do PING for next transfer if possible (highspeed OUT) uint32_t closing : 1; // endpoint is closing + uint32_t aborting : 1; // periodic DMA channel is waiting for its automatic halt uint32_t periodic_phase : 1; // periodic transfer phase is established uint32_t xfer_pending : 1; // periodic transfer waiting for its service interval - // uint32_t : 5; + // uint32_t : 4; }; uint32_t uframe_countdown; // micro-frame count down to transfer for periodic, only need 19-bit @@ -96,6 +97,7 @@ typedef struct { // be composed of multiple channel_xfer_start() (retry with NAK/NYET) uint16_t fifo_bytes; // bytes written/read from/to FIFO (may not be transferred on USB bus). uint8_t retry_disabled; // 1: channel was disabled to throttle a split retry (NAK in / XactErr out); re-arm on its halt + volatile bool aborting; // periodic DMA abort waiting for the channel's automatic halt } hcd_xfer_t; typedef struct { @@ -870,7 +872,7 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir, false); TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); hcd_endpoint_t *edpt = &_hcd_data.edpt[ep_id]; - TU_VERIFY(edpt->closing == 0); // skip if endpoint is closing + TU_VERIFY(edpt->closing == 0 && edpt->aborting == 0); // skip if endpoint is closing or aborting edpt->buffer = buffer; edpt->buflen = buflen; @@ -920,12 +922,27 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { edpt->xfer_pending = 0; edpt->uframe_countdown = 0; } - hcd_int_enable(rhport); if (xfer_pending) { + hcd_int_enable(rhport); return true; } + // A periodic DMA channel must halt naturally at the next service boundary. Prevent a replacement transfer until the + // halt ISR retires the channel, and suppress completion for the aborted transfer. + if (dma_host_enabled(dwc2) && channel_is_periodic(edpt->hcchar)) { + const uint8_t ch_id = channel_find_enabled(dwc2, dev_addr, ep_num, ep_dir); + if (ch_id < 16) { + hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; + edpt->aborting = 1; + xfer->aborting = true; + hcd_int_enable(rhport); + return true; + } + } + + hcd_int_enable(rhport); + // Channel disable may wait for request-queue space in slave mode. // Find enabled channeled and disable it, channel will be de-allocated in the interrupt handler const uint8_t ch_id = channel_find_enabled(dwc2, dev_addr, ep_num, ep_dir); @@ -1566,6 +1583,25 @@ static void handle_channel_irq(uint8_t rhport, bool in_isr) { const uint32_t hcint_clear = (!is_dma && (hcint & ~HCINT_HALTED)) ? (hcint & ~HCINT_HALTED) : hcint; channel->hcint = hcint_clear; + if (is_dma && xfer->aborting && (hcint & HCINT_HALTED)) { + hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; + const bool closing = xfer->closing; + // channel_xfer_start() predicts the PID after all requested packets; + // an aborted transfer may have completed fewer. + if (hcchar.ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { + const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; + edpt->next_pid = hctsiz.pid; + } + xfer->aborting = false; + channel_dealloc(dwc2, ch_id); + if (closing) { + edpt_dealloc(edpt); + } else { + edpt->aborting = 0; + } + continue; + } + bool is_done = false; if (is_dma) { #if CFG_TUH_DWC2_DMA_ENABLE -- cgit v1.3.1 From e461c6f9c139a0012ef01d53db1aab1c68789b56 Mon Sep 17 00:00:00 2001 From: HiFiPHile Date: Wed, 2 Sep 2026 16:18:44 +0200 Subject: fix(dwc2): bound periodic intervals to HFNUM range HFNUM retains only 16384 host-frame positions, while valid periodic endpoint intervals can be longer. Resubmission after the counter wraps can therefore alias the elapsed time and skip the next established service phase. Cap the host-selected interval to one HFNUM cycle using the root-port frame unit. USB permits a shorter host-provided period, and the bounded interval keeps phase calculation unambiguous for native and split endpoints. --- src/portable/synopsys/dwc2/hcd_dwc2.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'src/portable') diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 2055b320b..5a171f80e 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -44,7 +44,8 @@ enum { enum { HCD_XFER_PERIOD_SPLIT_NYET_MAX = 3, - HCD_FRAME_NUMBER_MASK = 0x3fff + HCD_FRAME_NUMBER_MASK = 0x3fff, + HCD_FRAME_COUNT = HCD_FRAME_NUMBER_MASK + 1 }; //-------------------------------------------------------------------- @@ -652,6 +653,13 @@ bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, const tusb_desc_endpoint_t* break; } + if (channel_is_periodic(edpt->hcchar)) { + // HFNUM cannot distinguish elapsed periods longer than one counter cycle. USB permits the host to provide a + // shorter period, so bound the selected period to the history available from HFNUM. + const uint32_t ucount = (rh_speed == TUSB_SPEED_HIGH) ? 1u : 8u; + edpt->uframe_interval = tu_min32(edpt->uframe_interval, HCD_FRAME_COUNT * ucount); + } + return true; } -- cgit v1.3.1