[PATCH 00/17] Misc PM patches

[PATCH 00/17] Misc PM patches

[Tero Kristo]

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

This set contains miscellaneous PM related fixes. Applies on top of PM branch.
Any comments welcome.

-Tero

[PATCH 01/17] PM: fix suspend control for IVA2

[Tero Kristo]

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

IVA2 controls its target power state individually, thus suspend should not
touch IVA2. Without this patch DSP suspend always fails.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Acked-by: Ameya Palande <ameya.palande@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index a3e3729..5e2ef63 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -81,7 +81,7 @@ 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 powerdomain *cam_pwrdm, *iva2_pwrdm;
 
 static struct prm_setup_vc prm_setup = {
 	.clksetup = 0xff,
@@ -676,6 +676,12 @@ static int omap3_pm_suspend(void)
 		pwrst->saved_state = pwrdm_read_next_pwrst(pwrst->pwrdm);
 	/* Set ones wanted by suspend */
 	list_for_each_entry(pwrst, &pwrst_list, node) {
+		/* Special handling for IVA2, just use current sleep state */
+		if (pwrst->pwrdm == iva2_pwrdm) {
+			state = pwrdm_read_pwrst(pwrst->pwrdm);
+			if (state < PWRDM_POWER_ON)
+				pwrst->next_state = state;
+		}
 		if (set_pwrdm_state(pwrst->pwrdm, pwrst->next_state))
 			goto restore;
 		if (pwrdm_clear_all_prev_pwrst(pwrst->pwrdm))
@@ -1183,6 +1189,7 @@ static int __init omap3_pm_init(void)
 	per_pwrdm = pwrdm_lookup("per_pwrdm");
 	core_pwrdm = pwrdm_lookup("core_pwrdm");
 	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
+	iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
 
 	omap_push_sram_idle();
 #ifdef CONFIG_SUSPEND
-- 
1.5.4.3

[PATCH 02/17] OMAP3: PM: Dynamic check for CORE target state

[Tero Kristo]

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

Sleep code will now check if core will actually enter power save state or
not. This is needed so that PER domain does not enter retention / off while
core stays on and thus preventing I/O pad wakeups.

This also optimizes core context save and restore logic, it will skip
saving / restoring core context when this is requested but not needed.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 arch/arm/mach-omap2/cm.h     |    1 +
 arch/arm/mach-omap2/pm34xx.c |   46 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h
index a2fcfcc..715ab14 100644
--- a/arch/arm/mach-omap2/cm.h
+++ b/arch/arm/mach-omap2/cm.h
@@ -76,6 +76,7 @@
 #define OMAP3430ES2_CM_CLKEN2				0x0004
 #define OMAP3430ES2_CM_FCLKEN3				0x0008
 #define OMAP3430_CM_IDLEST_PLL				CM_IDLEST2
+#define OMAP3430_CM_IDLEST3				0x0028
 #define OMAP3430_CM_AUTOIDLE_PLL			CM_AUTOIDLE2
 #define OMAP3430ES2_CM_AUTOIDLE2_PLL			CM_AUTOIDLE2
 #define OMAP3430_CM_CLKSEL1				CM_CLKSEL
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 5e2ef63..e8704a6 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -64,6 +64,28 @@ u32 sleep_while_idle;
 u32 wakeup_timer_seconds;
 u32 voltage_off_while_idle;
 
+/* IDLEST bitmasks for core status checks */
+#define CORE_IDLEST1_ALL		(\
+		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_HSOTGUSB_STDBY_MASK|\
+		OMAP3430ES2_ST_SSI_IDLE_MASK|OMAP3430_ST_SDMA_MASK|\
+		OMAP3430_ST_SSI_STDBY_MASK|OMAP3430_ST_D2D_MASK)
+#define CORE_IDLEST2_ALL		(\
+		OMAP3430_ST_PKA_MASK|OMAP3430_ST_AES1_MASK|\
+		OMAP3430_ST_RNG_MASK|OMAP3430_ST_SHA11_MASK|\
+		OMAP3430_ST_DES1_MASK)
+#define CORE_IDLEST3_ALL		(\
+		OMAP3430ES2_ST_USBTLL_MASK|OMAP3430ES2_ST_CPEFUSE_MASK)
+
 struct power_state {
 	struct powerdomain *pwrdm;
 	u32 next_state;
@@ -408,6 +430,7 @@ void omap_sram_idle(void)
 	int core_prev_state, per_prev_state;
 	u32 sdrc_pwr = 0;
 	int per_state_modified = 0;
+	int core_saved_state = PWRDM_POWER_ON;
 
 	if (!_omap_sram_idle)
 		return;
@@ -439,9 +462,28 @@ void omap_sram_idle(void)
 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
 
+	/*
+	 * Check whether core will enter idle or not. This is needed
+	 * because I/O pad wakeup will fail if core stays on and PER
+	 * enters off. This will also prevent unnecessary core context
+	 * save / restore.
+	 */
+	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
+	if (core_next_state < PWRDM_POWER_ON) {
+		core_saved_state = core_next_state;
+		if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & CORE_IDLEST1_ALL)
+				!= CORE_IDLEST1_ALL ||
+		    (cm_read_mod_reg(CORE_MOD, CM_IDLEST2) & CORE_IDLEST2_ALL)
+				!= CORE_IDLEST2_ALL ||
+		    (cm_read_mod_reg(CORE_MOD, OMAP3430_CM_IDLEST3) &
+				CORE_IDLEST3_ALL) != CORE_IDLEST3_ALL) {
+			core_next_state = PWRDM_POWER_ON;
+			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_ON);
+		}
+	}
+
 	/* PER */
 	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
-	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
 	if (per_next_state < PWRDM_POWER_ON) {
 		omap_uart_prepare_idle(2);
 		omap2_gpio_prepare_for_idle(per_next_state);
@@ -539,6 +581,8 @@ void omap_sram_idle(void)
 		enable_smartreflex(SR1);
 		enable_smartreflex(SR2);
 	}
+	if (core_saved_state != core_next_state)
+		pwrdm_set_next_pwrst(core_pwrdm, core_saved_state);
 
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
-- 
1.5.4.3

[PATCH 03/17] PM: Block CORE off when DSS is active

[Tero Kristo]

From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>

If CORE power domain is allowed to reach off state
while display is active, the display will go blank.
Fixed by adding a check in idle loop.

Signed-off-by: Kalle Jokiniemi <ext-kalle.jokiniemi@nokia.com>
Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index e8704a6..52a3f2b 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -103,7 +103,7 @@ 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, *iva2_pwrdm;
+static struct powerdomain *cam_pwrdm, *iva2_pwrdm, *dss_pwrdm;
 
 static struct prm_setup_vc prm_setup = {
 	.clksetup = 0xff,
@@ -427,6 +427,7 @@ void omap_sram_idle(void)
 	int mpu_next_state = PWRDM_POWER_ON;
 	int per_next_state = PWRDM_POWER_ON;
 	int core_next_state = PWRDM_POWER_ON;
+	int dss_state = PWRDM_POWER_ON;
 	int core_prev_state, per_prev_state;
 	u32 sdrc_pwr = 0;
 	int per_state_modified = 0;
@@ -469,6 +470,7 @@ void omap_sram_idle(void)
 	 * save / restore.
 	 */
 	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
+	dss_state = pwrdm_read_pwrst(dss_pwrdm);
 	if (core_next_state < PWRDM_POWER_ON) {
 		core_saved_state = core_next_state;
 		if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & CORE_IDLEST1_ALL)
@@ -479,6 +481,10 @@ void omap_sram_idle(void)
 				CORE_IDLEST3_ALL) != CORE_IDLEST3_ALL) {
 			core_next_state = PWRDM_POWER_ON;
 			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_ON);
+		} else if (core_next_state == PWRDM_POWER_OFF &&
+				 dss_state == PWRDM_POWER_ON) {
+			core_next_state = PWRDM_POWER_RET;
+			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET);
 		}
 	}
 
@@ -1234,6 +1240,7 @@ static int __init omap3_pm_init(void)
 	core_pwrdm = pwrdm_lookup("core_pwrdm");
 	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
 	iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
+	dss_pwrdm = pwrdm_lookup("dss_pwrdm");
 
 	omap_push_sram_idle();
 #ifdef CONFIG_SUSPEND
-- 
1.5.4.3

[PATCH 04/17] OMAP3: PM: Do not enable IO wake-up on boot

[Tero Kristo]

From: Jouni Hogander <jouni.hogander@nokia.com>

IO wake-ups are enabled/disabled in idle loop. No need to enable them on boot.

Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 52a3f2b..4addc41 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1027,9 +1027,8 @@ static void __init prcm_setup_regs(void)
 			     OMAP3_PRM_CLKSRC_CTRL_OFFSET);
 
 	/* setup wakup source */
-	prm_write_mod_reg(OMAP3430_EN_IO | OMAP3430_EN_GPIO1 |
-			  OMAP3430_EN_GPT1 | OMAP3430_EN_GPT12,
-			  WKUP_MOD, PM_WKEN);
+	prm_write_mod_reg(OMAP3430_EN_GPIO1 | OMAP3430_EN_GPT1 |
+			  OMAP3430_EN_GPT12, WKUP_MOD, PM_WKEN);
 	/* No need to write EN_IO, that is always enabled */
 	prm_write_mod_reg(OMAP3430_EN_GPIO1 | OMAP3430_EN_GPT1 |
 			  OMAP3430_EN_GPT12,
-- 
1.5.4.3

[PATCH 05/17] OMAP:PM: Precedence fix

[Tero Kristo]

From: Carlos Chinea <carlos.chinea@nokia.com>

Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 4addc41..588ab79 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -225,8 +225,8 @@ static void omap3_core_save_context(void)
 	control_padconf_off |= START_PADCONF_SAVE;
 	omap_ctrl_writel(control_padconf_off, OMAP343X_CONTROL_PADCONF_OFF);
 	/* wait for the save to complete */
-	while (!omap_ctrl_readl(OMAP343X_CONTROL_GENERAL_PURPOSE_STATUS)
-			& PADCONF_SAVE_DONE)
+	while (!(omap_ctrl_readl(OMAP343X_CONTROL_GENERAL_PURPOSE_STATUS)
+			& PADCONF_SAVE_DONE))
 		;
 	/* Save the Interrupt controller context */
 	omap3_intc_save_context();
-- 
1.5.4.3

[PATCH 06/17] OMAP3: PM: Added next state check for IVA2, USB and PER into idle loop

[Tero Kristo]

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

This is needed to prevent core from entering off mode if one of the IVA2,
USB or PER powerdomains are going to stay at least in retention state.
If this is not done, a powerdomain waking from RET may access core
domain which has just been reset.

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

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 588ab79..3e5ae14 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -85,6 +85,13 @@ u32 voltage_off_while_idle;
 		OMAP3430_ST_DES1_MASK)
 #define CORE_IDLEST3_ALL		(\
 		OMAP3430ES2_ST_USBTLL_MASK|OMAP3430ES2_ST_CPEFUSE_MASK)
+#define PER_IDLEST_ALL			(\
+		OMAP3430_ST_WDT3_MASK|OMAP3430_ST_MCBSP4_MASK|\
+		OMAP3430_ST_MCBSP3_MASK|OMAP3430_ST_MCBSP2_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)
 
 struct power_state {
 	struct powerdomain *pwrdm;
@@ -103,7 +110,7 @@ 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, *iva2_pwrdm, *dss_pwrdm;
+static struct powerdomain *cam_pwrdm, *iva2_pwrdm, *dss_pwrdm, *usb_pwrdm;
 
 static struct prm_setup_vc prm_setup = {
 	.clksetup = 0xff,
@@ -428,6 +435,8 @@ void omap_sram_idle(void)
 	int per_next_state = PWRDM_POWER_ON;
 	int core_next_state = PWRDM_POWER_ON;
 	int dss_state = PWRDM_POWER_ON;
+	int iva2_state = PWRDM_POWER_ON;
+	int usb_state = PWRDM_POWER_ON;
 	int core_prev_state, per_prev_state;
 	u32 sdrc_pwr = 0;
 	int per_state_modified = 0;
@@ -463,14 +472,28 @@ void omap_sram_idle(void)
 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
 
+	/* Get powerdomain state data */
+	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
+	dss_state = pwrdm_read_pwrst(dss_pwrdm);
+	iva2_state = pwrdm_read_pwrst(iva2_pwrdm);
+	usb_state = pwrdm_read_pwrst(usb_pwrdm);
+	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
+
+	/* Check if PER domain can enter OFF or not */
+	if (per_next_state == PWRDM_POWER_OFF) {
+		if ((cm_read_mod_reg(OMAP3430_PER_MOD, CM_IDLEST) &
+				PER_IDLEST_ALL) != PER_IDLEST_ALL) {
+			per_next_state = PWRDM_POWER_RET;
+			pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
+			per_state_modified = 1;
+		}
+	}
 	/*
 	 * Check whether core will enter idle or not. This is needed
 	 * because I/O pad wakeup will fail if core stays on and PER
 	 * enters off. This will also prevent unnecessary core context
 	 * save / restore.
 	 */
-	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
-	dss_state = pwrdm_read_pwrst(dss_pwrdm);
 	if (core_next_state < PWRDM_POWER_ON) {
 		core_saved_state = core_next_state;
 		if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & CORE_IDLEST1_ALL)
@@ -482,14 +505,16 @@ void omap_sram_idle(void)
 			core_next_state = PWRDM_POWER_ON;
 			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_ON);
 		} else if (core_next_state == PWRDM_POWER_OFF &&
-				 dss_state == PWRDM_POWER_ON) {
+				 (dss_state == PWRDM_POWER_ON ||
+				  iva2_state >= PWRDM_POWER_RET ||
+				  usb_state >= PWRDM_POWER_RET ||
+				  per_next_state >= PWRDM_POWER_RET)) {
 			core_next_state = PWRDM_POWER_RET;
 			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET);
 		}
 	}
 
 	/* PER */
-	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
 	if (per_next_state < PWRDM_POWER_ON) {
 		omap_uart_prepare_idle(2);
 		omap2_gpio_prepare_for_idle(per_next_state);
@@ -1240,6 +1265,7 @@ static int __init omap3_pm_init(void)
 	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
 	iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
 	dss_pwrdm = pwrdm_lookup("dss_pwrdm");
+	usb_pwrdm = pwrdm_lookup("usbhost_pwrdm");
 
 	omap_push_sram_idle();
 #ifdef CONFIG_SUSPEND
-- 
1.5.4.3

[PATCH 07/17] OMAP3: Disable Smartreflex before pwrdm enters RET

[Tero Kristo]

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

Smartreflex for the corresponding powerdomain (MPU/CORE) must be disabled
before the domain enters retention, otherwise the device may hang. This is
caused by overlapping smartreflex / auto retention command on the voltage
channel resulting in incorrect voltage.

This fix has been confirmed from TI.

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

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 3e5ae14..5854fa7 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -531,11 +531,17 @@ void omap_sram_idle(void)
 	if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
 		omap2_clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 
-	/* CORE */
-	if (core_next_state < PWRDM_POWER_ON) {
-		/* Disable smartreflex before entering WFI */
+	/*
+	 * Disable smartreflex before entering WFI.
+	 * Only needed if we are going to enter retention or off.
+	 */
+	if (mpu_next_state <= PWRDM_POWER_RET)
 		disable_smartreflex(SR1);
+	if (core_next_state <= PWRDM_POWER_RET)
 		disable_smartreflex(SR2);
+
+	/* CORE */
+	if (core_next_state < PWRDM_POWER_ON) {
 		omap_uart_prepare_idle(0);
 		omap_uart_prepare_idle(1);
 		if (core_next_state == PWRDM_POWER_OFF) {
@@ -608,10 +614,17 @@ void omap_sram_idle(void)
 			prm_clear_mod_reg_bits(OMAP3430_AUTO_RET,
 						OMAP3430_GR_MOD,
 						OMAP3_PRM_VOLTCTRL_OFFSET);
-		/* Enable smartreflex after WFI */
+	}
+
+	/*
+	 * Enable smartreflex after WFI. Only needed if we
+	 * entered retention or off.
+	 */
+	if (mpu_next_state <= PWRDM_POWER_RET)
 		enable_smartreflex(SR1);
+	if (core_next_state <= PWRDM_POWER_RET)
 		enable_smartreflex(SR2);
-	}
+
 	if (core_saved_state != core_next_state)
 		pwrdm_set_next_pwrst(core_pwrdm, core_saved_state);
 
-- 
1.5.4.3

[PATCH 08/17] OMAP2/3: GPTIMER: Clear pending interrupts when entering suspend

[Tero Kristo]

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

OMAP GP timers keep running for a few cycles after they are stopped,
which can cause the timer to expire and generate an interrupt. The pending
interrupt will prevent OMAP from entering suspend, thus we ack it manually.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/timer-gp.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/timer-gp.c b/arch/arm/mach-omap2/timer-gp.c
index 9c056ff..c9d47bb 100644
--- a/arch/arm/mach-omap2/timer-gp.c
+++ b/arch/arm/mach-omap2/timer-gp.c
@@ -92,9 +92,21 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode mode,
 	case CLOCK_EVT_MODE_ONESHOT:
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
-	case CLOCK_EVT_MODE_SHUTDOWN:
 	case CLOCK_EVT_MODE_RESUME:
 		break;
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		/*
+		 * Wait for min period x 2 to make sure that timer is
+		 * stopped
+		 */
+		udelay(evt->min_delta_ns / 500);
+		/*
+		 * Clear possibly pending interrupt, this will occasionally
+		 * generate spurious timer IRQs during suspend but this
+		 * is okay, as another option is not to enter suspend at all
+		 */
+		omap_dm_timer_write_status(gptimer, OMAP_TIMER_INT_OVERFLOW);
+		break;
 	}
 }
 
-- 
1.5.4.3

[PATCH 09/17] OMAP3: PM: Ack pending interrupts before entering suspend

[Tero Kristo]

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

Suspending drivers may still generate interrupts just before their suspend is
completed. Any pending interrupts here will prevent sleep.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/irq.c              |    2 +-
 arch/arm/mach-omap2/pm34xx.c           |    2 ++
 arch/arm/plat-omap/include/mach/irqs.h |    1 +
 3 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index aceedd8..4ed05e9 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -101,7 +101,7 @@ static int omap_check_spurious(unsigned int irq)
 }
 
 /* XXX: FIQ and additional INTC support (only MPU at the moment) */
-static void omap_ack_irq(unsigned int irq)
+void omap_ack_irq(unsigned int irq)
 {
 	intc_bank_write_reg(0x1, &irq_banks[0], INTC_CONTROL);
 }
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 5854fa7..6a41811 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -778,6 +778,8 @@ static int omap3_pm_suspend(void)
 
 	omap_uart_prepare_suspend();
 
+	/* Ack pending IRQs, as a pending IRQ will cause the suspend to fail */
+	omap_ack_irq(0);
 	regset_save_on_suspend = 1;
 	omap_sram_idle();
 	regset_save_on_suspend = 0;
diff --git a/arch/arm/plat-omap/include/mach/irqs.h b/arch/arm/plat-omap/include/mach/irqs.h
index 2473910..d56be1c 100644
--- a/arch/arm/plat-omap/include/mach/irqs.h
+++ b/arch/arm/plat-omap/include/mach/irqs.h
@@ -483,6 +483,7 @@
 #ifndef __ASSEMBLY__
 extern void omap_init_irq(void);
 extern int omap_irq_pending(void);
+extern void omap_ack_irq(unsigned int irq);
 void omap3_intc_save_context(void);
 void omap3_intc_restore_context(void);
 #endif
-- 
1.5.4.3

[PATCH 10/17] OMAP3: PM: Enable system control module autoidle

[Tero Kristo]

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

This saves some power.

Signed-off-by: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 6a41811..47f5738 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1039,6 +1039,8 @@ static void __init prcm_setup_regs(void)
 			CM_AUTOIDLE);
 	}
 
