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

usb: stm32: fix FIFO allocation sizes #70795

Merged

Conversation

arbrauns
Copy link
Contributor

@arbrauns arbrauns commented Mar 27, 2024

See commit messages.

Fixes #70789.

@@ -132,12 +132,13 @@ static const struct gpio_dt_spec ulpi_reset =

/* We need a minimum size for RX FIFO */
#define USB_FIFO_RX_MIN 160
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original PR (#62530) doesn't explain where this number comes from, and doesn't clarify whether it is supposed to be bytes or words. @Desvauxm-st?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Judging by #69897, it's probably supposed to be words, updated accordingly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @Desvauxm-st is now enjoying his retirement. @marwaiehm-st will have a look.

Copy link
Collaborator

@marwaiehm-st marwaiehm-st Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arbrauns yes it's supposed to be words, and for the PR (#62530) the documentation of the RX FIFO minimum value was not clear so we tried to find a value that make the usb sample tests worked on the board stm32h747i_disco

The RX FIFO size is in words, so needs to be subtracted from the total
memory size *after* it's divided by 4.

Fixes zephyrproject-rtos#70789.

Signed-off-by: Armin Brauns <armin.brauns@embedded-solutions.at>
Magic constants throughout the code made this difficult to reason about,
especially with two different units of measurement (bytes and words) at
play.

Signed-off-by: Armin Brauns <armin.brauns@embedded-solutions.at>
@marwaiehm-st
Copy link
Collaborator

LGTM

@jhedberg jhedberg merged commit e69ce5b into zephyrproject-rtos:main Apr 8, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: USB Universal Serial Bus platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

usb: writing to CDC-ACM interface other than first one crashes USB stack on stm32f767
6 participants