USB: simplify hub_restart() logic

This patch (as1081) straightens out the logic of the hub_restart()
routine.  Each port of the hub is scanned and the driver makes sure
that ports which are supposed to be disabled really _are_ disabled.
Any ports with a significant change in status are flagged in
hub->change_bits, so that khubd can focus on them without the need to
scan all the ports a second time -- which means the hub->activating
flag is no longer needed.

Also, it is now recognized explicitly that the only reason for
resuming a port which was not suspended is to carry out a reset-resume
operation, which happens only in a non-CONFIG_USB_SUSPEND setting.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index d0b37d7..bf1585b 100644 (file)
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -805,8 +805,6 @@ static int usb_resume_device(struct usb_device *udev)
 
        if (udev->state == USB_STATE_NOTATTACHED)
                goto done;
-       if (udev->state != USB_STATE_SUSPENDED && !udev->reset_resume)
-               goto done;
 
        /* Can't resume it if it doesn't have a driver. */
        if (udev->dev.driver == NULL) {
@@ -1173,11 +1171,8 @@ static int usb_resume_both(struct usb_device *udev)
                         * then we're stuck. */
                        status = usb_resume_device(udev);
                }
-       } else {
-
-               /* Needed for reset-resume */
+       } else if (udev->reset_resume)
                status = usb_resume_device(udev);
