#@retiredwizard In your github comment
south pilot
Okay, I think this will get the thread thing going
My suspicion has been that there's some soft reset occurring to the USB host device and then a timeout has to occur before the IDs become available,
...
it looked to me like there was a race condition (i.e. panic) caused by attempting to start a xfer (or perhaps a second abort) before a previous abort had completed
That seems consistent with what I've observed. My test code calls find() as fast as possible, then prints device descriptor details once it gets a device. I have a gamepad that will cycle through Xbox and Switch descriptors a couple times, seemingly triggered by not getting the kind of handshake it wants quickly enough.
When the gamepad disconnects (using that term loosely, not sure what's going on at the USB bus level), TinyUSB will return a failure result. At that point, if I immediately keep trying to call find() again, I keep getting errors. If I hold off for 15ms by spinning in a loop that calls RUN_BACKGROUND_TASKS (which calls tud_task() and tuh_task()), then I can use find() again normally.
For other devices that only use one device descriptor and don't disconnect on their own, I get similar results if I manually unplug the device. Definitely seems like there's some problem with a race condition, perhaps with a missing mutex, when a device gets unplugged or logically disconnected (whatever that means on the USB wire)
There seems to be some resource that gets tied up inside of TinyUSB with an associated timeout on the order of 10ms
I've also noticed that when things get stuck, it prevents the tuh_umount_cb() callback from getting called by TinyUSB to inform CircuitPython that a device has been unplugged (as monitored by adding LED toggling GPIO pin code to the mount an umount callbacks)
violet iris
I'm looking forward to digging back into this but it takes a bit of focus for me to get down into these weeds and I'm not sure I'm going to get there tonight. I'll keep an eye on the thread this evening but will probably wait until tomorrow to get myself dug back in to where I can start testing again.
violet iris
Well I've got your new PR changes loaded up and I think I've convinced myself I understand the choke points for my configuration (USB_DEVICE=0, USB_HOST=1). This configuration turns off the CIRCUITPY drive but allows the CircuitPython board to act as a USB host over the Raspberry PI USB pins rather than GPIO pins.
I've had to make a few core changes to get this working. I've also modified Device.c to avoid a race condition panic in TinyUSB (I'll add those changes to your PR so you can look them over and let me know what you think).
I'm currently dealing with what I think is an issue unrelated to the Panic and other changes I've made. It may not even be related to USB but there seems to be some minor issue in the data transfer. When a mouse is connected to the USB port it only communicates the button status when the x,y location changes.
I'll start digging into the mouse issue after I update your PR 😁
Actually I don't want to muddy up your PR so I'll post my Device.c changes here:
At the top:
// Track that a xfer has finished before next one starts
static volatile bool _is_transfer_pending = false;
Call back routines:
static void _transfer_done_cb(tuh_xfer_t *xfer) {
// Store the result so we stop waiting for the transfer.
_xfer_result = xfer->result;
// The passed in xfer is not the original one we passed in, so we need to
// copy any info out that we want (like actual_len.)
_actual_len = xfer->actual_len;
// Clear the flag when a transfer ends (or aborts)
_is_transfer_pending = false;
}
static bool _wait_for_callback(void) {
while (!mp_hal_is_interrupted() &&
_xfer_result == XFER_RESULT_INVALID) {
// The background tasks include TinyUSB which will call the function
// we provided above. In other words, the callback isn't in an interrupt.
RUN_BACKGROUND_TASKS;
}
xfer_result_t result = _xfer_result;
_xfer_result = XFER_RESULT_INVALID;
_is_transfer_pending = false;
return result == XFER_RESULT_SUCCESS;
}
static void _prepare_for_transfer(void) {
// Prepare for transfer. Unless there is a timeout, these static globals will
// get modified by the _transfer_done_cb() callback when tinyusb finishes the
// transfer or encounters an error condition.
// Before starting a new transfer, ensure the endpoint is not busy from a previous attempt.
// This prevents the double-submission race. This apparently only happens when TinyUSB
// device tud_task function is not being called.
#if !CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_HOST
while (_is_transfer_pending) {
RUN_BACKGROUND_TASKS;
}
#endif
_xfer_result = XFER_RESULT_INVALID;
_actual_len = 0;
_is_transfer_pending = true;
}
Basically this is adding a new static variable to flag if a transfer is being processed and then delay the start of a new transfer if the flag indicates the last transfer hasn't finished
I have no idea why this causes a Panic/crash when in the native USB port is used in host mode but not when it's used in device mode. I suspect it has something to do with the fact that CircuitPython handles the rpxxxx native USB with it's own PIO code which isn't used in host mode.
south pilot
oh... hmm. That's very interesting
Did you happen to see the latest changes I made about an hour ago?
violet iris
I did see them, I didn't give them a detailed look yet but from my quick scan I didn't think they changed the basic working of your update
south pilot
From looking at your changes, it seems that _is_transfer_pending == true, theoretically (i.e. apparently not actually), ought to serve the same role as _xfer_result == XFER_RESULT_INVALID. But, from the sound of it, that isn't actually happening.
violet iris
You could be right, I should have looked close at the existing variables. I'll look that over and see if I can dump the new one 😁
I also tried hot plugging since you mentioned it earlier and things hung up. I'm thinking I may have gotten stuck in the loop waiting for the call back to complete so I'm rethinking how things work. I don't think it's a good idea to add the untimed infinite loop.....
south pilot
Part of my last commit is meant to catch some edge case behavior where Device.product, Device.manufacturer, and Device.serial_number might leave _xfer_result in an unknown state prior to making a TinyUSB API call. That could potentially cause the _wait_for_callback() function to return immediately with an uncancelled TinyUSB callback still pending.
It would be great to have some documentation of your experiments and findings on GitHub if you're in the mood for that. This is the umbrella issue I made for all the USB stuff that's currently on my radar: https://github.com/adafruit/circuitpython/issues/10551
If one of those linked issues sounds close to what you're seeing, maybe you could leave your stuff in a comment. Otherwise, maybe you could make a new issue describing the test code, hardware, and results. I'm really curious about the panic that you mentioned. As far as I know, I haven't seen that (closest thing would be https://github.com/adafruit/circuitpython/issues/10562 )
violet iris
I suspect the panic is unique to running usb host on RHPORT 0 (non pio) which I don't think is a mode that CircuitPython has ever operated in. When I get things under control, I'll look into trying to create a simple reproducer
At least Rpi CircuitPython....
south pilot
mmm... yeah, I was just re-reading your first comment from earlier about doing host mode over the pins that are normally used for the device port. I think maybe I get what you're doing with the hardware now, sorta. It would be interesting to know if you can reproduce the panic on a Metro RP2350 or an RP2350 Feather using the USB host header pins.
... or on a Fruit Jam using the USB A ports
Everything I've been doing is on a Fruit Jam rev D with the USB A ports.
Could you remind me which board you're using? RPi Pico? RPi Pico 2? One of the Pico W variants? Something custom?
violet iris
I've been working on the Olimex RP2350pc. It's using the Rev B? rp2350, the same one that's on the Fruit Jam. It's very similar to the Fruit Jam except they wired their USB A ports up to the native RP2350 usb pins rather than GPIO pins :/
Has a DAC, SD Card, Speaker, headphone, 4 USBA, no WiFi though
DVI port as well
south pilot
do you have a fruit jam? I just noticed there are actually some in stock at the moment
violet iris
I do have one, it's an early one with problems so I'm going to grab another but I thought I'd wait until Adafruit can keep them in stock so more people can get their hands on them
south pilot
mmm... yeah, mine is from the first production batch. I swapped the pullup resistor for the IR receiver, but otherwise it's stock
I don't think that would have any effect on USB stuff one way or the other though
violet iris
Yea, mine has the IR issue and one of my headphone channels is dead, but I use the external speaker anyway and don't really plan on using the IR receiver so this one is fine for now
south pilot
So, is the Olimex wiring more or less the same as a Pico 2, but with a USB A connector?
(instead of microUSB)
kinda like a USB OTG configuration?
violet iris
No they have two USB C connectors, One for programming and one for power, then they have four USB A connectors connected via a USB hub chip to the same RP2350 pads that the USB C programming port is connected to
south pilot
I'm just thinking of what the closest mapping would be to reproducing an equivalent of that hardware configuration on boards that Adafruit sells
violet iris
SolderParty sells a board with a similar USB configuration. The rp23xx stamp carrier XL or something like that
south pilot
sounds like there could easily be some weird edge case behavior in TinyUSB that's getting triggered by an as-yet untested hardware configuration
I know that there was a recent thing where USB ports on the Feather RP2350 and Metro RP2350 wouldn't work unless you added a USB hub breakout with the same chip as the Fruit Jam
That's been fixed now, but I could easily imagine similar issues with other hub configurations
violet iris
could be
south pilot
No they have two USB C connectors, One for programming and one for power, then t...
oh... wow. that sounds kinda weird
south pilot
I looked at the rp2350pc schematics and manual. Think I more or less get how they've got it wired up and how the programming port is meant to be used. I have no idea how code running on that might or might not differ from code running on Fruit Jam
Very curious to hear more about what you come up with
violet iris
If I can get the USB stable I plan on submitting the PRs but I don't know if Adafruit is going to like the core changes. If the core changes get through it sounds like they'll accept the board 😁
south pilot
Don't know what your intended timeline is, but I get the feeling there will likely be some work by the core devs on USB stuff in the coming weeks
Possible my second attempt at the USB error handling PR will get merged this coming week if Scott likes my changes. That would close one of my USB issues that they put on the 10.0.0 milestone.
That would still leave another 10.0.0 milestone issue for the crash if you write to the CIRCUITPY drive while rapidly polling for input events on a USB host device
From the sound of it, they might have Thach track that one down.
south pilot
From reading the code more closely, it seems like there's a bit of an impedance mismatch between the usb.core.find() implementation and the TinyUSB API calls that it relies on (indirectly by way of tuh_mount_cb() and tuh_umount_cb()). I left some comments about it in the PR
Very likely that's resulting in a race between find() and the TinyUSB device enumeration sequence, which is fairly drawn out and involved.