Issues when using TinyUSB and FreeRTOS together #816

rleh · 2022-01-28T17:45:41Z

I guess this is a bug in our TinyUSB port or a version incompatability between our FreeRTOS and the FreeRTOS version expected by TinyUSB.

But could also be a bug with local setup, let's see if the example fails in CI...

$ cd examples/nucleo_f429zi/usb_freetros/
$ lbuild build
$ scons
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
Compiling C++·· release/main.o
In file included from modm/ext/tinyusb/osal/osal.h:53,
                 from modm/ext/tinyusb/tusb.h:38,
                 from main.cpp:12:
modm/ext/tinyusb/osal/osal_freertos.h: In function 'QueueDefinition* osal_semaphore_create(osal_semaphore_def_t*)':
modm/ext/tinyusb/osal/osal_freertos.h:56:10: error: 'xSemaphoreCreateBinaryStatic' was not declared in this scope; did you mean 'xSemaphoreCreateBinary'?
   56 |   return xSemaphoreCreateBinaryStatic(semdef);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |          xSemaphoreCreateBinary
modm/ext/tinyusb/osal/osal_freertos.h: In function 'QueueDefinition* osal_mutex_create(osal_mutex_def_t*)':
modm/ext/tinyusb/osal/osal_freertos.h:99:10: error: 'xSemaphoreCreateMutexStatic' was not declared in this scope; did you mean 'xSemaphoreCreateMutex'?
   99 |   return xSemaphoreCreateMutexStatic(mdef);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
      |          xSemaphoreCreateMutex
modm/ext/tinyusb/osal/osal_freertos.h: In function 'QueueDefinition* osal_queue_create(osal_queue_def_t*)':
modm/ext/tinyusb/osal/osal_freertos.h:134:10: error: 'xQueueCreateStatic' was not declared in this scope; did you mean 'xQueueCreateSet'?
  134 |   return xQueueCreateStatic(qdef->depth, qdef->item_sz, (uint8_t*) qdef->buf, &qdef->sq);
      |          ^~~~~~~~~~~~~~~~~~
      |          xQueueCreateSet
scons: *** [.../build/nucleo_f429zi/usb_freetros/scons-release/main.o] Error 1
scons: building terminated because of errors.

cc @salkinium @chris-durand

rleh · 2022-01-28T17:56:20Z

xSemaphoreCreateBinaryStatic()

