Re: [PATCH 09/13] clk: qcom: cpu-8996: fix PLL configuration sequence

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/01/2023 16:32, Konrad Dybcio wrote:


On 11.01.2023 23:05, Dmitry Baryshkov wrote:
On 11/01/2023 23:08, Konrad Dybcio wrote:


On 11.01.2023 20:20, Dmitry Baryshkov wrote:
Switch both power and performance clocks to the GPLL0/2 (sys_apcs_aux)
before PLL configuration. Switch them to the ACD afterwards.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
   drivers/clk/qcom/clk-cpu-8996.c | 14 ++++++++++++++
   1 file changed, 14 insertions(+)

diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
index 571ed52b3026..47c58bb5f21a 100644
--- a/drivers/clk/qcom/clk-cpu-8996.c
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -432,13 +432,27 @@ static int qcom_cpu_clk_msm8996_register_clks(struct device *dev,
   {
       int i, ret;
   +    /* Select GPLL0 for 300MHz for the both clusters */
superfluous 'the'

+    regmap_write(regmap, PERFCL_REG_OFFSET + MUX_OFFSET, 0xc);
+    regmap_write(regmap, PWRCL_REG_OFFSET + MUX_OFFSET, 0xc);
+
+    /* Ensure write goes through before PLLs are reconfigured */
+    udelay(5);
Is this value based on n clock cycles, or 'good enough'?

Don't know, this is based on downstream direclty.
Right, I see it now.



+
       clk_alpha_pll_configure(&pwrcl_pll, regmap, &hfpll_config);
       clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config);
       clk_alpha_pll_configure(&pwrcl_alt_pll, regmap, &altpll_config);
       clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config);
   +    /* Wait for PLL(s) to lock */
+        udelay(50);
Weird indentation

Fixing for v2.


Maybe wait_for_pll_enable_lock() to be super sure?

Does it work for HWFSM PLLs?
Not sure, but wait_for_pll_update_ack_clear() should, since it's
called by

I'd prefer to keep it as is. First, this seems to be the difference between normal and hwfsm PLLs, see clk_alpha_pll_is_enabled() vs clk_alpha_pll_hwfsm_is_enabled(). And second, the wait_for_pll() function is not exported from the clk-alpha-pll.c. Note, that downstream also does sleep instead of waiting.


clk_alpha_pll_hwfsm_set_rate() ->
   __clk_alpha_pll_set_rate() ->
     clk_alpha_pll_update_latch() ->
       __clk_alpha_pll_update_latch()

Konrad


+
       qcom_cpu_clk_msm8996_acd_init(regmap);
   +    /* Switch clusters to use the ACD leg */
+    regmap_write(regmap, PWRCL_REG_OFFSET + MUX_OFFSET, 0x2);
+    regmap_write(regmap, PERFCL_REG_OFFSET + MUX_OFFSET, 0x2);
+
No delays here?

No. Probably it isn't required since there is no additional PLL locking, etc.


Konrad
       for (i = 0; i < ARRAY_SIZE(cpu_msm8996_hw_clks); i++) {
           ret = devm_clk_hw_register(dev, cpu_msm8996_hw_clks[i]);
           if (ret)


--
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux