public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/7] Fix module-mode enable sequence on OMAP4
@ 2011-07-01 21:26 Benoit Cousson
  2011-07-01 21:27 ` [PATCH v4 1/7] OMAP2+: clockdomain: Add an api to read idle mode Benoit Cousson
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Benoit Cousson @ 2011-07-01 21:26 UTC (permalink / raw)
  To: paul; +Cc: rnayak, linux-omap, linux-arm-kernel, Benoit Cousson

Hi Paul,

Here is an updated version of the series started by Rajendra.
It takes into account comments from Todd.

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
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 because 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.1/5_auto_fck_clkdev 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.1/6_hwmod_clkdm_fixes

Regards,
Benoit


Changes since v1: http://www.spinics.net/lists/linux-omap/msg52265.html
    - Rebase series on top on modulemode series
    
Changes since v2: http://www.spinics.net/lists/linux-omap/msg53111.html
    - Remove #ifdef
    - Rename clkdm_is_idle
    - Remove confusing pr_debug message in clkdm_xxx functions
    - Add per clkdm spinlock to avoid concurrent access

Changes since v2:
    - Remove the TEMP hacks since the MMC driver was fixed.
    - Rename clkdm_is_idle by clkdm_allows_idle in the subject as well.
    - Fix some regression on OMAP2 thanks to Kevin Hilman


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

Rajendra Nayak (6):
  OMAP2+: clockdomain: Add an api to read idle mode
  OMAP2+: clockdomain: Add SoC support for clkdm_allows_idle
  OMAP2+: PM: Initialise sleep_switch to a non-valid value
  OMAP2+: PM: idle clkdms only if already in idle
  OMAP2+: clockdomain: Add per clkdm lock to prevent concurrent state programming
  OMAP2+: hwmod: Follow the recommended PRCM module enable sequence

 arch/arm/mach-omap2/clock.c                |   17 +--
 arch/arm/mach-omap2/clockdomain.c          |  204 ++++++++++++++++++++++------
 arch/arm/mach-omap2/clockdomain.h          |    8 +
 arch/arm/mach-omap2/clockdomain2xxx_3xxx.c |   18 ++-
 arch/arm/mach-omap2/clockdomain44xx.c      |   20 ++--
 arch/arm/mach-omap2/omap_hwmod.c           |   25 ++++
 arch/arm/mach-omap2/pm.c                   |    6 +-
 7 files changed, 230 insertions(+), 68 deletions(-)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 1/7] OMAP2+: clockdomain: Add an api to read idle mode
  2011-07-01 21:26 [PATCH v4 0/7] Fix module-mode enable sequence on OMAP4 Benoit Cousson
@ 2011-07-01 21:27 ` Benoit Cousson
  2011-07-09  0:06   ` Paul Walmsley
  2011-07-01 21:27 ` [PATCH v4 2/7] OMAP2+: clockdomain: Add SoC support for clkdm_allows_idle Benoit Cousson
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Benoit Cousson @ 2011-07-01 21:27 UTC (permalink / raw)
  To: paul; +Cc: rnayak, linux-omap, linux-arm-kernel, Todd Poynor

From: Rajendra Nayak <rnayak@ti.com>

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

