Re: [PATCH 1/5] clk: qcom: apcs-msm8916: Flag a53mux instead of a53pll as critical

[Stephen Boyd <sboyd@kernel.org>]

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@linaro.org>
> ---
>  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
>