On Wed, Nov 17, 2010 at 01:57:17PM -0800, Paul Zimmerman wrote: > > From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] > > [snip] > > > Please fix, commit, and run `git show HEAD | scripts/checkpatch.pl -`. > > Or if you want git to run checkpatch automatically before you commit, > > you can put this file in your .git/hooks directory: > > http://minilop.net/~sarah/pre-commit > > Depending your git version, you may need to make the file executable. > > > > Thanks for catching this! > > Hi Sarah, > > How's this? I also removed some extra braces per Sergei. The code looks fine, but I need you to resend it without the above comments and without the Re: in the subject line. Otherwise the commit message looks like this when I use git-am to import it: $ git show HEAD commit b5bc451d5aaf213711f7367f2ffa997b7dc2c608 Author: Paul Zimmerman <Paul.Zimmerman@xxxxxxxxxxxx> Date: Wed Nov 17 13:57:17 2010 -0800 RE: [RFC] xhci: Fix for reset-device and configure-endpoint commands > From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] [snip] > Please fix, commit, and run `git show HEAD | scripts/checkpatch.pl -`. > Or if you want git to run checkpatch automatically before you commit, ... So please resend with a correct subject line and the message you actually want committed in the body. In the future, if you want to comment on the patch's description, you can do so after the "---" line. Git will ignore any comments after that line, before the diff stat, and they won't get put in the git commit message. Or if you want to top-post above the patch description, you can use the scissors line just before the description starts, like so: 8<------------------------------------------------------------------>8 Later versions of git will ignore everything above that line. > 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> > --- > 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
