On 3/14/2025 5:09 AM, Bjorn Andersson wrote:
> On Thu, Mar 13, 2025 at 12:29:38PM +0530, Taniya Das wrote:
>> The alpha PLLs which slew to a new frequency at runtime would require
>> the PLL to calibrate at the mid point of the VCO. Add the new PLL ops
>> which can support the slewing of the PLL to a new frequency.
>>
>> Reviewed-by: Imran Shaik <quic_imrashai@xxxxxxxxxxx>
>> Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx>
>> ---
>> drivers/clk/qcom/clk-alpha-pll.c | 170 +++++++++++++++++++++++++++++++++++++++
>> drivers/clk/qcom/clk-alpha-pll.h | 1 +
>> 2 files changed, 171 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
>> index cec0afea8e446010f0d4140d4ef63121706dde47..7d784db8b7441e886d94ded1d3e3258dda46674c 100644
>> --- a/drivers/clk/qcom/clk-alpha-pll.c
>> +++ b/drivers/clk/qcom/clk-alpha-pll.c
>> @@ -2960,3 +2960,173 @@ const struct clk_ops clk_alpha_pll_regera_ops = {
>> .set_rate = clk_zonda_pll_set_rate,
>> };
>> EXPORT_SYMBOL_GPL(clk_alpha_pll_regera_ops);
>> +
>> +static int clk_alpha_pll_slew_update(struct clk_alpha_pll *pll)
>> +{
>> + int ret;
>> + u32 val;
>> +
>> + regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), PLL_UPDATE, PLL_UPDATE);
>> + regmap_read(pll->clkr.regmap, PLL_MODE(pll), &val);
>> +
>> + ret = wait_for_pll_update(pll);
>> + if (ret)
>> + return ret;
>> + /*
>> + * Hardware programming mandates a wait of at least 570ns before polling the LOCK
>> + * detect bit. Have a delay of 1us just to be safe.
>> + */
>> + mb();
>> + udelay(1);
>> +
>> + return wait_for_pll_enable_lock(pll);
>> +}
>> +
>> +static int clk_alpha_pll_slew_set_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long parent_rate)
>> +{
>> + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
>> + unsigned long freq_hz;
>> + const struct pll_vco *curr_vco, *vco;
>> + u32 l, alpha_width = pll_alpha_width(pll);
>> + u64 a;
>> +
>> + freq_hz = alpha_pll_round_rate(rate, parent_rate, &l, &a, alpha_width);
>
> Double space here.
Sure, needs a fix.
>
>> + if (freq_hz != rate) {
>> + pr_err("alpha_pll: Call clk_set_rate with rounded rates!\n");
>> + return -EINVAL;
>> + }
>> +
>> + curr_vco = alpha_pll_find_vco(pll, clk_hw_get_rate(hw));
>> + if (!curr_vco) {
>> + pr_err("alpha pll: not in a valid vco range\n");
>> + return -EINVAL;
>> + }
>> +
>> + vco = alpha_pll_find_vco(pll, freq_hz);
>> + if (!vco) {
>> + pr_err("alpha pll: not in a valid vco range\n");
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * Dynamic pll update will not support switching frequencies across
>> + * vco ranges. In those cases fall back to normal alpha set rate.
>> + */
>> + if (curr_vco->val != vco->val)
>> + return clk_alpha_pll_set_rate(hw, rate, parent_rate);
>> +
>> + a = a << (ALPHA_REG_BITWIDTH - ALPHA_BITWIDTH);
>
> Above this function is written to deal with both alpha bitwidths, but
> here it's assumed to only be one of the cases.
>
> It would be nice to get this cleaned up somehow, because we now have
> this shift 6 times in slightly different forms.
>
I will check if I can clean up.
>> +
>> + regmap_write(pll->clkr.regmap, PLL_L_VAL(pll), l);
>> + regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a);
>> + regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL_U(pll), a >> 32);
>
> In a number of places in the driver alpha_width is compared to 32 bits
> to see if this should be written or not. Perhaps that's not applicable
> here, but again, if so then why is it dynamic above?
>
>
> Also, how about upper_32_bits() and lower_32_bits() to make it clear
> what's going on here?
>
Sure.
>> +
>> + /* Ensure that the write above goes through before proceeding. */
>
> That's not what mb() does.
>
> Regards,
> Bjorn
>
>> + mb();
>> +
>> + if (clk_hw_is_enabled(hw))
>> + return clk_alpha_pll_slew_update(pll);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Slewing plls should be bought up at frequency which is in the middle of the
>> + * desired VCO range. So after bringing up the pll at calibration freq, set it
>> + * back to desired frequency(that was set by previous clk_set_rate).
>> + */
>> +static int clk_alpha_pll_calibrate(struct clk_hw *hw)
>> +{
>> + unsigned long calibration_freq, freq_hz;
>> + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
>> + struct clk_hw *parent;
>> + const struct pll_vco *vco;
>> + u32 l, alpha_width = pll_alpha_width(pll);
>> + int rc;
>> + u64 a;
>> +
>> + parent = clk_hw_get_parent(hw);
>> + if (!parent) {
>> + pr_err("alpha pll: no valid parent found\n");
>> + return -EINVAL;
>> + }
>> +
>> + vco = alpha_pll_find_vco(pll, clk_hw_get_rate(hw));
>> + if (!vco) {
>> + pr_err("alpha pll: not in a valid vco range\n");
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * As during slewing plls vco_sel won't be allowed to change, vco table
>> + * should have only one entry table, i.e. index = 0, find the
>> + * calibration frequency.
>> + */
>> + calibration_freq = (pll->vco_table[0].min_freq + pll->vco_table[0].max_freq) / 2;
>> +
>> + freq_hz = alpha_pll_round_rate(calibration_freq, clk_hw_get_rate(parent),
>> + &l, &a, alpha_width);
>> + if (freq_hz != calibration_freq) {
>> + pr_err("alpha_pll: call clk_set_rate with rounded rates!\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* Setup PLL for calibration frequency */
>> + a <<= (ALPHA_REG_BITWIDTH - ALPHA_BITWIDTH);
>> +
>> + regmap_write(pll->clkr.regmap, PLL_L_VAL(pll), l);
>> + regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a);
>> + regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL_U(pll), a >> 32);
>> +
>> + regmap_update_bits(pll->clkr.regmap, PLL_USER_CTL(pll), PLL_VCO_MASK << PLL_VCO_SHIFT,
>> + vco->val << PLL_VCO_SHIFT);
>> +
>> + /* Bringup the pll at calibration frequency */
>> + rc = clk_alpha_pll_enable(hw);
>> + if (rc) {
>> + pr_err("alpha pll calibration failed\n");
>> + return rc;
>> + }
>> +
>> + /*
>> + * PLL is already running at calibration frequency.
>> + * So slew pll to the previously set frequency.
>> + */
>> + freq_hz = alpha_pll_round_rate(clk_hw_get_rate(hw),
>> + clk_hw_get_rate(parent), &l, &a, alpha_width);
>> +
>> + pr_debug("pll %s: setting back to required rate %lu, freq_hz %ld\n",
>> + clk_hw_get_name(hw), clk_hw_get_rate(hw), freq_hz);
>> +
>> + /* Setup the PLL for the new frequency */
>> + a <<= (ALPHA_REG_BITWIDTH - ALPHA_BITWIDTH);
>> +
>> + regmap_write(pll->clkr.regmap, PLL_L_VAL(pll), l);
>> + regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a);
>> + regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL_U(pll), a >> 32);
>> +
>> + regmap_update_bits(pll->clkr.regmap, PLL_USER_CTL(pll), PLL_ALPHA_EN, PLL_ALPHA_EN);
>> +
>> + return clk_alpha_pll_slew_update(pll);
>> +}
>> +
>> +static int clk_alpha_pll_slew_enable(struct clk_hw *hw)
>> +{
>> + int rc;
>> +
>> + rc = clk_alpha_pll_calibrate(hw);
>> + if (rc)
>> + return rc;
>> +
>> + return clk_alpha_pll_enable(hw);
>> +}
>> +
>> +const struct clk_ops clk_alpha_pll_slew_ops = {
>> + .enable = clk_alpha_pll_slew_enable,
>> + .disable = clk_alpha_pll_disable,
>> + .recalc_rate = clk_alpha_pll_recalc_rate,
>> + .round_rate = clk_alpha_pll_round_rate,
>> + .set_rate = clk_alpha_pll_slew_set_rate,
>> +};
>> +EXPORT_SYMBOL(clk_alpha_pll_slew_ops);
>> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
>> index 79aca8525262211ae5295245427d4540abf1e09a..1d19001605eb10fd8ae8041c56d951e928cbbe9f 100644
>> --- a/drivers/clk/qcom/clk-alpha-pll.h
>> +++ b/drivers/clk/qcom/clk-alpha-pll.h
>> @@ -204,6 +204,7 @@ extern const struct clk_ops clk_alpha_pll_rivian_evo_ops;
>> #define clk_alpha_pll_postdiv_rivian_evo_ops clk_alpha_pll_postdiv_fabia_ops
>>
>> extern const struct clk_ops clk_alpha_pll_regera_ops;
>> +extern const struct clk_ops clk_alpha_pll_slew_ops;
>>
>> void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
>> const struct alpha_pll_config *config);
>>
>> --
>> 2.48.1
>>
