OMAP2/3: PM: remove unnecessary wakeup/sleep dependency clear

PATCH New Paul Walmsley 2 participants 5 messages Jan 27, 2011 – Feb 11, 2011

Subsystems (4)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP power management support

lore

Share & tools

lore

Paul Walmsley

author

2011-01-27 09:52 UTC #0 lore raw

PATCH 1/1 OMAP2/3: PM: remove unnecessary wakeup/sleep dependency clear

The OMAP2 and OMAP3 PM code clears clockdomain wakeup and sleep dependencies. This is unnecessary after commit 6f7f63cc9adf3192e6fcac4e8bed5cc10fd924aa ("OMAP clockdomain:
initialize clockdomain registers when the clockdomain layer starts") which clears these dependencies during clockdomain init.

Signed-off-by Paul Walmsley <[email protected]>

Cc Kevin Hilman <[email protected]>

---
Full-chip retention idle entry tested on OMAP35xx Beagleboard.

 arch/arm/mach-omap2/pm24xx.c |    8 +-------
 arch/arm/mach-omap2/pm34xx.c |   15 ---------------
 2 files changed, 1 insertions(+), 22 deletions(-)

arch/arm/mach-omap2/pm24xx.c +1 -7

index 97feb3a..10f8747 100644
@@ -363,9 +363,6 @@ static const struct platform_suspend_ops __initdata omap_pm_ops;
 /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
 static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
 {
-	clkdm_clear_all_wkdeps(clkdm);
-	clkdm_clear_all_sleepdeps(clkdm);
-
 	if (clkdm->flags & CLKDM_CAN_ENABLE_AUTO)
 		omap2_clkdm_allow_idle(clkdm);
 	else if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP &&

@@ -411,10 +408,7 @@ static void __init prcm_setup_regs(void)
 	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_OFF);
 	omap2_clkdm_sleep(gfx_clkdm);
 
-	/*
-	 * Clear clockdomain wakeup dependencies and enable
-	 * hardware-supervised idle for all clkdms
-	 */
+	/* Enable hardware-supervised idle for all clkdms */
 	clkdm_for_each(clkdms_setup, NULL);
 	clkdm_add_wkdep(mpu_clkdm, wkup_clkdm);

arch/arm/mach-omap2/pm34xx.c +0 ~~-15~~

index a4aa192..0eb9738 100644
@@ -694,21 +694,6 @@ static void __init prcm_setup_regs(void)
 	u32 omap3630_grpsel_uart4_mask = cpu_is_omap3630() ?
 					OMAP3630_GRPSEL_UART4_MASK : 0;
 
-
-	/* XXX Reset all wkdeps. This should be done when initializing
-	 * powerdomains */
-	omap2_prm_write_mod_reg(0, OMAP3430_IVA2_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, MPU_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_DSS_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_NEON_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_CAM_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_PER_MOD, PM_WKDEP);
-	if (omap_rev() > OMAP3430_REV_ES1_0) {
-		omap2_prm_write_mod_reg(0, OMAP3430ES2_SGX_MOD, PM_WKDEP);
-		omap2_prm_write_mod_reg(0, OMAP3430ES2_USBHOST_MOD, PM_WKDEP);
-	} else
-		omap2_prm_write_mod_reg(0, GFX_MOD, PM_WKDEP);
-
 	/*
 	 * Enable interface clock autoidle for all modules.
 	 * Note that in the long run this should be done by clockfw

Signature · 1 line

1.7.2.3

Kevin Hilman

2011-01-27 18:25 UTC #1 lore raw

PATCH 1/1 OMAP2/3: PM: remove unnecessary wakeup/sleep dependency clear

Quoted 10 lines · Paul Walmsley

The OMAP2 and OMAP3 PM code clears clockdomain wakeup and sleep dependencies. This is unnecessary after commit 6f7f63cc9adf3192e6fcac4e8bed5cc10fd924aa ("OMAP clockdomain:
initialize clockdomain registers when the clockdomain layer starts") which clears these dependencies during clockdomain init.
Signed-off-by Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
Full-chip retention idle entry tested on OMAP35xx Beagleboard.

Also tested on 3630/Zoom3.

Queueing for 2.6.39. Thanks,

Kevin

Quoted 56 lines

 arch/arm/mach-omap2/pm24xx.c |    8 +-------
 arch/arm/mach-omap2/pm34xx.c |   15 ---------------
 2 files changed, 1 insertions(+), 22 deletions(-)

arch/arm/mach-omap2/pm24xx.c +1 -7

index 97feb3a..10f8747 100644
@@ -363,9 +363,6 @@ static const struct platform_suspend_ops __initdata omap_pm_ops;
 /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
 static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
 {
-	clkdm_clear_all_wkdeps(clkdm);
-	clkdm_clear_all_sleepdeps(clkdm);
-
 	if (clkdm->flags & CLKDM_CAN_ENABLE_AUTO)
 		omap2_clkdm_allow_idle(clkdm);
 	else if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP &&

@@ -411,10 +408,7 @@ static void __init prcm_setup_regs(void)
 	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_OFF);
 	omap2_clkdm_sleep(gfx_clkdm);
 
-	/*
-	 * Clear clockdomain wakeup dependencies and enable
-	 * hardware-supervised idle for all clkdms
-	 */
+	/* Enable hardware-supervised idle for all clkdms */
 	clkdm_for_each(clkdms_setup, NULL);
 	clkdm_add_wkdep(mpu_clkdm, wkup_clkdm);

arch/arm/mach-omap2/pm34xx.c +0 ~~-15~~

index a4aa192..0eb9738 100644
@@ -694,21 +694,6 @@ static void __init prcm_setup_regs(void)
 	u32 omap3630_grpsel_uart4_mask = cpu_is_omap3630() ?
 					OMAP3630_GRPSEL_UART4_MASK : 0;
 
-
-	/* XXX Reset all wkdeps. This should be done when initializing
-	 * powerdomains */
-	omap2_prm_write_mod_reg(0, OMAP3430_IVA2_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, MPU_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_DSS_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_NEON_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_CAM_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_PER_MOD, PM_WKDEP);
-	if (omap_rev() > OMAP3430_REV_ES1_0) {
-		omap2_prm_write_mod_reg(0, OMAP3430ES2_SGX_MOD, PM_WKDEP);
-		omap2_prm_write_mod_reg(0, OMAP3430ES2_USBHOST_MOD, PM_WKDEP);
-	} else
-		omap2_prm_write_mod_reg(0, GFX_MOD, PM_WKDEP);
-
 	/*
 	 * Enable interface clock autoidle for all modules.
 	 * Note that in the long run this should be done by clockfw

Kevin Hilman

2011-02-10 23:11 UTC #2 lore raw

PATCH 1/1 OMAP2/3: PM: remove unnecessary wakeup/sleep dependency clear

Hi Paul,

Quoted 8 lines · Kevin Hilman

Quoted 6 lines · Paul Walmsley
The OMAP2 and OMAP3 PM code clears clockdomain wakeup and sleep dependencies. This is unnecessary after commit 6f7f63cc9adf3192e6fcac4e8bed5cc10fd924aa ("OMAP clockdomain:
initialize clockdomain registers when the clockdomain layer starts") which clears these dependencies during clockdomain init.

Quoted 9 lines

Quoted 4 lines
Signed-off-by Paul Walmsley <[email protected]>
Cc Kevin Hilman <[email protected]>
---
Full-chip retention idle entry tested on OMAP35xx Beagleboard.
Also tested on 3630/Zoom3.
Queueing for 2.6.39. Thanks,

While testing, I found out this patch breaks suspend/resume and idle on at least 34xx.

A first suspend/resume works fine, but on subsequent ones SGX powerdomain does not hit retention (and thus CORE powerdomain does not either....

More below...

Quoted 39 lines

 arch/arm/mach-omap2/pm24xx.c |    8 +-------
 arch/arm/mach-omap2/pm34xx.c |   15 ---------------
 2 files changed, 1 insertions(+), 22 deletions(-)

arch/arm/mach-omap2/pm24xx.c +1 -7

index 97feb3a..10f8747 100644
@@ -363,9 +363,6 @@ static const struct platform_suspend_ops __initdata omap_pm_ops;
 /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
 static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
 {
-	clkdm_clear_all_wkdeps(clkdm);
-	clkdm_clear_all_sleepdeps(clkdm);
-
 	if (clkdm->flags & CLKDM_CAN_ENABLE_AUTO)
 		omap2_clkdm_allow_idle(clkdm);
 	else if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP &&

@@ -411,10 +408,7 @@ static void __init prcm_setup_regs(void)
 	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_OFF);
 	omap2_clkdm_sleep(gfx_clkdm);
 
-	/*
-	 * Clear clockdomain wakeup dependencies and enable
-	 * hardware-supervised idle for all clkdms
-	 */
+	/* Enable hardware-supervised idle for all clkdms */
 	clkdm_for_each(clkdms_setup, NULL);
 	clkdm_add_wkdep(mpu_clkdm, wkup_clkdm);

arch/arm/mach-omap2/pm34xx.c +0 -0

index a4aa192..0eb9738 100644
@@ -694,21 +694,6 @@ static void __init prcm_setup_regs(void)
 	u32 omap3630_grpsel_uart4_mask = cpu_is_omap3630() ?
 					OMAP3630_GRPSEL_UART4_MASK : 0;

Quoted 14 lines

Quoted 4 lines

arch/arm/mach-omap2/pm34xx.c +0 -4

-
-	/* XXX Reset all wkdeps. This should be done when initializing
-	 * powerdomains */
-	omap2_prm_write_mod_reg(0, OMAP3430_IVA2_MOD, PM_WKDEP);

arch/arm/mach-omap2/pm34xx.c static void __init prcm_setup_regs(void) · line 710

Quoted 10 lines

arch/arm/mach-omap2/pm34xx.c +0 ~~-10~~

-	omap2_prm_write_mod_reg(0, MPU_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_DSS_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_NEON_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_CAM_MOD, PM_WKDEP);
-	omap2_prm_write_mod_reg(0, OMAP3430_PER_MOD, PM_WKDEP);
-	if (omap_rev() > OMAP3430_REV_ES1_0) {
-		omap2_prm_write_mod_reg(0, OMAP3430ES2_SGX_MOD, PM_WKDEP);
-		omap2_prm_write_mod_reg(0, OMAP3430ES2_USBHOST_MOD, PM_WKDEP);
-	} else
-		omap2_prm_write_mod_reg(0, GFX_MOD, PM_WKDEP);

Simply adding back the above 5 lines (the SGX ones) gets things working again, which suggests something isn't quite right with SGX, but I haven't found the exact reason yet.

Kevin

Quoted 3 lines

arch/arm/mach-omap2/pm34xx.c +0 -0

 	/*
 	 * Enable interface clock autoidle for all modules.
 	 * Note that in the long run this should be done by clockfw

Paul Walmsley

author

2011-02-10 23:17 UTC #3 lore raw

PATCH 1/1 OMAP2/3: PM: remove unnecessary wakeup/sleep dependency clear

Hi Kevin,

Quoted 6 lines · Kevin Hilman

While testing, I found out this patch breaks suspend/resume and idle on at least 34xx.
A first suspend/resume works fine, but on subsequent ones SGX powerdomain does not hit retention (and thus CORE powerdomain does not either....

...

Quoted 3 lines

Simply adding back the above 5 lines (the SGX ones) gets things working again, which suggests something isn't quite right with SGX, but I haven't found the exact reason yet.

Thanks for the report, I will take a look.

- Paul

Paul Walmsley

author

2011-02-11 01:39 UTC #4 lore raw

PATCH 1/1 OMAP2/3: PM: remove unnecessary wakeup/sleep dependency clear

Quoted 9 lines · Paul Walmsley

Quoted 6 lines · Kevin Hilman
While testing, I found out this patch breaks suspend/resume and idle on at least 34xx.
A first suspend/resume works fine, but on subsequent ones SGX powerdomain does not hit retention (and thus CORE powerdomain does not either....

Quoted 7 lines

...
Quoted 3 lines
Simply adding back the above 5 lines (the SGX ones) gets things working again, which suggests something isn't quite right with SGX, but I haven't found the exact reason yet.
Thanks for the report, I will take a look.

Kevin and I did some debugging on this. It turns out that there is a bug in the recent clockdomain abstraction code in the git.pwsan.com integration branch that caused the underlying problem. Will follow up with comments on the threads of the original patches.

Kevin is planning to keep the patch under discussion in this thread in his 2.6.39 queue...

- Paul