fix: separate host expected bytes from device intended bytes#5109
fix: separate host expected bytes from device intended bytes#5109deadprogram merged 3 commits intotinygo-org:devfrom
Conversation
fix: validate bmRequestType\nfix: mimic tinyusb behavior in mscStateNeedReset\nfix: CLEAR_FEATURE requests to iface addr are against spec
|
Can someone please test these changes out on one of the other devices to make sure there is no negative impact? |
|
Oh, I should note this was all tested on an rp2350 since it's more or less the reference implementation at the moment. Behind this one I've got a small backwards-compatible refactor of the machine and usb package interactions to allow unit testing the msc package along with a few tests |
|
This is the script I've been using for testing (aside from the actual usb snooping with wireshark and usbmon). The discard feature isn't really useful without udev rules or manual fiddling on linux and finding a way of making it work is something I've been poking at off and on. |
Thank you, I wanted something like that! |
|
Thank you so much for working on this @mikesmitty now merging. |
Here's a few bugfixes for issues I came across working on the nrf52840 usb msc support in #5011. I kept chasing a failure to return a CSW (post-command status report) after the host requested one of a few different unsupported commands. Fixing one broke the other and so I did some digging into places where the code was diverging from the original TinyUSB reference logic. I'm still working on the nrf52840 bit, but I want to make sure the msc library itself is correct and testable so I don't just shift the bug around.
This first commit solves a bug I introduced while porting the TinyUSB stack over. I naively assumed
p_msc->cbw.total_bytes(the host's expected transfer size) andp_msc->total_len(the device's expected transfer size) could be combined into one variable, originallym.totalBytes. As it turns out, the device's transfer size is routinely smaller than what the host provides for, such as in the case of inquiry commands where the host sends a large expected transfer size to give the device plenty of room to provide all the data it needs, while less complex devices like this usb msc stack will only return a small fraction of that, and the difference (referred to as residue in usb) needs to be kept track of in case of errors. This change separates the usage more clearly, always referencing the host's expected total byte value through the CBW.This one handles the situation where the device is returning less data than the host expects such as during inquiry or vpd commands where the returned output can vary greatly in length depending on the device.
This one's pretty self-explanatory, just ensure we don't override an error state set earlier when a CSW is sent after the endpoints are un-stalled.