Thanks to Todd Poynor <toddpoynor@google.com> for a
better function name suggestion.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Todd Poynor <toddpoynor@google.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..3f2a8e7 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_allows_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_allows_idle(struct clockdomain *clkdm)
+{
+	if (!clkdm)
+		return -EINVAL;
+
+	if (!arch_clkdm || !arch_clkdm->clkdm_allows_idle)
+		return -EINVAL;
+
+	pr_debug("clockdomain: reading idle state for %s\n", clkdm->name);
+
+	return arch_clkdm->clkdm_allows_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..2d3d970 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_allows_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_allows_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_allows_idle(struct clockdomain *clkdm);
 
 int clkdm_wakeup(struct clockdomain *clkdm);
 int clkdm_sleep(struct clockdomain *clkdm);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 2/7] OMAP2+: clockdomain: Add SoC support for clkdm_allows_idle
  2011-07-01 21:26 [PATCH v4 0/7] Fix module-mode enable sequence on OMAP4 Benoit Cousson
  2011-07-01 21:27 ` [PATCH v4 1/7] OMAP2+: clockdomain: Add an api to read idle mode Benoit Cousson
@ 2011-07-01 21:27 ` Benoit Cousson
  2011-07-01 21:27 ` [PATCH v4 3/7] OMAP2+: PM: Initialise sleep_switch to a non-valid value Benoit Cousson
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Benoit Cousson @ 2011-07-01 21:27 UTC (permalink / raw)
  To: paul; +Cc: rnayak, linux-omap, linux-arm-kernel, Benoit Cousson

From: Rajendra Nayak <rnayak@ti.com>

Add the SoC specific implementation for clkdm_allows_idle
for OMAP2/3 and OMAP4.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
[b-cousson@ti.com: Update the changelog and subject]
Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.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..81f3e46 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_allows_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_allows_idle	= omap2_clkdm_allows_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_allows_idle	= omap2_clkdm_allows_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..1599b18 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_allows_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_allows_idle	= omap4_clkdm_allows_idle,
 	.clkdm_clk_enable	= omap4_clkdm_clk_enable,
 	.clkdm_clk_disable	= omap4_clkdm_clk_disable,
 };
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 3/7] OMAP2+: PM: Initialise sleep_switch to a non-valid value
  2011-07-01 21:26 [PATCH v4 0/7] Fix module-mode enable sequence on OMAP4 Benoit Cousson
  2011-07-01 21:27 ` [PATCH v4 1/7] OMAP2+: clockdomain: Add an api to read idle mode Benoit Cousson
  2011-07-01 21:27 ` [PATCH v4 2/7] OMAP2+: clockdomain: Add SoC support for clkdm_allows_idle Benoit Cousson
@ 2011-07-01 21:27 ` Benoit Cousson
  2011-07-07 22:36   ` Paul Walmsley
  2011-07-01 21:27 ` [PATCH v4 4/7] OMAP2+: PM: idle clkdms only if already in idle Benoit Cousson
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Benoit Cousson @ 2011-07-01 21:27 UTC (permalink / raw)
  To: paul; +Cc: rnayak, linux-omap, linux-arm-kernel

From: Rajendra Nayak <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>
Cc: Paul Walmsley <paul@pwsan.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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 4/7] OMAP2+: PM: idle clkdms only if already in idle
  2011-07-01 21:26 [PATCH v4 0/7] Fix module-mode enable sequence on OMAP4 Benoit Cousson
                   ` (2 preceding siblings ...)
  2011-07-01 21:27 ` [PATCH v4 3/7] OMAP2+: PM: Initialise sleep_switch to a non-valid value Benoit Cousson
@ 2011-07-01 21:27 ` Benoit Cousson
  2011-07-07 22:37   ` Paul Walmsley
  2011-07-01 21:27 ` [PATCH v4 5/7] OMAP2+: clockdomain: Add 2 APIs to control clockdomain from hwmod framework Benoit Cousson
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Benoit Cousson @ 2011-07-01 21:27 UTC (permalink / raw)
  To: paul; +Cc: rnayak, linux-omap, linux-arm-kernel

From: Rajendra Nayak <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>
Cc: Paul Walmsley <paul@pwsan.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..ce1a9f3 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_allows_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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 5/7] OMAP2+: clockdomain: Add 2 APIs to control clockdomain from hwmod framework
  2011-07-01 21:26 [PATCH v4 0/7] Fix module-mode enable sequence on OMAP4 Benoit Cousson
                   ` (3 preceding siblings ...)
  2011-07-01 21:27 ` [PATCH v4 4/7] OMAP2+: PM: idle clkdms only if already in idle Benoit Cousson
@ 2011-07-01 21:27 ` Benoit Cousson
  2011-07-01 21:27 ` [PATCH v4 6/7] OMAP2+: clockdomain: Add per clkdm lock to prevent concurrent state programming Benoit Cousson
  2011-07-01 21:27 ` [PATCH v4 7/7] OMAP2+: hwmod: Follow the recommended PRCM module enable sequence Benoit Cousson
  6 siblings, 0 replies; 14+ messages in thread
From: Benoit Cousson @ 2011-07-01 21:27 UTC (permalink / raw)
  To: paul; +Cc: rnayak, linux-omap, linux-arm-kernel, Benoit Cousson,
	Kevin Hilman

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.

These APIs are returning 0 for the moment for OMAP2 and OMAP3 until
their hwmods are updated with the clksm attribute.

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

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 3f2a8e7..10f6bd2 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -817,7 +817,50 @@ int clkdm_allows_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);
+
+	pr_debug("clockdomain: clkdm %s: enabled\n", clkdm->name);
+
+	return 0;
+}
+
+static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
+{
+	if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_disable)
+		return -EINVAL;
+
+	if (atomic_read(&clkdm->usecount) == 0) {
+		WARN_ON(1); /* underflow */
+		return -ERANGE;
+	}
+
+	if (atomic_dec_return(&clkdm->usecount) > 0)
+		return 0;
+
+	arch_clkdm->clkdm_clk_disable(clkdm);
+	pwrdm_clkdm_state_switch(clkdm);
+
+	pr_debug("clockdomain: clkdm %s: disabled\n", clkdm->name);
+
+	return 0;
+}
 
 /**
  * clkdm_clk_enable - add an enabled downstream clock to this clkdm
@@ -840,25 +883,10 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
 	 * downstream clocks for debugging purposes?
 	 */
 
-	if (!clkdm || !clk)
-		return -EINVAL;
-
-	if (!arch_clkdm || !arch_clkdm->clkdm_clk_enable)
+	if (!clk)
 		return -EINVAL;
 
-	if (atomic_inc_return(&clkdm->usecount) > 1)
-		return 0;
-
-	/* 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 _clkdm_clk_hwmod_enable(clkdm);
 }
 
 /**
@@ -871,9 +899,8 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
  * 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.
+ * pointers; -ERANGE if the @clkdm usecount underflows; or returns 0
+ * upon success or if the clockdomain is in hwsup idle mode.
  */
 int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
 {
@@ -882,30 +909,72 @@ int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
 	 * downstream clocks for debugging purposes?
 	 */
 
-	if (!clkdm || !clk)
+	if (!clk)
 		return -EINVAL;
 
-	if (!arch_clkdm || !arch_clkdm->clkdm_clk_disable)
+	return _clkdm_clk_hwmod_disable(clkdm);
+}
+
+/**
+ * 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)
+{
+	/* The clkdm attribute does not exist yet prior OMAP4 */
+	if (cpu_is_omap24xx() || cpu_is_omap34xx())
+		return 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
+	return _clkdm_clk_hwmod_enable(clkdm);
+}
 
-	if (atomic_dec_return(&clkdm->usecount) > 0)
+/**
+ * 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; 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)
+{
+	/* The clkdm attribute does not exist yet prior OMAP4 */
+	if (cpu_is_omap24xx() || cpu_is_omap34xx())
 		return 0;
 
-	/* All downstream clocks of this clockdomain are now disabled */
-
-	pr_debug("clockdomain: clkdm %s: clk %s now disabled\n", clkdm->name,
-		 clk->name);
+	/*
+	 * XXX Rewrite this code to maintain a list of enabled
+	 * downstream hwmods for debugging purposes?
+	 */
 
-	arch_clkdm->clkdm_clk_disable(clkdm);
-	pwrdm_clkdm_state_switch(clkdm);
+	if (!oh)
+		return -EINVAL;
 
-	return 0;
+	return _clkdm_clk_hwmod_disable(clkdm);
 }
 
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 2d3d970..8f0a7d6 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 6/7] OMAP2+: clockdomain: Add per clkdm lock to prevent concurrent state programming
  2011-07-01 21:26 [PATCH v4 0/7] Fix module-mode enable sequence on OMAP4 Benoit Cousson
                   ` (4 preceding siblings ...)
  2011-07-01 21:27 ` [PATCH v4 5/7] OMAP2+: clockdomain: Add 2 APIs to control clockdomain from hwmod framework Benoit Cousson
@ 2011-07-01 21:27 ` Benoit Cousson
  2011-07-01 21:27 ` [PATCH v4 7/7] OMAP2+: hwmod: Follow the recommended PRCM module enable sequence Benoit Cousson
  6 siblings, 0 replies; 14+ messages in thread
From: Benoit Cousson @ 2011-07-01 21:27 UTC (permalink / raw)
  To: paul; +Cc: rnayak, linux-omap, linux-arm-kernel, Benoit Cousson

