Hi, On Tue, Jul 05, 2011 at 10:17:14AM -0400, Alan Stern wrote: > On Tue, 5 Jul 2011, Felipe Balbi wrote: > > > > The real problem here is that you guys are trying to use the runtime PM > > > framework to carry out activities during system suspend. That won't > > > work; it's just a bad idea all round. Use the proper callbacks to do > > > what you want. > > > > then what's the point in even having runtime PM if we will still have to > > implement the same functionality on the other callbacks ? > > You don't have to duplicate the functionality. You can use exactly the > same functions for both sets of callbacks if you want; just make sure > the callbacks point to them. true, good point. > > Well, of > > course runtime PM will conserve power on runtime, but system suspend > > should be no different other than an "always deepest sleep state" > > decision. > > No, it is significantly different for several reasons. Some of the > most important differences are concerned with freezing userspace and > deciding what events should be allowed to wake up the system. Also, > there are systems which can achieve greater power savings by system > sleep than they can by runtime PM + cpuidle. I remember we've been through this discussion before and it's just nonsensical to make such statement. What does freezing userspace have to do with power consumption ? If you can't reach lower power consumption with runtime PM it only means userspace is waking the system too much. > > The thing now is that pm_runtime was done so that drivers would stop > > caring about clocks, which is a big plus, but if we still have to handle > > ->suspend()/->resume() differently, we will still need to clk_get(); > > clk_enable(); clk_disable(); Then what was the big deal with runtime PM? > > I don't know about that. Clock usage has always been internal to the > implementation you guys have been working on, and I haven't followed > it. If your implementation was designed incorrectly, well, that's a > shame but it's understandable. Things like that happen. It shouldn't > be too hard to fix. > > But first you do need to understand that system suspend really _is_ > different from runtime suspend. Sure, you may be able to share some > code between them, but you should not expect to be able to use one in > place of the other. I really fail to see why not, and maybe it's only my fault and I need to read the Documentation/ more carefully :-s > > IMHO, we should have only one PM layer. System suspend/resume should be > > implemented so that core PM "forcefully" calls > > ->runtime_suspend()/->runtime_resume() of call drivers, all > > synchronously. Maybe we need an extra > > RPM_STATIC_SUSPEND_PLEASE_HANDLE_IT_ALL_SYNCHRONOUSLY flag, but that's > > another detail. > > Statements like this should be posted to linux-pm where they can be > discussed properly. It certainly isn't fair to make such claims > without even CC-ing the PM maintainer. > > Besides, handling runtime PM synchronously won't do you any good if the > user has disabled runtime PM via sysfs or not enabled > CONFIG_PM_RUNTIME in the first place. Have you forgotten about those > possibilities? I thought that the "we should have only one PM layer" already carried the idea that CONFIG_PM and CONFIG_PM_RUNTIME would be combined into one, and sysfs would need a little re-factoring... > Furthermore, from what I've gathered so far from this thread, the > _real_ problem is that nobody has written suspend and resume callbacks > for the parent device. You're relying on runtime PM to do things with > the parent, but instead you should make use of the usual system sleep > mechanism: Parents are always suspended after their children and > awakened before. Have the parent's suspend routine disable the clocks > and have the resume routine enable them. Problem solved, no changes > needed in the child's driver code. that's currently hidden on the omap rutime pm support. No driver is to talk to clk API directly anymore. Granted, now that I read what I just wrote it does sound like it's a limitation, although it's really nice not to have to remember all the numerous clocks needed for a particular device to work properly. So, if there would be a way, other than pm_runtime_resume(), to enable all clocks a particular device has without really having to clk_get(); clk_enable() each one of them, fine, this would be solved. But as of today, we only have pm_runtime_resume() to achieve that, unless I'm missing something. -- balbi
Attachment:
signature.asc
Description: Digital signature
