-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
stm32/usb: Add support for using TinyUSB stack. #15592
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #15592 +/- ##
=========================================
Coverage ? 98.38%
=========================================
Files ? 171
Lines ? 22283
Branches ? 0
=========================================
Hits ? 21924
Misses ? 359
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Code size report:
|
1b2bbff
to
a8e3d9d
Compare
Yes, please do allow support for either the STM or TinyUSB stack. |
1964763
to
4c7f096
Compare
4c7f096
to
12ef683
Compare
12ef683
to
7b072e8
Compare
7b072e8
to
52a6912
Compare
100e19c
to
02eb292
Compare
3864708
to
0f058f7
Compare
0f058f7
to
07b5638
Compare
07b5638
to
958eb7b
Compare
162d4c7
to
6aef189
Compare
a3d0313
to
6f475eb
Compare
Code size change for stm32 is still very high.... |
Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
6f475eb
to
210c4bc
Compare
210c4bc
to
7d8ef77
Compare
I believe the size issue (when building with stm usb stack) has been resolved finally. Though I want to check exactly how it's now smaller... On a positive note, switching to TinyUSB actually reduces the size by ~5.4KB. |
Yes, that's a bit concerning that it's smaller. Probably something has been unintentionally removed/disabled. |
Signed-off-by: Andrew Leech <andrew@alelec.net>
7d8ef77
to
916f558
Compare
nice work, heads up from my side. would be cool to use all the „dynamic python-USB configuration“ staff in the stm32 port! |
Whoops, I sliced and diced the feature commit every which way trying to find the size difference befrore remember the other commit at the base of this branch: runtime/pyexec: Remove legacy usb irq enable code. That's the drop in size. |
I tried to test this on PYBD_SF6, PYBV10 and NUCLEO_WB55 but it did not work:
To enable TinyUSB I changed --- a/ports/stm32/mpconfigboard_common.h
+++ b/ports/stm32/mpconfigboard_common.h
@@ -142,7 +142,7 @@
// Select whether TinyUSB or legacy STM stack is used to provide USB.
#ifndef MICROPY_HW_TINYUSB_STACK
-#define MICROPY_HW_TINYUSB_STACK (0)
+#define MICROPY_HW_TINYUSB_STACK (1)
#endif and it's definitely using that config option. Did I do something wrong? |
@@ -136,6 +136,24 @@ | |||
#define MICROPY_HW_ENABLE_USB (0) | |||
#endif | |||
|
|||
#if MICROPY_HW_ENABLE_USB | |||
#define MICROPY_HW_USB_CDC (1) | |||
#define MICROPY_HW_USB_FS (1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These options cannot be enabled unconditionally, because they may clash with board configurations (when using the STM USB stack).
Either just remove these lines (my preference, if that works) or at least only enable them if MICROPY_HW_TINYUSB_STACK
is enabled.
@@ -256,6 +274,8 @@ | |||
// Windows needs a different PID to distinguish different device configurations. | |||
#ifndef MICROPY_HW_USB_VID | |||
#define MICROPY_HW_USB_VID (0xf055) | |||
#define MICROPY_HW_USB_PID (0x9802) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a comment that this is only used in the TinyUSB configuration (and eventually we'll need to change this value depending on whether MSC is enabled etc).
Thanks for the feedback @dpgeorge I'll address your specific review comments and test / fix it on the nucleo and pybd - I'll let you know when it's ready to test again |
Summary
This PR adapts the stm32 port to allow switching from STM USB stack to TinyUSB stack.
Using TinyUSB improves consistancy with other micropython ports and brings in the ability to use the runtime usb definition support recently added to other TinyUSB based ports.
TinyUSB can be enabled in a build with
#define MICROPY_HW_TINYUSB_STACK (1)
It's also enabled by default with
MICROPY_PREVIEW_VERSION_2
From the command line it can be enabled with:
make -C ports/stm32 BOARD=PYBV11 CFLAGS_EXTRA='-DMICROPY_HW_TINYUSB_STACK=1'
Testing
Development and testing so far has been on the STM32WB55 and STM32F765
I've been more recently using and testing this in conjunction with shared/tinyusb: Add support for USB Network (NCM) interface. #16459 on the STM32F765 where the builtin CDC and NCM endpoints are both working very well.
I haven't yet done significant testing of dynamic / runtime usb devices.
Trade-offs and Alternatives
Currently the TinyUSB integration matches other ports for features, so CDC is enabled for repl use and machine.USBDevice support is available by default.
This means however the existing stm32 / pyb usb support has been removed, including pyb.USB_VCP and pyb.usb_mode.
Features like mass storage, hid and a second vcp could in theory be added via
machine.USBDevice
however this hasn't been tested and is a breaking change for the stm32 port.TODO: test lightsleep behaviour / compared with #8304 (comment)