On Tue, Jul 30, 2013 at 03:39:02PM -0400, Alan Stern wrote:
> The hub driver's usb_port_suspend() routine doesn't handle errors
> related to Link Power Management properly. It always returns failure,
> it doesn't try to clean up the wakeup setting, (in the case of system
> sleep) it doesn't try to go ahead with the port suspend regardless,
> and it doesn't try to apply the new power-off mechanism.
>
> This patch fixes these problems.
>
> Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> CC: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
Looks fine, thanks for catching this!
Acked-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
>
> ---
>
>
> [as1696]
>
> drivers/usb/core/hub.c | 48 ++++++++++++++++++++++++++----------------------
> 1 file changed, 26 insertions(+), 22 deletions(-)
>
> Index: usb-3.11/drivers/usb/core/hub.c
> ===================================================================
> --- usb-3.11.orig/drivers/usb/core/hub.c
> +++ usb-3.11/drivers/usb/core/hub.c
> @@ -2948,7 +2948,6 @@ int usb_port_suspend(struct usb_device *
> {
> struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
> struct usb_port *port_dev = hub->ports[udev->portnum - 1];
> - enum pm_qos_flags_status pm_qos_stat;
> int port1 = udev->portnum;
> int status;
> bool really_suspend = true;
> @@ -2966,7 +2965,7 @@ int usb_port_suspend(struct usb_device *
> status);
> /* bail if autosuspend is requested */
> if (PMSG_IS_AUTO(msg))
> - return status;
> + goto err_wakeup;
> }
> }
>
> @@ -2975,14 +2974,16 @@ int usb_port_suspend(struct usb_device *
> usb_set_usb2_hardware_lpm(udev, 0);
>
> if (usb_disable_ltm(udev)) {
> - dev_err(&udev->dev, "%s Failed to disable LTM before suspend\n.",
> - __func__);
> - return -ENOMEM;
> + dev_err(&udev->dev, "Failed to disable LTM before suspend\n.");
> + status = -ENOMEM;
> + if (PMSG_IS_AUTO(msg))
> + goto err_ltm;
> }
> if (usb_unlocked_disable_lpm(udev)) {
> - dev_err(&udev->dev, "%s Failed to disable LPM before suspend\n.",
> - __func__);
> - return -ENOMEM;
> + dev_err(&udev->dev, "Failed to disable LPM before suspend\n.");
> + status = -ENOMEM;
> + if (PMSG_IS_AUTO(msg))
> + goto err_lpm3;
> }
>
> /* see 7.1.7.6 */
> @@ -3010,17 +3011,19 @@ int usb_port_suspend(struct usb_device *
> if (status) {
> dev_dbg(hub->intfdev, "can't suspend port %d, status %d\n",
> port1, status);
> - /* paranoia: "should not happen" */
> - if (udev->do_remote_wakeup)
> - (void) usb_disable_remote_wakeup(udev);
>
> + /* Try to enable USB3 LPM and LTM again */
> + usb_unlocked_enable_lpm(udev);
> + err_lpm3:
> + usb_enable_ltm(udev);
> + err_ltm:
> /* Try to enable USB2 hardware LPM again */
> if (udev->usb2_hw_lpm_capable == 1)
> usb_set_usb2_hardware_lpm(udev, 1);
>
> - /* Try to enable USB3 LTM and LPM again */
> - usb_enable_ltm(udev);
> - usb_unlocked_enable_lpm(udev);
> + if (udev->do_remote_wakeup)
> + (void) usb_disable_remote_wakeup(udev);
> + err_wakeup:
>
> /* System sleep transitions should never fail */
> if (!PMSG_IS_AUTO(msg))
> @@ -3042,14 +3045,15 @@ int usb_port_suspend(struct usb_device *
> * Check whether current status meets the requirement of
> * usb port power off mechanism
> */
> - pm_qos_stat = dev_pm_qos_flags(&port_dev->dev,
> - PM_QOS_FLAG_NO_POWER_OFF);
> - if (!udev->do_remote_wakeup
> - && pm_qos_stat != PM_QOS_FLAGS_ALL
> - && udev->persist_enabled
> - && !status) {
> - pm_runtime_put_sync(&port_dev->dev);
> - port_dev->did_runtime_put = true;
> + if (status == 0 && !udev->do_remote_wakeup && udev->persist_enabled) {
> + enum pm_qos_flags_status pm_qos_stat;
> +
> + pm_qos_stat = dev_pm_qos_flags(&port_dev->dev,
> + PM_QOS_FLAG_NO_POWER_OFF);
> + if (pm_qos_stat != PM_QOS_FLAGS_ALL) {
> + pm_runtime_put_sync(&port_dev->dev);
> + port_dev->did_runtime_put = true;
> + }
> }
>
> usb_mark_last_busy(hub->hdev);
>
--
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
