CyU3PUsbSendEp0Data - internal race condition when making multiple calls per control transfer | Cypress Semiconductor
CyU3PUsbSendEp0Data - internal race condition when making multiple calls per control transfer
CyU3PUsbSendEp0Data has a race condition that can cause delays of 500ms per call, despite successful completion. I just wanted to discuss a work-around or a fix.
- implement a control IN transfer that spans multiple USB Control Endpoint packets (e.g. 2048kB)
- the firmware shall respond in chunks of MaxPacketSize (e.g. USB-3.0 super-speed connection: 512 Bytes; else 64 Bytes)
As per SDK API documentation of CyU3PUsbSendEp0Data(), this is an acceptable implementation: cyu3usb.h states "Multiple calls of this function can be made to respond to a single control request as long as each call sends an integral number of full packets to the host."
Recently, we noticed that this sometimes leads to long-lasting transfers, that often even hit the time-out of the host applications DeviceIoControl calls. For example, it happens using a Intel(R) 6 Series/C200 USB Enhanced Host Controller (USB-2.0 high-speed).
A work-around seems to be increasing chunk size, i.e. using 512 Bytes per call even for USB-2 connections. But how can I know, how much margin it provides?
The problem can be traced down to a race condition. In CyU3PUsbSendEp0Data(), after CyU3PDmaChannelSendData the DMA socket state and Egress Endpoint Manager state are checked (with time-out of 500ms):
while ((((UIB->sck.status & CY_U3P_UIB_STATE_MASK) >> CY_U3P_UIB_STATE_POS) != CY_U3P_UIB_STATE_ACTIVE) || ((UIB->eepm_endpoint & CY_U3P_UIB_EEPM_EP_READY) == 0))
Please find attached a modified version of the source code. It reports these status signals before and after the while-loop.
It can happen that socket state is already CY_U3P_UIB_STATE_STALL and the EEPM is not ready. This can be enforced by adding a small delay before "while" (also contained in the attached source file). This condition seems to tell that the transfer has already completed.
Obviously, the while-loop can be improved. My proposed solution would be adding the following lines to the loop body:
if ((((UIB->sck.status & CY_U3P_UIB_STATE_MASK) >> CY_U3P_UIB_STATE_POS) == CY_U3P_UIB_STATE_STALL) && ((UIB->eepm_endpoint & CY_U3P_UIB_EEPM_EP_READY) == 0)) break;
But is this safe? I maybe do not completely understand the meaning and side-effects of these flags.