pkg/tinyusb: add tinyUSB as package

[gschorcht]

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

1. to read from and write to the mass storage device with the usual file operations of the operating system
2. 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]

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

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

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

@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); https://github.com/RIOT-OS/RIOT/blob/369f8c04cef6c11c5790fa33b58a18e630530639/pkg/tinyusb/hw/hw_stm32.c#L36-L37

[dylad]

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

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]

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]

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.

[gschorcht]

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.

You could add -DCFG_TUSB_DEBUG=2 to get debug output from tinyUSB stack.

[gschorcht]

@dylad While testing this PR with a stm32f4discovery board I got yesterday, I ran into the problem that the tinyUSB thread hangs once it received the first SETUP message and waits for the next message at the OUT control endpoint. I couldn't see the problem with my nucleo-f767zi board, probably because of different timing on STM32F407VG. Maybe, it is the same problem you have.

After some investigation, it seems to be due to the mutex-based synchronization between the interrupt-driven writes of events to the tinyUSB queue _usbd_q of type osal_queue_def_t and the blocking reads from this queue: https://github.com/RIOT-OS/RIOT/blob/369f8c04cef6c11c5790fa33b58a18e630530639/pkg/tinyusb/contrib/include/tusb_os_custom.h#L228-L238 https://github.com/RIOT-OS/RIOT/blob/369f8c04cef6c11c5790fa33b58a18e630530639/pkg/tinyusb/contrib/include/tusb_os_custom.h#L248-L253 The problem occur if the check in line 228 gives true and an interrupt occurs before the temporary mutex is set in line 230. In that case, the osal_queue_send doesn't see the mutex that has to be unlocked and the receiving thread keeps blocked in osal_queue_receive. Maybe that is the same problem you have now.

[gschorcht]

After some investigation, it seems to be due to the mutex-based synchronization between the interrupt-driven writes of events to the tinyUSB queue _usbd_q of type osal_queue_def_t and the blocking reads from this queue:

@dylad Based on some assumptions we can make, I have simplified the handling of the queue a lot. The osal_queue_send function no longer blocks in thread context when the queue is full but simply returns false. The osal_queue_receive function no longer supports timeout handling since timeouts are not used in RIOT. The mutex handling for blocking in osal_queue_receive when the queue is empty has been replaced by sema_t, where the semaphore also represents the queue fill level.

With these changes the esp32s3-devkit, esp32s2-devkit and nucleo-f767zi boards work as stable as before. The stm32f4discovery works instable but doesn't hang anymore. Maybe, it also works now for your stm32f429i-disc and you have any idea what the instability could cause.

[gschorcht]

BTW, I have tinyUSB also running for nRF52, at least for boards that don't use stdio_cdc_acm.

[gschorcht]

@benpicco Why does the stm32f429i-disco add the highlevel-stdio feature while the stm32f429i-disc1 does not, even though it seems to be the same product according to the ST product page?

[dylad]

I think this is because of the old STLINK version. See here

[benpicco]

stm32f429i-disco has an old ST-Link that does not provide a virtual console, so the device USB port is used instead to provide a shell.

stm32f429i-disc1 fixes this and wires a UART to the ST-Link

[gschorcht]

It seems that I finally found the cause of the instabilities on my stm32f4discovery. The problem was too much deviation of the USB FS clock (about 7%) because the 48 MHz clock source was not activated correctly in cpu/stm32/stmclk/stmclk_f2f4f7.c. The MODULE_PERIPH_USBDEV define is used there several times to activate the correct clock for the USB peripheral. Since tinyUSB doesn't allow to use periph_usbdev at the same time, the USB clock was not generated using the right clock source. This will also affect other STM32 families.

#if IS_USED(MODULE_PERIPH_USBDEV) && (CLOCK_PLLQ == MHZ(48))
    IS_USED(MODULE_PERIPH_USBDEV) && !IS_ACTIVE(CLOCK_REQUIRE_PLLQ)
    IS_USED(MODULE_PERIPH_USBDEV) && !IS_ACTIVE(CLOCK_REQUIRE_PLLQ)