"configSUPPORT_STATIC_ALLOCATION must be set to 1 in FreeRTOSConfig.h for this RTOS API function to be available." (https://www.freertos.org/xSemaphoreCreateBinaryStatic.html)

salkinium · 2022-01-28T18:11:03Z

This is an issue with out FreeRTOS port that added the ability to use static memory in #750.
It needs to be adapted to allow for both configSUPPORT_STATIC_ALLOCATION and configSUPPORT_DYNAMIC_ALLOCATION at the same time, currently it's one of the other depending on whether the modm:platform:heap module is available, which is not a good choice…

salkinium · 2022-01-28T18:13:19Z

I guess a better implemention would be to enable configSUPPORT_STATIC_ALLOCATION always, and only enable configSUPPORT_DYNAMIC_ALLOCATION if modm:platform:heap is available. That would only be a small change iff FreeRTOS supports both.

salkinium · 2022-01-28T18:15:18Z

Yeah, this seems to be the right way: https://forums.freertos.org/t/static-and-dynamic-allocation-together/8138/3

Yes it is possible to use both static and dynamic allocation at the same
time - many of the demos do this and it is a very common scenario in
production (especially where libraries are using static allocation even
if application code is not).

However, if you have configSUPPORT_STATIC_ALLOCATION set to 1 then the
timer/daemon and idle tasks WILL use static allocation and you MUST
provide the statically allocated memory via callbacks. See the
following documentation to see how:
https://www.freertos.org/a00110.html#configSUPPORT_STATIC_ALLOCATION

salkinium · 2022-01-28T18:23:28Z

That's not enough you need to change this %% else to %% endif to always provide the required static memory. Also this here.

rleh · 2022-01-28T18:25:55Z

Already done, give me 15 seconds to write a commit message (without typos).

salkinium · 2022-01-28T18:27:40Z

Please also update the line in freertos.md.

rleh · 2022-01-28T18:51:44Z

Seems to work now with FreeRTOS, but TinyUSB does not work on my Nucleo-F429ZI board. I checked the generic/usb/ example: It does not work either.
The call to tusb_init() never returns.

@salkinium (On which hardware) have verified TinyUSB after the last update?

salkinium · 2022-01-28T19:00:33Z

Black Pill and F103 😬. Perhaps an issue with the VBUS sense config?

salkinium · 2022-01-28T19:07:13Z

The TinyUSB BSP for the F439 has the same though

rleh · 2022-01-28T21:03:28Z

TinyUSB hangs here: https://github.com/modm-ext/tinyusb-partial/blob/6a3450ec4ba9598136c1df345e1eaa1c62cc0f52/src/portable/st/synopsys/dcd_synopsys.c#L504
because Nucleo-F429ZI has USB FS and HS peripherals, modm defaults to the HS peripheral and the example does not set the speed option.

Buit with this fixed I run into an assertion once I plug in the USB to a computer...

rleh · 2022-01-28T21:06:33Z

Another bug: CFG_TUSB_DEBUG=2 seems to require heap.

salkinium · 2022-01-28T21:10:51Z

modm defaults to the HS peripheral

Hmm, this line seems to be a copy-paste error, it should just be "full".

rleh · 2022-01-28T21:13:47Z

So default to full for all devices?

chris-durand · 2022-01-28T21:14:29Z

TinyUSB hangs here: https://github.com/modm-ext/tinyusb-partial/blob/6a3450ec4ba9598136c1df345e1eaa1c62cc0f52/src/portable/st/synopsys/dcd_synopsys.c#L504 because Nucleo-F429ZI has USB FS and HS peripherals, modm defaults to the HS peripheral and the example does not set the speed option.

Buit with this fixed I run into an assertion once I plug in the USB to a computer...

Just wanted to post that I found it, but you were 8 min earlier 😃

salkinium · 2022-01-28T21:17:09Z

So default to full for all devices?

Yes, otherwise it breaks default behavior of devices without HS peripheral that don't have this option. Very nasty debugging.

chris-durand · 2022-01-28T21:23:38Z

So default to full for all devices?

It is a reasonable default and the most commonly used option. High speed USB requires an external PHY on almost all STM32. Some F730 have an internal high speed PHY but it uses completely different pins. On those users will be aware that they have to make the choice according to the hardware.

rleh · 2022-01-28T21:28:52Z

More bugs while trying to debug:

$ scons debug-coredump firmware=9AFFA6659846AE10021C2E4443054683898E8D9A
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
scons: *** [gdb_post_mortem_debug] Source `artifacts/9affa6659846ae10021c2e4443054683898e8d9a.elf' not found, needed by target `gdb_post_mortem_debug'.

and

$ scons profile=debug -j8 artifact
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
╭───Artifact─── build/usb/scons-debug/usb.elf
╰────Cache────> artifacts/None.elf
scons: *** [store_artifact] ValueError : Unable to find Build ID for 'build/usb/scons-debug/usb.elf'!
Traceback (most recent call last):
  File "/usr/lib/python3.10/site-packages/SCons/Action.py", line 1279, in execute
    result = self.execfunction(target=target, source=rsources, env=env)
  File ".../modm/examples/generic/usb/modm/scons/site_tools/artifact.py", line 19, in run_store_artifact
    build_id.cache_elf(source[0].path, env["CONFIG_ARTIFACT_PATH"])
  File ".../modm/examples/generic/usb/modm/modm_tools/build_id.py", line 54, in cache_elf
    raise ValueError("Unable to find Build ID for '{}'!".format(source))
ValueError: Unable to find Build ID for 'build/usb/scons-debug/usb.elf'!
scons: building terminated because of errors.

rleh · 2022-01-28T21:58:48Z

Seems like my build_id is empty:

$ arm-none-eabi-objdump -d build/usb/scons-debug/usb.elf

[...]
0800817c g       .rodata	00000000 __build_id
[...]

What?

chris-durand · 2022-01-28T21:59:15Z

That commit broke it:

commit 996e361d9ed9b59f0a4928bc3ddec22862d6665b
Author: Niklas Hauser <niklas.hauser@rwth-aachen.de>
Date:   Tue Jul 13 04:03:27 2021 +0200

    [core] Refactor linkerscript templates
    
    - Adds a generic linkerscript for all basic Cortex-M devices.
    - Adds .data_{ram} and .noinit_{ram} sections to all memories.
    - Uses MAX() to forward the heap location counter so that a very
      large .data section will collapse the heap to zero and "spill" into
      the next RAM section without overflowing the subsection SRAM1.
    - Generalizes RAM heap section placement via macro.

chris-durand · 2022-01-28T22:04:06Z

The build_id.py script looks for a .build_id section in the elf file. That was removed during the linker script refactoring.

salkinium · 2022-01-28T22:07:47Z

Ooops, I condensed the sections a little because otherwise the output of scons size would get spammed with a lot of unreleated stuff incl. .build_id. I'm checking how I can fix the modm_tools script. Bad Niklas, BAD!

rleh · 2022-01-28T22:22:24Z

Next problem:

scons debug-coredump

does not work at all because:

$ file ext/adamgreen/crashcatcher/CrashDebug/bins/lin64/CrashDebug 
ext/adamgreen/crashcatcher/CrashDebug/bins/lin64/CrashDebug: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), corrupted program header size, corrupted section header size

