clk: Provide option to unprepare unused clocks at late init

PATCH Superseded Ulf Hansson 3 participants 10 messages Jan 24, 2013 – Feb 8, 2013

Subsystems (2)

Common clk Ux500 clock drivers

lore

Share & tools

lore

Ulf Hansson

author

2013-01-24 16:45 UTC #0 lore raw

cover clk: Provide option to unprepare unused clocks at late init

From: Ulf Hansson <[email protected]>

The disable_unused sequence executed at late init, is already handling the fast unused ungated clocks to be gated. This patchset extends this sequence to include the slow unused prepared clocks to be unprepared.

The default behavior will not change in this patchset. To unprepare unused clocks during the disable_unused sequence, the clk_hw needs to implement the new optional callback, is_prepared.

The motivation for this patchset is to save power. Clocks that is from bootloaders prepared|enabled, but not used should be unprepared|disabled.

Patch 4 is specific for ux500, which implements the is_prepared callback for it's PRMCU clocks.

Note that patch 1-3 has been sent earlier, but since a proof of concept patch could be useful, as patch 4 is, the hole patchset is resent.

Ulf Hansson (4):
clk: Introduce optional is_prepared callback
clk: Unprepare the unused prepared slow clocks at late init
clk: Introduce optional unprepare_unused callback
clk: ux500: Support is_prepared callback for clk-prcmu

 drivers/clk/clk.c             |   53 ++++++++++++++++
 drivers/clk/ux500/clk-prcmu.c |  134 ++++++++++++++++++++++++-----------------
 include/linux/clk-provider.h  |   11 ++++
 3 files changed, 144 insertions(+), 54 deletions(-)

Signature · 1 line

1.7.10

Ulf Hansson

author

2013-01-24 16:45 UTC #1 lore raw

PATCH 1/4 clk: Introduce optional is_prepared callback

From: Ulf Hansson <[email protected]>

To reflect whether a clk_hw is prepared the clk_hw may implement the optional is_prepared callback. If not implemented we fall back to use the software prepare counter.

Signed-off-by Ulf Hansson <[email protected]>

Acked-by Linus Walleij <[email protected]>

---
 drivers/clk/clk.c            |   21 +++++++++++++++++++++
 include/linux/clk-provider.h |    6 ++++++
 2 files changed, 27 insertions(+)

drivers/clk/clk.c +21 -0

index 593a2e4..deb259a 100644
@@ -458,6 +458,27 @@ unsigned long __clk_get_flags(struct clk *clk)
 	return !clk ? 0 : clk->flags;
 }
 