From: Rajendra Nayak <rnayak@ti.com>

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>
Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.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      |   13 ++------
 4 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 10f6bd2..cc2e495 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_allows_idle(struct clockdomain *clkdm)
 {
+	int ret;
+	unsigned long flags;
+
 	if (!clkdm)
 		return -EINVAL;
 
@@ -813,7 +838,10 @@ int clkdm_allows_idle(struct clockdomain *clkdm)
 
 	pr_debug("clockdomain: reading idle state for %s\n", clkdm->name);
 
-	return arch_clkdm->clkdm_allows_idle(clkdm);
+	spin_lock_irqsave(&clkdm->lock, flags);
+	ret = arch_clkdm->clkdm_allows_idle(clkdm);
+	spin_unlock_irqrestore(&clkdm->lock, flags);
+	return ret;
 }
 
 
@@ -821,6 +849,8 @@ int clkdm_allows_idle(struct clockdomain *clkdm)
 
 static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 {
+	unsigned long flags;
+
 	if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_enable)
 		return -EINVAL;
 
@@ -832,9 +862,11 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 	if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps)
 		return 0;
 
+	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);
 
 	pr_debug("clockdomain: clkdm %s: enabled\n", clkdm->name);
 
@@ -843,6 +875,8 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
 
 static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
 {
+	unsigned long flags;
+
 	if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_disable)
 		return -EINVAL;
 
@@ -854,8 +888,10 @@ static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
 	if (atomic_dec_return(&clkdm->usecount) > 0)
 		return 0;
 
+	spin_lock_irqsave(&clkdm->lock, flags);
 	arch_clkdm->clkdm_clk_disable(clkdm);
 	pwrdm_clkdm_state_switch(clkdm);
+	spin_unlock_irqrestore(&clkdm->lock, flags);
 
 	pr_debug("clockdomain: clkdm %s: disabled\n", clkdm->name);
 
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 8f0a7d6..b51f558 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>
@@ -123,6 +124,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 81f3e46..d94a121 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 1599b18..3192a5e 100644
--- a/arch/arm/mach-omap2/clockdomain44xx.c
+++ b/arch/arm/mach-omap2/clockdomain44xx.c
@@ -101,13 +101,8 @@ static int omap4_clkdm_allows_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);
+	if (clkdm->flags & CLKDM_CAN_FORCE_WAKEUP)
+		return omap4_clkdm_wakeup(clkdm);
 
 	return 0;
 }
@@ -119,8 +114,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 7/7] OMAP2+: hwmod: Follow the recommended PRCM module enable sequence
  2011-07-01 21:26 [PATCH v4 0/7] Fix module-mode enable sequence on OMAP4 Benoit Cousson
                   ` (5 preceding siblings ...)
  2011-07-01 21:27 ` [PATCH v4 6/7] OMAP2+: clockdomain: Add per clkdm lock to prevent concurrent state programming Benoit Cousson
@ 2011-07-01 21:27 ` Benoit Cousson
  2011-07-09  8:15   ` Paul Walmsley
  6 siblings, 1 reply; 14+ messages in thread
From: Benoit Cousson @ 2011-07-01 21:27 UTC (permalink / raw)
  To: paul; +Cc: rnayak, linux-omap, linux-arm-kernel, Benoit Cousson

From: Rajendra Nayak <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/omap_hwmod.c |   25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 16 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/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 117a4d5..3d7a37e 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1362,6 +1362,7 @@ static int _reset(struct omap_hwmod *oh)
 static int _enable(struct omap_hwmod *oh)
 {
 	int r;
+	int hwsup = 0;
 
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
 
@@ -1380,6 +1381,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_allows_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);
 
