[PATCH 0/5] ARM: OMAP3+: DPLL: fix set_rate logic

[PATCH 0/5] ARM: OMAP3+: DPLL: fix set_rate logic

[Tero Kristo]

Hi,

OMAP3+ DPLL code is currently using set_rate op to change DPLL rates.
This is kind of wrong, as it also involves changing DPLL parent in certain
cases (switch between locked mode <-> bypass mode.) This set fixes these
issues by introducing the support of determine_rate and set_rate_and_parent
ops for the DPLL clocks. Also introduces support for set_parent, which
just switches a DPLL between locked <-> bypass modes.

Testing branch pushed at my tree: https://github.com/t-kristo/linux-pm.git
branch: 3.17-rc1-dpll-fixes

Testing done:
- omap2430-sdp : boot
- am335x-bone : boot
- am43xx-gpevm : boot
- dra7-evm : boot
- omap5-uevm : boot
- omap4-panda-es : boot
- omap3-beagle : boot
- omap3-beagle-xm : boot

-Tero

[PATCH 1/5] ARM: OMAP3: clock: use clk_features flags for omap3 DPLL4 checks

[Tero Kristo]

DPLL4 can't be reprogrammed on OMAP3430 ES1.0 due to hardware limitation.
Currently, the code does runtime omap_rev() check to see the chip it is
being executed on, instead, change this to use clk_features flags.
This avoids need for runtime omap_rev() checks.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/clock.c     |    4 ++++
 arch/arm/mach-omap2/clock.h     |    1 +
 arch/arm/mach-omap2/clock3xxx.c |    2 +-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 500530d..c2b2398 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -771,4 +771,8 @@ void __init ti_clk_init_features(void)
 		ti_clk_features.cm_idlest_val = OMAP24XX_CM_IDLEST_VAL;
 	else if (cpu_is_omap34xx())
 		ti_clk_features.cm_idlest_val = OMAP34XX_CM_IDLEST_VAL;
+
+	/* On OMAP3430 ES1.0, DPLL4 can't be re-programmed */
+	if (omap_rev() == OMAP3430_REV_ES1_0)
+		ti_clk_features.flags |= TI_CLK_DPLL4_DENY_REPROGRAM;
 }
diff --git a/arch/arm/mach-omap2/clock.h b/arch/arm/mach-omap2/clock.h
index 4592a27..641337c 100644
--- a/arch/arm/mach-omap2/clock.h
+++ b/arch/arm/mach-omap2/clock.h
@@ -234,6 +234,7 @@ struct ti_clk_features {
 };
 
 #define TI_CLK_DPLL_HAS_FREQSEL		(1 << 0)
+#define TI_CLK_DPLL4_DENY_REPROGRAM	(1 << 1)
 
 extern struct ti_clk_features ti_clk_features;
 
