usb: hub: rename hub_events() to hub_event() and handle only one event there

We would like to convert khubd kthread to a workqueue. As a result hub_events()
will handle only one event per call.

In fact, we could do this already now because there is another cycle in
hub_thread(). It calls hub_events() until hub_event_list is empty.

This patch renames the function to hub_event(), removes the while cycle, and
renames the goto targets from loop* to out*.

When touching the code, it fixes also formatting of dev_err() and dev_dbg()
calls to make checkpatch.pl happy :-)

Signed-off-by: Petr Mladek <pmladek@suse.cz>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

1 files changed, 111 insertions, 125 deletions

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 6a9f11fbc2eb..b75749c3029a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4996,8 +4996,7 @@ static void port_event(struct usb_hub *hub, int port1)
                 hub_port_connect_change(hub, port1, portstatus, portchange);
 }
 
-
-static void hub_events(void)
+static void hub_event(void)
 {
         struct list_head *tmp;
         struct usb_device *hdev;
@@ -5008,144 +5007,131 @@ static void hub_events(void)
         u16 hubchange;
         int i, ret;
 
-        /*
-         *  We restart the list every time to avoid a deadlock with
-         * deleting hubs downstream from this one. This should be
-         * safe since we delete the hub from the event list.
-         * Not the most efficient, but avoids deadlocks.
-         */
-        while (1) {
+        /* Grab the first entry at the beginning of the list */
+        spin_lock_irq(&hub_event_lock);
+        if (list_empty(&hub_event_list)) {
+                spin_unlock_irq(&hub_event_lock);
+                return;
+        }
 
-                /* Grab the first entry at the beginning of the list */
-                spin_lock_irq(&hub_event_lock);
-                if (list_empty(&hub_event_list)) {
-                        spin_unlock_irq(&hub_event_lock);
-                        break;
-                }
+        tmp = hub_event_list.next;
+        list_del_init(tmp);
 
-                tmp = hub_event_list.next;
-                list_del_init(tmp);
+        hub = list_entry(tmp, struct usb_hub, event_list);
+        kref_get(&hub->kref);
+        spin_unlock_irq(&hub_event_lock);
 
-                hub = list_entry(tmp, struct usb_hub, event_list);
-                kref_get(&hub->kref);
-                spin_unlock_irq(&hub_event_lock);
+        hdev = hub->hdev;
+        hub_dev = hub->intfdev;
+        intf = to_usb_interface(hub_dev);
+        dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
+                        hdev->state, hdev->maxchild,
+                        /* NOTE: expects max 15 ports... */
+                        (u16) hub->change_bits[0],
+                        (u16) hub->event_bits[0]);
+
+        /* Lock the device, then check to see if we were
+         * disconnected while waiting for the lock to succeed. */
+        usb_lock_device(hdev);
+        if (unlikely(hub->disconnected))
+                goto out_disconnected;
+
+        /* If the hub has died, clean up after it */
+        if (hdev->state == USB_STATE_NOTATTACHED) {
+                hub->error = -ENODEV;
+                hub_quiesce(hub, HUB_DISCONNECT);
+                goto out;
+        }
+
+        /* Autoresume */
+        ret = usb_autopm_get_interface(intf);
+        if (ret) {
+                dev_dbg(hub_dev, "Can't autoresume: %d\n", ret);
+                goto out;
+        }
 
-                hdev = hub->hdev;
-                hub_dev = hub->intfdev;
-                intf = to_usb_interface(hub_dev);
-                dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
-                                hdev->state, hdev->maxchild,
-                                /* NOTE: expects max 15 ports... */
-                                (u16) hub->change_bits[0],
-                                (u16) hub->event_bits[0]);
-
-                /* Lock the device, then check to see if we were
-                 * disconnected while waiting for the lock to succeed. */
-                usb_lock_device(hdev);
-                if (unlikely(hub->disconnected))
-                        goto loop_disconnected;
-
-                /* If the hub has died, clean up after it */
-                if (hdev->state == USB_STATE_NOTATTACHED) {
-                        hub->error = -ENODEV;
-                        hub_quiesce(hub, HUB_DISCONNECT);
-                        goto loop;
-                }
+        /* If this is an inactive hub, do nothing */
+        if (hub->quiescing)
+                goto out_autopm;
+
+        if (hub->error) {
+                dev_dbg(hub_dev, "resetting for error %d\n", hub->error);
 
-                /* Autoresume */
-                ret = usb_autopm_get_interface(intf);
+                ret = usb_reset_device(hdev);
                 if (ret) {
-                        dev_dbg(hub_dev, "Can't autoresume: %d\n", ret);
-                        goto loop;
+                        dev_dbg(hub_dev, "error resetting hub: %d\n", ret);
+                        goto out_autopm;
                 }
 
-                /* If this is an inactive hub, do nothing */
-                if (hub->quiescing)
-                        goto loop_autopm;
+                hub->nerrors = 0;
+                hub->error = 0;
+        }
 
