1 Reply Latest reply on Apr 16, 2015 8:19 AM by frank.leischnig

    CyU3PUsbSendEp0Data - internal race condition when making multiple calls per control transfer

    frank.leischnig

      Hi all,

         

      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.

         

      Scenario:

         
            
      1. implement a control IN transfer that spans multiple USB Control Endpoint packets (e.g. 2048kB)
      2.     
      3. the firmware shall respond in chunks of MaxPacketSize (e.g. USB-3.0 super-speed connection: 512 Bytes; else 64 Bytes)
      4.    
         

      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[0].status & CY_U3P_UIB_STATE_MASK) >> CY_U3P_UIB_STATE_POS) != CY_U3P_UIB_STATE_ACTIVE) ||                 ((UIB->eepm_endpoint[0] & 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[0].status & CY_U3P_UIB_STATE_MASK) >> CY_U3P_UIB_STATE_POS) == CY_U3P_UIB_STATE_STALL)         && ((UIB->eepm_endpoint[0] & 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.

         

      Thanks