On Fri, 10 Apr 2009, Kung James wrote:
> The USB 2.0 spec. section 11.18.1 says, "a microframe of time
> allows at most 187.5 raw bytes of signaling on a full-speed bus." and
> "The best case full-speed budget assumes that 188 full-speed bytes
> occur in each microframe."
>
> If a full-speed isoc. packet which max. size is bigger than 188 bytes
> or its max. transfer time is over 125us, the full-speed isoc. packet may
> fully occupy one or more microframe.
>
> So the "tt_available" function must to make sure that those fully
> occupied microframes are not used by other full- or low-speed
> transactions.
I don't understand the purpose of the entire region of code under
discussion:
/* special case for isoc transfers larger than 125us:
* the first and each subsequent fully used uframe
* must be empty, so as to not illegally delay
* already scheduled transactions
*/
if (125 < usecs) {
int ufs = (usecs / 125) - 1;
int i;
for (i = uframe; i < (uframe + ufs) && i < 8; i++)
if (0 < tt_usecs[i]) {
ehci_vdbg(ehci,
"multi-uframe xfer can't fit "
"in frame %d uframe %d\n",
frame, i);
return 0;
}
}
The comment's explanation isn't clear enough. What does "illegally
delay already scheduled transactions" mean? What's wrong with pushing
back existing transactions to later in the frame? And why do transfers
longer than 125 us need special treatment? Neither the USB 2.0 spec
nor the EHCI spec mentions anything about this.
> But currently the "tt_available" function doesn't check
> the last fully occupied microframe.
I don't see why it has to check any uframe at all... :-)
> My test case is described as follow:
>
> First, submit a intr. IN urb to a low-speed device (mouse), and then submit
> many isoc. OUT urbs with packet size 192 bytes to a full-speed device
> (audio device). The EHCI will put the isoc. OUT SSPLIT transaction
> at microframe (0,1), and put the intr. IN SSPLIT transaction at
> microframe (0), and the intr. IN CSPLIT transaction at microframe (1,2,3).
>
> Because the isoc. OUT packet's size is 192 bytes, the EHCI can execute
> 188 bytes isoc. OUT SSPLIT at most in microframe 0, and the rest 4 bytes
> will be executed in the microframe 1. We can observe that the EHCI need
> to execute an 188 bytes isoc. OUT SSPLIT and an intr. IN SSPLIT in
> microframe 0. This may makes the HUB have not enough time to execute
> the intr. IN SSPLIT in microframe 0. (The isoc. OUT SSPLIT will be executed
> first, because of the EHCI periodic frame list structure)
>
> So the schedule result is wrong, the EHCI driver can not put the two split
> transaction at the same microframe. This mistake is due to the "tt_available"
> function doest not check the last fully used uframe for the isoc. OUT split
> transaction. (The intr. IN SSPLIT is already scheduled at microframe 0 by
> EHCI driver)
You mean that the scheduler should move the interrupt transaction back
later in the frame when the isoc transfer is added, but it fails to
do so? (I.e., it doesn't rebalance the schedule as described in
4.12.2.5 of the EHCI spec?) Yes, that would be a bug.
In fact, does ehci_hcd ever rebalance the periodic schedule? If it
doesn't then the proposed fix to tt_available() is clearly inadequate.
For example, suppose that before the isoc transfer was added there
already were three interrupt transfers, of length 64, 64, and 60,
scheduled for uframe 1 (thus using up the entire uframe). Then there
still wouldn't be enough room to add the isoc transfer, but the patched
code wouldn't know there was a problem.
Or take another example. Suppose we already have an isoc transfer of
length 154 and an interrupt transfer of length 32 scheduled in uframe
0. Is there enough room to add a new isoc transfer of length 40? Not
if the interrupt transfer can't be pushed back to uframe 1 -- but
tt_available wouldn't realize this since none of the transfers lasts
more than 125 us.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
