Re: [PATCH v2 1/5] clk: qcom: cmnpll: Account for reference clock divider
From: Konrad Dybcio
Date: Wed Jan 07 2026 - 07:16:51 EST
On 1/7/26 6:35 AM, Luo Jie wrote:
> The clk_cmn_pll_recalc_rate() function must account for the reference clock
> divider programmed in CMN_PLL_REFCLK_CONFIG. Without this fix, platforms
> with a reference divider other than 1 calculate incorrect CMN PLL rates.
> For example, on IPQ5332 where the reference divider is 2, the computed rate
> becomes twice the actual output.
>
> Read CMN_PLL_REFCLK_DIV and divide the parent rate by this value before
> applying the 2 * FACTOR scaling. This yields the correct rate calculation:
> rate = (parent_rate / ref_div) * 2 * factor.
>
> Maintain backward compatibility with earlier platforms (e.g. IPQ9574,
> IPQ5424, IPQ5018) that use ref_div = 1.
>
> Fixes: f81715a4c87c ("clk: qcom: Add CMN PLL clock controller driver for IPQ SoC")
> Signed-off-by: Luo Jie <jie.luo@xxxxxxxxxxxxxxxx>
> ---
> drivers/clk/qcom/ipq-cmn-pll.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/qcom/ipq-cmn-pll.c b/drivers/clk/qcom/ipq-cmn-pll.c
> index dafbf5732048..369798d1ce42 100644
> --- a/drivers/clk/qcom/ipq-cmn-pll.c
> +++ b/drivers/clk/qcom/ipq-cmn-pll.c
> @@ -185,7 +185,7 @@ static unsigned long clk_cmn_pll_recalc_rate(struct clk_hw *hw,
> unsigned long parent_rate)
> {
> struct clk_cmn_pll *cmn_pll = to_clk_cmn_pll(hw);
> - u32 val, factor;
> + u32 val, factor, ref_div;
>
> /*
> * The value of CMN_PLL_DIVIDER_CTRL_FACTOR is automatically adjusted
> @@ -193,8 +193,15 @@ static unsigned long clk_cmn_pll_recalc_rate(struct clk_hw *hw,
> */
> regmap_read(cmn_pll->regmap, CMN_PLL_DIVIDER_CTRL, &val);
> factor = FIELD_GET(CMN_PLL_DIVIDER_CTRL_FACTOR, val);
> + if (WARN_ON(factor == 0))
> + factor = 1;
FWIW the docs tell me the value of this field is '192' on IPQ5332..
Konrad