[PATCH 0/8] Fix module-mode enable sequence on OMAP4

[PATCH 0/8] Fix module-mode enable sequence on OMAP4

[Rajendra Nayak]

Hi,

On OMAP4, the PRCM recommended sequence for enabling
a module after power-on-reset is
-1- Force clkdm to SW_WKUP
-2- Configure desired module mode to "enable" or "auto"
-3- Wait for the desired module idle status to be FUNC
-4- Program clkdm in HW_AUTO(if supported)

This sequence applies to all older OMAPs' as well,
however since they use 'autodeps', it makes sure that
no clkdm is in IDLE, and hence not requiring a force
SW_WKUP when a module is being enabled.

OMAP4 does not need to support autodeps, because
of the dynamic dependency feature, wherein
the HW takes care of waking up a clockdomain from
idle and hence the module, whenever an interconnect
access happens to the given module.

Implementing the sequence for OMAP4 requires some of
the clockdomain handling that is currently done in
clock framework to be done as part of hwmod framework
since the step -3- above to "Wait for the desired 
module idle status to be FUNC" is done as part of
hwmod framework.

This series moves the clockdomain handling into hwmod
framework and implements the above sequence for OMAP4.

The above sequence is expected to be followed even for
optional clocks (steps -1-, -2- and -4- only) and that
makes handling it difficult as optional clocks are enabled by
drivers using direct clock framework calls and hence
do not go through the hwmod framework.

Hence this series also adds the handling of the sequence
*only for optional clocks* in clock framework.

Lastly, with this series drivers which are yet to be
adapted to PM runtime and still rely on clock calls
to enable/disable the respective *main* clocks are
expected to be broken. MMC is one such which even breaks
boot, and hence the series has a TEMP workaround patch
added which keeps l3init clockdomain always force-enabled.
This TEMP patch will gate all CORE low power transitions
but should atleast allow MPU low power transitions to
work.

The series is tested for low power state transitions
(RET/OFF) on OMAP3sdp and boot tested on OMAP4sdp.
Applies on 3.0-rc2

Rajendra Nayak (8):
  OMAP2+: clockdomain: Add an api to read idle mode
  OMAP2+: clockdomain: Add SoC support for clkdm_is_idle
  OMAP2+: PM: Initialise sleep_switch to a non-valid value
  OMAP2+: PM: idle clkdms only if already in idle
  OMAP4: PM: TEMP: Prevent l3init from idling/force sleep
  OMAP2+: hwmod: Follow the recomended PRCM clock enable sequence
  OMAP: clock: Add flags to identify optional clock nodes
  OMAP: clock: Enable clockdomain only for optional clocks

 arch/arm/mach-omap2/clock.c                 |   30 ++++++++++++------------
 arch/arm/mach-omap2/clock44xx_data.c        |   34 +++++++++++++++++++++++++++
 arch/arm/mach-omap2/clockdomain.c           |   28 +++++++++++++++++++++-
 arch/arm/mach-omap2/clockdomain.h           |    3 ++
 arch/arm/mach-omap2/clockdomain2xxx_3xxx.c  |   12 +++++++++
 arch/arm/mach-omap2/clockdomain44xx.c       |   16 ++++++------
 arch/arm/mach-omap2/clockdomains44xx_data.c |    2 +-
 arch/arm/mach-omap2/omap_hwmod.c            |   20 +++++++++++++++-
 arch/arm/mach-omap2/pm.c                    |    6 +++-
 arch/arm/plat-omap/include/plat/clock.h     |    1 +
 10 files changed, 124 insertions(+), 28 deletions(-)

[PATCH 1/8] OMAP2+: clockdomain: Add an api to read idle mode

[Rajendra Nayak]

Add a clockdomain api to check if hardware supervised
idle transitions are enabled on a clockdomain.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/clockdomain.c |   21 +++++++++++++++++++++
 arch/arm/mach-omap2/clockdomain.h |    3 +++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 6cb6c03..2ab3686 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -795,6 +795,27 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
 	arch_clkdm->clkdm_deny_idle(clkdm);
 }
 
+/**
+ * clkdm_is_idle - Check if the clkdm hwsup/autoidle is enabled
+ * @clkdm: struct clockdomain *
+ *
+ * Returns true if the clockdomain is in hardware-supervised
+ * idle mode, or 0 otherwise.
+ *
+ */
+int clkdm_is_idle(struct clockdomain *clkdm)
+{
+	if (!clkdm)
+		return -EINVAL;
+
+	if (!arch_clkdm || !arch_clkdm->clkdm_is_idle)
+		return -EINVAL;
+
+	pr_debug("clockdomain: reading idle state for %s\n", clkdm->name);
+
+	return arch_clkdm->clkdm_is_idle(clkdm);
+}
+
 
 /* Clockdomain-to-clock framework interface code */
 
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 5823584..085ed82 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -138,6 +138,7 @@ struct clockdomain {
  * @clkdm_wakeup: Force a clockdomain to wakeup
  * @clkdm_allow_idle: Enable hw supervised idle transitions for clock domain
  * @clkdm_deny_idle: Disable hw supervised idle transitions for clock domain
+ * @clkdm_is_idle: Check if hw supervised idle transitions are enabled
  * @clkdm_clk_enable: Put the clkdm in right state for a clock enable
  * @clkdm_clk_disable: Put the clkdm in right state for a clock disable
  */
@@ -154,6 +155,7 @@ struct clkdm_ops {
 	int	(*clkdm_wakeup)(struct clockdomain *clkdm);
 	void	(*clkdm_allow_idle)(struct clockdomain *clkdm);
 	void	(*clkdm_deny_idle)(struct clockdomain *clkdm);
+	int	(*clkdm_is_idle)(struct clockdomain *clkdm);
 	int	(*clkdm_clk_enable)(struct clockdomain *clkdm);
 	int	(*clkdm_clk_disable)(struct clockdomain *clkdm);
 };
@@ -177,6 +179,7 @@ int clkdm_clear_all_sleepdeps(struct clockdomain *clkdm);
 
 void clkdm_allow_idle(struct clockdomain *clkdm);
 void clkdm_deny_idle(struct clockdomain *clkdm);
+int clkdm_is_idle(struct clockdomain *clkdm);
 
 int clkdm_wakeup(struct clockdomain *clkdm);
 int clkdm_sleep(struct clockdomain *clkdm);
-- 
1.7.0.4

[PATCH 2/8] OMAP2+: clockdomain: Add SoC support for clkdm_is_idle

[Rajendra Nayak]

Add the SoC specific implemenation for clkdm_is_idle
for OMAP2/3 and OMAP4.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/clockdomain2xxx_3xxx.c |   12 ++++++++++++
 arch/arm/mach-omap2/clockdomain44xx.c      |    7 +++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c b/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c
index 48d0db7..e0c393f 100644
--- a/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/types.h>
+#include <linux/errno.h>
 #include <plat/prcm.h>
 #include "prm.h"
 #include "prm2xxx_3xxx.h"
@@ -146,6 +147,15 @@ static void omap2_clkdm_deny_idle(struct clockdomain *clkdm)
 		_clkdm_del_autodeps(clkdm);
 }
 
