[PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[Dong Aisheng]

This patch series adds support in clock framework for clocks which operations
requires its parent clock is on.

Such clock type is initially met on Freescale i.MX7D platform that all clocks
operations, including enable/disable, rate change and re-parent, requires its
parent clock on. No sure if any other SoC has the similar clock type.

Current clock core can not support such type of clock well.

This patch introduce a new flag CLK_SET_PARENT_ON to handle this special case
in clock core that enable its parent clock firstly for each operation and disable
it later after operation complete.

The most special case is for set_parent() operation which requires both parent,
old one and new one, to be enabled at the same time during the operation.

The patch series is based on for-next branch of Michael's git:
git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git

Change Log:
v2->v3:
 * rebased version, no other changes.

v1->v2:
 Mainly addressed Stephen Boyd's comments
 * remove dupliciated code with __clk_set_parent_after
 * introduce more clk_core_x APIs for core easily use
 * move clk_disable_unused code position
 * use clk_core_x API to make code more clean and easily read

Dong Aisheng (5):
  clk: remove duplicated code with __clk_set_parent_after
  clk: introduce clk_core_enable_lock and clk_core_disable_lock
    functions
  clk: move clk_disable_unused after clk_core_disable_unprepare function
  clk: core: add CLK_OPS_PARENT_ON flags to support clocks require
    parent on
  clk: core: add CLK_OPS_PARENT_ON flags to support clocks require
    parent on

 drivers/clk/clk.c            | 338 +++++++++++++++++++++++++------------------
 include/linux/clk-provider.h |   5 +
 2 files changed, 200 insertions(+), 143 deletions(-)

-- 
1.9.1

[PATCH V3 1/5] clk: remove duplicated code with __clk_set_parent_after

[Dong Aisheng]

__clk_set_parent_after() actually used the second argument then we
could put this duplicate logic in there and call it with a different
order of arguments in the success vs. error paths in this function.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
---
 drivers/clk/clk.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 7bb9757..4c7f7b2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1185,14 +1185,8 @@ static int __clk_set_parent(struct clk_core *core, struct clk_core *parent,
 		flags = clk_enable_lock();
 		clk_reparent(core, old_parent);
 		clk_enable_unlock(flags);
+		__clk_set_parent_after(core, old_parent, parent);
 
-		if (core->prepare_count) {
-			flags = clk_enable_lock();
-			clk_core_disable(core);
-			clk_core_disable(parent);
-			clk_enable_unlock(flags);
-			clk_core_unprepare(parent);
-		}
 		return ret;
 	}
 
-- 
1.9.1

[PATCH V3 2/5] clk: introduce clk_core_enable_lock and clk_core_disable_lock functions

[Dong Aisheng]

This can be useful when clock core wants to enable/disable clocks.
Then we don't have to convert the struct clk_core to struct clk to call
clk_enable/clk_disable which is a bit un-align with exist using.

And after introduce clk_core_{enable|disable}_lock, we can refine
clk_eanble and clk_disable a bit.

As well as clk_core_{enable|disable}_lock, we also added
clk_core_{prepare|unprepare}_lock and clk_core_prepare_enable/
clk_core_unprepare_disable for clock core to easily use.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
---
 drivers/clk/clk.c | 85 +++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 63 insertions(+), 22 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 4c7f7b2..c2310d5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -583,6 +583,13 @@ static void clk_core_unprepare(struct clk_core *core)
 	clk_core_unprepare(core->parent);
 }
 
+static void clk_core_unprepare_lock(struct clk_core *core)
+{
+	clk_prepare_lock();
+	clk_core_unprepare(core);
+	clk_prepare_unlock();
+}
+
 /**
  * clk_unprepare - undo preparation of a clock source
  * @clk: the clk being unprepared
@@ -599,9 +606,7 @@ void clk_unprepare(struct clk *clk)
 	if (IS_ERR_OR_NULL(clk))
 		return;
 
-	clk_prepare_lock();
-	clk_core_unprepare(clk->core);
-	clk_prepare_unlock();
+	clk_core_unprepare_lock(clk->core);
 }
 EXPORT_SYMBOL_GPL(clk_unprepare);
 
@@ -637,6 +642,17 @@ static int clk_core_prepare(struct clk_core *core)
 	return 0;
 }
 
+static int clk_core_prepare_lock(struct clk_core *core)
+{
+	int ret;
+
+	clk_prepare_lock();
+	ret = clk_core_prepare(core);
+	clk_prepare_unlock();
+
+	return ret;
+}
+
 /**
  * clk_prepare - prepare a clock source
  * @clk: the clk being prepared
@@ -651,16 +667,10 @@ static int clk_core_prepare(struct clk_core *core)
  */
 int clk_prepare(struct clk *clk)
 {
-	int ret;
-
 	if (!clk)
 		return 0;
 
-	clk_prepare_lock();
-	ret = clk_core_prepare(clk->core);
-	clk_prepare_unlock();
-
-	return ret;
+	return clk_core_prepare_lock(clk->core);
 }
 EXPORT_SYMBOL_GPL(clk_prepare);
 
@@ -687,6 +697,15 @@ static void clk_core_disable(struct clk_core *core)
 	clk_core_disable(core->parent);
 }
 
+static void clk_core_disable_lock(struct clk_core *core)
+{
+	unsigned long flags;
+
+	flags = clk_enable_lock();
+	clk_core_disable(core);
+	clk_enable_unlock(flags);
+}
+
 /**
  * clk_disable - gate a clock
  * @clk: the clk being gated
@@ -701,14 +720,10 @@ static void clk_core_disable(struct clk_core *core)
  */
 void clk_disable(struct clk *clk)
 {
-	unsigned long flags;
-
 	if (IS_ERR_OR_NULL(clk))
 		return;
 
-	flags = clk_enable_lock();
-	clk_core_disable(clk->core);
-	clk_enable_unlock(flags);
+	clk_core_disable_lock(clk->core);
 }
 EXPORT_SYMBOL_GPL(clk_disable);
 
@@ -747,6 +762,18 @@ static int clk_core_enable(struct clk_core *core)
 	return 0;
 }
 
+static int clk_core_enable_lock(struct clk_core *core)
+{
+	unsigned long flags;
+	int ret;
+
+	flags = clk_enable_lock();
+	ret = clk_core_enable(core);
+	clk_enable_unlock(flags);
+
+	return ret;
+}
+
 /**
  * clk_enable - ungate a clock
  * @clk: the clk being ungated
@@ -762,19 +789,33 @@ static int clk_core_enable(struct clk_core *core)
  */
 int clk_enable(struct clk *clk)
 {
-	unsigned long flags;
-	int ret;
-
 	if (!clk)
 		return 0;
 
-	flags = clk_enable_lock();
-	ret = clk_core_enable(clk->core);
-	clk_enable_unlock(flags);
+	return clk_core_enable_lock(clk->core);
+}
+EXPORT_SYMBOL_GPL(clk_enable);
+
+static int clk_core_prepare_enable(struct clk_core *core)
+{
+	int ret;
+
+	ret = clk_core_prepare_lock(core);
+	if (ret)
+		return ret;
+
+	ret = clk_core_enable_lock(core);
+	if (ret)
+		clk_core_unprepare_lock(core);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(clk_enable);
+
+static void clk_core_disable_unprepare(struct clk_core *core)
+{
+	clk_core_disable_lock(core);
+	clk_core_unprepare_lock(core);
+}
 
 static int clk_core_round_rate_nolock(struct clk_core *core,
 				      struct clk_rate_request *req)
-- 
1.9.1

[PATCH V3 3/5] clk: move clk_disable_unused after clk_core_disable_unprepare function

[Dong Aisheng]

No function level change, just moving code place.
clk_disable_unused function will need to call clk_core_prepare_enable/
clk_core_disable_unprepare when adding CLK_OPS_PARENT_ON features.
So move it after clk_core_disable_unprepare to avoid adding forward
declared functions.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
---
 drivers/clk/clk.c | 196 +++++++++++++++++++++++++++---------------------------
 1 file changed, 98 insertions(+), 98 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c2310d5..ac158c4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -171,104 +171,6 @@ static bool clk_core_is_enabled(struct clk_core *core)
 	return core->ops->is_enabled(core->hw);
 }
 
-static void clk_unprepare_unused_subtree(struct clk_core *core)
-{
-	struct clk_core *child;
-
-	lockdep_assert_held(&prepare_lock);
-
-	hlist_for_each_entry(child, &core->children, child_node)
-		clk_unprepare_unused_subtree(child);
-
-	if (core->prepare_count)
-		return;
-
-	if (core->flags & CLK_IGNORE_UNUSED)
-		return;
-
-	if (clk_core_is_prepared(core)) {
-		trace_clk_unprepare(core);
-		if (core->ops->unprepare_unused)
-			core->ops->unprepare_unused(core->hw);
-		else if (core->ops->unprepare)
-			core->ops->unprepare(core->hw);
-		trace_clk_unprepare_complete(core);
-	}
-}
-
-static void clk_disable_unused_subtree(struct clk_core *core)
-{
-	struct clk_core *child;
-	unsigned long flags;
-
-	lockdep_assert_held(&prepare_lock);
-
-	hlist_for_each_entry(child, &core->children, child_node)
-		clk_disable_unused_subtree(child);
-
-	flags = clk_enable_lock();
-
-	if (core->enable_count)
-		goto unlock_out;
-
-	if (core->flags & CLK_IGNORE_UNUSED)
-		goto unlock_out;
-
-	/*
-	 * some gate clocks have special needs during the disable-unused
-	 * sequence.  call .disable_unused if available, otherwise fall
-	 * back to .disable
-	 */
-	if (clk_core_is_enabled(core)) {
-		trace_clk_disable(core);
-		if (core->ops->disable_unused)
-			core->ops->disable_unused(core->hw);
-		else if (core->ops->disable)
-			core->ops->disable(core->hw);
-		trace_clk_disable_complete(core);
-	}
-
-unlock_out:
-	clk_enable_unlock(flags);
-}
-
-static bool clk_ignore_unused;
-static int __init clk_ignore_unused_setup(char *__unused)
-{
-	clk_ignore_unused = true;
-	return 1;
-}
-__setup("clk_ignore_unused", clk_ignore_unused_setup);
-
-static int clk_disable_unused(void)
-{
-	struct clk_core *core;
-
-	if (clk_ignore_unused) {
-		pr_warn("clk: Not disabling unused clocks\n");
-		return 0;
-	}
-
-	clk_prepare_lock();
-
-	hlist_for_each_entry(core, &clk_root_list, child_node)
-		clk_disable_unused_subtree(core);
-
-	hlist_for_each_entry(core, &clk_orphan_list, child_node)
-		clk_disable_unused_subtree(core);
-
-	hlist_for_each_entry(core, &clk_root_list, child_node)
-		clk_unprepare_unused_subtree(core);
-
-	hlist_for_each_entry(core, &clk_orphan_list, child_node)
-		clk_unprepare_unused_subtree(core);
-
-	clk_prepare_unlock();
-
-	return 0;
-}
-late_initcall_sync(clk_disable_unused);
-
 /***    helper functions   ***/
 
 const char *__clk_get_name(struct clk *clk)
@@ -817,6 +719,104 @@ static void clk_core_disable_unprepare(struct clk_core *core)
 	clk_core_unprepare_lock(core);
 }
 
