On Fri, Aug 05, 2011 at 01:45:39PM +0800, Xu, Andiry wrote:
> > @@ -1252,6 +1252,13 @@ int xhci_urb_dequeue(struct usb_hcd *hcd,
> struct
> > urb *urb, int status)
> > if (temp == 0xffffffff || (xhci->xhc_state & XHCI_STATE_HALTED))
> > {
> > xhci_dbg(xhci, "HW died, freeing TD.\n");
> > urb_priv = urb->hcpriv;
> > + for (i = urb_priv->td_cnt; i < urb_priv->length; i++) {
> > + td = urb_priv->td[i];
> > + if (!list_empty(&td->td_list))
> > + list_del_init(&td->td_list);
> > + if (!list_empty(&td->cancelled_td_list))
> > + list_del_init(&td->cancelled_td_list);
> > + }
> >
>
> Can we just add the urb's unprocessed tds to cancelled_td_list here, and
> let the watchdog function give it back and free urb_priv later? I think
> that complies with the normal path we dequeue a urb.
The interesting thing is that the watchdog timer might not actually be
running when this code is hit. The conditional also checks to see if
the host controller has been physically removed from the ExpressCard
slot by checking temp. There's no guarantee that a command will be
running when the host is removed, so we can't count on the watchdog
timer removing the URB from the TD lists and giving it back to the
driver.
I've even submitted a patch to make sure the xHCI driver doesn't submit
commands to a halted host controller, so we'll never submit a stop
endpoint command when the host is removed, and the watchdog timer will
never run. So I think the code to remove the TDs is necessary.
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
