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

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

[Benoit Cousson]

Hi Paul & Rajendra,

Here is an updated version of the series started by Rajendra.
I had to update it because this series is mandatory for the hwmod
modulemodule control series.
I rebased it on top of the various fixes done on hwmod framework
and to take advantage of the new clkdm attribute in omap_hwmod.
I thus added 2 new APIs to handle clockdomain from hwmod instead
of using the clockdomain done for clock.

I drop the clockdomain control for optional clocks that is not
mandatory.


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.

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
at least allow MPU low power transitions to work.

The series is based on for_3.0.1/4_hwmod_modulemode and tested on
OMAP4430 ES2.1 + SDP. It should not affect OMAP2 & 3, but some
testing are definitively needed.

The patches are available here:
git://gitorious.org/omap-pm/linux.git for_3.0.1/5_hwmod_clkdm_fixes

Regards,
Benoit


v1:
    http://www.spinics.net/lists/linux-omap/msg52265.html


Benoit Cousson (1):
  OMAP2+: clockdomain: Add 2 APIs to control clockdomain from hwmod framework

Nayak, Rajendra (6):
  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 recommended PRCM module enable sequence

 arch/arm/mach-omap2/clock.c                 |   17 +---
 arch/arm/mach-omap2/clockdomain.c           |  171 ++++++++++++++++++++++-----
 arch/arm/mach-omap2/clockdomain.h           |    6 +
 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            |   28 ++++-
 arch/arm/mach-omap2/pm.c                    |    6 +-
 8 files changed, 200 insertions(+), 58 deletions(-)

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

[Benoit Cousson]

From: Nayak, Rajendra <rnayak@ti.com>

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 v2 2/7] OMAP2+: clockdomain: Add SoC support for clkdm_is_idle

[Benoit Cousson]

From: Nayak, Rajendra <rnayak@ti.com>

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 v2 3/7] OMAP2+: PM: Initialise sleep_switch to a non-valid value

[Benoit Cousson]

From: Nayak, Rajendra <rnayak@ti.com>

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 v2 4/7] OMAP2+: PM: idle clkdms only if already in idle

[Benoit Cousson]

From: Nayak, Rajendra <rnayak@ti.com>

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 v2 5/7] OMAP4: PM: TEMP: Prevent l3init from idling/force sleep

[Benoit Cousson]

From: Nayak, Rajendra <rnayak@ti.com>

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 66090f2..863006a 100644
--- a/arch/arm/mach-omap2/clockdomains44xx_data.c
+++ b/arch/arm/mach-omap2/clockdomains44xx_data.c
@@ -494,7 +494,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 v2 6/7] OMAP2+: clockdomain: Add 2 APIs to control clockdomain from hwmod framework

[Benoit Cousson]

Duplicate the existing API for clockdomain enable from clock to enable
a clock domain from hwmod framework.
This will be needed when the hwmod framework will move from the current
clock centric approach to the module based approach.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/clockdomain.c |  150 +++++++++++++++++++++++++++++-------
 arch/arm/mach-omap2/clockdomain.h |    3 +
 2 files changed, 124 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 2ab3686..7add49e 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -817,7 +817,48 @@ int clkdm_is_idle(struct clockdomain *clkdm)
 }
 
 
