On Sat, 17 Nov 2012, Lan Tianyu wrote:
> This patch is to add usb port auto power off mechanism.
> When usb device is suspending, usb core will suspend usb port and
> usb port runtime pm callback will clear PORT_POWER feature to
> power off port if all conditions were met. These conditions are
> remote wakeup disable, pm qos NO_POWER_OFF flag clear and persist
> enable. When device is suspended and power off, usb core
> will ignore port's connect and enable change event to keep the device
> not being disconnected. When it resumes, power on port again.
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2861,6 +2869,19 @@ int usb_port_suspend(struct usb_device *udev,
> pm_message_t msg)
> udev->port_is_suspended = 1;
> msleep(10);
> }
> +
> + /*
> + * Check whether current status is meeting requirement of
> + * usb port power off mechanism
> + */
> + if (!udev->do_remote_wakeup
> + && (dev_pm_qos_flags(&port_dev->dev,
> + PM_QOS_FLAG_NO_POWER_OFF) != PM_QOS_FLAGS_ALL)
> + && udev->persist_enabled
> + && !status)
> + if (!pm_runtime_put_sync(&port_dev->dev))
> + port_dev->power_on = false;
You shouldn't need to change port_dev->power_on here.
> @@ -2981,10 +3021,36 @@ 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 = hdev_to_hub(udev->parent);
> + struct usb_port *port_dev = hub->ports[udev->portnum - 1];
> int port1 = udev->portnum;
> int status;
> u16 portchange, portstatus;
>
> +
> + set_bit(port1, hub->busy_bits);
> +
> + if (!port_dev->power_on) {
> + status = pm_runtime_get_sync(&port_dev->dev);
> + if (status < 0) {
> + dev_dbg(&udev->dev, "can't resume usb port, status
> %d\n",
> + status);
> + return status;
You must not return without clearing hub->busy_bits. Same thing
applies later on.
> + }
> +
> + port_dev->power_on = true;
This isn't necessary.
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -72,6 +72,9 @@ static int usb_port_runtime_resume(struct device *dev)
> struct usb_device *hdev =
> to_usb_device(dev->parent->parent);
>
> + if (port_dev->power_on)
> + return 0;
> +
Move these lines into usb_hub_set_port_power, and have that routine set
port_dev->power_on when the Set-Feature request succeeds.
> return usb_hub_set_port_power(hdev, port_dev->portnum,
> true);
> }
> @@ -81,13 +84,21 @@ static int usb_port_runtime_suspend(struct device *dev)
> struct usb_port *port_dev = to_usb_port(dev);
> struct usb_device *hdev =
> to_usb_device(dev->parent->parent);
> + int retval;
>
> if (dev_pm_qos_flags(&port_dev->dev, PM_QOS_FLAG_NO_POWER_OFF)
> == PM_QOS_FLAGS_ALL)
> return -EAGAIN;
>
> - return usb_hub_set_port_power(hdev, port_dev->portnum,
> + if (!port_dev->power_on)
> + return 0;
> +
> + retval = usb_hub_set_port_power(hdev, port_dev->portnum,
> false);
> + if (!retval)
> + port_dev->power_on = false;
> +
Move all this port_dev->power_on stuff into usb_hub_set_port_power.
Then no changes will be needed here.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
