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

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 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

__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 <redacted>
Cc: Stephen Boyd <redacted>
Suggested-by: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>
---
 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 659f2b0..0e813ea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1159,14 +1159,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

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 <redacted>
Cc: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>
---
 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 597d6d2..44a2f55 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -736,6 +736,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)

@@ -760,6 +763,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 5378c2a..b37d9f3 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h

@@ -31,6 +31,11 @@
 #define CLK_GET_RATE_NOCACHE	BIT(6) /* do not use the cached clk rate */
 #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 */
+/*
+ * parent clock must be on across any operation including
+ * clock gate/ungate, rate change and re-parent
+ */
+#define CLK_OPS_PARENT_ON	BIT(9)
 
 struct clk_hw;
 struct clk_core;

-- 
1.9.1

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 <redacted>
Cc: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>
---
 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 44a2f55..348f371 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1133,7 +1133,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

@@ -1148,13 +1148,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 */

@@ -1169,18 +1173,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);
 	}
 }
 

@@ -1419,13 +1421,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);

@@ -1446,6 +1452,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);
 

@@ -1453,6 +1462,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

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 <redacted>
Cc: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>
---
 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 2f44c8c..597d6d2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -162,104 +162,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)

@@ -799,6 +701,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 unsigned long clk_core_round_rate_nolock(struct clk_core *core,
 						unsigned long rate,
 						unsigned long min_rate,

-- 
1.9.1

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 <redacted>
Cc: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>
---
 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 0e813ea..2f44c8c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -565,6 +565,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

@@ -581,9 +588,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);
 

@@ -619,6 +624,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

@@ -633,16 +649,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);
 

@@ -669,6 +679,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

@@ -683,14 +702,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);
 

@@ -729,6 +744,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