+static void clk_unprepare_unused_subtree(struct clk_core *core)
+{
+	struct clk_core *child;
+
+	lockdep_assert_held(&prepare_lock);
+
+	hlist_for_each_entry(child, &core->children, child_node)
+		clk_unprepare_unused_subtree(child);
+
+	if (core->prepare_count)
+		return;
+
+	if (core->flags & CLK_IGNORE_UNUSED)
+		return;
+
+	if (clk_core_is_prepared(core)) {
+		trace_clk_unprepare(core);
+		if (core->ops->unprepare_unused)
+			core->ops->unprepare_unused(core->hw);
+		else if (core->ops->unprepare)
+			core->ops->unprepare(core->hw);
+		trace_clk_unprepare_complete(core);
+	}
+}
+
+static void clk_disable_unused_subtree(struct clk_core *core)
+{
+	struct clk_core *child;
+	unsigned long flags;
+
+	lockdep_assert_held(&prepare_lock);
+
+	hlist_for_each_entry(child, &core->children, child_node)
+		clk_disable_unused_subtree(child);
+
+	flags = clk_enable_lock();
+
+	if (core->enable_count)
+		goto unlock_out;
+
+	if (core->flags & CLK_IGNORE_UNUSED)
+		goto unlock_out;
+
+	/*
+	 * some gate clocks have special needs during the disable-unused
+	 * sequence.  call .disable_unused if available, otherwise fall
+	 * back to .disable
+	 */
+	if (clk_core_is_enabled(core)) {
+		trace_clk_disable(core);
+		if (core->ops->disable_unused)
+			core->ops->disable_unused(core->hw);
+		else if (core->ops->disable)
+			core->ops->disable(core->hw);
+		trace_clk_disable_complete(core);
+	}
+
+unlock_out:
+	clk_enable_unlock(flags);
+}
+
+static bool clk_ignore_unused;
+static int __init clk_ignore_unused_setup(char *__unused)
+{
+	clk_ignore_unused = true;
+	return 1;
+}
+__setup("clk_ignore_unused", clk_ignore_unused_setup);
+
+static int clk_disable_unused(void)
+{
+	struct clk_core *core;
+
+	if (clk_ignore_unused) {
+		pr_warn("clk: Not disabling unused clocks\n");
+		return 0;
+	}
+
+	clk_prepare_lock();
+
+	hlist_for_each_entry(core, &clk_root_list, child_node)
+		clk_disable_unused_subtree(core);
+
+	hlist_for_each_entry(core, &clk_orphan_list, child_node)
+		clk_disable_unused_subtree(core);
+
+	hlist_for_each_entry(core, &clk_root_list, child_node)
+		clk_unprepare_unused_subtree(core);
+
+	hlist_for_each_entry(core, &clk_orphan_list, child_node)
+		clk_unprepare_unused_subtree(core);
+
+	clk_prepare_unlock();
+
+	return 0;
+}
+late_initcall_sync(clk_disable_unused);
+
 static int clk_core_round_rate_nolock(struct clk_core *core,
 				      struct clk_rate_request *req)
 {
-- 
1.9.1

[PATCH V3 4/5] clk: core: add CLK_OPS_PARENT_ON flags to support clocks require parent on

[Dong Aisheng]

On Freescale i.MX7D platform, all clocks operations, including
enable/disable, rate change and re-parent, requires its parent
clock on. Current clock core can not support it well.
This patch introduce a new flag CLK_OPS_PARENT_ON to handle this
special case in clock core that enable its parent clock firstly for
each operation and disable it later after operation complete.

This patch fixes disaling clocks while its parent is off.
This is a special case that is caused by a state mis-align between
HW and SW in clock tree during booting.
Usually in uboot, we may enable all clocks in HW by default.
And during kernel booting time, the parent clock could be disabled in its
driver probe due to calling clk_prepare_enable and clk_disable_unprepare.
Because it's child clock is only enabled in HW while its SW usecount
in clock tree is still 0, so clk_disable of parent clock will gate
the parent clock in both HW and SW usecount ultimately.
Then there will be a clock is on in HW while its parent is disabled.

Later when clock core does clk_disable_unused, this clock disable
will cause system hang due to the limitation of operation requiring
its parent clock on.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
---
 drivers/clk/clk.c            | 5 +++++
 include/linux/clk-provider.h | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ac158c4..cf31dc4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -754,6 +754,9 @@ static void clk_disable_unused_subtree(struct clk_core *core)
 	hlist_for_each_entry(child, &core->children, child_node)
 		clk_disable_unused_subtree(child);
 
+	if (core->flags & CLK_OPS_PARENT_ON)
+		clk_core_prepare_enable(core->parent);
+
 	flags = clk_enable_lock();
 
 	if (core->enable_count)
@@ -778,6 +781,8 @@ static void clk_disable_unused_subtree(struct clk_core *core)
 
 unlock_out:
 	clk_enable_unlock(flags);
+	if (core->flags & CLK_OPS_PARENT_ON)
+		clk_core_disable_unprepare(core->parent);
 }
 
 static bool clk_ignore_unused;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 06a56e5..006fafb 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -31,6 +31,11 @@
 #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
 #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
 #define CLK_RECALC_NEW_RATES	BIT(9) /* recalc rates after notifications */
+/*
+ * parent clock must be on across any operation including
+ * clock gate/ungate, rate change and re-parent
+ */
+#define CLK_OPS_PARENT_ON	BIT(10)
 
 struct clk;
 struct clk_hw;
-- 
1.9.1

[PATCH V3 5/5] clk: core: add CLK_OPS_PARENT_ON flags to support clocks require parent on

[Dong Aisheng]

On Freescale i.MX7D platform, all clocks operations, including
enable/disable, rate change and re-parent, requires its parent clock on.
Current clock core can not support it well.
This patch adding flag CLK_OPS_PARENT_ON to handle this special case in
clock core that enable its parent clock firstly for each operation and
disable it later after operation complete.

