Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stm32u5a9xx.h USB_OTG mess #31

Open
tdjastrzebski opened this issue Sep 19, 2023 · 9 comments
Open

stm32u5a9xx.h USB_OTG mess #31

tdjastrzebski opened this issue Sep 19, 2023 · 9 comments
Assignees
Labels
bug Something isn't working cmsis CMSIS-related issue or pull-request. internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system usb USB-related (host or device) issue or pull-request
Milestone

Comments

@tdjastrzebski
Copy link

tdjastrzebski commented Sep 19, 2023

  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 RJMSTM self-assigned this Sep 20, 2023
@RJMSTM RJMSTM added the cmsis CMSIS-related issue or pull-request. label Sep 20, 2023
@RJMSTM
Copy link
Contributor

RJMSTM commented Sep 20, 2023

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
Copy link
Author

tdjastrzebski commented Sep 21, 2023

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
Copy link
Author

tdjastrzebski commented Sep 28, 2023

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 ALABSTM added bug Something isn't working hal HAL-LL driver-related issue or pull-request. labels Oct 13, 2023
@ALABSTM ALABSTM added usb USB-related (host or device) issue or pull-request and removed hal HAL-LL driver-related issue or pull-request. labels Nov 29, 2023
@ALABSTM
Copy link
Contributor

ALABSTM commented Jan 12, 2024

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 ALABSTM added the internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system label Jan 12, 2024
@ALABSTM
Copy link
Contributor

ALABSTM commented Jan 12, 2024

ST Internal Reference: 170735

@ALABSTM
Copy link
Contributor

ALABSTM commented Jan 12, 2024

ST Internal Reference: 170738

@ALABSTM
Copy link
Contributor

ALABSTM commented Jan 12, 2024

ST Internal Reference: 170739

@ALABSTM
Copy link
Contributor

ALABSTM commented Jan 25, 2024

ST Internal Reference: 171759

@TOUNSTM
Copy link
Contributor

TOUNSTM commented Apr 5, 2024

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 2e053bc.

With Regards,

@ALABSTM ALABSTM added this to the v1.7.0 milestone Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cmsis CMSIS-related issue or pull-request. internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system usb USB-related (host or device) issue or pull-request
Projects
Status: In progress
Development

No branches or pull requests

4 participants