(The file is also not marked executable, bit that is solvable...)

rleh · 2022-01-28T22:29:28Z

corrupted program header size, corrupted section header size

Because decoding executable files as utf-8 and copying them line by line is a bad idea: https://github.com/modm-ext/CrashCatcher-partial/blob/beec94a93c42a8cad924330be16dc4888045ae9a/update.py#L39-L42
🤣

salkinium · 2022-01-28T22:30:42Z

🙈🙈🙈🙈🙈

chris-durand · 2022-01-29T00:29:15Z

@rleh It crashes.

rleh · 2022-01-29T00:41:48Z

Seems to be unrelated to USB:

I can also crash my Nucleo-F429ZI by just creating a second (uart based) IOStream:

modm::IODeviceWrapper< modm::platform::Usart2, modm::IOBuffer::BlockIfFull > uart2_io_device;
modm::IOStream uart2_stream(uart2_io_device);

// in main loop:
uart2_stream << "Hello World from UART2: " << (counter++) << modm::endl;

chris-durand · 2022-01-29T00:59:01Z

Seems to be unrelated to USB:

I can also crash my Nucleo-F429ZI by just creating a second (uart based) IOStream:

It seems to be USB-related. I can reproduce the crash but the code starts working if I don't call tud_task(); from the main loop.

salkinium · 2022-01-29T01:10:27Z

Fix fails for H7B3 which has no full-speed USB peripheral but only a high speed one.

I made the option only appear for devices with a real choice and defaulted it properly.

Both the USB generic and the usb_freertos examples hardfault for me. Not sure why…

chris-durand · 2022-01-29T20:50:28Z

I found the reason for the crash, classic off-by-one. The array _desc_str has size 32 but 32 elements are written into it starting from index 1 (tusb_get_device_serial(_desc_str + 1); in tusb_descriptors.c).

That was actually overwriting the lower half of the pointer to the logger device inside the modm::log::info object. When trying to call write() on the logger device the vtable pointer was pointing at the wrong address.

rleh · 2022-01-30T19:35:05Z

Now all my problems are solved!
I disabled the FreeRTOS abstraction layer in TinyUSB since I don't see it's benefit and it causes problems.

I also added the parameter (uint8_t priority=3) to Board::initializeUsbFs() for all BSPs with USB support. This does not change existing behavior because usb::initialize(uint8_t priority=3) defaulted to the same interrupt priority.

Review & merge?

chris-durand · 2022-01-30T20:41:09Z

I disabled the FreeRTOS abstraction layer in TinyUSB since I don't see it's benefit and it causes problems.

The FreeRTOS integration seems to be needed when accessing tinyusb functions from multiple threads. When it is enabled mutexes are added in some places and the tinyusb fifo is implemented in terms of a FreeRTOS thread safe queue. Otherwise you'll get race conditions and undefined behaviour if you do that. I am not sure if we should go with that as the default, at least not without a big warning somewhere. What do you think @salkinium @rleh ?

The PR looks good otherwise.

salkinium · 2022-01-30T20:43:17Z

I am not sure if we should go with that as the default, at least not without a big warning somewhere.

I mean, ideally we would solve the FreeRTOS issues in TinyUSB, otherwise we should definitely add a warning to the TinyUSB documentation about missing automatic support for FreeRTOS and perhaps open an issue to track it.

rleh

Every other driver in modm is also not thread safe with FreeRTOS, so I hope that all people who are using FreeRTOS are aware of this.

But adding a warning is probably a good idea!

ext/hathach/module.lb

rleh · 2022-01-31T00:34:53Z

