On 1/31/2023 3:10 PM, Dmitry Baryshkov wrote:
On 31/01/2023 11:17, Devi Priya wrote:Thanks for taking time to review the patch On 1/13/2023 8:50 PM, 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.Yeah, got it. Will drop the critical flagThe source selection is done by configuring the RCGR config register with the source entry (P_APSS_PLL_EARLY) added to the frequency table. Proper rate is achieved by configuring the PLL and hence M/N/D values are not configuredclk_rcg2_ops should be used for APSS clock RCG, as other ops will not configure the RCG registerRCG 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.. KonradBut the clk_rcg2_mux_closest_ops also programs the parent for the clock. So from your description it isn't obvious what is wrong with the current _ops used for the clock.
Okay, understood & agreed. Will re-verify it once and update it accordingly in V2
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.cindex 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.cindex 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
Best Regards, Devi Priya