On 2012年11月29日 03:37, Alan Stern wrote:
> 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.
Hi Alan:
Doing port_dev->power_on = true in usb_hub_set_port_power() just after
set PORT_POWER feature will cause device to be disconnected. If user set
PM Qos NO_POWER_OFF flag after the device was power off, the port would
be power on and do port_dev->power_on = true. But the port connect
change event couldn't be ignored and the device would be disconnected.
>
>> --- 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
>
>
--
Best regards
Tianyu Lan
--
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
