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    [W:0.041 / U:0.144 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site