This patch fixes changing clock rate and switch parent while its parent
is off. The most special case is for set_parent() operation which requires
both parent, including old one and new one, to be enabled at the same time
during the operation.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
---
 drivers/clk/clk.c | 46 +++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index cf31dc4..93eb9e3 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1159,7 +1159,7 @@ static struct clk_core *__clk_set_parent_before(struct clk_core *core,
 	struct clk_core *old_parent = core->parent;
 
 	/*
-	 * Migrate prepare state between parents and prevent race with
+	 * 1. Migrate prepare state between parents and prevent race with
 	 * clk_enable().
 	 *
 	 * If the clock is not prepared, then a race with
@@ -1174,13 +1174,17 @@ static struct clk_core *__clk_set_parent_before(struct clk_core *core,
 	 * hardware and software states.
 	 *
 	 * See also: Comment for clk_set_parent() below.
+	 *
+	 * 2. enable two parents clock for .set_parent() operation if finding
+	 * flag CLK_OPS_PARENT_ON
 	 */
-	if (core->prepare_count) {
-		clk_core_prepare(parent);
-		flags = clk_enable_lock();
-		clk_core_enable(parent);
-		clk_core_enable(core);
-		clk_enable_unlock(flags);
+	if (core->prepare_count || core->flags & CLK_OPS_PARENT_ON) {
+		clk_core_prepare_enable(parent);
+		if (core->prepare_count)
+			clk_core_enable_lock(core);
+		else
+			clk_core_prepare_enable(old_parent);
+
 	}
 
 	/* update the clk tree topology */
@@ -1195,18 +1199,16 @@ static void __clk_set_parent_after(struct clk_core *core,
 				   struct clk_core *parent,
 				   struct clk_core *old_parent)
 {
-	unsigned long flags;
-
 	/*
 	 * Finish the migration of prepare state and undo the changes done
 	 * for preventing a race with clk_enable().
 	 */
-	if (core->prepare_count) {
-		flags = clk_enable_lock();
-		clk_core_disable(core);
-		clk_core_disable(old_parent);
-		clk_enable_unlock(flags);
-		clk_core_unprepare(old_parent);
+	if (core->prepare_count || core->flags & CLK_OPS_PARENT_ON) {
+		clk_core_disable_unprepare(old_parent);
+		if (core->prepare_count)
+			clk_core_disable_lock(core);
+		else
+			clk_core_disable_unprepare(parent);
 	}
 }
 
@@ -1453,13 +1455,17 @@ static void clk_change_rate(struct clk_core *core)
 	unsigned long best_parent_rate = 0;
 	bool skip_set_rate = false;
 	struct clk_core *old_parent;
+	struct clk_core *parent = NULL;
 
 	old_rate = core->rate;
 
-	if (core->new_parent)
+	if (core->new_parent) {
+		parent = core->new_parent;
 		best_parent_rate = core->new_parent->rate;
-	else if (core->parent)
+	} else if (core->parent) {
+		parent = core->parent;
 		best_parent_rate = core->parent->rate;
+	}
 
 	if (core->new_parent && core->new_parent != core->parent) {
 		old_parent = __clk_set_parent_before(core, core->new_parent);
@@ -1480,6 +1486,9 @@ static void clk_change_rate(struct clk_core *core)
 
 	trace_clk_set_rate(core, core->new_rate);
 
+	if (core->flags & CLK_OPS_PARENT_ON)
+		clk_core_prepare_enable(parent);
+
 	if (!skip_set_rate && core->ops->set_rate)
 		core->ops->set_rate(core->hw, core->new_rate, best_parent_rate);
 
@@ -1487,6 +1496,9 @@ static void clk_change_rate(struct clk_core *core)
 
 	core->rate = clk_recalc(core, best_parent_rate);
 
+	if (core->flags & CLK_OPS_PARENT_ON)
+		clk_core_disable_unprepare(parent);
+
 	if (core->notifier_count && old_rate != core->rate)
 		__clk_notify(core, POST_RATE_CHANGE, old_rate, core->rate);
 
-- 
1.9.1

Re: [PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[Dong Aisheng]

On Tue, Jul 28, 2015 at 09:19:40PM +0800, Dong Aisheng wrote:
> This patch series adds support in clock framework for clocks which operations
> requires its parent clock is on.
> 
> Such clock type is initially met on Freescale i.MX7D platform that all clocks
> operations, including enable/disable, rate change and re-parent, requires its
> parent clock on. No sure if any other SoC has the similar clock type.
> 
> Current clock core can not support such type of clock well.
> 
> This patch introduce a new flag CLK_SET_PARENT_ON to handle this special case
> in clock core that enable its parent clock firstly for each operation and disable
> it later after operation complete.
> 
> The most special case is for set_parent() operation which requires both parent,
> old one and new one, to be enabled at the same time during the operation.
> 
> The patch series is based on for-next branch of Michael's git:
> git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
> 
> Change Log:
> v2->v3:
>  * rebased version, no other changes.
> 
> v1->v2:
>  Mainly addressed Stephen Boyd's comments
>  * remove dupliciated code with __clk_set_parent_after
>  * introduce more clk_core_x APIs for core easily use
>  * move clk_disable_unused code position
>  * use clk_core_x API to make code more clean and easily read
> 
> Dong Aisheng (5):
>   clk: remove duplicated code with __clk_set_parent_after
>   clk: introduce clk_core_enable_lock and clk_core_disable_lock
>     functions
>   clk: move clk_disable_unused after clk_core_disable_unprepare function
>   clk: core: add CLK_OPS_PARENT_ON flags to support clocks require
>     parent on
>   clk: core: add CLK_OPS_PARENT_ON flags to support clocks require
>     parent on
> 
>  drivers/clk/clk.c            | 338 +++++++++++++++++++++++++------------------
>  include/linux/clk-provider.h |   5 +
>  2 files changed, 200 insertions(+), 143 deletions(-)
> 

Sorry, forgot to update Mike's new email address.
Updated.

Regards
Dong Aisheng

> -- 
> 1.9.1
> 

Re: [PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[Dong Aisheng]

On Tue, Jul 28, 2015 at 10:38:25PM +0800, Dong Aisheng wrote:
> On Tue, Jul 28, 2015 at 09:19:40PM +0800, Dong Aisheng wrote:
> > This patch series adds support in clock framework for clocks which operations
> > requires its parent clock is on.
> > 
> > Such clock type is initially met on Freescale i.MX7D platform that all clocks
> > operations, including enable/disable, rate change and re-parent, requires its
> > parent clock on. No sure if any other SoC has the similar clock type.
> > 
> > Current clock core can not support such type of clock well.
> > 
> > This patch introduce a new flag CLK_SET_PARENT_ON to handle this special case
> > in clock core that enable its parent clock firstly for each operation and disable
> > it later after operation complete.
> > 
> > The most special case is for set_parent() operation which requires both parent,
> > old one and new one, to be enabled at the same time during the operation.
> > 
> > The patch series is based on for-next branch of Michael's git:
> > git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
> > 
> > Change Log:
> > v2->v3:
> >  * rebased version, no other changes.
> > 
> > v1->v2:
> >  Mainly addressed Stephen Boyd's comments
> >  * remove dupliciated code with __clk_set_parent_after
> >  * introduce more clk_core_x APIs for core easily use
> >  * move clk_disable_unused code position
> >  * use clk_core_x API to make code more clean and easily read
> > 
> > Dong Aisheng (5):
> >   clk: remove duplicated code with __clk_set_parent_after
> >   clk: introduce clk_core_enable_lock and clk_core_disable_lock
> >     functions
> >   clk: move clk_disable_unused after clk_core_disable_unprepare function
> >   clk: core: add CLK_OPS_PARENT_ON flags to support clocks require
> >     parent on
> >   clk: core: add CLK_OPS_PARENT_ON flags to support clocks require
> >     parent on
> > 
> >  drivers/clk/clk.c            | 338 +++++++++++++++++++++++++------------------
> >  include/linux/clk-provider.h |   5 +
> >  2 files changed, 200 insertions(+), 143 deletions(-)
> > 
> 
> Sorry, forgot to update Mike's new email address.
> Updated.
> 

Ping...

Regards
Dong Aisheng

> Regards
> Dong Aisheng
> 
> > -- 
> > 1.9.1
> > 

Re: [PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[Dong Aisheng]

On Tue, Aug 04, 2015 at 05:45:57PM +0800, Dong Aisheng wrote:
> On Tue, Jul 28, 2015 at 10:38:25PM +0800, Dong Aisheng wrote:
> > On Tue, Jul 28, 2015 at 09:19:40PM +0800, Dong Aisheng wrote:
> > > This patch series adds support in clock framework for clocks which operations
> > > requires its parent clock is on.
> > > 
> > > Such clock type is initially met on Freescale i.MX7D platform that all clocks
> > > operations, including enable/disable, rate change and re-parent, requires its
> > > parent clock on. No sure if any other SoC has the similar clock type.
> > > 
> > > Current clock core can not support such type of clock well.
> > > 
> > > This patch introduce a new flag CLK_SET_PARENT_ON to handle this special case
> > > in clock core that enable its parent clock firstly for each operation and disable
> > > it later after operation complete.
> > > 
> > > The most special case is for set_parent() operation which requires both parent,
> > > old one and new one, to be enabled at the same time during the operation.
> > > 
> > > The patch series is based on for-next branch of Michael's git:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
> > > 
> > > Change Log:
> > > v2->v3:
> > >  * rebased version, no other changes.
> > > 
> > > v1->v2:
> > >  Mainly addressed Stephen Boyd's comments
> > >  * remove dupliciated code with __clk_set_parent_after
> > >  * introduce more clk_core_x APIs for core easily use
> > >  * move clk_disable_unused code position
> > >  * use clk_core_x API to make code more clean and easily read
> > > 
> > > Dong Aisheng (5):
> > >   clk: remove duplicated code with __clk_set_parent_after
> > >   clk: introduce clk_core_enable_lock and clk_core_disable_lock
> > >     functions
> > >   clk: move clk_disable_unused after clk_core_disable_unprepare function
> > >   clk: core: add CLK_OPS_PARENT_ON flags to support clocks require
> > >     parent on
> > >   clk: core: add CLK_OPS_PARENT_ON flags to support clocks require
> > >     parent on
> > > 
> > >  drivers/clk/clk.c            | 338 +++++++++++++++++++++++++------------------
> > >  include/linux/clk-provider.h |   5 +
> > >  2 files changed, 200 insertions(+), 143 deletions(-)
> > > 
> > 
> > Sorry, forgot to update Mike's new email address.
> > Updated.
> > 
> 
> Ping...
> 

Hi Mike,

It's been quite a long time.
Can you help pick this?

Regards
Dong Aisheng

> Regards
> Dong Aisheng
> 
> > Regards
> > Dong Aisheng
> > 
> > > -- 
> > > 1.9.1
> > > 

Re: [PATCH V3 1/5] clk: remove duplicated code with __clk_set_parent_after

[Stephen Boyd]

On 07/28, Dong Aisheng wrote:
> __clk_set_parent_after() actually used the second argument then we
> could put this duplicate logic in there and call it with a different
> order of arguments in the success vs. error paths in this function.
> 
> Cc: Mike Turquette <mturquette@linaro.org>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[Joachim Eastwood]

Hi Dong,

On 28 July 2015 at 15:19, Dong Aisheng <aisheng.dong@freescale.com> wrote:
> This patch series adds support in clock framework for clocks which operations
> requires its parent clock is on.
>
> Such clock type is initially met on Freescale i.MX7D platform that all clocks
> operations, including enable/disable, rate change and re-parent, requires its
> parent clock on. No sure if any other SoC has the similar clock type.

Just noticed this patch set.

One of clock-controller blocks (CCU) on lpc18xx has a similar
requirement. The CCU is clock fanout block with gates and the gate
registers can not be accessed if the base (parent) clock for the gate
is not running. Doing so causes the cpu to wedge.

The workaround I have locally is to check in the is_enabled gate op if
the parent is running or not. This works fine, but I am all for a more
generic solution in the clk framework.

I'll see if I can find the time to test your patch set. Thanks for
working on this.


regards,
Joachim Eastwood

Re: [PATCH V3 4/5] clk: core: add CLK_OPS_PARENT_ON flags to support clocks require parent on

[Stephen Boyd]

On 07/28, Dong Aisheng wrote:
> On Freescale i.MX7D platform, all clocks operations, including
> enable/disable, rate change and re-parent, requires its parent
> clock on. Current clock core can not support it well.
> This patch introduce a new flag CLK_OPS_PARENT_ON to handle this
> special case in clock core that enable its parent clock firstly for
> each operation and disable it later after operation complete.
> 
> This patch fixes disaling clocks while its parent is off.
> This is a special case that is caused by a state mis-align between
> HW and SW in clock tree during booting.
> Usually in uboot, we may enable all clocks in HW by default.
> And during kernel booting time, the parent clock could be disabled in its
> driver probe due to calling clk_prepare_enable and clk_disable_unprepare.
> Because it's child clock is only enabled in HW while its SW usecount
> in clock tree is still 0, so clk_disable of parent clock will gate
> the parent clock in both HW and SW usecount ultimately.
> Then there will be a clock is on in HW while its parent is disabled.
> 
> Later when clock core does clk_disable_unused, this clock disable
> will cause system hang due to the limitation of operation requiring
> its parent clock on.
> 
> Cc: Mike Turquette <mturquette@linaro.org>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
> ---

Sorry, I still don't agree with this patch. There's no reason we
should be turning on clocks during late init so that we can turn
off clocks. If there's some sort of problem in doing that we
should figure it out and make it so that during late init we turn
off clocks from the leaves of the tree to the root.

I agree that there's a problem here where we don't properly
handle keeping children clocks on if a driver comes in and turns
off a clock in the middle of the tree before late init. That's a
real bug, and we should fix it. Mike Turquette has been doing
some work to have a way to indicate that certain clocks should be
kept on until client drivers grab them. I think it will also make
sure to up refcounts on parent clocks in the middle of the tree
when it figures out that a child clock is enabled. Would that be
all that we need to do to fix this problem?

Also, the subject of this patch and patch 5 are the same. Why?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V3 4/5] clk: core: add CLK_OPS_PARENT_ON flags to support clocks require parent on

[Dong Aisheng]

On Thu, Aug 13, 2015 at 06:01:09PM -0700, Stephen Boyd wrote:
> On 07/28, Dong Aisheng wrote:
> > On Freescale i.MX7D platform, all clocks operations, including
> > enable/disable, rate change and re-parent, requires its parent
> > clock on. Current clock core can not support it well.
> > This patch introduce a new flag CLK_OPS_PARENT_ON to handle this
> > special case in clock core that enable its parent clock firstly for
> > each operation and disable it later after operation complete.
> > 
> > This patch fixes disaling clocks while its parent is off.
> > This is a special case that is caused by a state mis-align between
> > HW and SW in clock tree during booting.
> > Usually in uboot, we may enable all clocks in HW by default.
> > And during kernel booting time, the parent clock could be disabled in its
> > driver probe due to calling clk_prepare_enable and clk_disable_unprepare.
> > Because it's child clock is only enabled in HW while its SW usecount
> > in clock tree is still 0, so clk_disable of parent clock will gate
> > the parent clock in both HW and SW usecount ultimately.
> > Then there will be a clock is on in HW while its parent is disabled.
> > 
> > Later when clock core does clk_disable_unused, this clock disable
> > will cause system hang due to the limitation of operation requiring
> > its parent clock on.
> > 
> > Cc: Mike Turquette <mturquette@linaro.org>
> > Cc: Stephen Boyd <sboyd@codeaurora.org>
> > Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
> > ---
> 
> Sorry, I still don't agree with this patch. There's no reason we
> should be turning on clocks during late init so that we can turn
> off clocks.

Can't the reason be that it's fairly possible one clock is enabled
in HW while it's parent is disabled in initial clock tree state
and to enable parent clocks to disable itself is required by its special
HW characteristic?

Dosen't it quite clear from HW point of view?

> If there's some sort of problem in doing that we
> should figure it out and make it so that during late init we turn
> off clocks from the leaves of the tree to the root.
> 

Turning off clocks from the leaves to root probably may require clock core
to provide a way to keep the parent clock enabled once finding its child
is still on in HW (clk_core_is_enabled() returns true) but enable_count
is zero before late init.

One possible solution may be leaving parent clocks on in HW during disable
once finding its child is on in HW and only decrease the parent's refcount,
and then replying on the later clk_disable_unused() to disable both child
and parent from leave to root.
e.g. clock A: parent, clock B: child of A
Initial state: clock B is enabled in HW while refcount is zero
Step1: Driver A enable clock A during probe
 A: refcount becomes 1 HW state: enabled
Step2: Driver A disable clock A after probe
 A: refcount becomes 0 HW state: enabled (only decrease refcount)

Then Clock A will be the same state as B, HW enabled while refcount is zero(
means no users), the later clk_disable_unusersd() will disable them all
from leave to root.

This is a workable solution but seems much more complicated than the exist
one in this patch which is only 5 lines of code changes.

And the question is:
since we already have the support of CLK_OPS_PARENT_ON (required by
clock set_rate/re-parent), why we still need invent another complicated
mechanism to support avoiding enable parent clock only for clk_disable_unused()?
Is that really worthy?
And it's also less power efficiency than the one in the patch.

> I agree that there's a problem here where we don't properly
> handle keeping children clocks on if a driver comes in and turns
> off a clock in the middle of the tree before late init. That's a
> real bug, and we should fix it.

Sorry, i still can't understand it's a bug.
Can you help explain more?

It looks to me like reasonable.
Enable/disable clock in driver is just one case, the initial clock tree may
also have such cases.
(Here i took the 'children clocks' you said as the one who's child clock is on
in HW while refcount is zero, fix me if wrong)

And it seems not so quite make sense to not physically disable the clock
when there's already no child users(refcount becomes zero) and i don't
think the child clock's default enablement state in HW means a valid user
since it's just caused by misalignment between HW and SW clock tree during
kernel booting phase which is meaningless.
And that seems is why the clk_disable_unused() function exist for fixing
this state misalignment issue.

> Mike Turquette has been doing
> some work to have a way to indicate that certain clocks should be
> kept on until client drivers grab them. 

Sorry i can't see how this help on my issue.

> I think it will also make
> sure to up refcounts on parent clocks in the middle of the tree
> when it figures out that a child clock is enabled. Would that be
> all that we need to do to fix this problem?
> 

Then when will we down the refcounts on parent clocks and when to disable it?
The current clk_disable_unused() only handle HW clk enable/disable, no
refcount operations.
Not sure how this is going to fix my issues.

And again, as i said above, i don't think it makes much sense to not disable
parent only if child is enabled in HW, unless there's more strong reasons.

> Also, the subject of this patch and patch 5 are the same. Why?
> 

Sorry, mainly because the full feature of CLK_OPS_PARENT_ON is divided into
two patches for better review, their commit message is different.
patch 4 is adding support for clk_disable_unused() while patch 5 is for
clk_setrate/clk_reparent.
I could reform the subject if needed.

Regards
Dong Aisheng

> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH V3 4/5] clk: core: add CLK_OPS_PARENT_ON flags to support clocks require parent on

[Dong Aisheng]

On Mon, Aug 17, 2015 at 08:45:18PM +0800, Dong Aisheng wrote:
> On Thu, Aug 13, 2015 at 06:01:09PM -0700, Stephen Boyd wrote:
> > On 07/28, Dong Aisheng wrote:
> > > On Freescale i.MX7D platform, all clocks operations, including
> > > enable/disable, rate change and re-parent, requires its parent
> > > clock on. Current clock core can not support it well.
> > > This patch introduce a new flag CLK_OPS_PARENT_ON to handle this
> > > special case in clock core that enable its parent clock firstly for
> > > each operation and disable it later after operation complete.
> > > 
> > > This patch fixes disaling clocks while its parent is off.
> > > This is a special case that is caused by a state mis-align between
> > > HW and SW in clock tree during booting.
> > > Usually in uboot, we may enable all clocks in HW by default.
> > > And during kernel booting time, the parent clock could be disabled in its
> > > driver probe due to calling clk_prepare_enable and clk_disable_unprepare.
> > > Because it's child clock is only enabled in HW while its SW usecount
> > > in clock tree is still 0, so clk_disable of parent clock will gate
> > > the parent clock in both HW and SW usecount ultimately.
> > > Then there will be a clock is on in HW while its parent is disabled.
> > > 
> > > Later when clock core does clk_disable_unused, this clock disable
> > > will cause system hang due to the limitation of operation requiring
> > > its parent clock on.
> > > 
> > > Cc: Mike Turquette <mturquette@linaro.org>
> > > Cc: Stephen Boyd <sboyd@codeaurora.org>
> > > Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
> > > ---
> > 
> > Sorry, I still don't agree with this patch. There's no reason we
> > should be turning on clocks during late init so that we can turn
> > off clocks.
> 
> Can't the reason be that it's fairly possible one clock is enabled
> in HW while it's parent is disabled in initial clock tree state
> and to enable parent clocks to disable itself is required by its special
> HW characteristic?
> 
> Dosen't it quite clear from HW point of view?
> 
> > If there's some sort of problem in doing that we
> > should figure it out and make it so that during late init we turn
> > off clocks from the leaves of the tree to the root.
> > 
> 
> Turning off clocks from the leaves to root probably may require clock core
> to provide a way to keep the parent clock enabled once finding its child
> is still on in HW (clk_core_is_enabled() returns true) but enable_count
> is zero before late init.
> 
> One possible solution may be leaving parent clocks on in HW during disable
> once finding its child is on in HW and only decrease the parent's refcount,
> and then replying on the later clk_disable_unused() to disable both child
> and parent from leave to root.
> e.g. clock A: parent, clock B: child of A
> Initial state: clock B is enabled in HW while refcount is zero
> Step1: Driver A enable clock A during probe
>  A: refcount becomes 1 HW state: enabled
> Step2: Driver A disable clock A after probe
>  A: refcount becomes 0 HW state: enabled (only decrease refcount)
> 
> Then Clock A will be the same state as B, HW enabled while refcount is zero(
> means no users), the later clk_disable_unusersd() will disable them all
> from leave to root.
> 
> This is a workable solution but seems much more complicated than the exist
> one in this patch which is only 5 lines of code changes.
> 
> And the question is:
> since we already have the support of CLK_OPS_PARENT_ON (required by
> clock set_rate/re-parent), why we still need invent another complicated
> mechanism to support avoiding enable parent clock only for clk_disable_unused()?
> Is that really worthy?
> And it's also less power efficiency than the one in the patch.
> 
> > I agree that there's a problem here where we don't properly
> > handle keeping children clocks on if a driver comes in and turns
> > off a clock in the middle of the tree before late init. That's a
> > real bug, and we should fix it.
> 
> Sorry, i still can't understand it's a bug.
> Can you help explain more?
> 
> It looks to me like reasonable.
> Enable/disable clock in driver is just one case, the initial clock tree may
> also have such cases.
> (Here i took the 'children clocks' you said as the one who's child clock is on
> in HW while refcount is zero, fix me if wrong)
> 
> And it seems not so quite make sense to not physically disable the clock
> when there's already no child users(refcount becomes zero) and i don't
> think the child clock's default enablement state in HW means a valid user
> since it's just caused by misalignment between HW and SW clock tree during
> kernel booting phase which is meaningless.
> And that seems is why the clk_disable_unused() function exist for fixing
> this state misalignment issue.
> 
> > Mike Turquette has been doing
> > some work to have a way to indicate that certain clocks should be
> > kept on until client drivers grab them. 
> 
> Sorry i can't see how this help on my issue.
> 
> > I think it will also make
> > sure to up refcounts on parent clocks in the middle of the tree
> > when it figures out that a child clock is enabled. Would that be
> > all that we need to do to fix this problem?
> > 
> 
> Then when will we down the refcounts on parent clocks and when to disable it?
> The current clk_disable_unused() only handle HW clk enable/disable, no
> refcount operations.
> Not sure how this is going to fix my issues.
> 
> And again, as i said above, i don't think it makes much sense to not disable
> parent only if child is enabled in HW, unless there's more strong reasons.
> 
> > Also, the subject of this patch and patch 5 are the same. Why?
> > 
> 
> Sorry, mainly because the full feature of CLK_OPS_PARENT_ON is divided into
> two patches for better review, their commit message is different.
> patch 4 is adding support for clk_disable_unused() while patch 5 is for
> clk_setrate/clk_reparent.
> I could reform the subject if needed.
> 
> Regards
> Dong Aisheng
> 

Hi Mike & Stephen,

Any comments about this?

Regards
Dong Aisheng

> > -- 
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> > a Linux Foundation Collaborative Project

Re: [PATCH V3 4/5] clk: core: add CLK_OPS_PARENT_ON flags to support clocks require parent on

[Dong Aisheng]

Ping...

On Wed, Aug 19, 2015 at 7:07 PM, Dong Aisheng
<aisheng.dong@freescale.com> wrote:
> On Mon, Aug 17, 2015 at 08:45:18PM +0800, Dong Aisheng wrote:
>> On Thu, Aug 13, 2015 at 06:01:09PM -0700, Stephen Boyd wrote:
>> > On 07/28, Dong Aisheng wrote:
>> > > On Freescale i.MX7D platform, all clocks operations, including
>> > > enable/disable, rate change and re-parent, requires its parent
>> > > clock on. Current clock core can not support it well.
>> > > This patch introduce a new flag CLK_OPS_PARENT_ON to handle this
>> > > special case in clock core that enable its parent clock firstly for
>> > > each operation and disable it later after operation complete.
>> > >
>> > > This patch fixes disaling clocks while its parent is off.
>> > > This is a special case that is caused by a state mis-align between
>> > > HW and SW in clock tree during booting.
>> > > Usually in uboot, we may enable all clocks in HW by default.
>> > > And during kernel booting time, the parent clock could be disabled in its
>> > > driver probe due to calling clk_prepare_enable and clk_disable_unprepare.
>> > > Because it's child clock is only enabled in HW while its SW usecount
>> > > in clock tree is still 0, so clk_disable of parent clock will gate
>> > > the parent clock in both HW and SW usecount ultimately.
>> > > Then there will be a clock is on in HW while its parent is disabled.
>> > >
>> > > Later when clock core does clk_disable_unused, this clock disable
>> > > will cause system hang due to the limitation of operation requiring
>> > > its parent clock on.
>> > >
>> > > Cc: Mike Turquette <mturquette@linaro.org>
>> > > Cc: Stephen Boyd <sboyd@codeaurora.org>
>> > > Signed-off-by: Dong Aisheng <aisheng.dong@freescale.com>
>> > > ---
>> >
>> > Sorry, I still don't agree with this patch. There's no reason we
>> > should be turning on clocks during late init so that we can turn
>> > off clocks.
>>
>> Can't the reason be that it's fairly possible one clock is enabled
>> in HW while it's parent is disabled in initial clock tree state
>> and to enable parent clocks to disable itself is required by its special
>> HW characteristic?
>>
>> Dosen't it quite clear from HW point of view?
>>
>> > If there's some sort of problem in doing that we
>> > should figure it out and make it so that during late init we turn
>> > off clocks from the leaves of the tree to the root.
>> >
>>
>> Turning off clocks from the leaves to root probably may require clock core
>> to provide a way to keep the parent clock enabled once finding its child
>> is still on in HW (clk_core_is_enabled() returns true) but enable_count
>> is zero before late init.
>>
>> One possible solution may be leaving parent clocks on in HW during disable
>> once finding its child is on in HW and only decrease the parent's refcount,
>> and then replying on the later clk_disable_unused() to disable both child
>> and parent from leave to root.
>> e.g. clock A: parent, clock B: child of A
>> Initial state: clock B is enabled in HW while refcount is zero
>> Step1: Driver A enable clock A during probe
>>  A: refcount becomes 1 HW state: enabled
>> Step2: Driver A disable clock A after probe
>>  A: refcount becomes 0 HW state: enabled (only decrease refcount)
>>
>> Then Clock A will be the same state as B, HW enabled while refcount is zero(
>> means no users), the later clk_disable_unusersd() will disable them all
>> from leave to root.
>>
>> This is a workable solution but seems much more complicated than the exist
>> one in this patch which is only 5 lines of code changes.
>>
>> And the question is:
>> since we already have the support of CLK_OPS_PARENT_ON (required by
>> clock set_rate/re-parent), why we still need invent another complicated
>> mechanism to support avoiding enable parent clock only for clk_disable_unused()?
>> Is that really worthy?
>> And it's also less power efficiency than the one in the patch.
>>
>> > I agree that there's a problem here where we don't properly
>> > handle keeping children clocks on if a driver comes in and turns
>> > off a clock in the middle of the tree before late init. That's a
>> > real bug, and we should fix it.
>>
>> Sorry, i still can't understand it's a bug.
>> Can you help explain more?
>>
>> It looks to me like reasonable.
>> Enable/disable clock in driver is just one case, the initial clock tree may
>> also have such cases.
>> (Here i took the 'children clocks' you said as the one who's child clock is on
>> in HW while refcount is zero, fix me if wrong)
>>
>> And it seems not so quite make sense to not physically disable the clock
>> when there's already no child users(refcount becomes zero) and i don't
>> think the child clock's default enablement state in HW means a valid user
>> since it's just caused by misalignment between HW and SW clock tree during
>> kernel booting phase which is meaningless.
>> And that seems is why the clk_disable_unused() function exist for fixing
>> this state misalignment issue.
>>
>> > Mike Turquette has been doing
>> > some work to have a way to indicate that certain clocks should be
>> > kept on until client drivers grab them.
>>
>> Sorry i can't see how this help on my issue.
>>
>> > I think it will also make
>> > sure to up refcounts on parent clocks in the middle of the tree
>> > when it figures out that a child clock is enabled. Would that be
>> > all that we need to do to fix this problem?
>> >
>>
>> Then when will we down the refcounts on parent clocks and when to disable it?
>> The current clk_disable_unused() only handle HW clk enable/disable, no
>> refcount operations.
>> Not sure how this is going to fix my issues.
>>
>> And again, as i said above, i don't think it makes much sense to not disable
>> parent only if child is enabled in HW, unless there's more strong reasons.
>>
>> > Also, the subject of this patch and patch 5 are the same. Why?
>> >
>>
>> Sorry, mainly because the full feature of CLK_OPS_PARENT_ON is divided into
>> two patches for better review, their commit message is different.
>> patch 4 is adding support for clk_disable_unused() while patch 5 is for
>> clk_setrate/clk_reparent.
>> I could reform the subject if needed.
>>
>> Regards
>> Dong Aisheng
>>
>
> Hi Mike & Stephen,
>
> Any comments about this?
>
> Regards
> Dong Aisheng
>
>> > --
>> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> > a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[Fabio Estevam]

Hi Mike,

On Thu, Aug 13, 2015 at 7:17 AM, Dong Aisheng
<aisheng.dong@freescale.com> wrote:

> Hi Mike,
>
> It's been quite a long time.
> Can you help pick this?

Yes, we are still getting lots of clock warnings in mx7d in mainline
without Dong's series:

[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:3382
lock_release+0x2f8/0x334()
[    0.000000] releasing a pinned lock
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.5.0-rc5-next-20160222 #256
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
(show_stack+0x18/0x1c)
[    0.000000]  r6:600000d3 r5:ffffffff r4:00000000 r3:00000000
[    0.000000] [<c010b878>] (show_stack) from [<c03dae8c>]
(dump_stack+0xb0/0xe8)
[    0.000000] [<c03daddc>] (dump_stack) from [<c012461c>]
(warn_slowpath_common+0x80/0xbc)
[    0.000000]  r8:c0ad86ec r7:00000009 r6:00000d36 r5:c0169e5c
r4:c0d01ba0 r3:c0d064c0
[    0.000000] [<c012459c>] (warn_slowpath_common) from [<c01246fc>]
(warn_slowpath_fmt+0x38/0x40)
[    0.000000]  r8:c0d06980 r7:00000002 r6:00000001 r5:c0d064c0 r4:ef7c2290
[    0.000000] [<c01246c8>] (warn_slowpath_fmt) from [<c0169e5c>]
(lock_release+0x2f8/0x334)
[    0.000000]  r3:00000026 r2:c0ad9698
[    0.000000] [<c0169b64>] (lock_release) from [<c08e31f8>]
(_raw_spin_unlock_irq+0x20/0x34)
[    0.000000]  r10:c0c75280 r9:ef7c2280 r8:00000000 r7:c0d02b10
r6:00000001 r5:ef7c2280
[    0.000000]  r4:ef7c2280
[    0.000000] [<c08e31d8>] (_raw_spin_unlock_irq) from [<c015219c>]
(dequeue_task_idle+0x14/0x30)
[    0.000000]  r4:ef7c2280 r3:c0152188
[    0.000000] [<c0152188>] (dequeue_task_idle) from [<c0149a50>]
(deactivate_task+0x64/0x68)
[    0.000000]  r4:c0d064c0 r3:c0152188
[    0.000000] [<c01499ec>] (deactivate_task) from [<c08ddd68>]
(__schedule+0x2cc/0x6d8)
[    0.000000]  r6:c0d06804 r5:ef7c2290 r4:c0d064c0 r3:00000002
[    0.000000] [<c08dda9c>] (__schedule) from [<c08de29c>] (schedule+0x3c/0xa0)
[    0.000000]  r10:c0d064c0 r9:c1560db8 r8:00000001 r7:00000000
r6:0006ddd0 r5:00000000
[    0.000000]  r4:0007a120
[    0.000000] [<c08de260>] (schedule) from [<c08e28b0>]
(schedule_hrtimeout_range_clock+0xd4/0x13c)
[    0.000000] [<c08e27dc>] (schedule_hrtimeout_range_clock) from
[<c08e293c>] (schedule_hrtimeout_range+0x24/0x2c)
[    0.000000]  r10:000006d8 r8:000000d8 r7:00000003 r6:ffff8ad1
r5:c0d02100 r4:ef00c040
[    0.000000] [<c08e2918>] (schedule_hrtimeout_range) from
[<c08e2360>] (usleep_range+0x54/0x5c)
[    0.000000] [<c08e230c>] (usleep_range) from [<c068b83c>]
(clk_pllv3_wait_lock+0x7c/0xb4)
[    0.000000] [<c068b7c0>] (clk_pllv3_wait_lock) from [<c068ba4c>]
(clk_pllv3_prepare+0x2c/0x30)
[    0.000000]  r6:c0d5c7f4 r5:00000000 r4:ef007680 r3:f08400f0
[    0.000000] [<c068ba20>] (clk_pllv3_prepare) from [<c0685e08>]
(clk_core_prepare+0xa0/0xcc)
[    0.000000] [<c0685d68>] (clk_core_prepare) from [<c0685e54>]
(clk_prepare+0x20/0x30)
[    0.000000]  r5:00000000 r4:ef00c100
[    0.000000] [<c0685e34>] (clk_prepare) from [<c0c4a700>]
(imx7d_clocks_init+0x6114/0x6198)
[    0.000000]  r4:ef00c100 r3:c0d064c0
[    0.000000] [<c0c445ec>] (imx7d_clocks_init) from [<c0c30c08>]
(of_clk_init+0x138/0x1e8)
[    0.000000]  r10:c0d01f70 r9:00000001 r8:c0d01f68 r7:00000000
r6:ef004340 r5:ef7e3d60
[    0.000000]  r4:00000003
[    0.000000] [<c0c30ad0>] (of_clk_init) from [<c0c049f0>]
(time_init+0x2c/0x38)
[    0.000000]  r10:00000001 r9:410fc075 r8:efffcb80 r7:c0c5ca48
r6:c0d76000 r5:ffffffff
[    0.000000]  r4:00000000
[    0.000000] [<c0c049c4>] (time_init) from [<c0c00bcc>]
(start_kernel+0x26c/0x400)
[    0.000000] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.000000]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
r5:c0d0296c r4:c0d76294
[    0.000000] ---[ end trace cb88537fdc8fa200 ]---
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2576
trace_hardirqs_on_caller+0x1ac/0x1f0()
[    0.000000] DEBUG_LOCKS_WARN_ON(unlikely(early_boot_irqs_disabled))
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.5.0-rc5-next-20160222 #256
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
(show_stack+0x18/0x1c)
[    0.000000]  r6:600000d3 r5:ffffffff r4:00000000 r3:00000000
[    0.000000] [<c010b878>] (show_stack) from [<c03dae8c>]
(dump_stack+0xb0/0xe8)
[    0.000000] [<c03daddc>] (dump_stack) from [<c012461c>]
(warn_slowpath_common+0x80/0xbc)
[    0.000000]  r8:c0ad86ec r7:00000009 r6:00000a10 r5:c016a948
r4:c0d01bc8 r3:c0d064c0
[    0.000000] [<c012459c>] (warn_slowpath_common) from [<c01246fc>]
(warn_slowpath_fmt+0x38/0x40)
[    0.000000]  r8:00000000 r7:c0d02b10 r6:00000001 r5:ef7c2280 r4:c08e3204
[    0.000000] [<c01246c8>] (warn_slowpath_fmt) from [<c016a948>]
(trace_hardirqs_on_caller+0x1ac/0x1f0)
[    0.000000]  r3:c0ad9700 r2:c0ad5cc8
[    0.000000] [<c016a79c>] (trace_hardirqs_on_caller) from
[<c016a9a0>] (trace_hardirqs_on+0x14/0x18)
[    0.000000]  r6:00000001 r5:ef7c2280 r4:ef7c2280 r3:600000d3
[    0.000000] [<c016a98c>] (trace_hardirqs_on) from [<c08e3204>]
(_raw_spin_unlock_irq+0x2c/0x34)
[    0.000000] [<c08e31d8>] (_raw_spin_unlock_irq) from [<c015219c>]
(dequeue_task_idle+0x14/0x30)
[    0.000000]  r4:ef7c2280 r3:c0152188
[    0.000000] [<c0152188>] (dequeue_task_idle) from [<c0149a50>]
(deactivate_task+0x64/0x68)
[    0.000000]  r4:c0d064c0 r3:c0152188
[    0.000000] [<c01499ec>] (deactivate_task) from [<c08ddd68>]
(__schedule+0x2cc/0x6d8)
[    0.000000]  r6:c0d06804 r5:ef7c2290 r4:c0d064c0 r3:00000002
[    0.000000] [<c08dda9c>] (__schedule) from [<c08de29c>] (schedule+0x3c/0xa0)
[    0.000000]  r10:c0d064c0 r9:c1560db8 r8:00000001 r7:00000000
r6:0006ddd0 r5:00000000
[    0.000000]  r4:0007a120
[    0.000000] [<c08de260>] (schedule) from [<c08e28b0>]
(schedule_hrtimeout_range_clock+0xd4/0x13c)
[    0.000000] [<c08e27dc>] (schedule_hrtimeout_range_clock) from
[<c08e293c>] (schedule_hrtimeout_range+0x24/0x2c)
[    0.000000]  r10:000006d8 r8:000000d8 r7:00000003 r6:ffff8ad1
r5:c0d02100 r4:ef00c040
[    0.000000] [<c08e2918>] (schedule_hrtimeout_range) from
[<c08e2360>] (usleep_range+0x54/0x5c)
[    0.000000] [<c08e230c>] (usleep_range) from [<c068b83c>]
(clk_pllv3_wait_lock+0x7c/0xb4)
[    0.000000] [<c068b7c0>] (clk_pllv3_wait_lock) from [<c068ba4c>]
(clk_pllv3_prepare+0x2c/0x30)
[    0.000000]  r6:c0d5c7f4 r5:00000000 r4:ef007680 r3:f08400f0
[    0.000000] [<c068ba20>] (clk_pllv3_prepare) from [<c0685e08>]
(clk_core_prepare+0xa0/0xcc)
[    0.000000] [<c0685d68>] (clk_core_prepare) from [<c0685e54>]
(clk_prepare+0x20/0x30)
[    0.000000]  r5:00000000 r4:ef00c100
[    0.000000] [<c0685e34>] (clk_prepare) from [<c0c4a700>]
(imx7d_clocks_init+0x6114/0x6198)
[    0.000000]  r4:ef00c100 r3:c0d064c0
[    0.000000] [<c0c445ec>] (imx7d_clocks_init) from [<c0c30c08>]
(of_clk_init+0x138/0x1e8)
[    0.000000]  r10:c0d01f70 r9:00000001 r8:c0d01f68 r7:00000000
r6:ef004340 r5:ef7e3d60
[    0.000000]  r4:00000003
[    0.000000] [<c0c30ad0>] (of_clk_init) from [<c0c049f0>]
(time_init+0x2c/0x38)
[    0.000000]  r10:00000001 r9:410fc075 r8:efffcb80 r7:c0c5ca48
r6:c0d76000 r5:ffffffff
[    0.000000]  r4:00000000
[    0.000000] [<c0c049c4>] (time_init) from [<c0c00bcc>]
(start_kernel+0x26c/0x400)
[    0.000000] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.000000]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
r5:c0d0296c r4:c0d76294
[    0.000000] ---[ end trace cb88537fdc8fa201 ]---
[    0.000000] bad: scheduling from the idle thread!
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.5.0-rc5-next-20160222 #256
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
(show_stack+0x18/0x1c)
[    0.000000]  r6:60000053 r5:ffffffff r4:00000000 r3:00000000
[    0.000000] [<c010b878>] (show_stack) from [<c03dae8c>]
(dump_stack+0xb0/0xe8)
[    0.000000] [<c03daddc>] (dump_stack) from [<c01521a8>]
(dequeue_task_idle+0x20/0x30)
[    0.000000]  r8:00000000 r7:c0d02b10 r6:00000001 r5:ef7c2280
r4:ef7c2280 r3:c0d064c0
[    0.000000] [<c0152188>] (dequeue_task_idle) from [<c0149a50>]
(deactivate_task+0x64/0x68)
[    0.000000]  r4:c0d064c0 r3:c0152188
[    0.000000] [<c01499ec>] (deactivate_task) from [<c08ddd68>]
(__schedule+0x2cc/0x6d8)
[    0.000000]  r6:c0d06804 r5:ef7c2290 r4:c0d064c0 r3:00000002
[    0.000000] [<c08dda9c>] (__schedule) from [<c08de29c>] (schedule+0x3c/0xa0)
[    0.000000]  r10:c0d064c0 r9:c1560db8 r8:00000001 r7:00000000
r6:0006ddd0 r5:00000000
[    0.000000]  r4:0007a120
[    0.000000] [<c08de260>] (schedule) from [<c08e28b0>]
(schedule_hrtimeout_range_clock+0xd4/0x13c)
[    0.000000] [<c08e27dc>] (schedule_hrtimeout_range_clock) from
[<c08e293c>] (schedule_hrtimeout_range+0x24/0x2c)
[    0.000000]  r10:000006d8 r8:000000d8 r7:00000003 r6:ffff8ad1
r5:c0d02100 r4:ef00c040
[    0.000000] [<c08e2918>] (schedule_hrtimeout_range) from
[<c08e2360>] (usleep_range+0x54/0x5c)
[    0.000000] [<c08e230c>] (usleep_range) from [<c068b83c>]
(clk_pllv3_wait_lock+0x7c/0xb4)
[    0.000000] [<c068b7c0>] (clk_pllv3_wait_lock) from [<c068ba4c>]
(clk_pllv3_prepare+0x2c/0x30)
[    0.000000]  r6:c0d5c7f4 r5:00000000 r4:ef007680 r3:f08400f0
[    0.000000] [<c068ba20>] (clk_pllv3_prepare) from [<c0685e08>]
(clk_core_prepare+0xa0/0xcc)
[    0.000000] [<c0685d68>] (clk_core_prepare) from [<c0685e54>]
(clk_prepare+0x20/0x30)
[    0.000000]  r5:00000000 r4:ef00c100
[    0.000000] [<c0685e34>] (clk_prepare) from [<c0c4a700>]
(imx7d_clocks_init+0x6114/0x6198)
[    0.000000]  r4:ef00c100 r3:c0d064c0
[    0.000000] [<c0c445ec>] (imx7d_clocks_init) from [<c0c30c08>]
(of_clk_init+0x138/0x1e8)
[    0.000000]  r10:c0d01f70 r9:00000001 r8:c0d01f68 r7:00000000
r6:ef004340 r5:ef7e3d60
[    0.000000]  r4:00000003
[    0.000000] [<c0c30ad0>] (of_clk_init) from [<c0c049f0>]
(time_init+0x2c/0x38)
[    0.000000]  r10:00000001 r9:410fc075 r8:efffcb80 r7:c0c5ca48
r6:c0d76000 r5:ffffffff
[    0.000000]  r4:00000000
[    0.000000] [<c0c049c4>] (time_init) from [<c0c00bcc>]
(start_kernel+0x26c/0x400)
[    0.000000] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.000000]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
r5:c0d0296c r4:c0d76294
[    0.000000] Architected cp15 timer(s) running at 8.00MHz (virt).
[    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff
max_cycles: 0x1d854df40, max_idle_ns: 440795202120 ns
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179
sched_clock_register+0x44/0x1f8()
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.5.0-rc5-next-20160222 #256
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
(show_stack+0x18/0x1c)
[    0.000000]  r6:60000053 r5:ffffffff r4:00000000 r3:00000000
[    0.000000] [<c010b878>] (show_stack) from [<c03dae8c>]
(dump_stack+0xb0/0xe8)
[    0.000000] [<c03daddc>] (dump_stack) from [<c012461c>]
(warn_slowpath_common+0x80/0xbc)
[    0.000000]  r8:c0add580 r7:00000009 r6:000000b3 r5:c0c13a40
r4:00000000 r3:c0d064c0
[    0.000000] [<c012459c>] (warn_slowpath_common) from [<c012467c>]
(warn_slowpath_null+0x24/0x2c)
[    0.000000]  r8:007a1200 r7:c155f688 r6:c0d5b2c0 r5:9ffdbd98 r4:a3aaa0c5
[    0.000000] [<c0124658>] (warn_slowpath_null) from [<c0c13a40>]
(sched_clock_register+0x44/0x1f8)
[    0.000000] [<c0c139fc>] (sched_clock_register) from [<c0c2de48>]
(arch_timer_common_init+0x1d4/0x234)
[    0.000000]  r10:c0b5a5b4 r9:00000008 r8:c0ae2f94 r7:c155f688
r6:c0d5b2c0 r5:9ffdbd98
[    0.000000]  r4:a3aaa0c5
[    0.000000] [<c0c2dc74>] (arch_timer_common_init) from [<c0c2e174>]
(arch_timer_of_init+0x2cc/0x30c)
[    0.000000]  r10:00000001 r9:410fc075 r8:efffcb80 r7:c0c5ca48
r6:00000001 r5:00000000
[    0.000000]  r4:00000012
[    0.000000] [<c0c2dea8>] (arch_timer_of_init) from [<c0c2db24>]
(clocksource_probe+0x50/0x94)
[    0.000000]  r6:00000001 r5:c0d01f84 r4:ef7dae9c r3:c0c2dea8
[    0.000000] [<c0c2dad4>] (clocksource_probe) from [<c0c049f4>]
(time_init+0x30/0x38)
[    0.000000]  r6:c0d76000 r5:ffffffff r4:00000000
[    0.000000] [<c0c049c4>] (time_init) from [<c0c00bcc>]
(start_kernel+0x26c/0x400)
[    0.000000] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.000000]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
r5:c0d0296c r4:c0d76294
[    0.000000] ---[ end trace cb88537fdc8fa202 ]---
[    0.000007] sched_clock: 56 bits at 8MHz, resolution 125ns, wraps
every 2199023255500ns
[    0.000023] Switching to timer-based delay loop, resolution 125ns
[    0.000603] Switching to timer-based delay loop, resolution 41ns
[    0.000614] ------------[ cut here ]------------
[    0.000632] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179
sched_clock_register+0x44/0x1f8()
[    0.000641] Modules linked in:
[    0.000658] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.5.0-rc5-next-20160222 #256
[    0.000668] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000676] Backtrace:
[    0.000700] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
(show_stack+0x18/0x1c)
[    0.000710]  r6:60000053 r5:ffffffff r4:00000000 r3:00000000
[    0.000746] [<c010b878>] (show_stack) from [<c03dae8c>]
(dump_stack+0xb0/0xe8)
[    0.000763] [<c03daddc>] (dump_stack) from [<c012461c>]
(warn_slowpath_common+0x80/0xbc)
[    0.000773]  r8:c0add580 r7:00000009 r6:000000b3 r5:c0c13a40
r4:00000000 r3:c0d064c0
[    0.000813] [<c012459c>] (warn_slowpath_common) from [<c012467c>]
(warn_slowpath_null+0x24/0x2c)
[    0.000823]  r8:016e3600 r7:f0880024 r6:016e3600 r5:c155f6f0 r4:ef002400
[    0.000863] [<c0124658>] (warn_slowpath_null) from [<c0c13a40>]
(sched_clock_register+0x44/0x1f8)
[    0.000881] [<c0c139fc>] (sched_clock_register) from [<c0c2e8a4>]
(_mxc_timer_init+0x148/0x240)
[    0.000891]  r10:00000001 r9:410fc075 r8:c0b5a984 r7:f0880024
r6:016e3600 r5:c155f6f0
[    0.000926]  r4:ef002400
[    0.000945] [<c0c2e75c>] (_mxc_timer_init) from [<c0c2ea54>]
(mxc_timer_init_dt+0xb8/0xe8)
[    0.000955]  r8:efffcb80 r7:00000003 r6:00000000 r5:ef7e084c r4:ef002400
[    0.000996] [<c0c2e99c>] (mxc_timer_init_dt) from [<c0c2ea98>]
(imx6dl_timer_init_dt+0x14/0x18)
[    0.001006]  r7:c0c5ca48 r6:00000002 r5:c0d01f84 r4:ef7e084c
[    0.001041] [<c0c2ea84>] (imx6dl_timer_init_dt) from [<c0c2db24>]
(clocksource_probe+0x50/0x94)
[    0.001061] [<c0c2dad4>] (clocksource_probe) from [<c0c049f4>]
(time_init+0x30/0x38)
[    0.001071]  r6:c0d76000 r5:ffffffff r4:00000000
[    0.001102] [<c0c049c4>] (time_init) from [<c0c00bcc>]
(start_kernel+0x26c/0x400)
[    0.001119] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.001128]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
r5:c0d0296c r4:c0d76294
[    0.001164] ---[ end trace cb88537fdc8fa203 ]---
[    0.001183] sched_clock: 32 bits at 24MHz, resolution 41ns, wraps
every 89478484971ns
[    0.001201] clocksource: mxc_timer1: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 79635851949 ns
[    0.002139] ------------[ cut here ]------------
[    0.002161] WARNING: CPU: 0 PID: 0 at init/main.c:593
start_kernel+0x298/0x400()
[    0.002170] Interrupts were enabled early
[    0.002179] Modules linked in:
[    0.002195] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.5.0-rc5-next-20160222 #256
[    0.002205] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.002214] Backtrace:
[    0.002237] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
(show_stack+0x18/0x1c)
[    0.002247]  r6:60000053 r5:ffffffff r4:00000000 r3:00000000
[    0.002283] [<c010b878>] (show_stack) from [<c03dae8c>]
(dump_stack+0xb0/0xe8)
[    0.002300] [<c03daddc>] (dump_stack) from [<c012461c>]
(warn_slowpath_common+0x80/0xbc)
[    0.002310]  r8:c0ace4a0 r7:00000009 r6:00000251 r5:c0c00bf8
r4:c0d01f98 r3:c0d064c0
[    0.002351] [<c012459c>] (warn_slowpath_common) from [<c01246fc>]
(warn_slowpath_fmt+0x38/0x40)
[    0.002361]  r8:efffcb80 r7:c0c5ca48 r6:c0d76000 r5:ffffffff r4:00000000
[    0.002402] [<c01246c8>] (warn_slowpath_fmt) from [<c0c00bf8>]
(start_kernel+0x298/0x400)
[    0.002413]  r3:60000053 r2:c0ace4e0
[    0.002436] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.002446]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
r5:c0d0296c r4:c0d76294
[    0.002482] ---[ end trace cb88537fdc8fa204 ]---
[    0.002594] Console: colour dummy device 80x30
[    0.002607] Lock dependency validator: Copyright (c) 2006 Red Hat,
Inc., Ingo Molnar
[    0.002616] ... MAX_LOCKDEP_SUBCLASSES:  8

