Port Adafruit_TinyUSB_Arduino lib

[hathach]

This PR integrate https://github.com/adafruit/Adafruit_TinyUSB_Arduino to this pico core. It requires adafruit/Adafruit_TinyUSB_Arduino#80 to be merged first, But submitting in prior to get feedbacks in advance. PR still keep the existing USB implementation with Serial and provide an extra menu to switch usb implementation (much like https://github.com/adafruit/ArduinoCore-samd), make sure you select TinyUSB when testing.

The PR is tested with both pico and feather rp2040 board with all driver CDC, HID MSC, MIDI, WebUSB examples without any issue. Changes includes:

• Added TinyUSB lib as submodule at libraries/Adafruit_TinyUSB_Arduino
• Added tusb_config.h to cores/rp2040/TinyUSB/, extra folder is needed, to make sure it included instead of the config file in the pico-sdk stdio usb
• platform.txt changes to mostly for USE_TINYUSB macro, and include path along with USB VID, PID, Manufacturer, Product string
• boads.txt mostly for usb selectable menu with vid, pid, manufacturer and product string. Note: I think we can actually separate the tools from board variant, picoprobe can use the programmers.txt which can allow us to add pyocd in addition to openocd. But that is for a separate PR I think.
• Most notable changes is in main.cpp and delay.cpp which call the API to init stack as well as task() to handle usb event. Reason for changes is mentioned in this porting guide https://github.com/adafruit/Adafruit_TinyUSB_Arduino/tree/major-v1#porting-guide

Note: since pico-sdk delay()/sleep() invoke wfi()/wfe(), extra effort is needed to ensure there is no pending usb events when needed similar to adafruit/circuitpython#2956. Issue can cause task to starve micropython/micropython#6881. Though I would like to have some feedback to see if this approach make sense to you first. Will fix it in later push.

@ladyada

[ladyada]

nice work!

[earlephilhower]

Very cool, thank you! I need to spend some time looking at this and doing the PR-pulls to try it out, but I think that since it needs a couple other PRs in other repos to land first it's not a big rush, yes?

Anyway, I see a couple minor things just eyeballing it:

• The boards.txt is programmatically generated. tools/makeboards.py is the magic, so I think we need to get your add'l fields added there, too, so they don't disappear on the next board added.
• The Pico's kind of unique with its 2-core setup. Have you considered making the tinyusb {CDC, etc} implementation multicore safe in some way?
• The delay() change seems logically correct, but we'd definitely want to move to something like the tud_task_event_ready check in your other PRs, if only for power savings. Right now I'm not so good about shutting off HW when not in use (see the PWM/analogWrite), but I'd like to get there some day to make battery operation better.

Do you think eventually it would make sense to move completely to this USB stack, vs. the TinyUSB from the SDK? There's already an ecosystem around the adafruit_tinyusb library, and the current one isn't compatible with Arduino's USB stack so there's no chance to reuse the Arduino HID/etc. libs anyway.

Thanks again!

[hathach]

Very cool, thank you! I need to spend some time looking at this and doing the PR-pulls to try it out, but I think that since it needs a couple other PRs in other repos to land first it's not a big rush, yes?

No problem, no rush at all, just tell me when you think it could be merged, I will just rebase this PR.

Anyway, I see a couple minor things just eyeballing it:

• The boards.txt is programmatically generated. tools/makeboards.py is the magic, so I think we need to get your add'l fields added there, too, so they don't disappear on the next board added.

Ah thanks, I didn't know this, will update it in the next push.

• The Pico's kind of unique with its 2-core setup. Have you considered making the tinyusb {CDC, etc} implementation multicore safe in some way?

tinyusb is thread-safe and will handle multiple core/thread mutex as long as we don't call its API within ISR. I have no problem using it with preempted FreeRTOS https://github.com/adafruit/Adafruit_nRF52_Arduino as well as esp32s2.

• The delay() change seems logically correct, but we'd definitely want to move to something like the tud_task_event_ready check in your other PRs, if only for power savings. Right now I'm not so good about shutting off HW when not in use (see the PWM/analogWrite), but I'd like to get there some day to make battery operation better.

Yeah, as mentioned, before the final merge, I will update the PR to correct behavior before going to low power. Just want to see if you are interested to merge the PR and/or getting feedback first. Will do it in the next push.

Do you think eventually it would make sense to move completely to this USB stack, vs. the TinyUSB from the SDK? There's already an ecosystem around the adafruit_tinyusb library,