+bool __clk_is_prepared(struct clk *clk)
+{
+	int ret;
+
+	if (!clk)
+		return false;
+
+	/*
+	 * .is_prepared is optional for clocks that can prepare
+	 * fall back to software usage counter if it is missing
+	 */
+	if (!clk->ops->is_prepared) {
+		ret = clk->prepare_count ? 1 : 0;
+		goto out;
+	}
+
+	ret = clk->ops->is_prepared(clk->hw);
+out:
+	return !!ret;
+}
+
 bool __clk_is_enabled(struct clk *clk)
 {
 	int ret;

include/linux/clk-provider.h +6 -0

index 4989b8a..86ff6be 100644
@@ -45,6 +45,10 @@ struct clk_hw;
  * 		undo any work done in the @prepare callback. Called with
  * 		prepare_lock held.
  *
+ * @is_prepared: Queries the hardware to determine if the clock is prepared.
+ *		This function is allowed to sleep. Optional, if this op is not
+ *		set then the prepare count will be used.
+ *
  * @enable:	Enable the clock atomically. This must not return until the
  * 		clock is generating a valid clock signal, usable by consumer
  * 		devices. Called with enable_lock held. This function must not

@@ -108,6 +112,7 @@ struct clk_hw;
 struct clk_ops {
 	int		(*prepare)(struct clk_hw *hw);
 	void		(*unprepare)(struct clk_hw *hw);
+	int		(*is_prepared)(struct clk_hw *hw);
 	int		(*enable)(struct clk_hw *hw);
 	void		(*disable)(struct clk_hw *hw);
 	int		(*is_enabled)(struct clk_hw *hw);

@@ -351,6 +356,7 @@ unsigned int __clk_get_enable_count(struct clk *clk);
 unsigned int __clk_get_prepare_count(struct clk *clk);
 unsigned long __clk_get_rate(struct clk *clk);
 unsigned long __clk_get_flags(struct clk *clk);
+bool __clk_is_prepared(struct clk *clk);
 bool __clk_is_enabled(struct clk *clk);
 struct clk *__clk_lookup(const char *name);

Signature · 1 line

1.7.10

Mike Turquette

2013-01-24 18:12 UTC #2 lore raw

PATCH 1/4 clk: Introduce optional is_prepared callback

Quoting Ulf Hansson (2013-01-24 08:45:53)

Quoted 22 lines

From: Ulf Hansson <[email protected]>
To reflect whether a clk_hw is prepared the clk_hw may implement the optional is_prepared callback. If not implemented we fall back to use the software prepare counter.
Signed-off-by Ulf Hansson <[email protected]>
Acked-by Linus Walleij <[email protected]>
---
 drivers/clk/clk.c            |   21 +++++++++++++++++++++
 include/linux/clk-provider.h |    6 ++++++
 2 files changed, 27 insertions(+)
drivers/clk/clk.c +1 -0
index 593a2e4..deb259a 100644
@@ -458,6 +458,27 @@ unsigned long __clk_get_flags(struct clk *clk)
        return !clk ? 0 : clk->flags;
 }
 
+bool __clk_is_prepared(struct clk *clk)

drivers/clk/clk.c unsigned long __clk_get_flags(struct clk *clk) · line 471

Quoted 10 lines

drivers/clk/clk.c +10 -0

+{
+       int ret;
+
+       if (!clk)
+               return false;
+
+       /*
+        * .is_prepared is optional for clocks that can prepare
+        * fall back to software usage counter if it is missing
+        */

Why not make it mandatory? This could be as simple as saying "it is mandatory", or we could even enforce a check in clk_init, though the latter suggestion would be noisy until existing clock providers were updated.

Note that .is_enabled is technically mandatory for any clock that implements .enable/.disable but there is no check for compliance. It is only in Documentation/clk.txt and the kerneldoc.

Regards,
Mike

Ulf Hansson

2013-01-24 20:28 UTC #3 lore raw

PATCH 1/4 clk: Introduce optional is_prepared callback

Quoted 22 lines · Mike Turquette

Quoting Ulf Hansson (2013-01-24 08:45:53)
Quoted 21 lines
From: Ulf Hansson <[email protected]>
To reflect whether a clk_hw is prepared the clk_hw may implement the optional is_prepared callback. If not implemented we fall back to use the software prepare counter.
Signed-off-by Ulf Hansson <[email protected]>
Acked-by Linus Walleij <[email protected]>
---
 drivers/clk/clk.c            |   21 +++++++++++++++++++++
 include/linux/clk-provider.h |    6 ++++++
 2 files changed, 27 insertions(+)
drivers/clk/clk.c +0 -0
index 593a2e4..deb259a 100644
@@ -458,6 +458,27 @@ unsigned long __clk_get_flags(struct clk *clk)
        return !clk ? 0 : clk->flags;
 }
 

Quoted 16 lines

drivers/clk/clk.c unsigned long __clk_get_flags(struct clk *clk) · line 471
Quoted 11 lines
drivers/clk/clk.c +11 -0
+bool __clk_is_prepared(struct clk *clk)
+{
+       int ret;
+
+       if (!clk)
+               return false;
+
+       /*
+        * .is_prepared is optional for clocks that can prepare
+        * fall back to software usage counter if it is missing
+        */
Why not make it mandatory? This could be as simple as saying "it is mandatory", or we could even enforce a check in clk_init, though the latter suggestion would be noisy until existing clock providers were updated.

I was trying to go "slow" forward and wanted to keep the same behaviour for how .is_enabled is handled.

I am positive in making this mandatory, but then this should be applied for is_enabled as well. If we make something mandatory I definitely think it shall be checked in clk_int, it makes sense to me.

Quoted 4 lines

Note that .is_enabled is technically mandatory for any clock that implements .enable/.disable but there is no check for compliance. It is only in Documentation/clk.txt and the kerneldoc.

According to the clk-provider.h file:
------
* @is_enabled: Queries the hardware to determine if the clock is enabled.
* This function must not sleep. Optional, if this op is not
* set then the enable count will be used.
-----

The Documentation/clk.txt says it is mandatory. :-) So we have a kind of mixed message here.

Quoted 3 lines

Regards,
Mike

Kind regards
Ulf Hansson

Ulf Hansson

author

2013-01-24 16:45 UTC #4 lore raw

PATCH 2/4 clk: Unprepare the unused prepared slow clocks at late init

In reply to Ulf Hansson

From: Ulf Hansson <[email protected]>

The unused ungated fast clocks are already being disabled from clk_disable_unused at late init. This patch extend this sequence to the slow unused prepared clocks to be unprepared.

Unless the optional .is_prepared callback is implemented by a clk_hw the clk_disable_unused sequence will not unprepare any unused clocks, since it will fall back to use the software prepare counter.

Signed-off-by Ulf Hansson <[email protected]>

Acked-by Linus Walleij <[email protected]>

---
 drivers/clk/clk.c |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

drivers/clk/clk.c +29 -0

index deb259a..0944348 100644
@@ -341,6 +341,29 @@ static inline int clk_debug_register(struct clk *clk) { return 0; }
 #endif
 
 /* caller must hold prepare_lock */
+static void clk_unprepare_unused_subtree(struct clk *clk)
+{
+	struct clk *child;
+	struct hlist_node *tmp;
+
+	if (!clk)
+		return;
+
+	hlist_for_each_entry(child, tmp, &clk->children, child_node)
+		clk_unprepare_unused_subtree(child);
+
+	if (clk->prepare_count)
+		return;
+
+	if (clk->flags & CLK_IGNORE_UNUSED)
+		return;
+
+	if (__clk_is_prepared(clk))
+		if (clk->ops->unprepare)
+			clk->ops->unprepare(clk->hw);
+}
+
+/* caller must hold prepare_lock */
 static void clk_disable_unused_subtree(struct clk *clk)
 {
 	struct clk *child;

@@ -393,6 +416,12 @@ static int clk_disable_unused(void)
 	hlist_for_each_entry(clk, tmp, &clk_orphan_list, child_node)
 		clk_disable_unused_subtree(clk);
 
+	hlist_for_each_entry(clk, tmp, &clk_root_list, child_node)
+		clk_unprepare_unused_subtree(clk);
+
+	hlist_for_each_entry(clk, tmp, &clk_orphan_list, child_node)
+		clk_unprepare_unused_subtree(clk);
+
 	mutex_unlock(&prepare_lock);
 
 	return 0;

Signature · 1 line

1.7.10

Ulf Hansson

author

2013-01-24 16:45 UTC #5 lore raw

PATCH 3/4 clk: Introduce optional unprepare_unused callback

In reply to Ulf Hansson

From: Ulf Hansson <[email protected]>

An unprepare_unused callback is introduced due to the same reasons to why the disable_unused callback was added.

During the clk_disable_unused sequence, those clk_hw that needs specific treatment with regards to being unprepared, shall implement the unprepare_unused callback.

Signed-off-by Ulf Hansson <[email protected]>

Acked-by Linus Walleij <[email protected]>

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

drivers/clk/clk.c +5 -2

index 0944348..d8dc6d7c 100644
@@ -358,9 +358,12 @@ static void clk_unprepare_unused_subtree(struct clk *clk)
 	if (clk->flags & CLK_IGNORE_UNUSED)
 		return;
 
-	if (__clk_is_prepared(clk))
-		if (clk->ops->unprepare)
+	if (__clk_is_prepared(clk)) {
+		if (clk->ops->unprepare_unused)
+			clk->ops->unprepare_unused(clk->hw);
+		else if (clk->ops->unprepare)
 			clk->ops->unprepare(clk->hw);
+	}
 }
 
 /* caller must hold prepare_lock */

include/linux/clk-provider.h +5 -0

index 86ff6be..9e97fb4 100644
@@ -49,6 +49,10 @@ struct clk_hw;
  *		This function is allowed to sleep. Optional, if this op is not
  *		set then the prepare count will be used.
  *
+ * @unprepare_unused: Unprepare the clock atomically.  Only called from
+ *		clk_disable_unused for prepare clocks with special needs.
+ *		Called with prepare mutex held. This function may sleep.
+ *
  * @enable:	Enable the clock atomically. This must not return until the
  * 		clock is generating a valid clock signal, usable by consumer
  * 		devices. Called with enable_lock held. This function must not

@@ -113,6 +117,7 @@ struct clk_ops {
 	int		(*prepare)(struct clk_hw *hw);
 	void		(*unprepare)(struct clk_hw *hw);
 	int		(*is_prepared)(struct clk_hw *hw);
+	void		(*unprepare_unused)(struct clk_hw *hw);
 	int		(*enable)(struct clk_hw *hw);
 	void		(*disable)(struct clk_hw *hw);
 	int		(*is_enabled)(struct clk_hw *hw);

Signature · 1 line

1.7.10

Ulf Hansson

author

2013-01-24 16:45 UTC #6 lore raw

PATCH 4/4 clk: ux500: Support is_prepared callback for clk-prcmu

In reply to Ulf Hansson

From: Ulf Hansson <[email protected]>

To be able to gate unused prcmu clocks from the clk_disable_unused sequence, clk-prcmu now implements the is_prepared callback.

Signed-off-by Ulf Hansson <[email protected]>

---
 drivers/clk/ux500/clk-prcmu.c |  134 ++++++++++++++++++++++++-----------------
 1 file changed, 80 insertions(+), 54 deletions(-)

drivers/clk/ux500/clk-prcmu.c +80 ~~-54~~

index 74faa7e..9d83256 100644
@@ -20,15 +20,23 @@
 struct clk_prcmu {
 	struct clk_hw hw;
 	u8 cg_sel;
+	int is_prepared;
 	int is_enabled;
+	int opp_requested;
 };
 
 /* PRCMU clock operations. */
 
 static int clk_prcmu_prepare(struct clk_hw *hw)
 {
+	int ret;
 	struct clk_prcmu *clk = to_clk_prcmu(hw);
-	return prcmu_request_clock(clk->cg_sel, true);
+
+	ret = prcmu_request_clock(clk->cg_sel, true);
+	if (!ret)
+		clk->is_prepared = 1;
+
+	return ret;;
 }
 
 static void clk_prcmu_unprepare(struct clk_hw *hw)

@@ -37,6 +45,14 @@ static void clk_prcmu_unprepare(struct clk_hw *hw)
 	if (prcmu_request_clock(clk->cg_sel, false))
 		pr_err("clk_prcmu: %s failed to disable %s.\n", __func__,
 			hw->init->name);
+	else
+		clk->is_prepared = 0;
+}
+
+static int clk_prcmu_is_prepared(struct clk_hw *hw)
+{
+	struct clk_prcmu *clk = to_clk_prcmu(hw);
+	return clk->is_prepared;
 }
 
 static int clk_prcmu_enable(struct clk_hw *hw)

@@ -79,58 +95,52 @@ static int clk_prcmu_set_rate(struct clk_hw *hw, unsigned long rate,
 	return prcmu_set_clock_rate(clk->cg_sel, rate);
 }
 
-static int request_ape_opp100(bool enable)
-{
-	static int reqs;
-	int err = 0;
-
-	if (enable) {
-		if (!reqs)
-			err = prcmu_qos_add_requirement(PRCMU_QOS_APE_OPP,
-							"clock", 100);
-		if (!err)
-			reqs++;
-	} else {
-		reqs--;
-		if (!reqs)
-			prcmu_qos_remove_requirement(PRCMU_QOS_APE_OPP,
-						"clock");
-	}
-	return err;
-}
-
 static int clk_prcmu_opp_prepare(struct clk_hw *hw)
 {
 	int err;
 	struct clk_prcmu *clk = to_clk_prcmu(hw);
 
-	err = request_ape_opp100(true);
-	if (err) {
-		pr_err("clk_prcmu: %s failed to request APE OPP100 for %s.\n",
-			__func__, hw->init->name);
-		return err;
+	if (!clk->opp_requested) {
+		err = prcmu_qos_add_requirement(PRCMU_QOS_APE_OPP,
+						(char *)__clk_get_name(hw->clk),
+						100);
+		if (err) {
+			pr_err("clk_prcmu: %s fail req APE OPP for %s.\n",
+				__func__, hw->init->name);
+			return err;
+		}
+		clk->opp_requested = 1;
 	}
 
 	err = prcmu_request_clock(clk->cg_sel, true);
-	if (err)
-		request_ape_opp100(false);
+	if (err) {
+		prcmu_qos_remove_requirement(PRCMU_QOS_APE_OPP,
+					(char *)__clk_get_name(hw->clk));
+		clk->opp_requested = 0;
+		return err;
+	}
 
-	return err;
+	clk->is_prepared = 1;
+	return 0;
 }
 
 static void clk_prcmu_opp_unprepare(struct clk_hw *hw)
 {
 	struct clk_prcmu *clk = to_clk_prcmu(hw);
 
-	if (prcmu_request_clock(clk->cg_sel, false))
-		goto out_error;
-	if (request_ape_opp100(false))
-		goto out_error;
-	return;
-
-out_error:
-	pr_err("clk_prcmu: %s failed to disable %s.\n", __func__,
-		hw->init->name);
+	if (prcmu_request_clock(clk->cg_sel, false)) {
+		pr_err("clk_prcmu: %s failed to disable %s.\n", __func__,
+			hw->init->name);
+		return;
+	}
+
+	if (clk->opp_requested) {
+		prcmu_qos_remove_requirement(PRCMU_QOS_APE_OPP,
+					(char *)__clk_get_name(hw->clk));
+		clk->opp_requested = 0;
+	}
+
+	clk->is_prepared = 0;
 }
 
 static int clk_prcmu_opp_volt_prepare(struct clk_hw *hw)

@@ -138,38 +148,49 @@ static int clk_prcmu_opp_volt_prepare(struct clk_hw *hw)
 	int err;
 	struct clk_prcmu *clk = to_clk_prcmu(hw);
 
-	err = prcmu_request_ape_opp_100_voltage(true);
-	if (err) {
-		pr_err("clk_prcmu: %s failed to request APE OPP VOLT for %s.\n",
-			__func__, hw->init->name);
-		return err;
+	if (!clk->opp_requested) {
+		err = prcmu_request_ape_opp_100_voltage(true);
+		if (err) {
+			pr_err("clk_prcmu: %s fail req APE OPP VOLT for %s.\n",
+				__func__, hw->init->name);
+			return err;
+		}
+		clk->opp_requested = 1;
 	}
 
 	err = prcmu_request_clock(clk->cg_sel, true);
-	if (err)
+	if (err) {
 		prcmu_request_ape_opp_100_voltage(false);
+		clk->opp_requested = 0;
+		return err;
+	}
 
-	return err;
+	clk->is_prepared = 1;
+	return 0;
 }
 
 static void clk_prcmu_opp_volt_unprepare(struct clk_hw *hw)
 {
 	struct clk_prcmu *clk = to_clk_prcmu(hw);
 
-	if (prcmu_request_clock(clk->cg_sel, false))
-		goto out_error;
-	if (prcmu_request_ape_opp_100_voltage(false))
-		goto out_error;
-	return;
-
-out_error:
-	pr_err("clk_prcmu: %s failed to disable %s.\n", __func__,
-		hw->init->name);
+	if (prcmu_request_clock(clk->cg_sel, false)) {
+		pr_err("clk_prcmu: %s failed to disable %s.\n", __func__,
+			hw->init->name);
+		return;
+	}
+
+	if (clk->opp_requested) {
+		prcmu_request_ape_opp_100_voltage(false);
+		clk->opp_requested = 0;
+	}
+
+	clk->is_prepared = 0;
 }
 
 static struct clk_ops clk_prcmu_scalable_ops = {
 	.prepare = clk_prcmu_prepare,
 	.unprepare = clk_prcmu_unprepare,
+	.is_prepared = clk_prcmu_is_prepared,
 	.enable = clk_prcmu_enable,
 	.disable = clk_prcmu_disable,
 	.is_enabled = clk_prcmu_is_enabled,

@@ -181,6 +202,7 @@ static struct clk_ops clk_prcmu_scalable_ops = {
 static struct clk_ops clk_prcmu_gate_ops = {
 	.prepare = clk_prcmu_prepare,
 	.unprepare = clk_prcmu_unprepare,
+	.is_prepared = clk_prcmu_is_prepared,
 	.enable = clk_prcmu_enable,
 	.disable = clk_prcmu_disable,
 	.is_enabled = clk_prcmu_is_enabled,

@@ -202,6 +224,7 @@ static struct clk_ops clk_prcmu_rate_ops = {
 static struct clk_ops clk_prcmu_opp_gate_ops = {
 	.prepare = clk_prcmu_opp_prepare,
 	.unprepare = clk_prcmu_opp_unprepare,
+	.is_prepared = clk_prcmu_is_prepared,
 	.enable = clk_prcmu_enable,
 	.disable = clk_prcmu_disable,
 	.is_enabled = clk_prcmu_is_enabled,

@@ -211,6 +234,7 @@ static struct clk_ops clk_prcmu_opp_gate_ops = {
 static struct clk_ops clk_prcmu_opp_volt_scalable_ops = {
 	.prepare = clk_prcmu_opp_volt_prepare,
 	.unprepare = clk_prcmu_opp_volt_unprepare,
+	.is_prepared = clk_prcmu_is_prepared,
 	.enable = clk_prcmu_enable,
 	.disable = clk_prcmu_disable,
 	.is_enabled = clk_prcmu_is_enabled,

@@ -242,7 +266,9 @@ static struct clk *clk_reg_prcmu(const char *name,
 	}
 
 	clk->cg_sel = cg_sel;
+	clk->is_prepared = 1;
 	clk->is_enabled = 1;
+	clk->opp_requested = 0;
 	/* "rate" can be used for changing the initial frequency */
 	if (rate)
 		prcmu_set_clock_rate(cg_sel, rate);

Signature · 1 line

1.7.10

Mike Turquette

2013-01-24 18:49 UTC #7 lore raw

cover clk: Provide option to unprepare unused clocks at late init

In reply to Ulf Hansson

Quoting Ulf Hansson (2013-01-24 08:45:52)

Quoted 10 lines

From: Ulf Hansson <[email protected]>
The disable_unused sequence executed at late init, is already handling the fast unused ungated clocks to be gated. This patchset extends this sequence to include the slow unused prepared clocks to be unprepared.
The default behavior will not change in this patchset. To unprepare unused clocks during the disable_unused sequence, the clk_hw needs to implement the new optional callback, is_prepared.

Quoted 9 lines

The motivation for this patchset is to save power. Clocks that is from bootloaders prepared|enabled, but not used should be unprepared|disabled.
Patch 4 is specific for ux500, which implements the is_prepared callback for it's PRMCU clocks.
Note that patch 1-3 has been sent earlier, but since a proof of concept patch could be useful, as patch 4 is, the hole patchset is resent.

Hi Ulf,

I had some minor discussion in patch #1. Otherwise patches #2 & #3 look good to me.

Patch #4 is a bit interesting. There is nothing wrong with it per se, but the general layering of the prcmu power stuff is the opposite of how I had envisioned things. Your .prepare callback is making calls to some pm_qos api but a quick grep did not reveal any definitions for those functions, just some stubs in include/linux/mfd/dbx500-prcmu.h.

Do these functions scale voltage? I had figured that some day per-device pm qos apis would call the clk api, not the other way around.

Anyways that is just food for thought and no reason to block patch #4.

Regards,
Mike

Quoted 13 lines

Ulf Hansson (4):
clk: Introduce optional is_prepared callback
clk: Unprepare the unused prepared slow clocks at late init
clk: Introduce optional unprepare_unused callback
clk: ux500: Support is_prepared callback for clk-prcmu
 drivers/clk/clk.c             |   53 ++++++++++++++++
 drivers/clk/ux500/clk-prcmu.c |  134 ++++++++++++++++++++++++-----------------
 include/linux/clk-provider.h  |   11 ++++
 3 files changed, 144 insertions(+), 54 deletions(-)
Signature · 1 line
1.7.10

Ulf Hansson

2013-01-24 20:41 UTC #8 lore raw

cover clk: Provide option to unprepare unused clocks at late init

Hi Mike,

Quoted 26 lines · Mike Turquette

Quoting Ulf Hansson (2013-01-24 08:45:52)
Quoted 10 lines
From: Ulf Hansson <[email protected]>
The disable_unused sequence executed at late init, is already handling the fast unused ungated clocks to be gated. This patchset extends this sequence to include the slow unused prepared clocks to be unprepared.
The default behavior will not change in this patchset. To unprepare unused clocks during the disable_unused sequence, the clk_hw needs to implement the new optional callback, is_prepared.
Quoted 9 lines
The motivation for this patchset is to save power. Clocks that is from bootloaders prepared|enabled, but not used should be unprepared|disabled.
Patch 4 is specific for ux500, which implements the is_prepared callback for it's PRMCU clocks.
Note that patch 1-3 has been sent earlier, but since a proof of concept patch could be useful, as patch 4 is, the hole patchset is resent.
Hi Ulf,
I had some minor discussion in patch #1. Otherwise patches #2 & #3 look good to me.

Quoted 8 lines

Patch #4 is a bit interesting. There is nothing wrong with it per se, but the general layering of the prcmu power stuff is the opposite of how I had envisioned things. Your .prepare callback is making calls to some pm_qos api but a quick grep did not reveal any definitions for those functions, just some stubs in include/linux/mfd/dbx500-prcmu.h.
Do these functions scale voltage? I had figured that some day per-device pm qos apis would call the clk api, not the other way around.

You are right, these scale voltages. I would like to keep the code as is, just as a reminder that some devices using some clocks will requires these kind of operations.
On our internal STE development track this is how it is used but we a are moving towards newer APIs.

So I am definitely sharing the same thought as you do and my plan is to remove this code along the road, once drivers is adapted properly.

Quoted 19 lines

Anyways that is just food for thought and no reason to block patch #4.
Regards,
Mike
Quoted 13 lines
Ulf Hansson (4):
clk: Introduce optional is_prepared callback
clk: Unprepare the unused prepared slow clocks at late init
clk: Introduce optional unprepare_unused callback
clk: ux500: Support is_prepared callback for clk-prcmu
 drivers/clk/clk.c             |   53 ++++++++++++++++
 drivers/clk/ux500/clk-prcmu.c |  134 ++++++++++++++++++++++++-----------------
 include/linux/clk-provider.h  |   11 ++++
 3 files changed, 144 insertions(+), 54 deletions(-)
--
1.7.10

Thanks for the review comments!

Kind regards
Ulf Hansson

Ulf Hansson

2013-02-08 08:33 UTC #9 lore raw

cover clk: Provide option to unprepare unused clocks at late init

Quoted 64 lines · Ulf Hansson

Hi Mike,
Quoted 26 lines · Mike Turquette
Quoting Ulf Hansson (2013-01-24 08:45:52)
Quoted 10 lines
From: Ulf Hansson <[email protected]>
The disable_unused sequence executed at late init, is already handling the fast unused ungated clocks to be gated. This patchset extends this sequence to include the slow unused prepared clocks to be unprepared.
The default behavior will not change in this patchset. To unprepare unused clocks during the disable_unused sequence, the clk_hw needs to implement the new optional callback, is_prepared.
Quoted 9 lines
The motivation for this patchset is to save power. Clocks that is from bootloaders prepared|enabled, but not used should be unprepared|disabled.
Patch 4 is specific for ux500, which implements the is_prepared callback for it's PRMCU clocks.
Note that patch 1-3 has been sent earlier, but since a proof of concept patch could be useful, as patch 4 is, the hole patchset is resent.
Hi Ulf,
I had some minor discussion in patch #1. Otherwise patches #2 & #3 look good to me.
Quoted 8 lines
Patch #4 is a bit interesting. There is nothing wrong with it per se, but the general layering of the prcmu power stuff is the opposite of how I had envisioned things. Your .prepare callback is making calls to some pm_qos api but a quick grep did not reveal any definitions for those functions, just some stubs in include/linux/mfd/dbx500-prcmu.h.
Do these functions scale voltage? I had figured that some day per-device pm qos apis would call the clk api, not the other way around.
You are right, these scale voltages. I would like to keep the code as is, just as a reminder that some devices using some clocks will requires these kind of operations.
On our internal STE development track this is how it is used but we a are moving towards newer APIs.
So I am definitely sharing the same thought as you do and my plan is to remove this code along the road, once drivers is adapted properly.
Quoted 17 lines
Anyways that is just food for thought and no reason to block patch #4.
Regards,
Mike
Quoted 11 lines
Ulf Hansson (4):
clk: Introduce optional is_prepared callback
clk: Unprepare the unused prepared slow clocks at late init
clk: Introduce optional unprepare_unused callback
clk: ux500: Support is_prepared callback for clk-prcmu
 drivers/clk/clk.c             |   53 ++++++++++++++++
 drivers/clk/ux500/clk-prcmu.c |  134 ++++++++++++++++++++++++-----------------
 include/linux/clk-provider.h  |   11 ++++
 3 files changed, 144 insertions(+), 54 deletions(-)

Quoted 7 lines

Quoted 2 lines
Quoted 2 lines
--
1.7.10
Thanks for the review comments!
Kind regards
Ulf Hansson

Ping, on this. Any issues merging this for 3.9?

Kind regards
Ulf Hansson