Re: [PATCH v3 1/1] USB: EHCI: wait more than 3ms until the device enters full-speed idle

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux