lkml.org 
[lkml]   [2015]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume
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.lu@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?

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

Alan Stern



\
 
 \ /
  Last update: 2015-05-06 17:01    [W:0.337 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site