[PATCHv6 0/9] Refreshed idlestatus patches with minor fixes

[PATCHv6 0/9] Refreshed idlestatus patches with minor fixes

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Following changes compared to previous set:

- Fixed a problem with PER domain handling when PER was active
- Removed rwlock usage from pwrdm_can_idle as the lock was removed from
  powerdomain code

Still applies on top of PM branch.

[PATCHv6 1/9] OMAP3: PM: Added support functions for omap3 pwrdm handling

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Added omap3_pwrdm_set_next_pwrst and omap3_pwrdm_read_next_pwrst. These
functions add support for INACTIVE and ON states to the standard OMAP
powerdomain functions, and add caching logic for the next state. These
functions are used in subsequent patches to simplify the logic of the idle
loop.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm.h     |    2 +
 arch/arm/mach-omap2/pm34xx.c |   60 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 61 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 75aa685..1d9a740 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -67,6 +67,8 @@ static inline void omap3_pm_init_vc(struct prm_setup_vc *setup_vc)
 
 extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
 extern int omap3_pm_set_suspend_state(struct powerdomain *pwrdm, int state);
+extern int omap3_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst);
+extern int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm);
 
 extern u32 wakeup_timer_seconds;
 extern struct omap_dm_timer *gptimer_wakeup;
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 1359ec9..f20d3d8 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -576,6 +576,64 @@ int omap3_can_sleep(void)
 	return 1;
 }
 
+struct powerdomain_data {
+	u8 next_state;
+	u8 init_done;
+};
+
+static struct powerdomain_data mpu_pwrdm_data;
+static struct powerdomain_data core_pwrdm_data;
+static struct powerdomain_data neon_pwrdm_data;
+
+static struct powerdomain_data *get_pwrdm_data(struct powerdomain *pwrdm)
+{
+	if (pwrdm == mpu_pwrdm)
+		return &mpu_pwrdm_data;
+	else if (pwrdm == core_pwrdm)
+		return &core_pwrdm_data;
+	else if (pwrdm == neon_pwrdm)
+		return &neon_pwrdm_data;
+	return NULL;
+}
+
+int omap3_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
+{
+	struct powerdomain_data *data = get_pwrdm_data(pwrdm);
+	u8 prg_pwrst;
+
+	if (!data)
+		return pwrdm_set_next_pwrst(pwrdm, pwrst);
+
+	if (!data->init_done)
+		data->init_done = 1;
+	else if (data->next_state == pwrst)
+		return 0;
+
+	if (pwrst == PWRDM_POWER_INACTIVE)
+		prg_pwrst = PWRDM_POWER_ON;
+	else
+		prg_pwrst = pwrst;
+
+	pwrdm_set_next_pwrst(pwrdm, prg_pwrst);
+
+	if (pwrst == PWRDM_POWER_ON)
+		omap2_clkdm_deny_idle(pwrdm->pwrdm_clkdms[0]);
+	else
+		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+
+	data->next_state = pwrst;
+	return 0;
+}
+
+int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
+{
+	struct powerdomain_data *data = get_pwrdm_data(pwrdm);
+
+	if (!data || !data->init_done)
+		return pwrdm_read_next_pwrst(pwrdm);
+	return data->next_state;
+}
+
 /* This sets pwrdm state (other than mpu & core. Currently only ON &
  * RET are supported. Function is assuming that clkdm doesn't have
  * hw_sup mode enabled. */
@@ -604,7 +662,7 @@ int set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 		pwrdm_wait_transition(pwrdm);
 	}
 
-	ret = pwrdm_set_next_pwrst(pwrdm, state);
+	ret = omap3_pwrdm_set_next_pwrst(pwrdm, state);
 	if (ret) {
 		printk(KERN_ERR "Unable to set state of powerdomain: %s\n",
 		       pwrdm->name);
-- 
1.5.4.3

[PATCHv6 2/9] OMAP3: PM: Added support for INACTIVE and ON states in omap_sram_idle

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Previously omap_sram_idle() did not know about the difference between ON and
INACTIVE states, which complicated the state handling in these cases. Now,
the following changes are done in the idle logic:

- Check for IO-chain arming is changed to reflect desired state (RET)
- UART clocks will be disabled if we attempt to enter INACTIVE (this allows
  the state change to actually happen)

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index f20d3d8..c2d80fc 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -383,9 +383,10 @@ void omap_sram_idle(void)
 	pwrdm_clear_all_prev_pwrst(core_pwrdm);
 	pwrdm_clear_all_prev_pwrst(per_pwrdm);
 
-	mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm);
+	mpu_next_state = omap3_pwrdm_read_next_pwrst(mpu_pwrdm);
 	switch (mpu_next_state) {
 	case PWRDM_POWER_ON:
+	case PWRDM_POWER_INACTIVE:
 	case PWRDM_POWER_RET:
 		/* No need to save context */
 		save_state = 0;
@@ -403,11 +404,11 @@ void omap_sram_idle(void)
 
 	/* NEON control */
 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
-		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
+		omap3_pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
 
 	/* PER */
-	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
-	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
+	per_next_state = omap3_pwrdm_read_next_pwrst(per_pwrdm);
+	core_next_state = omap3_pwrdm_read_next_pwrst(core_pwrdm);
 	if (per_next_state < PWRDM_POWER_ON) {
 		omap2_gpio_prepare_for_idle(per_next_state);
 		if (per_next_state == PWRDM_POWER_OFF) {
@@ -452,9 +453,11 @@ void omap_sram_idle(void)
 						OMAP3430_GR_MOD,
 						OMAP3_PRM_VOLTCTRL_OFFSET);
 		}
-		/* Enable IO-PAD and IO-CHAIN wakeups */
-		prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
-		omap3_enable_io_chain();
+		if (core_next_state <= PWRDM_POWER_RET) {
+			/* Enable IO-PAD and IO-CHAIN wakeups */
+			prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
+			omap3_enable_io_chain();
+		}
 	}
 	omap3_intc_prepare_idle();
 
@@ -556,15 +559,13 @@ void omap_sram_idle(void)
 	}
 
 	/* Disable IO-PAD and IO-CHAIN wakeup */
-	if (core_next_state < PWRDM_POWER_ON) {
+	if (core_next_state <= PWRDM_POWER_RET) {
 		prm_clear_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
 		omap3_disable_io_chain();
 	}
 
 
 	pwrdm_post_transition();
-
-	omap2_clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 }
 
 int omap3_can_sleep(void)
@@ -730,7 +731,7 @@ static int omap3_pm_suspend(void)
 
 	/* Read current next_pwrsts */
 	list_for_each_entry(pwrst, &pwrst_list, node)
-		pwrst->saved_state = pwrdm_read_next_pwrst(pwrst->pwrdm);
+		pwrst->saved_state = omap3_pwrdm_read_next_pwrst(pwrst->pwrdm);
 	/* Set ones wanted by suspend */
 	list_for_each_entry(pwrst, &pwrst_list, node) {
 		if (set_pwrdm_state(pwrst->pwrdm, pwrst->next_state))
-- 
1.5.4.3

[PATCHv6 3/9] OMAP3: CPUidle: Fixed support for ON / INACTIVE states

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

New powerdomain support code for INACTIVE state removes the need to control
clockdomains directly from cpuidle. Also, cpuidle state definitions can now
directly support ON / INACTIVE simplifying the implementation.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   36 ++++++------------------------------
 1 files changed, 6 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 1cfa5a6..86f580b 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -91,20 +91,6 @@ static int omap3_idle_bm_check(void)
 	return 0;
 }
 
-static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
-{
-	omap2_clkdm_allow_idle(clkdm);
-	return 0;
-}
-
-static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
-{
-	omap2_clkdm_deny_idle(clkdm);
-	return 0;
-}
-
 /**
  * omap3_enter_idle - Programs OMAP3 to enter the specified state
  * @dev: cpuidle device
@@ -135,25 +121,15 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 			core_state = PWRDM_POWER_RET;
 	}
 
-	pwrdm_set_next_pwrst(mpu_pd, mpu_state);
-	pwrdm_set_next_pwrst(core_pd, core_state);
+	omap3_pwrdm_set_next_pwrst(mpu_pd, mpu_state);
+	omap3_pwrdm_set_next_pwrst(core_pd, core_state);
 
 	if (omap_irq_pending() || need_resched())
 		goto return_sleep_time;
 
-	if (cx->type == OMAP3_STATE_C1) {
-		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
-		pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
-	}
-
 	/* Execute ARM wfi */
 	omap_sram_idle();
 
-	if (cx->type == OMAP3_STATE_C1) {
-		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
-		pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
-	}
-
 return_sleep_time:
 	getnstimeofday(&ts_postidle);
 	ts_idle = timespec_sub(ts_postidle, ts_preidle);
@@ -246,8 +222,8 @@ void omap_init_power_states(void)
 			cpuidle_params_table[OMAP3_STATE_C2].wake_latency;
 	omap3_power_states[OMAP3_STATE_C2].threshold =
 			cpuidle_params_table[OMAP3_STATE_C2].threshold;
-	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_ON;
-	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_ON;
+	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_INACTIVE;
+	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_INACTIVE;
 	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID;
 
 	/* C3 . MPU CSWR + Core inactive */
@@ -261,7 +237,7 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C3].threshold =
 			cpuidle_params_table[OMAP3_STATE_C3].threshold;
 	omap3_power_states[OMAP3_STATE_C3].mpu_state = PWRDM_POWER_RET;
-	omap3_power_states[OMAP3_STATE_C3].core_state = PWRDM_POWER_ON;
+	omap3_power_states[OMAP3_STATE_C3].core_state = PWRDM_POWER_INACTIVE;
 	omap3_power_states[OMAP3_STATE_C3].flags = CPUIDLE_FLAG_TIME_VALID |
 				CPUIDLE_FLAG_CHECK_BM;
 
@@ -276,7 +252,7 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C4].threshold =
 			cpuidle_params_table[OMAP3_STATE_C4].threshold;
 	omap3_power_states[OMAP3_STATE_C4].mpu_state = PWRDM_POWER_OFF;
-	omap3_power_states[OMAP3_STATE_C4].core_state = PWRDM_POWER_ON;
+	omap3_power_states[OMAP3_STATE_C4].core_state = PWRDM_POWER_INACTIVE;
 	omap3_power_states[OMAP3_STATE_C4].flags = CPUIDLE_FLAG_TIME_VALID |
 				CPUIDLE_FLAG_CHECK_BM;
 
-- 
1.5.4.3

[PATCHv6 4/9] OMAP3: PM: Removed pwrdm state hacking from omap_sram_idle

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Following hacks will be moved inside cpuidle in subsequent patch:

- CAM domain prevents idle completely
- PER should not go OFF if core remains active

