mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
* [PATCH 0/9] clk: meson: pll fixes
@ 2018-01-18 18:45 Jerome Brunet
  2018-01-18 18:45 ` [PATCH 1/9] clk: meson: check pll rate param table before using it Jerome Brunet
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

This changeset is collection of fixes and clean-up around the pll clock
provider. This has been triggered by the discussion around the ethernet
clock on the axg [0].

On the axg the rate reported by the fixed_pll was off by 8Mhz,
which led the internal mux of the ethernet driver to pick an mpll2
instead of the fdiv4.

With this series applied, the fixed_pll of the axg now reports
1999998046 Hz, which is coherent with measurements (~2GHz)

While debugging this, we uncovered quite a mess around the hdmi_pll
of the gxbb and gxl family. This is also fixed by this series.

Last, the parameters table provided to the read-only sys_plls have
been removed, saving a bit of memory

There is still work to be done on this clock provider. Someday,
I hope to see the parameter tables go away completely. This pll
is just a (quite complex) fractional divider, we sould be able to
figure something out at runtime.

Jerome Brunet (9):
  clk: meson: check pll rate param table before using it
  clk: meson: remove useless pll rate param tables
  clk: meson: remove unnecessary rounding in the pll clock
  clk: meson: use frac parameter width instead of a constant
  clk: meson: add od3 to the pll driver
  clk: meson: add gxl hdmi pll
  clk: meson: fix pll with fractional part calculation
  clk: meson: gxbb: add the fractional part of the fixed_pll
  clk: meson: axg: add the fractional part of the fixed_pll

 drivers/clk/meson/axg.c     |  99 ++------------------------
 drivers/clk/meson/clk-pll.c |  40 ++++++++---
 drivers/clk/meson/clkc.h    |   2 +
 drivers/clk/meson/gxbb.c    | 166 +++++++++++++++++++-------------------------
 drivers/clk/meson/gxbb.h    |   3 +-
 5 files changed, 110 insertions(+), 200 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/9] clk: meson: check pll rate param table before using it
  2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
@ 2018-01-18 18:45 ` Jerome Brunet
  2018-01-18 18:45 ` [PATCH 2/9] clk: meson: remove useless pll rate param tables Jerome Brunet
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

Make sure the rate param table is available before using it.
Some read-only pll don't provide it, which is ok because the
table is not used by read-only clock. R/W clock are supposed
to provide it, but it does not hurt check it.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/clk-pll.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 01341553f50b..2614341fc4ad 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -94,6 +94,13 @@ static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
 	const struct pll_rate_table *rate_table = pll->rate_table;
 	int i;
 
+	/*
+	 * if the table is missing, just return the current rate
+	 * since we don't have the other available frequencies
+	 */
+	if (!rate_table)
+		return meson_clk_pll_recalc_rate(hw, *parent_rate);
+
 	for (i = 0; i < pll->rate_count; i++) {
 		if (rate <= rate_table[i].rate)
 			return rate_table[i].rate;
@@ -109,6 +116,9 @@ static const struct pll_rate_table *meson_clk_get_pll_settings(struct meson_clk_
 	const struct pll_rate_table *rate_table = pll->rate_table;
 	int i;
 
+	if (!rate_table)
+		return NULL;
+
 	for (i = 0; i < pll->rate_count; i++) {
 		if (rate == rate_table[i].rate)
 			return &rate_table[i];
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/9] clk: meson: remove useless pll rate param tables
  2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
  2018-01-18 18:45 ` [PATCH 1/9] clk: meson: check pll rate param table before using it Jerome Brunet
@ 2018-01-18 18:45 ` Jerome Brunet
  2018-01-18 18:45 ` [PATCH 3/9] clk: meson: remove unnecessary rounding in the pll clock Jerome Brunet
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

Read-only pll don't need param table to recalculate the rate. Providing
them with a param table is just a waste of memory.

Remove the useless tables from sys_pll on gxbb and axg.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/axg.c  | 94 ------------------------------------------------
 drivers/clk/meson/gxbb.c | 94 ------------------------------------------------
 2 files changed, 188 deletions(-)

diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index 02f4401d7bc3..56806c6bd9d5 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -21,98 +21,6 @@
 
 static DEFINE_SPINLOCK(meson_clk_lock);
 
-static const struct pll_rate_table sys_pll_rate_table[] = {
-	PLL_RATE(24000000, 56, 1, 2),
-	PLL_RATE(48000000, 64, 1, 2),
-	PLL_RATE(72000000, 72, 1, 2),
-	PLL_RATE(96000000, 64, 1, 2),
-	PLL_RATE(120000000, 80, 1, 2),
-	PLL_RATE(144000000, 96, 1, 2),
-	PLL_RATE(168000000, 56, 1, 1),
-	PLL_RATE(192000000, 64, 1, 1),
-	PLL_RATE(216000000, 72, 1, 1),
-	PLL_RATE(240000000, 80, 1, 1),
-	PLL_RATE(264000000, 88, 1, 1),
-	PLL_RATE(288000000, 96, 1, 1),
-	PLL_RATE(312000000, 52, 1, 2),
-	PLL_RATE(336000000, 56, 1, 2),
-	PLL_RATE(360000000, 60, 1, 2),
-	PLL_RATE(384000000, 64, 1, 2),
-	PLL_RATE(408000000, 68, 1, 2),
-	PLL_RATE(432000000, 72, 1, 2),
-	PLL_RATE(456000000, 76, 1, 2),
-	PLL_RATE(480000000, 80, 1, 2),
-	PLL_RATE(504000000, 84, 1, 2),
-	PLL_RATE(528000000, 88, 1, 2),
-	PLL_RATE(552000000, 92, 1, 2),
-	PLL_RATE(576000000, 96, 1, 2),
-	PLL_RATE(600000000, 50, 1, 1),
-	PLL_RATE(624000000, 52, 1, 1),
-	PLL_RATE(648000000, 54, 1, 1),
-	PLL_RATE(672000000, 56, 1, 1),
-	PLL_RATE(696000000, 58, 1, 1),
-	PLL_RATE(720000000, 60, 1, 1),
-	PLL_RATE(744000000, 62, 1, 1),
-	PLL_RATE(768000000, 64, 1, 1),
-	PLL_RATE(792000000, 66, 1, 1),
-	PLL_RATE(816000000, 68, 1, 1),
-	PLL_RATE(840000000, 70, 1, 1),
-	PLL_RATE(864000000, 72, 1, 1),
-	PLL_RATE(888000000, 74, 1, 1),
-	PLL_RATE(912000000, 76, 1, 1),
-	PLL_RATE(936000000, 78, 1, 1),
-	PLL_RATE(960000000, 80, 1, 1),
-	PLL_RATE(984000000, 82, 1, 1),
-	PLL_RATE(1008000000, 84, 1, 1),
-	PLL_RATE(1032000000, 86, 1, 1),
-	PLL_RATE(1056000000, 88, 1, 1),
-	PLL_RATE(1080000000, 90, 1, 1),
-	PLL_RATE(1104000000, 92, 1, 1),
-	PLL_RATE(1128000000, 94, 1, 1),
-	PLL_RATE(1152000000, 96, 1, 1),
-	PLL_RATE(1176000000, 98, 1, 1),
-	PLL_RATE(1200000000, 50, 1, 0),
-	PLL_RATE(1224000000, 51, 1, 0),
-	PLL_RATE(1248000000, 52, 1, 0),
-	PLL_RATE(1272000000, 53, 1, 0),
-	PLL_RATE(1296000000, 54, 1, 0),
-	PLL_RATE(1320000000, 55, 1, 0),
-	PLL_RATE(1344000000, 56, 1, 0),
-	PLL_RATE(1368000000, 57, 1, 0),
-	PLL_RATE(1392000000, 58, 1, 0),
-	PLL_RATE(1416000000, 59, 1, 0),
-	PLL_RATE(1440000000, 60, 1, 0),
-	PLL_RATE(1464000000, 61, 1, 0),
-	PLL_RATE(1488000000, 62, 1, 0),
-	PLL_RATE(1512000000, 63, 1, 0),
-	PLL_RATE(1536000000, 64, 1, 0),
-	PLL_RATE(1560000000, 65, 1, 0),
-	PLL_RATE(1584000000, 66, 1, 0),
-	PLL_RATE(1608000000, 67, 1, 0),
-	PLL_RATE(1632000000, 68, 1, 0),
-	PLL_RATE(1656000000, 68, 1, 0),
-	PLL_RATE(1680000000, 68, 1, 0),
-	PLL_RATE(1704000000, 68, 1, 0),
-	PLL_RATE(1728000000, 69, 1, 0),
-	PLL_RATE(1752000000, 69, 1, 0),
-	PLL_RATE(1776000000, 69, 1, 0),
-	PLL_RATE(1800000000, 69, 1, 0),
-	PLL_RATE(1824000000, 70, 1, 0),
-	PLL_RATE(1848000000, 70, 1, 0),
-	PLL_RATE(1872000000, 70, 1, 0),
-	PLL_RATE(1896000000, 70, 1, 0),
-	PLL_RATE(1920000000, 71, 1, 0),
-	PLL_RATE(1944000000, 71, 1, 0),
-	PLL_RATE(1968000000, 71, 1, 0),
-	PLL_RATE(1992000000, 71, 1, 0),
-	PLL_RATE(2016000000, 72, 1, 0),
-	PLL_RATE(2040000000, 72, 1, 0),
-	PLL_RATE(2064000000, 72, 1, 0),
-	PLL_RATE(2088000000, 72, 1, 0),
-	PLL_RATE(2112000000, 73, 1, 0),
-	{ /* sentinel */ },
-};
-
 static struct meson_clk_pll axg_fixed_pll = {
 	.m = {
 		.reg_off = HHI_MPLL_CNTL,
@@ -154,8 +62,6 @@ static struct meson_clk_pll axg_sys_pll = {
 		.shift   = 10,
 		.width   = 2,
 	},
-	.rate_table = sys_pll_rate_table,
-	.rate_count = ARRAY_SIZE(sys_pll_rate_table),
 	.lock = &meson_clk_lock,
 	.hw.init = &(struct clk_init_data){
 		.name = "sys_pll",
diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index 472a3cfbfbc5..2d851bad13fa 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -29,98 +29,6 @@
 
 static DEFINE_SPINLOCK(meson_clk_lock);
 
-static const struct pll_rate_table sys_pll_rate_table[] = {
-	PLL_RATE(24000000, 56, 1, 2),
-	PLL_RATE(48000000, 64, 1, 2),
-	PLL_RATE(72000000, 72, 1, 2),
-	PLL_RATE(96000000, 64, 1, 2),
-	PLL_RATE(120000000, 80, 1, 2),
-	PLL_RATE(144000000, 96, 1, 2),
-	PLL_RATE(168000000, 56, 1, 1),
-	PLL_RATE(192000000, 64, 1, 1),
-	PLL_RATE(216000000, 72, 1, 1),
-	PLL_RATE(240000000, 80, 1, 1),
-	PLL_RATE(264000000, 88, 1, 1),
-	PLL_RATE(288000000, 96, 1, 1),
-	PLL_RATE(312000000, 52, 1, 2),
-	PLL_RATE(336000000, 56, 1, 2),
-	PLL_RATE(360000000, 60, 1, 2),
-	PLL_RATE(384000000, 64, 1, 2),
-	PLL_RATE(408000000, 68, 1, 2),
-	PLL_RATE(432000000, 72, 1, 2),
-	PLL_RATE(456000000, 76, 1, 2),
-	PLL_RATE(480000000, 80, 1, 2),
-	PLL_RATE(504000000, 84, 1, 2),
-	PLL_RATE(528000000, 88, 1, 2),
-	PLL_RATE(552000000, 92, 1, 2),
-	PLL_RATE(576000000, 96, 1, 2),
-	PLL_RATE(600000000, 50, 1, 1),
-	PLL_RATE(624000000, 52, 1, 1),
-	PLL_RATE(648000000, 54, 1, 1),
-	PLL_RATE(672000000, 56, 1, 1),
-	PLL_RATE(696000000, 58, 1, 1),
-	PLL_RATE(720000000, 60, 1, 1),
-	PLL_RATE(744000000, 62, 1, 1),
-	PLL_RATE(768000000, 64, 1, 1),
-	PLL_RATE(792000000, 66, 1, 1),
-	PLL_RATE(816000000, 68, 1, 1),
-	PLL_RATE(840000000, 70, 1, 1),
-	PLL_RATE(864000000, 72, 1, 1),
-	PLL_RATE(888000000, 74, 1, 1),
-	PLL_RATE(912000000, 76, 1, 1),
-	PLL_RATE(936000000, 78, 1, 1),
-	PLL_RATE(960000000, 80, 1, 1),
-	PLL_RATE(984000000, 82, 1, 1),
-	PLL_RATE(1008000000, 84, 1, 1),
-	PLL_RATE(1032000000, 86, 1, 1),
-	PLL_RATE(1056000000, 88, 1, 1),
-	PLL_RATE(1080000000, 90, 1, 1),
-	PLL_RATE(1104000000, 92, 1, 1),
-	PLL_RATE(1128000000, 94, 1, 1),
-	PLL_RATE(1152000000, 96, 1, 1),
-	PLL_RATE(1176000000, 98, 1, 1),
-	PLL_RATE(1200000000, 50, 1, 0),
-	PLL_RATE(1224000000, 51, 1, 0),
-	PLL_RATE(1248000000, 52, 1, 0),
-	PLL_RATE(1272000000, 53, 1, 0),
-	PLL_RATE(1296000000, 54, 1, 0),
-	PLL_RATE(1320000000, 55, 1, 0),
-	PLL_RATE(1344000000, 56, 1, 0),
-	PLL_RATE(1368000000, 57, 1, 0),
-	PLL_RATE(1392000000, 58, 1, 0),
-	PLL_RATE(1416000000, 59, 1, 0),
-	PLL_RATE(1440000000, 60, 1, 0),
-	PLL_RATE(1464000000, 61, 1, 0),
-	PLL_RATE(1488000000, 62, 1, 0),
-	PLL_RATE(1512000000, 63, 1, 0),
-	PLL_RATE(1536000000, 64, 1, 0),
-	PLL_RATE(1560000000, 65, 1, 0),
-	PLL_RATE(1584000000, 66, 1, 0),
-	PLL_RATE(1608000000, 67, 1, 0),
-	PLL_RATE(1632000000, 68, 1, 0),
-	PLL_RATE(1656000000, 68, 1, 0),
-	PLL_RATE(1680000000, 68, 1, 0),
-	PLL_RATE(1704000000, 68, 1, 0),
-	PLL_RATE(1728000000, 69, 1, 0),
-	PLL_RATE(1752000000, 69, 1, 0),
-	PLL_RATE(1776000000, 69, 1, 0),
-	PLL_RATE(1800000000, 69, 1, 0),
-	PLL_RATE(1824000000, 70, 1, 0),
-	PLL_RATE(1848000000, 70, 1, 0),
-	PLL_RATE(1872000000, 70, 1, 0),
-	PLL_RATE(1896000000, 70, 1, 0),
-	PLL_RATE(1920000000, 71, 1, 0),
-	PLL_RATE(1944000000, 71, 1, 0),
-	PLL_RATE(1968000000, 71, 1, 0),
-	PLL_RATE(1992000000, 71, 1, 0),
-	PLL_RATE(2016000000, 72, 1, 0),
-	PLL_RATE(2040000000, 72, 1, 0),
-	PLL_RATE(2064000000, 72, 1, 0),
-	PLL_RATE(2088000000, 72, 1, 0),
-	PLL_RATE(2112000000, 73, 1, 0),
-	{ /* sentinel */ },
-};
-
 static const struct pll_rate_table gxbb_gp0_pll_rate_table[] = {
 	PLL_RATE(96000000, 32, 1, 3),
 	PLL_RATE(99000000, 33, 1, 3),
@@ -356,8 +264,6 @@ static struct meson_clk_pll gxbb_sys_pll = {
 		.shift   = 10,
 		.width   = 2,
 	},
-	.rate_table = sys_pll_rate_table,
-	.rate_count = ARRAY_SIZE(sys_pll_rate_table),
 	.lock = &meson_clk_lock,
 	.hw.init = &(struct clk_init_data){
 		.name = "sys_pll",
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/9] clk: meson: remove unnecessary rounding in the pll clock
  2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
  2018-01-18 18:45 ` [PATCH 1/9] clk: meson: check pll rate param table before using it Jerome Brunet
  2018-01-18 18:45 ` [PATCH 2/9] clk: meson: remove useless pll rate param tables Jerome Brunet
@ 2018-01-18 18:45 ` Jerome Brunet
  2018-01-19  2:06   ` Yixun Lan
  2018-01-18 18:45 ` [PATCH 4/9] clk: meson: use frac parameter width instead of a constant Jerome Brunet
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

The pll driver perform the rate calculation in Mhz, which adds an
unnecessary rounding down to the Mhz of the rate. Use 64bits long
integer to perform this calculation safely on meson8b and perform the
calculation in Hz instead

Fixes: 7a29a869434e ("clk: meson: Add support for Meson clock controller")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/clk-pll.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 2614341fc4ad..fa4cec13d6e8 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -51,8 +51,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
 {
 	struct meson_clk_pll *pll = to_meson_clk_pll(hw);
 	struct parm *p;
-	unsigned long parent_rate_mhz = parent_rate / 1000000;
-	unsigned long rate_mhz;
+	u64 rate;
 	u16 n, m, frac = 0, od, od2 = 0;
 	u32 reg;
 
@@ -74,17 +73,18 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
 		od2 = PARM_GET(p->width, p->shift, reg);
 	}
 
+	rate = (u64)m * parent_rate;
+
 	p = &pll->frac;
 	if (p->width) {
 		reg = readl(pll->base + p->reg_off);
 		frac = PARM_GET(p->width, p->shift, reg);
-		rate_mhz = (parent_rate_mhz * m + \
-				(parent_rate_mhz * frac >> 12)) * 2 / n;
-		rate_mhz = rate_mhz >> od >> od2;
-	} else
-		rate_mhz = (parent_rate_mhz * m / n) >> od >> od2;
 
-	return rate_mhz * 1000000;
+		rate += (u64)parent_rate * frac >> 12;
+		rate *= 2;
+	}
+
+	return (rate / n) >> od >> od2;
 }
 
 static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/9] clk: meson: use frac parameter width instead of a constant
  2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
                   ` (2 preceding siblings ...)
  2018-01-18 18:45 ` [PATCH 3/9] clk: meson: remove unnecessary rounding in the pll clock Jerome Brunet
@ 2018-01-18 18:45 ` Jerome Brunet
  2018-01-18 18:45 ` [PATCH 5/9] clk: meson: add od3 to the pll driver Jerome Brunet
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

Use the fractional part width in the calculation instead of 12, which
happens to e the witdh right now. This is safer in case the field width
ever change in the future

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/clk-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index fa4cec13d6e8..d9efe725b7ad 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -80,7 +80,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
 		reg = readl(pll->base + p->reg_off);
 		frac = PARM_GET(p->width, p->shift, reg);
 
-		rate += (u64)parent_rate * frac >> 12;
+		rate += (u64)parent_rate * frac >>  p->width;
 		rate *= 2;
 	}
 
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 5/9] clk: meson: add od3 to the pll driver
  2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
                   ` (3 preceding siblings ...)
  2018-01-18 18:45 ` [PATCH 4/9] clk: meson: use frac parameter width instead of a constant Jerome Brunet
@ 2018-01-18 18:45 ` Jerome Brunet
  2018-01-18 18:45 ` [PATCH 6/9] clk: meson: add gxl hdmi pll Jerome Brunet
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

Some meson plls, such as the hdmi pll, are using a 3rd od parameter,
which is yet another "power of 2" post divider. Add it to fix the
calculation of the hdmi_pll rate

Fixes: 738f66d3211d ("clk: gxbb: add AmLogic GXBB clk controller driver")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/clk-pll.c | 17 +++++++++++++++--
 drivers/clk/meson/clkc.h    |  2 ++
 drivers/clk/meson/gxbb.c    |  5 +++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index d9efe725b7ad..3f560b2cd83b 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -52,7 +52,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
 	struct meson_clk_pll *pll = to_meson_clk_pll(hw);
 	struct parm *p;
 	u64 rate;
-	u16 n, m, frac = 0, od, od2 = 0;
+	u16 n, m, frac = 0, od, od2 = 0, od3 = 0;
 	u32 reg;
 
 	p = &pll->n;
@@ -73,6 +73,12 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
 		od2 = PARM_GET(p->width, p->shift, reg);
 	}
 
+	p = &pll->od3;
+	if (p->width) {
+		reg = readl(pll->base + p->reg_off);
+		od3 = PARM_GET(p->width, p->shift, reg);
+	}
+
 	rate = (u64)m * parent_rate;
 
 	p = &pll->frac;
@@ -84,7 +90,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
 		rate *= 2;
 	}
 
-	return (rate / n) >> od >> od2;
+	return (rate / n) >> od >> od2 >> od3;
 }
 
 static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
@@ -225,6 +231,13 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 		writel(reg, pll->base + p->reg_off);
 	}
 
+	p = &pll->od3;
+	if (p->width) {
+		reg = readl(pll->base + p->reg_off);
+		reg = PARM_SET(p->width, p->shift, reg, rate_set->od3);
+		writel(reg, pll->base + p->reg_off);
+	}
+
 	p = &pll->frac;
 	if (p->width) {
 		reg = readl(pll->base + p->reg_off);
diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
index c2ff0520ce53..4acb35bda669 100644
--- a/drivers/clk/meson/clkc.h
+++ b/drivers/clk/meson/clkc.h
@@ -41,6 +41,7 @@ struct pll_rate_table {
 	u16		n;
 	u16		od;
 	u16		od2;
+	u16		od3;
 	u16		frac;
 };
 
@@ -92,6 +93,7 @@ struct meson_clk_pll {
 	struct parm frac;
 	struct parm od;
 	struct parm od2;
+	struct parm od3;
 	const struct pll_setup_params params;
 	const struct pll_rate_table *rate_table;
 	unsigned int rate_count;
diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index 2d851bad13fa..cf083a1906d1 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -238,6 +238,11 @@ static struct meson_clk_pll gxbb_hdmi_pll = {
 		.shift   = 22,
 		.width   = 2,
 	},
+	.od3 = {
+		.reg_off = HHI_HDMI_PLL_CNTL2,
+		.shift   = 18,
+		.width   = 2,
+	},
 	.lock = &meson_clk_lock,
 	.hw.init = &(struct clk_init_data){
 		.name = "hdmi_pll",
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 6/9] clk: meson: add gxl hdmi pll
  2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
                   ` (4 preceding siblings ...)
  2018-01-18 18:45 ` [PATCH 5/9] clk: meson: add od3 to the pll driver Jerome Brunet
@ 2018-01-18 18:45 ` Jerome Brunet
  2018-01-18 18:45 ` [PATCH 7/9] clk: meson: fix pll with fractional part calculation Jerome Brunet
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

The hdmi pll used in the gxl family is actually different from the gxbb.
The register layout is completely different, which explain why the hdmi
pll rate has always been rubbish on the gxl.

Adding the correct register field is the first part of the fix to get a
correct rate out the hdmi pll

Fixes: 0d48fc558d01 ("clk: meson-gxbb: Add GXL/GXM GP0 Variant")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/gxbb.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index cf083a1906d1..08b3e1cdba5b 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -253,6 +253,52 @@ static struct meson_clk_pll gxbb_hdmi_pll = {
 	},
 };
 
+static struct meson_clk_pll gxl_hdmi_pll = {
+	.m = {
+		.reg_off = HHI_HDMI_PLL_CNTL,
+		.shift   = 0,
+		.width   = 9,
+	},
+	.n = {
+		.reg_off = HHI_HDMI_PLL_CNTL,
+		.shift   = 9,
+		.width   = 5,
+	},
+	.frac = {
+		/*
+		 * On gxl, there a register shift due to HHI_HDMI_PLL_CNTL1
+		 * which does not exist on gxbb, so we compute the register
+		 * offset based on the PLL base to get it right
+		 */
+		.reg_off = HHI_HDMI_PLL_CNTL + 4,
+		.shift   = 0,
+		.width   = 12,
+	},
+	.od = {
+		.reg_off = HHI_HDMI_PLL_CNTL + 8,
+		.shift   = 21,
+		.width   = 2,
+	},
+	.od2 = {
+		.reg_off = HHI_HDMI_PLL_CNTL + 8,
+		.shift   = 23,
+		.width   = 2,
+	},
+	.od3 = {
+		.reg_off = HHI_HDMI_PLL_CNTL + 8,
+		.shift   = 19,
+		.width   = 2,
+	},
+	.lock = &meson_clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "hdmi_pll",
+		.ops = &meson_clk_pll_ro_ops,
+		.parent_names = (const char *[]){ "xtal" },
+		.num_parents = 1,
+		.flags = CLK_GET_RATE_NOCACHE,
+	},
+};
+
 static struct meson_clk_pll gxbb_sys_pll = {
 	.m = {
 		.reg_off = HHI_SYS_PLL_CNTL,
@@ -1520,7 +1566,7 @@ static struct clk_hw_onecell_data gxbb_hw_onecell_data = {
 static struct clk_hw_onecell_data gxl_hw_onecell_data = {
 	.hws = {
 		[CLKID_SYS_PLL]		    = &gxbb_sys_pll.hw,
-		[CLKID_HDMI_PLL]	    = &gxbb_hdmi_pll.hw,
+		[CLKID_HDMI_PLL]	    = &gxl_hdmi_pll.hw,
 		[CLKID_FIXED_PLL]	    = &gxbb_fixed_pll.hw,
 		[CLKID_FCLK_DIV2]	    = &gxbb_fclk_div2.hw,
 		[CLKID_FCLK_DIV3]	    = &gxbb_fclk_div3.hw,
@@ -1675,7 +1721,7 @@ static struct meson_clk_pll *const gxbb_clk_plls[] = {
 
 static struct meson_clk_pll *const gxl_clk_plls[] = {
 	&gxbb_fixed_pll,
-	&gxbb_hdmi_pll,
+	&gxl_hdmi_pll,
 	&gxbb_sys_pll,
 	&gxl_gp0_pll,
 };
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 7/9] clk: meson: fix pll with fractional part calculation
  2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
                   ` (5 preceding siblings ...)
  2018-01-18 18:45 ` [PATCH 6/9] clk: meson: add gxl hdmi pll Jerome Brunet
@ 2018-01-18 18:45 ` Jerome Brunet
  2018-01-18 18:45 ` [PATCH 8/9] clk: meson: gxbb: add the fractional part of the fixed_pll Jerome Brunet
  2018-01-18 18:45 ` [PATCH 9/9] clk: meson: axg: " Jerome Brunet
  8 siblings, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

The rate of the parent should not be multiplied by 2 when there is a
fractional part to the pll. This is making the rate calculation of
gxl_hdmi_pll wrong (and others as well). This multiplication is specific
to the hdmi_pll of gxbb and is most likely due to a multiplier sitting
in front of this particular pll.

Add fixed factor clock in front on the gxbb pll and remove this constant
from the calculation

Fixes: 4a47295144dd ("clk: meson: fractional pll support")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/clk-pll.c |  1 -
 drivers/clk/meson/gxbb.c    | 14 +++++++++++++-
 drivers/clk/meson/gxbb.h    |  3 ++-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 3f560b2cd83b..8845f65aeb07 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -87,7 +87,6 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
 		frac = PARM_GET(p->width, p->shift, reg);
 
 		rate += (u64)parent_rate * frac >>  p->width;
-		rate *= 2;
 	}
 
 	return (rate / n) >> od >> od2 >> od3;
diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index 08b3e1cdba5b..9a92e64b428c 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -212,6 +212,17 @@ static struct meson_clk_pll gxbb_fixed_pll = {
 	},
 };
 
+static struct clk_fixed_factor gxbb_hdmi_pll_pre_mult = {
+	.mult = 2,
+	.div = 1,
+	.hw.init = &(struct clk_init_data){
+		.name = "hdmi_pll_pre_mult",
+		.ops = &clk_fixed_factor_ops,
+		.parent_names = (const char *[]){ "xtal" },
+		.num_parents = 1,
+	},
+};
+
 static struct meson_clk_pll gxbb_hdmi_pll = {
 	.m = {
 		.reg_off = HHI_HDMI_PLL_CNTL,
@@ -247,7 +258,7 @@ static struct meson_clk_pll gxbb_hdmi_pll = {
 	.hw.init = &(struct clk_init_data){
 		.name = "hdmi_pll",
 		.ops = &meson_clk_pll_ro_ops,
-		.parent_names = (const char *[]){ "xtal" },
+		.parent_names = (const char *[]){ "hdmi_pll_pre_mult" },
 		.num_parents = 1,
 		.flags = CLK_GET_RATE_NOCACHE,
 	},
@@ -1558,6 +1569,7 @@ static struct clk_hw_onecell_data gxbb_hw_onecell_data = {
 		[CLKID_VAPB_1]		    = &gxbb_vapb_1.hw,
 		[CLKID_VAPB_SEL]	    = &gxbb_vapb_sel.hw,
 		[CLKID_VAPB]		    = &gxbb_vapb.hw,
+		[CLKID_HDMI_PLL_PRE_MULT]   = &gxbb_hdmi_pll_pre_mult.hw,
 		[NR_CLKS]		    = NULL,
 	},
 	.num = NR_CLKS,
diff --git a/drivers/clk/meson/gxbb.h b/drivers/clk/meson/gxbb.h
index aee6fbba2004..42573b28a137 100644
--- a/drivers/clk/meson/gxbb.h
+++ b/drivers/clk/meson/gxbb.h
@@ -194,8 +194,9 @@
 #define CLKID_VPU_1_DIV		  130
 #define CLKID_VAPB_0_DIV	  134
 #define CLKID_VAPB_1_DIV	  137
+#define CLKID_HDMI_PLL_PRE_MULT	  141
 
-#define NR_CLKS			  141
+#define NR_CLKS			  142
 
 /* include the CLKIDs that have been made part of the DT binding */
 #include <dt-bindings/clock/gxbb-clkc.h>
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 8/9] clk: meson: gxbb: add the fractional part of the fixed_pll
  2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
                   ` (6 preceding siblings ...)
  2018-01-18 18:45 ` [PATCH 7/9] clk: meson: fix pll with fractional part calculation Jerome Brunet
@ 2018-01-18 18:45 ` Jerome Brunet
  2018-01-18 18:45 ` [PATCH 9/9] clk: meson: axg: " Jerome Brunet
  8 siblings, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

The fixed_pll of gxbb and gxl also has a fractional parameter. This has
not been a problem so far because fractional part is actually set to 0
on these platforms, so the rate remains correct when it is ignored.
Still, it is better represent the pll the way it is, so add the frac
parameter now

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/gxbb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index 9a92e64b428c..3202f85e69e5 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -202,6 +202,11 @@ static struct meson_clk_pll gxbb_fixed_pll = {
 		.shift   = 16,
 		.width   = 2,
 	},
+	.frac = {
+		.reg_off = HHI_MPLL_CNTL2,
+		.shift   = 0,
+		.width   = 12,
+	},
 	.lock = &meson_clk_lock,
 	.hw.init = &(struct clk_init_data){
 		.name = "fixed_pll",
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 9/9] clk: meson: axg: add the fractional part of the fixed_pll
  2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
                   ` (7 preceding siblings ...)
  2018-01-18 18:45 ` [PATCH 8/9] clk: meson: gxbb: add the fractional part of the fixed_pll Jerome Brunet
@ 2018-01-18 18:45 ` Jerome Brunet
  8 siblings, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2018-01-18 18:45 UTC (permalink / raw)
  To: linus-amlogic

The fixed_pll also has a fractional part. On axg s400 board, without
this parameter, the calculated rate is off by ~8Mhz (0,4%). The fixed_pll
being the root of peripheral clock tree, this error is propagated to the
rest of the clocks

Adding the definition of the parameter fixes the problem

Fixes: 78b4af312f91 ("clk: meson-axg: add clock controller drivers")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/axg.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index 56806c6bd9d5..953e119635a2 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -37,6 +37,11 @@ static struct meson_clk_pll axg_fixed_pll = {
 		.shift   = 16,
 		.width   = 2,
 	},
+	.frac = {
+		.reg_off = HHI_MPLL_CNTL2,
+		.shift   = 0,
+		.width   = 12,
+	},
 	.lock = &meson_clk_lock,
 	.hw.init = &(struct clk_init_data){
 		.name = "fixed_pll",
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/9] clk: meson: remove unnecessary rounding in the pll clock
  2018-01-18 18:45 ` [PATCH 3/9] clk: meson: remove unnecessary rounding in the pll clock Jerome Brunet
@ 2018-01-19  2:06   ` Yixun Lan
  0 siblings, 0 replies; 11+ messages in thread
From: Yixun Lan @ 2018-01-19  2:06 UTC (permalink / raw)
  To: linus-amlogic


On 01/19/18 02:45, Jerome Brunet wrote:
> The pll driver perform the rate calculation in Mhz, which adds an
> unnecessary rounding down to the Mhz of the rate. Use 64bits long
> integer to perform this calculation safely on meson8b and perform the
> calculation in Hz instead
> 
> Fixes: 7a29a869434e ("clk: meson: Add support for Meson clock controller")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/clk/meson/clk-pll.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index 2614341fc4ad..fa4cec13d6e8 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -51,8 +51,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
>  {
>  	struct meson_clk_pll *pll = to_meson_clk_pll(hw);
>  	struct parm *p;
> -	unsigned long parent_rate_mhz = parent_rate / 1000000;
> -	unsigned long rate_mhz;
> +	u64 rate;
>  	u16 n, m, frac = 0, od, od2 = 0;
>  	u32 reg;
>  
> @@ -74,17 +73,18 @@ static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
>  		od2 = PARM_GET(p->width, p->shift, reg);
>  	}
>  
> +	rate = (u64)m * parent_rate;
> +
>  	p = &pll->frac;
>  	if (p->width) {
>  		reg = readl(pll->base + p->reg_off);
>  		frac = PARM_GET(p->width, p->shift, reg);
> -		rate_mhz = (parent_rate_mhz * m + \
> -				(parent_rate_mhz * frac >> 12)) * 2 / n;
> -		rate_mhz = rate_mhz >> od >> od2;
> -	} else
> -		rate_mhz = (parent_rate_mhz * m / n) >> od >> od2;
>  
> -	return rate_mhz * 1000000;
> +		rate += (u64)parent_rate * frac >> 12;
> +		rate *= 2;
> +	}
> +
> +	return (rate / n) >> od >> od2;
>  }
>  
>  static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> 

Hi Jerome:
 This is exactly what I want to propose, thanks for pushing this!

 With the whole series, the fixed_pll is more accurate, and the ethernet
driver on axg is capable of choosing fclk_div2..

Yixun

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-01-19  2:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-18 18:45 [PATCH 0/9] clk: meson: pll fixes Jerome Brunet
2018-01-18 18:45 ` [PATCH 1/9] clk: meson: check pll rate param table before using it Jerome Brunet
2018-01-18 18:45 ` [PATCH 2/9] clk: meson: remove useless pll rate param tables Jerome Brunet
2018-01-18 18:45 ` [PATCH 3/9] clk: meson: remove unnecessary rounding in the pll clock Jerome Brunet
2018-01-19  2:06   ` Yixun Lan
2018-01-18 18:45 ` [PATCH 4/9] clk: meson: use frac parameter width instead of a constant Jerome Brunet
2018-01-18 18:45 ` [PATCH 5/9] clk: meson: add od3 to the pll driver Jerome Brunet
2018-01-18 18:45 ` [PATCH 6/9] clk: meson: add gxl hdmi pll Jerome Brunet
2018-01-18 18:45 ` [PATCH 7/9] clk: meson: fix pll with fractional part calculation Jerome Brunet
2018-01-18 18:45 ` [PATCH 8/9] clk: meson: gxbb: add the fractional part of the fixed_pll Jerome Brunet
2018-01-18 18:45 ` [PATCH 9/9] clk: meson: axg: " Jerome Brunet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®