On Tue, 1 Aug 2006, David Brownell wrote: > > Yes, the URB remained in ep->urb_list and hcd_endpoint_disable() did see > > it when it went through the loop again. But urb->status had been changed > > to -ESHUTDOWN, so hcd_endpoint_disable() simply skipped over it and did > > not keep retrying to unlink it. (It would be a bug for usbcore to call a > > dequeue method more than once for the same URB.) > > So that'd be a bug; it should busy-wait until list_empty(ep->urb_list) > somehow, maybe an msleep() would suffice.
People tend to prefer some sort of active signal over a busy-wait. Calling usb_kill_urb ought to work. > > The real problem was this: hcd_endpoint_disable() relies on > > hcd->driver->endpoint_disable() to wait until all URBs for an endpoint > > have completed. Normally that's fine, but it doesn't work with URBs for > > endpoints on the root hub -- because these URBs never go through the > > hcd->driver->enqueue() pathway to begin with. > > > > So we needed another way to wait for them. My solution was to wait in > > usb_rh_urb_dequeue(). Maybe there's a better way. > > I still don't follow. If the root hub control URBs don't go through > that code path, how would endpoint_disable() be able to notice them > in the first place? You are getting mixed up between two different levels of endpoint_disable. There's hcd_endpoint_disable() in hcd.c, and it calls the hcd->driver->endpoint_disable() method in the HCD. Thus both routines get used, even for root hubs, but the second (i.e., the HCD's method) is unaware of root-hub URBs. > > Note that hcd_endpoint_disable() doesn't wait for ep->urb_list to be > > empty. It merely waits for hcd->driver->endpoint_disable() to return. > > But _should_ it wait for urb_list to be empty, before calling the > driver's endpoint_disable()? My understanding was that the HCD's endpoint_disable wouldn't return until both the software and the hardware resources for that endpoint had been safely released. For regular endpoints, the software resources include all the URBs that the HCD knows are queued for the endpoint. But for root hubs, the HCD is not aware of the endpoint queue. It certainly doesn't have any hardware resources in use for root-hub endpoints. I don't think it matters when we wait for urb_list to be empty; either before or after calling the HCD's endpoint_disable should be okay. If we do it after then no waiting should be needed, since the HCD will have done the waiting for us -- except for root-hub endpoints. If you don't want to depend on the HCD method waiting, then we would indeed have to add code in hcd_endpoint_disable to make it wait. > To the extent that root hub urbs -- control or status -- get special > treatment, maybe there should just be a per-usb_device counter of > how many are in flight, and that should go to zero before stop(). Since we don't support queues for root-hub endpoints, it would suffice to wait for the current URB to complete. That's why I suggested calling usb_kill_urb. Or we could just do what my patch did: enable interrupts before calling calling the unlink1 routine, and continue to rely on usb_rh_urb_dequeue to do the waiting. Alan Stern ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
