Fix cxd56 msc #1075
Merged
Fix cxd56 msc #1075
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 2e23f3d.
kamtom480
approved these changes
Sep 7, 2021
|
thank you for your review |
kamtom480
reviewed
Sep 7, 2021
Windows only has python.exe there is no python3.exe even though you install 3.9x |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe the PR
Revert changes in #1070 , fix the root cause of the issue with cxd56. Following is the explanation (copied from other pr)
@kamtom480 the stall condition (before PR logic) is indeed correct here, according to the MSC BOT 13 cases. this particular if is for case 5 when Host expect more data than Device could send. In this particular case is Read Format Capacity command, where host expect 252 bytes, and we only send 12 bytes in response. Device really need to send an STALL first, then resume to send SCSI status afterwards when received clearing stall request https://github.com/hathach/tinyusb/blob/master/src/class/msc/msc_device.c#L318. Doing differently by this PR will cause MSC compliance test suite to fail.
The issue is spresense has its own usb stack under nuttx system, and it Doesn't forward the Clear Feature (Endpoint) request toward tinyusb. https://github.com/sonydevworld/spresense-nuttx/blob/2ffe58b6dcb8ca7e83a4d2acec5622276f7c6f5d/arch/arm/src/cxd56xx/cxd56_usbdev.c#L1367
It is quite troublesome, since most of the msc recover is based on the Clear Stall request. I will try to figure an way. In general the setup seem to be conditionally forwards (most but not all), and Status stage can also be auto ACKed. Which could cause race condition where next setup arrive while tinyusb task doesn't fully process previous packet hence the setup queue.
WALKAROUND
The workaround is unconditionally unstall the status endpoints for cxd56 and hope for the best, which won't pass compliance test suite, but will work for most of the case with sensible host driver.