On Wed, 2014-04-30 at 10:04 -0400, Alan Stern wrote:
> On Tue, 29 Apr 2014, Dan Williams wrote:
> 
> > > What happens if a thread tries to resume or suspend a port while the
> > > hub is being reset?  With nothing to prevent it, the request sent to
> > > the hub will fail and the port may end up in a runtime PM error state.
> > >
> > 
> > I'm expected to be protected by:
> > 
> >         /* Prevent autosuspend during the reset */
> >         usb_autoresume_device(udev);
> > 
> > ...in usb_reset_device() and usb_reset_and_verify_device()'s
> > requirement that the device in question not be suspended.  The hub
> > should pin it's parent port active during the reset.
> 
> My question wasn't clear enough.  What happens if, while hub H is being
> reset, a thread tries to resume a port on hub H (not on H's parent)?
> 

Ah, yes, I think we need something like the following.  I'll fold it
into the re-submit.

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 943366345beb..02b0833b0a67 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5183,6 +5183,31 @@ static int descriptors_changed(struct usb_device *udev,
        return changed;
 }
 
+static void usb_hub_ports_pm_get_sync(struct usb_device *hdev)
+{
+       struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+       int port1;
+
+       for (port1 = 1; port1 <= hdev->maxchild; port1++) {
+               struct usb_port *port_dev = hub->ports[port1 - 1];
+
+               pm_runtime_get_sync(&port_dev->dev);
+       }
+}
+
+static void usb_hub_ports_pm_put(struct usb_device *hdev)
+{
+       struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+       int port1;
+
+       for (port1 = 1; port1 <= hdev->maxchild; port1++) {
+               struct usb_port *port_dev = hub->ports[port1 - 1];
+
+               pm_runtime_put(&port_dev->dev);
+       }
+}
+
+
 /**
  * usb_reset_and_verify_device - perform a USB port reset to reinitialize a 
device
  * @udev: device to reset (not in SUSPENDED or NOTATTACHED state)
@@ -5239,6 +5264,9 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
 
        parent_hub = usb_hub_to_struct_hub(parent_hdev);
 
+       /* Disable usb_port power management */
+       usb_hub_ports_pm_get_sync(udev);
+
        /* Disable USB2 hardware LPM.
         * It will be re-enabled by the enumeration process.
         */
@@ -5358,9 +5386,11 @@ done:
        usb_enable_ltm(udev);
        usb_release_bos_descriptor(udev);
        udev->bos = bos;
+       usb_hub_ports_pm_put(udev);
        return 0;
 
 re_enumerate:
+       usb_hub_ports_pm_put(udev);
        /* LPM state doesn't matter when we're about to destroy the device. */
        hub_port_logical_disconnect(parent_hub, port1);
        usb_release_bos_descriptor(udev);


--
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