-                if (hub->error) {
-                        dev_dbg (hub_dev, "resetting for error %d\n",
-                                hub->error);
+        /* deal with port status changes */
+        for (i = 1; i <= hdev->maxchild; i++) {
+                struct usb_port *port_dev = hub->ports[i - 1];
 
-                        ret = usb_reset_device(hdev);
-                        if (ret) {
-                                dev_dbg (hub_dev,
-                                        "error resetting hub: %d\n", ret);
-                                goto loop_autopm;
-                        }
-
-                        hub->nerrors = 0;
-                        hub->error = 0;
+                if (test_bit(i, hub->event_bits)
+                                || test_bit(i, hub->change_bits)
+                                || test_bit(i, hub->wakeup_bits)) {
+                        /*
+                         * The get_noresume and barrier ensure that if
+                         * the port was in the process of resuming, we
+                         * flush that work and keep the port active for
+                         * the duration of the port_event().  However,
+                         * if the port is runtime pm suspended
+                         * (powered-off), we leave it in that state, run
+                         * an abbreviated port_event(), and move on.
+                         */
+                        pm_runtime_get_noresume(&port_dev->dev);
+                        pm_runtime_barrier(&port_dev->dev);
+                        usb_lock_port(port_dev);
+                        port_event(hub, i);
+                        usb_unlock_port(port_dev);
+                        pm_runtime_put_sync(&port_dev->dev);
                 }
+        }
 
-                /* deal with port status changes */
-                for (i = 1; i <= hdev->maxchild; i++) {
-                        struct usb_port *port_dev = hub->ports[i - 1];
-
-                        if (test_bit(i, hub->event_bits)
-                                        || test_bit(i, hub->change_bits)
-                                        || test_bit(i, hub->wakeup_bits)) {
-                                /*
-                                 * The get_noresume and barrier ensure that if
-                                 * the port was in the process of resuming, we
-                                 * flush that work and keep the port active for
-                                 * the duration of the port_event().  However,
-                                 * if the port is runtime pm suspended
-                                 * (powered-off), we leave it in that state, run
-                                 * an abbreviated port_event(), and move on.
-                                 */
-                                pm_runtime_get_noresume(&port_dev->dev);
-                                pm_runtime_barrier(&port_dev->dev);
-                                usb_lock_port(port_dev);
-                                port_event(hub, i);
-                                usb_unlock_port(port_dev);
-                                pm_runtime_put_sync(&port_dev->dev);
-                        }
+        /* deal with hub status changes */
+        if (test_and_clear_bit(0, hub->event_bits) == 0)
+                ;       /* do nothing */
+        else if (hub_hub_status(hub, &hubstatus, &hubchange) < 0)
+                dev_err(hub_dev, "get_hub_status failed\n");
+        else {
+                if (hubchange & HUB_CHANGE_LOCAL_POWER) {
+                        dev_dbg(hub_dev, "power change\n");
+                        clear_hub_feature(hdev, C_HUB_LOCAL_POWER);
+                        if (hubstatus & HUB_STATUS_LOCAL_POWER)
+                                /* FIXME: Is this always true? */
+                                hub->limited_power = 1;
+                        else
+                                hub->limited_power = 0;
                 }
+                if (hubchange & HUB_CHANGE_OVERCURRENT) {
+                        u16 status = 0;
+                        u16 unused;
 
-                /* deal with hub status changes */
-                if (test_and_clear_bit(0, hub->event_bits) == 0)
-                        ;       /* do nothing */
-                else if (hub_hub_status(hub, &hubstatus, &hubchange) < 0)
-                        dev_err (hub_dev, "get_hub_status failed\n");
-                else {
-                        if (hubchange & HUB_CHANGE_LOCAL_POWER) {
-                                dev_dbg (hub_dev, "power change\n");
-                                clear_hub_feature(hdev, C_HUB_LOCAL_POWER);
-                                if (hubstatus & HUB_STATUS_LOCAL_POWER)
-                                        /* FIXME: Is this always true? */
-                                        hub->limited_power = 1;
-                                else
-                                        hub->limited_power = 0;
-                        }
-                        if (hubchange & HUB_CHANGE_OVERCURRENT) {
-                                u16 status = 0;
-                                u16 unused;
-
-                                dev_dbg(hub_dev, "over-current change\n");
-                                clear_hub_feature(hdev, C_HUB_OVER_CURRENT);
-                                msleep(500);    /* Cool down */
-                                hub_power_on(hub, true);
-                                hub_hub_status(hub, &status, &unused);
-                                if (status & HUB_STATUS_OVERCURRENT)
-                                        dev_err(hub_dev, "over-current "
-                                                "condition\n");
-                        }
+                        dev_dbg(hub_dev, "over-current change\n");
+                        clear_hub_feature(hdev, C_HUB_OVER_CURRENT);
+                        msleep(500);    /* Cool down */
+                        hub_power_on(hub, true);
+                        hub_hub_status(hub, &status, &unused);
+                        if (status & HUB_STATUS_OVERCURRENT)
+                                dev_err(hub_dev, "over-current condition\n");
                 }
+        }
 
- loop_autopm:
-                /* Balance the usb_autopm_get_interface() above */
-                usb_autopm_put_interface_no_suspend(intf);
- loop:
-                /* Balance the usb_autopm_get_interface_no_resume() in
-                 * kick_khubd() and allow autosuspend.
-                 */
-                usb_autopm_put_interface(intf);
- loop_disconnected:
-                usb_unlock_device(hdev);
-                kref_put(&hub->kref, hub_release);
-
-        } /* end while (1) */
+out_autopm:
+        /* Balance the usb_autopm_get_interface() above */
+        usb_autopm_put_interface_no_suspend(intf);
+out:
+        /* Balance the usb_autopm_get_interface_no_resume() in
+         * kick_khubd() and allow autosuspend.
+         */
+        usb_autopm_put_interface(intf);
+out_disconnected:
+        usb_unlock_device(hdev);
+        kref_put(&hub->kref, hub_release);
 }
 
 static int hub_thread(void *__unused)
@@ -5158,7 +5144,7 @@ static int hub_thread(void *__unused)
         set_freezable();
 
         do {
-                hub_events();
+                hub_event();
                 wait_event_freezable(khubd_wait,
                                 !list_empty(&hub_event_list) ||
                                 kthread_should_stop());