Re: [PATCH 3/7] clk: qcom: clk-alpha-pll: Refactor and cleanup trion PLL

Bjorn Andersson
2020-01-24T18:33:15Z

On Thu 16 Jan 15:39 PST 2020, Venkata Narendra Kumar Gutta wrote:

> From: Taniya Das <tdas@codeaurora.org>
>
> The PLL run and standby modes are similar across the PLLs, thus rename
> and refactor the code accordingly.
>
> Remove duplicate function for calculating the round rate of PLL and also
> update the trion pll ops to use the common function.
>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> Signed-off-by: Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>
> ---
> drivers/clk/qcom/clk-alpha-pll.c | 71 +++++++++++++---------------------------
> 1 file changed, 22 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
> index 7c2936d..1b073b2 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.c
> +++ b/drivers/clk/qcom/clk-alpha-pll.c
> @@ -134,15 +134,10 @@
> #define PLL_HUAYRA_N_MASK 0xff
> #define PLL_HUAYRA_ALPHA_WIDTH 16
>
> -#define FABIA_OPMODE_STANDBY 0x0
> -#define FABIA_OPMODE_RUN 0x1
> -
> -#define FABIA_PLL_OUT_MASK 0x7
> -#define FABIA_PLL_RATE_MARGIN 500
> -
> -#define TRION_PLL_STANDBY 0x0
> -#define TRION_PLL_RUN 0x1
> -#define TRION_PLL_OUT_MASK 0x7
> +#define PLL_STANDBY 0x0
> +#define PLL_RUN 0x1
> +#define PLL_OUT_MASK 0x7
> +#define PLL_RATE_MARGIN 500
>
> #define pll_alpha_width(p) \
> ((PLL_ALPHA_VAL_U(p) - PLL_ALPHA_VAL(p) == 4) ? \
> @@ -765,7 +760,7 @@ static int trion_pll_is_enabled(struct clk_alpha_pll *pll,
> if (ret)
> return 0;
>
> - return ((opmode_regval & TRION_PLL_RUN) && (mode_regval & PLL_OUTCTRL));
> + return ((opmode_regval & PLL_RUN) && (mode_regval & PLL_OUTCTRL));
> }
>
> static int clk_trion_pll_is_enabled(struct clk_hw *hw)
> @@ -795,7 +790,7 @@ static int clk_trion_pll_enable(struct clk_hw *hw)
> }
>
> /* Set operation mode to RUN */
> - regmap_write(regmap, PLL_OPMODE(pll), TRION_PLL_RUN);
> + regmap_write(regmap, PLL_OPMODE(pll), PLL_RUN);
>
> ret = wait_for_pll_enable_lock(pll);
> if (ret)
> @@ -803,7 +798,7 @@ static int clk_trion_pll_enable(struct clk_hw *hw)
>
> /* Enable the PLL outputs */
> ret = regmap_update_bits(regmap, PLL_USER_CTL(pll),
> - TRION_PLL_OUT_MASK, TRION_PLL_OUT_MASK);
> + PLL_OUT_MASK, PLL_OUT_MASK);
> if (ret)
> return ret;
>
> @@ -836,12 +831,12 @@ static void clk_trion_pll_disable(struct clk_hw *hw)
>
> /* Disable the PLL outputs */
> ret = regmap_update_bits(regmap, PLL_USER_CTL(pll),
> - TRION_PLL_OUT_MASK, 0);
> + PLL_OUT_MASK, 0);
> if (ret)
> return;
>
> /* Place the PLL mode in STANDBY */
> - regmap_write(regmap, PLL_OPMODE(pll), TRION_PLL_STANDBY);
> + regmap_write(regmap, PLL_OPMODE(pll), PLL_STANDBY);
> regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N);
> }
>
> @@ -849,33 +844,12 @@ static void clk_trion_pll_disable(struct clk_hw *hw)
> clk_trion_pll_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, frac;
> - u64 prate = parent_rate;
> -
> - regmap_read(regmap, PLL_L_VAL(pll), &l);
> - regmap_read(regmap, PLL_ALPHA_VAL(pll), &frac);
> -
> - return alpha_pll_calc_rate(prate, l, frac, ALPHA_REG_16BIT_WIDTH);
> -}
> -
> -static long clk_trion_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> - unsigned long *prate)
> -{
> - struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
> - unsigned long min_freq, max_freq;
> - u32 l;
> - u64 a;
> -
> - rate = alpha_pll_round_rate(rate, *prate,
> - &l, &a, ALPHA_REG_16BIT_WIDTH);
> - if (!pll->vco_table || alpha_pll_find_vco(pll, rate))
> - return rate;
> + u32 l, frac, alpha_width = pll_alpha_width(pll);
>
> - min_freq = pll->vco_table[0].min_freq;
> - max_freq = pll->vco_table[pll->num_vco - 1].max_freq;
> + regmap_read(pll->clkr.regmap, PLL_L_VAL(pll), &l);
> + regmap_read(pll->clkr.regmap, PLL_ALPHA_VAL(pll), &frac);
>
> - return clamp(rate, min_freq, max_freq);
> + return alpha_pll_calc_rate(parent_rate, l, frac, alpha_width);
> }
>
> const struct clk_ops clk_alpha_pll_fixed_ops = {
> @@ -921,7 +895,7 @@ static long clk_trion_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> .disable = clk_trion_pll_disable,
> .is_enabled = clk_trion_pll_is_enabled,
> .recalc_rate = clk_trion_pll_recalc_rate,
> - .round_rate = clk_trion_pll_round_rate,
> + .round_rate = clk_alpha_pll_round_rate,
> };
> EXPORT_SYMBOL_GPL(clk_trion_fixed_pll_ops);
>
> @@ -1088,14 +1062,14 @@ static int alpha_pll_fabia_enable(struct clk_hw *hw)
> return ret;
>
> /* Skip If PLL is already running */
> - if ((opmode_val & FABIA_OPMODE_RUN) && (val & PLL_OUTCTRL))
> + if ((opmode_val & PLL_RUN) && (val & PLL_OUTCTRL))
> return 0;
>
> ret = regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, 0);
> if (ret)
> return ret;
>
> - ret = regmap_write(regmap, PLL_OPMODE(pll), FABIA_OPMODE_STANDBY);
> + ret = regmap_write(regmap, PLL_OPMODE(pll), PLL_STANDBY);
> if (ret)
> return ret;
>
> @@ -1104,7 +1078,7 @@ static int alpha_pll_fabia_enable(struct clk_hw *hw)
> if (ret)
> return ret;
>
> - ret = regmap_write(regmap, PLL_OPMODE(pll), FABIA_OPMODE_RUN);
> + ret = regmap_write(regmap, PLL_OPMODE(pll), PLL_RUN);
> if (ret)
> return ret;
>
> @@ -1113,7 +1087,7 @@ static int alpha_pll_fabia_enable(struct clk_hw *hw)
> return ret;
>
> ret = regmap_update_bits(regmap, PLL_USER_CTL(pll),
> - FABIA_PLL_OUT_MASK, FABIA_PLL_OUT_MASK);
> + PLL_OUT_MASK, PLL_OUT_MASK);
> if (ret)
> return ret;
>
> @@ -1143,13 +1117,12 @@ static void alpha_pll_fabia_disable(struct clk_hw *hw)
> return;
>
> /* Disable main outputs */
> - ret = regmap_update_bits(regmap, PLL_USER_CTL(pll), FABIA_PLL_OUT_MASK,
> - 0);
> + ret = regmap_update_bits(regmap, PLL_USER_CTL(pll), PLL_OUT_MASK, 0);
> if (ret)
> return;
>
> /* Place the PLL in STANDBY */
> - regmap_write(regmap, PLL_OPMODE(pll), FABIA_OPMODE_STANDBY);
> + regmap_write(regmap, PLL_OPMODE(pll), PLL_STANDBY);
> }
>
> static unsigned long alpha_pll_fabia_recalc_rate(struct clk_hw *hw,
> @@ -1178,7 +1151,7 @@ static int alpha_pll_fabia_set_rate(struct clk_hw *hw, unsigned long rate,
> * Due to limited number of bits for fractional rate programming, the
> * rounded up rate could be marginally higher than the requested rate.
> */
> - if (rrate > (rate + FABIA_PLL_RATE_MARGIN) || rrate < rate) {
> + if (rrate > (rate + PLL_RATE_MARGIN) || rrate < rate) {
> pr_err("Call set rate on the PLL with rounded rates!\n");
> return -EINVAL;
> }
> @@ -1227,7 +1200,7 @@ static int alpha_pll_fabia_prepare(struct clk_hw *hw)
> * Due to a limited number of bits for fractional rate programming, the
> * rounded up rate could be marginally higher than the requested rate.
> */
> - if (rrate > (cal_freq + FABIA_PLL_RATE_MARGIN) || rrate < cal_freq)
> + if (rrate > (cal_freq + PLL_RATE_MARGIN) || rrate < cal_freq)
> return -EINVAL;
>
> /* Setup PLL for calibration frequency */
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project