This patch (as1589) resolves some unlikely races involving system
shutdown or controller death in ehci-hcd:

        Shutdown races with both root-hub resume and controller
        resume.

        Controller death races with root-hub suspend.

A new bitflag is added to indicate that the controller has been shut
down (whether for system shutdown or because it died).  Tests are
added in the suspend and resume pathways to avoid reactivating the
controller after any sort of shutdown.

Signed-off-by: Alan Stern <st...@rowland.harvard.edu>

---

 drivers/usb/host/ehci-hcd.c |   20 +++++++++++++++++++-
 drivers/usb/host/ehci-hub.c |   27 +++++++++++++++++++++++----
 drivers/usb/host/ehci.h     |    1 +
 3 files changed, 43 insertions(+), 5 deletions(-)

Index: usb-3.4/drivers/usb/host/ehci.h
===================================================================
--- usb-3.4.orig/drivers/usb/host/ehci.h
+++ usb-3.4/drivers/usb/host/ehci.h
@@ -118,6 +118,7 @@ struct ehci_hcd {                   /* one per controlle
        bool                    need_rescan:1;
        bool                    intr_unlinking:1;
        bool                    async_unlinking:1;
+       bool                    shutdown:1;
        struct ehci_qh          *qh_scan_next;
 
        /* async schedule support */
Index: usb-3.4/drivers/usb/host/ehci-hcd.c
===================================================================
--- usb-3.4.orig/drivers/usb/host/ehci-hcd.c
+++ usb-3.4/drivers/usb/host/ehci-hcd.c
@@ -343,6 +343,7 @@ static void ehci_shutdown(struct usb_hcd
        struct ehci_hcd *ehci = hcd_to_ehci(hcd);
 
        spin_lock_irq(&ehci->lock);
+       ehci->shutdown = true;
        ehci->rh_state = EHCI_RH_STOPPING;
        ehci->enabled_hrtimer_events = 0;
        spin_unlock_irq(&ehci->lock);
@@ -823,6 +824,7 @@ dead:
                usb_hc_died(hcd);
 
                /* Don't let the controller do anything more */
+               ehci->shutdown = true;
                ehci->rh_state = EHCI_RH_STOPPING;
                ehci->command &= ~(CMD_RUN | CMD_ASE | CMD_PSE);
                ehci_writel(ehci, ehci->command, &ehci->regs->command);
@@ -1129,6 +1131,9 @@ static int __maybe_unused ehci_resume(st
        /* Mark hardware accessible again as we are back to full power by now */
        set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
 
+       if (ehci->shutdown)
+               return 0;               /* Controller is dead */
+
        /*
         * If CF is still set and we aren't resuming from hibernation
         * then we maintained suspend power.
@@ -1139,10 +1144,17 @@ static int __maybe_unused ehci_resume(st
                int     mask = INTR_MASK;
 
                ehci_prepare_ports_for_controller_resume(ehci);
+
+               spin_lock_irq(&ehci->lock);
+               if (ehci->shutdown)
+                       goto skip;
+
                if (!hcd->self.root_hub->do_remote_wakeup)
                        mask &= ~STS_PCD;
                ehci_writel(ehci, mask, &ehci->regs->intr_enable);
                ehci_readl(ehci, &ehci->regs->intr_enable);
+ skip:
+               spin_unlock_irq(&ehci->lock);
                return 0;
        }
 
@@ -1154,14 +1166,20 @@ static int __maybe_unused ehci_resume(st
        (void) ehci_halt(ehci);
        (void) ehci_reset(ehci);
 
+       spin_lock_irq(&ehci->lock);
+       if (ehci->shutdown)
+               goto skip;
+
        ehci_writel(ehci, ehci->command, &ehci->regs->command);
        ehci_writel(ehci, FLAG_CF, &ehci->regs->configured_flag);
        ehci_readl(ehci, &ehci->regs->command); /* unblock posted writes */
 
+       ehci->rh_state = EHCI_RH_SUSPENDED;
+       spin_unlock_irq(&ehci->lock);
+
        /* here we "know" root ports should always stay powered */
        ehci_port_power(ehci, 1);
 
-       ehci->rh_state = EHCI_RH_SUSPENDED;
        return 1;
 }
 
Index: usb-3.4/drivers/usb/host/ehci-hub.c
===================================================================
--- usb-3.4.orig/drivers/usb/host/ehci-hub.c
+++ usb-3.4/drivers/usb/host/ehci-hub.c
@@ -221,6 +221,8 @@ static int ehci_bus_suspend (struct usb_
        ehci_quiesce(ehci);
 
        spin_lock_irq (&ehci->lock);
+       if (ehci->rh_state < EHCI_RH_RUNNING)
+               goto done;
 
        /* Once the controller is stopped, port resumes that are already
         * in progress won't complete.  Hence if remote wakeup is enabled
@@ -306,6 +308,10 @@ static int ehci_bus_suspend (struct usb_
        ehci_halt (ehci);
 
        spin_lock_irq(&ehci->lock);
+       if (ehci->enabled_hrtimer_events & BIT(EHCI_HRTIMER_POLL_DEAD))
+               ehci_handle_controller_death(ehci);
+       if (ehci->rh_state != EHCI_RH_RUNNING)
+               goto done;
        ehci->rh_state = EHCI_RH_SUSPENDED;
 
        end_unlink_async(ehci);
@@ -320,6 +326,7 @@ static int ehci_bus_suspend (struct usb_
        ehci_writel(ehci, mask, &ehci->regs->intr_enable);
        ehci_readl(ehci, &ehci->regs->intr_enable);
 
+ done:
        ehci->next_statechange = jiffies + msecs_to_jiffies(10);
        ehci->enabled_hrtimer_events = 0;
        ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT;
@@ -342,10 +349,8 @@ static int ehci_bus_resume (struct usb_h
        if (time_before (jiffies, ehci->next_statechange))
                msleep(5);
        spin_lock_irq (&ehci->lock);
-       if (!HCD_HW_ACCESSIBLE(hcd)) {
-               spin_unlock_irq(&ehci->lock);
-               return -ESHUTDOWN;
-       }
+       if (!HCD_HW_ACCESSIBLE(hcd) || ehci->shutdown)
+               goto shutdown;
 
        if (unlikely(ehci->debug)) {
                if (!dbgp_reset_prep())
@@ -384,6 +389,8 @@ static int ehci_bus_resume (struct usb_h
        spin_unlock_irq(&ehci->lock);
        msleep(8);
        spin_lock_irq(&ehci->lock);
+       if (ehci->shutdown)
+               goto shutdown;
 
        /* clear phy low-power mode before resume */
        if (ehci->bus_suspended && ehci->has_hostpc) {
@@ -401,6 +408,8 @@ static int ehci_bus_resume (struct usb_h
                spin_unlock_irq(&ehci->lock);
                msleep(5);
                spin_lock_irq(&ehci->lock);
+               if (ehci->shutdown)
+                       goto shutdown;
        }
 
        /* manually resume the ports we suspended during bus_suspend() */
@@ -421,6 +430,8 @@ static int ehci_bus_resume (struct usb_h
                spin_unlock_irq(&ehci->lock);
                msleep(20);
                spin_lock_irq(&ehci->lock);
+               if (ehci->shutdown)
+                       goto shutdown;
        }
 
        i = HCS_N_PORTS (ehci->hcs_params);
@@ -439,10 +450,18 @@ static int ehci_bus_resume (struct usb_h
        ehci_handover_companion_ports(ehci);
 
        /* Now we can safely re-enable irqs */
+       spin_lock_irq(&ehci->lock);
+       if (ehci->shutdown)
+               goto shutdown;
        ehci_writel(ehci, INTR_MASK, &ehci->regs->intr_enable);
        (void) ehci_readl(ehci, &ehci->regs->intr_enable);
+       spin_unlock_irq(&ehci->lock);
 
        return 0;
+
+ shutdown:
+       spin_unlock_irq(&ehci->lock);
+       return -ESHUTDOWN;
 }
 
 #else


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