esp32/esp32_common.cmake: Switch back to the vendored TinyUSB copy. #17913
Conversation
This commit reverts d737112, fixing USB issues reported by macOS users on ESP32S2/ESP32S3 boards. The commit's intention was to rely on the TinyUSB tree provided by Espressif as a dependency of their USB support library (so maintenance and other bug fixes would come from them rather than from MicroPython). However, this seems to have opened a Pandora's box of incompatibilities triggered by macOS's USB stack behaviour (see discussion micropython#17465 and issues micropython#17560 and micropython#17865). Whilst this may look like a permanent solution, it is not. There are some important race condition fixes added in Espressif's patched USB code that are not available in MicroPython's TinyUSB copy (along other bug fixes and feature additions). Still, cutting macOS users out of their boards probably isn't the best thing to do. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
|
Thanks @agatti for digging into this.
Just to check all the details, you don't get the stress test lockup with the version in this PR? Has anyone else reported problems when running the newer versions of the Espressif components? Given that Espressif are likely to continue to debug and patch issues in their forks and then submit them upstream periodically into the future, strategically it might still be better to use their fork rather than upstream TinyUSB (especially with things like ESP32-P4 support looming on the horizon). What do you think? If we can isolate the stress test lockup to TinyUSB then we can also report this to them (although one thing I can't account for is neither ESP-IDF or esp-tinyusb have similar bug reports regarding macOS+USB that I can see, which makes me think our unusual TinyUSB integration in MicroPython might be a contributing factor...) Regarding JTAG debugging the stress test, I can borrow a Mac laptop and do some testing here if I don't need to set up a full development environment on the Mac. Is it easy to explain how to run it? |
|
I can run stress tests on an old Macbook Air if needed. |
That's correct, I don't encounter lockups with this commit applied. Regarding newer component versions: yes, there have been problem reports. #17865 (comment) for example, in which an S3 devkit board that used to work before now hangs at REPL with the updated Espressif components. However that same issue is also fixed by those same components on two different boards that also had the issue in the first place.
Personally I'm all for it. If once this gets sorted out a similar issue arises it should be easy to switch to a local TinyUSB repo copy, that's what we've been doing after all.
That's the interesting bit. The PHY initialisation code in MPy doesn't set external GPIO pin indices in the configuration structure, but since only the internal interface is used it shouldn't really matter. I'd wager if that was the problem it would have shown up on Linux and WSL too. Everything else is virtually the same, except for how the TinyUSB task is run. That could be related to the fact that the issue mainly presents on macOS as TinyUSB's state machine may be a step or two behind what it expects to be? I don't think modern macOS versions let you dump USB traffic, but I can take a look at that to see if a particular pattern emerges.
This should be enough: dd if=/dev/urandom of=file bs=250000 count=1
for i in $(seq 0 50); do mpremote cp file :file-$i; mpremote cp :file-$i file-$i; diff file file-$i; doneOn the laptop/board I run this it hangs on file number 35. Sometimes on smaller runs it'd complete all transfers but then hang on
For the record, I can make things hang on macOS 13.7.6 - maybe older versions (or newer, that is) may not exhibit the issue. I wish I could provide more help on this but things are "interesting" on my end to say the least. |
|
If that helps, here's a wireshark dump of the USB interface for a failed transfer between macOS 13.7.6 and an ESP32S3 using the release v1.26.0 firmware off micropython.org. There's some fluff at the beginning, but if you need any pointers in the timeline:
|
Summary
This PR reverts d737112, fixing USB issues reported by macOS users on ESP32S2/ESP32S3 boards.
The commit's intention was to rely on the TinyUSB tree provided by Espressif as a dependency of their USB support library (so maintenance and other bug fixes would come from them rather than from MicroPython).
However, this seems to have opened a Pandora's box of incompatibilities triggered by macOS's USB stack behaviour (see discussion #17465 and issues #17560 and #17865).
Whilst this may look like a permanent solution, it is not. There are some important race condition fixes added in Espressif's patched USB code that are not available in MicroPython's TinyUSB copy (along other bug fixes and feature additions). Still, cutting macOS users out of their boards probably isn't the best thing to do.
Testing
Before this change, copying a file to the board from a macOS machine via USB using
mpremotewould fail either on the first or on the second iteration.With this change applied, repeated copies to and from an ESP32S3-DevKitC-R8N16 succeed without problems.
Trade-offs and Alternatives
The only trade-off I can see right now is that by reverting to that particular TinyUSB version we're missing on race condition fixes being added both by the TinyUSB authors and by Espressif.
Alternatives? Where to start... There seems to be a bunch of incompatibilities manifesting at the same time only in a very specific situation:
espressif/esp_tinyusbandespressif/tinyusbcomponents inports/esp32/main/idf_component.ymlthings seem to be fixed for some users although with the same versions set I could still get my board to lock up during a stress testespressif/tinyusbmatch the version of what's present inlib/tinyusb, the issue still appears - so this seems related toespressif/esp_tinyusbinstead, but again, changing version numbers changes its final behaviourI got a macOS machine on loan from a friend and I can at least replicate the mpremote issues on my board so I can slowly work on this in the meantime (I've been busy with a family situation the past couple of weeks).
I've only got one S3 board and to have JTAG exposed on GPIOs in order to have USB CDC available an eFuse must be burnt (thanks @projectgus for the help on that!). I'm a bit hesitant to burn the eFuse on my board right now, being my only S3 specimen; I've ordered a new one but the shipping times combined with a lackluster postal service plus holiday season here means somebody else may fix this issue before I am ever able to get a JTAG probe attached to the new board (if that happens I won't mind at all!).
Incidentally, I am able to replicate the same issue on a whole different set of conditions (WCH CH32V208 single-core MCU using STM's DCD while connecting from Linux) using the vendored TinyUSB copy, but that may be caused by a coding error on my end.