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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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.lu@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.

thanks,

greg k-h