[PATCH v2 07/14] USB: make usb_ports proper parents of their child usb_devices

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

 



There are a number of places where pm_runtime manipulations are open
coded due to the fact that a usb_port is a device-model nephew of it's
"child" usb_device.  These can be cleaned up by making a usb_port the
device_model parent of its child.  With that relationship we rely on the
normal pm runtime rules that a parent device cannot be suspended until
all its children are suspended, and that attempts to resume children
will wake the parent first.

With this change we still want hubs to suspend when ports are inactive.
Previously ports would be forced to remain active when some power off
constraint was not met (like remote wakeup).  Now we let them suspend
when idle, but optionally keep the power enabled (return 0 from
usb_port_runtime_suspend).

In order to preserve the userspace abi, a link from the hub to the child
device is provided.

Note, the usb_device.parent remains the hub device.

Before:
# readlink -f /sys/devices/pci0000:00/0000:00:14.0/usb2/2-1
              /sys/devices/pci0000:00/0000:00:14.0/usb2/2-1
              +-------+
              |  hub  |
              +---^---+
                  |
            +-----+----+
            |          |
        +---+---+  +---+---+
        | intf  |  |device |
        +---^---+  +-------+
            |
        +---+---+
        | port  |
        +-------+

After:
# readlink -f /sys/devices/pci0000:00/0000:00:14.0/usb2/2-1
              /sys/devices/pci0000:00/0000:00:14.0/usb2/2-0:1.0/port1/2-1

              +-------+
              |  hub  |
              +---^---+
                  |
              +---+---+
              | intf  |
              +---^---+
                  |
              +---+---+
              | port  |
              +---^---+
                  |
              +---+---+
              |device |
              +-------+

Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
 drivers/usb/core/hub.c  |   60 +++++++----------------------------------------
 drivers/usb/core/hub.h  |    2 --
 drivers/usb/core/port.c |    9 ++++---
 drivers/usb/core/usb.c  |    9 +++++--
 4 files changed, 21 insertions(+), 59 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 0e4c85380760..7ba9b654a6fc 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1706,7 +1706,6 @@ descriptor_error:
 
 	usb_set_intfdata (intf, hub);
 	intf->needs_remote_wakeup = 1;
-	pm_suspend_ignore_children(&intf->dev, true);
 
 	if (hdev->speed == USB_SPEED_HIGH)
 		highspeed_hubs++;
@@ -2030,16 +2029,9 @@ void usb_disconnect(struct usb_device **pdev)
 	usb_hcd_synchronize_unlinks(udev);
 
 	if (udev->parent) {
-		struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
-		struct usb_port	*port_dev = hub->ports[udev->portnum - 1];
+		struct usb_device *hdev = udev->parent;
 
-		sysfs_remove_link(&udev->dev.kobj, "port");
-		sysfs_remove_link(&port_dev->dev.kobj, "device");
-
-		if (!port_dev->did_runtime_put)
-			pm_runtime_put(&port_dev->dev);
-		else
-			port_dev->did_runtime_put = false;
+		sysfs_remove_link(&hdev->dev.kobj, dev_name(&udev->dev));
 	}
 
 	usb_remove_ep_devs(&udev->ep0);
@@ -2355,22 +2347,12 @@ int usb_new_device(struct usb_device *udev)
 
 	/* Create link files between child device and usb port device. */
 	if (udev->parent) {
-		struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
-		struct usb_port	*port_dev = hub->ports[udev->portnum - 1];
+		struct usb_device *hdev = udev->parent;
 
-		err = sysfs_create_link(&udev->dev.kobj,
-				&port_dev->dev.kobj, "port");
+		err = sysfs_create_link(&hdev->dev.kobj, &udev->dev.kobj,
+					dev_name(&udev->dev));
 		if (err)
 			goto fail;
-
-		err = sysfs_create_link(&port_dev->dev.kobj,
-				&udev->dev.kobj, "device");
-		if (err) {
-			sysfs_remove_link(&udev->dev.kobj, "port");
-			goto fail;
-		}
-
-		pm_runtime_get_sync(&port_dev->dev);
 	}
 
 	(void) usb_create_ep_devs(&udev->dev, &udev->ep0, udev);
@@ -2959,7 +2941,6 @@ static unsigned wakeup_enabled_descendants(struct usb_device *udev)
 int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 {
 	struct usb_hub	*hub = usb_hub_to_struct_hub(udev->parent);
-	struct usb_port *port_dev = hub->ports[udev->portnum - 1];
 	int		port1 = udev->portnum;
 	int		status;
 	bool		really_suspend = true;
@@ -3053,11 +3034,6 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 		usb_set_device_state(udev, USB_STATE_SUSPENDED);
 	}
 
-	if (status == 0) {
-		pm_runtime_put_sync(&port_dev->dev);
-		port_dev->did_runtime_put = true;
-	}
-
 	usb_mark_last_busy(hub->hdev);
 	return status;
 }
