[RFC] clk: Allow forcing an unused clock to be disabled

[RFC] clk: Allow forcing an unused clock to be disabled

[Bjorn Andersson]

The process of disabling unused clocks will skip clocks that are not
enabled by Linux, but doesn't implement the is_enabled() callback to
read back the hardware state. In the case that it's determined that the
parent clock is enabled this might be turned off, causing the skipped
clock to lock up.

One such case is the RCG "mdp_clk_src" in the Qualcomm SDM845, which at
boot is parented by "disp_cc_pll0", which during clk_disable_unused() is
left untouched, while the parent is disabled.

Later the typical next operation for "mdp_clk_src" is an
assigned-clock-rates will cause the next enable attempt to reparent the
RCG. But the RCG needs both the old and new parent to be ticking to
perform this switch and will therefor not complete.

Introduce a new flag for clock drivers to mark that clocks that should
be assumed to be enabled even when is_enabled isn't implemented and mark
the "mdp_clk_src" clock for SDM845 as such.
This allows the driver to transition the RCG from the PLL to a clock
source that is known to be enabled when the future reparenting operation
is undertaken.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Naturally this patch should be split in a core and driver part, but for the
sake of the RFC I made both changes in the same patch.

 drivers/clk/clk.c                | 2 +-
 drivers/clk/qcom/dispcc-sdm845.c | 1 +
 include/linux/clk-provider.h     | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 65508eb89ec9..9e4789d106f3 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1265,7 +1265,7 @@ static void __init 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) || core->flags & CLK_ASSUME_ENABLED_BOOT) {
 		trace_clk_disable(core);
 		if (core->ops->disable_unused)
 			core->ops->disable_unused(core->hw);
diff --git a/drivers/clk/qcom/dispcc-sdm845.c b/drivers/clk/qcom/dispcc-sdm845.c
index 735adfefc379..046f7e656f7c 100644
--- a/drivers/clk/qcom/dispcc-sdm845.c
+++ b/drivers/clk/qcom/dispcc-sdm845.c
@@ -267,6 +267,7 @@ static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = {
 		.parent_data = disp_cc_parent_data_3,
 		.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
 		.ops = &clk_rcg2_shared_ops,
+		.flags = CLK_ASSUME_ENABLED_BOOT,
 	},
 };
 
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 162a2e5546a3..d00ca925842c 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -32,6 +32,8 @@
 #define CLK_OPS_PARENT_ENABLE	BIT(12)
 /* duty cycle call may be forwarded to the parent clock */
 #define CLK_DUTY_CYCLE_PARENT	BIT(13)
+/* assume clock is enabled if found unused in late init */
+#define CLK_ASSUME_ENABLED_BOOT	BIT(14)
 
 struct clk;
 struct clk_hw;
-- 
2.29.2

Re: [RFC] clk: Allow forcing an unused clock to be disabled

[Bjorn Andersson]

On Tue 06 Jul 23:38 CDT 2021, Bjorn Andersson wrote:

> The process of disabling unused clocks will skip clocks that are not
> enabled by Linux, but doesn't implement the is_enabled() callback to
> read back the hardware state. In the case that it's determined that the
> parent clock is enabled this might be turned off, causing the skipped
> clock to lock up.
> 
> One such case is the RCG "mdp_clk_src" in the Qualcomm SDM845, which at
> boot is parented by "disp_cc_pll0", which during clk_disable_unused() is
> left untouched, while the parent is disabled.
> 
> Later the typical next operation for "mdp_clk_src" is an
> assigned-clock-rates will cause the next enable attempt to reparent the
> RCG. But the RCG needs both the old and new parent to be ticking to
> perform this switch and will therefor not complete.
> 
> Introduce a new flag for clock drivers to mark that clocks that should
> be assumed to be enabled even when is_enabled isn't implemented and mark
> the "mdp_clk_src" clock for SDM845 as such.
> This allows the driver to transition the RCG from the PLL to a clock
> source that is known to be enabled when the future reparenting operation
> is undertaken.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Naturally this patch should be split in a core and driver part, but for the
> sake of the RFC I made both changes in the same patch.
> 

Ping?

>  drivers/clk/clk.c                | 2 +-
>  drivers/clk/qcom/dispcc-sdm845.c | 1 +
>  include/linux/clk-provider.h     | 2 ++
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 65508eb89ec9..9e4789d106f3 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1265,7 +1265,7 @@ static void __init 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) || core->flags & CLK_ASSUME_ENABLED_BOOT) {
>  		trace_clk_disable(core);
>  		if (core->ops->disable_unused)
>  			core->ops->disable_unused(core->hw);
> diff --git a/drivers/clk/qcom/dispcc-sdm845.c b/drivers/clk/qcom/dispcc-sdm845.c
> index 735adfefc379..046f7e656f7c 100644
> --- a/drivers/clk/qcom/dispcc-sdm845.c
> +++ b/drivers/clk/qcom/dispcc-sdm845.c
> @@ -267,6 +267,7 @@ static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = {
>  		.parent_data = disp_cc_parent_data_3,
>  		.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
>  		.ops = &clk_rcg2_shared_ops,
> +		.flags = CLK_ASSUME_ENABLED_BOOT,
>  	},
>  };
>  
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 162a2e5546a3..d00ca925842c 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -32,6 +32,8 @@
>  #define CLK_OPS_PARENT_ENABLE	BIT(12)
>  /* duty cycle call may be forwarded to the parent clock */
>  #define CLK_DUTY_CYCLE_PARENT	BIT(13)
> +/* assume clock is enabled if found unused in late init */
> +#define CLK_ASSUME_ENABLED_BOOT	BIT(14)
>  
>  struct clk;
>  struct clk_hw;
> -- 
> 2.29.2
>