3 Replies Latest reply on Dec 16, 2015 3:38 AM by karthik.sivaramakrishnan

    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

        • 1. Re: CyU3PUsbSendEp0Data - internal race condition when making multiple calls per control transfer
          Madhu Lakshmipathy

           Hi Frank,

             

          Thanks for reprting this to us. Also, I appreciate your intertest in fixing the issue.

             

          I will check this with the software team. Also, can you please let me know if you have tested this after modifying the souce code?

             

          If so did it fix the issue? Please let me know.

             

          Regards,

             

          -Madhu Sudhan

          • 2. Re: CyU3PUsbSendEp0Data - internal race condition when making multiple calls per control transfer
            frank.leischnig

            Hi Madhu Sudhan,

               

            Please find attached a modified version of the USB Bulk Loop Auto example firmware and Bulk Loop application. It is ready-to-use on the FX3 Evaluation Kit CYUSB3KIT-001.

               

            Screen shots show behaviour (including timing) of the original and modified data transfer function.

               

            The answer to your question is: yes and no. The source code above mitigates time-out. But IN transfers, if split into parts of MaxPacketSize, still last too long.

               

            Can you confirm the misbehaviour?

               

            What do you advise?

               

            BTW:  In another test I found that SDK1.3.1 libraries seem to not have this bug.

               

            Thanks - Frank

            • 3. Re: CyU3PUsbSendEp0Data - internal race condition when making multiple calls per control transfer
              karthik.sivaramakrishnan

              Hi Frank,

                 

              Thanks for bringing this up. The new code was added a check for infrequent DMA lock-up errors, and it did not take care of the possibility of multiple SendEp0Data calls in response to a single transfer. The correct fix is to skip the whole while loop if the ack has already been completed.

                 

              Something like the following (please note that I have moved the Ack call into the same If clause):

                 

                      if (glUibDeviceInfo.ackPending == CyTrue)
                      {
                          /* Wait until the DMA socket and EPM are in ready state. */
                          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))
                          {
                              if (tmo == 0)
                              {
                                  /* If the socket and EP state have not changed even after 500 ms, try to go ahead and push the data
                                     out. Errors from this transfer will be handled below.
                                   */
                                  break;
                              }

                 

                              CyU3PThreadSleep (5); 
                              tmo--;
                          }

                 

                          /* Clear the busy bit. */
                          CyU3PUsbAckSetup ();
                      }

                 

              We will apply the fix into the next release of the SDK.

                 

              Thanks,

                 

              Karthik