@@ -3183,21 +3159,10 @@ static int finish_port_resume(struct usb_device *udev)
 int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 {
 	struct usb_hub	*hub = usb_hub_to_struct_hub(udev->parent);
-	struct usb_port *port_dev = hub->ports[udev->portnum  - 1];
 	int		port1 = udev->portnum;
 	int		status;
 	u16		portchange, portstatus;
 
-	if (port_dev->did_runtime_put) {
-		status = pm_runtime_get_sync(&port_dev->dev);
-		port_dev->did_runtime_put = false;
-		if (status < 0) {
-			dev_dbg(&udev->dev, "can't resume usb port, status %d\n",
-					status);
-			return status;
-		}
-	}
-
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
 	if (status == 0 && !port_is_suspended(hub, portstatus))
@@ -3310,23 +3275,16 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg)
 	unsigned		port1;
 	int			status;
 
-	/*
-	 * Warn if children aren't already suspended.
-	 * Also, add up the number of wakeup-enabled descendants.
-	 */
+	/* add up the number of wakeup-enabled descendants. */
 	hub->wakeup_enabled_descendants = 0;
 	for (port1 = 1; port1 <= hdev->maxchild; port1++) {
-		struct usb_device	*udev;
+		struct usb_device *udev;
 
-		udev = hub->ports[port1 - 1]->child;
-		if (udev && udev->can_submit) {
-			dev_warn(&intf->dev, "port %d nyet suspended\n", port1);
-			if (PMSG_IS_AUTO(msg))
-				return -EBUSY;
-		}
+		udev = usb_port_get_child(hub->ports[port1 - 1]);
 		if (udev)
 			hub->wakeup_enabled_descendants +=
 					wakeup_enabled_descendants(udev);
+		usb_port_put_child(udev);
 	}
 
 	if (hdev->do_remote_wakeup && hub->quirk_check_port_auto_suspend) {
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 4e34365e41e3..c54501967179 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -84,7 +84,6 @@ struct usb_hub {
  * @port_owner: port's owner
  * @connect_type: port's connect type
  * @portnum: port index num based one
- * @did_runtime_put: port has done pm_runtime_put().
  */
 struct usb_port {
 	struct usb_device *child;
@@ -92,7 +91,6 @@ struct usb_port {
 	struct dev_state *port_owner;
 	enum usb_port_connect_type connect_type;
 	u8 portnum;
-	unsigned did_runtime_put:1;
 };
 
 #define to_usb_port(_dev) \
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 6257c1ec5e2d..6f20c0c8fe34 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -77,18 +77,19 @@ static int usb_port_runtime_resume(struct device *dev)
 	struct usb_interface *intf = to_usb_interface(dev->parent);
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 	int port1 = port_dev->portnum;
-	int retval;
+	int retval = 0;
 
 	if (!hub)
 		return -EINVAL;
 
 	usb_autopm_get_interface(intf);
-	retval = usb_set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
-	usb_autopm_put_interface(intf);
+	if (test_bit(port1, hub->poweroff_bits))
+		retval = usb_set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
 
 	/* no child? we're done recovering this port */
 	if (!port_dev->child)
 		usb_clear_port_poweroff(hub, port1);
+	usb_autopm_put_interface(intf);
 
 	return retval;
 }
@@ -125,7 +126,7 @@ static int usb_port_runtime_suspend(struct device *dev)
 		return -EINVAL;
 
 	if (power_on_reason(port_dev))
-		return -EAGAIN;
+		return 0;
 
 	usb_autopm_get_interface(intf);
 	usb_set_port_poweroff(hub, port1);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 4d1144990d4c..3991ed66fde9 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -42,7 +42,7 @@
 #include <linux/mm.h>
 #include <linux/dma-mapping.h>
 
-#include "usb.h"
+#include "hub.h"
 
 
 const char *usbcore_name = "usbcore";
@@ -458,6 +458,8 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 		dev_set_name(&dev->dev, "usb%d", bus->busnum);
 		root_hub = 1;
 	} else {
+		struct usb_hub *hub = usb_hub_to_struct_hub(parent);
+
 		/* match any labeling on the hubs; it's one-based */
 		if (parent->devpath[0] == '0') {
 			snprintf(dev->devpath, sizeof dev->devpath,
@@ -476,7 +478,10 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 					(15 << ((parent->level - 1)*4));
 		}
 
-		dev->dev.parent = &parent->dev;
+		/* usb hierarchy is hub->device device-model hierarchy is
+		 * hub->intf->port->device
+		 */
+		dev->dev.parent = &hub->ports[port1 - 1]->dev;
 		dev_set_name(&dev->dev, "%d-%s", bus->busnum, dev->devpath);
 
 		/* hub driver sets up TT records */

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