Quoting Shawn Guo (2021-05-03 22:28:40)
> The clock source for MSM8916 cpu cores is like below.
>
> |\
> a53pll --------| \ a53mux +------+
> | |------------| cpus |
> gpll0_vote --------| / +------+
> |/
>
> So clock a53mux rather than a53pll is actually the clock source of cpu
> cores. It makes more sense to flag a53mux rather than a53pll as
> critical, since a53pll could be irrelevant if a53mux switches its parent
> clock to be gpll0_vote.
Can you add some more detail here? I think the idea is to mark the mux
as critical so that either a53pll or gpll0_vote is kept enabled, but
only if they're used by the CPU. That isn't very clear from the commit
text. Otherwise it seems OK.
>
> Signed-off-by: Shawn Guo <shawn.guo@xxxxxxxxxx>
> ---
> drivers/clk/qcom/a53-pll.c | 1 -
> drivers/clk/qcom/apcs-msm8916.c | 2 +-
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
> index 45cfc57bff92..8614b0b0e82c 100644
> --- a/drivers/clk/qcom/a53-pll.c
> +++ b/drivers/clk/qcom/a53-pll.c
> @@ -70,7 +70,6 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
> init.parent_names = (const char *[]){ "xo" };
> init.num_parents = 1;
> init.ops = &clk_pll_sr2_ops;
> - init.flags = CLK_IS_CRITICAL;
> pll->clkr.hw.init = &init;
>
> ret = devm_clk_register_regmap(dev, &pll->clkr);
> diff --git a/drivers/clk/qcom/apcs-msm8916.c b/drivers/clk/qcom/apcs-msm8916.c
> index cf69a97d0439..d7ac6d6b15b6 100644
> --- a/drivers/clk/qcom/apcs-msm8916.c
> +++ b/drivers/clk/qcom/apcs-msm8916.c
> @@ -65,7 +65,7 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
> init.parent_data = pdata;
> init.num_parents = ARRAY_SIZE(pdata);
> init.ops = &clk_regmap_mux_div_ops;
> - init.flags = CLK_SET_RATE_PARENT;
> + init.flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT;
>
> a53cc->clkr.hw.init = &init;
> a53cc->clkr.regmap = regmap;
> --
> 2.17.1
>
