[PATCH 0/3] clk: let clock perform allocation in init

[PATCH 0/3] clk: let clock perform allocation in init

[Jerome Brunet]

This patchset is a follow up on this pinky swear [0].
Its purpose is:
 * Clarify the acceptable use of clk_ops init() callback
 * Let the init() callback return an error code in case anything
   fail.
 * Add the terminate() counter part of of init() to release the
   resources which may have been claimed in init()

After discussing with Stephen at LPC, I decided to drop the 2 last patches
of the RFC [1]. I can live without it for now and nobody expressed a
critical need to get the proposed placeholder.

[0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com
[1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com

Jerome Brunet (3):
  clk: actually call the clock init before any other callback of the
    clock
  clk: let init callback return an error code
  clk: add terminate callback to clk_ops

 drivers/clk/clk.c                     | 38 ++++++++++++++++++---------
 drivers/clk/meson/clk-mpll.c          |  4 ++-
 drivers/clk/meson/clk-phase.c         |  4 ++-
 drivers/clk/meson/clk-pll.c           |  4 ++-
 drivers/clk/meson/sclk-div.c          |  4 ++-
 drivers/clk/microchip/clk-core.c      |  8 ++++--
 drivers/clk/mmp/clk-frac.c            |  4 ++-
 drivers/clk/mmp/clk-mix.c             |  4 ++-
 drivers/clk/qcom/clk-hfpll.c          |  6 +++--
 drivers/clk/rockchip/clk-pll.c        | 28 ++++++++++++--------
 drivers/clk/ti/clock.h                |  2 +-
 drivers/clk/ti/clockdomain.c          |  8 +++---
 drivers/net/phy/mdio-mux-meson-g12a.c |  4 ++-
 include/linux/clk-provider.h          | 13 ++++++---
 14 files changed, 90 insertions(+), 41 deletions(-)

-- 
2.21.0

[PATCH 1/3] clk: actually call the clock init before any other callback of the clock

[Jerome Brunet]

 __clk_init_parent() will call the .get_parent() callback of the clock
 so .init() must run before.

Fixes: 541debae0adf ("clk: call the clock init() callback before any other ops callback")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/clk.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 1c677d7f7f53..232144cca6cf 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3294,6 +3294,21 @@ static int __clk_core_init(struct clk_core *core)
 		goto out;
 	}
 
+	/*
+	 * optional platform-specific magic
+	 *
+	 * The .init callback is not used by any of the basic clock types, but
+	 * exists for weird hardware that must perform initialization magic.
+	 * Please consider other ways of solving initialization problems before
+	 * using this callback, as its use is discouraged.
+	 *
+	 * If it exist, this callback should called before any other callback of
+	 * the clock
+	 */
+	if (core->ops->init)
+		core->ops->init(core->hw);
+
+
 	core->parent = __clk_init_parent(core);
 
 	/*
@@ -3318,17 +3333,6 @@ static int __clk_core_init(struct clk_core *core)
 		core->orphan = true;
 	}
 
-	/*
-	 * optional platform-specific magic
-	 *
-	 * The .init callback is not used by any of the basic clock types, but
-	 * exists for weird hardware that must perform initialization magic.
-	 * Please consider other ways of solving initialization problems before
-	 * using this callback, as its use is discouraged.
-	 */
-	if (core->ops->init)
-		core->ops->init(core->hw);
-
 	/*
 	 * Set clk's accuracy.  The preferred method is to use
 	 * .recalc_accuracy. For simple clocks and lazy developers the default
-- 
2.21.0

[PATCH 2/3] clk: let init callback return an error code

[Jerome Brunet]

If the init callback is allowed to request resources, it needs a return
value to report the outcome of such a request.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---

 Sorry about the spam.
 This patch change quite a few files so I have tried to Cc the
 relevant people. Stephen, You may notice that I have added a
 couple of the network people. You need an Ack from one of them
 since the Amlogic G12a mdio mux has a clock which uses the .init()
 callback

 drivers/clk/clk.c                     | 17 ++++++++++------
 drivers/clk/meson/clk-mpll.c          |  4 +++-
 drivers/clk/meson/clk-phase.c         |  4 +++-
 drivers/clk/meson/clk-pll.c           |  4 +++-
 drivers/clk/meson/sclk-div.c          |  4 +++-
 drivers/clk/microchip/clk-core.c      |  8 ++++++--
 drivers/clk/mmp/clk-frac.c            |  4 +++-
 drivers/clk/mmp/clk-mix.c             |  4 +++-
 drivers/clk/qcom/clk-hfpll.c          |  6 ++++--
 drivers/clk/rockchip/clk-pll.c        | 28 ++++++++++++++++-----------
 drivers/clk/ti/clock.h                |  2 +-
 drivers/clk/ti/clockdomain.c          |  8 +++++---
 drivers/net/phy/mdio-mux-meson-g12a.c |  4 +++-
 include/linux/clk-provider.h          | 10 +++++++---
 14 files changed, 72 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 232144cca6cf..df853a98fad2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3298,16 +3298,21 @@ static int __clk_core_init(struct clk_core *core)
 	 * optional platform-specific magic
 	 *
 	 * The .init callback is not used by any of the basic clock types, but
-	 * exists for weird hardware that must perform initialization magic.
-	 * Please consider other ways of solving initialization problems before
-	 * using this callback, as its use is discouraged.
+	 * exists for weird hardware that must perform initialization magic for
+	 * CCF to get an accurate view of clock for any other callbacks. It may
+	 * also be used needs to perform dynamic allocations. Such allocation
+	 * must be freed in the terminate() callback.
+	 * This callback shall not be used to initialize the parameters state,
+	 * such as rate, parent, etc ...
 	 *
 	 * If it exist, this callback should called before any other callback of
 	 * the clock
 	 */
-	if (core->ops->init)
-		core->ops->init(core->hw);
-
+	if (core->ops->init) {
+		ret = core->ops->init(core->hw);
+		if (ret)
+			goto out;
+	}
 
 	core->parent = __clk_init_parent(core);
 
diff --git a/drivers/clk/meson/clk-mpll.c b/drivers/clk/meson/clk-mpll.c
index 2d39a8bc367c..fc9df4860872 100644
--- a/drivers/clk/meson/clk-mpll.c
+++ b/drivers/clk/meson/clk-mpll.c
@@ -129,7 +129,7 @@ static int mpll_set_rate(struct clk_hw *hw,
 	return 0;
 }
 
-static void mpll_init(struct clk_hw *hw)
+static int mpll_init(struct clk_hw *hw)
 {
 	struct clk_regmap *clk = to_clk_regmap(hw);
 	struct meson_clk_mpll_data *mpll = meson_clk_mpll_data(clk);
@@ -151,6 +151,8 @@ static void mpll_init(struct clk_hw *hw)
 	/* Set the magic misc bit if required */
 	if (MESON_PARM_APPLICABLE(&mpll->misc))
 		meson_parm_write(clk->map, &mpll->misc, 1);
+
+	return 0;
 }
 
 const struct clk_ops meson_clk_mpll_ro_ops = {
diff --git a/drivers/clk/meson/clk-phase.c b/drivers/clk/meson/clk-phase.c
index 80c3ada193a4..fe22e171121a 100644
--- a/drivers/clk/meson/clk-phase.c
+++ b/drivers/clk/meson/clk-phase.c
@@ -78,7 +78,7 @@ meson_clk_triphase_data(struct clk_regmap *clk)
 	return (struct meson_clk_triphase_data *)clk->data;
 }
 
-static void meson_clk_triphase_sync(struct clk_hw *hw)
+static int meson_clk_triphase_sync(struct clk_hw *hw)
 {
 	struct clk_regmap *clk = to_clk_regmap(hw);
 	struct meson_clk_triphase_data *tph = meson_clk_triphase_data(clk);
@@ -88,6 +88,8 @@ static void meson_clk_triphase_sync(struct clk_hw *hw)
 	val = meson_parm_read(clk->map, &tph->ph0);
 	meson_parm_write(clk->map, &tph->ph1, val);
 	meson_parm_write(clk->map, &tph->ph2, val);
+
+	return 0;
 }
 
 static int meson_clk_triphase_get_phase(struct clk_hw *hw)
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index ddb1e5634739..489092dde3a6 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -277,7 +277,7 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw)
 	return -ETIMEDOUT;
 }
 
