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 <[email protected]> > CC: Sarah Sharp <[email protected]>
Looks fine, thanks for catching this! Acked-by: Sarah Sharp <[email protected]> > > --- > > > [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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
