EHCI: Fix root-hub and port suspend/resume problems
authorAlan Stern <stern@rowland.harvard.edu>
Thu, 9 Nov 2006 19:42:16 +0000 (14:42 -0500)
committerGreg Kroah-Hartman <gregkh@suse.de>
Fri, 1 Dec 2006 22:25:52 +0000 (14:25 -0800)
This patch (as738b) fixes numerous problems in the controller/root-hub
suspend/resume/remote-wakeup support in ehci-hcd:

The bus_resume() routine should wake up only the ports that
were suspended by bus_suspend().  Ports that were already
suspended should remain that way.

The interrupt mask is used to detect loss of power in the
bus_resume() routine (if the mask is 0 then power was lost).
However bus_suspend() always sets the mask to 0.  Instead the
mask should retain its normal value, with port-change-detect
interrupts disabled if remote wakeup is turned off.

The interrupt mask should be reset to its correct value at the
end of bus_resume() regardless of whether power was lost.

bus_resume() reinitializes the operational registers if power
was lost.  However those registers are not in the aux power
well, hence they can lose their values whenever the controller
is put into D3.  They should always be reinitialized.

When a port-change interrupt occurs and the root hub is
suspended, the interrupt handler should request a root-hub
resume instead of starting up the controller all by itself.

There's no need for the interrupt handler to request a
root-hub resume every time a suspended port sends a
remote-wakeup request.

The pci_resume() method doesn't need to check for connected
ports when deciding whether or not to reset the controller.
It can make that decision based on whether Vaux power was
maintained.

Even when the controller does not need to be reset,
pci_resume() must undo the effect of pci_suspend() by
re-enabling the interrupt mask.

If power was lost, pci_resume() must not call ehci_run().
At this point the root hub is still supposed to be suspended,
not running.  It's enough to rewrite the command register and
set the configured_flag.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/usb/host/ehci-hcd.c
drivers/usb/host/ehci-hub.c
drivers/usb/host/ehci-pci.c
drivers/usb/host/ehci.h

index f2ceb5fdbeb77456be120edd57b04181fa1d1895..025d333136817da006700cfc7852fe06111caabf 100644 (file)
@@ -619,9 +619,8 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
                unsigned        i = HCS_N_PORTS (ehci->hcs_params);
 
                /* resume root hub? */
-               status = readl (&ehci->regs->command);
-               if (!(status & CMD_RUN))
-                       writel (status | CMD_RUN, &ehci->regs->command);
+               if (!(readl(&ehci->regs->command) & CMD_RUN))
+                       usb_hcd_resume_root_hub(hcd);
 
                while (i--) {
                        int pstatus = readl (&ehci->regs->port_status [i]);
@@ -638,7 +637,6 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
                         */
                        ehci->reset_done [i] = jiffies + msecs_to_jiffies (20);
                        ehci_dbg (ehci, "port %d remote wakeup\n", i + 1);
-                       usb_hcd_resume_root_hub(hcd);
                }
        }
 