-/* Clockdomain-to-clock framework interface code */
+/* Clockdomain-to-clock/hwmod framework interface code */
+
+static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
+{
+	if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_enable)
+		return -EINVAL;
+
+	/*
+	 * For arch's with no autodeps, clkcm_clk_enable
+	 * should be called for every clock instance or hwmod that is
+	 * enabled, so the clkdm can be force woken up.
+	 */
+	if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps)
+		return 0;
+
+	arch_clkdm->clkdm_clk_enable(clkdm);
+	pwrdm_wait_transition(clkdm->pwrdm.ptr);
+	pwrdm_clkdm_state_switch(clkdm);
+
+	return 0;
+}
+
+static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
+{
+	if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_disable)
+		return -EINVAL;
+
+#ifdef DEBUG
+	if (atomic_read(&clkdm->usecount) == 0) {
+		WARN_ON(1); /* underflow */
+		return -ERANGE;
+	}
+#endif
+
+	if (atomic_dec_return(&clkdm->usecount) > 0)
+		return 0;
+
+	arch_clkdm->clkdm_clk_disable(clkdm);
+	pwrdm_clkdm_state_switch(clkdm);
+
+	return 0;
+}
 
 /**
  * clkdm_clk_enable - add an enabled downstream clock to this clkdm
@@ -835,30 +876,23 @@ int clkdm_is_idle(struct clockdomain *clkdm)
  */
 int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
 {
+	int ret = 0;
+
 	/*
 	 * XXX Rewrite this code to maintain a list of enabled
 	 * downstream clocks for debugging purposes?
 	 */
 
-	if (!clkdm || !clk)
+	if (!clk)
 		return -EINVAL;
 
-	if (!arch_clkdm || !arch_clkdm->clkdm_clk_enable)
-		return -EINVAL;
-
-	if (atomic_inc_return(&clkdm->usecount) > 1)
-		return 0;
+	ret = _clkdm_clk_hwmod_enable(clkdm);
 
 	/* Clockdomain now has one enabled downstream clock */
-
 	pr_debug("clockdomain: clkdm %s: clk %s now enabled\n", clkdm->name,
 		 clk->name);
 
-	arch_clkdm->clkdm_clk_enable(clkdm);
-	pwrdm_wait_transition(clkdm->pwrdm.ptr);
-	pwrdm_clkdm_state_switch(clkdm);
-
-	return 0;
+	return ret;
 }
 
 /**
@@ -877,35 +911,93 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
  */
 int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
 {
+	int ret = 0;
+
 	/*
 	 * XXX Rewrite this code to maintain a list of enabled
 	 * downstream clocks for debugging purposes?
 	 */
 
-	if (!clkdm || !clk)
+	if (!clk)
 		return -EINVAL;
 
-	if (!arch_clkdm || !arch_clkdm->clkdm_clk_disable)
+	ret = _clkdm_clk_hwmod_disable(clkdm);
+
+	/* All downstream clocks of this clockdomain are now disabled */
+	pr_debug("clockdomain: clkdm %s: clk %s now disabled\n", clkdm->name,
+		 clk->name);
+
+	return ret;
+}
+
+/**
+ * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm
+ * @clkdm: struct clockdomain *
+ * @oh: struct omap_hwmod * of the enabled downstream hwmod
+ *
+ * Increment the usecount of the clockdomain @clkdm and ensure that it
+ * is awake before @oh is enabled. Intended to be called by
+ * module_enable() code.
+ * If the clockdomain is in software-supervised idle mode, force the
+ * clockdomain to wake.  If the clockdomain is in hardware-supervised idle
+ * mode, add clkdm-pwrdm autodependencies, to ensure that devices in the
+ * clockdomain can be read from/written to by on-chip processors.
+ * Returns -EINVAL if passed null pointers;
+ * returns 0 upon success or if the clockdomain is in hwsup idle mode.
+ */
+int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh)
+{
+	int ret = 0;
+
+	/*
+	 * XXX Rewrite this code to maintain a list of enabled
+	 * downstream hwmods for debugging purposes?
+	 */
+
+	if (!oh)
 		return -EINVAL;
 
-#ifdef DEBUG
-	if (atomic_read(&clkdm->usecount) == 0) {
-		WARN_ON(1); /* underflow */
-		return -ERANGE;
-	}
-#endif
+	ret = _clkdm_clk_hwmod_enable(clkdm);
 
-	if (atomic_dec_return(&clkdm->usecount) > 0)
-		return 0;
+	/* Clockdomain now has one enabled downstream hwmod */
+	pr_debug("clockdomain: clkdm %s: oh %s now enabled\n", clkdm->name,
+		 oh->name);
 
-	/* All downstream clocks of this clockdomain are now disabled */
+	return ret;
+}
 
