In CubeMX generated USB code HAL_PCD_ResetCallback() always calls Error_Handler

[Edwin]

Because of a mismatch in the enumeration of USB speeds the HAL_PCD_ResetCallback() can not determine the correct USB speed and calls Error_Handler().

​

In this example it occurs in CubeIDE v1.0.1 on STM32F767VIT6 with the USB-FS interface configured as CDC-ACM device. But it also happens with different targets and on CubeMX 5.1.0, 5.2.0 and 5.2.1.

​

Here's a copy of the HAL_PCD_ResetCallback() function in "usbd_conf.c":

void HAL_PCD_ResetCallback(PCD_HandleTypeDef *hpcd)
{ 
 USBD_SpeedTypeDef speed = USBD_SPEED_FULL;
​
 if ( hpcd->Init.speed == PCD_SPEED_HIGH)
 {
 speed = USBD_SPEED_HIGH;
 }
 else if ( hpcd->Init.speed == PCD_SPEED_FULL)
 {
 speed = USBD_SPEED_FULL;
 }
 else
 {
 Error_Handler();
 }
 /* Set Speed. */
 USBD_LL_SetSpeed((USBD_HandleTypeDef*)hpcd->pData, speed);
​
 /* Reset Device. */
 USBD_LL_Reset((USBD_HandleTypeDef*)hpcd->pData);
}

When I debug I can see that "hpcd->Init.speed" is set to 3, while the only valid options here are 0 (USBD_SPEED_HIGH) and 1 (USBD_SPEED_FULL), so it ends up calling Error_Handler().

​

The problem is that the values in hpcd->Init.speed are set using the defines in "stm32f7xx_ll_usb.h", where you can see that value 3 corresponds to "full speed" :

#define USB_OTG_SPEED_HIGH 0U
#define USB_OTG_SPEED_HIGH_IN_FULL 1U
#define USB_OTG_SPEED_FULL 3U

​While the variable is compared to values as defined in "stm32f7xx_hal_pcd.h", where "full speed" has value 2 and 3 is not defined at all:

#define PCD_SPEED_HIGH 0U
#define PCD_SPEED_HIGH_IN_FULL 1U
#define PCD_SPEED_FULL 2U

Finally in "usbd_def.h" it's again different and you can find this:

typedef enum
{
 USBD_SPEED_HIGH = 0U,
 USBD_SPEED_FULL = 1U,
 USBD_SPEED_LOW = 2U,
}USBD_SpeedTypeDef;

​

This should be fixed by either getting rid of the different enumerations or at least properly matching them e.g.:

SPEED_HIGH = 0U;

SPEED_HIGH_IN_FULL = 1U;

SPEED_LOW = 2U;

SPEED_FULL = 3U;

​

Related forum posts with same issue on different targets:

https://community.st.com/s/question/0D50X0000AVU8zXSQT/cubemx-usb-bug

https://community.st.com/s/question/0D50X0000ASpDuj/cubemx-5-errorhandler-in-usb-cdc-is-called-too-often

Please fix this in the next release of CubeMX/CubeIDE.

[therealergo]

Updated my codebase to the latest STM32Cube* today and experienced the exact same issue. As far as I can tell this entirely breaks the USB peripheral on the STM32F767VIT in full-speed mode.

As a bit of context, here is how the "HAL_PCD_ResetCallback(...)" function in "usbd_conf.c" was implemented in the last version of STM32CubeMX:

void HAL_PCD_ResetCallback(PCD_HandleTypeDef *hpcd)
{ 
 USBD_SpeedTypeDef speed = USBD_SPEED_FULL;
 
 /* Set USB current speed. */
 switch (hpcd->Init.speed)
 {
 case PCD_SPEED_HIGH:
 speed = USBD_SPEED_HIGH;
 break;
 case PCD_SPEED_FULL:
 speed = USBD_SPEED_FULL;
 break;
	
 default:
 speed = USBD_SPEED_FULL;
 break; 
 }
 USBD_LL_SetSpeed((USBD_HandleTypeDef*)hpcd->pData, speed);
 
 /* Reset Device. */
 USBD_LL_Reset((USBD_HandleTypeDef*)hpcd->pData);
}

