On Mon, 25 Nov 2013, Xenia Ragiadakou wrote: > > You can simplify part of the problem by not allowing an endpoint to be > > reset if it has any pending URBs. Just fail the reset in this case. > > Yes, you are right since, from what i understand, it is the > responsibility of the device driver to unlink any pending URBs before > issuing a clear halt to the endpoint. So we expect the device driver to > call usb_unlink_urb() for each pending URB, which in turn calls Actually you should expect the driver to call usb_kill_urb(), not usb_unlink_urb(). The difference is that usb_kill_urb() waits for the URB to complete. > xhci_urb_dequeue(). The function xhci_urb_dequeue() will add the URB to > endpoint's cancelled_td_list and will issue a Stop Endpoint command. The > completion handler of the Stop Endpoint command, > xhci_handle_cmd_stop_ep(), will eventually remove the unlinked URBs from > endpoint's both cancelled_td_list and td_list and give them back to core. > However, since xhci_urb_dequeue() does not wait on the completion of the > Stop Endpoint command, it is still possible for the URB not to have been > removed yet from the td_list when the xhci_reset_endpoint() is called, > right? Not if the driver uses usb_kill_urb(). In that case the URB will already be removed from the td_list. > In that case, is it better to check if td_list is not empty and fail the > reset after xhci_stop_endpoint_for_config() rather than before? It probably doesn't matter. > Also, regarding the case that there is an attempt to enqueue a URB while > the endpoint is being reset, is it ok to fail the enqueue by adding a > check in prepare_ring() to fail when the endpoint's state is > EP_CONFIG_PENDING ? That's up to Sarah (but I can't think of any alternative). If you decide to do this, add an explanation of the new error code to Documentation/usb/error-codes.txt. 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