+static int omap2_clkdm_is_idle(struct clockdomain *clkdm)
+{
+	if (!clkdm->clktrctrl_mask)
+		return -EINVAL;
+
+	return omap2_cm_is_clkdm_in_hwsup(clkdm->pwrdm.ptr->prcm_offs,
+				clkdm->clktrctrl_mask);
+}
+
 static void _enable_hwsup(struct clockdomain *clkdm)
 {
 	if (cpu_is_omap24xx())
@@ -252,6 +262,7 @@ struct clkdm_ops omap2_clkdm_operations = {
 	.clkdm_wakeup		= omap2_clkdm_wakeup,
 	.clkdm_allow_idle	= omap2_clkdm_allow_idle,
 	.clkdm_deny_idle	= omap2_clkdm_deny_idle,
+	.clkdm_is_idle		= omap2_clkdm_is_idle,
 	.clkdm_clk_enable	= omap2_clkdm_clk_enable,
 	.clkdm_clk_disable	= omap2_clkdm_clk_disable,
 };
@@ -269,6 +280,7 @@ struct clkdm_ops omap3_clkdm_operations = {
 	.clkdm_wakeup		= omap3_clkdm_wakeup,
 	.clkdm_allow_idle	= omap3_clkdm_allow_idle,
 	.clkdm_deny_idle	= omap3_clkdm_deny_idle,
+	.clkdm_is_idle		= omap2_clkdm_is_idle,
 	.clkdm_clk_enable	= omap2_clkdm_clk_enable,
 	.clkdm_clk_disable	= omap2_clkdm_clk_disable,
 };
diff --git a/arch/arm/mach-omap2/clockdomain44xx.c b/arch/arm/mach-omap2/clockdomain44xx.c
index a1a4ecd..4b10727 100644
--- a/arch/arm/mach-omap2/clockdomain44xx.c
+++ b/arch/arm/mach-omap2/clockdomain44xx.c
@@ -93,6 +93,12 @@ static void omap4_clkdm_deny_idle(struct clockdomain *clkdm)
 					clkdm->cm_inst, clkdm->clkdm_offs);
 }
 
+static int omap4_clkdm_is_idle(struct clockdomain *clkdm)
+{
+	return omap4_cminst_is_clkdm_in_hwsup(clkdm->prcm_partition,
+				clkdm->cm_inst, clkdm->clkdm_offs);
+}
+
 static int omap4_clkdm_clk_enable(struct clockdomain *clkdm)
 {
 	bool hwsup = false;
@@ -132,6 +138,7 @@ struct clkdm_ops omap4_clkdm_operations = {
 	.clkdm_wakeup		= omap4_clkdm_wakeup,
 	.clkdm_allow_idle	= omap4_clkdm_allow_idle,
 	.clkdm_deny_idle	= omap4_clkdm_deny_idle,
+	.clkdm_is_idle		= omap4_clkdm_is_idle,
 	.clkdm_clk_enable	= omap4_clkdm_clk_enable,
 	.clkdm_clk_disable	= omap4_clkdm_clk_disable,
 };
-- 
1.7.0.4

[PATCH 3/8] OMAP2+: PM: Initialise sleep_switch to a non-valid value

[Rajendra Nayak]

sleep_switch which is initialised to 0 in omap_set_pwrdm_state
happens to be a valid sleep_switch type (FORCEWAKEUP_SWITCH)
which are defined as
#define FORCEWAKEUP_SWITCH      0
#define LOWPOWERSTATE_SWITCH    1

This causes the function to wrongly program some clock domains
even when the Powerdomain is in ON state.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/pm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 49486f5..d48813f 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -106,7 +106,7 @@ static void omap2_init_processor_devices(void)
 int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 {
 	u32 cur_state;
-	int sleep_switch = 0;
+	int sleep_switch = -1;
 	int ret = 0;
 
 	if (pwrdm == NULL || IS_ERR(pwrdm))
-- 
1.7.0.4

[PATCH 4/8] OMAP2+: PM: idle clkdms only if already in idle

[Rajendra Nayak]

The omap_set_pwrdm_state function forces clockdomains
to idle, without checking the existing idle state
programmed, instead based solely on the HW capability
of the clockdomain to support idle.
This is wrong and the clockdomains should be idled
post a state_switch *only* if idle transitions on the
clockdomain were already enabled.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/pm.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index d48813f..840b0e1 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -108,6 +108,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 	u32 cur_state;
 	int sleep_switch = -1;
 	int ret = 0;
+	int hwsup = 0;
 
 	if (pwrdm == NULL || IS_ERR(pwrdm))
 		return -EINVAL;
@@ -127,6 +128,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
 			sleep_switch = LOWPOWERSTATE_SWITCH;
 		} else {
+			hwsup = clkdm_is_idle(pwrdm->pwrdm_clkdms[0]);
 			clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
 			pwrdm_wait_transition(pwrdm);
 			sleep_switch = FORCEWAKEUP_SWITCH;
@@ -142,7 +144,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 
 	switch (sleep_switch) {
 	case FORCEWAKEUP_SWITCH:
-		if (pwrdm->pwrdm_clkdms[0]->flags & CLKDM_CAN_ENABLE_AUTO)
+		if (hwsup)
 			clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
 		else
 			clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
-- 
1.7.0.4

[PATCH 5/8] OMAP4: PM: TEMP: Prevent l3init from idling/force sleep

[Rajendra Nayak]

Since MMC driver is yet to be adapted to
runtime PM and still uses direct clock
calls to enable/disable module, its needed
that the clockdomain (for MMC) is always kept force
enabled since the next few patches move
the clockdomain handling from clock framework
to hwmod framework and break MMC.

This will certainlly gate any CORE low power
transitions.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/clockdomains44xx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c
index a607ec1..ff38764 100644
--- a/arch/arm/mach-omap2/clockdomains44xx_data.c
+++ b/arch/arm/mach-omap2/clockdomains44xx_data.c
@@ -493,7 +493,7 @@ static struct clockdomain l3_init_44xx_clkdm = {
 	.dep_bit	  = OMAP4430_L3INIT_STATDEP_SHIFT,
 	.wkdep_srcs	  = l3_init_wkup_sleep_deps,
 	.sleepdep_srcs	  = l3_init_wkup_sleep_deps,
-	.flags		  = CLKDM_CAN_HWSUP_SWSUP,
+	.flags		  = CLKDM_CAN_FORCE_WAKEUP,
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
 };
 
-- 
1.7.0.4

[PATCH 6/8] OMAP2+: hwmod: Follow the recomended PRCM clock enable sequence

[Rajendra Nayak]

On OMAP4, the PRCM recommended sequence for enabling
a module after power-on-reset is
-1- Force clkdm to SW_WKUP
-2- Configure desired module mode to "enable" or "auto"
-3- Wait for the desired module idle status to be FUNC
-4- Program clkdm in HW_AUTO(if supported)

This sequence applies to all older OMAPs' as well,
however since they use autodeps, it makes sure that
no clkdm is in IDLE, and hence not requiring a force
SW_WKUP when a module is being enabled.

OMAP4 does not need to support autodeps, because
of the dyanamic dependency feature, wherein
the HW takes care of waking up a clockdomain from
idle and hence the module, whenever an interconnect
access happens to the given module.

Implementing the sequence for OMAP4 requires
the clockdomain handling that is currently done in
clock framework to be done as part of hwmod framework
since the step -3- above to "Wait for the desired
module idle status to be FUNC" is done as part of
hwmod framework.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/clock.c           |   17 +----------------
 arch/arm/mach-omap2/clockdomain.c     |    7 ++++++-
 arch/arm/mach-omap2/clockdomain44xx.c |    9 +--------
 arch/arm/mach-omap2/omap_hwmod.c      |   20 +++++++++++++++++++-
 4 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 180299e..2828d29 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -268,9 +268,6 @@ void omap2_clk_disable(struct clk *clk)
 		clk->ops->disable(clk);
 	}
 
-	if (clk->clkdm)
-		clkdm_clk_disable(clk->clkdm, clk);
-
 	if (clk->parent)
 		omap2_clk_disable(clk->parent);
 }
@@ -308,30 +305,18 @@ int omap2_clk_enable(struct clk *clk)
 		}
 	}
 