index 0a56dfa2745d645d9dd3a1041478d5cb03a50a24..bfe5f307cba68b6acfe5c28112bc095159f435bd 100644 (file)
@@ -34,6 +34,7 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
 {
        struct ehci_hcd         *ehci = hcd_to_ehci (hcd);
        int                     port;
+       int                     mask;
 
        if (time_before (jiffies, ehci->next_statechange))
                msleep(5);
@@ -51,14 +52,25 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
                ehci->reclaim_ready = 1;
        ehci_work(ehci);
 
-       /* suspend any active/unsuspended ports, maybe allow wakeup */
+       /* Unlike other USB host controller types, EHCI doesn't have
+        * any notion of "global" or bus-wide suspend.  The driver has
+        * to manually suspend all the active unsuspended ports, and
+        * then manually resume them in the bus_resume() routine.
+        */
+       ehci->bus_suspended = 0;
        while (port--) {
                u32 __iomem     *reg = &ehci->regs->port_status [port];
                u32             t1 = readl (reg) & ~PORT_RWC_BITS;
                u32             t2 = t1;
 
-               if ((t1 & PORT_PE) && !(t1 & PORT_OWNER))
+               /* keep track of which ports we suspend */
+               if ((t1 & PORT_PE) && !(t1 & PORT_OWNER) &&
+                               !(t1 & PORT_SUSPEND)) {
                        t2 |= PORT_SUSPEND;
+                       set_bit(port, &ehci->bus_suspended);
+               }
+
+               /* enable remote wakeup on all ports */
                if (device_may_wakeup(&hcd->self.root_hub->dev))
                        t2 |= PORT_WKOC_E|PORT_WKDISC_E|PORT_WKCONN_E;
                else
@@ -76,6 +88,13 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
        ehci_halt (ehci);
        hcd->state = HC_STATE_SUSPENDED;
 
+       /* allow remote wakeup */
+       mask = INTR_MASK;
+       if (!device_may_wakeup(&hcd->self.root_hub->dev))
+               mask &= ~STS_PCD;
+       writel(mask, &ehci->regs->intr_enable);
+       readl(&ehci->regs->intr_enable);
+
        ehci->next_statechange = jiffies + msecs_to_jiffies(10);
        spin_unlock_irq (&ehci->lock);
        return 0;
@@ -88,7 +107,6 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
        struct ehci_hcd         *ehci = hcd_to_ehci (hcd);
        u32                     temp;
        int                     i;
-       int                     intr_enable;
 
        if (time_before (jiffies, ehci->next_statechange))
                msleep(5);
@@ -100,31 +118,30 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
         * the last user of the controller, not reset/pm hardware keeping
         * state we gave to it.
         */
+       temp = readl(&ehci->regs->intr_enable);
+       ehci_dbg(ehci, "resume root hub%s\n", temp ? "" : " after power loss");
 
-       /* re-init operational registers in case we lost power */
-       if (readl (&ehci->regs->intr_enable) == 0) {
-               /* at least some APM implementations will try to deliver
-                * IRQs right away, so delay them until we're ready.
-                */
-               intr_enable = 1;
-               writel (0, &ehci->regs->segment);
-               writel (ehci->periodic_dma, &ehci->regs->frame_list);
-               writel ((u32)ehci->async->qh_dma, &ehci->regs->async_next);
-       } else
-               intr_enable = 0;
-       ehci_dbg(ehci, "resume root hub%s\n",
-                       intr_enable ? " after power loss" : "");
+       /* at least some APM implementations will try to deliver
+        * IRQs right away, so delay them until we're ready.
+        */
+       writel(0, &ehci->regs->intr_enable);
+
+       /* re-init operational registers */
+       writel(0, &ehci->regs->segment);
+       writel(ehci->periodic_dma, &ehci->regs->frame_list);
+       writel((u32) ehci->async->qh_dma, &ehci->regs->async_next);
 
        /* restore CMD_RUN, framelist size, and irq threshold */
        writel (ehci->command, &ehci->regs->command);
 
-       /* take ports out of suspend */
+       /* manually resume the ports we suspended during bus_suspend() */
        i = HCS_N_PORTS (ehci->hcs_params);
        while (i--) {
                temp = readl (&ehci->regs->port_status [i]);
                temp &= ~(PORT_RWC_BITS
                        | PORT_WKOC_E | PORT_WKDISC_E | PORT_WKCONN_E);
-               if (temp & PORT_SUSPEND) {
+               if (test_bit(i, &ehci->bus_suspended) &&
+                               (temp & PORT_SUSPEND)) {
                        ehci->reset_done [i] = jiffies + msecs_to_jiffies (20);
                        temp |= PORT_RESUME;
                }
@@ -134,11 +151,12 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
        mdelay (20);
        while (i--) {
                temp = readl (&ehci->regs->port_status [i]);
-               if ((temp & PORT_SUSPEND) == 0)
-                       continue;
-               temp &= ~(PORT_RWC_BITS | PORT_RESUME);
-               writel (temp, &ehci->regs->port_status [i]);
-               ehci_vdbg (ehci, "resumed port %d\n", i + 1);
+               if (test_bit(i, &ehci->bus_suspended) &&
+                               (temp & PORT_SUSPEND)) {
+                       temp &= ~(PORT_RWC_BITS | PORT_RESUME);
+                       writel (temp, &ehci->regs->port_status [i]);
+                       ehci_vdbg (ehci, "resumed port %d\n", i + 1);
+               }
        }
        (void) readl (&ehci->regs->command);
 
@@ -157,8 +175,7 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
        hcd->state = HC_STATE_RUNNING;
 
        /* Now we can safely re-enable irqs */
-       if (intr_enable)
-               writel (INTR_MASK, &ehci->regs->intr_enable);
+       writel(INTR_MASK, &ehci->regs->intr_enable);
 
        spin_unlock_irq (&ehci->lock);
        return 0;
index e51c1ed81ac463cdc677c1a84883ede4e6a2ec9c..4bc7970ba3ef5d208d6341fe958d8895fda2d6f5 100644 (file)
@@ -257,9 +257,7 @@ static int ehci_pci_suspend(struct usb_hcd *hcd, pm_message_t message)
 static int ehci_pci_resume(struct usb_hcd *hcd)
 {
        struct ehci_hcd         *ehci = hcd_to_ehci(hcd);
-       unsigned                port;
        struct pci_dev          *pdev = to_pci_dev(hcd->self.controller);
-       int                     retval = -EINVAL;
 
        // maybe restore FLADJ
 
@@ -269,27 +267,19 @@ static int ehci_pci_resume(struct usb_hcd *hcd)
        /* Mark hardware accessible again as we are out of D3 state by now */
        set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
 
-       /* If CF is clear, we lost PCI Vaux power and need to restart.  */
-       if (readl(&ehci->regs->configured_flag) != FLAG_CF)
-               goto restart;
-
-       /* If any port is suspended (or owned by the companion),
-        * we know we can/must resume the HC (and mustn't reset it).
-        * We just defer that to the root hub code.
+       /* If CF is still set, we maintained PCI Vaux power.
+        * Just undo the effect of ehci_pci_suspend().
         */
-       for (port = HCS_N_PORTS(ehci->hcs_params); port > 0; ) {
-               u32     status;
-               port--;
-               status = readl(&ehci->regs->port_status [port]);
-               if (!(status & PORT_POWER))
-                       continue;
-               if (status & (PORT_SUSPEND | PORT_RESUME | PORT_OWNER)) {
-                       usb_hcd_resume_root_hub(hcd);
-                       return 0;
-               }
+       if (readl(&ehci->regs->configured_flag) == FLAG_CF) {
+               int     mask = INTR_MASK;
+
+               if (!device_may_wakeup(&hcd->self.root_hub->dev))
+                       mask &= ~STS_PCD;
+               writel(mask, &ehci->regs->intr_enable);
+               readl(&ehci->regs->intr_enable);
+               return 0;
        }
 
-restart:
        ehci_dbg(ehci, "lost power, restarting\n");
        usb_root_hub_lost_power(hcd->self.root_hub);
 
@@ -307,13 +297,15 @@ restart:
        ehci_work(ehci);
        spin_unlock_irq(&ehci->lock);
 
-       /* restart; khubd will disconnect devices */
-       retval = ehci_run(hcd);
-
        /* here we "know" root ports should always stay powered */
        ehci_port_power(ehci, 1);
 
-       return retval;
+       writel(ehci->command, &ehci->regs->command);
+       writel(FLAG_CF, &ehci->regs->configured_flag);
+       readl(&ehci->regs->command);    /* unblock posted writes */
+
+       hcd->state = HC_STATE_SUSPENDED;
+       return 0;
 }
 #endif
 
index bbc3082a73d752ae359b4c45fb1ae59cd0764699..74dbc6c8228ff634be1925907f9780bcbe77882f 100644 (file)
@@ -74,6 +74,7 @@ struct ehci_hcd {                     /* one per controller */
 
        /* per root hub port */
        unsigned long           reset_done [EHCI_MAX_ROOT_PORTS];
+       unsigned long           bus_suspended;
 
        /* per-HC memory pools (could be per-bus, but ...) */
        struct dma_pool         *qh_pool;       /* qh per active urb */