usb: notify hcd when USB device suspend or resume

PATCH Superseded Lu Baolu 2 participants 8 messages May 7, 2015 – May 8, 2015

Subsystems (2)

USB USB xHCI driver

lore

Share & tools

lore

Lu Baolu

author

2015-05-07 02:18 UTC #0 lore raw

v3 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.

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(-)

Signature · 1 line

2.1.0

Lu Baolu

author

2015-05-07 02:18 UTC #1 lore raw

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.

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.

Signed-off-by Lu Baolu <[email protected]>

---
 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(-)

drivers/usb/core/hcd.c +29 -0

index 45a915c..a4cfc2d 100644
@@ -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 */
 
 /*-------------------------------------------------------------------------*/

drivers/usb/core/hub.c +5 -0

index 3b71516..b3f0dc8 100644
@@ -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)) {

include/linux/usb/hcd.h +9 -1

index 68b1e83..621d9b7 100644
@@ -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);
 };
 
 static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd)

@@ -632,6 +638,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, pm_message_t msg);
+extern void hcd_resume_notify(struct usb_device *udev, pm_message_t msg);
 #else
 static inline void usb_hcd_resume_root_hub(struct usb_hcd *hcd)
 {

Signature · 1 line

2.1.0

Lu Baolu

author

2015-05-07 02:18 UTC #2 lore raw

PATCH v3 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     | 40 ++++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci.h     | 11 +++++++++++
 3 files changed, 52 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 +40 -0

index ec8ac16..6b61833 100644
@@ -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 *))

drivers/usb/host/xhci.h +11 -0

index 8e421b8..d826439 100644
@@ -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_suspend	NULL
+#define xhci_device_resume	NULL
+#endif /* CONFIG_PM */
+
 #endif /* __LINUX_XHCI_HCD_H */

Signature · 1 line

2.1.0

Greg Kroah-Hartman

maintainer

2015-05-07 20:47 UTC #3 lore raw

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

Quoted 34 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.
Signed-off-by Lu Baolu <[email protected]>
---
 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(-)
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 +0 -0
index ec8ac16..6b61833 100644
@@ -4680,6 +4680,40 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
 		return ret;

drivers/usb/host/xhci.c int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd, · line 4690

Quoted 10 lines

drivers/usb/host/xhci.c +8 -0

 	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.

Quoted 7 lines

drivers/usb/host/xhci.c +7 -0

+{
+	struct xhci_hcd	*xhci;
+
+	xhci = hcd_to_xhci(hcd);
+	if (!xhci || !xhci->devs[udev->slot_id])
+		return;
+

drivers/usb/host/xhci.c int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd, · line 4707

Quoted 10 lines

drivers/usb/host/xhci.c +10 -0

+	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.

Lu, Baolu

author

2015-05-08 01:23 UTC #4 lore raw

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

Quoted 38 lines · Greg Kroah-Hartman

Quoted 37 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.
Signed-off-by Lu Baolu <[email protected]>
---
  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(-)
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 +1 -0
index ec8ac16..6b61833 100644
@@ -4680,6 +4680,40 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
  		return ret;
  	return 0;
  }
+

Quoted 8 lines

drivers/usb/host/xhci.c int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd, · line 4690
Quoted 7 lines
drivers/usb/host/xhci.c +7 -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.

Quoted 11 lines

drivers/usb/host/xhci.c +10 -0

+{
+	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);
+}
+

Quoted 8 lines

drivers/usb/host/xhci.c int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd, · line 4707

Quoted 7 lines

drivers/usb/host/xhci.c +7 -0

+/*
+ * 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

Quoted 8 lines

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

Greg Kroah-Hartman

maintainer

2015-05-08 07:28 UTC #5 lore raw

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

Quoted 50 lines · Baolu

Quoted 38 lines · Greg Kroah-Hartman
Quoted 37 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.
Signed-off-by Lu Baolu <[email protected]>
---
 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(-)
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 +1 -0
index ec8ac16..6b61833 100644
@@ -4680,6 +4680,40 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
 		return ret;
 	return 0;
 }
+
Quoted 8 lines
drivers/usb/host/xhci.c int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd, · line 4690
Quoted 7 lines
drivers/usb/host/xhci.c +7 -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.

Quoted 4 lines

'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.

thanks,

greg k-h

Lu, Baolu

author

2015-05-08 07:46 UTC #6 lore raw

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

Quoted 49 lines · Greg Kroah-Hartman

Quoted 48 lines · Baolu
Quoted 46 lines · Greg Kroah-Hartman
Quoted 34 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.
Signed-off-by Lu Baolu <[email protected]>
---
  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(-)
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 +0 -0
index ec8ac16..6b61833 100644
@@ -4680,6 +4680,40 @@ int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd,
  		return ret;
drivers/usb/host/xhci.c int xhci_disable_usb3_lpm_timeout(struct usb_hcd *hcd, · line 4690
Quoted 10 lines
drivers/usb/host/xhci.c +8 -0
  	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.

Quoted 7 lines

Quoted 4 lines
'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.

Quoted 4 lines

thanks,
greg k-h

Thank you.
Baolu

Quoted 6 lines

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

Lu Baolu

author

2015-05-07 02:18 UTC #7 lore raw

PATCH v3 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