From 3a2dc1a34460847ca0e2423ad3a605d7144fa290 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Thu, 8 Dec 2022 22:32:50 -0600 Subject: [PATCH] WIP timeouts --- cores/arduino/USBCore.cpp | 124 ++++++++++++++++++++++++++++++++++++-- cores/arduino/USBCore.h | 6 ++ 2 files changed, 124 insertions(+), 6 deletions(-) diff --git a/cores/arduino/USBCore.cpp b/cores/arduino/USBCore.cpp index ee1d0343..edf5f061 100644 --- a/cores/arduino/USBCore.cpp +++ b/cores/arduino/USBCore.cpp @@ -133,6 +133,14 @@ void EPBuffer::init(uint8_t ep) this->reset(); this->rxWaiting = false; this->txWaiting = false; + this->timedOut = false; + this->timeout = 100; +} + +template +void EPBuffer::setTimeout(uint16_t timeout) +{ + this->timeout = timeout; } template @@ -236,6 +244,9 @@ void EPBuffer::flush() // fall through case USBD_CONFIGURED: case USBD_SUSPENDED: { + if (this->timedOut) { + break; + } // This will temporarily reenable and disable interrupts auto canWrite = this->waitForWriteComplete(); if (canWrite) { @@ -246,6 +257,7 @@ void EPBuffer::flush() // Only start the next transmission if the device hasn't been // reset. this->txWaiting = true; + this->startTime = millis(); usbd_ep_send(&USBCore().usbDev(), this->ep, (uint8_t *)this->buf, this->len()); USBCore().logEP('>', this->ep, '>', this->len()); } @@ -287,6 +299,7 @@ template void EPBuffer::transcIn() { this->txWaiting = false; + this->timedOut = false; } // Unused? @@ -302,11 +315,21 @@ uint8_t* EPBuffer::ptr() template bool EPBuffer::waitForWriteComplete() { - // auto start = getCurrentMillis(); auto ok = true; + auto desc = *EPBuffers().desc(this->ep); + auto udev = &USBCore().usbDev(); + + /* + * For interrupt EPs, count from now instead of from previous send. + * Otherwise, if a send occurs while the bus is suspended, the packet will + * be cheated of its full timeout. + */ + if (desc.type() == USB_EP_ATTR_INT) { + this->startTime = millis(); + } do { usb_enable_interrupts(); - switch (USBCore().usbDev().cur_status) { + switch (udev->cur_status) { case USBD_DEFAULT: case USBD_ADDRESSED: ok = false; @@ -314,11 +337,95 @@ bool EPBuffer::waitForWriteComplete() default: break; } - // if (getCurrentMillis() - start > 5) { - // EPBuffers().buf(ep).transcIn(); - // ok = false; - // } usb_disable_interrupts(); + if (this->txWaiting && millis() - this->startTime > this->timeout) { + USBCore().logEP('X', this->ep, '>', this->len()); + /* + * For interrupt EPs, overwrite the previous packet. In the common + * case of a key press HID report initiating a remote wakeup, + * transmission of the key release event might be delayed, because + * the host can't poll while the bus is suspended. If the + * application sends a key release event before the key press event + * is transmitted, that might time out. If the key release event is + * discarded due to the timeout, the host will never receive the + * key release event, leading to a stuck or repeating key. + */ + if (desc.type() == USB_EP_ATTR_INT) { + /* + * Set the endpoint status to NAK, in an attempt to prevent + * the queued packet from being transmitted while being + * overwritten. + * + * This must be done in two steps. Unfortunately, the USBD + * peripheral's design doesn't seem to make it possible to + * atomically abort a queued transmission. Even if interrupts + * are disabled, the peripheral state machine continues to run. + * + * The endpoint status control bits are toggle-only (toggle on + * writing 1; unchanged on writing 0). If the TX state + * transitions from VALID to NAK during the read-modify-write, + * a toggle operation (XOR 0b01) intended to transition from + * VALID (0b11) to NAK (0b10) will set the status back to + * VALID. This will cause the previous packet to be sent again. + * + * Instead, set the status to DISABLED (0b00, via XOR 0b11), + * which will become STALL (0b01) if the peripheral transitions + * the status to NAK between the read and the write. This can + * happen if the transmission was in progress and the host ACKs + * exactly between the read and the write to update the status + * bits. + * + * Possible states after the first USBD_EP_TX_STAT_SET: + * + * - DISABLED (metastable if TX in progress) + * - STALL (stable; only possible if we lost the r-m-w race) + * - NAK (stable; only possible if TX completed after toggle) + * + * There is a small theoretical chance that the host will poll + * the endpoint again between the two updates to the status + * bits, which could lead to the host receiving a STALL + * handshake, which would put the data toggle bit out of sync. + * + * The second host poll cannot complete sooner than 34 bit times + * after the ACK from the first host poll. (2 bits inter-packet + * delay, 8 bits sync, 8 bits PID, 16 bits addr/endpoint/CRC, + * about 2.83 microseconds) The second status bit update will + * complete before then, assuming reasonable CPU clock speeds + * and no lengthy interrupt handler executions occurring. + * + * On the other hand, it's unknown when exactly the peripheral + * checks the endpoint status bits to decide which response to + * send to an IN poll. It might latch them at EOP, or SOP, or + * maybe PID, or it might wait until after checking the CRC. So + * the actual available time might be between 0 and 34 bit + * times. + * + * We could globally disable interrupts to mitigate that, if + * necessary. + */ + USBD_EP_TX_STAT_SET(this->ep, EPTX_DISABLED); + if (USBD_EPxCS(this->ep) & EPxCS_TX_STA == EPTX_DISABLED) { + /* + * If the endpoint is disabled, there might still be a + * transmission in progress. Wait for it to complete: + * + * 1 byte sync + 1 byte PID + 64 bytes data + 2 bytes CRC, + * 1 turn-around time, + * 1 byte sync + 1 byte PID (handshake) + * assuming maximum bit stuffing at full speed (12MHz). + */ + delayMicroseconds(56); + } + // In case TX was in progress and completed + USBD_EP_TX_ST_CLEAR(this->ep); + USBD_EP_TX_STAT_SET(this->ep, EPTX_NAK); + break; + } else { + ok = false; + this->timedOut = true; + break; + } + } } while (ok && this->txWaiting); return ok; } @@ -874,6 +981,11 @@ int USBCore_::flush(uint8_t ep) return 0; } +void USBCore_::setTimeout(uint8_t ep, uint16_t timeout) +{ + EPBuffers().buf(ep).setTimeout(timeout); +} + void USBCore_::transcSetupHelper(usb_dev* usbd, uint8_t ep) { USBCore_* core = (USBCore_*)usbd->user_data; diff --git a/cores/arduino/USBCore.h b/cores/arduino/USBCore.h index b24df391..280bf409 100644 --- a/cores/arduino/USBCore.h +++ b/cores/arduino/USBCore.h @@ -95,6 +95,7 @@ class EPBuffer void flush(); uint8_t* ptr(); void enableOutEndpoint(); + void setTimeout(uint16_t timeout); void transcIn(); void transcOut(); @@ -131,6 +132,10 @@ class EPBuffer */ volatile bool currentlyFlushing = false; + volatile uint32_t startTime; + uint16_t timeout; + volatile bool timedOut; + uint8_t ep; }; @@ -174,6 +179,7 @@ class USBCore_ int recv(uint8_t ep); int flush(uint8_t ep); void setResetHook(void (*hook)()); + void setTimeout(uint8_t ep, uint16_t timeout); // Debug counters volatile uint16_t nreset;