remove (almost all of) transcSetup

Author: tlyu

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<L>::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<size_t L>
-bool EPBuffer<L>::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<L>::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_<L, C>::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<size_t L, size_t C>
-bool EPBuffers_<L, C>::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_<USB_EP_SIZE, EP_COUNT>& EPBuffers()
 {
     static EPBuffers_<USB_EP_SIZE, EP_COUNT> 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)

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<L> 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);
 };