On 01/12/2012 05:50 AM, Sarah Sharp wrote:
> On Tue, Dec 20, 2011 at 02:10:04PM -0800, Sarah Sharp wrote:
>> On Tue, Dec 20, 2011 at 04:01:32PM +0800, Andiry Xu wrote:
>>> There is some code related to Set TRDP command completion, in patch 3
>>> "count free TRBs on transfer ring", when process a set TRDP event,
>>> driver will update num_trbs_free by walking over the ring when update
>>> dequeue pointer. Can you check the code to see if that's right?
>
> Hi Andiry,
>
> I haven't had time to test your patchset closely, but I think it is the
> Set TR dequeue pointer code that's bad. I was able to trigger a
> hard-hang when I plugged in a new USB 3.0 card reader that I got for
> Christmas. The card reader was empty, and it stalled one of the first
> SCSI commands that came to it. When handling the Set TR dequeue pointer
> event, the machine hung. So you don't have to trigger the ring
> expansion at all to trigger the bug.
>
> I don't have time to look at your code this week, but maybe you could
> re-look at the TRB counting code and test with a USB storage device that
> stalls SCSI commands?
>
Sarah,
I've tested a USB2.0 HDD which stalls SCSI command, and I can see Set
TRDP command issued and handled several times. However I cannot trigger
the hang, everything seems normal. I made a test patch to revert the Set
TRDP part. Can you help test it? Thanks for your help.
Thanks,
Andiry
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 520ec6e..9477aa7 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -945,19 +945,8 @@ static void handle_set_deq_completion(struct xhci_hcd *xhci,
/* Update the ring's dequeue segment and dequeue pointer
* to reflect the new position.
*/
- while (ep_ring->dequeue !=
- dev->eps[ep_index].queued_deq_ptr) {
- /* We have more usable TRBs */
- ep_ring->num_trbs_free++;
- ep_ring->dequeue++;
- while (last_trb(xhci, ep_ring, ep_ring->deq_seg,
- ep_ring->dequeue)) {
- ep_ring->deq_seg =
- ep_ring->deq_seg->next;
- ep_ring->dequeue =
- ep_ring->deq_seg->trbs;
- }
- }
+ ep_ring->deq_seg = dev->eps[ep_index].queued_deq_seg;
+ ep_ring->dequeue = dev->eps[ep_index].queued_deq_ptr;
} else {
xhci_warn(xhci, "Mismatch between completed Set TR Deq "
"Ptr command & xHCI internal state.\n");