-static void meson_clk_pll_init(struct clk_hw *hw)
+static int meson_clk_pll_init(struct clk_hw *hw)
 {
 	struct clk_regmap *clk = to_clk_regmap(hw);
 	struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
@@ -288,6 +288,8 @@ static void meson_clk_pll_init(struct clk_hw *hw)
 				       pll->init_count);
 		meson_parm_write(clk->map, &pll->rst, 0);
 	}
+
+	return 0;
 }
 
 static int meson_clk_pll_is_enabled(struct clk_hw *hw)
diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
index 3acf03780221..76d31c0a3342 100644
--- a/drivers/clk/meson/sclk-div.c
+++ b/drivers/clk/meson/sclk-div.c
@@ -216,7 +216,7 @@ static int sclk_div_is_enabled(struct clk_hw *hw)
 	return 0;
 }
 
-static void sclk_div_init(struct clk_hw *hw)
+static int sclk_div_init(struct clk_hw *hw)
 {
 	struct clk_regmap *clk = to_clk_regmap(hw);
 	struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk);
@@ -231,6 +231,8 @@ static void sclk_div_init(struct clk_hw *hw)
 		sclk->cached_div = val + 1;
 
 	sclk_div_get_duty_cycle(hw, &sclk->cached_duty);
+
+	return 0;
 }
 
 const struct clk_ops meson_sclk_div_ops = {
diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c
index 567755d6f844..1b4f023cdc8b 100644
--- a/drivers/clk/microchip/clk-core.c
+++ b/drivers/clk/microchip/clk-core.c
@@ -266,10 +266,12 @@ static void roclk_disable(struct clk_hw *hw)
 	writel(REFO_ON | REFO_OE, PIC32_CLR(refo->ctrl_reg));
 }
 
-static void roclk_init(struct clk_hw *hw)
+static int roclk_init(struct clk_hw *hw)
 {
 	/* initialize clock in disabled state */
 	roclk_disable(hw);
+
+	return 0;
 }
 
 static u8 roclk_get_parent(struct clk_hw *hw)
@@ -880,7 +882,7 @@ static int sclk_set_parent(struct clk_hw *hw, u8 index)
 	return err;
 }
 
-static void sclk_init(struct clk_hw *hw)
+static int sclk_init(struct clk_hw *hw)
 {
 	struct pic32_sys_clk *sclk = clkhw_to_sys_clk(hw);
 	unsigned long flags;
@@ -899,6 +901,8 @@ static void sclk_init(struct clk_hw *hw)
 		writel(v, sclk->slew_reg);
 		spin_unlock_irqrestore(&sclk->core->reg_lock, flags);
 	}
+
+	return 0;
 }
 
 /* sclk with post-divider */
diff --git a/drivers/clk/mmp/clk-frac.c b/drivers/clk/mmp/clk-frac.c
index 90bf181f191a..fabc09aca6c4 100644
--- a/drivers/clk/mmp/clk-frac.c
+++ b/drivers/clk/mmp/clk-frac.c
@@ -109,7 +109,7 @@ static int clk_factor_set_rate(struct clk_hw *hw, unsigned long drate,
 	return 0;
 }
 
-static void clk_factor_init(struct clk_hw *hw)
+static int clk_factor_init(struct clk_hw *hw)
 {
 	struct mmp_clk_factor *factor = to_clk_factor(hw);
 	struct mmp_clk_factor_masks *masks = factor->masks;
@@ -146,6 +146,8 @@ static void clk_factor_init(struct clk_hw *hw)
 
 	if (factor->lock)
 		spin_unlock_irqrestore(factor->lock, flags);
+
+	return 0;
 }
 
 static const struct clk_ops clk_factor_ops = {
diff --git a/drivers/clk/mmp/clk-mix.c b/drivers/clk/mmp/clk-mix.c
index 90814b2613c0..d2cd36c54474 100644
--- a/drivers/clk/mmp/clk-mix.c
+++ b/drivers/clk/mmp/clk-mix.c
@@ -419,12 +419,14 @@ static int mmp_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	}
 }
 
-static void mmp_clk_mix_init(struct clk_hw *hw)
+static int mmp_clk_mix_init(struct clk_hw *hw)
 {
 	struct mmp_clk_mix *mix = to_clk_mix(hw);
 
 	if (mix->table)
 		_filter_clk_table(mix, mix->table, mix->table_size);
+
+	return 0;
 }
 
 const struct clk_ops mmp_clk_mix_ops = {
diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
index 3c04805f2a55..e847d586a73a 100644
--- a/drivers/clk/qcom/clk-hfpll.c
+++ b/drivers/clk/qcom/clk-hfpll.c
@@ -196,7 +196,7 @@ static unsigned long clk_hfpll_recalc_rate(struct clk_hw *hw,
 	return l_val * parent_rate;
 }
 
-static void clk_hfpll_init(struct clk_hw *hw)
+static int clk_hfpll_init(struct clk_hw *hw)
 {
 	struct clk_hfpll *h = to_clk_hfpll(hw);
 	struct hfpll_data const *hd = h->d;
@@ -206,7 +206,7 @@ static void clk_hfpll_init(struct clk_hw *hw)
 	regmap_read(regmap, hd->mode_reg, &mode);
 	if (mode != (PLL_BYPASSNL | PLL_RESET_N | PLL_OUTCTRL)) {
 		__clk_hfpll_init_once(hw);
-		return;
+		return 0;
 	}
 
 	if (hd->status_reg) {
@@ -218,6 +218,8 @@ static void clk_hfpll_init(struct clk_hw *hw)
 			__clk_hfpll_init_once(hw);
 		}
 	}
+
+	return 0;
 }
 
 static int hfpll_is_enabled(struct clk_hw *hw)
diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
index 198417d56300..10560d963baf 100644
--- a/drivers/clk/rockchip/clk-pll.c
+++ b/drivers/clk/rockchip/clk-pll.c
@@ -282,7 +282,7 @@ static int rockchip_rk3036_pll_is_enabled(struct clk_hw *hw)
 	return !(pllcon & RK3036_PLLCON1_PWRDOWN);
 }
 
-static void rockchip_rk3036_pll_init(struct clk_hw *hw)
+static int rockchip_rk3036_pll_init(struct clk_hw *hw)
 {
 	struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
 	const struct rockchip_pll_rate_table *rate;
@@ -290,14 +290,14 @@ static void rockchip_rk3036_pll_init(struct clk_hw *hw)
 	unsigned long drate;
 
 	if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE))
-		return;
+		return 0;
 
 	drate = clk_hw_get_rate(hw);
 	rate = rockchip_get_pll_settings(pll, drate);
 
 	/* when no rate setting for the current rate, rely on clk_set_rate */
 	if (!rate)
-		return;
+		return 0;
 
 	rockchip_rk3036_pll_get_params(pll, &cur);
 
@@ -319,13 +319,15 @@ static void rockchip_rk3036_pll_init(struct clk_hw *hw)
 		if (!parent) {
 			pr_warn("%s: parent of %s not available\n",
 				__func__, __clk_get_name(hw->clk));
-			return;
+			return 0;
 		}
 
 		pr_debug("%s: pll %s: rate params do not match rate table, adjusting\n",
 			 __func__, __clk_get_name(hw->clk));
 		rockchip_rk3036_pll_set_params(pll, rate);
 	}
+
+	return 0;
 }
 
 static const struct clk_ops rockchip_rk3036_pll_clk_norate_ops = {
@@ -515,7 +517,7 @@ static int rockchip_rk3066_pll_is_enabled(struct clk_hw *hw)
 	return !(pllcon & RK3066_PLLCON3_PWRDOWN);
 }
 
-static void rockchip_rk3066_pll_init(struct clk_hw *hw)
+static int rockchip_rk3066_pll_init(struct clk_hw *hw)
 {
 	struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
 	const struct rockchip_pll_rate_table *rate;
@@ -523,14 +525,14 @@ static void rockchip_rk3066_pll_init(struct clk_hw *hw)
 	unsigned long drate;
 
 	if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE))
-		return;
+		return 0;
 
 	drate = clk_hw_get_rate(hw);
 	rate = rockchip_get_pll_settings(pll, drate);
 
 	/* when no rate setting for the current rate, rely on clk_set_rate */
 	if (!rate)
-		return;
+		return 0;
 
 	rockchip_rk3066_pll_get_params(pll, &cur);
 
@@ -543,6 +545,8 @@ static void rockchip_rk3066_pll_init(struct clk_hw *hw)
 			 __func__, clk_hw_get_name(hw));
 		rockchip_rk3066_pll_set_params(pll, rate);
 	}
+
+	return 0;
 }
 
 static const struct clk_ops rockchip_rk3066_pll_clk_norate_ops = {
@@ -761,7 +765,7 @@ static int rockchip_rk3399_pll_is_enabled(struct clk_hw *hw)
 	return !(pllcon & RK3399_PLLCON3_PWRDOWN);
 }
 
-static void rockchip_rk3399_pll_init(struct clk_hw *hw)
+static int rockchip_rk3399_pll_init(struct clk_hw *hw)
 {
 	struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
 	const struct rockchip_pll_rate_table *rate;
@@ -769,14 +773,14 @@ static void rockchip_rk3399_pll_init(struct clk_hw *hw)
 	unsigned long drate;
 
 	if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE))
-		return;
+		return 0;
 
 	drate = clk_hw_get_rate(hw);
 	rate = rockchip_get_pll_settings(pll, drate);
 
 	/* when no rate setting for the current rate, rely on clk_set_rate */
 	if (!rate)
-		return;
+		return 0;
 
 	rockchip_rk3399_pll_get_params(pll, &cur);
 
@@ -798,13 +802,15 @@ static void rockchip_rk3399_pll_init(struct clk_hw *hw)
 		if (!parent) {
 			pr_warn("%s: parent of %s not available\n",
 				__func__, __clk_get_name(hw->clk));
-			return;
+			return 0;
 		}
 
 		pr_debug("%s: pll %s: rate params do not match rate table, adjusting\n",
 			 __func__, __clk_get_name(hw->clk));
 		rockchip_rk3399_pll_set_params(pll, rate);
 	}
+
+	return 0;
 }
 
 static const struct clk_ops rockchip_rk3399_pll_clk_norate_ops = {
diff --git a/drivers/clk/ti/clock.h b/drivers/clk/ti/clock.h
index e4b8392ff63c..adef9c16e43b 100644
--- a/drivers/clk/ti/clock.h
+++ b/drivers/clk/ti/clock.h
@@ -252,7 +252,7 @@ extern const struct clk_ops omap_gate_clk_ops;
 
 extern struct ti_clk_features ti_clk_features;
 
-void omap2_init_clk_clkdm(struct clk_hw *hw);
+int omap2_init_clk_clkdm(struct clk_hw *hw);
 int omap2_clkops_enable_clkdm(struct clk_hw *hw);
 void omap2_clkops_disable_clkdm(struct clk_hw *hw);
 
diff --git a/drivers/clk/ti/clockdomain.c b/drivers/clk/ti/clockdomain.c
index 423a99b9f10c..ee56306f79d5 100644
--- a/drivers/clk/ti/clockdomain.c
+++ b/drivers/clk/ti/clockdomain.c
@@ -101,16 +101,16 @@ void omap2_clkops_disable_clkdm(struct clk_hw *hw)
  *
  * Convert a clockdomain name stored in a struct clk 'clk' into a
  * clockdomain pointer, and save it into the struct clk.  Intended to be
- * called during clk_register().  No return value.
+ * called during clk_register(). Returns 0 on success, -EERROR otherwise.
  */
-void omap2_init_clk_clkdm(struct clk_hw *hw)
+int omap2_init_clk_clkdm(struct clk_hw *hw)
 {
 	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
 	struct clockdomain *clkdm;
 	const char *clk_name;
 
 	if (!clk->clkdm_name)
-		return;
+		return 0;
 
 	clk_name = __clk_get_name(hw->clk);
 
@@ -123,6 +123,8 @@ void omap2_init_clk_clkdm(struct clk_hw *hw)
 		pr_debug("clock: could not associate clk %s to clkdm %s\n",
 			 clk_name, clk->clkdm_name);
 	}
+
+	return 0;
 }
 
 static void __init of_ti_clockdomain_setup(struct device_node *node)
diff --git a/drivers/net/phy/mdio-mux-meson-g12a.c b/drivers/net/phy/mdio-mux-meson-g12a.c
index 7a9ad54582e1..bf86c9c7a288 100644
--- a/drivers/net/phy/mdio-mux-meson-g12a.c
+++ b/drivers/net/phy/mdio-mux-meson-g12a.c
@@ -123,7 +123,7 @@ static int g12a_ephy_pll_is_enabled(struct clk_hw *hw)
 	return (val & PLL_CTL0_LOCK_DIG) ? 1 : 0;
 }
 
-static void g12a_ephy_pll_init(struct clk_hw *hw)
+static int g12a_ephy_pll_init(struct clk_hw *hw)
 {
 	struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw);
 
