Hi Chintan, I think I have a fix for the TD size issue. Can you install a custom kernel and test it out on your host controller? The directions for building a custom kernel are here: http://kernelnewbies.org/KernelBuild Instead of running any of the commands in "Which kernel to build?" section, use these commands instead: git clone git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git -b for-usb-linus-queue cd xhci Use the "Duplicating your current config" section. If you have trouble booting the 3.7-rc2 kernel, let me know and I'll rebase the patch against a stable kernel version. Sarah Sharp On Thu, Oct 25, 2012 at 03:32:08PM -0700, Sarah Sharp wrote: > 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 -- 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