I added a warning to the example and to the TinyUSB lbuild module. Example works on my hardware. Ready to merge from my side!

chris-durand

Thanks!

rleh · 2022-01-31T00:54:33Z

Thanks for your help @chris-durand and @salkinium!

rleh changed the title ~~Issues when using TinyUSB and FreeRTOS in one project~~ Issues when using TinyUSB and FreeRTOS together Jan 28, 2022

salkinium added advanced 🤯 fix 💎 labels Jan 28, 2022

[freertos] Enable static allocations always

5da2f77

rleh force-pushed the tinyusb_freertos branch from cd558a8 to 55a8ced Compare January 28, 2022 18:28

rleh and others added 6 commits January 29, 2022 22:13

[ext] Update CrashCatcher

37b66db

[linkerscript] Fix missing .build_id section

26490e1

[tools] Fix duplicate section names in size tool

7d839cf

[tinyusb] Use STM32 full-speed by default

9602775

[tinyusb] Fix crash reading device descriptor

ce0ad5b

[board] Add priority argument to Board::initializeUsbFs()

6b257c6

rleh force-pushed the tinyusb_freertos branch from 208b7ab to 2e30a2c Compare January 30, 2022 19:19

rleh requested review from salkinium and chris-durand January 30, 2022 19:29

salkinium approved these changes Jan 30, 2022

View reviewed changes

rleh commented Jan 30, 2022

View reviewed changes

ext/hathach/module.lb Show resolved Hide resolved

rleh added 2 commits January 31, 2022 01:31

[tinyusb] Do not use FreeRTOS abstraction layer

aa24f4a

[examples] Add example with TinyUSB and FreeRTOS (for Nucleo-F429ZI)

3f3ff3d

rleh force-pushed the tinyusb_freertos branch from 2e30a2c to 3f3ff3d Compare January 31, 2022 00:33

salkinium approved these changes Jan 31, 2022

View reviewed changes

chris-durand approved these changes Jan 31, 2022

View reviewed changes

salkinium added the ci:hal Triggers the exhaustive HAL compile CI jobs label Jan 31, 2022

rleh merged commit 3f3ff3d into modm-io:develop Jan 31, 2022

rleh deleted the tinyusb_freertos branch January 31, 2022 02:21

salkinium added this to the 2022q1 milestone Jan 31, 2022

Issues when using TinyUSB and FreeRTOS together #816

Issues when using TinyUSB and FreeRTOS together #816

Conversation

rleh commented Jan 28, 2022

rleh commented Jan 28, 2022

salkinium commented Jan 28, 2022

salkinium commented Jan 28, 2022

salkinium commented Jan 28, 2022

salkinium commented Jan 28, 2022 • edited

rleh commented Jan 28, 2022

salkinium commented Jan 28, 2022

rleh commented Jan 28, 2022 • edited

salkinium commented Jan 28, 2022

salkinium commented Jan 28, 2022 • edited

rleh commented Jan 28, 2022

rleh commented Jan 28, 2022

salkinium commented Jan 28, 2022

rleh commented Jan 28, 2022

chris-durand commented Jan 28, 2022

salkinium commented Jan 28, 2022

chris-durand commented Jan 28, 2022 • edited

rleh commented Jan 28, 2022

rleh commented Jan 28, 2022

chris-durand commented Jan 28, 2022

chris-durand commented Jan 28, 2022

salkinium commented Jan 28, 2022

rleh commented Jan 28, 2022 • edited

rleh commented Jan 28, 2022

salkinium commented Jan 28, 2022

chris-durand commented Jan 29, 2022

rleh commented Jan 29, 2022

chris-durand commented Jan 29, 2022 • edited

salkinium commented Jan 29, 2022

chris-durand commented Jan 29, 2022 • edited

rleh commented Jan 30, 2022

chris-durand commented Jan 30, 2022

salkinium commented Jan 30, 2022

rleh left a comment

Choose a reason for hiding this comment

rleh commented Jan 31, 2022

chris-durand left a comment

Choose a reason for hiding this comment

rleh commented Jan 31, 2022

salkinium commented Jan 28, 2022 •

edited

rleh commented Jan 28, 2022 •

edited

salkinium commented Jan 28, 2022 •

edited

chris-durand commented Jan 28, 2022 •

edited

rleh commented Jan 28, 2022 •

edited

chris-durand commented Jan 29, 2022 •

edited

chris-durand commented Jan 29, 2022 •

edited