Re: [PATCH] clk: export function clk_disable_unused()

["Grygorii.Strashko@linaro.org" <grygorii.strashko@linaro.org>]

On 05/28/2015 05:21 PM, Jim Quinlan wrote:

> We are currently doing [1].  We'd like to change this so that we are
> the same as upstream barring a small syscore driver that will call
> clk_disable_unused() on S2/S3 resume.  My patch reflects the
> suggestion found in [2], which says, "... It could still be generic by
> exporting the clk_disable_unused() function to drivers...".
> 
> We do not have the general problems you are alluding to.  With few
> exceptions, we do not yet use power domains or runtime PM, although of
> course this may change.  Our drivers  use either syscore calls or
> SIMPLE_DEV_PM_OPS() to turn off/on clocks as needed during S2/S3
> suspend/resume.  The problem we have is when the software stack omits
> a driver or we have a new HW block which has no driver.  On boot,
> clk_disable_unused() turns off the clocks that these missing drivers
> would normally use*.  But on resumption of S2/S3, these clocks get
> turned on by HW, and all I am asking is that clk_disable_unused() be
> exported so a driver may invoke it to do the same thing it does during
> boot.

This issue is much more simpler then saving/restoring of clocks state, so
may be it can be solved first.

Taking to the account comments from [2] I think that could be
done inside clk core using PM notifiers, 
and sysfs/module parameter can be used to enable/disable this feature, like:
 PM_POST_SUSPEND:
 PM_POST_HIBERNATION:
 PM_RESTORE_PREPARE:
	if (clk_disable_unused_on_suspend)
		clk_disable_unused();


Mike, What do you think?

> 
> * Why the HW turns these clocks on in the first place is a question I
> cannot answer.
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/311568.html
> [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/311575.html
> 
> On Sat, May 23, 2015 at 3:33 PM, Michael Turquette
> <mturquette@linaro.org> wrote:
>> Quoting Jim Quinlan (2015-05-15 15:22:36)
>>> For Broadcom STB chips, clocks may come up after resume in an "on"
>>> state, so calling clk_disable_unused() again will turn off unused
>>> clocks.  This commit exports clk_disable_unused() so it may be
>>> called in such cases.
>>
>> Jim,
>>
>> Thanks for the patch.
>>
>> I think a more general solution to the problem might be needed. E.g. how
>> do we solve for the opposite case where after a low-power suspend/resume
>> we turn on clocks that were enabled by drivers before suspending, but
>> are disabled out of reset/post-transition?
>>
>> Additionally, it would be helpful to see your driver changes and how
>> exactly you plan to use this newly exported function.
>>
>> I've Cc'd Grygorii Strashko from TI. I faintly recall that he had an
>> out-of-tree solution to add some .suspend/.resume callbacks to struct
>> clk_ops. I didn't like that solution at the time either, but maybe we
>> can figure out a more natural way to handle these cases.
>>
>> (Grygorii: sorry in advance if this is a case of mistaken identity. I
>> could not find the code in question from my email archive)
>>
>> Regards,
>> Mike



-- 
regards,
-grygorii