@@ -1396,11 +1410,20 @@ static int _enable(struct omap_hwmod *oh)
 	if (r) {
 		pr_debug("omap_hwmod: %s: _wait_target_ready: %d\n",
 			 oh->name, r);
+
 		_disable_clocks(oh);
+		clkdm_hwmod_disable(oh->clkdm, oh);
 
 		return 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 */
@@ -1448,6 +1471,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)
@@ -1545,6 +1569,7 @@ static int _shutdown(struct omap_hwmod *oh)
 			pr_debug("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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/7] OMAP2+: PM: Initialise sleep_switch to a non-valid value
  2011-07-01 21:27 ` [PATCH v4 3/7] OMAP2+: PM: Initialise sleep_switch to a non-valid value Benoit Cousson
@ 2011-07-07 22:36   ` Paul Walmsley
  2011-07-08 14:19     ` Kevin Hilman
  0 siblings, 1 reply; 14+ messages in thread
From: Paul Walmsley @ 2011-07-07 22:36 UTC (permalink / raw)
  To: Benoit Cousson; +Cc: rnayak, khilman, linux-omap, linux-arm-kernel

cc'ing Kevin, since this touches mach-omap2/pm.c - an ack would be great 
if you have the chance

On Fri, 1 Jul 2011, Benoit Cousson wrote:

> From: Rajendra Nayak <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>
> Cc: Paul Walmsley <paul@pwsan.com>

Thanks, this patch has been queued for 3.1 at 
git://git.pwsan.com/linux-2.6 in the 'powerdomain_fixes_3.1' branch.


- Paul

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 4/7] OMAP2+: PM: idle clkdms only if already in idle
  2011-07-01 21:27 ` [PATCH v4 4/7] OMAP2+: PM: idle clkdms only if already in idle Benoit Cousson
@ 2011-07-07 22:37   ` Paul Walmsley
  2011-07-08 14:19     ` Kevin Hilman
  0 siblings, 1 reply; 14+ messages in thread
From: Paul Walmsley @ 2011-07-07 22:37 UTC (permalink / raw)
  To: Benoit Cousson; +Cc: rnayak, khilman, linux-omap, linux-arm-kernel

cc'ing Kevin, since this touches mach-omap2/pm.c - an ack would be great  
if you have the chance

On Fri, 1 Jul 2011, Benoit Cousson wrote:

> From: Rajendra Nayak <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>
> Cc: Paul Walmsley <paul@pwsan.com>

Thanks, this patch has been queued for 3.1 at 
git://git.pwsan.com/linux-2.6 in the 'powerdomain_fixes_3.1' branch.


- Paul

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/7] OMAP2+: PM: Initialise sleep_switch to a non-valid value
  2011-07-07 22:36   ` Paul Walmsley
@ 2011-07-08 14:19     ` Kevin Hilman
  0 siblings, 0 replies; 14+ messages in thread
From: Kevin Hilman @ 2011-07-08 14:19 UTC (permalink / raw)
  To: Paul Walmsley; +Cc: Benoit Cousson, rnayak, linux-omap, linux-arm-kernel

Paul Walmsley <paul@pwsan.com> writes:

> cc'ing Kevin, since this touches mach-omap2/pm.c - an ack would be great 
> if you have the chance
>
> On Fri, 1 Jul 2011, Benoit Cousson wrote:
>
>> From: Rajendra Nayak <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>
>> Cc: Paul Walmsley <paul@pwsan.com>


Acked-by: Kevin Hilman <khilman@ti.com>


>
> Thanks, this patch has been queued for 3.1 at 
> git://git.pwsan.com/linux-2.6 in the 'powerdomain_fixes_3.1' branch.
>
>
> - Paul

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 4/7] OMAP2+: PM: idle clkdms only if already in idle
  2011-07-07 22:37   ` Paul Walmsley
@ 2011-07-08 14:19     ` Kevin Hilman
  0 siblings, 0 replies; 14+ messages in thread
From: Kevin Hilman @ 2011-07-08 14:19 UTC (permalink / raw)
  To: Paul Walmsley; +Cc: Benoit Cousson, rnayak, linux-omap, linux-arm-kernel

Paul Walmsley <paul@pwsan.com> writes:

> cc'ing Kevin, since this touches mach-omap2/pm.c - an ack would be great  
> if you have the chance
>
> On Fri, 1 Jul 2011, Benoit Cousson wrote:
>
>> From: Rajendra Nayak <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>
>> Cc: Paul Walmsley <paul@pwsan.com>

Acked-by: Kevin Hilman <khilman@ti.com>

> Thanks, this patch has been queued for 3.1 at 
> git://git.pwsan.com/linux-2.6 in the 'powerdomain_fixes_3.1' branch.
>
>
> - Paul

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/7] OMAP2+: clockdomain: Add an api to read idle mode
  2011-07-01 21:27 ` [PATCH v4 1/7] OMAP2+: clockdomain: Add an api to read idle mode Benoit Cousson
@ 2011-07-09  0:06   ` Paul Walmsley
  0 siblings, 0 replies; 14+ messages in thread
