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