On Mon, 2006-06-26 at 23:18 -0700, Linus Torvalds wrote: > > On Tue, 27 Jun 2006, Adam Belay wrote: > > > > Yes, and pci_set_power_state() can require msleep(). > > Actually, I was looking at that, and it's a problem right now. > > For all the silly (and wrong) reasons. > > The msleep() shouldn't actually be in pci_set_power_state(), but in the > infrastructure that calls it. In particular, when actually powering down, > there's no point in doing a msleep() between each device - we'll be > sleeping a lot longer than 10ms after we've gone down. > > The fact that D3hot won't necessarily take effect until 10 ms after we've > done the "go to sleep" thing obviously doesn't really mean that we should > actually sleep 10 msec _there_. Agreed... though I still (heh, do I sound like I insist a bit there ? :) think that we should look into not having interrupts off for this second pass... it's just too much of a pain not to be able to hit a code path that uses a mutex or whatever else and starts insulting you with might_sleep() backtraces... And yes, even in the second phase. The console is a good example I took earlier, fb_set_suspend() really wants the console sem to be held, that's the only remotely sane way to make sure the fbcon isn't currently trying to draw to you or other things like that and the console is typically what you want to have suspended late and/or resumed early.... In fact, for radeonfb, I also need a lot of long delays when bringing the chip back up. Right now, I have ugly hacks to do either mdelay or msleep depending if it uses my early wakeup hook or the real resume()... I'm not sure actually _why_ we should have irqs off if we do the job properly, that is have either subsystems, class devices or child devices, having taken care of blocking IOs to the driver in the first place. (If we need a generic netdev suspend/resume, then so be it, that will block the queues, and ethX should/could be a child of the device like hda is a child of the controller in the IDE stack). Ben.
