RE: [PATCHv2 1/4] xhci: don't start a halted endpoint before its new dequeue is set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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.

For safety you shouldn't change the 'cycle' bit until it is safe
for the controller to process the ring(s).

	David



--
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




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]