[PATCH 1/5] clk: ingenic: Use to_clk_info() macro for all clocks

[PATCH 1/5] clk: ingenic: Use to_clk_info() macro for all clocks

[Paul Cercueil]

The to_clk_info() previously had a BUG_ON() to check that it was only
called for PLL clocks. Yet, all the other clocks were doing the exact
same thing the macro does, in-line.

Move the to_clk_info() macro to the top of the file, remove the
hardcoded BUG_ON(), and use it everywhere it makes sense.

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

diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
index d7981b670221..12b14286734c 100644
--- a/drivers/clk/ingenic/cgu.c
+++ b/drivers/clk/ingenic/cgu.c
@@ -21,6 +21,12 @@
 
 #define MHZ (1000 * 1000)
 
+static inline const struct ingenic_cgu_clk_info *
+to_clk_info(struct ingenic_clk *clk)
+{
+	return &clk->cgu->clock_info[clk->idx];
+}
+
 /**
  * ingenic_cgu_gate_get() - get the value of clock gate register bit
  * @cgu: reference to the CGU whose registers should be read
@@ -71,14 +77,13 @@ static unsigned long
 ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 {
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
+	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
 	const struct ingenic_cgu_pll_info *pll_info;
 	unsigned m, n, od_enc, od;
 	bool bypass;
 	u32 ctl;
 
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
 	BUG_ON(clk_info->type != CGU_CLK_PLL);
 	pll_info = &clk_info->pll;
 
@@ -144,18 +149,6 @@ ingenic_pll_calc(const struct ingenic_cgu_clk_info *clk_info,
 		n * od);
 }
 
-static inline const struct ingenic_cgu_clk_info *to_clk_info(
-		struct ingenic_clk *ingenic_clk)
-{
-	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
-
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
-	BUG_ON(clk_info->type != CGU_CLK_PLL);
-
-	return clk_info;
-}
-
 static long
 ingenic_pll_round_rate(struct clk_hw *hw, unsigned long req_rate,
 		       unsigned long *prate)
@@ -290,13 +283,11 @@ static const struct clk_ops ingenic_pll_ops = {
 static u8 ingenic_clk_get_parent(struct clk_hw *hw)
 {
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
+	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
 	u32 reg;
 	u8 i, hw_idx, idx = 0;
 
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
-
 	if (clk_info->type & CGU_CLK_MUX) {
 		reg = readl(cgu->base + clk_info->mux.reg);
 		hw_idx = (reg >> clk_info->mux.shift) &
@@ -318,14 +309,12 @@ static u8 ingenic_clk_get_parent(struct clk_hw *hw)
 static int ingenic_clk_set_parent(struct clk_hw *hw, u8 idx)
 {
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
+	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
 	unsigned long flags;
 	u8 curr_idx, hw_idx, num_poss;
 	u32 reg, mask;
 
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
-
 	if (clk_info->type & CGU_CLK_MUX) {
 		/*
 		 * Convert the parent index to the hardware index by adding
@@ -368,13 +357,11 @@ static unsigned long
 ingenic_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 {
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
+	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
 	unsigned long rate = parent_rate;
 	u32 div_reg, div;
 
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
-
 	if (clk_info->type & CGU_CLK_DIV) {
 		div_reg = readl(cgu->base + clk_info->div.reg);
 		div = (div_reg >> clk_info->div.shift) &
@@ -443,12 +430,9 @@ ingenic_clk_round_rate(struct clk_hw *hw, unsigned long req_rate,
 		       unsigned long *parent_rate)
 {
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
-	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
+	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	unsigned int div = 1;
 
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
-
 	if (clk_info->type & CGU_CLK_DIV)
 		div = ingenic_clk_calc_div(clk_info, *parent_rate, req_rate);
 	else if (clk_info->type & CGU_CLK_FIXDIV)
@@ -462,16 +446,14 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate,
 		     unsigned long parent_rate)
 {
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
+	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
 	const unsigned timeout = 100;
 	unsigned long rate, flags;
 	unsigned int hw_div, div, i;
 	u32 reg, mask;
 	int ret = 0;
 
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
-
 	if (clk_info->type & CGU_CLK_DIV) {
 		div = ingenic_clk_calc_div(clk_info, parent_rate, req_rate);
 		rate = DIV_ROUND_UP(parent_rate, div);
@@ -525,12 +507,10 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate,
 static int ingenic_clk_enable(struct clk_hw *hw)
 {
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
+	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
 	unsigned long flags;
 
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
-
 	if (clk_info->type & CGU_CLK_GATE) {
 		/* ungate the clock */
 		spin_lock_irqsave(&cgu->lock, flags);
@@ -547,12 +527,10 @@ static int ingenic_clk_enable(struct clk_hw *hw)
 static void ingenic_clk_disable(struct clk_hw *hw)
 {
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
+	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
 	unsigned long flags;
 
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
-
 	if (clk_info->type & CGU_CLK_GATE) {
 		/* gate the clock */
 		spin_lock_irqsave(&cgu->lock, flags);
@@ -564,12 +542,10 @@ static void ingenic_clk_disable(struct clk_hw *hw)
 static int ingenic_clk_is_enabled(struct clk_hw *hw)
 {
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
+	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const struct ingenic_cgu_clk_info *clk_info;
 	int enabled = 1;
 
-	clk_info = &cgu->clock_info[ingenic_clk->idx];
-
 	if (clk_info->type & CGU_CLK_GATE)
 		enabled = !ingenic_cgu_gate_get(cgu, &clk_info->gate);
 
-- 
2.28.0

[PATCH 2/5] clk: ingenic: Use readl_poll_timeout instead of custom loop

[Paul Cercueil]

Use the readl_poll_timeout() function instead of rolling our own
busy-wait loops. This makes the code simpler.

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

diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
index 12b14286734c..6bb5dedf0252 100644
--- a/drivers/clk/ingenic/cgu.c
+++ b/drivers/clk/ingenic/cgu.c
@@ -12,11 +12,14 @@
 #include <linux/clkdev.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/math64.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/time.h>
+
 #include "cgu.h"
 
 #define MHZ (1000 * 1000)
@@ -159,6 +162,16 @@ ingenic_pll_round_rate(struct clk_hw *hw, unsigned long req_rate,
 	return ingenic_pll_calc(clk_info, req_rate, *prate, NULL, NULL, NULL);
 }
 
+static inline int ingenic_pll_check_stable(struct ingenic_cgu *cgu,
+					   const struct ingenic_cgu_pll_info *pll_info)
+{
+	u32 ctl;
+
+	return readl_poll_timeout(cgu->base + pll_info->reg, ctl,
+				  ctl & BIT(pll_info->stable_bit),
+				  0, 100 * USEC_PER_MSEC);
+}
+
 static int
 ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate,
 		     unsigned long parent_rate)
@@ -201,9 +214,8 @@ static int ingenic_pll_enable(struct clk_hw *hw)
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
 	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	const struct ingenic_cgu_pll_info *pll_info = &clk_info->pll;
-	const unsigned int timeout = 100;
 	unsigned long flags;
-	unsigned int i;
+	int ret;
 	u32 ctl;
 
 	spin_lock_irqsave(&cgu->lock, flags);
@@ -219,20 +231,10 @@ static int ingenic_pll_enable(struct clk_hw *hw)
 
 	writel(ctl, cgu->base + pll_info->reg);
 
-	/* wait for the PLL to stabilise */
-	for (i = 0; i < timeout; i++) {
-		ctl = readl(cgu->base + pll_info->reg);
-		if (ctl & BIT(pll_info->stable_bit))
-			break;
-		mdelay(1);
-	}
-
+	ret = ingenic_pll_check_stable(cgu, pll_info);
 	spin_unlock_irqrestore(&cgu->lock, flags);
 
-	if (i == timeout)
-		return -EBUSY;
-
-	return 0;
+	return ret;
 }
 
 static void ingenic_pll_disable(struct clk_hw *hw)
@@ -441,6 +443,16 @@ ingenic_clk_round_rate(struct clk_hw *hw, unsigned long req_rate,
 	return DIV_ROUND_UP(*parent_rate, div);
 }
 
+static inline int ingenic_clk_check_stable(struct ingenic_cgu *cgu,
+					   const struct ingenic_cgu_clk_info *clk_info)
+{
+	u32 reg;
+
+	return readl_poll_timeout(cgu->base + clk_info->div.reg, reg,
+				  !(reg & BIT(clk_info->div.busy_bit)),
+				  0, 100 * USEC_PER_MSEC);
+}
+
 static int
 ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate,
 		     unsigned long parent_rate)
