STMicroelectronics / STM32CubeH7

STM32Cube MCU Full Package for the STM32H7 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
https://www.st.com/en/embedded-software/stm32cubeh7.html
Other
479 stars 298 forks source link

USBCDC not working with V1.11, but OK with V1.10 #251

Closed frondxl closed 5 months ago

frondxl commented 1 year ago

After upgrade my project with v1.11, the USB port is no longer seen win Win11 device manager. So I created a simple project on Nucleo-H743ZI2 board (mcu revV), when I use V1.10 firmware the USB CDC worked fine, but with 1.11, I don't see the port. IOC file attached. My IDE is Keil MDK 5.38a.

N743_CDC.zip

SarimMuqeet commented 1 year ago

Hello, I am currently experiencing a similar issue, although in IAR IDE. I've upgraded from V1.9.0 to 1.11.0 and this is the change that is causing my port detection to fail, with the left side being V1.11.0 and right side being V1.9.0: image

This is a new change in V1.11.0, as V1.9.0 and 1.10.0 both use the same conditions for the if statement. Also, this image is specifically from the the HAL_PCD_Start function, although similar conditions are updated in other processes such as HAL_PCD_Stop, DevConnect, DevDisconnect, etc.

radioflash commented 1 year ago

This change also broke USB for my device (custom class, Full-Speed internal interface on STM32H730).

I have investigated this and here is what I found out: In short: v1.11.0 contains incorrect/nonworking code for STM32H7 controllers that breaks under certain conditions.

How to trigger the bug

The most urgent problem occurs when the device is set to "battery charging enable" (implies VBUS sensing). It also only happens with the internal Full-Speed USB PHY (external High-Speed PHY should be unaffected but I was unable to verify that).

Detailed bug description

What should happen: With battery_charging_enable == 0, the internal full-speed PHY should be turned on during USB-Initialization With battery_charging_enable == 1, the internal full-speed PHY should be turned on during USB-Start

USB-Start frequently happens directly after initialization.

Note: Turning on the internal PHY happens via USBx->GCCFG |= USB_OTG_GCCFG_PWRDWN; which looks wrong but the bit/flag just has a very confusing name: if that bit is set, then the internal PHY is powered ON, not powered down.

But turning on the PHY during USB-Start NEVER works on H7, because it is reading the PRODUCT_ID register of the USB peripheral. Note: This is NOT the 16bit USB Product ID (that can be used together with the vendor ID to identify a device). Instead, this seems to be an ST-specific field that is exclusively used internally.

That register used to give different values depending on which PHY was selected for e.g. the STM32F7, but that simply does not work for the H7: The check expects a product ID where bit 8 indicates "High speed external PHY".

For an F75xx this works, because you get either 0x00002000 (Full speed internal PHY) or 0x00002100 (High speed external PHY).

But the H7 ALWAYS gives 0x00002300 (=> note how bit 8 is set): That makes the HAL_PCD_Start function think that the external high-speed PHY is used, even if that is not the case. And this happens in multiple places, and presumably breaks other things, too (speed selection, endpoint fifo buffer size configuration, etc.).

SUGGESTED FIX

Just revert this change. This is simply a regression, as far as I can tell every occurence of USBx->CID & (0x1U << 8) is a bug, because this is not suitable for determining whether the external high-speed PHY is used or not on STM32H7 (it would e.g. work on STM32F75xx).

Instead this could be used: (USBx->GUSBCFG & USB_OTG_GUSBCFG_PHYSEL_Msk) != 0U that flag is non-zero when the internal full-speed PHY is used, and exists both on STM32F75xx and STM32H7.

@ALABSTM I can do a pull request if you tell me how you want this bug fixed (already signed the CLA).

ALABSTM commented 1 year ago

Hi guys,

Thank you very much for all these details. This issue has been logged into an internal bug tracker at the attention of our development teams. I'll keep you informed. Thank you again for your reports.

With regards,

ALABSTM commented 1 year ago

ST Internal Reference: 157168

ALABSTM commented 5 months ago

Fixed in commit 37cbebdbc41f2fbbab3c57af0190ea097fe9b842