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 <[email protected]>
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 <[email protected]>
Alan Stern
--
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
