On Fri, 28 Feb 2014, Dan Williams wrote: > A hub indicates whether it supports per-port power control via the > wHubCharacteristics field in its descriptor. If it is not supported > a hub will still emulate ClearPortPower(PORT_POWER) requests by > stopping the link state machine. However, since this does not save > power do not bother suspending. > > This also consolidates support checks into a > hub_is_port_power_switchable() helper. > > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> This patch looks pretty good. I would change only a comment: > --- a/drivers/usb/core/port.c > +++ b/drivers/usb/core/port.c > @@ -171,12 +171,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1) > > pm_runtime_set_active(&port_dev->dev); > > - /* It would be dangerous if user space couldn't > - * prevent usb device from being powered off. So don't > - * enable port runtime pm if failed to expose port's pm qos. > + /* It would be dangerous if user space couldn't prevent usb The comment style should be fixed: /* * It would... > + * device from being powered off. So don't enable port runtime > + * pm if failed to expose port's pm qos, or if the hub does not > + * support power switching "if failed" lacks a subject. And the final sentence lacks a period. > */ > - if (!dev_pm_qos_expose_flags(&port_dev->dev, > - PM_QOS_FLAG_NO_POWER_OFF)) > + if (hub_is_port_power_switchable(hub) > + && dev_pm_qos_expose_flags(&port_dev->dev, > + PM_QOS_FLAG_NO_POWER_OFF) == 0) > pm_runtime_enable(&port_dev->dev); The order of the tests described in the comment should match the order of the tests in the code. Aside from these things, Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> Alan Stern -- 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
