Skip to content

Commit

Permalink
drivers: udc_dwc2: Workaround endpoint disable race condition
Browse files Browse the repository at this point in the history
Endpoint disable function is racing against bus traffic. If the bus
traffic leads to transfer completion immediately before the endpoint
disable is executed, then the transfer complete interrupt will remain
set when the endpoint is disabled. For OUT endpoints this leads to "No
buffer for ep" errors, while for IN endpoint this can lead to double
buffer pull which causes assertion failure.

The proper solution would be to change endpoint disable to not actually
wait for the individual events (and accept that the endpoint may not
need to be disabled because the transfer can just finish). For the time
being workaround the issue by clearing XferCompl bit on endpoint
disable.

Signed-off-by: Tomasz Moń <[email protected]>
  • Loading branch information
tmon-nordic authored and kartben committed Nov 30, 2024
1 parent c19d34c commit a26d3c2
Showing 1 changed file with 15 additions and 2 deletions.
17 changes: 15 additions & 2 deletions drivers/usb/udc/udc_dwc2.c
Original file line number Diff line number Diff line change
Expand Up @@ -1402,6 +1402,19 @@ static void udc_dwc2_ep_disable(const struct device *dev,
return;
}

/* FIXME: This function needs to be changed to not synchronously wait
* for the events to happen because the actions here are racing against
* the USB host packets. It is possible that the IN token or OUT DATA
* gets sent shortly before this function disables the endpoint. If this
* happens, the XferCompl would be set and driver will incorrectly think
* that either:
* * never queued transfer finished, OR
* * transfer queued in incompisoin handler finished (before it really
* does and then it'll "double"-finish when it actually finishes)
*
* For the time being XferCompl is cleared as a workaround.
*/

if (USB_EP_DIR_IS_OUT(cfg->addr)) {
mem_addr_t dctl_reg, gintsts_reg, doepint_reg;
uint32_t dctl;
Expand Down Expand Up @@ -1440,7 +1453,7 @@ static void udc_dwc2_ep_disable(const struct device *dev,
}

/* Clear Endpoint Disabled interrupt */
sys_write32(USB_DWC2_DIEPINT_EPDISBLD, doepint_reg);
sys_write32(USB_DWC2_DOEPINT_EPDISBLD | USB_DWC2_DOEPINT_XFERCOMPL, doepint_reg);

dctl |= USB_DWC2_DCTL_CGOUTNAK;
sys_write32(dctl, dctl_reg);
Expand All @@ -1464,7 +1477,7 @@ static void udc_dwc2_ep_disable(const struct device *dev,
dwc2_wait_for_bit(dev, diepint_reg, USB_DWC2_DIEPINT_EPDISBLD);

/* Clear Endpoint Disabled interrupt */
sys_write32(USB_DWC2_DIEPINT_EPDISBLD, diepint_reg);
sys_write32(USB_DWC2_DIEPINT_EPDISBLD | USB_DWC2_DIEPINT_XFERCOMPL, diepint_reg);

/* TODO: Read DIEPTSIZn here? Programming Guide suggest it to
* let application know how many bytes of interrupted transfer
Expand Down

0 comments on commit a26d3c2

Please sign in to comment.