Both email addresses are valid (one is an alias for the other, or something like that). -- Paul > -----Original Message----- > From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] > Sent: Wednesday, November 17, 2010 4:56 PM > To: Paul Zimmerman > Cc: linux-usb@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] xhci: Fix reset-device and configure-endpoint commands > > 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
