[PATCH 1/5] clk: ingenic: Add support for divider tables

[PATCH 1/5] clk: ingenic: Add support for divider tables

[Paul Cercueil]

Some clocks provided on Ingenic SoCs have dividers, whose hardware value
as written in the register cannot be expressed as an affine function
to the actual divider value.

For instance, for the CPU clock on the JZ4770, the dividers are coded as
follows:

    ------------------
    | Bits     | Div |
    ------------------
    | 0  0  0  |  1  |
    | 0  0  1  |  2  |
    | 0  1  0  |  3  |
    | 0  1  1  |  4  |
    | 1  0  0  |  6  |
    | 1  0  1  |  8  |
    | 1  1  0  | 12  |
    ------------------

To support this setup, we introduce a new field in the
ingenic_cgu_div_info structure that allows to specify the divider table.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/clk/ingenic/cgu.c | 41 +++++++++++++++++++++++++++++++++------
 drivers/clk/ingenic/cgu.h |  3 +++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
index 510b685212d3..6a8c4fb0f6d5 100644
--- a/drivers/clk/ingenic/cgu.c
+++ b/drivers/clk/ingenic/cgu.c
@@ -383,8 +383,11 @@ ingenic_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 		div_reg = readl(cgu->base + clk_info->div.reg);
 		div = (div_reg >> clk_info->div.shift) &
 		      GENMASK(clk_info->div.bits - 1, 0);
-		div += 1;
-		div *= clk_info->div.div;
+
+		if (clk_info->div.div_table)
+			div = clk_info->div.div_table[div];
+		else
+			div = (div + 1) * clk_info->div.div;
 
 		rate /= div;
 	} else if (clk_info->type & CGU_CLK_FIXDIV) {
@@ -394,16 +397,37 @@ ingenic_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	return rate;
 }
 
+static unsigned int
+ingenic_clk_calc_hw_div(const struct ingenic_cgu_clk_info *clk_info,
+			unsigned int div)
+{
+	unsigned int i;
+
+	for (i = 0; i < (1 << clk_info->div.bits)
+				&& clk_info->div.div_table[i]; i++) {
+		if (clk_info->div.div_table[i] >= div)
+			return i;
+	}
+
+	return i - 1;
+}
+
 static unsigned
 ingenic_clk_calc_div(const struct ingenic_cgu_clk_info *clk_info,
 		     unsigned long parent_rate, unsigned long req_rate)
 {
-	unsigned div;
+	unsigned int div, hw_div;
 
 	/* calculate the divide */
 	div = DIV_ROUND_UP(parent_rate, req_rate);
 
-	/* and impose hardware constraints */
+	if (clk_info->div.div_table) {
+		hw_div = ingenic_clk_calc_hw_div(clk_info, div);
+
+		return clk_info->div.div_table[hw_div];
+	}
+
+	/* Impose hardware constraints */
 	div = min_t(unsigned, div, 1 << clk_info->div.bits);
 	div = max_t(unsigned, div, 1);
 
@@ -446,7 +470,7 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate,
 	const struct ingenic_cgu_clk_info *clk_info;
 	const unsigned timeout = 100;
 	unsigned long rate, flags;
-	unsigned div, i;
+	unsigned int hw_div, div, i;
 	u32 reg, mask;
 	int ret = 0;
 
@@ -459,13 +483,18 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate,
 		if (rate != req_rate)
 			return -EINVAL;
 
+		if (clk_info->div.div_table)
+			hw_div = ingenic_clk_calc_hw_div(clk_info, div);
+		else
+			hw_div = ((div / clk_info->div.div) - 1);
+
 		spin_lock_irqsave(&cgu->lock, flags);
 		reg = readl(cgu->base + clk_info->div.reg);
 
 		/* update the divide */
 		mask = GENMASK(clk_info->div.bits - 1, 0);
 		reg &= ~(mask << clk_info->div.shift);
-		reg |= ((div / clk_info->div.div) - 1) << clk_info->div.shift;
+		reg |= hw_div << clk_info->div.shift;
 
 		/* clear the stop bit */
 		if (clk_info->div.stop_bit != -1)
diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
index e12716d8ce3c..8dcd83aeab84 100644
--- a/drivers/clk/ingenic/cgu.h
+++ b/drivers/clk/ingenic/cgu.h
@@ -88,6 +88,8 @@ struct ingenic_cgu_mux_info {
  *          isn't one
  * @busy_bit: the index of the busy bit within reg, or -1 if there isn't one
  * @stop_bit: the index of the stop bit within reg, or -1 if there isn't one
+ * @div_table: optional table to map the value read from the register to the
+ *             actual divider value
  */
 struct ingenic_cgu_div_info {
 	unsigned reg;
@@ -97,6 +99,7 @@ struct ingenic_cgu_div_info {
 	s8 ce_bit;
 	s8 busy_bit;
 	s8 stop_bit;
+	const u8 *div_table;
 };
 
 /**
-- 
2.21.0.593.g511ec345e18

[PATCH 2/5] clk: ingenic/jz4740: Fix incorrect dividers for main clocks

[Paul Cercueil]

The main clocks (cclk, hclk, pclk, mclk, lcd) were using
incorrect dividers, and thus reported an incorrect rate.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/clk/ingenic/jz4740-cgu.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/ingenic/jz4740-cgu.c b/drivers/clk/ingenic/jz4740-cgu.c
index b86edd328249..ec6c71806687 100644
--- a/drivers/clk/ingenic/jz4740-cgu.c
+++ b/drivers/clk/ingenic/jz4740-cgu.c
@@ -57,6 +57,10 @@ static const s8 pll_od_encoding[4] = {
 	0x0, 0x1, -1, 0x3,
 };
 
+static const u8 jz4740_cgu_cpccr_div_table[] = {
+	1, 2, 3, 4, 6, 8, 12, 16, 24, 32,
+};
+
 static const struct ingenic_cgu_clk_info jz4740_cgu_clocks[] = {
 
 	/* External clocks */
@@ -96,31 +100,46 @@ static const struct ingenic_cgu_clk_info jz4740_cgu_clocks[] = {
 	[JZ4740_CLK_CCLK] = {
 		"cclk", CGU_CLK_DIV,
 		.parents = { JZ4740_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1,
+			jz4740_cgu_cpccr_div_table,
+		},
 	},
 
 	[JZ4740_CLK_HCLK] = {
 		"hclk", CGU_CLK_DIV,
 		.parents = { JZ4740_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 4, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 4, 1, 4, 22, -1, -1,
+			jz4740_cgu_cpccr_div_table,
+		},
 	},
 
 	[JZ4740_CLK_PCLK] = {
 		"pclk", CGU_CLK_DIV,
 		.parents = { JZ4740_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 8, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 8, 1, 4, 22, -1, -1,
+			jz4740_cgu_cpccr_div_table,
+		},
 	},
 
 	[JZ4740_CLK_MCLK] = {
 		"mclk", CGU_CLK_DIV,
 		.parents = { JZ4740_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 12, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 12, 1, 4, 22, -1, -1,
+			jz4740_cgu_cpccr_div_table,
+		},
 	},
 
 	[JZ4740_CLK_LCD] = {
 		"lcd", CGU_CLK_DIV | CGU_CLK_GATE,
 		.parents = { JZ4740_CLK_PLL_HALF, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 16, 1, 5, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 16, 1, 5, 22, -1, -1,
+			jz4740_cgu_cpccr_div_table,
+		},
 		.gate = { CGU_REG_CLKGR, 10 },
 	},
 
-- 
2.21.0.593.g511ec345e18

[PATCH 3/5] clk: ingenic/jz4770: Fix incorrect dividers for main clocks

[Paul Cercueil]

The main clocks (cclk, h0clk, h1clk, h2clk, c1clk, pclk) were using
incorrect dividers, and thus reported an incorrect rate.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/clk/ingenic/jz4770-cgu.c | 34 ++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/ingenic/jz4770-cgu.c b/drivers/clk/ingenic/jz4770-cgu.c
index bf46a0df2004..3479ad30b040 100644
--- a/drivers/clk/ingenic/jz4770-cgu.c
+++ b/drivers/clk/ingenic/jz4770-cgu.c
@@ -86,6 +86,10 @@ static const s8 pll_od_encoding[8] = {
 	0x0, 0x1, -1, 0x2, -1, -1, -1, 0x3,
 };
 
+static const u8 jz4770_cgu_cpccr_div_table[] = {
+	1, 2, 3, 4, 6, 8, 12,
+};
+
 static const struct ingenic_cgu_clk_info jz4770_cgu_clocks[] = {
 
 	/* External clocks */
@@ -143,34 +147,52 @@ static const struct ingenic_cgu_clk_info jz4770_cgu_clocks[] = {
 	[JZ4770_CLK_CCLK] = {
 		"cclk", CGU_CLK_DIV,
 		.parents = { JZ4770_CLK_PLL0, },
-		.div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1,
+			jz4770_cgu_cpccr_div_table,
+		},
 	},
 	[JZ4770_CLK_H0CLK] = {
 		"h0clk", CGU_CLK_DIV,
 		.parents = { JZ4770_CLK_PLL0, },
-		.div = { CGU_REG_CPCCR, 4, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 4, 1, 4, 22, -1, -1,
+			jz4770_cgu_cpccr_div_table,
+		},
 	},
 	[JZ4770_CLK_H1CLK] = {
 		"h1clk", CGU_CLK_DIV | CGU_CLK_GATE,
 		.parents = { JZ4770_CLK_PLL0, },
-		.div = { CGU_REG_CPCCR, 24, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 24, 1, 4, 22, -1, -1,
+			jz4770_cgu_cpccr_div_table,
+		},
 		.gate = { CGU_REG_CLKGR1, 7 },
 	},
 	[JZ4770_CLK_H2CLK] = {
 		"h2clk", CGU_CLK_DIV,
 		.parents = { JZ4770_CLK_PLL0, },
-		.div = { CGU_REG_CPCCR, 16, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 16, 1, 4, 22, -1, -1,
+			jz4770_cgu_cpccr_div_table,
+		},
 	},
 	[JZ4770_CLK_C1CLK] = {
 		"c1clk", CGU_CLK_DIV | CGU_CLK_GATE,
 		.parents = { JZ4770_CLK_PLL0, },
-		.div = { CGU_REG_CPCCR, 12, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 12, 1, 4, 22, -1, -1,
+			jz4770_cgu_cpccr_div_table,
+		},
 		.gate = { CGU_REG_OPCR, 31, true }, // disable CCLK stop on idle
 	},
 	[JZ4770_CLK_PCLK] = {
 		"pclk", CGU_CLK_DIV,
 		.parents = { JZ4770_CLK_PLL0, },
-		.div = { CGU_REG_CPCCR, 8, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 8, 1, 4, 22, -1, -1,
+			jz4770_cgu_cpccr_div_table,
+		},
 	},
 
 	/* Those divided clocks can connect to PLL0 or PLL1 */
-- 
2.21.0.593.g511ec345e18

[PATCH 4/5] clk: ingenic/jz4725b: Fix incorrect dividers for main clocks

[Paul Cercueil]

The main clocks (cclk, hclk, pclk, mclk, ipu) were using
incorrect dividers, and thus reported an incorrect rate.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/clk/ingenic/jz4725b-cgu.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/ingenic/jz4725b-cgu.c b/drivers/clk/ingenic/jz4725b-cgu.c
index 8901ea0295b7..31325dd40a0f 100644
--- a/drivers/clk/ingenic/jz4725b-cgu.c
+++ b/drivers/clk/ingenic/jz4725b-cgu.c
@@ -33,6 +33,10 @@ static const s8 pll_od_encoding[4] = {
 	0x0, 0x1, -1, 0x3,
 };
 
+static const u8 jz4725b_cgu_cpccr_div_table[] = {
+	1, 2, 3, 4, 6, 8,
+};
+
 static const struct ingenic_cgu_clk_info jz4725b_cgu_clocks[] = {
 
 	/* External clocks */
@@ -72,31 +76,46 @@ static const struct ingenic_cgu_clk_info jz4725b_cgu_clocks[] = {
 	[JZ4725B_CLK_CCLK] = {
 		"cclk", CGU_CLK_DIV,
 		.parents = { JZ4725B_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1,
+			jz4725b_cgu_cpccr_div_table,
+		},
 	},
 
 	[JZ4725B_CLK_HCLK] = {
 		"hclk", CGU_CLK_DIV,
 		.parents = { JZ4725B_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 4, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 4, 1, 4, 22, -1, -1,
+			jz4725b_cgu_cpccr_div_table,
+		},
 	},
 
 	[JZ4725B_CLK_PCLK] = {
 		"pclk", CGU_CLK_DIV,
 		.parents = { JZ4725B_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 8, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 8, 1, 4, 22, -1, -1,
+			jz4725b_cgu_cpccr_div_table,
+		},
 	},
 
 	[JZ4725B_CLK_MCLK] = {
 		"mclk", CGU_CLK_DIV,
 		.parents = { JZ4725B_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 12, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 12, 1, 4, 22, -1, -1,
+			jz4725b_cgu_cpccr_div_table,
+		},
 	},
 
 	[JZ4725B_CLK_IPU] = {
 		"ipu", CGU_CLK_DIV | CGU_CLK_GATE,
 		.parents = { JZ4725B_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 16, 1, 4, 22, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 16, 1, 4, 22, -1, -1,
+			jz4725b_cgu_cpccr_div_table,
+		},
 		.gate = { CGU_REG_CLKGR, 13 },
 	},
 
-- 
2.21.0.593.g511ec345e18

[PATCH 5/5] clk: ingenic/jz4725b: Fix "pll half" divider not read/written properly

[Paul Cercueil]

The code was setting the bit 21 of the CPCCR register to use a divider
of 2 for the "pll half" clock, and clearing the bit to use a divider
of 1.

This is the opposite of how this register field works: a cleared bit
means that the /2 divider is used, and a set bit means that the divider
is 1.

Restore the correct behaviour using the newly introduced .div_table
field.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/clk/ingenic/jz4725b-cgu.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/ingenic/jz4725b-cgu.c b/drivers/clk/ingenic/jz4725b-cgu.c
index 31325dd40a0f..47287956824b 100644
--- a/drivers/clk/ingenic/jz4725b-cgu.c
+++ b/drivers/clk/ingenic/jz4725b-cgu.c
@@ -37,6 +37,10 @@ static const u8 jz4725b_cgu_cpccr_div_table[] = {
 	1, 2, 3, 4, 6, 8,
 };
 
+static const u8 jz4725b_cgu_pll_half_div_table[] = {
+	2, 1,
+};
+
 static const struct ingenic_cgu_clk_info jz4725b_cgu_clocks[] = {
 
 	/* External clocks */
@@ -70,7 +74,10 @@ static const struct ingenic_cgu_clk_info jz4725b_cgu_clocks[] = {
 	[JZ4725B_CLK_PLL_HALF] = {
 		"pll half", CGU_CLK_DIV,
 		.parents = { JZ4725B_CLK_PLL, -1, -1, -1 },
-		.div = { CGU_REG_CPCCR, 21, 1, 1, -1, -1, -1 },
+		.div = {
+			CGU_REG_CPCCR, 21, 1, 1, -1, -1, -1,
+			jz4725b_cgu_pll_half_div_table,
+		},
 	},
 
 	[JZ4725B_CLK_CCLK] = {
-- 
2.21.0.593.g511ec345e18

Re: [PATCH 2/5] clk: ingenic/jz4740: Fix incorrect dividers for main clocks

[Stephen Boyd]

Quoting Paul Cercueil (2019-05-02 14:24:59)
> The main clocks (cclk, hclk, pclk, mclk, lcd) were using
> incorrect dividers, and thus reported an incorrect rate.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Applied to clk-next

Did these "Fix" subject patches need a Fixes: tag?

Re: [PATCH 1/5] clk: ingenic: Add support for divider tables

[Stephen Boyd]

Quoting Paul Cercueil (2019-05-02 14:24:58)
> Some clocks provided on Ingenic SoCs have dividers, whose hardware value
> as written in the register cannot be expressed as an affine function
> to the actual divider value.
> 
> For instance, for the CPU clock on the JZ4770, the dividers are coded as
> follows:
> 
>     ------------------

Applied to clk-next

Re: [PATCH 3/5] clk: ingenic/jz4770: Fix incorrect dividers for main clocks

[Stephen Boyd]

Quoting Paul Cercueil (2019-05-02 14:25:00)
> The main clocks (cclk, h0clk, h1clk, h2clk, c1clk, pclk) were using
> incorrect dividers, and thus reported an incorrect rate.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Applied to clk-next

Re: [PATCH 4/5] clk: ingenic/jz4725b: Fix incorrect dividers for main clocks

[Stephen Boyd]

Quoting Paul Cercueil (2019-05-02 14:25:01)
> The main clocks (cclk, hclk, pclk, mclk, ipu) were using
> incorrect dividers, and thus reported an incorrect rate.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Applied to clk-next

Re: [PATCH 5/5] clk: ingenic/jz4725b: Fix "pll half" divider not read/written properly

[Stephen Boyd]

Quoting Paul Cercueil (2019-05-02 14:25:02)
> The code was setting the bit 21 of the CPCCR register to use a divider
> of 2 for the "pll half" clock, and clearing the bit to use a divider
> of 1.
> 
> This is the opposite of how this register field works: a cleared bit
> means that the /2 divider is used, and a set bit means that the divider
> is 1.
> 
> Restore the correct behaviour using the newly introduced .div_table
> field.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Applied to clk-next

Re: [PATCH 2/5] clk: ingenic/jz4740: Fix incorrect dividers for main clocks

[Paul Cercueil]

Le ven. 7 juin 2019 à 23:03, Stephen Boyd <sboyd@kernel.org> a écrit :
> Quoting Paul Cercueil (2019-05-02 14:24:59)
>>  The main clocks (cclk, hclk, pclk, mclk, lcd) were using
>>  incorrect dividers, and thus reported an incorrect rate.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
> 
> Applied to clk-next
> 
> Did these "Fix" subject patches need a Fixes: tag?

In theory yes, in practice this fix requires patch [1/5] to be
applied as well and there has been a lot of changes made between
the introduction of the file (which introduced the bug) and this
fix, so it wouldn't be easy to handle.