| Message ID | 20180717095617.12240-4-jbrunet@baylibre.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clk: meson: clk-pll driver update | expand |
On 17/07/2018 11:56, Jerome Brunet wrote: > Putting hard-coded rates inside the parameter tables assumes that > the parent is known and will never change. That's a big assumption > we should not make. > > We have everything we need to recalculate the output rate using > the parent rate and the rest of the parameters. Let's do so and > drop the rates from the tables. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/clk/meson/axg.c | 73 +++++++++++++-------------- > drivers/clk/meson/clk-pll.c | 69 ++++++++++++++++--------- > drivers/clk/meson/clkc.h | 8 ++- > drivers/clk/meson/gxbb.c | 120 ++++++++++++++++++++++---------------------- > drivers/clk/meson/meson8b.c | 34 ++++++------- > 5 files changed, 162 insertions(+), 142 deletions(-) > > diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c > index 572358062459..d34954bd8c5e 100644 > --- a/drivers/clk/meson/axg.c > +++ b/drivers/clk/meson/axg.c > @@ -130,36 +130,36 @@ static struct clk_regmap axg_sys_pll = { > }, > }; > > -static const struct pll_rate_table axg_gp0_pll_rate_table[] = { > - PLL_RATE(960000000, 40, 1), > - PLL_RATE(984000000, 41, 1), > - PLL_RATE(1008000000, 42, 1), > - PLL_RATE(1032000000, 43, 1), > - PLL_RATE(1056000000, 44, 1), > - PLL_RATE(1080000000, 45, 1), > - PLL_RATE(1104000000, 46, 1), > - PLL_RATE(1128000000, 47, 1), > - PLL_RATE(1152000000, 48, 1), > - PLL_RATE(1176000000, 49, 1), > - PLL_RATE(1200000000, 50, 1), > - PLL_RATE(1224000000, 51, 1), > - PLL_RATE(1248000000, 52, 1), > - PLL_RATE(1272000000, 53, 1), > - PLL_RATE(1296000000, 54, 1), > - PLL_RATE(1320000000, 55, 1), > - PLL_RATE(1344000000, 56, 1), > - PLL_RATE(1368000000, 57, 1), > - PLL_RATE(1392000000, 58, 1), > - PLL_RATE(1416000000, 59, 1), > - PLL_RATE(1440000000, 60, 1), > - PLL_RATE(1464000000, 61, 1), > - PLL_RATE(1488000000, 62, 1), > - PLL_RATE(1512000000, 63, 1), > - PLL_RATE(1536000000, 64, 1), > - PLL_RATE(1560000000, 65, 1), > - PLL_RATE(1584000000, 66, 1), > - PLL_RATE(1608000000, 67, 1), > - PLL_RATE(1632000000, 68, 1), > +static const struct pll_params_table axg_gp0_pll_params_table[] = { > + PLL_PARAMS(40, 1), > + PLL_PARAMS(41, 1), > + PLL_PARAMS(42, 1), > + PLL_PARAMS(43, 1), > + PLL_PARAMS(44, 1), > + PLL_PARAMS(45, 1), > + PLL_PARAMS(46, 1), > + PLL_PARAMS(47, 1), > + PLL_PARAMS(48, 1), > + PLL_PARAMS(49, 1), > + PLL_PARAMS(50, 1), > + PLL_PARAMS(51, 1), > + PLL_PARAMS(52, 1), > + PLL_PARAMS(53, 1), > + PLL_PARAMS(54, 1), > + PLL_PARAMS(55, 1), > + PLL_PARAMS(56, 1), > + PLL_PARAMS(57, 1), > + PLL_PARAMS(58, 1), > + PLL_PARAMS(59, 1), > + PLL_PARAMS(60, 1), > + PLL_PARAMS(61, 1), > + PLL_PARAMS(62, 1), > + PLL_PARAMS(63, 1), > + PLL_PARAMS(64, 1), > + PLL_PARAMS(65, 1), > + PLL_PARAMS(66, 1), > + PLL_PARAMS(67, 1), > + PLL_PARAMS(68, 1), > { /* sentinel */ }, > }; > > @@ -203,7 +203,7 @@ static struct clk_regmap axg_gp0_pll_dco = { > .shift = 29, > .width = 1, > }, > - .table = axg_gp0_pll_rate_table, > + .table = axg_gp0_pll_params_table, > .init_regs = axg_gp0_init_regs, > .init_count = ARRAY_SIZE(axg_gp0_init_regs), > }, > @@ -271,7 +271,7 @@ static struct clk_regmap axg_hifi_pll_dco = { > .shift = 29, > .width = 1, > }, > - .table = axg_gp0_pll_rate_table, > + .table = axg_gp0_pll_params_table, > .init_regs = axg_hifi_init_regs, > .init_count = ARRAY_SIZE(axg_hifi_init_regs), > .flags = CLK_MESON_PLL_ROUND_CLOSEST, > @@ -627,11 +627,10 @@ static struct clk_regmap axg_mpll3 = { > }, > }; > > -static const struct pll_rate_table axg_pcie_pll_rate_table[] = { > +static const struct pll_params_table axg_pcie_pll_params_table[] = { > { > - .rate = 1600000000, > - .m = 200, > - .n = 3, > + .m = 200, > + .n = 3, > }, > { /* sentinel */ }, > }; > @@ -678,7 +677,7 @@ static struct clk_regmap axg_pcie_pll_dco = { > .shift = 29, > .width = 1, > }, > - .table = axg_pcie_pll_rate_table, > + .table = axg_pcie_pll_params_table, > .init_regs = axg_pcie_init_regs, > .init_count = ARRAY_SIZE(axg_pcie_init_regs), > }, > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > index 348a866f09eb..f5b5b3fabe3c 100644 > --- a/drivers/clk/meson/clk-pll.c > +++ b/drivers/clk/meson/clk-pll.c > @@ -45,7 +45,7 @@ meson_clk_pll_data(struct clk_regmap *clk) > } > > static unsigned long __pll_params_to_rate(unsigned long parent_rate, > - const struct pll_rate_table *pllt, > + const struct pll_params_table *pllt, > u16 frac, > struct meson_clk_pll_data *pll) > { > @@ -66,7 +66,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw, > { > struct clk_regmap *clk = to_clk_regmap(hw); > struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > - struct pll_rate_table pllt; > + struct pll_params_table pllt; > u16 frac; > > pllt.n = meson_parm_read(clk->map, &pll->n); > @@ -81,7 +81,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw, > > static u16 __pll_params_with_frac(unsigned long rate, > unsigned long parent_rate, > - const struct pll_rate_table *pllt, > + const struct pll_params_table *pllt, > struct meson_clk_pll_data *pll) > { > u16 frac_max = (1 << pll->frac.width); > @@ -97,29 +97,50 @@ static u16 __pll_params_with_frac(unsigned long rate, > return min((u16)val, (u16)(frac_max - 1)); > } > > -static const struct pll_rate_table * > +static bool meson_clk_pll_is_better(unsigned long rate, > + unsigned long best, > + unsigned long now, > + struct meson_clk_pll_data *pll) > +{ > + if (!(pll->flags & CLK_MESON_PLL_ROUND_CLOSEST) || > + MESON_PARM_APPLICABLE(&pll->frac)) { > + /* Round down */ > + if (now < rate && best < now) > + return true; > + } else { > + /* Round Closest */ > + if (abs(now - rate) < abs(best - rate)) > + return true; > + } > + > + return false; > +} > + > +static const struct pll_params_table * > meson_clk_get_pll_settings(unsigned long rate, > + unsigned long parent_rate, > struct meson_clk_pll_data *pll) > { > - const struct pll_rate_table *table = pll->table; > - unsigned int i = 0; > + const struct pll_params_table *table = pll->table; > + unsigned long best = 0, now = 0; > + unsigned int i, best_i = 0; > > if (!table) > return NULL; > > - /* Find the first table element exceeding rate */ > - while (table[i].rate && table[i].rate <= rate) > - i++; > + for (i = 0; table[i].n; i++) { > + now = __pll_params_to_rate(parent_rate, &table[i], 0, pll); > > - if (i != 0) { > - if (MESON_PARM_APPLICABLE(&pll->frac) || > - !(pll->flags & CLK_MESON_PLL_ROUND_CLOSEST) || > - (abs(rate - table[i - 1].rate) < > - abs(rate - table[i].rate))) > - i--; > + /* If we get an exact match, don't bother any further */ > + if (now == rate) { > + return &table[i]; > + } else if (meson_clk_pll_is_better(rate, best, now, pll)) { > + best = now; > + best_i = i; > + } > } > > - return (struct pll_rate_table *)&table[i]; > + return (struct pll_params_table *)&table[best_i]; > } > > static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate, > @@ -127,16 +148,18 @@ static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate, > { > struct clk_regmap *clk = to_clk_regmap(hw); > struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > - const struct pll_rate_table *pllt = > - meson_clk_get_pll_settings(rate, pll); > + const struct pll_params_table *pllt = > + meson_clk_get_pll_settings(rate, *parent_rate, pll); > + unsigned long round; > u16 frac; > > if (!pllt) > return meson_clk_pll_recalc_rate(hw, *parent_rate); > > - if (!MESON_PARM_APPLICABLE(&pll->frac) > - || rate == pllt->rate) > - return pllt->rate; > + round = __pll_params_to_rate(*parent_rate, pllt, 0, pll); > + > + if (!MESON_PARM_APPLICABLE(&pll->frac) || rate == round) > + return round; > > /* > * The rate provided by the setting is not an exact match, let's > @@ -214,7 +237,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > { > struct clk_regmap *clk = to_clk_regmap(hw); > struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > - const struct pll_rate_table *pllt; > + const struct pll_params_table *pllt; > unsigned int enabled; > unsigned long old_rate; > u16 frac = 0; > @@ -224,7 +247,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > > old_rate = rate; > > - pllt = meson_clk_get_pll_settings(rate, pll); > + pllt = meson_clk_get_pll_settings(rate, parent_rate, pll); > if (!pllt) > return -EINVAL; > > diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h > index a2245e857f70..6b96d55c047d 100644 > --- a/drivers/clk/meson/clkc.h > +++ b/drivers/clk/meson/clkc.h > @@ -43,15 +43,13 @@ static inline void meson_parm_write(struct regmap *map, struct parm *p, > } > > > -struct pll_rate_table { > - unsigned long rate; > +struct pll_params_table { > u16 m; > u16 n; > }; > > -#define PLL_RATE(_r, _m, _n) \ > +#define PLL_PARAMS(_m, _n) \ > { \ > - .rate = (_r), \ > .m = (_m), \ > .n = (_n), \ > } > @@ -67,7 +65,7 @@ struct meson_clk_pll_data { > struct parm rst; > const struct reg_sequence *init_regs; > unsigned int init_count; > - const struct pll_rate_table *table; > + const struct pll_params_table *table; > u8 flags; > }; > > diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c > index e2d94498f098..9ac03c7bfbaa 100644 > --- a/drivers/clk/meson/gxbb.c > +++ b/drivers/clk/meson/gxbb.c > @@ -18,67 +18,67 @@ > > static DEFINE_SPINLOCK(meson_clk_lock); > > -static const struct pll_rate_table gxbb_gp0_pll_rate_table[] = { > - PLL_RATE(768000000, 32, 1), > - PLL_RATE(792000000, 33, 1), > - PLL_RATE(816000000, 34, 1), > - PLL_RATE(840000000, 35, 1), > - PLL_RATE(864000000, 36, 1), > - PLL_RATE(888000000, 37, 1), > - PLL_RATE(912000000, 38, 1), > - PLL_RATE(936000000, 39, 1), > - PLL_RATE(960000000, 40, 1), > - PLL_RATE(984000000, 41, 1), > - PLL_RATE(1008000000, 42, 1), > - PLL_RATE(1032000000, 43, 1), > - PLL_RATE(1056000000, 44, 1), > - PLL_RATE(1080000000, 45, 1), > - PLL_RATE(1104000000, 46, 1), > - PLL_RATE(1128000000, 47, 1), > - PLL_RATE(1152000000, 48, 1), > - PLL_RATE(1176000000, 49, 1), > - PLL_RATE(1200000000, 50, 1), > - PLL_RATE(1224000000, 51, 1), > - PLL_RATE(1248000000, 52, 1), > - PLL_RATE(1272000000, 53, 1), > - PLL_RATE(1296000000, 54, 1), > - PLL_RATE(1320000000, 55, 1), > - PLL_RATE(1344000000, 56, 1), > - PLL_RATE(1368000000, 57, 1), > - PLL_RATE(1392000000, 58, 1), > - PLL_RATE(1416000000, 59, 1), > - PLL_RATE(1440000000, 60, 1), > - PLL_RATE(1464000000, 61, 1), > - PLL_RATE(1488000000, 62, 1), > +static const struct pll_params_table gxbb_gp0_pll_params_table[] = { > + PLL_PARAMS(32, 1), > + PLL_PARAMS(33, 1), > + PLL_PARAMS(34, 1), > + PLL_PARAMS(35, 1), > + PLL_PARAMS(36, 1), > + PLL_PARAMS(37, 1), > + PLL_PARAMS(38, 1), > + PLL_PARAMS(39, 1), > + PLL_PARAMS(40, 1), > + PLL_PARAMS(41, 1), > + PLL_PARAMS(42, 1), > + PLL_PARAMS(43, 1), > + PLL_PARAMS(44, 1), > + PLL_PARAMS(45, 1), > + PLL_PARAMS(46, 1), > + PLL_PARAMS(47, 1), > + PLL_PARAMS(48, 1), > + PLL_PARAMS(49, 1), > + PLL_PARAMS(50, 1), > + PLL_PARAMS(51, 1), > + PLL_PARAMS(52, 1), > + PLL_PARAMS(53, 1), > + PLL_PARAMS(54, 1), > + PLL_PARAMS(55, 1), > + PLL_PARAMS(56, 1), > + PLL_PARAMS(57, 1), > + PLL_PARAMS(58, 1), > + PLL_PARAMS(59, 1), > + PLL_PARAMS(60, 1), > + PLL_PARAMS(61, 1), > + PLL_PARAMS(62, 1), > { /* sentinel */ }, > }; > > -static const struct pll_rate_table gxl_gp0_pll_rate_table[] = { > - PLL_RATE(1008000000, 42, 1), > - PLL_RATE(1032000000, 43, 1), > - PLL_RATE(1056000000, 44, 1), > - PLL_RATE(1080000000, 45, 1), > - PLL_RATE(1104000000, 46, 1), > - PLL_RATE(1128000000, 47, 1), > - PLL_RATE(1152000000, 48, 1), > - PLL_RATE(1176000000, 49, 1), > - PLL_RATE(1200000000, 50, 1), > - PLL_RATE(1224000000, 51, 1), > - PLL_RATE(1248000000, 52, 1), > - PLL_RATE(1272000000, 53, 1), > - PLL_RATE(1296000000, 54, 1), > - PLL_RATE(1320000000, 55, 1), > - PLL_RATE(1344000000, 56, 1), > - PLL_RATE(1368000000, 57, 1), > - PLL_RATE(1392000000, 58, 1), > - PLL_RATE(1416000000, 59, 1), > - PLL_RATE(1440000000, 60, 1), > - PLL_RATE(1464000000, 61, 1), > - PLL_RATE(1488000000, 62, 1), > - PLL_RATE(1512000000, 63, 1), > - PLL_RATE(1536000000, 64, 1), > - PLL_RATE(1560000000, 65, 1), > - PLL_RATE(1584000000, 66, 1), > +static const struct pll_params_table gxl_gp0_pll_params_table[] = { > + PLL_PARAMS(42, 1), > + PLL_PARAMS(43, 1), > + PLL_PARAMS(44, 1), > + PLL_PARAMS(45, 1), > + PLL_PARAMS(46, 1), > + PLL_PARAMS(47, 1), > + PLL_PARAMS(48, 1), > + PLL_PARAMS(49, 1), > + PLL_PARAMS(50, 1), > + PLL_PARAMS(51, 1), > + PLL_PARAMS(52, 1), > + PLL_PARAMS(53, 1), > + PLL_PARAMS(54, 1), > + PLL_PARAMS(55, 1), > + PLL_PARAMS(56, 1), > + PLL_PARAMS(57, 1), > + PLL_PARAMS(58, 1), > + PLL_PARAMS(59, 1), > + PLL_PARAMS(60, 1), > + PLL_PARAMS(61, 1), > + PLL_PARAMS(62, 1), > + PLL_PARAMS(63, 1), > + PLL_PARAMS(64, 1), > + PLL_PARAMS(65, 1), > + PLL_PARAMS(66, 1), > { /* sentinel */ }, > }; > > @@ -374,7 +374,7 @@ static struct clk_regmap gxbb_gp0_pll_dco = { > .shift = 29, > .width = 1, > }, > - .table = gxbb_gp0_pll_rate_table, > + .table = gxbb_gp0_pll_params_table, > .init_regs = gxbb_gp0_init_regs, > .init_count = ARRAY_SIZE(gxbb_gp0_init_regs), > }, > @@ -427,7 +427,7 @@ static struct clk_regmap gxl_gp0_pll_dco = { > .shift = 29, > .width = 1, > }, > - .table = gxl_gp0_pll_rate_table, > + .table = gxl_gp0_pll_params_table, > .init_regs = gxl_gp0_init_regs, > .init_count = ARRAY_SIZE(gxl_gp0_init_regs), > }, > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index 17bcf0be56ba..30ae849ff53f 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -29,22 +29,22 @@ struct meson8b_clk_reset { > void __iomem *base; > }; > > -static const struct pll_rate_table sys_pll_rate_table[] = { > - PLL_RATE(1200000000, 50, 1), > - PLL_RATE(1224000000, 51, 1), > - PLL_RATE(1248000000, 52, 1), > - PLL_RATE(1272000000, 53, 1), > - PLL_RATE(1296000000, 54, 1), > - PLL_RATE(1320000000, 55, 1), > - PLL_RATE(1344000000, 56, 1), > - PLL_RATE(1368000000, 57, 1), > - PLL_RATE(1392000000, 58, 1), > - PLL_RATE(1416000000, 59, 1), > - PLL_RATE(1440000000, 60, 1), > - PLL_RATE(1464000000, 61, 1), > - PLL_RATE(1488000000, 62, 1), > - PLL_RATE(1512000000, 63, 1), > - PLL_RATE(1536000000, 64, 1), > +static const struct pll_params_table sys_pll_params_table[] = { > + PLL_PARAMS(50, 1), > + PLL_PARAMS(51, 1), > + PLL_PARAMS(52, 1), > + PLL_PARAMS(53, 1), > + PLL_PARAMS(54, 1), > + PLL_PARAMS(55, 1), > + PLL_PARAMS(56, 1), > + PLL_PARAMS(57, 1), > + PLL_PARAMS(58, 1), > + PLL_PARAMS(59, 1), > + PLL_PARAMS(60, 1), > + PLL_PARAMS(61, 1), > + PLL_PARAMS(62, 1), > + PLL_PARAMS(63, 1), > + PLL_PARAMS(64, 1), > { /* sentinel */ }, > }; > > @@ -195,7 +195,7 @@ static struct clk_regmap meson8b_sys_pll_dco = { > .shift = 29, > .width = 1, > }, > - .table = sys_pll_rate_table, > + .table = sys_pll_params_table, > }, > .hw.init = &(struct clk_init_data){ > .name = "sys_pll_dco", > We could even add ranges instead of table when we know the PLL supports a well-known continuous dividers range. Acked-by: Neil Armstrong <narmstrong@baylibre.com>
On Thu, 2018-07-19 at 10:44 +0200, Neil Armstrong wrote: > We could even add ranges instead of table when we know the PLL supports a well-known continuous dividers range. I was thinking about this too. I did not went for it because it would mean yet another rework of the pll driver, which I did not had time to do now. I suspect that the min and max value of 'm' the pll can lock on might depend on the input rate of the DCO, so past the 'n' prediv. So, to replace the tuple (m, n) table with ranges, I think it would be best to take the predivider 'n' out first and try to clarify the contraints on the input rate of the DCO with amlogic ... we can also try and see ;) > > Acked-by: Neil Armstrong <narmstrong@baylibre.com>
On Thu, Jul 19, 2018 at 10:44 AM Neil Armstrong <narmstrong@baylibre.com> wrote: > > On 17/07/2018 11:56, Jerome Brunet wrote: > > Putting hard-coded rates inside the parameter tables assumes that > > the parent is known and will never change. That's a big assumption > > we should not make. > > > > We have everything we need to recalculate the output rate using > > the parent rate and the rest of the parameters. Let's do so and > > drop the rates from the tables. > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > > --- > > drivers/clk/meson/axg.c | 73 +++++++++++++-------------- > > drivers/clk/meson/clk-pll.c | 69 ++++++++++++++++--------- > > drivers/clk/meson/clkc.h | 8 ++- > > drivers/clk/meson/gxbb.c | 120 ++++++++++++++++++++++---------------------- > > drivers/clk/meson/meson8b.c | 34 ++++++------- > > 5 files changed, 162 insertions(+), 142 deletions(-) > > > > diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c > > index 572358062459..d34954bd8c5e 100644 > > --- a/drivers/clk/meson/axg.c > > +++ b/drivers/clk/meson/axg.c > > @@ -130,36 +130,36 @@ static struct clk_regmap axg_sys_pll = { > > }, > > }; > > > > -static const struct pll_rate_table axg_gp0_pll_rate_table[] = { > > - PLL_RATE(960000000, 40, 1), > > - PLL_RATE(984000000, 41, 1), > > - PLL_RATE(1008000000, 42, 1), > > - PLL_RATE(1032000000, 43, 1), > > - PLL_RATE(1056000000, 44, 1), > > - PLL_RATE(1080000000, 45, 1), > > - PLL_RATE(1104000000, 46, 1), > > - PLL_RATE(1128000000, 47, 1), > > - PLL_RATE(1152000000, 48, 1), > > - PLL_RATE(1176000000, 49, 1), > > - PLL_RATE(1200000000, 50, 1), > > - PLL_RATE(1224000000, 51, 1), > > - PLL_RATE(1248000000, 52, 1), > > - PLL_RATE(1272000000, 53, 1), > > - PLL_RATE(1296000000, 54, 1), > > - PLL_RATE(1320000000, 55, 1), > > - PLL_RATE(1344000000, 56, 1), > > - PLL_RATE(1368000000, 57, 1), > > - PLL_RATE(1392000000, 58, 1), > > - PLL_RATE(1416000000, 59, 1), > > - PLL_RATE(1440000000, 60, 1), > > - PLL_RATE(1464000000, 61, 1), > > - PLL_RATE(1488000000, 62, 1), > > - PLL_RATE(1512000000, 63, 1), > > - PLL_RATE(1536000000, 64, 1), > > - PLL_RATE(1560000000, 65, 1), > > - PLL_RATE(1584000000, 66, 1), > > - PLL_RATE(1608000000, 67, 1), > > - PLL_RATE(1632000000, 68, 1), > > +static const struct pll_params_table axg_gp0_pll_params_table[] = { > > + PLL_PARAMS(40, 1), > > + PLL_PARAMS(41, 1), > > + PLL_PARAMS(42, 1), > > + PLL_PARAMS(43, 1), > > + PLL_PARAMS(44, 1), > > + PLL_PARAMS(45, 1), > > + PLL_PARAMS(46, 1), > > + PLL_PARAMS(47, 1), > > + PLL_PARAMS(48, 1), > > + PLL_PARAMS(49, 1), > > + PLL_PARAMS(50, 1), > > + PLL_PARAMS(51, 1), > > + PLL_PARAMS(52, 1), > > + PLL_PARAMS(53, 1), > > + PLL_PARAMS(54, 1), > > + PLL_PARAMS(55, 1), > > + PLL_PARAMS(56, 1), > > + PLL_PARAMS(57, 1), > > + PLL_PARAMS(58, 1), > > + PLL_PARAMS(59, 1), > > + PLL_PARAMS(60, 1), > > + PLL_PARAMS(61, 1), > > + PLL_PARAMS(62, 1), > > + PLL_PARAMS(63, 1), > > + PLL_PARAMS(64, 1), > > + PLL_PARAMS(65, 1), > > + PLL_PARAMS(66, 1), > > + PLL_PARAMS(67, 1), > > + PLL_PARAMS(68, 1), > > { /* sentinel */ }, > > }; > > > > @@ -203,7 +203,7 @@ static struct clk_regmap axg_gp0_pll_dco = { > > .shift = 29, > > .width = 1, > > }, > > - .table = axg_gp0_pll_rate_table, > > + .table = axg_gp0_pll_params_table, > > .init_regs = axg_gp0_init_regs, > > .init_count = ARRAY_SIZE(axg_gp0_init_regs), > > }, > > @@ -271,7 +271,7 @@ static struct clk_regmap axg_hifi_pll_dco = { > > .shift = 29, > > .width = 1, > > }, > > - .table = axg_gp0_pll_rate_table, > > + .table = axg_gp0_pll_params_table, > > .init_regs = axg_hifi_init_regs, > > .init_count = ARRAY_SIZE(axg_hifi_init_regs), > > .flags = CLK_MESON_PLL_ROUND_CLOSEST, > > @@ -627,11 +627,10 @@ static struct clk_regmap axg_mpll3 = { > > }, > > }; > > > > -static const struct pll_rate_table axg_pcie_pll_rate_table[] = { > > +static const struct pll_params_table axg_pcie_pll_params_table[] = { > > { > > - .rate = 1600000000, > > - .m = 200, > > - .n = 3, > > + .m = 200, > > + .n = 3, > > }, > > { /* sentinel */ }, > > }; > > @@ -678,7 +677,7 @@ static struct clk_regmap axg_pcie_pll_dco = { > > .shift = 29, > > .width = 1, > > }, > > - .table = axg_pcie_pll_rate_table, > > + .table = axg_pcie_pll_params_table, > > .init_regs = axg_pcie_init_regs, > > .init_count = ARRAY_SIZE(axg_pcie_init_regs), > > }, > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > > index 348a866f09eb..f5b5b3fabe3c 100644 > > --- a/drivers/clk/meson/clk-pll.c > > +++ b/drivers/clk/meson/clk-pll.c > > @@ -45,7 +45,7 @@ meson_clk_pll_data(struct clk_regmap *clk) > > } > > > > static unsigned long __pll_params_to_rate(unsigned long parent_rate, > > - const struct pll_rate_table *pllt, > > + const struct pll_params_table *pllt, > > u16 frac, > > struct meson_clk_pll_data *pll) > > { > > @@ -66,7 +66,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw, > > { > > struct clk_regmap *clk = to_clk_regmap(hw); > > struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > > - struct pll_rate_table pllt; > > + struct pll_params_table pllt; > > u16 frac; > > > > pllt.n = meson_parm_read(clk->map, &pll->n); > > @@ -81,7 +81,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw, > > > > static u16 __pll_params_with_frac(unsigned long rate, > > unsigned long parent_rate, > > - const struct pll_rate_table *pllt, > > + const struct pll_params_table *pllt, > > struct meson_clk_pll_data *pll) > > { > > u16 frac_max = (1 << pll->frac.width); > > @@ -97,29 +97,50 @@ static u16 __pll_params_with_frac(unsigned long rate, > > return min((u16)val, (u16)(frac_max - 1)); > > } > > > > -static const struct pll_rate_table * > > +static bool meson_clk_pll_is_better(unsigned long rate, > > + unsigned long best, > > + unsigned long now, > > + struct meson_clk_pll_data *pll) > > +{ > > + if (!(pll->flags & CLK_MESON_PLL_ROUND_CLOSEST) || > > + MESON_PARM_APPLICABLE(&pll->frac)) { > > + /* Round down */ > > + if (now < rate && best < now) > > + return true; > > + } else { > > + /* Round Closest */ > > + if (abs(now - rate) < abs(best - rate)) > > + return true; > > + } > > + > > + return false; > > +} > > + > > +static const struct pll_params_table * > > meson_clk_get_pll_settings(unsigned long rate, > > + unsigned long parent_rate, > > struct meson_clk_pll_data *pll) > > { > > - const struct pll_rate_table *table = pll->table; > > - unsigned int i = 0; > > + const struct pll_params_table *table = pll->table; > > + unsigned long best = 0, now = 0; > > + unsigned int i, best_i = 0; > > > > if (!table) > > return NULL; > > > > - /* Find the first table element exceeding rate */ > > - while (table[i].rate && table[i].rate <= rate) > > - i++; > > + for (i = 0; table[i].n; i++) { > > + now = __pll_params_to_rate(parent_rate, &table[i], 0, pll); > > > > - if (i != 0) { > > - if (MESON_PARM_APPLICABLE(&pll->frac) || > > - !(pll->flags & CLK_MESON_PLL_ROUND_CLOSEST) || > > - (abs(rate - table[i - 1].rate) < > > - abs(rate - table[i].rate))) > > - i--; > > + /* If we get an exact match, don't bother any further */ > > + if (now == rate) { > > + return &table[i]; > > + } else if (meson_clk_pll_is_better(rate, best, now, pll)) { > > + best = now; > > + best_i = i; > > + } > > } > > > > - return (struct pll_rate_table *)&table[i]; > > + return (struct pll_params_table *)&table[best_i]; > > } > > > > static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate, > > @@ -127,16 +148,18 @@ static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate, > > { > > struct clk_regmap *clk = to_clk_regmap(hw); > > struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > > - const struct pll_rate_table *pllt = > > - meson_clk_get_pll_settings(rate, pll); > > + const struct pll_params_table *pllt = > > + meson_clk_get_pll_settings(rate, *parent_rate, pll); > > + unsigned long round; > > u16 frac; > > > > if (!pllt) > > return meson_clk_pll_recalc_rate(hw, *parent_rate); > > > > - if (!MESON_PARM_APPLICABLE(&pll->frac) > > - || rate == pllt->rate) > > - return pllt->rate; > > + round = __pll_params_to_rate(*parent_rate, pllt, 0, pll); > > + > > + if (!MESON_PARM_APPLICABLE(&pll->frac) || rate == round) > > + return round; > > > > /* > > * The rate provided by the setting is not an exact match, let's > > @@ -214,7 +237,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > > { > > struct clk_regmap *clk = to_clk_regmap(hw); > > struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > > - const struct pll_rate_table *pllt; > > + const struct pll_params_table *pllt; > > unsigned int enabled; > > unsigned long old_rate; > > u16 frac = 0; > > @@ -224,7 +247,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > > > > old_rate = rate; > > > > - pllt = meson_clk_get_pll_settings(rate, pll); > > + pllt = meson_clk_get_pll_settings(rate, parent_rate, pll); > > if (!pllt) > > return -EINVAL; > > > > diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h > > index a2245e857f70..6b96d55c047d 100644 > > --- a/drivers/clk/meson/clkc.h > > +++ b/drivers/clk/meson/clkc.h > > @@ -43,15 +43,13 @@ static inline void meson_parm_write(struct regmap *map, struct parm *p, > > } > > > > > > -struct pll_rate_table { > > - unsigned long rate; > > +struct pll_params_table { > > u16 m; > > u16 n; > > }; > > > > -#define PLL_RATE(_r, _m, _n) \ > > +#define PLL_PARAMS(_m, _n) \ > > { \ > > - .rate = (_r), \ > > .m = (_m), \ > > .n = (_n), \ > > } > > @@ -67,7 +65,7 @@ struct meson_clk_pll_data { > > struct parm rst; > > const struct reg_sequence *init_regs; > > unsigned int init_count; > > - const struct pll_rate_table *table; > > + const struct pll_params_table *table; > > u8 flags; > > }; > > > > diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c > > index e2d94498f098..9ac03c7bfbaa 100644 > > --- a/drivers/clk/meson/gxbb.c > > +++ b/drivers/clk/meson/gxbb.c > > @@ -18,67 +18,67 @@ > > > > static DEFINE_SPINLOCK(meson_clk_lock); > > > > -static const struct pll_rate_table gxbb_gp0_pll_rate_table[] = { > > - PLL_RATE(768000000, 32, 1), > > - PLL_RATE(792000000, 33, 1), > > - PLL_RATE(816000000, 34, 1), > > - PLL_RATE(840000000, 35, 1), > > - PLL_RATE(864000000, 36, 1), > > - PLL_RATE(888000000, 37, 1), > > - PLL_RATE(912000000, 38, 1), > > - PLL_RATE(936000000, 39, 1), > > - PLL_RATE(960000000, 40, 1), > > - PLL_RATE(984000000, 41, 1), > > - PLL_RATE(1008000000, 42, 1), > > - PLL_RATE(1032000000, 43, 1), > > - PLL_RATE(1056000000, 44, 1), > > - PLL_RATE(1080000000, 45, 1), > > - PLL_RATE(1104000000, 46, 1), > > - PLL_RATE(1128000000, 47, 1), > > - PLL_RATE(1152000000, 48, 1), > > - PLL_RATE(1176000000, 49, 1), > > - PLL_RATE(1200000000, 50, 1), > > - PLL_RATE(1224000000, 51, 1), > > - PLL_RATE(1248000000, 52, 1), > > - PLL_RATE(1272000000, 53, 1), > > - PLL_RATE(1296000000, 54, 1), > > - PLL_RATE(1320000000, 55, 1), > > - PLL_RATE(1344000000, 56, 1), > > - PLL_RATE(1368000000, 57, 1), > > - PLL_RATE(1392000000, 58, 1), > > - PLL_RATE(1416000000, 59, 1), > > - PLL_RATE(1440000000, 60, 1), > > - PLL_RATE(1464000000, 61, 1), > > - PLL_RATE(1488000000, 62, 1), > > +static const struct pll_params_table gxbb_gp0_pll_params_table[] = { > > + PLL_PARAMS(32, 1), > > + PLL_PARAMS(33, 1), > > + PLL_PARAMS(34, 1), > > + PLL_PARAMS(35, 1), > > + PLL_PARAMS(36, 1), > > + PLL_PARAMS(37, 1), > > + PLL_PARAMS(38, 1), > > + PLL_PARAMS(39, 1), > > + PLL_PARAMS(40, 1), > > + PLL_PARAMS(41, 1), > > + PLL_PARAMS(42, 1), > > + PLL_PARAMS(43, 1), > > + PLL_PARAMS(44, 1), > > + PLL_PARAMS(45, 1), > > + PLL_PARAMS(46, 1), > > + PLL_PARAMS(47, 1), > > + PLL_PARAMS(48, 1), > > + PLL_PARAMS(49, 1), > > + PLL_PARAMS(50, 1), > > + PLL_PARAMS(51, 1), > > + PLL_PARAMS(52, 1), > > + PLL_PARAMS(53, 1), > > + PLL_PARAMS(54, 1), > > + PLL_PARAMS(55, 1), > > + PLL_PARAMS(56, 1), > > + PLL_PARAMS(57, 1), > > + PLL_PARAMS(58, 1), > > + PLL_PARAMS(59, 1), > > + PLL_PARAMS(60, 1), > > + PLL_PARAMS(61, 1), > > + PLL_PARAMS(62, 1), > > { /* sentinel */ }, > > }; > > > > -static const struct pll_rate_table gxl_gp0_pll_rate_table[] = { > > - PLL_RATE(1008000000, 42, 1), > > - PLL_RATE(1032000000, 43, 1), > > - PLL_RATE(1056000000, 44, 1), > > - PLL_RATE(1080000000, 45, 1), > > - PLL_RATE(1104000000, 46, 1), > > - PLL_RATE(1128000000, 47, 1), > > - PLL_RATE(1152000000, 48, 1), > > - PLL_RATE(1176000000, 49, 1), > > - PLL_RATE(1200000000, 50, 1), > > - PLL_RATE(1224000000, 51, 1), > > - PLL_RATE(1248000000, 52, 1), > > - PLL_RATE(1272000000, 53, 1), > > - PLL_RATE(1296000000, 54, 1), > > - PLL_RATE(1320000000, 55, 1), > > - PLL_RATE(1344000000, 56, 1), > > - PLL_RATE(1368000000, 57, 1), > > - PLL_RATE(1392000000, 58, 1), > > - PLL_RATE(1416000000, 59, 1), > > - PLL_RATE(1440000000, 60, 1), > > - PLL_RATE(1464000000, 61, 1), > > - PLL_RATE(1488000000, 62, 1), > > - PLL_RATE(1512000000, 63, 1), > > - PLL_RATE(1536000000, 64, 1), > > - PLL_RATE(1560000000, 65, 1), > > - PLL_RATE(1584000000, 66, 1), > > +static const struct pll_params_table gxl_gp0_pll_params_table[] = { > > + PLL_PARAMS(42, 1), > > + PLL_PARAMS(43, 1), > > + PLL_PARAMS(44, 1), > > + PLL_PARAMS(45, 1), > > + PLL_PARAMS(46, 1), > > + PLL_PARAMS(47, 1), > > + PLL_PARAMS(48, 1), > > + PLL_PARAMS(49, 1), > > + PLL_PARAMS(50, 1), > > + PLL_PARAMS(51, 1), > > + PLL_PARAMS(52, 1), > > + PLL_PARAMS(53, 1), > > + PLL_PARAMS(54, 1), > > + PLL_PARAMS(55, 1), > > + PLL_PARAMS(56, 1), > > + PLL_PARAMS(57, 1), > > + PLL_PARAMS(58, 1), > > + PLL_PARAMS(59, 1), > > + PLL_PARAMS(60, 1), > > + PLL_PARAMS(61, 1), > > + PLL_PARAMS(62, 1), > > + PLL_PARAMS(63, 1), > > + PLL_PARAMS(64, 1), > > + PLL_PARAMS(65, 1), > > + PLL_PARAMS(66, 1), > > { /* sentinel */ }, > > }; > > > > @@ -374,7 +374,7 @@ static struct clk_regmap gxbb_gp0_pll_dco = { > > .shift = 29, > > .width = 1, > > }, > > - .table = gxbb_gp0_pll_rate_table, > > + .table = gxbb_gp0_pll_params_table, > > .init_regs = gxbb_gp0_init_regs, > > .init_count = ARRAY_SIZE(gxbb_gp0_init_regs), > > }, > > @@ -427,7 +427,7 @@ static struct clk_regmap gxl_gp0_pll_dco = { > > .shift = 29, > > .width = 1, > > }, > > - .table = gxl_gp0_pll_rate_table, > > + .table = gxl_gp0_pll_params_table, > > .init_regs = gxl_gp0_init_regs, > > .init_count = ARRAY_SIZE(gxl_gp0_init_regs), > > }, > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > > index 17bcf0be56ba..30ae849ff53f 100644 > > --- a/drivers/clk/meson/meson8b.c > > +++ b/drivers/clk/meson/meson8b.c > > @@ -29,22 +29,22 @@ struct meson8b_clk_reset { > > void __iomem *base; > > }; > > > > -static const struct pll_rate_table sys_pll_rate_table[] = { > > - PLL_RATE(1200000000, 50, 1), > > - PLL_RATE(1224000000, 51, 1), > > - PLL_RATE(1248000000, 52, 1), > > - PLL_RATE(1272000000, 53, 1), > > - PLL_RATE(1296000000, 54, 1), > > - PLL_RATE(1320000000, 55, 1), > > - PLL_RATE(1344000000, 56, 1), > > - PLL_RATE(1368000000, 57, 1), > > - PLL_RATE(1392000000, 58, 1), > > - PLL_RATE(1416000000, 59, 1), > > - PLL_RATE(1440000000, 60, 1), > > - PLL_RATE(1464000000, 61, 1), > > - PLL_RATE(1488000000, 62, 1), > > - PLL_RATE(1512000000, 63, 1), > > - PLL_RATE(1536000000, 64, 1), > > +static const struct pll_params_table sys_pll_params_table[] = { > > + PLL_PARAMS(50, 1), > > + PLL_PARAMS(51, 1), > > + PLL_PARAMS(52, 1), > > + PLL_PARAMS(53, 1), > > + PLL_PARAMS(54, 1), > > + PLL_PARAMS(55, 1), > > + PLL_PARAMS(56, 1), > > + PLL_PARAMS(57, 1), > > + PLL_PARAMS(58, 1), > > + PLL_PARAMS(59, 1), > > + PLL_PARAMS(60, 1), > > + PLL_PARAMS(61, 1), > > + PLL_PARAMS(62, 1), > > + PLL_PARAMS(63, 1), > > + PLL_PARAMS(64, 1), > > { /* sentinel */ }, > > }; > > > > @@ -195,7 +195,7 @@ static struct clk_regmap meson8b_sys_pll_dco = { > > .shift = 29, > > .width = 1, > > }, > > - .table = sys_pll_rate_table, > > + .table = sys_pll_params_table, > > }, > > .hw.init = &(struct clk_init_data){ > > .name = "sys_pll_dco", > > > > We could even add ranges instead of table when we know the PLL supports a well-known continuous dividers range. I had a look at the sys_pll settings on Meson8b, here's what Meson8/Meson8b/Meson8m2 support for sys_pll: - 50..74 - 76 - 78 - 80 - 82 - 84 - 86 - 88 - 90 - 92 - 94 - 96 - 98 (I'm providing this info because it may help finding a decision whether ranges are good or not. I have no preference) Regards Martin [0] https://github.com/endlessm/linux-meson/blob/master/arch/arm/mach-meson8/clock.c#L140
On Sat, 2018-07-21 at 22:16 +0200, Martin Blumenstingl wrote: > > We could even add ranges instead of table when we know the PLL supports a well-known continuous dividers range. > > I had a look at the sys_pll settings on Meson8b, here's what > Meson8/Meson8b/Meson8m2 support for sys_pll: > - 50..74 > - 76 > - 78 > - 80 > - 82 > - 84 > - 86 > - 88 > - 90 > - 92 > - 94 > - 96 > - 98 Are those values with the same predivider (n) value ? I suspect the ability of the DCO to lock might depends on its input rate and an m range So if n change, it might possible that the m range will be different. ... at least, that's my guess :) > > (I'm providing this info because it may help finding a decision > whether ranges are good or not. I have no preference)
Hi Jerome, On Sat, Jul 21, 2018 at 10:46 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > On Sat, 2018-07-21 at 22:16 +0200, Martin Blumenstingl wrote: > > > We could even add ranges instead of table when we know the PLL supports a well-known continuous dividers range. > > > > I had a look at the sys_pll settings on Meson8b, here's what > > Meson8/Meson8b/Meson8m2 support for sys_pll: > > - 50..74 > > - 76 > > - 78 > > - 80 > > - 82 > > - 84 > > - 86 > > - 88 > > - 90 > > - 92 > > - 94 > > - 96 > > - 98 > > Are those values with the same predivider (n) value ? yes, all are using n = 1 > I suspect the ability of the DCO to lock might depends on its input rate and an > m range > > So if n change, it might possible that the m range will be different. > > ... at least, that's my guess :) for the Meson8b's SYS_PLL Amlogic's GPL kernel is using bits 15:14 in HHI_SYS_PLL_CNTL for frequencies above 1.64GHz (below that these reserved bits are always 0). if the values are useful for you (format is: frequency, m, n, od and bits 15:14. what's missing here is the "cpu_scale_div" divider which is for example why you'll see the "56, 1, 2, 0" tuple multiple times): SYS_PLL(24000, 56, 1, 2, 0) SYS_PLL(48000, 64, 1, 2, 0) SYS_PLL(72000, 72, 1, 2, 0) SYS_PLL(96000, 64, 1, 2, 0) SYS_PLL(120000, 80, 1, 2, 0) SYS_PLL(144000, 96, 1, 2, 0) SYS_PLL(168000, 56, 1, 1, 0) SYS_PLL(192000, 64, 1, 1, 0) SYS_PLL(216000, 72, 1, 1, 0) SYS_PLL(240000, 80, 1, 1, 0) SYS_PLL(264000, 88, 1, 1, 0) SYS_PLL(288000, 96, 1, 1, 0) SYS_PLL(312000, 52, 1, 2, 0) SYS_PLL(336000, 56, 1, 2, 0) SYS_PLL(360000, 60, 1, 2, 0) SYS_PLL(384000, 64, 1, 2, 0) SYS_PLL(408000, 68, 1, 2, 0) SYS_PLL(432000, 72, 1, 2, 0) SYS_PLL(456000, 76, 1, 2, 0) SYS_PLL(480000, 80, 1, 2, 0) SYS_PLL(504000, 84, 1, 2, 0) SYS_PLL(528000, 88, 1, 2, 0) SYS_PLL(552000, 92, 1, 2, 0) SYS_PLL(576000, 96, 1, 2, 0) SYS_PLL(600000, 50, 1, 1, 0) SYS_PLL(624000, 52, 1, 1, 0) SYS_PLL(648000, 54, 1, 1, 0) SYS_PLL(672000, 56, 1, 1, 0) SYS_PLL(696000, 58, 1, 1, 0) SYS_PLL(720000, 60, 1, 1, 0) SYS_PLL(744000, 62, 1, 1, 0) SYS_PLL(768000, 64, 1, 1, 0) SYS_PLL(792000, 66, 1, 1, 0) SYS_PLL(816000, 68, 1, 1, 0) SYS_PLL(840000, 70, 1, 1, 0) SYS_PLL(864000, 72, 1, 1, 0) SYS_PLL(888000, 74, 1, 1, 0) SYS_PLL(912000, 76, 1, 1, 0) SYS_PLL(936000, 78, 1, 1, 0) SYS_PLL(960000, 80, 1, 1, 0) SYS_PLL(984000, 82, 1, 1, 0) SYS_PLL(1008000, 84, 1, 1, 0) SYS_PLL(1032000, 86, 1, 1, 0) SYS_PLL(1056000, 88, 1, 1, 0) SYS_PLL(1080000, 90, 1, 1, 0) SYS_PLL(1104000, 92, 1, 1, 0) SYS_PLL(1128000, 94, 1, 1, 0) SYS_PLL(1152000, 96, 1, 1, 0) SYS_PLL(1176000, 98, 1, 1, 0) SYS_PLL(1200000, 50, 1, 0, 0) SYS_PLL(1224000, 51, 1, 0, 0) SYS_PLL(1248000, 52, 1, 0, 0) SYS_PLL(1272000, 53, 1, 0, 0) SYS_PLL(1296000, 54, 1, 0, 0) SYS_PLL(1320000, 55, 1, 0, 0) SYS_PLL(1344000, 56, 1, 0, 0) SYS_PLL(1368000, 57, 1, 0, 0) SYS_PLL(1392000, 58, 1, 0, 0) SYS_PLL(1416000, 59, 1, 0, 0) SYS_PLL(1440000, 60, 1, 0, 0) SYS_PLL(1464000, 61, 1, 0, 0) SYS_PLL(1488000, 62, 1, 0, 0) SYS_PLL(1512000, 63, 1, 0, 0) SYS_PLL(1536000, 64, 1, 0, 0) SYS_PLL(1560000, 65, 1, 0, 0) SYS_PLL(1584000, 66, 1, 0, 0) SYS_PLL(1608000, 67, 1, 0, 0) SYS_PLL(1632000, 68, 1, 0, 0) SYS_PLL(1656000, 68, 1, 0, 1) SYS_PLL(1680000, 68, 1, 0, 2) SYS_PLL(1704000, 68, 1, 0, 3) SYS_PLL(1728000, 69, 1, 0, 0) SYS_PLL(1752000, 69, 1, 0, 1) SYS_PLL(1776000, 69, 1, 0, 2) SYS_PLL(1800000, 69, 1, 0, 3) SYS_PLL(1824000, 70, 1, 0, 0) SYS_PLL(1848000, 70, 1, 0, 1) SYS_PLL(1872000, 70, 1, 0, 2) SYS_PLL(1896000, 70, 1, 0, 3) SYS_PLL(1920000, 71, 1, 0, 0) SYS_PLL(1944000, 71, 1, 0, 1) SYS_PLL(1968000, 71, 1, 0, 2) SYS_PLL(1992000, 71, 1, 0, 3) SYS_PLL(2016000, 72, 1, 0, 0) SYS_PLL(2040000, 72, 1, 0, 1) SYS_PLL(2064000, 72, 1, 0, 2) SYS_PLL(2088000, 72, 1, 0, 3) SYS_PLL(2112000, 73, 1, 0, 0) Regards Martin
On Sat, 2018-07-21 at 23:34 +0200, Martin Blumenstingl wrote: > On Sat, Jul 21, 2018 at 10:46 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > > > On Sat, 2018-07-21 at 22:16 +0200, Martin Blumenstingl wrote: > > > > We could even add ranges instead of table when we know the PLL supports a well-known continuous dividers range. > > > > > > I had a look at the sys_pll settings on Meson8b, here's what > > > Meson8/Meson8b/Meson8m2 support for sys_pll: > > > - 50..74 > > > - 76 > > > - 78 > > > - 80 > > > - 82 > > > - 84 > > > - 86 > > > - 88 > > > - 90 > > > - 92 > > > - 94 > > > - 96 > > > - 98 > > > > Are those values with the same predivider (n) value ? > > yes, all are using n = 1 The table proposed in this patch keeps things the way they were before the change. We could extend the table with these values in a follow up patch. If those value are with n=1, then I would guess that odd values from 75 to 97 work as well.
diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c index 572358062459..d34954bd8c5e 100644 --- a/drivers/clk/meson/axg.c +++ b/drivers/clk/meson/axg.c @@ -130,36 +130,36 @@ static struct clk_regmap axg_sys_pll = { }, }; -static const struct pll_rate_table axg_gp0_pll_rate_table[] = { - PLL_RATE(960000000, 40, 1), - PLL_RATE(984000000, 41, 1), - PLL_RATE(1008000000, 42, 1), - PLL_RATE(1032000000, 43, 1), - PLL_RATE(1056000000, 44, 1), - PLL_RATE(1080000000, 45, 1), - PLL_RATE(1104000000, 46, 1), - PLL_RATE(1128000000, 47, 1), - PLL_RATE(1152000000, 48, 1), - PLL_RATE(1176000000, 49, 1), - PLL_RATE(1200000000, 50, 1), - PLL_RATE(1224000000, 51, 1), - PLL_RATE(1248000000, 52, 1), - PLL_RATE(1272000000, 53, 1), - PLL_RATE(1296000000, 54, 1), - PLL_RATE(1320000000, 55, 1), - PLL_RATE(1344000000, 56, 1), - PLL_RATE(1368000000, 57, 1), - PLL_RATE(1392000000, 58, 1), - PLL_RATE(1416000000, 59, 1), - PLL_RATE(1440000000, 60, 1), - PLL_RATE(1464000000, 61, 1), - PLL_RATE(1488000000, 62, 1), - PLL_RATE(1512000000, 63, 1), - PLL_RATE(1536000000, 64, 1), - PLL_RATE(1560000000, 65, 1), - PLL_RATE(1584000000, 66, 1), - PLL_RATE(1608000000, 67, 1), - PLL_RATE(1632000000, 68, 1), +static const struct pll_params_table axg_gp0_pll_params_table[] = { + PLL_PARAMS(40, 1), + PLL_PARAMS(41, 1), + PLL_PARAMS(42, 1), + PLL_PARAMS(43, 1), + PLL_PARAMS(44, 1), + PLL_PARAMS(45, 1), + PLL_PARAMS(46, 1), + PLL_PARAMS(47, 1), + PLL_PARAMS(48, 1), + PLL_PARAMS(49, 1), + PLL_PARAMS(50, 1), + PLL_PARAMS(51, 1), + PLL_PARAMS(52, 1), + PLL_PARAMS(53, 1), + PLL_PARAMS(54, 1), + PLL_PARAMS(55, 1), + PLL_PARAMS(56, 1), + PLL_PARAMS(57, 1), + PLL_PARAMS(58, 1), + PLL_PARAMS(59, 1), + PLL_PARAMS(60, 1), + PLL_PARAMS(61, 1), + PLL_PARAMS(62, 1), + PLL_PARAMS(63, 1), + PLL_PARAMS(64, 1), + PLL_PARAMS(65, 1), + PLL_PARAMS(66, 1), + PLL_PARAMS(67, 1), + PLL_PARAMS(68, 1), { /* sentinel */ }, }; @@ -203,7 +203,7 @@ static struct clk_regmap axg_gp0_pll_dco = { .shift = 29, .width = 1, }, - .table = axg_gp0_pll_rate_table, + .table = axg_gp0_pll_params_table, .init_regs = axg_gp0_init_regs, .init_count = ARRAY_SIZE(axg_gp0_init_regs), }, @@ -271,7 +271,7 @@ static struct clk_regmap axg_hifi_pll_dco = { .shift = 29, .width = 1, }, - .table = axg_gp0_pll_rate_table, + .table = axg_gp0_pll_params_table, .init_regs = axg_hifi_init_regs, .init_count = ARRAY_SIZE(axg_hifi_init_regs), .flags = CLK_MESON_PLL_ROUND_CLOSEST, @@ -627,11 +627,10 @@ static struct clk_regmap axg_mpll3 = { }, }; -static const struct pll_rate_table axg_pcie_pll_rate_table[] = { +static const struct pll_params_table axg_pcie_pll_params_table[] = { { - .rate = 1600000000, - .m = 200, - .n = 3, + .m = 200, + .n = 3, }, { /* sentinel */ }, }; @@ -678,7 +677,7 @@ static struct clk_regmap axg_pcie_pll_dco = { .shift = 29, .width = 1, }, - .table = axg_pcie_pll_rate_table, + .table = axg_pcie_pll_params_table, .init_regs = axg_pcie_init_regs, .init_count = ARRAY_SIZE(axg_pcie_init_regs), }, diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c index 348a866f09eb..f5b5b3fabe3c 100644 --- a/drivers/clk/meson/clk-pll.c +++ b/drivers/clk/meson/clk-pll.c @@ -45,7 +45,7 @@ meson_clk_pll_data(struct clk_regmap *clk) } static unsigned long __pll_params_to_rate(unsigned long parent_rate, - const struct pll_rate_table *pllt, + const struct pll_params_table *pllt, u16 frac, struct meson_clk_pll_data *pll) { @@ -66,7 +66,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw, { struct clk_regmap *clk = to_clk_regmap(hw); struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); - struct pll_rate_table pllt; + struct pll_params_table pllt; u16 frac; pllt.n = meson_parm_read(clk->map, &pll->n); @@ -81,7 +81,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw, static u16 __pll_params_with_frac(unsigned long rate, unsigned long parent_rate, - const struct pll_rate_table *pllt, + const struct pll_params_table *pllt, struct meson_clk_pll_data *pll) { u16 frac_max = (1 << pll->frac.width); @@ -97,29 +97,50 @@ static u16 __pll_params_with_frac(unsigned long rate, return min((u16)val, (u16)(frac_max - 1)); } -static const struct pll_rate_table * +static bool meson_clk_pll_is_better(unsigned long rate, + unsigned long best, + unsigned long now, + struct meson_clk_pll_data *pll) +{ + if (!(pll->flags & CLK_MESON_PLL_ROUND_CLOSEST) || + MESON_PARM_APPLICABLE(&pll->frac)) { + /* Round down */ + if (now < rate && best < now) + return true; + } else { + /* Round Closest */ + if (abs(now - rate) < abs(best - rate)) + return true; + } + + return false; +} + +static const struct pll_params_table * meson_clk_get_pll_settings(unsigned long rate, + unsigned long parent_rate, struct meson_clk_pll_data *pll) { - const struct pll_rate_table *table = pll->table; - unsigned int i = 0; + const struct pll_params_table *table = pll->table; + unsigned long best = 0, now = 0; + unsigned int i, best_i = 0; if (!table) return NULL; - /* Find the first table element exceeding rate */ - while (table[i].rate && table[i].rate <= rate) - i++; + for (i = 0; table[i].n; i++) { + now = __pll_params_to_rate(parent_rate, &table[i], 0, pll); - if (i != 0) { - if (MESON_PARM_APPLICABLE(&pll->frac) || - !(pll->flags & CLK_MESON_PLL_ROUND_CLOSEST) || - (abs(rate - table[i - 1].rate) < - abs(rate - table[i].rate))) - i--; + /* If we get an exact match, don't bother any further */ + if (now == rate) { + return &table[i]; + } else if (meson_clk_pll_is_better(rate, best, now, pll)) { + best = now; + best_i = i; + } } - return (struct pll_rate_table *)&table[i]; + return (struct pll_params_table *)&table[best_i]; } static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate, @@ -127,16 +148,18 @@ static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate, { struct clk_regmap *clk = to_clk_regmap(hw); struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); - const struct pll_rate_table *pllt = - meson_clk_get_pll_settings(rate, pll); + const struct pll_params_table *pllt = + meson_clk_get_pll_settings(rate, *parent_rate, pll); + unsigned long round; u16 frac; if (!pllt) return meson_clk_pll_recalc_rate(hw, *parent_rate); - if (!MESON_PARM_APPLICABLE(&pll->frac) - || rate == pllt->rate) - return pllt->rate; + round = __pll_params_to_rate(*parent_rate, pllt, 0, pll); + + if (!MESON_PARM_APPLICABLE(&pll->frac) || rate == round) + return round; /* * The rate provided by the setting is not an exact match, let's @@ -214,7 +237,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, { struct clk_regmap *clk = to_clk_regmap(hw); struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); - const struct pll_rate_table *pllt; + const struct pll_params_table *pllt; unsigned int enabled; unsigned long old_rate; u16 frac = 0; @@ -224,7 +247,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, old_rate = rate; - pllt = meson_clk_get_pll_settings(rate, pll); + pllt = meson_clk_get_pll_settings(rate, parent_rate, pll); if (!pllt) return -EINVAL; diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h index a2245e857f70..6b96d55c047d 100644 --- a/drivers/clk/meson/clkc.h +++ b/drivers/clk/meson/clkc.h @@ -43,15 +43,13 @@ static inline void meson_parm_write(struct regmap *map, struct parm *p, } -struct pll_rate_table { - unsigned long rate; +struct pll_params_table { u16 m; u16 n; }; -#define PLL_RATE(_r, _m, _n) \ +#define PLL_PARAMS(_m, _n) \ { \ - .rate = (_r), \ .m = (_m), \ .n = (_n), \ } @@ -67,7 +65,7 @@ struct meson_clk_pll_data { struct parm rst; const struct reg_sequence *init_regs; unsigned int init_count; - const struct pll_rate_table *table; + const struct pll_params_table *table; u8 flags; }; diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c index e2d94498f098..9ac03c7bfbaa 100644 --- a/drivers/clk/meson/gxbb.c +++ b/drivers/clk/meson/gxbb.c @@ -18,67 +18,67 @@ static DEFINE_SPINLOCK(meson_clk_lock); -static const struct pll_rate_table gxbb_gp0_pll_rate_table[] = { - PLL_RATE(768000000, 32, 1), - PLL_RATE(792000000, 33, 1), - PLL_RATE(816000000, 34, 1), - PLL_RATE(840000000, 35, 1), - PLL_RATE(864000000, 36, 1), - PLL_RATE(888000000, 37, 1), - PLL_RATE(912000000, 38, 1), - PLL_RATE(936000000, 39, 1), - PLL_RATE(960000000, 40, 1), - PLL_RATE(984000000, 41, 1), - PLL_RATE(1008000000, 42, 1), - PLL_RATE(1032000000, 43, 1), - PLL_RATE(1056000000, 44, 1), - PLL_RATE(1080000000, 45, 1), - PLL_RATE(1104000000, 46, 1), - PLL_RATE(1128000000, 47, 1), - PLL_RATE(1152000000, 48, 1), - PLL_RATE(1176000000, 49, 1), - PLL_RATE(1200000000, 50, 1), - PLL_RATE(1224000000, 51, 1), - PLL_RATE(1248000000, 52, 1), - PLL_RATE(1272000000, 53, 1), - PLL_RATE(1296000000, 54, 1), - PLL_RATE(1320000000, 55, 1), - PLL_RATE(1344000000, 56, 1), - PLL_RATE(1368000000, 57, 1), - PLL_RATE(1392000000, 58, 1), - PLL_RATE(1416000000, 59, 1), - PLL_RATE(1440000000, 60, 1), - PLL_RATE(1464000000, 61, 1), - PLL_RATE(1488000000, 62, 1), +static const struct pll_params_table gxbb_gp0_pll_params_table[] = { + PLL_PARAMS(32, 1), + PLL_PARAMS(33, 1), + PLL_PARAMS(34, 1), + PLL_PARAMS(35, 1), + PLL_PARAMS(36, 1), + PLL_PARAMS(37, 1), + PLL_PARAMS(38, 1), + PLL_PARAMS(39, 1), + PLL_PARAMS(40, 1), + PLL_PARAMS(41, 1), + PLL_PARAMS(42, 1), + PLL_PARAMS(43, 1), + PLL_PARAMS(44, 1), + PLL_PARAMS(45, 1), + PLL_PARAMS(46, 1), + PLL_PARAMS(47, 1), + PLL_PARAMS(48, 1), + PLL_PARAMS(49, 1), + PLL_PARAMS(50, 1), + PLL_PARAMS(51, 1), + PLL_PARAMS(52, 1), + PLL_PARAMS(53, 1), + PLL_PARAMS(54, 1), + PLL_PARAMS(55, 1), + PLL_PARAMS(56, 1), + PLL_PARAMS(57, 1), + PLL_PARAMS(58, 1), + PLL_PARAMS(59, 1), + PLL_PARAMS(60, 1), + PLL_PARAMS(61, 1), + PLL_PARAMS(62, 1), { /* sentinel */ }, }; -static const struct pll_rate_table gxl_gp0_pll_rate_table[] = { - PLL_RATE(1008000000, 42, 1), - PLL_RATE(1032000000, 43, 1), - PLL_RATE(1056000000, 44, 1), - PLL_RATE(1080000000, 45, 1), - PLL_RATE(1104000000, 46, 1), - PLL_RATE(1128000000, 47, 1), - PLL_RATE(1152000000, 48, 1), - PLL_RATE(1176000000, 49, 1), - PLL_RATE(1200000000, 50, 1), - PLL_RATE(1224000000, 51, 1), - PLL_RATE(1248000000, 52, 1), - PLL_RATE(1272000000, 53, 1), - PLL_RATE(1296000000, 54, 1), - PLL_RATE(1320000000, 55, 1), - PLL_RATE(1344000000, 56, 1), - PLL_RATE(1368000000, 57, 1), - PLL_RATE(1392000000, 58, 1), - PLL_RATE(1416000000, 59, 1), - PLL_RATE(1440000000, 60, 1), - PLL_RATE(1464000000, 61, 1), - PLL_RATE(1488000000, 62, 1), - PLL_RATE(1512000000, 63, 1), - PLL_RATE(1536000000, 64, 1), - PLL_RATE(1560000000, 65, 1), - PLL_RATE(1584000000, 66, 1), +static const struct pll_params_table gxl_gp0_pll_params_table[] = { + PLL_PARAMS(42, 1), + PLL_PARAMS(43, 1), + PLL_PARAMS(44, 1), + PLL_PARAMS(45, 1), + PLL_PARAMS(46, 1), + PLL_PARAMS(47, 1), + PLL_PARAMS(48, 1), + PLL_PARAMS(49, 1), + PLL_PARAMS(50, 1), + PLL_PARAMS(51, 1), + PLL_PARAMS(52, 1), + PLL_PARAMS(53, 1), + PLL_PARAMS(54, 1), + PLL_PARAMS(55, 1), + PLL_PARAMS(56, 1), + PLL_PARAMS(57, 1), + PLL_PARAMS(58, 1), + PLL_PARAMS(59, 1), + PLL_PARAMS(60, 1), + PLL_PARAMS(61, 1), + PLL_PARAMS(62, 1), + PLL_PARAMS(63, 1), + PLL_PARAMS(64, 1), + PLL_PARAMS(65, 1), + PLL_PARAMS(66, 1), { /* sentinel */ }, }; @@ -374,7 +374,7 @@ static struct clk_regmap gxbb_gp0_pll_dco = { .shift = 29, .width = 1, }, - .table = gxbb_gp0_pll_rate_table, + .table = gxbb_gp0_pll_params_table, .init_regs = gxbb_gp0_init_regs, .init_count = ARRAY_SIZE(gxbb_gp0_init_regs), }, @@ -427,7 +427,7 @@ static struct clk_regmap gxl_gp0_pll_dco = { .shift = 29, .width = 1, }, - .table = gxl_gp0_pll_rate_table, + .table = gxl_gp0_pll_params_table, .init_regs = gxl_gp0_init_regs, .init_count = ARRAY_SIZE(gxl_gp0_init_regs), }, diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c index 17bcf0be56ba..30ae849ff53f 100644 --- a/drivers/clk/meson/meson8b.c +++ b/drivers/clk/meson/meson8b.c @@ -29,22 +29,22 @@ struct meson8b_clk_reset { void __iomem *base; }; -static const struct pll_rate_table sys_pll_rate_table[] = { - PLL_RATE(1200000000, 50, 1), - PLL_RATE(1224000000, 51, 1), - PLL_RATE(1248000000, 52, 1), - PLL_RATE(1272000000, 53, 1), - PLL_RATE(1296000000, 54, 1), - PLL_RATE(1320000000, 55, 1), - PLL_RATE(1344000000, 56, 1), - PLL_RATE(1368000000, 57, 1), - PLL_RATE(1392000000, 58, 1), - PLL_RATE(1416000000, 59, 1), - PLL_RATE(1440000000, 60, 1), - PLL_RATE(1464000000, 61, 1), - PLL_RATE(1488000000, 62, 1), - PLL_RATE(1512000000, 63, 1), - PLL_RATE(1536000000, 64, 1), +static const struct pll_params_table sys_pll_params_table[] = { + PLL_PARAMS(50, 1), + PLL_PARAMS(51, 1), + PLL_PARAMS(52, 1), + PLL_PARAMS(53, 1), + PLL_PARAMS(54, 1), + PLL_PARAMS(55, 1), + PLL_PARAMS(56, 1), + PLL_PARAMS(57, 1), + PLL_PARAMS(58, 1), + PLL_PARAMS(59, 1), + PLL_PARAMS(60, 1), + PLL_PARAMS(61, 1), + PLL_PARAMS(62, 1), + PLL_PARAMS(63, 1), + PLL_PARAMS(64, 1), { /* sentinel */ }, }; @@ -195,7 +195,7 @@ static struct clk_regmap meson8b_sys_pll_dco = { .shift = 29, .width = 1, }, - .table = sys_pll_rate_table, + .table = sys_pll_params_table, }, .hw.init = &(struct clk_init_data){ .name = "sys_pll_dco",
Putting hard-coded rates inside the parameter tables assumes that the parent is known and will never change. That's a big assumption we should not make. We have everything we need to recalculate the output rate using the parent rate and the rest of the parameters. Let's do so and drop the rates from the tables. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/clk/meson/axg.c | 73 +++++++++++++-------------- drivers/clk/meson/clk-pll.c | 69 ++++++++++++++++--------- drivers/clk/meson/clkc.h | 8 ++- drivers/clk/meson/gxbb.c | 120 ++++++++++++++++++++++---------------------- drivers/clk/meson/meson8b.c | 34 ++++++------- 5 files changed, 162 insertions(+), 142 deletions(-) -- 2.14.4