electronics
/
libopencm3
9
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

usb: fix race between setup complete and endpoint nak 

See https://github.com/libopencm3/libopencm3/issues/873

Commentary describing this patch originally by zyp:

```
After looking further into it, I've concluded that my preliminary
analysis looks correct. The problem is that setting CNAK before
the SETUP complete event is received causes a race condition. The
SETUP callback is called when the SETUP packet event is received,
which means that setting CNAK from the callback is too early.

Originally the problem was that CNAK was set by ep_read() which is
called by the callback. #672 solved this by moving CNAK out of
ep_read() and calling it after the SETUP complete event is received
instead.

The regression by #785 is caused by the introduction of flow control
calls into the SETUP callback. They also set CNAK.

To solve this properly, I propose changing the event handling code
to only call the SETUP callback after the SETUP complete event is
received. Unfortunately, this implies that the callback can't call
ep_read() itself anymore, because the packet has to be read out of
the FIFO before the SETUP complete event arrives. This implies a
change of the API between the hardware drivers and _usbd_control_setup().
```

L1 (st_usbfs) works and passes tests as before change
F4 (dwc_otg_fs) works and now passes tests. (yay)
LM4f still compiles, and has had the same style of implementation as
st_usbfs, however has not been tested on any hardware.
This commit is contained in:
Karl Palsson 2018-08-27 14:20:27 +00:00
parent 42e43515c6
commit 26ab78a710
4 changed files with 12 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
lib/stm32/common/st_usbfs_core.c
View File

@ -251,6 +251,7 @@ void st_usbfs_poll(usbd_device *dev)
/* OUT or SETUP? */
if (*USB_EP_REG(ep) & USB_EP_SETUP) {
type = USB_TRANSACTION_SETUP;
st_usbfs_ep_read_packet(dev, ep, &dev->control_state.req, 8);
} else {
type = USB_TRANSACTION_OUT;
}

5
lib/usb/usb_control.c
View File

@ -228,11 +228,6 @@ void _usbd_control_setup(usbd_device *usbd_dev, uint8_t ea)
usbd_ep_nak_set(usbd_dev, 0, 1);
if (usbd_ep_read_packet(usbd_dev, 0, req, 8) != 8) {
stall_transaction(usbd_dev);
return;
}
if (req->wLength == 0) {
usb_control_setup_read(usbd_dev, req);
} else if (req->bmRequestType & 0x80) {

9
lib/usb/usb_dwc_common.c
View File

@ -358,6 +358,11 @@ void dwc_poll(usbd_device *usbd_dev)
uint32_t rxstsp = REBASE(OTG_GRXSTSP);
uint32_t pktsts = rxstsp & OTG_GRXSTSP_PKTSTS_MASK;
uint8_t ep = rxstsp & OTG_GRXSTSP_EPNUM_MASK;
if (pktsts == OTG_GRXSTSP_PKTSTS_SETUP_COMP) {
usbd_dev->user_callback_ctr[ep][USB_TRANSACTION_SETUP] (usbd_dev, ep);
}
if (pktsts == OTG_GRXSTSP_PKTSTS_OUT_COMP
|| pktsts == OTG_GRXSTSP_PKTSTS_SETUP_COMP) {
REBASE(OTG_DOEPTSIZ(ep)) = usbd_dev->doeptsiz[ep];
@ -390,7 +395,9 @@ void dwc_poll(usbd_device *usbd_dev)
/* Save packet size for dwc_ep_read_packet(). */
usbd_dev->rxbcnt = (rxstsp & OTG_GRXSTSP_BCNT_MASK) >> 4;
if (usbd_dev->user_callback_ctr[ep][type]) {
if (type == USB_TRANSACTION_SETUP) {
dwc_ep_read_packet(usbd_dev, ep, &usbd_dev->control_state.req, 8);
} else if (usbd_dev->user_callback_ctr[ep][type]) {
usbd_dev->user_callback_ctr[ep][type] (usbd_dev, ep);
}

4
lib/usb/usb_lm4f.c
View File

@ -503,7 +503,9 @@ static void lm4f_poll(usbd_device *usbd_dev)
usbd_dev->control_state.state != LAST_DATA_OUT)
? USB_TRANSACTION_SETUP :
USB_TRANSACTION_OUT;
if (type == USB_TRANSACTION_SETUP) {
lm4f_ep_read_packet(usbd_dev, 0, &usbd_dev->control_state.req, 8);
}
if (usbd_dev->user_callback_ctr[0][type]) {
usbd_dev->
user_callback_ctr[0][type](usbd_dev, 0);