Re: [PATCH 2/3] clk: qcom: clk-alpha-pll: Add Pongo PLL

[Krzysztof Kozlowski]

On 03/12/2024 23:07, Stephen Boyd wrote:
>> -       for (count = 200; count > 0; count--) {
>> +       /* Pongo PLLs using a 32KHz reference can take upwards of 1500us to lock. */
>> +       for (count = 1500; count > 0; count--) {
>>                 ret = regmap_read(pll->clkr.regmap, PLL_MODE(pll), &val);
>>                 if (ret)
>>                         return ret;
>> @@ -378,6 +403,13 @@ static int wait_for_pll(struct clk_alpha_pll *pll, u32 mask, bool inverse,
>>                 udelay(1);
>>         }
>>  
>> +       /* Called with clocks already registered ... */
>> +       if (pll->clkr.hw.core)
>> +               name = clk_hw_get_name(&pll->clkr.hw);
>> +       else
>> +               /* or before registering, when init data is present */
>> +               name = pll->clkr.hw.init->name;
> 
> This is sad. Why can't we enable the PLL from the clk prepare/enable
> path? PLLs are typically calibrated during clk_prepare().

I don't know. I'll move it to prepare() and see what happens.

> 
>> +
>>         WARN(1, "%s failed to %s!\n", name, action);
>>         return -ETIMEDOUT;
>>  }
>> @@ -2524,6 +2556,129 @@ const struct clk_ops clk_alpha_pll_reset_lucid_evo_ops = {
>>  };
>>  EXPORT_SYMBOL_GPL(clk_alpha_pll_reset_lucid_evo_ops);
>>  
>> +static int alpha_pll_pongo_elu_enable(struct clk_hw *hw)
>> +{
>> +       struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
>> +       struct regmap *regmap = pll->clkr.regmap;
>> +       int ret;
>> +
>> +       /* Check if PLL is already enabled */
>> +       if (trion_pll_is_enabled(pll, regmap))
>> +               return 0;
>> +
>> +       ret = regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* Set operation mode to RUN */
>> +       regmap_write(regmap, PLL_OPMODE(pll), PLL_RUN);
>> +
>> +       ret = wait_for_pll_enable_lock(pll);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* Enable the global PLL outputs */
>> +       ret = regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, PLL_OUTCTRL);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* Ensure that the write above goes through before returning. */
>> +       mb();
>> +
>> +       return ret;
>> +}
>> +
>> +static void alpha_pll_pongo_elu_disable(struct clk_hw *hw)
>> +{
>> +       struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
>> +       struct regmap *regmap = pll->clkr.regmap;
>> +       int ret;
>> +
>> +       /* Disable the global PLL output */
>> +       ret = regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, 0);
>> +       if (ret)
>> +               return;
>> +
>> +       /* Place the PLL mode in STANDBY */
>> +       regmap_write(regmap, PLL_OPMODE(pll), PLL_STANDBY);
>> +}
>> +
>> +static unsigned long alpha_pll_pongo_elu_recalc_rate(struct clk_hw *hw,
>> +                                                    unsigned long parent_rate)
>> +{
>> +       struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
>> +       struct regmap *regmap = pll->clkr.regmap;
>> +       u32 l;
>> +
>> +       if (regmap_read(regmap, PLL_L_VAL(pll), &l))
>> +               return 0;
>> +
>> +       l &= PONGO_PLL_L_VAL_MASK;
>> +
>> +       return alpha_pll_calc_rate(parent_rate, l, 0, pll_alpha_width(pll));
>> +}
>> +
>> +const struct clk_ops clk_alpha_pll_pongo_elu_ops = {
>> +       .enable = alpha_pll_pongo_elu_enable,
>> +       .disable = alpha_pll_pongo_elu_disable,
>> +       .recalc_rate = alpha_pll_pongo_elu_recalc_rate,
>> +};
>> +EXPORT_SYMBOL(clk_alpha_pll_pongo_elu_ops);
> 
> GPL please.

Ack (and one more as well)

Best regards,
Krzysztof