Re: [PATCH 5/6] clk: qcom: gcc-sdm660: Account for needed adjustments in probe function

From: Stephen Boyd
Date: Wed Mar 31 2021 - 21:58:33 EST


Quoting Konrad Dybcio (2021-02-25 11:09:14)
> Hi and sorry for the late reply,
>

I'm sorry too. This fell off my review queue for some time.

>
> >> +
> >> + /* Keep bimc gfx clock port on all the time */
> >> + clk_prepare_enable(gcc_bimc_gfx_clk.clkr.hw.clk);
> >> +
> > Preferably just set these various bits with regmap_update_bits() during
> > probe. Also, please do it before regsitering the clks, not after.
>
> To be fair, now I think that simply adding CLK_IS_CRITICAL flag to the clocks in question is the smartest thing to do. Magic writes don't tell a whole lot.

This is how it's been done in various other qcom clk drivers. Usually
there is a comment about what is enabled, but really it's just setting
random bits that sadly aren't already set by default.

>
>
> >> + /* Set the HMSS_GPLL0_SRC for 300MHz to CPU subsystem */
> >> + clk_set_rate(hmss_gpll0_clk_src.clkr.hw.clk, 300000000);
> > Is this not already the case?
>
>
> This is a mission-critical clock and we cannot trust the bootloader with setting it. Otherwise dragons might appear.
>

What does the bootloader set it to?