@@ -448,9 +460,8 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate,
 	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
 	const struct ingenic_cgu_clk_info *clk_info = to_clk_info(ingenic_clk);
 	struct ingenic_cgu *cgu = ingenic_clk->cgu;
-	const unsigned timeout = 100;
 	unsigned long rate, flags;
-	unsigned int hw_div, div, i;
+	unsigned int hw_div, div;
 	u32 reg, mask;
 	int ret = 0;
 
@@ -486,16 +497,8 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate,
 		writel(reg, cgu->base + clk_info->div.reg);
 
 		/* wait for the change to take effect */
-		if (clk_info->div.busy_bit != -1) {
-			for (i = 0; i < timeout; i++) {
-				reg = readl(cgu->base + clk_info->div.reg);
-				if (!(reg & BIT(clk_info->div.busy_bit)))
-					break;
-				mdelay(1);
-			}
-			if (i == timeout)
-				ret = -EBUSY;
-		}
+		if (clk_info->div.busy_bit != -1)
+			ret = ingenic_clk_check_stable(cgu, clk_info);
 
 		spin_unlock_irqrestore(&cgu->lock, flags);
 		return ret;
-- 
2.28.0

[PATCH 3/5] clk: ingenic: Don't use CLK_SET_RATE_GATE for PLL

[Paul Cercueil]

CLK_SET_RATE_GATE means that the clock must be gated when being
reclocked. This is not the case for the PLLs in Ingenic SoCs.

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

diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
index 6bb5dedf0252..521a40dfcb72 100644
--- a/drivers/clk/ingenic/cgu.c
+++ b/drivers/clk/ingenic/cgu.c
@@ -182,6 +182,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate,
 	const struct ingenic_cgu_pll_info *pll_info = &clk_info->pll;
 	unsigned long rate, flags;
 	unsigned int m, n, od;
+	int ret = 0;
 	u32 ctl;
 
 	rate = ingenic_pll_calc(clk_info, req_rate, parent_rate,
@@ -203,9 +204,14 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate,
 	ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
 
 	writel(ctl, cgu->base + pll_info->reg);
+
+	/* If the PLL is enabled, verify that it's stable */
+	if (ctl & BIT(pll_info->enable_bit))
+		ret = ingenic_pll_check_stable(cgu, pll_info);
+
 	spin_unlock_irqrestore(&cgu->lock, flags);
 
-	return 0;
+	return ret;
 }
 
 static int ingenic_pll_enable(struct clk_hw *hw)
@@ -662,7 +668,6 @@ static int ingenic_register_clock(struct ingenic_cgu *cgu, unsigned idx)
 		}
 	} else if (caps & CGU_CLK_PLL) {
 		clk_init.ops = &ingenic_pll_ops;
-		clk_init.flags |= CLK_SET_RATE_GATE;
 
 		caps &= ~CGU_CLK_PLL;
 
-- 
2.28.0

[PATCH 4/5] clk: ingenic: Don't tag custom clocks with CLK_SET_RATE_PARENT

[Paul Cercueil]

The custom clocks have custom functions to round, get or set their rate.
Therefore, we can't assume that they need the CLK_SET_RATE_PARENT flag.

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

diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
index 521a40dfcb72..a1a4f1adaa3a 100644
--- a/drivers/clk/ingenic/cgu.c
+++ b/drivers/clk/ingenic/cgu.c
@@ -629,6 +629,13 @@ static int ingenic_register_clock(struct ingenic_cgu *cgu, unsigned idx)
 
 	caps = clk_info->type;
 
+	if (caps & CGU_CLK_DIV) {
+		caps &= ~CGU_CLK_DIV;
+	} else if (!(caps & CGU_CLK_CUSTOM)) {
+		/* pass rate changes to the parent clock */
+		clk_init.flags |= CLK_SET_RATE_PARENT;
+	}
+
 	if (caps & (CGU_CLK_MUX | CGU_CLK_CUSTOM)) {
 		clk_init.num_parents = 0;
 
@@ -690,13 +697,6 @@ static int ingenic_register_clock(struct ingenic_cgu *cgu, unsigned idx)
 		caps &= ~(CGU_CLK_MUX | CGU_CLK_MUX_GLITCHFREE);
 	}
 
-	if (caps & CGU_CLK_DIV) {
-		caps &= ~CGU_CLK_DIV;
-	} else {
-		/* pass rate changes to the parent clock */
-		clk_init.flags |= CLK_SET_RATE_PARENT;
-	}
-
 	if (caps) {
 		pr_err("%s: unknown clock type 0x%x\n", __func__, caps);
 		goto out;
-- 
2.28.0

[PATCH 5/5] clk: ingenic: Respect CLK_SET_RATE_PARENT in .round_rate

[Paul Cercueil]

Clocks that don't have a divider are in our case all marked with the
CLK_SET_RATE_PARENT flag. In this case, the .round_rate implementation
should modify the value pointed to by parent_rate, in order to propagate
the rate change to the parent, as explained in the documentation of
clk_set_rate().

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

diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
index a1a4f1adaa3a..dac6edc670cc 100644
--- a/drivers/clk/ingenic/cgu.c
+++ b/drivers/clk/ingenic/cgu.c
@@ -445,6 +445,8 @@ ingenic_clk_round_rate(struct clk_hw *hw, unsigned long req_rate,
 		div = ingenic_clk_calc_div(clk_info, *parent_rate, req_rate);
 	else if (clk_info->type & CGU_CLK_FIXDIV)
 		div = clk_info->fixdiv.div;
+	else if (clk_hw_can_set_rate_parent(hw))
+		*parent_rate = req_rate;
 
 	return DIV_ROUND_UP(*parent_rate, div);
 }
-- 
2.28.0

Re: [PATCH 1/5] clk: ingenic: Use to_clk_info() macro for all clocks

[Stephen Boyd]

Quoting Paul Cercueil (2020-09-02 18:50:44)
> The to_clk_info() previously had a BUG_ON() to check that it was only
> called for PLL clocks. Yet, all the other clocks were doing the exact
> same thing the macro does, in-line.
> 
> Move the to_clk_info() macro to the top of the file, remove the
> hardcoded BUG_ON(), and use it everywhere it makes sense.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Applied to clk-next

Re: [PATCH 2/5] clk: ingenic: Use readl_poll_timeout instead of custom loop

[Stephen Boyd]

Quoting Paul Cercueil (2020-09-02 18:50:45)
> Use the readl_poll_timeout() function instead of rolling our own
> busy-wait loops. This makes the code simpler.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Applied to clk-next

Re: [PATCH 3/5] clk: ingenic: Don't use CLK_SET_RATE_GATE for PLL

[Stephen Boyd]

Quoting Paul Cercueil (2020-09-02 18:50:46)
> CLK_SET_RATE_GATE means that the clock must be gated when being
> reclocked. This is not the case for the PLLs in Ingenic SoCs.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Applied to clk-next

Re: [PATCH 4/5] clk: ingenic: Don't tag custom clocks with CLK_SET_RATE_PARENT

[Stephen Boyd]

Quoting Paul Cercueil (2020-09-02 18:50:47)
> The custom clocks have custom functions to round, get or set their rate.
> Therefore, we can't assume that they need the CLK_SET_RATE_PARENT flag.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Applied to clk-next

Re: [PATCH 5/5] clk: ingenic: Respect CLK_SET_RATE_PARENT in .round_rate

[Stephen Boyd]

Quoting Paul Cercueil (2020-09-02 18:50:48)
> Clocks that don't have a divider are in our case all marked with the
> CLK_SET_RATE_PARENT flag. In this case, the .round_rate implementation
> should modify the value pointed to by parent_rate, in order to propagate
> the rate change to the parent, as explained in the documentation of
> clk_set_rate().
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Applied to clk-next