This simplifies the design and allows cpuidle to keep better track of which
power states system will actually enter.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |   19 ++-----------------
 1 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index c2d80fc..cdbedcf 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -93,7 +93,6 @@ static int (*_omap_save_secure_sram)(u32 *addr);
 
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
-static struct powerdomain *cam_pwrdm;
 
 static struct prm_setup_vc prm_setup = {
 	.clksetup = 0xff,
@@ -373,7 +372,6 @@ void omap_sram_idle(void)
 	int core_next_state = PWRDM_POWER_ON;
 	int core_prev_state, per_prev_state;
 	u32 sdrc_pwr = 0;
-	int per_state_modified = 0;
 
 	if (!_omap_sram_idle)
 		return;
@@ -411,20 +409,11 @@ void omap_sram_idle(void)
 	core_next_state = omap3_pwrdm_read_next_pwrst(core_pwrdm);
 	if (per_next_state < PWRDM_POWER_ON) {
 		omap2_gpio_prepare_for_idle(per_next_state);
-		if (per_next_state == PWRDM_POWER_OFF) {
-			if (core_next_state == PWRDM_POWER_ON) {
-				per_next_state = PWRDM_POWER_RET;
-				pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
-				per_state_modified = 1;
-			} else
-				omap3_per_save_context();
-		}
+		if (per_next_state == PWRDM_POWER_OFF)
+			omap3_per_save_context();
 		omap_uart_prepare_idle(2);
 	}
 
-	if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
-		omap2_clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
-
 	/*
 	 * Disable smartreflex before entering WFI.
 	 * Only needed if we are going to enter retention or off.
@@ -554,8 +543,6 @@ void omap_sram_idle(void)
 		}
 		omap2_gpio_resume_after_idle();
 		omap_uart_resume_idle(2);
-		if (per_state_modified)
-			pwrdm_set_next_pwrst(per_pwrdm, PWRDM_POWER_OFF);
 	}
 
 	/* Disable IO-PAD and IO-CHAIN wakeup */
@@ -564,7 +551,6 @@ void omap_sram_idle(void)
 		omap3_disable_io_chain();
 	}
 
-
 	pwrdm_post_transition();
 }
 
@@ -1239,7 +1225,6 @@ static int __init omap3_pm_init(void)
 	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
 	per_pwrdm = pwrdm_lookup("per_pwrdm");
 	core_pwrdm = pwrdm_lookup("core_pwrdm");
-	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
 
 	neon_clkdm = clkdm_lookup("neon_clkdm");
 	mpu_clkdm = clkdm_lookup("mpu_clkdm");
-- 
1.5.4.3

[PATCHv6 5/9] OMAP3: Clock: Added IDLEST definitions for SGX

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Added definitions for OMAP3430ES2_ST_SGX_SHIFT and OMAP3430ES2_ST_SGX_MASK
as these were missing.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/cm-regbits-34xx.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/cm-regbits-34xx.h b/arch/arm/mach-omap2/cm-regbits-34xx.h
index 6923deb..60f9b16 100644
--- a/arch/arm/mach-omap2/cm-regbits-34xx.h
+++ b/arch/arm/mach-omap2/cm-regbits-34xx.h
@@ -379,6 +379,10 @@
 #define OMAP3430ES2_CM_FCLKEN_SGX_EN_SGX_SHIFT		1
 #define OMAP3430ES2_CM_FCLKEN_SGX_EN_SGX_MASK		(1 << 1)
 
+/* CM_IDLEST_SGX */
+#define OMAP3430ES2_ST_SGX_SHIFT			1
+#define OMAP3430ES2_ST_SGX_MASK				(1 << 1)
+
 /* CM_ICLKEN_SGX */
 #define OMAP3430ES2_CM_ICLKEN_SGX_EN_SGX_SHIFT		0
 #define OMAP3430ES2_CM_ICLKEN_SGX_EN_SGX_MASK		(1 << 0)
-- 
1.5.4.3

[PATCHv6 6/9] OMAP: Powerdomains: Add support for checking if pwrdm/clkdm can idle

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

pwrdm_can_idle(pwrdm) will check if the specified powerdomain can enter
idle. This is done by checking all clockdomains under the powerdomain
if they can idle also.

omap2_clkdm_can_idle(clkdm) will check if the specified clockdomain can
enter idle. This checks the functional clocks and idle status bits of the
domain according to following rules:
1) functional clock check
  * get FCLK register content
  * ignore all clocks defined in idle_def.fclk_ignore
  * if any active functional clocks remain, domain can't idle
2) idle status check
  * get IDLEST register content
  * inverse it (any non-idle blocks are now as 1)
  * mask against idle_def.idlest_mask
  * if any bits remain high, domain can't idle

These calls can be used e.g. inside cpuidle to decide which power states
core and mpu should enter during idle, as there are certain dependencies
between wakeup capabilities and reset logic.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/clockdomain.c             |   32 ++++++++
 arch/arm/mach-omap2/clockdomains.h            |  106 +++++++++++++++++++++++++
 arch/arm/mach-omap2/powerdomain.c             |   20 +++++
 arch/arm/plat-omap/include/plat/clockdomain.h |   17 ++++
 arch/arm/plat-omap/include/plat/powerdomain.h |    1 +
 5 files changed, 176 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index a38a615..9ebff51 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -867,6 +867,38 @@ int omap2_clkdm_wakeup(struct clockdomain *clkdm)
 	return 0;
 }
 
+
+/**
+ * omap2_clkdm_can_idle - check if clockdomain has any active clocks or not
+ * @clkdm: struct clockdomain *
+ *
+ * Checks if the clockdomain has any active clock or not, i.e. whether it
+ * can enter idle. Returns -EINVAL if clkdm is NULL; 0 if unable to idle;
+ * 1 if can idle.
+ */
+int omap2_clkdm_can_idle(struct clockdomain *clkdm)
+{
+	int i;
+
+	if (!clkdm)
+		return -EINVAL;
+
+	for (i = 0; i < clkdm->clk_reg_num; i++) {
+		u32 idlest, fclk;
+
+		fclk = cm_read_mod_reg(clkdm->pwrdm.ptr->prcm_offs,
+				CM_FCLKEN + 4 * i);
+		if (fclk & ~clkdm->idle_def[i].fclk_ignore)
+			return 0;
+
+		idlest = cm_read_mod_reg(clkdm->pwrdm.ptr->prcm_offs,
+				CM_IDLEST + 4 * i);
+		if (~idlest & clkdm->idle_def[i].idlest_mask)
+			return 0;
+	}
+	return 1;
+}
+
 /**
  * omap2_clkdm_allow_idle - enable hwsup idle transitions for clkdm
  * @clkdm: struct clockdomain *
diff --git a/arch/arm/mach-omap2/clockdomains.h b/arch/arm/mach-omap2/clockdomains.h
index 8fc19ff..5e29de8 100644
--- a/arch/arm/mach-omap2/clockdomains.h
+++ b/arch/arm/mach-omap2/clockdomains.h
@@ -663,6 +663,12 @@ static struct clockdomain iva2_clkdm = {
 	.wkdep_srcs	= iva2_wkdeps,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_IVA2_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.clk_reg_num	= 1,
+	.idle_def	= {
+		[0] = {
+			.idlest_mask = OMAP3430_ST_IVA2,
+		},
+	},
 };
 
 static struct clockdomain gfx_3430es1_clkdm = {
@@ -686,6 +692,12 @@ static struct clockdomain sgx_clkdm = {
 	.sleepdep_srcs	= gfx_sgx_sleepdeps,
 	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_SGX_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
+	.clk_reg_num	= 1,
+	.idle_def	= {
+		[0] = {
+			.idlest_mask = OMAP3430ES2_ST_SGX_SHIFT,
+		},
+	},
 };
 
 /*
@@ -717,6 +729,57 @@ static struct clockdomain core_l3_3xxx_clkdm = {
 	.dep_bit	= OMAP3430_EN_CORE_SHIFT,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_L3_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.clk_reg_num	= 3,
+	.idle_def	= {
+		[0] = {
+			/* UARTs are controlled by idle loop so ignore */
+			.fclk_ignore = OMAP3430_EN_UART2 |
+				OMAP3430_EN_UART1,
+			/*
+			 * Reason for IDLEST ignores:
+			 * - SDRC and OMAPCTRL controlled by HW
+			 * - HSOTGUSB_IDLE bit is autoidled by HW
+			 * - MAILBOX is controlled by HW
+			 */
+			.idlest_mask =
+				OMAP3430ES2_ST_MMC3_MASK |
+				OMAP3430_ST_ICR_MASK |
+				OMAP3430_ST_AES2_MASK |
+				OMAP3430_ST_SHA12_MASK |
+				OMAP3430_ST_DES2_MASK |
+				OMAP3430_ST_MMC2_MASK |
+				OMAP3430_ST_MMC1_MASK |
+				OMAP3430_ST_MSPRO_MASK |
+				OMAP3430_ST_HDQ_MASK |
+				OMAP3430_ST_MCSPI4_MASK |
+				OMAP3430_ST_MCSPI3_MASK |
+				OMAP3430_ST_MCSPI2_MASK |
+				OMAP3430_ST_MCSPI1_MASK |
+				OMAP3430_ST_I2C3_MASK |
+				OMAP3430_ST_I2C2_MASK |
+				OMAP3430_ST_I2C1_MASK |
+				OMAP3430_ST_GPT11_MASK |
+				OMAP3430_ST_GPT10_MASK |
+				OMAP3430_ST_MCBSP5_MASK |
+				OMAP3430_ST_MCBSP1_MASK |
+				OMAP3430ES2_ST_SSI_IDLE_MASK |
+				OMAP3430ES2_ST_HSOTGUSB_STDBY_MASK |
+				OMAP3430_ST_D2D_MASK |
+				OMAP3430_ST_SDMA_MASK |
+				OMAP3430_ST_SSI_STDBY_MASK,
+		},
+		[1] = {
+			.idlest_mask = OMAP3430_ST_PKA_MASK |
+				OMAP3430_ST_AES1_MASK |
+				OMAP3430_ST_RNG_MASK |
+				OMAP3430_ST_SHA11_MASK |
+				OMAP3430_ST_DES1_MASK,
+		},
+		[2] = {
+			.idlest_mask = OMAP3430ES2_ST_USBTLL_MASK |
+				OMAP3430ES2_ST_CPEFUSE_MASK,
+		},
+	},
 };
 
 /*
@@ -746,6 +809,13 @@ static struct clockdomain dss_3xxx_clkdm = {
 	.sleepdep_srcs	= dss_sleepdeps,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_DSS_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.clk_reg_num	= 1,
+	.idle_def	= {
+		[0] = {
+			.idlest_mask = OMAP3430ES2_ST_DSS_IDLE_MASK |
+				OMAP3430ES2_ST_DSS_STDBY_MASK,
+		},
+	},
 };
 
 static struct clockdomain cam_clkdm = {
@@ -758,6 +828,12 @@ static struct clockdomain cam_clkdm = {
 	.sleepdep_srcs	= cam_sleepdeps,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_CAM_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.clk_reg_num	= 1,
+	.idle_def	= {
+		[0] = {
+			.idlest_mask = OMAP3430_ST_CAM,
+		},
+	},
 };
 
 static struct clockdomain usbhost_clkdm = {
@@ -770,6 +846,13 @@ static struct clockdomain usbhost_clkdm = {
 	.sleepdep_srcs	= usbhost_sleepdeps,
 	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_USBHOST_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
+	.clk_reg_num	= 1,
+	.idle_def	= {
+		[0] = {
+			.idlest_mask = OMAP3430ES2_ST_USBHOST_IDLE_MASK |
+				OMAP3430ES2_ST_USBHOST_STDBY_MASK,
+		},
+	},
 };
 
 static struct clockdomain per_clkdm = {
@@ -783,6 +866,29 @@ static struct clockdomain per_clkdm = {
 	.sleepdep_srcs	= per_sleepdeps,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_PER_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.clk_reg_num	= 1,
+	.idle_def	= {
+		[0] = {
+			.fclk_ignore = OMAP3430_ST_UART3_MASK,
+			/*
+			 * GPIO2...6 are dropped out from this mask
+			 * as they are always non-idle, UART3 is also
+			 * out as it is handled by idle loop
+			 * */
+			.idlest_mask = OMAP3430_ST_WDT3_MASK |
+				OMAP3430_ST_GPT9_MASK |
+				OMAP3430_ST_GPT8_MASK |
+				OMAP3430_ST_GPT7_MASK |
+				OMAP3430_ST_GPT6_MASK |
+				OMAP3430_ST_GPT5_MASK |
+				OMAP3430_ST_GPT4_MASK |
+				OMAP3430_ST_GPT3_MASK |
+				OMAP3430_ST_GPT2_MASK |
+				OMAP3430_ST_MCBSP4_MASK |
+				OMAP3430_ST_MCBSP3_MASK |
+				OMAP3430_ST_MCBSP2_MASK,
+		},
+	},
 };
 
 /*
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index dc03289..a9ca884 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -919,6 +919,26 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm)
 	return 0;
 }
 
+/**
+ * pwrdm_can_idle - check if the powerdomain can enter idle
+ * @pwrdm: struct powerdomain * the powerdomain to check status of
+ *
+ * Checks all associated clockdomains if they can idle or not.
+ * Returns 1 if the powerdomain can idle, 0 if not.
+ */
+int pwrdm_can_idle(struct powerdomain *pwrdm)
+{
+	int i;
+	int ret = 1;
+
+	for (i = 0; i < PWRDM_MAX_CLKDMS; i++)
+		if (pwrdm->pwrdm_clkdms[i] &&
+		    !omap2_clkdm_can_idle(pwrdm->pwrdm_clkdms[i]))
+			ret = 0;
+
+	return ret;
+}
+
 int pwrdm_state_switch(struct powerdomain *pwrdm)
 {
 	return _pwrdm_state_switch(pwrdm, PWRDM_STATE_NOW);
diff --git a/arch/arm/plat-omap/include/plat/clockdomain.h b/arch/arm/plat-omap/include/plat/clockdomain.h
index ba0a6c0..ecf7f88 100644
--- a/arch/arm/plat-omap/include/plat/clockdomain.h
+++ b/arch/arm/plat-omap/include/plat/clockdomain.h
@@ -30,6 +30,12 @@
 #define CLKDM_CAN_SWSUP		(CLKDM_CAN_FORCE_SLEEP | CLKDM_CAN_FORCE_WAKEUP)
 #define CLKDM_CAN_HWSUP_SWSUP	(CLKDM_CAN_SWSUP | CLKDM_CAN_HWSUP)
 
+/*
+ * Maximum number of FCLK register masks that can be associated with a
+ * clockdomain. CORE powerdomain on OMAP3 is the worst case
+ */
+#define CLKDM_MAX_CLK_REGS			3
+
 /* OMAP24XX CM_CLKSTCTRL_*.AUTOSTATE_* register bit values */
 #define OMAP24XX_CLKSTCTRL_DISABLE_AUTO		0x0
 #define OMAP24XX_CLKSTCTRL_ENABLE_AUTO		0x1
@@ -82,6 +88,11 @@ struct clkdm_dep {
 	const struct omap_chip_id omap_chip;
 };
 
+struct clkdm_idle_def {
+	u32 fclk_ignore;
+	u32 idlest_mask;
+};
+
 /**
  * struct clockdomain - OMAP clockdomain
  * @name: clockdomain name
@@ -95,6 +106,8 @@ struct clkdm_dep {
  * @omap_chip: OMAP chip types that this clockdomain is valid on
  * @usecount: Usecount tracking
  * @node: list_head to link all clockdomains together
+ * @clk_reg_num: Number of idle definitions for clockdomain idle status checks
+ * @idle_def: Declarations for idle checks for clockdomain
  */
 struct clockdomain {
 	const char *name;
@@ -111,6 +124,8 @@ struct clockdomain {
 	const struct omap_chip_id omap_chip;
 	atomic_t usecount;
 	struct list_head node;
+	u8 clk_reg_num;
+	struct clkdm_idle_def idle_def[CLKDM_MAX_CLK_REGS];
 };
 
 void clkdm_init(struct clockdomain **clkdms, struct clkdm_autodep *autodeps);
@@ -138,4 +153,6 @@ int omap2_clkdm_sleep(struct clockdomain *clkdm);
 int omap2_clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk);
 int omap2_clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk);
 
