lkml.org 
[lkml]   [2011]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [Update][PATCH 7/9] PM / Runtime: Add generic clock manipulation rountines for runtime PM
    Date
    On Tuesday, April 19, 2011, Paul Mundt wrote:
    > On Tue, Apr 19, 2011 at 11:42:26PM +0200, Rafael J. Wysocki wrote:
    > > On Tuesday, April 19, 2011, Magnus Damm wrote:
    > > > Do you have any plans to add support for multiple clocks per struct
    > > > device? I had some plans to play around with that myself, but if we're
    > > > moving the code to a common place then this obviously becomes a bit
    > > > more complicated.
    > > >
    > > > It's rather common that each hardware block in an SoC is connected to
    > > > more than a single clock. This needs to be managed by software
    > > > somehow.
    > > >
    > > > So if the plan is to make to the code generic, how about allowing the
    > > > architecture to associate clocks with each struct device somehow?
    > >
    > > Hmm. For now, my patchset generally reorganizes the existing code without
    > > adding new functionality. Of course, it is possible to add new functionality
    > > on top of it, but I'd prefer to focus on the "real" power domains support
    > > first (which I think should be done in a generic way too).
    > >
    > Multiple clocks is not new functionality, it's the common case for the
    > bulk of the platforms, and something that is already presently handled.

    OK

    > > The plan is to share as much code as it makes sense between platforms and
    > > architectures.
    >
    > An admirable plan, but the framework needs to be able to provide at least
    > the current required level of functionality in order for it to be
    > adopted, too.

    Sure.

    > On Mon, Apr 18, 2011 at 09:57:28PM +0200, Rafael J. Wysocki wrote:
    > > @@ -24,23 +24,18 @@
    > > #ifdef CONFIG_PM_RUNTIME
    > > static int omap1_pm_runtime_suspend(struct device *dev)
    > > {
    > > - struct clk *iclk, *fclk;
    > > - int ret = 0;
    > > + int ret;
    > >
    > > dev_dbg(dev, "%s\n", __func__);
    > >
    > > ret = pm_generic_runtime_suspend(dev);
    > > + if (ret)
    > > + return ret;
    > >
    > > - fclk = clk_get(dev, "fck");
    > > - if (!IS_ERR(fclk)) {
    > > - clk_disable(fclk);
    > > - clk_put(fclk);
    > > - }
    > > -
    > > - iclk = clk_get(dev, "ick");
    > > - if (!IS_ERR(iclk)) {
    > > - clk_disable(iclk);
    > > - clk_put(iclk);
    > > + ret = pm_runtime_clock_suspend(dev);
    > > + if (ret) {
    > > + pm_generic_runtime_resume(dev);
    > > + return ret;
    > > }
    > >
    > > return 0;
    >
    > The before and after changes here are not functionally equivalent. You've
    > gone from an explicit multi-clock scheme to a single encapsulated one via
    > pm_runtime_clock_suspend().

    You're refferring to the OMAP changes I suppose?

    > Almost every single SH IP block is likewise abstracted in to a function
    > and interface clock, and OMAP and others are where we modelled this
    > abstraction on top of in the first place, so there are certainly users
    > there too.

    In fact, the shmobile runtime PM code in arch/arm/mach-shmobile/pm_runtime.c
    uses only one clock right now.

    Or am I missing anything?

    Rafael


    \
     
     \ /
      Last update: 2011-04-20 00:13    [W:3.658 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site