cancel
Showing results for 
Search instead for 
Did you mean: 

Known bugs in the OTG / USB library v2.1.0

ivous2
Associate
Posted on January 21, 2014 at 14:04

Here is the list of known bugs in the USB / OTG library v2.1.0:

files - usbd_cdc_core.c / usbd_conf.h

ep descriptors should not use 

#ifdef USE_USB_OTG_HS

but use instead

#ifndef USE_EMBEDDED_PHY 

file - usb_bsp.c

function USB_OTG_BSP_Init 

For OTH-HS using Internal Phy

RCC_AHB1PeriphClockLPModeCmd( RCC_AHB1Periph_OTG_HS_ULPI, DISABLE);  

This problem was discovered some time ago, when OTG-HS is in use and device is put in sleep mode, the ULPI clocks must be switched off, otherwise the OTG functionality is broken.

Whenever GPIO_InitStructure is used, it must be properly initialized first (otherwise it is causing strange side effects):

GPIO_StructInit(&GPIO_InitStructure);

file - usb_dcd_int.c

Search for ''emptyintr'' occurence, where the source code is trying to clear this flag. There are two occurrences:

CLEAR_IN_EP_INTR(1, emptyintr);

CLEAR_IN_EP_INTR( epnum , emptyintr);

It is read only flag, which cannot be cleared, so this interrupt must be masked instead:

fifoemptymsk = 0x1 << epnum;

USB_OTG_MODIFY_REG32(&pdev->regs.DREGS->DIEPEMPMSK, fifoemptymsk, 0)

This problem is causing high CPU load, but the USB is working.

file - usbd_core.c

USBD_Init function

correct order:  

/* Force Device Mode*/

USB_OTG_SetCurrentMode(pdev, DEVICE_MODE);

/* Enable Interrupts */

USB_OTG_BSP_EnableInterrupt(pdev);

On some hosts this is blocking proper enumeration of our device.

file usbh_core.c

missing proper error handling 

USBH_HandleControl function

case CTRL_STATUS_IN_WAIT:

   

    URB_Status = HCD_GetURB_State(pdev , phost->Control.hc_num_in);

    

    if  ( URB_Status == URB_DONE)

    { /* Control transfers completed, Exit the State Machine */

      phost->gState =   phost->gStateBkp;

      phost->Control.state = CTRL_COMPLETE;

    }

   

    else if (URB_Status == URB_ERROR)

    {

      phost->Control.state = CTRL_ERROR; 

    }

    

    else if((HCD_GetCurrentFrame(pdev)\

      - phost->Control.timer) > timeout)

    {

      phost->Control.state = CTRL_ERROR;

    }

     else if(URB_Status == URB_STALL)

    {

      /* Control transfers completed, Exit the State Machine */

      phost->gState =   phost->gStateBkp;

      phost->Control.state = CTRL_STALL; //added

 

      status = USBH_NOT_SUPPORTED; //added

    }

    break;

13 REPLIES 13
Scott.Nathan
Associate
Posted on September 01, 2014 at 18:42

I have both USB ports on STM32F4 working (ish) and both as hosts using the v2.1.0 library.  The FS core works faultlessly but the HS core works for a few hours sometimes days then tends to fall over somewhere.  Very difficult for me to pin down where and when due to the hours involved.  I used the bug list here to fix as much as I could now on my own unless someone else can share their experiences.

Great to see a list of known bugs however I must take issue with;

 phost->Control.state = CTRL_STALL; //added

 

Control.state should refer to enum CTRL_State but this does not have an entry for CTRL_STALL (defined as 4 in CTRL_STATUS), but does have CTRL_STALLED (==12).  Setting this here tells the state machine it is in CTRL_DATA_IN_WAIT (== 12).

On  further investigation I see the library v2.1.0 now comes with a fix here that says    

 else if(URB_Status == URB_STALL)

    {

      /* Control transfers completed, Exit the State Machine */

      phost->gState =   phost->gStateBkp;

      phost->Control.status = CTRL_STALL;

      status = USBH_NOT_SUPPORTED;

    }

    break;

Now Control.status = CTRL_STALL is valid but I am at a loss to see what this code is going to do as it is not really used in the state machine.  I may have a go at coding the .status out to see what happens...

One more area that needs some mention is hard coded delays in this library that could do with adding additional states to allow the processor to get on with other things when using both cores / other peripherals - yes I know I might be able to use a RTOS to solve this...

For reference from usbh_core.h;

      typedef enum {

        CTRL_IDLE =0,

        CTRL_SETUP,

        CTRL_SETUP_WAIT,

        CTRL_DATA_IN,

        CTRL_DATA_IN_WAIT, // == 4

        CTRL_DATA_OUT,

        CTRL_DATA_OUT_WAIT,

        CTRL_STATUS_IN,

        CTRL_STATUS_IN_WAIT,

        CTRL_STATUS_OUT,

        CTRL_STATUS_OUT_WAIT,

        CTRL_ERROR,

        CTRL_STALLED, // == 12

        CTRL_COMPLETE

      }

      CTRL_State;

      typedef enum {

        CTRL_START = 0,

        CTRL_XFRC,

        CTRL_HALTED,

        CTRL_NAK,

        CTRL_STALL,  // == 4

        CTRL_XACTERR,

        CTRL_BBLERR,

        CTRL_DATATGLERR,

        CTRL_FAIL

      }CTRL_STATUS;

jfolsom
Associate II
Posted on May 11, 2015 at 19:12

File: usb_core.c.

The USB_OTG_ EnableGlobalInt() and USB_OTG_ DisableGlobalInt() are identical. It looks like the Disable function was a copy of the Enable function, and never got modified. In USB_OTG_DisableGlobalInt(), change:

ahbcfg.b.glblintrmsk = 1; /* Enable interrupts */

to:

ahbcfg.b.glblintrmsk = 0; /* Disable interrupts */

and change the comments header as desired.
qwer.asdf
Senior
Posted on May 12, 2015 at 10:21

Why doesn't ST make a small fixed release with all these known bugs out there? Please, ST?

StefanoBettega1
Associate II
Posted on June 20, 2016 at 11:06

<cite>

File: usb_core.c. The USB_OTG_ EnableGlobalInt() and USB_OTG_ DisableGlobalInt() are identical. It looks like the Disable function was a copy of the Enable function, and never got modified. In USB_OTG_DisableGlobalInt(), change:

ahbcfg.b.glblintrmsk = 1; /* Enable interrupts */

to:

ahbcfg.b.glblintrmsk = 0; /* Disable interrupts */

and change the comments header as desired. </cite> I think the library is correct in that point; library uses USB_OTG_MODIFY_REG32 macro, in which you specifiy what bits are to be cleared and what bits are to be set. In either cases you use the bits put to 1 to indicate on which bit macro has to operate.