-	pr_debug("clockdomain: clkdm %s: clk %s now disabled\n", clkdm->name,
-		 clk->name);
+/**
+ * clkdm_hwmod_disable - remove an enabled downstream hwmod from this clkdm
+ * @clkdm: struct clockdomain *
+ * @oh: struct omap_hwmod * of the disabled downstream hwmod
+ *
+ * Decrement the usecount of this clockdomain @clkdm when @oh is
+ * disabled. Intended to be called by module_disable() code.
+ * If the clockdomain usecount goes to 0, put the clockdomain to sleep
+ * (software-supervised mode) or remove the clkdm autodependencies
+ * (hardware-supervised mode).
+ * Returns -EINVAL if passed null pointers; -ERANGE if the @clkdm usecount
+ * underflows and debugging is enabled; or returns 0 upon success or if the
+ * clockdomain is in hwsup idle mode.
+ */
+int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh)
+{
+	int ret = 0;
 
-	arch_clkdm->clkdm_clk_disable(clkdm);
-	pwrdm_clkdm_state_switch(clkdm);
+	/*
+	 * XXX Rewrite this code to maintain a list of enabled
+	 * downstream hwmods for debugging purposes?
+	 */
 
-	return 0;
+	if (!oh)
+		return -EINVAL;
+
+	ret = _clkdm_clk_hwmod_disable(clkdm);
+
+	/* All downstream hwmods of this clockdomain are now disabled */
+	pr_debug("clockdomain: clkdm %s: oh %s now disabled\n", clkdm->name,
+		 oh->name);
+
+	return ret;
 }
 
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 085ed82..77244b3 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -20,6 +20,7 @@
 
 #include "powerdomain.h"
 #include <plat/clock.h>
+#include <plat/omap_hwmod.h>
 #include <plat/cpu.h>
 
 /*
@@ -186,6 +187,8 @@ int clkdm_sleep(struct clockdomain *clkdm);
 
 int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk);
 int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk);
+int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh);
+int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh);
 
 extern void __init omap2xxx_clockdomains_init(void);
 extern void __init omap3xxx_clockdomains_init(void);
-- 
1.7.0.4

[PATCH v2 7/7] OMAP2+: hwmod: Follow the recommended PRCM module enable sequence

[Benoit Cousson]

From: Nayak, Rajendra <rnayak@ti.com>

On OMAP4, the PRCM recommended sequence for enabling
a module after power-on-reset is:
-1- Force clkdm to SW_WKUP
-2- Enabling the clocks
-3- Configure desired module mode to "enable" or "auto"
-4- Wait for the desired module idle status to be FUNC
-5- 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 -4- 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>
[b-cousson@ti.com: Adapt it to the new clkdm hwmod attribute and API]
Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clock.c           |   17 +----------------
 arch/arm/mach-omap2/clockdomain44xx.c |    9 +--------
 arch/arm/mach-omap2/omap_hwmod.c      |   28 ++++++++++++++++++++++++++--
 3 files changed, 28 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/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 6b144c4..cb85f65 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1360,6 +1360,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 &&
@@ -1378,6 +1379,19 @@ static int _enable(struct omap_hwmod *oh)
 		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
 
 	_add_initiator_dep(oh, mpu_oh);
+
+	/*
+	 * A clockdomain must be in SW_SUP before enabling completely the
+	 * module. The clockdomain can be set in HW_AUTO only when the module
+	 * become ready.
+	 */
+	hwsup = clkdm_is_idle(oh->clkdm);
+	r = clkdm_hwmod_enable(oh->clkdm, oh);
+	if (r) {
+		WARN(1, "omap_hwmod: %s: could not enable clockdomain %s: %d\n",
+		     oh->name, oh->clkdm->name, r);
+		return r;
+	}
 	_enable_clocks(oh);
 	_enable_module(oh);
 
@@ -1392,6 +1406,13 @@ static int _enable(struct omap_hwmod *oh)
 
 	r = _wait_target_ready(oh);
 	if (!r) {
+		/*
+		 * Set the clockdomain to HW_AUTO only if the target is ready,
+		 * assuming that the previous state was HW_AUTO
+		 */
+		if (hwsup)
+			clkdm_allow_idle(oh->clkdm);
+
 		oh->_state = _HWMOD_STATE_ENABLED;
 
 		/* Access the sysconfig only if the target is ready */
@@ -1402,8 +1423,9 @@ static int _enable(struct omap_hwmod *oh)
 		}
 	} else {
 		_disable_clocks(oh);
-		pr_debug("omap_hwmod: %s: _wait_target_ready: %d\n",
-			 oh->name, r);
+		clkdm_hwmod_disable(oh->clkdm, oh);
+		pr_warning("omap_hwmod: %s: _wait_target_ready failed: %d\n",
+			   oh->name, r);
 	}
 
 	return r;
@@ -1444,6 +1466,7 @@ static int _idle(struct omap_hwmod *oh)
 	 * transition to complete properly.
 	 */
 	_disable_clocks(oh);
+	clkdm_hwmod_disable(oh->clkdm, oh);
 
 	/* Mux pins for device idle if populated */
 	if (oh->mux && oh->mux->pads_dynamic)
