| Message ID | 20190607101234.30449-1-vkoul@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] clk: qcom: clk-alpha-pll: Add support for Trion PLLs | expand |
Quoting Vinod Koul (2019-06-07 03:12:34) > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c > new file mode 100644 > index 000000000000..1cbc884444c9 > --- /dev/null > +++ b/drivers/clk/qcom/gcc-sm8150.c > +static const struct parent_map gcc_parent_map_0[] = { > + { P_BI_TCXO, 0 }, > + { P_GPLL0_OUT_MAIN, 1 }, > + { P_GPLL0_OUT_EVEN, 6 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const char * const gcc_parent_names_0[] = { We have a new way of specifying clk parents now. Can you use that instead of using strings everywhere? > + "bi_tcxo", > + "gpll0", > + "gpll0_out_even", > + "core_bi_pll_test_se",
Quoting Vinod Koul (2019-06-07 03:12:33) > From: Deepak Katragadda <dkatraga@codeaurora.org> > > Add programming sequence support for managing the Trion > PLLs. > > Signed-off-by: Deepak Katragadda <dkatraga@codeaurora.org> > Signed-off-by: Taniya Das <tdas@codeaurora.org> > [vkoul: port to upstream and tidy-up] This tag isn't very useful. Maybe you can list out what you actually did instead of just "tidying"? > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > index 0ced4a5a9a17..bf36a929458b 100644 > --- a/drivers/clk/qcom/clk-alpha-pll.c > +++ b/drivers/clk/qcom/clk-alpha-pll.c > @@ -120,6 +140,15 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); > #define FABIA_PLL_OUT_MASK 0x7 > #define FABIA_PLL_RATE_MARGIN 500 > > +#define TRION_PLL_CAL_VAL 0x44 > +#define TRION_PLL_STANDBY 0x0 > +#define TRION_PLL_RUN 0x1 > +#define TRION_PLL_OUT_MASK 0x7 > +#define TRION_PCAL_DONE BIT(26) > +#define TRION_PLL_RATE_MARGIN 500 These last two aren't used. Do we need them? > + > +#define XO_RATE 19200000 Please remove this define. It isn't used (thankfully!). > + > #define pll_alpha_width(p) \ > ((PLL_ALPHA_VAL_U(p) - PLL_ALPHA_VAL(p) == 4) ? \ > ALPHA_REG_BITWIDTH : ALPHA_REG_16BIT_WIDTH) > @@ -392,6 +421,15 @@ alpha_pll_round_rate(unsigned long rate, unsigned long prate, u32 *l, u64 *a, > u64 remainder; > u64 quotient; > > + /* > + * The PLLs parent rate is zero probably since the parent hasn't > + * registered yet. Return early with the requested rate. > + */ > + if (!prate) { This hasn't been a problem before. Why is it a problem now? > + pr_warn("PLLs parent rate hasn't been initialized.\n"); > + return rate; > + } > + > quotient = rate; > remainder = do_div(quotient, prate); > *l = quotient; > @@ -730,6 +768,136 @@ static long alpha_pll_huayra_round_rate(struct clk_hw *hw, unsigned long rate, > return alpha_huayra_pll_round_rate(rate, *prate, &l, &a); > } > > +static int trion_pll_is_enabled(struct clk_alpha_pll *pll, > + struct regmap *regmap) > +{ > + u32 mode_regval, opmode_regval; > + int ret; > + > + ret = regmap_read(regmap, PLL_MODE(pll), &mode_regval); > + ret |= regmap_read(regmap, PLL_OPMODE(pll), &opmode_regval); > + if (ret) > + return 0; > + > + return ((opmode_regval & TRION_PLL_RUN) && (mode_regval & PLL_OUTCTRL)); > +} > + > +static int clk_trion_pll_enable(struct clk_hw *hw) > +{ > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > + struct regmap *regmap = pll->clkr.regmap; > + u32 val; > + int ret; > + > + ret = regmap_read(regmap, PLL_MODE(pll), &val); > + if (ret) > + return ret; > + > + /* If in FSM mode, just vote for it */ > + if (val & PLL_VOTE_FSM_ENA) { > + ret = clk_enable_regmap(hw); > + if (ret) > + return ret; > + return wait_for_pll_enable_active(pll); > + } > + > + /* Set operation mode to RUN */ > + regmap_write(regmap, PLL_OPMODE(pll), TRION_PLL_RUN); > + > + ret = wait_for_pll_enable_lock(pll); > + if (ret) > + return ret; > + > + /* Enable the PLL outputs */ > + ret = regmap_update_bits(regmap, PLL_USER_CTL(pll), > + TRION_PLL_OUT_MASK, TRION_PLL_OUT_MASK); > + 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; This if (ret) can be removed. > + > + /* Ensure that the write above goes through before returning. */ > + mb(); As far as I recall mb() does nothing to ensure the write above goes through, just that writes after this one are ordered with the write above. > + return ret; > +} > + > +static void clk_trion_pll_disable(struct clk_hw *hw) > +{ > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > + struct regmap *regmap = pll->clkr.regmap; > + u32 val; > + int ret; > + > + ret = regmap_read(regmap, PLL_MODE(pll), &val); > + if (ret) > + return; > + > + /* If in FSM mode, just unvote it */ > + if (val & PLL_VOTE_FSM_ENA) { > + clk_disable_regmap(hw); > + return; > + } > + > + /* Disable the global PLL output */ > + ret = regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, 0); > + if (ret) > + return; > + > + /* Disable the PLL outputs */ > + ret = regmap_update_bits(regmap, PLL_USER_CTL(pll), > + TRION_PLL_OUT_MASK, 0); > + if (ret) > + return; > + > + /* Place the PLL mode in STANDBY */ > + regmap_write(regmap, PLL_OPMODE(pll), TRION_PLL_STANDBY); > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N); > +} > + > +static unsigned long > +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 int clk_trion_pll_is_enabled(struct clk_hw *hw) > +{ > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > + > + return trion_pll_is_enabled(pll, pll->clkr.regmap); > +} Can you move this function right below trion_pll_is_enabled()? > + > +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; > + > + min_freq = pll->vco_table[0].min_freq; > + max_freq = pll->vco_table[pll->num_vco - 1].max_freq; > + > + return clamp(rate, min_freq, max_freq); > +} > + > const struct clk_ops clk_alpha_pll_ops = { > .enable = clk_alpha_pll_enable, > .disable = clk_alpha_pll_disable, > @@ -902,6 +1079,10 @@ static int alpha_pll_fabia_enable(struct clk_hw *hw) > ret = regmap_read(regmap, PLL_OPMODE(pll), &opmode_val); > if (ret) > return ret; > + ret = regmap_update_bits(regmap, PLL_MODE(pll), > + PLL_BYPASSNL, PLL_BYPASSNL); > + if (ret) > + return ret; What is this? > > /* Skip If PLL is already running */ > if ((opmode_val & FABIA_OPMODE_RUN) && (val & PLL_OUTCTRL)) > @@ -1058,6 +1239,91 @@ static unsigned long clk_alpha_pll_postdiv_fabia_recalc_rate(struct clk_hw *hw, > return (parent_rate / div); > } > > +static unsigned long > +clk_trion_pll_postdiv_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) > +{ > + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); > + struct regmap *regmap = pll->clkr.regmap; > + u32 i, div = 1, val; > + > + if (!pll->post_div_table) { > + pr_err("Missing the post_div_table for the PLL\n"); Ahhh! > + return -EINVAL; > + } > + > + regmap_read(regmap, PLL_USER_CTL(pll), &val); > + > + val >>= pll->post_div_shift; > + val &= PLL_POST_DIV_MASK(pll); > + > + for (i = 0; i < pll->num_post_div; i++) { > + if (pll->post_div_table[i].val == val) { > + div = pll->post_div_table[i].div; > + break; > + } > + } > + > + return (parent_rate / div); > +} > + > +static long > +clk_trion_pll_postdiv_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *prate) > +{ > + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); > + > + if (!pll->post_div_table) { > + pr_err("Missing the post_div_table for the PLL\n"); > + return -EINVAL; Does this ever happen? I'd rather see this removed and the code blow up if the driver author didn't test this. > + } > + > + return divider_round_rate(hw, rate, prate, pll->post_div_table, > + pll->width, CLK_DIVIDER_ROUND_CLOSEST); > +}; > + > +static int > +clk_trion_pll_postdiv_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); > + struct regmap *regmap = pll->clkr.regmap; > + int i, val = 0, div, ret; > + > + /* > + * If the PLL is in FSM mode, then treat the set_rate callback > + * as a no-operation. > + */ > + ret = regmap_read(regmap, PLL_MODE(pll), &val); > + if (ret) > + return ret; > + > + if (val & PLL_VOTE_FSM_ENA) > + return 0; > + > + if (!pll->post_div_table) { > + pr_err("Missing the post_div_table for the PLL\n"); Again! > + return -EINVAL; > + } > + > + div = DIV_ROUND_UP_ULL((u64)parent_rate, rate); Is the cast necessary? > + for (i = 0; i < pll->num_post_div; i++) { > + if (pll->post_div_table[i].div == div) { > + val = pll->post_div_table[i].val; > + break; > + } > + } > + return regmap_update_bits(regmap, PLL_USER_CTL(pll), > + PLL_POST_DIV_MASK(pll) << PLL_POST_DIV_SHIFT, > + val << PLL_POST_DIV_SHIFT); > +} > + > +const struct clk_ops clk_trion_pll_postdiv_ops = { > + .recalc_rate = clk_trion_pll_postdiv_recalc_rate, > + .round_rate = clk_trion_pll_postdiv_round_rate, > + .set_rate = clk_trion_pll_postdiv_set_rate, > +}; > +EXPORT_SYMBOL_GPL(clk_trion_pll_postdiv_ops); > + > static long clk_alpha_pll_postdiv_fabia_round_rate(struct clk_hw *hw, > unsigned long rate, unsigned long *prate) > {
Thanks Stephen for a quick review On 07-06-19, 10:55, Stephen Boyd wrote: > Quoting Vinod Koul (2019-06-07 03:12:33) > > From: Deepak Katragadda <dkatraga@codeaurora.org> > > > > Add programming sequence support for managing the Trion > > PLLs. > > > > Signed-off-by: Deepak Katragadda <dkatraga@codeaurora.org> > > Signed-off-by: Taniya Das <tdas@codeaurora.org> > > [vkoul: port to upstream and tidy-up] > > This tag isn't very useful. Maybe you can list out what you actually did > instead of just "tidying"? First was to port and in process remove bunch of code which cant be upstreamed and has no user in current work. Had to rewrite bunch of stuff while upstreaming. Then fix format and style issue I will try to list.. > > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > > index 0ced4a5a9a17..bf36a929458b 100644 > > --- a/drivers/clk/qcom/clk-alpha-pll.c > > +++ b/drivers/clk/qcom/clk-alpha-pll.c > > @@ -120,6 +140,15 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); > > #define FABIA_PLL_OUT_MASK 0x7 > > #define FABIA_PLL_RATE_MARGIN 500 > > > > +#define TRION_PLL_CAL_VAL 0x44 > > +#define TRION_PLL_STANDBY 0x0 > > +#define TRION_PLL_RUN 0x1 > > +#define TRION_PLL_OUT_MASK 0x7 > > +#define TRION_PCAL_DONE BIT(26) > > +#define TRION_PLL_RATE_MARGIN 500 > > These last two aren't used. Do we need them? Not anymore, I removed the user as that was required for this, I will > > > + > > +#define XO_RATE 19200000 > > Please remove this define. It isn't used (thankfully!). Yes will remove this and other is the series :) > > #define pll_alpha_width(p) \ > > ((PLL_ALPHA_VAL_U(p) - PLL_ALPHA_VAL(p) == 4) ? \ > > ALPHA_REG_BITWIDTH : ALPHA_REG_16BIT_WIDTH) > > @@ -392,6 +421,15 @@ alpha_pll_round_rate(unsigned long rate, unsigned long prate, u32 *l, u64 *a, > > u64 remainder; > > u64 quotient; > > > > + /* > > + * The PLLs parent rate is zero probably since the parent hasn't > > + * registered yet. Return early with the requested rate. > > + */ > > + if (!prate) { > > This hasn't been a problem before. Why is it a problem now? Good point, some how downstream thinks so, I will remove these checks > > +static int clk_trion_pll_enable(struct clk_hw *hw) > > +{ > > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > > + struct regmap *regmap = pll->clkr.regmap; > > + u32 val; > > + int ret; > > + > > + ret = regmap_read(regmap, PLL_MODE(pll), &val); > > + if (ret) > > + return ret; > > + > > + /* If in FSM mode, just vote for it */ > > + if (val & PLL_VOTE_FSM_ENA) { > > + ret = clk_enable_regmap(hw); > > + if (ret) > > + return ret; > > + return wait_for_pll_enable_active(pll); > > + } > > + > > + /* Set operation mode to RUN */ > > + regmap_write(regmap, PLL_OPMODE(pll), TRION_PLL_RUN); > > + > > + ret = wait_for_pll_enable_lock(pll); > > + if (ret) > > + return ret; > > + > > + /* Enable the PLL outputs */ > > + ret = regmap_update_bits(regmap, PLL_USER_CTL(pll), > > + TRION_PLL_OUT_MASK, TRION_PLL_OUT_MASK); > > + 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; > > This if (ret) can be removed. Yes we should return! > > + > > + /* Ensure that the write above goes through before returning. */ > > + mb(); > > As far as I recall mb() does nothing to ensure the write above goes > through, just that writes after this one are ordered with the write > above. Agreed someone wasn't convinced, will remove this. > > +static int clk_trion_pll_is_enabled(struct clk_hw *hw) > > +{ > > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > > + > > + return trion_pll_is_enabled(pll, pll->clkr.regmap); > > +} > > Can you move this function right below trion_pll_is_enabled()? Sure > > const struct clk_ops clk_alpha_pll_ops = { > > .enable = clk_alpha_pll_enable, > > .disable = clk_alpha_pll_disable, > > @@ -902,6 +1079,10 @@ static int alpha_pll_fabia_enable(struct clk_hw *hw) > > ret = regmap_read(regmap, PLL_OPMODE(pll), &opmode_val); > > if (ret) > > return ret; > > + ret = regmap_update_bits(regmap, PLL_MODE(pll), > > + PLL_BYPASSNL, PLL_BYPASSNL); > > + if (ret) > > + return ret; > > What is this? Sorry am not sure I understood the question. care to elaborate please? > > +static long > > +clk_trion_pll_postdiv_round_rate(struct clk_hw *hw, unsigned long rate, > > + unsigned long *prate) > > +{ > > + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); > > + > > + if (!pll->post_div_table) { > > + pr_err("Missing the post_div_table for the PLL\n"); > > + return -EINVAL; > > Does this ever happen? I'd rather see this removed and the code blow up > if the driver author didn't test this. Sounds right! will remove this and others. > > + return -EINVAL; > > + } > > + > > + div = DIV_ROUND_UP_ULL((u64)parent_rate, rate); > > Is the cast necessary? Dont think so, will remove
On 07-06-19, 10:43, Stephen Boyd wrote: > Quoting Vinod Koul (2019-06-07 03:12:34) > > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c > > new file mode 100644 > > index 000000000000..1cbc884444c9 > > --- /dev/null > > +++ b/drivers/clk/qcom/gcc-sm8150.c > > +static const struct parent_map gcc_parent_map_0[] = { > > + { P_BI_TCXO, 0 }, > > + { P_GPLL0_OUT_MAIN, 1 }, > > + { P_GPLL0_OUT_EVEN, 6 }, > > + { P_CORE_BI_PLL_TEST_SE, 7 }, > > +}; > > + > > +static const char * const gcc_parent_names_0[] = { > > We have a new way of specifying clk parents now. Can you use that > instead of using strings everywhere? Okay I will update, any pointers on new implementation I can look up? Thanks
Quoting Vinod Koul (2019-06-08 02:14:36) > On 07-06-19, 10:55, Stephen Boyd wrote: > > Quoting Vinod Koul (2019-06-07 03:12:33) > > > > const struct clk_ops clk_alpha_pll_ops = { > > > .enable = clk_alpha_pll_enable, > > > .disable = clk_alpha_pll_disable, > > > @@ -902,6 +1079,10 @@ static int alpha_pll_fabia_enable(struct clk_hw *hw) > > > ret = regmap_read(regmap, PLL_OPMODE(pll), &opmode_val); > > > if (ret) > > > return ret; > > > + ret = regmap_update_bits(regmap, PLL_MODE(pll), > > > + PLL_BYPASSNL, PLL_BYPASSNL); > > > + if (ret) > > > + return ret; > > > > What is this? > > Sorry am not sure I understood the question. care to elaborate please? The bypass bit of a PLL is very generic so I'm confused why the enable function is only gaining this bit setting logic now. Plus, it's all grouped together with the previous line so it looks like a possible stray addition to the code? And after this there's an early exit from the function if the PLL is already running, so we would put the PLL into bypass and then return? What's going on here?
Quoting Vinod Koul (2019-06-08 02:15:37) > On 07-06-19, 10:43, Stephen Boyd wrote: > > Quoting Vinod Koul (2019-06-07 03:12:34) > > > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c > > > new file mode 100644 > > > index 000000000000..1cbc884444c9 > > > --- /dev/null > > > +++ b/drivers/clk/qcom/gcc-sm8150.c > > > +static const struct parent_map gcc_parent_map_0[] = { > > > + { P_BI_TCXO, 0 }, > > > + { P_GPLL0_OUT_MAIN, 1 }, > > > + { P_GPLL0_OUT_EVEN, 6 }, > > > + { P_CORE_BI_PLL_TEST_SE, 7 }, > > > +}; > > > + > > > +static const char * const gcc_parent_names_0[] = { > > > > We have a new way of specifying clk parents now. Can you use that > > instead of using strings everywhere? > > Okay I will update, any pointers on new implementation I can look up? > Not really yet. Chen-Yu is working on the Allwinner driver (drivers/clk/sunxi-ng/) and some work from Jeff for MSM8998 is on the list that you can look at.
On 10-06-19, 08:08, Stephen Boyd wrote: > Quoting Vinod Koul (2019-06-08 02:15:37) > > On 07-06-19, 10:43, Stephen Boyd wrote: > > > Quoting Vinod Koul (2019-06-07 03:12:34) > > > > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c > > > > new file mode 100644 > > > > index 000000000000..1cbc884444c9 > > > > --- /dev/null > > > > +++ b/drivers/clk/qcom/gcc-sm8150.c > > > > +static const struct parent_map gcc_parent_map_0[] = { > > > > + { P_BI_TCXO, 0 }, > > > > + { P_GPLL0_OUT_MAIN, 1 }, > > > > + { P_GPLL0_OUT_EVEN, 6 }, > > > > + { P_CORE_BI_PLL_TEST_SE, 7 }, > > > > +}; > > > > + > > > > +static const char * const gcc_parent_names_0[] = { > > > > > > We have a new way of specifying clk parents now. Can you use that > > > instead of using strings everywhere? > > > > Okay I will update, any pointers on new implementation I can look up? > > > > Not really yet. Chen-Yu is working on the Allwinner driver > (drivers/clk/sunxi-ng/) and some work from Jeff for MSM8998 is on the > list that you can look at. Thanks for the pointer, I have done the conversion and will post the update in v2
On 10-06-19, 08:06, Stephen Boyd wrote: > Quoting Vinod Koul (2019-06-08 02:14:36) > > On 07-06-19, 10:55, Stephen Boyd wrote: > > > Quoting Vinod Koul (2019-06-07 03:12:33) > > > > > > const struct clk_ops clk_alpha_pll_ops = { > > > > .enable = clk_alpha_pll_enable, > > > > .disable = clk_alpha_pll_disable, > > > > @@ -902,6 +1079,10 @@ static int alpha_pll_fabia_enable(struct clk_hw *hw) > > > > ret = regmap_read(regmap, PLL_OPMODE(pll), &opmode_val); > > > > if (ret) > > > > return ret; > > > > + ret = regmap_update_bits(regmap, PLL_MODE(pll), > > > > + PLL_BYPASSNL, PLL_BYPASSNL); > > > > + if (ret) > > > > + return ret; > > > > > > What is this? > > > > Sorry am not sure I understood the question. care to elaborate please? > > The bypass bit of a PLL is very generic so I'm confused why the enable > function is only gaining this bit setting logic now. Plus, it's all > grouped together with the previous line so it looks like a possible > stray addition to the code? And after this there's an early exit from > the function if the PLL is already running, so we would put the PLL into > bypass and then return? What's going on here? Thanks for spotting that is wrong. I am not sure why this crept in , I am not supposed to change this, will fix it in v2
On Fri, Jun 07, 2019 at 03:42:34PM +0530, Vinod Koul wrote: > From: Deepak Katragadda <dkatraga@codeaurora.org> > > Add the clocks supported in global clock controller which clock the > peripherals like BLSPs, SDCC, USB, MDSS etc. Register all the clocks > to the clock framework for the clients to be able to request for them. > > Signed-off-by: Deepak Katragadda <dkatraga@codeaurora.org> > Signed-off-by: Taniya Das <tdas@codeaurora.org> > [vkoul: port to upstream and tidy-up] > Signed-off-by: Vinod Koul <vkoul@kernel.org> > --- > .../devicetree/bindings/clock/qcom,gcc.txt | 1 + > drivers/clk/qcom/Kconfig | 7 + > drivers/clk/qcom/Makefile | 1 + > drivers/clk/qcom/gcc-sm8150.c | 3649 +++++++++++++++++ > include/dt-bindings/clock/qcom,gcc-sm8150.h | 243 ++ Next time, please split bindings to separate patch. For the DT parts, Acked-by: Rob Herring <robh@kernel.org> > 5 files changed, 3901 insertions(+) > create mode 100644 drivers/clk/qcom/gcc-sm8150.c > create mode 100644 include/dt-bindings/clock/qcom,gcc-sm8150.h
diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c index 0ced4a5a9a17..bf36a929458b 100644 --- a/drivers/clk/qcom/clk-alpha-pll.c +++ b/drivers/clk/qcom/clk-alpha-pll.c @@ -32,6 +32,7 @@ # define PLL_LOCK_DET BIT(31) #define PLL_L_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_L_VAL]) +#define PLL_CAL_L_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_CAL_L_VAL]) #define PLL_ALPHA_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_ALPHA_VAL]) #define PLL_ALPHA_VAL_U(p) ((p)->offset + (p)->regs[PLL_OFF_ALPHA_VAL_U]) @@ -44,14 +45,17 @@ # define PLL_VCO_MASK 0x3 #define PLL_USER_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_USER_CTL_U]) +#define PLL_USER_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_USER_CTL_U1]) #define PLL_CONFIG_CTL(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL]) #define PLL_CONFIG_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL_U]) +#define PLL_CONFIG_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL_U11]) #define PLL_TEST_CTL(p) ((p)->offset + (p)->regs[PLL_OFF_TEST_CTL]) #define PLL_TEST_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_TEST_CTL_U]) #define PLL_STATUS(p) ((p)->offset + (p)->regs[PLL_OFF_STATUS]) #define PLL_OPMODE(p) ((p)->offset + (p)->regs[PLL_OFF_OPMODE]) #define PLL_FRAC(p) ((p)->offset + (p)->regs[PLL_OFF_FRAC]) +#define PLL_CAL_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_CAL_VAL]) const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { [CLK_ALPHA_PLL_TYPE_DEFAULT] = { @@ -96,6 +100,22 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { [PLL_OFF_OPMODE] = 0x2c, [PLL_OFF_FRAC] = 0x38, }, + [CLK_ALPHA_PLL_TYPE_TRION] = { + [PLL_OFF_L_VAL] = 0x04, + [PLL_OFF_CAL_L_VAL] = 0x08, + [PLL_OFF_USER_CTL] = 0x0c, + [PLL_OFF_USER_CTL_U] = 0x10, + [PLL_OFF_USER_CTL_U1] = 0x14, + [PLL_OFF_CONFIG_CTL] = 0x18, + [PLL_OFF_CONFIG_CTL_U] = 0x1c, + [PLL_OFF_CONFIG_CTL_U1] = 0x20, + [PLL_OFF_TEST_CTL] = 0x24, + [PLL_OFF_TEST_CTL_U] = 0x28, + [PLL_OFF_STATUS] = 0x30, + [PLL_OFF_OPMODE] = 0x38, + [PLL_OFF_ALPHA_VAL] = 0x40, + [PLL_OFF_CAL_VAL] = 0x44, + }, }; EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); @@ -120,6 +140,15 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); #define FABIA_PLL_OUT_MASK 0x7 #define FABIA_PLL_RATE_MARGIN 500 +#define TRION_PLL_CAL_VAL 0x44 +#define TRION_PLL_STANDBY 0x0 +#define TRION_PLL_RUN 0x1 +#define TRION_PLL_OUT_MASK 0x7 +#define TRION_PCAL_DONE BIT(26) +#define TRION_PLL_RATE_MARGIN 500 + +#define XO_RATE 19200000 + #define pll_alpha_width(p) \ ((PLL_ALPHA_VAL_U(p) - PLL_ALPHA_VAL(p) == 4) ? \ ALPHA_REG_BITWIDTH : ALPHA_REG_16BIT_WIDTH) @@ -392,6 +421,15 @@ alpha_pll_round_rate(unsigned long rate, unsigned long prate, u32 *l, u64 *a, u64 remainder; u64 quotient; + /* + * The PLLs parent rate is zero probably since the parent hasn't + * registered yet. Return early with the requested rate. + */ + if (!prate) { + pr_warn("PLLs parent rate hasn't been initialized.\n"); + return rate; + } + quotient = rate; remainder = do_div(quotient, prate); *l = quotient; @@ -730,6 +768,136 @@ static long alpha_pll_huayra_round_rate(struct clk_hw *hw, unsigned long rate, return alpha_huayra_pll_round_rate(rate, *prate, &l, &a); } +static int trion_pll_is_enabled(struct clk_alpha_pll *pll, + struct regmap *regmap) +{ + u32 mode_regval, opmode_regval; + int ret; + + ret = regmap_read(regmap, PLL_MODE(pll), &mode_regval); + ret |= regmap_read(regmap, PLL_OPMODE(pll), &opmode_regval); + if (ret) + return 0; + + return ((opmode_regval & TRION_PLL_RUN) && (mode_regval & PLL_OUTCTRL)); +} + +static int clk_trion_pll_enable(struct clk_hw *hw) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + struct regmap *regmap = pll->clkr.regmap; + u32 val; + int ret; + + ret = regmap_read(regmap, PLL_MODE(pll), &val); + if (ret) + return ret; + + /* If in FSM mode, just vote for it */ + if (val & PLL_VOTE_FSM_ENA) { + ret = clk_enable_regmap(hw); + if (ret) + return ret; + return wait_for_pll_enable_active(pll); + } + + /* Set operation mode to RUN */ + regmap_write(regmap, PLL_OPMODE(pll), TRION_PLL_RUN); + + ret = wait_for_pll_enable_lock(pll); + if (ret) + return ret; + + /* Enable the PLL outputs */ + ret = regmap_update_bits(regmap, PLL_USER_CTL(pll), + TRION_PLL_OUT_MASK, TRION_PLL_OUT_MASK); + 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 clk_trion_pll_disable(struct clk_hw *hw) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + struct regmap *regmap = pll->clkr.regmap; + u32 val; + int ret; + + ret = regmap_read(regmap, PLL_MODE(pll), &val); + if (ret) + return; + + /* If in FSM mode, just unvote it */ + if (val & PLL_VOTE_FSM_ENA) { + clk_disable_regmap(hw); + return; + } + + /* Disable the global PLL output */ + ret = regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, 0); + if (ret) + return; + + /* Disable the PLL outputs */ + ret = regmap_update_bits(regmap, PLL_USER_CTL(pll), + TRION_PLL_OUT_MASK, 0); + if (ret) + return; + + /* Place the PLL mode in STANDBY */ + regmap_write(regmap, PLL_OPMODE(pll), TRION_PLL_STANDBY); + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N); +} + +static unsigned long +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 int clk_trion_pll_is_enabled(struct clk_hw *hw) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + + return trion_pll_is_enabled(pll, pll->clkr.regmap); +} + +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; + + min_freq = pll->vco_table[0].min_freq; + max_freq = pll->vco_table[pll->num_vco - 1].max_freq; + + return clamp(rate, min_freq, max_freq); +} + const struct clk_ops clk_alpha_pll_ops = { .enable = clk_alpha_pll_enable, .disable = clk_alpha_pll_disable, @@ -760,6 +928,15 @@ const struct clk_ops clk_alpha_pll_hwfsm_ops = { }; EXPORT_SYMBOL_GPL(clk_alpha_pll_hwfsm_ops); +const struct clk_ops clk_trion_fixed_pll_ops = { + .enable = clk_trion_pll_enable, + .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, +}; +EXPORT_SYMBOL_GPL(clk_trion_fixed_pll_ops); + static unsigned long clk_alpha_pll_postdiv_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { @@ -902,6 +1079,10 @@ static int alpha_pll_fabia_enable(struct clk_hw *hw) ret = regmap_read(regmap, PLL_OPMODE(pll), &opmode_val); if (ret) return ret; + ret = regmap_update_bits(regmap, PLL_MODE(pll), + PLL_BYPASSNL, PLL_BYPASSNL); + if (ret) + return ret; /* Skip If PLL is already running */ if ((opmode_val & FABIA_OPMODE_RUN) && (val & PLL_OUTCTRL)) @@ -1058,6 +1239,91 @@ static unsigned long clk_alpha_pll_postdiv_fabia_recalc_rate(struct clk_hw *hw, return (parent_rate / div); } +static unsigned long +clk_trion_pll_postdiv_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) +{ + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); + struct regmap *regmap = pll->clkr.regmap; + u32 i, div = 1, val; + + if (!pll->post_div_table) { + pr_err("Missing the post_div_table for the PLL\n"); + return -EINVAL; + } + + regmap_read(regmap, PLL_USER_CTL(pll), &val); + + val >>= pll->post_div_shift; + val &= PLL_POST_DIV_MASK(pll); + + for (i = 0; i < pll->num_post_div; i++) { + if (pll->post_div_table[i].val == val) { + div = pll->post_div_table[i].div; + break; + } + } + + return (parent_rate / div); +} + +static long +clk_trion_pll_postdiv_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); + + if (!pll->post_div_table) { + pr_err("Missing the post_div_table for the PLL\n"); + return -EINVAL; + } + + return divider_round_rate(hw, rate, prate, pll->post_div_table, + pll->width, CLK_DIVIDER_ROUND_CLOSEST); +}; + +static int +clk_trion_pll_postdiv_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); + struct regmap *regmap = pll->clkr.regmap; + int i, val = 0, div, ret; + + /* + * If the PLL is in FSM mode, then treat the set_rate callback + * as a no-operation. + */ + ret = regmap_read(regmap, PLL_MODE(pll), &val); + if (ret) + return ret; + + if (val & PLL_VOTE_FSM_ENA) + return 0; + + if (!pll->post_div_table) { + pr_err("Missing the post_div_table for the PLL\n"); + return -EINVAL; + } + + div = DIV_ROUND_UP_ULL((u64)parent_rate, rate); + for (i = 0; i < pll->num_post_div; i++) { + if (pll->post_div_table[i].div == div) { + val = pll->post_div_table[i].val; + break; + } + } + return regmap_update_bits(regmap, PLL_USER_CTL(pll), + PLL_POST_DIV_MASK(pll) << PLL_POST_DIV_SHIFT, + val << PLL_POST_DIV_SHIFT); +} + +const struct clk_ops clk_trion_pll_postdiv_ops = { + .recalc_rate = clk_trion_pll_postdiv_recalc_rate, + .round_rate = clk_trion_pll_postdiv_round_rate, + .set_rate = clk_trion_pll_postdiv_set_rate, +}; +EXPORT_SYMBOL_GPL(clk_trion_pll_postdiv_ops); + static long clk_alpha_pll_postdiv_fabia_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *prate) { diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h index 66755f0f84fc..a2f844c66207 100644 --- a/drivers/clk/qcom/clk-alpha-pll.h +++ b/drivers/clk/qcom/clk-alpha-pll.h @@ -13,22 +13,27 @@ enum { CLK_ALPHA_PLL_TYPE_HUAYRA, CLK_ALPHA_PLL_TYPE_BRAMMO, CLK_ALPHA_PLL_TYPE_FABIA, + CLK_ALPHA_PLL_TYPE_TRION, CLK_ALPHA_PLL_TYPE_MAX, }; enum { PLL_OFF_L_VAL, + PLL_OFF_CAL_L_VAL, PLL_OFF_ALPHA_VAL, PLL_OFF_ALPHA_VAL_U, PLL_OFF_USER_CTL, PLL_OFF_USER_CTL_U, + PLL_OFF_USER_CTL_U1, PLL_OFF_CONFIG_CTL, PLL_OFF_CONFIG_CTL_U, + PLL_OFF_CONFIG_CTL_U1, PLL_OFF_TEST_CTL, PLL_OFF_TEST_CTL_U, PLL_OFF_STATUS, PLL_OFF_OPMODE, PLL_OFF_FRAC, + PLL_OFF_CAL_VAL, PLL_OFF_MAX_REGS }; @@ -43,6 +48,7 @@ struct pll_vco { /** * struct clk_alpha_pll - phase locked loop (PLL) * @offset: base address of registers + * @inited: flag that's set when the PLL is initialized * @vco_table: array of VCO settings * @regs: alpha pll register map (see @clk_alpha_pll_regs) * @clkr: regmap clock handle @@ -50,6 +56,7 @@ struct pll_vco { struct clk_alpha_pll { u32 offset; const u8 *regs; + bool inited; const struct pll_vco *vco_table; size_t num_vco; @@ -117,5 +124,7 @@ void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, const struct alpha_pll_config *config); void clk_fabia_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, const struct alpha_pll_config *config); +extern const struct clk_ops clk_trion_fixed_pll_ops; +extern const struct clk_ops clk_trion_pll_postdiv_ops; #endif