lkml.org 
[lkml]   [2011]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci
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
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2011-07-05 17:55    [from the cache]
©2003-2011 Jasper Spaans