-	if (clk->clkdm) {
-		ret = clkdm_clk_enable(clk->clkdm, clk);
-		if (ret) {
-			WARN(1, "clock: %s: could not enable clockdomain %s: "
-			     "%d\n", clk->name, clk->clkdm->name, ret);
-			goto oce_err2;
-		}
-	}
-
 	if (clk->ops && clk->ops->enable) {
 		trace_clock_enable(clk->name, 1, smp_processor_id());
 		ret = clk->ops->enable(clk);
 		if (ret) {
 			WARN(1, "clock: %s: could not enable: %d\n",
 			     clk->name, ret);
-			goto oce_err3;
+			goto oce_err2;
 		}
 	}
 
 	return 0;
 
-oce_err3:
-	if (clk->clkdm)
-		clkdm_clk_disable(clk->clkdm, clk);
 oce_err2:
 	if (clk->parent)
 		omap2_clk_disable(clk->parent);
diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 2ab3686..b98a972 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -846,7 +846,12 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
 	if (!arch_clkdm || !arch_clkdm->clkdm_clk_enable)
 		return -EINVAL;
 
-	if (atomic_inc_return(&clkdm->usecount) > 1)
+	/*
+	 * For arch's with no autodeps, clkcm_clk_enable
+	 * should be called for every clock instance that is
+	 * enabled, so the clkdm can be force woken up.
+	 */
+	if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps)
 		return 0;
 
 	/* Clockdomain now has one enabled downstream clock */
diff --git a/arch/arm/mach-omap2/clockdomain44xx.c b/arch/arm/mach-omap2/clockdomain44xx.c
index 4b10727..e4b8e06 100644
--- a/arch/arm/mach-omap2/clockdomain44xx.c
+++ b/arch/arm/mach-omap2/clockdomain44xx.c
@@ -101,14 +101,7 @@ static int omap4_clkdm_is_idle(struct clockdomain *clkdm)
 
 static int omap4_clkdm_clk_enable(struct clockdomain *clkdm)
 {
-	bool hwsup = false;
-
-	hwsup = omap4_cminst_is_clkdm_in_hwsup(clkdm->prcm_partition,
-					clkdm->cm_inst, clkdm->clkdm_offs);
-
-	if (!hwsup)
-		clkdm_wakeup(clkdm);
-
+	clkdm_wakeup(clkdm);
 	return 0;
 }
 
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e034294..ef709a5 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1223,6 +1223,7 @@ static int _reset(struct omap_hwmod *oh)
 static int _enable(struct omap_hwmod *oh)
 {
 	int r;
+	int hwsup = 0;
 
 	if (oh->_state != _HWMOD_STATE_INITIALIZED &&
 	    oh->_state != _HWMOD_STATE_IDLE &&
@@ -1250,10 +1251,21 @@ static int _enable(struct omap_hwmod *oh)
 		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
 
 	_add_initiator_dep(oh, mpu_oh);
+	if (oh->_clk && oh->_clk->clkdm) {
+		hwsup = clkdm_is_idle(oh->_clk->clkdm);
+		r = clkdm_clk_enable(oh->_clk->clkdm, oh->_clk);
+		if (r) {
+			WARN(1, "clock: %s: could not enable clockdomain %s: "
+			     "%d\n", oh->_clk->name, oh->_clk->clkdm->name, r);
+			return r;
+		}
+	}
 	_enable_clocks(oh);
-
 	r = _wait_target_ready(oh);
 	if (!r) {
+		if (oh->_clk && oh->_clk->clkdm && hwsup)
+			clkdm_allow_idle(oh->_clk->clkdm);
+
 		oh->_state = _HWMOD_STATE_ENABLED;
 
 		/* Access the sysconfig only if the target is ready */
@@ -1264,6 +1276,8 @@ static int _enable(struct omap_hwmod *oh)
 		}
 	} else {
 		_disable_clocks(oh);
+		if (oh->_clk && oh->_clk->clkdm)
+			clkdm_clk_disable(oh->_clk->clkdm, oh->_clk);
 		pr_debug("omap_hwmod: %s: _wait_target_ready: %d\n",
 			 oh->name, r);
 	}
@@ -1293,6 +1307,8 @@ static int _idle(struct omap_hwmod *oh)
 		_idle_sysc(oh);
 	_del_initiator_dep(oh, mpu_oh);
 	_disable_clocks(oh);
+	if (oh->_clk && oh->_clk->clkdm)
+		clkdm_clk_disable(oh->_clk->clkdm, oh->_clk);
 
 	/* Mux pins for device idle if populated */
 	if (oh->mux && oh->mux->pads_dynamic)
@@ -1389,6 +1405,8 @@ static int _shutdown(struct omap_hwmod *oh)
 		_del_initiator_dep(oh, mpu_oh);
 		/* XXX what about the other system initiators here? dma, dsp */
 		_disable_clocks(oh);
+		if (oh->_clk && oh->_clk->clkdm)
+			clkdm_clk_disable(oh->_clk->clkdm, oh->_clk);
 	}
 	/* XXX Should this code also force-disable the optional clocks? */
 
