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@xxxxxxxxxx>
> ---
> 
> 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
>