Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756155Ab1DSWVG (ORCPT ); Tue, 19 Apr 2011 18:21:06 -0400 Received: from linux-sh.org ([111.68.239.195]:35018 "EHLO linux-sh.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753862Ab1DSWVE (ORCPT ); Tue, 19 Apr 2011 18:21:04 -0400 Date: Wed, 20 Apr 2011 07:20:04 +0900 From: Paul Mundt To: "Rafael J. Wysocki" Cc: Magnus Damm , Linux PM mailing list , Kevin Hilman , LKML , Grant Likely , Len Brown , linux-sh@vger.kernel.org, Alan Stern , Greg KH Subject: Re: [Update][PATCH 7/9] PM / Runtime: Add generic clock manipulation rountines for runtime PM Message-ID: <20110419222004.GA9664@linux-sh.org> References: <201104130205.26988.rjw@sisk.pl> <201104192342.26670.rjw@sisk.pl> <20110419215944.GA10031@linux-sh.org> <201104200010.50656.rjw@sisk.pl> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201104200010.50656.rjw@sisk.pl> User-Agent: Mutt/1.4.1i Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4188 Lines: 103 On Wed, Apr 20, 2011 at 12:10:50AM +0200, Rafael J. Wysocki wrote: > 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? Yes, but we have similar use cases on SH, too. > > 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. > That's more due to general laziness than design. The code in arch/sh/kernel/cpu/shmobile/pm_runtime.c goes through a hwblk abstraction that functionally maps out for some CPUs via one API what is the function clock on other CPUs. The hwblk API was never carried over to the ARM side, and so a simplistic single clock case was implemented instead, and the drivers with multiple clocks all performed manual clock gating on their multiple clocks outside of the context of runtime PM. OMAP1 clearly has a demonstratable case for multiple clocks that are runtime PM managed, and this is exactly the sort of use case that SH requires, too. If we can migrate off of and kill off some short-lived ill-conceived APIs in the process, great. IOW, if you solve the OMAP1 problem then we can easily fix up ARM SH/R-Mobile and regular SH parts to comply uniformly. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/