Skip to contentSkip to conversation

usb: notify hcd when USB device suspend or resume

PATCH In review Lu Baolu 3 participants 7 messages May 9, 2015 – Jun 26, 2015

Subsystems (2)
USB USB xHCI driver
lore
Share & tools
lore

Lu Baolu

author
#0 lore raw

v5 cover 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(-)
Signature · 1 line
2.1.0
author
#1 lore raw

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 <[email protected]>
Acked-by Alan Stern <[email protected]>
---
 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(-)
drivers/usb/core/hcd.c +27 -0
index 45a915c..007450d 100644
@@ -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 */
 
 /*-------------------------------------------------------------------------*/
drivers/usb/core/hub.c +5 -0
index 52178bc..28d76b7 100644
@@ -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)) {
include/linux/usb/hcd.h +7 -1
index 68b1e83..bc2eb1c 100644
@@ -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);
 extern int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg);
 extern void usb_hcd_resume_root_hub(struct usb_hcd *hcd);
+extern void hcd_suspend_notify(struct usb_device *udev);
+extern void hcd_resume_notify(struct usb_device *udev);
 #else
 static inline void usb_hcd_resume_root_hub(struct usb_hcd *hcd)
 {
Signature · 1 line
2.1.0
author
#2 lore raw

PATCH v5 2/3 usb: xhci: implement device_suspend/device_resume entries

In reply to Lu Baolu

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 <[email protected]>
---
 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(-)
drivers/usb/host/xhci-hub.c +1 -1
index 0827d7c..a83e82e 100644
@@ -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;
drivers/usb/host/xhci.c +38 -0
index ec8ac16..330961d 100644
@@ -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 *))
drivers/usb/host/xhci.h +9 -0
index 8e421b8..67c13c5 100644
@@ -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_suspend	NULL
+#define xhci_device_resume	NULL
+#endif /* CONFIG_PM */
+
 #endif /* __LINUX_XHCI_HCD_H */
Signature · 1 line
2.1.0
maintainer
#3 lore raw

Re: [PATCH v5 2/3] usb: xhci: implement device_suspend/device_resume entries

Quoted 8 lines · Lu Baolu

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.

Quoted 6 lines
Signed-off-by Lu Baolu <[email protected]>
---
 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(-)

I want to see these come through Mathias, after he has verified that they work properly. I'll take them that way.

thanks,

greg k-h

author
#4 lore raw

PATCH v5 3/3 usb: xhci: remove stop device and ring doorbell in hub control and bus suspend

In reply to Lu Baolu

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 <[email protected]>
---
 drivers/usb/host/xhci-hub.c | 47 ---------------------------------------------
 1 file changed, 47 deletions(-)
drivers/usb/host/xhci-hub.c +0 -47
index a83e82e..f12e1b7 100644
@@ -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 */
 			xhci_test_and_clear_bit(xhci, port_array, port_index,
 						PORT_PLC);
-
-			slot_id = xhci_find_slot_id_by_port(hcd,
-					xhci, port_index + 1);
-			if (slot_id)
-				xhci_ring_device(xhci, slot_id);
 		} else
 			writel(temp, port_array[port_index]);
 	}
Signature · 1 line
2.1.0
#5 lore raw

Re: [PATCH v5 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend

Quoted 3 lines · Lu Baolu

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

-Mathias

author
#6 lore raw

Re: [PATCH v5 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend

Quoted 8 lines · Mathias Nyman
Quoted 3 lines · Lu Baolu

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.

Quoted 8 lines

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

Quoted 10 lines

-Mathias

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html