From: Paul Walmsley @ 2011-07-09  0:06 UTC (permalink / raw)
  To: rnayak, Benoit Cousson; +Cc: linux-omap, linux-arm-kernel, Todd Poynor

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6249 bytes --]

Hi

On Fri, 1 Jul 2011, Benoit Cousson wrote:

> From: Rajendra Nayak <rnayak@ti.com>
> 
> Add a clockdomain api to check if hardware supervised
> idle transitions are enabled on a clockdomain.
> 
> Thanks to Todd Poynor <toddpoynor@google.com> for a
> better function name suggestion.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Todd Poynor <toddpoynor@google.com>

I've been looking at this patch and the subsequent one.  Two thoughts 
occurred to me.

The first is that clkdm_allows_idle() shouldn't need to touch the CM 
hardware at all, since all clockdomain auto-idle mode changes should go 
through the clockdomain code.  So we can just keep an in-memory flag in 
the struct clockdomain and avoid the slow CM register access.

The second is that the name 'clkdm_allows_idle', while a better name 
than the original, still doesn't seem right.  Naïvely, I'd expect a 
function with that name to reveal whether the clockdomain supports 
auto-idle mode, not to test its current state.  So this function has been 
renamed to 'clkdm_in_hwsup'.  This name seems slightly uglier, but more 
accurate.

Updated patch below, queued for 3.1 at git://git.pwsan.com/linux-2.6 in 
the 'clockdomain_a_3.1' branch.


- Paul

From: Paul Walmsley <paul@pwsan.com>
Date: Thu, 7 Jul 2011 22:55:08 -0600
Subject: [PATCH] OMAP2+: clockdomain: add clkdm_in_hwsup()

Add a new function, clkdm_in_hwsup(), that returns true if a clockdomain
is configured for hardware-supervised idle.  It does not actually read the
hardware; rather, it checks an internal flag in the struct clockdomain, which
is changed when the clockdomain is switched in and out of hardware-supervised
idle.  This should be safe, since all changes to the idle mode should
pass through the clockdomain code.

Based on a set of patches by Rajendra Nayak <rnayak@ti.com> which do
the same thing by checking the hardware bits.  This approach should be
faster and more compact.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Todd Poynor <toddpoynor@google.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/clockdomain.c |   30 ++++++++++++++++++++++++++++--
 arch/arm/mach-omap2/clockdomain.h |    6 ++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 5a57de5..239b558 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -1,8 +1,8 @@
 /*
  * OMAP2/3/4 clockdomain framework functions
  *
- * Copyright (C) 2008-2010 Texas Instruments, Inc.
- * Copyright (C) 2008-2010 Nokia Corporation
+ * Copyright (C) 2008-2011 Texas Instruments, Inc.
+ * Copyright (C) 2008-2011 Nokia Corporation
  *
  * Written by Paul Walmsley and Jouni Högander
  * Added OMAP4 specific support by Abhijit Pagare <abhijitpagare@ti.com>
@@ -704,6 +704,8 @@ int clkdm_sleep(struct clockdomain *clkdm)
 
 	pr_debug("clockdomain: forcing sleep on %s\n", clkdm->name);
 
+	clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
+
 	return arch_clkdm->clkdm_sleep(clkdm);
 }
 
@@ -732,6 +734,8 @@ int clkdm_wakeup(struct clockdomain *clkdm)
 
 	pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
 
+	clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
+
 	return arch_clkdm->clkdm_wakeup(clkdm);
 }
 
@@ -762,6 +766,8 @@ void clkdm_allow_idle(struct clockdomain *clkdm)
 	pr_debug("clockdomain: enabling automatic idle transitions for %s\n",
 		 clkdm->name);
 
+	clkdm->_flags |= _CLKDM_FLAG_HWSUP_ENABLED;
+
 	arch_clkdm->clkdm_allow_idle(clkdm);
 	pwrdm_clkdm_state_switch(clkdm);
 }
@@ -792,9 +798,29 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
 	pr_debug("clockdomain: disabling automatic idle transitions for %s\n",
 		 clkdm->name);
 
+	clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
+
 	arch_clkdm->clkdm_deny_idle(clkdm);
 }
 
+/**
+ * clkdm_in_hwsup - is clockdomain @clkdm have hardware-supervised idle enabled?
+ * @clkdm: struct clockdomain *
+ *
+ * Returns true if clockdomain @clkdm currently has
+ * hardware-supervised idle enabled, or false if it does not or if
+ * @clkdm is NULL.  It is only valid to call this function after
+ * clkdm_init() has been called.  This function does not actually read
+ * bits from the hardware; it instead tests an in-memory flag that is
+ * changed whenever the clockdomain code changes the auto-idle mode.
+ */
+bool clkdm_in_hwsup(struct clockdomain *clkdm)
+{
+	if (!clkdm)
+		return false;
+
+	return (clkdm->_flags & _CLKDM_FLAG_HWSUP_ENABLED) ? true : false;
+}
 
 /* Clockdomain-to-clock/hwmod framework interface code */
 
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 8e0da64..8782a5c 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -83,6 +83,9 @@ struct clkdm_dep {
 	const struct omap_chip_id omap_chip;
 };
 
