| Message ID | 20230913-gpll_cleanup-v2-1-c8ceb1a37680@quicinc.com |
|---|---|
| State | Accepted |
| Commit | e641a070137dd959932c7c222e000d9d941167a2 |
| Headers | show |
| Series | [v2,01/11] clk: qcom: ipq8074: drop the CLK_SET_RATE_PARENT flag from PLL clocks | expand |
Quoting Konrad Dybcio (2023-09-15 05:19:56) > On 14.09.2023 08:59, Kathiravan Thirumoorthy wrote: > > GPLL, NSS crypto PLL clock rates are fixed and shouldn't be scaled based > > on the request from dependent clocks. Doing so will result in the > > unexpected behaviour. So drop the CLK_SET_RATE_PARENT flag from the PLL > > clocks. > > > > Cc: stable@vger.kernel.org > > Fixes: b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s") > > Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> > > --- > Stephen, do you think there should be some sort of error > or at least warning thrown when SET_RATE_PARENT is used with > RO ops? > Sure? How would that be implemented?
On 10/19/23 02:16, Stephen Boyd wrote: > Quoting Konrad Dybcio (2023-09-15 05:19:56) >> On 14.09.2023 08:59, Kathiravan Thirumoorthy wrote: >>> GPLL, NSS crypto PLL clock rates are fixed and shouldn't be scaled based >>> on the request from dependent clocks. Doing so will result in the >>> unexpected behaviour. So drop the CLK_SET_RATE_PARENT flag from the PLL >>> clocks. >>> >>> Cc: stable@vger.kernel.org >>> Fixes: b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s") >>> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> >>> --- >> Stephen, do you think there should be some sort of error >> or at least warning thrown when SET_RATE_PARENT is used with >> RO ops? >> > > Sure? How would that be implemented? drivers/clk/clk.c : static void clk_change_rate() if (!skip_set_rate && core->ops->set_rate) core->ops->set_rate(core->hw, core->new_rate, best_parent_rate); -> if (!skip_set_rate) { if (core->ops->set_rate) core->ops->set_rate(core->hw, core->new_rate, best_parent_rate); else pr_err("bad idea"); } Konrad
diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c index 63ac2ced76bb..b7faf12a511a 100644 --- a/drivers/clk/qcom/gcc-ipq8074.c +++ b/drivers/clk/qcom/gcc-ipq8074.c @@ -75,7 +75,6 @@ static struct clk_fixed_factor gpll0_out_main_div2 = { &gpll0_main.clkr.hw }, .num_parents = 1, .ops = &clk_fixed_factor_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -121,7 +120,6 @@ static struct clk_alpha_pll_postdiv gpll2 = { &gpll2_main.clkr.hw }, .num_parents = 1, .ops = &clk_alpha_pll_postdiv_ro_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -154,7 +152,6 @@ static struct clk_alpha_pll_postdiv gpll4 = { &gpll4_main.clkr.hw }, .num_parents = 1, .ops = &clk_alpha_pll_postdiv_ro_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -188,7 +185,6 @@ static struct clk_alpha_pll_postdiv gpll6 = { &gpll6_main.clkr.hw }, .num_parents = 1, .ops = &clk_alpha_pll_postdiv_ro_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -201,7 +197,6 @@ static struct clk_fixed_factor gpll6_out_main_div2 = { &gpll6_main.clkr.hw }, .num_parents = 1, .ops = &clk_fixed_factor_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -266,7 +261,6 @@ static struct clk_alpha_pll_postdiv nss_crypto_pll = { &nss_crypto_pll_main.clkr.hw }, .num_parents = 1, .ops = &clk_alpha_pll_postdiv_ro_ops, - .flags = CLK_SET_RATE_PARENT, }, };
GPLL, NSS crypto PLL clock rates are fixed and shouldn't be scaled based on the request from dependent clocks. Doing so will result in the unexpected behaviour. So drop the CLK_SET_RATE_PARENT flag from the PLL clocks. Cc: stable@vger.kernel.org Fixes: b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s") Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> --- Changes in V2: - Include the stable mailing list - Keep the CLK_SET_RATE_PARENT in UBI32 PLL, looks like these PLL rates can be changed. So don't drop the flag. --- drivers/clk/qcom/gcc-ipq8074.c | 6 ------ 1 file changed, 6 deletions(-)