+int omap2_clkdm_can_idle(struct clockdomain *clkdm);
+
 #endif
diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h b/arch/arm/plat-omap/include/plat/powerdomain.h
index e15c7e9..a1ecd47 100644
--- a/arch/arm/plat-omap/include/plat/powerdomain.h
+++ b/arch/arm/plat-omap/include/plat/powerdomain.h
@@ -145,6 +145,7 @@ int pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm);
 bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm);
 
 int pwrdm_wait_transition(struct powerdomain *pwrdm);
+int pwrdm_can_idle(struct powerdomain *pwrdm);
 
 int pwrdm_state_switch(struct powerdomain *pwrdm);
 int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
-- 
1.5.4.3

[PATCHv6 7/9] OMAP3: CPUidle: Added peripheral pwrdm checks into bm check

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Following checks are made (and their reasoning):

- If CAM domain is active, prevent idle completely
  * CAM pwrdm does not have HW wakeup capability
- If PER is likely to remain on, prevent PER off
  * Saves on unnecessary context save/restore
- If CORE domain is active, prevent PER off-mode
  * PER off in this case would prevent wakeups from PER completely
- Only allow CORE off, if all peripheral domains are off
  * CORE off will cause a chipwide reset

Also, enabled CHECK_BM flag for C2, as this is needed for the camera case.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |  104 ++++++++++++++++++++++++++++++++++---
 1 files changed, 97 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 86f580b..914022f 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -58,7 +58,8 @@ struct omap3_processor_cx {
 
 struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
 struct omap3_processor_cx current_cx_state;
-struct powerdomain *mpu_pd, *core_pd;
+static struct powerdomain *mpu_pd, *core_pd, *per_pd;
+static struct powerdomain *sgx_pd, *usb_pd, *cam_pd, *dss_pd;
 
 /*
  * The latencies/thresholds for various C states have
@@ -153,14 +154,96 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 			       struct cpuidle_state *state)
 {
 	struct cpuidle_state *new_state = state;
-
-	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && omap3_idle_bm_check()) {
-		BUG_ON(!dev->safe_state);
-		new_state = dev->safe_state;
+	u32 per_state = 0, saved_per_state = 0, cam_state, usb_state;
+	u32 iva2_state, sgx_state, dss_state, new_core_state;
+	struct omap3_processor_cx *cx;
+	int ret;
+
+	if (state->flags & CPUIDLE_FLAG_CHECK_BM) {
+		if (omap3_idle_bm_check()) {
+			BUG_ON(!dev->safe_state);
+			new_state = dev->safe_state;
+			goto select_state;
+		}
+		cx = cpuidle_get_statedata(state);
+		new_core_state = cx->core_state;
+
+		/* Check if CORE is active, if yes, fallback to inactive */
+		if (!pwrdm_can_idle(core_pd))
+			new_core_state = PWRDM_POWER_INACTIVE;
+
+		/*
+		 * Prevent idle completely if CAM is active.
+		 * CAM does not have wakeup capability in OMAP3.
+		 */
+		cam_state = pwrdm_read_pwrst(cam_pd);
+		if (cam_state == PWRDM_POWER_ON) {
+			new_state = dev->safe_state;
+			goto select_state;
+		}
+
+		/*
+		 * Check if PER can idle or not. If we are not likely
+		 * to idle, deny PER off. This prevents unnecessary
+		 * context save/restore.
+		 */
+		saved_per_state = omap3_pwrdm_read_next_pwrst(per_pd);
+		if (pwrdm_can_idle(per_pd)) {
+			per_state = saved_per_state;
+			/*
+			 * Prevent PER off if CORE is active as this
+			 * would disable PER wakeups completely
+			 */
+			if (per_state == PWRDM_POWER_OFF &&
+			    new_core_state > PWRDM_POWER_RET)
+				per_state = PWRDM_POWER_RET;
+
+		} else if (saved_per_state == PWRDM_POWER_OFF)
+			per_state = PWRDM_POWER_RET;
+		else
+			per_state = saved_per_state;
+
+		/*
+		 * If we are attempting CORE off, check if any other
+		 * powerdomains are at retention or higher. CORE off causes
+		 * chipwide reset which would reset these domains also.
+		 */
+		if (new_core_state == PWRDM_POWER_OFF) {
+			dss_state = pwrdm_read_pwrst(dss_pd);
+			iva2_state = pwrdm_read_pwrst(iva2_pd);
+			sgx_state = pwrdm_read_pwrst(sgx_pd);
+			usb_state = pwrdm_read_pwrst(usb_pd);
+
+			if (cam_state > PWRDM_POWER_OFF ||
+			    dss_state > PWRDM_POWER_OFF ||
+			    iva2_state > PWRDM_POWER_OFF ||
+			    per_state > PWRDM_POWER_OFF ||
+			    sgx_state > PWRDM_POWER_OFF ||
+			    usb_state > PWRDM_POWER_OFF)
+				new_core_state = PWRDM_POWER_RET;
+		}
+
+		/* Fallback to new target core state */
+		while (cx->core_state < new_core_state) {
+			state--;
+			cx = cpuidle_get_statedata(state);
+		}
+		new_state = state;
+
+		/* Are we changing PER target state? */
+		if (per_state != saved_per_state)
+			omap3_pwrdm_set_next_pwrst(per_pd, per_state);
 	}
 
+select_state:
 	dev->last_state = new_state;
-	return omap3_enter_idle(dev, new_state);
+	ret = omap3_enter_idle(dev, new_state);
+
+	/* Restore potentially tampered PER state */
+	if (per_state != saved_per_state)
+		omap3_pwrdm_set_next_pwrst(per_pd, saved_per_state);
+
+	return ret;
 }
 
 DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
@@ -224,7 +307,8 @@ void omap_init_power_states(void)
 			cpuidle_params_table[OMAP3_STATE_C2].threshold;
 	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_INACTIVE;
 	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_INACTIVE;
-	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID;
+	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_CHECK_BM;
 
 	/* C3 . MPU CSWR + Core inactive */
 	omap3_power_states[OMAP3_STATE_C3].valid =
@@ -322,6 +406,12 @@ int __init omap3_idle_init(void)
 
 	mpu_pd = pwrdm_lookup("mpu_pwrdm");
 	core_pd = pwrdm_lookup("core_pwrdm");
+	per_pd = pwrdm_lookup("per_pwrdm");
+	iva2_pd = pwrdm_lookup("iva2_pwrdm");
+	sgx_pd = pwrdm_lookup("sgx_pwrdm");
+	usb_pd = pwrdm_lookup("usbhost_pwrdm");
+	cam_pd = pwrdm_lookup("cam_pwrdm");
+	dss_pd = pwrdm_lookup("dss_pwrdm");
 
 	omap_init_power_states();
 	cpuidle_register_driver(&omap3_idle_driver);
-- 
1.5.4.3

[PATCHv6 8/9] OMAP3: CPUidle: Fixed off-mode support to fall-back to proper C state

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

If off-mode is disabled, cpuidle now falls back to proper C state. Previously
the state was just simply hacked inside omap3_enter_idle which resulted
in wrong state residencies to be reported.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 914022f..e41019f 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -115,13 +115,6 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 	local_irq_disable();
 	local_fiq_disable();
 
-	if (!enable_off_mode) {
-		if (mpu_state < PWRDM_POWER_RET)
-			mpu_state = PWRDM_POWER_RET;
-		if (core_state < PWRDM_POWER_RET)
-			core_state = PWRDM_POWER_RET;
-	}
-
 	omap3_pwrdm_set_next_pwrst(mpu_pd, mpu_state);
 	omap3_pwrdm_set_next_pwrst(core_pd, core_state);
 
@@ -155,7 +148,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 {
 	struct cpuidle_state *new_state = state;
 	u32 per_state = 0, saved_per_state = 0, cam_state, usb_state;
-	u32 iva2_state, sgx_state, dss_state, new_core_state;
+	u32 iva2_state, sgx_state, dss_state, new_core_state, new_mpu_state;
 	struct omap3_processor_cx *cx;
 	int ret;
 
@@ -167,6 +160,14 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 		}
 		cx = cpuidle_get_statedata(state);
 		new_core_state = cx->core_state;
+		new_mpu_state = cx->mpu_state;
+
+		if (!enable_off_mode) {
+			if (new_mpu_state < PWRDM_POWER_RET)
+				new_mpu_state = PWRDM_POWER_RET;
+			if (new_core_state < PWRDM_POWER_RET)
+				new_core_state = PWRDM_POWER_RET;
+		}
 
 		/* Check if CORE is active, if yes, fallback to inactive */
 		if (!pwrdm_can_idle(core_pd))
@@ -223,8 +224,9 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 				new_core_state = PWRDM_POWER_RET;
 		}
 
-		/* Fallback to new target core state */
-		while (cx->core_state < new_core_state) {
+		/* Fallback to new target core/mpu state */
+		while (cx->core_state < new_core_state ||
+		       cx->mpu_state < new_mpu_state) {
 			state--;
 			cx = cpuidle_get_statedata(state);
 		}
-- 
1.5.4.3

[PATCHv6 9/9] OMAP3: PM: Added support for suspending to INACTIVE state

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

With the new support functions this is now possible. Suspending to INACTIVE
is useful for testing purposes.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index cdbedcf..41d6cc3 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -633,11 +633,12 @@ int set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 	if (pwrdm == NULL || IS_ERR(pwrdm))
 		return -EINVAL;
 
-	while (!(pwrdm->pwrsts & (1 << state))) {
-		if (state == PWRDM_POWER_OFF)
-			return ret;
-		state--;
-	}
+	if (state != PWRDM_POWER_INACTIVE)
+		while (!(pwrdm->pwrsts & (1 << state))) {
+			if (state == PWRDM_POWER_OFF)
+				return ret;
+			state--;
+		}
 
 	cur_state = pwrdm_read_next_pwrst(pwrdm);
 	if (cur_state == state)
-- 
1.5.4.3

Re: [PATCHv6 1/9] OMAP3: PM: Added support functions for omap3 pwrdm handling

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Added omap3_pwrdm_set_next_pwrst and omap3_pwrdm_read_next_pwrst. These
> functions add support for INACTIVE and ON states to the standard OMAP
> powerdomain functions, and add caching logic for the next state. These
> functions are used in subsequent patches to simplify the logic of the idle
> loop.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> ---
>  arch/arm/mach-omap2/pm.h     |    2 +
>  arch/arm/mach-omap2/pm34xx.c |   60 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 61 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 75aa685..1d9a740 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -67,6 +67,8 @@ static inline void omap3_pm_init_vc(struct prm_setup_vc *setup_vc)
>  
>  extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
>  extern int omap3_pm_set_suspend_state(struct powerdomain *pwrdm, int state);
> +extern int omap3_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst);
> +extern int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm);
>  
>  extern u32 wakeup_timer_seconds;
>  extern struct omap_dm_timer *gptimer_wakeup;
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 1359ec9..f20d3d8 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -576,6 +576,64 @@ int omap3_can_sleep(void)
>  	return 1;
>  }
>  
> +struct powerdomain_data {
> +	u8 next_state;
> +	u8 init_done;
> +};
> +
> +static struct powerdomain_data mpu_pwrdm_data;
> +static struct powerdomain_data core_pwrdm_data;
> +static struct powerdomain_data neon_pwrdm_data;
> +
> +static struct powerdomain_data *get_pwrdm_data(struct powerdomain *pwrdm)
> +{
> +	if (pwrdm == mpu_pwrdm)
> +		return &mpu_pwrdm_data;
> +	else if (pwrdm == core_pwrdm)
> +		return &core_pwrdm_data;
> +	else if (pwrdm == neon_pwrdm)
> +		return &neon_pwrdm_data;
> +	return NULL;
> +}
> +
> +int omap3_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)

I think this func should be documented along the lines of the changelog.
It should describe the caching and the fact that 'INACTIVE' is a state
that can be read, but not written.

> +{
> +	struct powerdomain_data *data = get_pwrdm_data(pwrdm);
> +	u8 prg_pwrst;
> +
> +	if (!data)
> +		return pwrdm_set_next_pwrst(pwrdm, pwrst);
> +
> +	if (!data->init_done)
> +		data->init_done = 1;

Not sure I follow the purpose of the 'init_done' flag here, but I'm having
an averse reaction to it.

> +	else if (data->next_state == pwrst)
> +		return 0;
> +
> +	if (pwrst == PWRDM_POWER_INACTIVE)
> +		prg_pwrst = PWRDM_POWER_ON;
> +	else
> +		prg_pwrst = pwrst;
> +
> +	pwrdm_set_next_pwrst(pwrdm, prg_pwrst);
> +
> +	if (pwrst == PWRDM_POWER_ON)
> +		omap2_clkdm_deny_idle(pwrdm->pwrdm_clkdms[0]);
> +	else
> +		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> +
> +	data->next_state = pwrst;
> +	return 0;
> +}
> +
> +int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
> +{
> +	struct powerdomain_data *data = get_pwrdm_data(pwrdm);
> +
> +	if (!data || !data->init_done)
> +		return pwrdm_read_next_pwrst(pwrdm);
> +	return data->next_state;
> +}
> +
>  /* This sets pwrdm state (other than mpu & core. Currently only ON &
>   * RET are supported. Function is assuming that clkdm doesn't have
>   * hw_sup mode enabled. */
> @@ -604,7 +662,7 @@ int set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  		pwrdm_wait_transition(pwrdm);
>  	}
>  
> -	ret = pwrdm_set_next_pwrst(pwrdm, state);
> +	ret = omap3_pwrdm_set_next_pwrst(pwrdm, state);
>  	if (ret) {
>  		printk(KERN_ERR "Unable to set state of powerdomain: %s\n",
>  		       pwrdm->name);
> -- 
> 1.5.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 0/9] Refreshed idlestatus patches with minor fixes

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Following changes compared to previous set:
>
> - Fixed a problem with PER domain handling when PER was active
> - Removed rwlock usage from pwrdm_can_idle as the lock was removed from
>   powerdomain code
>
> Still applies on top of PM branch.

This is looking good now, and I'm mostly ready to merge this.
However, a couple more small things need attention.  I'll have some
small comments on a couple of the individual patches.

The CPUidle patches (3, 7, and 8) need a refresh against current PM
branch.  In fact, I think 8 can be dropped as a similar fix from
Sanjeev is merged in the PM branch now.  Also, patch 7 doesn't compile
against PM branch as it's missing the definition of 'iva2_pd'.

Kevin

Re: [PATCHv6 4/9] OMAP3: PM: Removed pwrdm state hacking from omap_sram_idle

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Following hacks will be moved inside cpuidle in subsequent patch:
>
> - CAM domain prevents idle completely
> - PER should not go OFF if core remains active
>
> This simplifies the design and allows cpuidle to keep better track of which
> power states system will actually enter.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

I think this should be combined with PATCH 7.  IOW, the removal and
re-addition should be in the same patch.

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c |   19 ++-----------------
>  1 files changed, 2 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index c2d80fc..cdbedcf 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -93,7 +93,6 @@ static int (*_omap_save_secure_sram)(u32 *addr);
>  
>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>  static struct powerdomain *core_pwrdm, *per_pwrdm;
> -static struct powerdomain *cam_pwrdm;
>  
>  static struct prm_setup_vc prm_setup = {
>  	.clksetup = 0xff,
> @@ -373,7 +372,6 @@ void omap_sram_idle(void)
>  	int core_next_state = PWRDM_POWER_ON;
>  	int core_prev_state, per_prev_state;
>  	u32 sdrc_pwr = 0;
> -	int per_state_modified = 0;
>  
>  	if (!_omap_sram_idle)
>  		return;
> @@ -411,20 +409,11 @@ void omap_sram_idle(void)
>  	core_next_state = omap3_pwrdm_read_next_pwrst(core_pwrdm);
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		omap2_gpio_prepare_for_idle(per_next_state);
> -		if (per_next_state == PWRDM_POWER_OFF) {
> -			if (core_next_state == PWRDM_POWER_ON) {
> -				per_next_state = PWRDM_POWER_RET;
> -				pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
> -				per_state_modified = 1;
> -			} else
> -				omap3_per_save_context();
> -		}
> +		if (per_next_state == PWRDM_POWER_OFF)
> +			omap3_per_save_context();
>  		omap_uart_prepare_idle(2);
>  	}
>  
> -	if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
> -		omap2_clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
> -
>  	/*
>  	 * Disable smartreflex before entering WFI.
>  	 * Only needed if we are going to enter retention or off.
> @@ -554,8 +543,6 @@ void omap_sram_idle(void)
>  		}
>  		omap2_gpio_resume_after_idle();
>  		omap_uart_resume_idle(2);
> -		if (per_state_modified)
> -			pwrdm_set_next_pwrst(per_pwrdm, PWRDM_POWER_OFF);
>  	}
>  
>  	/* Disable IO-PAD and IO-CHAIN wakeup */
> @@ -564,7 +551,6 @@ void omap_sram_idle(void)
>  		omap3_disable_io_chain();
>  	}
>  
> -
>  	pwrdm_post_transition();
>  }
>  
> @@ -1239,7 +1225,6 @@ static int __init omap3_pm_init(void)
>  	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
>  	per_pwrdm = pwrdm_lookup("per_pwrdm");
>  	core_pwrdm = pwrdm_lookup("core_pwrdm");
> -	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>  
>  	neon_clkdm = clkdm_lookup("neon_clkdm");
>  	mpu_clkdm = clkdm_lookup("mpu_clkdm");
> -- 
> 1.5.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 6/9] OMAP: Powerdomains: Add support for checking if pwrdm/clkdm can idle

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> pwrdm_can_idle(pwrdm) will check if the specified powerdomain can enter
> idle. This is done by checking all clockdomains under the powerdomain
> if they can idle also.
>
> omap2_clkdm_can_idle(clkdm) will check if the specified clockdomain can
> enter idle. This checks the functional clocks and idle status bits of the
> domain according to following rules:
> 1) functional clock check
>   * get FCLK register content
>   * ignore all clocks defined in idle_def.fclk_ignore
>   * if any active functional clocks remain, domain can't idle
> 2) idle status check
>   * get IDLEST register content
>   * inverse it (any non-idle blocks are now as 1)
>   * mask against idle_def.idlest_mask
>   * if any bits remain high, domain can't idle
>
> These calls can be used e.g. inside cpuidle to decide which power states
> core and mpu should enter during idle, as there are certain dependencies
> between wakeup capabilities and reset logic.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

