search

hathach / tinyusb

An open source cross-platform USB stack for embedded system
https://www.tinyusb.org
MIT License
4.66k stars 997 forks source link

Add support for dcd_sof_enable() to some additional ports. #2647

Closed andrewleech closed 1 month ago

andrewleech commented 1 month ago

Describe the PR This PR fills out the stubbed dcd_sof_enable() function for mimxrt, samd and renesas-ra ports.

Additional context I'm working on a TinyUSB feature in micropython that uses SOF to provide some simple timing / delay shortly after connection of CDC: https://github.com/micropython/micropython/pull/14485

This also relates to https://github.com/hathach/tinyusb/discussions/2595 where it was found a delay is needed after CDC connection before sending / flushing TX data from TinyUSB (device) to host so its not lost.

In the micropython stm port (not using tinyusb) the SOF interrupt is enabled on CDC connection / DTR enable and is used to count 8ms before disabling the SOF interrupt again and flushing buffered data to the host. I'm planning on replicating this process using TinyUSB on other micropython ports, eventually replacing the STM USB stack with TinyUSB on that port also.

HiFiPhile commented 1 month ago

Thanks for your PR, since your are working on it , could you please also add dcd_event_sof() to pass event to upper layer ? Like: https://github.com/hathach/tinyusb/blob/ccc7a36043e055ded1f478a979a303e694123187/src/portable/synopsys/dwc2/dcd_dwc2.c#L1161

For ci_hs it can be read from FRINDEX, samd21 and nrf5x I'm less familiar...

andrewleech commented 1 month ago

could you please also add dcd_event_sof() to pass event to upper layer ?

Is this different? I noticed they already had something like this? https://github.com/hathach/tinyusb/blob/ccc7a36043e055ded1f478a979a303e694123187/src/portable/chipidea/ci_hs/dcd_ci_hs.c#L682

My testing from within micropython is using the tud_sof_cb() function which is being called when it's enabled at least.

HiFiPhile commented 1 month ago

Is this different? I noticed they already had something like this?

They are similar, except dcd_event_sof() pass the frame number which is useful mostly for audio application.

_I need to fix tud_sof_cb() always receive framecount 0

HiFiPhile commented 1 month ago

I just fixed the CI issue and frame count in tud_sof_cb().

andrewleech commented 1 month ago

I've added dcd_event_sof() for the 4 ports now covered in this PR. I think I've got the frame count reading correct but haven't had a chance to test any of these yet. I'll get these tested before calling this PR ready.

I haven't tested renesas changes at all yet as I'm chasing some support to get USB working at all on the renesas evk I've got.

The 4 ports covered at this point are the main ones I'm planning on adding at this time as these (mimx, samd, nrf, renesas) are the only ones I've got access to where the matching micropython ports directly use libusb, other than rp2 where these functions have already been added.

I will also look at esp32s3 and esp32s2 are a separate issue, I do want that added too but in expecting some complications from IDF integration. If that's in fact trivial I'll add support for this platform too.

andrewleech commented 1 month ago

mimx and samd frame_count are tested working, eg. image

HiFiPhile commented 1 month ago

I will also look at esp32s3 and esp32s2 are a separate issue, I do want that added too but in expecting some complications from IDF integration. If that's in fact trivial I'll add support for this platform too.

esp32 use dcd_dwc2 which already have sof support, it is little more complicate if you want to try:

# 1. Source ESP-IDF
# Windows
%userprofile%\esp\esp-idf\export.bat
# Linux
. $HOME/esp/esp-idf/export.sh
# 2. Enter example directory
cd tinyusb/examples/device/hid_composite_freertos
# 3. Run cmake in project directory specifying the board
cmake -DBOARD=espressif_s3_devkitc -B build -G Ninja .
ninja.exe -C build
andrewleech commented 1 month ago

I would have assumed esp32s2/3 uses this driver which doesn't have these sof functions configured?

HiFiPhile commented 1 month ago

dcd_esp32sx is old driver still maintained by espressif (with some specific logging), a new driver has been created for all devices with dwc2: src/portable/synopsys/dwc2/dcd_dwc2.c

andrewleech commented 1 month ago

Thanks @HiFiPhile that's really interesting about the esp32sX driver! With that info I've been able to get a build of micropython working on the S3 that's using our existing tinyusb submodule and this driver directly, ignoring most of the esp32 usb implementation. It just took a little bit of extra support to manage switching from USB serial/jtag mode to USB OTG mode, but my test code for that seems to be working pretty well so far. Being able to use the same shared tinyunb intergration code across all ports in micropython will be a great advantage, I appreciate your input here!

andrewleech commented 1 month ago

Thanks for your SOF frame count fix, I've also now finished adding dcd_event_sof() for each of the ports I'm targeting here.

I've been able to test all of them and have confirmed the frame counts are passed through as expected.

In case it helps, the code in using sof interrupt for plus the test printf to confirm SOF frame count is here: https://github.com/andrewleech/micropython/blob/54e0abf5d15441ad7a2a9d8058e45c6f8e193f68/shared/tinyusb/mp_usbd_cdc.c#L124

I now consider this PR complete and ready for final review thanks (pending me rebasing it / fixing the merge conflict).

andrewleech commented 1 month ago

Thanks for your support and reviews @HiFiPhile !