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
>