I'm OK with this version.  

Paul?

Kevin


> ---
>  arch/arm/mach-omap2/clockdomain.c             |   32 ++++++++
>  arch/arm/mach-omap2/clockdomains.h            |  106 +++++++++++++++++++++++++
>  arch/arm/mach-omap2/powerdomain.c             |   20 +++++
>  arch/arm/plat-omap/include/plat/clockdomain.h |   17 ++++
>  arch/arm/plat-omap/include/plat/powerdomain.h |    1 +
>  5 files changed, 176 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
> index a38a615..9ebff51 100644
> --- a/arch/arm/mach-omap2/clockdomain.c
> +++ b/arch/arm/mach-omap2/clockdomain.c
> @@ -867,6 +867,38 @@ int omap2_clkdm_wakeup(struct clockdomain *clkdm)
>  	return 0;
>  }
>  
> +
> +/**
> + * omap2_clkdm_can_idle - check if clockdomain has any active clocks or not
> + * @clkdm: struct clockdomain *
> + *
> + * Checks if the clockdomain has any active clock or not, i.e. whether it
> + * can enter idle. Returns -EINVAL if clkdm is NULL; 0 if unable to idle;
> + * 1 if can idle.
> + */
> +int omap2_clkdm_can_idle(struct clockdomain *clkdm)
> +{
> +	int i;
> +
> +	if (!clkdm)
> +		return -EINVAL;
> +
> +	for (i = 0; i < clkdm->clk_reg_num; i++) {
> +		u32 idlest, fclk;
> +
> +		fclk = cm_read_mod_reg(clkdm->pwrdm.ptr->prcm_offs,
> +				CM_FCLKEN + 4 * i);
> +		if (fclk & ~clkdm->idle_def[i].fclk_ignore)
> +			return 0;
> +
> +		idlest = cm_read_mod_reg(clkdm->pwrdm.ptr->prcm_offs,
> +				CM_IDLEST + 4 * i);
> +		if (~idlest & clkdm->idle_def[i].idlest_mask)
> +			return 0;
> +	}
> +	return 1;
> +}
> +
>  /**
>   * omap2_clkdm_allow_idle - enable hwsup idle transitions for clkdm
>   * @clkdm: struct clockdomain *
> diff --git a/arch/arm/mach-omap2/clockdomains.h b/arch/arm/mach-omap2/clockdomains.h
> index 8fc19ff..5e29de8 100644
> --- a/arch/arm/mach-omap2/clockdomains.h
> +++ b/arch/arm/mach-omap2/clockdomains.h
> @@ -663,6 +663,12 @@ static struct clockdomain iva2_clkdm = {
>  	.wkdep_srcs	= iva2_wkdeps,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_IVA2_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.clk_reg_num	= 1,
> +	.idle_def	= {
> +		[0] = {
> +			.idlest_mask = OMAP3430_ST_IVA2,
> +		},
> +	},
>  };
>  
>  static struct clockdomain gfx_3430es1_clkdm = {
> @@ -686,6 +692,12 @@ static struct clockdomain sgx_clkdm = {
>  	.sleepdep_srcs	= gfx_sgx_sleepdeps,
>  	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_SGX_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
> +	.clk_reg_num	= 1,
> +	.idle_def	= {
> +		[0] = {
> +			.idlest_mask = OMAP3430ES2_ST_SGX_SHIFT,
> +		},
> +	},
>  };
>  
>  /*
> @@ -717,6 +729,57 @@ static struct clockdomain core_l3_3xxx_clkdm = {
>  	.dep_bit	= OMAP3430_EN_CORE_SHIFT,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_L3_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.clk_reg_num	= 3,
> +	.idle_def	= {
> +		[0] = {
> +			/* UARTs are controlled by idle loop so ignore */
> +			.fclk_ignore = OMAP3430_EN_UART2 |
> +				OMAP3430_EN_UART1,
> +			/*
> +			 * Reason for IDLEST ignores:
> +			 * - SDRC and OMAPCTRL controlled by HW
> +			 * - HSOTGUSB_IDLE bit is autoidled by HW
> +			 * - MAILBOX is controlled by HW
> +			 */
> +			.idlest_mask =
> +				OMAP3430ES2_ST_MMC3_MASK |
> +				OMAP3430_ST_ICR_MASK |
> +				OMAP3430_ST_AES2_MASK |
> +				OMAP3430_ST_SHA12_MASK |
> +				OMAP3430_ST_DES2_MASK |
> +				OMAP3430_ST_MMC2_MASK |
> +				OMAP3430_ST_MMC1_MASK |
> +				OMAP3430_ST_MSPRO_MASK |
> +				OMAP3430_ST_HDQ_MASK |
> +				OMAP3430_ST_MCSPI4_MASK |
> +				OMAP3430_ST_MCSPI3_MASK |
> +				OMAP3430_ST_MCSPI2_MASK |
> +				OMAP3430_ST_MCSPI1_MASK |
> +				OMAP3430_ST_I2C3_MASK |
> +				OMAP3430_ST_I2C2_MASK |
> +				OMAP3430_ST_I2C1_MASK |
> +				OMAP3430_ST_GPT11_MASK |
> +				OMAP3430_ST_GPT10_MASK |
> +				OMAP3430_ST_MCBSP5_MASK |
> +				OMAP3430_ST_MCBSP1_MASK |
> +				OMAP3430ES2_ST_SSI_IDLE_MASK |
> +				OMAP3430ES2_ST_HSOTGUSB_STDBY_MASK |
> +				OMAP3430_ST_D2D_MASK |
> +				OMAP3430_ST_SDMA_MASK |
> +				OMAP3430_ST_SSI_STDBY_MASK,
> +		},
> +		[1] = {
> +			.idlest_mask = OMAP3430_ST_PKA_MASK |
> +				OMAP3430_ST_AES1_MASK |
> +				OMAP3430_ST_RNG_MASK |
> +				OMAP3430_ST_SHA11_MASK |
> +				OMAP3430_ST_DES1_MASK,
> +		},
> +		[2] = {
> +			.idlest_mask = OMAP3430ES2_ST_USBTLL_MASK |
> +				OMAP3430ES2_ST_CPEFUSE_MASK,
> +		},
> +	},
>  };
>  
>  /*
> @@ -746,6 +809,13 @@ static struct clockdomain dss_3xxx_clkdm = {
>  	.sleepdep_srcs	= dss_sleepdeps,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_DSS_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.clk_reg_num	= 1,
> +	.idle_def	= {
> +		[0] = {
> +			.idlest_mask = OMAP3430ES2_ST_DSS_IDLE_MASK |
> +				OMAP3430ES2_ST_DSS_STDBY_MASK,
> +		},
> +	},
>  };
>  
>  static struct clockdomain cam_clkdm = {
> @@ -758,6 +828,12 @@ static struct clockdomain cam_clkdm = {
>  	.sleepdep_srcs	= cam_sleepdeps,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_CAM_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.clk_reg_num	= 1,
> +	.idle_def	= {
> +		[0] = {
> +			.idlest_mask = OMAP3430_ST_CAM,
> +		},
> +	},
>  };
>  
>  static struct clockdomain usbhost_clkdm = {
> @@ -770,6 +846,13 @@ static struct clockdomain usbhost_clkdm = {
>  	.sleepdep_srcs	= usbhost_sleepdeps,
>  	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_USBHOST_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
> +	.clk_reg_num	= 1,
> +	.idle_def	= {
> +		[0] = {
> +			.idlest_mask = OMAP3430ES2_ST_USBHOST_IDLE_MASK |
> +				OMAP3430ES2_ST_USBHOST_STDBY_MASK,
> +		},
> +	},
>  };
>  
>  static struct clockdomain per_clkdm = {
> @@ -783,6 +866,29 @@ static struct clockdomain per_clkdm = {
>  	.sleepdep_srcs	= per_sleepdeps,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_PER_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.clk_reg_num	= 1,
> +	.idle_def	= {
> +		[0] = {
> +			.fclk_ignore = OMAP3430_ST_UART3_MASK,
> +			/*
> +			 * GPIO2...6 are dropped out from this mask
> +			 * as they are always non-idle, UART3 is also
> +			 * out as it is handled by idle loop
> +			 * */
> +			.idlest_mask = OMAP3430_ST_WDT3_MASK |
> +				OMAP3430_ST_GPT9_MASK |
> +				OMAP3430_ST_GPT8_MASK |
> +				OMAP3430_ST_GPT7_MASK |
> +				OMAP3430_ST_GPT6_MASK |
> +				OMAP3430_ST_GPT5_MASK |
> +				OMAP3430_ST_GPT4_MASK |
> +				OMAP3430_ST_GPT3_MASK |
> +				OMAP3430_ST_GPT2_MASK |
> +				OMAP3430_ST_MCBSP4_MASK |
> +				OMAP3430_ST_MCBSP3_MASK |
> +				OMAP3430_ST_MCBSP2_MASK,
> +		},
> +	},
>  };
>  
>  /*
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index dc03289..a9ca884 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -919,6 +919,26 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm)
>  	return 0;
>  }
>  
> +/**
> + * pwrdm_can_idle - check if the powerdomain can enter idle
> + * @pwrdm: struct powerdomain * the powerdomain to check status of
> + *
> + * Checks all associated clockdomains if they can idle or not.
> + * Returns 1 if the powerdomain can idle, 0 if not.
> + */
> +int pwrdm_can_idle(struct powerdomain *pwrdm)
> +{
> +	int i;
> +	int ret = 1;
> +
> +	for (i = 0; i < PWRDM_MAX_CLKDMS; i++)
> +		if (pwrdm->pwrdm_clkdms[i] &&
> +		    !omap2_clkdm_can_idle(pwrdm->pwrdm_clkdms[i]))
> +			ret = 0;
> +
> +	return ret;
> +}
> +
>  int pwrdm_state_switch(struct powerdomain *pwrdm)
>  {
>  	return _pwrdm_state_switch(pwrdm, PWRDM_STATE_NOW);
> diff --git a/arch/arm/plat-omap/include/plat/clockdomain.h b/arch/arm/plat-omap/include/plat/clockdomain.h
> index ba0a6c0..ecf7f88 100644
> --- a/arch/arm/plat-omap/include/plat/clockdomain.h
> +++ b/arch/arm/plat-omap/include/plat/clockdomain.h
> @@ -30,6 +30,12 @@
>  #define CLKDM_CAN_SWSUP		(CLKDM_CAN_FORCE_SLEEP | CLKDM_CAN_FORCE_WAKEUP)
>  #define CLKDM_CAN_HWSUP_SWSUP	(CLKDM_CAN_SWSUP | CLKDM_CAN_HWSUP)
>  
> +/*
> + * Maximum number of FCLK register masks that can be associated with a
> + * clockdomain. CORE powerdomain on OMAP3 is the worst case
> + */
> +#define CLKDM_MAX_CLK_REGS			3
> +
>  /* OMAP24XX CM_CLKSTCTRL_*.AUTOSTATE_* register bit values */
>  #define OMAP24XX_CLKSTCTRL_DISABLE_AUTO		0x0
>  #define OMAP24XX_CLKSTCTRL_ENABLE_AUTO		0x1
> @@ -82,6 +88,11 @@ struct clkdm_dep {
>  	const struct omap_chip_id omap_chip;
>  };
>  
> +struct clkdm_idle_def {
> +	u32 fclk_ignore;
> +	u32 idlest_mask;
> +};
> +
>  /**
>   * struct clockdomain - OMAP clockdomain
>   * @name: clockdomain name
> @@ -95,6 +106,8 @@ struct clkdm_dep {
>   * @omap_chip: OMAP chip types that this clockdomain is valid on
>   * @usecount: Usecount tracking
>   * @node: list_head to link all clockdomains together
> + * @clk_reg_num: Number of idle definitions for clockdomain idle status checks
> + * @idle_def: Declarations for idle checks for clockdomain
>   */
>  struct clockdomain {
>  	const char *name;
> @@ -111,6 +124,8 @@ struct clockdomain {
>  	const struct omap_chip_id omap_chip;
>  	atomic_t usecount;
>  	struct list_head node;
> +	u8 clk_reg_num;
> +	struct clkdm_idle_def idle_def[CLKDM_MAX_CLK_REGS];
>  };
>  
>  void clkdm_init(struct clockdomain **clkdms, struct clkdm_autodep *autodeps);
> @@ -138,4 +153,6 @@ int omap2_clkdm_sleep(struct clockdomain *clkdm);
>  int omap2_clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk);
>  int omap2_clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk);
>  
> +int omap2_clkdm_can_idle(struct clockdomain *clkdm);
> +
>  #endif
> diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h b/arch/arm/plat-omap/include/plat/powerdomain.h
> index e15c7e9..a1ecd47 100644
> --- a/arch/arm/plat-omap/include/plat/powerdomain.h
> +++ b/arch/arm/plat-omap/include/plat/powerdomain.h
> @@ -145,6 +145,7 @@ int pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm);
>  bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm);
>  
>  int pwrdm_wait_transition(struct powerdomain *pwrdm);
> +int pwrdm_can_idle(struct powerdomain *pwrdm);
>  
>  int pwrdm_state_switch(struct powerdomain *pwrdm);
>  int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
> -- 
> 1.5.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 9/9] OMAP3: PM: Added support for suspending to INACTIVE state

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> With the new support functions this is now possible. Suspending to INACTIVE
> is useful for testing purposes.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index cdbedcf..41d6cc3 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -633,11 +633,12 @@ int set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  	if (pwrdm == NULL || IS_ERR(pwrdm))
>  		return -EINVAL;
>  
> -	while (!(pwrdm->pwrsts & (1 << state))) {
> -		if (state == PWRDM_POWER_OFF)
> -			return ret;
> -		state--;
> -	}