+	omap_ctrl_writel(OMAP3430_AUTOIDLE, OMAP2_CONTROL_SYSCONFIG);
+
 	/*
 	 * Set all plls to autoidle. This is needed until autoidle is
 	 * enabled by clockfw
-- 
1.5.4.3

[PATCH 11/17] Make USBHOST powerdomain go to sleep after warm reset.

[Tero Kristo]

From: Peter De-Schrijver <peter.de-schrijver@nokia.com>

This patch makes sure the USBHOST and SGX powerdomains can go to any sleep
state after a warm reset.

Signed-off-by: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 arch/arm/mach-omap2/cm-regbits-34xx.h |    1 +
 arch/arm/mach-omap2/pm34xx.c          |   18 ++++++++++++++++++
 2 files changed, 19 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..3152553 100644
--- a/arch/arm/mach-omap2/cm-regbits-34xx.h
+++ b/arch/arm/mach-omap2/cm-regbits-34xx.h
@@ -680,6 +680,7 @@
 #define OMAP3430_CLKSEL_GPT2_SHIFT			0
 
 /* CM_SLEEPDEP_PER specific bits */
+#define OMAP3430_CM_SLEEPDEP_PER_EN_MPU			(1 << 1)
 #define OMAP3430_CM_SLEEPDEP_PER_EN_IVA2		(1 << 2)
 
 /* CM_CLKSTCTRL_PER */
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 47f5738..1fa778f 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -26,6 +26,7 @@
 #include <linux/err.h>
 #include <linux/gpio.h>
 #include <linux/clk.h>
+#include <linux/delay.h>
 
 #include <mach/sram.h>
 #include <mach/prcm.h>
@@ -934,6 +935,24 @@ static void __init prcm_setup_regs(void)
 	prm_write_mod_reg(0, OMAP3430_CAM_MOD, PM_WKDEP);
 	prm_write_mod_reg(0, OMAP3430_PER_MOD, PM_WKDEP);
 	if (omap_rev() > OMAP3430_REV_ES1_0) {
+
+		/*
+		 * This workaround is needed to prevent SGX and USBHOST from
+		 * failing to transition to RET/OFF after a warm reset in OFF
+		 * mode. Workaround sets a sleepdep of each of these domains
+		 * with MPU, waits for a min 2 sysclk cycles and clears the
+		 * sleepdep.
+		 */
+		cm_write_mod_reg(OMAP3430_CM_SLEEPDEP_PER_EN_MPU,
+				OMAP3430ES2_USBHOST_MOD, OMAP3430_CM_SLEEPDEP);
+		cm_write_mod_reg(OMAP3430_CM_SLEEPDEP_PER_EN_MPU,
+				OMAP3430ES2_SGX_MOD, OMAP3430_CM_SLEEPDEP);
+		udelay(100);
+		cm_write_mod_reg(0, OMAP3430ES2_USBHOST_MOD,
+				OMAP3430_CM_SLEEPDEP);
+		cm_write_mod_reg(0, OMAP3430ES2_SGX_MOD,
+				OMAP3430_CM_SLEEPDEP);
+
 		prm_write_mod_reg(0, OMAP3430ES2_SGX_MOD, PM_WKDEP);
 		prm_write_mod_reg(0, OMAP3430ES2_USBHOST_MOD, PM_WKDEP);
 	} else
-- 
1.5.4.3

[PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Tero Kristo]

From: Jouni Hogander <jouni.hogander@nokia.com>

OMAP interrupt controller goes to unknown state when there is right
combination of l3,l4 sleep/wake-up transitions, l4 autoidle in
interrupt controller and some interrupt. When this happens, interrupts
are not delivered to ARM anymore and ARM will remain in WFI (wait for
interrupt) until interrupt controller is forced to wake-up
(i.e. lauterbach). Disable AUTOIDLE in INTC for now.

Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 arch/arm/mach-omap2/irq.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 4ed05e9..76179a2 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -167,8 +167,10 @@ static void __init omap_irq_bank_init_one(struct omap_irq_bank *bank)
 	while (!(intc_bank_read_reg(bank, INTC_SYSSTATUS) & 0x1))
 		/* Wait for reset to complete */;
 
-	/* Enable autoidle */
+	/* Do not enable autoidle as it seems to cause problems */
+#if 0
 	intc_bank_write_reg(1 << 0, bank, INTC_SYSCONFIG);
+#endif
 }
 
 int omap_irq_pending(void)
-- 
1.5.4.3

[PATCH 13/17] OMAP3: Fixed ARM aux ctrl register save/restore

[Tero Kristo]

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

Current value is stored on SDRAM and it is written back during wakeup.
Previously a static value of 0x72 was written there.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 arch/arm/mach-omap2/control.c   |    1 +
 arch/arm/mach-omap2/sleep34xx.S |   23 ++++++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 296f2c2..730fc53 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -94,6 +94,7 @@ void *omap3_secure_ram_storage;
  * during the restore path.
  */
 u32 omap3_arm_context[128];
+u32 omap3_aux_ctrl[2] = { 0x1, 0x0 };
 
 struct omap3_control_regs {
 	u32 sysconfig;
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index f4f5ebe..0b03bf9 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -26,6 +26,7 @@
  */
 #include <linux/linkage.h>
 #include <asm/assembler.h>
+#include <asm/memory.h>
 #include <mach/io.h>
 #include <mach/control.h>
 
@@ -278,7 +279,11 @@ restore:
 	mov	r1, #0		@ set task id for ROM code in r1
 	mov	r2, #4		@ set some flags in r2, r6
 	mov	r6, #0xff
-	adr	r3, write_aux_control_params	@ r3 points to parameters
+	ldr	r3, write_aux_control_params	@ r3 points to parameters
+	ldr	r4, phys_offset
+	adds	r3, r3, r4
+	ldr	r4, page_offset
+	subs	r3, r3, r4
 	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
 	.word	0xE1600071		@ call SMI monitor (smi #1)
@@ -287,13 +292,18 @@ restore:
 l2_inv_api_params:
 	.word   0x1, 0x00
 write_aux_control_params:
-	.word   0x1, 0x72
+	.word	omap3_aux_ctrl
 l2_inv_gp:
 	/* Execute smi to invalidate L2 cache */
 	mov r12, #0x1                         @ set up to invalide L2
 smi:    .word 0xE1600070		@ Call SMI monitor (smieq)
 	/* Write to Aux control register to set some bits */
-	mov	r0, #0x72
+	ldr	r1, write_aux_control_params
+	ldr	r0, phys_offset
+	adds	r1, r1, r0
+	ldr	r0, page_offset
+	subs	r1, r1, r0
+	ldr	r0, [r1, #4]
 	mov	r12, #0x3
 	.word 0xE1600070	@ Call SMI monitor (smieq)
 logic_l1_restore:
@@ -420,6 +430,9 @@ usettbr0:
 save_context_wfi:
 	/*b	save_context_wfi*/	@ enable to debug save code
 	mov	r8, r0 /* Store SDRAM address in r8 */
+	mrc	p15, 0, r4, c1, c0, 1	@ Read Auxiliary Control Register
+	ldr	r5, write_aux_control_params
+	str	r4, [r5, #4]
         /* Check what that target sleep state is:stored in r1*/
         /* 1 - Only L1 and logic lost */
         /* 2 - Only L2 lost */
@@ -605,6 +618,10 @@ wait_dll_lock:
         bne     wait_dll_lock
         bx      lr
 
+phys_offset:
+	.word	PHYS_OFFSET
+page_offset:
+	.word	PAGE_OFFSET
 cm_idlest1_core:
 	.word	CM_IDLEST1_CORE_V
 sdrc_dlla_status:
-- 
1.5.4.3

[PATCH 14/17] OMAP3: PM: Disabled I2C4 repeated start operation mode

[Tero Kristo]

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

Repeated start forces I2C4 pads low during idle, which increases power
consumption through external pull-ups. On the other hand, this change
increases I2C4 command latencies a bit.

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

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 1fa778f..f492142 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1375,7 +1375,7 @@ static void __init configure_vc(void)
 	prm_write_mod_reg(OMAP3430_CMD1 | OMAP3430_RAV1, OMAP3430_GR_MOD,
 			  OMAP3_PRM_VC_CH_CONF_OFFSET);
 
-	prm_write_mod_reg(OMAP3430_MCODE_SHIFT | OMAP3430_HSEN | OMAP3430_SREN,
+	prm_write_mod_reg(OMAP3430_MCODE_SHIFT | OMAP3430_HSEN,
 			  OMAP3430_GR_MOD,
 			  OMAP3_PRM_VC_I2C_CFG_OFFSET);
 
-- 
1.5.4.3

[PATCH 15/17] OMAP3: PM: Added support for L2 aux ctrl register save and restore

[Tero Kristo]

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

This patch adds a save and restore mechanism for ARM L2 auxiliary control
register. This feature is enabled via Kconfig option
OMAP3_L2_AUX_SECURE_SAVE_RESTORE and the service ID for PPA can be provided
via option OMAP3_L2_AUX_SECURE_SERVICE_SET_ID. If bootloader does not modify
L2 aux register, using this save and restore logic for it is not needed.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/control.c   |    1 +
 arch/arm/mach-omap2/sleep34xx.S |   31 +++++++++++++++++++++++++++++++
 arch/arm/plat-omap/Kconfig      |   17 +++++++++++++++++
 3 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 730fc53..82d8f1a 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -95,6 +95,7 @@ void *omap3_secure_ram_storage;
  */
 u32 omap3_arm_context[128];
 u32 omap3_aux_ctrl[2] = { 0x1, 0x0 };
+u32 omap3_l2_aux_ctrl[2] = { 0x1, 0x0 };
 
 struct omap3_control_regs {
 	u32 sysconfig;
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index 0b03bf9..e291d6d 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -288,11 +288,30 @@ restore:
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
 	.word	0xE1600071		@ call SMI monitor (smi #1)
 
+#ifdef CONFIG_OMAP3_L2_AUX_SECURE_SAVE_RESTORE
+	/* Restore L2 aux control register */
+	@ set service ID for PPA
+	mov	r0, #CONFIG_OMAP3_L2_AUX_SECURE_SERVICE_SET_ID
+	mov	r12, r0		@ copy service ID in r12
+	mov	r1, #0		@ set task ID for ROM code in r1
+	mov	r2, #4		@ set some flags in r2, r6
+	mov	r6, #0xff
+	ldr	r3, write_l2_aux_control_params	@ r3 points to parameters
+	ldr	r4, phys_offset
+	adds	r3, r3, r4
+	ldr	r4, page_offset
+	subs	r3, r3, r4
+	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
+	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
+	.word	0xE1600071		@ call SMI monitor (smi #1)
+#endif
 	b	logic_l1_restore
 l2_inv_api_params:
 	.word   0x1, 0x00
 write_aux_control_params:
 	.word	omap3_aux_ctrl
+write_l2_aux_control_params:
+	.word	omap3_l2_aux_ctrl
 l2_inv_gp:
 	/* Execute smi to invalidate L2 cache */
 	mov r12, #0x1                         @ set up to invalide L2
@@ -306,6 +325,15 @@ smi:    .word 0xE1600070		@ Call SMI monitor (smieq)
 	ldr	r0, [r1, #4]
 	mov	r12, #0x3
 	.word 0xE1600070	@ Call SMI monitor (smieq)
+	/* Restore L2 AUX control register */
+	ldr	r1, write_l2_aux_control_params
+	ldr	r0, phys_offset
+	adds	r1, r1, r0
+	ldr	r0, page_offset
+	subs	r1, r1, r0
+	ldr	r0, [r1, #4]
+	mov	r12, #0x2
+	.word 0xE1600070	@ Call SMI monitor (smieq)
 logic_l1_restore:
 	mov	r1, #0
 	/* Invalidate all instruction caches to PoU
@@ -433,6 +461,9 @@ save_context_wfi:
 	mrc	p15, 0, r4, c1, c0, 1	@ Read Auxiliary Control Register
 	ldr	r5, write_aux_control_params
 	str	r4, [r5, #4]
+	mrc	p15, 1, r4, c9, c0, 2	@ Read L2 AUX ctrl register
+	ldr	r5, write_l2_aux_control_params
+	str	r4, [r5, #4]
         /* Check what that target sleep state is:stored in r1*/
         /* 1 - Only L1 and logic lost */
         /* 2 - Only L2 lost */
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 2143db5..3ff1a5f 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -185,6 +185,23 @@ config OMAP3_DEBOBS
 	help
 	  Use ETK pads for debug observability
 
+config OMAP3_L2_AUX_SECURE_SAVE_RESTORE
+	bool "OMAP3 HS/EMU save and restore for L2 AUX control register"
+	depends on ARCH_OMAP3 && PM
+	default n
+	help
+	  Without this option, L2 Auxiliary control register contents are
+	  lost during off-mode entry on HS/EMU devices. This feature
+	  requires support from PPA / boot-loader in HS/EMU devices, which
+	  currently does not exist by default.
+
+config OMAP3_L2_AUX_SECURE_SERVICE_SET_ID
+	int "Service ID for the support routine to set L2 AUX control"
+	depends on OMAP3_L2_AUX_SECURE_SAVE_RESTORE
+	default 43
+	help
+	  PPA routine service ID for setting L2 auxiliary control register.
+
 config OMAP_32K_TIMER_HZ
        int "Kernel internal timer frequency for 32KHz timer"
        range 32 1024
-- 
1.5.4.3

[PATCH 16/17] OMAP3: PM: Write voltage and clock setup times dynamically in idle loop

[Tero Kristo]

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

It is suggested by TI (SWPA152 February 2009) to write clksetup,
voltsetup_time1, voltsetup_time2, voltsetup2 dynamically in idle loop.

Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |   36 +++++++++++++++++++++++++-----------
 1 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index f492142..ae83121 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -575,6 +575,30 @@ void omap_sram_idle(void)
 	if (regset_save_on_suspend)
 		pm_dbg_regset_save(1);
 
+	/* Write voltage setup times which are changed dynamically */
+	if (core_next_state == PWRDM_POWER_OFF) {
+		prm_write_mod_reg(0, OMAP3430_GR_MOD,
+				OMAP3_PRM_VOLTSETUP1_OFFSET);
+		prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
+				OMAP3_PRM_VOLTSETUP2_OFFSET);
+		prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
+				OMAP3_PRM_CLKSETUP_OFFSET);
+	} else {
+		prm_write_mod_reg((prm_setup.voltsetup_time2 <<
+				OMAP3430_SETUP_TIME2_SHIFT) |
+				(prm_setup.voltsetup_time1 <<
+				OMAP3430_SETUP_TIME1_SHIFT),
+				OMAP3430_GR_MOD, OMAP3_PRM_VOLTSETUP1_OFFSET);
+		prm_write_mod_reg(0, OMAP3430_GR_MOD,
+				OMAP3_PRM_VOLTSETUP2_OFFSET);
+		/*
+		 * Use static 1 as only HF_CLKOUT is turned off.
+		 * Value taken from application note SWPA152
+		 */
+		prm_write_mod_reg(0x1, OMAP3430_GR_MOD,
+				OMAP3_PRM_CLKSETUP_OFFSET);
+	}
+
 	/*
 	 * omap3_arm_context is the location where ARM registers
 	 * get saved. The restore path then reads from this
@@ -1379,19 +1403,9 @@ static void __init configure_vc(void)
 			  OMAP3430_GR_MOD,
 			  OMAP3_PRM_VC_I2C_CFG_OFFSET);
 
-	/* Write setup times */
-	prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
-			OMAP3_PRM_CLKSETUP_OFFSET);
-	prm_write_mod_reg((prm_setup.voltsetup_time2 <<
-			OMAP3430_SETUP_TIME2_SHIFT) |
-			(prm_setup.voltsetup_time1 <<
-			OMAP3430_SETUP_TIME1_SHIFT),
-			OMAP3430_GR_MOD, OMAP3_PRM_VOLTSETUP1_OFFSET);
-
+	/* Write static setup times */
 	prm_write_mod_reg(prm_setup.voltoffset, OMAP3430_GR_MOD,
 			OMAP3_PRM_VOLTOFFSET_OFFSET);
-	prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
-			OMAP3_PRM_VOLTSETUP2_OFFSET);
 
 	pm_dbg_regset_init(1);
 	pm_dbg_regset_init(2);
-- 
1.5.4.3

[PATCH 17/17] OMAP3: PM: Force disable OTG autoidle

[Tero Kristo]

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

OMAP3 sleep can be prevented in some cases where OTG autoidle is enabled.
This patch force disables autoidle during boot and after wakeup from off-mode.
See omap erratas 1.164 and 1.165.

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

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index ae83121..5f351f2 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -94,6 +94,8 @@ u32 voltage_off_while_idle;
 		OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
 		OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
 
+#define OTG_SYSCONFIG	(OMAP34XX_HSUSB_OTG_BASE + 0x404)
+
 struct power_state {
 	struct powerdomain *pwrdm;
 	u32 next_state;
@@ -423,6 +425,16 @@ static void restore_table_entry(void)
 	restore_control_register(control_reg_value);
 }
 
+static inline void disable_otg_autoidle(void)
+{
+	/*
+	 * OTG autoidle can prevent core domain sleep in some cases, thus
+	 * disable it. See omap erratas 1.164 and 1.165.
+	 */
+	cm_set_mod_reg_bits(OMAP3430_EN_HSOTGUSB, CORE_MOD, CM_ICLKEN1);
+	omap_writel(0x0, OTG_SYSCONFIG);
+}
+
 void omap_sram_idle(void)
 {
 	/* Variable to tell what needs to be saved and restored
@@ -628,6 +640,7 @@ void omap_sram_idle(void)
 			omap3_prcm_restore_context();
 			omap3_sram_restore_context();
 			omap2_sms_restore_context();
+			disable_otg_autoidle();
 		}
 		omap_uart_resume_idle(0);
 		omap_uart_resume_idle(1);
@@ -1417,6 +1430,7 @@ static int __init omap3_pm_early_init(void)
 				OMAP3_PRM_POLCTRL_OFFSET);
 
 	configure_vc();
+	disable_otg_autoidle();
 
 	return 0;
 }
-- 
1.5.4.3

Re: [PATCH 17/17] OMAP3: PM: Force disable OTG autoidle

[Kevin Hilman]

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

> From: Tero Kristo <tero.kristo@nokia.com>
>
> OMAP3 sleep can be prevented in some cases where OTG autoidle is enabled.
> This patch force disables autoidle during boot and after wakeup from off-mode.
> See omap erratas 1.164 and 1.165.

The init-at-boot step isn't needed as this is already done in 
PM branch (see usb-muxb.c:usb_musb_pm_init()).

> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index ae83121..5f351f2 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -94,6 +94,8 @@ u32 voltage_off_while_idle;
>  		OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
>  		OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
>  
> +#define OTG_SYSCONFIG	(OMAP34XX_HSUSB_OTG_BASE + 0x404)
> +
>  struct power_state {
>  	struct powerdomain *pwrdm;
>  	u32 next_state;
> @@ -423,6 +425,16 @@ static void restore_table_entry(void)
>  	restore_control_register(control_reg_value);
>  }
>  
> +static inline void disable_otg_autoidle(void)
> +{
> +	/*
> +	 * OTG autoidle can prevent core domain sleep in some cases, thus
> +	 * disable it. See omap erratas 1.164 and 1.165.
> +	 */
> +	cm_set_mod_reg_bits(OMAP3430_EN_HSOTGUSB, CORE_MOD, CM_ICLKEN1);

Is the ICLK enable required as part of the fix too?  I don't see
mention of enabling the iclk as part of the workaround for either of
these errata.

If the iclk needs to be enabled, it should be done at init using
the clk API.

> +	omap_writel(0x0, OTG_SYSCONFIG);

omap_writel() deprecated.  

> +}
> +

For readability, this should be a function in the USB core code
(either usb-musb.c or a static inline in <mach/usb.h>)

Kevin

>  void omap_sram_idle(void)
>  {
>  	/* Variable to tell what needs to be saved and restored
> @@ -628,6 +640,7 @@ void omap_sram_idle(void)
>  			omap3_prcm_restore_context();
>  			omap3_sram_restore_context();
>  			omap2_sms_restore_context();
> +			disable_otg_autoidle();
>  		}
>  		omap_uart_resume_idle(0);
>  		omap_uart_resume_idle(1);
> @@ -1417,6 +1430,7 @@ static int __init omap3_pm_early_init(void)
>  				OMAP3_PRM_POLCTRL_OFFSET);
>  
>  	configure_vc();
> +	disable_otg_autoidle();
>  
>  	return 0;
>  }
> -- 
> 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: [PATCH 17/17] OMAP3: PM: Force disable OTG autoidle

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 20 October, 2009 21:44
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 17/17] OMAP3: PM: Force disable OTG autoidle
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> OMAP3 sleep can be prevented in some cases where OTG 
>autoidle is enabled.
>> This patch force disables autoidle during boot and after 
>wakeup from off-mode.
>> See omap erratas 1.164 and 1.165.
>
>The init-at-boot step isn't needed as this is already done in 
>PM branch (see usb-muxb.c:usb_musb_pm_init()).

True, that reset part actually does this, which is also part of the errata.

>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |   14 ++++++++++++++
>>  1 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index ae83121..5f351f2 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -94,6 +94,8 @@ u32 voltage_off_while_idle;
>>  		OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
>>  		OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
>>  
>> +#define OTG_SYSCONFIG	(OMAP34XX_HSUSB_OTG_BASE + 0x404)
>> +
>>  struct power_state {
>>  	struct powerdomain *pwrdm;
>>  	u32 next_state;
>> @@ -423,6 +425,16 @@ static void restore_table_entry(void)
>>  	restore_control_register(control_reg_value);
>>  }
>>  
>> +static inline void disable_otg_autoidle(void)
>> +{
>> +	/*
>> +	 * OTG autoidle can prevent core domain sleep in some 
>cases, thus
>> +	 * disable it. See omap erratas 1.164 and 1.165.
>> +	 */
>> +	cm_set_mod_reg_bits(OMAP3430_EN_HSOTGUSB, CORE_MOD, CM_ICLKEN1);
>
>Is the ICLK enable required as part of the fix too?  I don't see
>mention of enabling the iclk as part of the workaround for either of
>these errata.
>
>If the iclk needs to be enabled, it should be done at init using
>the clk API.

ICLK enable part is here because we will crash unless ICLK is active. It does not really need to be on, but it was rather simpler to do it this way. I can try to look if adding this into USB code would make sense.

>
>> +	omap_writel(0x0, OTG_SYSCONFIG);
>
>omap_writel() deprecated.  

Again...

>
>> +}
>> +
>
>For readability, this should be a function in the USB core code
>(either usb-musb.c or a static inline in <mach/usb.h>)
>
>Kevin
>
>>  void omap_sram_idle(void)
>>  {
>>  	/* Variable to tell what needs to be saved and restored
>> @@ -628,6 +640,7 @@ void omap_sram_idle(void)
>>  			omap3_prcm_restore_context();
>>  			omap3_sram_restore_context();
>>  			omap2_sms_restore_context();
>> +			disable_otg_autoidle();
>>  		}
>>  		omap_uart_resume_idle(0);
>>  		omap_uart_resume_idle(1);
>> @@ -1417,6 +1430,7 @@ static int __init omap3_pm_early_init(void)
>>  				OMAP3_PRM_POLCTRL_OFFSET);
>>  
>>  	configure_vc();
>> +	disable_otg_autoidle();
>>  
>>  	return 0;
>>  }
>> -- 
>> 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: [PATCH 16/17] OMAP3: PM: Write voltage and clock setup times dynamically in idle loop

[Kevin Hilman]

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

> From: Tero Kristo <tero.kristo@nokia.com>
>
> It is suggested by TI (SWPA152 February 2009) to write clksetup,
> voltsetup_time1, voltsetup_time2, voltsetup2 dynamically in idle loop.

Can you summarize the reasons why?

> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |   36 +++++++++++++++++++++++++-----------
>  1 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index f492142..ae83121 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -575,6 +575,30 @@ void omap_sram_idle(void)
>  	if (regset_save_on_suspend)
>  		pm_dbg_regset_save(1);
>  
> +	/* Write voltage setup times which are changed dynamically */

AFAICT, we only set these values at init time and they are never
changed.  Are there some forthcoming patches that change these
dynamically?

Kevin

> +	if (core_next_state == PWRDM_POWER_OFF) {
> +		prm_write_mod_reg(0, OMAP3430_GR_MOD,
> +				OMAP3_PRM_VOLTSETUP1_OFFSET);
> +		prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
> +				OMAP3_PRM_VOLTSETUP2_OFFSET);
> +		prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
> +				OMAP3_PRM_CLKSETUP_OFFSET);
> +	} else {
> +		prm_write_mod_reg((prm_setup.voltsetup_time2 <<
> +				OMAP3430_SETUP_TIME2_SHIFT) |
> +				(prm_setup.voltsetup_time1 <<
> +				OMAP3430_SETUP_TIME1_SHIFT),
> +				OMAP3430_GR_MOD, OMAP3_PRM_VOLTSETUP1_OFFSET);
> +		prm_write_mod_reg(0, OMAP3430_GR_MOD,
> +				OMAP3_PRM_VOLTSETUP2_OFFSET);
> +		/*
> +		 * Use static 1 as only HF_CLKOUT is turned off.
> +		 * Value taken from application note SWPA152
> +		 */
> +		prm_write_mod_reg(0x1, OMAP3430_GR_MOD,
> +				OMAP3_PRM_CLKSETUP_OFFSET);
> +	}
> +
>  	/*
>  	 * omap3_arm_context is the location where ARM registers
>  	 * get saved. The restore path then reads from this
> @@ -1379,19 +1403,9 @@ static void __init configure_vc(void)
>  			  OMAP3430_GR_MOD,
>  			  OMAP3_PRM_VC_I2C_CFG_OFFSET);
>  
> -	/* Write setup times */
> -	prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
> -			OMAP3_PRM_CLKSETUP_OFFSET);
> -	prm_write_mod_reg((prm_setup.voltsetup_time2 <<
> -			OMAP3430_SETUP_TIME2_SHIFT) |
> -			(prm_setup.voltsetup_time1 <<
> -			OMAP3430_SETUP_TIME1_SHIFT),
> -			OMAP3430_GR_MOD, OMAP3_PRM_VOLTSETUP1_OFFSET);
> -
> +	/* Write static setup times */
>  	prm_write_mod_reg(prm_setup.voltoffset, OMAP3430_GR_MOD,
>  			OMAP3_PRM_VOLTOFFSET_OFFSET);
> -	prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
> -			OMAP3_PRM_VOLTSETUP2_OFFSET);
>  
>  	pm_dbg_regset_init(1);
>  	pm_dbg_regset_init(2);
> -- 
> 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: [PATCH 16/17] OMAP3: PM: Write voltage and clock setup times dynamically in idle loop

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 20 October, 2009 20:48
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 16/17] OMAP3: PM: Write voltage and clock 
>setup times dynamically in idle loop
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> It is suggested by TI (SWPA152 February 2009) to write clksetup,
>> voltsetup_time1, voltsetup_time2, voltsetup2 dynamically in 
>idle loop.
>
>Can you summarize the reasons why?

Basically this optimizes the clksetup / voltsetup times according to the sleep mode. Currently the settings are too high in both retention and off-mode, because the hardware appears to use for example VOLTSETUP2 even if we are not in off-mode, adding extra delay to wakeup. Also, CLKSETUP is too high for retention mode because oscillator is not actually shut-off here.

However, now that I think about it, it might be better to change this in a way that it is user configurable whether we want to change the settings or not, maybe add new items in to the prm_setup struct for alternate settings for ret / off and only use these if available. Some boards might actually switch oscillator off in retention mode which would require higher setup time.

>
>> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |   36 
>+++++++++++++++++++++++++-----------
>>  1 files changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index f492142..ae83121 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -575,6 +575,30 @@ void omap_sram_idle(void)
>>  	if (regset_save_on_suspend)
>>  		pm_dbg_regset_save(1);
>>  
>> +	/* Write voltage setup times which are changed dynamically */
>
>AFAICT, we only set these values at init time and they are never
>changed.  Are there some forthcoming patches that change these
>dynamically?

Following bit of the code actually changes them dynamically, you can see that e.g. CLKSETUP time is either prm_setup.clksetup or 0x1 depending on the sleep mode. But as previously said, I think these should probably be added as new items to the prm_setup struct.

>
>Kevin
>
>> +	if (core_next_state == PWRDM_POWER_OFF) {
>> +		prm_write_mod_reg(0, OMAP3430_GR_MOD,
>> +				OMAP3_PRM_VOLTSETUP1_OFFSET);
>> +		prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
>> +				OMAP3_PRM_VOLTSETUP2_OFFSET);
>> +		prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
>> +				OMAP3_PRM_CLKSETUP_OFFSET);
>> +	} else {
>> +		prm_write_mod_reg((prm_setup.voltsetup_time2 <<
>> +				OMAP3430_SETUP_TIME2_SHIFT) |
>> +				(prm_setup.voltsetup_time1 <<
>> +				OMAP3430_SETUP_TIME1_SHIFT),
>> +				OMAP3430_GR_MOD, 
>OMAP3_PRM_VOLTSETUP1_OFFSET);
>> +		prm_write_mod_reg(0, OMAP3430_GR_MOD,
>> +				OMAP3_PRM_VOLTSETUP2_OFFSET);
>> +		/*
>> +		 * Use static 1 as only HF_CLKOUT is turned off.
>> +		 * Value taken from application note SWPA152
>> +		 */
>> +		prm_write_mod_reg(0x1, OMAP3430_GR_MOD,
>> +				OMAP3_PRM_CLKSETUP_OFFSET);
>> +	}
>> +
>>  	/*
>>  	 * omap3_arm_context is the location where ARM registers
>>  	 * get saved. The restore path then reads from this
>> @@ -1379,19 +1403,9 @@ static void __init configure_vc(void)
>>  			  OMAP3430_GR_MOD,
>>  			  OMAP3_PRM_VC_I2C_CFG_OFFSET);
>>  
>> -	/* Write setup times */
>> -	prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
>> -			OMAP3_PRM_CLKSETUP_OFFSET);
>> -	prm_write_mod_reg((prm_setup.voltsetup_time2 <<
>> -			OMAP3430_SETUP_TIME2_SHIFT) |
>> -			(prm_setup.voltsetup_time1 <<
>> -			OMAP3430_SETUP_TIME1_SHIFT),
>> -			OMAP3430_GR_MOD, OMAP3_PRM_VOLTSETUP1_OFFSET);
>> -
>> +	/* Write static setup times */
>>  	prm_write_mod_reg(prm_setup.voltoffset, OMAP3430_GR_MOD,
>>  			OMAP3_PRM_VOLTOFFSET_OFFSET);
>> -	prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
>> -			OMAP3_PRM_VOLTSETUP2_OFFSET);
>>  
>>  	pm_dbg_regset_init(1);
>>  	pm_dbg_regset_init(2);
>> -- 
>> 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: [PATCH 16/17] OMAP3: PM: Write voltage and clock setup times dynamically in idle loop

[Kevin Hilman]

<Tero.Kristo@nokia.com> writes:

>  
>
>>-----Original Message-----
>>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>>Sent: 20 October, 2009 20:48
>>To: Kristo Tero (Nokia-D/Tampere)
>>Cc: linux-omap@vger.kernel.org
>>Subject: Re: [PATCH 16/17] OMAP3: PM: Write voltage and clock 
>>setup times dynamically in idle loop
>>
>>Tero Kristo <tero.kristo@nokia.com> writes:
>>
>>> From: Tero Kristo <tero.kristo@nokia.com>
>>>
>>> It is suggested by TI (SWPA152 February 2009) to write clksetup,
>>> voltsetup_time1, voltsetup_time2, voltsetup2 dynamically in 
>>idle loop.
>>
>>Can you summarize the reasons why?
>
> Basically this optimizes the clksetup / voltsetup times according to the sleep mode. Currently the settings are too high in both retention and off-mode, because the hardware appears to use for example VOLTSETUP2 even if we are not in off-mode, adding extra delay to wakeup. Also, CLKSETUP is too high for retention mode because oscillator is not actually shut-off here.
>
> However, now that I think about it, it might be better to change this in a way that it is user configurable whether we want to change the settings or not, maybe add new items in to the prm_setup struct for alternate settings for ret / off and only use these if available. Some boards might actually switch oscillator off in retention mode which would require higher setup time.
>
>>
>>> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
>>> ---
>>>  arch/arm/mach-omap2/pm34xx.c |   36 
>>+++++++++++++++++++++++++-----------
>>>  1 files changed, 25 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>>b/arch/arm/mach-omap2/pm34xx.c
>>> index f492142..ae83121 100644
>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>> @@ -575,6 +575,30 @@ void omap_sram_idle(void)
>>>  	if (regset_save_on_suspend)
>>>  		pm_dbg_regset_save(1);
>>>  
>>> +	/* Write voltage setup times which are changed dynamically */
>>
>>AFAICT, we only set these values at init time and they are never
>>changed.  Are there some forthcoming patches that change these
>>dynamically?
>
> Following bit of the code actually changes them dynamically, you can
> see that e.g. CLKSETUP time is either prm_setup.clksetup or 0x1
> depending on the sleep mode. 

Doh, I see now.

> But as previously said, I think these should probably be added as
> new items to the prm_setup struct.

Yes, I would rather see the prm_setup struct extended so these can be
passed in by board code.

Kevin

>>
>>Kevin
>>
>>> +	if (core_next_state == PWRDM_POWER_OFF) {
>>> +		prm_write_mod_reg(0, OMAP3430_GR_MOD,
>>> +				OMAP3_PRM_VOLTSETUP1_OFFSET);
>>> +		prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
>>> +				OMAP3_PRM_VOLTSETUP2_OFFSET);
>>> +		prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
>>> +				OMAP3_PRM_CLKSETUP_OFFSET);
>>> +	} else {
>>> +		prm_write_mod_reg((prm_setup.voltsetup_time2 <<
>>> +				OMAP3430_SETUP_TIME2_SHIFT) |
>>> +				(prm_setup.voltsetup_time1 <<
>>> +				OMAP3430_SETUP_TIME1_SHIFT),
>>> +				OMAP3430_GR_MOD, 
>>OMAP3_PRM_VOLTSETUP1_OFFSET);
>>> +		prm_write_mod_reg(0, OMAP3430_GR_MOD,
>>> +				OMAP3_PRM_VOLTSETUP2_OFFSET);
>>> +		/*
>>> +		 * Use static 1 as only HF_CLKOUT is turned off.
>>> +		 * Value taken from application note SWPA152
>>> +		 */
>>> +		prm_write_mod_reg(0x1, OMAP3430_GR_MOD,
>>> +				OMAP3_PRM_CLKSETUP_OFFSET);
>>> +	}
>>> +
>>>  	/*
>>>  	 * omap3_arm_context is the location where ARM registers
>>>  	 * get saved. The restore path then reads from this
>>> @@ -1379,19 +1403,9 @@ static void __init configure_vc(void)
>>>  			  OMAP3430_GR_MOD,
>>>  			  OMAP3_PRM_VC_I2C_CFG_OFFSET);
>>>  
>>> -	/* Write setup times */
>>> -	prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
>>> -			OMAP3_PRM_CLKSETUP_OFFSET);
>>> -	prm_write_mod_reg((prm_setup.voltsetup_time2 <<
>>> -			OMAP3430_SETUP_TIME2_SHIFT) |
>>> -			(prm_setup.voltsetup_time1 <<
>>> -			OMAP3430_SETUP_TIME1_SHIFT),
>>> -			OMAP3430_GR_MOD, OMAP3_PRM_VOLTSETUP1_OFFSET);
>>> -
>>> +	/* Write static setup times */
>>>  	prm_write_mod_reg(prm_setup.voltoffset, OMAP3430_GR_MOD,
>>>  			OMAP3_PRM_VOLTOFFSET_OFFSET);
>>> -	prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
>>> -			OMAP3_PRM_VOLTSETUP2_OFFSET);
>>>  
>>>  	pm_dbg_regset_init(1);
>>>  	pm_dbg_regset_init(2);
>>> -- 
>>> 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: [PATCH 16/17] OMAP3: PM: Write voltage and clock setup times dynamically in idle loop

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 21 October, 2009 17:15
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 16/17] OMAP3: PM: Write voltage and clock 
>setup times dynamically in idle loop
>
><Tero.Kristo@nokia.com> writes:
>
>>  
>>
>>>-----Original Message-----
>>>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>>>Sent: 20 October, 2009 20:48
>>>To: Kristo Tero (Nokia-D/Tampere)
>>>Cc: linux-omap@vger.kernel.org
>>>Subject: Re: [PATCH 16/17] OMAP3: PM: Write voltage and clock 
>>>setup times dynamically in idle loop
>>>
>>>Tero Kristo <tero.kristo@nokia.com> writes:
>>>
>>>> From: Tero Kristo <tero.kristo@nokia.com>
>>>>
>>>> It is suggested by TI (SWPA152 February 2009) to write clksetup,
>>>> voltsetup_time1, voltsetup_time2, voltsetup2 dynamically in 
>>>idle loop.
>>>
>>>Can you summarize the reasons why?
>>
>> Basically this optimizes the clksetup / voltsetup times 
>according to the sleep mode. Currently the settings are too 
>high in both retention and off-mode, because the hardware 
>appears to use for example VOLTSETUP2 even if we are not in 
>off-mode, adding extra delay to wakeup. Also, CLKSETUP is too 
>high for retention mode because oscillator is not actually 
>shut-off here.
>>
>> However, now that I think about it, it might be better to 
>change this in a way that it is user configurable whether we 
>want to change the settings or not, maybe add new items in to 
>the prm_setup struct for alternate settings for ret / off and 
>only use these if available. Some boards might actually switch 
>oscillator off in retention mode which would require higher setup time.
>>
>>>
>>>> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
>>>> ---
>>>>  arch/arm/mach-omap2/pm34xx.c |   36 
>>>+++++++++++++++++++++++++-----------
>>>>  1 files changed, 25 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>>>b/arch/arm/mach-omap2/pm34xx.c
>>>> index f492142..ae83121 100644
>>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>>> @@ -575,6 +575,30 @@ void omap_sram_idle(void)
>>>>  	if (regset_save_on_suspend)
>>>>  		pm_dbg_regset_save(1);
>>>>  
>>>> +	/* Write voltage setup times which are changed dynamically */
>>>
>>>AFAICT, we only set these values at init time and they are never
>>>changed.  Are there some forthcoming patches that change these
>>>dynamically?
>>
>> Following bit of the code actually changes them dynamically, you can
>> see that e.g. CLKSETUP time is either prm_setup.clksetup or 0x1
>> depending on the sleep mode. 
>
>Doh, I see now.
>
>> But as previously said, I think these should probably be added as
>> new items to the prm_setup struct.
>
>Yes, I would rather see the prm_setup struct extended so these can be
>passed in by board code.

I'll change the patch accordingly.


>>>
>>>Kevin
>>>
>>>> +	if (core_next_state == PWRDM_POWER_OFF) {
>>>> +		prm_write_mod_reg(0, OMAP3430_GR_MOD,
>>>> +				OMAP3_PRM_VOLTSETUP1_OFFSET);
>>>> +		prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
>>>> +				OMAP3_PRM_VOLTSETUP2_OFFSET);
>>>> +		prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
>>>> +				OMAP3_PRM_CLKSETUP_OFFSET);
>>>> +	} else {
>>>> +		prm_write_mod_reg((prm_setup.voltsetup_time2 <<
>>>> +				OMAP3430_SETUP_TIME2_SHIFT) |
>>>> +				(prm_setup.voltsetup_time1 <<
>>>> +				OMAP3430_SETUP_TIME1_SHIFT),
>>>> +				OMAP3430_GR_MOD, 
>>>OMAP3_PRM_VOLTSETUP1_OFFSET);
>>>> +		prm_write_mod_reg(0, OMAP3430_GR_MOD,
>>>> +				OMAP3_PRM_VOLTSETUP2_OFFSET);
>>>> +		/*
>>>> +		 * Use static 1 as only HF_CLKOUT is turned off.
>>>> +		 * Value taken from application note SWPA152
>>>> +		 */
>>>> +		prm_write_mod_reg(0x1, OMAP3430_GR_MOD,
>>>> +				OMAP3_PRM_CLKSETUP_OFFSET);
>>>> +	}
>>>> +
>>>>  	/*
>>>>  	 * omap3_arm_context is the location where ARM registers
>>>>  	 * get saved. The restore path then reads from this
>>>> @@ -1379,19 +1403,9 @@ static void __init configure_vc(void)
>>>>  			  OMAP3430_GR_MOD,
>>>>  			  OMAP3_PRM_VC_I2C_CFG_OFFSET);
>>>>  
>>>> -	/* Write setup times */
>>>> -	prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
>>>> -			OMAP3_PRM_CLKSETUP_OFFSET);
>>>> -	prm_write_mod_reg((prm_setup.voltsetup_time2 <<
>>>> -			OMAP3430_SETUP_TIME2_SHIFT) |
>>>> -			(prm_setup.voltsetup_time1 <<
>>>> -			OMAP3430_SETUP_TIME1_SHIFT),
>>>> -			OMAP3430_GR_MOD, OMAP3_PRM_VOLTSETUP1_OFFSET);
>>>> -
>>>> +	/* Write static setup times */
>>>>  	prm_write_mod_reg(prm_setup.voltoffset, OMAP3430_GR_MOD,
>>>>  			OMAP3_PRM_VOLTOFFSET_OFFSET);
>>>> -	prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
>>>> -			OMAP3_PRM_VOLTSETUP2_OFFSET);
>>>>  
>>>>  	pm_dbg_regset_init(1);
>>>>  	pm_dbg_regset_init(2);
>>>> -- 
>>>> 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: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Woodruff, Richard]

> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Tero Kristo
> Sent: Friday, October 16, 2009 5:49 AM

> OMAP interrupt controller goes to unknown state when there is right
> combination of l3,l4 sleep/wake-up transitions, l4 autoidle in
> interrupt controller and some interrupt. When this happens, interrupts
> are not delivered to ARM anymore and ARM will remain in WFI (wait for
> interrupt) until interrupt controller is forced to wake-up
> (i.e. lauterbach). Disable AUTOIDLE in INTC for now.

Optimal work around enables and disables this around WFI.

Regards,
Richard W.

RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Girish S G]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
> Woodruff, Richard
> Sent: Friday, October 16, 2009 9:40 AM
> To: Tero Kristo; linux-omap@vger.kernel.org
> Cc: Jouni Hogander
> Subject: RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller
> 
> 
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Tero Kristo
> > Sent: Friday, October 16, 2009 5:49 AM
> 
> > OMAP interrupt controller goes to unknown state when there is right
> > combination of l3,l4 sleep/wake-up transitions, l4 autoidle in
> > interrupt controller and some interrupt. When this happens, interrupts
> > are not delivered to ARM anymore and ARM will remain in WFI (wait for
> > interrupt) until interrupt controller is forced to wake-up
> > (i.e. lauterbach). Disable AUTOIDLE in INTC for now.
> 
> Optimal work around enables and disables this around WFI.

On one of the custom board the power measured didn't show any major impact, with just one time disabling of INTC-AUTOIDL. However,
optimizing this WA looks good though. I did give it a try,
Disabling/enabling INTC autoidle around WFI on custom board, works well but didn't get chance to
measure the numbers.

Regards,
-Girish

RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Woodruff, Richard]

> From: Ghongdemath, Girish
> Sent: Friday, October 16, 2009 12:48 PM

> On one of the custom board the power measured didn't show any major impact,
> with just one time disabling of INTC-AUTOIDL. However,
> optimizing this WA looks good though. I did give it a try,
> Disabling/enabling INTC autoidle around WFI on custom board, works well but
> didn't get chance to
> measure the numbers.

There was one report from a custom board when it made a difference.  I've not tried to double check this data.  I don't have the reporters setup so there is no guarantee I could reproduce anyway.  In general optimization seemed to make sense.

Regards,
Richard W.

RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Girish S G]

> -----Original Message-----
> From: Woodruff, Richard [mailto:r-woodruff2@ti.com]
> Sent: Friday, October 16, 2009 1:04 PM
> To: Ghongdemath, Girish; 'Tero Kristo'; linux-omap@vger.kernel.org
> Cc: 'Jouni Hogander'
> Subject: RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller


> 
> There was one report from a custom board when it made a difference.  I've not tried to double check
> this data.  I don't have the reporters setup so there is no guarantee I could reproduce anyway.  In
> general optimization seemed to make sense.

I second it.


Regards,
Girish

RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Girish S G [mailto:girishsg@ti.com] 
>Sent: 16 October, 2009 21:10
>To: 'Woodruff, Richard'; Kristo Tero (Nokia-D/Tampere); 
>linux-omap@vger.kernel.org
>Cc: Hogander Jouni (Nokia-D/Tampere)
>Subject: RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in 
>interrupt controller
>
>
>
>> -----Original Message-----
>> From: Woodruff, Richard [mailto:r-woodruff2@ti.com]
>> Sent: Friday, October 16, 2009 1:04 PM
>> To: Ghongdemath, Girish; 'Tero Kristo'; linux-omap@vger.kernel.org
>> Cc: 'Jouni Hogander'
>> Subject: RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE 
>in interrupt controller
>
>
>> 
>> There was one report from a custom board when it made a 
>difference.  I've not tried to double check
>> this data.  I don't have the reporters setup so there is no 
>guarantee I could reproduce anyway.  In
>> general optimization seemed to make sense.
>
>I second it.
>
>
>Regards,
>Girish
>
>

Is there errata number available for this issue by the way? I could attach this to the patch.

Anyway, I guess the optimization would look something like this:

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 210a806..7a98321 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -93,6 +93,8 @@ u32 voltage_off_while_idle;
                OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
                OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)

+#define INTC_SYSCONFIG                 0x10
+
 struct power_state {
        struct powerdomain *pwrdm;
        u32 next_state;
@@ -505,6 +507,12 @@ void omap_sram_idle(void)
                prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
                omap3_enable_io_chain();
        }
+       /*
+        * Disable INTC autoidle as it can cause interrupt controller
+        * to enter unknown state with right combination of sleep / wakeup
+        * transitions
+        */
+       omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);

        /*
        * On EMU/HS devices ROM code restores a SRDC value
@@ -561,6 +569,8 @@ void omap_sram_idle(void)
                                                OMAP3430_GR_MOD,
                                                OMAP3_PRM_VOLTCTRL_OFFSET);
        }
+       /* Re-enable interrupt controller autoidle */
+       omap_writel(OMAP3430_AUTOIDLE, OMAP34XX_IC_BASE + INTC_SYSCONFIG);

        /*
         * Enable smartreflex after WFI. Only needed if we
--
1.5.4.3

RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Woodruff, Richard]

> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Tero.Kristo@nokia.com
> Sent: Monday, October 19, 2009 5:19 AM

> Is there errata number available for this issue by the way? I could attach
> this to the patch.

No.  I expect in the next few weeks it will be published.  The issue is pretty well understood now down to gate level so time to publish should be process.

> Anyway, I guess the optimization would look something like this:

The below looks good.

> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 210a806..7a98321 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -93,6 +93,8 @@ u32 voltage_off_while_idle;
>                 OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
>                 OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
>
> +#define INTC_SYSCONFIG                 0x10
> +
>  struct power_state {
>         struct powerdomain *pwrdm;
>         u32 next_state;
> @@ -505,6 +507,12 @@ void omap_sram_idle(void)
>                 prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
>                 omap3_enable_io_chain();
>         }
> +       /*
> +        * Disable INTC autoidle as it can cause interrupt controller
> +        * to enter unknown state with right combination of sleep / wakeup
> +        * transitions
> +        */
> +       omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
>
>         /*
>         * On EMU/HS devices ROM code restores a SRDC value
> @@ -561,6 +569,8 @@ void omap_sram_idle(void)
>                                                 OMAP3430_GR_MOD,
>
> OMAP3_PRM_VOLTCTRL_OFFSET);
>         }
> +       /* Re-enable interrupt controller autoidle */
> +       omap_writel(OMAP3430_AUTOIDLE, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
>
>         /*
>          * Enable smartreflex after WFI. Only needed if we

RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Girish S G]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
> Tero.Kristo@nokia.com
> Sent: Monday, October 19, 2009 5:19 AM
> 
> Anyway, I guess the optimization would look something like this:
> 
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 210a806..7a98321 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -93,6 +93,8 @@ u32 voltage_off_while_idle;
>                 OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
>                 OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
> 
> +#define INTC_SYSCONFIG                 0x10
> +
>  struct power_state {
>         struct powerdomain *pwrdm;
>         u32 next_state;
> @@ -505,6 +507,12 @@ void omap_sram_idle(void)
>                 prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
>                 omap3_enable_io_chain();
>         }
> +       /*
> +        * Disable INTC autoidle as it can cause interrupt controller
> +        * to enter unknown state with right combination of sleep / wakeup
> +        * transitions
> +        */
> +       omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
> 
>         /*
>         * On EMU/HS devices ROM code restores a SRDC value
> @@ -561,6 +569,8 @@ void omap_sram_idle(void)
>                                                 OMAP3430_GR_MOD,
>                                                 OMAP3_PRM_VOLTCTRL_OFFSET);
>         }
> +       /* Re-enable interrupt controller autoidle */
> +       omap_writel(OMAP3430_AUTOIDLE, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
> 
>         /*
>          * Enable smartreflex after WFI. Only needed if we


Ack from me.

-Girish


> --
> 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: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Kevin Hilman]

<Tero.Kristo@nokia.com> writes:

>>> 
>>> There was one report from a custom board when it made a 
>>difference.  I've not tried to double check
>>> this data.  I don't have the reporters setup so there is no 
>>guarantee I could reproduce anyway.  In
>>> general optimization seemed to make sense.
>>
>>I second it.
>>
>>
>>Regards,
>>Girish
>>
>>
>
> Is there errata number available for this issue by the way? I could attach this to the patch.
>
> Anyway, I guess the optimization would look something like this:
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 210a806..7a98321 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -93,6 +93,8 @@ u32 voltage_off_while_idle;
>                 OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
>                 OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
>
> +#define INTC_SYSCONFIG                 0x10
> +
>  struct power_state {
>         struct powerdomain *pwrdm;
>         u32 next_state;
> @@ -505,6 +507,12 @@ void omap_sram_idle(void)
>                 prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
>                 omap3_enable_io_chain();
>         }
> +       /*
> +        * Disable INTC autoidle as it can cause interrupt controller
> +        * to enter unknown state with right combination of sleep / wakeup
> +        * transitions
> +        */
> +       omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);

Except omap_write* functions are deprecated.

I'd rather see a call into the interrupt code.  Something like
omap_intc_prepare_idle()

Also, isn't this only needed if CORE != ON?

>         /*
>         * On EMU/HS devices ROM code restores a SRDC value
> @@ -561,6 +569,8 @@ void omap_sram_idle(void)
>                                                 OMAP3430_GR_MOD,
>                                                 OMAP3_PRM_VOLTCTRL_OFFSET);
>         }
> +       /* Re-enable interrupt controller autoidle */
> +       omap_writel(OMAP3430_AUTOIDLE, OMAP34XX_IC_BASE + INTC_SYSCONFIG);

and omap_intc_resume_idle()

Kevin

RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 20 October, 2009 19:02
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: girishsg@ti.com; r-woodruff2@ti.com; 
>linux-omap@vger.kernel.org; Hogander Jouni (Nokia-D/Tampere)
>Subject: Re: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in 
>interrupt controller
>
><Tero.Kristo@nokia.com> writes:
>
>>>> 
>>>> There was one report from a custom board when it made a 
>>>difference.  I've not tried to double check
>>>> this data.  I don't have the reporters setup so there is no 
>>>guarantee I could reproduce anyway.  In
>>>> general optimization seemed to make sense.
>>>
>>>I second it.
>>>
>>>
>>>Regards,
>>>Girish
>>>
>>>
>>
>> Is there errata number available for this issue by the way? 
>I could attach this to the patch.
>>
>> Anyway, I guess the optimization would look something like this:
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index 210a806..7a98321 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -93,6 +93,8 @@ u32 voltage_off_while_idle;
>>                 OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
>>                 OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
>>
>> +#define INTC_SYSCONFIG                 0x10
>> +
>>  struct power_state {
>>         struct powerdomain *pwrdm;
>>         u32 next_state;
>> @@ -505,6 +507,12 @@ void omap_sram_idle(void)
>>                 prm_set_mod_reg_bits(OMAP3430_EN_IO, 
>WKUP_MOD, PM_WKEN);
>>                 omap3_enable_io_chain();
>>         }
>> +       /*
>> +        * Disable INTC autoidle as it can cause interrupt controller
>> +        * to enter unknown state with right combination of 
>sleep / wakeup
>> +        * transitions
>> +        */
>> +       omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
>
>Except omap_write* functions are deprecated.

I see, will use __raw_writel here then.

>
>I'd rather see a call into the interrupt code.  Something like
>omap_intc_prepare_idle()

Can do these changes to add the interface yes. Though name would be omap3_intc_prepare_idle() / ...resume_idle() as this is only needed for OMAP3.

>
>Also, isn't this only needed if CORE != ON?

I understood from the problem description that this is needed always when we are entering WFI, because we might have L3/L4 sleep/run transitions there even if CORE is on.

>
>>         /*
>>         * On EMU/HS devices ROM code restores a SRDC value
>> @@ -561,6 +569,8 @@ void omap_sram_idle(void)
>>                                                 OMAP3430_GR_MOD,
>>                                                 
>OMAP3_PRM_VOLTCTRL_OFFSET);
>>         }
>> +       /* Re-enable interrupt controller autoidle */
>> +       omap_writel(OMAP3430_AUTOIDLE, OMAP34XX_IC_BASE + 
>INTC_SYSCONFIG);
>
>and omap_intc_resume_idle()


-Tero

Re: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Kevin Hilman]

<Tero.Kristo@nokia.com> writes:

[...]

>>>
>>> Anyway, I guess the optimization would look something like this:
>>>
>>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>>b/arch/arm/mach-omap2/pm34xx.c
>>> index 210a806..7a98321 100644
>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>> @@ -93,6 +93,8 @@ u32 voltage_off_while_idle;
>>>                 OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
>>>                 OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
>>>
>>> +#define INTC_SYSCONFIG                 0x10
>>> +
>>>  struct power_state {
>>>         struct powerdomain *pwrdm;
>>>         u32 next_state;
>>> @@ -505,6 +507,12 @@ void omap_sram_idle(void)
>>>                 prm_set_mod_reg_bits(OMAP3430_EN_IO, 
>>WKUP_MOD, PM_WKEN);
>>>                 omap3_enable_io_chain();
>>>         }
>>> +       /*
>>> +        * Disable INTC autoidle as it can cause interrupt controller
>>> +        * to enter unknown state with right combination of 
>>sleep / wakeup
>>> +        * transitions
>>> +        */
>>> +       omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
>>
>>Except omap_write* functions are deprecated.
>
> I see, will use __raw_writel here then.
>
>>
>>I'd rather see a call into the interrupt code.  Something like
>>omap_intc_prepare_idle()
>
> Can do these changes to add the interface yes. Though name would be omap3_intc_prepare_idle() / ...resume_idle() as this is only needed for OMAP3.

OK.

>>
>>Also, isn't this only needed if CORE != ON?
>
> I understood from the problem description that this is needed always when we are entering WFI, because we might have L3/L4 sleep/run transitions there even if CORE is on.
>

OK.

Kevin

RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Gopinath, Thara]

Treo,

>>@@ -505,6 +507,12 @@ void omap_sram_idle(void)
>>                prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
>>                omap3_enable_io_chain();
>>        }
>>+       /*
>>+        * Disable INTC autoidle as it can cause interrupt controller
>>+        * to enter unknown state with right combination of sleep / wakeup
>>+        * transitions
>>+        */
>>+       omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
>>
>>        /*
>>        * On EMU/HS devices ROM code restores a SRDC value
>>@@ -561,6 +569,8 @@ void omap_sram_idle(void)
>>                                                OMAP3430_GR_MOD,
>>                                                OMAP3_PRM_VOLTCTRL_OFFSET);
>>        }
>>+       /* Re-enable interrupt controller autoidle */
>>+       omap_writel(OMAP3430_AUTOIDLE, OMAP34XX_IC_BASE + INTC_SYSCONFIG);

Autoidle is being enabled inside the  if (core_next_state < PWRDM_POWER_ON). This is a bug because it is disabled irrespective of core pwr domain state. So if Core domain is on , this code will end up not enabling INTC autoidle during resume.

Regards
Thara

>>-----Original Message-----
>>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Kevin
>>Hilman
>>Sent: Tuesday, October 20, 2009 10:02 PM
>>To: Tero.Kristo@nokia.com
>>Cc: Ghongdemath, Girish; Woodruff, Richard; linux-omap@vger.kernel.org; jouni.hogander@nokia.com
>>Subject: Re: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller
>>
>><Tero.Kristo@nokia.com> writes:
>>
>>[...]
>>
>>>>>
>>>>> Anyway, I guess the optimization would look something like this:
>>>>>
>>>>> diff --git a/arch/arm/mach-omap2/pm34xx.c
>>>>b/arch/arm/mach-omap2/pm34xx.c
>>>>> index 210a806..7a98321 100644
>>>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>>>> @@ -93,6 +93,8 @@ u32 voltage_off_while_idle;
>>>>>                 OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
>>>>>                 OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
>>>>>
>>>>> +#define INTC_SYSCONFIG                 0x10
>>>>> +
>>>>>  struct power_state {
>>>>>         struct powerdomain *pwrdm;
>>>>>         u32 next_state;
>>>>> @@ -505,6 +507,12 @@ void omap_sram_idle(void)
>>>>>                 prm_set_mod_reg_bits(OMAP3430_EN_IO,
>>>>WKUP_MOD, PM_WKEN);
>>>>>                 omap3_enable_io_chain();
>>>>>         }
>>>>> +       /*
>>>>> +        * Disable INTC autoidle as it can cause interrupt controller
>>>>> +        * to enter unknown state with right combination of
>>>>sleep / wakeup
>>>>> +        * transitions
>>>>> +        */
>>>>> +       omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
>>>>
>>>>Except omap_write* functions are deprecated.
>>>
>>> I see, will use __raw_writel here then.
>>>
>>>>
>>>>I'd rather see a call into the interrupt code.  Something like
>>>>omap_intc_prepare_idle()
>>>
>>> Can do these changes to add the interface yes. Though name would be omap3_intc_prepare_idle() /
>>...resume_idle() as this is only needed for OMAP3.
>>
>>OK.
>>
>>>>
>>>>Also, isn't this only needed if CORE != ON?
>>>
>>> I understood from the problem description that this is needed always when we are entering WFI,
>>because we might have L3/L4 sleep/run transitions there even if CORE is on.
>>>
>>
>>OK.
>>
>>Kevin
>>--
>>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: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Girish S G]

