search

STMicroelectronics / STM32CubeU5

Full Firmware Package for the STM32U5 series: HAL+LL drivers, CMSIS, BSP, MW, plus a set of Projects (examples and demos) running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits).
Other
116 stars 61 forks source link

stm32u5a9xx.h USB_OTG mess #31

Open tdjastrzebski opened 10 months ago

tdjastrzebski commented 10 months ago

  1. File stm32u5a9xx.h, line 27339: #define USB_OTG_GCCFG_FSVMINUS_Msk 0x1U << USB_OTG_GCCFG_FSVMINUS_Pos) /*!< 0x00000004 */ has missing opening parenthesis!

  2. GINTSTS.RSTDET register field definition (USB_OTG_GINTSTS_RSTDET) is missing. See: RM0456, 73.14.6 OTG core interrupt register [alternate] (OTG_GINTSTS)

  3. Many USB_OTG register field names differ from those described in RM0456. Some of them are completely different. Examples: GINTSTS.WKUPINT -> USB_OTG_GINTSTS_WKUINT GINTSTS.IPXFR -> USB_OTG_GINTSTS_PXFR_INCOMPISOOUT GINTSTS.GONAKEFF -> USB_OTG_GINTSTS_BOUTNAKEFF GCCFG.FORCEHOSTPD -> USB_OTG_GCCFG_PULLDOWNEN GCCFG.VBVALOVEN -> USB_OTG_GCCFG_VBVALEXTOEN GCCFG.HVDMSRCEN -> USB_OTG_GCCFG_H_VDMSRCEN GCCFG.HCDPDETEN -> USB_OTG_GCCFG_H_CDPDETEN GCCFG.HCDPEN -> USB_OTG_GCCFG_H_CDPEN

  4. It is difficult to understand why only one particular register - OTG_HPRT (RM0456, 73.14.30) is defined as USBx_HPRT0 in stm32u5xx_ll_usb.h header, rather than in stm32u5a9xx.h where it belongs to. USB_OTG_HostTypeDef might be a better place to define it. The same time, this register's fields are defined in stm32u5a9xx.h and correctly named USB_OTG_HPRT_PSPD etc. Not to mention that USBx_HPRT0 definition depends on USBx_BASE defined later like this. I think it is overengineered and overcomplicated. OTG host supports only one port. If one really feels like more then one USB port needs to be handled in the future, why not to simply define USB1_HPRT0? - just like e.g. UCPD1 is defined. This is just inconsistent with how other IPs' registers are defined.

It is difficult to avoid impression that this STM32U5 HAL release was prepared in rush and never updated after the first RM0456 release version had been finalized.

RJMSTM commented 9 months ago

Hello @tdjastrzebski,

Thank you for this report. We will get back to you as soon as we analyze it further. This may take some time. Thank you for your comprehension.

With Regards,

tdjastrzebski commented 9 months ago

Hello @RJMSTM. I just discovered that more USB_OTG register fields definitions are missing.

See RM0456, 73.14.1 OTG control and status register (OTG_GOTGCTL)

tdjastrzebski commented 9 months ago

There are two more USB_OTG related register flags which in RM0465 are named RCC.AHB2ENR1.OTGEN and RCC.AHB2ENR1.OTGHSPHYEN (section 11.8.30 RCC AHB2 peripheral clock enable register 1). In HAL we have RCC_AHB2ENR1_OTGEN, RCC_AHB2ENR1_USBPHYCEN, LL_AHB2_GRP1_PERIPH_OTG_HS and LL_AHB2_GRP1_PERIPH_USBPHY corresponding constants as well as __HAL_RCC_USB_OTG_HS_CLK_ENABLE() and __HAL_RCC_USBPHYC_CLK_ENABLE() macros. This probably could not be more inconsistent.

ALABSTM commented 6 months ago

Hi @tdjastrzebski,

Please excuse this delayed reply. Thank you very much for this detailed report. It has been forwarded to our development teams. I will keep you informed.

With regards,

ALABSTM commented 6 months ago

ST Internal Reference: 170735

ALABSTM commented 6 months ago

ST Internal Reference: 170738

ALABSTM commented 6 months ago

ST Internal Reference: 170739

ALABSTM commented 5 months ago

ST Internal Reference: 171759

TOUNSTM commented 3 months ago

Hello @tdjastrzebski,

  1. File stm32u5a9xx.h, line 27339: #define USB_OTG_GCCFG_FSVMINUS_Msk 0x1U << USB_OTG_GCCFG_FSVMINUS_Pos) /*!< 0x00000004 */ has missing opening parenthesis!

The issue you have raised has been fixed in commit 2e053bcab594083bc519a69fbc72bb95d57881db.

With Regards,