The comment above set_pwrdm_state() says only ON & RET are supported.
Please update that comment as well.


> +	if (state != PWRDM_POWER_INACTIVE)
> +		while (!(pwrdm->pwrsts & (1 << state))) {
> +			if (state == PWRDM_POWER_OFF)
> +				return ret;
> +			state--;
> +		}

I think all powerdomains can be inactive right?

I think it would be cleaner to just have all the pwrdm->pwrsts fields
include intactive as a valid option.

Something like the patch below.  IIRC, you did something like this in
one of the earlier versions of the patch.

Kevin

diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h b/arch/arm/plat-omap/include/plat/powerdomain.h
index a1ecd47..c692472 100644
--- a/arch/arm/plat-omap/include/plat/powerdomain.h
+++ b/arch/arm/plat-omap/include/plat/powerdomain.h
@@ -31,17 +31,17 @@
 #define PWRDM_MAX_PWRSTS	4
 
 /* Powerdomain allowable state bitfields */
-#define PWRSTS_OFF_ON		((1 << PWRDM_POWER_OFF) | \
+#define PWRSTS_ON		((1 << PWRDM_POWER_INACTIVE) | \
 				 (1 << PWRDM_POWER_ON))
 
+#define PWRSTS_OFF_ON		((1 << PWRDM_POWER_OFF) | PWRSTS_ON)
+
 #define PWRSTS_OFF_RET		((1 << PWRDM_POWER_OFF) | \
 				 (1 << PWRDM_POWER_RET))
 
-#define PWRSTS_RET_ON		((1 << PWRDM_POWER_RET) | \
-				 (1 << PWRDM_POWER_ON))
-
-#define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON))
+#define PWRSTS_RET_ON		((1 << PWRDM_POWER_RET) | PWRSTS_ON)
 
+#define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | PWRSTS_ON)
 
 /* Powerdomain flags */
 #define PWRDM_HAS_HDWR_SAR	(1 << 0) /* hardware save-and-restore support */

RE: [PATCHv6 9/9] OMAP3: PM: Added support for suspending to INACTIVE state

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 02 March, 2010 01:43
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCHv6 9/9] OMAP3: PM: Added support for 
>suspending to INACTIVE state
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> With the new support functions this is now possible. 
>Suspending to INACTIVE
>> is useful for testing purposes.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |   11 ++++++-----
>>  1 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index cdbedcf..41d6cc3 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -633,11 +633,12 @@ int set_pwrdm_state(struct powerdomain 
>*pwrdm, u32 state)
>>  	if (pwrdm == NULL || IS_ERR(pwrdm))
>>  		return -EINVAL;
>>  
>> -	while (!(pwrdm->pwrsts & (1 << state))) {
>> -		if (state == PWRDM_POWER_OFF)
>> -			return ret;
>> -		state--;
>> -	}
>
>The comment above set_pwrdm_state() says only ON & RET are supported.
>Please update that comment as well.

True, ancient info there. OFF for example has been supported for ages already.

>
>
>> +	if (state != PWRDM_POWER_INACTIVE)
>> +		while (!(pwrdm->pwrsts & (1 << state))) {
>> +			if (state == PWRDM_POWER_OFF)
>> +				return ret;
>> +			state--;
>> +		}
>
>I think all powerdomains can be inactive right?

Yes.

>I think it would be cleaner to just have all the pwrdm->pwrsts fields
>include intactive as a valid option.
>
>Something like the patch below.  IIRC, you did something like this in
>one of the earlier versions of the patch.

Yeah, something like this was done previously, however Paul did not like the idea of changing the generic powerdomain code too much so I dropped it completely. It is now done only via the support functions in patch #1, and only done for the powerdomains that actually need it for the cpuidle (mpu/core/neon.) It would be possible to add support for the rest of the powerdomains also, but I decided to drop this in favor of getting the patch set in.

>
>Kevin
>
>diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h 
>b/arch/arm/plat-omap/include/plat/powerdomain.h
>index a1ecd47..c692472 100644
>--- a/arch/arm/plat-omap/include/plat/powerdomain.h
>+++ b/arch/arm/plat-omap/include/plat/powerdomain.h
>@@ -31,17 +31,17 @@
> #define PWRDM_MAX_PWRSTS	4
> 
> /* Powerdomain allowable state bitfields */
>-#define PWRSTS_OFF_ON		((1 << PWRDM_POWER_OFF) | \
>+#define PWRSTS_ON		((1 << PWRDM_POWER_INACTIVE) | \
> 				 (1 << PWRDM_POWER_ON))
> 
>+#define PWRSTS_OFF_ON		((1 << PWRDM_POWER_OFF) | PWRSTS_ON)
>+
> #define PWRSTS_OFF_RET		((1 << PWRDM_POWER_OFF) | \
> 				 (1 << PWRDM_POWER_RET))
> 
>-#define PWRSTS_RET_ON		((1 << PWRDM_POWER_RET) | \
>-				 (1 << PWRDM_POWER_ON))
>-
>-#define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON))
>+#define PWRSTS_RET_ON		((1 << PWRDM_POWER_RET) | PWRSTS_ON)
> 
>+#define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | PWRSTS_ON)
> 
> /* Powerdomain flags */
> #define PWRDM_HAS_HDWR_SAR	(1 << 0) /* hardware 
>save-and-restore support */
>

RE: [PATCHv6 4/9] OMAP3: PM: Removed pwrdm state hacking from omap_sram_idle

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 02 March, 2010 01:32
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCHv6 4/9] OMAP3: PM: Removed pwrdm state 
>hacking from omap_sram_idle
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> Following hacks will be moved inside cpuidle in subsequent patch:
>>
>> - CAM domain prevents idle completely
>> - PER should not go OFF if core remains active
>>
>> This simplifies the design and allows cpuidle to keep better 
>track of which
>> power states system will actually enter.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>
>I think this should be combined with PATCH 7.  IOW, the removal and
>re-addition should be in the same patch.

Ok, will change this.

