>-----Original Message----- >From: ext Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >Sent: 02 March, 2010 01:43 >To: Kristo Tero (Nokia-D/Tampere) >Cc: linux-omap@xxxxxxxxxxxxxxx >Subject: Re: [PATCHv6 9/9] OMAP3: PM: Added support for >suspending to INACTIVE state > >Tero Kristo <tero.kristo@xxxxxxxxx> writes: > >> From: Tero Kristo <tero.kristo@xxxxxxxxx> >> >> With the new support functions this is now possible. >Suspending to INACTIVE >> is useful for testing purposes. >> >> Signed-off-by: Tero Kristo <tero.kristo@xxxxxxxxx> >> --- >> arch/arm/mach-omap2/pm34xx.c | 11 ++++++----- >> 1 files changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/pm34xx.c >b/arch/arm/mach-omap2/pm34xx.c >> index cdbedcf..41d6cc3 100644 >> --- a/arch/arm/mach-omap2/pm34xx.c >> +++ b/arch/arm/mach-omap2/pm34xx.c >> @@ -633,11 +633,12 @@ int set_pwrdm_state(struct powerdomain >*pwrdm, u32 state) >> if (pwrdm == NULL || IS_ERR(pwrdm)) >> return -EINVAL; >> >> - while (!(pwrdm->pwrsts & (1 << state))) { >> - if (state == PWRDM_POWER_OFF) >> - return ret; >> - state--; >> - } > >The comment above set_pwrdm_state() says only ON & RET are supported. >Please update that comment as well. True, ancient info there. OFF for example has been supported for ages already. > > >> + if (state != PWRDM_POWER_INACTIVE) >> + while (!(pwrdm->pwrsts & (1 << state))) { >> + if (state == PWRDM_POWER_OFF) >> + return ret; >> + state--; >> + } > >I think all powerdomains can be inactive right? Yes. >I think it would be cleaner to just have all the pwrdm->pwrsts fields >include intactive as a valid option. > >Something like the patch below. IIRC, you did something like this in >one of the earlier versions of the patch. Yeah, something like this was done previously, however Paul did not like the idea of changing the generic powerdomain code too much so I dropped it completely. It is now done only via the support functions in patch #1, and only done for the powerdomains that actually need it for the cpuidle (mpu/core/neon.) It would be possible to add support for the rest of the powerdomains also, but I decided to drop this in favor of getting the patch set in. > >Kevin > >diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h >b/arch/arm/plat-omap/include/plat/powerdomain.h >index a1ecd47..c692472 100644 >--- a/arch/arm/plat-omap/include/plat/powerdomain.h >+++ b/arch/arm/plat-omap/include/plat/powerdomain.h >@@ -31,17 +31,17 @@ > #define PWRDM_MAX_PWRSTS 4 > > /* Powerdomain allowable state bitfields */ >-#define PWRSTS_OFF_ON ((1 << PWRDM_POWER_OFF) | \ >+#define PWRSTS_ON ((1 << PWRDM_POWER_INACTIVE) | \ > (1 << PWRDM_POWER_ON)) > >+#define PWRSTS_OFF_ON ((1 << PWRDM_POWER_OFF) | PWRSTS_ON) >+ > #define PWRSTS_OFF_RET ((1 << PWRDM_POWER_OFF) | \ > (1 << PWRDM_POWER_RET)) > >-#define PWRSTS_RET_ON ((1 << PWRDM_POWER_RET) | \ >- (1 << PWRDM_POWER_ON)) >- >-#define PWRSTS_OFF_RET_ON (PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON)) >+#define PWRSTS_RET_ON ((1 << PWRDM_POWER_RET) | PWRSTS_ON) > >+#define PWRSTS_OFF_RET_ON (PWRSTS_OFF_RET | PWRSTS_ON) > > /* Powerdomain flags */ > #define PWRDM_HAS_HDWR_SAR (1 << 0) /* hardware >save-and-restore support */ >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
