Re: [PATCH v2 1/3] clk: qcom: clk-alpha-pll: support a 28-bit ALPHA_VAL width

[Konrad Dybcio]

On 9/23/26 9:21 PM, Taniya Das wrote:
> On some PLLs the PLL_ALPHA_VAL register has been increased to 28 bits to
> carry the fractional word at a finer resolution, for example the
> ZONDA_OLE instances on the Qualcomm Nord DPRX. The alpha-PLL core,
> however, derives the ALPHA_VAL width solely from the register-map layout
> and can only express 16 or 40 bits, so it programs and reads back the
> 28-bit configuration at the wrong scale on both the set_rate/slew and
> recalc_rate paths, making the PLL lock to and report the wrong rate.
> 
> Add a way to identify the wider register: an optional alpha_width field
> in struct clk_alpha_pll that, when non-zero, supersedes the
> register-derived width. This feeds pll_alpha_width(), so the recalc_rate
> and round_rate math scale the fractional word correctly, and the Zonda
> set_rate L-adjust MSB check is derived from the active width instead of
> a hardcoded BIT(15). PLLs that leave the field zero retain the previous
> behaviour.
> 
> Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>
> Signed-off-by: Taniya Das <taniya.das@xxxxxxxxxxxxxxxx>
> ---

Acked-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>

While reviewing this, GPT reports a pre-existing issue, please take a look:

zonda_pll_adjust_l_val() in drivers/clk/qcom/clk-alpha-pll.c writes the
requested frequency into the PLL's integer divider instead of the
calculated quotient. After remainder = do_div(quotient, prate), the
assignment should use quotient, but currently reads
*l = rate + (u32)(remainder * 2 >= prate). For example, a 1.2 GHz request
with a 19.2 MHz parent produces L = 1200000001 instead of 63, which is
then written to PLL_L_VAL. This path is taken when the fractional alpha
word's MSB is set, so it affects existing Zonda users as well as the
proposed 28-bit support.

The regression was introduced by a4d89b11aca3 ("clk: qcom: clk-alpha-pll:
Simplify the zonda_pll_adjust_l_val()"), which accidentally replaced the
quotient with the original rate while simplifying the rounding
condition. Replacing rate with quotient in that assignment restores the
original calculation without reintroducing the division that caused
the ARM32 build failure.

Suggested fix:

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -2271,7 +2271,7 @@ static void zonda_pll_adjust_l_val(unsigned long rate, unsigned long prate, u32 *l)
 	quotient = rate;
 	remainder = do_div(quotient, prate);
 
-	*l = rate + (u32)(remainder * 2 >= prate);
+	*l = quotient + (u32)(remainder * 2 >= prate);
 }

Konrad