diff --git a/examples/boot-dfu-spiflash/Makefile b/examples/boot-dfu-spiflash/Makefile index cd39117..4acbf13 100644 --- a/examples/boot-dfu-spiflash/Makefile +++ b/examples/boot-dfu-spiflash/Makefile @@ -2,5 +2,8 @@ TARGET = boot-dfu-spiflash LIBRARIES = fx2 fx2usb fx2dfu fx2isrs MODEL = medium +CODE_SIZE ?= 0x3c00 +XRAM_SIZE ?= 0x0400 + LIBFX2 = ../../firmware/library include $(LIBFX2)/fx2rules.mk diff --git a/examples/boot-dfu-spiflash/main.c b/examples/boot-dfu-spiflash/main.c index b0c2ecd..8dfe49f 100644 --- a/examples/boot-dfu-spiflash/main.c +++ b/examples/boot-dfu-spiflash/main.c @@ -268,7 +268,7 @@ usb_dfu_iface_state_t usb_dfu_iface_state = { void handle_usb_get_interface(uint8_t interface) { if(interface == 0) { EP0BUF[0] = dfu_alt_setting; - SETUP_EP0_BUF(1); + SETUP_EP0_IN_BUF(1); return; } STALL_EP0(); diff --git a/examples/boot-uf2-dfu/Makefile b/examples/boot-uf2-dfu/Makefile index 8c67148..b98a008 100644 --- a/examples/boot-uf2-dfu/Makefile +++ b/examples/boot-uf2-dfu/Makefile @@ -2,5 +2,8 @@ TARGET = boot-uf2-dfu LIBRARIES = fx2 fx2usb fx2dfu fx2usbmassstor fx2uf2 fx2isrs MODEL = medium +CODE_SIZE ?= 0x3c00 +XRAM_SIZE ?= 0x0400 + LIBFX2 = ../../firmware/library include $(LIBFX2)/fx2rules.mk diff --git a/examples/cdc-acm/main.c b/examples/cdc-acm/main.c index cd18149..4c27273 100644 --- a/examples/cdc-acm/main.c +++ b/examples/cdc-acm/main.c @@ -190,7 +190,7 @@ void handle_usb_setup(__xdata struct usb_req_setup *req) { line_coding->bCharFormat = USB_CDC_REQ_LINE_CODING_STOP_BITS_1; line_coding->bParityType = USB_CDC_REQ_LINE_CODING_PARITY_NONE; line_coding->bDataBits = 8; - SETUP_EP0_BUF(sizeof(struct usb_cdc_req_line_coding)); + SETUP_EP0_IN_BUF(sizeof(struct usb_cdc_req_line_coding)); return; } @@ -199,7 +199,8 @@ void handle_usb_setup(__xdata struct usb_req_setup *req) { if(req->bmRequestType == (USB_RECIP_IFACE|USB_TYPE_CLASS|USB_DIR_OUT) && req->bRequest == USB_CDC_PSTN_REQ_SET_LINE_CODING && req->wIndex == 0 && req->wLength == 7) { - SETUP_EP0_BUF(0); + SETUP_EP0_OUT_BUF(); + ACK_EP0(); // Since we're throwing away the buffer, it's okay to immediately just ACK it. return; } diff --git a/firmware/boot-cypress/main.c b/firmware/boot-cypress/main.c index b8f9d31..ad30856 100644 --- a/firmware/boot-cypress/main.c +++ b/firmware/boot-cypress/main.c @@ -145,15 +145,16 @@ void handle_pending_usb_setup(void) { STALL_EP0(); break; } - SETUP_EP0_BUF(len); + SETUP_EP0_IN_BUF(len); } else { - SETUP_EP0_BUF(0); + SETUP_EP0_OUT_BUF(); while(EP0CS & _BUSY); if(!eeprom_write(arg_chip, arg_addr, EP0BUF, len, arg_dbyte, page_size, /*timeout=*/166)) { STALL_EP0(); break; } + ACK_EP0(); } arg_len -= len; @@ -177,11 +178,12 @@ void handle_pending_usb_setup(void) { if(arg_read) { while(EP0CS & _BUSY); xmemcpy(EP0BUF, (__xdata void *)arg_addr, len); - SETUP_EP0_BUF(len); + SETUP_EP0_IN_BUF(len); } else { - SETUP_EP0_BUF(0); + SETUP_EP0_OUT_BUF(); while(EP0CS & _BUSY); xmemcpy((__xdata void *)arg_addr, EP0BUF, arg_len); + ACK_EP0(); } arg_len -= len; diff --git a/firmware/boot-dfu/Makefile b/firmware/boot-dfu/Makefile index d9a861f..2402ea6 100644 --- a/firmware/boot-dfu/Makefile +++ b/firmware/boot-dfu/Makefile @@ -2,5 +2,8 @@ TARGET = boot-dfu LIBRARIES = fx2 fx2usb fx2dfu fx2isrs MODEL = small +CODE_SIZE ?= 0x3c00 +XRAM_SIZE ?= 0x0400 + LIBFX2 = ../library include $(LIBFX2)/fx2rules.mk diff --git a/firmware/library/defusbgetconfig.c b/firmware/library/defusbgetconfig.c index 6c96ea3..087a497 100644 --- a/firmware/library/defusbgetconfig.c +++ b/firmware/library/defusbgetconfig.c @@ -2,5 +2,5 @@ void handle_usb_get_configuration(void) { EP0BUF[0] = usb_config_value; - SETUP_EP0_BUF(1); + SETUP_EP0_IN_BUF(1); } diff --git a/firmware/library/defusbgetiface.c b/firmware/library/defusbgetiface.c index c3409d7..cfa61ee 100644 --- a/firmware/library/defusbgetiface.c +++ b/firmware/library/defusbgetiface.c @@ -3,5 +3,5 @@ void handle_usb_get_interface(uint8_t interface) { interface; EP0BUF[0] = 0; - SETUP_EP0_BUF(1); + SETUP_EP0_IN_BUF(1); } diff --git a/firmware/library/include/fx2usb.h b/firmware/library/include/fx2usb.h index 77745f8..d96cfde 100644 --- a/firmware/library/include/fx2usb.h +++ b/firmware/library/include/fx2usb.h @@ -53,10 +53,40 @@ void usb_init(bool disconnect); } while(0) /** - * Configure EP0 for an IN or OUT transfer from or to `EP0BUF`. - * For an OUT transfer, specify `length` as `0`. - */ + * This call is deprecated. + * + * Old documentation said: + * Configure EP0 for an IN or OUT transfer from or to `EP0BUF`. + * For an OUT transfer, specify `length` as `0`. + * + * However using `SETUP_EP0_BUF(0)` for OUT transfers will expose a + * race condition when the host is too eager to send more control + * transfers. + * + * For IN control transfers, please use `SETUP_EP0_IN_BUF(length)` instead. + * + * For OUT control transfers, please use one or more `SETUP_EP0_OUT_BUF()`, + * followed by a single `ACK_EP0()` call, but only after all data has been + * processed from `EP0BUF`. + * + * If one desires to use the deprecated SETUP_EP0_BUF, then it can be + * re-enabled with: + * `CFLAGS=-DSETUP_EP0_BUF_is_deprecated_use_SETUP_EP0_IN_BUF_or_SETUP_EP0_OUT_BUF_instead=1` + */ #define SETUP_EP0_BUF(length) \ + do { \ + if (SETUP_EP0_BUF_is_deprecated_use_SETUP_EP0_IN_BUF_or_SETUP_EP0_OUT_BUF_instead) { \ + SUDPTRCTL = _SDPAUTO; \ + EP0BCH = 0; \ + EP0BCL = length; \ + EP0CS = _HSNAK; \ + } \ + } while(0) + +/** + * Configure EP0 for an IN transfer from `EP0BUF`. + */ +#define SETUP_EP0_IN_BUF(length) \ do { \ SUDPTRCTL = _SDPAUTO; \ EP0BCH = 0; \ @@ -64,6 +94,20 @@ void usb_init(bool disconnect); EP0CS = _HSNAK; \ } while(0) +/** + * Configure EP0 for an OUT transfer from `EP0BUF`. + * + * For OUT transfers please use one or more calls to + * `SETUP_EP0_OUT_BUF()` followed by a single call to `ACK_EP0()`. + * Do not call `ACK_EP0()` before processing all pending data in `EP0BUF` + */ +#define SETUP_EP0_OUT_BUF() \ + do { \ + SUDPTRCTL = _SDPAUTO; \ + EP0BCH = 0; \ + EP0BCL = 0; \ + } while(0) + /** * Acknowledge an EP0 SETUP or OUT transfer. */ diff --git a/firmware/library/usb.c b/firmware/library/usb.c index 0bb0994..5402945 100644 --- a/firmware/library/usb.c +++ b/firmware/library/usb.c @@ -113,13 +113,13 @@ __endasm; EP0BUF[0] = (usb_self_powered << 0) | (usb_remote_wakeup << 1); EP0BUF[1] = 0; - SETUP_EP0_BUF(2); + SETUP_EP0_IN_BUF(2); // Get Status - Interface } else if(req->bmRequestType == (USB_RECIP_IFACE|USB_TYPE_STANDARD|USB_DIR_IN) && req->bRequest == USB_REQ_GET_STATUS) { EP0BUF[0] = 0; EP0BUF[1] = 0; - SETUP_EP0_BUF(2); + SETUP_EP0_IN_BUF(2); // Set Feature - Endpoint // Clear Feature - Endpoint } else if(req->bmRequestType == (USB_RECIP_ENDPT|USB_TYPE_STANDARD|USB_DIR_OUT) && @@ -147,7 +147,7 @@ __endasm; if(EPnCS != 0) { EP0BUF[0] = ((*EPnCS & _STALL) != 0); EP0BUF[1] = 0; - SETUP_EP0_BUF(2); + SETUP_EP0_IN_BUF(2); } } else { handle_usb_setup(req); diff --git a/firmware/library/usbdfu.c b/firmware/library/usbdfu.c index 38ca144..a259dc1 100644 --- a/firmware/library/usbdfu.c +++ b/firmware/library/usbdfu.c @@ -3,6 +3,9 @@ #pragma save #pragma nooverlay + +__xdata uint8_t scratch2[512]; + bool usb_dfu_setup(usb_dfu_iface_state_t *dfu, __xdata struct usb_req_setup *req) { uint8_t interface = dfu->state > USB_DFU_STATE_appDETACH ? 0 : dfu->interface; @@ -23,7 +26,7 @@ bool usb_dfu_setup(usb_dfu_iface_state_t *dfu, __xdata struct usb_req_setup *req req->bRequest == USB_DFU_REQ_GETSTATE && req->wValue == 0 && req->wLength == sizeof(uint8_t)) { EP0BUF[0] = dfu->state; - SETUP_EP0_BUF(1); + SETUP_EP0_IN_BUF(1); return true; } @@ -38,7 +41,9 @@ bool usb_dfu_setup(usb_dfu_iface_state_t *dfu, __xdata struct usb_req_setup *req !dfu->sync) { // If we're here, then EP0BUF is still in use, but the host already sent GETSTATUS. // If we do SETUP_EP0_* right now, we'll overwrite EP0BUF, and get corrupted data. - // So, delay responding to this packet until after EP0BUF is copied to scratch space. + // So, delay responding to this packet until after EP0BUF is copied a second scratch + // space. We cannot use the normal scratch space, because it's used when getting + // descriptors, and our downloaded data might get overwritten. // // (GETSTATUS in dfuMANIFEST-SYNC does not have this restriction, but these requests // use identical flows in the DFU spec, and it is simpler to handle them the same way.) @@ -55,7 +60,7 @@ bool usb_dfu_setup(usb_dfu_iface_state_t *dfu, __xdata struct usb_req_setup *req status->bwPollTimeoutHigh = 0; status->bState = dfu->state; status->iString = 0; - SETUP_EP0_BUF(sizeof(struct usb_dfu_req_get_status)); + SETUP_EP0_IN_BUF(sizeof(struct usb_dfu_req_get_status)); return true; } @@ -93,14 +98,14 @@ bool usb_dfu_setup(usb_dfu_iface_state_t *dfu, __xdata struct usb_req_setup *req dfu->length = req->wLength; dfu->pending = true; dfu->sync = false; - SETUP_EP0_BUF(0); + SETUP_EP0_OUT_BUF(); return true; } else if(dfu->state == USB_DFU_STATE_dfuDNLOAD_IDLE && req->wLength > 0) { dfu->state = USB_DFU_STATE_dfuDNLOAD_SYNC; dfu->length = req->wLength; dfu->pending = true; dfu->sync = false; - SETUP_EP0_BUF(0); + SETUP_EP0_OUT_BUF(); return true; } else if(dfu->state == USB_DFU_STATE_dfuDNLOAD_IDLE) { dfu->state = USB_DFU_STATE_dfuMANIFEST_SYNC; @@ -139,7 +144,7 @@ void usb_dfu_setup_deferred(usb_dfu_iface_state_t *dfu) { uint16_t length = dfu->length; dfu->status = dfu->firmware_upload(dfu->offset, &EP0BUF[0], &dfu->length); if(dfu->status == USB_DFU_STATUS_OK) { - SETUP_EP0_BUF(dfu->length); + SETUP_EP0_IN_BUF(dfu->length); if(dfu->length < length) { dfu->state = USB_DFU_STATE_dfuIDLE; } @@ -149,7 +154,8 @@ void usb_dfu_setup_deferred(usb_dfu_iface_state_t *dfu) { } } else if(dfu->state == USB_DFU_STATE_dfuDNLOAD_SYNC) { while(EP0CS & _BUSY); - xmemcpy(scratch, &EP0BUF[0], dfu->length); + xmemcpy(scratch2, &EP0BUF[0], dfu->length); + ACK_EP0(); // Wait until we get a GETSTATUS request (in case we still haven't got one), and then reply // to it from here, after we've safely stashed away EP0BUF contents. @@ -157,7 +163,7 @@ void usb_dfu_setup_deferred(usb_dfu_iface_state_t *dfu) { usb_dfu_setup(dfu, (__xdata struct usb_req_setup *)SETUPDAT); return; } else if(dfu->state == USB_DFU_STATE_dfuDNBUSY) { - dfu->status = dfu->firmware_dnload(dfu->offset, scratch, dfu->length); + dfu->status = dfu->firmware_dnload(dfu->offset, scratch2, dfu->length); if(dfu->status == USB_DFU_STATUS_OK) { dfu->offset += dfu->length; dfu->state = USB_DFU_STATE_dfuDNLOAD_IDLE; diff --git a/firmware/library/usbmassstor.c b/firmware/library/usbmassstor.c index 70551ab..a35edc8 100644 --- a/firmware/library/usbmassstor.c +++ b/firmware/library/usbmassstor.c @@ -18,7 +18,7 @@ bool usb_mass_storage_bbb_setup(usb_mass_storage_bbb_state_t *state, request->bRequest == USB_REQ_MASS_STORAGE_GET_MAX_LUN && request->wValue == 0 && request->wIndex == state->interface && request->wLength == 1) { EP0BUF[0] = state->max_lun; - SETUP_EP0_BUF(1); + SETUP_EP0_IN_BUF(1); return true; }