pkg/tinyusb: add tinyUSB as package by gschorcht · Pull Request #18592 · RIOT-OS/RIOT

gschorcht · 2022-09-14T08:09:43Z

Contribution description

This PR adds tinyUSB open-source cross-platform USB Host/Device stack for embedded systems as package.

The initial intention was to add USB support for ESP32-S2 and ESP32-S3 since the tinyUSB stack includes a hardware driver for these ESP32x SoCs. The ESP-IDF also uses tinyUSB for USB support, but integrates tinyUSB. Instead of using the tinyUSB from ESP-IDF, this PR adds tinyUSB as a package to be able to use it for other platforms like STM32 and nRF. STM32 support is also provided by this PR.

Testing procedure

Compile and flash the test application tests/pkg_tinyusb_cdc_msc.

BOARD=esp32s3-devkit make -j8 -C tests/pkg_tinyusb_cdc_msc flash

It should be possible to use any STM32 board that also supports feature periph_usbdev

This application uses the tinyUSB device stack to emulate a mass storage device (MSC) with a communication interface (CDC).

Once the application is flashed, the device should be mounted when it is
connected to a host. That is,

the mass storage interface is mounted as volume TinyUSB MSC in the operating system and
the communication interface is mounted as a serial device, for example as /dev/ttyACM0 on Linux.

It should then be possible

to read from and write to the mass storage device with the usual file operations of the operating system
to connect to the serial interface of the device with a terminal program, e.g.
```
python -m serial.tools.miniterm /dev/ttyACM0 115200
```
and get the entered characters sent back.

The test application uses LED0, if present, to indicate the status of the device by blinking at different frequencies.

Issues/PRs references

dylad

Thanks @gschorcht That's a pretty nice package !
I took a quick glance at it. I think I have a supported ST boards somewhere in my basement so I should be able to run some tests. But I cannot test on ESP side.
I'll dive further when I'll run some tests.

Makefile.features

pkg/tinyusb/Kconfig

pkg/tinyusb/Makefile.dep

pkg/tinyusb/doc.txt

pkg/tinyusb/hw/Makefile

tests/pkg_tinyusb_cdc_msc/README.md

dylad · 2022-09-16T21:03:55Z

@gschorcht I give this PR a try with a stm32f429i-disc1 but with no luck so far. It seems we are stuck here
Which STM32-based did you try on your side ?

gschorcht · 2022-09-17T07:10:48Z

@gschorcht I give this PR a try with a stm32f429i-disc1 but with no luck so far. It seems we are stuck here Which STM32-based did you try on your side ?

Hm, I have tested it with a nucleo-f767zi withouth any problems. So I thought that it works with STM32F4 family supported by the tinyUSB also.

gschorcht · 2022-09-17T07:14:05Z

@gschorcht I give this PR a try with a stm32f429i-disc1 but with no luck so far. It seems we are stuck here Which STM32-based did you try on your side ?

If it sucks here, the Synopsys DWC2 core seems to be not available at this point. I will check.

gschorcht · 2022-09-22T10:29:09Z

@gschorcht I give this PR a try with a stm32f429i-disc1 but with no luck so far. It seems we are stuck here Which STM32-based did you try on your side ?

If it sucks here, the Synopsys DWC2 core seems to be not available at this point. I will check.

@dylad Could you please check the DWC2_CORE_REV used by STM32F429i at address 0x50000040 with debugger or printf?

