On 17.11.2014 16:52, David Laight wrote: > From: Mathias Nyman >> A halted endpoint ring must first be reset, then move the ring >> dequeue pointer past the problematic TRB. If we start the ring too >> early after reset, but before moving the dequeue pointer we >> will end up executing the same problematic TRB again. >> >> As we always issue a set transfer dequeue command after a reset >> endpoint command we can skip starting endpoint rings at reset endpoint >> command completion. >> >> Without this fix we end up trying to handle the same faulty TD for >> contol endpoints. causing timeout, and failing testusb ctrl_out write >> tests. >> >> Fixes: e9df17e (USB: xhci: Correct assumptions about number of rings per >> endpoint.) >> Cc: <sta...@vger.kernel.org> #v2.6.35 >> Tested-by: Felipe Balbi <ba...@ti.com> >> Signed-off-by: Mathias Nyman <mathias.ny...@linux.intel.com> >> --- >> drivers/usb/host/xhci-ring.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c >> index bc6fcbc..d6646db 100644 >> --- a/drivers/usb/host/xhci-ring.c >> +++ b/drivers/usb/host/xhci-ring.c >> @@ -1069,7 +1069,6 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd >> *xhci, int slot_id, >> } else { >> /* Clear our internal halted state and restart the ring(s) */ >> xhci->devs[slot_id]->eps[ep_index].ep_state &= ~EP_HALTED; >> - ring_doorbell_for_active_rings(xhci, slot_id, ep_index); > > The comment is now wrong - you aren't restarting the ring(s) here.
Thats right, comment is still wrong, thanks. I'll resend. > > Also I'm not at all certain that delaying 'ringing the doorbell' is in anyway > guaranteed to stop the xhci hardware from processing the next ring entry. > > It might be true if the ring is absolutely known to be inactive, and that > the controller isn't polling the rings on timeout. > But if the controller is still processing the previous ring entry it > will process the next one without the bell being rung. > Plausibly some other driver path might also ring the bell. The ring is known to be inactive. The only use for reset endpoint command is to revocer a halted endpoint ring. A reset endpoint on a non-halted endpoint will fail. A reset endpoint command will move the ring from a halted to a stopped state. Ringing the doorbell will restart the stopped ring. But we don't want to ring the doorbell until we moved past the problematic TRB with a set dq pointer command. xhci 4.6.8: The Reset Endpoint Command is issued by software to recover from a halted condition on an endpoint. ... If the endpoint is not in the Halted state when an Reset Endpoint Command is executed: The xHC shall reject the command and generate a Command Completion Event with the Completion Code set to Context State Error. > > For safety you shouldn't change the 'cycle' bit until it is safe > for the controller to process the ring(s). > It won't, as said, reset endpoint will only work if endpoint is halted. -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html