* [PATCH 0/5] xhci fixes for 3.16-rc usb-linus
@ 2014-06-24 13:54 Mathias Nyman
2014-06-24 14:14 ` [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command Mathias Nyman
0 siblings, 1 reply; 10+ messages in thread
From: Mathias Nyman @ 2014-06-24 13:54 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, linux-kernel, dan.j.williams, Mathias Nyman
Hi Greg
These xhci fixes are for usb-linus 3.16-rc.
Most of them are small fixes that also go to stable.
Julius "Correct last context entry" is the only one with a
bit more content.
-Mathias
Julius Werner (1):
usb: xhci: Correct last context entry calculation for Configure
Endpoint
Lu Baolu (1):
xhci: clear root port wake on bits if controller isn't wake-up capable
Mathias Nyman (2):
xhci: Use correct SLOT ID when handling a reset device command
xhci: correct burst count field for isoc transfers on 1.0 xhci hosts
Wang, Yu (1):
xhci: Fix runtime suspended xhci from blocking system suspend.
drivers/usb/host/xhci-hub.c | 5 +++-
drivers/usb/host/xhci-ring.c | 9 ++++---
drivers/usb/host/xhci.c | 61 ++++++++++++++++++--------------------------
3 files changed, 35 insertions(+), 40 deletions(-)
--
1.8.3.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command
2014-06-24 14:14 ` [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command Mathias Nyman
@ 2014-06-24 14:06 ` David Laight
2014-06-24 14:14 ` [PATCH 2/5] xhci: correct burst count field for isoc transfers on 1.0 xhci hosts Mathias Nyman
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: David Laight @ 2014-06-24 14:06 UTC (permalink / raw)
To: 'Mathias Nyman', gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
dan.j.williams@intel.com, stable@vger.kernel.org
From: Of Mathias Nyman
> Command completion events normally include command completion status,
> SLOT_ID, and a pointer to the original command. Reset device command
> completion SLOT_ID may be zero according to xhci specs 4.6.11.
>
> VIA controllers set the SLOT_ID to zero, triggering a WARN_ON in the
> command completion handler.
>
> Use the SLOT ID found from the original command instead.
>
> This patch should be applied to stable kernels since 3.13 that contain
> the commit 20e7acb13ff48fbc884d5918c3697c27de63922a
> "xhci: use completion event's slot id rather than dig it out of command"
>
> Cc: stable@vger.kernel.org # 3.13
> Reported-by: Saran Neti <sarannmr@gmail.com>
> Tested-by: Saran Neti <sarannmr@gmail.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
> drivers/usb/host/xhci-ring.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index d67ff71..71657d3 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1433,8 +1433,11 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
> xhci_handle_cmd_reset_ep(xhci, slot_id, cmd_trb, cmd_comp_code);
> break;
> case TRB_RESET_DEV:
> - WARN_ON(slot_id != TRB_TO_SLOT_ID(
> - le32_to_cpu(cmd_trb->generic.field[3])));
> + /* SLOT_ID field in reset device cmd completion event TRB is 0.
Minor nit...
Surely is would be better to say 'is undefined' or 'may be zero'.
David
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint
2014-06-24 14:14 ` [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint Mathias Nyman
@ 2014-06-24 14:10 ` Greg KH
2014-06-24 14:51 ` Mathias Nyman
0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2014-06-24 14:10 UTC (permalink / raw)
To: Mathias Nyman; +Cc: linux-usb, linux-kernel, dan.j.williams, Julius Werner
On Tue, Jun 24, 2014 at 05:14:42PM +0300, Mathias Nyman wrote:
> From: Julius Werner <jwerner@chromium.org>
>
> The current XHCI driver recalculates the Context Entries field in the
> Slot Context on every add_endpoint() and drop_endpoint() call. In the
> case of drop_endpoint(), it seems to assume that the add_flags will
> always contain every endpoint for the new configuration, which is not
> necessarily correct if you don't make assumptions about how the USB core
> uses the add_endpoint/drop_endpoint interface (add_flags only contains
> endpoints that are new additions in the new configuration).
>
> Furthermore, EP0_FLAG is not consistently set in add_flags throughout
> the lifetime of a device. This means that when all endpoints are
> dropped, the Context Entries field can be set to 0 (which is invalid and
> may cause a Parameter Error) or -1 (which is interpreted as 31 and
> causes the driver to keep using the old, incorrect value).
>
> The only surefire way to set this field right is to also take all
> existing endpoints into account, and to force the value to 1 (meaning
> only EP0 is active) if no other endpoint is found. This patch implements
> that as a single step in the final check_bandwidth() call and removes
> the intermediary calculations from add_endpoint() and drop_endpoint().
>
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
> drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
> 1 file changed, 18 insertions(+), 33 deletions(-)
Is this something for older (i.e. stable) kernels?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command
2014-06-24 13:54 [PATCH 0/5] xhci fixes for 3.16-rc usb-linus Mathias Nyman
@ 2014-06-24 14:14 ` Mathias Nyman
2014-06-24 14:06 ` David Laight
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Mathias Nyman @ 2014-06-24 14:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, linux-kernel, dan.j.williams, Mathias Nyman, stable
Command completion events normally include command completion status,
SLOT_ID, and a pointer to the original command. Reset device command
completion SLOT_ID may be zero according to xhci specs 4.6.11.
VIA controllers set the SLOT_ID to zero, triggering a WARN_ON in the
command completion handler.
Use the SLOT ID found from the original command instead.
This patch should be applied to stable kernels since 3.13 that contain
the commit 20e7acb13ff48fbc884d5918c3697c27de63922a
"xhci: use completion event's slot id rather than dig it out of command"
Cc: stable@vger.kernel.org # 3.13
Reported-by: Saran Neti <sarannmr@gmail.com>
Tested-by: Saran Neti <sarannmr@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d67ff71..71657d3 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1433,8 +1433,11 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_reset_ep(xhci, slot_id, cmd_trb, cmd_comp_code);
break;
case TRB_RESET_DEV:
- WARN_ON(slot_id != TRB_TO_SLOT_ID(
- le32_to_cpu(cmd_trb->generic.field[3])));
+ /* SLOT_ID field in reset device cmd completion event TRB is 0.
+ * Use the SLOT_ID from the command TRB instead (xhci 4.6.11)
+ */
+ slot_id = TRB_TO_SLOT_ID(
+ le32_to_cpu(cmd_trb->generic.field[3]));
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
case TRB_NEC_GET_FW:
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] xhci: correct burst count field for isoc transfers on 1.0 xhci hosts
2014-06-24 14:14 ` [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command Mathias Nyman
2014-06-24 14:06 ` David Laight
@ 2014-06-24 14:14 ` Mathias Nyman
2014-06-24 14:14 ` [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint Mathias Nyman
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2014-06-24 14:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, linux-kernel, dan.j.williams, Mathias Nyman, stable
The transfer burst count (TBC) field in xhci 1.0 hosts should be set
to the number of bursts needed to transfer all packets in a isoc TD.
Supported values are 0-2 (1 to 3 bursts per service interval).
Formula for TBC calculation is given in xhci spec section 4.11.2.3:
TBC = roundup( Transfer Descriptor Packet Count / Max Burst Size +1 ) - 1
This patch should be applied to stable kernels since 3.0 that contain
the commit 5cd43e33b9519143f06f507dd7cbee6b7a621885
"xhci 1.0: Set transfer burst count field."
Cc: stable@vger.kernel.org # 3.0
Suggested-by: ShiChun Ma <masc2008@qq.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 71657d3..749fc68 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3537,7 +3537,7 @@ static unsigned int xhci_get_burst_count(struct xhci_hcd *xhci,
return 0;
max_burst = urb->ep->ss_ep_comp.bMaxBurst;
- return roundup(total_packet_count, max_burst + 1) - 1;
+ return DIV_ROUND_UP(total_packet_count, max_burst + 1) - 1;
}
/*
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint
2014-06-24 14:14 ` [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command Mathias Nyman
2014-06-24 14:06 ` David Laight
2014-06-24 14:14 ` [PATCH 2/5] xhci: correct burst count field for isoc transfers on 1.0 xhci hosts Mathias Nyman
@ 2014-06-24 14:14 ` Mathias Nyman
2014-06-24 14:10 ` Greg KH
2014-06-24 14:14 ` [PATCH 4/5] xhci: clear root port wake on bits if controller isn't wake-up capable Mathias Nyman
2014-06-24 14:14 ` [PATCH 5/5] xhci: Fix runtime suspended xhci from blocking system suspend Mathias Nyman
4 siblings, 1 reply; 10+ messages in thread
From: Mathias Nyman @ 2014-06-24 14:14 UTC (permalink / raw)
To: gregkh
Cc: linux-usb, linux-kernel, dan.j.williams, Julius Werner,
Mathias Nyman
From: Julius Werner <jwerner@chromium.org>
The current XHCI driver recalculates the Context Entries field in the
Slot Context on every add_endpoint() and drop_endpoint() call. In the
case of drop_endpoint(), it seems to assume that the add_flags will
always contain every endpoint for the new configuration, which is not
necessarily correct if you don't make assumptions about how the USB core
uses the add_endpoint/drop_endpoint interface (add_flags only contains
endpoints that are new additions in the new configuration).
Furthermore, EP0_FLAG is not consistently set in add_flags throughout
the lifetime of a device. This means that when all endpoints are
dropped, the Context Entries field can be set to 0 (which is invalid and
may cause a Parameter Error) or -1 (which is interpreted as 31 and
causes the driver to keep using the old, incorrect value).
The only surefire way to set this field right is to also take all
existing endpoints into account, and to force the value to 1 (meaning
only EP0 is active) if no other endpoint is found. This patch implements
that as a single step in the final check_bandwidth() call and removes
the intermediary calculations from add_endpoint() and drop_endpoint().
Signed-off-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
1 file changed, 18 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2b8d9a2..013aabb 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1582,12 +1582,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
- struct xhci_slot_ctx *slot_ctx;
- unsigned int last_ctx;
unsigned int ep_index;
struct xhci_ep_ctx *ep_ctx;
u32 drop_flag;
- u32 new_add_flags, new_drop_flags, new_slot_info;
+ u32 new_add_flags, new_drop_flags;
int ret;
ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
@@ -1634,24 +1632,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
- last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
- slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
- /* Update the last valid endpoint context, if we deleted the last one */
- if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
- LAST_CTX(last_ctx)) {
- slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
- slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
- }
- new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
- xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
+ xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
- (unsigned int) new_add_flags,
- (unsigned int) new_slot_info);
+ (unsigned int) new_add_flags);
return 0;
}
@@ -1674,11 +1661,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
unsigned int ep_index;
- struct xhci_slot_ctx *slot_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
u32 added_ctxs;
- unsigned int last_ctx;
- u32 new_add_flags, new_drop_flags, new_slot_info;
+ u32 new_add_flags, new_drop_flags;
struct xhci_virt_device *virt_dev;
int ret = 0;
@@ -1693,7 +1678,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
return -ENODEV;
added_ctxs = xhci_get_endpoint_flag(&ep->desc);
- last_ctx = xhci_last_valid_endpoint(added_ctxs);
if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
/* FIXME when we have to issue an evaluate endpoint command to
* deal with ep0 max packet size changing once we get the
@@ -1759,24 +1743,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
*/
new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
- slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
- /* Update the last valid endpoint context, if we just added one past */
- if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
- LAST_CTX(last_ctx)) {
- slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
- slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
- }
- new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
/* Store the usb_device pointer for later use */
ep->hcpriv = udev;
- xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
+ xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
- (unsigned int) new_add_flags,
- (unsigned int) new_slot_info);
+ (unsigned int) new_add_flags);
return 0;
}
@@ -2746,8 +2720,19 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
ret = 0;
goto command_cleanup;
}
- xhci_dbg(xhci, "New Input Control Context:\n");
+ /* Fix up Context Entries field. Minimum value is EP0 == BIT(1). */
slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
+ for (i = 31; i >= 1; i--) {
+ __le32 le32 = cpu_to_le32(BIT(i));
+
+ if ((virt_dev->eps[i-1].ring && !(ctrl_ctx->drop_flags & le32))
+ || (ctrl_ctx->add_flags & le32) || i == 1) {
+ slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
+ slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(i));
+ break;
+ }
+ }
+ xhci_dbg(xhci, "New Input Control Context:\n");
xhci_dbg_ctx(xhci, virt_dev->in_ctx,
LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx->dev_info)));
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] xhci: clear root port wake on bits if controller isn't wake-up capable
2014-06-24 14:14 ` [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command Mathias Nyman
` (2 preceding siblings ...)
2014-06-24 14:14 ` [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint Mathias Nyman
@ 2014-06-24 14:14 ` Mathias Nyman
2014-06-24 14:14 ` [PATCH 5/5] xhci: Fix runtime suspended xhci from blocking system suspend Mathias Nyman
4 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2014-06-24 14:14 UTC (permalink / raw)
To: gregkh
Cc: linux-usb, linux-kernel, dan.j.williams, Lu Baolu, stable,
Mathias Nyman
From: Lu Baolu <baolu.lu@linux.intel.com>
When xHCI PCI host is suspended, if do_wakeup is false in xhci_pci_suspend,
xhci_bus_suspend needs to clear all root port wake on bits. Otherwise some Intel
platforms may get a spurious wakeup, even if PCI PME# is disabled.
This patch should be back-ported to kernels as old as 2.6.37, that
contains the commit 9777e3ce907d4cb5a513902a87ecd03b52499569
"USB: xHCI: bus power management implementation".
Cc: stable@vger.kernel.org # 2.6.37
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-hub.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 2b998c6..aa79e87 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -22,6 +22,7 @@
#include <linux/slab.h>
+#include <linux/device.h>
#include <asm/unaligned.h>
#include "xhci.h"
@@ -1139,7 +1140,9 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
* including the USB 3.0 roothub, but only if CONFIG_PM_RUNTIME
* is enabled, so also enable remote wake here.
*/
- if (hcd->self.root_hub->do_remote_wakeup) {
+ if (hcd->self.root_hub->do_remote_wakeup
+ && device_may_wakeup(hcd->self.controller)) {
+
if (t1 & PORT_CONNECT) {
t2 |= PORT_WKOC_E | PORT_WKDISC_E;
t2 &= ~PORT_WKCONN_E;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] xhci: Fix runtime suspended xhci from blocking system suspend.
2014-06-24 14:14 ` [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command Mathias Nyman
` (3 preceding siblings ...)
2014-06-24 14:14 ` [PATCH 4/5] xhci: clear root port wake on bits if controller isn't wake-up capable Mathias Nyman
@ 2014-06-24 14:14 ` Mathias Nyman
4 siblings, 0 replies; 10+ messages in thread
From: Mathias Nyman @ 2014-06-24 14:14 UTC (permalink / raw)
To: gregkh
Cc: linux-usb, linux-kernel, dan.j.williams, Wang, Yu, stable,
Mathias Nyman
From: "Wang, Yu" <yu.y.wang@intel.com>
The system suspend flow as following:
1, Freeze all user processes and kenrel threads.
2, Try to suspend all devices.
2.1, If pci device is in RPM suspended state, then pci driver will try
to resume it to RPM active state in the prepare stage.
2.2, xhci_resume function calls usb_hcd_resume_root_hub to queue two
workqueue items to resume usb2&usb3 roothub devices.
2.3, Call suspend callbacks of devices.
2.3.1, All suspend callbacks of all hcd's children, including
roothub devices are called.
2.3.2, Finally, hcd_pci_suspend callback is called.
Due to workqueue threads were already frozen in step 1, the workqueue
items can't be scheduled, and the roothub devices can't be resumed in
this flow. The HCD_FLAG_WAKEUP_PENDING flag which is set in
usb_hcd_resume_root_hub won't be cleared. Finally,
hcd_pci_suspend will return -EBUSY, and system suspend fails.
The reason why this issue doesn't show up very often is due to that
choose_wakeup will be called in step 2.3.1. In step 2.3.1, if
udev->do_remote_wakeup is not equal to device_may_wakeup(&udev->dev), then
udev will resume to RPM active for changing the wakeup settings. This
has been a lucky hit which hides this issue.
For some special xHCI controllers which have no USB2 port, then roothub
will not match hub driver due to probe failed. Then its
do_remote_wakeup will be set to zero, and we won't be as lucky.
xhci driver doesn't need to resume roothub devices everytime like in
the above case. It's only needed when there are pending event TRBs.
This patch should be back-ported to kernels as old as 3.2, that
contains the commit f69e3120df82391a0ee8118e0a156239a06b2afb
"USB: XHCI: resume root hubs when the controller resumes"
Cc: stable@vger.kernel.org # 3.2
Signed-off-by: Wang, Yu <yu.y.wang@intel.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
[use readl() instead of removed xhci_readl(), reword commit message -Mathias]
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 013aabb..f07be65 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -936,7 +936,7 @@ int xhci_suspend(struct xhci_hcd *xhci)
*/
int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
{
- u32 command, temp = 0;
+ u32 command, temp = 0, status;
struct usb_hcd *hcd = xhci_to_hcd(xhci);
struct usb_hcd *secondary_hcd;
int retval = 0;
@@ -1054,8 +1054,12 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
done:
if (retval == 0) {
- usb_hcd_resume_root_hub(hcd);
- usb_hcd_resume_root_hub(xhci->shared_hcd);
+ /* Resume root hubs only when have pending events. */
+ status = readl(&xhci->op_regs->status);
+ if (status & STS_EINT) {
+ usb_hcd_resume_root_hub(hcd);
+ usb_hcd_resume_root_hub(xhci->shared_hcd);
+ }
}
/*
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint
2014-06-24 14:51 ` Mathias Nyman
@ 2014-06-24 14:47 ` Greg KH
0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2014-06-24 14:47 UTC (permalink / raw)
To: Mathias Nyman; +Cc: linux-usb, linux-kernel, dan.j.williams, Julius Werner
On Tue, Jun 24, 2014 at 05:51:16PM +0300, Mathias Nyman wrote:
> On 06/24/2014 05:10 PM, Greg KH wrote:
> > On Tue, Jun 24, 2014 at 05:14:42PM +0300, Mathias Nyman wrote:
> >> From: Julius Werner <jwerner@chromium.org>
> >>
> >> The current XHCI driver recalculates the Context Entries field in the
> >> Slot Context on every add_endpoint() and drop_endpoint() call. In the
> >> case of drop_endpoint(), it seems to assume that the add_flags will
> >> always contain every endpoint for the new configuration, which is not
> >> necessarily correct if you don't make assumptions about how the USB core
> >> uses the add_endpoint/drop_endpoint interface (add_flags only contains
> >> endpoints that are new additions in the new configuration).
> >>
> >> Furthermore, EP0_FLAG is not consistently set in add_flags throughout
> >> the lifetime of a device. This means that when all endpoints are
> >> dropped, the Context Entries field can be set to 0 (which is invalid and
> >> may cause a Parameter Error) or -1 (which is interpreted as 31 and
> >> causes the driver to keep using the old, incorrect value).
> >>
> >> The only surefire way to set this field right is to also take all
> >> existing endpoints into account, and to force the value to 1 (meaning
> >> only EP0 is active) if no other endpoint is found. This patch implements
> >> that as a single step in the final check_bandwidth() call and removes
> >> the intermediary calculations from add_endpoint() and drop_endpoint().
> >>
> >> Signed-off-by: Julius Werner <jwerner@chromium.org>
> >> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> >> ---
> >> drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
> >> 1 file changed, 18 insertions(+), 33 deletions(-)
> >
> > Is this something for older (i.e. stable) kernels?
> >
>
> This was intentionally left out of stable, reasoning in a previous mail was:
>
> "We discussed with Sarah that we should try this out and queue it for
> usb-linus. This clearly fixes the generation of a invalid slot context
> if all endpoints are dropped.
>
> But because there hasn't been any reported issue about the other case
> this changes (always setting context entries to last valid ep in target
> configuration), and spec still has this tiny contradiction in this case,
> we should keep this out of stable. At least for now, until it gets some
> real world testing coverage."
>
> The mail:
> http://marc.info/?l=linux-usb&m=139879120000807&w=2
>
> Or if you prefer this patch could go to usb-next instead.
If it doesn't fix a known issue, then it shouldn't go to 3.16-final at
this point in time, as I can't justify it being added. So I'll queue it
up for usb-next instead right now.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint
2014-06-24 14:10 ` Greg KH
@ 2014-06-24 14:51 ` Mathias Nyman
2014-06-24 14:47 ` Greg KH
0 siblings, 1 reply; 10+ messages in thread
From: Mathias Nyman @ 2014-06-24 14:51 UTC (permalink / raw)
To: Greg KH; +Cc: linux-usb, linux-kernel, dan.j.williams, Julius Werner
On 06/24/2014 05:10 PM, Greg KH wrote:
> On Tue, Jun 24, 2014 at 05:14:42PM +0300, Mathias Nyman wrote:
>> From: Julius Werner <jwerner@chromium.org>
>>
>> The current XHCI driver recalculates the Context Entries field in the
>> Slot Context on every add_endpoint() and drop_endpoint() call. In the
>> case of drop_endpoint(), it seems to assume that the add_flags will
>> always contain every endpoint for the new configuration, which is not
>> necessarily correct if you don't make assumptions about how the USB core
>> uses the add_endpoint/drop_endpoint interface (add_flags only contains
>> endpoints that are new additions in the new configuration).
>>
>> Furthermore, EP0_FLAG is not consistently set in add_flags throughout
>> the lifetime of a device. This means that when all endpoints are
>> dropped, the Context Entries field can be set to 0 (which is invalid and
>> may cause a Parameter Error) or -1 (which is interpreted as 31 and
>> causes the driver to keep using the old, incorrect value).
>>
>> The only surefire way to set this field right is to also take all
>> existing endpoints into account, and to force the value to 1 (meaning
>> only EP0 is active) if no other endpoint is found. This patch implements
>> that as a single step in the final check_bandwidth() call and removes
>> the intermediary calculations from add_endpoint() and drop_endpoint().
>>
>> Signed-off-by: Julius Werner <jwerner@chromium.org>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>> drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
>> 1 file changed, 18 insertions(+), 33 deletions(-)
>
> Is this something for older (i.e. stable) kernels?
>
This was intentionally left out of stable, reasoning in a previous mail was:
"We discussed with Sarah that we should try this out and queue it for
usb-linus. This clearly fixes the generation of a invalid slot context
if all endpoints are dropped.
But because there hasn't been any reported issue about the other case
this changes (always setting context entries to last valid ep in target
configuration), and spec still has this tiny contradiction in this case,
we should keep this out of stable. At least for now, until it gets some
real world testing coverage."
The mail:
http://marc.info/?l=linux-usb&m=139879120000807&w=2
Or if you prefer this patch could go to usb-next instead.
-Mathias
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-06-24 14:47 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-24 13:54 [PATCH 0/5] xhci fixes for 3.16-rc usb-linus Mathias Nyman
2014-06-24 14:14 ` [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command Mathias Nyman
2014-06-24 14:06 ` David Laight
2014-06-24 14:14 ` [PATCH 2/5] xhci: correct burst count field for isoc transfers on 1.0 xhci hosts Mathias Nyman
2014-06-24 14:14 ` [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint Mathias Nyman
2014-06-24 14:10 ` Greg KH
2014-06-24 14:51 ` Mathias Nyman
2014-06-24 14:47 ` Greg KH
2014-06-24 14:14 ` [PATCH 4/5] xhci: clear root port wake on bits if controller isn't wake-up capable Mathias Nyman
2014-06-24 14:14 ` [PATCH 5/5] xhci: Fix runtime suspended xhci from blocking system suspend Mathias Nyman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox