[PATCHv2 0/6] Idle status patches revisited

[PATCHv2 0/6] Idle status patches revisited

[Tero Kristo]

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

Improvements compared to previous set:

- Fixed wrong pwrdm state check on IO chain arming on patch 2
- Improved changelog on patch 2 & 4
- Moved FCLK checks inside clockdomain code from powerdomain code in patch 5
- Some style changes on patch 6

Still, I believe there is no simple way of implementing the idle status
check by using the usecounts inside clock framework. It would probably be
possible by adding a new usecount variable that would be updated according
to slightly different rules, but this also looks bit complicated compared
to the need.

-Tero

[PATCHv2 1/6] OMAP: Powerdomains: Add support for INACTIVE state on pwrdm level

[Tero Kristo]

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

Currently only ON, RET and OFF are supported, and ON is arguably broken as it
allows the powerdomain to enter INACTIVE state unless idle is prevented.
Now, pwrdm code prevents idle if ON is selected, and also adds support for
INACTIVE. This simplifies the control needed inside sleep code.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/powerdomain.c             |   32 +++++++++++++++++++++----
 arch/arm/mach-omap2/powerdomains34xx.h        |   26 ++++++++++----------
 arch/arm/plat-omap/include/plat/powerdomain.h |    6 ++++-
 3 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index b6990e3..1237717 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -112,8 +112,8 @@ static struct powerdomain *_pwrdm_deps_lookup(struct powerdomain *pwrdm,
 static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 {
 
-	int prev;
-	int state;
+	u8 prev;
+	u8 state;
 
 	if (pwrdm == NULL)
 		return -EINVAL;
@@ -220,7 +220,7 @@ int pwrdm_register(struct powerdomain *pwrdm)
 
 	pr_debug("powerdomain: registered %s\n", pwrdm->name);
 	ret = 0;
-
+	pwrdm->next_state = -1;
 pr_unlock:
 	write_unlock_irqrestore(&pwrdm_rwlock, flags);
 
@@ -701,19 +701,38 @@ int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm)
  */
 int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
+	u8 prg_pwrst;
+
 	if (!pwrdm)
 		return -EINVAL;
 
+	if (pwrdm->next_state == pwrst)
+		return 0;
+
 	if (!(pwrdm->pwrsts & (1 << pwrst)))
 		return -EINVAL;
 
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
 
+	/* INACTIVE is reserved, so we program pwrdm as ON */
+	if (pwrst == PWRDM_POWER_INACTIVE)
+		prg_pwrst = PWRDM_POWER_ON;
+	else
+		prg_pwrst = pwrst;
+
 	prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
-			     (pwrst << OMAP_POWERSTATE_SHIFT),
+			     (prg_pwrst << OMAP_POWERSTATE_SHIFT),
 			     pwrdm->prcm_offs, PM_PWSTCTRL);
 
+	/* If next state is ON, prevent idle */
+	if (pwrst == PWRDM_POWER_ON)
+		omap2_clkdm_deny_idle(pwrdm->pwrdm_clkdms[0]);
+	else
+		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+
+	pwrdm->next_state = pwrst;
+
 	return 0;
 }
 
@@ -730,8 +749,11 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 	if (!pwrdm)
 		return -EINVAL;
 
+	if (pwrdm->next_state > -1)
+		return pwrdm->next_state;
+
 	return prm_read_mod_bits_shift(pwrdm->prcm_offs, PM_PWSTCTRL,
-					OMAP_POWERSTATE_MASK);
+				       OMAP_POWERSTATE_MASK);
 }
 
 /**
diff --git a/arch/arm/mach-omap2/powerdomains34xx.h b/arch/arm/mach-omap2/powerdomains34xx.h
index fd09b08..9eb2dc5 100644
--- a/arch/arm/mach-omap2/powerdomains34xx.h
+++ b/arch/arm/mach-omap2/powerdomains34xx.h
@@ -165,7 +165,7 @@ static struct powerdomain iva2_pwrdm = {
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 	.dep_bit	  = OMAP3430_PM_WKDEP_MPU_EN_IVA2_SHIFT,
 	.wkdep_srcs	  = iva2_wkdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 4,
 	.pwrsts_mem_ret	  = {
@@ -188,7 +188,7 @@ static struct powerdomain mpu_34xx_pwrdm = {
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 	.dep_bit	  = OMAP3430_EN_MPU_SHIFT,
 	.wkdep_srcs	  = mpu_34xx_wkdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	  = {
@@ -206,7 +206,7 @@ static struct powerdomain core_34xx_pre_es3_1_pwrdm = {
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES1 |
 					   CHIP_IS_OMAP3430ES2 |
 					   CHIP_IS_OMAP3430ES3_0),
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.dep_bit	  = OMAP3430_EN_CORE_SHIFT,
 	.banks		  = 2,
 	.pwrsts_mem_ret	  = {
@@ -214,8 +214,8 @@ static struct powerdomain core_34xx_pre_es3_1_pwrdm = {
 		[1] = PWRSTS_OFF_RET,	 /* MEM2RETSTATE */
 	},
 	.pwrsts_mem_on	  = {
-		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
-		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
+		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
+		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
 	},
 };
 
@@ -224,7 +224,7 @@ static struct powerdomain core_34xx_es3_1_pwrdm = {
 	.name		  = "core_pwrdm",
 	.prcm_offs	  = CORE_MOD,
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES3_1),
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.dep_bit	  = OMAP3430_EN_CORE_SHIFT,
 	.flags		  = PWRDM_HAS_HDWR_SAR, /* for USBTLL only */
 	.banks		  = 2,
@@ -233,8 +233,8 @@ static struct powerdomain core_34xx_es3_1_pwrdm = {
 		[1] = PWRSTS_OFF_RET,	 /* MEM2RETSTATE */
 	},
 	.pwrsts_mem_on	  = {
-		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
-		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
+		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
+		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
 	},
 };
 
@@ -246,7 +246,7 @@ static struct powerdomain dss_pwrdm = {
 	.dep_bit	  = OMAP3430_PM_WKDEP_MPU_EN_DSS_SHIFT,
 	.wkdep_srcs	  = cam_dss_wkdeps,
 	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRDM_POWER_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	  = {
@@ -286,7 +286,7 @@ static struct powerdomain cam_pwrdm = {
 	.prcm_offs	  = OMAP3430_CAM_MOD,
 	.wkdep_srcs	  = cam_dss_wkdeps,
 	.sleepdep_srcs	  = cam_gfx_sleepdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRDM_POWER_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	  = {
@@ -304,7 +304,7 @@ static struct powerdomain per_pwrdm = {
 	.dep_bit	  = OMAP3430_EN_PER_SHIFT,
 	.wkdep_srcs	  = per_usbhost_wkdeps,
 	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	  = {
@@ -326,7 +326,7 @@ static struct powerdomain neon_pwrdm = {
 	.prcm_offs	  = OMAP3430_NEON_MOD,
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 	.wkdep_srcs	  = neon_wkdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRDM_POWER_RET,
 };
 
@@ -336,7 +336,7 @@ static struct powerdomain usbhost_pwrdm = {
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
 	.wkdep_srcs	  = per_usbhost_wkdeps,
 	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRDM_POWER_RET,
 	/*
 	 * REVISIT: Enabling usb host save and restore mechanism seems to
diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h b/arch/arm/plat-omap/include/plat/powerdomain.h
index 3d45ee1..55350d0 100644
--- a/arch/arm/plat-omap/include/plat/powerdomain.h
+++ b/arch/arm/plat-omap/include/plat/powerdomain.h
@@ -37,6 +37,9 @@
 
 #define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON))
 
+#define PWRSTS_OFF_RET_INA_ON	(PWRSTS_OFF_RET_ON | \
+				 (1 << PWRDM_POWER_INACTIVE))
+
 
 /* Powerdomain flags */
 #define PWRDM_HAS_HDWR_SAR	(1 << 0) /* hardware save-and-restore support */
@@ -117,7 +120,8 @@ struct powerdomain {
 
 	struct list_head node;
 
-	int state;
+	u8 state;
+	s8 next_state;
 	unsigned state_counter[4];
 
 #ifdef CONFIG_PM_DEBUG
-- 
1.5.4.3

[PATCHv2 2/6] OMAP3: PM: Added support for INACTIVE and ON states for powerdomains

[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 |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 3b62c96..574492e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -391,6 +391,7 @@ void omap_sram_idle(void)
 	mpu_next_state = 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;
@@ -460,9 +461,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();
 
@@ -561,15 +564,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)
@@ -617,7 +618,6 @@ int set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 	}
 
 	if (sleep_switch) {
-		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
 		pwrdm_wait_transition(pwrdm);
 		pwrdm_state_switch(pwrdm);
 	}
-- 
1.5.4.3

[PATCHv2 3/6] OMAP3: CPUidle: Fixed support for ON / INACTIVE states

[Tero Kristo]

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

New powerdomain code support 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 |   32 ++++----------------------------
 1 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 1cfa5a6..4a81ef1 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
@@ -141,19 +127,9 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 	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

[PATCHv2 4/6] 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 |   18 ++----------------
 1 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 574492e..d77fc88 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -81,7 +81,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_times_vc prm_setup_times_default = {
 	.clksetup = 0xff,
@@ -378,7 +377,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;
@@ -421,19 +419,10 @@ void omap_sram_idle(void)
 	if (per_next_state < PWRDM_POWER_ON) {
 		omap_uart_prepare_idle(2);
 		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();
 	}
 
-	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.
@@ -559,8 +548,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 */
@@ -1174,7 +1161,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");
 
 	omap_push_sram_idle();
 #ifdef CONFIG_SUSPEND
-- 
1.5.4.3

[PATCHv2 5/6] 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 clock status in the clockdomain.

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             |   24 ++++++++++++++++++++++++
 arch/arm/mach-omap2/clockdomains.h            |   14 ++++++++++++++
 arch/arm/mach-omap2/powerdomain.c             |   25 +++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/clockdomain.h |   12 ++++++++++++
 arch/arm/plat-omap/include/plat/powerdomain.h |    1 +
 5 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index fcd8232..9ddeb96 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -474,6 +474,30 @@ 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;
+	const int fclk_regs[] = { CM_FCLKEN, OMAP3430ES2_CM_FCLKEN3 };
+
+	if (!clkdm)
+		return -EINVAL;
+
+	for (i = 0; i < clkdm->fclk_reg_amt; i++)
+		if (cm_read_mod_reg(clkdm->pwrdm.ptr->prcm_offs,
+				    fclk_regs[i]) & ~clkdm->fclk_masks[i])
+			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 c4ee076..2cde82a 100644
--- a/arch/arm/mach-omap2/clockdomains.h
+++ b/arch/arm/mach-omap2/clockdomains.h
@@ -167,6 +167,7 @@ static struct clockdomain iva2_clkdm = {
 	.flags		= CLKDM_CAN_HWSUP_SWSUP,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_IVA2_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.fclk_reg_amt	= 1,
 };
 
 static struct clockdomain gfx_3430es1_clkdm = {
@@ -183,6 +184,7 @@ static struct clockdomain sgx_clkdm = {
 	.flags		= CLKDM_CAN_HWSUP_SWSUP,
 	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_SGX_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
+	.fclk_reg_amt	= 1,
 };
 
 /*
@@ -206,6 +208,11 @@ static struct clockdomain core_l3_34xx_clkdm = {
 	.flags		= CLKDM_CAN_HWSUP,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_L3_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.fclk_reg_amt	= 2,
+	.fclk_masks	= {
+		[0] = OMAP3430_EN_UART2 | OMAP3430_EN_UART1,
+		[1] = 0,
+	},
 };
 
 static struct clockdomain core_l4_34xx_clkdm = {
@@ -222,6 +229,7 @@ static struct clockdomain dss_34xx_clkdm = {
 	.flags		= CLKDM_CAN_HWSUP_SWSUP,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_DSS_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.fclk_reg_amt	= 1,
 };
 
 static struct clockdomain cam_clkdm = {
@@ -230,6 +238,7 @@ static struct clockdomain cam_clkdm = {
 	.flags		= CLKDM_CAN_HWSUP_SWSUP,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_CAM_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.fclk_reg_amt	= 1,
 };
 
 static struct clockdomain usbhost_clkdm = {
@@ -238,6 +247,7 @@ static struct clockdomain usbhost_clkdm = {
 	.flags		= CLKDM_CAN_HWSUP_SWSUP,
 	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_USBHOST_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
+	.fclk_reg_amt	= 1,
 };
 
 static struct clockdomain per_clkdm = {
@@ -246,6 +256,10 @@ static struct clockdomain per_clkdm = {
 	.flags		= CLKDM_CAN_HWSUP_SWSUP,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_PER_MASK,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+	.fclk_reg_amt	= 1,
+	.fclk_masks	= {
+		[0] = OMAP3430_EN_UART3,
+	},
 };
 
 /*
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 1237717..2bef099 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -1217,6 +1217,31 @@ 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)
+{
+	unsigned long flags;
+	int i;
+	int ret = 1;
+
+	read_lock_irqsave(&pwrdm_rwlock, flags);
+
+	for (i = 0; i < PWRDM_MAX_CLKDMS; i++)
+		if (pwrdm->pwrdm_clkdms[i] &&
+		    !omap2_clkdm_can_idle(pwrdm->pwrdm_clkdms[i]))
+			ret = 0;
+
+	read_unlock_irqrestore(&pwrdm_rwlock, flags);
+
+	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 eb73482..4dfb820 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_FCLK				2
+
 /* OMAP24XX CM_CLKSTCTRL_*.AUTOSTATE_* register bit values */
 #define OMAP24XX_CLKSTCTRL_DISABLE_AUTO		0x0
 #define OMAP24XX_CLKSTCTRL_ENABLE_AUTO		0x1
@@ -83,6 +89,10 @@ struct clockdomain {
 	/* OMAP chip types that this clockdomain is valid on */
 	const struct omap_chip_id omap_chip;
 
+	/* For idle checks */
+	u8 fclk_reg_amt;
+	u32 fclk_masks[CLKDM_MAX_FCLK];
+
 	/* Usecount tracking */
 	atomic_t usecount;
 
@@ -108,4 +118,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 55350d0..e6a227f 100644
--- a/arch/arm/plat-omap/include/plat/powerdomain.h
+++ b/arch/arm/plat-omap/include/plat/powerdomain.h
@@ -177,6 +177,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

[PATCHv2 6/6] 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 |  118 ++++++++++++++++++++++++++++++++++--
 1 files changed, 111 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 4a81ef1..dad64a9 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;
+struct powerdomain *mpu_pd, *core_pd, *per_pd, *iva2_pd;
+struct powerdomain *sgx_pd, *usb_pd, *cam_pd, *dss_pd;
 
 /*
  * The latencies/thresholds for various C states have
@@ -92,6 +93,22 @@ static int omap3_idle_bm_check(void)
 }
 
 /**
+ * pwrdm_get_idle_state - Get the power state a pwrdm will enter during idle
+ * @pwrdm: powerdomain to check state for
+ *
+ * Checks if the powerdomain can enter idle or not, if yes, will return
+ * the programmed target state for the domain. Otherwise will indicate
+ * that the domain will stay on.
+ * Returns the power state the pwrdm will enter.
+ */
+static int pwrdm_get_idle_state(struct powerdomain *pwrdm)
+{
+	if (pwrdm_can_idle(pwrdm))
+		return pwrdm_read_next_pwrst(pwrdm);
+	return PWRDM_POWER_ON;
+}
+
+/**
  * omap3_enter_idle - Programs OMAP3 to enter the specified state
  * @dev: cpuidle device
  * @state: The target state to be programmed
@@ -153,14 +170,94 @@ 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_get_idle_state(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 = 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;
+
+		/*
+		 * 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_get_idle_state(dss_pd);
+			iva2_state = pwrdm_get_idle_state(iva2_pd);
+			sgx_state = pwrdm_get_idle_state(sgx_pd);
+			usb_state = pwrdm_get_idle_state(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)
+			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)
+		pwrdm_set_next_pwrst(per_pd, saved_per_state);
+
+	return ret;
 }
 
 DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
@@ -224,7 +321,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 +420,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

Re: [PATCHv2 6/6] OMAP3: CPUidle: Added peripheral pwrdm checks into bm check

[Kevin Hilman]

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

> 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>

Looks good, will queue when dependencies are ready.

Kevin

> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |  118 ++++++++++++++++++++++++++++++++++--
>  1 files changed, 111 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 4a81ef1..dad64a9 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;
> +struct powerdomain *mpu_pd, *core_pd, *per_pd, *iva2_pd;
> +struct powerdomain *sgx_pd, *usb_pd, *cam_pd, *dss_pd;
>  
>  /*
>   * The latencies/thresholds for various C states have
> @@ -92,6 +93,22 @@ static int omap3_idle_bm_check(void)
>  }
>  
>  /**
> + * pwrdm_get_idle_state - Get the power state a pwrdm will enter during idle
> + * @pwrdm: powerdomain to check state for
> + *
> + * Checks if the powerdomain can enter idle or not, if yes, will return
> + * the programmed target state for the domain. Otherwise will indicate
> + * that the domain will stay on.
> + * Returns the power state the pwrdm will enter.
> + */
> +static int pwrdm_get_idle_state(struct powerdomain *pwrdm)
> +{
> +	if (pwrdm_can_idle(pwrdm))
> +		return pwrdm_read_next_pwrst(pwrdm);
> +	return PWRDM_POWER_ON;
> +}
> +
> +/**
>   * omap3_enter_idle - Programs OMAP3 to enter the specified state
>   * @dev: cpuidle device
>   * @state: The target state to be programmed
> @@ -153,14 +170,94 @@ 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_get_idle_state(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 = 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;
> +
> +		/*
> +		 * 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_get_idle_state(dss_pd);
> +			iva2_state = pwrdm_get_idle_state(iva2_pd);
> +			sgx_state = pwrdm_get_idle_state(sgx_pd);
> +			usb_state = pwrdm_get_idle_state(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)
> +			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)
> +		pwrdm_set_next_pwrst(per_pd, saved_per_state);
> +
> +	return ret;
>  }
>  
>  DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
> @@ -224,7 +321,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 +420,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
>
> --
> 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: [PATCHv2 5/6] 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 clock status in the clockdomain.
>
> 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>

This looks good by me, but needs a review from Paul to go upstream with
the rest of his clockdomain/powerdomain updates for 2.6.34.

One question... I'm wondering if it might be good to also check the
CM_IDLEST_<pwrdm> register in the can_idle function.  Even if all
clocks are disabled, if the idle modes of the module are not set correctly,
it may not enter the target state.

Kevin

> ---
>  arch/arm/mach-omap2/clockdomain.c             |   24 ++++++++++++++++++++++++
>  arch/arm/mach-omap2/clockdomains.h            |   14 ++++++++++++++
>  arch/arm/mach-omap2/powerdomain.c             |   25 +++++++++++++++++++++++++
>  arch/arm/plat-omap/include/plat/clockdomain.h |   12 ++++++++++++
>  arch/arm/plat-omap/include/plat/powerdomain.h |    1 +
>  5 files changed, 76 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
> index fcd8232..9ddeb96 100644
> --- a/arch/arm/mach-omap2/clockdomain.c
> +++ b/arch/arm/mach-omap2/clockdomain.c
> @@ -474,6 +474,30 @@ 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;
> +	const int fclk_regs[] = { CM_FCLKEN, OMAP3430ES2_CM_FCLKEN3 };
> +
> +	if (!clkdm)
> +		return -EINVAL;
> +
> +	for (i = 0; i < clkdm->fclk_reg_amt; i++)
> +		if (cm_read_mod_reg(clkdm->pwrdm.ptr->prcm_offs,
> +				    fclk_regs[i]) & ~clkdm->fclk_masks[i])
> +			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 c4ee076..2cde82a 100644
> --- a/arch/arm/mach-omap2/clockdomains.h
> +++ b/arch/arm/mach-omap2/clockdomains.h
> @@ -167,6 +167,7 @@ static struct clockdomain iva2_clkdm = {
>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_IVA2_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.fclk_reg_amt	= 1,
>  };
>  
>  static struct clockdomain gfx_3430es1_clkdm = {
> @@ -183,6 +184,7 @@ static struct clockdomain sgx_clkdm = {
>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>  	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_SGX_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
> +	.fclk_reg_amt	= 1,
>  };
>  
>  /*
> @@ -206,6 +208,11 @@ static struct clockdomain core_l3_34xx_clkdm = {
>  	.flags		= CLKDM_CAN_HWSUP,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_L3_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.fclk_reg_amt	= 2,
> +	.fclk_masks	= {
> +		[0] = OMAP3430_EN_UART2 | OMAP3430_EN_UART1,
> +		[1] = 0,
> +	},
>  };
>  
>  static struct clockdomain core_l4_34xx_clkdm = {
> @@ -222,6 +229,7 @@ static struct clockdomain dss_34xx_clkdm = {
>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_DSS_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.fclk_reg_amt	= 1,
>  };
>  
>  static struct clockdomain cam_clkdm = {
> @@ -230,6 +238,7 @@ static struct clockdomain cam_clkdm = {
>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_CAM_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.fclk_reg_amt	= 1,
>  };
>  
>  static struct clockdomain usbhost_clkdm = {
> @@ -238,6 +247,7 @@ static struct clockdomain usbhost_clkdm = {
>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>  	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_USBHOST_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
> +	.fclk_reg_amt	= 1,
>  };
>  
>  static struct clockdomain per_clkdm = {
> @@ -246,6 +256,10 @@ static struct clockdomain per_clkdm = {
>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_PER_MASK,
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
> +	.fclk_reg_amt	= 1,
> +	.fclk_masks	= {
> +		[0] = OMAP3430_EN_UART3,
> +	},
>  };
>  
>  /*
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index 1237717..2bef099 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -1217,6 +1217,31 @@ 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)
> +{
> +	unsigned long flags;
> +	int i;
> +	int ret = 1;
> +
> +	read_lock_irqsave(&pwrdm_rwlock, flags);
> +
> +	for (i = 0; i < PWRDM_MAX_CLKDMS; i++)
> +		if (pwrdm->pwrdm_clkdms[i] &&
> +		    !omap2_clkdm_can_idle(pwrdm->pwrdm_clkdms[i]))
> +			ret = 0;
> +
> +	read_unlock_irqrestore(&pwrdm_rwlock, flags);
> +
> +	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 eb73482..4dfb820 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_FCLK				2
> +
>  /* OMAP24XX CM_CLKSTCTRL_*.AUTOSTATE_* register bit values */
>  #define OMAP24XX_CLKSTCTRL_DISABLE_AUTO		0x0
>  #define OMAP24XX_CLKSTCTRL_ENABLE_AUTO		0x1
> @@ -83,6 +89,10 @@ struct clockdomain {
>  	/* OMAP chip types that this clockdomain is valid on */
>  	const struct omap_chip_id omap_chip;
>  
> +	/* For idle checks */
> +	u8 fclk_reg_amt;
> +	u32 fclk_masks[CLKDM_MAX_FCLK];
> +
>  	/* Usecount tracking */
>  	atomic_t usecount;
>  
> @@ -108,4 +118,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 55350d0..e6a227f 100644
> --- a/arch/arm/plat-omap/include/plat/powerdomain.h
> +++ b/arch/arm/plat-omap/include/plat/powerdomain.h
> @@ -177,6 +177,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: [PATCHv2 5/6] OMAP: Powerdomains: Add support for checking if pwrdm/clkdm can idle

[Tero.Kristo]

 

>-----Original Message-----
>From: linux-omap-owner@vger.kernel.org 
>[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of ext Kevin Hilman
>Sent: 12 January, 2010 20:57
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCHv2 5/6] OMAP: Powerdomains: Add support for 
>checking if pwrdm/clkdm can idle
>
>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 clock status in the 
>clockdomain.
>>
>> 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>
>
>This looks good by me, but needs a review from Paul to go upstream with
>the rest of his clockdomain/powerdomain updates for 2.6.34.
>
>One question... I'm wondering if it might be good to also check the
>CM_IDLEST_<pwrdm> register in the can_idle function.  Even if all
>clocks are disabled, if the idle modes of the module are not 
>set correctly,
>it may not enter the target state.

Hmm I guess you are right here. I think at least one case would be secure devices which do not have FCLK control on the non-secure kernel, but their IDLEST would indeed block state transition. I'll investigate this a bit and update the patch.


>
>Kevin
>
>> ---
>>  arch/arm/mach-omap2/clockdomain.c             |   24 
>++++++++++++++++++++++++
>>  arch/arm/mach-omap2/clockdomains.h            |   14 ++++++++++++++
>>  arch/arm/mach-omap2/powerdomain.c             |   25 
>+++++++++++++++++++++++++
>>  arch/arm/plat-omap/include/plat/clockdomain.h |   12 ++++++++++++
>>  arch/arm/plat-omap/include/plat/powerdomain.h |    1 +
>>  5 files changed, 76 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/clockdomain.c 
>b/arch/arm/mach-omap2/clockdomain.c
>> index fcd8232..9ddeb96 100644
>> --- a/arch/arm/mach-omap2/clockdomain.c
>> +++ b/arch/arm/mach-omap2/clockdomain.c
>> @@ -474,6 +474,30 @@ 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;
>> +	const int fclk_regs[] = { CM_FCLKEN, OMAP3430ES2_CM_FCLKEN3 };
>> +
>> +	if (!clkdm)
>> +		return -EINVAL;
>> +
>> +	for (i = 0; i < clkdm->fclk_reg_amt; i++)
>> +		if (cm_read_mod_reg(clkdm->pwrdm.ptr->prcm_offs,
>> +				    fclk_regs[i]) & 
>~clkdm->fclk_masks[i])
>> +			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 c4ee076..2cde82a 100644
>> --- a/arch/arm/mach-omap2/clockdomains.h
>> +++ b/arch/arm/mach-omap2/clockdomains.h
>> @@ -167,6 +167,7 @@ static struct clockdomain iva2_clkdm = {
>>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_IVA2_MASK,
>>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>> +	.fclk_reg_amt	= 1,
>>  };
>>  
>>  static struct clockdomain gfx_3430es1_clkdm = {
>> @@ -183,6 +184,7 @@ static struct clockdomain sgx_clkdm = {
>>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>>  	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_SGX_MASK,
>>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
>> +	.fclk_reg_amt	= 1,
>>  };
>>  
>>  /*
>> @@ -206,6 +208,11 @@ static struct clockdomain core_l3_34xx_clkdm = {
>>  	.flags		= CLKDM_CAN_HWSUP,
>>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_L3_MASK,
>>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>> +	.fclk_reg_amt	= 2,
>> +	.fclk_masks	= {
>> +		[0] = OMAP3430_EN_UART2 | OMAP3430_EN_UART1,
>> +		[1] = 0,
>> +	},
>>  };
>>  
>>  static struct clockdomain core_l4_34xx_clkdm = {
>> @@ -222,6 +229,7 @@ static struct clockdomain dss_34xx_clkdm = {
>>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_DSS_MASK,
>>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>> +	.fclk_reg_amt	= 1,
>>  };
>>  
>>  static struct clockdomain cam_clkdm = {
>> @@ -230,6 +238,7 @@ static struct clockdomain cam_clkdm = {
>>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_CAM_MASK,
>>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>> +	.fclk_reg_amt	= 1,
>>  };
>>  
>>  static struct clockdomain usbhost_clkdm = {
>> @@ -238,6 +247,7 @@ static struct clockdomain usbhost_clkdm = {
>>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>>  	.clktrctrl_mask = OMAP3430ES2_CLKTRCTRL_USBHOST_MASK,
>>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
>> +	.fclk_reg_amt	= 1,
>>  };
>>  
>>  static struct clockdomain per_clkdm = {
>> @@ -246,6 +256,10 @@ static struct clockdomain per_clkdm = {
>>  	.flags		= CLKDM_CAN_HWSUP_SWSUP,
>>  	.clktrctrl_mask = OMAP3430_CLKTRCTRL_PER_MASK,
>>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>> +	.fclk_reg_amt	= 1,
>> +	.fclk_masks	= {
>> +		[0] = OMAP3430_EN_UART3,
>> +	},
>>  };
>>  
>>  /*
>> diff --git a/arch/arm/mach-omap2/powerdomain.c 
>b/arch/arm/mach-omap2/powerdomain.c
>> index 1237717..2bef099 100644
>> --- a/arch/arm/mach-omap2/powerdomain.c
>> +++ b/arch/arm/mach-omap2/powerdomain.c
>> @@ -1217,6 +1217,31 @@ 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)
>> +{
>> +	unsigned long flags;
>> +	int i;
>> +	int ret = 1;
>> +
>> +	read_lock_irqsave(&pwrdm_rwlock, flags);
>> +
>> +	for (i = 0; i < PWRDM_MAX_CLKDMS; i++)
>> +		if (pwrdm->pwrdm_clkdms[i] &&
>> +		    !omap2_clkdm_can_idle(pwrdm->pwrdm_clkdms[i]))
>> +			ret = 0;
>> +
>> +	read_unlock_irqrestore(&pwrdm_rwlock, flags);
>> +
>> +	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 eb73482..4dfb820 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_FCLK				2
>> +
>>  /* OMAP24XX CM_CLKSTCTRL_*.AUTOSTATE_* register bit values */
>>  #define OMAP24XX_CLKSTCTRL_DISABLE_AUTO		0x0
>>  #define OMAP24XX_CLKSTCTRL_ENABLE_AUTO		0x1
>> @@ -83,6 +89,10 @@ struct clockdomain {
>>  	/* OMAP chip types that this clockdomain is valid on */
>>  	const struct omap_chip_id omap_chip;
>>  
>> +	/* For idle checks */
>> +	u8 fclk_reg_amt;
>> +	u32 fclk_masks[CLKDM_MAX_FCLK];
>> +
>>  	/* Usecount tracking */
>>  	atomic_t usecount;
>>  
>> @@ -108,4 +118,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 55350d0..e6a227f 100644
>> --- a/arch/arm/plat-omap/include/plat/powerdomain.h
>> +++ b/arch/arm/plat-omap/include/plat/powerdomain.h
>> @@ -177,6 +177,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
>--
>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: [PATCHv2 4/6] 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>

Looks good.

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c |   18 ++----------------
>  1 files changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 574492e..d77fc88 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -81,7 +81,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_times_vc prm_setup_times_default = {
>  	.clksetup = 0xff,
> @@ -378,7 +377,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;
> @@ -421,19 +419,10 @@ void omap_sram_idle(void)
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		omap_uart_prepare_idle(2);
>  		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();
>  	}
>  
> -	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.
> @@ -559,8 +548,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 */
> @@ -1174,7 +1161,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");
>  
>  	omap_push_sram_idle();
>  #ifdef CONFIG_SUSPEND
> -- 
> 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: [PATCHv2 3/6] OMAP3: CPUidle: Fixed support for ON / INACTIVE states

[Kevin Hilman]

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

> From: Tero Kristo <tero.kristo@nokia.com>
>
> New powerdomain code support 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>

Looks good, will queue after patch 1 resolved.

Kevin

> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |   32 ++++----------------------------
>  1 files changed, 4 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 1cfa5a6..4a81ef1 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
> @@ -141,19 +127,9 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
>  	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
>
> --
> 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: [PATCHv2 2/6] OMAP3: PM: Added support for INACTIVE and ON states for powerdomains

[Kevin Hilman]

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

> 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>

Looks good, will add to PM branch after issues with patch 1 are resolved.

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 3b62c96..574492e 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -391,6 +391,7 @@ void omap_sram_idle(void)
>  	mpu_next_state = 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;
> @@ -460,9 +461,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();
>  
> @@ -561,15 +564,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)
> @@ -617,7 +618,6 @@ int set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  	}
>  
>  	if (sleep_switch) {
> -		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>  		pwrdm_wait_transition(pwrdm);
>  		pwrdm_state_switch(pwrdm);
>  	}
> -- 
> 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: [PATCHv2 1/6] OMAP: Powerdomains: Add support for INACTIVE state on pwrdm level

[Kevin Hilman]

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

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Currently only ON, RET and OFF are supported, and ON is arguably broken as it
> allows the powerdomain to enter INACTIVE state unless idle is prevented.
> Now, pwrdm code prevents idle if ON is selected, and also adds support for
> INACTIVE. This simplifies the control needed inside sleep code.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

Hi Tero,

apologies for the long delay in reviewing this updated series.

I really like this idea as it indeed simplifies the control code
inside the idle path.  This also needs a review by Paul and should
merge via his powerdomain updates for 2.6.34.

The changelog should also describe that the powerdomain struct
now caches the next_state.

One minor comment.  I think the introduction of signed compares in
certain cases leads to possible confusion and readability problems.

I'm not sure I realy follow the need for the invalid state.  Instead
of setting next_state to -1 in pwrdm_register, why not read the
current HW value and use that as the starting value?

If the invalid state is really needed, instead of using -1 and having
to change to using signed comparisons in certain cases, it seems like
we could just define a new invalid state as zero, and move the others
up a notch.

Then, use something like this to check for a valid state:

static inline bool pwrdm_is_valid_state(struct powerdomain *pwrdm) {
       return (pwrdm->state > PWRDM_POWER_INVALID) ? true : false;
}

Kevin

> ---
>  arch/arm/mach-omap2/powerdomain.c             |   32 +++++++++++++++++++++----
>  arch/arm/mach-omap2/powerdomains34xx.h        |   26 ++++++++++----------
>  arch/arm/plat-omap/include/plat/powerdomain.h |    6 ++++-
>  3 files changed, 45 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index b6990e3..1237717 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -112,8 +112,8 @@ static struct powerdomain *_pwrdm_deps_lookup(struct powerdomain *pwrdm,
>  static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
>  {
>  
> -	int prev;
> -	int state;
> +	u8 prev;
> +	u8 state;
>  
>  	if (pwrdm == NULL)
>  		return -EINVAL;
> @@ -220,7 +220,7 @@ int pwrdm_register(struct powerdomain *pwrdm)
>  
>  	pr_debug("powerdomain: registered %s\n", pwrdm->name);
>  	ret = 0;
> -
> +	pwrdm->next_state = -1;

>  pr_unlock:
>  	write_unlock_irqrestore(&pwrdm_rwlock, flags);
>  
> @@ -701,19 +701,38 @@ int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm)
>   */
>  int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
>  {
> +	u8 prg_pwrst;
> +
>  	if (!pwrdm)
>  		return -EINVAL;
>  
> +	if (pwrdm->next_state == pwrst)
> +		return 0;
> +
>  	if (!(pwrdm->pwrsts & (1 << pwrst)))
>  		return -EINVAL;
>  
>  	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
>  		 pwrdm->name, pwrst);
>  
> +	/* INACTIVE is reserved, so we program pwrdm as ON */
> +	if (pwrst == PWRDM_POWER_INACTIVE)
> +		prg_pwrst = PWRDM_POWER_ON;
> +	else
> +		prg_pwrst = pwrst;
> +
>  	prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
> -			     (pwrst << OMAP_POWERSTATE_SHIFT),
> +			     (prg_pwrst << OMAP_POWERSTATE_SHIFT),
>  			     pwrdm->prcm_offs, PM_PWSTCTRL);
>  
> +	/* If next state is ON, prevent idle */
> +	if (pwrst == PWRDM_POWER_ON)
> +		omap2_clkdm_deny_idle(pwrdm->pwrdm_clkdms[0]);
> +	else
> +		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> +
> +	pwrdm->next_state = pwrst;
> +
>  	return 0;
>  }
>  
> @@ -730,8 +749,11 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
>  	if (!pwrdm)
>  		return -EINVAL;
>  
> +	if (pwrdm->next_state > -1)
> +		return pwrdm->next_state;
> +
>  	return prm_read_mod_bits_shift(pwrdm->prcm_offs, PM_PWSTCTRL,
> -					OMAP_POWERSTATE_MASK);
> +				       OMAP_POWERSTATE_MASK);
>  }
>  
>  /**
> diff --git a/arch/arm/mach-omap2/powerdomains34xx.h b/arch/arm/mach-omap2/powerdomains34xx.h
> index fd09b08..9eb2dc5 100644
> --- a/arch/arm/mach-omap2/powerdomains34xx.h
> +++ b/arch/arm/mach-omap2/powerdomains34xx.h
> @@ -165,7 +165,7 @@ static struct powerdomain iva2_pwrdm = {
>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>  	.dep_bit	  = OMAP3430_PM_WKDEP_MPU_EN_IVA2_SHIFT,
>  	.wkdep_srcs	  = iva2_wkdeps,
> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>  	.pwrsts_logic_ret = PWRSTS_OFF_RET,
>  	.banks		  = 4,
>  	.pwrsts_mem_ret	  = {
> @@ -188,7 +188,7 @@ static struct powerdomain mpu_34xx_pwrdm = {
>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>  	.dep_bit	  = OMAP3430_EN_MPU_SHIFT,
>  	.wkdep_srcs	  = mpu_34xx_wkdeps,
> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>  	.pwrsts_logic_ret = PWRSTS_OFF_RET,
>  	.banks		  = 1,
>  	.pwrsts_mem_ret	  = {
> @@ -206,7 +206,7 @@ static struct powerdomain core_34xx_pre_es3_1_pwrdm = {
>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES1 |
>  					   CHIP_IS_OMAP3430ES2 |
>  					   CHIP_IS_OMAP3430ES3_0),
> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>  	.dep_bit	  = OMAP3430_EN_CORE_SHIFT,
>  	.banks		  = 2,
>  	.pwrsts_mem_ret	  = {
> @@ -214,8 +214,8 @@ static struct powerdomain core_34xx_pre_es3_1_pwrdm = {
>  		[1] = PWRSTS_OFF_RET,	 /* MEM2RETSTATE */
>  	},
>  	.pwrsts_mem_on	  = {
> -		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
> -		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
> +		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
> +		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
>  	},
>  };
>  
> @@ -224,7 +224,7 @@ static struct powerdomain core_34xx_es3_1_pwrdm = {
>  	.name		  = "core_pwrdm",
>  	.prcm_offs	  = CORE_MOD,
>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES3_1),
> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>  	.dep_bit	  = OMAP3430_EN_CORE_SHIFT,
>  	.flags		  = PWRDM_HAS_HDWR_SAR, /* for USBTLL only */
>  	.banks		  = 2,
> @@ -233,8 +233,8 @@ static struct powerdomain core_34xx_es3_1_pwrdm = {
>  		[1] = PWRSTS_OFF_RET,	 /* MEM2RETSTATE */
>  	},
>  	.pwrsts_mem_on	  = {
> -		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
> -		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
> +		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
> +		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
>  	},
>  };
>  
> @@ -246,7 +246,7 @@ static struct powerdomain dss_pwrdm = {
>  	.dep_bit	  = OMAP3430_PM_WKDEP_MPU_EN_DSS_SHIFT,
>  	.wkdep_srcs	  = cam_dss_wkdeps,
>  	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>  	.pwrsts_logic_ret = PWRDM_POWER_RET,
>  	.banks		  = 1,
>  	.pwrsts_mem_ret	  = {
> @@ -286,7 +286,7 @@ static struct powerdomain cam_pwrdm = {
>  	.prcm_offs	  = OMAP3430_CAM_MOD,
>  	.wkdep_srcs	  = cam_dss_wkdeps,
>  	.sleepdep_srcs	  = cam_gfx_sleepdeps,
> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>  	.pwrsts_logic_ret = PWRDM_POWER_RET,
>  	.banks		  = 1,
>  	.pwrsts_mem_ret	  = {
> @@ -304,7 +304,7 @@ static struct powerdomain per_pwrdm = {
>  	.dep_bit	  = OMAP3430_EN_PER_SHIFT,
>  	.wkdep_srcs	  = per_usbhost_wkdeps,
>  	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>  	.pwrsts_logic_ret = PWRSTS_OFF_RET,
>  	.banks		  = 1,
>  	.pwrsts_mem_ret	  = {
> @@ -326,7 +326,7 @@ static struct powerdomain neon_pwrdm = {
>  	.prcm_offs	  = OMAP3430_NEON_MOD,
>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>  	.wkdep_srcs	  = neon_wkdeps,
> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>  	.pwrsts_logic_ret = PWRDM_POWER_RET,
>  };
>  
> @@ -336,7 +336,7 @@ static struct powerdomain usbhost_pwrdm = {
>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
>  	.wkdep_srcs	  = per_usbhost_wkdeps,
>  	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>  	.pwrsts_logic_ret = PWRDM_POWER_RET,
>  	/*
>  	 * REVISIT: Enabling usb host save and restore mechanism seems to
> diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h b/arch/arm/plat-omap/include/plat/powerdomain.h
> index 3d45ee1..55350d0 100644
> --- a/arch/arm/plat-omap/include/plat/powerdomain.h
> +++ b/arch/arm/plat-omap/include/plat/powerdomain.h
> @@ -37,6 +37,9 @@
>  
>  #define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON))
>  
> +#define PWRSTS_OFF_RET_INA_ON	(PWRSTS_OFF_RET_ON | \
> +				 (1 << PWRDM_POWER_INACTIVE))
> +
>  
>  /* Powerdomain flags */
>  #define PWRDM_HAS_HDWR_SAR	(1 << 0) /* hardware save-and-restore support */
> @@ -117,7 +120,8 @@ struct powerdomain {
>  
>  	struct list_head node;
>  
> -	int state;
> +	u8 state;
> +	s8 next_state;
>  	unsigned state_counter[4];
>  
>  #ifdef CONFIG_PM_DEBUG
> -- 
> 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: [PATCHv2 1/6] OMAP: Powerdomains: Add support for INACTIVE state on pwrdm level

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 12 January, 2010 20:25
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCHv2 1/6] OMAP: Powerdomains: Add support for 
>INACTIVE state on pwrdm level
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> Currently only ON, RET and OFF are supported, and ON is 
>arguably broken as it
>> allows the powerdomain to enter INACTIVE state unless idle 
>is prevented.
>> Now, pwrdm code prevents idle if ON is selected, and also 
>adds support for
>> INACTIVE. This simplifies the control needed inside sleep code.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>
>Hi Tero,
>
>apologies for the long delay in reviewing this updated series.

No problem, I have actually been on a holiday myself so I did not really miss review comments that much. Thanks for taking the time to look at this.

>I really like this idea as it indeed simplifies the control code
>inside the idle path.  This also needs a review by Paul and should
>merge via his powerdomain updates for 2.6.34.
>
>The changelog should also describe that the powerdomain struct
>now caches the next_state.

Yeah, I can add this comment.

>One minor comment.  I think the introduction of signed compares in
>certain cases leads to possible confusion and readability problems.
>
>I'm not sure I realy follow the need for the invalid state.  Instead
>of setting next_state to -1 in pwrdm_register, why not read the
>current HW value and use that as the starting value?

I guess this invalid stuff comes from my old lazy initialization habits. :) But yes, I can change this into a format where we just read the current value from the register.

>If the invalid state is really needed, instead of using -1 and having
>to change to using signed comparisons in certain cases, it seems like
>we could just define a new invalid state as zero, and move the others
>up a notch.

This is probably not good, as it would break the direct SW to HW value mapping, so I will go with the previous one.

>
>Then, use something like this to check for a valid state:
>
>static inline bool pwrdm_is_valid_state(struct powerdomain *pwrdm) {
>       return (pwrdm->state > PWRDM_POWER_INVALID) ? true : false;
>}
>
>Kevin
>
>> ---
>>  arch/arm/mach-omap2/powerdomain.c             |   32 
>+++++++++++++++++++++----
>>  arch/arm/mach-omap2/powerdomains34xx.h        |   26 
>++++++++++----------
>>  arch/arm/plat-omap/include/plat/powerdomain.h |    6 ++++-
>>  3 files changed, 45 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/powerdomain.c 
>b/arch/arm/mach-omap2/powerdomain.c
>> index b6990e3..1237717 100644
>> --- a/arch/arm/mach-omap2/powerdomain.c
>> +++ b/arch/arm/mach-omap2/powerdomain.c
>> @@ -112,8 +112,8 @@ static struct powerdomain 
>*_pwrdm_deps_lookup(struct powerdomain *pwrdm,
>>  static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
>>  {
>>  
>> -	int prev;
>> -	int state;
>> +	u8 prev;
>> +	u8 state;
>>  
>>  	if (pwrdm == NULL)
>>  		return -EINVAL;
>> @@ -220,7 +220,7 @@ int pwrdm_register(struct powerdomain *pwrdm)
>>  
>>  	pr_debug("powerdomain: registered %s\n", pwrdm->name);
>>  	ret = 0;
>> -
>> +	pwrdm->next_state = -1;
>
>>  pr_unlock:
>>  	write_unlock_irqrestore(&pwrdm_rwlock, flags);
>>  
>> @@ -701,19 +701,38 @@ int pwrdm_get_mem_bank_count(struct 
>powerdomain *pwrdm)
>>   */
>>  int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
>>  {
>> +	u8 prg_pwrst;
>> +
>>  	if (!pwrdm)
>>  		return -EINVAL;
>>  
>> +	if (pwrdm->next_state == pwrst)
>> +		return 0;
>> +
>>  	if (!(pwrdm->pwrsts & (1 << pwrst)))
>>  		return -EINVAL;
>>  
>>  	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
>>  		 pwrdm->name, pwrst);
>>  
>> +	/* INACTIVE is reserved, so we program pwrdm as ON */
>> +	if (pwrst == PWRDM_POWER_INACTIVE)
>> +		prg_pwrst = PWRDM_POWER_ON;
>> +	else
>> +		prg_pwrst = pwrst;
>> +
>>  	prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
>> -			     (pwrst << OMAP_POWERSTATE_SHIFT),
>> +			     (prg_pwrst << OMAP_POWERSTATE_SHIFT),
>>  			     pwrdm->prcm_offs, PM_PWSTCTRL);
>>  
>> +	/* If next state is ON, prevent idle */
>> +	if (pwrst == PWRDM_POWER_ON)
>> +		omap2_clkdm_deny_idle(pwrdm->pwrdm_clkdms[0]);
>> +	else
>> +		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>> +
>> +	pwrdm->next_state = pwrst;
>> +
>>  	return 0;
>>  }
>>  
>> @@ -730,8 +749,11 @@ int pwrdm_read_next_pwrst(struct 
>powerdomain *pwrdm)
>>  	if (!pwrdm)
>>  		return -EINVAL;
>>  
>> +	if (pwrdm->next_state > -1)
>> +		return pwrdm->next_state;
>> +
>>  	return prm_read_mod_bits_shift(pwrdm->prcm_offs, PM_PWSTCTRL,
>> -					OMAP_POWERSTATE_MASK);
>> +				       OMAP_POWERSTATE_MASK);
>>  }
>>  
>>  /**
>> diff --git a/arch/arm/mach-omap2/powerdomains34xx.h 
>b/arch/arm/mach-omap2/powerdomains34xx.h
>> index fd09b08..9eb2dc5 100644
>> --- a/arch/arm/mach-omap2/powerdomains34xx.h
>> +++ b/arch/arm/mach-omap2/powerdomains34xx.h
>> @@ -165,7 +165,7 @@ static struct powerdomain iva2_pwrdm = {
>>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>>  	.dep_bit	  = OMAP3430_PM_WKDEP_MPU_EN_IVA2_SHIFT,
>>  	.wkdep_srcs	  = iva2_wkdeps,
>> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
>> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>>  	.pwrsts_logic_ret = PWRSTS_OFF_RET,
>>  	.banks		  = 4,
>>  	.pwrsts_mem_ret	  = {
>> @@ -188,7 +188,7 @@ static struct powerdomain mpu_34xx_pwrdm = {
>>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>>  	.dep_bit	  = OMAP3430_EN_MPU_SHIFT,
>>  	.wkdep_srcs	  = mpu_34xx_wkdeps,
>> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
>> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>>  	.pwrsts_logic_ret = PWRSTS_OFF_RET,
>>  	.banks		  = 1,
>>  	.pwrsts_mem_ret	  = {
>> @@ -206,7 +206,7 @@ static struct powerdomain 
>core_34xx_pre_es3_1_pwrdm = {
>>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES1 |
>>  					   CHIP_IS_OMAP3430ES2 |
>>  					   CHIP_IS_OMAP3430ES3_0),
>> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
>> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>>  	.dep_bit	  = OMAP3430_EN_CORE_SHIFT,
>>  	.banks		  = 2,
>>  	.pwrsts_mem_ret	  = {
>> @@ -214,8 +214,8 @@ static struct powerdomain 
>core_34xx_pre_es3_1_pwrdm = {
>>  		[1] = PWRSTS_OFF_RET,	 /* MEM2RETSTATE */
>>  	},
>>  	.pwrsts_mem_on	  = {
>> -		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
>> -		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
>> +		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
>> +		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
>>  	},
>>  };
>>  
>> @@ -224,7 +224,7 @@ static struct powerdomain 
>core_34xx_es3_1_pwrdm = {
>>  	.name		  = "core_pwrdm",
>>  	.prcm_offs	  = CORE_MOD,
>>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES3_1),
>> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
>> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>>  	.dep_bit	  = OMAP3430_EN_CORE_SHIFT,
>>  	.flags		  = PWRDM_HAS_HDWR_SAR, /* for USBTLL only */
>>  	.banks		  = 2,
>> @@ -233,8 +233,8 @@ static struct powerdomain 
>core_34xx_es3_1_pwrdm = {
>>  		[1] = PWRSTS_OFF_RET,	 /* MEM2RETSTATE */
>>  	},
>>  	.pwrsts_mem_on	  = {
>> -		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
>> -		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
>> +		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
>> +		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
>>  	},
>>  };
>>  
>> @@ -246,7 +246,7 @@ static struct powerdomain dss_pwrdm = {
>>  	.dep_bit	  = OMAP3430_PM_WKDEP_MPU_EN_DSS_SHIFT,
>>  	.wkdep_srcs	  = cam_dss_wkdeps,
>>  	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
>> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
>> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>>  	.pwrsts_logic_ret = PWRDM_POWER_RET,
>>  	.banks		  = 1,
>>  	.pwrsts_mem_ret	  = {
>> @@ -286,7 +286,7 @@ static struct powerdomain cam_pwrdm = {
>>  	.prcm_offs	  = OMAP3430_CAM_MOD,
>>  	.wkdep_srcs	  = cam_dss_wkdeps,
>>  	.sleepdep_srcs	  = cam_gfx_sleepdeps,
>> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
>> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>>  	.pwrsts_logic_ret = PWRDM_POWER_RET,
>>  	.banks		  = 1,
>>  	.pwrsts_mem_ret	  = {
>> @@ -304,7 +304,7 @@ static struct powerdomain per_pwrdm = {
>>  	.dep_bit	  = OMAP3430_EN_PER_SHIFT,
>>  	.wkdep_srcs	  = per_usbhost_wkdeps,
>>  	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
>> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
>> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>>  	.pwrsts_logic_ret = PWRSTS_OFF_RET,
>>  	.banks		  = 1,
>>  	.pwrsts_mem_ret	  = {
>> @@ -326,7 +326,7 @@ static struct powerdomain neon_pwrdm = {
>>  	.prcm_offs	  = OMAP3430_NEON_MOD,
>>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>>  	.wkdep_srcs	  = neon_wkdeps,
>> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
>> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>>  	.pwrsts_logic_ret = PWRDM_POWER_RET,
>>  };
>>  
>> @@ -336,7 +336,7 @@ static struct powerdomain usbhost_pwrdm = {
>>  	.omap_chip	  = OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
>>  	.wkdep_srcs	  = per_usbhost_wkdeps,
>>  	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
>> -	.pwrsts		  = PWRSTS_OFF_RET_ON,
>> +	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
>>  	.pwrsts_logic_ret = PWRDM_POWER_RET,
>>  	/*
>>  	 * REVISIT: Enabling usb host save and restore 
>mechanism seems to
>> diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h 
>b/arch/arm/plat-omap/include/plat/powerdomain.h
>> index 3d45ee1..55350d0 100644
>> --- a/arch/arm/plat-omap/include/plat/powerdomain.h
>> +++ b/arch/arm/plat-omap/include/plat/powerdomain.h
>> @@ -37,6 +37,9 @@
>>  
>>  #define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON))
>>  
>> +#define PWRSTS_OFF_RET_INA_ON	(PWRSTS_OFF_RET_ON | \
>> +				 (1 << PWRDM_POWER_INACTIVE))
>> +
>>  
>>  /* Powerdomain flags */
>>  #define PWRDM_HAS_HDWR_SAR	(1 << 0) /* hardware 
>save-and-restore support */
>> @@ -117,7 +120,8 @@ struct powerdomain {
>>  
>>  	struct list_head node;
>>  
>> -	int state;
>> +	u8 state;
>> +	s8 next_state;
>>  	unsigned state_counter[4];
>>  
>>  #ifdef CONFIG_PM_DEBUG
>> -- 
>> 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: [PATCHv2 0/6] Idle status patches revisited

[Kevin Hilman]

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

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Improvements compared to previous set:
>
> - Fixed wrong pwrdm state check on IO chain arming on patch 2
> - Improved changelog on patch 2 & 4
> - Moved FCLK checks inside clockdomain code from powerdomain code in patch 5
> - Some style changes on patch 6
>
> Still, I believe there is no simple way of implementing the idle status
> check by using the usecounts inside clock framework. It would probably be
> possible by adding a new usecount variable that would be updated according
> to slightly different rules, but this also looks bit complicated compared
> to the need.

Tero,

Thanks for this series and the descriptive changelogs and comments.
Very nice.

Along with the comments on individual patches, this should probably
also be rebased on current PM branch.

Also, Paul has lots of other clockdomain/powerdomain changes queued up
for 2.6.34, so it might tbe helpful to Paul if those were based on his
upcoming changes.

Thanks,

Kevin