[PATCH] clk: disable unused clk if enable_count == 0

[PATCH] clk: disable unused clk if enable_count == 0

[Jun Nie]

If .is_enabled callback is not implemented, clk_core_is_enabled
will take software enable_count as enable status. If enable_count
is zero, .disable will not be called as software assume clock
is not enabled though clock may be booted with  free running state.
Disable the clock in this situation.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/clk/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index b4db67a..49c5934 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -220,7 +220,7 @@ static void clk_disable_unused_subtree(struct clk_core *core)
 	 * sequence.  call .disable_unused if available, otherwise fall
 	 * back to .disable
 	 */
-	if (clk_core_is_enabled(core)) {
+	if (clk_core_is_enabled(core) || !clk->enable_count) {
 		trace_clk_disable(core);
 		if (core->ops->disable_unused)
 			core->ops->disable_unused(core->hw);
-- 
1.9.1

Re: [PATCH] clk: disable unused clk if enable_count == 0

[Stephen Boyd]

On 04/07/2016 02:15 AM, Jun Nie wrote:
> If .is_enabled callback is not implemented, clk_core_is_enabled
> will take software enable_count as enable status. If enable_count
> is zero, .disable will not be called as software assume clock
> is not enabled though clock may be booted with  free running state.
> Disable the clock in this situation.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---

Why not implement the is_enabled callback then?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] clk: disable unused clk if enable_count == 0

[Jun Nie]

2016-04-08 8:14 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
> On 04/07/2016 02:15 AM, Jun Nie wrote:
>> If .is_enabled callback is not implemented, clk_core_is_enabled
>> will take software enable_count as enable status. If enable_count
>> is zero, .disable will not be called as software assume clock
>> is not enabled though clock may be booted with  free running state.
>> Disable the clock in this situation.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>
> Why not implement the is_enabled callback then?

The is_enabled callback is not mandatory according to sanity check
code, so there is risk that some clock is not gated even we have
called clk_disable_unused_subtree. If is_enabled callback is mandatory
when CLK_IGNORE_UNUSED is not set, Adding a sanity check will fill
this gap.

Jun

>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

Re: [PATCH] clk: disable unused clk if enable_count == 0

[Stephen Boyd]

On 04/08, Jun Nie wrote:
> 2016-04-08 8:14 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
> > On 04/07/2016 02:15 AM, Jun Nie wrote:
> >> If .is_enabled callback is not implemented, clk_core_is_enabled
> >> will take software enable_count as enable status. If enable_count
> >> is zero, .disable will not be called as software assume clock
> >> is not enabled though clock may be booted with  free running state.
> >> Disable the clock in this situation.
> >>
> >> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> >> ---
> >
> > Why not implement the is_enabled callback then?
> 
> The is_enabled callback is not mandatory according to sanity check
> code, so there is risk that some clock is not gated even we have
> called clk_disable_unused_subtree. If is_enabled callback is mandatory
> when CLK_IGNORE_UNUSED is not set, Adding a sanity check will fill
> this gap.

The is_enabled op is technically mandatory if enable/disable ops
are implemented. We could add a sanity check, but I would guess
it will warn quite a bit because not everyone is following the
documentation.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project