@@ -1541,6 +1564,7 @@ static int _shutdown(struct omap_hwmod *oh)
 			pr_warning("omap_hwmod: %s: _wait_target_disable failed\n",
 				   oh->name);
 		_disable_clocks(oh);
+		clkdm_hwmod_disable(oh->clkdm, oh);
 	}
 	/* XXX Should this code also force-disable the optional clocks? */
 
-- 
1.7.0.4

Re: [PATCH v2 0/7] Fix module-mode enable sequence on OMAP4

[Rajendra Nayak]

On 6/24/2011 5:06 AM, Benoit Cousson wrote:
> Hi Paul&  Rajendra,
>
> Here is an updated version of the series started by Rajendra.
> I had to update it because this series is mandatory for the hwmod
> modulemodule control series.
> I rebased it on top of the various fixes done on hwmod framework
> and to take advantage of the new clkdm attribute in omap_hwmod.
> I thus added 2 new APIs to handle clockdomain from hwmod instead
> of using the clockdomain done for clock.
>
> I drop the clockdomain control for optional clocks that is not
> mandatory.

There were also some locking issues with this series pointed out
by Todd, for which I did a RFC patch to add a per-clkdm lock.
Any thoughts on that? We might need that or something similar to
prevent concurrent clkdm states being programmed, now that they
are done from hwmod (with a per-hwmod lock held) instead of being
earlier done from clk framework with a global lock held.

>
>
> 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.
>
> 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
> at least allow MPU low power transitions to work.
>
> The series is based on for_3.0.1/4_hwmod_modulemode and tested on
> OMAP4430 ES2.1 + SDP. It should not affect OMAP2&  3, but some
> testing are definitively needed.
>
> The patches are available here:
> git://gitorious.org/omap-pm/linux.git for_3.0.1/5_hwmod_clkdm_fixes
>
> Regards,
> Benoit
>
>
> v1:
>      http://www.spinics.net/lists/linux-omap/msg52265.html
>
>
> Benoit Cousson (1):
>    OMAP2+: clockdomain: Add 2 APIs to control clockdomain from hwmod framework
>
> Nayak, Rajendra (6):
>    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 recommended PRCM module enable sequence
>
>   arch/arm/mach-omap2/clock.c                 |   17 +---
>   arch/arm/mach-omap2/clockdomain.c           |  171 ++++++++++++++++++++++-----
>   arch/arm/mach-omap2/clockdomain.h           |    6 +
>   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            |   28 ++++-
>   arch/arm/mach-omap2/pm.c                    |    6 +-
>   8 files changed, 200 insertions(+), 58 deletions(-)
>

Re: [PATCH v2 0/7] Fix module-mode enable sequence on OMAP4

[Cousson, Benoit]

On 6/24/2011 3:23 PM, Nayak, Rajendra wrote:
> On 6/24/2011 5:06 AM, Benoit Cousson wrote:
>> Hi Paul&   Rajendra,
>>
>> Here is an updated version of the series started by Rajendra.
>> I had to update it because this series is mandatory for the hwmod
>> modulemodule control series.
>> I rebased it on top of the various fixes done on hwmod framework
>> and to take advantage of the new clkdm attribute in omap_hwmod.
>> I thus added 2 new APIs to handle clockdomain from hwmod instead
>> of using the clockdomain done for clock.
>>
>> I drop the clockdomain control for optional clocks that is not
>> mandatory.
>
> There were also some locking issues with this series pointed out
> by Todd, for which I did a RFC patch to add a per-clkdm lock.
> Any thoughts on that? We might need that or something similar to
> prevent concurrent clkdm states being programmed, now that they
> are done from hwmod (with a per-hwmod lock held) instead of being
> earlier done from clk framework with a global lock held.

Oops, good point I forgot that one.
I'm fine with it, but I think Paul was wondering if one global lock for 
clockdomain will not be enough. For the moment, it is maybe safer to 
keep one lock per domain.
I'll add it for the next revision.

Thanks,
Benoit

Re: [PATCH v2 6/7] OMAP2+: clockdomain: Add 2 APIs to control clockdomain from hwmod framework

[Todd Poynor]