-- 
1.7.0.4

[PATCH 7/8] OMAP: clock: Add flags to identify optional clock nodes

[Rajendra Nayak]

There is a need to identify optional clock nodes in
the clock framework, so some specific sequence
to enable them can be supported, which should be
evident in the subsequent patches.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/clock44xx_data.c    |   34 +++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/clock.h |    1 +
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 8c96567..58564fd 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -1394,6 +1394,7 @@ static struct clk bandgap_fclk = {
 	.clkdm_name	= "l4_wkup_clkdm",
 	.parent		= &sys_32k_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk des3des_fck = {
@@ -1464,6 +1465,7 @@ static struct clk dss_sys_clk = {
 	.clkdm_name	= "l3_dss_clkdm",
 	.parent		= &syc_clk_div_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk dss_tv_clk = {
@@ -1474,6 +1476,7 @@ static struct clk dss_tv_clk = {
 	.clkdm_name	= "l3_dss_clkdm",
 	.parent		= &extalt_clkin_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk dss_dss_clk = {
@@ -1484,6 +1487,7 @@ static struct clk dss_dss_clk = {
 	.clkdm_name	= "l3_dss_clkdm",
 	.parent		= &dpll_per_m5x2_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk dss_48mhz_clk = {
@@ -1494,6 +1498,7 @@ static struct clk dss_48mhz_clk = {
 	.clkdm_name	= "l3_dss_clkdm",
 	.parent		= &func_48mc_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk dss_fck = {
@@ -1577,6 +1582,7 @@ static struct clk gpio1_dbclk = {
 	.clkdm_name	= "l4_wkup_clkdm",
 	.parent		= &sys_32k_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk gpio1_ick = {
@@ -1597,6 +1603,7 @@ static struct clk gpio2_dbclk = {
 	.clkdm_name	= "l4_per_clkdm",
 	.parent		= &sys_32k_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk gpio2_ick = {
@@ -1617,6 +1624,7 @@ static struct clk gpio3_dbclk = {
 	.clkdm_name	= "l4_per_clkdm",
 	.parent		= &sys_32k_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk gpio3_ick = {
@@ -1637,6 +1645,7 @@ static struct clk gpio4_dbclk = {
 	.clkdm_name	= "l4_per_clkdm",
 	.parent		= &sys_32k_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk gpio4_ick = {
@@ -1657,6 +1666,7 @@ static struct clk gpio5_dbclk = {
 	.clkdm_name	= "l4_per_clkdm",
 	.parent		= &sys_32k_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk gpio5_ick = {
@@ -1677,6 +1687,7 @@ static struct clk gpio6_dbclk = {
 	.clkdm_name	= "l4_per_clkdm",
 	.parent		= &sys_32k_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk gpio6_ick = {
@@ -1809,6 +1820,7 @@ static struct clk iss_ctrlclk = {
 	.clkdm_name	= "iss_clkdm",
 	.parent		= &func_96m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk iss_fck = {
@@ -2145,6 +2157,7 @@ static struct clk ocp2scp_usb_phy_phy_48m = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &func_48m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk ocp2scp_usb_phy_ick = {
@@ -2206,6 +2219,7 @@ static struct clk slimbus1_fclk_1 = {
 	.clkdm_name	= "abe_clkdm",
 	.parent		= &func_24m_clk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk slimbus1_fclk_0 = {
@@ -2216,6 +2230,7 @@ static struct clk slimbus1_fclk_0 = {
 	.clkdm_name	= "abe_clkdm",
 	.parent		= &abe_24m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk slimbus1_fclk_2 = {
@@ -2226,6 +2241,7 @@ static struct clk slimbus1_fclk_2 = {
 	.clkdm_name	= "abe_clkdm",
 	.parent		= &pad_clks_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk slimbus1_slimbus_clk = {
@@ -2236,6 +2252,7 @@ static struct clk slimbus1_slimbus_clk = {
 	.clkdm_name	= "abe_clkdm",
 	.parent		= &slimbus_clk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk slimbus1_fck = {
@@ -2256,6 +2273,7 @@ static struct clk slimbus2_fclk_1 = {
 	.clkdm_name	= "l4_per_clkdm",
 	.parent		= &per_abe_24m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk slimbus2_fclk_0 = {
@@ -2266,6 +2284,7 @@ static struct clk slimbus2_fclk_0 = {
 	.clkdm_name	= "l4_per_clkdm",
 	.parent		= &func_24mc_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk slimbus2_slimbus_clk = {
@@ -2276,6 +2295,7 @@ static struct clk slimbus2_slimbus_clk = {
 	.clkdm_name	= "l4_per_clkdm",
 	.parent		= &pad_slimbus_core_clks_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk slimbus2_fck = {
@@ -2564,6 +2584,7 @@ static struct clk usb_host_hs_utmi_p1_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &utmi_p1_gfclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static const struct clksel utmi_p2_gfclk_sel[] = {
@@ -2591,6 +2612,7 @@ static struct clk usb_host_hs_utmi_p2_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &utmi_p2_gfclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_host_hs_utmi_p3_clk = {
@@ -2601,6 +2623,7 @@ static struct clk usb_host_hs_utmi_p3_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &init_60m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_host_hs_hsic480m_p1_clk = {
@@ -2611,6 +2634,7 @@ static struct clk usb_host_hs_hsic480m_p1_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &dpll_usb_m2_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_host_hs_hsic60m_p1_clk = {
@@ -2621,6 +2645,7 @@ static struct clk usb_host_hs_hsic60m_p1_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &init_60m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_host_hs_hsic60m_p2_clk = {
@@ -2631,6 +2656,7 @@ static struct clk usb_host_hs_hsic60m_p2_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &init_60m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_host_hs_hsic480m_p2_clk = {
@@ -2641,6 +2667,7 @@ static struct clk usb_host_hs_hsic480m_p2_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &dpll_usb_m2_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_host_hs_func48mclk = {
@@ -2651,6 +2678,7 @@ static struct clk usb_host_hs_func48mclk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &func_48mc_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_host_hs_fck = {
@@ -2688,6 +2716,7 @@ static struct clk usb_otg_hs_xclk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &otg_60m_gfclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_otg_hs_ick = {
@@ -2708,6 +2737,7 @@ static struct clk usb_phy_cm_clk32k = {
 	.clkdm_name	= "l4_ao_clkdm",
 	.parent		= &sys_32k_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_tll_hs_usb_ch2_clk = {
@@ -2718,6 +2748,7 @@ static struct clk usb_tll_hs_usb_ch2_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &init_60m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_tll_hs_usb_ch0_clk = {
@@ -2728,6 +2759,7 @@ static struct clk usb_tll_hs_usb_ch0_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &init_60m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_tll_hs_usb_ch1_clk = {
@@ -2738,6 +2770,7 @@ static struct clk usb_tll_hs_usb_ch1_clk = {
 	.clkdm_name	= "l3_init_clkdm",
 	.parent		= &init_60m_fclk,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usb_tll_hs_ick = {
@@ -2781,6 +2814,7 @@ static struct clk usim_fclk = {
 	.clkdm_name	= "l4_wkup_clkdm",
 	.parent		= &usim_ck,
 	.recalc		= &followparent_recalc,
+	.flags		= CLOCK_OPTCLK,
 };
 
 static struct clk usim_fck = {
diff --git a/arch/arm/plat-omap/include/plat/clock.h b/arch/arm/plat-omap/include/plat/clock.h
index 006e599..cb2de7d 100644
--- a/arch/arm/plat-omap/include/plat/clock.h
+++ b/arch/arm/plat-omap/include/plat/clock.h
@@ -189,6 +189,7 @@ struct dpll_data {
 #define ENABLE_ON_INIT		(1 << 3)	/* Enable upon framework init */
 #define INVERT_ENABLE		(1 << 4)	/* 0 enables, 1 disables */
 #define CLOCK_CLKOUTX2		(1 << 5)
+#define CLOCK_OPTCLK		(1 << 6)
 
 /**
  * struct clk - OMAP struct clk
-- 
1.7.0.4

[PATCH 8/8] OMAP: clock: Enable clockdomain only for optional clocks

[Rajendra Nayak]

Optional clocks have a requirement to have the clockdomain
force enabled (SW_WKUP) before the optional clock itself
is enabled.
Since optional clocks are currently handled directly by
drivers using the clock framework, this needs to be handled
at the clock framework. This sequence is already handled
in the omap_hwmod framework for the essential/main
clocks.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/clock.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 2828d29..ff71ff7 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -286,6 +286,7 @@ void omap2_clk_disable(struct clk *clk)
 int omap2_clk_enable(struct clk *clk)
 {
 	int ret;
+	int hwsup = 0;
 
 	pr_debug("clock: %s: incrementing usecount\n", clk->name);
 
@@ -304,6 +305,17 @@ int omap2_clk_enable(struct clk *clk)
 			goto oce_err1;
 		}
 	}
+	/*
+	 * TODO: This is needed here only as long as drivers use
+	 * clock framework to enable optional clocks. For all the
+	 * essential clocks, this sequence is handled in the
+	 * omap_hwmod framework.
+	 */
+	/* Enable the clockdomain, if its an optional clock */
+	if ((clk->flags & CLOCK_OPTCLK) && (clk->clkdm)) {
+		hwsup = clkdm_is_idle(clk->clkdm);
+		clkdm_wakeup(clk->clkdm);
+	}
 
 	if (clk->ops && clk->ops->enable) {
 		trace_clock_enable(clk->name, 1, smp_processor_id());
@@ -315,6 +327,9 @@ int omap2_clk_enable(struct clk *clk)
 		}
 	}
 
+	if ((clk->flags & CLOCK_OPTCLK) && (clk->clkdm) && hwsup)
+		clkdm_allow_idle(clk->clkdm);
+
 	return 0;
 
 oce_err2:
-- 
1.7.0.4

[PATCH 5/8] OMAP4: PM: TEMP: Prevent l3init from idling/force sleep

[Paul Walmsley]

Hi Rajendra,

On Thu, 9 Jun 2011, Rajendra Nayak wrote:

> Since MMC driver is yet to be adapted to
> runtime PM and still uses direct clock
> calls to enable/disable module, its needed
> that the clockdomain (for MMC) is always kept force
> enabled since the next few patches move
> the clockdomain handling from clock framework
> to hwmod framework and break MMC.
> 
> This will certainlly gate any CORE low power
> transitions.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  arch/arm/mach-omap2/clockdomains44xx_data.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c
> index a607ec1..ff38764 100644
> --- a/arch/arm/mach-omap2/clockdomains44xx_data.c
> +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c
> @@ -493,7 +493,7 @@ static struct clockdomain l3_init_44xx_clkdm = {
>  	.dep_bit	  = OMAP4430_L3INIT_STATDEP_SHIFT,
>  	.wkdep_srcs	  = l3_init_wkup_sleep_deps,
>  	.sleepdep_srcs	  = l3_init_wkup_sleep_deps,
> -	.flags		  = CLKDM_CAN_HWSUP_SWSUP,
> +	.flags		  = CLKDM_CAN_FORCE_WAKEUP,
>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>  };

Since this is basically a hack to work around limitations of the current 
MMC driver, if we accept something like this, I think it should come with 
a big warning message and comment.  Something like this in 
omap44xx_clockdomains_init():

	/*
	 * XXX The OMAP L3 interconnect hardware is able to enter
	 * hardware-supervised idle.  But because the OMAP HSMMC
	 * driver still hasn't been converted to use runtime PM, if
	 * the L3 is allowed to enter hwsup idle, the kernel will
	 * crash.  Once the MMC driver is fixed (which patches have
	 * been posted to do, with subject line "OMAP: HSMMC: cleanup
	 * and runtime pm") the change to the l3_init_44xx_clkdm
	 * flags should be dropped.  It limits the low-power state that
	 * the chip can enter.
	 */
	pr_warn("WARNING: OMAP4 low power states artificially limited, due 
to unconverted HSMMC driver\n");


- Paul

[PATCH 5/8] OMAP4: PM: TEMP: Prevent l3init from idling/force sleep

[Russell King - ARM Linux]

On Thu, Jun 23, 2011 at 09:04:20AM -0600, Paul Walmsley wrote:
> Since this is basically a hack to work around limitations of the current 
> MMC driver, if we accept something like this, I think it should come with 
> a big warning message and comment.  Something like this in 
> omap44xx_clockdomains_init():
> 
> 	/*
> 	 * XXX The OMAP L3 interconnect hardware is able to enter
> 	 * hardware-supervised idle.  But because the OMAP HSMMC
> 	 * driver still hasn't been converted to use runtime PM, if
> 	 * the L3 is allowed to enter hwsup idle, the kernel will
> 	 * crash.  Once the MMC driver is fixed (which patches have
> 	 * been posted to do, with subject line "OMAP: HSMMC: cleanup
> 	 * and runtime pm") the change to the l3_init_44xx_clkdm
> 	 * flags should be dropped.  It limits the low-power state that
> 	 * the chip can enter.
> 	 */
> 	pr_warn("WARNING: OMAP4 low power states artificially limited, due 
> to unconverted HSMMC driver\n");

Do you really want to continue pissing Linus off with churn like this
rather than pressing to get problems fixed _properly_ (eg, getting the
HSMMC driver fixed) ?

[PATCH 5/8] OMAP4: PM: TEMP: Prevent l3init from idling/force sleep

[Paul Walmsley]

Hi

On Thu, 23 Jun 2011, Russell King - ARM Linux wrote:

> Do you really want to continue pissing Linus off with churn like this
> rather than pressing to get problems fixed _properly_ (eg, getting the
> HSMMC driver fixed) ?

Thanks for your comments.  We won't queue this patch.


- Paul

[PATCH 4/8] OMAP2+: PM: idle clkdms only if already in idle

[Todd Poynor]

On Thu, Jun 09, 2011 at 04:24:09PM +0530, Rajendra Nayak wrote:
> The omap_set_pwrdm_state function forces clockdomains
> to idle, without checking the existing idle state
> programmed, instead based solely on the HW capability
> of the clockdomain to support idle.
> This is wrong and the clockdomains should be idled
> post a state_switch *only* if idle transitions on the
> clockdomain were already enabled.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  arch/arm/mach-omap2/pm.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index d48813f..840b0e1 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -108,6 +108,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  	u32 cur_state;
>  	int sleep_switch = -1;
>  	int ret = 0;
> +	int hwsup = 0;
>  
>  	if (pwrdm == NULL || IS_ERR(pwrdm))
>  		return -EINVAL;
> @@ -127,6 +128,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
>  			sleep_switch = LOWPOWERSTATE_SWITCH;
>  		} else {
> +			hwsup = clkdm_is_idle(pwrdm->pwrdm_clkdms[0]);
>  			clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
>  			pwrdm_wait_transition(pwrdm);
>  			sleep_switch = FORCEWAKEUP_SWITCH;
> @@ -142,7 +144,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  
>  	switch (sleep_switch) {
>  	case FORCEWAKEUP_SWITCH:
> -		if (pwrdm->pwrdm_clkdms[0]->flags & CLKDM_CAN_ENABLE_AUTO)
> +		if (hwsup)
>  			clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>  		else
>  			clkdm_sleep(pwrdm->pwrdm_clkdms[0]);

Is concurrency protection needed here?  Not sure if it's expected that
multiple threads would simultaneously manage the state of the same power
domain, or that the associated clock domain would change state
concurrently.


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

[PATCH 4/8] OMAP2+: PM: idle clkdms only if already in idle

[Rajendra Nayak]

On 6/10/2011 5:45 AM, Todd Poynor wrote:
>> +	int hwsup = 0;
>> >
>> >    	if (pwrdm == NULL || IS_ERR(pwrdm))
>> >    		return -EINVAL;
>> >  @@ -127,6 +128,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>> >    			(pwrdm->flags&  PWRDM_HAS_LOWPOWERSTATECHANGE)) {
>> >    			sleep_switch = LOWPOWERSTATE_SWITCH;
>> >    		} else {
>> >  +			hwsup = clkdm_is_idle(pwrdm->pwrdm_clkdms[0]);
>> >    			clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
>> >    			pwrdm_wait_transition(pwrdm);
>> >    			sleep_switch = FORCEWAKEUP_SWITCH;
>> >  @@ -142,7 +144,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>> >
>> >    	switch (sleep_switch) {
>> >    	case FORCEWAKEUP_SWITCH:
>> >  -		if (pwrdm->pwrdm_clkdms[0]->flags&  CLKDM_CAN_ENABLE_AUTO)
>> >  +		if (hwsup)
>> >    			clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>> >    		else
>> >    			clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
> Is concurrency protection needed here?  Not sure if it's expected that
> multiple threads would simultaneously manage the state of the same power
> domain, or that the associated clock domain would change state
> concurrently.

Yeah, maybe there is a need for a per-clkdm lock to prevent these
concurrency issues. This race between omap_set_pwrdm_state and
clk_enable/disable all programming clkdm state was one possibility
for a race (even without this series), but with this series to move the
clkdm handling into hwmod there are certainly more possibilities (across 
concurrent omap_hwmod_enable/idle) because the hwmod framework uses 
per-hwmod lock while the clock framework used a global lock.
Thanks for bringing this up.
Paul/Benoit any thoughts on if a per-clkdm lock seems reasonable?

I just did a quick patch to add per-clkdm locking, it also has a
couple instances of arch-specific clkdm calls making
calls back to generic clkdm framework fixed, which
otherwise would result in recursive locking.

----
From: Rajendra Nayak <rnayak@ti.com>
Date: Fri, 10 Jun 2011 15:42:25 +0530
Subject: [PATCH] OMAP: clockdomain: Add per clkdm lock to prevent 
concurrent state programming

Since the clkdm state programming is now done from
within the hwmod framework (which uses a per-hwmod
lock) instead of the being done from the clock
framework (which used a global lock), there is
now a need to have per-clkdm locking to prevent
races between different hwmods/modules belonging to the
same clock domain concurrently programming the
clkdm state.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
  arch/arm/mach-omap2/clockdomain.c          |   42 
++++++++++++++++++++++++++--
  arch/arm/mach-omap2/clockdomain.h          |    2 +
  arch/arm/mach-omap2/clockdomain2xxx_3xxx.c |    6 ++-
  arch/arm/mach-omap2/clockdomain44xx.c      |    7 ++--
  4 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c 
b/arch/arm/mach-omap2/clockdomain.c
index b98a972..374e669 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -92,6 +92,8 @@ static int _clkdm_register(struct clockdomain *clkdm)

  	pwrdm_add_clkdm(pwrdm, clkdm);

+	spin_lock_init(&clkdm->lock);
+
  	pr_debug("clockdomain: registered %s\n", clkdm->name);

  	return 0;
@@ -690,6 +692,9 @@ int clkdm_clear_all_sleepdeps(struct clockdomain *clkdm)
   */
  int clkdm_sleep(struct clockdomain *clkdm)
  {
+	int ret;
+	unsigned long flags;
+
  	if (!clkdm)
  		return -EINVAL;

@@ -704,7 +709,10 @@ int clkdm_sleep(struct clockdomain *clkdm)

  	pr_debug("clockdomain: forcing sleep on %s\n", clkdm->name);

-	return arch_clkdm->clkdm_sleep(clkdm);
+	spin_lock_irqsave(&clkdm->lock, flags);
+	ret = arch_clkdm->clkdm_sleep(clkdm);
+	spin_unlock_irqrestore(&clkdm->lock, flags);
+	return ret;
  }

  /**
@@ -718,6 +726,9 @@ int clkdm_sleep(struct clockdomain *clkdm)
   */
  int clkdm_wakeup(struct clockdomain *clkdm)
  {
+	int ret;
+	unsigned long flags;
+
  	if (!clkdm)
  		return -EINVAL;

@@ -732,7 +743,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)

  	pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);

-	return arch_clkdm->clkdm_wakeup(clkdm);
+	spin_lock_irqsave(&clkdm->lock, flags);
+	ret = arch_clkdm->clkdm_wakeup(clkdm);
+	spin_unlock_irqrestore(&clkdm->lock, flags);
+	return ret;
  }

  /**
@@ -747,6 +761,8 @@ int clkdm_wakeup(struct clockdomain *clkdm)
   */
  void clkdm_allow_idle(struct clockdomain *clkdm)
  {
+	unsigned long flags;
+
  	if (!clkdm)
  		return;

@@ -762,8 +778,10 @@ void clkdm_allow_idle(struct clockdomain *clkdm)
  	pr_debug("clockdomain: enabling automatic idle transitions for %s\n",
  		 clkdm->name);

+	spin_lock_irqsave(&clkdm->lock, flags);
  	arch_clkdm->clkdm_allow_idle(clkdm);
  	pwrdm_clkdm_state_switch(clkdm);
+	spin_unlock_irqrestore(&clkdm->lock, flags);
  }

  /**
@@ -777,6 +795,8 @@ void clkdm_allow_idle(struct clockdomain *clkdm)
   */
  void clkdm_deny_idle(struct clockdomain *clkdm)
  {
+	unsigned long flags;
+
  	if (!clkdm)
  		return;

@@ -792,7 +812,9 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
  	pr_debug("clockdomain: disabling automatic idle transitions for %s\n",
  		 clkdm->name);

+	spin_lock_irqsave(&clkdm->lock, flags);
  	arch_clkdm->clkdm_deny_idle(clkdm);
+	spin_unlock_irqrestore(&clkdm->lock, flags);
  }

  /**
@@ -805,6 +827,9 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
   */
  int clkdm_is_idle(struct clockdomain *clkdm)
  {
+	int ret;
+	unsigned long flags;
+
  	if (!clkdm)
  		return -EINVAL;

@@ -813,7 +838,10 @@ int clkdm_is_idle(struct clockdomain *clkdm)

  	pr_debug("clockdomain: reading idle state for %s\n", clkdm->name);

-	return arch_clkdm->clkdm_is_idle(clkdm);
+	spin_lock_irqsave(&clkdm->lock, flags);
+	ret = arch_clkdm->clkdm_is_idle(clkdm);
+	spin_unlock_irqrestore(&clkdm->lock, flags);
+	return ret;
  }


@@ -835,6 +863,8 @@ int clkdm_is_idle(struct clockdomain *clkdm)
   */
  int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
  {
+	unsigned long flags;
+
  	/*
  	 * XXX Rewrite this code to maintain a list of enabled
  	 * downstream clocks for debugging purposes?
@@ -859,9 +889,11 @@ int clkdm_clk_enable(struct clockdomain *clkdm, 
struct clk *clk)
  	pr_debug("clockdomain: clkdm %s: clk %s now enabled\n", clkdm->name,
  		 clk->name);

+	spin_lock_irqsave(&clkdm->lock, flags);
  	arch_clkdm->clkdm_clk_enable(clkdm);
  	pwrdm_wait_transition(clkdm->pwrdm.ptr);
  	pwrdm_clkdm_state_switch(clkdm);
+	spin_unlock_irqrestore(&clkdm->lock, flags);

  	return 0;
  }
@@ -882,6 +914,8 @@ int clkdm_clk_enable(struct clockdomain *clkdm, 
struct clk *clk)
   */
  int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
  {
+	unsigned long flags;
+
  	/*
  	 * XXX Rewrite this code to maintain a list of enabled
  	 * downstream clocks for debugging purposes?
@@ -908,8 +942,10 @@ int clkdm_clk_disable(struct clockdomain *clkdm, 
struct clk *clk)
  	pr_debug("clockdomain: clkdm %s: clk %s now disabled\n", clkdm->name,
  		 clk->name);

+	spin_lock_irqsave(&clkdm->lock, flags);
  	arch_clkdm->clkdm_clk_disable(clkdm);
  	pwrdm_clkdm_state_switch(clkdm);
+	spin_unlock_irqrestore(&clkdm->lock, flags);

  	return 0;
  }
diff --git a/arch/arm/mach-omap2/clockdomain.h 
b/arch/arm/mach-omap2/clockdomain.h
index 085ed82..9fd4eb5 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -17,6 +17,7 @@
  #define __ARCH_ARM_MACH_OMAP2_CLOCKDOMAIN_H

  #include <linux/init.h>
+#include <linux/spinlock.h>

  #include "powerdomain.h"
  #include <plat/clock.h>
@@ -122,6 +123,7 @@ struct clockdomain {
  	const struct omap_chip_id omap_chip;
  	atomic_t usecount;
  	struct list_head node;
+	spinlock_t lock;
  };

  /**
diff --git a/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c 
b/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c
index e0c393f..f841ac8 100644
--- a/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c
@@ -193,7 +193,8 @@ static int omap2_clkdm_clk_enable(struct clockdomain 
*clkdm)
  		_clkdm_add_autodeps(clkdm);
  		_enable_hwsup(clkdm);
  	} else {
-		clkdm_wakeup(clkdm);
+		if (clkdm->flags & CLKDM_CAN_FORCE_WAKEUP)
+			omap2_clkdm_wakeup(clkdm);
  	}

  	return 0;
@@ -215,7 +216,8 @@ static int omap2_clkdm_clk_disable(struct 
clockdomain *clkdm)
  		_clkdm_del_autodeps(clkdm);
  		_enable_hwsup(clkdm);
  	} else {
-		clkdm_sleep(clkdm);
+		if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP)
+			omap2_clkdm_sleep(clkdm);
  	}

  	return 0;
diff --git a/arch/arm/mach-omap2/clockdomain44xx.c 
b/arch/arm/mach-omap2/clockdomain44xx.c
index e4b8e06..911770b 100644
--- a/arch/arm/mach-omap2/clockdomain44xx.c
+++ b/arch/arm/mach-omap2/clockdomain44xx.c
@@ -101,7 +101,8 @@ static int omap4_clkdm_is_idle(struct clockdomain 
*clkdm)

  static int omap4_clkdm_clk_enable(struct clockdomain *clkdm)
  {
-	clkdm_wakeup(clkdm);
+	if (clkdm->flags & CLKDM_CAN_FORCE_WAKEUP)
+		return omap4_clkdm_wakeup(clkdm);
  	return 0;
  }

@@ -112,8 +113,8 @@ static int omap4_clkdm_clk_disable(struct 
clockdomain *clkdm)
  	hwsup = omap4_cminst_is_clkdm_in_hwsup(clkdm->prcm_partition,
  					clkdm->cm_inst, clkdm->clkdm_offs);

-	if (!hwsup)
-		clkdm_sleep(clkdm);
+	if (!hwsup && (clkdm->flags & CLKDM_CAN_FORCE_SLEEP))
+		omap4_clkdm_sleep(clkdm);

  	return 0;
  }
-- 
1.7.0.4

[PATCH 4/8] OMAP2+: PM: idle clkdms only if already in idle

[Paul Walmsley]

Hi Rajendra, Todd,

On Fri, 10 Jun 2011, Rajendra Nayak wrote:

> Paul/Benoit any thoughts on if a per-clkdm lock seems reasonable?

Sounds okay to me.

The experimental patch that you sent didn't add the locking to the *wkdep, 
*sleepdep functions; I guess we'd better add it there at the same time, 
since some of the register access there does a read-modify-write.

It should be possible to get rid of the atomic_t usage in the clockdomain 
code as part of the same series.

Todd, thanks for pointing this out.


- Paul

[PATCH 4/8] OMAP2+: PM: idle clkdms only if already in idle

[Rajendra Nayak]

Hi Paul,

On 6/26/2011 11:34 PM, Paul Walmsley wrote:
> Hi Rajendra, Todd,
>
> On Fri, 10 Jun 2011, Rajendra Nayak wrote:
>
>> Paul/Benoit any thoughts on if a per-clkdm lock seems reasonable?
>
> Sounds okay to me.
>
> The experimental patch that you sent didn't add the locking to the *wkdep,
> *sleepdep functions; I guess we'd better add it there at the same time,
> since some of the register access there does a read-modify-write.

My initial idea was to just guard the functions which program the
target clockdomain state, since that's something which had a possibility
of racing.
For the sleepdep/wkupdep programming, I thought they are all done from
within frameworks and during pm-core init at boot and might not run into
concurrency issues. But maybe it makes sense to guard those as well.

>
> It should be possible to get rid of the atomic_t usage in the clockdomain
> code as part of the same series.

Sure, I'll get rid of those.

Thanks,
Rajendra

>
> Todd, thanks for pointing this out.
>
>
> - Paul

[PATCH 1/8] OMAP2+: clockdomain: Add an api to read idle mode

[Todd Poynor]

On Thu, Jun 09, 2011 at 04:24:06PM +0530, Rajendra Nayak wrote:
> Add a clockdomain api to check if hardware supervised
> idle transitions are enabled on a clockdomain.
> 
...
> + * clkdm_is_idle - Check if the clkdm hwsup/autoidle is enabled

A minor suggestion on naming: "clkdm_allows_idle" seems more accurate
and a natural complement to the existing clkdm_allow_idle().


Todd

[PATCH 1/8] OMAP2+: clockdomain: Add an api to read idle mode

[Rajendra Nayak]

On 6/10/2011 5:37 AM, Todd Poynor wrote:
> On Thu, Jun 09, 2011 at 04:24:06PM +0530, Rajendra Nayak wrote:
>> Add a clockdomain api to check if hardware supervised
>> idle transitions are enabled on a clockdomain.
>>
> ...
>> + * clkdm_is_idle - Check if the clkdm hwsup/autoidle is enabled
>
> A minor suggestion on naming: "clkdm_allows_idle" seems more accurate
> and a natural complement to the existing clkdm_allow_idle().

Yeah, I agree. I'll change it in my next spin.

Thanks,
Rajendra
>
>
> Todd