Skip to content
This repository was archived by the owner on Mar 26, 2026. It is now read-only.

Commit 539adf8

Browse files
purdeaandreiwhitequark
authored andcommitted
Add SETUP_EP0_OUT_BUF(), for OUT control transfers.
When processing OUT control transfers, the EP0 buffer is armed when wriging EP0BCL, while SDPAUTO is high. At this stage it's not necessary to set HSNAK, because that bit has an effect on the status stage, not on the data stage. Beyond it being not necessary to write HSNAK early, it introduces a race condition, because the host sees the status stage complete early, before we have fully processed the content of EP0BUF, and as such it thinks it can send more control transfers. If the host actually sends more control transfers, then EP0BUF might be overwritten before we have completed processing EP0BUF, and this can cause data corruption. The clean way to handle this, is to force the host to wait in the status stage until we have fully processed the EP0BUF, by NAK-ink the status stage. This does not mean that the packets in the Data stage will be NAK'd because, that is controlled by whether the buffer is armed or not. This commit deprecates `SETUP_EP0_BUF(length)`, and introduces: - `SETUP_EP0_IN_BUF(length)` as a direct replacement for IN transfers - `SETUP_EP0_OUT_BUF()` as a new macro to perform OUT transfers The new recommended way to do OUT control transfers is: ```c for (each_expected_packet) { SETUP_EP0_OUT_BUF(); handle_packet(EP0BUF, EP0BCL); // EP0BCL will have the size of the received packet } ACK_EP0(); ``` That is: only call `ACK_EP0()`, which clears HSNAK by writing 1 to it, after we know it's safe to overwrite EP0BUF. Note: a simple way to reproduce the issue when not following this procedure, is to run: ```bash while true; do lsusb -v -d $(DEVICE_VID):$(DEVICE_PID) > /dev/null; done ``` In a terminal. (replacing DEVICE_VID/DEVICE_PID with correct values) While this loop is running, we expected most applications doing control out transfers with the old method to become unusable. Technically speaking a single run of `lsusb` could even cause trouble if it happes at the wrong time, and I think this could be triggered in other situations as well, this is just the easiest way to reproduce the problem.
1 parent cdebf41 commit 539adf8

1 file changed

Lines changed: 41 additions & 3 deletions

File tree

firmware/library/include/fx2usb.h

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,22 @@ void usb_init(bool disconnect);
5353
} while(0)
5454

5555
/**
56-
* Configure EP0 for an IN or OUT transfer from or to `EP0BUF`.
57-
* For an OUT transfer, specify `length` as `0`.
58-
*/
56+
* This call is deprecated.
57+
*
58+
* Old documentation said:
59+
* Configure EP0 for an IN or OUT transfer from or to `EP0BUF`.
60+
* For an OUT transfer, specify `length` as `0`.
61+
*
62+
* However using `SETUP_EP0_BUF(0)` for OUT transfers will expose a
63+
* race condition when the host is too eager to send more control
64+
* transfers.
65+
*
66+
* For IN control transfers, please use `SETUP_EP0_IN_BUF(length)` instead.
67+
*
68+
* For OUT control transfers, please use one or more `SETUP_EP0_OUT_BUF()`,
69+
* followed by a single `ACK_EP0()` call, but only after all data has been
70+
* processed from `EP0BUF`.
71+
*/
5972
#define SETUP_EP0_BUF(length) \
6073
do { \
6174
SUDPTRCTL = _SDPAUTO; \
@@ -64,6 +77,31 @@ void usb_init(bool disconnect);
6477
EP0CS = _HSNAK; \
6578
} while(0)
6679

80+
/**
81+
* Configure EP0 for an IN transfer from `EP0BUF`.
82+
*/
83+
#define SETUP_EP0_IN_BUF(length) \
84+
do { \
85+
SUDPTRCTL = _SDPAUTO; \
86+
EP0BCH = 0; \
87+
EP0BCL = length; \
88+
EP0CS = _HSNAK; \
89+
} while(0)
90+
91+
/**
92+
* Configure EP0 for an OUT transfer from `EP0BUF`.
93+
*
94+
* For OUT transfers please use one or more calls to
95+
* `SETUP_EP0_OUT_BUF()` followed by a single call to `ACK_EP0()`.
96+
* Do not call `ACK_EP0()` before processing all pending data in `EP0BUF`
97+
*/
98+
#define SETUP_EP0_OUT_BUF() \
99+
do { \
100+
SUDPTRCTL = _SDPAUTO; \
101+
EP0BCH = 0; \
102+
EP0BCL = 0; \
103+
} while(0)
104+
67105
/**
68106
* Acknowledge an EP0 SETUP or OUT transfer.
69107
*/

0 commit comments

Comments
 (0)