Thanks again!

This is totally up to you to decide (that is why this PR add it as menu selection), they both run the very same stack underneath anyway. The pico-sdk is probably slightly slower in keeping up with the upstream. And indeed the TinyUSB lib can be used with other library such as SDFat for MSC, so yeah, it makes thing easier if user are moving from a core that already using this library. I also have plan to add audio, host etc (just need more time and coffee) in the future. Using only this as usb stack surely could reduce lots of #ifelse within the core, though I would still recommend you to try out the PR before making decision.

and the current one isn't compatible with Arduino's USB stack so there's no chance to reuse the Arduino HID/etc. libs anyway.

For pure HID API, it is rather simple, it is only an matter of API naming and invocation order. I just haven't payed enough attention to arduino API to make it compatible. Though there is other library that does the apdation https://github.com/cyborg5/TinyUSB_Mouse_and_Keyboard . Maybe later on I will add it to the library itself, or if someone want to make an PR I am happy to merge.

Note: recent Arduino core use mbedOS which has its own usb stack, I haven't looked at their rp2040 implementation just yet. Though if we could manage to forward the IRQ toward Tinyusb handler, which is totally possible due to how pico-sdk manage/install handler. We may (or not) port it to the official core.

[earlephilhower]

@hathach for this merge to work, I think we need the other PRs to land first (in other repos), right? OTW, I'm OK pulling it in under a user-selected menu even if it is a bit rough. Stepwise refinement (check the git log for this repo!) is fine, especially since it can land initially under experimental use.

I got a chance to actually look at TinyUSB instead of just hacking the RPi CDC example and was able to get a working (AFAICT!) shared USB stack. It's a lot simpler than I feared, other than the creation of the USB descriptors which I imagine is just a pain every where. I'm not going to say I fully understand it, but your library made porting Keyboard and Mouse a matter of minutes vs the Arduino USB stack!

[hathach]

@hathach for this merge to work, I think we need the other PRs to land first (in other repos), right? OTW, I'm OK pulling it in under a user-selected menu even if it is a bit rough. Stepwise refinement (check the git log for this repo!) is fine, especially since it can land initially under experimental use.

Ahha, sorry, I totally mis-read your previous comment. I thought you need to merge some PRs for this repo first, and I need to rebase this PR. My bad, English is not my native language. Yeah for this PR to be merged, TinyUSB lib PR need to be merged first, though since we may need to make some changes to the lib to implement port for this PR. They are kind of waiting for each other. That is why I made this as draft since I want to know your opinion/feedback first before finalizing API, port etc..

I got a chance to actually look at TinyUSB instead of just hacking the RPi CDC example and was able to get a working (AFAICT!) shared USB stack. It's a lot simpler than I feared, other than the creation of the USB descriptors which I imagine is just a pain every where. I'm not going to say I fully understand it, but your library made porting Keyboard and Mouse a matter of minutes vs the Arduino USB stack!

I am glad you like the idea with shared USB lib across the cores. Indeed descriptors is the most complicated thing for USB, it is a pain for multi-purpose dynamic descriptor. Regarding the wfe() thing, pico-sdk develop branch has a major rework for sleep_until().

https://github.com/raspberrypi/pico-sdk/pull/378/files#diff-420245f64604c7417651cff7fca2529def3ac94b84cb4a4bf4fd26a7e2612e68R347

In both implementation, it doesn't seem to allow non-isr code to run background work. I will probably need to ask pico team for advice. Anyway I am finalizing the PR and will try to push as soon as I could.

[hathach]

@earlephilhower I have rebase and push force to the PR since your master got major update recently. I am trying to test if I could also use your mutex to run tud_task() in both irq and thread context (instead of only thread context with yield() ). However, I got into a couple of linker issue with latest changes. (1) is solvable, but I really need your help with (2).

1. -Wl,--whole-archive "{runtime.platform.path}/lib/libpico.a causes duplication symbol since tinyusb function are compiled in both pico-sdk and the Adafruit_TinyUSB_Arduino lib. Temporarily remove it for testing.
2. Once --whole-archive is removed, I still got another linker issue, I haven't got any clues where is the undefined reference to '__wrap___aeabi_fcmpun' issue