On Fri, Jun 24, 2011 at 02:06:32PM +0200, Benoit Cousson wrote:
> Duplicate the existing API for clockdomain enable from clock to enable
> a clock domain from hwmod framework.
> This will be needed when the hwmod framework will move from the current
> clock centric approach to the module based approach.
> 
...
> +static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
> +{
> +	if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_disable)
> +		return -EINVAL;
> +
> +#ifdef DEBUG
> +	if (atomic_read(&clkdm->usecount) == 0) {
> +		WARN_ON(1); /* underflow */
> +		return -ERANGE;
> +	}
> +#endif
> +
> +	if (atomic_dec_return(&clkdm->usecount) > 0)
> +		return 0;

Suggest taking the error return out of the #ifdef DEBUG code (mainly
to lessen the chance that enabling debugging features makes problems
appear or disappear, although the WARN_ON should be a prominent clue
in this case), and maybe just have the function always handle
underflow.

...
> @@ -877,35 +911,93 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
>   */
>  int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
>  {
> +	int ret = 0;
> +
>  	/*
>  	 * XXX Rewrite this code to maintain a list of enabled
>  	 * downstream clocks for debugging purposes?
>  	 */
>  
> -	if (!clkdm || !clk)
> +	if (!clk)
>  		return -EINVAL;
>  
> -	if (!arch_clkdm || !arch_clkdm->clkdm_clk_disable)
> +	ret = _clkdm_clk_hwmod_disable(clkdm);
> +
> +	/* All downstream clocks of this clockdomain are now disabled */
> +	pr_debug("clockdomain: clkdm %s: clk %s now disabled\n", clkdm->name,
> +		 clk->name);

The pr_debug should be printed only if ret == 0.

The comment seems true only if the clock domain's usecount went to zero.

The terminology here may be a bit confusing: the function does not actually
disable the named downstream clock in the usual sense
(as in clk_disable(clk), or any action to individually gate that
clock)  Similar comments for clkdm_clk_enable). 

Similar comments for clkdm_hwmod_disable.

Re: [PATCH v2 6/7] OMAP2+: clockdomain: Add 2 APIs to control clockdomain from hwmod framework

[Cousson, Benoit]

Hi Todd,

On 6/26/2011 10:07 PM, Todd Poynor wrote:
> On Fri, Jun 24, 2011 at 02:06:32PM +0200, Benoit Cousson wrote:
>> Duplicate the existing API for clockdomain enable from clock to enable
>> a clock domain from hwmod framework.
>> This will be needed when the hwmod framework will move from the current
>> clock centric approach to the module based approach.
>>
> ...
>> +static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
>> +{
>> +	if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_disable)
>> +		return -EINVAL;
>> +
>> +#ifdef DEBUG
>> +	if (atomic_read(&clkdm->usecount) == 0) {
>> +		WARN_ON(1); /* underflow */
>> +		return -ERANGE;
>> +	}
>> +#endif
>> +
>> +	if (atomic_dec_return(&clkdm->usecount)>  0)
>> +		return 0;
>
> Suggest taking the error return out of the #ifdef DEBUG code (mainly
> to lessen the chance that enabling debugging features makes problems
> appear or disappear, although the WARN_ON should be a prominent clue
> in this case), and maybe just have the function always handle
> underflow.

If your final point is that we'd better remove the #ifdef, I do agree.

> ...
>> @@ -877,35 +911,93 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
>>    */
>>   int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
>>   {
>> +	int ret = 0;
>> +
>>   	/*
>>   	 * XXX Rewrite this code to maintain a list of enabled
>>   	 * downstream clocks for debugging purposes?
>>   	 */
>>
>> -	if (!clkdm || !clk)
>> +	if (!clk)
>>   		return -EINVAL;
>>
>> -	if (!arch_clkdm || !arch_clkdm->clkdm_clk_disable)
>> +	ret = _clkdm_clk_hwmod_disable(clkdm);
>> +
>> +	/* All downstream clocks of this clockdomain are now disabled */
>> +	pr_debug("clockdomain: clkdm %s: clk %s now disabled\n", clkdm->name,
>> +		 clk->name);
>
> The pr_debug should be printed only if ret == 0.

That's not even enough, because the function is returning 0 even if the 
domain is already enabled.

> The comment seems true only if the clock domain's usecount went to zero.
>
> The terminology here may be a bit confusing: the function does not actually
> disable the named downstream clock in the usual sense
> (as in clk_disable(clk), or any action to individually gate that
> clock)  Similar comments for clkdm_clk_enable).

Yeah, I kept that from the original code, but in fact, I'll probably get 
rid of the pr_debug here and put it in the two helper functions.

Benoit