-
Notifications
You must be signed in to change notification settings - Fork 130
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
Issues when using TinyUSB and FreeRTOS together #816
Conversation
"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) |
This is an issue with out FreeRTOS port that added the ability to use static memory in #750. |
I guess a better implemention would be to enable |
Yeah, this seems to be the right way: https://forums.freertos.org/t/static-and-dynamic-allocation-together/8138/3
|
That's not enough you need to change this |
Already done, give me 15 seconds to write a commit message (without typos). |
Please also update the line in |
cd558a8
to
55a8ced
Compare
Seems to work now with FreeRTOS, but TinyUSB does not work on my Nucleo-F429ZI board. I checked the @salkinium (On which hardware) have verified TinyUSB after the last update? |
Black Pill and F103 😬. Perhaps an issue with the VBUS sense config? |
TinyUSB hangs here: https://github.com/modm-ext/tinyusb-partial/blob/6a3450ec4ba9598136c1df345e1eaa1c62cc0f52/src/portable/st/synopsys/dcd_synopsys.c#L504 Buit with this fixed I run into an assertion once I plug in the USB to a computer... |
Another bug: |
Hmm, this line seems to be a copy-paste error, it should just be |
So default to |
Just wanted to post that I found it, but you were 8 min earlier 😃 |
Yes, otherwise it breaks default behavior of devices without HS peripheral that don't have this option. Very nasty debugging. |
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. |
More bugs while trying to debug:
and
|
Seems like my build_id is empty:
What? |
That commit broke it:
|
The |
Ooops, I condensed the sections a little because otherwise the output of |
Next problem:
does not work at all because:
(The file is also not marked executable, bit that is solvable...) |
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 |
🙈🙈🙈🙈🙈 |
@rleh It crashes. |
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; |
It seems to be USB-related. I can reproduce the crash but the code starts working if I don't call |
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… |
I found the reason for the crash, classic off-by-one. The array That was actually overwriting the lower half of the pointer to the logger device inside the |
208b7ab
to
2e30a2c
Compare
Now all my problems are solved! I also added the parameter ( Review & merge? |
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. |
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. |
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.
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!
2e30a2c
to
3f3ff3d
Compare
I added a warning to the example and to the TinyUSB lbuild module. Example works on my hardware. Ready to merge from my side! |
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.
Thanks!
Thanks for your help @chris-durand and @salkinium! |
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...
cc @salkinium @chris-durand