+/* Possible flags for struct clockdomain._flags */
+#define _CLKDM_FLAG_HWSUP_ENABLED		BIT(0)
+
 /**
  * struct clockdomain - OMAP clockdomain
  * @name: clockdomain name
@@ -90,6 +93,7 @@ struct clkdm_dep {
  * @clktrctrl_reg: CLKSTCTRL reg for the given clock domain
  * @clktrctrl_mask: CLKTRCTRL/AUTOSTATE field mask in CM_CLKSTCTRL reg
  * @flags: Clockdomain capability flags
+ * @_flags: Flags for use only by internal clockdomain code
  * @dep_bit: Bit shift of this clockdomain's PM_WKDEP/CM_SLEEPDEP bit
  * @prcm_partition: (OMAP4 only) PRCM partition ID for this clkdm's registers
  * @cm_inst: (OMAP4 only) CM instance register offset
@@ -114,6 +118,7 @@ struct clockdomain {
 	} pwrdm;
 	const u16 clktrctrl_mask;
 	const u8 flags;
+	u8 _flags;
 	const u8 dep_bit;
 	const u8 prcm_partition;
 	const s16 cm_inst;
@@ -178,6 +183,7 @@ int clkdm_clear_all_sleepdeps(struct clockdomain *clkdm);
 
 void clkdm_allow_idle(struct clockdomain *clkdm);
 void clkdm_deny_idle(struct clockdomain *clkdm);
+bool clkdm_in_hwsup(struct clockdomain *clkdm);
 
 int clkdm_wakeup(struct clockdomain *clkdm);
 int clkdm_sleep(struct clockdomain *clkdm);
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 7/7] OMAP2+: hwmod: Follow the recommended PRCM module enable sequence
  2011-07-01 21:27 ` [PATCH v4 7/7] OMAP2+: hwmod: Follow the recommended PRCM module enable sequence Benoit Cousson
@ 2011-07-09  8:15   ` Paul Walmsley
  0 siblings, 0 replies; 14+ messages in thread
From: Paul Walmsley @ 2011-07-09  8:15 UTC (permalink / raw)
  To: Benoit Cousson; +Cc: rnayak, linux-omap, linux-arm-kernel

On Fri, 1 Jul 2011, Benoit Cousson wrote:

> From: Rajendra Nayak <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>

This patch breaks OMAP2/3.  It appears this version wasn't tested on 
either of those.  I'm not even sure it's 100% right for OMAP4 either, 
since we know that some of the "optional" clocks do factor into whether a 
clockdomain can go idle or not.  The current example we've been using is 
the GPIO debounce clock, but there are probably others too.  The 
impression I get from talking with people working on OMAP4, however, is 
that this approach to handling clockdomains fixes some big problems on 
OMAP4.  

So the clock framework part of this patch has been reimplemented as a 
separate patch.  The default is to preserve the OMAP2/3 behavior. The 
OMAP4 clock init code will be able to disable this during init.  This 
patch will be posted separately.

Then the remaining hwmod changes in this patch also need to be made 
conditional on whether oh->clkdm is non-null, otherwise the kernel won't 
work on OMAP2/3.  The updated patch is below.

Although I've chosen to clean up this patch this time to try to deal with 
some of the recent uncertainty, in the future, I'm not going to spend 
any time fixing series with patches that break OMAP2/3 boards.


- Paul


From: Rajendra Nayak <rnayak@ti.com>
Date: Fri, 1 Jul 2011 23:27:06 +0200
Subject: [PATCH] OMAP2+: hwmod: Follow the recommended PRCM module enable
 sequence

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>
[paul@pwsan.com: dropped mach-omap2/clock.c changes; modified to only
 call the clockdomain code if oh->clkdm is set; disable clock->clockdomain
 interaction on OMAP4]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clock44xx_data.c |    1 +
 arch/arm/mach-omap2/omap_hwmod.c     |   31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 8c96567..28b5de1 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -3296,6 +3296,7 @@ int __init omap4xxx_clk_init(void)
 	}
 
 	clk_init(&omap2_clk_functions);
+	omap2_clk_disable_clkdm_control();
 
 	for (c = omap44xx_clks; c < omap44xx_clks + ARRAY_SIZE(omap44xx_clks);
 									  c++)
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 293fa6c..e74b9c7 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,33 @@ static int _enable(struct omap_hwmod *oh)
 		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
 
 	_add_initiator_dep(oh, mpu_oh);
+
+	if (oh->clkdm) {
+		/*
+		 * 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_in_hwsup(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);
 
 	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 (oh->clkdm && hwsup)
+			clkdm_allow_idle(oh->clkdm);
+
 		oh->_state = _HWMOD_STATE_ENABLED;
 
 		/* Access the sysconfig only if the target is ready */
