From: Mathias Nyman mathias.nyman@linux.intel.com
commit 229bc19fd7aca4f37964af06e3583c1c8f36b5d6 upstream.
Don't rely on event interrupt (EINT) bit alone to detect pending port change in resume. If no change event is detected the host may be suspended again, oterwise roothubs are resumed.
There is a lag in xHC setting EINT. If we don't notice the pending change in resume, and the controller is runtime suspeded again, it causes the event handler to assume host is dead as it will fail to read xHC registers once PCI puts the controller to D3 state.
[ 268.520969] xhci_hcd: xhci_resume: starting port polling. [ 268.520985] xhci_hcd: xhci_hub_status_data: stopping port polling. [ 268.521030] xhci_hcd: xhci_suspend: stopping port polling. [ 268.521040] xhci_hcd: // Setting command ring address to 0x349bd001 [ 268.521139] xhci_hcd: Port Status Change Event for port 3 [ 268.521149] xhci_hcd: resume root hub [ 268.521163] xhci_hcd: port resume event for port 3 [ 268.521168] xhci_hcd: xHC is not running. [ 268.521174] xhci_hcd: handle_port_status: starting port polling. [ 268.596322] xhci_hcd: xhci_hc_died: xHCI host controller not responding, assume dead
The EINT lag is described in a additional note in xhci specs 4.19.2:
"Due to internal xHC scheduling and system delays, there will be a lag between a change bit being set and the Port Status Change Event that it generated being written to the Event Ring. If SW reads the PORTSC and sees a change bit set, there is no guarantee that the corresponding Port Status Change Event has already been written into the Event Ring."
Cc: stable@vger.kernel.org Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Kai-Heng Feng kai.heng.feng@canonical.com --- drivers/usb/host/xhci.c | 40 +++++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.h | 4 ++++ 2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index e5bccc6d49cf..fe84b36627ec 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -856,6 +856,41 @@ static void xhci_disable_port_wake_on_bits(struct xhci_hcd *xhci) spin_unlock_irqrestore(&xhci->lock, flags); }
+static bool xhci_pending_portevent(struct xhci_hcd *xhci) +{ + __le32 __iomem **port_array; + int port_index; + u32 status; + u32 portsc; + + status = readl(&xhci->op_regs->status); + if (status & STS_EINT) + return true; + /* + * Checking STS_EINT is not enough as there is a lag between a change + * bit being set and the Port Status Change Event that it generated + * being written to the Event Ring. See note in xhci 1.1 section 4.19.2. + */ + + port_index = xhci->num_usb2_ports; + port_array = xhci->usb2_ports; + while (port_index--) { + portsc = readl(port_array[port_index]); + if (portsc & PORT_CHANGE_MASK || + (portsc & PORT_PLS_MASK) == XDEV_RESUME) + return true; + } + port_index = xhci->num_usb3_ports; + port_array = xhci->usb3_ports; + while (port_index--) { + portsc = readl(port_array[port_index]); + if (portsc & PORT_CHANGE_MASK || + (portsc & PORT_PLS_MASK) == XDEV_RESUME) + return true; + } + return false; +} + /* * Stop HC (not bus-specific) * @@ -955,7 +990,7 @@ EXPORT_SYMBOL_GPL(xhci_suspend); */ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) { - u32 command, temp = 0, status; + u32 command, temp = 0; struct usb_hcd *hcd = xhci_to_hcd(xhci); struct usb_hcd *secondary_hcd; int retval = 0; @@ -1077,8 +1112,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) done: if (retval == 0) { /* Resume root hubs only when have pending events. */ - status = readl(&xhci->op_regs->status); - if (status & STS_EINT) { + if (xhci_pending_portevent(xhci)) { usb_hcd_resume_root_hub(xhci->shared_hcd); usb_hcd_resume_root_hub(hcd); } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 2a72060dda1b..11232e62b898 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -392,6 +392,10 @@ struct xhci_op_regs { #define PORT_PLC (1 << 22) /* port configure error change - port failed to configure its link partner */ #define PORT_CEC (1 << 23) +#define PORT_CHANGE_MASK (PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | \ + PORT_RC | PORT_PLC | PORT_CEC) + + /* Cold Attach Status - xHC can set this bit to report device attached during * Sx state. Warm port reset should be perfomed to clear this bit and move port * to connected state.
On Thu, Jul 19, 2018 at 12:23:22AM +0800, Kai-Heng Feng wrote:
From: Mathias Nyman mathias.nyman@linux.intel.com
commit 229bc19fd7aca4f37964af06e3583c1c8f36b5d6 upstream.
Don't rely on event interrupt (EINT) bit alone to detect pending port change in resume. If no change event is detected the host may be suspended again, oterwise roothubs are resumed.
There is a lag in xHC setting EINT. If we don't notice the pending change in resume, and the controller is runtime suspeded again, it causes the event handler to assume host is dead as it will fail to read xHC registers once PCI puts the controller to D3 state.
[ 268.520969] xhci_hcd: xhci_resume: starting port polling. [ 268.520985] xhci_hcd: xhci_hub_status_data: stopping port polling. [ 268.521030] xhci_hcd: xhci_suspend: stopping port polling. [ 268.521040] xhci_hcd: // Setting command ring address to 0x349bd001 [ 268.521139] xhci_hcd: Port Status Change Event for port 3 [ 268.521149] xhci_hcd: resume root hub [ 268.521163] xhci_hcd: port resume event for port 3 [ 268.521168] xhci_hcd: xHC is not running. [ 268.521174] xhci_hcd: handle_port_status: starting port polling. [ 268.596322] xhci_hcd: xhci_hc_died: xHCI host controller not responding, assume dead
The EINT lag is described in a additional note in xhci specs 4.19.2:
"Due to internal xHC scheduling and system delays, there will be a lag between a change bit being set and the Port Status Change Event that it generated being written to the Event Ring. If SW reads the PORTSC and sees a change bit set, there is no guarantee that the corresponding Port Status Change Event has already been written into the Event Ring."
Cc: stable@vger.kernel.org Signed-off-by: Mathias Nyman mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Kai-Heng Feng kai.heng.feng@canonical.com
drivers/usb/host/xhci.c | 40 +++++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.h | 4 ++++ 2 files changed, 41 insertions(+), 3 deletions(-)
Any specific reason you sent this old patch that is already in Linus's tree out again to everyone?
confused,
greg k-h
Hi Greg,
On 2018Jul19, at 00:26, Greg KH gregkh@linuxfoundation.org wrote:
Any specific reason you sent this old patch that is already in Linus's tree out again to everyone?
The original patch can be cleanly cherry-picked but failed to compile to older stable kernels due to some XHCI structure change.
Kai-Heng
confused,
greg k-h
On Thu, Jul 19, 2018 at 12:29:30AM +0800, Kai-Heng Feng wrote:
Hi Greg,
On 2018Jul19, at 00:26, Greg KH gregkh@linuxfoundation.org wrote:
Any specific reason you sent this old patch that is already in Linus's tree out again to everyone?
The original patch can be cleanly cherry-picked but failed to compile to older stable kernels due to some XHCI structure change.
Ok, so what am I supposed to do with it? Please always give a hint as to why you are sending a patch below the --- line. In this case, you want it applied to some stable tree, yet you didn't say which one, right?
greg k-h
On 2018Jul19, at 00:34, Greg KH gregkh@linuxfoundation.org wrote:
On Thu, Jul 19, 2018 at 12:29:30AM +0800, Kai-Heng Feng wrote:
Hi Greg,
On 2018Jul19, at 00:26, Greg KH gregkh@linuxfoundation.org wrote:
Any specific reason you sent this old patch that is already in Linus's tree out again to everyone?
The original patch can be cleanly cherry-picked but failed to compile to older stable kernels due to some XHCI structure change.
Ok, so what am I supposed to do with it? Please always give a hint as to why you are sending a patch below the --- line. In this case, you want it applied to some stable tree, yet you didn't say which one, right?
I’ll do that for my future backport for linux stable.
This is for 4.14.y stable tree. I’d like to have Mathias to ack my backport first.
Kai-Heng
greg k-h
On 18.07.2018 21:19, Kai-Heng Feng wrote:
On 2018Jul19, at 00:34, Greg KH gregkh@linuxfoundation.org wrote:
On Thu, Jul 19, 2018 at 12:29:30AM +0800, Kai-Heng Feng wrote:
Hi Greg,
On 2018Jul19, at 00:26, Greg KH gregkh@linuxfoundation.org wrote:
Any specific reason you sent this old patch that is already in Linus's tree out again to everyone?
The original patch can be cleanly cherry-picked but failed to compile to older stable kernels due to some XHCI structure change.
Ok, so what am I supposed to do with it? Please always give a hint as to why you are sending a patch below the --- line. In this case, you want it applied to some stable tree, yet you didn't say which one, right?
I’ll do that for my future backport for linux stable.
This is for 4.14.y stable tree. I’d like to have Mathias to ack my backport first.
Backport looks good, thanks for writing it.
Acked-by: Mathias Nyman mathias.nyman@linux.intel.com
On 2018Jul19, at 17:11, Mathias Nyman mathias.nyman@intel.com wrote:
On 18.07.2018 21:19, Kai-Heng Feng wrote:
On 2018Jul19, at 00:34, Greg KH gregkh@linuxfoundation.org wrote:
On Thu, Jul 19, 2018 at 12:29:30AM +0800, Kai-Heng Feng wrote:
Hi Greg,
On 2018Jul19, at 00:26, Greg KH gregkh@linuxfoundation.org wrote:
Any specific reason you sent this old patch that is already in Linus's tree out again to everyone?
The original patch can be cleanly cherry-picked but failed to compile to older stable kernels due to some XHCI structure change.
Ok, so what am I supposed to do with it? Please always give a hint as to why you are sending a patch below the --- line. In this case, you want it applied to some stable tree, yet you didn't say which one, right?
I’ll do that for my future backport for linux stable. This is for 4.14.y stable tree. I’d like to have Mathias to ack my backport first.
Backport looks good, thanks for writing it.
Thanks for review.
Greg, please include this patch for v4.14.y stable, thanks!
Kai-Heng
Acked-by: Mathias Nyman mathias.nyman@linux.intel.com
Hi Mathias,
Please take a look if this is a valid backport for v4.14 stable, thanks!
Kai-Heng
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Kai-Heng Feng -
Mathias Nyman