On Thu, 2014-05-08 at 14:11 -0400, Alan Stern wrote:
> On Thu, 8 May 2014, Dan Williams wrote:
>
> > > Also, instead of adding another #ifdef here, you could add a #else
> > > section to the existing #ifdef in which you define an inline version of
> > > hub_handle_remote_wakeup() (or a macro version) that always returns 0.
> >
> > I originally started down that path, and then noticed:
> >
> > #ifdef CONFIG_PM_RUNTIME
> > } else if (udev->state == USB_STATE_SUSPENDED &&
> > udev->persist_enabled) {
> > /* For a suspended device, treat this as a
> > * remote wakeup event.
> > */
> > usb_unlock_port(port_dev);
> > status = usb_remote_wakeup(udev);
> > usb_lock_port(port_dev);
> > #endif
> >
> > ...right above and made the wrong call to add more ugliness. Will
> > fix, but will leave the other ifdef excursions alone for now.
>
> Yeah, I never tried very hard to encapsulate that bit away from the
> rest of the subroutine.
>
Updated patch:
8<--------------
Subject: usb: hub_handle_remote_wakeup() depends on CONFIG_PM_RUNTIME=y
From: Dan Williams <[email protected]>
Per Alan:
"You mean from within hub_handle_remote_wakeup()? That routine will
never get called if CONFIG_PM_RUNTIME isn't enabled, because khubd
never sees wakeup requests if they arise during system suspend.
In fact, that routine ought to go inside the "#ifdef CONFIG_PM_RUNTIME"
portion of hub.c, along with the other suspend/resume code."
Suggested-by: Alan Stern <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
drivers/usb/core/hub.c | 90 ++++++++++++++++++++++++++----------------------
drivers/usb/core/usb.h | 5 ---
2 files changed, 49 insertions(+), 46 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d91e6493abf7..b9aff5c6f855 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3347,6 +3347,55 @@ int usb_remote_wakeup(struct usb_device *udev)
return status;
}
+/* Returns 1 if there was a remote wakeup and a connect status change. */
+static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
+ u16 portstatus, u16 portchange)
+ __must_hold(&port_dev->status_lock)
+{
+ struct usb_port *port_dev = hub->ports[port - 1];
+ struct usb_device *hdev;
+ struct usb_device *udev;
+ int connect_change = 0;
+ int ret;
+
+ hdev = hub->hdev;
+ udev = port_dev->child;
+ if (!hub_is_superspeed(hdev)) {
+ if (!(portchange & USB_PORT_STAT_C_SUSPEND))
+ return 0;
+ usb_clear_port_feature(hdev, port, USB_PORT_FEAT_C_SUSPEND);
+ } else {
+ if (!udev || udev->state != USB_STATE_SUSPENDED ||
+ (portstatus & USB_PORT_STAT_LINK_STATE) !=
+ USB_SS_PORT_LS_U0)
+ return 0;
+ }
+
+ if (udev) {
+ /* TRSMRCY = 10 msec */
+ msleep(10);
+
+ usb_unlock_port(port_dev);
+ ret = usb_remote_wakeup(udev);
+ usb_lock_port(port_dev);
+ if (ret < 0)
+ connect_change = 1;
+ } else {
+ ret = -ENODEV;
+ hub_port_disable(hub, port, 1);
+ }
+ dev_dbg(&port_dev->dev, "resume, status %d\n", ret);
+ return connect_change;
+}
+
+#else
+
+static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
+ u16 portstatus, u16 portchange)
+{
+ return 0;
+}
+
#endif
static int check_ports_changed(struct usb_hub *hub)
@@ -4700,47 +4749,6 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
usb_lock_port(port_dev);
}
-/* Returns 1 if there was a remote wakeup and a connect status change. */
-static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
- u16 portstatus, u16 portchange)
- __must_hold(&port_dev->status_lock)
-{
- struct usb_port *port_dev = hub->ports[port - 1];
- struct usb_device *hdev;
- struct usb_device *udev;
- int connect_change = 0;
- int ret;
-
- hdev = hub->hdev;
- udev = port_dev->child;
- if (!hub_is_superspeed(hdev)) {
- if (!(portchange & USB_PORT_STAT_C_SUSPEND))
- return 0;
- usb_clear_port_feature(hdev, port, USB_PORT_FEAT_C_SUSPEND);
- } else {
- if (!udev || udev->state != USB_STATE_SUSPENDED ||
- (portstatus & USB_PORT_STAT_LINK_STATE) !=
- USB_SS_PORT_LS_U0)
- return 0;
- }
-
- if (udev) {
- /* TRSMRCY = 10 msec */
- msleep(10);
-
- usb_unlock_port(port_dev);
- ret = usb_remote_wakeup(udev);
- usb_lock_port(port_dev);
- if (ret < 0)
- connect_change = 1;
- } else {
- ret = -ENODEV;
- hub_port_disable(hub, port, 1);
- }
- dev_dbg(&port_dev->dev, "resume, status %d\n", ret);
- return connect_change;
-}
-
static void port_event(struct usb_hub *hub, int port1)
__must_hold(&port_dev->status_lock)
{
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 98dc08e13448..d9d08720c386 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -107,11 +107,6 @@ static inline int usb_autoresume_device(struct usb_device
*udev)
return 0;
}
-static inline int usb_remote_wakeup(struct usb_device *udev)
-{
- return 0;
-}
-
static inline int usb_set_usb2_hardware_lpm(struct usb_device *udev, int
enable)
{
return 0;
--
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