>
>Kevin
>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |   19 ++-----------------
>>  1 files changed, 2 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index c2d80fc..cdbedcf 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -93,7 +93,6 @@ static int (*_omap_save_secure_sram)(u32 *addr);
>>  
>>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>>  static struct powerdomain *core_pwrdm, *per_pwrdm;
>> -static struct powerdomain *cam_pwrdm;
>>  
>>  static struct prm_setup_vc prm_setup = {
>>  	.clksetup = 0xff,
>> @@ -373,7 +372,6 @@ void omap_sram_idle(void)
>>  	int core_next_state = PWRDM_POWER_ON;
>>  	int core_prev_state, per_prev_state;
>>  	u32 sdrc_pwr = 0;
>> -	int per_state_modified = 0;
>>  
>>  	if (!_omap_sram_idle)
>>  		return;
>> @@ -411,20 +409,11 @@ void omap_sram_idle(void)
>>  	core_next_state = omap3_pwrdm_read_next_pwrst(core_pwrdm);
>>  	if (per_next_state < PWRDM_POWER_ON) {
>>  		omap2_gpio_prepare_for_idle(per_next_state);
>> -		if (per_next_state == PWRDM_POWER_OFF) {
>> -			if (core_next_state == PWRDM_POWER_ON) {
>> -				per_next_state = PWRDM_POWER_RET;
>> -				pwrdm_set_next_pwrst(per_pwrdm, 
>per_next_state);
>> -				per_state_modified = 1;
>> -			} else
>> -				omap3_per_save_context();
>> -		}
>> +		if (per_next_state == PWRDM_POWER_OFF)
>> +			omap3_per_save_context();
>>  		omap_uart_prepare_idle(2);
>>  	}
>>  
>> -	if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
>> -		omap2_clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
>> -
>>  	/*
>>  	 * Disable smartreflex before entering WFI.
>>  	 * Only needed if we are going to enter retention or off.
>> @@ -554,8 +543,6 @@ void omap_sram_idle(void)
>>  		}
>>  		omap2_gpio_resume_after_idle();
>>  		omap_uart_resume_idle(2);
>> -		if (per_state_modified)
>> -			pwrdm_set_next_pwrst(per_pwrdm, 
>PWRDM_POWER_OFF);
>>  	}
>>  
>>  	/* Disable IO-PAD and IO-CHAIN wakeup */
>> @@ -564,7 +551,6 @@ void omap_sram_idle(void)
>>  		omap3_disable_io_chain();
>>  	}
>>  
>> -
>>  	pwrdm_post_transition();
>>  }
>>  
>> @@ -1239,7 +1225,6 @@ static int __init omap3_pm_init(void)
>>  	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
>>  	per_pwrdm = pwrdm_lookup("per_pwrdm");
>>  	core_pwrdm = pwrdm_lookup("core_pwrdm");
>> -	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>>  
>>  	neon_clkdm = clkdm_lookup("neon_clkdm");
>>  	mpu_clkdm = clkdm_lookup("mpu_clkdm");
>> -- 
>> 1.5.4.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe 
>linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

RE: [PATCHv6 1/9] OMAP3: PM: Added support functions for omap3 pwrdm handling

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 02 March, 2010 01:17
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCHv6 1/9] OMAP3: PM: Added support functions 
>for omap3 pwrdm handling
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> Added omap3_pwrdm_set_next_pwrst and 
>omap3_pwrdm_read_next_pwrst. These
>> functions add support for INACTIVE and ON states to the standard OMAP
>> powerdomain functions, and add caching logic for the next 
>state. These
>> functions are used in subsequent patches to simplify the 
>logic of the idle
>> loop.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm.h     |    2 +
>>  arch/arm/mach-omap2/pm34xx.c |   60 
>+++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 61 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
>> index 75aa685..1d9a740 100644
>> --- a/arch/arm/mach-omap2/pm.h
>> +++ b/arch/arm/mach-omap2/pm.h
>> @@ -67,6 +67,8 @@ static inline void omap3_pm_init_vc(struct 
>prm_setup_vc *setup_vc)
>>  
>>  extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
>>  extern int omap3_pm_set_suspend_state(struct powerdomain 
>*pwrdm, int state);
>> +extern int omap3_pwrdm_set_next_pwrst(struct powerdomain 
>*pwrdm, u8 pwrst);
>> +extern int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm);
>>  
>>  extern u32 wakeup_timer_seconds;
>>  extern struct omap_dm_timer *gptimer_wakeup;
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index 1359ec9..f20d3d8 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -576,6 +576,64 @@ int omap3_can_sleep(void)
>>  	return 1;
>>  }
>>  
>> +struct powerdomain_data {
>> +	u8 next_state;
>> +	u8 init_done;
>> +};
>> +
>> +static struct powerdomain_data mpu_pwrdm_data;
>> +static struct powerdomain_data core_pwrdm_data;
>> +static struct powerdomain_data neon_pwrdm_data;
>> +
>> +static struct powerdomain_data *get_pwrdm_data(struct 
>powerdomain *pwrdm)
>> +{
>> +	if (pwrdm == mpu_pwrdm)
>> +		return &mpu_pwrdm_data;
>> +	else if (pwrdm == core_pwrdm)
>> +		return &core_pwrdm_data;
>> +	else if (pwrdm == neon_pwrdm)
>> +		return &neon_pwrdm_data;
>> +	return NULL;
>> +}
>> +
>> +int omap3_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
>
>I think this func should be documented along the lines of the 
>changelog.
>It should describe the caching and the fact that 'INACTIVE' is a state
>that can be read, but not written.

Ok, will do the change.

>> +{
>> +	struct powerdomain_data *data = get_pwrdm_data(pwrdm);
>> +	u8 prg_pwrst;
>> +
>> +	if (!data)
>> +		return pwrdm_set_next_pwrst(pwrdm, pwrst);
>> +
>> +	if (!data->init_done)
>> +		data->init_done = 1;
>
>Not sure I follow the purpose of the 'init_done' flag here, 
>but I'm having
>an averse reaction to it.

init_done actually means if the cache is valid or not. If not, we ignore the current cached state. Another way of doing this would be to initialize all values at proper point during boot by reading from HW, or just put the HW reset values to the cache during compile time. The cache validity bit is needed in the read_next_pwrst code also below.

>
>> +	else if (data->next_state == pwrst)
>> +		return 0;
>> +
>> +	if (pwrst == PWRDM_POWER_INACTIVE)
>> +		prg_pwrst = PWRDM_POWER_ON;
>> +	else
>> +		prg_pwrst = pwrst;
>> +
>> +	pwrdm_set_next_pwrst(pwrdm, prg_pwrst);
>> +
>> +	if (pwrst == PWRDM_POWER_ON)
>> +		omap2_clkdm_deny_idle(pwrdm->pwrdm_clkdms[0]);
>> +	else
>> +		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>> +
>> +	data->next_state = pwrst;
>> +	return 0;
>> +}
>> +
>> +int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
>> +{
>> +	struct powerdomain_data *data = get_pwrdm_data(pwrdm);
>> +
>> +	if (!data || !data->init_done)
>> +		return pwrdm_read_next_pwrst(pwrdm);
>> +	return data->next_state;
>> +}
>> +
>>  /* This sets pwrdm state (other than mpu & core. Currently only ON &
>>   * RET are supported. Function is assuming that clkdm doesn't have
>>   * hw_sup mode enabled. */
>> @@ -604,7 +662,7 @@ int set_pwrdm_state(struct powerdomain 
>*pwrdm, u32 state)
>>  		pwrdm_wait_transition(pwrdm);
>>  	}
>>  
>> -	ret = pwrdm_set_next_pwrst(pwrdm, state);
>> +	ret = omap3_pwrdm_set_next_pwrst(pwrdm, state);
>>  	if (ret) {
>>  		printk(KERN_ERR "Unable to set state of 
>powerdomain: %s\n",
>>  		       pwrdm->name);
>> -- 
>> 1.5.4.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe 
>linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCHv6 1/9] OMAP3: PM: Added support functions for omap3 pwrdm handling

[Kevin Hilman]

<Tero.Kristo@nokia.com> writes:

