Re: [PATCH v9 16/19] usb: hub_handle_remote_wakeup() depends on CONFIG_PM_RUNTIME=y

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <dan.j.williams@xxxxxxxxx>

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 <stern@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux