From 89305b2f8bdb1d7819cb91dd6781bf6e9c3afe79 Mon Sep 17 00:00:00 2001 From: iceman1001 Date: Tue, 15 Sep 2026 18:12:34 +0200 Subject: [PATCH] usb: bound the wait for the host to collect a STALL AT91F_USB_SendStall spun on STALLSENT with no exit. That bit only arrives once the host polls the endpoint and takes the handshake, so a host that vanishes in between wedges the device. It was the last unbounded wait in the file without an escape. Exit on RXSETUP (host abandoned the request and sent a new SETUP), on ENDBUSRES, or on a spin count, reusing the 0x1FFF that usb_read already uses. usb_check() is not usable here: it calls back into AT91F_CDC_Enumerate, which is what calls this. Do not test RXSUSP. Nothing writes it back to UDP_ICR, so it latches on the first bus idle and stays set, which makes the guard fire on every stall and stops the STALL from ever being delivered. The second wait is bounded too, since leaving the first one early can let the host set STALLSENT after the clear. Also ISOERROR -> STALLSENT. Same bit 3, but only one of those names is true on a control endpoint. Co-Authored-By: Claude Opus 5 (1M context) --- common_arm/usb/usb_cdc_at91.c | 37 ++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/common_arm/usb/usb_cdc_at91.c b/common_arm/usb/usb_cdc_at91.c index e79cb310a..649b9e9cd 100644 --- a/common_arm/usb/usb_cdc_at91.c +++ b/common_arm/usb/usb_cdc_at91.c @@ -704,10 +704,41 @@ void AT91F_USB_SendZlp(AT91PS_UDP pudp) { //* \brief Stall the control endpoint //*---------------------------------------------------------------------------- void AT91F_USB_SendStall(AT91PS_UDP pudp) { + UDP_SET_EP_FLAGS(AT91C_EP_CONTROL, AT91C_UDP_FORCESTALL); - while (!(pudp->UDP_CSR[AT91C_EP_CONTROL] & AT91C_UDP_ISOERROR)) {}; - UDP_CLEAR_EP_FLAGS(AT91C_EP_CONTROL, (AT91C_UDP_FORCESTALL | AT91C_UDP_ISOERROR)); - while (pudp->UDP_CSR[AT91C_EP_CONTROL] & (AT91C_UDP_FORCESTALL | AT91C_UDP_ISOERROR)) {}; + + // STALLSENT only arrives once the host polls the endpoint and takes the + // handshake, so every way the host can fail to do that needs an exit. + // usb_check() is not one of them, it calls back into AT91F_CDC_Enumerate(). + uint16_t time_out = 0; + while (!(pudp->UDP_CSR[AT91C_EP_CONTROL] & AT91C_UDP_STALLSENT)) { + + // host dropped this request and started another one + if (pudp->UDP_CSR[AT91C_EP_CONTROL] & AT91C_UDP_RXSETUP) { + break; + } + + // bus reset. Not RXSUSP: nothing in this driver writes it back to + // UDP_ICR, so once the bus first idles it stays latched for good. + if (pudp->UDP_ISR & AT91C_UDP_ENDBUSRES) { + break; + } + + if (time_out++ == 0x1FFF) { + break; + } + } + + UDP_CLEAR_EP_FLAGS(AT91C_EP_CONTROL, (AT91C_UDP_FORCESTALL | AT91C_UDP_STALLSENT)); + + // leaving the loop above early can let the host set STALLSENT after the + // clear, so this wait is bounded too + time_out = 0; + while (pudp->UDP_CSR[AT91C_EP_CONTROL] & (AT91C_UDP_FORCESTALL | AT91C_UDP_STALLSENT)) { + if (time_out++ == 0x1FFF) { + break; + } + } } //*----------------------------------------------------------------------------