>  
>
>>-----Original Message-----
>>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>>Sent: 02 March, 2010 01:17
>>To: Kristo Tero (Nokia-D/Tampere)
>>Cc: linux-omap@vger.kernel.org
>>Subject: Re: [PATCHv6 1/9] OMAP3: PM: Added support functions 
>>for omap3 pwrdm handling
>>
>>Tero Kristo <tero.kristo@nokia.com> writes:
>>
>>> From: Tero Kristo <tero.kristo@nokia.com>
>>>
>>> Added omap3_pwrdm_set_next_pwrst and 
>>omap3_pwrdm_read_next_pwrst. These
>>> functions add support for INACTIVE and ON states to the standard OMAP
>>> powerdomain functions, and add caching logic for the next 
>>state. These
>>> functions are used in subsequent patches to simplify the 
>>logic of the idle
>>> loop.
>>>
>>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>>> ---
>>>  arch/arm/mach-omap2/pm.h     |    2 +
>>>  arch/arm/mach-omap2/pm34xx.c |   60 
>>+++++++++++++++++++++++++++++++++++++++++-
>>>  2 files changed, 61 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
>>> index 75aa685..1d9a740 100644
>>> --- a/arch/arm/mach-omap2/pm.h
>>> +++ b/arch/arm/mach-omap2/pm.h
>>> @@ -67,6 +67,8 @@ static inline void omap3_pm_init_vc(struct 
>>prm_setup_vc *setup_vc)
>>>  
>>>  extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
>>>  extern int omap3_pm_set_suspend_state(struct powerdomain 
>>*pwrdm, int state);
>>> +extern int omap3_pwrdm_set_next_pwrst(struct powerdomain 
>>*pwrdm, u8 pwrst);
>>> +extern int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm);
>>>  
>>>  extern u32 wakeup_timer_seconds;
>>>  extern struct omap_dm_timer *gptimer_wakeup;
>>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>>b/arch/arm/mach-omap2/pm34xx.c
>>> index 1359ec9..f20d3d8 100644
>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>> @@ -576,6 +576,64 @@ int omap3_can_sleep(void)
>>>  	return 1;
>>>  }
>>>  
>>> +struct powerdomain_data {
>>> +	u8 next_state;
>>> +	u8 init_done;
>>> +};
>>> +
>>> +static struct powerdomain_data mpu_pwrdm_data;
>>> +static struct powerdomain_data core_pwrdm_data;
>>> +static struct powerdomain_data neon_pwrdm_data;
>>> +
>>> +static struct powerdomain_data *get_pwrdm_data(struct 
>>powerdomain *pwrdm)
>>> +{
>>> +	if (pwrdm == mpu_pwrdm)
>>> +		return &mpu_pwrdm_data;
>>> +	else if (pwrdm == core_pwrdm)
>>> +		return &core_pwrdm_data;
>>> +	else if (pwrdm == neon_pwrdm)
>>> +		return &neon_pwrdm_data;
>>> +	return NULL;
>>> +}
>>> +
>>> +int omap3_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
>>
>>I think this func should be documented along the lines of the 
>>changelog.
>>It should describe the caching and the fact that 'INACTIVE' is a state
>>that can be read, but not written.
>
> Ok, will do the change.
>
>>> +{
>>> +	struct powerdomain_data *data = get_pwrdm_data(pwrdm);
>>> +	u8 prg_pwrst;
>>> +
>>> +	if (!data)
>>> +		return pwrdm_set_next_pwrst(pwrdm, pwrst);
>>> +
>>> +	if (!data->init_done)
>>> +		data->init_done = 1;
>>
>>Not sure I follow the purpose of the 'init_done' flag here, 
>>but I'm having
>>an averse reaction to it.
>
> init_done actually means if the cache is valid or not. If not, we ignore the current cached state. Another way of doing this would be to initialize all values at proper point during boot by reading from HW, or just put the HW reset values to the cache during compile time. The cache validity bit is needed in the read_next_pwrst code also below.

I'd rather see this done using init-time reads from the HW.

Kevin


>>
>>> +	else if (data->next_state == pwrst)
>>> +		return 0;
>>> +
>>> +	if (pwrst == PWRDM_POWER_INACTIVE)
>>> +		prg_pwrst = PWRDM_POWER_ON;
>>> +	else
>>> +		prg_pwrst = pwrst;
>>> +
>>> +	pwrdm_set_next_pwrst(pwrdm, prg_pwrst);
>>> +
>>> +	if (pwrst == PWRDM_POWER_ON)
>>> +		omap2_clkdm_deny_idle(pwrdm->pwrdm_clkdms[0]);
>>> +	else
>>> +		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>>> +
>>> +	data->next_state = pwrst;
>>> +	return 0;
>>> +}
>>> +
>>> +int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
>>> +{
>>> +	struct powerdomain_data *data = get_pwrdm_data(pwrdm);
>>> +
>>> +	if (!data || !data->init_done)
>>> +		return pwrdm_read_next_pwrst(pwrdm);
>>> +	return data->next_state;
>>> +}
>>> +
>>>  /* This sets pwrdm state (other than mpu & core. Currently only ON &
>>>   * RET are supported. Function is assuming that clkdm doesn't have
>>>   * hw_sup mode enabled. */
>>> @@ -604,7 +662,7 @@ int set_pwrdm_state(struct powerdomain 
>>*pwrdm, u32 state)
>>>  		pwrdm_wait_transition(pwrdm);
>>>  	}
>>>  
>>> -	ret = pwrdm_set_next_pwrst(pwrdm, state);
>>> +	ret = omap3_pwrdm_set_next_pwrst(pwrdm, state);
>>>  	if (ret) {
>>>  		printk(KERN_ERR "Unable to set state of 
>>powerdomain: %s\n",
>>>  		       pwrdm->name);
>>> -- 
>>> 1.5.4.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe 
>>linux-omap" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

Re: [PATCHv6 9/9] OMAP3: PM: Added support for suspending to INACTIVE state

[Kevin Hilman]

<Tero.Kristo@nokia.com> writes:

[...]

> True, ancient info there. OFF for example has been supported for ages already.
>
>>
>>
>>> +	if (state != PWRDM_POWER_INACTIVE)
>>> +		while (!(pwrdm->pwrsts & (1 << state))) {
>>> +			if (state == PWRDM_POWER_OFF)
>>> +				return ret;
>>> +			state--;
>>> +		}
>>
>>I think all powerdomains can be inactive right?
>
> Yes.
>
>>I think it would be cleaner to just have all the pwrdm->pwrsts fields
>>include intactive as a valid option.
>>
>>Something like the patch below.  IIRC, you did something like this in
>>one of the earlier versions of the patch.
>
> Yeah, something like this was done previously, however Paul did not
> like the idea of changing the generic powerdomain code too much so I
> dropped it completely. It is now done only via the support functions
> in patch #1, and only done for the powerdomains that actually need
> it for the cpuidle (mpu/core/neon.) It would be possible to add
> support for the rest of the powerdomains also, but I decided to drop
> this in favor of getting the patch set in.

I'm not proposing changing any of the other powerdomain code.  Just
changing the PWRSTS_* defines, essentially so that INACTIVE is
a valid state.

That will eliminate the need for a special check for inactive in this
patch.

Kevin

>>
>>diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h 
>>b/arch/arm/plat-omap/include/plat/powerdomain.h
>>index a1ecd47..c692472 100644
>>--- a/arch/arm/plat-omap/include/plat/powerdomain.h
>>+++ b/arch/arm/plat-omap/include/plat/powerdomain.h
>>@@ -31,17 +31,17 @@
>> #define PWRDM_MAX_PWRSTS	4
>> 
>> /* Powerdomain allowable state bitfields */
>>-#define PWRSTS_OFF_ON		((1 << PWRDM_POWER_OFF) | \
>>+#define PWRSTS_ON		((1 << PWRDM_POWER_INACTIVE) | \
>> 				 (1 << PWRDM_POWER_ON))
>> 
>>+#define PWRSTS_OFF_ON		((1 << PWRDM_POWER_OFF) | PWRSTS_ON)
>>+
>> #define PWRSTS_OFF_RET		((1 << PWRDM_POWER_OFF) | \
>> 				 (1 << PWRDM_POWER_RET))
>> 
>>-#define PWRSTS_RET_ON		((1 << PWRDM_POWER_RET) | \
>>-				 (1 << PWRDM_POWER_ON))
>>-
>>-#define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON))
>>+#define PWRSTS_RET_ON		((1 << PWRDM_POWER_RET) | PWRSTS_ON)
>> 
>>+#define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | PWRSTS_ON)
>> 
>> /* Powerdomain flags */
>> #define PWRDM_HAS_HDWR_SAR	(1 << 0) /* hardware 
>>save-and-restore support */
>>

RE: [PATCHv6 9/9] OMAP3: PM: Added support for suspending to INACTIVE state

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 08 March, 2010 19:16
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCHv6 9/9] OMAP3: PM: Added support for 
>suspending to INACTIVE state
>
><Tero.Kristo@nokia.com> writes:
>
>[...]
>
>> True, ancient info there. OFF for example has been supported 
>for ages already.
>>
>>>
>>>
>>>> +	if (state != PWRDM_POWER_INACTIVE)
>>>> +		while (!(pwrdm->pwrsts & (1 << state))) {
>>>> +			if (state == PWRDM_POWER_OFF)
>>>> +				return ret;
>>>> +			state--;
>>>> +		}
>>>
>>>I think all powerdomains can be inactive right?
>>
>> Yes.
>>
>>>I think it would be cleaner to just have all the pwrdm->pwrsts fields
>>>include intactive as a valid option.
>>>
>>>Something like the patch below.  IIRC, you did something like this in
>>>one of the earlier versions of the patch.
>>
>> Yeah, something like this was done previously, however Paul did not
>> like the idea of changing the generic powerdomain code too much so I
>> dropped it completely. It is now done only via the support functions
>> in patch #1, and only done for the powerdomains that actually need
>> it for the cpuidle (mpu/core/neon.) It would be possible to add
>> support for the rest of the powerdomains also, but I decided to drop
>> this in favor of getting the patch set in.
>
>I'm not proposing changing any of the other powerdomain code.  Just
>changing the PWRSTS_* defines, essentially so that INACTIVE is
>a valid state.
>
>That will eliminate the need for a special check for inactive in this
>patch.

This is a chicken-egg problem. If you alter the PWRSTS_* defines, you need to change implementation of pwrdm_set_next_pwrst() as it would accept INACTIVE also, which is not supported by the code right now.

>
>Kevin
>
>>>
>>>diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h 
>>>b/arch/arm/plat-omap/include/plat/powerdomain.h
>>>index a1ecd47..c692472 100644
>>>--- a/arch/arm/plat-omap/include/plat/powerdomain.h
>>>+++ b/arch/arm/plat-omap/include/plat/powerdomain.h
>>>@@ -31,17 +31,17 @@
>>> #define PWRDM_MAX_PWRSTS	4
>>> 
>>> /* Powerdomain allowable state bitfields */
>>>-#define PWRSTS_OFF_ON		((1 << PWRDM_POWER_OFF) | \
>>>+#define PWRSTS_ON		((1 << PWRDM_POWER_INACTIVE) | \
>>> 				 (1 << PWRDM_POWER_ON))
>>> 
>>>+#define PWRSTS_OFF_ON		((1 << PWRDM_POWER_OFF) 
>| PWRSTS_ON)
>>>+
>>> #define PWRSTS_OFF_RET		((1 << PWRDM_POWER_OFF) | \
>>> 				 (1 << PWRDM_POWER_RET))
>>> 
>>>-#define PWRSTS_RET_ON		((1 << PWRDM_POWER_RET) | \
>>>-				 (1 << PWRDM_POWER_ON))
>>>-
>>>-#define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON))
>>>+#define PWRSTS_RET_ON		((1 << PWRDM_POWER_RET) 
>| PWRSTS_ON)
>>> 
>>>+#define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | PWRSTS_ON)
>>> 
>>> /* Powerdomain flags */
>>> #define PWRDM_HAS_HDWR_SAR	(1 << 0) /* hardware 
>>>save-and-restore support */
>>>
>

Re: [PATCHv6 9/9] OMAP3: PM: Added support for suspending to INACTIVE state

[Kevin Hilman]

<Tero.Kristo@nokia.com> writes:

>  
>
>>-----Original Message-----
>>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>>Sent: 08 March, 2010 19:16
>>To: Kristo Tero (Nokia-D/Tampere)
>>Cc: linux-omap@vger.kernel.org
>>Subject: Re: [PATCHv6 9/9] OMAP3: PM: Added support for 
>>suspending to INACTIVE state
>>
>><Tero.Kristo@nokia.com> writes:
>>
>>[...]
>>
>>> True, ancient info there. OFF for example has been supported 
>>for ages already.
>>>
>>>>
>>>>
>>>>> +	if (state != PWRDM_POWER_INACTIVE)
>>>>> +		while (!(pwrdm->pwrsts & (1 << state))) {
>>>>> +			if (state == PWRDM_POWER_OFF)
>>>>> +				return ret;
>>>>> +			state--;
>>>>> +		}
>>>>
>>>>I think all powerdomains can be inactive right?
>>>
>>> Yes.
>>>
>>>>I think it would be cleaner to just have all the pwrdm->pwrsts fields
>>>>include intactive as a valid option.
>>>>
>>>>Something like the patch below.  IIRC, you did something like this in
>>>>one of the earlier versions of the patch.
>>>
>>> Yeah, something like this was done previously, however Paul did not
>>> like the idea of changing the generic powerdomain code too much so I
>>> dropped it completely. It is now done only via the support functions
>>> in patch #1, and only done for the powerdomains that actually need
>>> it for the cpuidle (mpu/core/neon.) It would be possible to add
>>> support for the rest of the powerdomains also, but I decided to drop
>>> this in favor of getting the patch set in.
>>
>>I'm not proposing changing any of the other powerdomain code.  Just
>>changing the PWRSTS_* defines, essentially so that INACTIVE is
>>a valid state.
>>
>>That will eliminate the need for a special check for inactive in this
>>patch.
>
> This is a chicken-egg problem. If you alter the PWRSTS_* defines,
> you need to change implementation of pwrdm_set_next_pwrst() as it
> would accept INACTIVE also, which is not supported by the code right
> now.

OK, I see the chicken-egg problem now.  

You're original version is ok with me.

Thanks,

Kevin