diff --git a/arch/arm/mach-omap2/clock3xxx.c b/arch/arm/mach-omap2/clock3xxx.c
index 0b02b41..9a25601 100644
--- a/arch/arm/mach-omap2/clock3xxx.c
+++ b/arch/arm/mach-omap2/clock3xxx.c
@@ -46,7 +46,7 @@ int omap3_dpll4_set_rate(struct clk_hw *hw, unsigned long rate,
 	 * on 3430ES1 prevents us from changing DPLL multipliers or dividers
 	 * on DPLL4.
 	 */
-	if (omap_rev() == OMAP3430_REV_ES1_0) {
+	if (ti_clk_features.flags & TI_CLK_DPLL4_DENY_REPROGRAM) {
 		pr_err("clock: DPLL4 cannot change rate due to silicon 'Limitation 2.5' on 3430ES1.\n");
 		return -EINVAL;
 	}
-- 
1.7.9.5

[PATCH 2/5] ARM: OMAP3: clock: add new rate changing logic support for noncore DPLLs

[Tero Kristo]

Currently, DPLL code hides the re-parenting within its internals, which
is wrong. This needs to be exposed to the common clock code via
determine_rate and set_rate_and_parent APIs. This patch adds support
for these, which will be taken into use in the following patches.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/dpll3xxx.c |  147 ++++++++++++++++++++++++++++++++++++++++
 include/linux/clk/ti.h         |    9 +++
 2 files changed, 156 insertions(+)

diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c
index ac3d789..cfe7c30 100644
--- a/arch/arm/mach-omap2/dpll3xxx.c
+++ b/arch/arm/mach-omap2/dpll3xxx.c
@@ -546,6 +546,153 @@ int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
 	return 0;
 }
 
+/**
+ * omap3_noncore_dpll_determine_rate - determine rate for a DPLL
+ * @hw: pointer to the clock to determine rate for
+ * @rate: target rate for the DPLL
+ * @best_parent_rate: pointer for returning best parent rate
+ * @best_parent_clk: pointer for returning best parent clock
+ *
+ * Determines which DPLL mode to use for reaching a desired target rate.
+ * Checks whether the DPLL shall be in bypass or locked mode, and if
+ * locked, calculates the M,N values for the DPLL via round-rate.
+ * Returns a positive clock rate with success, negative error value
+ * in failure.
+ */
+long omap3_noncore_dpll_determine_rate(struct clk_hw *hw, unsigned long rate,
+				       unsigned long *best_parent_rate,
+				       struct clk **best_parent_clk)
+{
+	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
+	struct dpll_data *dd;
+
+	if (!hw || !rate)
+		return -EINVAL;
+
+	dd = clk->dpll_data;
+	if (!dd)
+		return -EINVAL;
+
+	if (__clk_get_rate(dd->clk_bypass) == rate &&
+	    (dd->modes & (1 << DPLL_LOW_POWER_BYPASS))) {
+		*best_parent_clk = dd->clk_bypass;
+	} else {
+		rate = omap2_dpll_round_rate(hw, rate, best_parent_rate);
+		*best_parent_clk = dd->clk_ref;
+	}
+
+	*best_parent_rate = rate;
+
+	return rate;
+}
+
+/**
+ * omap3_noncore_dpll_set_parent - set parent for a DPLL clock
+ * @hw: pointer to the clock to set parent for
+ * @index: parent index to select
+ *
+ * Sets parent for a DPLL clock. This sets the DPLL into bypass or
+ * locked mode. Returns 0 with success, negative error value otherwise.
+ */
+int omap3_noncore_dpll_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
+	int ret;
+
+	if (!hw)
+		return -EINVAL;
+
+	if (index)
+		ret = _omap3_noncore_dpll_bypass(clk);
+	else
+		ret = _omap3_noncore_dpll_lock(clk);
+
+	return ret;
+}
+
+/**
+ * omap3_noncore_dpll_set_rate_new - set rate for a DPLL clock
+ * @hw: pointer to the clock to set parent for
+ * @rate: target rate for the clock
+ * @parent_rate: rate of the parent clock
+ *
+ * Sets rate for a DPLL clock. First checks if the clock parent is
+ * reference clock (in bypass mode, the rate of the clock can't be
+ * changed) and proceeds with the rate change operation. Returns 0
+ * with success, negative error value otherwise.
+ */
+static int omap3_noncore_dpll_set_rate_new(struct clk_hw *hw,
+					   unsigned long rate,
+					   unsigned long parent_rate)
+{
+	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
+	struct dpll_data *dd;
+	u16 freqsel = 0;
+	int ret;
+
+	if (!hw || !rate)
+		return -EINVAL;
+
+	dd = clk->dpll_data;
+	if (!dd)
+		return -EINVAL;
+
+	if (__clk_get_parent(hw->clk) != dd->clk_ref)
+		return -EINVAL;
+
+	if (dd->last_rounded_rate == 0)
+		return -EINVAL;
+
+	/* Freqsel is available only on OMAP343X devices */
+	if (ti_clk_features.flags & TI_CLK_DPLL_HAS_FREQSEL) {
+		freqsel = _omap3_dpll_compute_freqsel(clk, dd->last_rounded_n);
+		WARN_ON(!freqsel);
+	}
+
+	pr_debug("%s: %s: set rate: locking rate to %lu.\n", __func__,
+		 __clk_get_name(hw->clk), rate);
+
+	ret = omap3_noncore_dpll_program(clk, freqsel);
+
+	return ret;
+}
+
+/**
+ * omap3_noncore_dpll_set_rate_and_parent - set rate and parent for a DPLL clock
+ * @hw: pointer to the clock to set rate and parent for
+ * @rate: target rate for the DPLL
+ * @parent_rate: clock rate of the DPLL parent
+ * @index: new parent index for the DPLL, 0 - reference, 1 - bypass
+ *
+ * Sets rate and parent for a DPLL clock. If new parent is the bypass
+ * clock, only selects the parent. Otherwise proceeds with a rate
+ * change, as this will effectively also change the parent as the
+ * DPLL is put into locked mode. Returns 0 with success, negative error
+ * value otherwise.
+ */
+int omap3_noncore_dpll_set_rate_and_parent(struct clk_hw *hw,
+					   unsigned long rate,
+					   unsigned long parent_rate,
+					   u8 index)
+{
+	int ret;
+
+	if (!hw || !rate)
+		return -EINVAL;
+
+	/*
+	 * clk-ref at index[0], in which case we only need to set rate,
+	 * the parent will be changed automatically with the lock sequence.
+	 * With clk-bypass case we only need to change parent.
+	 */
+	if (index)
+		ret = omap3_noncore_dpll_set_parent(hw, index);
+	else
+		ret = omap3_noncore_dpll_set_rate_new(hw, rate, parent_rate);
+
+	return ret;
+}
+
 /* DPLL autoidle read/set code */
 
 /**
diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
index e8d8a35..47d0c63 100644
--- a/include/linux/clk/ti.h
+++ b/include/linux/clk/ti.h
@@ -254,8 +254,17 @@ extern const struct clk_ops ti_clk_mux_ops;
 void omap2_init_clk_hw_omap_clocks(struct clk *clk);
 int omap3_noncore_dpll_enable(struct clk_hw *hw);
 void omap3_noncore_dpll_disable(struct clk_hw *hw);
+int omap3_noncore_dpll_set_parent(struct clk_hw *hw, u8 index);
 int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
 				unsigned long parent_rate);
+int omap3_noncore_dpll_set_rate_and_parent(struct clk_hw *hw,
+					   unsigned long rate,
+					   unsigned long parent_rate,
+					   u8 index);
+long omap3_noncore_dpll_determine_rate(struct clk_hw *hw,
+				       unsigned long rate,
+				       unsigned long *best_parent_rate,
+				       struct clk **best_parent_clk);
 unsigned long omap4_dpll_regm4xen_recalc(struct clk_hw *hw,
 					 unsigned long parent_rate);
 long omap4_dpll_regm4xen_round_rate(struct clk_hw *hw,
-- 
1.7.9.5

[PATCH 3/5] ARM: OMAP4: clock: add support for determine_rate for omap4 regm4xen DPLL

[Tero Kristo]

Similarly to OMAP3 noncore DPLL, the implementation of this DPLL clock
type is wrong. This patch adds basic functionality for determine_rate
for this clock type which will be taken into use in the patches following
later.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/dpll44xx.c |   41 ++++++++++++++++++++++++++++++++++++++++
 include/linux/clk/ti.h         |    4 ++++
 2 files changed, 45 insertions(+)

diff --git a/arch/arm/mach-omap2/dpll44xx.c b/arch/arm/mach-omap2/dpll44xx.c
index 4613f1e..535822f 100644
--- a/arch/arm/mach-omap2/dpll44xx.c
+++ b/arch/arm/mach-omap2/dpll44xx.c
@@ -207,3 +207,44 @@ out:
 
 	return dd->last_rounded_rate;
 }
+
+/**
+ * omap4_dpll_regm4xen_determine_rate - determine rate for a DPLL
+ * @hw: pointer to the clock to determine rate for
+ * @rate: target rate for the DPLL
+ * @best_parent_rate: pointer for returning best parent rate
+ * @best_parent_clk: pointer for returning best parent clock
+ *
+ * Determines which DPLL mode to use for reaching a desired rate.
+ * Checks whether the DPLL shall be in bypass or locked mode, and if
+ * locked, calculates the M,N values for the DPLL via round-rate.
+ * Returns a positive clock rate with success, negative error value
+ * in failure.
+ */
+long omap4_dpll_regm4xen_determine_rate(struct clk_hw *hw, unsigned long rate,
+					unsigned long *best_parent_rate,
+					struct clk **best_parent_clk)
+{
+	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
+	struct dpll_data *dd;
+
+	if (!hw || !rate)
+		return -EINVAL;
+
+	dd = clk->dpll_data;
+	if (!dd)
+		return -EINVAL;
+
+	if (__clk_get_rate(dd->clk_bypass) == rate &&
+	    (dd->modes & (1 << DPLL_LOW_POWER_BYPASS))) {
+		*best_parent_clk = dd->clk_bypass;
+	} else {
+		rate = omap4_dpll_regm4xen_round_rate(hw, rate,
+						      best_parent_rate);
+		*best_parent_clk = dd->clk_ref;
+	}
+
+	*best_parent_rate = rate;
+
+	return rate;
+}
diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
index 47d0c63..541dc33 100644
--- a/include/linux/clk/ti.h
+++ b/include/linux/clk/ti.h
@@ -270,6 +270,10 @@ unsigned long omap4_dpll_regm4xen_recalc(struct clk_hw *hw,
 long omap4_dpll_regm4xen_round_rate(struct clk_hw *hw,
 				    unsigned long target_rate,
 				    unsigned long *parent_rate);
+long omap4_dpll_regm4xen_determine_rate(struct clk_hw *hw,
+					unsigned long rate,
+					unsigned long *best_parent_rate,
+					struct clk **best_parent_clk);
 u8 omap2_init_dpll_parent(struct clk_hw *hw);
 unsigned long omap3_dpll_recalc(struct clk_hw *hw, unsigned long parent_rate);
 long omap2_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate,
-- 
1.7.9.5

[PATCH 4/5] ARM: OMAP3: clock: add support for dpll4_set_rate_and_parent

[Tero Kristo]

Expand the support of omap4 per-dpll to provide set_rate_and_parent.
This is required for proper behavior of clk_change_rate with
determine_rate support.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/clock3xxx.c |   36 ++++++++++++++++++++++++++++++++++++
 include/linux/clk/ti.h          |    2 ++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm/mach-omap2/clock3xxx.c b/arch/arm/mach-omap2/clock3xxx.c
index 9a25601..a9e86db 100644
--- a/arch/arm/mach-omap2/clock3xxx.c
+++ b/arch/arm/mach-omap2/clock3xxx.c
@@ -38,6 +38,18 @@
 
 /* needed by omap3_core_dpll_m2_set_rate() */
 struct clk *sdrc_ick_p, *arm_fck_p;
+
+/**
+ * omap3_dpll4_set_rate - set rate for omap3 per-dpll
+ * @hw: clock to change
+ * @rate: target rate for clock
+ * @parent_rate: rate of the parent clock
+ *
+ * Check if the current SoC supports the per-dpll reprogram operation
+ * or not, and then do the rate change if supported. Returns -EINVAL
+ * if not supported, 0 for success, and potential error codes from the
+ * clock rate change.
+ */
 int omap3_dpll4_set_rate(struct clk_hw *hw, unsigned long rate,
 				unsigned long parent_rate)
 {
@@ -54,6 +66,30 @@ int omap3_dpll4_set_rate(struct clk_hw *hw, unsigned long rate,
 	return omap3_noncore_dpll_set_rate(hw, rate, parent_rate);
 }
 
+/**
+ * omap3_dpll4_set_rate_and_parent - set rate and parent for omap3 per-dpll
+ * @hw: clock to change
+ * @rate: target rate for clock
+ * @parent_rate: rate of the parent clock
+ * @index: parent index, 0 - reference clock, 1 - bypass clock
+ *
+ * Check if the current SoC support the per-dpll reprogram operation
+ * or not, and then do the rate + parent change if supported. Returns
+ * -EINVAL if not supported, 0 for success, and potential error codes
+ * from the clock rate change.
+ */
+int omap3_dpll4_set_rate_and_parent(struct clk_hw *hw, unsigned long rate,
+				    unsigned long parent_rate, u8 index)
+{
+	if (ti_clk_features.flags & TI_CLK_DPLL4_DENY_REPROGRAM) {
+		pr_err("clock: DPLL4 cannot change rate due to silicon 'Limitation 2.5' on 3430ES1.\n");
+		return -EINVAL;
+	}
+
+	return omap3_noncore_dpll_set_rate_and_parent(hw, rate, parent_rate,
+						      index);
+}
+
 void __init omap3_clk_lock_dpll5(void)
 {
 	struct clk *dpll5_clk;
diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
index 541dc33..57242ba 100644
--- a/include/linux/clk/ti.h
+++ b/include/linux/clk/ti.h
@@ -291,6 +291,8 @@ int omap2_clk_disable_autoidle_all(void);
 void omap2_clk_enable_init_clocks(const char **clk_names, u8 num_clocks);
 int omap3_dpll4_set_rate(struct clk_hw *clk, unsigned long rate,
 			 unsigned long parent_rate);
+int omap3_dpll4_set_rate_and_parent(struct clk_hw *hw, unsigned long rate,
+				    unsigned long parent_rate, u8 index);
 int omap2_dflt_clk_enable(struct clk_hw *hw);
 void omap2_dflt_clk_disable(struct clk_hw *hw);
 int omap2_dflt_clk_is_enabled(struct clk_hw *hw);
-- 
1.7.9.5

[PATCH 5/5] ARM: OMAP3+: DPLL: use determine_rate() and set_rate_and_parent()

[Tero Kristo]

Currently, DPLLs are hiding the gory details of switching parent
within set_rate, which confuses the common clock code and is wrong.
Fixed by applying the new determine_rate() and set_rate_and_parent()
functionality to any clock-ops previously using the broken approach.
This patch also removes the broken legacy code.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/cclock3xxx_data.c |    6 +++
 arch/arm/mach-omap2/dpll3xxx.c        |   96 ++-------------------------------
 drivers/clk/ti/dpll.c                 |   15 ++++++
 3 files changed, 25 insertions(+), 92 deletions(-)

diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c
index eb8c75e..5c5ebb4 100644
--- a/arch/arm/mach-omap2/cclock3xxx_data.c
+++ b/arch/arm/mach-omap2/cclock3xxx_data.c
@@ -257,6 +257,9 @@ static const struct clk_ops dpll1_ck_ops = {
 	.get_parent	= &omap2_init_dpll_parent,
 	.recalc_rate	= &omap3_dpll_recalc,
 	.set_rate	= &omap3_noncore_dpll_set_rate,
+	.set_parent	= &omap3_noncore_dpll_set_parent,
+	.set_rate_and_parent	= &omap3_noncore_dpll_set_rate_and_parent,
+	.determine_rate	= &omap3_noncore_dpll_determine_rate,
 	.round_rate	= &omap2_dpll_round_rate,
 };
 
@@ -367,6 +370,9 @@ static const struct clk_ops dpll4_ck_ops = {
 	.get_parent	= &omap2_init_dpll_parent,
 	.recalc_rate	= &omap3_dpll_recalc,
 	.set_rate	= &omap3_dpll4_set_rate,
+	.set_parent	= &omap3_noncore_dpll_set_parent,
+	.set_rate_and_parent	= &omap3_dpll4_set_rate_and_parent,
+	.determine_rate	= &omap3_noncore_dpll_determine_rate,
 	.round_rate	= &omap2_dpll_round_rate,
 };
 
diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c
index cfe7c30..20e120d 100644
--- a/arch/arm/mach-omap2/dpll3xxx.c
+++ b/arch/arm/mach-omap2/dpll3xxx.c
@@ -460,93 +460,6 @@ void omap3_noncore_dpll_disable(struct clk_hw *hw)
 /* Non-CORE DPLL rate set code */
 
 /**
- * omap3_noncore_dpll_set_rate - set non-core DPLL rate
- * @clk: struct clk * of DPLL to set
- * @rate: rounded target rate
- *
- * Set the DPLL CLKOUT to the target rate.  If the DPLL can enter
- * low-power bypass, and the target rate is the bypass source clock
- * rate, then configure the DPLL for bypass.  Otherwise, round the
- * target rate if it hasn't been done already, then program and lock
- * the DPLL.  Returns -EINVAL upon error, or 0 upon success.
- */
-int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
-					unsigned long parent_rate)
-{
-	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
-	struct clk *new_parent = NULL;
-	unsigned long rrate;
-	u16 freqsel = 0;
-	struct dpll_data *dd;
-	int ret;
-
-	if (!hw || !rate)
-		return -EINVAL;
-
-	dd = clk->dpll_data;
-	if (!dd)
-		return -EINVAL;
-
-	if (__clk_get_rate(dd->clk_bypass) == rate &&
-	    (dd->modes & (1 << DPLL_LOW_POWER_BYPASS))) {
-		pr_debug("%s: %s: set rate: entering bypass.\n",
-			 __func__, __clk_get_name(hw->clk));
-
-		__clk_prepare(dd->clk_bypass);
-		clk_enable(dd->clk_bypass);
-		ret = _omap3_noncore_dpll_bypass(clk);
-		if (!ret)
-			new_parent = dd->clk_bypass;
-		clk_disable(dd->clk_bypass);
-		__clk_unprepare(dd->clk_bypass);
-	} else {
-		__clk_prepare(dd->clk_ref);
-		clk_enable(dd->clk_ref);
-
-		/* XXX this check is probably pointless in the CCF context */
-		if (dd->last_rounded_rate != rate) {
-			rrate = __clk_round_rate(hw->clk, rate);
-			if (rrate != rate) {
-				pr_warn("%s: %s: final rate %lu does not match desired rate %lu\n",
-					__func__, __clk_get_name(hw->clk),
-					rrate, rate);
-				rate = rrate;
-			}
-		}
-
-		if (dd->last_rounded_rate == 0)
-			return -EINVAL;
-
-		/* Freqsel is available only on OMAP343X devices */
-		if (ti_clk_features.flags & TI_CLK_DPLL_HAS_FREQSEL) {
-			freqsel = _omap3_dpll_compute_freqsel(clk,
-						dd->last_rounded_n);
-			WARN_ON(!freqsel);
-		}
-
-		pr_debug("%s: %s: set rate: locking rate to %lu.\n",
-			 __func__, __clk_get_name(hw->clk), rate);
-
-		ret = omap3_noncore_dpll_program(clk, freqsel);
-		if (!ret)
-			new_parent = dd->clk_ref;
-		clk_disable(dd->clk_ref);
-		__clk_unprepare(dd->clk_ref);
-	}
-	/*
-	* FIXME - this is all wrong.  common code handles reparenting and
-	* migrating prepare/enable counts.  dplls should be a multiplexer
-	* clock and this should be a set_parent operation so that all of that
-	* stuff is inherited for free
-	*/
-
-	if (!ret && clk_get_parent(hw->clk) != new_parent)
-		__clk_reparent(hw->clk, new_parent);
-
-	return 0;
-}
-
-/**
  * omap3_noncore_dpll_determine_rate - determine rate for a DPLL
  * @hw: pointer to the clock to determine rate for
  * @rate: target rate for the DPLL
@@ -611,7 +524,7 @@ int omap3_noncore_dpll_set_parent(struct clk_hw *hw, u8 index)
 }
 
 /**
- * omap3_noncore_dpll_set_rate_new - set rate for a DPLL clock
+ * omap3_noncore_dpll_set_rate - set rate for a DPLL clock
  * @hw: pointer to the clock to set parent for
  * @rate: target rate for the clock
  * @parent_rate: rate of the parent clock
@@ -621,9 +534,8 @@ int omap3_noncore_dpll_set_parent(struct clk_hw *hw, u8 index)
  * changed) and proceeds with the rate change operation. Returns 0
  * with success, negative error value otherwise.
  */
-static int omap3_noncore_dpll_set_rate_new(struct clk_hw *hw,
-					   unsigned long rate,
-					   unsigned long parent_rate)
+int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_rate)
 {
 	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
 	struct dpll_data *dd;
@@ -688,7 +600,7 @@ int omap3_noncore_dpll_set_rate_and_parent(struct clk_hw *hw,
 	if (index)
 		ret = omap3_noncore_dpll_set_parent(hw, index);
 	else
-		ret = omap3_noncore_dpll_set_rate_new(hw, rate, parent_rate);
+		ret = omap3_noncore_dpll_set_rate(hw, rate, parent_rate);
 
 	return ret;
 }
diff --git a/drivers/clk/ti/dpll.c b/drivers/clk/ti/dpll.c
index 79791e1..85ac0dd 100644
--- a/drivers/clk/ti/dpll.c
+++ b/drivers/clk/ti/dpll.c
@@ -33,6 +33,9 @@ static const struct clk_ops dpll_m4xen_ck_ops = {
 	.recalc_rate	= &omap4_dpll_regm4xen_recalc,
 	.round_rate	= &omap4_dpll_regm4xen_round_rate,
 	.set_rate	= &omap3_noncore_dpll_set_rate,
+	.set_parent	= &omap3_noncore_dpll_set_parent,
+	.set_rate_and_parent	= &omap3_noncore_dpll_set_rate_and_parent,
+	.determine_rate	= &omap4_dpll_regm4xen_determine_rate,
 	.get_parent	= &omap2_init_dpll_parent,
 };
 #else
@@ -53,6 +56,9 @@ static const struct clk_ops dpll_ck_ops = {
 	.recalc_rate	= &omap3_dpll_recalc,
 	.round_rate	= &omap2_dpll_round_rate,
 	.set_rate	= &omap3_noncore_dpll_set_rate,
+	.set_parent	= &omap3_noncore_dpll_set_parent,
+	.set_rate_and_parent	= &omap3_noncore_dpll_set_rate_and_parent,
+	.determine_rate	= &omap3_noncore_dpll_determine_rate,
 	.get_parent	= &omap2_init_dpll_parent,
 };
 
@@ -61,6 +67,9 @@ static const struct clk_ops dpll_no_gate_ck_ops = {
 	.get_parent	= &omap2_init_dpll_parent,
 	.round_rate	= &omap2_dpll_round_rate,
 	.set_rate	= &omap3_noncore_dpll_set_rate,
+	.set_parent	= &omap3_noncore_dpll_set_parent,
+	.set_rate_and_parent	= &omap3_noncore_dpll_set_rate_and_parent,
+	.determine_rate	= &omap3_noncore_dpll_determine_rate,
 };
 #else
 static const struct clk_ops dpll_core_ck_ops = {};
@@ -97,6 +106,9 @@ static const struct clk_ops omap3_dpll_ck_ops = {
 	.get_parent	= &omap2_init_dpll_parent,
 	.recalc_rate	= &omap3_dpll_recalc,
 	.set_rate	= &omap3_noncore_dpll_set_rate,
+	.set_parent	= &omap3_noncore_dpll_set_parent,
+	.set_rate_and_parent	= &omap3_noncore_dpll_set_rate_and_parent,
+	.determine_rate	= &omap3_noncore_dpll_determine_rate,
 	.round_rate	= &omap2_dpll_round_rate,
 };
 
@@ -106,6 +118,9 @@ static const struct clk_ops omap3_dpll_per_ck_ops = {
 	.get_parent	= &omap2_init_dpll_parent,
 	.recalc_rate	= &omap3_dpll_recalc,
 	.set_rate	= &omap3_dpll4_set_rate,
+	.set_parent	= &omap3_noncore_dpll_set_parent,
+	.set_rate_and_parent	= &omap3_dpll4_set_rate_and_parent,
+	.determine_rate	= &omap3_noncore_dpll_determine_rate,
 	.round_rate	= &omap2_dpll_round_rate,
 };
 #endif
-- 
1.7.9.5

[PATCH 0/5] ARM: OMAP3+: DPLL: fix set_rate logic

[Paul Walmsley]

On Fri, 3 Oct 2014, Tero Kristo wrote:

> OMAP3+ DPLL code is currently using set_rate op to change DPLL rates.
> This is kind of wrong, as it also involves changing DPLL parent in certain
> cases (switch between locked mode <-> bypass mode.) This set fixes these
> issues by introducing the support of determine_rate and set_rate_and_parent
> ops for the DPLL clocks. Also introduces support for set_parent, which
> just switches a DPLL between locked <-> bypass modes.

Thanks, queued for v3.19.


- Paul

[PATCH 5/5] ARM: OMAP3+: DPLL: use determine_rate() and set_rate_and_parent()

[Kevin Hilman]

Hi Tero,

On Fri, Oct 3, 2014 at 6:57 AM, Tero Kristo <t-kristo@ti.com> wrote:
> Currently, DPLLs are hiding the gory details of switching parent
> within set_rate, which confuses the common clock code and is wrong.
> Fixed by applying the new determine_rate() and set_rate_and_parent()
> functionality to any clock-ops previously using the broken approach.
> This patch also removes the broken legacy code.
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>

This patch arrived in linux-next (as commit 2e1a7b014f9c) and broke
the omap2plus_defconfig, non-DT boot for the omap3-beagle-xm.  By
default, there's no output on the console, but turning on DEBUG_LL, I
got the crash below[1].

Reverting this commit on next-20141210 gets things booting again for me.

Kevin


[1]
[    0.000000] Clocking rate (Crystal/Core/MPU): 26.0/400/600 MHz
[    0.000000] Unable to handle kernel paging request at virtual
address 5f737973
[    0.000000] pgd = c0004000
[    0.000000] [5f737973] *pgd=00000000
[    0.000000] Internal error: Oops: 5 [#1] SMP ARM
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
3.18.0-11367-g6791358f417e #85
[    0.000000] Hardware name: OMAP3 Beagle Board
[    0.000000] task: c08da288 ti: c08ce000 task.ti: c08ce000
[    0.000000] PC is at strcmp+0x4/0x30
[    0.000000] LR is at clk_fetch_parent_index+0x80/0xd8
[    0.000000] pc : [<c032f3dc>]    lr : [<c04d81c0>]    psr: 600001d3
[    0.000000] sp : c08cff20  ip : 00000000  fp : 00000000
[    0.000000] r10: c08ec168  r9 : 5f737973  r8 : 00000001
[    0.000000] r7 : de00d280  r6 : c0770eb4  r5 : de00d284  r4 : 00000000
[    0.000000] r3 : 00000073  r2 : 00000000  r1 : 5f737973  r0 : c0770eb5
[    0.000000] Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM
Segment kernel
[    0.000000] Control: 10c5387d  Table: 80004019  DAC: 00000015
[    0.000000] Process swapper/0 (pid: 0, stack limit = 0xc08ce240)
[    0.000000] Stack: (0xc08cff20 to 0xc08d0000)
[    0.000000] ff20: c08ec168 c0770eb4 c08ebbf0 23c34600 07270e00
413fc082 dfeff140 c04d82b0
[    0.000000] ff40: c08ebbf0 07270e00 c08ec168 c08ec168 07270e00
0000001a 23c34600 c08ab890
[    0.000000] ff60: dfeff140 c04d8bfc 34300133 00000190 c08ec168
c086ffc8 34300133 00000190
[    0.000000] ff80: 00000000 c0870318 00000258 c09768c4 c09768c4
c09768c4 00000001 ffffffff
[    0.000000] ffa0: c0976480 c08681a8 00000000 c086a114 c08aa1e8
c0862684 00000002 c085eb08
[    0.000000] ffc0: ffffffff ffffffff c085e670 00000000 00000000
c08ab890 00000000 c0976694
[    0.000000] ffe0: c08d6968 c08ab88c c08dbc2c 80004059 00000000
80008074 00000000 00000000
[    0.000000] [<c032f3dc>] (strcmp) from [<c04d81c0>]
(clk_fetch_parent_index+0x80/0xd8)
[    0.000000] [<c04d81c0>] (clk_fetch_parent_index) from [<c04d82b0>]
(clk_calc_new_rates+0x98/0x194)
[    0.000000] [<c04d82b0>] (clk_calc_new_rates) from [<c04d8bfc>]
(clk_set_rate+0x50/0x90)
[    0.000000] [<c04d8bfc>] (clk_set_rate) from [<c086ffc8>]
(omap3_clk_lock_dpll5+0x1c/0xb4)
[    0.000000] [<c086ffc8>] (omap3_clk_lock_dpll5) from [<c0870318>]
(omap3xxx_clk_init+0x2b8/0x398)
[    0.000000] [<c0870318>] (omap3xxx_clk_init) from [<c08681a8>]
(omap_clk_init+0x3c/0x50)
[    0.000000] [<c08681a8>] (omap_clk_init) from [<c086a114>]
(omap3_secure_sync32k_timer_init+0x8/0x58)
[    0.000000] [<c086a114>] (omap3_secure_sync32k_timer_init) from
[<c0862684>] (time_init+0x1c/0x30)
[    0.000000] [<c0862684>] (time_init) from [<c085eb08>]
(start_kernel+0x25c/0x3fc)
[    0.000000] [<c085eb08>] (start_kernel) from [<80008074>] (0x80008074)
[    0.000000] Code: e12fff1e e1a03000 eafffff7 e4d03001 (e4d12001)
[

[PATCH 5/5] ARM: OMAP3+: DPLL: use determine_rate() and set_rate_and_parent()

[Tero Kristo]

On 12/10/2014 09:41 PM, Kevin Hilman wrote:
> Hi Tero,
>
> On Fri, Oct 3, 2014 at 6:57 AM, Tero Kristo <t-kristo@ti.com> wrote:
>> Currently, DPLLs are hiding the gory details of switching parent
>> within set_rate, which confuses the common clock code and is wrong.
>> Fixed by applying the new determine_rate() and set_rate_and_parent()
>> functionality to any clock-ops previously using the broken approach.
>> This patch also removes the broken legacy code.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>
> This patch arrived in linux-next (as commit 2e1a7b014f9c) and broke
> the omap2plus_defconfig, non-DT boot for the omap3-beagle-xm.  By
> default, there's no output on the console, but turning on DEBUG_LL, I
> got the crash below[1].
>
> Reverting this commit on next-20141210 gets things booting again for me.

Interesting... I'll pull latest linux-next today and try this out.

-Tero

>
> Kevin
>
>
> [1]
> [    0.000000] Clocking rate (Crystal/Core/MPU): 26.0/400/600 MHz
> [    0.000000] Unable to handle kernel paging request at virtual
> address 5f737973
> [    0.000000] pgd = c0004000
> [    0.000000] [5f737973] *pgd=00000000
> [    0.000000] Internal error: Oops: 5 [#1] SMP ARM
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 3.18.0-11367-g6791358f417e #85
> [    0.000000] Hardware name: OMAP3 Beagle Board
> [    0.000000] task: c08da288 ti: c08ce000 task.ti: c08ce000
> [    0.000000] PC is at strcmp+0x4/0x30
> [    0.000000] LR is at clk_fetch_parent_index+0x80/0xd8
> [    0.000000] pc : [<c032f3dc>]    lr : [<c04d81c0>]    psr: 600001d3
> [    0.000000] sp : c08cff20  ip : 00000000  fp : 00000000
> [    0.000000] r10: c08ec168  r9 : 5f737973  r8 : 00000001
> [    0.000000] r7 : de00d280  r6 : c0770eb4  r5 : de00d284  r4 : 00000000
> [    0.000000] r3 : 00000073  r2 : 00000000  r1 : 5f737973  r0 : c0770eb5
> [    0.000000] Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM
> Segment kernel
> [    0.000000] Control: 10c5387d  Table: 80004019  DAC: 00000015
> [    0.000000] Process swapper/0 (pid: 0, stack limit = 0xc08ce240)
> [    0.000000] Stack: (0xc08cff20 to 0xc08d0000)
> [    0.000000] ff20: c08ec168 c0770eb4 c08ebbf0 23c34600 07270e00
> 413fc082 dfeff140 c04d82b0
> [    0.000000] ff40: c08ebbf0 07270e00 c08ec168 c08ec168 07270e00
> 0000001a 23c34600 c08ab890
> [    0.000000] ff60: dfeff140 c04d8bfc 34300133 00000190 c08ec168
> c086ffc8 34300133 00000190
> [    0.000000] ff80: 00000000 c0870318 00000258 c09768c4 c09768c4
> c09768c4 00000001 ffffffff
> [    0.000000] ffa0: c0976480 c08681a8 00000000 c086a114 c08aa1e8
> c0862684 00000002 c085eb08
> [    0.000000] ffc0: ffffffff ffffffff c085e670 00000000 00000000
> c08ab890 00000000 c0976694
> [    0.000000] ffe0: c08d6968 c08ab88c c08dbc2c 80004059 00000000
> 80008074 00000000 00000000
> [    0.000000] [<c032f3dc>] (strcmp) from [<c04d81c0>]
> (clk_fetch_parent_index+0x80/0xd8)
> [    0.000000] [<c04d81c0>] (clk_fetch_parent_index) from [<c04d82b0>]
> (clk_calc_new_rates+0x98/0x194)
> [    0.000000] [<c04d82b0>] (clk_calc_new_rates) from [<c04d8bfc>]
> (clk_set_rate+0x50/0x90)
> [    0.000000] [<c04d8bfc>] (clk_set_rate) from [<c086ffc8>]
> (omap3_clk_lock_dpll5+0x1c/0xb4)
> [    0.000000] [<c086ffc8>] (omap3_clk_lock_dpll5) from [<c0870318>]
> (omap3xxx_clk_init+0x2b8/0x398)
> [    0.000000] [<c0870318>] (omap3xxx_clk_init) from [<c08681a8>]
> (omap_clk_init+0x3c/0x50)
> [    0.000000] [<c08681a8>] (omap_clk_init) from [<c086a114>]
> (omap3_secure_sync32k_timer_init+0x8/0x58)
> [    0.000000] [<c086a114>] (omap3_secure_sync32k_timer_init) from
> [<c0862684>] (time_init+0x1c/0x30)
> [    0.000000] [<c0862684>] (time_init) from [<c085eb08>]
> (start_kernel+0x25c/0x3fc)
> [    0.000000] [<c085eb08>] (start_kernel) from [<80008074>] (0x80008074)
> [    0.000000] Code: e12fff1e e1a03000 eafffff7 e4d03001 (e4d12001)
> [
>

[PATCH 5/5] ARM: OMAP3+: DPLL: use determine_rate() and set_rate_and_parent()

[Tero Kristo]

On 12/10/2014 09:41 PM, Kevin Hilman wrote:
> Hi Tero,
>
> On Fri, Oct 3, 2014 at 6:57 AM, Tero Kristo <t-kristo@ti.com> wrote:
>> Currently, DPLLs are hiding the gory details of switching parent
>> within set_rate, which confuses the common clock code and is wrong.
>> Fixed by applying the new determine_rate() and set_rate_and_parent()
>> functionality to any clock-ops previously using the broken approach.
>> This patch also removes the broken legacy code.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>
> This patch arrived in linux-next (as commit 2e1a7b014f9c) and broke
> the omap2plus_defconfig, non-DT boot for the omap3-beagle-xm.  By
> default, there's no output on the console, but turning on DEBUG_LL, I
> got the crash below[1].
>
> Reverting this commit on next-20141210 gets things booting again for me.

Posted a fix to this problem today to l-o list.

-Tero

>
> Kevin
>
>
> [1]
> [    0.000000] Clocking rate (Crystal/Core/MPU): 26.0/400/600 MHz
> [    0.000000] Unable to handle kernel paging request at virtual
> address 5f737973
> [    0.000000] pgd = c0004000
> [    0.000000] [5f737973] *pgd=00000000
> [    0.000000] Internal error: Oops: 5 [#1] SMP ARM
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 3.18.0-11367-g6791358f417e #85
> [    0.000000] Hardware name: OMAP3 Beagle Board
> [    0.000000] task: c08da288 ti: c08ce000 task.ti: c08ce000
> [    0.000000] PC is at strcmp+0x4/0x30
> [    0.000000] LR is at clk_fetch_parent_index+0x80/0xd8
> [    0.000000] pc : [<c032f3dc>]    lr : [<c04d81c0>]    psr: 600001d3
> [    0.000000] sp : c08cff20  ip : 00000000  fp : 00000000
> [    0.000000] r10: c08ec168  r9 : 5f737973  r8 : 00000001
> [    0.000000] r7 : de00d280  r6 : c0770eb4  r5 : de00d284  r4 : 00000000
> [    0.000000] r3 : 00000073  r2 : 00000000  r1 : 5f737973  r0 : c0770eb5
> [    0.000000] Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM
> Segment kernel
> [    0.000000] Control: 10c5387d  Table: 80004019  DAC: 00000015
> [    0.000000] Process swapper/0 (pid: 0, stack limit = 0xc08ce240)
> [    0.000000] Stack: (0xc08cff20 to 0xc08d0000)
> [    0.000000] ff20: c08ec168 c0770eb4 c08ebbf0 23c34600 07270e00
> 413fc082 dfeff140 c04d82b0
> [    0.000000] ff40: c08ebbf0 07270e00 c08ec168 c08ec168 07270e00
> 0000001a 23c34600 c08ab890
> [    0.000000] ff60: dfeff140 c04d8bfc 34300133 00000190 c08ec168
> c086ffc8 34300133 00000190
> [    0.000000] ff80: 00000000 c0870318 00000258 c09768c4 c09768c4
> c09768c4 00000001 ffffffff
> [    0.000000] ffa0: c0976480 c08681a8 00000000 c086a114 c08aa1e8
> c0862684 00000002 c085eb08
> [    0.000000] ffc0: ffffffff ffffffff c085e670 00000000 00000000
> c08ab890 00000000 c0976694
> [    0.000000] ffe0: c08d6968 c08ab88c c08dbc2c 80004059 00000000
> 80008074 00000000 00000000
> [    0.000000] [<c032f3dc>] (strcmp) from [<c04d81c0>]
> (clk_fetch_parent_index+0x80/0xd8)
> [    0.000000] [<c04d81c0>] (clk_fetch_parent_index) from [<c04d82b0>]
> (clk_calc_new_rates+0x98/0x194)
> [    0.000000] [<c04d82b0>] (clk_calc_new_rates) from [<c04d8bfc>]
> (clk_set_rate+0x50/0x90)
> [    0.000000] [<c04d8bfc>] (clk_set_rate) from [<c086ffc8>]
> (omap3_clk_lock_dpll5+0x1c/0xb4)
> [    0.000000] [<c086ffc8>] (omap3_clk_lock_dpll5) from [<c0870318>]
> (omap3xxx_clk_init+0x2b8/0x398)
> [    0.000000] [<c0870318>] (omap3xxx_clk_init) from [<c08681a8>]
> (omap_clk_init+0x3c/0x50)
> [    0.000000] [<c08681a8>] (omap_clk_init) from [<c086a114>]
> (omap3_secure_sync32k_timer_init+0x8/0x58)
> [    0.000000] [<c086a114>] (omap3_secure_sync32k_timer_init) from
> [<c0862684>] (time_init+0x1c/0x30)
> [    0.000000] [<c0862684>] (time_init) from [<c085eb08>]
> (start_kernel+0x25c/0x3fc)
> [    0.000000] [<c085eb08>] (start_kernel) from [<80008074>] (0x80008074)
> [    0.000000] Code: e12fff1e e1a03000 eafffff7 e4d03001 (e4d12001)
> [
>