@@ -136,6 +136,8 @@ static void g12a_ephy_pll_init(struct clk_hw *hw)
 	writel(0x20200000, pll->base + ETH_PLL_CTL5);
 	writel(0x0000c002, pll->base + ETH_PLL_CTL6);
 	writel(0x00000023, pll->base + ETH_PLL_CTL7);
+
+	return 0;
 }
 
 static const struct clk_ops g12a_ephy_pll_ops = {
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 2fdfe8061363..b82ec4c4ca95 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -190,8 +190,12 @@ struct clk_duty {
  *
  * @init:	Perform platform-specific initialization magic.
  *		This is not not used by any of the basic clock types.
- *		Please consider other ways of solving initialization problems
- *		before using this callback, as its use is discouraged.
+ *		This callback exist for HW which needs to perform some
+ *		initialisation magic for CCF to get an accurate view of the
+ *		clock. It may also be used dynamic resource allocation is
+ *		required. It shall not used to deal with clock parameters,
+ *		such as rate or parents.
+ *		Returns 0 on success, -EERROR otherwise.
  *
  * @debug_init:	Set up type-specific debugfs entries for this clock.  This
  *		is called once, after the debugfs directory entry for this
@@ -243,7 +247,7 @@ struct clk_ops {
 					  struct clk_duty *duty);
 	int		(*set_duty_cycle)(struct clk_hw *hw,
 					  struct clk_duty *duty);
-	void		(*init)(struct clk_hw *hw);
+	int		(*init)(struct clk_hw *hw);
 	void		(*debug_init)(struct clk_hw *hw, struct dentry *dentry);
 };
 
-- 
2.21.0

[PATCH 3/3] clk: add terminate callback to clk_ops

[Jerome Brunet]

Add a terminate callback to the clk_ops to release the resources
claimed in .init()

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/clk.c            | 7 ++++++-
 include/linux/clk-provider.h | 3 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index df853a98fad2..926f49c78b5d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3844,6 +3844,7 @@ static void clk_core_evict_parent_cache(struct clk_core *core)
 void clk_unregister(struct clk *clk)
 {
 	unsigned long flags;
+	const struct clk_ops *ops;
 
 	if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
 		return;
@@ -3852,7 +3853,8 @@ void clk_unregister(struct clk *clk)
 
 	clk_prepare_lock();
 
-	if (clk->core->ops == &clk_nodrv_ops) {
+	ops = clk->core->ops;
+	if (ops == &clk_nodrv_ops) {
 		pr_err("%s: unregistered clock: %s\n", __func__,
 		       clk->core->name);
 		goto unlock;
@@ -3865,6 +3867,9 @@ void clk_unregister(struct clk *clk)
 	clk->core->ops = &clk_nodrv_ops;
 	clk_enable_unlock(flags);
 
+	if (ops->terminate)
+		ops->terminate(clk->core->hw);
+
 	if (!hlist_empty(&clk->core->children)) {
 		struct clk_core *child;
 		struct hlist_node *t;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index b82ec4c4ca95..5a5a64785923 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -197,6 +197,8 @@ struct clk_duty {
  *		such as rate or parents.
  *		Returns 0 on success, -EERROR otherwise.
  *
+ * @terminate:  Free any resource allocated by init.
+ *
  * @debug_init:	Set up type-specific debugfs entries for this clock.  This
  *		is called once, after the debugfs directory entry for this
  *		clock has been created.  The dentry pointer representing that
@@ -248,6 +250,7 @@ struct clk_ops {
 	int		(*set_duty_cycle)(struct clk_hw *hw,
 					  struct clk_duty *duty);
 	int		(*init)(struct clk_hw *hw);
+	void		(*terminate)(struct clk_hw *hw);
 	void		(*debug_init)(struct clk_hw *hw, struct dentry *dentry);
 };
 
-- 
2.21.0

RE: [PATCH 2/3] clk: let init callback return an error code

[Ankur Tyagi]

Hi,

I am no expert here but just looked at the patch and found few
discrepancy that I have mentioned inline.

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org <linux-omap-
> owner@vger.kernel.org> On Behalf Of Jerome Brunet
> Sent: Wednesday, 25 September 2019 12:40 AM
> To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>
> Cc: Jerome Brunet <jbrunet@baylibre.com>; linux-clk@vger.kernel.org; linux-
> kernel@vger.kernel.org; Heiko Stuebner <heiko@sntech.de>; Tero Kristo <t-
> kristo@ti.com>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
> <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; David S.
> Miller <davem@davemloft.net>; netdev@vger.kernel.org; linux-
> amlogic@lists.infradead.org; linux-arm-msm@vger.kernel.org; linux-
> rockchip@lists.infradead.org; linux-omap@vger.kernel.org
> Subject: [PATCH 2/3] clk: let init callback return an error code
>
> If the init callback is allowed to request resources, it needs a return
> value to report the outcome of such a request.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>
>  Sorry about the spam.
>  This patch change quite a few files so I have tried to Cc the
>  relevant people. Stephen, You may notice that I have added a
>  couple of the network people. You need an Ack from one of them
>  since the Amlogic G12a mdio mux has a clock which uses the .init()
>  callback
>
>  drivers/clk/clk.c                     | 17 ++++++++++------
>  drivers/clk/meson/clk-mpll.c          |  4 +++-
>  drivers/clk/meson/clk-phase.c         |  4 +++-
>  drivers/clk/meson/clk-pll.c           |  4 +++-
>  drivers/clk/meson/sclk-div.c          |  4 +++-
>  drivers/clk/microchip/clk-core.c      |  8 ++++++--
>  drivers/clk/mmp/clk-frac.c            |  4 +++-
>  drivers/clk/mmp/clk-mix.c             |  4 +++-
>  drivers/clk/qcom/clk-hfpll.c          |  6 ++++--
>  drivers/clk/rockchip/clk-pll.c        | 28 ++++++++++++++++-----------
>  drivers/clk/ti/clock.h                |  2 +-
>  drivers/clk/ti/clockdomain.c          |  8 +++++---
>  drivers/net/phy/mdio-mux-meson-g12a.c |  4 +++-
>  include/linux/clk-provider.h          | 10 +++++++---
>  14 files changed, 72 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 232144cca6cf..df853a98fad2 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3298,16 +3298,21 @@ static int __clk_core_init(struct clk_core *core)
>   * optional platform-specific magic
>   *
>   * The .init callback is not used by any of the basic clock types, but
> - * exists for weird hardware that must perform initialization magic.
> - * Please consider other ways of solving initialization problems before
> - * using this callback, as its use is discouraged.
> + * exists for weird hardware that must perform initialization magic for
> + * CCF to get an accurate view of clock for any other callbacks. It may
> + * also be used needs to perform dynamic allocations. Such allocation
> + * must be freed in the terminate() callback.
> + * This callback shall not be used to initialize the parameters state,
> + * such as rate, parent, etc ...
>   *
>   * If it exist, this callback should called before any other callback of
>   * the clock
>   */
> -if (core->ops->init)
> -core->ops->init(core->hw);
> -
> +if (core->ops->init) {
> +ret = core->ops->init(core->hw);
> +if (ret)
> +goto out;
> +}
>
>  core->parent = __clk_init_parent(core);
>
> diff --git a/drivers/clk/meson/clk-mpll.c b/drivers/clk/meson/clk-mpll.c
> index 2d39a8bc367c..fc9df4860872 100644
> --- a/drivers/clk/meson/clk-mpll.c
> +++ b/drivers/clk/meson/clk-mpll.c
> @@ -129,7 +129,7 @@ static int mpll_set_rate(struct clk_hw *hw,
>  return 0;
>  }
>
> -static void mpll_init(struct clk_hw *hw)
> +static int mpll_init(struct clk_hw *hw)
>  {
>  struct clk_regmap *clk = to_clk_regmap(hw);
>  struct meson_clk_mpll_data *mpll = meson_clk_mpll_data(clk);
> @@ -151,6 +151,8 @@ static void mpll_init(struct clk_hw *hw)
>  /* Set the magic misc bit if required */
>  if (MESON_PARM_APPLICABLE(&mpll->misc))
>  meson_parm_write(clk->map, &mpll->misc, 1);
> +
> +return 0;
>  }
>
>  const struct clk_ops meson_clk_mpll_ro_ops = {
> diff --git a/drivers/clk/meson/clk-phase.c b/drivers/clk/meson/clk-phase.c
> index 80c3ada193a4..fe22e171121a 100644
> --- a/drivers/clk/meson/clk-phase.c
> +++ b/drivers/clk/meson/clk-phase.c
> @@ -78,7 +78,7 @@ meson_clk_triphase_data(struct clk_regmap *clk)
>  return (struct meson_clk_triphase_data *)clk->data;
>  }
>
> -static void meson_clk_triphase_sync(struct clk_hw *hw)
> +static int meson_clk_triphase_sync(struct clk_hw *hw)
>  {
>  struct clk_regmap *clk = to_clk_regmap(hw);
>  struct meson_clk_triphase_data *tph = meson_clk_triphase_data(clk);
> @@ -88,6 +88,8 @@ static void meson_clk_triphase_sync(struct clk_hw *hw)
>  val = meson_parm_read(clk->map, &tph->ph0);
>  meson_parm_write(clk->map, &tph->ph1, val);
>  meson_parm_write(clk->map, &tph->ph2, val);
> +
> +return 0;
>  }
>
>  static int meson_clk_triphase_get_phase(struct clk_hw *hw)
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index ddb1e5634739..489092dde3a6 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -277,7 +277,7 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw)
>  return -ETIMEDOUT;
>  }
>
> -static void meson_clk_pll_init(struct clk_hw *hw)
> +static int meson_clk_pll_init(struct clk_hw *hw)
>  {
>  struct clk_regmap *clk = to_clk_regmap(hw);
>  struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
> @@ -288,6 +288,8 @@ static void meson_clk_pll_init(struct clk_hw *hw)
>         pll->init_count);
>  meson_parm_write(clk->map, &pll->rst, 0);
>  }
> +
> +return 0;
>  }
>
>  static int meson_clk_pll_is_enabled(struct clk_hw *hw)
> diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
> index 3acf03780221..76d31c0a3342 100644
> --- a/drivers/clk/meson/sclk-div.c
> +++ b/drivers/clk/meson/sclk-div.c
> @@ -216,7 +216,7 @@ static int sclk_div_is_enabled(struct clk_hw *hw)
>  return 0;
>  }
>
> -static void sclk_div_init(struct clk_hw *hw)
> +static int sclk_div_init(struct clk_hw *hw)
>  {
>  struct clk_regmap *clk = to_clk_regmap(hw);
>  struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk);
> @@ -231,6 +231,8 @@ static void sclk_div_init(struct clk_hw *hw)
>  sclk->cached_div = val + 1;
>
>  sclk_div_get_duty_cycle(hw, &sclk->cached_duty);
> +
> +return 0;
>  }
>
>  const struct clk_ops meson_sclk_div_ops = {
> diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c
> index 567755d6f844..1b4f023cdc8b 100644
> --- a/drivers/clk/microchip/clk-core.c
> +++ b/drivers/clk/microchip/clk-core.c
> @@ -266,10 +266,12 @@ static void roclk_disable(struct clk_hw *hw)
>  writel(REFO_ON | REFO_OE, PIC32_CLR(refo->ctrl_reg));
>  }
>
> -static void roclk_init(struct clk_hw *hw)
> +static int roclk_init(struct clk_hw *hw)
>  {
>  /* initialize clock in disabled state */
>  roclk_disable(hw);
> +
> +return 0;
>  }
>
>  static u8 roclk_get_parent(struct clk_hw *hw)
> @@ -880,7 +882,7 @@ static int sclk_set_parent(struct clk_hw *hw, u8 index)
>  return err;
>  }
>
> -static void sclk_init(struct clk_hw *hw)
> +static int sclk_init(struct clk_hw *hw)
>  {
>  struct pic32_sys_clk *sclk = clkhw_to_sys_clk(hw);
>  unsigned long flags;
> @@ -899,6 +901,8 @@ static void sclk_init(struct clk_hw *hw)
>  writel(v, sclk->slew_reg);
>  spin_unlock_irqrestore(&sclk->core->reg_lock, flags);
>  }
> +
> +return 0;
>  }
>
>  /* sclk with post-divider */
> diff --git a/drivers/clk/mmp/clk-frac.c b/drivers/clk/mmp/clk-frac.c
> index 90bf181f191a..fabc09aca6c4 100644
> --- a/drivers/clk/mmp/clk-frac.c
> +++ b/drivers/clk/mmp/clk-frac.c
> @@ -109,7 +109,7 @@ static int clk_factor_set_rate(struct clk_hw *hw,
> unsigned long drate,
>  return 0;
>  }
>
> -static void clk_factor_init(struct clk_hw *hw)
> +static int clk_factor_init(struct clk_hw *hw)
>  {
>  struct mmp_clk_factor *factor = to_clk_factor(hw);
>  struct mmp_clk_factor_masks *masks = factor->masks;
> @@ -146,6 +146,8 @@ static void clk_factor_init(struct clk_hw *hw)
>
>  if (factor->lock)
>  spin_unlock_irqrestore(factor->lock, flags);
> +
> +return 0;
>  }
>
>  static const struct clk_ops clk_factor_ops = {
> diff --git a/drivers/clk/mmp/clk-mix.c b/drivers/clk/mmp/clk-mix.c
> index 90814b2613c0..d2cd36c54474 100644
> --- a/drivers/clk/mmp/clk-mix.c
> +++ b/drivers/clk/mmp/clk-mix.c
> @@ -419,12 +419,14 @@ static int mmp_clk_set_rate(struct clk_hw *hw,
> unsigned long rate,
>  }
>  }
>
> -static void mmp_clk_mix_init(struct clk_hw *hw)
> +static int mmp_clk_mix_init(struct clk_hw *hw)
>  {
>  struct mmp_clk_mix *mix = to_clk_mix(hw);
>
>  if (mix->table)
>  _filter_clk_table(mix, mix->table, mix->table_size);
> +
> +return 0;
>  }
>
>  const struct clk_ops mmp_clk_mix_ops = {
> diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
> index 3c04805f2a55..e847d586a73a 100644
> --- a/drivers/clk/qcom/clk-hfpll.c
> +++ b/drivers/clk/qcom/clk-hfpll.c
> @@ -196,7 +196,7 @@ static unsigned long clk_hfpll_recalc_rate(struct clk_hw
> *hw,
>  return l_val * parent_rate;
>  }
>
> -static void clk_hfpll_init(struct clk_hw *hw)
> +static int clk_hfpll_init(struct clk_hw *hw)
>  {
>  struct clk_hfpll *h = to_clk_hfpll(hw);
>  struct hfpll_data const *hd = h->d;
> @@ -206,7 +206,7 @@ static void clk_hfpll_init(struct clk_hw *hw)
>  regmap_read(regmap, hd->mode_reg, &mode);
>  if (mode != (PLL_BYPASSNL | PLL_RESET_N | PLL_OUTCTRL)) {
>  __clk_hfpll_init_once(hw);
> -return;
> +return 0;
>  }
>
>  if (hd->status_reg) {
> @@ -218,6 +218,8 @@ static void clk_hfpll_init(struct clk_hw *hw)
>  __clk_hfpll_init_once(hw);
>  }
>  }
> +
> +return 0;
>  }
>
>  static int hfpll_is_enabled(struct clk_hw *hw)
> diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
> index 198417d56300..10560d963baf 100644
> --- a/drivers/clk/rockchip/clk-pll.c
> +++ b/drivers/clk/rockchip/clk-pll.c
> @@ -282,7 +282,7 @@ static int rockchip_rk3036_pll_is_enabled(struct clk_hw
> *hw)
>  return !(pllcon & RK3036_PLLCON1_PWRDOWN);
>  }
>
> -static void rockchip_rk3036_pll_init(struct clk_hw *hw)
> +static int rockchip_rk3036_pll_init(struct clk_hw *hw)
>  {
>  struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
>  const struct rockchip_pll_rate_table *rate;
> @@ -290,14 +290,14 @@ static void rockchip_rk3036_pll_init(struct clk_hw
> *hw)
>  unsigned long drate;
>
>  if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE))
> -return;
> +return 0;
>
>  drate = clk_hw_get_rate(hw);
>  rate = rockchip_get_pll_settings(pll, drate);
>
>  /* when no rate setting for the current rate, rely on clk_set_rate */
>  if (!rate)
> -return;
> +return 0;
>
>  rockchip_rk3036_pll_get_params(pll, &cur);
>
> @@ -319,13 +319,15 @@ static void rockchip_rk3036_pll_init(struct clk_hw
> *hw)
>  if (!parent) {
>  pr_warn("%s: parent of %s not available\n",
>  __func__, __clk_get_name(hw->clk));
> -return;
> +return 0;
>  }
>
>  pr_debug("%s: pll %s: rate params do not match rate table,
> adjusting\n",
>   __func__, __clk_get_name(hw->clk));
>  rockchip_rk3036_pll_set_params(pll, rate);
>  }
> +
> +return 0;
>  }
>
>  static const struct clk_ops rockchip_rk3036_pll_clk_norate_ops = {
> @@ -515,7 +517,7 @@ static int rockchip_rk3066_pll_is_enabled(struct clk_hw
> *hw)
>  return !(pllcon & RK3066_PLLCON3_PWRDOWN);
>  }
>
> -static void rockchip_rk3066_pll_init(struct clk_hw *hw)
> +static int rockchip_rk3066_pll_init(struct clk_hw *hw)
>  {
>  struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
>  const struct rockchip_pll_rate_table *rate;
> @@ -523,14 +525,14 @@ static void rockchip_rk3066_pll_init(struct clk_hw
> *hw)
>  unsigned long drate;
>
>  if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE))
> -return;
> +return 0;
>
>  drate = clk_hw_get_rate(hw);
>  rate = rockchip_get_pll_settings(pll, drate);
>
>  /* when no rate setting for the current rate, rely on clk_set_rate */
>  if (!rate)
> -return;
> +return 0;
>
>  rockchip_rk3066_pll_get_params(pll, &cur);
>
> @@ -543,6 +545,8 @@ static void rockchip_rk3066_pll_init(struct clk_hw *hw)
>   __func__, clk_hw_get_name(hw));
>  rockchip_rk3066_pll_set_params(pll, rate);
>  }
> +
> +return 0;
>  }
>
>  static const struct clk_ops rockchip_rk3066_pll_clk_norate_ops = {
> @@ -761,7 +765,7 @@ static int rockchip_rk3399_pll_is_enabled(struct clk_hw
> *hw)
>  return !(pllcon & RK3399_PLLCON3_PWRDOWN);
>  }
>
> -static void rockchip_rk3399_pll_init(struct clk_hw *hw)
> +static int rockchip_rk3399_pll_init(struct clk_hw *hw)
>  {
>  struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
>  const struct rockchip_pll_rate_table *rate;
> @@ -769,14 +773,14 @@ static void rockchip_rk3399_pll_init(struct clk_hw
> *hw)
>  unsigned long drate;
>
>  if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE))
> -return;
> +return 0;
>
>  drate = clk_hw_get_rate(hw);
>  rate = rockchip_get_pll_settings(pll, drate);
>
>  /* when no rate setting for the current rate, rely on clk_set_rate */
>  if (!rate)
> -return;
> +return 0;
>
>  rockchip_rk3399_pll_get_params(pll, &cur);
>
> @@ -798,13 +802,15 @@ static void rockchip_rk3399_pll_init(struct clk_hw
> *hw)
>  if (!parent) {
>  pr_warn("%s: parent of %s not available\n",
>  __func__, __clk_get_name(hw->clk));
> -return;
> +return 0;
>  }
>
>  pr_debug("%s: pll %s: rate params do not match rate table,
> adjusting\n",
>   __func__, __clk_get_name(hw->clk));
>  rockchip_rk3399_pll_set_params(pll, rate);
>  }
> +
> +return 0;
>  }
>
>  static const struct clk_ops rockchip_rk3399_pll_clk_norate_ops = {
> diff --git a/drivers/clk/ti/clock.h b/drivers/clk/ti/clock.h
> index e4b8392ff63c..adef9c16e43b 100644
> --- a/drivers/clk/ti/clock.h
> +++ b/drivers/clk/ti/clock.h
> @@ -252,7 +252,7 @@ extern const struct clk_ops omap_gate_clk_ops;
>
>  extern struct ti_clk_features ti_clk_features;
>
> -void omap2_init_clk_clkdm(struct clk_hw *hw);
> +int omap2_init_clk_clkdm(struct clk_hw *hw);
>  int omap2_clkops_enable_clkdm(struct clk_hw *hw);
>  void omap2_clkops_disable_clkdm(struct clk_hw *hw);
>
> diff --git a/drivers/clk/ti/clockdomain.c b/drivers/clk/ti/clockdomain.c
> index 423a99b9f10c..ee56306f79d5 100644
> --- a/drivers/clk/ti/clockdomain.c
> +++ b/drivers/clk/ti/clockdomain.c
> @@ -101,16 +101,16 @@ void omap2_clkops_disable_clkdm(struct clk_hw
> *hw)
>   *
>   * Convert a clockdomain name stored in a struct clk 'clk' into a
>   * clockdomain pointer, and save it into the struct clk.  Intended to be
> - * called during clk_register().  No return value.
> + * called during clk_register(). Returns 0 on success, -EERROR otherwise.
>   */
> -void omap2_init_clk_clkdm(struct clk_hw *hw)
> +int omap2_init_clk_clkdm(struct clk_hw *hw)
>  {
>  struct clk_hw_omap *clk = to_clk_hw_omap(hw);
>  struct clockdomain *clkdm;
>  const char *clk_name;
>
>  if (!clk->clkdm_name)
> -return;
> +return 0;
>
>  clk_name = __clk_get_name(hw->clk);
>
> @@ -123,6 +123,8 @@ void omap2_init_clk_clkdm(struct clk_hw *hw)
>  pr_debug("clock: could not associate clk %s to clkdm %s\n",
>   clk_name, clk->clkdm_name);
>  }
> +
> +return 0;
>  }

Where is it returning -EERROR as mentioned in the description?

>
>  static void __init of_ti_clockdomain_setup(struct device_node *node)
> diff --git a/drivers/net/phy/mdio-mux-meson-g12a.c b/drivers/net/phy/mdio-
> mux-meson-g12a.c
> index 7a9ad54582e1..bf86c9c7a288 100644
> --- a/drivers/net/phy/mdio-mux-meson-g12a.c
> +++ b/drivers/net/phy/mdio-mux-meson-g12a.c
> @@ -123,7 +123,7 @@ static int g12a_ephy_pll_is_enabled(struct clk_hw *hw)
>  return (val & PLL_CTL0_LOCK_DIG) ? 1 : 0;
>  }
>
> -static void g12a_ephy_pll_init(struct clk_hw *hw)
> +static int g12a_ephy_pll_init(struct clk_hw *hw)
>  {
>  struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw);
>
> @@ -136,6 +136,8 @@ static void g12a_ephy_pll_init(struct clk_hw *hw)
>  writel(0x20200000, pll->base + ETH_PLL_CTL5);
>  writel(0x0000c002, pll->base + ETH_PLL_CTL6);
>  writel(0x00000023, pll->base + ETH_PLL_CTL7);
> +
> +return 0;
>  }
>
>  static const struct clk_ops g12a_ephy_pll_ops = {
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 2fdfe8061363..b82ec4c4ca95 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -190,8 +190,12 @@ struct clk_duty {
>   *
>   * @init:Perform platform-specific initialization magic.
>   *This is not not used by any of the basic clock types.
> - *Please consider other ways of solving initialization problems
> - *before using this callback, as its use is discouraged.
> + *This callback exist for HW which needs to perform some
> + *initialisation magic for CCF to get an accurate view of the
> + *clock. It may also be used dynamic resource allocation is
> + *required. It shall not used to deal with clock parameters,
> + *such as rate or parents.
> + *Returns 0 on success, -EERROR otherwise.

Aren't all functions returning 0 always?

>   *
>   * @debug_init:Set up type-specific debugfs entries for this clock.  This
>   *is called once, after the debugfs directory entry for this
> @@ -243,7 +247,7 @@ struct clk_ops {
>    struct clk_duty *duty);
>  int(*set_duty_cycle)(struct clk_hw *hw,
>    struct clk_duty *duty);
> -void(*init)(struct clk_hw *hw);
> +int(*init)(struct clk_hw *hw);
>  void(*debug_init)(struct clk_hw *hw, struct dentry *dentry);
>  };
>
> --
> 2.21.0

________________________________
 This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof.
________________________________

RE: [PATCH 2/3] clk: let init callback return an error code

[Jerome Brunet]

On Tue 24 Sep 2019 at 13:38, Ankur Tyagi <Ankur.Tyagi@gallagher.com> wrote:

> Hi,
>
> I am no expert here but just looked at the patch and found few
> discrepancy that I have mentioned inline.
>

[...]

>
> Aren't all functions returning 0 always?
>

Yes, on purpose. This patch is an API conversion to let the init()
callback of the clock ops return an error code or 0.

The patch is not meant to change anything in the prior behavior of the
clock drivers which is why every exit path return 0 with this change.

IOW, yes there are all returning 0 for now, but it will eventually
change.


>>   *
>>   * @debug_init:Set up type-specific debugfs entries for this clock.  This
>>   *is called once, after the debugfs directory entry for this
>> @@ -243,7 +247,7 @@ struct clk_ops {
>>    struct clk_duty *duty);
>>  int(*set_duty_cycle)(struct clk_hw *hw,
>>    struct clk_duty *duty);
>> -void(*init)(struct clk_hw *hw);
>> +int(*init)(struct clk_hw *hw);
>>  void(*debug_init)(struct clk_hw *hw, struct dentry *dentry);
>>  };
>>
>> --
>> 2.21.0
>
> ________________________________
>  This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof.
> ________________________________

Re: [PATCH 2/3] clk: let init callback return an error code

[Andrew Lunn]

On Tue, Sep 24, 2019 at 02:39:53PM +0200, Jerome Brunet wrote:
> If the init callback is allowed to request resources, it needs a return
> value to report the outcome of such a request.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> 
>  Sorry about the spam.
>  This patch change quite a few files so I have tried to Cc the
>  relevant people. Stephen, You may notice that I have added a
>  couple of the network people. You need an Ack from one of them
>  since the Amlogic G12a mdio mux has a clock which uses the .init()
>  callback

>  static void __init of_ti_clockdomain_setup(struct device_node *node)
> diff --git a/drivers/net/phy/mdio-mux-meson-g12a.c b/drivers/net/phy/mdio-mux-meson-g12a.c
> index 7a9ad54582e1..bf86c9c7a288 100644
> --- a/drivers/net/phy/mdio-mux-meson-g12a.c
> +++ b/drivers/net/phy/mdio-mux-meson-g12a.c
> @@ -123,7 +123,7 @@ static int g12a_ephy_pll_is_enabled(struct clk_hw *hw)
>  	return (val & PLL_CTL0_LOCK_DIG) ? 1 : 0;
>  }
>  
> -static void g12a_ephy_pll_init(struct clk_hw *hw)
> +static int g12a_ephy_pll_init(struct clk_hw *hw)
>  {
>  	struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw);
>  
> @@ -136,6 +136,8 @@ static void g12a_ephy_pll_init(struct clk_hw *hw)
>  	writel(0x20200000, pll->base + ETH_PLL_CTL5);
>  	writel(0x0000c002, pll->base + ETH_PLL_CTL6);
>  	writel(0x00000023, pll->base + ETH_PLL_CTL7);
> +
> +	return 0;
>  }

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

It should be safe to merge this via the clk tree. You would probably
know about an possible merge conflicts, since you wrote this driver!

    Andrew

Re: [PATCH 2/3] clk: let init callback return an error code

[Heiko Stuebner]

Am Dienstag, 24. September 2019, 14:39:53 CEST schrieb Jerome Brunet:
> If the init callback is allowed to request resources, it needs a return
> value to report the outcome of such a request.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
[...]
>  drivers/clk/rockchip/clk-pll.c        | 28 ++++++++++++++++-----------

for the Rockchip part
Acked-by: Heiko Stuebner <heiko@sntech.de>

Re: [PATCH 0/3] clk: let clock perform allocation in init

[Jerome Brunet]

On Tue 24 Sep 2019 at 14:39, Jerome Brunet <jbrunet@baylibre.com> wrote:

> This patchset is a follow up on this pinky swear [0].
> Its purpose is:
>  * Clarify the acceptable use of clk_ops init() callback
>  * Let the init() callback return an error code in case anything
>    fail.
>  * Add the terminate() counter part of of init() to release the
>    resources which may have been claimed in init()
>
> After discussing with Stephen at LPC, I decided to drop the 2 last patches
> of the RFC [1]. I can live without it for now and nobody expressed a
> critical need to get the proposed placeholder.
>
> [0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com
> [1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com
>
> Jerome Brunet (3):
>   clk: actually call the clock init before any other callback of the
>     clock
>   clk: let init callback return an error code
>   clk: add terminate callback to clk_ops
>
>  drivers/clk/clk.c                     | 38 ++++++++++++++++++---------
>  drivers/clk/meson/clk-mpll.c          |  4 ++-
>  drivers/clk/meson/clk-phase.c         |  4 ++-
>  drivers/clk/meson/clk-pll.c           |  4 ++-
>  drivers/clk/meson/sclk-div.c          |  4 ++-
>  drivers/clk/microchip/clk-core.c      |  8 ++++--
>  drivers/clk/mmp/clk-frac.c            |  4 ++-
>  drivers/clk/mmp/clk-mix.c             |  4 ++-
>  drivers/clk/qcom/clk-hfpll.c          |  6 +++--
>  drivers/clk/rockchip/clk-pll.c        | 28 ++++++++++++--------
>  drivers/clk/ti/clock.h                |  2 +-
>  drivers/clk/ti/clockdomain.c          |  8 +++---
>  drivers/net/phy/mdio-mux-meson-g12a.c |  4 ++-
>  include/linux/clk-provider.h          | 13 ++++++---
>  14 files changed, 90 insertions(+), 41 deletions(-)

Hi Stephen,

Is this series Ok with you ?
Do you think you can take it at the beginning of the next cycle ?

Thx
Jerome

Re: [PATCH 0/3] clk: let clock perform allocation in init

[Jerome Brunet]

On Tue 24 Sep 2019 at 14:39, Jerome Brunet <jbrunet@baylibre.com> wrote:

> This patchset is a follow up on this pinky swear [0].
> Its purpose is:
>  * Clarify the acceptable use of clk_ops init() callback
>  * Let the init() callback return an error code in case anything
>    fail.
>  * Add the terminate() counter part of of init() to release the
>    resources which may have been claimed in init()
>
> After discussing with Stephen at LPC, I decided to drop the 2 last patches
> of the RFC [1]. I can live without it for now and nobody expressed a
> critical need to get the proposed placeholder.
>
> [0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com
> [1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com
>

Hi Stephen,

Do you think we can fit this into the incoming cycle ?

Cheers
Jerome

> Jerome Brunet (3):
>   clk: actually call the clock init before any other callback of the
>     clock
>   clk: let init callback return an error code
>   clk: add terminate callback to clk_ops
>
>  drivers/clk/clk.c                     | 38 ++++++++++++++++++---------
>  drivers/clk/meson/clk-mpll.c          |  4 ++-
>  drivers/clk/meson/clk-phase.c         |  4 ++-
>  drivers/clk/meson/clk-pll.c           |  4 ++-
>  drivers/clk/meson/sclk-div.c          |  4 ++-
>  drivers/clk/microchip/clk-core.c      |  8 ++++--
>  drivers/clk/mmp/clk-frac.c            |  4 ++-
>  drivers/clk/mmp/clk-mix.c             |  4 ++-
>  drivers/clk/qcom/clk-hfpll.c          |  6 +++--
>  drivers/clk/rockchip/clk-pll.c        | 28 ++++++++++++--------
>  drivers/clk/ti/clock.h                |  2 +-
>  drivers/clk/ti/clockdomain.c          |  8 +++---
>  drivers/net/phy/mdio-mux-meson-g12a.c |  4 ++-
>  include/linux/clk-provider.h          | 13 ++++++---
>  14 files changed, 90 insertions(+), 41 deletions(-)

Re: [PATCH 0/3] clk: let clock perform allocation in init

[Stephen Boyd]

Quoting Jerome Brunet (2019-11-29 07:36:28)
> 
> On Tue 24 Sep 2019 at 14:39, Jerome Brunet <jbrunet@baylibre.com> wrote:
> 
> > This patchset is a follow up on this pinky swear [0].
> > Its purpose is:
> >  * Clarify the acceptable use of clk_ops init() callback
> >  * Let the init() callback return an error code in case anything
> >    fail.
> >  * Add the terminate() counter part of of init() to release the
> >    resources which may have been claimed in init()
> >
> > After discussing with Stephen at LPC, I decided to drop the 2 last patches
> > of the RFC [1]. I can live without it for now and nobody expressed a
> > critical need to get the proposed placeholder.
> >
> > [0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com
> > [1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com
> >
> 
> Hi Stephen,
> 
> Do you think we can fit this into the incoming cycle ?
> 

Sorry I missed this one. I'll apply it soon but won't be for this merge
window.

Re: [PATCH 0/3] clk: let clock perform allocation in init

[Jerome Brunet]

On Tue 03 Dec 2019 at 10:05, Stephen Boyd <sboyd@kernel.org> wrote:

> Quoting Jerome Brunet (2019-11-29 07:36:28)
>> 
>> On Tue 24 Sep 2019 at 14:39, Jerome Brunet <jbrunet@baylibre.com> wrote:
>> 
>> > This patchset is a follow up on this pinky swear [0].
>> > Its purpose is:
>> >  * Clarify the acceptable use of clk_ops init() callback
>> >  * Let the init() callback return an error code in case anything
>> >    fail.
>> >  * Add the terminate() counter part of of init() to release the
>> >    resources which may have been claimed in init()
>> >
>> > After discussing with Stephen at LPC, I decided to drop the 2 last patches
>> > of the RFC [1]. I can live without it for now and nobody expressed a
>> > critical need to get the proposed placeholder.
>> >
>> > [0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com
>> > [1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com
>> >
>> 
>> Hi Stephen,
>> 
>> Do you think we can fit this into the incoming cycle ?
>> 
>
> Sorry I missed this one. I'll apply it soon but won't be for this merge
> window.

No worries, I was referring to the v5.6 cycle, not the v5.5 merge
window. 

Re: [PATCH 1/3] clk: actually call the clock init before any other callback of the clock

[Stephen Boyd]

Quoting Jerome Brunet (2019-09-24 05:39:52)
>  __clk_init_parent() will call the .get_parent() callback of the clock
>  so .init() must run before.
> 
> Fixes: 541debae0adf ("clk: call the clock init() callback before any other ops callback")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---

Applied to clk-next

Re: [PATCH 3/3] clk: add terminate callback to clk_ops

[Stephen Boyd]

Quoting Jerome Brunet (2019-09-24 05:39:54)
> Add a terminate callback to the clk_ops to release the resources
> claimed in .init()
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---

Applied to clk-next