So the reason that this previously worked was because the case statement would fall through to "default:" when in full-speed mode, defaulting to the right value.

This can be reproduced on a blank STM32CubeMX project with "USB_OTG_FS" set to "Device_Only" and "Class for FS IP" set to "Mass Storage Class."

[Edwin]

The new version will also fail silently and use the default setting, until you implement the Error_Handler() function (it's generated empty by default).

It was also unnoticed by us at first and gave problems later when we integrated the new code into our framework.

​

Both the old and the new version are wrong in the same way, because different definitions are used to set and compare the "hpcd->Init.speed" value.​

Here is a snippet from "stm32f7xx_hal_pcd.c" where "hpcd->Init.speed" gets set using USB_OTG_SPEED_* enums:

 if (USB_GetDevSpeed(hpcd->Instance) == USB_OTG_SPEED_HIGH)
 {
 hpcd->Init.speed = USB_OTG_SPEED_HIGH;
 }
 else
 {
 hpcd->Init.speed = USB_OTG_SPEED_FULL;
 }

While in "HAL_PCD_ResetCallback()" it gets compared to the PCD_SPEED_* defines.

​

If a single enum would be used to number the different USB speeds in all the libraries, then "hpcd->Init.speed" could be passed as an argument directly to USBD_LL_SetSpeed(). Now a case statement is used to translate between the different definitons, because the HAL, LL and PCD libraries each maintain their own.

If it's not possible to consolidate these, then at least they should be used consistently!

​

For instance with the following changes to "HAL_PCD_ResetCallback()":

void HAL_PCD_ResetCallback(PCD_HandleTypeDef *hpcd)
{ 
 USBD_SpeedTypeDef speed = USBD_SPEED_FULL;
 
 if ( hpcd->Init.speed == USB_OTG_SPEED_HIGH) // was: PCD_SPEED_HIGH
 {
 speed = USBD_SPEED_HIGH;
 }
 else if ( hpcd->Init.speed == USB_OTG_SPEED_FULL) // was: PCD_SPEED_FULL
 {
 speed = USBD_SPEED_FULL;
 }
 else
 {
 Error_Handler();
 }
 /* Set Speed. */
 USBD_LL_SetSpeed((USBD_HandleTypeDef*)hpcd->pData, speed);
 
 /* Reset Device. */
 USBD_LL_Reset((USBD_HandleTypeDef*)hpcd->pData);
}

And also this setting in "usbd_conf.c":

USBD_StatusTypeDef USBD_LL_Init(USBD_HandleTypeDef *pdev)
{
 ...
 hpcd_USB_OTG_FS.Init.speed = USB_OTG_SPEED_FULL; // was: PCD_SPEED_FULL
 ...
}

After these changes there doesn't seem to be any need for the PCD_SPEED_* defines anymore, they are not even used in "stm32f7xx_hal_pcd.c"!

In any way this wouldn't change anything in the HAL, but only Cube generated code and it seems to work fine.

[sasha]

Hello - I've found the same bug with CubeMX 5.2.1 & STM32F7 package 1.15.0. Error handler is always called.

STM: Is this bug going to be fixed soon?

[BPric.557]

Echoing others here... I am still experiencing this bug with the latest CubeMX and STM32F7 package 1.15.0...

This could be fixed by changing:

#define USB_OTG_SPEED_FULL 3U

to

#define USB_OTG_SPEED_FULL 2U

in stm32f7xx_ll_usb.h so that the USB_OTG_SPEED_x defines (HIGH, HIGH_IN_FULL, and FULL) match the PCD_SPEED_x defines in stm32f7xx_hal_pcd.h?

Or better yet, eliminate PCD_SPEED_x defines completely as suggested above?