Re: [Update][PATCH 7/9] PM / Runtime: Add generic clock manipulation rountines for runtime PM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 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.

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().

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.
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm


[Index of Archives]     [Linux ACPI]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [CPU Freq]     [Kernel Newbies]     [Fedora Kernel]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux