[asus-nb-wmi] thermal device detected by asus-nb-wmi platform driver
Hi, I booted Ubuntu13.10 onto ASUS T100 Tablet and upgraded it with upstream kernel (3.14.0-rc7+). There is a thermal device under /sys/devices/platform/asus-nb-wmi. root@t100:~/linux# tree /sys/class/hwmon/ /sys/class/hwmon/ ├── hwmon0 - ../../devices/platform/coretemp.0/hwmon/hwmon0 └── hwmon1 - ../../devices/platform/asus-nb-wmi/hwmon/hwmon1 root@t100:/sys/devices/platform/asus-nb-wmi# ls cpufv driver hwmon input modalias power subsystem uevent root@t100:~/linux# sensors coretemp-isa- Adapter: ISA adapter Core 0: +38.0°C (high = +90.0°C, crit = +90.0°C) Core 1: +38.0°C (high = +90.0°C, crit = +90.0°C) Core 2: +39.0°C (high = +90.0°C, crit = +90.0°C) Core 3: +38.0°C (high = +90.0°C, crit = +90.0°C) asus-isa- Adapter: ISA adapter temp1: +6280.0°C I didn't get any information about thermal interface in Documentation/ABI/testing/sysfs-platform-asus-wmi. Is this a thermal sensor? If it's the fact, how can I change the environment so that this sensor gives me variable data? Thanks, baolu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] xhci: clear root port wake on bits if controller isn't wake-up capable
When the 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
platform may get a spurious wakeup, even if PCI PME# is disabled.
http://marc.info/?l=linux-usbm=138194006009255w=2
Signed-off-by: Lu Baolu baolu...@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 6231ce6..fb771bd 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.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/1] xhci: clear root port wake on bits if controller isn't wake-up capable
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.
Signed-off-by: Lu Baolu baolu...@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 6231ce6..fb771bd 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.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] USB: Add device quirk for ASUS T100 Base Station keyboard
This full-speed USB device generates spurious remote wakeup event
as soon as USB_DEVICE_REMOTE_WAKEUP feature is set. As the result,
Linux can't enter system suspend and S0ix power saving modes once
this keyboard is used.
This patch tries to introduce USB_QUIRK_IGNOR_REMOTE_WAKEUP quirk.
With this quirk set, wakeup capability will be ignored during
device configure.
This patch could be back-ported to kernels as old as 2.6.39.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hub.c | 6 --
drivers/usb/core/quirks.c | 4
include/linux/usb/quirks.h | 3 +++
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 8a4dcbc..5df1457 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1977,8 +1977,10 @@ void usb_set_device_state(struct usb_device *udev,
|| new_state == USB_STATE_SUSPENDED)
; /* No change to wakeup settings */
else if (new_state == USB_STATE_CONFIGURED)
- wakeup = udev-actconfig-desc.bmAttributes
- USB_CONFIG_ATT_WAKEUP;
+ wakeup = (udev-quirks
+ USB_QUIRK_IGNOR_REMOTE_WAKEUP) ? 0 :
+ udev-actconfig-desc.bmAttributes
+ USB_CONFIG_ATT_WAKEUP;
else
wakeup = 0;
}
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index bae636e..e7d1e3c 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -159,6 +159,10 @@ static const struct usb_device_id usb_quirk_list[] = {
/* USB3503 */
{ USB_DEVICE(0x0424, 0x3503), .driver_info = USB_QUIRK_RESET_RESUME },
+ /* ASUS Base Station(T100) */
+ { USB_DEVICE(0x0b05, 0x17e0), .driver_info =
+ USB_QUIRK_IGNOR_REMOTE_WAKEUP },
+
{ } /* terminating entry must be last */
};
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index 55a17b1..0f784c3 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -41,4 +41,7 @@
*/
#define USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL 0x0080
+/* device generates spurious wakeup, ignore remote wakeup capability */
+#define USB_QUIRK_IGNOR_REMOTE_WAKEUP 0x0100
+
#endif /* __LINUX_USB_QUIRKS_H */
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] USB: Add device quirk for ASUS T100 Base Station keyboard
On 8/29/2014 7:59 PM, Sergei Shtylyov wrote: Hello. On 8/29/2014 8:26 AM, Lu Baolu wrote: This full-speed USB device generates spurious remote wakeup event as soon as USB_DEVICE_REMOTE_WAKEUP feature is set. As the result, Linux can't enter system suspend and S0ix power saving modes once this keyboard is used. This patch tries to introduce USB_QUIRK_IGNOR_REMOTE_WAKEUP quirk. Why not IGNORE? This doesn't look like a big save... Sure, will change it. With this quirk set, wakeup capability will be ignored during device configure. This patch could be back-ported to kernels as old as 2.6.39. Signed-off-by: Lu Baolu baolu...@linux.intel.com WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/1] USB: Add device quirk for ASUS T100 Base Station keyboard
This full-speed USB device generates spurious remote wakeup event
as soon as USB_DEVICE_REMOTE_WAKEUP feature is set. As the result,
Linux can't enter system suspend and S0ix power saving modes once
this keyboard is used.
This patch tries to introduce USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk.
With this quirk set, wakeup capability will be ignored during
device configure.
This patch could be back-ported to kernels as old as 2.6.39.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hub.c | 6 --
drivers/usb/core/quirks.c | 4
include/linux/usb/quirks.h | 3 +++
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 8a4dcbc..5df1457 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1977,8 +1977,10 @@ void usb_set_device_state(struct usb_device *udev,
|| new_state == USB_STATE_SUSPENDED)
; /* No change to wakeup settings */
else if (new_state == USB_STATE_CONFIGURED)
- wakeup = udev-actconfig-desc.bmAttributes
- USB_CONFIG_ATT_WAKEUP;
+ wakeup = (udev-quirks
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP) ? 0 :
+ udev-actconfig-desc.bmAttributes
+ USB_CONFIG_ATT_WAKEUP;
else
wakeup = 0;
}
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index bae636e..e7d1e3c 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -159,6 +159,10 @@ static const struct usb_device_id usb_quirk_list[] = {
/* USB3503 */
{ USB_DEVICE(0x0424, 0x3503), .driver_info = USB_QUIRK_RESET_RESUME },
+ /* ASUS Base Station(T100) */
+ { USB_DEVICE(0x0b05, 0x17e0), .driver_info =
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
{ } /* terminating entry must be last */
};
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index 55a17b1..0f784c3 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -41,4 +41,7 @@
*/
#define USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL 0x0080
+/* device generates spurious wakeup, ignore remote wakeup capability */
+#define USB_QUIRK_IGNORE_REMOTE_WAKEUP 0x0100
+
#endif /* __LINUX_USB_QUIRKS_H */
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] Rework xhci: clear root port wake on bits if controller isn't wake-up capable
This serie of patch reworks commit ff8cbf250b448aac35589f6075082c3fcad8a8fe. This has been discussed at http://www.spinics.net/lists/linux-usb/msg114986.html It also includes a patch to fix a comment in drivers/usb/host/xhci.h. Lu Baolu (3): usb: xhci: Revert xhci: clear root port wake on bits if controller isn't wake-up capable usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe usb: xhci: fix comment for PORT_DEV_REMOVE drivers/usb/host/xhci-hub.c | 5 + drivers/usb/host/xhci-pci.c | 42 ++ drivers/usb/host/xhci.h | 8 +++- 3 files changed, 50 insertions(+), 5 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] usb: xhci: fix comment for PORT_DEV_REMOVE
According to xHCI specification, PORT_DEV_REMOVE(bit 30) in PORTSC
true means Device is non-removable.
Reported-by: Juro Bystricky jurobystri...@hotmail.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2f6131b..4e160db 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -358,7 +358,7 @@ struct xhci_op_regs {
/* wake on over-current (enable) */
#define PORT_WKOC_E(1 27)
/* bits 28:29 reserved */
-/* true: device is removable - for USB 3.0 roothub emulation */
+/* true: device is non-removable - for USB 3.0 roothub emulation */
#define PORT_DEV_REMOVE(1 30)
/* Initiate a warm port reset - complete when PORT_WRC is '1' */
#define PORT_WR(1 31)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe
xhci: clear root port wake on bits if controller isn't wake-up capable
When xHCI PCI host is suspended, if do_wakeup is false in xhci_pci_suspend,
xhci_pci_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.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-pci.c | 42 ++
drivers/usb/host/xhci.h | 6 ++
2 files changed, 48 insertions(+)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 280dde9..3e7441a 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -27,6 +27,8 @@
#include xhci.h
#include xhci-trace.h
+#definePORT_WAKE_BITS (PORT_WKOC_E | PORT_WKDISC_E | PORT_WKCONN_E)
+
/* Device for a quirk */
#define PCI_VENDOR_ID_FRESCO_LOGIC 0x1b73
#define PCI_DEVICE_ID_FRESCO_LOGIC_PDK 0x1000
@@ -126,6 +128,7 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
*/
xhci-quirks |= XHCI_SPURIOUS_REBOOT;
xhci-quirks |= XHCI_AVOID_BEI;
+ xhci-quirks |= XHCI_DISABLE_PORT_WOB;
}
if (pdev-vendor == PCI_VENDOR_ID_INTEL
(pdev-device == PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI ||
@@ -279,6 +282,42 @@ static void xhci_pci_remove(struct pci_dev *dev)
}
#ifdef CONFIG_PM
+static void xhci_disable_port_wake_bits(struct xhci_hcd *xhci)
+{
+ int port_index;
+ __le32 __iomem **port_array;
+ unsigned long flags;
+ u32 t1, t2;
+
+ spin_lock_irqsave(xhci-lock, flags);
+
+ /* disble usb3 ports Wake bits*/
+ port_index = xhci-num_usb3_ports;
+ port_array = xhci-usb3_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ /* disble usb2 ports Wake bits*/
+ port_index = xhci-num_usb2_ports;
+ port_array = xhci-usb2_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ spin_unlock_irqrestore(xhci-lock, flags);
+}
+
static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
@@ -291,6 +330,9 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool
do_wakeup)
if (xhci-quirks XHCI_COMP_MODE_QUIRK)
pdev-no_d3cold = true;
+ if (xhci-quirks XHCI_DISABLE_PORT_WOB !do_wakeup)
+ xhci_disable_port_wake_bits(xhci);
+
return xhci_suspend(xhci);
}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index df76d64..2f6131b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1560,6 +1560,12 @@ struct xhci_hcd {
#define XHCI_SPURIOUS_WAKEUP (1 18)
/* For controllers with a broken beyond repair streams implementation */
#define XHCI_BROKEN_STREAMS(1 19)
+/*
+ * Some Intel platforms may get a spurious wakeup after entering system
+ * suspend with PCI PME# disabled. Software can workaround this by dis-
+ * abling the wake on bits of all root ports.
+ */
+#define XHCI_DISABLE_PORT_WOB (1 20)
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
/* There are two roothubs to keep track of bus suspend info for */
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] usb: xhci: Revert xhci: clear root port wake on bits if controller isn't wake-up capable
This reverts commit ff8cbf250b448aac35589f6075082c3fcad8a8fe.
Commit ff8cbf250b448aac35589f6075082c3fcad8a8fe triggers the bug logged at
https://bugzilla.kernel.org/show_bug.cgi?id=85701
Reported-by: Dmitry Nezhevenko d...@inhex.net
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 696160d..388cfd8 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -22,7 +22,6 @@
#include linux/slab.h
-#include linux/device.h
#include asm/unaligned.h
#include xhci.h
@@ -1149,9 +1148,7 @@ 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
-device_may_wakeup(hcd-self.controller)) {
-
+ if (hcd-self.root_hub-do_remote_wakeup) {
if (t1 PORT_CONNECT) {
t2 |= PORT_WKOC_E | PORT_WKDISC_E;
t2 = ~PORT_WKCONN_E;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] usb: xhci: fix comment for PORT_DEV_REMOVE
According to xHCI specification, PORT_DEV_REMOVE(bit 30) in PORTSC
true means Device is non-removable.
Reported-by: Juro Bystricky jurobystri...@hotmail.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index dace515..2682f18 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -358,7 +358,7 @@ struct xhci_op_regs {
/* wake on over-current (enable) */
#define PORT_WKOC_E(1 27)
/* bits 28:29 reserved */
-/* true: device is removable - for USB 3.0 roothub emulation */
+/* true: device is non-removable - for USB 3.0 roothub emulation */
#define PORT_DEV_REMOVE(1 30)
/* Initiate a warm port reset - complete when PORT_WRC is '1' */
#define PORT_WR(1 31)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 0/3] Rework xhci: clear root port wake on bits if controller isn't wake-up capable
Hi Mathias, This patch series has been acked by Alan Stern. There seems no further comments from others. Can you please pull in it? Thanks, -baolu On 2014年11月06日 10:50, Lu Baolu wrote: This serie of patch reworks commit ff8cbf250b448aac35589f6075082c3fcad8a8fe. This has been discussed at http://www.spinics.net/lists/linux-usb/msg114986.html It also includes a patch to fix a comment in drivers/usb/host/xhci.h. Changes in v4: - Refine xhci_disable_port_wake_on_bits(). - Add Acked-by: Alan Stern Changes in v3: - Need to consider run-time suspend as well. - Change wake-up capable to allowed to do wakeup in both comments and patch description. - Add Suggested-by: Alan Stern Changes in v2: - Should not be a quirk. - Should be applied to all xhci controllers. Lu Baolu (3): usb: xhci: Revert xhci: clear root port wake on bits if controller isn't wake-up capable usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe usb: xhci: fix comment for PORT_DEV_REMOVE drivers/usb/host/xhci-hub.c | 5 + drivers/usb/host/xhci-pci.c | 2 +- drivers/usb/host/xhci-plat.c | 10 +- drivers/usb/host/xhci.c | 42 +- drivers/usb/host/xhci.h | 4 ++-- 5 files changed, 54 insertions(+), 9 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/19] usb: dwc3: host: Pass the XHCI_DRD_SUPPORT and XHCI_NEEDS_LHC_RESET quirk
On 2014年11月25日 21:11, George Cherian wrote:
Pass the quir flag XHCI_DRD_SUPPORT from DWC3 host to xhci platform driver.
quir to quirk
Regards,
Baolu
This enables xhci driver to handle deallocation's differently while in DRD mode.
Pass the quirk flag XHCI_NEEDS_LHC_RESET from DWC3 host to xhci platform
driver. This enables to do LHRESET during xhci_reset().
Signed-off-by: George Cherian george.cher...@ti.com
---
drivers/usb/dwc3/host.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index dcb8ca0..257b5b5 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -53,6 +53,8 @@ int dwc3_host_init(struct dwc3 *dwc)
#ifdef CONFIG_DWC3_HOST_USB3_LPM_ENABLE
pdata.usb3_lpm_capable = 1;
#endif
+ pdata.usb_drd_support = 1;
+ pdata.usb_needs_lhc_reset = 1;
ret = platform_device_add_data(xhci, pdata, sizeof(pdata));
if (ret) {
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe
On 10/31/2014 10:28 PM, Alan Stern wrote:
On Fri, 31 Oct 2014, Lu Baolu wrote:
xhci: clear root port wake on bits if controller isn't wake-up capable
When xHCI PCI host is suspended, if do_wakeup is false in xhci_pci_suspend,
xhci_pci_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.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-pci.c | 42 ++
drivers/usb/host/xhci.h | 6 ++
2 files changed, 48 insertions(+)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 280dde9..3e7441a 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -27,6 +27,8 @@
#include xhci.h
#include xhci-trace.h
+#define PORT_WAKE_BITS (PORT_WKOC_E | PORT_WKDISC_E | PORT_WKCONN_E)
+
/* Device for a quirk */
#define PCI_VENDOR_ID_FRESCO_LOGIC0x1b73
#define PCI_DEVICE_ID_FRESCO_LOGIC_PDK0x1000
@@ -126,6 +128,7 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
*/
xhci-quirks |= XHCI_SPURIOUS_REBOOT;
xhci-quirks |= XHCI_AVOID_BEI;
+ xhci-quirks |= XHCI_DISABLE_PORT_WOB;
}
if (pdev-vendor == PCI_VENDOR_ID_INTEL
(pdev-device == PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI ||
@@ -279,6 +282,42 @@ static void xhci_pci_remove(struct pci_dev *dev)
}
#ifdef CONFIG_PM
+static void xhci_disable_port_wake_bits(struct xhci_hcd *xhci)
+{
+ int port_index;
+ __le32 __iomem **port_array;
+ unsigned long flags;
+ u32 t1, t2;
+
+ spin_lock_irqsave(xhci-lock, flags);
+
+ /* disble usb3 ports Wake bits*/
+ port_index = xhci-num_usb3_ports;
+ port_array = xhci-usb3_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ /* disble usb2 ports Wake bits*/
+ port_index = xhci-num_usb2_ports;
+ port_array = xhci-usb2_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ spin_unlock_irqrestore(xhci-lock, flags);
+}
+
static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
@@ -291,6 +330,9 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool
do_wakeup)
if (xhci-quirks XHCI_COMP_MODE_QUIRK)
pdev-no_d3cold = true;
+ if (xhci-quirks XHCI_DISABLE_PORT_WOB !do_wakeup)
+ xhci_disable_port_wake_bits(xhci);
+
return xhci_suspend(xhci);
}
There are two things wrong here. First, this should not be a quirk.
The Wake-On bits should be disabled whenever they aren't needed, on
every controller.
Second, this code (or something like it) belongs in xhci.c or
xhci-hub.c, not xhci-pci.c. This is because it should apply to all
xHCI controllers, not just the PCI-based ones.
Hi Alan,
I agree with you. I will resubmit my patches.
BR,
baolu
(In fact, instead of disabling the Wake-On bits when the controller is
suspended and do_wakeup is 0, it probably would be better to leave them
disabled normally and then _enable_ them if do_wakeup is 1.)
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/3] usb: xhci: fix comment for PORT_DEV_REMOVE
According to xHCI specification, PORT_DEV_REMOVE(bit 30) in PORTSC
true means Device is non-removable.
Reported-by: Juro Bystricky jurobystri...@hotmail.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index df76d64..66c168a9 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -358,7 +358,7 @@ struct xhci_op_regs {
/* wake on over-current (enable) */
#define PORT_WKOC_E(1 27)
/* bits 28:29 reserved */
-/* true: device is removable - for USB 3.0 roothub emulation */
+/* true: device is non-removable - for USB 3.0 roothub emulation */
#define PORT_DEV_REMOVE(1 30)
/* Initiate a warm port reset - complete when PORT_WRC is '1' */
#define PORT_WR(1 31)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/3] Rework xhci: clear root port wake on bits if controller isn't wake-up capable
This serie of patch reworks commit ff8cbf250b448aac35589f6075082c3fcad8a8fe. This has been discussed at http://www.spinics.net/lists/linux-usb/msg114986.html It also includes a patch to fix a comment in drivers/usb/host/xhci.h. Changes in v2: * Should not be a quirk. * Should be applied to all xhci controllers. Lu Baolu (3): usb: xhci: Revert xhci: clear root port wake on bits if controller isn't wake-up capable usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe usb: xhci: fix comment for PORT_DEV_REMOVE drivers/usb/host/xhci-hub.c | 5 + drivers/usb/host/xhci-pci.c | 42 ++ drivers/usb/host/xhci.h | 8 +++- 3 files changed, 50 insertions(+), 5 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/3] usb: xhci: Revert xhci: clear root port wake on bits if controller isn't wake-up capable
This reverts commit ff8cbf250b448aac35589f6075082c3fcad8a8fe.
Commit ff8cbf250b448aac35589f6075082c3fcad8a8fe triggers the bug logged at
https://bugzilla.kernel.org/show_bug.cgi?id=85701
Reported-by: Dmitry Nezhevenko d...@inhex.net
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 696160d..388cfd8 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -22,7 +22,6 @@
#include linux/slab.h
-#include linux/device.h
#include asm/unaligned.h
#include xhci.h
@@ -1149,9 +1148,7 @@ 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
-device_may_wakeup(hcd-self.controller)) {
-
+ if (hcd-self.root_hub-do_remote_wakeup) {
if (t1 PORT_CONNECT) {
t2 |= PORT_WKOC_E | PORT_WKDISC_E;
t2 = ~PORT_WKCONN_E;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/3] usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe
xhci: clear root port wake on bits if controller isn't wake-up capable
When system is being suspended, if host device is not wakeup capable,
xhci_suspend() needs to clear all root port wake on bits. Otherwise,
some platforms may generate spurious wakeup, even if PCI PME# is dis-
abled.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2a5d45b..cd57aae 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -35,6 +35,8 @@
#define DRIVER_AUTHOR Sarah Sharp
#define DRIVER_DESC 'eXtensible' Host Controller (xHC) Driver
+#definePORT_WAKE_BITS (PORT_WKOC_E | PORT_WKDISC_E | PORT_WKCONN_E)
+
/* Some 0.95 hardware can't handle the chain bit on a Link TRB being cleared */
static int link_quirk;
module_param(link_quirk, int, S_IRUGO | S_IWUSR);
@@ -851,6 +853,42 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
xhci_set_cmd_ring_deq(xhci);
}
+static void xhci_disable_port_wake_on_bits(struct xhci_hcd *xhci)
+{
+ int port_index;
+ __le32 __iomem **port_array;
+ unsigned long flags;
+ u32 t1, t2;
+
+ spin_lock_irqsave(xhci-lock, flags);
+
+ /* disble usb3 ports Wake bits*/
+ port_index = xhci-num_usb3_ports;
+ port_array = xhci-usb3_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ /* disble usb2 ports Wake bits*/
+ port_index = xhci-num_usb2_ports;
+ port_array = xhci-usb2_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ spin_unlock_irqrestore(xhci-lock, flags);
+}
+
/*
* Stop HC (not bus-specific)
*
@@ -868,6 +906,10 @@ int xhci_suspend(struct xhci_hcd *xhci)
xhci-shared_hcd-state != HC_STATE_SUSPENDED)
return -EINVAL;
+ /* Clear root port wake on bits if not wakeup capable. */
+ if (!device_may_wakeup(hcd-self.controller))
+ xhci_disable_port_wake_on_bits(xhci);
+
/* Don't poll the roothubs on bus suspend. */
xhci_dbg(xhci, %s: stopping port polling.\n, __func__);
clear_bit(HCD_FLAG_POLL_RH, hcd-flags);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/3] usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe
On 11/5/2014 12:58 AM, Alan Stern wrote:
On Tue, 4 Nov 2014, Lu Baolu wrote:
xhci: clear root port wake on bits if controller isn't wake-up capable
When system is being suspended, if host device is not wakeup capable,
xhci_suspend() needs to clear all root port wake on bits. Otherwise,
some platforms may generate spurious wakeup, even if PCI PME# is dis-
abled.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2a5d45b..cd57aae 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -35,6 +35,8 @@
#define DRIVER_AUTHOR Sarah Sharp
#define DRIVER_DESC 'eXtensible' Host Controller (xHC) Driver
+#define PORT_WAKE_BITS (PORT_WKOC_E | PORT_WKDISC_E | PORT_WKCONN_E)
+
/* Some 0.95 hardware can't handle the chain bit on a Link TRB being cleared
*/
static int link_quirk;
module_param(link_quirk, int, S_IRUGO | S_IWUSR);
@@ -851,6 +853,42 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
xhci_set_cmd_ring_deq(xhci);
}
+static void xhci_disable_port_wake_on_bits(struct xhci_hcd *xhci)
+{
+ int port_index;
+ __le32 __iomem **port_array;
+ unsigned long flags;
+ u32 t1, t2;
+
+ spin_lock_irqsave(xhci-lock, flags);
+
+ /* disble usb3 ports Wake bits*/
+ port_index = xhci-num_usb3_ports;
+ port_array = xhci-usb3_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ /* disble usb2 ports Wake bits*/
+ port_index = xhci-num_usb2_ports;
+ port_array = xhci-usb2_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ spin_unlock_irqrestore(xhci-lock, flags);
+}
+
/*
* Stop HC (not bus-specific)
*
@@ -868,6 +906,10 @@ int xhci_suspend(struct xhci_hcd *xhci)
xhci-shared_hcd-state != HC_STATE_SUSPENDED)
return -EINVAL;
+ /* Clear root port wake on bits if not wakeup capable. */
+ if (!device_may_wakeup(hcd-self.controller))
+ xhci_disable_port_wake_on_bits(xhci);
+
/* Don't poll the roothubs on bus suspend. */
xhci_dbg(xhci, %s: stopping port polling.\n, __func__);
clear_bit(HCD_FLAG_POLL_RH, hcd-flags);
This is better but still wrong. Remember, this same code gets called
for system suspend _and_ for runtime suspend. During runtime suspend,
wakeup is always supposed to be turned on, even if device_may_wakeup()
is false. That's because device_may_wakeup() refers only to system
suspend. What you need to test is the do_wakeup flag, which should be
passed into xhci_suspend() by xhci_pci_suspend() and
xhci_plat_suspend().
Yes, it should cover runtime suspend as well. Thanks for the comments. I
will resend the patch.
Another problem is in the patch description and the comments. If
device_may_wakeup() returns false, it doesn't mean the controller isn't
wakeup-capable -- it means the controller isn't _allowed_ to wake up
the system. Those are two different things.
Accept, I will change this in new patch version.
Finally, the code in xhci_disable_port_wake_on_bits() looks a little
peculiar -- I'm not sure about all those calls to
xhci_port_state_to_neutral(). But I'm not an expert on that; Mathias
will have to advise on it.
This part of code was written with reference to code in xhci-hub.c.
Comments of xhci_port_state_to_neutral():
/*
* Given a port state, this function returns a value that would result
in the
* port being in the same state, if the value was written to the port
status
* control register.
* Save Read Only (RO) bits and save read/write bits where
* writing a 0 clears the bit and writing a 1 sets the bit (RWS).
* For all other types (RW1S, RW1CS, RW, and RZ), writing a '0' has no
effect.
*/
This calculation is used to avoid side effect of changing other bit fields.
Thanks,
-baolu
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 0/3] Rework xhci: clear root port wake on bits if controller isn't wake-up capable
This serie of patch reworks commit ff8cbf250b448aac35589f6075082c3fcad8a8fe. This has been discussed at http://www.spinics.net/lists/linux-usb/msg114986.html It also includes a patch to fix a comment in drivers/usb/host/xhci.h. Changes in v3: * Need to consider run-time suspend as well. * Change wake-up capable to allowed to do wakeup in both comments and patch description. * Add Suggested-by: Alan Stern Changes in v2: * Should not be a quirk. * Should be applied to all xhci controllers. Lu Baolu (3): usb: xhci: Revert xhci: clear root port wake on bits if controller isn't wake-up capable usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe usb: xhci: fix comment for PORT_DEV_REMOVE drivers/usb/host/xhci-hub.c | 5 + drivers/usb/host/xhci-pci.c | 2 +- drivers/usb/host/xhci-plat.c | 10 +- drivers/usb/host/xhci.c | 44 +++- drivers/usb/host/xhci.h | 4 ++-- 5 files changed, 56 insertions(+), 9 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 3/3] usb: xhci: fix comment for PORT_DEV_REMOVE
According to xHCI specification, PORT_DEV_REMOVE(bit 30) in PORTSC
true means Device is non-removable.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Reported-by: Juro Bystricky jurobystri...@hotmail.com
---
drivers/usb/host/xhci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d745715..ab36bbb 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -358,7 +358,7 @@ struct xhci_op_regs {
/* wake on over-current (enable) */
#define PORT_WKOC_E(1 27)
/* bits 28:29 reserved */
-/* true: device is removable - for USB 3.0 roothub emulation */
+/* true: device is non-removable - for USB 3.0 roothub emulation */
#define PORT_DEV_REMOVE(1 30)
/* Initiate a warm port reset - complete when PORT_WRC is '1' */
#define PORT_WR(1 31)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 2/3] usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe
xhci: clear root port wake on bits if controller isn't allowed to do wakeup
When system is being suspended, if host device is not allowed to do wakeup,
xhci_suspend() needs to clear all root port wake on bits. Otherwise, some
platforms may generate spurious wakeup, even if PCI PME# is disabled.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Suggested-by: Alan Stern st...@rowland.harvard.edu
---
drivers/usb/host/xhci-pci.c | 2 +-
drivers/usb/host/xhci-plat.c | 10 +-
drivers/usb/host/xhci.c | 44 +++-
drivers/usb/host/xhci.h | 2 +-
4 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 280dde9..d3fa3c2 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -291,7 +291,7 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool
do_wakeup)
if (xhci-quirks XHCI_COMP_MODE_QUIRK)
pdev-no_d3cold = true;
- return xhci_suspend(xhci);
+ return xhci_suspend(xhci, do_wakeup);
}
static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3d78b0c..646300c 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -204,7 +204,15 @@ static int xhci_plat_suspend(struct device *dev)
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
- return xhci_suspend(xhci);
+ /*
+* xhci_suspend() needs `do_wakeup` to know whether host is allowed
+* to do wakeup during suspend. Since xhci_plat_suspend is currently
+* only designed for system suspend, device_may_wakeup() is enough
+* to dertermine whether host is allowed to do wakeup. Need to
+* reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
+* also applies to runtime suspend.
+*/
+ return xhci_suspend(xhci, device_may_wakeup(dev));
}
static int xhci_plat_resume(struct device *dev)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2a5d45b..70d607b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -35,6 +35,8 @@
#define DRIVER_AUTHOR Sarah Sharp
#define DRIVER_DESC 'eXtensible' Host Controller (xHC) Driver
+#definePORT_WAKE_BITS (PORT_WKOC_E | PORT_WKDISC_E | PORT_WKCONN_E)
+
/* Some 0.95 hardware can't handle the chain bit on a Link TRB being cleared */
static int link_quirk;
module_param(link_quirk, int, S_IRUGO | S_IWUSR);
@@ -851,13 +853,49 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
xhci_set_cmd_ring_deq(xhci);
}
+static void xhci_disable_port_wake_on_bits(struct xhci_hcd *xhci)
+{
+ int port_index;
+ __le32 __iomem **port_array;
+ unsigned long flags;
+ u32 t1, t2;
+
+ spin_lock_irqsave(xhci-lock, flags);
+
+ /* disble usb3 ports Wake bits*/
+ port_index = xhci-num_usb3_ports;
+ port_array = xhci-usb3_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ /* disble usb2 ports Wake bits*/
+ port_index = xhci-num_usb2_ports;
+ port_array = xhci-usb2_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ spin_unlock_irqrestore(xhci-lock, flags);
+}
+
/*
* Stop HC (not bus-specific)
*
* This is called when the machine transition into S3/S4 mode.
*
*/
-int xhci_suspend(struct xhci_hcd *xhci)
+int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
{
int rc = 0;
unsigned intdelay = XHCI_MAX_HALT_USEC;
@@ -868,6 +906,10 @@ int xhci_suspend(struct xhci_hcd *xhci)
xhci-shared_hcd-state != HC_STATE_SUSPENDED)
return -EINVAL;
+ /* Clear root port wake on bits if wakeup not allowed. */
+ if (!do_wakeup)
+ xhci_disable_port_wake_on_bits(xhci);
+
/* Don't poll the roothubs on bus suspend. */
xhci_dbg(xhci, %s: stopping port polling.\n, __func__);
clear_bit(HCD_FLAG_POLL_RH, hcd-flags);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index df76d64..d745715 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1746,7 +1746,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t
get_quirks);
void xhci_init_driver(struct hc_driver *drv, int
[PATCH v3 1/3] usb: xhci: Revert xhci: clear root port wake on bits if controller isn't wake-up capable
This reverts commit ff8cbf250b448aac35589f6075082c3fcad8a8fe.
Commit ff8cbf250b448aac35589f6075082c3fcad8a8fe triggers the bug logged at
https://bugzilla.kernel.org/show_bug.cgi?id=85701
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Reported-by: Dmitry Nezhevenko d...@inhex.net
---
drivers/usb/host/xhci-hub.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 696160d..388cfd8 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -22,7 +22,6 @@
#include linux/slab.h
-#include linux/device.h
#include asm/unaligned.h
#include xhci.h
@@ -1149,9 +1148,7 @@ 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
-device_may_wakeup(hcd-self.controller)) {
-
+ if (hcd-self.root_hub-do_remote_wakeup) {
if (t1 PORT_CONNECT) {
t2 |= PORT_WKOC_E | PORT_WKDISC_E;
t2 = ~PORT_WKCONN_E;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/3] usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe
On 11/6/2014 5:24 AM, Alan Stern wrote:
On Wed, 5 Nov 2014, Lu Baolu wrote:
xhci: clear root port wake on bits if controller isn't allowed to do wakeup
When system is being suspended, if host device is not allowed to do wakeup,
xhci_suspend() needs to clear all root port wake on bits. Otherwise, some
platforms may generate spurious wakeup, even if PCI PME# is disabled.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Suggested-by: Alan Stern st...@rowland.harvard.edu
This looks pretty good now.
+static void xhci_disable_port_wake_on_bits(struct xhci_hcd *xhci)
+{
+ int port_index;
+ __le32 __iomem **port_array;
+ unsigned long flags;
+ u32 t1, t2;
+
+ spin_lock_irqsave(xhci-lock, flags);
+
+ /* disble usb3 ports Wake bits*/
+ port_index = xhci-num_usb3_ports;
+ port_array = xhci-usb3_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t2 = xhci_port_state_to_neutral(t1);
+ t2 = ~PORT_WAKE_BITS;
+ t1 = xhci_port_state_to_neutral(t1);
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
Why not just do:
t1 = readl(port_array[port_index]);
t1 = xhci_port_state_to_neutral(t1);
t2 = t1 ~PORT_WAKE_BITS;
if (t1 != t2) ...
Right, this looks better. I will test and resend the patches with this
change.
Apart from that minor point,
Acked-by: Alan Stern st...@rowland.harvard.edu
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 2/3] usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe
xhci: clear root port wake on bits if controller isn't allowed to do wakeup
When system is being suspended, if host device is not allowed to do wakeup,
xhci_suspend() needs to clear all root port wake on bits. Otherwise, some
platforms may generate spurious wakeup, even if PCI PME# is disabled.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Suggested-by: Alan Stern st...@rowland.harvard.edu
Acked-by: Alan Stern st...@rowland.harvard.edu
---
drivers/usb/host/xhci-pci.c | 2 +-
drivers/usb/host/xhci-plat.c | 10 +-
drivers/usb/host/xhci.c | 42 +-
drivers/usb/host/xhci.h | 2 +-
4 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 280dde9..d3fa3c2 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -291,7 +291,7 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool
do_wakeup)
if (xhci-quirks XHCI_COMP_MODE_QUIRK)
pdev-no_d3cold = true;
- return xhci_suspend(xhci);
+ return xhci_suspend(xhci, do_wakeup);
}
static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3d78b0c..646300c 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -204,7 +204,15 @@ static int xhci_plat_suspend(struct device *dev)
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
- return xhci_suspend(xhci);
+ /*
+* xhci_suspend() needs `do_wakeup` to know whether host is allowed
+* to do wakeup during suspend. Since xhci_plat_suspend is currently
+* only designed for system suspend, device_may_wakeup() is enough
+* to dertermine whether host is allowed to do wakeup. Need to
+* reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
+* also applies to runtime suspend.
+*/
+ return xhci_suspend(xhci, device_may_wakeup(dev));
}
static int xhci_plat_resume(struct device *dev)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2a5d45b..b832841 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -35,6 +35,8 @@
#define DRIVER_AUTHOR Sarah Sharp
#define DRIVER_DESC 'eXtensible' Host Controller (xHC) Driver
+#definePORT_WAKE_BITS (PORT_WKOC_E | PORT_WKDISC_E | PORT_WKCONN_E)
+
/* Some 0.95 hardware can't handle the chain bit on a Link TRB being cleared */
static int link_quirk;
module_param(link_quirk, int, S_IRUGO | S_IWUSR);
@@ -851,13 +853,47 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
xhci_set_cmd_ring_deq(xhci);
}
+static void xhci_disable_port_wake_on_bits(struct xhci_hcd *xhci)
+{
+ int port_index;
+ __le32 __iomem **port_array;
+ unsigned long flags;
+ u32 t1, t2;
+
+ spin_lock_irqsave(xhci-lock, flags);
+
+ /* disble usb3 ports Wake bits*/
+ port_index = xhci-num_usb3_ports;
+ port_array = xhci-usb3_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t1 = xhci_port_state_to_neutral(t1);
+ t2 = t1 ~PORT_WAKE_BITS;
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ /* disble usb2 ports Wake bits*/
+ port_index = xhci-num_usb2_ports;
+ port_array = xhci-usb2_ports;
+ while (port_index--) {
+ t1 = readl(port_array[port_index]);
+ t1 = xhci_port_state_to_neutral(t1);
+ t2 = t1 ~PORT_WAKE_BITS;
+ if (t1 != t2)
+ writel(t2, port_array[port_index]);
+ }
+
+ spin_unlock_irqrestore(xhci-lock, flags);
+}
+
/*
* Stop HC (not bus-specific)
*
* This is called when the machine transition into S3/S4 mode.
*
*/
-int xhci_suspend(struct xhci_hcd *xhci)
+int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
{
int rc = 0;
unsigned intdelay = XHCI_MAX_HALT_USEC;
@@ -868,6 +904,10 @@ int xhci_suspend(struct xhci_hcd *xhci)
xhci-shared_hcd-state != HC_STATE_SUSPENDED)
return -EINVAL;
+ /* Clear root port wake on bits if wakeup not allowed. */
+ if (!do_wakeup)
+ xhci_disable_port_wake_on_bits(xhci);
+
/* Don't poll the roothubs on bus suspend. */
xhci_dbg(xhci, %s: stopping port polling.\n, __func__);
clear_bit(HCD_FLAG_POLL_RH, hcd-flags);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index df76d64..d745715 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1746,7 +1746,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t
get_quirks);
void xhci_init_driver(struct hc_driver *drv, int (*setup_fn)(struct usb_hcd
*));
#ifdef CONFIG_PM
-int
[PATCH v4 3/3] usb: xhci: fix comment for PORT_DEV_REMOVE
According to xHCI specification, PORT_DEV_REMOVE(bit 30) in PORTSC
true means Device is non-removable.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Reported-by: Juro Bystricky jurobystri...@hotmail.com
---
drivers/usb/host/xhci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d745715..ab36bbb 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -358,7 +358,7 @@ struct xhci_op_regs {
/* wake on over-current (enable) */
#define PORT_WKOC_E(1 27)
/* bits 28:29 reserved */
-/* true: device is removable - for USB 3.0 roothub emulation */
+/* true: device is non-removable - for USB 3.0 roothub emulation */
#define PORT_DEV_REMOVE(1 30)
/* Initiate a warm port reset - complete when PORT_WRC is '1' */
#define PORT_WR(1 31)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 0/3] Rework xhci: clear root port wake on bits if controller isn't wake-up capable
This serie of patch reworks commit ff8cbf250b448aac35589f6075082c3fcad8a8fe. This has been discussed at http://www.spinics.net/lists/linux-usb/msg114986.html It also includes a patch to fix a comment in drivers/usb/host/xhci.h. Changes in v4: - Refine xhci_disable_port_wake_on_bits(). - Add Acked-by: Alan Stern Changes in v3: - Need to consider run-time suspend as well. - Change wake-up capable to allowed to do wakeup in both comments and patch description. - Add Suggested-by: Alan Stern Changes in v2: - Should not be a quirk. - Should be applied to all xhci controllers. Lu Baolu (3): usb: xhci: Revert xhci: clear root port wake on bits if controller isn't wake-up capable usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe usb: xhci: fix comment for PORT_DEV_REMOVE drivers/usb/host/xhci-hub.c | 5 + drivers/usb/host/xhci-pci.c | 2 +- drivers/usb/host/xhci-plat.c | 10 +- drivers/usb/host/xhci.c | 42 +- drivers/usb/host/xhci.h | 4 ++-- 5 files changed, 54 insertions(+), 9 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 1/3] usb: xhci: Revert xhci: clear root port wake on bits if controller isn't wake-up capable
This reverts commit ff8cbf250b448aac35589f6075082c3fcad8a8fe.
Commit ff8cbf250b448aac35589f6075082c3fcad8a8fe triggers the bug logged at
https://bugzilla.kernel.org/show_bug.cgi?id=85701
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Reported-by: Dmitry Nezhevenko d...@inhex.net
---
drivers/usb/host/xhci-hub.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 696160d..388cfd8 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -22,7 +22,6 @@
#include linux/slab.h
-#include linux/device.h
#include asm/unaligned.h
#include xhci.h
@@ -1149,9 +1148,7 @@ 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
-device_may_wakeup(hcd-self.controller)) {
-
+ if (hcd-self.root_hub-do_remote_wakeup) {
if (t1 PORT_CONNECT) {
t2 |= PORT_WKOC_E | PORT_WKDISC_E;
t2 = ~PORT_WKCONN_E;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to Intel ValleyView and LynxPoint LP
On 03/12/2015 04:46 PM, Mathias Nyman wrote: On 12.03.2015 03:39, Lu Baolu wrote: When a device with an isochronous endpoint is plugged into the Intel xHCI host controller, and the driver submits multiple frames per URB, the xHCI driver will set the Block Event Interrupt (BEI) flag on all but the last TD for the URB. This causes the host controller to place an event on the event ring, but not send an interrupt. When the last TD for the URB completes, BEI is cleared, and we get an interrupt for the whole URB. However, under some Intel xHCI host controllers like ValleyView and Lynx Point LP, if the event ring is full of events from transfers with BEI set, a Event Ring is Full event will be posted to the last entry of the event ring, but no interrupt is generated. Host will cease all transfer and command executions and wait until software completes handling the pending events in event ring. That means xHC stops but event of event ring is full is not notified. As the result, the xHC looks like dead to user. The patch is to apply XHCI_AVOID_BEI to Intel VallyView and Lynx Point LP devices. And it should be backported to kernels as old as 3.0, that contains the commit 69e848c2090a (Intel xhci: Support EHCI/xHCI port switching.) Maybe we should set the quirk for all Intel xHCI controllers. So far the list includes Pantherpoint, Lynxpoint and valleyview. I bet it concerns most Intel platforms. Yes, I agree with you. I will send out a new patch for this. -Mathias -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to Intel ValleyView and LynxPoint LP
On 03/12/2015 03:54 PM, Greg Kroah-Hartman wrote: On Thu, Mar 12, 2015 at 09:39:06AM +0800, Lu Baolu wrote: When a device with an isochronous endpoint is plugged into the Intel xHCI host controller, and the driver submits multiple frames per URB, the xHCI driver will set the Block Event Interrupt (BEI) flag on all but the last TD for the URB. This causes the host controller to place an event on the event ring, but not send an interrupt. When the last TD for the URB completes, BEI is cleared, and we get an interrupt for the whole URB. However, under some Intel xHCI host controllers like ValleyView and Lynx Point LP, if the event ring is full of events from transfers with BEI set, a Event Ring is Full event will be posted to the last entry of the event ring, but no interrupt is generated. Host will cease all transfer and command executions and wait until software completes handling the pending events in event ring. That means xHC stops but event of event ring is full is not notified. As the result, the xHC looks like dead to user. The patch is to apply XHCI_AVOID_BEI to Intel VallyView and Lynx Point LP devices. And it should be backported to kernels as old as 3.0, that contains the commit 69e848c2090a (Intel xhci: Support EHCI/xHCI port switching.) Signed-off-by: Lu Baolu baolu...@linux.intel.com Cc: sta...@vger.kernel.org --- drivers/usb/host/xhci-pci.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index fd53c9e..5aa4893 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -40,6 +40,7 @@ #define PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI 0x22b5 #define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_XHCI 0xa12f #define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_XHCI 0x9d2f +#definePCI_DEVICE_ID_INTEL_VALLEYVIEW_XHCI 0x0f35 Minor nit, you added a tab where the rest of the file was using a space :( Yes, thanks for reminding. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers
When a device with an isochronous endpoint is plugged into the Intel
xHCI host controller, and the driver submits multiple frames per URB,
the xHCI driver will set the Block Event Interrupt (BEI) flag on all
but the last TD for the URB. This causes the host controller to place
an event on the event ring, but not send an interrupt. When the last
TD for the URB completes, BEI is cleared, and we get an interrupt for
the whole URB.
However, under Intel xHCI host controllers, if the event ring is full
of events from transfers with BEI set, an Event Ring is Full event
will be posted to the last entry of the event ring, but no interrupt
is generated. Host will cease all transfer and command executions and
wait until software completes handling the pending events in the event
ring. That means xHC stops, but event of event ring is full is not
notified. As the result, the xHC looks like dead to user.
This patch is to apply XHCI_AVOID_BEI quirk to Intel xHC devices. And
it should be backported to kernels as old as 3.0, that contains the
commit 69e848c2090a (Intel xhci: Support EHCI/xHCI port switching.).
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Cc: sta...@vger.kernel.org
---
drivers/usb/host/xhci-pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index fd53c9e..2af32e2 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -115,6 +115,7 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
if (pdev-vendor == PCI_VENDOR_ID_INTEL) {
xhci-quirks |= XHCI_LPM_SUPPORT;
xhci-quirks |= XHCI_INTEL_HOST;
+ xhci-quirks |= XHCI_AVOID_BEI;
}
if (pdev-vendor == PCI_VENDOR_ID_INTEL
pdev-device == PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI) {
@@ -130,7 +131,6 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
* PPT chipsets.
*/
xhci-quirks |= XHCI_SPURIOUS_REBOOT;
- xhci-quirks |= XHCI_AVOID_BEI;
}
if (pdev-vendor == PCI_VENDOR_ID_INTEL
pdev-device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] usb: xhci: handle Config Error Change (CEC) in xhci driver
Hi Alan,
Do you have any comments for this patch?
Thanks,
Baolu
On 03/06/2015 04:12 PM, Lu Baolu wrote:
Linux xHCI driver doesn't report and handle port cofig error change.
If Port Configure Error for root hub port occurs, CEC bit in PORTSC
would be set by xHC and remains 1. This happends when the root port
fails to configure its link partner, e.g. the port fails to exchange
port capabilities information using Port Capability LMPs.
Then the Port Status Change Events will be blocked until all status
change bits(CEC is one of the change bits) are cleared('0') (refer to
xHCI spec 4.19.2). Otherwise, the port status change event for this
root port will not be generated anymore, then root port would look
like dead for user and can't be recovered until a Host Controller
Reset(HCRST).
This patch is to check CEC bit in PORTSC in xhci_get_port_status()
and set a Config Error in the return status if CEC is set. This will
cause a ClearPortFeature request, where CEC bit is cleared in
xhci_clear_port_change_bit().
[Mathias Nyman contributed the idea. The commit log is based on patch
posted at http://marc.info/?l=linux-kernelm=142323612321434w=2]
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Cc: stable sta...@vger.kernel.org # v3.2+
---
drivers/usb/host/xhci-hub.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a7865c4..0827d7c 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -387,6 +387,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd
*xhci, u16 wValue,
status = PORT_PLC;
port_change_bit = link state;
break;
+ case USB_PORT_FEAT_C_PORT_CONFIG_ERROR:
+ status = PORT_CEC;
+ port_change_bit = config error;
+ break;
default:
/* Should never happen */
return;
@@ -588,6 +592,8 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
status |= USB_PORT_STAT_C_LINK_STATE 16;
if ((raw_port_status PORT_WRC))
status |= USB_PORT_STAT_C_BH_RESET 16;
+ if ((raw_port_status PORT_CEC))
+ status |= USB_PORT_STAT_C_CONFIG_ERROR 16;
}
if (hcd-speed != HCD_USB3) {
@@ -1005,6 +1011,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq,
u16 wValue,
case USB_PORT_FEAT_C_OVER_CURRENT:
case USB_PORT_FEAT_C_ENABLE:
case USB_PORT_FEAT_C_PORT_LINK_STATE:
+ case USB_PORT_FEAT_C_PORT_CONFIG_ERROR:
xhci_clear_port_change_bit(xhci, wValue, wIndex,
port_array[wIndex], temp);
break;
@@ -1069,7 +1076,7 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
*/
status = bus_state-resuming_ports;
- mask = PORT_CSC | PORT_PEC | PORT_OCC | PORT_PLC | PORT_WRC;
+ mask = PORT_CSC | PORT_PEC | PORT_OCC | PORT_PLC | PORT_WRC | PORT_CEC;
spin_lock_irqsave(xhci-lock, flags);
/* For each port, did anything change? If so, set that bit in buf. */
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD
On 03/11/2015 02:49 AM, Alan Stern wrote: On Tue, 10 Mar 2015, Mathias Nyman wrote: Mathias: Your patch description says this: The endpoint might already processesed some TRBs on the endpiont ring before we soft reset the endpoint. Make sure we set the dequeue pointer to where we were befere soft reset However, if a driver tries to issue an endpoint reset while there are still some URBs queued, it is a bug. Host controller drivers shouldn't have to worry about this -- xhci_endpoint_reset() should simply return an error if the endpoint ring isn't empty. I suppose we should check for this in the USB core. I'll write a patch and CC: you. Alan Stern It's possible that there's something in usb core as well, but I think the following was what happened: 1. First a normal configure endpoint command is issued, it sets endpoint dequeue pointer to xxx400 = start of ring segment 2. two urbs get queued - two TDs put on endpoint ring. 3. xhci executes those, ring is in running (idle) state. sw dequeue at xxx430, No TDs queued. Endpoint dequeue pointer is not written to the endpoint output context as the ring is still in running state (even if idle, not advancing with no TDs queued) it still shows xxx400 4. - something happends, xhci_endpoint_reset() is called, we do a new configure endpoint to 'soft reset' the endpiont, but we copy the dequeue pointer from the old endpoint output context to the configure endpoint input context, which re-initializes the old dequeue xxx400 pointer to xhci hardware, and it starts executing the old TDs from the ring. Is it possible to return an error message up to client driver? The client driver then decides how to handle this kind of error. It, possibly, unlink all ongoing transfers and ask host driver to soft reset this endpoint. When xhci_endpoint_reset is called, there should be no ongoing transfers. Thanks, Baolu Obviously that's bad. But don't you have to stop the endpoint ring in order to configure it? When you stop the ring, doesn't the controller store the correct current value of the dequeue pointer somewhere? 5. xhci driver notices that we get events for old TRBs that do not belong to the TD the driver thinks we should be handling Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers
When a device with an isochronous endpoint is plugged into the Intel
xHCI host controller, and the driver submits multiple frames per URB,
the xHCI driver will set the Block Event Interrupt (BEI) flag on all
but the last TD for the URB. This causes the host controller to place
an event on the event ring, but not send an interrupt. When the last
TD for the URB completes, BEI is cleared, and we get an interrupt for
the whole URB.
However, under Intel xHCI host controllers, if the event ring is full
of events from transfers with BEI set, an Event Ring is Full event
will be posted to the last entry of the event ring, but no interrupt
is generated. Host will cease all transfer and command executions and
wait until software completes handling the pending events in the event
ring. That means xHC stops, but event of event ring is full is not
notified. As the result, the xHC looks like dead to user.
This patch is to apply XHCI_AVOID_BEI quirk to Intel xHC devices. And
it should be backported to kernels as old as 3.0, that contains the
commit 69e848c2090a (Intel xhci: Support EHCI/xHCI port switching.).
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Tested-by: Alistair Grant akgrant0...@gmail.com
Tested-by: Devin Heitmueller dheitmuel...@kernellabs.com
Cc: sta...@vger.kernel.org
---
drivers/usb/host/xhci-pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Change log:
v2: apply to all Intel xHC devices as suggested by Mathias
v3: add two Tested-by's
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index fd53c9e..2af32e2 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -115,6 +115,7 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
if (pdev-vendor == PCI_VENDOR_ID_INTEL) {
xhci-quirks |= XHCI_LPM_SUPPORT;
xhci-quirks |= XHCI_INTEL_HOST;
+ xhci-quirks |= XHCI_AVOID_BEI;
}
if (pdev-vendor == PCI_VENDOR_ID_INTEL
pdev-device == PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI) {
@@ -130,7 +131,6 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
* PPT chipsets.
*/
xhci-quirks |= XHCI_SPURIOUS_REBOOT;
- xhci-quirks |= XHCI_AVOID_BEI;
}
if (pdev-vendor == PCI_VENDOR_ID_INTEL
pdev-device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to Intel ValleyView and LynxPoint LP
When a device with an isochronous endpoint is plugged into the Intel
xHCI host controller, and the driver submits multiple frames per URB,
the xHCI driver will set the Block Event Interrupt (BEI) flag on all
but the last TD for the URB. This causes the host controller to place
an event on the event ring, but not send an interrupt. When the last
TD for the URB completes, BEI is cleared, and we get an interrupt for
the whole URB.
However, under some Intel xHCI host controllers like ValleyView and
Lynx Point LP, if the event ring is full of events from transfers with
BEI set, a Event Ring is Full event will be posted to the last entry
of the event ring, but no interrupt is generated. Host will cease all
transfer and command executions and wait until software completes
handling the pending events in event ring. That means xHC stops but
event of event ring is full is not notified. As the result, the xHC
looks like dead to user.
The patch is to apply XHCI_AVOID_BEI to Intel VallyView and Lynx Point
LP devices. And it should be backported to kernels as old as 3.0, that
contains the commit 69e848c2090a (Intel xhci: Support EHCI/xHCI port
switching.)
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Cc: sta...@vger.kernel.org
---
drivers/usb/host/xhci-pci.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index fd53c9e..5aa4893 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -40,6 +40,7 @@
#define PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI0x22b5
#define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_XHCI0xa12f
#define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_XHCI 0x9d2f
+#definePCI_DEVICE_ID_INTEL_VALLEYVIEW_XHCI 0x0f35
static const char hcd_name[] = xhci_hcd;
@@ -135,6 +136,7 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
if (pdev-vendor == PCI_VENDOR_ID_INTEL
pdev-device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
xhci-quirks |= XHCI_SPURIOUS_REBOOT;
+ xhci-quirks |= XHCI_AVOID_BEI;
}
if (pdev-vendor == PCI_VENDOR_ID_INTEL
(pdev-device == PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_XHCI ||
@@ -142,6 +144,9 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
pdev-device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI)) {
xhci-quirks |= XHCI_PME_STUCK_QUIRK;
}
+ if (pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_VALLEYVIEW_XHCI)
+ xhci-quirks |= XHCI_AVOID_BEI;
if (pdev-vendor == PCI_VENDOR_ID_ETRON
pdev-device == PCI_DEVICE_ID_EJ168) {
xhci-quirks |= XHCI_RESET_ON_RESUME;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 01/12] usb: add bus type for USB ULPI
On 03/18/2015 08:40 PM, Heikki Krogerus wrote:
+
+/**
+ * ulpi_register_driver - unregister a driver with the ULPI bus
Hi Heikki,
ulpi_register_driver should be changed to ulpi_unregister_driver.
Thanks,
Baolu
+ * @drv: driver to unregister
+ *
+ * Unregisters a driver with the ULPI bus.
+ */
+void ulpi_unregister_driver(struct ulpi_driver *drv)
+{
+ driver_unregister(drv-driver);
+}
+EXPORT_SYMBOL_GPL(ulpi_unregister_driver);
+
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] usb: xhci: handle Config Error Change (CEC) in xhci driver
Linux xHCI driver doesn't report and handle port cofig error change.
If Port Configure Error for root hub port occurs, CEC bit in PORTSC
would be set by xHC and remains 1. This happends when the root port
fails to configure its link partner, e.g. the port fails to exchange
port capabilities information using Port Capability LMPs.
Then the Port Status Change Events will be blocked until all status
change bits(CEC is one of the change bits) are cleared('0') (refer to
xHCI spec 4.19.2). Otherwise, the port status change event for this
root port will not be generated anymore, then root port would look
like dead for user and can't be recovered until a Host Controller
Reset(HCRST).
This patch is to check CEC bit in PORTSC in xhci_get_port_status()
and set a Config Error in the return status if CEC is set. This will
cause a ClearPortFeature request, where CEC bit is cleared in
xhci_clear_port_change_bit().
[Mathias Nyman contributed the idea. The commit log is based on patch
posted at http://marc.info/?l=linux-kernelm=142323612321434w=2]
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Cc: stable sta...@vger.kernel.org # v3.2+
---
drivers/usb/host/xhci-hub.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a7865c4..0827d7c 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -387,6 +387,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd
*xhci, u16 wValue,
status = PORT_PLC;
port_change_bit = link state;
break;
+ case USB_PORT_FEAT_C_PORT_CONFIG_ERROR:
+ status = PORT_CEC;
+ port_change_bit = config error;
+ break;
default:
/* Should never happen */
return;
@@ -588,6 +592,8 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
status |= USB_PORT_STAT_C_LINK_STATE 16;
if ((raw_port_status PORT_WRC))
status |= USB_PORT_STAT_C_BH_RESET 16;
+ if ((raw_port_status PORT_CEC))
+ status |= USB_PORT_STAT_C_CONFIG_ERROR 16;
}
if (hcd-speed != HCD_USB3) {
@@ -1005,6 +1011,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq,
u16 wValue,
case USB_PORT_FEAT_C_OVER_CURRENT:
case USB_PORT_FEAT_C_ENABLE:
case USB_PORT_FEAT_C_PORT_LINK_STATE:
+ case USB_PORT_FEAT_C_PORT_CONFIG_ERROR:
xhci_clear_port_change_bit(xhci, wValue, wIndex,
port_array[wIndex], temp);
break;
@@ -1069,7 +1076,7 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
*/
status = bus_state-resuming_ports;
- mask = PORT_CSC | PORT_PEC | PORT_OCC | PORT_PLC | PORT_WRC;
+ mask = PORT_CSC | PORT_PEC | PORT_OCC | PORT_PLC | PORT_WRC | PORT_CEC;
spin_lock_irqsave(xhci-lock, flags);
/* For each port, did anything change? If so, set that bit in buf. */
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH 0/3] usb: add a hcd notify entry in hc_driver
This patch adds a new entry pointer in hc_driver. With this new entry, USB core can notify host driver when something happens and host driver is willing to be notified. One use case of this interface comes from xHCI compatible host controller driver. The xHCI spec is designed to allow an xHC implementation to cache the endpoint state. Caching endpoint state allows an xHC to reduce latency when handling ERDYs and other USB asynchronous events. However holding this state in xHC consumes resources and power. The xHCI spec designs some methods through which host controller driver can hint xHC about how to optimize its operation, e.g. to determine when it holds state internally or pushes it out to memory, when to power down logic, etc. When a USB device is going to suspend, states of all endpoints cached in the xHC should be pushed out to memory to save power and resources. Vice versa, when a USB device resumes, those states should be brought back to the cache. USB core suspends or resumes a USB device by sending set/clear port feature requests to the parent hub where the USB device is connected. Unfortunately, these operations are transparent to xHCI driver unless the USB device is plugged in a root port. This patch utilizes the notify interface to notify xHCI driver whenever a USB device's power state is changed. It is harmless if a USB devices under USB 3.0 host controller suspends or resumes without a notification to hcd driver. However there may be less opportunities for power savings and there may be increased latency for restarting an endpoint. The precise impact will be different for each xHC implementation. It all depends on what an implementation does with the hints. Lu Baolu (3): usb: add a hcd notify entry in hc_driver usb: xhci: implement hc_driver notify entry usb: xhci: cleanup unnecessary stop device and ring doorbell operations drivers/usb/core/generic.c | 10 +++-- drivers/usb/core/hcd.c | 23 + drivers/usb/host/xhci-hub.c | 49 + drivers/usb/host/xhci.c | 28 ++ drivers/usb/host/xhci.h | 3 +++ include/linux/usb/hcd.h | 11 +- 6 files changed, 73 insertions(+), 51 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH 2/3] usb: xhci: implement hc_driver notify entry
This patch implements the notify entry defined in hc_driver for xHC
driver. For HCD_MSG_DEV_SUSPEND message, it will issue stop endpoint
command for each endpoint in the device. The Suspend(SP) bit in the
command TRB will be set, which gives xHC a hint about the suspend.
For HCD_MSG_DEV_RESUME, it will ring doorbells for all endpoints
unconditionally. XHC may use these hints to optimize its operation
on endpoint state cashes.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 2 +-
drivers/usb/host/xhci.c | 28
drivers/usb/host/xhci.h | 3 +++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0827d7c..a83e82e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -266,7 +266,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct
xhci_hcd *xhci,
* to complete.
* suspend will set to 1, if suspend bit need to set in command.
*/
-static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
{
struct xhci_virt_device *virt_dev;
struct xhci_command *cmd;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec8ac16..1e4aa78 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4917,6 +4917,29 @@ error:
}
EXPORT_SYMBOL_GPL(xhci_gen_setup);
+void xhci_notify(struct usb_hcd *hcd, struct usb_device *udev,
+ enum hcd_notification_type type, void *data)
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ switch (type) {
+ case HCD_MSG_DEV_SUSPEND:
+ xhci_stop_device(xhci, udev-slot_id, 1);
+ break;
+ case HCD_MSG_DEV_RESUME:
+ xhci_ring_device(xhci, udev-slot_id);
+ break;
+ default:
+ xhci_dbg(xhci, unknown notification message %d.\n, type);
+ break;
+ }
+
+}
+
static const struct hc_driver xhci_hc_driver = {
.description = xhci-hcd,
.product_desc = xHCI Host Controller,
@@ -4976,6 +4999,11 @@ static const struct hc_driver xhci_hc_driver = {
.enable_usb3_lpm_timeout = xhci_enable_usb3_lpm_timeout,
.disable_usb3_lpm_timeout = xhci_disable_usb3_lpm_timeout,
.find_raw_port_number = xhci_find_raw_port_number,
+
+ /*
+* notification call back
+*/
+ .notify = xhci_notify,
};
void xhci_init_driver(struct hc_driver *drv, int (*setup_fn)(struct usb_hcd *))
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8e421b8..f71c643 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1867,10 +1867,13 @@ u32 xhci_port_state_to_neutral(u32 state);
int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
u16 port);
void xhci_ring_device(struct xhci_hcd *xhci, int slot_id);
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend);
/* xHCI contexts */
struct xhci_input_control_ctx *xhci_get_input_control_ctx(struct
xhci_container_ctx *ctx);
struct xhci_slot_ctx *xhci_get_slot_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx);
struct xhci_ep_ctx *xhci_get_ep_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx, unsigned int ep_index);
+void xhci_notify(struct usb_hcd *hcd, struct usb_device *udev,
+ enum hcd_notification_type type, void *data);
#endif /* __LINUX_XHCI_HCD_H */
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH 3/3] usb: xhci: cleanup unnecessary stop device and ring doorbell operations
There is no need to call xhci_stop_device() and xhci_ring_device() in
hub control and bus suspend functions since all device suspend and
resume have been notified through the new notify entry in hc_driver.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 47 -
1 file changed, 47 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a83e82e..f12e1b7 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -704,7 +704,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
u32 temp, status;
int retval = 0;
__le32 __iomem **port_array;
- int slot_id;
struct xhci_bus_state *bus_state;
u16 link_state = 0;
u16 wake_mask = 0;
@@ -818,17 +817,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_warn(xhci, slot_id is zero\n);
- goto error;
- }
- /* unlock to execute stop endpoint commands */
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
-
xhci_set_link_state(xhci, port_array, wIndex, XDEV_U3);
spin_unlock_irqrestore(xhci-lock, flags);
@@ -876,19 +864,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- if (link_state == USB_SS_PORT_LS_U3) {
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (slot_id) {
- /* unlock to execute stop endpoint
-* commands */
- spin_unlock_irqrestore(xhci-lock,
- flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
- }
-
xhci_set_link_state(xhci, port_array, wIndex,
link_state);
@@ -994,14 +969,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
XDEV_U0);
}
bus_state-port_c_suspend |= 1 wIndex;
-
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_dbg(xhci, slot_id is zero\n);
- goto error;
- }
- xhci_ring_device(xhci, slot_id);
break;
case USB_PORT_FEAT_C_SUSPEND:
bus_state-port_c_suspend = ~(1 wIndex);
@@ -1133,20 +1100,12 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
while (port_index--) {
/* suspend the port if the port is not suspended */
u32 t1, t2;
- int slot_id;
t1 = readl(port_array[port_index]);
t2 = xhci_port_state_to_neutral(t1);
if ((t1 PORT_PE) !(t1 PORT_PLS_MASK)) {
xhci_dbg(xhci, port %d not suspended\n, port_index);
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- port_index + 1);
- if (slot_id) {
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
t2 = ~PORT_PLS_MASK;
t2 |= PORT_LINK_STROBE | XDEV_U3;
set_bit(port_index, bus_state-bus_suspended);
@@ -1207,7 +1166,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Check whether need resume ports. If needed
resume port and disable remote wakeup */
u32 temp;
- int slot_id;
temp = readl(port_array[port_index]);
if (DEV_SUPERSPEED(temp))
@@ -1240,11 +1198,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Clear PLC
[RFC][PATCH 1/3] usb: add a hcd notify entry in hc_driver
This patch adds a new entry pointer in hc_driver. With this new entry,
USB core can notify host driver when something happens and host driver
is willing to be notified. One use case of this interface comes from
xHCI compatible host controller driver.
The xHCI spec is designed to allow an xHC implementation to cache the
endpoint state. Caching endpoint state allows an xHC to reduce latency
when handling ERDYs and other USB asynchronous events. However holding
this state in xHC consumes resources and power. The xHCI spec designs
some methods through which host controller driver can hint xHC about
how to optimize its operation, e.g. to determine when it holds state
internally or pushes it out to memory, when to power down logic, etc.
When a USB device is going to suspend, states of all endpoints cached
in the xHC should be pushed out to memory to save power and resources.
Vice versa, when a USB device resumes, those states should be brought
back to the cache. USB core suspends or resumes a USB device by sending
set/clear port feature requests to the parent hub where the USB device
is connected. Unfortunately, these operations are transparent to xHCI
driver unless the USB device is plugged in a root port. This patch
utilizes the notify interface to notify xHCI driver whenever a USB
device's power state is changed.
It is harmless if a USB devices under USB 3.0 host controller suspends
or resumes without a notification to hcd driver. However there may be
less opportunities for power savings and there may be increased latency
for restarting an endpoint. The precise impact will be different for
each xHC implementation. It all depends on what an implementation does
with the hints.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/generic.c | 10 --
drivers/usb/core/hcd.c | 23 +++
include/linux/usb/hcd.h| 11 ++-
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index 358ca8d..92bee33 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -211,8 +211,12 @@ static int generic_suspend(struct usb_device *udev,
pm_message_t msg)
/* Non-root devices don't need to do anything for FREEZE or PRETHAW */
else if (msg.event == PM_EVENT_FREEZE || msg.event == PM_EVENT_PRETHAW)
rc = 0;
- else
+ else {
+ hcd_notify(udev, HCD_MSG_DEV_SUSPEND, msg);
rc = usb_port_suspend(udev, msg);
+ if (rc)
+ hcd_notify(udev, HCD_MSG_DEV_RESUME, msg);
+ }
return rc;
}
@@ -228,8 +232,10 @@ static int generic_resume(struct usb_device *udev,
pm_message_t msg)
*/
if (!udev-parent)
rc = hcd_bus_resume(udev, msg);
- else
+ else {
rc = usb_port_resume(udev, msg);
+ hcd_notify(udev, HCD_MSG_DEV_RESUME, msg);
+ }
return rc;
}
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..725d611 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2289,6 +2289,29 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
}
EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
+/**
+ * hcd_notify - notify hcd driver with a message
+ * @udev: USB device
+ * @type: message type of this notification
+ * @data: message type specific data
+ *
+ * Call back to hcd driver to notify an event.
+ */
+void hcd_notify(struct usb_device *udev,
+ enum hcd_notification_type type, void *data)
+{
+ struct usb_hcd *hcd;
+
+ if (!udev)
+ return;
+
+ hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-notify)
+ hcd-driver-notify(hcd, udev, type, data);
+}
+EXPORT_SYMBOL_GPL(hcd_notify);
+
#endif /* CONFIG_PM */
/*-*/
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 68b1e83..bdb422c 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -76,6 +76,11 @@ struct giveback_urb_bh {
struct usb_host_endpoint *completing_ep;
};
+enum hcd_notification_type {
+ HCD_MSG_DEV_SUSPEND = 0,/* USB device suspend */
+ HCD_MSG_DEV_RESUME, /* USB device resume */
+};
+
struct usb_hcd {
/*
@@ -383,7 +388,9 @@ struct hc_driver {
int (*find_raw_port_number)(struct usb_hcd *, int);
/* Call for power on/off the port if necessary */
int (*port_power)(struct usb_hcd *hcd, int portnum, bool enable);
-
+ /* Call to notify hcd when it is necessary. */
+ void(*notify)(struct usb_hcd *, struct usb_device *,
+ enum hcd_notification_type, void *);
};
static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd)
@@ -632,6 +639,8 @@ extern void usb_root_hub_lost_power(struct usb_device
*rhdev);
extern int
Re: [RFC][PATCH 1/3] usb: add a hcd notify entry in hc_driver
Hi Alan,
Thanks for your review comments. Below has my response inline.
On 05/04/2015 10:28 PM, Alan Stern wrote:
On Mon, 4 May 2015, Lu Baolu wrote:
This patch adds a new entry pointer in hc_driver. With this new entry,
USB core can notify host driver when something happens and host driver
is willing to be notified. One use case of this interface comes from
xHCI compatible host controller driver.
Notify is too generic. It's better to make the callback routine
specific to the activity in question. So this patch series should add
device_suspend and device_resume callbacks, not a general notify
callback.
Fair enough. I will make this change in v2 if there is no objection.
The xHCI spec is designed to allow an xHC implementation to cache the
endpoint state. Caching endpoint state allows an xHC to reduce latency
when handling ERDYs and other USB asynchronous events. However holding
this state in xHC consumes resources and power. The xHCI spec designs
some methods through which host controller driver can hint xHC about
how to optimize its operation, e.g. to determine when it holds state
internally or pushes it out to memory, when to power down logic, etc.
When a USB device is going to suspend, states of all endpoints cached
in the xHC should be pushed out to memory to save power and resources.
Vice versa, when a USB device resumes, those states should be brought
back to the cache. USB core suspends or resumes a USB device by sending
set/clear port feature requests to the parent hub where the USB device
is connected. Unfortunately, these operations are transparent to xHCI
driver unless the USB device is plugged in a root port. This patch
utilizes the notify interface to notify xHCI driver whenever a USB
device's power state is changed.
It is harmless if a USB devices under USB 3.0 host controller suspends
or resumes without a notification to hcd driver. However there may be
less opportunities for power savings and there may be increased latency
for restarting an endpoint. The precise impact will be different for
each xHC implementation. It all depends on what an implementation does
with the hints.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/generic.c | 10 --
drivers/usb/core/hcd.c | 23 +++
include/linux/usb/hcd.h| 11 ++-
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index 358ca8d..92bee33 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -211,8 +211,12 @@ static int generic_suspend(struct usb_device *udev,
pm_message_t msg)
/* Non-root devices don't need to do anything for FREEZE or PRETHAW */
else if (msg.event == PM_EVENT_FREEZE || msg.event == PM_EVENT_PRETHAW)
rc = 0;
- else
+ else {
+ hcd_notify(udev, HCD_MSG_DEV_SUSPEND, msg);
rc = usb_port_suspend(udev, msg);
+ if (rc)
+ hcd_notify(udev, HCD_MSG_DEV_RESUME, msg);
+ }
return rc;
}
@@ -228,8 +232,10 @@ static int generic_resume(struct usb_device *udev,
pm_message_t msg)
*/
if (!udev-parent)
rc = hcd_bus_resume(udev, msg);
- else
+ else {
rc = usb_port_resume(udev, msg);
+ hcd_notify(udev, HCD_MSG_DEV_RESUME, msg);
+ }
Don't you want to tell the HCD about the resume _before_ it happens?
After all, the devices endpoint will be used as soon as the resume
takes place.
The order that software should do during device suspend/resume is
defined in 4.15.1.1 of xHCI spec 1.1.
Spec 4.15.1.1:
Software shall stop all endpoints of a device using the Stop Endpoint
Command and setting the Suspend
(SP) flag to ‘1’ prior to selectively suspending a device. After the
device is resumed software shall ring an
endpoint’s doorbell to restart it.
--end--
So the order looks like:
tell hcd device suspend
usb_port_suspend()
usb_port_resume()
tell hcd device resume
And besides, this should be symmetrical with the code above, where you
tell the HCD about a suspend _after_ it happens.
This was done in above change in generic_suspend():
@@ -211,8 +211,12 @@ static int generic_suspend(struct usb_device *udev,
pm_message_t msg)
/* Non-root devices don't need to do anything for FREEZE or PRETHAW */
else if (msg.event == PM_EVENT_FREEZE || msg.event == PM_EVENT_PRETHAW)
rc = 0;
- else
+ else {
+ hcd_notify(udev, HCD_MSG_DEV_SUSPEND, msg);
rc = usb_port_suspend(udev, msg);
+ if (rc)
+ hcd_notify(udev, HCD_MSG_DEV_RESUME, msg);
+ }
return rc;
}
If usb_port_suspend() returns error, we should roll back HCD to the previous
state.
Alan Stern
Thanks again,
Baolu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body
Re: [RFC][PATCH 1/3] usb: add a hcd notify entry in hc_driver
On 05/05/2015 10:50 PM, Alan Stern wrote: On Tue, 5 May 2015, Lu, Baolu wrote: The order that software should do during device suspend/resume is defined in 4.15.1.1 of xHCI spec 1.1. Spec 4.15.1.1: Software shall stop all endpoints of a device using the Stop Endpoint Command and setting the Suspend (SP) flag to 1 prior to selectively suspending a device. But _after_ all the URBs sent to the device have completed, right? Yes, that's right. After the device is resumed software shall ring an endpoint's doorbell to restart it. The driver would ring the endpoint's doorbell anyway when a new URB is submitted, wouldn't it? Which means the resume callback doesn't actually have to do anything. The value of ringing door bell after resume is that hcd can fetch endpoint state from memory to cache and get ready for transfer as early as possible. --end-- So the order looks like: tell hcd device suspend usb_port_suspend() You have forgotten that usb_port_suspend() can send URBs to the device (to enable remote wakeup, for example). Therefore you shouldn't notify the HCD until usb_port_suspend() is partly or totally finished. Yes, I agree with you. I will move the notification into usb_port_suspend(), just before sending suspend request to parent hub. usb_port_resume() tell hcd device resume You have also forgotten that usb_port_resume() calls various functions that send URBs to ep0 on the device. Therefore if the HCD's device_resume callback needs to do something (like ringing ep0's doorbell), you had better invoke the callback _before_ calling usb_port_resume(). Or maybe you had better do this _within_ usb_port_resume(). Agree. I will do this within usb_port_resume() just after completing sending clear port feature. Alan Stern Thank you for the comments. I will send the v2 patch series with all your comments addressed. Thanks, Baolu -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/3] usb: xhci: implement device_suspend/device_resume entries
This patch implements device_suspend/device_resume entries for xHC driver.
device_suspend will be called when a USB device is about to suspend. It
will issue a stop endpoint command for each endpoint in this device. The
Suspend(SP) bit in the command TRB will set which will give xHC a hint
about the suspend. device_resume will be called when a USB device is just
resumed. It will ring doorbells of all endpoint unconditionally. XHC may
use these suspend/resume hints to optimize its operation.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 2 +-
drivers/usb/host/xhci.c | 40
drivers/usb/host/xhci.h | 5 +
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0827d7c..a83e82e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -266,7 +266,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct
xhci_hcd *xhci,
* to complete.
* suspend will set to 1, if suspend bit need to set in command.
*/
-static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
{
struct xhci_virt_device *virt_dev;
struct xhci_command *cmd;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec8ac16..b639627 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4680,6 +4680,30 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
return ret;
return 0;
}
+
+void xhci_device_suspend(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ xhci_stop_device(xhci, udev-slot_id, 1);
+}
+
+void xhci_device_resume(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ xhci_ring_device(xhci, udev-slot_id);
+}
#else /* CONFIG_PM */
int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
@@ -4704,6 +4728,16 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
{
return 0;
}
+
+void xhci_device_suspend(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
+{
+}
+
+void xhci_device_resume(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
+{
+}
#endif /* CONFIG_PM */
/*-*/
@@ -4976,6 +5010,12 @@ static const struct hc_driver xhci_hc_driver = {
.enable_usb3_lpm_timeout = xhci_enable_usb3_lpm_timeout,
.disable_usb3_lpm_timeout = xhci_disable_usb3_lpm_timeout,
.find_raw_port_number = xhci_find_raw_port_number,
+
+ /*
+* call back when devices suspend or resume
+*/
+ .device_suspend = xhci_device_suspend,
+ .device_resume =xhci_device_resume,
};
void xhci_init_driver(struct hc_driver *drv, int (*setup_fn)(struct usb_hcd *))
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8e421b8..3d1f73b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1867,10 +1867,15 @@ u32 xhci_port_state_to_neutral(u32 state);
int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
u16 port);
void xhci_ring_device(struct xhci_hcd *xhci, int slot_id);
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend);
/* xHCI contexts */
struct xhci_input_control_ctx *xhci_get_input_control_ctx(struct
xhci_container_ctx *ctx);
struct xhci_slot_ctx *xhci_get_slot_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx);
struct xhci_ep_ctx *xhci_get_ep_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx, unsigned int ep_index);
+void xhci_device_suspend(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg);
+void xhci_device_resume(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg);
#endif /* __LINUX_XHCI_HCD_H */
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/3] usb: notify hcd when USB device suspend or resume
This patch series try to meet a design requirement in xHCI spec. The xHCI spec is designed to allow an xHC implementation to cache the endpoint state. Caching endpoint state allows an xHC to reduce latency when handling ERDYs and other USB asynchronous events. However holding this state in xHC consumes resources and power. The xHCI spec designs some methods through which host controller driver can hint xHC about how to optimize its operation, e.g. to determine when it holds state internally or pushes it out to memory, when to power down logic, etc. When a USB device is going to suspend, states of all endpoints cached in the xHC should be pushed out to memory to save power and resources. Vice versa, when a USB device resumes, those states should be brought back to the cache. It is harmless if a USB devices under USB 3.0 host controller suspends or resumes without a notification to hcd driver. However there may be less opportunities for power savings and there may be increased latency for restarting an endpoint. The precise impact will be different for each xHC implementation. It all depends on what an implementation does with the hints. Change log: v1-v2: - make the callback name specific to the activity in question - no need to export hcd_notify - put the callback in the right place Lu Baolu (3): usb: notify hcd when USB device suspend or resume usb: xhci: implement device_suspend/device_resume entries usb: xhci: remove stop device and ring doorbell in hub control and bus suspend drivers/usb/core/hcd.c | 29 +++ drivers/usb/core/hub.c | 16 +++ drivers/usb/host/xhci-hub.c | 49 + drivers/usb/host/xhci.c | 40 drivers/usb/host/xhci.h | 5 + include/linux/usb/hcd.h | 10 - 6 files changed, 100 insertions(+), 49 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend
There is no need to call xhci_stop_device() and xhci_ring_device() in
hub control and bus suspend functions since all device suspend and
resume have been notified through device_suspend/device_resume interfaces.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 47 -
1 file changed, 47 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a83e82e..f12e1b7 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -704,7 +704,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
u32 temp, status;
int retval = 0;
__le32 __iomem **port_array;
- int slot_id;
struct xhci_bus_state *bus_state;
u16 link_state = 0;
u16 wake_mask = 0;
@@ -818,17 +817,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_warn(xhci, slot_id is zero\n);
- goto error;
- }
- /* unlock to execute stop endpoint commands */
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
-
xhci_set_link_state(xhci, port_array, wIndex, XDEV_U3);
spin_unlock_irqrestore(xhci-lock, flags);
@@ -876,19 +864,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- if (link_state == USB_SS_PORT_LS_U3) {
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (slot_id) {
- /* unlock to execute stop endpoint
-* commands */
- spin_unlock_irqrestore(xhci-lock,
- flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
- }
-
xhci_set_link_state(xhci, port_array, wIndex,
link_state);
@@ -994,14 +969,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
XDEV_U0);
}
bus_state-port_c_suspend |= 1 wIndex;
-
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_dbg(xhci, slot_id is zero\n);
- goto error;
- }
- xhci_ring_device(xhci, slot_id);
break;
case USB_PORT_FEAT_C_SUSPEND:
bus_state-port_c_suspend = ~(1 wIndex);
@@ -1133,20 +1100,12 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
while (port_index--) {
/* suspend the port if the port is not suspended */
u32 t1, t2;
- int slot_id;
t1 = readl(port_array[port_index]);
t2 = xhci_port_state_to_neutral(t1);
if ((t1 PORT_PE) !(t1 PORT_PLS_MASK)) {
xhci_dbg(xhci, port %d not suspended\n, port_index);
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- port_index + 1);
- if (slot_id) {
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
t2 = ~PORT_PLS_MASK;
t2 |= PORT_LINK_STROBE | XDEV_U3;
set_bit(port_index, bus_state-bus_suspended);
@@ -1207,7 +1166,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Check whether need resume ports. If needed
resume port and disable remote wakeup */
u32 temp;
- int slot_id;
temp = readl(port_array[port_index]);
if (DEV_SUPERSPEED(temp))
@@ -1240,11 +1198,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Clear PLC
[PATCH v2 1/3] usb: notify hcd when USB device suspend or resume
This patch adds two new entries in hc_driver. With these new entries,
USB core can notify host driver when a USB device is about to suspend
or just resumed.
The xHCI spec is designed to allow an xHC implementation to cache the
endpoint state. Caching endpoint state allows an xHC to reduce latency
when handling ERDYs and other USB asynchronous events. However holding
this state in xHC consumes resources and power. The xHCI spec designs
some methods through which host controller driver can hint xHC about
how to optimize its operation, e.g. to determine when it holds state
internally or pushes it out to memory, when to power down logic, etc.
When a USB device is going to suspend, states of all endpoints cached
in the xHC should be pushed out to memory to save power and resources.
Vice versa, when a USB device resumes, those states should be brought
back to the cache. USB core suspends or resumes a USB device by sending
set/clear port feature requests to the parent hub where the USB device
is connected. Unfortunately, these operations are transparent to xHCI
driver unless the USB device is plugged in a root port. This patch
utilizes the callback entries to notify xHCI driver whenever a USB
device suspends or resumes.
It is harmless if a USB devices under USB 3.0 host controller suspends
or resumes without a notification to hcd driver. However there may be
less opportunities for power savings and there may be increased latency
for restarting an endpoint. The precise impact will be different for
each xHC implementation. It all depends on what an implementation does
with the hints.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hcd.c | 29 +
drivers/usb/core/hub.c | 16
include/linux/usb/hcd.h | 10 +-
3 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..a4cfc2d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2289,6 +2289,35 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
}
EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
+/**
+ * hcd_suspend_notify - notify hcd driver when a device is going to suspend
+ * @udev: USB device to be suspended
+ * @msg: Power Management message describing this state transition
+ *
+ * Call back to hcd driver to notify that a USB device is going to suspend.
+ */
+void hcd_suspend_notify(struct usb_device *udev, pm_message_t msg)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-device_suspend)
+ hcd-driver-device_suspend(hcd, udev, msg);
+}
+
+/**
+ * hcd_resume_notify - notify hcd driver when a device is just resumed
+ * @udev: USB device just resumed
+ * @msg: Power Management message describing this state transition
+ *
+ * Call back to hcd driver to notify that a USB device is just resumed.
+ */
+void hcd_resume_notify(struct usb_device *udev, pm_message_t msg)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-device_resume)
+ hcd-driver-device_resume(hcd, udev, msg);
+}
#endif /* CONFIG_PM */
/*-*/
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3b71516..ed22b51 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3144,6 +3144,14 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
goto err_lpm3;
}
+ /*
+* Call back to hcd if it expects. xHCI compatible host controller
+* driver expects to be notified prior to selectively suspending a
+* device. xHCI hcd could optimize the endpoint cache for power
+* saving purpose. Refer to 4.15.1.1 of xHCI 1.1 for more information.
+*/
+ hcd_suspend_notify(udev, msg);
+
/* see 7.1.7.6 */
if (hub_is_superspeed(hub-hdev))
status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3);
@@ -3169,6 +3177,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
if (status) {
dev_dbg(port_dev-dev, can't suspend, status %d\n, status);
+ hcd_resume_notify(udev, msg);
+
/* Try to enable USB3 LPM and LTM again */
usb_unlocked_enable_lpm(udev);
err_lpm3:
@@ -3422,6 +3432,12 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
}
SuspendCleared:
+ /* Call back to hcd if it expects. xHCI compatible host controller
+* driver expects to be notified after a device is resumed. xHCI
+* hcd could optimize the endpoint cache for latency reducing
+* purpose. Refer to 4.15.1.1 of xHCI 1.1 for more information.
+*/
+ hcd_resume_notify(udev, msg);
if (status == 0) {
udev-port_is_suspended = 0;
if (hub_is_superspeed(hub-hdev)) {
diff --git a/include/linux/usb/hcd.h b
Re: [PATCH v3 2/3] usb: xhci: implement device_suspend/device_resume entries
On 05/08/2015 04:47 AM, Greg Kroah-Hartman wrote:
On Thu, May 07, 2015 at 10:18:15AM +0800, Lu Baolu wrote:
This patch implements device_suspend/device_resume entries for xHC driver.
device_suspend will be called when a USB device is about to suspend. It
will issue a stop endpoint command for each endpoint in this device. The
Suspend(SP) bit in the command TRB will set which will give xHC a hint
about the suspend. device_resume will be called when a USB device is just
resumed. It will ring doorbells of all endpoint unconditionally. XHC may
use these suspend/resume hints to optimize its operation.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 2 +-
drivers/usb/host/xhci.c | 40
drivers/usb/host/xhci.h | 11 +++
3 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0827d7c..a83e82e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -266,7 +266,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct
xhci_hcd *xhci,
* to complete.
* suspend will set to 1, if suspend bit need to set in command.
*/
-static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
{
struct xhci_virt_device *virt_dev;
struct xhci_command *cmd;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec8ac16..6b61833 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4680,6 +4680,40 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
return ret;
return 0;
}
+
+/*
+ * xHCI compatible host controller driver expects to be notified prior to
+ * selectively suspending a device. xHCI hcd could optimize the endpoint
+ * cache for power saving purpose. Refer to 4.15.1.1 of xHCI 1.1.
+ */
+void xhci_device_suspend(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
You never use the 'msg' field, so please remove it from the callback.
'msg' could be used in another patch to determine the pm event types:
auto-suspend or system suspend. I didn't bring up that patch together
with this patch series because 1) that's part of xhci 1.1 features, and
2) it's still not tested yet.
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ xhci_stop_device(xhci, udev-slot_id, 1);
+}
+
+/*
+ * xHCI compatible host controller driver expects to be notified after a
+ * USB device is resumed. xHCI hcd could optimize the endpoint cache
+ * to reduce the latency. Refer to 4.15.1.1 of xHCI 1.1.
+ */
+void xhci_device_resume(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
Same here.
I will remove 'msg' field in this call back. I don't see any cases where
msg could be used.
Thank you,
Baolu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume
On 05/07/2015 10:34 PM, Alan Stern wrote:
On Thu, 7 May 2015, Lu, Baolu wrote:
+ void(*device_suspend)(struct usb_hcd *, struct usb_device *udev,
+ pm_message_t msg);
+ void(*device_resume)(struct usb_hcd *, struct usb_device *udev,
+ pm_message_t msg);
};
Your callbacks don't use the msg argument. What makes you think it is
needed?
This msg argument is valuable. XHCI spec defines a capability named FSC
(Force Save context Capability). When this capability is implemented, the
Save State operation (do during host suspend) shall save any cached Slot,
Endpoint, Stream or other Context information to memory. xHCI hcd could
use this msg to determine whether it needs to issue stop endpoint with
SP (suspend) bit set.
I don't understand. What is the advantage of using FSC?
I'm sorry, I didn't make it clear.
As part of host suspend, controller save state will be issued to save
host internal state in xhci_suspend():
/* step 4: set CSS flag */
command = readl(xhci-op_regs-command);
command |= CMD_CSS;
writel(command, xhci-op_regs-command);
if (xhci_handshake(xhci-op_regs-status,
STS_SAVE, 0, 10 * 1000)) {
xhci_warn(xhci, WARN: xHC save state timeout\n);
spin_unlock_irq(xhci-lock);
return -ETIMEDOUT;
}
If FSC is supported, the cached Slot, Endpoint, Stream, or other
Context information are also saved.
Hence, when FSC is supported, software does not have to issue Stop
Endpoint Command to push public and private endpoint state into
memory as part of system suspend process.
The logic in xhci_device_suspend() will look like:
if xhci_device_suspend() callback was called due to system suspend,
(mesg.event PM_EVENT_SUSPEND is true) and FSC is supported by
the xHC implementation, xhci_device_suspend() could ignore stop
endpoint command, since CSS will be done in xhc_suspend() later and
where all the endpoint caches will be pushed to memory.
How would xhci-hcd use msg to determine this? And why doesn't your
2/3 patch already do it?
xhci-hcd can use msg to determine which case the callback is called,
run-time suspend or system suspend.
FSC needs a separate patch, I already have it in Mathias' gate. I didn't
bring it together with this patch series because it's still not tested yet.
Alan Stern
Thank you!
Baolu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/3] usb: xhci: implement device_suspend/device_resume entries
On 05/08/2015 03:28 PM, Greg Kroah-Hartman wrote:
On Fri, May 08, 2015 at 09:23:40AM +0800, Lu, Baolu wrote:
On 05/08/2015 04:47 AM, Greg Kroah-Hartman wrote:
On Thu, May 07, 2015 at 10:18:15AM +0800, Lu Baolu wrote:
This patch implements device_suspend/device_resume entries for xHC driver.
device_suspend will be called when a USB device is about to suspend. It
will issue a stop endpoint command for each endpoint in this device. The
Suspend(SP) bit in the command TRB will set which will give xHC a hint
about the suspend. device_resume will be called when a USB device is just
resumed. It will ring doorbells of all endpoint unconditionally. XHC may
use these suspend/resume hints to optimize its operation.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 2 +-
drivers/usb/host/xhci.c | 40
drivers/usb/host/xhci.h | 11 +++
3 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0827d7c..a83e82e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -266,7 +266,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct
xhci_hcd *xhci,
* to complete.
* suspend will set to 1, if suspend bit need to set in command.
*/
-static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
{
struct xhci_virt_device *virt_dev;
struct xhci_command *cmd;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec8ac16..6b61833 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4680,6 +4680,40 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
return ret;
return 0;
}
+
+/*
+ * xHCI compatible host controller driver expects to be notified prior to
+ * selectively suspending a device. xHCI hcd could optimize the endpoint
+ * cache for power saving purpose. Refer to 4.15.1.1 of xHCI 1.1.
+ */
+void xhci_device_suspend(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
You never use the 'msg' field, so please remove it from the callback.
'msg' could be used in another patch to determine the pm event types:
auto-suspend or system suspend. I didn't bring up that patch together
with this patch series because 1) that's part of xhci 1.1 features, and
2) it's still not tested yet.
We don't add features to kernel code that is not used when it is
added. We can always change function calls later as we do not have a
stable api at all.
Understand it now. I will remove 'msg' fields in v4 patch series.
thanks,
greg k-h
Thank you.
Baolu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume
On 05/06/2015 10:35 PM, Alan Stern wrote:
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 68b1e83..621d9b7 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -383,7 +383,13 @@ struct hc_driver {
int (*find_raw_port_number)(struct usb_hcd *, int);
/* Call for power on/off the port if necessary */
int (*port_power)(struct usb_hcd *hcd, int portnum, bool enable);
-
+ /* Call back to hcd when a USB device is going to suspend or just
+* resumed.
+*/
+ void(*device_suspend)(struct usb_hcd *, struct usb_device *udev,
+ pm_message_t msg);
+ void(*device_resume)(struct usb_hcd *, struct usb_device *udev,
+ pm_message_t msg);
};
Your callbacks don't use the msg argument. What makes you think it is
needed?
'msg' arguments are not used in this patch series. I will remove them.
Alan Stern
Thank you,
Baolu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 1/3] usb: notify hcd when USB device suspend or resume
This patch adds two new entries in hc_driver. With these new entries,
USB core can notify host driver when a USB device is about to suspend
or just resumed.
The xHCI spec is designed to allow an xHC implementation to cache the
endpoint state. Caching endpoint state allows an xHC to reduce latency
when handling ERDYs and other USB asynchronous events. However holding
this state in xHC consumes resources and power. The xHCI spec designs
some methods through which host controller driver can hint xHC about
how to optimize its operation, e.g. to determine when it holds state
internally or pushes it out to memory, when to power down logic, etc.
When a USB device is going to suspend, states of all endpoints cached
in the xHC should be pushed out to memory to save power and resources.
Vice versa, when a USB device resumes, those states should be brought
back to the cache. USB core suspends or resumes a USB device by sending
set/clear port feature requests to the parent hub where the USB device
is connected. Unfortunately, these operations are transparent to xHCI
driver unless the USB device is plugged in a root port. This patch
utilizes the callback entries to notify xHCI driver whenever a USB
device suspends or resumes.
It is harmless if a USB devices under USB 3.0 host controller suspends
or resumes without a notification to hcd driver. However there may be
less opportunities for power savings and there may be increased latency
for restarting an endpoint. The precise impact will be different for
each xHC implementation. It all depends on what an implementation does
with the hints.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hcd.c | 27 +++
drivers/usb/core/hub.c | 5 +
include/linux/usb/hcd.h | 8 +++-
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..007450d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2289,6 +2289,33 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
}
EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
+/**
+ * hcd_suspend_notify - notify hcd driver when a device is going to suspend
+ * @udev: USB device to be suspended
+ *
+ * Call back to hcd driver to notify that a USB device is going to suspend.
+ */
+void hcd_suspend_notify(struct usb_device *udev)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-device_suspend)
+ hcd-driver-device_suspend(hcd, udev);
+}
+
+/**
+ * hcd_resume_notify - notify hcd driver when a device is just resumed
+ * @udev: USB device just resumed
+ *
+ * Call back to hcd driver to notify that a USB device is just resumed.
+ */
+void hcd_resume_notify(struct usb_device *udev)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-device_resume)
+ hcd-driver-device_resume(hcd, udev);
+}
#endif /* CONFIG_PM */
/*-*/
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 52178bc..28d76b7 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3149,6 +3149,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
goto err_lpm3;
}
+ hcd_suspend_notify(udev);
+
/* see 7.1.7.6 */
if (hub_is_superspeed(hub-hdev))
status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3);
@@ -3174,6 +3176,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
if (status) {
dev_dbg(port_dev-dev, can't suspend, status %d\n, status);
+ hcd_resume_notify(udev);
+
/* Try to enable USB3 LPM and LTM again */
usb_unlocked_enable_lpm(udev);
err_lpm3:
@@ -3427,6 +3431,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
}
SuspendCleared:
+ hcd_resume_notify(udev);
if (status == 0) {
udev-port_is_suspended = 0;
if (hub_is_superspeed(hub-hdev)) {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 68b1e83..bc2eb1c 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -383,7 +383,11 @@ struct hc_driver {
int (*find_raw_port_number)(struct usb_hcd *, int);
/* Call for power on/off the port if necessary */
int (*port_power)(struct usb_hcd *hcd, int portnum, bool enable);
-
+ /* Call back to hcd when a USB device is going to suspend or just
+* resumed.
+*/
+ void(*device_suspend)(struct usb_hcd *, struct usb_device *udev);
+ void(*device_resume)(struct usb_hcd *, struct usb_device *udev);
};
static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd)
@@ -632,6 +636,8 @@ extern void usb_root_hub_lost_power(struct usb_device
*rhdev);
extern int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg
[PATCH v4 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend
There is no need to call xhci_stop_device() and xhci_ring_device() in
hub control and bus suspend functions since all device suspend and
resume have been notified through device_suspend/device_resume interfaces.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 47 -
1 file changed, 47 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a83e82e..f12e1b7 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -704,7 +704,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
u32 temp, status;
int retval = 0;
__le32 __iomem **port_array;
- int slot_id;
struct xhci_bus_state *bus_state;
u16 link_state = 0;
u16 wake_mask = 0;
@@ -818,17 +817,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_warn(xhci, slot_id is zero\n);
- goto error;
- }
- /* unlock to execute stop endpoint commands */
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
-
xhci_set_link_state(xhci, port_array, wIndex, XDEV_U3);
spin_unlock_irqrestore(xhci-lock, flags);
@@ -876,19 +864,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- if (link_state == USB_SS_PORT_LS_U3) {
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (slot_id) {
- /* unlock to execute stop endpoint
-* commands */
- spin_unlock_irqrestore(xhci-lock,
- flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
- }
-
xhci_set_link_state(xhci, port_array, wIndex,
link_state);
@@ -994,14 +969,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
XDEV_U0);
}
bus_state-port_c_suspend |= 1 wIndex;
-
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_dbg(xhci, slot_id is zero\n);
- goto error;
- }
- xhci_ring_device(xhci, slot_id);
break;
case USB_PORT_FEAT_C_SUSPEND:
bus_state-port_c_suspend = ~(1 wIndex);
@@ -1133,20 +1100,12 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
while (port_index--) {
/* suspend the port if the port is not suspended */
u32 t1, t2;
- int slot_id;
t1 = readl(port_array[port_index]);
t2 = xhci_port_state_to_neutral(t1);
if ((t1 PORT_PE) !(t1 PORT_PLS_MASK)) {
xhci_dbg(xhci, port %d not suspended\n, port_index);
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- port_index + 1);
- if (slot_id) {
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
t2 = ~PORT_PLS_MASK;
t2 |= PORT_LINK_STROBE | XDEV_U3;
set_bit(port_index, bus_state-bus_suspended);
@@ -1207,7 +1166,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Check whether need resume ports. If needed
resume port and disable remote wakeup */
u32 temp;
- int slot_id;
temp = readl(port_array[port_index]);
if (DEV_SUPERSPEED(temp))
@@ -1240,11 +1198,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Clear PLC
[PATCH v4 0/3] usb: notify hcd when USB device suspend or resume
This patch series try to meet a design requirement in xHCI spec. The xHCI spec is designed to allow an xHC implementation to cache the endpoint state. Caching endpoint state allows an xHC to reduce latency when handling ERDYs and other USB asynchronous events. However holding this state in xHC consumes resources and power. The xHCI spec designs some methods through which host controller driver can hint xHC about how to optimize its operation, e.g. to determine when it holds state internally or pushes it out to memory, when to power down logic, etc. When a USB device is going to suspend, states of all endpoints cached in the xHC should be pushed out to memory to save power and resources. Vice versa, when a USB device resumes, those states should be brought back to the cache. It is harmless if a USB devices under USB 3.0 host controller suspends or resumes without a notification to hcd driver. However there may be less opportunities for power savings and there may be increased latency for restarting an endpoint. The precise impact will be different for each xHC implementation. It all depends on what an implementation does with the hints. Change log: v3-v4: - remove unused 'msg' parameter in the callbacks v2-v3: - move two xhci specific comments from hub to xhci - define xhci_device_suspend(resume) as NULL when no PM_CONFIG v1-v2: - make the callback name specific to the activity in question - no need to export hcd_notify - put the callback in the right place Lu Baolu (3): usb: notify hcd when USB device suspend or resume usb: xhci: implement device_suspend/device_resume entries usb: xhci: remove stop device and ring doorbell in hub control and bus suspend drivers/usb/core/hcd.c | 27 + drivers/usb/core/hub.c | 5 + drivers/usb/host/xhci-hub.c | 49 + drivers/usb/host/xhci.c | 38 +++ drivers/usb/host/xhci.h | 9 + include/linux/usb/hcd.h | 8 +++- 6 files changed, 87 insertions(+), 49 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 2/3] usb: xhci: implement device_suspend/device_resume entries
This patch implements device_suspend/device_resume entries for xHC driver.
device_suspend will be called when a USB device is about to suspend. It
will issue a stop endpoint command for each endpoint in this device. The
Suspend(SP) bit in the command TRB will set which will give xHC a hint
about the suspend. device_resume will be called when a USB device is just
resumed. It will ring doorbells of all endpoint unconditionally. XHC may
use these suspend/resume hints to optimize its operation.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 2 +-
drivers/usb/host/xhci.c | 38 ++
drivers/usb/host/xhci.h | 9 +
3 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0827d7c..a83e82e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -266,7 +266,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct
xhci_hcd *xhci,
* to complete.
* suspend will set to 1, if suspend bit need to set in command.
*/
-static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
{
struct xhci_virt_device *virt_dev;
struct xhci_command *cmd;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec8ac16..330961d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4680,6 +4680,38 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
return ret;
return 0;
}
+
+/*
+ * xHCI compatible host controller driver expects to be notified prior to
+ * selectively suspending a device. xHCI hcd could optimize the endpoint
+ * cache for power saving purpose. Refer to 4.15.1.1 of xHCI 1.1.
+ */
+void xhci_device_suspend(struct usb_hcd *hcd, struct usb_device *udev)
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ xhci_stop_device(xhci, udev-slot_id, 1);
+}
+
+/*
+ * xHCI compatible host controller driver expects to be notified after a
+ * USB device is resumed. xHCI hcd could optimize the endpoint cache
+ * to reduce the latency. Refer to 4.15.1.1 of xHCI 1.1.
+ */
+void xhci_device_resume(struct usb_hcd *hcd, struct usb_device *udev)
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ xhci_ring_device(xhci, udev-slot_id);
+}
#else /* CONFIG_PM */
int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
@@ -4976,6 +5008,12 @@ static const struct hc_driver xhci_hc_driver = {
.enable_usb3_lpm_timeout = xhci_enable_usb3_lpm_timeout,
.disable_usb3_lpm_timeout = xhci_disable_usb3_lpm_timeout,
.find_raw_port_number = xhci_find_raw_port_number,
+
+ /*
+* call back when devices suspend or resume
+*/
+ .device_suspend = xhci_device_suspend,
+ .device_resume =xhci_device_resume,
};
void xhci_init_driver(struct hc_driver *drv, int (*setup_fn)(struct usb_hcd *))
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8e421b8..67c13c5 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1867,10 +1867,19 @@ u32 xhci_port_state_to_neutral(u32 state);
int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
u16 port);
void xhci_ring_device(struct xhci_hcd *xhci, int slot_id);
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend);
/* xHCI contexts */
struct xhci_input_control_ctx *xhci_get_input_control_ctx(struct
xhci_container_ctx *ctx);
struct xhci_slot_ctx *xhci_get_slot_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx);
struct xhci_ep_ctx *xhci_get_ep_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx, unsigned int ep_index);
+#ifdef CONFIG_PM
+void xhci_device_suspend(struct usb_hcd *hcd, struct usb_device *udev);
+void xhci_device_resume(struct usb_hcd *hcd, struct usb_device *udev);
+#else
+#define xhci_device_suspendNULL
+#define xhci_device_resume NULL
+#endif /* CONFIG_PM */
+
#endif /* __LINUX_XHCI_HCD_H */
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume
On 05/06/2015 10:35 PM, Alan Stern wrote:
On Wed, 6 May 2015, Lu Baolu wrote:
This patch adds two new entries in hc_driver. With these new entries,
USB core can notify host driver when a USB device is about to suspend
or just resumed.
The xHCI spec is designed to allow an xHC implementation to cache the
endpoint state. Caching endpoint state allows an xHC to reduce latency
when handling ERDYs and other USB asynchronous events. However holding
this state in xHC consumes resources and power. The xHCI spec designs
some methods through which host controller driver can hint xHC about
how to optimize its operation, e.g. to determine when it holds state
internally or pushes it out to memory, when to power down logic, etc.
When a USB device is going to suspend, states of all endpoints cached
in the xHC should be pushed out to memory to save power and resources.
Vice versa, when a USB device resumes, those states should be brought
back to the cache. USB core suspends or resumes a USB device by sending
set/clear port feature requests to the parent hub where the USB device
is connected. Unfortunately, these operations are transparent to xHCI
driver unless the USB device is plugged in a root port. This patch
utilizes the callback entries to notify xHCI driver whenever a USB
device suspends or resumes.
It is harmless if a USB devices under USB 3.0 host controller suspends
or resumes without a notification to hcd driver. However there may be
less opportunities for power savings and there may be increased latency
for restarting an endpoint. The precise impact will be different for
each xHC implementation. It all depends on what an implementation does
with the hints.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3144,6 +3144,14 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
goto err_lpm3;
}
+ /*
+* Call back to hcd if it expects. xHCI compatible host controller
+* driver expects to be notified prior to selectively suspending a
+* device. xHCI hcd could optimize the endpoint cache for power
+* saving purpose. Refer to 4.15.1.1 of xHCI 1.1 for more information.
+*/
Doesn't this comment belong in the xhci-hcd source code rather than the
hub driver source code?
Yes, I agree. I will move this and below comments to xhci driver.
+ hcd_suspend_notify(udev, msg);
+
/* see 7.1.7.6 */
if (hub_is_superspeed(hub-hdev))
status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3);
@@ -3169,6 +3177,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
if (status) {
dev_dbg(port_dev-dev, can't suspend, status %d\n, status);
+ hcd_resume_notify(udev, msg);
+
/* Try to enable USB3 LPM and LTM again */
usb_unlocked_enable_lpm(udev);
err_lpm3:
@@ -3422,6 +3432,12 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
}
SuspendCleared:
+ /* Call back to hcd if it expects. xHCI compatible host controller
+* driver expects to be notified after a device is resumed. xHCI
+* hcd could optimize the endpoint cache for latency reducing
+* purpose. Refer to 4.15.1.1 of xHCI 1.1 for more information.
+*/
Same for this comment.
+ hcd_resume_notify(udev, msg);
if (status == 0) {
udev-port_is_suspended = 0;
if (hub_is_superspeed(hub-hdev)) {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 68b1e83..621d9b7 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -383,7 +383,13 @@ struct hc_driver {
int (*find_raw_port_number)(struct usb_hcd *, int);
/* Call for power on/off the port if necessary */
int (*port_power)(struct usb_hcd *hcd, int portnum, bool enable);
-
+ /* Call back to hcd when a USB device is going to suspend or just
+* resumed.
+*/
+ void(*device_suspend)(struct usb_hcd *, struct usb_device *udev,
+ pm_message_t msg);
+ void(*device_resume)(struct usb_hcd *, struct usb_device *udev,
+ pm_message_t msg);
};
Your callbacks don't use the msg argument. What makes you think it is
needed?
This msg argument is valuable. XHCI spec defines a capability named FSC
(Force Save context Capability). When this capability is implemented, the
Save State operation (do during host suspend) shall save any cached Slot,
Endpoint, Stream or other Context information to memory. xHCI hcd could
use this msg to determine whether it needs to issue stop endpoint with
SP (suspend) bit set.
FSC is optional prior 1.1 and mandatory since 1.1. I have patches for 1.1
features in Mathias' gate. We could bring them up after we test them on
the real hardware.
Alan Stern
--
To unsubscribe from this list: send
Re: [PATCH v2 2/3] usb: xhci: implement device_suspend/device_resume entries
On 05/06/2015 10:30 PM, Alan Stern wrote:
On Wed, 6 May 2015, Lu Baolu wrote:
This patch implements device_suspend/device_resume entries for xHC driver.
device_suspend will be called when a USB device is about to suspend. It
will issue a stop endpoint command for each endpoint in this device. The
Suspend(SP) bit in the command TRB will set which will give xHC a hint
about the suspend. device_resume will be called when a USB device is just
resumed. It will ring doorbells of all endpoint unconditionally. XHC may
use these suspend/resume hints to optimize its operation.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
...
#else /* CONFIG_PM */
int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
@@ -4704,6 +4728,16 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
{
return 0;
}
+
+void xhci_device_suspend(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
+{
+}
+
+void xhci_device_resume(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
+{
+}
You don't need to have empty functions. Just do this:
#define xhci_device_suspend NULL
#define xhci_device_resume NULL
in the appropriate place, when CONFIG_PM is not enabled.
Yes, I agree. I will change it.
Alan Stern
Thank you,
Baolu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/3] usb: notify hcd when USB device suspend or resume
This patch adds two new entries in hc_driver. With these new entries,
USB core can notify host driver when a USB device is about to suspend
or just resumed.
The xHCI spec is designed to allow an xHC implementation to cache the
endpoint state. Caching endpoint state allows an xHC to reduce latency
when handling ERDYs and other USB asynchronous events. However holding
this state in xHC consumes resources and power. The xHCI spec designs
some methods through which host controller driver can hint xHC about
how to optimize its operation, e.g. to determine when it holds state
internally or pushes it out to memory, when to power down logic, etc.
When a USB device is going to suspend, states of all endpoints cached
in the xHC should be pushed out to memory to save power and resources.
Vice versa, when a USB device resumes, those states should be brought
back to the cache. USB core suspends or resumes a USB device by sending
set/clear port feature requests to the parent hub where the USB device
is connected. Unfortunately, these operations are transparent to xHCI
driver unless the USB device is plugged in a root port. This patch
utilizes the callback entries to notify xHCI driver whenever a USB
device suspends or resumes.
It is harmless if a USB devices under USB 3.0 host controller suspends
or resumes without a notification to hcd driver. However there may be
less opportunities for power savings and there may be increased latency
for restarting an endpoint. The precise impact will be different for
each xHC implementation. It all depends on what an implementation does
with the hints.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hcd.c | 29 +
drivers/usb/core/hub.c | 5 +
include/linux/usb/hcd.h | 10 +-
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..a4cfc2d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2289,6 +2289,35 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
}
EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
+/**
+ * hcd_suspend_notify - notify hcd driver when a device is going to suspend
+ * @udev: USB device to be suspended
+ * @msg: Power Management message describing this state transition
+ *
+ * Call back to hcd driver to notify that a USB device is going to suspend.
+ */
+void hcd_suspend_notify(struct usb_device *udev, pm_message_t msg)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-device_suspend)
+ hcd-driver-device_suspend(hcd, udev, msg);
+}
+
+/**
+ * hcd_resume_notify - notify hcd driver when a device is just resumed
+ * @udev: USB device just resumed
+ * @msg: Power Management message describing this state transition
+ *
+ * Call back to hcd driver to notify that a USB device is just resumed.
+ */
+void hcd_resume_notify(struct usb_device *udev, pm_message_t msg)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-device_resume)
+ hcd-driver-device_resume(hcd, udev, msg);
+}
#endif /* CONFIG_PM */
/*-*/
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3b71516..b3f0dc8 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3144,6 +3144,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
goto err_lpm3;
}
+ hcd_suspend_notify(udev, msg);
+
/* see 7.1.7.6 */
if (hub_is_superspeed(hub-hdev))
status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3);
@@ -3169,6 +3171,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
if (status) {
dev_dbg(port_dev-dev, can't suspend, status %d\n, status);
+ hcd_resume_notify(udev, msg);
+
/* Try to enable USB3 LPM and LTM again */
usb_unlocked_enable_lpm(udev);
err_lpm3:
@@ -3422,6 +3426,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
}
SuspendCleared:
+ hcd_resume_notify(udev, msg);
if (status == 0) {
udev-port_is_suspended = 0;
if (hub_is_superspeed(hub-hdev)) {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 68b1e83..621d9b7 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -383,7 +383,13 @@ struct hc_driver {
int (*find_raw_port_number)(struct usb_hcd *, int);
/* Call for power on/off the port if necessary */
int (*port_power)(struct usb_hcd *hcd, int portnum, bool enable);
-
+ /* Call back to hcd when a USB device is going to suspend or just
+* resumed.
+*/
+ void(*device_suspend)(struct usb_hcd *, struct usb_device *udev,
+ pm_message_t msg);
+ void(*device_resume)(struct usb_hcd *, struct
[PATCH v3 2/3] usb: xhci: implement device_suspend/device_resume entries
This patch implements device_suspend/device_resume entries for xHC driver.
device_suspend will be called when a USB device is about to suspend. It
will issue a stop endpoint command for each endpoint in this device. The
Suspend(SP) bit in the command TRB will set which will give xHC a hint
about the suspend. device_resume will be called when a USB device is just
resumed. It will ring doorbells of all endpoint unconditionally. XHC may
use these suspend/resume hints to optimize its operation.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 2 +-
drivers/usb/host/xhci.c | 40
drivers/usb/host/xhci.h | 11 +++
3 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0827d7c..a83e82e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -266,7 +266,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct
xhci_hcd *xhci,
* to complete.
* suspend will set to 1, if suspend bit need to set in command.
*/
-static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
{
struct xhci_virt_device *virt_dev;
struct xhci_command *cmd;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec8ac16..6b61833 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4680,6 +4680,40 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
return ret;
return 0;
}
+
+/*
+ * xHCI compatible host controller driver expects to be notified prior to
+ * selectively suspending a device. xHCI hcd could optimize the endpoint
+ * cache for power saving purpose. Refer to 4.15.1.1 of xHCI 1.1.
+ */
+void xhci_device_suspend(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ xhci_stop_device(xhci, udev-slot_id, 1);
+}
+
+/*
+ * xHCI compatible host controller driver expects to be notified after a
+ * USB device is resumed. xHCI hcd could optimize the endpoint cache
+ * to reduce the latency. Refer to 4.15.1.1 of xHCI 1.1.
+ */
+void xhci_device_resume(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg)
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ xhci_ring_device(xhci, udev-slot_id);
+}
#else /* CONFIG_PM */
int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
@@ -4976,6 +5010,12 @@ static const struct hc_driver xhci_hc_driver = {
.enable_usb3_lpm_timeout = xhci_enable_usb3_lpm_timeout,
.disable_usb3_lpm_timeout = xhci_disable_usb3_lpm_timeout,
.find_raw_port_number = xhci_find_raw_port_number,
+
+ /*
+* call back when devices suspend or resume
+*/
+ .device_suspend = xhci_device_suspend,
+ .device_resume =xhci_device_resume,
};
void xhci_init_driver(struct hc_driver *drv, int (*setup_fn)(struct usb_hcd *))
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8e421b8..d826439 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1867,10 +1867,21 @@ u32 xhci_port_state_to_neutral(u32 state);
int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
u16 port);
void xhci_ring_device(struct xhci_hcd *xhci, int slot_id);
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend);
/* xHCI contexts */
struct xhci_input_control_ctx *xhci_get_input_control_ctx(struct
xhci_container_ctx *ctx);
struct xhci_slot_ctx *xhci_get_slot_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx);
struct xhci_ep_ctx *xhci_get_ep_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx, unsigned int ep_index);
+#ifdef CONFIG_PM
+void xhci_device_suspend(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg);
+void xhci_device_resume(struct usb_hcd *hcd,
+ struct usb_device *udev, pm_message_t msg);
+#else
+#define xhci_device_suspendNULL
+#define xhci_device_resume NULL
+#endif /* CONFIG_PM */
+
#endif /* __LINUX_XHCI_HCD_H */
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend
There is no need to call xhci_stop_device() and xhci_ring_device() in
hub control and bus suspend functions since all device suspend and
resume have been notified through device_suspend/device_resume interfaces.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 47 -
1 file changed, 47 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a83e82e..f12e1b7 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -704,7 +704,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
u32 temp, status;
int retval = 0;
__le32 __iomem **port_array;
- int slot_id;
struct xhci_bus_state *bus_state;
u16 link_state = 0;
u16 wake_mask = 0;
@@ -818,17 +817,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_warn(xhci, slot_id is zero\n);
- goto error;
- }
- /* unlock to execute stop endpoint commands */
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
-
xhci_set_link_state(xhci, port_array, wIndex, XDEV_U3);
spin_unlock_irqrestore(xhci-lock, flags);
@@ -876,19 +864,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- if (link_state == USB_SS_PORT_LS_U3) {
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (slot_id) {
- /* unlock to execute stop endpoint
-* commands */
- spin_unlock_irqrestore(xhci-lock,
- flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
- }
-
xhci_set_link_state(xhci, port_array, wIndex,
link_state);
@@ -994,14 +969,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
XDEV_U0);
}
bus_state-port_c_suspend |= 1 wIndex;
-
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_dbg(xhci, slot_id is zero\n);
- goto error;
- }
- xhci_ring_device(xhci, slot_id);
break;
case USB_PORT_FEAT_C_SUSPEND:
bus_state-port_c_suspend = ~(1 wIndex);
@@ -1133,20 +1100,12 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
while (port_index--) {
/* suspend the port if the port is not suspended */
u32 t1, t2;
- int slot_id;
t1 = readl(port_array[port_index]);
t2 = xhci_port_state_to_neutral(t1);
if ((t1 PORT_PE) !(t1 PORT_PLS_MASK)) {
xhci_dbg(xhci, port %d not suspended\n, port_index);
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- port_index + 1);
- if (slot_id) {
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
t2 = ~PORT_PLS_MASK;
t2 |= PORT_LINK_STROBE | XDEV_U3;
set_bit(port_index, bus_state-bus_suspended);
@@ -1207,7 +1166,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Check whether need resume ports. If needed
resume port and disable remote wakeup */
u32 temp;
- int slot_id;
temp = readl(port_array[port_index]);
if (DEV_SUPERSPEED(temp))
@@ -1240,11 +1198,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Clear PLC
[PATCH v3 0/3] usb: notify hcd when USB device suspend or resume
This patch series try to meet a design requirement in xHCI spec. The xHCI spec is designed to allow an xHC implementation to cache the endpoint state. Caching endpoint state allows an xHC to reduce latency when handling ERDYs and other USB asynchronous events. However holding this state in xHC consumes resources and power. The xHCI spec designs some methods through which host controller driver can hint xHC about how to optimize its operation, e.g. to determine when it holds state internally or pushes it out to memory, when to power down logic, etc. When a USB device is going to suspend, states of all endpoints cached in the xHC should be pushed out to memory to save power and resources. Vice versa, when a USB device resumes, those states should be brought back to the cache. It is harmless if a USB devices under USB 3.0 host controller suspends or resumes without a notification to hcd driver. However there may be less opportunities for power savings and there may be increased latency for restarting an endpoint. The precise impact will be different for each xHC implementation. It all depends on what an implementation does with the hints. Change log: v2-v3: - move two xhci specific comments from hub to xhci - define xhci_device_suspend(resume) as NULL when no PM_CONFIG v1-v2: - make the callback name specific to the activity in question - no need to export hcd_notify - put the callback in the right place Lu Baolu (3): usb: notify hcd when USB device suspend or resume usb: xhci: implement device_suspend/device_resume entries usb: xhci: remove stop device and ring doorbell in hub control and bus suspend drivers/usb/core/hcd.c | 29 +++ drivers/usb/core/hub.c | 5 + drivers/usb/host/xhci-hub.c | 49 + drivers/usb/host/xhci.c | 40 drivers/usb/host/xhci.h | 11 ++ include/linux/usb/hcd.h | 10 - 6 files changed, 95 insertions(+), 49 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 1/3] usb: add a hcd notify entry in hc_driver
On 05/04/2015 04:14 PM, Greg Kroah-Hartman wrote:
On Mon, May 04, 2015 at 11:15:30AM +0800, Lu Baolu wrote:
This patch adds a new entry pointer in hc_driver. With this new entry,
USB core can notify host driver when something happens and host driver
is willing to be notified. One use case of this interface comes from
xHCI compatible host controller driver.
The xHCI spec is designed to allow an xHC implementation to cache the
endpoint state. Caching endpoint state allows an xHC to reduce latency
when handling ERDYs and other USB asynchronous events. However holding
this state in xHC consumes resources and power. The xHCI spec designs
some methods through which host controller driver can hint xHC about
how to optimize its operation, e.g. to determine when it holds state
internally or pushes it out to memory, when to power down logic, etc.
When a USB device is going to suspend, states of all endpoints cached
in the xHC should be pushed out to memory to save power and resources.
Vice versa, when a USB device resumes, those states should be brought
back to the cache. USB core suspends or resumes a USB device by sending
set/clear port feature requests to the parent hub where the USB device
is connected. Unfortunately, these operations are transparent to xHCI
driver unless the USB device is plugged in a root port. This patch
utilizes the notify interface to notify xHCI driver whenever a USB
device's power state is changed.
It is harmless if a USB devices under USB 3.0 host controller suspends
or resumes without a notification to hcd driver. However there may be
less opportunities for power savings and there may be increased latency
for restarting an endpoint. The precise impact will be different for
each xHC implementation. It all depends on what an implementation does
with the hints.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/generic.c | 10 --
drivers/usb/core/hcd.c | 23 +++
include/linux/usb/hcd.h| 11 ++-
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index 358ca8d..92bee33 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -211,8 +211,12 @@ static int generic_suspend(struct usb_device *udev,
pm_message_t msg)
/* Non-root devices don't need to do anything for FREEZE or PRETHAW */
else if (msg.event == PM_EVENT_FREEZE || msg.event == PM_EVENT_PRETHAW)
rc = 0;
- else
+ else {
+ hcd_notify(udev, HCD_MSG_DEV_SUSPEND, msg);
rc = usb_port_suspend(udev, msg);
+ if (rc)
+ hcd_notify(udev, HCD_MSG_DEV_RESUME, msg);
+ }
return rc;
}
@@ -228,8 +232,10 @@ static int generic_resume(struct usb_device *udev,
pm_message_t msg)
*/
if (!udev-parent)
rc = hcd_bus_resume(udev, msg);
- else
+ else {
rc = usb_port_resume(udev, msg);
+ hcd_notify(udev, HCD_MSG_DEV_RESUME, msg);
+ }
return rc;
}
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..725d611 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2289,6 +2289,29 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
}
EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
+/**
+ * hcd_notify - notify hcd driver with a message
+ * @udev: USB device
+ * @type: message type of this notification
+ * @data: message type specific data
+ *
+ * Call back to hcd driver to notify an event.
+ */
+void hcd_notify(struct usb_device *udev,
+ enum hcd_notification_type type, void *data)
+{
+ struct usb_hcd *hcd;
+
+ if (!udev)
+ return;
+
+ hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-notify)
+ hcd-driver-notify(hcd, udev, type, data);
+}
+EXPORT_SYMBOL_GPL(hcd_notify);
Why does this have to be exported?
It doesn't have to be exported. I will remove it in v2.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume
On 05/11/2015 10:25 PM, Alan Stern wrote: On Sat, 9 May 2015, Lu, Baolu wrote: If FSC is supported, the cached Slot, Endpoint, Stream, or other Context information are also saved. Hence, when FSC is supported, software does not have to issue Stop Endpoint Command to push public and private endpoint state into memory as part of system suspend process. Why do you have to push this state into memory at all? Does the controller hardware lose the cached state information when it is in low power? I don't think controller hardware will lose the cached state information when it is in low power. But since cache in controller consumes power and resources, by pushing state into memory, hardware can power off the cache logic during suspend. Hence more power saving gains. The logic in xhci_device_suspend() will look like: if xhci_device_suspend() callback was called due to system suspend, (mesg.event PM_EVENT_SUSPEND is true) and FSC is supported by the xHC implementation, xhci_device_suspend() could ignore stop endpoint command, since CSS will be done in xhc_suspend() later and where all the endpoint caches will be pushed to memory. I still don't understand this. You said earlier that according to section 4.15.1.1 of the xHCI spec, the endpoint rings should _always_ be stopped with SP set when a device is suspended. Now you're The intention of stop endpoint with SP set is to tell hardware that a device is going to suspend, hardware don't need to contain the endpoint state in internal cache anymore. Hardware _could_ use this hint to push endpoint state into memory to reduce power consumption. saying that they don't need to be stopped during a system suspend if the controller supports FSC. (Or maybe you're saying they need to be stopped but SP doesn't need to be set -- it's hard to tell.) Even FSC is supported, controller hardware still need to push cached endpoint state into memory when a USB device is suspended. The difference is when FSC is enforced, CSS command will push any cached endpoint state into memory unconditionally. You said above that the hardware _could_ push endpoint state into memory. Now you're saying it _needs_ to do this! Make up your mind. I'm sorry that I confused you. FSC is a different thing from what this patch series does. I should say software could ask hardware to push endpoint state into memory even FSC is supported. But in some cases, it can be optimized as I will describe it below. So, when xhci_device_suspend() knows that CSS command will be executed later and CSS command will push cached endpoint state into memory (a.k.a. FSC is enforced), it could skip issuing stop endpoint command with SP bit set to avoid duplication and reduce the suspend time. This is the case for system suspend since CSS command is part of xhci_suspend() and xhci_suspend() will be executed after all USB devices have been suspended. But it's not case for run-time suspend (auto-pm) since USB device suspend and host controller suspend are independent for run-time case. That's the reason why I wanted to keep 'msg' parameter. But just as Greg said, we don't need to keep a parameter when it's not used and can add it later when it is required. So which is it? Do you need to stop the endpoint rings? Is it okay not to set SP? stop endpoint and stop endpoint with SP set serve different purposes in Linux xhci driver as my understanding. stop endpoint command is used to stop a active ring when upper layer want to cancel a URB. stop endpoint with SP set is used to hint hardware that a USB is going to suspend. Hence stop endpoint with SP set must be executed in case that the transfer ring is empty. (How does the contents of the transfer ring affect anything? Besides, there are never any active URBs when a device gets suspended, so the transfer ring will _always_ be empty at such times.) This is still extremely confusing. You're not doing a good job of explaining the situation clearly and logically. I'm sorry for that. Let's see if I understand it correctly: When the controller goes into suspend, you want the cache to be pushed into memory so that the cache can be powered down, thereby saving additional energy. Not the controller goes into suspend, but USB devices. In order to talking to USB devices, xHCI has an endpoint state for each endpoint of a device. When a USB device goes into suspend, xHC driver could ask the hardware to push the state from cache to the memory. Thereby saving additional energy. This is the intention of this patch series. If the hardware supports FSC, this will happen automatically. If the hardware doesn't support FSC, the cached data won't get pushed to memory unless the driver tells the controller to do so at the time the device is suspended. But this will slow things down, so the driver should avoid doing it when it's not needed. During system suspend you know
Re: [PATCH v4 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend
On 05/08/2015 07:01 PM, Greg Kroah-Hartman wrote: On Fri, May 08, 2015 at 06:26:28PM +0800, Lu Baolu wrote: There is no need to call xhci_stop_device() and xhci_ring_device() in hub control and bus suspend functions since all device suspend and resume have been notified through device_suspend/device_resume interfaces. Does this mean that after patch 2, things are broken and require this patch to prevent problems? No. things work well without patch 3. stop device and ring doorbell operations in hub control and bus suspend is harmless, but duplicated and unnecessary, so I remove them. I don't want to have any patch to make the system unstable. thanks, greg k-h Thank you, Baolu -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume
On 05/08/2015 10:21 PM, Alan Stern wrote: On Fri, 8 May 2015, Lu, Baolu wrote: On 05/07/2015 10:34 PM, Alan Stern wrote: On Thu, 7 May 2015, Lu, Baolu wrote: + void(*device_suspend)(struct usb_hcd *, struct usb_device *udev, + pm_message_t msg); + void(*device_resume)(struct usb_hcd *, struct usb_device *udev, + pm_message_t msg); }; Your callbacks don't use the msg argument. What makes you think it is needed? This msg argument is valuable. XHCI spec defines a capability named FSC (Force Save context Capability). When this capability is implemented, the Save State operation (do during host suspend) shall save any cached Slot, Endpoint, Stream or other Context information to memory. xHCI hcd could use this msg to determine whether it needs to issue stop endpoint with SP (suspend) bit set. I don't understand. What is the advantage of using FSC? I'm sorry, I didn't make it clear. As part of host suspend, controller save state will be issued to save host internal state in xhci_suspend(): ... If FSC is supported, the cached Slot, Endpoint, Stream, or other Context information are also saved. Hence, when FSC is supported, software does not have to issue Stop Endpoint Command to push public and private endpoint state into memory as part of system suspend process. Why do you have to push this state into memory at all? Does the controller hardware lose the cached state information when it is in low power? I don't think controller hardware will lose the cached state information when it is in low power. But since cache in controller consumes power and resources, by pushing state into memory, hardware can power off the cache logic during suspend. Hence more power saving gains. The logic in xhci_device_suspend() will look like: if xhci_device_suspend() callback was called due to system suspend, (mesg.event PM_EVENT_SUSPEND is true) and FSC is supported by the xHC implementation, xhci_device_suspend() could ignore stop endpoint command, since CSS will be done in xhc_suspend() later and where all the endpoint caches will be pushed to memory. I still don't understand this. You said earlier that according to section 4.15.1.1 of the xHCI spec, the endpoint rings should _always_ be stopped with SP set when a device is suspended. Now you're The intention of stop endpoint with SP set is to tell hardware that a device is going to suspend, hardware don't need to contain the endpoint state in internal cache anymore. Hardware _could_ use this hint to push endpoint state into memory to reduce power consumption. saying that they don't need to be stopped during a system suspend if the controller supports FSC. (Or maybe you're saying they need to be stopped but SP doesn't need to be set -- it's hard to tell.) Even FSC is supported, controller hardware still need to push cached endpoint state into memory when a USB device is suspended. The difference is when FSC is enforced, CSS command will push any cached endpoint state into memory unconditionally. So, when xhci_device_suspend() knows that CSS command will be executed later and CSS command will push cached endpoint state into memory (a.k.a. FSC is enforced), it could skip issuing stop endpoint command with SP bit set to avoid duplication and reduce the suspend time. This is the case for system suspend since CSS command is part of xhci_suspend() and xhci_suspend() will be executed after all USB devices have been suspended. But it's not case for run-time suspend (auto-pm) since USB device suspend and host controller suspend are independent for run-time case. That's the reason why I wanted to keep 'msg' parameter. But just as Greg said, we don't need to keep a parameter when it's not used and can add it later when it is required. So which is it? Do you need to stop the endpoint rings? Is it okay not to set SP? stop endpoint and stop endpoint with SP set serve different purposes in Linux xhci driver as my understanding. stop endpoint command is used to stop a active ring when upper layer want to cancel a URB. stop endpoint with SP set is used to hint hardware that a USB is going to suspend. Hence stop endpoint with SP set must be executed in case that the transfer ring is empty. Alan Stern Thank you, Baolu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend
There is no need to call xhci_stop_device() and xhci_ring_device() in
hub control and bus suspend functions since all device suspend and
resume have been notified through device_suspend/device_resume interfaces.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 47 -
1 file changed, 47 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a83e82e..f12e1b7 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -704,7 +704,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
u32 temp, status;
int retval = 0;
__le32 __iomem **port_array;
- int slot_id;
struct xhci_bus_state *bus_state;
u16 link_state = 0;
u16 wake_mask = 0;
@@ -818,17 +817,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_warn(xhci, slot_id is zero\n);
- goto error;
- }
- /* unlock to execute stop endpoint commands */
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
-
xhci_set_link_state(xhci, port_array, wIndex, XDEV_U3);
spin_unlock_irqrestore(xhci-lock, flags);
@@ -876,19 +864,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
goto error;
}
- if (link_state == USB_SS_PORT_LS_U3) {
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (slot_id) {
- /* unlock to execute stop endpoint
-* commands */
- spin_unlock_irqrestore(xhci-lock,
- flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
- }
-
xhci_set_link_state(xhci, port_array, wIndex,
link_state);
@@ -994,14 +969,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
XDEV_U0);
}
bus_state-port_c_suspend |= 1 wIndex;
-
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- wIndex + 1);
- if (!slot_id) {
- xhci_dbg(xhci, slot_id is zero\n);
- goto error;
- }
- xhci_ring_device(xhci, slot_id);
break;
case USB_PORT_FEAT_C_SUSPEND:
bus_state-port_c_suspend = ~(1 wIndex);
@@ -1133,20 +1100,12 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
while (port_index--) {
/* suspend the port if the port is not suspended */
u32 t1, t2;
- int slot_id;
t1 = readl(port_array[port_index]);
t2 = xhci_port_state_to_neutral(t1);
if ((t1 PORT_PE) !(t1 PORT_PLS_MASK)) {
xhci_dbg(xhci, port %d not suspended\n, port_index);
- slot_id = xhci_find_slot_id_by_port(hcd, xhci,
- port_index + 1);
- if (slot_id) {
- spin_unlock_irqrestore(xhci-lock, flags);
- xhci_stop_device(xhci, slot_id, 1);
- spin_lock_irqsave(xhci-lock, flags);
- }
t2 = ~PORT_PLS_MASK;
t2 |= PORT_LINK_STROBE | XDEV_U3;
set_bit(port_index, bus_state-bus_suspended);
@@ -1207,7 +1166,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Check whether need resume ports. If needed
resume port and disable remote wakeup */
u32 temp;
- int slot_id;
temp = readl(port_array[port_index]);
if (DEV_SUPERSPEED(temp))
@@ -1240,11 +1198,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
/* Clear PLC
[PATCH v5 1/3] usb: notify hcd when USB device suspend or resume
This patch adds two new entries in hc_driver. With these new entries,
USB core can notify host driver when a USB device is about to suspend
or just resumed.
The xHCI spec is designed to allow an xHC implementation to cache the
endpoint state. Caching endpoint state allows an xHC to reduce latency
when handling ERDYs and other USB asynchronous events. However holding
this state in xHC consumes resources and power. The xHCI spec designs
some methods through which host controller driver can hint xHC about
how to optimize its operation, e.g. to determine when it holds state
internally or pushes it out to memory, when to power down logic, etc.
When a USB device is going to suspend, states of all endpoints cached
in the xHC should be pushed out to memory to save power and resources.
Vice versa, when a USB device resumes, those states should be brought
back to the cache. USB core suspends or resumes a USB device by sending
set/clear port feature requests to the parent hub where the USB device
is connected. Unfortunately, these operations are transparent to xHCI
driver unless the USB device is plugged in a root port. This patch
utilizes the callback entries to notify xHCI driver whenever a USB
device suspends or resumes.
It is harmless if a USB devices under USB 3.0 host controller suspends
or resumes without a notification to hcd driver. However there may be
less opportunities for power savings and there may be increased latency
for restarting an endpoint. The precise impact will be different for
each xHC implementation. It all depends on what an implementation does
with the hints.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Acked-by: Alan Stern st...@rowland.harvard.edu
---
drivers/usb/core/hcd.c | 27 +++
drivers/usb/core/hub.c | 5 +
include/linux/usb/hcd.h | 8 +++-
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..007450d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2289,6 +2289,33 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
}
EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
+/**
+ * hcd_suspend_notify - notify hcd driver when a device is going to suspend
+ * @udev: USB device to be suspended
+ *
+ * Call back to hcd driver to notify that a USB device is going to suspend.
+ */
+void hcd_suspend_notify(struct usb_device *udev)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-device_suspend)
+ hcd-driver-device_suspend(hcd, udev);
+}
+
+/**
+ * hcd_resume_notify - notify hcd driver when a device is just resumed
+ * @udev: USB device just resumed
+ *
+ * Call back to hcd driver to notify that a USB device is just resumed.
+ */
+void hcd_resume_notify(struct usb_device *udev)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev-bus);
+
+ if (hcd-driver-device_resume)
+ hcd-driver-device_resume(hcd, udev);
+}
#endif /* CONFIG_PM */
/*-*/
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 52178bc..28d76b7 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3149,6 +3149,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
goto err_lpm3;
}
+ hcd_suspend_notify(udev);
+
/* see 7.1.7.6 */
if (hub_is_superspeed(hub-hdev))
status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3);
@@ -3174,6 +3176,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
if (status) {
dev_dbg(port_dev-dev, can't suspend, status %d\n, status);
+ hcd_resume_notify(udev);
+
/* Try to enable USB3 LPM and LTM again */
usb_unlocked_enable_lpm(udev);
err_lpm3:
@@ -3427,6 +3431,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
}
SuspendCleared:
+ hcd_resume_notify(udev);
if (status == 0) {
udev-port_is_suspended = 0;
if (hub_is_superspeed(hub-hdev)) {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 68b1e83..bc2eb1c 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -383,7 +383,11 @@ struct hc_driver {
int (*find_raw_port_number)(struct usb_hcd *, int);
/* Call for power on/off the port if necessary */
int (*port_power)(struct usb_hcd *hcd, int portnum, bool enable);
-
+ /* Call back to hcd when a USB device is going to suspend or just
+* resumed.
+*/
+ void(*device_suspend)(struct usb_hcd *, struct usb_device *udev);
+ void(*device_resume)(struct usb_hcd *, struct usb_device *udev);
};
static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd)
@@ -632,6 +636,8 @@ extern void usb_root_hub_lost_power(struct usb_device
*rhdev);
extern int hcd_bus_suspend
[PATCH v5 2/3] usb: xhci: implement device_suspend/device_resume entries
This patch implements device_suspend/device_resume entries for xHC driver.
device_suspend will be called when a USB device is about to suspend. It
will issue a stop endpoint command for each endpoint in this device. The
Suspend(SP) bit in the command TRB will set which will give xHC a hint
about the suspend. device_resume will be called when a USB device is just
resumed. It will ring doorbells of all endpoint unconditionally. XHC may
use these suspend/resume hints to optimize its operation.
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 2 +-
drivers/usb/host/xhci.c | 38 ++
drivers/usb/host/xhci.h | 9 +
3 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0827d7c..a83e82e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -266,7 +266,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct
xhci_hcd *xhci,
* to complete.
* suspend will set to 1, if suspend bit need to set in command.
*/
-static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
{
struct xhci_virt_device *virt_dev;
struct xhci_command *cmd;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec8ac16..330961d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4680,6 +4680,38 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
return ret;
return 0;
}
+
+/*
+ * xHCI compatible host controller driver expects to be notified prior to
+ * selectively suspending a device. xHCI hcd could optimize the endpoint
+ * cache for power saving purpose. Refer to 4.15.1.1 of xHCI 1.1.
+ */
+void xhci_device_suspend(struct usb_hcd *hcd, struct usb_device *udev)
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ xhci_stop_device(xhci, udev-slot_id, 1);
+}
+
+/*
+ * xHCI compatible host controller driver expects to be notified after a
+ * USB device is resumed. xHCI hcd could optimize the endpoint cache
+ * to reduce the latency. Refer to 4.15.1.1 of xHCI 1.1.
+ */
+void xhci_device_resume(struct usb_hcd *hcd, struct usb_device *udev)
+{
+ struct xhci_hcd *xhci;
+
+ xhci = hcd_to_xhci(hcd);
+ if (!xhci || !xhci-devs[udev-slot_id])
+ return;
+
+ xhci_ring_device(xhci, udev-slot_id);
+}
#else /* CONFIG_PM */
int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
@@ -4976,6 +5008,12 @@ static const struct hc_driver xhci_hc_driver = {
.enable_usb3_lpm_timeout = xhci_enable_usb3_lpm_timeout,
.disable_usb3_lpm_timeout = xhci_disable_usb3_lpm_timeout,
.find_raw_port_number = xhci_find_raw_port_number,
+
+ /*
+* call back when devices suspend or resume
+*/
+ .device_suspend = xhci_device_suspend,
+ .device_resume =xhci_device_resume,
};
void xhci_init_driver(struct hc_driver *drv, int (*setup_fn)(struct usb_hcd *))
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8e421b8..67c13c5 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1867,10 +1867,19 @@ u32 xhci_port_state_to_neutral(u32 state);
int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,
u16 port);
void xhci_ring_device(struct xhci_hcd *xhci, int slot_id);
+int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend);
/* xHCI contexts */
struct xhci_input_control_ctx *xhci_get_input_control_ctx(struct
xhci_container_ctx *ctx);
struct xhci_slot_ctx *xhci_get_slot_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx);
struct xhci_ep_ctx *xhci_get_ep_ctx(struct xhci_hcd *xhci, struct
xhci_container_ctx *ctx, unsigned int ep_index);
+#ifdef CONFIG_PM
+void xhci_device_suspend(struct usb_hcd *hcd, struct usb_device *udev);
+void xhci_device_resume(struct usb_hcd *hcd, struct usb_device *udev);
+#else
+#define xhci_device_suspendNULL
+#define xhci_device_resume NULL
+#endif /* CONFIG_PM */
+
#endif /* __LINUX_XHCI_HCD_H */
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 0/3] usb: notify hcd when USB device suspend or resume
This patch series try to meet a design requirement in xHCI spec. The xHCI spec is designed to allow an xHC implementation to cache the endpoint state. Caching endpoint state allows an xHC to reduce latency when handling ERDYs and other USB asynchronous events. However holding this state in xHC consumes resources and power. The xHCI spec designs some methods through which host controller driver can hint xHC about how to optimize its operation, e.g. to determine when it holds state internally or pushes it out to memory, when to power down logic, etc. When a USB device is going to suspend, states of all endpoints cached in the xHC should be pushed out to memory to save power and resources. Vice versa, when a USB device resumes, those states should be brought back to the cache. It is harmless if a USB devices under USB 3.0 host controller suspends or resumes without a notification to hcd driver. However there may be less opportunities for power savings and there may be increased latency for restarting an endpoint. The precise impact will be different for each xHC implementation. It all depends on what an implementation does with the hints. Change log: v4-v5: - add Alan's ACK for 1/3 v3-v4: - remove unused 'msg' parameter in the callbacks v2-v3: - move two xhci specific comments from hub to xhci - define xhci_device_suspend(resume) as NULL when no PM_CONFIG v1-v2: - make the callback name specific to the activity in question - no need to export hcd_notify - put the callback in the right place Lu Baolu (3): usb: notify hcd when USB device suspend or resume usb: xhci: implement device_suspend/device_resume entries usb: xhci: remove stop device and ring doorbell in hub control and bus suspend drivers/usb/core/hcd.c | 27 + drivers/usb/core/hub.c | 5 + drivers/usb/host/xhci-hub.c | 49 + drivers/usb/host/xhci.c | 38 +++ drivers/usb/host/xhci.h | 9 + include/linux/usb/hcd.h | 8 +++- 6 files changed, 87 insertions(+), 49 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume
On 05/12/2015 11:54 PM, Alan Stern wrote: On Tue, 12 May 2015, Lu, Baolu wrote: I'm sorry that I confused you. FSC is a different thing from what this patch series does. I know that. The patch series, in its current form, is fine. Now I'm trying to understand what you originally wanted to do. Let's see if I understand it correctly: When the controller goes into suspend, you want the cache to be pushed into memory so that the cache can be powered down, thereby saving additional energy. Not the controller goes into suspend, but USB devices. Suppose a USB device goes into runtime suspend, and suppose the hardware pushes the cached information for the endpoints on that device into memory. The cache will still contain information for other, non-suspended devices. Consequently the cache _can't_ be powered down at this time, right? You seem to be saying: When a USB device goes into suspend, you want the cache to be pushed into memory so that the cache can be powered down, thereby saving additional energy. But that is just wrong! The only time you know the hardware can safely power-down the cache is when the _controller_ goes into suspend. At that time you _know_ all the attached devices are suspended, so the cache isn't needed. Therefore, when the controller goes into suspend, you want to make sure that the cache has been pushed into memory. The actual push operation can occur earlier, during xhci_device_suspend, but the time when the information's location matters is during xhci_suspend. In order to talking to USB devices, xHCI has an endpoint state for each endpoint of a device. When a USB device goes into suspend, xHC driver could ask the hardware to push the state from cache to the memory. Thereby saving additional energy. This is the intention of this patch series. You don't save any energy when the _device_ goes into suspend. You save energy only when the _controller_ goes into suspend, because that's the only time when the cache can be powered down. Right? Yes, I agree with you. Hardware could only power down the cache after all data has been pushed out. That could happen during host suspend as far as I can see. If the hardware supports FSC, this will happen automatically. If the hardware doesn't support FSC, the cached data won't get pushed to memory unless the driver tells the controller to do so at the time the device is suspended. But this will slow things down, so the driver should avoid doing it when it's not needed. During system suspend you know in advance that the controller will be suspended. Therefore the driver should push the cache to memory if the hardware doesn't support FSC. During runtime suspend you don't know in advance whether the controller will be suspended, so the driver should not push the cache to memory. But what happens in the case where all the devices have gone into runtime suspend, so the controller also goes into runtime suspend, and the hardware doesn't support FSC? It seems that in this case the cache would have to remain powered on. FSC is not part of this patch series. It was introduced when I tried to explain the reason why I kept 'msg' parameter in the callback. If the hardware support FSC, another operation named save context operation will push everything in hardware cache into memory. So when a USB device is going to suspend and xhci_device_suspend() callback is being called, software can do an optimization. That is, if save context operation will push everything in hardware cache to memory later, xhci_device_suspend() could skip asking hardware for cache operation and let save context operation do it later. That's more or less the same as what I wrote above, except that I said it would happen automatically if the hardware supports FSC. It isn't automatic; it requires the driver to issue a save context operation command. Yes, exactly. One example of this situation is system suspend. During system suspend, below process will be done for a USB3 fabric. 1) all USB devices suspend 2) root hub suspend 3) host controller suspend In 1), xhci_device_suspend() call back will be called for each device suspend. In 3) save context operation will be executed. In this case, if FSC is supported, xhci_device_suspend() could skip asking host hardware for cache operation. That's basically what I said. But now why should msg matter? It seems that xhci_device_suspend() should skip asking for the cache operation whenever FSC is supported, regardless of whether you're talking about runtime suspend or system suspend. In runtime case, a USB device could be selected to suspend while host controller is busy with other USB devices. Since FSC only impacts the behavior of save context operation, it's probably that a USB device is selectively suspended while it's
Re: [PATCH 1/1] usb: ulpi: ulpi_init should be used in subsys_initcall
On 05/21/2015 03:33 PM, Heikki Krogerus wrote: On Thu, May 21, 2015 at 01:40:43PM +0800, Lu Baolu wrote: The intention of this change is to fix below kernel panic when USB_ULPI_BUS was configured as buildin. That is actually incorrect. Having the bus build-in does not cause this panic.. [0.746856] kernel BUG at drivers/base/driver.c:153! [0.752418] invalid opcode: [#1] PREEMPT SMP [0.757804] Modules linked in: [0.893985] Call Trace: [0.896729] [81870cb1] ? ulpi_register_driver+0x21/0x30 [0.903654] [823d5cac] tusb1210_driver_init+0x10/0x12 [0.910386] [81000318] do_one_initcall+0xd8/0x200 [0.916729] [8239b094] kernel_init_freeable+0x196/0x21e [0.923655] [81affed0] ? rest_init+0x90/0x90 [0.929509] [81affede] kernel_init+0xe/0xf0 [0.935266] [81b0f442] ret_from_fork+0x42/0x70 [0.941315] [81affed0] ? rest_init+0x90/0x90 This happens if the PHY drivers are build-in. If the bus is build-in, but the drivers are modules, you are still fine. I ever tried the config of bus =y and driver =n. It results in panic as well. The call trace looks like below. 1.058876] [8165a6d2] device_add+0x402/0x640 [1.064829] [8165a92e] device_register+0x1e/0x30 [1.071072] [81870bdc] ulpi_register_interface+0x14c/0x1f0 [1.078291] [817c68d4] dwc3_ulpi_init+0x24/0x60 [1.084437] [817be052] dwc3_probe+0x802/0x1650 [1.090487] [8165fb54] platform_drv_probe+0x34/0xa0 [1.097022] [8165d9e9] driver_probe_device+0x209/0x4b0 [1.103850] [8165dc90] ? driver_probe_device+0x4b0/0x4b0 [1.110871] [8165dccb] __device_attach+0x3b/0x40 [1.117114] [8165b6d3] bus_for_each_drv+0x63/0xa0 [1.123454] [8165d778] device_attach+0x98/0xb0 [1.129503] [8165cb60] bus_probe_device+0xa0/0xc0 [1.135843] [8165a73f] device_add+0x46f/0x640 [1.141799] [81090fe0] ? __insert_resource+0x60/0x150 [1.148530] [8165f780] platform_device_add+0xd0/0x2b0 [1.155260] [817c7386] dwc3_pci_probe+0xf6/0x2c0 [1.161505] [8149d86c] pci_device_probe+0x8c/0xf0 [1.167846] [8165d9e9] driver_probe_device+0x209/0x4b0 [1.174673] [8165dd6b] __driver_attach+0x9b/0xa0 [1.180917] [8165dcd0] ? __device_attach+0x40/0x40 [1.187354] [8165b60b] bus_for_each_dev+0x6b/0xb0 [1.193694] [8165d2be] driver_attach+0x1e/0x20 [1.199742] [8165ce80] bus_add_driver+0x180/0x250 [1.206086] [823e453c] ? ftrace_define_fields_dwc3_log_trb+0x126/0x126 [1.214474] [8165e5d4] driver_register+0x64/0xf0 [1.220718] [8149c51b] __pci_register_driver+0x4b/0x50 [1.227546] [823e4555] dwc3_pci_driver_init+0x19/0x1b [1.234277] [81000318] do_one_initcall+0xd8/0x200 [1.240619] [8239b094] kernel_init_freeable+0x196/0x21e [1.247545] [81affbd0] ? rest_init+0x90/0x90 [1.253399] [81affbde] kernel_init+0xe/0xf0 [1.259156] [81b0f142] ret_from_fork+0x42/0x70 [1.265205] [81affbd0] ? rest_init+0x90/0x90 Maybe it would have been better to explain that this is addressing an issue with the execution sequence, and consider Sasha's patch as the actual fix for panic. Fair enough. I will resend the patch. Thanks guys, Thanks, Baolu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] usb: ulpi: ulpi_init should be used in subsys_initcall
The intention of this change is to fix below kernel panic when
USB_ULPI_BUS was configured as buildin.
[0.746856] kernel BUG at drivers/base/driver.c:153!
[0.752418] invalid opcode: [#1] PREEMPT SMP
[0.757804] Modules linked in:
[0.893985] Call Trace:
[0.896729] [81870cb1] ? ulpi_register_driver+0x21/0x30
[0.903654] [823d5cac] tusb1210_driver_init+0x10/0x12
[0.910386] [81000318] do_one_initcall+0xd8/0x200
[0.916729] [8239b094] kernel_init_freeable+0x196/0x21e
[0.923655] [81affed0] ? rest_init+0x90/0x90
[0.929509] [81affede] kernel_init+0xe/0xf0
[0.935266] [81b0f442] ret_from_fork+0x42/0x70
[0.941315] [81affed0] ? rest_init+0x90/0x90
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/common/ulpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..01c0c04 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -242,7 +242,7 @@ static int __init ulpi_init(void)
{
return bus_register(ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
static void __exit ulpi_exit(void)
{
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist
On 05/21/2015 05:22 AM, David Cohen wrote: Hi, On Wed, May 20, 2015 at 03:33:26PM -0400, Sasha Levin wrote: ULPI registers it's bus at module_init so if the bus fails to register, the A minor comment: s/it's/its/ module will fail to load and all will be well in the world. However, if the ULPI code is built-in rather than a module, the bus initialization may fail but we'd still try to register drivers later onto a non-existant bus, which will panic the kernel. Fix that by checking that the bus was indeed initialized before trying to register drivers on top of it. Signed-off-by: Sasha Levin sasha.le...@oracle.com --- drivers/usb/common/ulpi.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c index 0e6f968..0b0a5e7 100644 --- a/drivers/usb/common/ulpi.c +++ b/drivers/usb/common/ulpi.c @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) if (!drv-probe) return -EINVAL; + /* Was the bus registered successfully? */ + if (!ulpi_bus.p) + return -ENODEV; + Good catch. Otherwise it may trigger BUG() on driver_register(). I wonder if it would be nice to have a macro for that checking :) Anyway, Reviewed-by: David Cohen david.a.co...@linux.intel.com Well, I was also encountering panic issue when running it on Intel Bay Trail tablets. In my case, it's due to the execution sequence. When ulpi bus is built-in, driver or device registered before ulpi bus registration. Thanks, Baolu drv-driver.bus = ulpi_bus; return driver_register(drv-driver); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist
ULPI registers its bus at module_init, so if the bus fails to register, the
module will fail to load and all will be well in the world.
However, if the ULPI code is built-in rather than a module, the bus
initialization may fail, but we'd still try to register drivers later onto
a non-existent bus, which will panic the kernel.
Fix that by checking that the bus was indeed initialized before trying to
register drivers on top of it.
Signed-off-by: Sasha Levin sasha.le...@oracle.com
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Reviewed-by: David Cohen david.a.co...@linux.intel.com
---
drivers/usb/common/ulpi.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..af52b46 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv)
if (!drv-probe)
return -EINVAL;
+ /* Was the bus registered successfully? */
+ if (unlikely(WARN_ON(!ulpi_bus.p)))
+ return -ENODEV;
+
drv-driver.bus = ulpi_bus;
return driver_register(drv-driver);
@@ -242,7 +246,7 @@ static int __init ulpi_init(void)
{
return bus_register(ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
static void __exit ulpi_exit(void)
{
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On 06/04/2015 08:17 PM, Heikki Krogerus wrote: Hi Baolu, On Thu, May 28, 2015 at 08:50:12AM +0800, Lu, Baolu wrote: On 05/28/2015 12:53 AM, David Cohen wrote: Hi, On Tue, May 26, 2015 at 07:37:02PM -0700, Greg Kroah-Hartman wrote: On Wed, May 27, 2015 at 09:45:37AM +0800, Lu Baolu wrote: Phy drivers and the ulpi interface providers depend on the registration of the ulpi bus. Ulpi registers the bus in module_init(). This could cause unnecessary probe delays. What do you mean by probe delays? I believe he meant the bus users' probe delays. i.e. unnecessary -EPROBE_DEFER happening on ulpi_drivers in case they're registered before ulpi bus itself. Is that correct, Baolu? Yes. Could you prepare one more version of this patche with the change log included. Sure. I will resent it soon. Thanks, -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend
On 06/25/2015 10:53 PM, Mathias Nyman wrote: On 09.05.2015 04:15, Lu Baolu wrote: There is no need to call xhci_stop_device() and xhci_ring_device() in hub control and bus suspend functions since all device suspend and resume have been notified through device_suspend/device_resume interfaces. I was looking through this code again before sending it forward, and it occurred to me that this might be breaking the PORT_SUSPEND and PORT_SET_LINK_STATE port features for xhci root hub. In normal use these requests are called by usb core in usb_port_suspend(), which also now notifies xhci, which makes sure xhci_stop_device() is called. But I don't think there is anything preventing an URB to be sent to the xhci roothub with a PORT_SUSPEND or PORT_SET_LINK_STATE port feature request. In this case the usb_port_suspend() is not called, and no notify will stop the device. For example hub validation tests might do this. If that, we can drop this patch. It doesn't impact the other two patches in this patch series. Thanks, Baolu -Mathias -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] usb: core: lpm: set lpm_capable for root hub device
On 06/12/2015 12:32 PM, Greg Kroah-Hartman wrote:
On Fri, Jun 12, 2015 at 09:29:37AM +0800, Lu Baolu wrote:
Commit 25cd2882e2fc (usb/xhci: Change how we indicate a host supports
Link PM.) removed the code to set lpm_capable for USB 3.0 super-speed
root hub. The intention of that change was to avoid touching usb core
internal field, a.k.a. lpm_capable, and let usb core to set it by
checking U1 and U2 exit latency values in the descriptor.
Usb core checks and sets lpm_capable in hub_port_init(). Unfortunately,
root hub is a special usb device as it has no parent. Hub_port_init()
will never be called for a root hub device. That means lpm_capable will
by no means be set for the root hub. As the result, lpm isn't functional
at all in Linux kernel.
This patch add the code to check and set lpm_capable when registering a
root hub device. It could be back-ported to kernels as old as v3.15,
that contains the Commit 25cd2882e2fc (usb/xhci: Change how we indicate
a host supports Link PM.).
Cc: sta...@vger.kernel.org # 3.15
Reported-by: Kevin Strasser kevin.stras...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hcd.c | 6 ++
drivers/usb/core/hub.c | 2 +-
drivers/usb/core/usb.h | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..48b208d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1032,6 +1032,12 @@ static int register_root_hub(struct usb_hcd *hcd)
}
}
+ if (le16_to_cpu(usb_dev-descriptor.bcdUSB) = 0x0201) {
Why are you treating a binary coded value as a hex number to compare
against?
The value of the bcdUSB field is 0xJJMN for version JJ.M.N, where
JJ – major version number,
M – minor version number,
N – sub-minor version number
I saw several places in usb core where it is treated as a hex and
check the version requirement like this. Do you want me to
separate it into three numbers and check major/minor/sub-minor
versions separately?
Thanks,
Baolu
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/1] usb: core: lpm: set lpm_capable for root hub device
Commit 25cd2882e2fc (usb/xhci: Change how we indicate a host supports
Link PM.) removed the code to set lpm_capable for USB 3.0 super-speed
root hub. The intention of that change was to avoid touching usb core
internal field, a.k.a. lpm_capable, and let usb core to set it by
checking U1 and U2 exit latency values in the descriptor.
Usb core checks and sets lpm_capable in hub_port_init(). Unfortunately,
root hub is a special usb device as it has no parent. Hub_port_init()
will never be called for a root hub device. That means lpm_capable will
by no means be set for the root hub. As the result, lpm isn't functional
at all in Linux kernel.
This patch add the code to check and set lpm_capable when registering a
root hub device. It could be back-ported to kernels as old as v3.15,
that contains the Commit 25cd2882e2fc (usb/xhci: Change how we indicate
a host supports Link PM.).
Cc: sta...@vger.kernel.org # 3.15
Reported-by: Kevin Strasser kevin.stras...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hcd.c | 7 +--
drivers/usb/core/hub.c | 2 +-
drivers/usb/core/usb.h | 6 ++
3 files changed, 12 insertions(+), 3 deletions(-)
v1-v2 change log:
1. two code blocks merged (Alan's feedback)
2. fix build error when CONFIG_PM is not set (Alan's feedback)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..1c1385e 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1022,9 +1022,12 @@ static int register_root_hub(struct usb_hcd *hcd)
dev_name(usb_dev-dev), retval);
return (retval 0) ? retval : -EMSGSIZE;
}
- if (usb_dev-speed == USB_SPEED_SUPER) {
+
+ if (le16_to_cpu(usb_dev-descriptor.bcdUSB) = 0x0201) {
retval = usb_get_bos_descriptor(usb_dev);
- if (retval 0) {
+ if (!retval) {
+ usb_dev-lpm_capable = usb_device_supports_lpm(usb_dev);
+ } else if (usb_dev-speed == USB_SPEED_SUPER) {
mutex_unlock(usb_bus_list_lock);
dev_dbg(parent_dev, can't read %s bos descriptor %d\n,
dev_name(usb_dev-dev), retval);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3b71516..884d702 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -122,7 +122,7 @@ struct usb_hub *usb_hub_to_struct_hub(struct usb_device
*hdev)
return usb_get_intfdata(hdev-actconfig-interface[0]);
}
-static int usb_device_supports_lpm(struct usb_device *udev)
+int usb_device_supports_lpm(struct usb_device *udev)
{
/* USB 2.1 (and greater) devices indicate LPM support through
* their USB 2.0 Extended Capabilities BOS descriptor.
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 7eb1e26..d54a669 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -82,6 +82,7 @@ extern int usb_runtime_suspend(struct device *dev);
extern int usb_runtime_resume(struct device *dev);
extern int usb_runtime_idle(struct device *dev);
extern int usb_set_usb2_hardware_lpm(struct usb_device *udev, int enable);
+extern int usb_device_supports_lpm(struct usb_device *udev);
#else
@@ -106,6 +107,11 @@ static inline int usb_set_usb2_hardware_lpm(struct
usb_device *udev, int enable)
return 0;
}
+static inline int usb_device_supports_lpm(struct usb_device *udev)
+{
+ return 0;
+}
+
#endif
extern struct bus_type usb_bus_type;
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] usb: core: lpm: set lpm_capable for root hub device
On 06/13/2015 01:43 AM, Alan Stern wrote:
On Fri, 12 Jun 2015, Lu Baolu wrote:
Commit 25cd2882e2fc (usb/xhci: Change how we indicate a host supports
Link PM.) removed the code to set lpm_capable for USB 3.0 super-speed
root hub. The intention of that change was to avoid touching usb core
internal field, a.k.a. lpm_capable, and let usb core to set it by
checking U1 and U2 exit latency values in the descriptor.
Usb core checks and sets lpm_capable in hub_port_init(). Unfortunately,
root hub is a special usb device as it has no parent. Hub_port_init()
will never be called for a root hub device. That means lpm_capable will
by no means be set for the root hub. As the result, lpm isn't functional
at all in Linux kernel.
This patch add the code to check and set lpm_capable when registering a
root hub device. It could be back-ported to kernels as old as v3.15,
that contains the Commit 25cd2882e2fc (usb/xhci: Change how we indicate
a host supports Link PM.).
Cc: sta...@vger.kernel.org # 3.15
Reported-by: Kevin Strasser kevin.stras...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hcd.c | 6 ++
drivers/usb/core/hub.c | 2 +-
drivers/usb/core/usb.h | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..48b208d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1032,6 +1032,12 @@ static int register_root_hub(struct usb_hcd *hcd)
}
}
+ if (le16_to_cpu(usb_dev-descriptor.bcdUSB) = 0x0201) {
+ retval = usb_get_bos_descriptor(usb_dev);
+ if (!retval)
+ usb_dev-lpm_capable = usb_device_supports_lpm(usb_dev);
+ }
+
This should be integrated with the code immediately above it, which
also calls usb_get_bos_descriptor(). In fact, maybe you should simply
change the existing code to check bcdUSB = 0x0201 instead of speed ==
USB_SPEED_SUPER.
I agree with you and will change it in v2 patch.
retval = usb_new_device (usb_dev);
if (retval) {
dev_err (parent_dev, can't register root hub for %s, %d\n,
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3b71516..884d702 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -122,7 +122,7 @@ struct usb_hub *usb_hub_to_struct_hub(struct usb_device
*hdev)
return usb_get_intfdata(hdev-actconfig-interface[0]);
}
-static int usb_device_supports_lpm(struct usb_device *udev)
+int usb_device_supports_lpm(struct usb_device *udev)
{
/* USB 2.1 (and greater) devices indicate LPM support through
* their USB 2.0 Extended Capabilities BOS descriptor.
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 7eb1e26..d5668c4 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -82,6 +82,7 @@ extern int usb_runtime_suspend(struct device *dev);
extern int usb_runtime_resume(struct device *dev);
extern int usb_runtime_idle(struct device *dev);
extern int usb_set_usb2_hardware_lpm(struct usb_device *udev, int enable);
+extern int usb_device_supports_lpm(struct usb_device *udev);
#else
This will break if you build it with CONFIG_PM disabled.
Yes. Will fix it in v2 patch.
Alan Stern
Thank you,
Baolu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 1/1] usb: core: lpm: set lpm_capable for root hub device
Commit 25cd2882e2fc (usb/xhci: Change how we indicate a host supports
Link PM.) removed the code to set lpm_capable for USB 3.0 super-speed
root hub. The intention of that change was to avoid touching usb core
internal field, a.k.a. lpm_capable, and let usb core to set it by
checking U1 and U2 exit latency values in the descriptor.
Usb core checks and sets lpm_capable in hub_port_init(). Unfortunately,
root hub is a special usb device as it has no parent. Hub_port_init()
will never be called for a root hub device. That means lpm_capable will
by no means be set for the root hub. As the result, lpm isn't functional
at all in Linux kernel.
This patch add the code to check and set lpm_capable when registering a
root hub device. It could be back-ported to kernels as old as v3.15,
that contains the Commit 25cd2882e2fc (usb/xhci: Change how we indicate
a host supports Link PM.).
Cc: sta...@vger.kernel.org # 3.15
Reported-by: Kevin Strasser kevin.stras...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Acked-by: Alan Stern st...@rowland.harvard.edu
---
drivers/usb/core/hcd.c | 7 +--
drivers/usb/core/hub.c | 2 +-
drivers/usb/core/usb.h | 1 +
3 files changed, 7 insertions(+), 3 deletions(-)
v1-v2 change log:
1. two code blocks merged
2. fix build error when CONFIG_PM is not set
v2-v3 change log:
1. further fix of build error when CONFIG_PM is not set
v3-v4 change log:
1. Add Alan's ACK
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..1c1385e 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1022,9 +1022,12 @@ static int register_root_hub(struct usb_hcd *hcd)
dev_name(usb_dev-dev), retval);
return (retval 0) ? retval : -EMSGSIZE;
}
- if (usb_dev-speed == USB_SPEED_SUPER) {
+
+ if (le16_to_cpu(usb_dev-descriptor.bcdUSB) = 0x0201) {
retval = usb_get_bos_descriptor(usb_dev);
- if (retval 0) {
+ if (!retval) {
+ usb_dev-lpm_capable = usb_device_supports_lpm(usb_dev);
+ } else if (usb_dev-speed == USB_SPEED_SUPER) {
mutex_unlock(usb_bus_list_lock);
dev_dbg(parent_dev, can't read %s bos descriptor %d\n,
dev_name(usb_dev-dev), retval);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3b71516..884d702 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -122,7 +122,7 @@ struct usb_hub *usb_hub_to_struct_hub(struct usb_device
*hdev)
return usb_get_intfdata(hdev-actconfig-interface[0]);
}
-static int usb_device_supports_lpm(struct usb_device *udev)
+int usb_device_supports_lpm(struct usb_device *udev)
{
/* USB 2.1 (and greater) devices indicate LPM support through
* their USB 2.0 Extended Capabilities BOS descriptor.
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 7eb1e26..457255a 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -65,6 +65,7 @@ extern int usb_hub_init(void);
extern void usb_hub_cleanup(void);
extern int usb_major_init(void);
extern void usb_major_cleanup(void);
+extern int usb_device_supports_lpm(struct usb_device *udev);
#ifdef CONFIG_PM
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] usb: core: lpm: set lpm_capable for root hub device
Commit 25cd2882e2fc (usb/xhci: Change how we indicate a host supports
Link PM.) removed the code to set lpm_capable for USB 3.0 super-speed
root hub. The intention of that change was to avoid touching usb core
internal field, a.k.a. lpm_capable, and let usb core to set it by
checking U1 and U2 exit latency values in the descriptor.
Usb core checks and sets lpm_capable in hub_port_init(). Unfortunately,
root hub is a special usb device as it has no parent. Hub_port_init()
will never be called for a root hub device. That means lpm_capable will
by no means be set for the root hub. As the result, lpm isn't functional
at all in Linux kernel.
This patch add the code to check and set lpm_capable when registering a
root hub device. It could be back-ported to kernels as old as v3.15,
that contains the Commit 25cd2882e2fc (usb/xhci: Change how we indicate
a host supports Link PM.).
Cc: sta...@vger.kernel.org # 3.15
Reported-by: Kevin Strasser kevin.stras...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hcd.c | 6 ++
drivers/usb/core/hub.c | 2 +-
drivers/usb/core/usb.h | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..48b208d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1032,6 +1032,12 @@ static int register_root_hub(struct usb_hcd *hcd)
}
}
+ if (le16_to_cpu(usb_dev-descriptor.bcdUSB) = 0x0201) {
+ retval = usb_get_bos_descriptor(usb_dev);
+ if (!retval)
+ usb_dev-lpm_capable = usb_device_supports_lpm(usb_dev);
+ }
+
retval = usb_new_device (usb_dev);
if (retval) {
dev_err (parent_dev, can't register root hub for %s, %d\n,
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3b71516..884d702 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -122,7 +122,7 @@ struct usb_hub *usb_hub_to_struct_hub(struct usb_device
*hdev)
return usb_get_intfdata(hdev-actconfig-interface[0]);
}
-static int usb_device_supports_lpm(struct usb_device *udev)
+int usb_device_supports_lpm(struct usb_device *udev)
{
/* USB 2.1 (and greater) devices indicate LPM support through
* their USB 2.0 Extended Capabilities BOS descriptor.
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 7eb1e26..d5668c4 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -82,6 +82,7 @@ extern int usb_runtime_suspend(struct device *dev);
extern int usb_runtime_resume(struct device *dev);
extern int usb_runtime_idle(struct device *dev);
extern int usb_set_usb2_hardware_lpm(struct usb_device *udev, int enable);
+extern int usb_device_supports_lpm(struct usb_device *udev);
#else
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/1] usb: core: lpm: set lpm_capable for root hub device
Commit 25cd2882e2fc (usb/xhci: Change how we indicate a host supports
Link PM.) removed the code to set lpm_capable for USB 3.0 super-speed
root hub. The intention of that change was to avoid touching usb core
internal field, a.k.a. lpm_capable, and let usb core to set it by
checking U1 and U2 exit latency values in the descriptor.
Usb core checks and sets lpm_capable in hub_port_init(). Unfortunately,
root hub is a special usb device as it has no parent. Hub_port_init()
will never be called for a root hub device. That means lpm_capable will
by no means be set for the root hub. As the result, lpm isn't functional
at all in Linux kernel.
This patch add the code to check and set lpm_capable when registering a
root hub device. It could be back-ported to kernels as old as v3.15,
that contains the Commit 25cd2882e2fc (usb/xhci: Change how we indicate
a host supports Link PM.).
Cc: sta...@vger.kernel.org # 3.15
Reported-by: Kevin Strasser kevin.stras...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/core/hcd.c | 7 +--
drivers/usb/core/hub.c | 2 +-
drivers/usb/core/usb.h | 1 +
3 files changed, 7 insertions(+), 3 deletions(-)
v1-v2 change log:
1. two code blocks merged
2. fix build error when CONFIG_PM is not set
v2-v3 change log:
1. further fix of build error when CONFIG_PM is not set
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 45a915c..1c1385e 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1022,9 +1022,12 @@ static int register_root_hub(struct usb_hcd *hcd)
dev_name(usb_dev-dev), retval);
return (retval 0) ? retval : -EMSGSIZE;
}
- if (usb_dev-speed == USB_SPEED_SUPER) {
+
+ if (le16_to_cpu(usb_dev-descriptor.bcdUSB) = 0x0201) {
retval = usb_get_bos_descriptor(usb_dev);
- if (retval 0) {
+ if (!retval) {
+ usb_dev-lpm_capable = usb_device_supports_lpm(usb_dev);
+ } else if (usb_dev-speed == USB_SPEED_SUPER) {
mutex_unlock(usb_bus_list_lock);
dev_dbg(parent_dev, can't read %s bos descriptor %d\n,
dev_name(usb_dev-dev), retval);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3b71516..884d702 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -122,7 +122,7 @@ struct usb_hub *usb_hub_to_struct_hub(struct usb_device
*hdev)
return usb_get_intfdata(hdev-actconfig-interface[0]);
}
-static int usb_device_supports_lpm(struct usb_device *udev)
+int usb_device_supports_lpm(struct usb_device *udev)
{
/* USB 2.1 (and greater) devices indicate LPM support through
* their USB 2.0 Extended Capabilities BOS descriptor.
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 7eb1e26..457255a 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -65,6 +65,7 @@ extern int usb_hub_init(void);
extern void usb_hub_cleanup(void);
extern int usb_major_init(void);
extern void usb_major_cleanup(void);
+extern int usb_device_supports_lpm(struct usb_device *udev);
#ifdef CONFIG_PM
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] usb: core: lpm: set lpm_capable for root hub device
On 06/13/2015 11:00 PM, Alan Stern wrote:
On Sat, 13 Jun 2015, Lu Baolu wrote:
Commit 25cd2882e2fc (usb/xhci: Change how we indicate a host supports
Link PM.) removed the code to set lpm_capable for USB 3.0 super-speed
root hub. The intention of that change was to avoid touching usb core
internal field, a.k.a. lpm_capable, and let usb core to set it by
checking U1 and U2 exit latency values in the descriptor.
Usb core checks and sets lpm_capable in hub_port_init(). Unfortunately,
root hub is a special usb device as it has no parent. Hub_port_init()
will never be called for a root hub device. That means lpm_capable will
by no means be set for the root hub. As the result, lpm isn't functional
at all in Linux kernel.
This patch add the code to check and set lpm_capable when registering a
root hub device. It could be back-ported to kernels as old as v3.15,
that contains the Commit 25cd2882e2fc (usb/xhci: Change how we indicate
a host supports Link PM.).
Cc: sta...@vger.kernel.org # 3.15
Reported-by: Kevin Strasser kevin.stras...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -82,6 +82,7 @@ extern int usb_runtime_suspend(struct device *dev);
extern int usb_runtime_resume(struct device *dev);
extern int usb_runtime_idle(struct device *dev);
extern int usb_set_usb2_hardware_lpm(struct usb_device *udev, int enable);
+extern int usb_device_supports_lpm(struct usb_device *udev);
#else
@@ -106,6 +107,11 @@ static inline int usb_set_usb2_hardware_lpm(struct usb_device *udev, int enable)
return 0;
}
+static inline int usb_device_supports_lpm(struct usb_device *udev)
+{
+ return 0;
+}
+
#endif
extern struct bus_type usb_bus_type;
In fact, usb_device_supports_lpm() is compiled even when CONFIG_PM
isn't set. Maybe this should be changed. But if you don't want to
change it now, you need to put the declaration outside the #ifdef
CONFIG_PM region. As it is, your patch is still broken (did you try
building it with CONFIG_PM unset?).
I am sorry for this silly mistake. I will move it out of CONFIG_PM region.
I tried building with CONFIG_PM unset. But it turns out that CONFIG_PM
was auto selected by some other items. I will test my next version of
patch with CONFIG_PM *really* unset before sending it out.
Thanks,
Baolu
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
Phy drivers and the ulpi interface providers depend on the
registration of the ulpi bus. Ulpi registers the bus in
module_init(). This could cause unnecessary bus users'
probe delays. i.e. unnecessary -EPROBE_DEFER happening on
ulpi_drivers in case they're registered before ulpi bus
itself.
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Acked-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
drivers/usb/common/ulpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
v0-v5 change log:
Various changes in the commit description.
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..01c0c04 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -242,7 +242,7 @@ static int __init ulpi_init(void)
{
return bus_register(ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
static void __exit ulpi_exit(void)
{
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On 05/26/2015 10:50 PM, Felipe Balbi wrote: Hi, On Mon, May 25, 2015 at 02:24:00PM +0800, Lu, Baolu wrote: On 05/23/2015 12:08 AM, David Cohen wrote: Hi, On Fri, May 22, 2015 at 07:29:15PM +0800, Lu Baolu wrote: Phy drivers and the ulpi interface providers depend on the registeration of the ulpi bus. Ulpi registers the bus in module_init(). This could result in a load order issue, i.e. It's still not an issue :( I'd say unnecessary probe delays. I managed to boot a kernel built from the top of Felipe's remotes/origin/next branch under an Ubuntu environment on Intel's Bay Trail tablet. The same panic (as I found in the Android environment previously) shows up as well. And if I replace module_init() with sys_initcall(), the panic disappears. the problem is something else... Moving things around in the init levels is just a workaround for another issue. Seems like there's some missing EPROBE_DEFER somewhere. Yes, I agree. Heikki, I assume missing EPROBE_DEFER issue will be fixed by Sasha's patch. I will resend the patch with a new commit message. Thanks, Baolu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
Phy drivers and the ulpi interface providers depend on the
registration of the ulpi bus. Ulpi registers the bus in
module_init(). This could cause unnecessary probe delays.
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Acked-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
drivers/usb/common/ulpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..01c0c04 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -242,7 +242,7 @@ static int __init ulpi_init(void)
{
return bus_register(ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
static void __exit ulpi_exit(void)
{
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On 05/28/2015 12:53 AM, David Cohen wrote: Hi, On Tue, May 26, 2015 at 07:37:02PM -0700, Greg Kroah-Hartman wrote: On Wed, May 27, 2015 at 09:45:37AM +0800, Lu Baolu wrote: Phy drivers and the ulpi interface providers depend on the registration of the ulpi bus. Ulpi registers the bus in module_init(). This could cause unnecessary probe delays. What do you mean by probe delays? I believe he meant the bus users' probe delays. i.e. unnecessary -EPROBE_DEFER happening on ulpi_drivers in case they're registered before ulpi bus itself. Is that correct, Baolu? Yes. Br, David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On 05/22/2015 11:11 AM, David Cohen wrote: On Thu, May 21, 2015 at 08:09:54PM -0700, David Cohen wrote: Hi, On Fri, May 22, 2015 at 10:07:05AM +0800, Lu Baolu wrote: Many drivers and modules depend on ULPI bus registeration to register ULPI interfaces and drivers. It's more appropriate to register ULPI bus in subsys_initcall instead of module_init. Kernel panic has been reported with some kind of kernel config. Even though I agree subsys_initcall is better to register ulpi bus, it's still no excuse to have kernel panic. What about ULPI bus being compiled as module? No kernel panic if ULPI is built as a module. IMHO this is avoiding the proper kernel panic fix which should be failing gracefully (or defer probe) from tusb1210 driver. Maybe I need to express myself better :) IMHO we should not consider work done with this patch only. It's still incomplete. I am with you on that we should know the real problem. I could go ahead with further debugging. Do you have any suggestions about which direction should I go? Br, David Thank you, -Baolu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On 05/22/2015 02:46 PM, Lu, Baolu wrote: On 05/22/2015 11:11 AM, David Cohen wrote: On Thu, May 21, 2015 at 08:09:54PM -0700, David Cohen wrote: Hi, On Fri, May 22, 2015 at 10:07:05AM +0800, Lu Baolu wrote: Many drivers and modules depend on ULPI bus registeration to register ULPI interfaces and drivers. It's more appropriate to register ULPI bus in subsys_initcall instead of module_init. Kernel panic has been reported with some kind of kernel config. Even though I agree subsys_initcall is better to register ulpi bus, it's still no excuse to have kernel panic. What about ULPI bus being compiled as module? No kernel panic if ULPI is built as a module. IMHO this is avoiding the proper kernel panic fix which should be failing gracefully (or defer probe) from tusb1210 driver. Maybe I need to express myself better :) IMHO we should not consider work done with this patch only. It's still incomplete. I am with you on that we should know the real problem. I could go ahead with further debugging. Do you have any suggestions about which direction should I go? I forgot to mention that the panic was found in an Android environment. The kernel version is v4.1-rc3. Br, David Thank you, -Baolu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On 05/23/2015 12:08 AM, David Cohen wrote:
Hi,
On Fri, May 22, 2015 at 07:29:15PM +0800, Lu Baolu wrote:
Phy drivers and the ulpi interface providers depend on the
registeration of the ulpi bus. Ulpi registers the bus in
module_init(). This could result in a load order issue, i.e.
It's still not an issue :(
I'd say unnecessary probe delays.
I managed to boot a kernel built from the top of Felipe's
remotes/origin/next branch under an Ubuntu environment
on Intel's Bay Trail tablet.
The same panic (as I found in the Android environment previously)
shows up as well. And if I replace module_init() with sys_initcall(),
the panic disappears.
Thanks,
-Baolu
But of cource it's Felipe's call :) Description looks better now.
BR, David
ulpi phy drivers or the ulpi interface providers loading
before the bus registeration.
This patch fixes this load order issue by putting ulpi_init
in subsys_initcall().
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/common/ulpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..01c0c04 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -242,7 +242,7 @@ static int __init ulpi_init(void)
{
return bus_register(ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
static void __exit ulpi_exit(void)
{
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist
Hi Heikki, Sasha and others,
Please ignore this patch. I should not squash these two patches into one and
sign it off behalf of other people. I apologize for this and I'm sorry
if this lets
you feel affronted.
Thanks,
Baolu
On 05/21/2015 04:57 PM, Lu Baolu wrote:
ULPI registers its bus at module_init, so if the bus fails to register, the
module will fail to load and all will be well in the world.
However, if the ULPI code is built-in rather than a module, the bus
initialization may fail, but we'd still try to register drivers later onto
a non-existent bus, which will panic the kernel.
Fix that by checking that the bus was indeed initialized before trying to
register drivers on top of it.
Signed-off-by: Sasha Levin sasha.le...@oracle.com
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Reviewed-by: David Cohen david.a.co...@linux.intel.com
---
drivers/usb/common/ulpi.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..af52b46 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv)
if (!drv-probe)
return -EINVAL;
+ /* Was the bus registered successfully? */
+ if (unlikely(WARN_ON(!ulpi_bus.p)))
+ return -ENODEV;
+
drv-driver.bus = ulpi_bus;
return driver_register(drv-driver);
@@ -242,7 +246,7 @@ static int __init ulpi_init(void)
{
return bus_register(ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
static void __exit ulpi_exit(void)
{
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
Many drivers and modules depend on ULPI bus registeration to
register ULPI interfaces and drivers. It's more appropriate
to register ULPI bus in subsys_initcall instead of module_init.
Kernel panic has been reported with some kind of kernel config.
[0.746856] kernel BUG at drivers/base/driver.c:153!
[0.752418] invalid opcode: [#1] PREEMPT SMP
[0.757804] Modules linked in:
[0.893985] Call Trace:
[0.896729] [81870cb1] ? ulpi_register_driver+0x21/0x30
[0.903654] [823d5cac] tusb1210_driver_init+0x10/0x12
[0.910386] [81000318] do_one_initcall+0xd8/0x200
[0.916729] [8239b094] kernel_init_freeable+0x196/0x21e
[0.923655] [81affed0] ? rest_init+0x90/0x90
[0.929509] [81affede] kernel_init+0xe/0xf0
[0.935266] [81b0f442] ret_from_fork+0x42/0x70
[0.941315] [81affed0] ? rest_init+0x90/0x90
This patch fixes this kind of kernel panic by putting ulpi_init in
subsys_initcall().
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/common/ulpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..01c0c04 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -242,7 +242,7 @@ static int __init ulpi_init(void)
{
return bus_register(ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
static void __exit ulpi_exit(void)
{
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
Phy drivers and the ulpi interface providers depend on the
registeration of the ulpi bus. Ulpi registers the bus in
module_init(). This could result in a load order issue, i.e.
ulpi phy drivers or the ulpi interface providers loading
before the bus registeration.
This patch fixes this load order issue by putting ulpi_init
in subsys_initcall().
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
---
drivers/usb/common/ulpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..01c0c04 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -242,7 +242,7 @@ static int __init ulpi_init(void)
{
return bus_register(ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
static void __exit ulpi_exit(void)
{
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] usb: xhci: fix checking ep busy for CFC
On 10/29/2015 08:51 PM, Sergei Shtylyov wrote:
Hello.
On 10/29/2015 5:46 AM, Lu Baolu wrote:
Function ep_ring_is_processing() checks the dequeue pointer
in endpoint context to know whether an endpoint is busy with
processing TRBs. This is not correct since dequeue pointer
field in an endpoint context is only valid when the endpoint
is in Halted or Stopped states. This buggy code causes audio
noise when playing sound with USB headset connected to host
controllers which support CFC (one of xhci 1.1 features).
This patch should exist in stable kernel since v4.3.
Reported-and-tested-by: YD Tseng <yd_ts...@asmedia.com.tw>
Signed-off-by: Lu Baolu <baolu...@linux.intel.com>
---
v1->v2:
Implement the logic in xhci_queue_isoc_tx_prepare() instead of
a seperated function as suggested by Mathias.
---
drivers/usb/host/xhci-ring.c | 32 ++--
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index fa83625..8edc286 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
[...]
@@ -3983,10 +3961,12 @@ int xhci_queue_isoc_tx_prepare(struct
xhci_hcd *xhci, gfp_t mem_flags,
}
/* Calculate the start frame and put it in urb->start_frame. */
-if (HCC_CFC(xhci->hcc_params) &&
-ep_ring_is_processing(xhci, slot_id, ep_index)) {
-urb->start_frame = xep->next_frame_id;
-goto skip_start_over;
+if (HCC_CFC(xhci->hcc_params)) {
+if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK)
+== EP_STATE_RUNNING &&
+!list_empty(_ring->td_list))
+urb->start_frame = xep->next_frame_id;
+goto skip_start_over;
Forgot {}?
Oh, I am sorry. I am wondering how it passed my test.
I will send v3 patch soon any way.
}
start_frame = readl(>run_regs->microframe_index);
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/1] usb: xhci: fix checking ep busy for CFC
Function ep_ring_is_processing() checks the dequeue pointer
in endpoint context to know whether an endpoint is busy with
processing TRBs. This is not correct since dequeue pointer
field in an endpoint context is only valid when the endpoint
is in Halted or Stopped states. This buggy code causes audio
noise when playing sound with USB headset connected to host
controllers which support CFC (one of xhci 1.1 features).
This patch should exist in stable kernel since v4.3.
Reported-and-tested-by: YD Tseng <yd_ts...@asmedia.com.tw>
Signed-off-by: Lu Baolu <baolu...@linux.intel.com>
---
v1->v2:
Implement the logic in xhci_queue_isoc_tx_prepare() instead of
a seperated function as suggested by Mathias.
v2->v3:
Add the missing {} pair found by Sergei.
Adjust line for code readability as suggested by Mathias.
---
drivers/usb/host/xhci-ring.c | 32 ++--
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index fa83625..6c5e813 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3896,28 +3896,6 @@ cleanup:
return ret;
}
-static int ep_ring_is_processing(struct xhci_hcd *xhci,
- int slot_id, unsigned int ep_index)
-{
- struct xhci_virt_device *xdev;
- struct xhci_ring *ep_ring;
- struct xhci_ep_ctx *ep_ctx;
- struct xhci_virt_ep *xep;
- dma_addr_t hw_deq;
-
- xdev = xhci->devs[slot_id];
- xep = >devs[slot_id]->eps[ep_index];
- ep_ring = xep->ring;
- ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index);
-
- if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK) != EP_STATE_RUNNING)
- return 0;
-
- hw_deq = le64_to_cpu(ep_ctx->deq) & ~EP_CTX_CYCLE_MASK;
- return (hw_deq !=
- xhci_trb_virt_to_dma(ep_ring->enq_seg, ep_ring->enqueue));
-}
-
/*
* Check transfer ring to guarantee there is enough room for the urb.
* Update ISO URB start_frame and interval.
@@ -3983,10 +3961,12 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci,
gfp_t mem_flags,
}
/* Calculate the start frame and put it in urb->start_frame. */
- if (HCC_CFC(xhci->hcc_params) &&
- ep_ring_is_processing(xhci, slot_id, ep_index)) {
- urb->start_frame = xep->next_frame_id;
- goto skip_start_over;
+ if (HCC_CFC(xhci->hcc_params) && !list_empty(_ring->td_list)) {
+ if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK) ==
+ EP_STATE_RUNNING) {
+ urb->start_frame = xep->next_frame_id;
+ goto skip_start_over;
+ }
}
start_frame = readl(>run_regs->microframe_index);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] usb: xhci: fix checking ep busy for CFC
On 10/29/2015 10:08 PM, Mathias Nyman wrote:
On 29.10.2015 14:58, Lu, Baolu wrote:
On 10/29/2015 08:51 PM, Sergei Shtylyov wrote:
Hello.
On 10/29/2015 5:46 AM, Lu Baolu wrote:
Function ep_ring_is_processing() checks the dequeue pointer
in endpoint context to know whether an endpoint is busy with
processing TRBs. This is not correct since dequeue pointer
field in an endpoint context is only valid when the endpoint
is in Halted or Stopped states. This buggy code causes audio
noise when playing sound with USB headset connected to host
controllers which support CFC (one of xhci 1.1 features).
This patch should exist in stable kernel since v4.3.
Reported-and-tested-by: YD Tseng <yd_ts...@asmedia.com.tw>
Signed-off-by: Lu Baolu <baolu...@linux.intel.com>
---
v1->v2:
Implement the logic in xhci_queue_isoc_tx_prepare() instead of
a seperated function as suggested by Mathias.
---
drivers/usb/host/xhci-ring.c | 32 ++--
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c
b/drivers/usb/host/xhci-ring.c
index fa83625..8edc286 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
[...]
@@ -3983,10 +3961,12 @@ int xhci_queue_isoc_tx_prepare(struct
xhci_hcd *xhci, gfp_t mem_flags,
}
/* Calculate the start frame and put it in urb->start_frame. */
-if (HCC_CFC(xhci->hcc_params) &&
-ep_ring_is_processing(xhci, slot_id, ep_index)) {
-urb->start_frame = xep->next_frame_id;
-goto skip_start_over;
+if (HCC_CFC(xhci->hcc_params)) {
+if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK)
+== EP_STATE_RUNNING &&
+!list_empty(_ring->td_list))
+urb->start_frame = xep->next_frame_id;
+goto skip_start_over;
Forgot {}?
Oh, I am sorry. I am wondering how it passed my test.
I will send v3 patch soon any way.
If you are anyway making a v3 then maybe one more change,
just for readability, no (real) functional change:
if (HCC_CFC(xhci->hcc_params) && !list_empty(_ring->td_list)) {
if (le32_to_cpu(ep_...
Done.
While thinking about code cleanup I also think we should use a local
variable
u32 ep_info = le32_to_cpu(ep_ctx->ep_info) as it's used several times
in xhci_queue_isoc_tx_preapare(),
causing a lot of line splitting.
It should be ok as we are under the same spinlock so ep_ctx should not
change.
But that is not a fix sent to a rc and stable, I can make a separate
cleanup patch for it later.
-Mathias
--
To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] usb: xhci: fix checking ep busy for CFC
Function ep_ring_is_processing() checks the dequeue pointer in endpoint context to know whether an endpoint is busy with processing TRBs. This is not correct since dequeue pointer field in an endpoint context is only valid when the endpoint is in Halted or Stopped states. This buggy code causes audio noise when playing sound with USB headset connected to host controllers which support CFC (one of xhci 1.1 features). This patch should exist in stable kernel since v4.3. Reported-and-tested-by: YD Tseng <yd_ts...@asmedia.com.tw> Signed-off-by: Lu Baolu <baolu...@linux.intel.com> --- drivers/usb/host/xhci-ring.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index fa83625..2b50d45 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -3903,7 +3903,6 @@ static int ep_ring_is_processing(struct xhci_hcd *xhci, struct xhci_ring *ep_ring; struct xhci_ep_ctx *ep_ctx; struct xhci_virt_ep *xep; - dma_addr_t hw_deq; xdev = xhci->devs[slot_id]; xep = >devs[slot_id]->eps[ep_index]; @@ -3913,9 +3912,7 @@ static int ep_ring_is_processing(struct xhci_hcd *xhci, if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK) != EP_STATE_RUNNING) return 0; - hw_deq = le64_to_cpu(ep_ctx->deq) & ~EP_CTX_CYCLE_MASK; - return (hw_deq != - xhci_trb_virt_to_dma(ep_ring->enq_seg, ep_ring->enqueue)); + return !list_empty(_ring->td_list); } /* -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 12/12] usb: doc: add document for xHCI DbC driver
Add Documentation/usb/xhci-dbc.txt. This document includes development status and usage guide for USB3 debug port. Signed-off-by: Lu Baolu <baolu...@linux.intel.com> --- Documentation/usb/xhci-dbc.txt | 325 + MAINTAINERS| 1 + drivers/usb/early/xhci-dbc.c | 3 + 3 files changed, 329 insertions(+) create mode 100644 Documentation/usb/xhci-dbc.txt diff --git a/Documentation/usb/xhci-dbc.txt b/Documentation/usb/xhci-dbc.txt new file mode 100644 index 000..94b75a9 --- /dev/null +++ b/Documentation/usb/xhci-dbc.txt @@ -0,0 +1,325 @@ +xHCI debug capability driver + + Lu Baolu <baolu...@linux.intel.com> + +Last-updated: September 2015 + + + Contents: + - + * What is xHCI DbC? + * Debug topologies + * Debug stacks + * Port Multiplexing + * Hardware initialization + * External reset + * Port reset + * Interrupt/DMA/Memory during early boot + * Endpoint STALL + * Debug device information + * How to use DbC early printk? + * Limitations + + What is xHCI DbC? + - + +The xHCI Debugging Capability defined in section 7.6 of xHCI spec 1.1 +provides an optional functionality that enables low-level system debug +over USB. It provides a means of connecting two systems where one system +is a Debug Host and the other a Debug Target (System Under Test). The +Debug Capability provides an interface that is completely independent +of the xHCI interface. A Debug Target enumerates as a USB debug device +to the Debug Host, allowing a Debug Host to access a Debug Target through +the standard USB software stack. + + Debug topologies + + +Multiple Debug Targets may be attached to a single Debug Host. Debug +Targets may be connected to any downstream facing port below a Debug +Host (i.e. anywhere in the fabric, root port or external hub puts). +A Debug Target may only connect to a Debug Host through a Root Hub port +of the target. That means connection of a Debug Target to a Debug Host +through the ports of an external hub is not supported. + +Below is a typical connection between Debug Host and Debug target. Two +Debug targets are connected to a single Debug host. + + + +| Debug Host | | Debug Target | +|| || +|xHC without DbC | | xHC with DbC | +|or DbC disabled | | enabled| +|| || +|P1| |p2| |P1| |p2| +|__| |__| |__| |__| + || | + ||_| + |_ +| + ___| +| HUB | | Debug Target | +|| || +| Superspeed hub | | xHC with DbC | +|| | enabled| +|| || +|P1| |p2| |P1| |p2| +|__| |__| |__| |__| + | | + |_| + + Debug stacks + + +Below is a software stack diagram of both Debug Host and Debug Target. + + +| Debug Host | | Debug Target | +|| || +| debug App| || +|| | system debug | +| usb_debug| | hooks | +|| || +|usbcore | || +|| |debug capability| +|xhci_hcd| | driver | +|| || +|xHC without DbC | | xHC with DbC | +|or DbC disabled | | enabled| +|| || +|P1| |p2| |P1| |p2| +|__| |__| |__| |__| + | | + |_| + + + Port Multiplexing + - + +A debug port is always multiplexed with the first xHCI root hub port. +Whenever debug capability is supported and enabled, and the first root +hub port is detected to be connected to a downstream super-speed port +of a Debug Host,
[PATCH 03/12] usb: xhci: dbc: probe and setup xhci debug capability
xHCI debug capability (DbC) is an optional functionality provided
by an xHCI host controller. Software learns this capability by
walking through the extended capability list in mmio of the host.
This patch introduces the code to probe and initialize the debug
capability hardware during early boot. With hardware initialization
done, the debug target (system under debug which has DbC enabled)
will present a debug device through the debug port. The debug device
is fully compliant with the USB framework and provides the equivalent
of a very high performance (USB3) full-duplex serial link between the
debug host and target.
Signed-off-by: Lu Baolu <baolu...@linux.intel.com>
---
MAINTAINERS | 7 +
arch/x86/Kconfig.debug | 12 +
drivers/usb/early/Makefile | 1 +
drivers/usb/early/xhci-dbc.c | 787 +++
include/linux/usb/xhci-dbc.h | 187 ++
5 files changed, 994 insertions(+)
create mode 100644 drivers/usb/early/xhci-dbc.c
create mode 100644 include/linux/usb/xhci-dbc.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 0425167..585a369 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11040,6 +11040,13 @@ S: Supported
F: drivers/usb/host/xhci*
F: drivers/usb/host/pci-quirks*
+USB XHCI DEBUG PORT
+M: Lu Baolu <baolu...@linux.intel.com>
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/usb/early/xhci-dbc.c
+F: include/linux/usb/xhci-dbc.h
+
USB ZD1201 DRIVER
L: linux-wirel...@vger.kernel.org
W: http://linux-lc100020.sourceforge.net
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index d8c0d32..8d95abd 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -65,6 +65,18 @@ config EARLY_PRINTK_EFI
This is useful for kernel debugging when your machine crashes very
early before the console code is initialized.
+config EARLY_PRINTK_XDBC
+ bool "Early printk via xHCI debug port"
+ depends on EARLY_PRINTK && PCI
+ ---help---
+ Write kernel log output directly into the xHCI debug port.
+
+ This is useful for kernel debugging when your machine crashes very
+ early before the console code is initialized. For normal operation
+ it is not recommended because it looks ugly and doesn't cooperate
+ with klogd/syslogd or the X server. You should normally N here,
+ unless you want to debug such a crash.
+
config X86_PTDUMP
bool "Export kernel pagetable layout to userspace via debugfs"
depends on DEBUG_KERNEL
diff --git a/drivers/usb/early/Makefile b/drivers/usb/early/Makefile
index 24bbe51..2db5906 100644
--- a/drivers/usb/early/Makefile
+++ b/drivers/usb/early/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_EARLY_PRINTK_DBGP) += ehci-dbgp.o
+obj-$(CONFIG_EARLY_PRINTK_XDBC) += xhci-dbc.o
diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
new file mode 100644
index 000..22a1de9
--- /dev/null
+++ b/drivers/usb/early/xhci-dbc.c
@@ -0,0 +1,787 @@
+/**
+ * xhci-dbc.c - xHCI debug capability driver
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu...@linux.intel.com>
+ * Some code shared with EHCI debug port and xHCI driver.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "../host/xhci.h"
+
+#defineXDBC_PROTOCOL 1 /* GNU Remote Debug Command Set
*/
+#defineXDBC_VENDOR_ID 0x1d6b /* Linux Foundation 0x1d6b */
+#defineXDBC_PRODUCT_ID 0x0004 /* __le16 idProduct; device
0004 */
+#defineXDBC_DEVICE_REV 0x0010 /* 0.10 */
+
+static struct xdbc_state xdbc_stat;
+static struct xdbc_state *xdbcp = _stat;
+
+#ifdef DBC_DEBUG
+/* place holder */
+#definexdbc_trace printk
+static void xdbc_dbg_dump_regs(char *str)
+{
+ if (!xdbcp->xdbc_reg) {
+ xdbc_trace("register not mapped\n");
+ return;
+ }
+
+ xdbc_trace("XDBC registers: %s\n", str);
+ xdbc_trace(" Capability: %08x\n",
+ readl(>xdbc_reg->capability));
+ xdbc_trace(" Door bell: %08x\n",
+ readl(>xdbc_reg->doorbell));
+ xdbc_trace(" Event Ring Segment Table Size: %08x\n",
+ readl(>xdbc_reg->ersts));
+ xdbc_trace(" Event Ring Segment Table Base Address: %16llx\n",
+ xdbc_read64(>xdbc_reg->erstba));
+ xdbc_trace(" Event Ring Dequeue Pointer: %16llx\n",
+ xdbc_read64(>xdbc_reg->erdp));
+ xdbc
[PATCH 00/12] usb: early: add support for early printk through USB3 debug port
This patch series adds support for early printk through USB3 debug port. USB3 debug port is described in xHCI specification as an optional extended capability. The first patch adds a file in debugfs, through which users can check whether the debug capability is supported by a specific host controller. Patch 2 to 10 add the driver for xHCI debug capability. It interfaces with the register set and provides the required ops (read/write/control) to upper layers. Early printk is one consumer of these ops. The hooks for early printk are introduced in patch 9. This design is similar to what we have done in drivers/usb/early/ehci-dbgp.c. Patch 11 is a minor change to usb_debug module. This change is required to bind usb_debug with the USB3 debug device. Patch 12 is the design document and user guide. Lu Baolu (12): usb: xhci: expose xhci extended capabilities via debugfs x86: fixmap: add permanent fixmap for xhci debug port usb: xhci: dbc: probe and setup xhci debug capability usb: xhci: dbc: add support for Intel xHCI dbc quirk usb: xhci: dbc: add debug buffer usb: xhci: dbc: add bulk out and bulk in interfaces usb: xhci: dbc: handle dbc-configured exit usb: xhci: dbc: handle endpoint stall x86: early_printk: add USB3 debug port earlyprintk support usb: xhci: dbc: add handshake between debug target and host usb: serial: usb_debug: add support for dbc debug device usb: doc: add document for xHCI DbC driver Documentation/kernel-parameters.txt |1 + Documentation/usb/xhci-dbc.txt | 325 MAINTAINERS |8 + arch/x86/Kconfig.debug | 12 + arch/x86/include/asm/fixmap.h |4 + arch/x86/kernel/early_printk.c |5 + drivers/usb/early/Makefile |1 + drivers/usb/early/xhci-dbc.c| 1407 +++ drivers/usb/host/xhci-dbg.c | 212 ++ drivers/usb/host/xhci-ext-caps.h|9 +- drivers/usb/host/xhci.c | 27 +- drivers/usb/host/xhci.h | 10 + drivers/usb/serial/usb_debug.c | 29 +- include/linux/usb/xhci-dbc.h| 224 ++ 14 files changed, 2269 insertions(+), 5 deletions(-) create mode 100644 Documentation/usb/xhci-dbc.txt create mode 100644 drivers/usb/early/xhci-dbc.c create mode 100644 include/linux/usb/xhci-dbc.h -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