-       }
 
        if (status == 0 && udev->actconfig) {
                for (i = 0; i < udev->actconfig->desc.bNumInterfaces; i++) {

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 976da1c..054a76d 100644 (file)
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -72,7 +72,6 @@ struct usb_hub {
 
        unsigned                limited_power:1;
        unsigned                quiescing:1;
-       unsigned                activating:1;
        unsigned                disconnected:1;
 
        unsigned                has_indicators:1;
@@ -539,7 +538,6 @@ static void hub_quiesce(struct usb_hub *hub)
 {
        /* (nonblocking) khubd and related activity won't re-trigger */
        hub->quiescing = 1;
-       hub->activating = 0;
 
        /* (blocking) stop khubd and related activity */
        usb_kill_urb(hub->urb);
@@ -554,7 +552,6 @@ static void hub_activate(struct usb_hub *hub)
        int     status;
 
        hub->quiescing = 0;
-       hub->activating = 1;
 
        status = usb_submit_urb(hub->urb, GFP_NOIO);
        if (status < 0)
@@ -638,81 +635,83 @@ static void hub_stop(struct usb_hub *hub)
        hub_quiesce(hub);
 }
 
-#define HUB_RESET              1
-#define HUB_RESUME             2
-#define HUB_RESET_RESUME       3
-
-#ifdef CONFIG_PM
+enum hub_activation_type {
+       HUB_INIT, HUB_POST_RESET, HUB_RESUME, HUB_RESET_RESUME
+};
 
-static void hub_restart(struct usb_hub *hub, int type)
+static void hub_restart(struct usb_hub *hub, enum hub_activation_type type)
 {
        struct usb_device *hdev = hub->hdev;
        int port1;
 
-       /* Check each of the children to see if they require
-        * USB-PERSIST handling or disconnection.  Also check
-        * each unoccupied port to make sure it is still disabled.
+       /* Check each port and set hub->change_bits to let khubd know
+        * which ports need attention.
         */
        for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
                struct usb_device *udev = hdev->children[port1-1];
-               int status = 0;
+               int status;
                u16 portstatus, portchange;
 
-               if (!udev || udev->state == USB_STATE_NOTATTACHED) {
-                       if (type != HUB_RESET) {
-                               status = hub_port_status(hub, port1,
-                                               &portstatus, &portchange);
-                               if (status == 0 && (portstatus &
-                                               USB_PORT_STAT_ENABLE))
-                                       clear_port_feature(hdev, port1,
-                                                       USB_PORT_FEAT_ENABLE);
-                       }
-                       continue;
+               portstatus = portchange = 0;
+               status = hub_port_status(hub, port1, &portstatus, &portchange);
+               if (udev || (portstatus & USB_PORT_STAT_CONNECTION))
+                       dev_dbg(hub->intfdev,
+                                       "port %d: status %04x change %04x\n",
+                                       port1, portstatus, portchange);
+
+               /* After anything other than HUB_RESUME (i.e., initialization
+                * or any sort of reset), every port should be disabled.
+                * Unconnected ports should likewise be disabled (paranoia),
+                * and so should ports for which we have no usb_device.
+                */
+               if ((portstatus & USB_PORT_STAT_ENABLE) && (
+                               type != HUB_RESUME ||
+                               !(portstatus & USB_PORT_STAT_CONNECTION) ||
+                               !udev ||
+                               udev->state == USB_STATE_NOTATTACHED)) {
+                       clear_port_feature(hdev, port1, USB_PORT_FEAT_ENABLE);
+                       portstatus &= ~USB_PORT_STAT_ENABLE;
                }
 
-               /* Was the power session lost while we were suspended? */
-               switch (type) {
-               case HUB_RESET_RESUME:
-                       portstatus = 0;
-                       portchange = USB_PORT_STAT_C_CONNECTION;
-                       break;
-
-               case HUB_RESET:
-               case HUB_RESUME:
-                       status = hub_port_status(hub, port1,
-                                       &portstatus, &portchange);
-                       break;
-               }
+               if (!udev || udev->state == USB_STATE_NOTATTACHED) {
+                       /* Tell khubd to disconnect the device or
+                        * check for a new connection
+                        */
+                       if (udev || (portstatus & USB_PORT_STAT_CONNECTION))
+                               set_bit(port1, hub->change_bits);
+
+               } else if (portstatus & USB_PORT_STAT_ENABLE) {
+                       /* The power session apparently survived the resume.
+                        * If there was an overcurrent or suspend change
+                        * (i.e., remote wakeup request), have khubd
+                        * take care of it.
+                        */
+                       if (portchange)
+                               set_bit(port1, hub->change_bits);
 
-               /* For "USB_PERSIST"-enabled children we must
-                * mark the child device for reset-resume and
-                * turn off the various status changes to prevent
-                * khubd from disconnecting it later.
-                */
-               if (udev->persist_enabled && status == 0 &&
-                               !(portstatus & USB_PORT_STAT_ENABLE)) {
+               } else if (udev->persist_enabled) {
+                       /* Turn off the status changes to prevent khubd
+                        * from disconnecting the device.
+                        */
                        if (portchange & USB_PORT_STAT_C_ENABLE)
                                clear_port_feature(hub->hdev, port1,
                                                USB_PORT_FEAT_C_ENABLE);
                        if (portchange & USB_PORT_STAT_C_CONNECTION)
                                clear_port_feature(hub->hdev, port1,
                                                USB_PORT_FEAT_C_CONNECTION);
+#ifdef CONFIG_PM
                        udev->reset_resume = 1;
+#endif
+               } else {
+                       /* The power session is gone; tell khubd */
+                       usb_set_device_state(udev, USB_STATE_NOTATTACHED);
+                       set_bit(port1, hub->change_bits);
                }
-
-               /* Otherwise for a reset_resume we must disconnect the child,
-                * but as we may not lock the child device here
-                * we have to do a "logical" disconnect.
-                */
-               else if (type == HUB_RESET_RESUME)
-                       hub_port_logical_disconnect(hub, port1);
        }
 
        hub_activate(hub);
 }
 
-#endif /* CONFIG_PM */
-
 /* caller has locked the hub device */
 static int hub_pre_reset(struct usb_interface *intf)
 {
@@ -728,7 +727,7 @@ static int hub_post_reset(struct usb_interface *intf)
        struct usb_hub *hub = usb_get_intfdata(intf);
 
        hub_power_on(hub);
-       hub_activate(hub);
+       hub_restart(hub, HUB_POST_RESET);
        return 0;
 }
 
@@ -2903,7 +2902,7 @@ static void hub_events(void)
                                continue;
                        connect_change = test_bit(i, hub->change_bits);
                        if (!test_and_clear_bit(i, hub->event_bits) &&
-                                       !connect_change && !hub->activating)
+                                       !connect_change)
                                continue;
 
                        ret = hub_port_status(hub, i,
@@ -2911,11 +2910,6 @@ static void hub_events(void)
                        if (ret < 0)
                                continue;
 
-                       if (hub->activating && !hdev->children[i-1] &&
-                                       (portstatus &
-                                               USB_PORT_STAT_CONNECTION))
-                               connect_change = 1;
-
                        if (portchange & USB_PORT_STAT_C_CONNECTION) {
                                clear_port_feature(hdev, i,
                                        USB_PORT_FEAT_C_CONNECTION);
@@ -3011,8 +3005,6 @@ static void hub_events(void)
                        }
                }
 
-               hub->activating = 0;
-
                /* If this is a root hub, tell the HCD it's okay to
                 * re-enable port-change interrupts now. */
                if (!hdev->parent && !hub->busy_bits[0])