On Fri, 4 Jan 2013, 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 it resumes, power on port again. Add did_runtime_put
> in the struct usb_port in order to call pm_runtime_get/put(portdev)
> paired during suspending and resuming.

> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c

> @@ -718,6 +719,9 @@ int usb_hub_set_port_power(struct usb_device *hdev, int 
> port1,
>               bool set)
>  {
>       int ret;
> +     struct usb_hub *hub = hdev_to_hub(hdev);
> +     struct usb_port *port_dev
> +             = hub->ports[port1 - 1];

Unnecessary line break.

> @@ -2862,6 +2880,20 @@ 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) {
> +             pm_runtime_put_sync(&port_dev->dev);
> +             port_dev->did_runtime_put = true;

You forgot to check port_dev->did_runtime_put in usb_disconnect.  
Unless you check it, you could end up calling pm_runtime_put a second 
time.

> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c

> @@ -84,13 +109,22 @@ 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);
> +     struct usb_hub *hub = hdev_to_hub(hdev);
> +     int port1 = port_dev->portnum;
> +     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,
> -                     false);
> +     set_bit(port1, hub->busy_bits);
> +     retval = usb_hub_set_port_power(hdev, port1, false);
> +     clear_port_feature(hdev, port1,
> +                     USB_PORT_FEAT_C_CONNECTION);
> +     clear_port_feature(hdev, port1,
> +                     USB_PORT_FEAT_C_ENABLE);
> +     clear_bit(port1, hub->busy_bits);
> +     return retval;
>  }

Several unnecessary line breaks.

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

Reply via email to