On Wed, Nov 17, 2010 at 04:26:50PM -0800, Paul Zimmerman wrote:
> We have been having problems with the USB-IF Gold Tree tests when plugging
> and unplugging devices from the tree. I have seen that the reset-device
> and configure-endpoint commands, which are invoked from
> xhci_discover_or_reset_device() and xhci_configure_endpoint(), will sometimes
> time out.
>
> After much debugging, I determined that the commands themselves do not actually
> time out, but rather their completion events do not get delivered to the right
> place.
>
> This happens when the command ring has just wrapped around, and it's enqueue
> pointer is left pointing to the link TRB. xhci_discover_or_reset_device() and
> xhci_configure_endpoint() use the enqueue pointer directly as their command
> TRB pointer, without checking whether it's pointing to the link TRB.
>
> When the completion event arrives, if the command TRB is pointing to the link
> TRB, the check against the command ring dequeue pointer in
> handle_cmd_in_cmd_wait_list() fails, so the completion inside the command does
> not get signaled.
>
> The patch below fixes the timeout problem for me.
>
> Signed-off-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
Your signed-off-by email address is different from the email you're
currently sending from. Which one is correct? Otherwise looks fine.
Sarah Sharp
> ---
> drivers/usb/host/xhci.c | 18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 06fca08..45e4a31 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1549,6 +1549,15 @@ static int xhci_configure_endpoint(struct xhci_hcd *xhci,
> cmd_completion = command->completion;
> cmd_status = &command->status;
> command->command_trb = xhci->cmd_ring->enqueue;
> +
> + /* Enqueue pointer can be left pointing to the link TRB,
> + * we must handle that
> + */
> + if ((command->command_trb->link.control & TRB_TYPE_BITMASK)
> + == TRB_TYPE(TRB_LINK))
> + command->command_trb =
> + xhci->cmd_ring->enq_seg->next->trbs;
> +
> list_add_tail(&command->cmd_list, &virt_dev->cmd_list);
> } else {
> in_ctx = virt_dev->in_ctx;
> @@ -2272,6 +2281,15 @@ int xhci_discover_or_reset_device(struct usb_hcd *hcd, struct usb_device *udev)
> /* Attempt to submit the Reset Device command to the command ring */
> spin_lock_irqsave(&xhci->lock, flags);
> reset_device_cmd->command_trb = xhci->cmd_ring->enqueue;
> +
> + /* Enqueue pointer can be left pointing to the link TRB,
> + * we must handle that
> + */
> + if ((reset_device_cmd->command_trb->link.control & TRB_TYPE_BITMASK)
> + == TRB_TYPE(TRB_LINK))
> + reset_device_cmd->command_trb =
> + xhci->cmd_ring->enq_seg->next->trbs;
> +
> list_add_tail(&reset_device_cmd->cmd_list, &virt_dev->cmd_list);
> ret = xhci_queue_reset_device(xhci, slot_id);
> if (ret) {
> --
> 1.6.5.2
>
--
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
