Opened 11 years ago
Closed 10 years ago
#10867 closed bug (fixed)
EHCI Control Transfers : Hang if length of buffer > size of input data
Reported by: | akshay1994 | Owned by: | phoudoin |
---|---|---|---|
Priority: | normal | Milestone: | R1 |
Component: | Drivers/USB | Version: | R1/Development |
Keywords: | USB EHCI | Cc: | |
Blocked By: | Blocking: | ||
Platform: | All |
Description
EHCI control transfers hang indefinitely, if the length of buffer (wLength) specified, is greater than the size of the object received. This is repeat-able, by performing a control transfer asking for any descriptor, with length set to (size of descriptor + 1).
Attachments (3)
Change History (14)
comment:1 by , 11 years ago
patch: | 0 → 1 |
---|
comment:2 by , 11 years ago
Component: | - General → Drivers/USB |
---|---|
Owner: | changed from | to
comment:3 by , 11 years ago
by , 11 years ago
Attachment: | 0001-Fix-Short-Packet-EHCI-Control-Transfers.patch added |
---|
comment:5 by , 11 years ago
Updated the patch. In case of a Short Packet Control Transfer, EHCI falls back to Alternate_Next_qTD. This made it bypass the Status Stage, due to which an interrupt was not generated on completion of the transfer, causing a delay. :)
comment:6 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 10 comment:7 by , 11 years ago
Thanks for the patch and the bug report. It seems like this isn't the correct fix for the problem. Active transfers are active for a reason, the spec implies that the software shouldn't mess with them, and other implementations actually don't.
comment:8 by , 11 years ago
Thanks a lot for the response. Yes. You are right. Even on a short packet transaction, the HC does set the active bit to 0. (EHCI Spec, Pg 83, Point 4)
(I read the specification wrong, taking a zero length transfer by Host Controller, to be a Short Packet transfer. Sorry!!)
Okay, so why does this still anyway fail. Upon encountering a Short Packet, the HC goes to the Alternate_Next_qTD, which is set to a stray descriptor. This has its active bit set to zero, making the HC take a horizontal jump to the next QueueHead, completing this transaction. Now while processing this in FinishTransfers thread, after checking the short packet qTD, we go to the next qTD, which was never processed by the HC, and thus has its Active Bit set.
Solution I propose : 1) Instead of setting Alternate_Next_qTD to the stray descriptor, we set it to the status descriptor (for all the data descriptors), completing the requirement of a status stage after every control request, and also generating the required interrupt. 2) Upon encountering a short packet descriptor, while processing, we skip all descriptors following this, till we reach the status descriptor, which was next processed by the HC.
Please review this solution, while I make a patch for this. Thanks :)
comment:9 by , 11 years ago
I have modified and corrected the patch.
Changes :
- The Alternate_Next_qTD for all the data descriptors in a control request point to the Status Descriptor of that request. For all others, it points to the Stray Descriptor, as before.
- In the Finishing Transfers Worker, if we detect a Short Packet, we mark the transfer as done, if it is a Bulk/Interrupt transfer (as no further descriptors were processed, and a horizontal jump to the next queue head took place); and jump to the Status descriptor, in case of a Control Transfer.
comment:10 by , 10 years ago
Replying to korli:
Please review this also! This will make libusb work with EHCI :)
Style: missing the != 0 for the first part of the if condition.