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: <stable@xxxxxxxxxxxxxxx> #v2.6.35
>> Tested-by: Felipe Balbi <balbi@xxxxxx>
>> Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
>> ---
>> 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 stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html