On Wed, 29 Jan 2014, Peter Chen wrote: > On Tue, Jan 28, 2014 at 11:32:28AM -0500, Alan Stern wrote: > > On Tue, 28 Jan 2014, Peter Chen wrote: > > > > > > It sounds like this is a bug in your EHCI hardware. The > > > > wake-on-disconnect logic should never take effect until after the port > > > > goes into full-speed idle. > > > > > > Where EHCI spec said that? I can't find it at 2.3.9 and 4.3. > > > > It doesn't say that. Not explicitly. > > > > On the other hand, it doesn't say that you have to wait for the port to > > enter full-speed idle after turning on the Suspend bit before you can > > turn on WKDSCNNT_E bit. > > > > So, it is vendor defined, some vendors enable wakeup on disconnect > after full speed idle, some may not wait. Okay. > > > > This doesn't look like it will do what you want. t2 already has the > > > > PORT_WKDISC_E bit set, so once the > > > > > > > > ehci_writel(ehci, t2, reg); > > > > > > > > has executed, it is already too late. Instead, you should write (t2 & > > > > ~PORT_WKDISC_E), then wait a few ms, and then write t2. > > > > > > Yes, it is safe for writing suspendM first, wait 3-4ms, then write > > > PORT_WKDISC_E. The reason why my proposal change is ok is the wakeup logic > > > has still not taken effect before the PHY enters low power mode. > > > > What happens if you start putting a different PHY on the board, one > > that takes longer to enter low-power mode? > > A little difficult to change a PHY on the board. > > Just like I said above, it depends on when the hardware enables wake on > disconnect, full-speed idle, full-speed idle + PHY enters low power, > or only just PHY enters low power? The code should work correctly no matter when the hardware enables wake-on-disconnect. > > So far I have not seen any complaints about this happening from any > > user except you. I just tried doing the experiment on my own computer > > (enable wakeup for the root hub, plug in a high-speed device, and > > suspend the computer). It worked correctly. > > Have you enabled wakeup on your high speed device? No. > This problem has not occurred for wakeup enabled device or old kernel > (before you enable global suspend), since there is a 10ms delay at > usb_port_suspend. I just tried the test again on a different computer. This time I was running 3.13-rc8 (before was 3.12.something). In both cases, the device does not support wakeup (it is a flash drive). > > Also, there already is a 5-ms sleep just below the code you changed. > > It depends on ehci->has_tdi_phy_lpm. Is that flag set for your system? > > If it isn't, you could simply remove the test for has_tdi_phy_lpm. > > That should have the same effect as your patch. > > > > It is just for a specific platform which has tdi phy. This case may > be generic, in a word, do we need to make sure the bus enters full-speed > idle after ehci_bus_suspend has finished? You mean _before_ ehci_bus_suspend has finished. As far as I can see, it doesn't matter. The important thing is whether the bus enters full-speed idle before we enable wake-on-disconnect. > If we have not guaranteed > it, platform code needs to make sure the bus will not change before > the wakeup logic takes effect, of cos, these kinds of platform have > no hardware logic to make sure above. So ehci-hcd has to take care of it. Alan Stern -- 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
