On Sat, 29 Jun 2013, Ming Lei wrote: > > The ehci_endpoint_disable() routine can be improved. In the > > QH_STATE_LINNKED or QH_STATE_COMPLETING case, we now need to handle > > interrupt QHs -- the comment about "periodic qh self-unlinks on empty" > > isn't entirely correct any more, because now the unlink doesn't occur > > until the QH has been empty for 5 ms. > > Actually, almost all interrupt URBs are resubmitted in its completion handler, > that means they are basically stopped by unlinking before disabling endpoint, > so I am wondering if we need to consider improving ehci_endpoint_disable() > on interrupt endpoint. Come to think of it, we never should see an endpoint being disabled while there are active URBs. It might be a better idea to put a WARN_ON there and simply return, without unlinking anything. If this ever triggers, it means there's a bug in usbcore. > But I suggest to do the above on in another patch because most of the > change is not much related with this patch. Okay. > >> @@ -921,7 +966,7 @@ static void scan_intr(struct ehci_hcd *ehci) > >> temp = qh_completions(ehci, qh); > >> if (unlikely(temp || (list_empty(&qh->qtd_list) && > >> qh->qh_state == QH_STATE_LINKED))) > >> - start_unlink_intr(ehci, qh); > >> + start_unlink_intr_wait(ehci, qh); > > > > This is not quite right. If temp is nonzero then we want to unlink the > > QH right away (because a fault occurred), so we should call > > It might depend on the fault type, looks we need to unlink qh > immediately on SHUTDOWN and qh->dequeue_during_giveback, and Yes. > for other non-unlink faults, drivers may not treat it as fault and continue > to resubmit URB, such as hub_irq(). No. All faults have to cause the QH to be unlinked. That's another invariant of the driver. This is partly related to the silicon quirk present in some controllers (they perform the overlay even when the Halt bit is set in the qTD). In any case, whenever the QH is halted, the only safe way to recover is to unlink it and then relink. Alan Stern -- 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
