From 75ca72f665be9c0c1948f78e45d2cbe0e02458be Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Fri, 25 Nov 2022 15:58:15 -0600 Subject: [PATCH] remove (almost all of) transcSetup --- cores/arduino/USBCore.cpp | 242 ++++---------------------------------- cores/arduino/USBCore.h | 11 +- 2 files changed, 29 insertions(+), 224 deletions(-) diff --git a/cores/arduino/USBCore.cpp b/cores/arduino/USBCore.cpp index 07880a23..a45b6bdd 100644 --- a/cores/arduino/USBCore.cpp +++ b/cores/arduino/USBCore.cpp @@ -120,7 +120,7 @@ static uint8_t* stringDescs[] = { usb_desc desc = { .dev_desc = (uint8_t *)&devDesc, - .config_desc = (uint8_t *)&configDesc, + .config_desc = nullptr, .bos_desc = nullptr, .strings = stringDescs }; @@ -304,29 +304,6 @@ uint8_t* EPBuffer::ptr() return this->buf; } -// Busy loop until an OUT packet has been received. Returns ‘false’ if -// the device has been reset. -// Must be called via ISR -template -bool EPBuffer::waitForReadComplete() -{ - Serial1.print(F("RW")); - Serial1.println(this->ep); - Serial1.flush(); - // auto start = getCurrentMillis(); - auto ok = true; - while (ok && this->rxWaiting) { - ok = EPBuffers().pollEPStatus(); - // if (getCurrentMillis() - start > 5) { - // EPBuffers().buf(ep).transcIn(); - // return false; - // } - } - Serial1.println(ok); - Serial1.flush(); - return ok; -} - // Busy loop until the latest IN packet has been sent. Returns ‘true’ // if a new packet can be queued when this call completes. // Must run with interrupts disabled, which will be temporarily reenabled @@ -337,16 +314,11 @@ bool EPBuffer::waitForWriteComplete() auto ok = true; do { usb_enable_interrupts(); - ok = EPBuffers().pollEPStatus(); switch (USBCore().usbDev().cur_status) { case USBD_DEFAULT: case USBD_ADDRESSED: - // For non-EP0, abort if no longer configured - if (ep != 0) { - ok = false; - break; - } - // fall through + ok = false; + break; default: break; } @@ -387,90 +359,6 @@ EPDesc* EPBuffers_::desc(uint8_t ep) return &descs[ep]; } -// Check if any endpoints have a received data or finished sending -// data, updating their ‘waiting’ flags as a side-effect. -// -// Returns ‘false’ if the host has reset the device. -template -bool EPBuffers_::pollEPStatus() -{ - /* - * I’m not sure how much of this is necessary, but this is the - * series of checks that’s used by ‘usbd_isr’ to verify the IN - * packet has been sent. - */ - - uint16_t int_status = (uint16_t)USBD_INTF; - uint8_t ep_num = int_status & INTF_EPNUM; - /* - * If we are in interrupt context, we need to check for a - * device reset and terminate early so we don't spin forever - * waiting to complete a packet the host is no longer paying - * attention to. - * - * We /do not/ clear the flag, allowing the ISR to fire as - * soon as we've left this call, which will call into the - * normal reset routine. - */ - auto ok = true; - if ((int_status & INTF_RSTIF) == INTF_RSTIF) { - // Indicate the device was reset to callers. - ok = false; - } else if ((int_status & INTF_STIF) == INTF_STIF) { - if ((int_status & INTF_DIR) == INTF_DIR - && (USBD_EPxCS(ep_num) & EPxCS_RX_ST) == EPxCS_RX_ST) { - - if (USBD_EPxCS(ep_num) & EPxCS_SETUP) { - /* - * We should abort a control transfer if we receive an - * unexpected SETUP token, but unwinding all of that deeply - * nested control flow is too error-prone. - * - * Also, we'd have to check whether the current flush is - * part of a control transfer, or a non-control transfer, - * so we can decide whether to abort the flush. - */ - Serial1.println(F("SURPRISE SETUP")); - Serial1.flush(); - } - usb_transc *transc = &USBCore().usbDev().transc_out[ep_num]; - auto count = 0; - if (transc->xfer_buf) { - count = USBCore().usbDev().drv_handler->ep_read(transc->xfer_buf, ep_num, (uint8_t)EP_BUF_SNG); - transc->xfer_buf += count; - transc->xfer_count += count; - } - if ((transc->xfer_count >= transc->xfer_len) - || (count < transc->max_len)) { - /* - * This bypasses the low-level Setup Data OUT stage - * completion handlers, because we might need to read more - * than one packet. - */ - EPBuffers().buf(ep_num).transcOut(); - } else { - /* - * Low-level firmware does the following, which we won't - * do, because we only ever configure the transc to read a - * single packet at a time. - */ - // udev->drv_handler->ep_rx_enable(udev, ep_num); - } - USBD_EP_RX_ST_CLEAR(ep_num); - } else if ((int_status & INTF_DIR) == 0 - && (USBD_EPxCS(ep_num) & EPxCS_TX_ST) == EPxCS_TX_ST) { - /* - * This bypasses low-level Setup Data IN stage completion - * handlers, because we might need to write more than one - * packet. - */ - EPBuffers().buf(ep_num).transcIn(); - USBD_EP_TX_ST_CLEAR(ep_num); - } - } - return ok; -} - EPBuffers_& EPBuffers() { static EPBuffers_ obj; @@ -558,6 +446,7 @@ class ClassCore // TODO: remove this copy. arduino::USBSetup setup; memcpy(&setup, req, sizeof(setup)); + USBCore().setupClass(req->wLength); if (setup.bRequest == USB_GET_DESCRIPTOR) { auto sent = PluggableUSB().getDescriptor(setup); if (sent > 0) { @@ -671,16 +560,11 @@ USBCore_::USBCore_() this->oldTranscSetup = usbd.ep_transc[0][TRANSC_SETUP]; usbd.ep_transc[0][TRANSC_SETUP] = USBCore_::transcSetupHelper; - - this->oldTranscOut = usbd.ep_transc[0][TRANSC_OUT]; - usbd.ep_transc[0][TRANSC_OUT] = USBCore_::transcOutHelper; - - this->oldTranscIn = usbd.ep_transc[0][TRANSC_IN]; - usbd.ep_transc[0][TRANSC_IN] = USBCore_::transcInHelper; } void USBCore_::connect() { + USBCore().buildDeviceConfigDescriptor(); usb_connect(); } @@ -689,6 +573,14 @@ void USBCore_::disconnect() usb_disconnect(); } +void USBCore_::setupClass(uint16_t wLength) +{ + this->ctlIdx = 0; + this->ctlOutBuf = NULL; + this->ctlOutLen = 0; + this->maxWrite = wLength; +} + // Send ‘len’ octets of ‘d’ through the control pipe (endpoint 0). // Blocks until ‘len’ octets are sent. Returns the number of octets // sent, or -1 on error. @@ -912,36 +804,11 @@ usb_dev& USBCore_::usbDev() return usbd; } -/* - * TODO: This is a heck of a monkey patch that just seems to get more - * fragile every time functionality is needed in the rest of the - * Arduino core. - * - * It was initially intended to try and use as much of the firmware - * library’s code as possible, but it’s just not a good fit, and - * should probably be scrapped and started again now that more of its - * scope is known. - */ +/* Log the raw Setup stage data packet */ void USBCore_::transcSetup(usb_dev* usbd, uint8_t ep) { (void)ep; - this->didCtlIn = false; - this->didCtlOut = false; - this->ctlIdx = 0; - this->ctlOutBuf = NULL; - // Configure empty transactions for default - usb_transc_config(&usbd->transc_in[0], NULL, 0, 0); - usb_transc_config(&usbd->transc_out[0], NULL, 0, 0); - - usb_reqsta reqstat = REQ_NOTSUPP; - - uint16_t count = usbd->drv_handler->ep_read((uint8_t *)(&usbd->control.req), 0, (uint8_t)EP_BUF_SNG); - - if (count != USB_SETUP_PACKET_LEN) { - usbd_ep_stall(usbd, 0); - - return; - } + (void)usbd->drv_handler->ep_read((uint8_t *)(&usbd->control.req), 0, (uint8_t)EP_BUF_SNG); for (int i = 0; i < 8; i++) { uint8_t x = ((uint8_t *)&usbd->control.req)[i]; @@ -952,88 +819,30 @@ void USBCore_::transcSetup(usb_dev* usbd, uint8_t ep) Serial1.println(); Serial1.flush(); - this->maxWrite = usbd->control.req.wLength; - switch (usbd->control.req.bmRequestType & USB_REQTYPE_MASK) { - /* standard device request */ - case USB_REQTYPE_STRD: - if (usbd->control.req.bRequest == USB_GET_DESCRIPTOR - && (usbd->control.req.bmRequestType & USB_RECPTYPE_MASK) == USB_RECPTYPE_DEV) { - if ((usbd->control.req.wValue >> 8) == USB_DESCTYPE_CONFIG) { - this->sendDeviceConfigDescriptor(); - reqstat = REQ_SUPP; - break; - } - } - // This calls into ClassCore for class descriptors - reqstat = usbd_standard_request(usbd, &usbd->control.req); - break; - - /* device class request */ - case USB_REQTYPE_CLASS: - // Calls into class_core->req_process, does nothing else. - reqstat = usbd_class_request(usbd, &usbd->control.req); - break; - - /* vendor defined request */ - case USB_REQTYPE_VENDOR: - // Does nothing. - reqstat = usbd_vendor_request(usbd, &usbd->control.req); - break; - - default: - break; - } - - if (reqstat != REQ_SUPP) { - usbd_ep_stall(usbd, 0); - return; - } - if (usbd->control.req.wLength == 0) { - /* USB control transfer status in stage */ - this->sendZLP(usbd, 0); - return; - } - if (usbd->control.req.bmRequestType & USB_TRX_IN) { - if (!this->didCtlIn) { - // Low-level firmware configured IN buffer, - // or ZLP because PluggableUSB accepted but sent nothing - usbd_ep_send(usbd, 0, usbd->transc_in[0].xfer_buf, usbd->transc_in[0].xfer_len); - } - // Either way, _usb_in0_transc will take care of Status OUT - } else { - if (!this->didCtlOut) { - // Low-level firmware configured OUT buffer, - // or force a read because PluggableUSB accepted but read nothing - usbd->drv_handler->ep_rx_enable(usbd, 0); - // _usb_out0_transc will take care of Status IN - } else { - // Status IN after PluggableUSB did a read - this->sendZLP(usbd, 0); - } - } + this->oldTranscSetup(usbd, ep); } // Called in interrupt context. void USBCore_::transcOut(usb_dev* usbd, uint8_t ep) { - EPBuffers().buf(ep).transcOut(); if (ep == 0) { - this->oldTranscOut(usbd, ep); + return; } + EPBuffers().buf(ep).transcOut(); } // Called in interrupt context. void USBCore_::transcIn(usb_dev* usbd, uint8_t ep) { - EPBuffers().buf(ep).transcIn(); if (ep == 0) { - this->oldTranscIn(usbd, ep); + return; } + EPBuffers().buf(ep).transcIn(); } -void USBCore_::sendDeviceConfigDescriptor() +void USBCore_::buildDeviceConfigDescriptor() { - auto oldMaxWrite = this->maxWrite; + this->ctlIdx = 0; this->maxWrite = 0; uint8_t interfaceCount = 0; uint16_t len = 0; @@ -1045,7 +854,7 @@ void USBCore_::sendDeviceConfigDescriptor() configDesc.wTotalLength = sizeof(configDesc) + len; configDesc.bNumInterfaces = interfaceCount; - this->maxWrite = oldMaxWrite; + this->maxWrite = this->CTL_BUFSZ; this->ctlIdx = 0; this->sendControl(0, &configDesc, sizeof(configDesc)); interfaceCount = 0; @@ -1054,9 +863,8 @@ void USBCore_::sendDeviceConfigDescriptor() CDCACM().getInterface(); #endif PluggableUSB().getInterface(&interfaceCount); - // TODO: verify this sends ZLP properly when: - // wTotalLength % sizeof(this->buf) == 0 - this->flush(0); + memcpy(this->cfgDesc, this->ctlBuf, this->ctlIdx); + USBCore().usbDev().desc->config_desc = this->cfgDesc; } void USBCore_::sendZLP(usb_dev* usbd, uint8_t ep) diff --git a/cores/arduino/USBCore.h b/cores/arduino/USBCore.h index d714d651..19b9e034 100644 --- a/cores/arduino/USBCore.h +++ b/cores/arduino/USBCore.h @@ -82,10 +82,6 @@ class EPBuffer void transcIn(); void transcOut(); - /* - * Busy loop until the endpoint has a packet available. - */ - bool waitForReadComplete(); /* * Busy loop until the endpoint has finished its current * transmission. @@ -132,8 +128,6 @@ class EPBuffers_ static EPDesc* desc(uint8_t ep); - bool pollEPStatus(); - private: EPBuffer epBufs[C]; }; @@ -162,6 +156,7 @@ class USBCore_ int recv(uint8_t ep); int flush(uint8_t ep); + void setupClass(uint16_t wLength); void ctlOut(usb_dev* udev); /* * Static member function helpers called from ISR. @@ -195,6 +190,8 @@ class USBCore_ uint8_t *ctlOutBuf; size_t ctlOutLen; + uint8_t cfgDesc[CTL_BUFSZ]; + /* * Pointers to the transaction routines specified by ‘usbd_init’. */ @@ -206,7 +203,7 @@ class USBCore_ void transcOut(usb_dev* usbd, uint8_t ep); void transcIn(usb_dev* usbd, uint8_t ep); - void sendDeviceConfigDescriptor(); + void buildDeviceConfigDescriptor(); void sendZLP(usb_dev* usbd, uint8_t ep); };