On Wed, 2 Aug 2006, David Brownell wrote: > On Thursday 06 July 2006 12:47 pm, Alan Stern wrote: > > Greg and David: > > > > This patch (as738) fixes the root-hub remote-wakeup support in ehci-hcd. > > The existing code has two problems. First, when the bus is suspended all > > root-hub interrupts are disabled. > > A better way of saying that is probably that ehci_halt() is the wrong > way to turn off the RUN bit a few lines before that.
Okay. If you prefer to change the call to ehci_halt() instead of adding a line to re-enable IRQs, that's fine with me. > ... now that I have a machine that can partially resume from STR, > I can maybe spend some time chasing some of those issues; the box > that can resume partially from "standby" is tied up with other stuff, > not available for testing. I still have zero faith in how the Linux > ACPI code does its wakeup event processing, since the number of times > I've seen it behave sanely can be counted on the fingers of a hand > that's been through industrial accidents that cost fingers. Before worrying about STR, standby, or STD, you should just try some simple tests using runtime suspend. They should be doable on any machine with an EHCI controller. > > Second, the call to > > usb_hcd_resume_root_hub() is in the wrong place -- it is invoked when a > > wakeup request arrives on a suspended port rather than when the root hub > > itself generates a wakeup request. > > All hcd_resume_root() does is kick khubd, and it doesn't matter how > often that happens since it merges events, and can't even talk to the > root hub until that routine finishes. More accurately, it doesn't matter if that happens too often. It _does_ matter if that doesn't happen often enough, which is the problem I was trying to fix. > Your change makes it so the > controller isn't necessarily going to be running when khubd comes in, > and kicks hubd even when there's nothing for it to do. All right, one thing at a time: > Your change makes it so the > controller isn't necessarily going to be running when khubd comes in, I.e., the controller might be suspended (as in PCI D3)? I don't see how that could happen. The routine I changed is ehci_irq(), and when the controller is suspended we are very careful to make sure that it can't generate IRQs. HCD_FLAG_HW_ACCESSIBLE and all that. So how could my changes cause khubd to kick in when the controller is suspended? But perhaps you meant to say that my patch might kick khubd at a time when the root hub isn't running? Well of course it does -- that's the whole point. The root hub is suspended and it needs to be resumed; that's exactly what usb_hcd_resume_root_hub() is intended to accomplish. > and kicks hubd even when there's nothing for it to do. There _is_ something for it do to: It can resume the root hub! By contrasting, the existing code calls usb_hcd_resume_root_hub() at times when there really is nothing for it to do (because the root hub is already resumed when a remote-wakeup request is received on one of the ports). Are we miscommunicating somehow? > > It's easy to test that the existing code doesn't work and the patched > > driver does. Just unplug all the ports on an EHCI controller, suspend the > > root hub interface and the root hub device through sysfs, and then try > > plugging in a USB device. Without the patch, nothing happens. With the > > patch, the driver wakes up and processes the new connection. Like I said above, you should try doing this experiment. Alan Stern ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
