8000 lib/tinyusb: Update TinyUSB to release 0.17.0 by dpgeorge · Pull Request #15902 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

lib/tinyusb: Update TinyUSB to release 0.17.0#15902

Merged
dpgeorge merged 2 commits intomicropython:masterfrom
dpgeorge:lib-tinyusb-update-to-0170
Sep 26, 2024
Merged

lib/tinyusb: Update TinyUSB to release 0.17.0#15902
dpgeorge merged 2 commits intomicropython:masterfrom
dpgeorge:lib-tinyusb-update-to-0170

Conversation

@dpgeorge
Copy link
Member
@dpgeorge dpgeorge commented Sep 24, 2024 •
edited
Loading

Summary

TinyUSB (finally!) has a new release, 0.17.0. Usually we track latest master but now there's a release it would be good to update to that.

More importantly, an updated TinyUSB is needed for both RP2350 support #15619 and ESP32 shared TinyUSB integration #15108. Both those PRs update TinyUSB to a commit prior to the 0.17.0 release, and it would be good to consolidate those updates into a single update of TinyUSB, hence this PR.

Testing

Testing is needed for all existing TinyUSB ports:

  • mimxrt
  • nrf
  • renesas-ra FS
  • renesas-ra HS
  • rp2
  • samd

@dpgeorge dpgeorge added the lib Relates to lib/ directory in source label Sep 24, 2024
@dpgeorge dpgeorge added this to the release-1.24.0 milestone Sep 24, 2024
This was referenced Sep 24, 2024
Merged
Merged
@codecov
Copy link
codecov bot commented Sep 24, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.57%. Comparing base (40048f0) to head (b0ba151).
Report is 2 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #15902   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files         164      164           
  Lines       21336    21336           
=======================================
  Hits        21031    21031           
  Misses        305      305

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link
github-actions bot commented Sep 24, 2024

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:   +56 +0.015% TEENSY40
        rp2:   +40 +0.004% RPI_PICO_W
       samd:   +68 +0.025% ADAFRUIT_ITSYBITSY_M4_EXPRESS[incl +4(bss)]
  qemu rv32:    +0 +0.000% VIRT_RV32

@robert-hh
Copy link
Contributor
robert-hh commented Sep 24, 2024 •
edited
Loading

Tested with

  • MIMXRT 1062 (Teensy 4.0), MIMXRT1011, MIMXRT1015, MIMXRT1021, MIMXRT1052, MIMXRT1172
  • SAMD21 with and without external flash ,
  • SAMD51,
  • RP2040, RP2350 ARM, RP2350 RISCV
  • NRF52840

all build and work. NRF seems better than before. Code size increase for SAMD21 is 48bytes. Lacking a ARDUINO_PORTENTA_C33 board I could not test it with Renesas-RA. But at least the firmware builds.

@andrewleech
Copy link
Contributor
andrewleech commented Sep 24, 2024

Thanks @robert-hh I've got a renesas FS USB board I'll be able to test with.

@iabdalkader no pressure but if you get a chance to test on the portenta renesas or would be good to confirm it hasn't been broken by this change.

@dpgeorge
Copy link
Member Author
dpgeorge commented Sep 25, 2024

@robert-hh thanks for testing! That's very helpful.

I also tested with the following:

  • rp2 / Pico
  • samd / Itsy Bitsy M0 Express
  • mimxrt / Teensy 4.0

I didn't see any regressions. USB CDC throughput is unchanged.

@andrewleech
Copy link
Contributor
andrewleech commented Sep 25, 2024

USB CDC throughput is unchanged.

Hi @dpgeorge do you have a script/pattern you could share that you use to test CDC throughout? I'd like to check that before/after on renesas and esp boards.

@dpgeorge
Copy link
Member Author
dpgeorge commented Sep 25, 2024

do you have a script/pattern you could share that you use to test CDC throughout?

Yes! See #15909.

@andrewleech
Copy link
Contributor
andrewleech commented Sep 25, 2024

Thanks for that @dpgeorge !
Renesas FS USB is slightly slower (right is with this PR applied), but not dramatically:
image

@dpgeorge
Copy link
Member Author
dpgeorge commented Sep 25, 2024

You probably need to do a few runs before you can compare code changes. Eg the load of your PC can change the throughput.

@andrewleech
Copy link
Contributor
andrewleech commented Sep 25, 2024 •
edited
Loading

Yeah I did a few repeats and got similar results, though to be fair I didn't go back to original to re-check that.
I did also try applying [renesas-ra/usb: Use interrupt rather than polling for usb task.(]#15550 ) but didn't see any change to the speed - I guess the polling will be working just fine with this test code where it calls sleep in loops etc.

edit: Ok I did rerun both MR code then went back to master and got similar results on the large block sizes: 0.53 MBits/sec vs 0.56 MBits/sec. Not a deal breaker I don't think, but curious.

@dpgeorge
Copy link
Member Author
dpgeorge commented Sep 26, 2024

@andrewleech so you were able to test the renesas-ra port? If it worked then I think we are good to go with this PR.

@andrewleech
Copy link
Contributor
andrewleech commented Sep 26, 2024 •
edited
Loading

@dpgeorge sorry I wasn't entirely clear - yes renesas-ra port is tested for FS connections and it does work. I don't have hardware to test HS myself though. FS is slightly slower but not enough to prevent moving forward I don't think.

@dpgeorge
Copy link
Member Author
dpgeorge commented Sep 26, 2024

@andrewleech OK, thanks. So the remaining test would be for renesas-ra port in HS mode.

@iabdalkader are you able to test this PR on an Arduino Portenta C33?

@iabdalkader
Copy link
Contributor
iabdalkader commented Sep 26, 2024

@iabdalkader are you able to test this PR on an Arduino Portenta C33?

@andrewleech @dpgeorge Yes I can test it today. I tested this too #15550 and it works fine, if you merge it first it will be easier for me to test both.

@dpgeorge
Copy link
Member Author
dpgeorge commented Sep 26, 2024

I tested this too #15550 and it works fine, if you merge it first it will be easier for me to test both.

OK, that's now merged.

@iabdalkader
Copy link
Contributor
iabdalkader commented Sep 26, 2024

I pulled this PR locally, and rebased on master and tested C33, and it seems to work fine.

@dpgeorge
lib/tinyusb: Update to version 0.17.0.
09fa90e 
Includes support for RP2350, and improvements for ESP32.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
shared/tinyusb: Use new persistent-tx-fifo configure interface.
b0ba151 
The old configuration option has been removed from TinyUSB.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge force-pushed the lib-tinyusb-update-to-0170 branch from 46e6ee8 to b0ba151 Compare September 26, 2024 13:15
@dpgeorge
Copy link
Member Author
dpgeorge commented Sep 26, 2024

Thanks @iabdalkader for testing.

@dpgeorge dpgeorge merged commit b0ba151 into micropython:master Sep 26, 2024
@dpgeorge dpgeorge deleted the lib-tinyusb-update-to-0170 branch September 26, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

lib Relates to lib/ directory in source

Projects

None yet

Milestone

release-1.24.0

Development

Successfully merging this pull request may close these issues.

4 participants

@dpgeorge @robert-hh @andrewleech @iabdalkader
0