On 11/15/2011 02:01 AM, Sarah Sharp wrote:
On Fri, Nov 11, 2011 at 10:48:36AM +0800, Andiry Xu wrote:
On 11/11/2011 04:36 AM, Sarah Sharp wrote:
I think there might be a couple other subtle issues, but I need to apply
your patchset and check.
Thanks, will submit again after your test.
Ok, I think there's one more thing you need to think about. What
happens if a driver is enqueueing an isochronous URB, and
xhci_queue_isoc_tx_prepare() succeeds because there is enough room on
the ring, but prepare_transfer() fails for the Nth TD because the
endpoint ring state changed (see commit 522989a27)?
That error handling code will mess with your TRB counting. inc_enq()
will have been called for each isochronous TD that was queued, but when
the Nth TD fails, the TDs will get turned into no-ops (with the cycle
bit set to hardware-owned), and xhci_queue_isoc_tx() will simply reset
the enqueue pointer to the TRB where the isochronous URB first started
being enqueued. That means inc_enq() will be subtracting TRBs from the
number of free TRBs that really need to get added back when the enqueue
pointer gets moved back.
You might want to search for other places where ep_ring->enqueue or
ep_ring->dequeue gets set, to make sure the TRB counting behavior is
correct.
Yes, free TRB numbers should be modified whenever the enqueue/dequeue
pointer is modified. Thanks for the point, I'll revise the patch.
Thanks,
Andiry
--
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
