Skip to content

Conversation

@mikesmitty
Copy link
Contributor

@mikesmitty mikesmitty commented Dec 5, 2025

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.

fix: separate host expected bytes from device intended bytes

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) and p_msc->total_len (the device's expected transfer size) could be combined into one variable, originally m.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.

fix: update m.queuedBytes when clamping output to avoid corrupting sentBytes

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.

fix: don't hardcode success return state

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.

fix: validate bmRequestType\nfix: mimic tinyusb behavior in mscStateNeedReset\nfix: CLEAR_FEATURE requests to iface addr are against spec
@deadprogram
Copy link
Member

Can someone please test these changes out on one of the other devices to make sure there is no negative impact?

@mikesmitty
Copy link
Contributor Author

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

@mikesmitty
Copy link
Contributor Author

mikesmitty commented Dec 5, 2025

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.
test_msc.sh

@deadprogram
Copy link
Member

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. test_msc.sh

Thank you, I wanted something like that!

@deadprogram
Copy link
Member

Thank you so much for working on this @mikesmitty now merging.

@deadprogram deadprogram merged commit 39e0333 into tinygo-org:dev Dec 6, 2025
19 checks passed
@mikesmitty mikesmitty deleted the ms/usb-msc-cleanup branch December 6, 2025 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants