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

drivers: usb: udc: add STM32 UDC driver #53408

Merged
merged 2 commits into from Jul 13, 2023

Conversation

loicpoulain
Copy link
Collaborator

@loicpoulain loicpoulain commented Jan 1, 2023

Add UDC driver for STM32 based MCU, relying on HAL/PCD.

Implementing: #42066

Tested-on:
96b_carbon - cdc_acm sample - CONF_FILE=usbd_next_prj.conf
disco_l475_iot1 - cdc_acm sample - CONF_FILE=usbd_next_prj.conf
nucleo_wb55rg - cdc_acm sample - CONF_FILE=usbd_next_prj.conf
nucleo_h723zg - cdc_acm sample - CONF_FILE=usbd_next_prj.conf

west build -b nucleo_wb55rg -- -DCONF_FILE=usbd_next_prj.conf

@loicpoulain loicpoulain added the DNM This PR should not be merged (Do Not Merge) label Jan 1, 2023
@carlescufi carlescufi added the area: USB Universal Serial Bus label Jan 1, 2023
@desowin desowin requested review from tmon-nordic and removed request for desowin January 26, 2023 13:26
@loicpoulain
Copy link
Collaborator Author

@erwango , this PR now working with udc next stack and 96b_carbon, still need a bit of cleanup and should be ready for review/testing.

@loicpoulain loicpoulain marked this pull request as ready for review March 6, 2023 17:27
@zephyrbot zephyrbot added the platform: STM32 ST Micro STM32 label Mar 6, 2023
@loicpoulain loicpoulain changed the title [ONGOING] drivers: usb: udc: add STM32 UDC driver drivers: usb: udc: add STM32 UDC driver Mar 6, 2023
@loicpoulain loicpoulain removed the DNM This PR should not be merged (Do Not Merge) label Mar 6, 2023
#include <zephyr/logging/log.h>
#include <zephyr/irq.h>

#include <zephyr/logging/log.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

included second time, see L27

Comment on lines +33 to +38
#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32_otghs)
#define DT_DRV_COMPAT st_stm32_otghs
#elif DT_HAS_COMPAT_STATUS_OKAY(st_stm32_otgfs)
#define DT_DRV_COMPAT st_stm32_otgfs
#elif DT_HAS_COMPAT_STATUS_OKAY(st_stm32_usb)
#define DT_DRV_COMPAT st_stm32_usb
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will it work with multiple driver instances, FS + HS?

Copy link
Contributor

Choose a reason for hiding this comment

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

The way I remember it, the HS and FS are mutually exclusive at the hardware level. Do I recall correctly or can these actually be used simultaneously?

Copy link
Member

Choose a reason for hiding this comment

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

It appears to be possible, see stm32f7-usbfs-and-usbhs-together This being said, it isn't possible in current driver version so this is not a new limitation which is being added here. Driver could be converted later to multiple, heterogeneous instances.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay. Just for notice, the native implementation for DWC2 controller will support multiple FS/HS instances combination.

{
struct udc_stm32_data *priv = hpcd2data(hpcd);

LOG_DBG("");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not mind, but this kind of tracing is now unwanted.
Better something like LOG_DBG("Submit reset event");.

}
#endif /* USB_OTG_FS || USB_OTG_HS */

static void __pcd_prepare(const struct device *dev)
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIRC, we must not use __ for this, better hal_driver_ prefix, or priv_, or instance_.

@loicpoulain
Copy link
Collaborator Author

@erwango , it should work now with nucleo_wb55rg!

tmon-nordic
tmon-nordic previously approved these changes May 9, 2023
@erwango erwango added the block: HW Test Testing on hardware required before merging label May 9, 2023
@erwango
Copy link
Member

erwango commented May 9, 2023

Thanks for the fix. Performing extended validation, I'm seeing following issues:

  • stm32f3_disco > compil failing
  • nucleo_h723zg > cdc_acm failing
  • b_u585_iot02a > cdc_acm failing

Copy link
Collaborator

@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.

@loicpoulain can you please document in the message what STM32 platform is supported/tested in this state of the driver (g.e STM32F4 family is supported) so we can move forward? Support for additional platforms can be added later. Thanks.

@erwango erwango self-requested a review June 28, 2023 15:11
@erwango erwango dismissed their stale review June 28, 2023 15:12

As this is still experimental, ok to move on.

@loicpoulain
Copy link
Collaborator Author

@loicpoulain can you please document in the message what STM32 platform is supported/tested in this state of the driver (g.e STM32F4 family is supported) so we can move forward? Support for additional platforms can be added later. Thanks.

Done, note that my testing shows that b_u585i_iot2a fails with cdc_acm sample at runtime, but it seems the same with 'legacy' driver, so... need further investigations.

Loic Poulain added 2 commits July 12, 2023 13:46
Add UDC driver for STM32 based MCU, relying on HAL/PCD.
This has been tested with cdc_acm sample on the following boards:

- 96b_carbon (STM32F4)
- disco_l475_iot1 (STM32L4)
- nucleo_wb55rg (STM32WB)
- nucleo_h723zg (STM32H7)
- stm32f3_disco (STM32F3)

This fails at runtime for the following:

- b_u585i_iot2a (STM32U5)

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
This board can rely on udc_stm32 usbd-next driver.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
@carlescufi carlescufi merged commit df77491 into zephyrproject-rtos:main Jul 13, 2023
17 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 block: HW Test Testing on hardware required before merging platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants