Re: [PATCH 07/13] clk: qcom: clk-alpha-pll: Add support to skip PLL configuration

Konrad Dybcio
2026-04-30T15:44:09Z

On 4/28/26 7:22 PM, Jagadeesh Kona wrote:
>
>
> On 4/23/2026 4:43 PM, Konrad Dybcio wrote:
>> On 4/22/26 8:28 PM, Dmitry Baryshkov wrote:
>>> On Mon, Apr 20, 2026 at 09:59:00PM +0530, Jagadeesh Kona wrote:
>>>> Some PLLs are already configured as part of CRM(CESTA Resource
>>>> manager) initialization. Add support to skip PLL reconfiguration
>>>> for such PLLs that are already configured.
>>>>
>>>> Signed-off-by: Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>
>>>> ---
>>>> drivers/clk/qcom/clk-alpha-pll.c | 8 +++++++-
>>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
>>>> index 67fc97739d0d4c26aec0bac5d43d1b87d297bc6a..2f4ebf4d3884b92c981dbe0e67245704a88881ad 100644
>>>> --- a/drivers/clk/qcom/clk-alpha-pll.c
>>>> +++ b/drivers/clk/qcom/clk-alpha-pll.c
>>>> @@ -2332,7 +2332,7 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_zonda_ops);
>>>> void clk_lucid_evo_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
>>>> const struct alpha_pll_config *config)
>>>> {
>>>> - u32 lval = config->l;
>>>> + u32 lval = config->l, regval;
>>>>
>>>> /*
>>>> * If the bootloader left the PLL enabled it's likely that there are
>>>> @@ -2343,6 +2343,12 @@ void clk_lucid_evo_pll_configure(struct clk_alpha_pll *pll, struct regmap *regma
>>>> return;
>>>> }
>>>>
>>>> + /* Return early if PLL is already configured */
>>>> + regmap_read(regmap, PLL_L_VAL(pll), &regval);
>>>> + regval &= LUCID_EVO_PLL_L_VAL_MASK;
>>>> + if (regval)
>>>> + return;
>>>> +
>>>
>>> Why is it being applied only to Lucid EVO PLLs?
>>
>
> Thanks Dmitry and Konrad for your reviews.
>
> This is the function used to configure all Taycan PLLs, currently all the PLLs
> configured during CESTA initialization belong to Taycan type only. I will recheck
> if similar logic is required for any additional PLL types also.
>
>
>> These clocks already have a an .is_enabled() callback, could that be
>> treated as equivalent?
>>
>
> We already have is_enabled check to avoid configuring PLL's that are already enabled.
> There can be case where PLL is configured from bootloader but not enabled during bootup.
> This check avoids re-configuring such PLLs that are already configured by bootloader but
> not enabled.

Okay, that was the missing piece.

Is this a micro-optimization, or something highly necessary (for e.g.
glitch-free display)?

Konrad