@@ -1266,6 +1290,9 @@ static int _enable(struct omap_hwmod *oh)
 		_disable_clocks(oh);
 		pr_debug("omap_hwmod: %s: _wait_target_ready: %d\n",
 			 oh->name, r);
+
+		if (oh->clkdm)
+			clkdm_hwmod_disable(oh->clkdm, oh);
 	}
 
 	return r;
@@ -1293,6 +1320,8 @@ static int _idle(struct omap_hwmod *oh)
 		_idle_sysc(oh);
 	_del_initiator_dep(oh, mpu_oh);
 	_disable_clocks(oh);
+	if (oh->clkdm)
+		clkdm_hwmod_disable(oh->clkdm, oh);
 
 	/* Mux pins for device idle if populated */
 	if (oh->mux && oh->mux->pads_dynamic)
@@ -1389,6 +1418,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->clkdm)
+			clkdm_hwmod_disable(oh->clkdm, oh);
 	}
 	/* XXX Should this code also force-disable the optional clocks? */
 
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-07-09  8:15 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-01 21:26 [PATCH v4 0/7] Fix module-mode enable sequence on OMAP4 Benoit Cousson
2011-07-01 21:27 ` [PATCH v4 1/7] OMAP2+: clockdomain: Add an api to read idle mode Benoit Cousson
2011-07-09  0:06   ` Paul Walmsley
2011-07-01 21:27 ` [PATCH v4 2/7] OMAP2+: clockdomain: Add SoC support for clkdm_allows_idle Benoit Cousson
2011-07-01 21:27 ` [PATCH v4 3/7] OMAP2+: PM: Initialise sleep_switch to a non-valid value Benoit Cousson
2011-07-07 22:36   ` Paul Walmsley
2011-07-08 14:19     ` Kevin Hilman
2011-07-01 21:27 ` [PATCH v4 4/7] OMAP2+: PM: idle clkdms only if already in idle Benoit Cousson
2011-07-07 22:37   ` Paul Walmsley
2011-07-08 14:19     ` Kevin Hilman
2011-07-01 21:27 ` [PATCH v4 5/7] OMAP2+: clockdomain: Add 2 APIs to control clockdomain from hwmod framework Benoit Cousson
2011-07-01 21:27 ` [PATCH v4 6/7] OMAP2+: clockdomain: Add per clkdm lock to prevent concurrent state programming Benoit Cousson
2011-07-01 21:27 ` [PATCH v4 7/7] OMAP2+: hwmod: Follow the recommended PRCM module enable sequence Benoit Cousson
2011-07-09  8:15   ` Paul Walmsley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox