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 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 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