/home/hathach/Arduino/hardware/pico/rp2040/system/arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.0/../../../../arm-none-eabi/bin/ld: /home/hathach/Arduino/hardware/pico/rp2040/system/arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.0/../../../../arm-none-eabi/lib/thumb/libc.a(lib_a-strtod.o): in function `strtof_l':
/workdir/repo/newlib/newlib/libc/stdlib/strtod.c:1295: undefined reference to `__wrap___aeabi_fcmpun'
/home/hathach/Arduino/hardware/pico/rp2040/system/arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.0/../../../../arm-none-eabi/bin/ld: /workdir/repo/newlib/newlib/libc/stdlib/strtod.c:1295: undefined reference to `__wrap___aeabi_fcmple'
/home/hathach/Arduino/hardware/pico/rp2040/system/arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.0/../../../../arm-none-eabi/bin/ld: /home/hathach/Arduino/hardware/pico/rp2040/system/arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.0/../../../../arm-none-eabi/lib/thumb/libc.a(lib_a-strtod.o): in function `strtof':
/workdir/repo/newlib/newlib/libc/stdlib/strtod.c:1310: undefined reference to `__wrap___aeabi_fcmpun'
/home/hathach/Arduino/hardware/pico/rp2040/system/arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.0/../../../../arm-none-eabi/bin/ld: /workdir/repo/newlib/newlib/libc/stdlib/strtod.c:1310: undefined reference to `__wrap___aeabi_fcmple'
collect2: error: ld returned 1 exit status

[earlephilhower]

I was just about to warn you about that when I saw the GH mail.

The way the pico-sdk is set up, I was unable to get the __wrap_xxx functions to not be discarded by the linker except via the --whole-archive trick. The way the dependencies are between Newlib, pico-sdk, libmath, etc. , I'm not sure there is another way.

I would suggest reapplying the whole-archive bit, but then using ar d to extract the conflicting Pico-SDK compilation units (.objs) from libpico.a. That's good enough to get hacking with, I think.

For a permanent solution, some AR magic to extract and move the sdk tinuysb objects to their own .a as part of tools/libpico/make-libpico.sh, and a platform.txt update to include the new lib depending on the USB stack in use.

[earlephilhower]

Can you give #143 a try and see if it helps w/o having to adjust libpico as mentioned in my prior comment? I think it may make your job easier by avoiding the whole-archive bit.

[hathach]

Can you give #143 a try and see if it helps w/o having to adjust libpico as mentioned in my prior comment? I think it may make your job easier by avoiding the whole-archive bit.

Superb !! thank you very much, it compiles now, work like a charm. I am trying to wrap up this PR asap.

[hathach]

The rp2040 port from the Arduino Lib is changed to call the tud_task() from irq context like "pico-sdk" stack. The TinyUSB_Device_Task() is also implemented in the way the require mutex to run. Therefore it no longer needs to use the task ready event API as previous commit.

The reason for not pursuing thread-context task, is that the sleep_ms() from pico-sdk call wfe() does not provide hook() to run background work in thread-context. In short CPU will wake in case of USB IRQ, run the handler and then sleep again without any chances to run tud_task() in thread-context.

PS: Let me know if you want to revert to have extern "C" for each function instead of block {}.

[hathach]

@earlephilhower adafruit/Adafruit_TinyUSB_Arduino#80 has been merged, this should be ready for review and feedback for a merge. Please give it a try. Thank you in advance.

[earlephilhower]

This is very awesome, runs very well and doesn't affect anything when the Pico SDK option is selected! I've tried quite a few of the examples without any issue.

One thing I may ask for your help on is how we can make there still be some way to upload code w/o requiring an unplug/hold BOOTSEL/replug operation (i.e. the CDC 1200bps DFU mode). But for users looking to just, say, make a MSC or MIDI device this rocks.

Thanks for spending the time making the boards.txt prettier, too!

[earlephilhower]

Ah, I see that all that's required is a Serial.begin(115200) call. So, I think the same "call Serial.begin in main()" that's used for the SDK USB stack could work here just fine, too.

[hathach]

Ah, I see that all that's required is a Serial.begin(115200) call. So, I think the same "call Serial.begin in main()" that's used for the SDK USB stack could work here just fine, too.

Yeah, reset to bootloader is done using touch1200 via Serial. which is done when USBDevice.begin() (TinyUSB_Device_Init) is called. It can be done in main as well though I kind of want to give only 1 single API TinyUSB_Device_Init() to the core. Serial.begin() can be called multiple times, so it is safe anyway :). I am glad you like the PRs.