On 11/01/2023 23:03, Konrad Dybcio wrote:
On 11.01.2023 20:19, Dmitry Baryshkov wrote:
- Do not use the Alt PLL completely. Switch to smux when necessary to
prevent overvolting
Is this empirical evidence, or did Qualcomm recommendations change since
msm-3.18 was released?
I think this is what they are doing, see
https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/blob/LA.UM.7.5.r1-05300-8x96.0/drivers/clk/msm/clock-cpu-8996.c#L675
They switch altpll frequency for whatever reasons, then they do the
dance of switching the parent rate to half rate if necessary and then
they finally switch the parent's rate to the target rate. That's the
only way I can interpret the cpu_clk_8996_set_rate().
https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/blob/LA.UM.7.5.r1-05300-8x96.0/drivers/clk/msm/clock-cpu-8996.c#L675
- Restore the parent in case the rate change aborts for some reason
- Do not duplicate resetting the parent in set_parent operation.
These sound good.
Konrad
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/clk/qcom/clk-cpu-8996.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
index 7e5246ca7e7f..ee7e18b37832 100644
--- a/drivers/clk/qcom/clk-cpu-8996.c
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -506,27 +506,34 @@ static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
{
struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_nb(nb);
struct clk_notifier_data *cnd = data;
- int ret;
switch (event) {
case PRE_RATE_CHANGE:
- ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, ALT_INDEX);
qcom_cpu_clk_msm8996_acd_init(base);
+
+ /*
+ * Avoid overvolting. clk_core_set_rate_nolock() walks from top
+ * to bottom, so it will change the rate of the PLL before
+ * chaging the parent of PMUX. This can result in pmux getting
+ * clocked twice the expected rate.
+ *
+ * Manually switch to PLL/2 here.
+ */
+ if (cnd->new_rate < DIV_2_THRESHOLD &&
+ cnd->old_rate > DIV_2_THRESHOLD)
+ clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, SMUX_INDEX);
+
break;
- case POST_RATE_CHANGE:
- if (cnd->new_rate < DIV_2_THRESHOLD)
- ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw,
- SMUX_INDEX);
- else
- ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw,
- ACD_INDEX);
- break;
+ case ABORT_RATE_CHANGE:
+ /* Revert manual change */
+ if (cnd->new_rate < DIV_2_THRESHOLD &&
+ cnd->old_rate > DIV_2_THRESHOLD)
+ clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, ACD_INDEX);
default:
- ret = 0;
break;
}
- return notifier_from_errno(ret);
+ return NOTIFY_OK;
};
static int qcom_cpu_clk_msm8996_driver_probe(struct platform_device *pdev)
--
With best wishes
Dmitry
