Going back over your example, it does look there is a couple bugs in the
Linux xHCI TD size calculations. Notes are below, I'll send you a patch
to test out on your host controller shortly.
Thanks for catching this!
Sarah Sharp
On Thu, Oct 25, 2012 at 02:24:04PM -0700, Sarah Sharp wrote:
> On Fri, Oct 19, 2012 at 11:29:44AM +0530, Chintan Mehta wrote:
> > > > > 2. For Bulk Endpoint:
> > > > >
> > > > > - *Driver can put a TD with total TD transfer size less than
> > > maxpacket
> > > > > size and more than 1 TRB?*
> > > > > - For example, Maxpacketsize is 1K. And TD contains 3 TRBs as below:
> > > > > - 1st trb with TRB transfer length 600 Bytes, chain bit 1 and
> > > > > TDSize 0
> > > > > - 2nd trb with TRB transfer length 200 Bytes, chain bit 1 and
> > > > > TDSize 0
> > > > > - 3rd trb with TRB transfer length 100 Bytes, chain bit 0 and
> > > > > TDSize 0
> > > > > - *What should be the value of TDSize in above TRBs of TD?*
> > >
> > > Again, see section 4.11.2.4.
> > >
> > > TRB 1 600 (600 + 200 + 100) >> 10 = 0
> > > TRB 2 200 (200 + 100) >> 10 = 0
> > > TRB 3 100 (100) >> 10 = 0
Let's see what the TD size for a 1.0 host controller should be here.
TD packet count =
roundup(TD size / max packet size) =
roundup(900 / 1024) = 1
Packets Transferred (TRB 1) =
rounddown(TRB length sum(n) / max packet size)
where TRB length sum is the sum of the trb lengths up to and including
this TRB, so
Packets Transferred (TRB 1) = rounddown(600 / 1024) = 0
TD size = (TD packet count - Packets Transferred)
Therefore,
TD size(TRB 1) = (1 - 0) = 1
Packets Transferred (TRB 2) =
rounddown((600 + 200) / 1024) = 0
TD size(TRB 2) = (1 - 0) = 1
The TD size for TRB 3 is supposed to be set to 0, since it is the last
TRB in the TD.
So, the final answer should be
TRB 1: TD size = 1
TRB 2: TD size = 1
TRB 3: TD size = 0
Now let's see what the xHCI driver actually does.
static u32 xhci_td_remainder(unsigned int remainder)
{
u32 max = (1 << (21 - 17 + 1)) - 1;
if ((remainder >> 10) >= max)
return max << 17;
else
return (remainder >> 10) << 17;
}
static u32 xhci_v1_0_td_remainder(int running_total, int trb_buff_len,
unsigned int total_packet_count, struct urb *urb)
{
int packets_transferred;
/* One TRB with a zero-length data packet. */
if (running_total == 0 && trb_buff_len == 0)
return 0;
/* All the TRB queueing functions don't count the current TRB in
* running_total.
*/
packets_transferred = (running_total + trb_buff_len) /
usb_endpoint_maxp(&urb->ep->desc);
return xhci_td_remainder(total_packet_count - packets_transferred);
}
That doesn't look right from the start, because passing the result to
xhci_td_remainder() will left shift it by 10, which isn't what we want.
I'll assume I've fixed that, and make sure the math is right from there.
The total_packet_count passed to xhci_v1_0_td_remainder() looks sane,
looking at how it's calculated in the isochronous and bulk queueing
functions (which also handles the interrupt TD queueing).
running_total is the number of bytes in the previous TRBs (not including
this TRB), and trb_buff_len is the number of bytes in this TRB.
So, for the first TRB, running_total = 0, trb_buff_len = 600, and
total_packet_count = 1.
packets_transferred = (0 + 600) / 1024 = 0
TD size (TRB 1) = (1 - 0) = 1
TRB 2:
packets_transferred = (600 + 200) / 1024 = 0
TD size (TRB 2) = (1 - 0) = 1
TRB 3:
packets_transferred = (600 + 200 + 100) / 1024 = 0
TD size (TRB 3) = (1 - 0) = 1
That last TD size is wrong, of course, since the xHCI spec says it has
to be special-cased. Probably the URB enqueueing functions should
special case that, since they know whether this this the last TRB in a
TD.
So, yes, there are two bugs in the Linux xHCI TD size code, and I'll
send you a patch shortly to fix it.
Sarah Sharp
--
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