> -----Original Message-----
> From: Gopinath, Thara [mailto:thara@ti.com]
> Sent: Tuesday, October 20, 2009 11:37 PM
> To: Kevin Hilman; Tero.Kristo@nokia.com
> Cc: Ghongdemath, Girish; Woodruff, Richard; linux-omap@vger.kernel.org; jouni.hogander@nokia.com
> Subject: RE: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller
> >>                                                OMAP3_PRM_VOLTCTRL_OFFSET);
> >>        }
> >>+       /* Re-enable interrupt controller autoidle */
> >>+       omap_writel(OMAP3430_AUTOIDLE, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
> 
> Autoidle is being enabled inside the  if (core_next_state < PWRDM_POWER_ON). This is a bug because it
> is disabled irrespective of core pwr domain state. So if Core domain is on , this code will end up
> not enabling INTC autoidle during resume.

It's outside "if (core_next_state < *)" check in restore path. I see this patch is dependent on some previous set of patches.

-Girish

> 
> >>-----Original Message-----
> >>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Kevin
> >>Hilman
> >>Sent: Tuesday, October 20, 2009 10:02 PM
> >>To: Tero.Kristo@nokia.com
> >>Cc: Ghongdemath, Girish; Woodruff, Richard; linux-omap@vger.kernel.org; jouni.hogander@nokia.com
> >>Subject: Re: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller
> >>
> >><Tero.Kristo@nokia.com> writes:
> >>
> >>[...]
> >>
> >>>>>
> >>>>> Anyway, I guess the optimization would look something like this:
> >>>>>
> >>>>> diff --git a/arch/arm/mach-omap2/pm34xx.c
> >>>>b/arch/arm/mach-omap2/pm34xx.c
> >>>>> index 210a806..7a98321 100644
> >>>>> --- a/arch/arm/mach-omap2/pm34xx.c
> >>>>> +++ b/arch/arm/mach-omap2/pm34xx.c
> >>>>> @@ -93,6 +93,8 @@ u32 voltage_off_while_idle;
> >>>>>                 OMAP3430_ST_GPT5_MASK|OMAP3430_ST_GPT4_MASK|\
> >>>>>                 OMAP3430_ST_GPT3_MASK|OMAP3430_ST_GPT2_MASK)
> >>>>>
> >>>>> +#define INTC_SYSCONFIG                 0x10
> >>>>> +
> >>>>>  struct power_state {
> >>>>>         struct powerdomain *pwrdm;
> >>>>>         u32 next_state;
> >>>>> @@ -505,6 +507,12 @@ void omap_sram_idle(void)
> >>>>>                 prm_set_mod_reg_bits(OMAP3430_EN_IO,
> >>>>WKUP_MOD, PM_WKEN);
> >>>>>                 omap3_enable_io_chain();
> >>>>>         }
> >>>>> +       /*
> >>>>> +        * Disable INTC autoidle as it can cause interrupt controller
> >>>>> +        * to enter unknown state with right combination of
> >>>>sleep / wakeup
> >>>>> +        * transitions
> >>>>> +        */
> >>>>> +       omap_writel(0x0, OMAP34XX_IC_BASE + INTC_SYSCONFIG);
> >>>>
> >>>>Except omap_write* functions are deprecated.
> >>>
> >>> I see, will use __raw_writel here then.
> >>>
> >>>>
> >>>>I'd rather see a call into the interrupt code.  Something like
> >>>>omap_intc_prepare_idle()
> >>>
> >>> Can do these changes to add the interface yes. Though name would be omap3_intc_prepare_idle() /
> >>...resume_idle() as this is only needed for OMAP3.
> >>
> >>OK.
> >>
> >>>>
> >>>>Also, isn't this only needed if CORE != ON?
> >>>
> >>> I understood from the problem description that this is needed always when we are entering WFI,
> >>because we might have L3/L4 sleep/run transitions there even if CORE is on.
> >>>
> >>
> >>OK.
> >>
> >>Kevin
> >>--
> >>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: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller

[Girish S G]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> Sent: Tuesday, October 20, 2009 11:02 AM
> To: Tero.Kristo@nokia.com
> Cc: girishsg@ti.com; r-woodruff2@ti.com; linux-omap@vger.kernel.org; jouni.hogander@nokia.com
> Subject: Re: [PATCH 12/17] OMAP2/3: Do not enable AUTOIDLE in interrupt controller
> 
> <Tero.Kristo@nokia.com> writes:
> 
> Except omap_write* functions are deprecated.
> 
> I'd rather see a call into the interrupt code.  Something like
> omap_intc_prepare_idle()
> 
> Also, isn't this only needed if CORE != ON?

In all the failure cases, the CORE was ON. It was programmed for RET/OFF but didn't really hit the state. Also, I have seen this
issue in C2 state as well. 

-Girish

Re: [PATCH 11/17] Make USBHOST powerdomain go to sleep after warm reset.

[Kevin Hilman]

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

> From: Peter De-Schrijver <peter.de-schrijver@nokia.com>
>
> This patch makes sure the USBHOST and SGX powerdomains can go to any sleep
> state after a warm reset.

Workaround looks ok, but the changelog should describe the cause of
problem.  IOW, why weren't these domains transitioning after a warm
reset?

Kevin

> Signed-off-by: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
> ---
>  arch/arm/mach-omap2/cm-regbits-34xx.h |    1 +
>  arch/arm/mach-omap2/pm34xx.c          |   18 ++++++++++++++++++
>  2 files changed, 19 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..3152553 100644
> --- a/arch/arm/mach-omap2/cm-regbits-34xx.h
> +++ b/arch/arm/mach-omap2/cm-regbits-34xx.h
> @@ -680,6 +680,7 @@
>  #define OMAP3430_CLKSEL_GPT2_SHIFT			0
>  
>  /* CM_SLEEPDEP_PER specific bits */
> +#define OMAP3430_CM_SLEEPDEP_PER_EN_MPU			(1 << 1)
>  #define OMAP3430_CM_SLEEPDEP_PER_EN_IVA2		(1 << 2)
>  
>  /* CM_CLKSTCTRL_PER */
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 47f5738..1fa778f 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -26,6 +26,7 @@
>  #include <linux/err.h>
>  #include <linux/gpio.h>
>  #include <linux/clk.h>
> +#include <linux/delay.h>
>  
>  #include <mach/sram.h>
>  #include <mach/prcm.h>
> @@ -934,6 +935,24 @@ static void __init prcm_setup_regs(void)
>  	prm_write_mod_reg(0, OMAP3430_CAM_MOD, PM_WKDEP);
>  	prm_write_mod_reg(0, OMAP3430_PER_MOD, PM_WKDEP);
>  	if (omap_rev() > OMAP3430_REV_ES1_0) {
> +
> +		/*
> +		 * This workaround is needed to prevent SGX and USBHOST from
> +		 * failing to transition to RET/OFF after a warm reset in OFF
> +		 * mode. Workaround sets a sleepdep of each of these domains
> +		 * with MPU, waits for a min 2 sysclk cycles and clears the
> +		 * sleepdep.
> +		 */
> +		cm_write_mod_reg(OMAP3430_CM_SLEEPDEP_PER_EN_MPU,
> +				OMAP3430ES2_USBHOST_MOD, OMAP3430_CM_SLEEPDEP);
> +		cm_write_mod_reg(OMAP3430_CM_SLEEPDEP_PER_EN_MPU,
> +				OMAP3430ES2_SGX_MOD, OMAP3430_CM_SLEEPDEP);
> +		udelay(100);
> +		cm_write_mod_reg(0, OMAP3430ES2_USBHOST_MOD,
> +				OMAP3430_CM_SLEEPDEP);
> +		cm_write_mod_reg(0, OMAP3430ES2_SGX_MOD,
> +				OMAP3430_CM_SLEEPDEP);
> +
>  		prm_write_mod_reg(0, OMAP3430ES2_SGX_MOD, PM_WKDEP);
>  		prm_write_mod_reg(0, OMAP3430ES2_USBHOST_MOD, PM_WKDEP);
>  	} else
> -- 
> 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: [PATCH 09/17] OMAP3: PM: Ack pending interrupts before entering suspend

[Kevin Hilman]

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

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Suspending drivers may still generate interrupts just before their suspend is
> completed. Any pending interrupts here will prevent sleep.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

This could also be done in omap3_intc_prepare_idle() hook.

Kevin

> ---
>  arch/arm/mach-omap2/irq.c              |    2 +-
>  arch/arm/mach-omap2/pm34xx.c           |    2 ++
>  arch/arm/plat-omap/include/mach/irqs.h |    1 +
>  3 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> index aceedd8..4ed05e9 100644
> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -101,7 +101,7 @@ static int omap_check_spurious(unsigned int irq)
>  }
>  
>  /* XXX: FIQ and additional INTC support (only MPU at the moment) */
> -static void omap_ack_irq(unsigned int irq)
> +void omap_ack_irq(unsigned int irq)
>  {
>  	intc_bank_write_reg(0x1, &irq_banks[0], INTC_CONTROL);
>  }
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 5854fa7..6a41811 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -778,6 +778,8 @@ static int omap3_pm_suspend(void)
>  
>  	omap_uart_prepare_suspend();
>  
> +	/* Ack pending IRQs, as a pending IRQ will cause the suspend to fail */
> +	omap_ack_irq(0);
>  	regset_save_on_suspend = 1;
>  	omap_sram_idle();
>  	regset_save_on_suspend = 0;
> diff --git a/arch/arm/plat-omap/include/mach/irqs.h b/arch/arm/plat-omap/include/mach/irqs.h
> index 2473910..d56be1c 100644
> --- a/arch/arm/plat-omap/include/mach/irqs.h
> +++ b/arch/arm/plat-omap/include/mach/irqs.h
> @@ -483,6 +483,7 @@
>  #ifndef __ASSEMBLY__
>  extern void omap_init_irq(void);
>  extern int omap_irq_pending(void);
> +extern void omap_ack_irq(unsigned int irq);
>  void omap3_intc_save_context(void);
>  void omap3_intc_restore_context(void);
>  #endif
> -- 
> 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: [PATCH 09/17] OMAP3: PM: Ack pending interrupts before entering suspend

[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: 20 October, 2009 20:31
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 09/17] OMAP3: PM: Ack pending interrupts 
>before entering suspend
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> Suspending drivers may still generate interrupts just before 
>their suspend is
>> completed. Any pending interrupts here will prevent sleep.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>
>This could also be done in omap3_intc_prepare_idle() hook.

I thought it is better to do this in suspend path only, because in normal sleep case we most likely don't want to miss any interrupts. In suspend case, we usually want to enter the suspend no matter what, and this is used here only to clean up the mess left by some of the drivers. The GPT case is one of the main things we try to fix here.

-Tero

>
>> ---
>>  arch/arm/mach-omap2/irq.c              |    2 +-
>>  arch/arm/mach-omap2/pm34xx.c           |    2 ++
>>  arch/arm/plat-omap/include/mach/irqs.h |    1 +
>>  3 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
>> index aceedd8..4ed05e9 100644
>> --- a/arch/arm/mach-omap2/irq.c
>> +++ b/arch/arm/mach-omap2/irq.c
>> @@ -101,7 +101,7 @@ static int omap_check_spurious(unsigned int irq)
>>  }
>>  
>>  /* XXX: FIQ and additional INTC support (only MPU at the moment) */
>> -static void omap_ack_irq(unsigned int irq)
>> +void omap_ack_irq(unsigned int irq)
>>  {
>>  	intc_bank_write_reg(0x1, &irq_banks[0], INTC_CONTROL);
>>  }
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index 5854fa7..6a41811 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -778,6 +778,8 @@ static int omap3_pm_suspend(void)
>>  
>>  	omap_uart_prepare_suspend();
>>  
>> +	/* Ack pending IRQs, as a pending IRQ will cause the 
>suspend to fail */
>> +	omap_ack_irq(0);
>>  	regset_save_on_suspend = 1;
>>  	omap_sram_idle();
>>  	regset_save_on_suspend = 0;
>> diff --git a/arch/arm/plat-omap/include/mach/irqs.h 
>b/arch/arm/plat-omap/include/mach/irqs.h
>> index 2473910..d56be1c 100644
>> --- a/arch/arm/plat-omap/include/mach/irqs.h
>> +++ b/arch/arm/plat-omap/include/mach/irqs.h
>> @@ -483,6 +483,7 @@
>>  #ifndef __ASSEMBLY__
>>  extern void omap_init_irq(void);
>>  extern int omap_irq_pending(void);
>> +extern void omap_ack_irq(unsigned int irq);
>>  void omap3_intc_save_context(void);
>>  void omap3_intc_restore_context(void);
>>  #endif
>> -- 
>> 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: [PATCH 09/17] OMAP3: PM: Ack pending interrupts before entering suspend

[Kevin Hilman]

<Tero.Kristo@nokia.com> writes:

>>-----Original Message-----
>>From: linux-omap-owner@vger.kernel.org 
>>[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of ext Kevin Hilman
>>Sent: 20 October, 2009 20:31
>>To: Kristo Tero (Nokia-D/Tampere)
>>Cc: linux-omap@vger.kernel.org
>>Subject: Re: [PATCH 09/17] OMAP3: PM: Ack pending interrupts 
>>before entering suspend
>>
>>Tero Kristo <tero.kristo@nokia.com> writes:
>>
>>> From: Tero Kristo <tero.kristo@nokia.com>
>>>
>>> Suspending drivers may still generate interrupts just before 
>>their suspend is
>>> completed. Any pending interrupts here will prevent sleep.
>>>
>>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>>
>>This could also be done in omap3_intc_prepare_idle() hook.
>
> I thought it is better to do this in suspend path only, because in
> normal sleep case we most likely don't want to miss any
> interrupts. 

Agreed, then I'd suggest doing this in omap3_intc_[suspend|resume]()
hooks.

> In suspend case, we usually want to enter the suspend no
> matter what, and this is used here only to clean up the mess left by
> some of the drivers. The GPT case is one of the main things we try
> to fix here.

So is this still needed with the GPTIMER fix in PATCH 8/17?

If there are other drivers having delayed interrupt triggering, it
sounds like the drivers need to be fixed instead of this brute force
approach.

Kevin

>
>>
>>> ---
>>>  arch/arm/mach-omap2/irq.c              |    2 +-
>>>  arch/arm/mach-omap2/pm34xx.c           |    2 ++
>>>  arch/arm/plat-omap/include/mach/irqs.h |    1 +
>>>  3 files changed, 4 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
>>> index aceedd8..4ed05e9 100644
>>> --- a/arch/arm/mach-omap2/irq.c
>>> +++ b/arch/arm/mach-omap2/irq.c
>>> @@ -101,7 +101,7 @@ static int omap_check_spurious(unsigned int irq)
>>>  }
>>>  
>>>  /* XXX: FIQ and additional INTC support (only MPU at the moment) */
>>> -static void omap_ack_irq(unsigned int irq)
>>> +void omap_ack_irq(unsigned int irq)
>>>  {
>>>  	intc_bank_write_reg(0x1, &irq_banks[0], INTC_CONTROL);
>>>  }
>>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>>b/arch/arm/mach-omap2/pm34xx.c
>>> index 5854fa7..6a41811 100644
>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>> @@ -778,6 +778,8 @@ static int omap3_pm_suspend(void)
>>>  
>>>  	omap_uart_prepare_suspend();
>>>  
>>> +	/* Ack pending IRQs, as a pending IRQ will cause the 
>>suspend to fail */
>>> +	omap_ack_irq(0);
>>>  	regset_save_on_suspend = 1;
>>>  	omap_sram_idle();
>>>  	regset_save_on_suspend = 0;
>>> diff --git a/arch/arm/plat-omap/include/mach/irqs.h 
>>b/arch/arm/plat-omap/include/mach/irqs.h
>>> index 2473910..d56be1c 100644
>>> --- a/arch/arm/plat-omap/include/mach/irqs.h
>>> +++ b/arch/arm/plat-omap/include/mach/irqs.h
>>> @@ -483,6 +483,7 @@
>>>  #ifndef __ASSEMBLY__
>>>  extern void omap_init_irq(void);
>>>  extern int omap_irq_pending(void);
>>> +extern void omap_ack_irq(unsigned int irq);
>>>  void omap3_intc_save_context(void);
>>>  void omap3_intc_restore_context(void);
>>>  #endif
>>> -- 
>>> 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: [PATCH 09/17] OMAP3: PM: Ack pending interrupts before entering suspend

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 21 October, 2009 17:20
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 09/17] OMAP3: PM: Ack pending interrupts 
>before entering suspend
>
><Tero.Kristo@nokia.com> writes:
>
>>>-----Original Message-----
>>>From: linux-omap-owner@vger.kernel.org 
>>>[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of ext 
>Kevin Hilman
>>>Sent: 20 October, 2009 20:31
>>>To: Kristo Tero (Nokia-D/Tampere)
>>>Cc: linux-omap@vger.kernel.org
>>>Subject: Re: [PATCH 09/17] OMAP3: PM: Ack pending interrupts 
>>>before entering suspend
>>>
>>>Tero Kristo <tero.kristo@nokia.com> writes:
>>>
>>>> From: Tero Kristo <tero.kristo@nokia.com>
>>>>
>>>> Suspending drivers may still generate interrupts just before 
>>>their suspend is
>>>> completed. Any pending interrupts here will prevent sleep.
>>>>
>>>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>>>
>>>This could also be done in omap3_intc_prepare_idle() hook.
>>
>> I thought it is better to do this in suspend path only, because in
>> normal sleep case we most likely don't want to miss any
>> interrupts. 
>
>Agreed, then I'd suggest doing this in omap3_intc_[suspend|resume]()
>hooks.
>
>> In suspend case, we usually want to enter the suspend no
>> matter what, and this is used here only to clean up the mess left by
>> some of the drivers. The GPT case is one of the main things we try
>> to fix here.
>
>So is this still needed with the GPTIMER fix in PATCH 8/17?

Yeah, it is not enough to ack the interrupt at module level, we still have the pending IRQ in the interrupt controller itself.

>
>If there are other drivers having delayed interrupt triggering, it
>sounds like the drivers need to be fixed instead of this brute force
>approach.

I think any system device can have similar issue if they receive an interrupt while we are entering suspend. The main problem is the sequence executed during suspend: interrupts are disabled before we suspend the system devices (e.g. clock source) and it is very possible for the devices to receive interrupts during this window. GPT is a special case because it also has internal buffering for the register writes, so it continues to tick a while after it is stopped. GPT is also special in a sense that it can trigger an interrupt at arbitrary points of time.

I don't know if similar issue exists on other platforms so changing the suspend sequence itself is probably not an option. And.. acking all interrupts at every suspending device is probably neither.

-Tero

>
>>
>>>
>>>> ---
>>>>  arch/arm/mach-omap2/irq.c              |    2 +-
>>>>  arch/arm/mach-omap2/pm34xx.c           |    2 ++
>>>>  arch/arm/plat-omap/include/mach/irqs.h |    1 +
>>>>  3 files changed, 4 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
>>>> index aceedd8..4ed05e9 100644
>>>> --- a/arch/arm/mach-omap2/irq.c
>>>> +++ b/arch/arm/mach-omap2/irq.c
>>>> @@ -101,7 +101,7 @@ static int 
>omap_check_spurious(unsigned int irq)
>>>>  }
>>>>  
>>>>  /* XXX: FIQ and additional INTC support (only MPU at the 
>moment) */
>>>> -static void omap_ack_irq(unsigned int irq)
>>>> +void omap_ack_irq(unsigned int irq)
>>>>  {
>>>>  	intc_bank_write_reg(0x1, &irq_banks[0], INTC_CONTROL);
>>>>  }
>>>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>>>b/arch/arm/mach-omap2/pm34xx.c
>>>> index 5854fa7..6a41811 100644
>>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>>> @@ -778,6 +778,8 @@ static int omap3_pm_suspend(void)
>>>>  
>>>>  	omap_uart_prepare_suspend();
>>>>  
>>>> +	/* Ack pending IRQs, as a pending IRQ will cause the 
>>>suspend to fail */
>>>> +	omap_ack_irq(0);
>>>>  	regset_save_on_suspend = 1;
>>>>  	omap_sram_idle();
>>>>  	regset_save_on_suspend = 0;
>>>> diff --git a/arch/arm/plat-omap/include/mach/irqs.h 
>>>b/arch/arm/plat-omap/include/mach/irqs.h
>>>> index 2473910..d56be1c 100644
>>>> --- a/arch/arm/plat-omap/include/mach/irqs.h
>>>> +++ b/arch/arm/plat-omap/include/mach/irqs.h
>>>> @@ -483,6 +483,7 @@
>>>>  #ifndef __ASSEMBLY__
>>>>  extern void omap_init_irq(void);
>>>>  extern int omap_irq_pending(void);
>>>> +extern void omap_ack_irq(unsigned int irq);
>>>>  void omap3_intc_save_context(void);
>>>>  void omap3_intc_restore_context(void);
>>>>  #endif
>>>> -- 
>>>> 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: [PATCH 08/17] OMAP2/3: GPTIMER: Clear pending interrupts when entering suspend

[Kevin Hilman]

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

> From: Tero Kristo <tero.kristo@nokia.com>
>
> OMAP GP timers keep running for a few cycles after they are stopped,
> which can cause the timer to expire and generate an interrupt. The pending
> interrupt will prevent OMAP from entering suspend, thus we ack it manually.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>
> ---
>  arch/arm/mach-omap2/timer-gp.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/timer-gp.c b/arch/arm/mach-omap2/timer-gp.c
> index 9c056ff..c9d47bb 100644
> --- a/arch/arm/mach-omap2/timer-gp.c
> +++ b/arch/arm/mach-omap2/timer-gp.c
> @@ -92,9 +92,21 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode mode,
>  	case CLOCK_EVT_MODE_ONESHOT:
>  		break;
>  	case CLOCK_EVT_MODE_UNUSED:
> -	case CLOCK_EVT_MODE_SHUTDOWN:
>  	case CLOCK_EVT_MODE_RESUME:
>  		break;
> +	case CLOCK_EVT_MODE_SHUTDOWN:
> +		/*
> +		 * Wait for min period x 2 to make sure that timer is
> +		 * stopped
> +		 */
> +		udelay(evt->min_delta_ns / 500);
> +		/*
> +		 * Clear possibly pending interrupt, this will occasionally
> +		 * generate spurious timer IRQs during suspend but this
> +		 * is okay, as another option is not to enter suspend at all
> +		 */
> +		omap_dm_timer_write_status(gptimer, OMAP_TIMER_INT_OVERFLOW);
> +		break;

Seems to me that this fix should just be done in omap_dm_timer_stop()
since it could also result in extra interrupts in when using oneshot
mode under dyntick.

Kevin

RE: [PATCH 08/17] OMAP2/3: GPTIMER: Clear pending interrupts when entering suspend

[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: 20 October, 2009 20:37
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 08/17] OMAP2/3: GPTIMER: Clear pending 
>interrupts when entering suspend
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> OMAP GP timers keep running for a few cycles after they are stopped,
>> which can cause the timer to expire and generate an 
>interrupt. The pending
>> interrupt will prevent OMAP from entering suspend, thus we 
>ack it manually.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>>
>> ---
>>  arch/arm/mach-omap2/timer-gp.c |   14 +++++++++++++-
>>  1 files changed, 13 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/timer-gp.c 
>b/arch/arm/mach-omap2/timer-gp.c
>> index 9c056ff..c9d47bb 100644
>> --- a/arch/arm/mach-omap2/timer-gp.c
>> +++ b/arch/arm/mach-omap2/timer-gp.c
>> @@ -92,9 +92,21 @@ static void omap2_gp_timer_set_mode(enum 
>clock_event_mode mode,
>>  	case CLOCK_EVT_MODE_ONESHOT:
>>  		break;
>>  	case CLOCK_EVT_MODE_UNUSED:
>> -	case CLOCK_EVT_MODE_SHUTDOWN:
>>  	case CLOCK_EVT_MODE_RESUME:
>>  		break;
>> +	case CLOCK_EVT_MODE_SHUTDOWN:
>> +		/*
>> +		 * Wait for min period x 2 to make sure that timer is
>> +		 * stopped
>> +		 */
>> +		udelay(evt->min_delta_ns / 500);
>> +		/*
>> +		 * Clear possibly pending interrupt, this will 
>occasionally
>> +		 * generate spurious timer IRQs during suspend but this
>> +		 * is okay, as another option is not to enter 
>suspend at all
>> +		 */
>> +		omap_dm_timer_write_status(gptimer, 
>OMAP_TIMER_INT_OVERFLOW);
>> +		break;
>
>Seems to me that this fix should just be done in omap_dm_timer_stop()
>since it could also result in extra interrupts in when using oneshot
>mode under dyntick.

Hmm yea, I can attempt to move this in there.

>
>Kevin
>
>--
>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: [PATCH 06/17] OMAP3: PM: Added next state check for IVA2, USB and PER into idle loop

[Sripathy, Vishwanath]

 >
 >-----Original Message-----
 >From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Tero Kristo
 >Sent: Friday, October 16, 2009 5:49 AM
 >To: linux-omap@vger.kernel.org
 >Subject: [PATCH 06/17] OMAP3: PM: Added next state check for IVA2, USB and PER into idle loop
 >
 >From: Tero Kristo <tero.kristo@nokia.com>
 >
 >This is needed to prevent core from entering off mode if one of the IVA2,
 >USB or PER powerdomains are going to stay at least in retention state.
 >If this is not done, a powerdomain waking from RET may access core
 >domain which has just been reset.
 >
>From PRCM HW point of view, if any of non core domains do not enter OFF mode, then Core cannot enter off mode. So I do not understand why you want to do this change.

 >Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
 >---
 > arch/arm/mach-omap2/pm34xx.c |   36 +++++++++++++++++++++++++++++++-----
 > 1 files changed, 31 insertions(+), 5 deletions(-)
 >
 >diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
 >index 588ab79..3e5ae14 100644
 >--- a/arch/arm/mach-omap2/pm34xx.c
 >+++ b/arch/arm/mach-omap2/pm34xx.c
 >@@ -85,6 +85,13 @@ u32 voltage_off_while_idle;
 > 		OMAP3430_ST_DES1_MASK)
 > #define CORE_IDLEST3_ALL		(\
 > 		OMAP3430ES2_ST_USBTLL_MASK|OMAP3430ES2_ST_CPEFUSE_MASK)
 >+#define PER_IDLEST_ALL			(\
 >+		OMAP3430_ST_WDT3_MASK|OMAP3430_ST_MCBSP4_MASK|\
 >+		OMAP3430_ST_MCBSP3_MASK|OMAP3430_ST_MCBSP2_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)
 > 
 > struct power_state {
 > 	struct powerdomain *pwrdm;
 >@@ -103,7 +110,7 @@ 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, *iva2_pwrdm, *dss_pwrdm;
 >+static struct powerdomain *cam_pwrdm, *iva2_pwrdm, *dss_pwrdm, *usb_pwrdm;
 > 
 > static struct prm_setup_vc prm_setup = {
 > 	.clksetup = 0xff,
 >@@ -428,6 +435,8 @@ void omap_sram_idle(void)
 > 	int per_next_state = PWRDM_POWER_ON;
 > 	int core_next_state = PWRDM_POWER_ON;
 > 	int dss_state = PWRDM_POWER_ON;
 >+	int iva2_state = PWRDM_POWER_ON;
 >+	int usb_state = PWRDM_POWER_ON;
 > 	int core_prev_state, per_prev_state;
 > 	u32 sdrc_pwr = 0;
 > 	int per_state_modified = 0;
 >@@ -463,14 +472,28 @@ void omap_sram_idle(void)
 > 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
 > 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
 > 
 >+	/* Get powerdomain state data */
 >+	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
 >+	dss_state = pwrdm_read_pwrst(dss_pwrdm);
 >+	iva2_state = pwrdm_read_pwrst(iva2_pwrdm);
 >+	usb_state = pwrdm_read_pwrst(usb_pwrdm);
 >+	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
 >+
 >+	/* Check if PER domain can enter OFF or not */
 >+	if (per_next_state == PWRDM_POWER_OFF) {
 >+		if ((cm_read_mod_reg(OMAP3430_PER_MOD, CM_IDLEST) &
 >+				PER_IDLEST_ALL) != PER_IDLEST_ALL) {
 >+			per_next_state = PWRDM_POWER_RET;
 >+			pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
 >+			per_state_modified = 1;
 >+		}
 >+	}
 > 	/*
 > 	 * Check whether core will enter idle or not. This is needed
 > 	 * because I/O pad wakeup will fail if core stays on and PER
 > 	 * enters off. This will also prevent unnecessary core context
 > 	 * save / restore.
 > 	 */
 >-	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
 >-	dss_state = pwrdm_read_pwrst(dss_pwrdm);
 > 	if (core_next_state < PWRDM_POWER_ON) {
 > 		core_saved_state = core_next_state;
 > 		if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & CORE_IDLEST1_ALL)
 >@@ -482,14 +505,16 @@ void omap_sram_idle(void)
 > 			core_next_state = PWRDM_POWER_ON;
 > 			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_ON);
 > 		} else if (core_next_state == PWRDM_POWER_OFF &&
 >-				 dss_state == PWRDM_POWER_ON) {
 >+				 (dss_state == PWRDM_POWER_ON ||
 >+				  iva2_state >= PWRDM_POWER_RET ||
 >+				  usb_state >= PWRDM_POWER_RET ||
 >+				  per_next_state >= PWRDM_POWER_RET)) {
 > 			core_next_state = PWRDM_POWER_RET;
 > 			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET);
 > 		}
 > 	}
 > 
 > 	/* PER */
 >-	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
 > 	if (per_next_state < PWRDM_POWER_ON) {
 > 		omap_uart_prepare_idle(2);
 > 		omap2_gpio_prepare_for_idle(per_next_state);
 >@@ -1240,6 +1265,7 @@ static int __init omap3_pm_init(void)
 > 	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
 > 	iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
 > 	dss_pwrdm = pwrdm_lookup("dss_pwrdm");
 >+	usb_pwrdm = pwrdm_lookup("usbhost_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: [PATCH 06/17] OMAP3: PM: Added next state check for IVA2, USB and PER into idle loop

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Sripathy, Vishwanath [mailto:vishwanath.bs@ti.com] 
>Sent: 16 October, 2009 16:16
>To: Kristo Tero (Nokia-D/Tampere); linux-omap@vger.kernel.org
>Subject: RE: [PATCH 06/17] OMAP3: PM: Added next state check 
>for IVA2, USB and PER into idle loop
>
>
> >
> >-----Original Message-----
> >From: linux-omap-owner@vger.kernel.org 
>[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Tero Kristo
> >Sent: Friday, October 16, 2009 5:49 AM
> >To: linux-omap@vger.kernel.org
> >Subject: [PATCH 06/17] OMAP3: PM: Added next state check for 
>IVA2, USB and PER into idle loop
> >
> >From: Tero Kristo <tero.kristo@nokia.com>
> >
> >This is needed to prevent core from entering off mode if one 
>of the IVA2,
> >USB or PER powerdomains are going to stay at least in 
>retention state.
> >If this is not done, a powerdomain waking from RET may access core
> >domain which has just been reset.
> >
>From PRCM HW point of view, if any of non core domains do not 
>enter OFF mode, then Core cannot enter off mode. So I do not 
>understand why you want to do this change.

It appears that at least if IVA2 domain is in retention, core can and will enter off. The PER domain clock check can be used to prevent unnecessary PER domain context save/restores also.


>
> >Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> >---
> > arch/arm/mach-omap2/pm34xx.c |   36 
>+++++++++++++++++++++++++++++++-----
> > 1 files changed, 31 insertions(+), 5 deletions(-)
> >
> >diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
> >index 588ab79..3e5ae14 100644
> >--- a/arch/arm/mach-omap2/pm34xx.c
> >+++ b/arch/arm/mach-omap2/pm34xx.c
> >@@ -85,6 +85,13 @@ u32 voltage_off_while_idle;
> > 		OMAP3430_ST_DES1_MASK)
> > #define CORE_IDLEST3_ALL		(\
> > 		OMAP3430ES2_ST_USBTLL_MASK|OMAP3430ES2_ST_CPEFUSE_MASK)
> >+#define PER_IDLEST_ALL			(\
> >+		OMAP3430_ST_WDT3_MASK|OMAP3430_ST_MCBSP4_MASK|\
> >+		OMAP3430_ST_MCBSP3_MASK|OMAP3430_ST_MCBSP2_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)
> > 
> > struct power_state {
> > 	struct powerdomain *pwrdm;
> >@@ -103,7 +110,7 @@ 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, *iva2_pwrdm, *dss_pwrdm;
> >+static struct powerdomain *cam_pwrdm, *iva2_pwrdm, 
>*dss_pwrdm, *usb_pwrdm;
> > 
> > static struct prm_setup_vc prm_setup = {
> > 	.clksetup = 0xff,
> >@@ -428,6 +435,8 @@ void omap_sram_idle(void)
> > 	int per_next_state = PWRDM_POWER_ON;
> > 	int core_next_state = PWRDM_POWER_ON;
> > 	int dss_state = PWRDM_POWER_ON;
> >+	int iva2_state = PWRDM_POWER_ON;
> >+	int usb_state = PWRDM_POWER_ON;
> > 	int core_prev_state, per_prev_state;
> > 	u32 sdrc_pwr = 0;
> > 	int per_state_modified = 0;
> >@@ -463,14 +472,28 @@ void omap_sram_idle(void)
> > 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
> > 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
> > 
> >+	/* Get powerdomain state data */
> >+	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
> >+	dss_state = pwrdm_read_pwrst(dss_pwrdm);
> >+	iva2_state = pwrdm_read_pwrst(iva2_pwrdm);
> >+	usb_state = pwrdm_read_pwrst(usb_pwrdm);
> >+	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
> >+
> >+	/* Check if PER domain can enter OFF or not */
> >+	if (per_next_state == PWRDM_POWER_OFF) {
> >+		if ((cm_read_mod_reg(OMAP3430_PER_MOD, CM_IDLEST) &
> >+				PER_IDLEST_ALL) != PER_IDLEST_ALL) {
> >+			per_next_state = PWRDM_POWER_RET;
> >+			pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
> >+			per_state_modified = 1;
> >+		}
> >+	}
> > 	/*
> > 	 * Check whether core will enter idle or not. This is needed
> > 	 * because I/O pad wakeup will fail if core stays on and PER
> > 	 * enters off. This will also prevent unnecessary core context
> > 	 * save / restore.
> > 	 */
> >-	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
> >-	dss_state = pwrdm_read_pwrst(dss_pwrdm);
> > 	if (core_next_state < PWRDM_POWER_ON) {
> > 		core_saved_state = core_next_state;
> > 		if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & 
>CORE_IDLEST1_ALL)
> >@@ -482,14 +505,16 @@ void omap_sram_idle(void)
> > 			core_next_state = PWRDM_POWER_ON;
> > 			pwrdm_set_next_pwrst(core_pwrdm, 
>PWRDM_POWER_ON);
> > 		} else if (core_next_state == PWRDM_POWER_OFF &&
> >-				 dss_state == PWRDM_POWER_ON) {
> >+				 (dss_state == PWRDM_POWER_ON ||
> >+				  iva2_state >= PWRDM_POWER_RET ||
> >+				  usb_state >= PWRDM_POWER_RET ||
> >+				  per_next_state >= PWRDM_POWER_RET)) {
> > 			core_next_state = PWRDM_POWER_RET;
> > 			pwrdm_set_next_pwrst(core_pwrdm, 
>PWRDM_POWER_RET);
> > 		}
> > 	}
> > 
> > 	/* PER */
> >-	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
> > 	if (per_next_state < PWRDM_POWER_ON) {
> > 		omap_uart_prepare_idle(2);
> > 		omap2_gpio_prepare_for_idle(per_next_state);
> >@@ -1240,6 +1265,7 @@ static int __init omap3_pm_init(void)
> > 	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
> > 	iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
> > 	dss_pwrdm = pwrdm_lookup("dss_pwrdm");
> >+	usb_pwrdm = pwrdm_lookup("usbhost_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: [PATCH 06/17] OMAP3: PM: Added next state check for IVA2, USB and PER into idle loop

[Cousson, Benoit]

>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>owner@vger.kernel.org] On Behalf Of Sripathy, Vishwanath
>Sent: Friday, October 16, 2009 3:16 PM
>To: Tero Kristo; linux-omap@vger.kernel.org
>Subject: RE: [PATCH 06/17] OMAP3: PM: Added next state check for IVA2, USB
>and PER into idle loop
>
> >From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>owner@vger.kernel.org] On Behalf Of Tero Kristo
> >Sent: Friday, October 16, 2009 5:49 AM
> >To: linux-omap@vger.kernel.org
> >Subject: [PATCH 06/17] OMAP3: PM: Added next state check for IVA2, USB
>and PER into idle loop
> >
> >From: Tero Kristo <tero.kristo@nokia.com>
> >
> >This is needed to prevent core from entering off mode if one of the IVA2,
> >USB or PER powerdomains are going to stay at least in retention state.
> >If this is not done, a powerdomain waking from RET may access core
> >domain which has just been reset.
> >
>From PRCM HW point of view, if any of non core domains do not enter OFF
>mode, then Core cannot enter off mode. So I do not understand why you want
>to do this change.

This is perfectly valid for the PRCM point of view to have the CORE OFF while another power domain is ON. The sleep dependency is at the clock domains level. It means that all initiators need to be mute and all targets must be in idle, but the power domain can be ON.

Regards,
Benoit


> >Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> >---
> > arch/arm/mach-omap2/pm34xx.c |   36 +++++++++++++++++++++++++++++++-----
> > 1 files changed, 31 insertions(+), 5 deletions(-)
> >
> >diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> >index 588ab79..3e5ae14 100644
> >--- a/arch/arm/mach-omap2/pm34xx.c
> >+++ b/arch/arm/mach-omap2/pm34xx.c
> >@@ -85,6 +85,13 @@ u32 voltage_off_while_idle;
> >             OMAP3430_ST_DES1_MASK)
> > #define CORE_IDLEST3_ALL            (\
> >             OMAP3430ES2_ST_USBTLL_MASK|OMAP3430ES2_ST_CPEFUSE_MASK)
> >+#define PER_IDLEST_ALL                      (\
> >+            OMAP3430_ST_WDT3_MASK|OMAP3430_ST_MCBSP4_MASK|\
> >+            OMAP3430_ST_MCBSP3_MASK|OMAP3430_ST_MCBSP2_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)
> >
> > struct power_state {
> >     struct powerdomain *pwrdm;
> >@@ -103,7 +110,7 @@ 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, *iva2_pwrdm, *dss_pwrdm;
> >+static struct powerdomain *cam_pwrdm, *iva2_pwrdm, *dss_pwrdm,
>*usb_pwrdm;
> >
> > static struct prm_setup_vc prm_setup = {
> >     .clksetup = 0xff,
> >@@ -428,6 +435,8 @@ void omap_sram_idle(void)
> >     int per_next_state = PWRDM_POWER_ON;
> >     int core_next_state = PWRDM_POWER_ON;
> >     int dss_state = PWRDM_POWER_ON;
> >+    int iva2_state = PWRDM_POWER_ON;
> >+    int usb_state = PWRDM_POWER_ON;
> >     int core_prev_state, per_prev_state;
> >     u32 sdrc_pwr = 0;
> >     int per_state_modified = 0;
> >@@ -463,14 +472,28 @@ void omap_sram_idle(void)
> >     if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
> >             pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
> >
> >+    /* Get powerdomain state data */
> >+    core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
> >+    dss_state = pwrdm_read_pwrst(dss_pwrdm);
> >+    iva2_state = pwrdm_read_pwrst(iva2_pwrdm);
> >+    usb_state = pwrdm_read_pwrst(usb_pwrdm);
> >+    per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
> >+
> >+    /* Check if PER domain can enter OFF or not */
> >+    if (per_next_state == PWRDM_POWER_OFF) {
> >+            if ((cm_read_mod_reg(OMAP3430_PER_MOD, CM_IDLEST) &
> >+                            PER_IDLEST_ALL) != PER_IDLEST_ALL) {
> >+                    per_next_state = PWRDM_POWER_RET;
> >+                    pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
> >+                    per_state_modified = 1;
> >+            }
> >+    }
> >     /*
> >      * Check whether core will enter idle or not. This is needed
> >      * because I/O pad wakeup will fail if core stays on and PER
> >      * enters off. This will also prevent unnecessary core context
> >      * save / restore.
> >      */
> >-    core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
> >-    dss_state = pwrdm_read_pwrst(dss_pwrdm);
> >     if (core_next_state < PWRDM_POWER_ON) {
> >             core_saved_state = core_next_state;
> >             if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & CORE_IDLEST1_ALL)
> >@@ -482,14 +505,16 @@ void omap_sram_idle(void)
> >                     core_next_state = PWRDM_POWER_ON;
> >                     pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_ON);
> >             } else if (core_next_state == PWRDM_POWER_OFF &&
> >-                             dss_state == PWRDM_POWER_ON) {
> >+                             (dss_state == PWRDM_POWER_ON ||
> >+                              iva2_state >= PWRDM_POWER_RET ||
> >+                              usb_state >= PWRDM_POWER_RET ||
> >+                              per_next_state >= PWRDM_POWER_RET)) {
> >                     core_next_state = PWRDM_POWER_RET;
> >                     pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET);
> >             }
> >     }
> >
> >     /* PER */
> >-    per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
> >     if (per_next_state < PWRDM_POWER_ON) {
> >             omap_uart_prepare_idle(2);
> >             omap2_gpio_prepare_for_idle(per_next_state);
> >@@ -1240,6 +1265,7 @@ static int __init omap3_pm_init(void)
> >     cam_pwrdm = pwrdm_lookup("cam_pwrdm");
> >     iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
> >     dss_pwrdm = pwrdm_lookup("dss_pwrdm");
> >+    usb_pwrdm = pwrdm_lookup("usbhost_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
> >
> >
>--
>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


Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920

Re: [PATCH 06/17] OMAP3: PM: Added next state check for IVA2, USB and PER into idle loop

[Kevin Hilman]

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

> From: Tero Kristo <tero.kristo@nokia.com>
>
> This is needed to prevent core from entering off mode if one of the IVA2,
> USB or PER powerdomains are going to stay at least in retention state.
> If this is not done, a powerdomain waking from RET may access core
> domain which has just been reset.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

The problem I have with this one is simalar to patches 2 and 3.

It think all this state changing should be part of the CPUidle "bus
master" checking.  If any of the conditions are met, then a different
C-state should be picked.  With the current approach, the manual
prevention of CORE-off of will never be accounted for in CPUidle
stats.

Also, intead of all the manual checking of IDLEST bits, this should be
pushed into the powerdomain layer.  Maybe we need a function like
pwrdm_is_idle() which can check the per-pwrdm IDLEST bits.

Then the BM check could simply call pwrdm_is_idle() on each of DSS,
IVA2, USB and PER for the C-states where CORE is going OFF. If any are
not idle, then a C-state with CORE=RET will be picked and the CPUidle
accounting will properly reflect the choice.

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c |   36 +++++++++++++++++++++++++++++++-----
>  1 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 588ab79..3e5ae14 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -85,6 +85,13 @@ u32 voltage_off_while_idle;
>  		OMAP3430_ST_DES1_MASK)
>  #define CORE_IDLEST3_ALL		(\
>  		OMAP3430ES2_ST_USBTLL_MASK|OMAP3430ES2_ST_CPEFUSE_MASK)
> +#define PER_IDLEST_ALL			(\
> +		OMAP3430_ST_WDT3_MASK|OMAP3430_ST_MCBSP4_MASK|\
> +		OMAP3430_ST_MCBSP3_MASK|OMAP3430_ST_MCBSP2_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)
>  
>  struct power_state {
>  	struct powerdomain *pwrdm;
> @@ -103,7 +110,7 @@ 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, *iva2_pwrdm, *dss_pwrdm;
> +static struct powerdomain *cam_pwrdm, *iva2_pwrdm, *dss_pwrdm, *usb_pwrdm;
>  
>  static struct prm_setup_vc prm_setup = {
>  	.clksetup = 0xff,
> @@ -428,6 +435,8 @@ void omap_sram_idle(void)
>  	int per_next_state = PWRDM_POWER_ON;
>  	int core_next_state = PWRDM_POWER_ON;
>  	int dss_state = PWRDM_POWER_ON;
> +	int iva2_state = PWRDM_POWER_ON;
> +	int usb_state = PWRDM_POWER_ON;
>  	int core_prev_state, per_prev_state;
>  	u32 sdrc_pwr = 0;
>  	int per_state_modified = 0;
> @@ -463,14 +472,28 @@ void omap_sram_idle(void)
>  	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
>  		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
>  
> +	/* Get powerdomain state data */
> +	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
> +	dss_state = pwrdm_read_pwrst(dss_pwrdm);
> +	iva2_state = pwrdm_read_pwrst(iva2_pwrdm);
> +	usb_state = pwrdm_read_pwrst(usb_pwrdm);
> +	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
> +
> +	/* Check if PER domain can enter OFF or not */
> +	if (per_next_state == PWRDM_POWER_OFF) {
> +		if ((cm_read_mod_reg(OMAP3430_PER_MOD, CM_IDLEST) &
> +				PER_IDLEST_ALL) != PER_IDLEST_ALL) {
> +			per_next_state = PWRDM_POWER_RET;
> +			pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
> +			per_state_modified = 1;
> +		}
> +	}
>  	/*
>  	 * Check whether core will enter idle or not. This is needed
>  	 * because I/O pad wakeup will fail if core stays on and PER
>  	 * enters off. This will also prevent unnecessary core context
>  	 * save / restore.
>  	 */
> -	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
> -	dss_state = pwrdm_read_pwrst(dss_pwrdm);
>  	if (core_next_state < PWRDM_POWER_ON) {
>  		core_saved_state = core_next_state;
>  		if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & CORE_IDLEST1_ALL)
> @@ -482,14 +505,16 @@ void omap_sram_idle(void)
>  			core_next_state = PWRDM_POWER_ON;
>  			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_ON);
>  		} else if (core_next_state == PWRDM_POWER_OFF &&
> -				 dss_state == PWRDM_POWER_ON) {
> +				 (dss_state == PWRDM_POWER_ON ||
> +				  iva2_state >= PWRDM_POWER_RET ||
> +				  usb_state >= PWRDM_POWER_RET ||
> +				  per_next_state >= PWRDM_POWER_RET)) {
>  			core_next_state = PWRDM_POWER_RET;
>  			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET);
>  		}
>  	}
>  
>  	/* PER */
> -	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		omap_uart_prepare_idle(2);
>  		omap2_gpio_prepare_for_idle(per_next_state);
> @@ -1240,6 +1265,7 @@ static int __init omap3_pm_init(void)
>  	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>  	iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
>  	dss_pwrdm = pwrdm_lookup("dss_pwrdm");
> +	usb_pwrdm = pwrdm_lookup("usbhost_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: [PATCH 05/17] OMAP:PM: Precedence fix

[Kevin Hilman]

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

> From: Carlos Chinea <carlos.chinea@nokia.com>
>
> Signed-off-by: Carlos Chinea <carlos.chinea@nokia.com>
> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>

Patch looks good, but needs better subject and a changelog.

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 4addc41..588ab79 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -225,8 +225,8 @@ static void omap3_core_save_context(void)
>  	control_padconf_off |= START_PADCONF_SAVE;
>  	omap_ctrl_writel(control_padconf_off, OMAP343X_CONTROL_PADCONF_OFF);
>  	/* wait for the save to complete */
> -	while (!omap_ctrl_readl(OMAP343X_CONTROL_GENERAL_PURPOSE_STATUS)
> -			& PADCONF_SAVE_DONE)
> +	while (!(omap_ctrl_readl(OMAP343X_CONTROL_GENERAL_PURPOSE_STATUS)
> +			& PADCONF_SAVE_DONE))
>  		;
>  	/* Save the Interrupt controller context */
>  	omap3_intc_save_context();
> -- 
> 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: [PATCH 04/17] OMAP3: PM: Do not enable IO wake-up on boot

[Kevin Hilman]

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

> From: Jouni Hogander <jouni.hogander@nokia.com>
>
> IO wake-ups are enabled/disabled in idle loop. No need to enable them on boot.
>
> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>

Thanks, pulling into PM branch.

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 52a3f2b..4addc41 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -1027,9 +1027,8 @@ static void __init prcm_setup_regs(void)
>  			     OMAP3_PRM_CLKSRC_CTRL_OFFSET);
>  
>  	/* setup wakup source */
> -	prm_write_mod_reg(OMAP3430_EN_IO | OMAP3430_EN_GPIO1 |
> -			  OMAP3430_EN_GPT1 | OMAP3430_EN_GPT12,
> -			  WKUP_MOD, PM_WKEN);
> +	prm_write_mod_reg(OMAP3430_EN_GPIO1 | OMAP3430_EN_GPT1 |
> +			  OMAP3430_EN_GPT12, WKUP_MOD, PM_WKEN);
>  	/* No need to write EN_IO, that is always enabled */
>  	prm_write_mod_reg(OMAP3430_EN_GPIO1 | OMAP3430_EN_GPT1 |
>  			  OMAP3430_EN_GPT12,
> -- 
> 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: [PATCH 03/17] PM: Block CORE off when DSS is active

[Cousson, Benoit]

Hi Tero,

>
Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920

-----Original Message-----

>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>owner@vger.kernel.org] On Behalf Of Tero Kristo
>Sent: Friday, October 16, 2009 12:49 PM
>To: linux-omap@vger.kernel.org
>Cc: Kalle Jokiniemi
>Subject: [PATCH 03/17] PM: Block CORE off when DSS is active
>
>From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
>
>If CORE power domain is allowed to reach off state
>while display is active, the display will go blank.
>Fixed by adding a check in idle loop.

What is root cause? I guess it is due to extra latency required to wakeup from OFF?
In that case, it should be better to use the omap_pm_set_max_dev_wakeup_lat() function and set a latency constraint to the CORE.

On the other hand, I remember a discussion between Kevin and Tomi saying that this API might not be fully functional yet.

Regards,
Benoit


>Signed-off-by: Kalle Jokiniemi <ext-kalle.jokiniemi@nokia.com>
>Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
>---
> arch/arm/mach-omap2/pm34xx.c |    9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
>diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>index e8704a6..52a3f2b 100644
>--- a/arch/arm/mach-omap2/pm34xx.c
>+++ b/arch/arm/mach-omap2/pm34xx.c
>@@ -103,7 +103,7 @@ 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, *iva2_pwrdm;
>+static struct powerdomain *cam_pwrdm, *iva2_pwrdm, *dss_pwrdm;
>
> static struct prm_setup_vc prm_setup = {
>       .clksetup = 0xff,
>@@ -427,6 +427,7 @@ void omap_sram_idle(void)
>       int mpu_next_state = PWRDM_POWER_ON;
>       int per_next_state = PWRDM_POWER_ON;
>       int core_next_state = PWRDM_POWER_ON;
>+      int dss_state = PWRDM_POWER_ON;
>       int core_prev_state, per_prev_state;
>       u32 sdrc_pwr = 0;
>       int per_state_modified = 0;
>@@ -469,6 +470,7 @@ void omap_sram_idle(void)
>        * save / restore.
>        */
>       core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
>+      dss_state = pwrdm_read_pwrst(dss_pwrdm);
>       if (core_next_state < PWRDM_POWER_ON) {
>               core_saved_state = core_next_state;
>               if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & CORE_IDLEST1_ALL)
>@@ -479,6 +481,10 @@ void omap_sram_idle(void)
>                               CORE_IDLEST3_ALL) != CORE_IDLEST3_ALL) {
>                       core_next_state = PWRDM_POWER_ON;
>                       pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_ON);
>+              } else if (core_next_state == PWRDM_POWER_OFF &&
>+                               dss_state == PWRDM_POWER_ON) {
>+                      core_next_state = PWRDM_POWER_RET;
>+                      pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET);
>               }
>       }
>
>@@ -1234,6 +1240,7 @@ static int __init omap3_pm_init(void)
>       core_pwrdm = pwrdm_lookup("core_pwrdm");
>       cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>       iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
>+      dss_pwrdm = pwrdm_lookup("dss_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: [PATCH 03/17] PM: Block CORE off when DSS is active

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Cousson, Benoit [mailto:b-cousson@ti.com] 
>Sent: 16 October, 2009 18:27
>To: Kristo Tero (Nokia-D/Tampere); linux-omap@vger.kernel.org
>Cc: Kalle Jokiniemi
>Subject: RE: [PATCH 03/17] PM: Block CORE off when DSS is active
>
>Hi Tero,
>
>>
>Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 
>Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920
>
>-----Original Message-----
>
>>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>>owner@vger.kernel.org] On Behalf Of Tero Kristo
>>Sent: Friday, October 16, 2009 12:49 PM
>>To: linux-omap@vger.kernel.org
>>Cc: Kalle Jokiniemi
>>Subject: [PATCH 03/17] PM: Block CORE off when DSS is active
>>
>>From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
>>
>>If CORE power domain is allowed to reach off state
>>while display is active, the display will go blank.
>>Fixed by adding a check in idle loop.
>
>What is root cause? I guess it is due to extra latency 
>required to wakeup from OFF?
>In that case, it should be better to use the 
>omap_pm_set_max_dev_wakeup_lat() function and set a latency 
>constraint to the CORE.

I believe the root cause is that if core enters off while display is doing something (between DSS data bursts for example), then when the display next wakes up and tries to access memories, core has lost its context which is rather bad. It is not really a wakeup latency issue as such.

>On the other hand, I remember a discussion between Kevin and 
>Tomi saying that this API might not be fully functional yet.
>
>Regards,
>Benoit
>
>
>>Signed-off-by: Kalle Jokiniemi <ext-kalle.jokiniemi@nokia.com>
>>Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
>>---
>> arch/arm/mach-omap2/pm34xx.c |    9 ++++++++-
>> 1 files changed, 8 insertions(+), 1 deletions(-)
>>
>>diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>>index e8704a6..52a3f2b 100644
>>--- a/arch/arm/mach-omap2/pm34xx.c
>>+++ b/arch/arm/mach-omap2/pm34xx.c
>>@@ -103,7 +103,7 @@ 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, *iva2_pwrdm;
>>+static struct powerdomain *cam_pwrdm, *iva2_pwrdm, *dss_pwrdm;
>>
>> static struct prm_setup_vc prm_setup = {
>>       .clksetup = 0xff,
>>@@ -427,6 +427,7 @@ void omap_sram_idle(void)
>>       int mpu_next_state = PWRDM_POWER_ON;
>>       int per_next_state = PWRDM_POWER_ON;
>>       int core_next_state = PWRDM_POWER_ON;
>>+      int dss_state = PWRDM_POWER_ON;
>>       int core_prev_state, per_prev_state;
>>       u32 sdrc_pwr = 0;
>>       int per_state_modified = 0;
>>@@ -469,6 +470,7 @@ void omap_sram_idle(void)
>>        * save / restore.
>>        */
>>       core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
>>+      dss_state = pwrdm_read_pwrst(dss_pwrdm);
>>       if (core_next_state < PWRDM_POWER_ON) {
>>               core_saved_state = core_next_state;
>>               if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & 
>CORE_IDLEST1_ALL)
>>@@ -479,6 +481,10 @@ void omap_sram_idle(void)
>>                               CORE_IDLEST3_ALL) != 
>CORE_IDLEST3_ALL) {
>>                       core_next_state = PWRDM_POWER_ON;
>>                       pwrdm_set_next_pwrst(core_pwrdm, 
>PWRDM_POWER_ON);
>>+              } else if (core_next_state == PWRDM_POWER_OFF &&
>>+                               dss_state == PWRDM_POWER_ON) {
>>+                      core_next_state = PWRDM_POWER_RET;
>>+                      pwrdm_set_next_pwrst(core_pwrdm, 
>PWRDM_POWER_RET);
>>               }
>>       }
>>
>>@@ -1234,6 +1240,7 @@ static int __init omap3_pm_init(void)
>>       core_pwrdm = pwrdm_lookup("core_pwrdm");
>>       cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>>       iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
>>+      dss_pwrdm = pwrdm_lookup("dss_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: [PATCH 03/17] PM: Block CORE off when DSS is active

[Kevin Hilman]

<Tero.Kristo@nokia.com> writes:

>>>
>>>From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
>>>
>>>If CORE power domain is allowed to reach off state
>>>while display is active, the display will go blank.
>>>Fixed by adding a check in idle loop.
>>
>>What is root cause? I guess it is due to extra latency 
>>required to wakeup from OFF?
>>In that case, it should be better to use the 
>>omap_pm_set_max_dev_wakeup_lat() function and set a latency 
>>constraint to the CORE.
>
> I believe the root cause is that if core enters off while display is
> doing something (between DSS data bursts for example), then when the
> display next wakes up and tries to access memories, core has lost
> its context which is rather bad. It is not really a wakeup latency
> issue as such.

Still trying to fully understand...

So when the DSS wakes and tries to access memories, CORE should be
woken up from OFF but the context restore (done by MPU) will not have
happenend resulting in badness.  

If that's the case, this really should be a constraint set by the DSS
driver.  I'm not crazy about adding more special case handling in the
idle path.

While it's not technically a latency constraint we could use
omap_pm_set_max_mpu_wakeup_lat() in the DSS driver with a FIXME
comment to the effect that its not a latency constraint but rather a
prevent CORE context loss type of constraint.

Longer term, we'll need a new OMAP PM function for this.  Something like

    omap_pm_set_maximum_context_loss(struct device *initiator,
                                     strict device *target,
                                     u32 count);

Where count is 0 to prevent context loss and 1 to allow.  In this case,
the initiator would be the dss device and the target would be a new
device used for L3.   Comments?

Kevin

>
>>On the other hand, I remember a discussion between Kevin and 
>>Tomi saying that this API might not be fully functional yet.
>>
>>Regards,
>>Benoit
>>
>>
>>>Signed-off-by: Kalle Jokiniemi <ext-kalle.jokiniemi@nokia.com>
>>>Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
>>>---
>>> arch/arm/mach-omap2/pm34xx.c |    9 ++++++++-
>>> 1 files changed, 8 insertions(+), 1 deletions(-)
>>>
>>>diff --git a/arch/arm/mach-omap2/pm34xx.c 
>>b/arch/arm/mach-omap2/pm34xx.c
>>>index e8704a6..52a3f2b 100644
>>>--- a/arch/arm/mach-omap2/pm34xx.c
>>>+++ b/arch/arm/mach-omap2/pm34xx.c
>>>@@ -103,7 +103,7 @@ 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, *iva2_pwrdm;
>>>+static struct powerdomain *cam_pwrdm, *iva2_pwrdm, *dss_pwrdm;
>>>
>>> static struct prm_setup_vc prm_setup = {
>>>       .clksetup = 0xff,
>>>@@ -427,6 +427,7 @@ void omap_sram_idle(void)
>>>       int mpu_next_state = PWRDM_POWER_ON;
>>>       int per_next_state = PWRDM_POWER_ON;
>>>       int core_next_state = PWRDM_POWER_ON;
>>>+      int dss_state = PWRDM_POWER_ON;
>>>       int core_prev_state, per_prev_state;
>>>       u32 sdrc_pwr = 0;
>>>       int per_state_modified = 0;
>>>@@ -469,6 +470,7 @@ void omap_sram_idle(void)
>>>        * save / restore.
>>>        */
>>>       core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
>>>+      dss_state = pwrdm_read_pwrst(dss_pwrdm);
>>>       if (core_next_state < PWRDM_POWER_ON) {
>>>               core_saved_state = core_next_state;
>>>               if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & 
>>CORE_IDLEST1_ALL)
>>>@@ -479,6 +481,10 @@ void omap_sram_idle(void)
>>>                               CORE_IDLEST3_ALL) != 
>>CORE_IDLEST3_ALL) {
>>>                       core_next_state = PWRDM_POWER_ON;
>>>                       pwrdm_set_next_pwrst(core_pwrdm, 
>>PWRDM_POWER_ON);
>>>+              } else if (core_next_state == PWRDM_POWER_OFF &&
>>>+                               dss_state == PWRDM_POWER_ON) {
>>>+                      core_next_state = PWRDM_POWER_RET;
>>>+                      pwrdm_set_next_pwrst(core_pwrdm, 
>>PWRDM_POWER_RET);
>>>               }
>>>       }
>>>
>>>@@ -1234,6 +1240,7 @@ static int __init omap3_pm_init(void)
>>>       core_pwrdm = pwrdm_lookup("core_pwrdm");
>>>       cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>>>       iva2_pwrdm = pwrdm_lookup("iva2_pwrdm");
>>>+      dss_pwrdm = pwrdm_lookup("dss_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
>>
>>
>>--
> 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: [PATCH 02/17] OMAP3: PM: Dynamic check for CORE target state

[Kevin Hilman]

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

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Sleep code will now check if core will actually enter power save state or
> not. This is needed so that PER domain does not enter retention / off while
> core stays on and thus preventing I/O pad wakeups.
>
> This also optimizes core context save and restore logic, it will skip
> saving / restoring core context when this is requested but not needed.

I understand this being useful to save an extra save/restore but not
sure about the wakeup issue.

If CORE stays ON, then yes, IO pad will not be armed so IOpad wakeups
will not work.  But module level wakeups should work fine.  It seems
the bug is actually that module level wakeups are not properly
configured.

There were definitely problems in the PM branch with module level
wakeups.  We now have fixes at least for GPIO and UART3 module level
wakeups which were found to be broken if CORE was left on.

This is easy enough to test/debug by forcing CORE to say on:

   # echo 3 > /debug/pm_debug/core_pwrdm/suspend

going into suspend and testing the various wakeup sources.

Kevin


> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
> ---
>  arch/arm/mach-omap2/cm.h     |    1 +
>  arch/arm/mach-omap2/pm34xx.c |   46 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 46 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h
> index a2fcfcc..715ab14 100644
> --- a/arch/arm/mach-omap2/cm.h
> +++ b/arch/arm/mach-omap2/cm.h
> @@ -76,6 +76,7 @@
>  #define OMAP3430ES2_CM_CLKEN2				0x0004
>  #define OMAP3430ES2_CM_FCLKEN3				0x0008
>  #define OMAP3430_CM_IDLEST_PLL				CM_IDLEST2
> +#define OMAP3430_CM_IDLEST3				0x0028
>  #define OMAP3430_CM_AUTOIDLE_PLL			CM_AUTOIDLE2
>  #define OMAP3430ES2_CM_AUTOIDLE2_PLL			CM_AUTOIDLE2
>  #define OMAP3430_CM_CLKSEL1				CM_CLKSEL
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 5e2ef63..e8704a6 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -64,6 +64,28 @@ u32 sleep_while_idle;
>  u32 wakeup_timer_seconds;
>  u32 voltage_off_while_idle;
>  
> +/* IDLEST bitmasks for core status checks */
> +#define CORE_IDLEST1_ALL		(\
> +		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_HSOTGUSB_STDBY_MASK|\
> +		OMAP3430ES2_ST_SSI_IDLE_MASK|OMAP3430_ST_SDMA_MASK|\
> +		OMAP3430_ST_SSI_STDBY_MASK|OMAP3430_ST_D2D_MASK)
> +#define CORE_IDLEST2_ALL		(\
> +		OMAP3430_ST_PKA_MASK|OMAP3430_ST_AES1_MASK|\
> +		OMAP3430_ST_RNG_MASK|OMAP3430_ST_SHA11_MASK|\
> +		OMAP3430_ST_DES1_MASK)
> +#define CORE_IDLEST3_ALL		(\
> +		OMAP3430ES2_ST_USBTLL_MASK|OMAP3430ES2_ST_CPEFUSE_MASK)
> +
>  struct power_state {
>  	struct powerdomain *pwrdm;
>  	u32 next_state;
> @@ -408,6 +430,7 @@ void omap_sram_idle(void)
>  	int core_prev_state, per_prev_state;
>  	u32 sdrc_pwr = 0;
>  	int per_state_modified = 0;
> +	int core_saved_state = PWRDM_POWER_ON;
>  
>  	if (!_omap_sram_idle)
>  		return;
> @@ -439,9 +462,28 @@ void omap_sram_idle(void)
>  	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
>  		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
>  
> +	/*
> +	 * Check whether core will enter idle or not. This is needed
> +	 * because I/O pad wakeup will fail if core stays on and PER
> +	 * enters off. This will also prevent unnecessary core context
> +	 * save / restore.
> +	 */
> +	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
> +	if (core_next_state < PWRDM_POWER_ON) {
> +		core_saved_state = core_next_state;
> +		if ((cm_read_mod_reg(CORE_MOD, CM_IDLEST1) & CORE_IDLEST1_ALL)
> +				!= CORE_IDLEST1_ALL ||
> +		    (cm_read_mod_reg(CORE_MOD, CM_IDLEST2) & CORE_IDLEST2_ALL)
> +				!= CORE_IDLEST2_ALL ||
> +		    (cm_read_mod_reg(CORE_MOD, OMAP3430_CM_IDLEST3) &
> +				CORE_IDLEST3_ALL) != CORE_IDLEST3_ALL) {
> +			core_next_state = PWRDM_POWER_ON;
> +			pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_ON);
> +		}
> +	}
> +
>  	/* PER */
>  	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
> -	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		omap_uart_prepare_idle(2);
>  		omap2_gpio_prepare_for_idle(per_next_state);
> @@ -539,6 +581,8 @@ void omap_sram_idle(void)
>  		enable_smartreflex(SR1);
>  		enable_smartreflex(SR2);
>  	}
> +	if (core_saved_state != core_next_state)
> +		pwrdm_set_next_pwrst(core_pwrdm, core_saved_state);
>  
>  	/* PER */
>  	if (per_next_state < PWRDM_POWER_ON) {
> -- 
> 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: [PATCH 01/17] PM: fix suspend control for IVA2

[Girish S G]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Tero
> Kristo
> 
> From: Tero Kristo <tero.kristo@nokia.com>
> 
> IVA2 controls its target power state individually, thus suspend should not
> touch IVA2. Without this patch DSP suspend always fails.
> 
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> Acked-by: Ameya Palande <ameya.palande@nokia.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
>  static struct prm_setup_vc prm_setup = {
>  	.clksetup = 0xff,
> @@ -676,6 +676,12 @@ static int omap3_pm_suspend(void)
>  		pwrst->saved_state = pwrdm_read_next_pwrst(pwrst->pwrdm);
>  	/* Set ones wanted by suspend */
>  	list_for_each_entry(pwrst, &pwrst_list, node) {
> +		/* Special handling for IVA2, just use current sleep state */
> +		if (pwrst->pwrdm == iva2_pwrdm) {
> +			state = pwrdm_read_pwrst(pwrst->pwrdm);
> +			if (state < PWRDM_POWER_ON)
> +				pwrst->next_state = state;
> +		}

Agree, IVA2 pwrdm is handled autonomously by bridge. I think this needs some additional change to remove all the redundant
configuration of iva pwdm. There are some inconsistencies like, 
	- Say enable_off_mode is disabled. Before doing system wide suspend if DSP hibernates then IVA2 will be put to OFF. In that
case we have IVA2 going to OFF and other domains in RET. This might not be an issue, but it's bad from sytem PM framework integrity
perspective.
	- enable_off_mode->omap3_pm_off_mode_enable will also touch IVA2 power domain next state. This we don't want to do if dsp
bridge is already taking care of IVA2.

IMO, we need to have some mechanism wherein if bridge PM takes care of IVA then PM framework should not configure the IVA
powerstate. It should only do if bridge PM is disabled.


-Girish

  

RE: [PATCH 01/17] PM: fix suspend control for IVA2

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Girish S G [mailto:girishsg@ti.com] 
>Sent: 16 October, 2009 20:16
>To: Kristo Tero (Nokia-D/Tampere); linux-omap@vger.kernel.org
>Subject: RE: [PATCH 01/17] PM: fix suspend control for IVA2
>
>
>
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org 
>[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Tero
>> Kristo
>> 
>> From: Tero Kristo <tero.kristo@nokia.com>
>> 
>> IVA2 controls its target power state individually, thus 
>suspend should not
>> touch IVA2. Without this patch DSP suspend always fails.
>> 
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> Acked-by: Ameya Palande <ameya.palande@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |    9 ++++++++-
>>  1 files changed, 8 insertions(+), 1 deletions(-)
>> 
>>  static struct prm_setup_vc prm_setup = {
>>  	.clksetup = 0xff,
>> @@ -676,6 +676,12 @@ static int omap3_pm_suspend(void)
>>  		pwrst->saved_state = 
>pwrdm_read_next_pwrst(pwrst->pwrdm);
>>  	/* Set ones wanted by suspend */
>>  	list_for_each_entry(pwrst, &pwrst_list, node) {
>> +		/* Special handling for IVA2, just use current 
>sleep state */
>> +		if (pwrst->pwrdm == iva2_pwrdm) {
>> +			state = pwrdm_read_pwrst(pwrst->pwrdm);
>> +			if (state < PWRDM_POWER_ON)
>> +				pwrst->next_state = state;
>> +		}
>
>Agree, IVA2 pwrdm is handled autonomously by bridge. I think 
>this needs some additional change to remove all the redundant
>configuration of iva pwdm. There are some inconsistencies like, 
>	- Say enable_off_mode is disabled. Before doing system 
>wide suspend if DSP hibernates then IVA2 will be put to OFF. In that
>case we have IVA2 going to OFF and other domains in RET. This 
>might not be an issue, but it's bad from sytem PM framework integrity
>perspective.

This is an issue with bridge driver, and I am not sure how this should be fixed. Currently bridge driver does not care whether off mode is enabled or not.

>	- enable_off_mode->omap3_pm_off_mode_enable will also 
>touch IVA2 power domain next state. This we don't want to do if dsp
>bridge is already taking care of IVA2.
>
>IMO, we need to have some mechanism wherein if bridge PM takes 
>care of IVA then PM framework should not configure the IVA
>powerstate. It should only do if bridge PM is disabled.

Should we have a Kconfig option for this? Like CONFIG_OMAP3_BRIDGE_PM, and disable all iva2 controls from pm34xx.c if it is enabled? Otherwise control IVA2 as currently done.

-Tero

RE: [PATCH 01/17] PM: fix suspend control for IVA2

[Girish S G]

> -----Original Message-----
> From: Tero.Kristo@nokia.com [mailto:Tero.Kristo@nokia.com]
> Sent: Monday, October 19, 2009 4:23 AM
> To: girishsg@ti.com; linux-omap@vger.kernel.org
> Subject: RE: [PATCH 01/17] PM: fix suspend control for IVA2

> >
> >Agree, IVA2 pwrdm is handled autonomously by bridge. I think
> >this needs some additional change to remove all the redundant
> >configuration of iva pwdm. There are some inconsistencies like,
> >	- Say enable_off_mode is disabled. Before doing system
> >wide suspend if DSP hibernates then IVA2 will be put to OFF. In that
> >case we have IVA2 going to OFF and other domains in RET. This
> >might not be an issue, but it's bad from sytem PM framework integrity
> >perspective.
> 
> This is an issue with bridge driver, and I am not sure how this should be fixed. Currently bridge
> driver does not care whether off mode is enabled or not.

I have seen bridge considering enable_off_mode flag in suspend/resume path. But while hibernation (idle timeout) it goes to OFF,
irrespective of the OFF flag.

> 
> >	- enable_off_mode->omap3_pm_off_mode_enable will also
> >touch IVA2 power domain next state. This we don't want to do if dsp
> >bridge is already taking care of IVA2.
> >
> >IMO, we need to have some mechanism wherein if bridge PM takes
> >care of IVA then PM framework should not configure the IVA
> >powerstate. It should only do if bridge PM is disabled.
> 
> Should we have a Kconfig option for this? Like CONFIG_OMAP3_BRIDGE_PM, and disable all iva2 controls
> from pm34xx.c if it is enabled? Otherwise control IVA2 as currently done.

Yes, this looks ok to me.


-Girish

Re: [PATCH 01/17] PM: fix suspend control for IVA2

[Kevin Hilman]

<Tero.Kristo@nokia.com> writes:

>>-----Original Message-----
>>From: ext Girish S G [mailto:girishsg@ti.com] 
>>Sent: 16 October, 2009 20:16
>>To: Kristo Tero (Nokia-D/Tampere); linux-omap@vger.kernel.org
>>Subject: RE: [PATCH 01/17] PM: fix suspend control for IVA2
>>
>>
>>
>>> -----Original Message-----
>>> From: linux-omap-owner@vger.kernel.org 
>>[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Tero
>>> Kristo
>>> 
>>> From: Tero Kristo <tero.kristo@nokia.com>
>>> 
>>> IVA2 controls its target power state individually, thus 
>>suspend should not
>>> touch IVA2. Without this patch DSP suspend always fails.
>>> 
>>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>>> Acked-by: Ameya Palande <ameya.palande@nokia.com>
>>> ---
>>>  arch/arm/mach-omap2/pm34xx.c |    9 ++++++++-
>>>  1 files changed, 8 insertions(+), 1 deletions(-)
>>> 
>>>  static struct prm_setup_vc prm_setup = {
>>>  	.clksetup = 0xff,
>>> @@ -676,6 +676,12 @@ static int omap3_pm_suspend(void)
>>>  		pwrst->saved_state = 
>>pwrdm_read_next_pwrst(pwrst->pwrdm);
>>>  	/* Set ones wanted by suspend */
>>>  	list_for_each_entry(pwrst, &pwrst_list, node) {
>>> +		/* Special handling for IVA2, just use current 
>>sleep state */
>>> +		if (pwrst->pwrdm == iva2_pwrdm) {
>>> +			state = pwrdm_read_pwrst(pwrst->pwrdm);
>>> +			if (state < PWRDM_POWER_ON)
>>> +				pwrst->next_state = state;
>>> +		}
>>
>>Agree, IVA2 pwrdm is handled autonomously by bridge. I think 
>>this needs some additional change to remove all the redundant
>>configuration of iva pwdm. There are some inconsistencies like, 
>>	- Say enable_off_mode is disabled. Before doing system 
>>wide suspend if DSP hibernates then IVA2 will be put to OFF. In that
>>case we have IVA2 going to OFF and other domains in RET. This 
>>might not be an issue, but it's bad from sytem PM framework integrity
>>perspective.
>
> This is an issue with bridge driver, and I am not sure how this should be fixed. Currently bridge driver does not care whether off mode is enabled or not.
>
>>	- enable_off_mode->omap3_pm_off_mode_enable will also 
>>touch IVA2 power domain next state. This we don't want to do if dsp
>>bridge is already taking care of IVA2.
>>
>>IMO, we need to have some mechanism wherein if bridge PM takes 
>>care of IVA then PM framework should not configure the IVA
>>powerstate. It should only do if bridge PM is disabled.
>
> Should we have a Kconfig option for this? Like CONFIG_OMAP3_BRIDGE_PM, and disable all iva2 controls from pm34xx.c if it is enabled? Otherwise control IVA2 as currently done.
>

Rather than Kconfig, this should be run-time.

There should be some sort of flag for indpendent IVA2 control and a
register function that bridge (or any other DSP controller) could
register for and take over control of IVA2.

This way, if no bridge is loaded, IVA2 can still properly be suspended.

Kevin

Re: [PATCH 01/17] PM: fix suspend control for IVA2

[Paul Walmsley]

Hi Tero, Ameya, Girish,

On Fri, 16 Oct 2009, Tero Kristo wrote:

> From: Tero Kristo <tero.kristo@nokia.com>
> 
> IVA2 controls its target power state individually, thus suspend should not
> touch IVA2. Without this patch DSP suspend always fails.

We don't allow other device drivers to touch PRCM bits, so we should 
probably should remove all PRCM register accesses from the DSPBridge code, 
so all power control should go through the ARM.

Is there a reason why the ARM code can't handle the DSP powerdomain?


- Paul

RE: [PATCH 01/17] PM: fix suspend control for IVA2

[Woodruff, Richard]

> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Paul Walmsley
> Sent: Thursday, October 22, 2009 5:24 AM

> > IVA2 controls its target power state individually, thus suspend should not
> > touch IVA2. Without this patch DSP suspend always fails.
>
> We don't allow other device drivers to touch PRCM bits, so we should
> probably should remove all PRCM register accesses from the DSPBridge code,
> so all power control should go through the ARM.
>
> Is there a reason why the ARM code can't handle the DSP powerdomain?

Sharing with DSP is something which probably could use some improvement.

Today DSP self-manages its domain.  Its (bios) micro-kernel makes decisions to optimize its domain. The ARM can't really micro-manage the DSP as he doesn't even want to know at the detail level what the DSP is up to at every instant.

- During idle time cpuidle should just be checking dsp status to see if its current state gets in the way of a low c-state.

- bridge does register with suspend frame work so he should do the right thing when in the system.

* problem is when bridge isn't there what to do.  This is especially after an unload of the bridge.

What has been proposed in the past is like what Kevin inputted in related thread about having maintenance hand off.  But for some reason it never quite to the top of the list.

- bridge does request thought clock frame work clocks especially of those which are public peripherals.

- bridge could request everything but it was not projected as power efficient waking up the big arm core for something the dsp could do itself and has all control over.  This is especially true if you have dsp doing the rendering for something like mp3.  it gets the wake ups and streaming and only every great while wakes the arm to give it a pile more data.  Waking the arm every time it runs its equivalent of cpuidle was discourged.

- main other sharing conflict was with irq routing between arm and bridge.  Right now its kind of init mode setup.  I guess this is ok for todays usage.

...... so after context current code is not requesting through pm code purposefully.  The hardware has been evolving from omap1,2,3,4 to make for more of a distributed model.  After all the details/constraints are understood with silicon there is some time to re-evaluate if its paying back or not.

Regards,
Richard W.

RE: [PATCH 01/17] PM: fix suspend control for IVA2

[Artem Bityutskiy]

On Thu, 2009-10-22 at 16:21 -0500, Woodruff, Richard wrote:
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Paul Walmsley
> > Sent: Thursday, October 22, 2009 5:24 AM
> 
> > > IVA2 controls its target power state individually, thus suspend should not
> > > touch IVA2. Without this patch DSP suspend always fails.
> >
> > We don't allow other device drivers to touch PRCM bits, so we should
> > probably should remove all PRCM register accesses from the DSPBridge code,
> > so all power control should go through the ARM.
> >
> > Is there a reason why the ARM code can't handle the DSP powerdomain?
> 
> Sharing with DSP is something which probably could use some improvement.
> 
> Today DSP self-manages its domain.  Its (bios) micro-kernel makes decisions to optimize its domain. The ARM can't really micro-manage the DSP as he doesn't even want to know at the detail level what the DSP is up to at every instant.
> 
> - During idle time cpuidle should just be checking dsp status to see if its current state gets in the way of a low c-state.
> 
> - bridge does register with suspend frame work so he should do the right thing when in the system.
> 
> * problem is when bridge isn't there what to do.  This is especially after an unload of the bridge.

Just a side note reminder: bridge does not exist in the real world.

[dedekind@sauron linux-omap-2.6]\$ ls drivers/dsp/bridge
ls: cannot access drivers/dsp/bridge: No such file or directory

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

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