#if IS_USED(MODULE_PERIPH_USBDEV) && \
cpu/stm32/stmclk/stmclk_f2f4f7.c
#if IS_USED(MODULE_PERIPH_HWRNG) || IS_USED(MODULE_PERIPH_USBDEV)
#if IS_USED(MODULE_PERIPH_USBDEV) && defined(RCC_APB1RSTR1_USBRST)
cpu/stm32/stmclk/stmclk_l4wx.c
#if IS_USED(MODULE_PERIPH_USBDEV) && defined(CPU_LINE_STM32F411xE)
#endif /* MODULE_PERIPH_USBDEV */
cpu/stm32/include/clk/f2f4f7/cfg_clock_default_100.h
#if IS_USED(MODULE_PERIPH_USBDEV) && \
#endif /* MODULE_PERIPH_USBDEV */
#if IS_USED(MODULE_PERIPH_USBDEV) && \
cpu/stm32/include/clk/f2f4f7/cfg_clock_default_180.h

So we have three options now:

1. We extend all the conditionals :worried:

   - if (IS_USED(MODULE_PERIPH_USBDEV) ...
   + if ((IS_USED(MODULE_PERIPH_USBDEV) || IS_USED(MODULE_TINYUSB)) ...
   

2. We require feature periph_usbdev also for tinyUSB and

   • either define a dummy periph_usdev module implementation which only contains a dummy usbdev_init_lowlevel function
   • or embed the real periph_usbdev implementation into conditional of MODULE_TINYUSB.

   The use of usbus would then have to be prevented.

3. We define a pseudomodule periph_usbdev_clk or stm32_periph_usbdev_clk for STM32 and change the conditionals to

   - if (IS_USED(MODULE_PERIPH_USBDEV) ...
   + if (IS_USED(MODULE_PERIPH_USBDEV_CLK) ... 
   

   periph_usbdev and tinyusb would then enable also periph_usbdev_clk.

The advantage of option 2 would be that feature tinyusb_device and tinyusb_host could be removed from board definitions (later one isn't used anyway). These features could be defined on CPU level and would only work if the board has feature periph_usbdev.

What do you think?

By the way, I will get a stm32f429i-disc1 today so I can further investigate the changes needed for a STM32 board with only one USB OTG HS port. I also ordered a stm32f764-disc1 that has both USB OGT FS and USB OTG HS with external PHY using ULPI on the board. This will allow to see how the tinyUSB interface needs to be changed to work with two USB controllers, at least I hope so.

[gschorcht]

@dylad I got also a nucleo-wb55 today. tinyUSB is working as expected with pkg/tinyusb/portable/st/stm32_fsdev and some small changes in pkg/tinyUSB/contrib/hw/hw_stm32.c. I still need to sort the changes a bit. So I wonder if it might be useful to split this file into hw_stm32_otg.c and hw_stm32_fsdev.c. On the other hand, there are code duplications after all and it would be easier with some #ifdef ... #else ... #endif.

[dylad]

I got also a nucleo-wb55 today. tinyUSB is working as expected with pkg/tinyusb/portable/st/stm32_fsdev and some small changes in pkg/tinyUSB/contrib/hw/hw_stm32.c

Great ! IMO, let's keep the current way for now and add supports for stm32_fsdev in a followup PR. No need to add support to everything at once. Since we both have the hardware, we can easily add more CPU support after this one.

So we have three options now:

I think we must have some discussions before to decide the right direction. Once this PR will be merged, we will have two different USB stacks within RIOT. This is not a problem but do we want to maintain both stacks in the future ? Features-wise, tinyusb seems more complete and also handle lowlevel peripheral drivers. If we managed to properly integrate everything within RIOT, we might consider dropping usbus at some point and even periph_usbdev. @bergzand What's your opinion on this ? I think the third option would be a good compromise for now.

By the way, I will get a stm32f429i-disc1 today

Good news ! It would be cool to have this working at the same time so the whole F4/F7 families are supported.

[gschorcht]

Once this PR will be merged, we will have two different USB stacks within RIOT. This is not a problem but do we want to maintain both stacks in the future ?

Hm, we have the same situation with GNRC stack and lwIP.

Features-wise, tinyusb seems more complete and also handle lowlevel peripheral drivers.

Yes, but it also requires more ROM/RAM. For comparison:

   text	   data	    bss	    dec	    hex	filename
  19008	    192	   4344	  23544	   5bf8	tests/usbus_hid/bin/nucleo-f767zi/tests_usbus_hid.elf

For same application tinyUSB requires:

   text	   data	    bss	    dec	    hex	filename
  20708	    160	   5864	  26732	   686c	tests/pkg_tinyusb_hid_generic_inout/bin/nucleo-f767zi/tests_pkg_tinyusb_hid_generic_inout.elf

Furthermore, I don't know whether tinyUSB has driver for all platforms that periph_usdev support.

[gschorcht]

@dylad I pushed the changes for option 3 using a pseudomodule periph_usbdev_clk.

[dylad]

@gschorcht Please squash ! I think we're almost done here. I'll re-run some test and have a last look tomorrow.

[gschorcht]

I got tinyUSB also working on nucleo-f303ze for which cpu/stm32/periph/usbdev_fs.c is not working yet.

[riot-ci]

Murdock results

:heavy_check_mark: PASSED

75071319ded33efaeb0532e8a00749ada05928a7 cpu/stm32: define HSE_VALUE for tinyUSB Synopsys DWC2 driver

Success	Failures	Total	Runtime
1979	0	1979	06m:36s

[gschorcht]

@dylad What do think about some logic to derive TINYUSB_TUx_RHPORT automatically for STM32 board from the STM32_USB_OTG_FS_ENABLED and STM32_USB_OTG_HS_ENABLED defines if they are not defined explicitly?

#ifndef TINYUSB_TUD_RHPORT
#if defined(STM32_USB_OTG_FS_ENABLED)
#define TINYUSB_TUD_RHPORT   0
#elif defined(STM32_USB_OTG_HS_ENABLED)
#define TINYUSB_TUD_RHPORT   1
#else
#error ...
#endif

#ifndef TINYUSB_TUH_RHPORT
#if defined(STM32_USB_OTG_HS_ENABLED)
#define TINYUSB_TUH_RHPORT  1
#elif defined(STM32_USB_OTG_HS_ENABLED)
#define TINYUSB_TUH_RHPORT   0
#else
#error ...
#endif

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

[dylad]

@gschorcht I likea the idea ! Please go ahead.

Note: There is a glitch in:

#ifndef TINYUSB_TUH_RHPORT
#if defined(STM32_USB_OTG_HS_ENABLED)
#define TINYUSB_TUH_RHPORT  1
#elif defined(STM32_USB_OTG_HS_ENABLED)
#define TINYUSB_TUH_RHPORT   0
#else
#error ...
#endif

STM32_USB_OTG_HS_ENABLED is used for both if and elif so elif will never be reached.

[dylad]

I got tinyUSB also working on nucleo-f303ze for which cpu/stm32/periph/usbdev_fs.c is not working yet.

That's a good news. I'd really like to add more CPU families support to usbdev_fs but I lack of hardware on my side :(

[gschorcht]

@gschorcht I likea the idea ! Please go ahead.

Unfortunatly, I could not find a common location like periph_conf_common.h for STM32 boards where these definitions could be placed. I'm thinking about whether a platform-specific tinyusb_defaults.h file which is included.

[gschorcht]

@dylad I had to provide two small fixes due to compilation errors after rebase and the introduction of periph_usbdev_clk pseudomodule. If they are ok, I would like t squash before next CI compilation cycle.

[dylad]

@dylad I had to provide two small fixes due to compilation errors after rebase and the introduction of periph_usbdev_clk pseudomodule. If they are ok, I would like t squash before next CI compilation cycle.

Go ahead. I should have a look at it tonight.

[gschorcht]

@dylad With commit 4fd2ea513e3d4570bc66a60f663a63b64988beeb I pushed a change that sets TINYUSB_TUD_RHPORT and TINYUSB_TUH_RHPORT automatically depending on the definition of DWC2_USB_OTG_HS_ENABLED and DWC2_USB_OTG_FS_ENABLED. If this makes no sense in that way, we can simlpy drop this commit.

At the moment it is not possible at all to use OTG HS and OTG FS at the same time, because cfg_usb_otg_hs_fs.h and cfg_usb_otg_fs.h declare the same variable dwc2_usb_otg_fshs_config. That is, we would have to change these files a bit before both USB OTG port could be used.