Skip to content

Conversation

@kbidani
Copy link
Contributor

@kbidani kbidani commented Apr 4, 2025

This pull request fix the USB speed configuration by leveraging the device tree's status, ensuring the correct full-speed setting is applied dynamically.

@kbidani kbidani force-pushed the fix_usb_clean_speed branch 7 times, most recently from 2cb412b to 33b1153 Compare April 4, 2025 12:43
Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

For commit "drivers: usb: udc_stm32: fix maximum speed for usb_fs":
Note the CI test failing on commit message containing line > 74 chars. -> addressed
Since this fixes a previous commit, could you add a Fixes: tag in the commit message:
Fixes: ba1ad38f998d02d624e9d2f6d1605a5fbbdf44f1

For commit "drivers: usb: device_stm32: fix maximum speed for usb_f":
Fixes: 01ddccdf626a2d767d4b76b648fc9e520c35ded5

kbidani added 2 commits April 4, 2025 15:06
update the definition of FULL_SPEED based on the presence of USB and
USB_DRD_FS.

Signed-off-by: Khaoula Bidani <khaoula.bidani-ext@st.com>
update the definition of FULL_SPEED based on the presence of USB
and USB_DRD_FS.

Signed-off-by: Khaoula Bidani <khaoula.bidani-ext@st.com>
@kbidani kbidani force-pushed the fix_usb_clean_speed branch from 33b1153 to f0217f9 Compare April 4, 2025 13:06
@kbidani kbidani marked this pull request as ready for review April 4, 2025 13:11
@kbidani kbidani requested a review from etienne-lms April 4, 2025 13:11
@github-actions github-actions bot added platform: STM32 ST Micro STM32 area: USB Universal Serial Bus labels Apr 4, 2025
@kbidani kbidani changed the title Fix usb clean speed USB STM32: correct compilation error for USB clean speed configuration Apr 4, 2025
@fabiobaltieri fabiobaltieri added the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Apr 6, 2025
@jfischer-no jfischer-no removed the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Apr 6, 2025
Comment on lines +53 to +57
#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32_usb)
#define FULL_SPEED PCD_SPEED_FULL
#else
#define FULL_SPEED USB_OTG_SPEED_FULL
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

HIGH_SPEED/FULL_SPEED is a very bad choice. I see this was introduced in 01ddccd. No idea how this could get enough approvals and be merged. Not that there are no macros that are already prefixed correctly with UDC_STM32_.

Copy link
Member

Choose a reason for hiding this comment

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

@jfischer-no so you are suggesting these should be renamed to UDC_STM32_FULL_SPEED and UDC_STM32_FULL_SPEED? Not super clear to me from your comment, trying to land this to an approvable state with few turnarounds since it break the build for load of platforms.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

it break the build for load of platforms.

Just revert #86922

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, I think it's fine to wait for a fix forward no need to rush a revert, just wanted to go straight to the right fix.

Copy link
Member

Choose a reason for hiding this comment

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

Just revert #86922

Given that there's still no progress on this, I suggest we perform a revert. This is breaking downstream users/CI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Outdated, should be fixed by #88244

Copy link
Contributor

@jfischer-no jfischer-no left a comment

Choose a reason for hiding this comment

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

drivers: usb: udc_stm32: fix maximum speed for usb_fs

What is usb_fs?

update the definition of FULL_SPEED based on the presence of USB
and USB_DRD_FS.

What does "presence of USB" mean?

@jfischer-no
Copy link
Contributor

See #88244

@jfischer-no jfischer-no closed this Apr 8, 2025
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.

6 participants