Re: [PATCH 6/6] clk: qcom: Fix APSS PLL and RCG Configuration

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

 



Thanks for taking time to review the patch!

On 1/13/2023 9:47 PM, Robert Marko wrote:

On 13. 01. 2023. 16:20, Konrad Dybcio wrote:

On 13.01.2023 15:36, devi priya wrote:
Included CLK_IS_CRITICAL flag which helps to properly enable
the APSS PLL during bootup.
Please describe the issue and not only the user-visible impact it
makes. Does the PLL get shut down by clk_ignore_unused? Maybe you
would be interested in the sync_state changes that landed in recent
-next that may solve it for you?

I don't think it should be always-on, as you have an alternate source
for low power modes, adding CLK_IS_CRITICAL will keep the PLL enabled
even if you're not using it.

I have the same opinion, as this is working fine on IPQ6018 and IPQ8074
and I have not experienced any issues with it.

Sure, will drop the critical flag

clk_rcg2_ops should be used for APSS clock RCG, as other ops
will not configure the RCG register
RCG register meaning RCG register*s*, meaning in this case M/N/D
which would be required for proper rate setting and not only input
switching (which arguably doesn't seem to be of much concern on a
single-parent clock)? This all is not obvious..
Same question from me as well, why do you need clk_rcg2_ops with
a dummy frequency table since this is just a mux using RCG2 control
bits?

Regards,
Robert

The RCGR is used for source selection whereas the rate setting is done by configuring the PLL. The source is configured in the RCGR using the source entry (P_APSS_PLL_EARLY) added to the frequency table.

Konrad
Co-developed-by: Praveenkumar I <quic_ipkumar@xxxxxxxxxxx>
Signed-off-by: Praveenkumar I <quic_ipkumar@xxxxxxxxxxx>
Signed-off-by: devi priya <quic_devipriy@xxxxxxxxxxx>
---
  drivers/clk/qcom/apss-ipq-pll.c | 1 +
  drivers/clk/qcom/apss-ipq6018.c | 8 +++++++-
  2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index dd0c01bf5a98..75486a124fcd 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -33,6 +33,7 @@ static struct clk_alpha_pll ipq_pll = {
              },
              .num_parents = 1,
              .ops = &clk_alpha_pll_huayra_ops,
+            .flags = CLK_IS_CRITICAL,
          },
      },
  };
diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
index f2f502e2d5a4..0d0e7196a4dc 100644
--- a/drivers/clk/qcom/apss-ipq6018.c
+++ b/drivers/clk/qcom/apss-ipq6018.c
@@ -33,15 +33,21 @@ static const struct parent_map parents_apcs_alias0_clk_src_map[] = {
      { P_APSS_PLL_EARLY, 5 },
  };
+static const struct freq_tbl ftbl_apcs_alias0_clk_src[] = {
+    { .src = P_APSS_PLL_EARLY, .pre_div = 1 },
+    { }
+};
+
  static struct clk_rcg2 apcs_alias0_clk_src = {
      .cmd_rcgr = 0x0050,
+    .freq_tbl = ftbl_apcs_alias0_clk_src,
      .hid_width = 5,
      .parent_map = parents_apcs_alias0_clk_src_map,
      .clkr.hw.init = &(struct clk_init_data){
          .name = "apcs_alias0_clk_src",
          .parent_data = parents_apcs_alias0_clk_src,
          .num_parents = ARRAY_SIZE(parents_apcs_alias0_clk_src),
-        .ops = &clk_rcg2_mux_closest_ops,
+        .ops = &clk_rcg2_ops,
          .flags = CLK_SET_RATE_PARENT,
      },
  };

Best Regards,
Devi Priya



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux