On Tue, 1 Nov 2011, Linus Torvalds wrote: > On Tue, Nov 1, 2011 at 7:28 AM, Sarah Sharp <sarah.a.sharp@xxxxxxxxx> wrote: > > > > Can you try the attached patch and see if it helps your issues with > > enabling runtime PM for the NEC USB 3.0 host controller? �You may have > > to increase the timeout for your system. > > Will try. > > HOWEVER. > > I think the patch is wrong even if it works. Matthew's suggestion > sounds much better, but the *real* problem seems to be that the PCI > "probe the device if PME is set" logic is broken, and does > "resume+suspend" back-to-back - resulting in you needing to add the > delay to your resume function. > > Doing back-to-back "enable/disable" things like that is simply wrong > in general. It's wrong for the same reason that it is wrong to do > "sti ; nop ; cli" on a CPU - we've had that bug too, where we wanted > to enable interrupts, but left the window so small that real > microarchitectures never actually got them. > > I think that the PCI "poll for PME" logic should just resume the > device - and *not* suspend it again. Maybe it can suspend it next time > around when it polls, if PME has been cleared. That should be (a) sane > and (b) obviate any need for some random delay in some random driver. > > Jesse? > > That said, Matthew's suggestion sounds like a good idea regardless. We shouldn't have to wait for an interrupt; we can safely assume that whenever a host controller is runtime-resumed, it's because the root hub has requested it. Hence there's no real reason not to resume the root hub at the same time. uhci-hcd polls the root hub status when the controller resumes; if there's a status change then the root hub will automatically be resumed. ohci-hcd used to do the same thing, but Matthew found that some controllers weren't reporting the status change correctly and so now it always resumes the root hub. As far as I can see, xhci-hcd does not do either one. (Nor does ehci-hcd...) With xhci-hcd it's a little more complicated because there are two root hubs to consider. xhci-resume could check the status of both and resume whichever got a status change. Or it could simply resume both of them every time. Sarah, which do you think is better? One more thing -- the _mechanism_ we use for resuming the root hub is a little awkward. It involves adding a resume request to a workqueue, and there's nothing to keep the controller awake until the workqueue runs. I suppose this could cause the controller to bounce back and forth between awake and asleep for a while. We should be able to avoid this, with a little care. I'll post a patch for testing later today. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