@@ -744,19 +771,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 unsigned long clk_core_round_rate_nolock(struct clk_core *core,
 						unsigned long rate,

-- 
1.9.1

Hi Stephen,

On Thu, May 14, 2015 at 09:28:58PM +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 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

Can you help review this updated series?

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

-- 
1.9.1

Regards
Dong Aisheng

Quoting Dong Aisheng (2015-05-14 06:28:59)

__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 <redacted>
Cc: Stephen Boyd <redacted>
Suggested-by: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>

Patch looks good to me.

Also just noticed that the second argument to __clk_set_parent_after is
never used :-(

Regards,
Mike

On Wed, May 27, 2015 at 09:09:19PM -0700, Michael Turquette wrote:

Quoting Dong Aisheng (2015-05-14 06:28:59)

quoted

__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 <redacted>
Cc: Stephen Boyd <redacted>
Suggested-by: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>

Patch looks good to me.

Thanks for the review.

Also just noticed that the second argument to __clk_set_parent_after is
never used :-(

Yes, before this patch series, it's never used. :)

Regards
Dong Aisheng

Regards,
Mike

quoted

---
 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 659f2b0..0e813ea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1159,14 +1159,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

Hi Mike,

On Wed, May 27, 2015 at 09:09:19PM -0700, Michael Turquette wrote:

Quoting Dong Aisheng (2015-05-14 06:28:59)

quoted

__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 <redacted>
Cc: Stephen Boyd <redacted>
Suggested-by: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>

Patch looks good to me.

Also just noticed that the second argument to __clk_set_parent_after is
never used :-(

Ping...

I did not see this patch series in your tree for v4.2 rc.
Is this got missed?

Regards
Dong Aisheng

Regards,
Mike

quoted

---
 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 659f2b0..0e813ea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1159,14 +1159,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

On Wed, Jul 15, 2015 at 08:25:24PM +0800, Dong Aisheng wrote:

Hi Mike,

On Wed, May 27, 2015 at 09:09:19PM -0700, Michael Turquette wrote:

quoted

Quoting Dong Aisheng (2015-05-14 06:28:59)

quoted

__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 <redacted>
Cc: Stephen Boyd <redacted>
Suggested-by: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>

Patch looks good to me.

Also just noticed that the second argument to __clk_set_parent_after is
never used :-(

Ping...

I did not see this patch series in your tree for v4.2 rc.
Is this got missed?

Updated Mike's email address.

Regards
Dong Aisheng

Regards
Dong Aisheng

quoted

Regards,
Mike

quoted

---
 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 659f2b0..0e813ea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1159,14 +1159,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

Ping...

On Wed, Jul 15, 2015 at 08:29:56PM +0800, Dong Aisheng wrote:

On Wed, Jul 15, 2015 at 08:25:24PM +0800, Dong Aisheng wrote:

quoted

Hi Mike,

On Wed, May 27, 2015 at 09:09:19PM -0700, Michael Turquette wrote:

quoted

Quoting Dong Aisheng (2015-05-14 06:28:59)

quoted

__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 <redacted>
Cc: Stephen Boyd <redacted>
Suggested-by: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>

Patch looks good to me.

Also just noticed that the second argument to __clk_set_parent_after is
never used :-(

Ping...

I did not see this patch series in your tree for v4.2 rc.
Is this got missed?

Updated Mike's email address.

Regards
Dong Aisheng

quoted

Regards
Dong Aisheng

quoted

Regards,
Mike

quoted

---
 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 659f2b0..0e813ea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1159,14 +1159,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

Quoting Dong Aisheng (2015-07-22 07:08:10)

Ping...

On Wed, Jul 15, 2015 at 08:29:56PM +0800, Dong Aisheng wrote:

quoted

On Wed, Jul 15, 2015 at 08:25:24PM +0800, Dong Aisheng wrote:

quoted

Hi Mike,

On Wed, May 27, 2015 at 09:09:19PM -0700, Michael Turquette wrote:

quoted

Quoting Dong Aisheng (2015-05-14 06:28:59)

quoted

__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 <redacted>
Cc: Stephen Boyd <redacted>
Suggested-by: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>

Patch looks good to me.

Also just noticed that the second argument to __clk_set_parent_after is
never used :-(

Ping...

I did not see this patch series in your tree for v4.2 rc.
Is this got missed?

Dong Aisheng,

Yes it was missed. My apologies. Can you refresh it and re-send?

Thanks,
Mike

quoted

quoted

Updated Mike's email address.

Regards
Dong Aisheng

quoted

Regards
Dong Aisheng

quoted

Regards,
Mike

quoted

---
 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 659f2b0..0e813ea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1159,14 +1159,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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

On Thu, Jul 23, 2015 at 05:11:35PM -0700, Michael Turquette wrote:

Quoting Dong Aisheng (2015-07-22 07:08:10)

quoted

Ping...

On Wed, Jul 15, 2015 at 08:29:56PM +0800, Dong Aisheng wrote:

quoted

On Wed, Jul 15, 2015 at 08:25:24PM +0800, Dong Aisheng wrote:

quoted

Hi Mike,

On Wed, May 27, 2015 at 09:09:19PM -0700, Michael Turquette wrote:

quoted

Quoting Dong Aisheng (2015-05-14 06:28:59)

quoted

__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 <redacted>
Cc: Stephen Boyd <redacted>
Suggested-by: Stephen Boyd <redacted>
Signed-off-by: Dong Aisheng <redacted>

Patch looks good to me.

Also just noticed that the second argument to __clk_set_parent_after is
never used :-(

Ping...

I did not see this patch series in your tree for v4.2 rc.
Is this got missed?

Dong Aisheng,

Yes it was missed. My apologies. Can you refresh it and re-send?

Will do it.
Thanks

Regards
Dong Aisheng

Thanks,
Mike

quoted

quoted

quoted

Updated Mike's email address.

Regards
Dong Aisheng

quoted

Regards
Dong Aisheng

quoted

Regards,
Mike

quoted

---
 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 659f2b0..0e813ea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1159,14 +1159,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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/