STM32CubeU5 icon indicating copy to clipboard operation
STM32CubeU5 copied to clipboard

stm32u5a9xx.h USB_OTG mess

Open tdjastrzebski opened this issue 2 years ago • 11 comments

  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.

tdjastrzebski avatar Sep 19 '23 21:09 tdjastrzebski

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,

RJMSTM avatar Sep 20 '23 16:09 RJMSTM

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

  • GOTGCTL.CURMOD
  • GOTGCTL.OTGVER
  • GOTGCTL.ASVLD
  • GOTGCTL.DBCT
  • GOTGCTL.CIDSTS
  • GOTGCTL.EHEN
  • and USB_OTG_GOTGCTL_BSESVLD name is inconsistent with docs (GOTGCTL.BSVLD)

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

tdjastrzebski avatar Sep 21 '23 22:09 tdjastrzebski

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.

tdjastrzebski avatar Sep 28 '23 19:09 tdjastrzebski

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 avatar Jan 12 '24 17:01 ALABSTM

ST Internal Reference: 170735

ALABSTM avatar Jan 12 '24 17:01 ALABSTM

ST Internal Reference: 170738

ALABSTM avatar Jan 12 '24 17:01 ALABSTM

ST Internal Reference: 170739

ALABSTM avatar Jan 12 '24 17:01 ALABSTM

ST Internal Reference: 171759

ALABSTM avatar Jan 25 '24 16:01 ALABSTM

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,

TOUNSTM avatar Apr 05 '24 14:04 TOUNSTM