printf("XXXX %08lx\n", *((uint32_t *)0x50000040);

or

printf("XXXX %08lx\n", _global_regs->Reserved40[0]);

in function tinyusb_hw_init_dev in pkg/tinyusb/hw/hw_stm32.c below periph_clk_en(conf->ahb, conf->rcc_mask);

RIOT/pkg/tinyusb/hw/hw_stm32.c

Lines 36 to 37 in 369f8c0

	periph_clk_en(conf->ahb, conf->rcc_mask);

dylad · 2022-09-22T19:02:19Z

@gschorcht
I've added

    printf("XXXX %08lx\n", *((uint32_t *)0x50000040));
    printf("XXXX %08lx\n", global_regs->Reserved40[0]);
    printf("XXXX %08lx\n", *((uint32_t *)0x50000040));

just after the periph_clk_en() function as requested.

Got this output:

2022-09-22 21:00:28,281 # main(): This is RIOT! (Version: 2022.10-devel-586-gdc3257-pkg/tinyusb)
2022-09-22 21:00:28,471 # XXXX 00000000
2022-09-22 21:00:28,472 # XXXX 4f54281a
2022-09-22 21:00:28,473 # XXXX 00000000

dylad · 2022-09-22T19:09:15Z

I've replaced 0x50000000ULby 0x40040000UL and I got the same result as global_regs->Reserved40[0]
Seems like we are using USB_OTG_HS_PERIPH_BASE, not USB_OTG_FS_PERIPH_BASE

dylad · 2022-09-22T20:26:33Z

I've made some progress with:
CFLAGS="-DTINYUSB_TUD_RHPORT=1" make BOARD=stm32f429i-disc1 -C tests/pkg_tinyusb_cdc_msc flash
I'm not stuck anymore but enumeration fails now. I'll investigate further this weekend.

gschorcht · 2022-09-23T04:17:38Z

I've replaced 0x50000000ULby 0x40040000UL and I got the same result as global_regs->Reserved40[0]
Seems like we are using USB_OTG_HS_PERIPH_BASE, not USB_OTG_FS_PERIPH_BASE

Ah, I see, the peripheral configuration includes cfg_usb_otg_hs_fs.h instead of cfg_usb_otg_fs.h. It seems that the stm32f429-dis1 board is the only STM32 board that uses the OTG HS controller instead of the OTG FS controller which is RHPORT=1 in tinyUSB terminology for whatever RHPORT stands. And, only the OTG HS controller seems to be a Sysopsys DWC2 core.

chrysn · 2022-09-30T17:11:52Z

pkg/tinyusb/doc.txt

+ *     ...
+ *     // initialize the tinyUSB stack including used peripherals and
+ *     // start the tinyUSB thread
+ *     tinyusb_setup();


Why is this not auto-init'ed?

Do you mind if we add this in a followup PR ?
This PR is already in good shape.

Why is this not auto-init'ed?

Good question. If it is possible to create a thread during the auto-init and the scheduler is already running, it should be possible.

I have a local commit that implements auto-initialization. It only needed a few changes:

pkg/tinyusb/Makefile.dep | 2 ++ pkg/tinyusb/contrib/tinyusb.c | 13 +++++++++++++ pkg/tinyusb/doc.txt | 6 ++++-- sys/auto_init/include/auto_init_priorities.h | 6 ++++++ tests/pkg_tinyusb_cdc_msc/main.c | 4 ---- 5 files changed, 25 insertions(+), 6 deletions(-)

Should I provide it or should we add this in a followup PR?

See commit 1879a1c.

chrysn · 2022-09-30T17:13:43Z

pkg/tinyusb/doc.txt

+ *
+ * tinyUSB is an open-source cross-platform USB Host/Device stack for
+ * embedded systems.
+ *


How does tinyUSB relate to the usbus native RIOT modules? Is it (or can it be) used on top of the existing usbus? Or is it mutually exclusive? (E.g. because the set of supported hardware is distinct -- and what when one gains support for a device another has?)

tinyUSB and USBUS are mutually exclusive.
tinyUSB provides both the USB stack and the lowlevel driver whereas usbus depends on periph_usbdev
FWICT, tinyUSB has more features and support more lowlevel peripherals than usbus/periph_usbdev but as @gschorcht pointed out here, it looks like tinyUSB uses more RAM/ROM than our stack.

How does tinyUSB relate to the usbus native RIOT modules?

As @dylad said, tinyUSB includes the USB host and device stack as well as device drivers for a variety of platforms. Also, the stack supports many more device classes than usbus.

Is it (or can it be) used on top of the existing usbus? Or is it mutually exclusive? (E.g. because the set of supported hardware is distinct -- and what when one gains support for a device another has?)

tinyUSB device driver use the same hardware as periph_usbdev. So either tinyUSB or usbus can be used. But, theoretically, it should be possible to implement an interface to use the tinyUSB stack on-top of a periph_usbdev device driver or to use usbus on top of a tinyUSB device driver. This would require to map the interface of tinyUSB device drivers to the periph_usbdev interface.

dylad · 2022-09-30T18:24:55Z

Makefile.features

+FEATURES_CONFLICT_MSG += "Only one standard C library can be used."

 FEATURES_CONFLICT += periph_gpio_irq:periph_gpio_ll_irq
-FEATURES_CONFLICT_MSG += "Only one GPIO IRQ implementation can be used"
+FEATURES_CONFLICT_MSG += "Only one GPIO IRQ implementation can be used."


Shouldn't this belong to another PR ?

Hm, it's such a small change that improves readability. Since we are touching the file anyway, I thought we could improve it. Without this small change, the message looks like this:

Rationale: Only one standard C library can be used Only one GPIO IRQ implementation can be used Package tinyUSB is not yet compatible with periph_usbdev.

That is, the message is completely unstructured.

With this small change, the message looks like this:

Rationale: Only one standard C library can be used. Only one GPIO IRQ implementation can be used. Package tinyUSB is not yet compatible with periph_usbdev.

That is, the problem is that all messages are simply strung together without punctuation.

dylad · 2022-09-30T20:33:34Z

Got tinyUSB working on my SAME54-XPRO 😍

gschorcht · 2022-10-01T06:49:31Z

Got tinyUSB working on my SAME54-XPRO heart_eyes

I got USB OTG HS with external ULPI PHY working on stm32f746g-disco.

gschorcht · 2022-10-01T09:39:13Z

This should result into following definitions that could be overridden in the makefile:

the board uses only USB OTG FS: TINYUSB_TUD_RHPORT=0 and TINYUSB_TUH_RHPORT=0

the board uses only USB OTG HS: TINYUSB_TUD_RHPORT=1 and TINYUSB_TUH_RHPORT=1

the board uses both: TINYUSB_TUD_RHPORT=0 and TINYUSB_TUH_RHPORT=1

Now that I am experimenting with the USB OTG HS port with UPLI HS PHY on the board stm32f746g-disco, I am wondering if defining these defaults is really the way to go. tinyUSB host stack does not work at all for STM32 and the other platforms that RIOT supports.

The reason is that tinyUSB needs one driver for the USB device stack according to the API defined in tinyusb/src/device/dcd.h and another one for the USB host stack according to the API defined in tinyusb/src/host/hcd.h. At the moment, tinyUSB has only USB device stack drivers for STM32 and the other platforms that RIOT supports. That is, the host stack can't be used at the moment.

To use bothe the USB OTG FS and the USB OTG HS port for the device stack, we have to change the logic. Defining TINYUSB_TUD_RHPORT is not sufficient. We would need something like TINYUSB_TUD0_RHPORT and TINYUSB_TUD1_RHPORT.

gschorcht · 2022-10-01T11:21:49Z

It seems that the stm32f429-dis1 board is the only STM32 board that uses the OTG HS controller instead of the OTG FS controller which is RHPORT=1 in tinyUSB terminology for whatever RHPORT stands.

Ah, now I got it. RHPORT stands for Root-Hub port.

dylad

Last comment on my side. Feels free to squash at the same time.

dylad · 2022-10-03T19:29:11Z

cpu/stm32/include/tinyusb_hw_defaults.h

+#define TINYUSB_TUH_RHPORT  0
+#endif
+
+#define HSE_VALUE           CLOCK_HSE


Why is this define here ?
Could you add a comment about that if it's really needed please ?

Why is this define here ?

Hm, it could be a leftover from trying to use tinyusb/src/portable/st/synopsys/dcd_synopsys.c as a driver. I have removed it. Let's see if Murdock is happy. If so, it was just a leftover. If not, Murdock will tell us why.

Indeed, Murdock has the answer ! This was not a leftover.

Hm, it's not really to me, why this is required for this one board only.

Ah ok, I see. stm32f723e-disco is the only board that uses a STM32F723 for which the CMSIS defines USB_HS_PHYC, that is, the internal UTMI HS PHY is used. Interesting, this will help us to support also UTMI in RIOT's Synopsys DWC2 driver for OTG HS interfaces (PR #18679).

BTW, according to the schematic, the stm32f723e-disco is also a board with USB OTG FS as well as USB OTG HS connector. The internal UTMI HS PHY is used with the USB OTG HS interface.

dylad · 2022-10-03T19:47:35Z

To use bothe the USB OTG FS and the USB OTG HS port for the device stack, we have to change the logic. Defining TINYUSB_TUD_RHPORT is not sufficient. We would need something like TINYUSB_TUD0_RHPORT and TINYUSB_TUD1_RHPORT

The current state looks good to me, we can prepare something later to enable support for both controllers at the same time.

Some error checks had to be removed to get `make info-boards-supported` working.

dylad

ACK.
Code looks good, tested work on stm32f429i-disc1 and I trust @gschorcht testing on ESP32.

dylad · 2022-10-04T11:08:31Z

Here we go.

gschorcht · 2022-10-04T11:20:24Z

@dylad Thanks for reviewing, testing and merging.

gschorcht requested review from MichelRottleuthner, MrKevinWeiss, aabadie, fjmolinas, jia200x, leandrolanzieri and smlng as code owners September 14, 2022 08:09

gschorcht added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 14, 2022

benpicco requested review from bergzand and dylad September 14, 2022 08:37

dylad reviewed Sep 14, 2022

View reviewed changes

gschorcht added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Sep 17, 2022

gschorcht mentioned this pull request Sep 22, 2022

cpu/esp32: add periph_usbdev for ESP32-S2 and ESP32S3 #18624

Closed

gschorcht force-pushed the pkg/tinyusb branch from c21869f to 8cdd96b Compare September 30, 2022 17:07

chrysn reviewed Sep 30, 2022

View reviewed changes

dylad reviewed Sep 30, 2022

View reviewed changes

github-actions bot added the Area: sys Area: System label Oct 1, 2022

gschorcht force-pushed the pkg/tinyusb branch from 154152d to 7f25fab Compare October 1, 2022 07:58

gschorcht force-pushed the pkg/tinyusb branch from 7f25fab to 1879a1c Compare October 1, 2022 11:02

dylad reviewed Oct 3, 2022

View reviewed changes

gschorcht added 3 commits October 4, 2022 07:40

pkg/tinyusb: add tinyusb_hw_defaults.h for platform specific defaults

9cf0119

pkg/tinyusb: remove some error checks from Makefiles

bb883cd

Some error checks had to be removed to get `make info-boards-supported` working.

pkg/tinyusb: use auto_init for tinyusb stack and thread setup

bb337cd

gschorcht force-pushed the pkg/tinyusb branch from 1879a1c to bb337cd Compare October 4, 2022 05:40

gschorcht mentioned this pull request Oct 4, 2022

cpu/stm32/periph/usbdev_fs: add support for STM32F3 family #18680

Merged

cpu/stm32: define HSE_VALUE for tinyUSB Synopsys DWC2 driver

7507131

dylad approved these changes Oct 4, 2022

View reviewed changes

dylad merged commit 11aebb6 into RIOT-OS:master Oct 4, 2022

dylad added this to the Release 2022.10 milestone Oct 4, 2022

gschorcht deleted the pkg/tinyusb branch October 22, 2022 11:36

gschorcht mentioned this pull request Dec 4, 2022

pkg/tinyusb: add Kconfig support for stdio_tinyusb_cdc_acm #19007

Merged

Conversation

gschorcht commented Sep 14, 2022

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

dylad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dylad commented Sep 16, 2022

Uh oh!

gschorcht commented Sep 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschorcht commented Sep 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschorcht commented Sep 22, 2022

Uh oh!

dylad commented Sep 22, 2022

Uh oh!

dylad commented Sep 22, 2022

Uh oh!

dylad commented Sep 22, 2022

Uh oh!

gschorcht commented Sep 23, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gschorcht Oct 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dylad commented Sep 30, 2022

Uh oh!

gschorcht commented Oct 1, 2022

Uh oh!

gschorcht commented Oct 1, 2022

Uh oh!

gschorcht commented Oct 1, 2022

Uh oh!

dylad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

gschorcht commented Sep 17, 2022 •

edited

Loading

gschorcht commented Sep 17, 2022 •

edited

Loading

gschorcht Oct 1, 2022 •

edited

Loading