public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Rajesh Bhagat <rajesh.bhagat@nxp.com>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Sriram Dash <sriram.dash@nxp.com>
Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout
Date: Wed, 23 Mar 2016 16:22:15 +0200	[thread overview]
Message-ID: <56F2A697.1000701@linux.intel.com> (raw)
In-Reply-To: <HE1PR0401MB20280AAAEC685FF3FC1DBF0EE3810@HE1PR0401MB2028.eurprd04.prod.outlook.com>

On 23.03.2016 05:53, Rajesh Bhagat wrote:

>>> IMO, The assumption that "xhci_abort_cmd_ring would always generate an
>>> event and handle_cmd_completion would be called" will not be always be true if HW
>> is in bad state.
>>>
>>> Please share your opinion.
>>>
>>
>> writing the CA (command abort) bit in CRCR (command ring control register)  will stop
>> the command ring, and CRR (command ring running) will be set to 0 by xHC.
>> xhci_abort_cmd_ring() polls this bit up to 5 seconds.
>> If it's not 0 then the driver considers the command abort as failed.
>>
>> The scenario you're thinking of is that xHC would still react to CA bit set, it would stop
>> the command ring and set CRR 0, but not send a command completion event.
>>
>> Have you tried adding some debug to handle_cmd_completion() and see if you receive
>> any event after command abortion?
>>
>
> Yes. We have added debug prints at first line of handle_cmd_completion, and we are not getting
> those prints. The last print messages that we get are as below from xhci_alloc_dev while resume
> operation:
>
> xhci-hcd xhci-hcd.0.auto: Command timeout
> xhci-hcd xhci-hcd.0.auto: Abort command ring
>
> May be somehow, USB controller is in bad state and not responding to the commands.
>
> Please suggest how XHCI driver can handle such situations.
>

Restart the command timeout timer when writing the command abort bit.
If we get theIf we get the abort event the timer is deleted.

Otherwise if the timout triggers a second time we end up calling
xhci_handle_command_timeout() with a stopped ring,
This will call xhci_handle_stopped_cmd_ring(), turn the aborted command to no-op, restart the
command ring, and finally when the no-op completes it should call the missing completion.

If command ring doesn't start then additional code could be added to xhci_handle_command_timeout()
that clears the command ring if it is called a second time (=current command is already in
abort state and command ring is stopped when entering xhci_handle_command_timeout)

There might be some details missing, I'm not able to test any of this, but try
something like this:

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 3e1d24c..576819e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -319,7 +319,10 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
                 xhci_halt(xhci);
                 return -ESHUTDOWN;
         }
-
+       /* writing the CMD_RING_ABORT bit should create a command completion
+        * event, add a command completion timeout for it as well
+        */
+       mod_timer(&xhci->cmd_timer, jiffies + XHCI_CMD_DEFAULT_TIMEOUT);
         return 0;
  }

-Mathias

  reply	other threads:[~2016-03-23 14:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-18  7:01 [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout Rajesh Bhagat
2016-03-18 11:20 ` Mathias Nyman
2016-03-21  4:18   ` Rajesh Bhagat
2016-03-21  9:16     ` Mathias Nyman
2016-03-22  5:19       ` Rajesh Bhagat
2016-03-22 12:05         ` Mathias Nyman
2016-03-23  3:53           ` Rajesh Bhagat
2016-03-23 14:22             ` Mathias Nyman [this message]
2016-03-28  6:13               ` Rajesh Bhagat
2016-03-29 17:21                 ` Mathias Nyman
2016-03-31  3:51                   ` Rajesh Bhagat
2016-03-31 14:36                     ` Mathias Nyman
2016-04-01  3:55                       ` Rajesh Bhagat
2016-04-01 10:49                         ` Mathias Nyman
2016-04-11  3:46                           ` Rajesh Bhagat
2016-03-18 14:21 ` Alan Stern
2016-03-21  4:22   ` Rajesh Bhagat
2016-03-21 15:02     ` Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56F2A697.1000701@linux.intel.com \
    --to=mathias.nyman@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=rajesh.bhagat@nxp.com \
    --cc=sriram.dash@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox