On Thu, 1 May 2014, Dan Williams wrote: > > @@ -122,6 +127,11 @@ static int usb_port_runtime_suspend(stru > > == PM_QOS_FLAGS_ALL) > > return -EAGAIN; > > > > + if (hub->in_reset) { > > + port_dev->power_is_on = 0; > > + return 0; > > + } > > Why lie? Just return -EBUSY and be done. Good idea; the complete, updated patch is below. (It's against 3.15-rc2, without any of your RFC changes.) > In fact that also has the > nice side effect of clearing runtime errors it seems. Not really, because if there was an earlier runtime PM error then dev->power.runtime_error would be set, so rpm_check_suspend_allowed() would fail and the suspend callback wouldn't get called in the first place. Alan Stern 8<---------------------------------------------------------------->8 The USB core doesn't properly handle mutual exclusion between resetting a hub and changing the power states of the hub's ports. We need to avoid sending port-power requests to the hub while it is being reset, because such requests cannot succeed. This patch fixes the problem by keeping track of when a reset is in progress. At such times, attempts to suspend (power-off) a port will fail immediately with -EBUSY, and calls to usb_port_runtime_resume() will update the power_is_on flag and return immediately. When the reset is complete, hub_activate() will automatically restore each port to the proper power state. Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> --- [as1745] drivers/usb/core/hub.c | 12 ++++++++++++ drivers/usb/core/hub.h | 1 + drivers/usb/core/port.c | 6 ++++++ 3 files changed, 19 insertions(+) Index: usb-3.15/drivers/usb/core/hub.h =================================================================== --- usb-3.15.orig/drivers/usb/core/hub.h +++ usb-3.15/drivers/usb/core/hub.h @@ -66,6 +66,7 @@ struct usb_hub { unsigned limited_power:1; unsigned quiescing:1; unsigned disconnected:1; + unsigned in_reset:1; unsigned quirk_check_port_auto_suspend:1; Index: usb-3.15/drivers/usb/core/hub.c =================================================================== --- usb-3.15.orig/drivers/usb/core/hub.c +++ usb-3.15/drivers/usb/core/hub.c @@ -1276,12 +1276,22 @@ static void hub_quiesce(struct usb_hub * flush_work(&hub->tt.clear_work); } +static void hub_pm_barrier_for_all_ports(struct usb_hub *hub) +{ + int i; + + for (i = 0; i < hub->hdev->maxchild; ++i) + pm_runtime_barrier(&hub->ports[i]->dev); +} + /* caller has locked the hub device */ static int hub_pre_reset(struct usb_interface *intf) { struct usb_hub *hub = usb_get_intfdata(intf); hub_quiesce(hub, HUB_PRE_RESET); + hub->in_reset = 1; + hub_pm_barrier_for_all_ports(hub); return 0; } @@ -1290,6 +1300,8 @@ static int hub_post_reset(struct usb_int { struct usb_hub *hub = usb_get_intfdata(intf); + hub->in_reset = 0; + hub_pm_barrier_for_all_ports(hub); hub_activate(hub, HUB_POST_RESET); return 0; } Index: usb-3.15/drivers/usb/core/port.c =================================================================== --- usb-3.15.orig/drivers/usb/core/port.c +++ usb-3.15/drivers/usb/core/port.c @@ -81,6 +81,10 @@ static int usb_port_runtime_resume(struc if (!hub) return -EINVAL; + if (hub->in_reset) { + port_dev->power_is_on = 1; + return 0; + } usb_autopm_get_interface(intf); set_bit(port1, hub->busy_bits); @@ -117,6 +121,8 @@ static int usb_port_runtime_suspend(stru if (!hub) return -EINVAL; + if (hub->in_reset) + return -EBUSY; if (dev_pm_qos_flags(&port_dev->dev, PM_QOS_FLAG_NO_POWER_OFF) == PM_QOS_FLAGS_ALL) -- 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