Re: [PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[Dong Aisheng]

On Mon, Feb 22, 2016 at 10:55 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Mike,
>
> On Thu, Aug 13, 2015 at 7:17 AM, Dong Aisheng
> <aisheng.dong@freescale.com> wrote:
>
>> Hi Mike,
>>
>> It's been quite a long time.
>> Can you help pick this?
>
> Yes, we are still getting lots of clock warnings in mx7d in mainline
> without Dong's series:
>

Hi Mike & Stephen,

It's been pending a long time.

What's your fix plan?
Can we merge this patch series first?
I believe there're also other platforms need this fix too.

Regards
Dong Aisheng


> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:3382
> lock_release+0x2f8/0x334()
> [    0.000000] releasing a pinned lock
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.5.0-rc5-next-20160222 #256
> [    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
> [    0.000000] Backtrace:
> [    0.000000] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
> (show_stack+0x18/0x1c)
> [    0.000000]  r6:600000d3 r5:ffffffff r4:00000000 r3:00000000
> [    0.000000] [<c010b878>] (show_stack) from [<c03dae8c>]
> (dump_stack+0xb0/0xe8)
> [    0.000000] [<c03daddc>] (dump_stack) from [<c012461c>]
> (warn_slowpath_common+0x80/0xbc)
> [    0.000000]  r8:c0ad86ec r7:00000009 r6:00000d36 r5:c0169e5c
> r4:c0d01ba0 r3:c0d064c0
> [    0.000000] [<c012459c>] (warn_slowpath_common) from [<c01246fc>]
> (warn_slowpath_fmt+0x38/0x40)
> [    0.000000]  r8:c0d06980 r7:00000002 r6:00000001 r5:c0d064c0 r4:ef7c2290
> [    0.000000] [<c01246c8>] (warn_slowpath_fmt) from [<c0169e5c>]
> (lock_release+0x2f8/0x334)
> [    0.000000]  r3:00000026 r2:c0ad9698
> [    0.000000] [<c0169b64>] (lock_release) from [<c08e31f8>]
> (_raw_spin_unlock_irq+0x20/0x34)
> [    0.000000]  r10:c0c75280 r9:ef7c2280 r8:00000000 r7:c0d02b10
> r6:00000001 r5:ef7c2280
> [    0.000000]  r4:ef7c2280
> [    0.000000] [<c08e31d8>] (_raw_spin_unlock_irq) from [<c015219c>]
> (dequeue_task_idle+0x14/0x30)
> [    0.000000]  r4:ef7c2280 r3:c0152188
> [    0.000000] [<c0152188>] (dequeue_task_idle) from [<c0149a50>]
> (deactivate_task+0x64/0x68)
> [    0.000000]  r4:c0d064c0 r3:c0152188
> [    0.000000] [<c01499ec>] (deactivate_task) from [<c08ddd68>]
> (__schedule+0x2cc/0x6d8)
> [    0.000000]  r6:c0d06804 r5:ef7c2290 r4:c0d064c0 r3:00000002
> [    0.000000] [<c08dda9c>] (__schedule) from [<c08de29c>] (schedule+0x3c/0xa0)
> [    0.000000]  r10:c0d064c0 r9:c1560db8 r8:00000001 r7:00000000
> r6:0006ddd0 r5:00000000
> [    0.000000]  r4:0007a120
> [    0.000000] [<c08de260>] (schedule) from [<c08e28b0>]
> (schedule_hrtimeout_range_clock+0xd4/0x13c)
> [    0.000000] [<c08e27dc>] (schedule_hrtimeout_range_clock) from
> [<c08e293c>] (schedule_hrtimeout_range+0x24/0x2c)
> [    0.000000]  r10:000006d8 r8:000000d8 r7:00000003 r6:ffff8ad1
> r5:c0d02100 r4:ef00c040
> [    0.000000] [<c08e2918>] (schedule_hrtimeout_range) from
> [<c08e2360>] (usleep_range+0x54/0x5c)
> [    0.000000] [<c08e230c>] (usleep_range) from [<c068b83c>]
> (clk_pllv3_wait_lock+0x7c/0xb4)
> [    0.000000] [<c068b7c0>] (clk_pllv3_wait_lock) from [<c068ba4c>]
> (clk_pllv3_prepare+0x2c/0x30)
> [    0.000000]  r6:c0d5c7f4 r5:00000000 r4:ef007680 r3:f08400f0
> [    0.000000] [<c068ba20>] (clk_pllv3_prepare) from [<c0685e08>]
> (clk_core_prepare+0xa0/0xcc)
> [    0.000000] [<c0685d68>] (clk_core_prepare) from [<c0685e54>]
> (clk_prepare+0x20/0x30)
> [    0.000000]  r5:00000000 r4:ef00c100
> [    0.000000] [<c0685e34>] (clk_prepare) from [<c0c4a700>]
> (imx7d_clocks_init+0x6114/0x6198)
> [    0.000000]  r4:ef00c100 r3:c0d064c0
> [    0.000000] [<c0c445ec>] (imx7d_clocks_init) from [<c0c30c08>]
> (of_clk_init+0x138/0x1e8)
> [    0.000000]  r10:c0d01f70 r9:00000001 r8:c0d01f68 r7:00000000
> r6:ef004340 r5:ef7e3d60
> [    0.000000]  r4:00000003
> [    0.000000] [<c0c30ad0>] (of_clk_init) from [<c0c049f0>]
> (time_init+0x2c/0x38)
> [    0.000000]  r10:00000001 r9:410fc075 r8:efffcb80 r7:c0c5ca48
> r6:c0d76000 r5:ffffffff
> [    0.000000]  r4:00000000
> [    0.000000] [<c0c049c4>] (time_init) from [<c0c00bcc>]
> (start_kernel+0x26c/0x400)
> [    0.000000] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
> [    0.000000]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
> r5:c0d0296c r4:c0d76294
> [    0.000000] ---[ end trace cb88537fdc8fa200 ]---
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2576
> trace_hardirqs_on_caller+0x1ac/0x1f0()
> [    0.000000] DEBUG_LOCKS_WARN_ON(unlikely(early_boot_irqs_disabled))
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
> 4.5.0-rc5-next-20160222 #256
> [    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
> [    0.000000] Backtrace:
> [    0.000000] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
> (show_stack+0x18/0x1c)
> [    0.000000]  r6:600000d3 r5:ffffffff r4:00000000 r3:00000000
> [    0.000000] [<c010b878>] (show_stack) from [<c03dae8c>]
> (dump_stack+0xb0/0xe8)
> [    0.000000] [<c03daddc>] (dump_stack) from [<c012461c>]
> (warn_slowpath_common+0x80/0xbc)
> [    0.000000]  r8:c0ad86ec r7:00000009 r6:00000a10 r5:c016a948
> r4:c0d01bc8 r3:c0d064c0
> [    0.000000] [<c012459c>] (warn_slowpath_common) from [<c01246fc>]
> (warn_slowpath_fmt+0x38/0x40)
> [    0.000000]  r8:00000000 r7:c0d02b10 r6:00000001 r5:ef7c2280 r4:c08e3204
> [    0.000000] [<c01246c8>] (warn_slowpath_fmt) from [<c016a948>]
> (trace_hardirqs_on_caller+0x1ac/0x1f0)
> [    0.000000]  r3:c0ad9700 r2:c0ad5cc8
> [    0.000000] [<c016a79c>] (trace_hardirqs_on_caller) from
> [<c016a9a0>] (trace_hardirqs_on+0x14/0x18)
> [    0.000000]  r6:00000001 r5:ef7c2280 r4:ef7c2280 r3:600000d3
> [    0.000000] [<c016a98c>] (trace_hardirqs_on) from [<c08e3204>]
> (_raw_spin_unlock_irq+0x2c/0x34)
> [    0.000000] [<c08e31d8>] (_raw_spin_unlock_irq) from [<c015219c>]
> (dequeue_task_idle+0x14/0x30)
> [    0.000000]  r4:ef7c2280 r3:c0152188
> [    0.000000] [<c0152188>] (dequeue_task_idle) from [<c0149a50>]
> (deactivate_task+0x64/0x68)
> [    0.000000]  r4:c0d064c0 r3:c0152188
> [    0.000000] [<c01499ec>] (deactivate_task) from [<c08ddd68>]
> (__schedule+0x2cc/0x6d8)
> [    0.000000]  r6:c0d06804 r5:ef7c2290 r4:c0d064c0 r3:00000002
> [    0.000000] [<c08dda9c>] (__schedule) from [<c08de29c>] (schedule+0x3c/0xa0)
> [    0.000000]  r10:c0d064c0 r9:c1560db8 r8:00000001 r7:00000000
> r6:0006ddd0 r5:00000000
> [    0.000000]  r4:0007a120
> [    0.000000] [<c08de260>] (schedule) from [<c08e28b0>]
> (schedule_hrtimeout_range_clock+0xd4/0x13c)
> [    0.000000] [<c08e27dc>] (schedule_hrtimeout_range_clock) from
> [<c08e293c>] (schedule_hrtimeout_range+0x24/0x2c)
> [    0.000000]  r10:000006d8 r8:000000d8 r7:00000003 r6:ffff8ad1
> r5:c0d02100 r4:ef00c040
> [    0.000000] [<c08e2918>] (schedule_hrtimeout_range) from
> [<c08e2360>] (usleep_range+0x54/0x5c)
> [    0.000000] [<c08e230c>] (usleep_range) from [<c068b83c>]
> (clk_pllv3_wait_lock+0x7c/0xb4)
> [    0.000000] [<c068b7c0>] (clk_pllv3_wait_lock) from [<c068ba4c>]
> (clk_pllv3_prepare+0x2c/0x30)
> [    0.000000]  r6:c0d5c7f4 r5:00000000 r4:ef007680 r3:f08400f0
> [    0.000000] [<c068ba20>] (clk_pllv3_prepare) from [<c0685e08>]
> (clk_core_prepare+0xa0/0xcc)
> [    0.000000] [<c0685d68>] (clk_core_prepare) from [<c0685e54>]
> (clk_prepare+0x20/0x30)
> [    0.000000]  r5:00000000 r4:ef00c100
> [    0.000000] [<c0685e34>] (clk_prepare) from [<c0c4a700>]
> (imx7d_clocks_init+0x6114/0x6198)
> [    0.000000]  r4:ef00c100 r3:c0d064c0
> [    0.000000] [<c0c445ec>] (imx7d_clocks_init) from [<c0c30c08>]
> (of_clk_init+0x138/0x1e8)
> [    0.000000]  r10:c0d01f70 r9:00000001 r8:c0d01f68 r7:00000000
> r6:ef004340 r5:ef7e3d60
> [    0.000000]  r4:00000003
> [    0.000000] [<c0c30ad0>] (of_clk_init) from [<c0c049f0>]
> (time_init+0x2c/0x38)
> [    0.000000]  r10:00000001 r9:410fc075 r8:efffcb80 r7:c0c5ca48
> r6:c0d76000 r5:ffffffff
> [    0.000000]  r4:00000000
> [    0.000000] [<c0c049c4>] (time_init) from [<c0c00bcc>]
> (start_kernel+0x26c/0x400)
> [    0.000000] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
> [    0.000000]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
> r5:c0d0296c r4:c0d76294
> [    0.000000] ---[ end trace cb88537fdc8fa201 ]---
> [    0.000000] bad: scheduling from the idle thread!
> [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
> 4.5.0-rc5-next-20160222 #256
> [    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
> [    0.000000] Backtrace:
> [    0.000000] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
> (show_stack+0x18/0x1c)
> [    0.000000]  r6:60000053 r5:ffffffff r4:00000000 r3:00000000
> [    0.000000] [<c010b878>] (show_stack) from [<c03dae8c>]
> (dump_stack+0xb0/0xe8)
> [    0.000000] [<c03daddc>] (dump_stack) from [<c01521a8>]
> (dequeue_task_idle+0x20/0x30)
> [    0.000000]  r8:00000000 r7:c0d02b10 r6:00000001 r5:ef7c2280
> r4:ef7c2280 r3:c0d064c0
> [    0.000000] [<c0152188>] (dequeue_task_idle) from [<c0149a50>]
> (deactivate_task+0x64/0x68)
> [    0.000000]  r4:c0d064c0 r3:c0152188
> [    0.000000] [<c01499ec>] (deactivate_task) from [<c08ddd68>]
> (__schedule+0x2cc/0x6d8)
> [    0.000000]  r6:c0d06804 r5:ef7c2290 r4:c0d064c0 r3:00000002
> [    0.000000] [<c08dda9c>] (__schedule) from [<c08de29c>] (schedule+0x3c/0xa0)
> [    0.000000]  r10:c0d064c0 r9:c1560db8 r8:00000001 r7:00000000
> r6:0006ddd0 r5:00000000
> [    0.000000]  r4:0007a120
> [    0.000000] [<c08de260>] (schedule) from [<c08e28b0>]
> (schedule_hrtimeout_range_clock+0xd4/0x13c)
> [    0.000000] [<c08e27dc>] (schedule_hrtimeout_range_clock) from
> [<c08e293c>] (schedule_hrtimeout_range+0x24/0x2c)
> [    0.000000]  r10:000006d8 r8:000000d8 r7:00000003 r6:ffff8ad1
> r5:c0d02100 r4:ef00c040
> [    0.000000] [<c08e2918>] (schedule_hrtimeout_range) from
> [<c08e2360>] (usleep_range+0x54/0x5c)
> [    0.000000] [<c08e230c>] (usleep_range) from [<c068b83c>]
> (clk_pllv3_wait_lock+0x7c/0xb4)
> [    0.000000] [<c068b7c0>] (clk_pllv3_wait_lock) from [<c068ba4c>]
> (clk_pllv3_prepare+0x2c/0x30)
> [    0.000000]  r6:c0d5c7f4 r5:00000000 r4:ef007680 r3:f08400f0
> [    0.000000] [<c068ba20>] (clk_pllv3_prepare) from [<c0685e08>]
> (clk_core_prepare+0xa0/0xcc)
> [    0.000000] [<c0685d68>] (clk_core_prepare) from [<c0685e54>]
> (clk_prepare+0x20/0x30)
> [    0.000000]  r5:00000000 r4:ef00c100
> [    0.000000] [<c0685e34>] (clk_prepare) from [<c0c4a700>]
> (imx7d_clocks_init+0x6114/0x6198)
> [    0.000000]  r4:ef00c100 r3:c0d064c0
> [    0.000000] [<c0c445ec>] (imx7d_clocks_init) from [<c0c30c08>]
> (of_clk_init+0x138/0x1e8)
> [    0.000000]  r10:c0d01f70 r9:00000001 r8:c0d01f68 r7:00000000
> r6:ef004340 r5:ef7e3d60
> [    0.000000]  r4:00000003
> [    0.000000] [<c0c30ad0>] (of_clk_init) from [<c0c049f0>]
> (time_init+0x2c/0x38)
> [    0.000000]  r10:00000001 r9:410fc075 r8:efffcb80 r7:c0c5ca48
> r6:c0d76000 r5:ffffffff
> [    0.000000]  r4:00000000
> [    0.000000] [<c0c049c4>] (time_init) from [<c0c00bcc>]
> (start_kernel+0x26c/0x400)
> [    0.000000] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
> [    0.000000]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
> r5:c0d0296c r4:c0d76294
> [    0.000000] Architected cp15 timer(s) running at 8.00MHz (virt).
> [    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff
> max_cycles: 0x1d854df40, max_idle_ns: 440795202120 ns
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179
> sched_clock_register+0x44/0x1f8()
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
> 4.5.0-rc5-next-20160222 #256
> [    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
> [    0.000000] Backtrace:
> [    0.000000] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
> (show_stack+0x18/0x1c)
> [    0.000000]  r6:60000053 r5:ffffffff r4:00000000 r3:00000000
> [    0.000000] [<c010b878>] (show_stack) from [<c03dae8c>]
> (dump_stack+0xb0/0xe8)
> [    0.000000] [<c03daddc>] (dump_stack) from [<c012461c>]
> (warn_slowpath_common+0x80/0xbc)
> [    0.000000]  r8:c0add580 r7:00000009 r6:000000b3 r5:c0c13a40
> r4:00000000 r3:c0d064c0
> [    0.000000] [<c012459c>] (warn_slowpath_common) from [<c012467c>]
> (warn_slowpath_null+0x24/0x2c)
> [    0.000000]  r8:007a1200 r7:c155f688 r6:c0d5b2c0 r5:9ffdbd98 r4:a3aaa0c5
> [    0.000000] [<c0124658>] (warn_slowpath_null) from [<c0c13a40>]
> (sched_clock_register+0x44/0x1f8)
> [    0.000000] [<c0c139fc>] (sched_clock_register) from [<c0c2de48>]
> (arch_timer_common_init+0x1d4/0x234)
> [    0.000000]  r10:c0b5a5b4 r9:00000008 r8:c0ae2f94 r7:c155f688
> r6:c0d5b2c0 r5:9ffdbd98
> [    0.000000]  r4:a3aaa0c5
> [    0.000000] [<c0c2dc74>] (arch_timer_common_init) from [<c0c2e174>]
> (arch_timer_of_init+0x2cc/0x30c)
> [    0.000000]  r10:00000001 r9:410fc075 r8:efffcb80 r7:c0c5ca48
> r6:00000001 r5:00000000
> [    0.000000]  r4:00000012
> [    0.000000] [<c0c2dea8>] (arch_timer_of_init) from [<c0c2db24>]
> (clocksource_probe+0x50/0x94)
> [    0.000000]  r6:00000001 r5:c0d01f84 r4:ef7dae9c r3:c0c2dea8
> [    0.000000] [<c0c2dad4>] (clocksource_probe) from [<c0c049f4>]
> (time_init+0x30/0x38)
> [    0.000000]  r6:c0d76000 r5:ffffffff r4:00000000
> [    0.000000] [<c0c049c4>] (time_init) from [<c0c00bcc>]
> (start_kernel+0x26c/0x400)
> [    0.000000] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
> [    0.000000]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
> r5:c0d0296c r4:c0d76294
> [    0.000000] ---[ end trace cb88537fdc8fa202 ]---
> [    0.000007] sched_clock: 56 bits at 8MHz, resolution 125ns, wraps
> every 2199023255500ns
> [    0.000023] Switching to timer-based delay loop, resolution 125ns
> [    0.000603] Switching to timer-based delay loop, resolution 41ns
> [    0.000614] ------------[ cut here ]------------
> [    0.000632] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179
> sched_clock_register+0x44/0x1f8()
> [    0.000641] Modules linked in:
> [    0.000658] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
> 4.5.0-rc5-next-20160222 #256
> [    0.000668] Hardware name: Freescale i.MX7 Dual (Device Tree)
> [    0.000676] Backtrace:
> [    0.000700] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
> (show_stack+0x18/0x1c)
> [    0.000710]  r6:60000053 r5:ffffffff r4:00000000 r3:00000000
> [    0.000746] [<c010b878>] (show_stack) from [<c03dae8c>]
> (dump_stack+0xb0/0xe8)
> [    0.000763] [<c03daddc>] (dump_stack) from [<c012461c>]
> (warn_slowpath_common+0x80/0xbc)
> [    0.000773]  r8:c0add580 r7:00000009 r6:000000b3 r5:c0c13a40
> r4:00000000 r3:c0d064c0
> [    0.000813] [<c012459c>] (warn_slowpath_common) from [<c012467c>]
> (warn_slowpath_null+0x24/0x2c)
> [    0.000823]  r8:016e3600 r7:f0880024 r6:016e3600 r5:c155f6f0 r4:ef002400
> [    0.000863] [<c0124658>] (warn_slowpath_null) from [<c0c13a40>]
> (sched_clock_register+0x44/0x1f8)
> [    0.000881] [<c0c139fc>] (sched_clock_register) from [<c0c2e8a4>]
> (_mxc_timer_init+0x148/0x240)
> [    0.000891]  r10:00000001 r9:410fc075 r8:c0b5a984 r7:f0880024
> r6:016e3600 r5:c155f6f0
> [    0.000926]  r4:ef002400
> [    0.000945] [<c0c2e75c>] (_mxc_timer_init) from [<c0c2ea54>]
> (mxc_timer_init_dt+0xb8/0xe8)
> [    0.000955]  r8:efffcb80 r7:00000003 r6:00000000 r5:ef7e084c r4:ef002400
> [    0.000996] [<c0c2e99c>] (mxc_timer_init_dt) from [<c0c2ea98>]
> (imx6dl_timer_init_dt+0x14/0x18)
> [    0.001006]  r7:c0c5ca48 r6:00000002 r5:c0d01f84 r4:ef7e084c
> [    0.001041] [<c0c2ea84>] (imx6dl_timer_init_dt) from [<c0c2db24>]
> (clocksource_probe+0x50/0x94)
> [    0.001061] [<c0c2dad4>] (clocksource_probe) from [<c0c049f4>]
> (time_init+0x30/0x38)
> [    0.001071]  r6:c0d76000 r5:ffffffff r4:00000000
> [    0.001102] [<c0c049c4>] (time_init) from [<c0c00bcc>]
> (start_kernel+0x26c/0x400)
> [    0.001119] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
> [    0.001128]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
> r5:c0d0296c r4:c0d76294
> [    0.001164] ---[ end trace cb88537fdc8fa203 ]---
> [    0.001183] sched_clock: 32 bits at 24MHz, resolution 41ns, wraps
> every 89478484971ns
> [    0.001201] clocksource: mxc_timer1: mask: 0xffffffff max_cycles:
> 0xffffffff, max_idle_ns: 79635851949 ns
> [    0.002139] ------------[ cut here ]------------
> [    0.002161] WARNING: CPU: 0 PID: 0 at init/main.c:593
> start_kernel+0x298/0x400()
> [    0.002170] Interrupts were enabled early
> [    0.002179] Modules linked in:
> [    0.002195] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
> 4.5.0-rc5-next-20160222 #256
> [    0.002205] Hardware name: Freescale i.MX7 Dual (Device Tree)
> [    0.002214] Backtrace:
> [    0.002237] [<c010b6f4>] (dump_backtrace) from [<c010b890>]
> (show_stack+0x18/0x1c)
> [    0.002247]  r6:60000053 r5:ffffffff r4:00000000 r3:00000000
> [    0.002283] [<c010b878>] (show_stack) from [<c03dae8c>]
> (dump_stack+0xb0/0xe8)
> [    0.002300] [<c03daddc>] (dump_stack) from [<c012461c>]
> (warn_slowpath_common+0x80/0xbc)
> [    0.002310]  r8:c0ace4a0 r7:00000009 r6:00000251 r5:c0c00bf8
> r4:c0d01f98 r3:c0d064c0
> [    0.002351] [<c012459c>] (warn_slowpath_common) from [<c01246fc>]
> (warn_slowpath_fmt+0x38/0x40)
> [    0.002361]  r8:efffcb80 r7:c0c5ca48 r6:c0d76000 r5:ffffffff r4:00000000
> [    0.002402] [<c01246c8>] (warn_slowpath_fmt) from [<c0c00bf8>]
> (start_kernel+0x298/0x400)
> [    0.002413]  r3:60000053 r2:c0ace4e0
> [    0.002436] [<c0c00960>] (start_kernel) from [<8000807c>] (0x8000807c)
> [    0.002446]  r10:00000000 r8:8000406a r7:c0d07e4c r6:c0c5ca44
> r5:c0d0296c r4:c0d76294
> [    0.002482] ---[ end trace cb88537fdc8fa204 ]---
> [    0.002594] Console: colour dummy device 80x30
> [    0.002607] Lock dependency validator: Copyright (c) 2006 Red Hat,
> Inc., Ingo Molnar
> [    0.002616] ... MAX_LOCKDEP_SUBCLASSES:  8
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[Fabio Estevam]

Hi Dong,

On Tue, Jul 28, 2015 at 10:19 AM, Dong Aisheng
<aisheng.dong@freescale.com> wrote:
> This patch series adds support in clock framework for clocks which operations
> requires its parent clock is on.
>
> Such clock type is initially met on Freescale i.MX7D platform that all clocks
> operations, including enable/disable, rate change and re-parent, requires its
> parent clock on. No sure if any other SoC has the similar clock type.
>
> Current clock core can not support such type of clock well.
>
> This patch introduce a new flag CLK_SET_PARENT_ON to handle this special case
> in clock core that enable its parent clock firstly for each operation and disable
> it later after operation complete.
>
> The most special case is for set_parent() operation which requires both parent,
> old one and new one, to be enabled at the same time during the operation.
>
> The patch series is based on for-next branch of Michael's git:
> git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git

Do you plan to resend this series?

Re: [PATCH V3 0/5] clk: support clocks which requires parent clock on during operation

[Dong Aisheng]

Hi Fabio,

On Thu, Mar 31, 2016 at 12:00 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Dong,
>
> On Tue, Jul 28, 2015 at 10:19 AM, Dong Aisheng
> <aisheng.dong@freescale.com> wrote:
>> This patch series adds support in clock framework for clocks which operations
>> requires its parent clock is on.
>>
>> Such clock type is initially met on Freescale i.MX7D platform that all clocks
>> operations, including enable/disable, rate change and re-parent, requires its
>> parent clock on. No sure if any other SoC has the similar clock type.
>>
>> Current clock core can not support such type of clock well.
>>
>> This patch introduce a new flag CLK_SET_PARENT_ON to handle this special case
>> in clock core that enable its parent clock firstly for each operation and disable
>> it later after operation complete.
>>
>> The most special case is for set_parent() operation which requires both parent,
>> old one and new one, to be enabled at the same time during the operation.
>>
>> The patch series is based on for-next branch of Michael's git:
>> git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
>
> Do you plan to resend this series?

It is in my TODO list.
I'm currently busy with some other items.
Will be able to do it in a few days.

Regards
Dong Aisheng

> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html