On Mon, 10 Feb 2014, Alan Stern wrote: > On Fri, 7 Feb 2014, Peter Chen wrote: > > > If the high-speed device does not enter full-speed idle after > > wakeup on disconnect logic has effected, there will be an > > unexpected disconnect wakeup interrupt due to the bus is still SE0. > > > > Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx> > > > > Hi Alan, > > > > If you can't met this issue, it may due to your hardware enables > > WKDN after detecting full-speed idle or the time between stop > > SoF and enable WKDN is more than 3ms (You need to know when > > the WKDN has really taken effect). > > Yes. However, I don't like the way your patch can increase the total > time if several ports have to be put into suspend. Consider this patch > instead. I decided that patch was more complicated than it needed to be. On your system, a simple delay is all you need, right? I assume this means the wakeup signal isn't "sticky". That is, once it has turned on, it remains on only as long as the wakeup condition is present; it doesn't stay on even after the wakeup condition goes away. In your case, the wakeup condition will be present while the bus uses high-speed terminations, but when the bus switches over to full-speed idle the wakeup condition will go away. Thus, I assume the wakeup signal will turn off after a few milliseconds, instead of remaining on indefinitely. So tell me what you think of this patch instead. Alan Stern Index: usb-3.14/drivers/usb/host/ehci-hub.c =================================================================== --- usb-3.14.orig/drivers/usb/host/ehci-hub.c +++ usb-3.14/drivers/usb/host/ehci-hub.c @@ -238,6 +238,7 @@ static int ehci_bus_suspend (struct usb_ int port; int mask; int changed; + bool fs_idle_delay; ehci_dbg(ehci, "suspend root hub\n"); @@ -272,6 +273,7 @@ static int ehci_bus_suspend (struct usb_ ehci->bus_suspended = 0; ehci->owned_ports = 0; changed = 0; + fs_idle_delay = false; port = HCS_N_PORTS(ehci->hcs_params); while (port--) { u32 __iomem *reg = &ehci->regs->port_status [port]; @@ -300,16 +302,32 @@ static int ehci_bus_suspend (struct usb_ } if (t1 != t2) { + /* + * On some controllers, Wake-On-Disconnect will + * generate false wakeup signals until the bus + * switches over to full-speed idle. For their + * sake, add a delay if we need one. + */ + if ((t2 & PORT_WKDISC_E) && + ehci_port_speed(ehci, t2) == + USB_PORT_STAT_HIGH_SPEED) + fs_idle_delay = true; ehci_writel(ehci, t2, reg); changed = 1; } } + spin_unlock_irq(&ehci->lock); + + if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) { + /* + * Wait for HCD to enter low-power mode or for the bus + * to switch to full-speed idle. + */ + usleep_range(5000, 5500); + } if (changed && ehci->has_tdi_phy_lpm) { - spin_unlock_irq(&ehci->lock); - msleep(5); /* 5 ms for HCD to enter low-power mode */ spin_lock_irq(&ehci->lock); - port = HCS_N_PORTS(ehci->hcs_params); while (port--) { u32 __iomem *hostpc_reg = &ehci->regs->hostpc[port]; @@ -322,8 +340,8 @@ static int ehci_bus_suspend (struct usb_ port, (t3 & HOSTPC_PHCD) ? "succeeded" : "failed"); } + spin_unlock_irq(&ehci->lock); } - spin_unlock_irq(&ehci->lock); /* Apparently some devices need a >= 1-uframe delay here */ if (ehci->bus_suspended) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html