>-----Original Message----- >From: ext Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: 08 March, 2010 19:16 >To: Kristo Tero (Nokia-D/Tampere) >Cc: linux-omap@vger.kernel.org >Subject: Re: [PATCHv6 9/9] OMAP3: PM: Added support for >suspending to INACTIVE state > ><tero.kri...@nokia.com> writes: > >[...] > >> True, ancient info there. OFF for example has been supported >for ages already. >> >>> >>> >>>> + if (state != PWRDM_POWER_INACTIVE) >>>> + while (!(pwrdm->pwrsts & (1 << state))) { >>>> + if (state == PWRDM_POWER_OFF) >>>> + return ret; >>>> + state--; >>>> + } >>> >>>I think all powerdomains can be inactive right? >> >> Yes. >> >>>I think it would be cleaner to just have all the pwrdm->pwrsts fields >>>include intactive as a valid option. >>> >>>Something like the patch below. IIRC, you did something like this in >>>one of the earlier versions of the patch. >> >> Yeah, something like this was done previously, however Paul did not >> like the idea of changing the generic powerdomain code too much so I >> dropped it completely. It is now done only via the support functions >> in patch #1, and only done for the powerdomains that actually need >> it for the cpuidle (mpu/core/neon.) It would be possible to add >> support for the rest of the powerdomains also, but I decided to drop >> this in favor of getting the patch set in. > >I'm not proposing changing any of the other powerdomain code. Just >changing the PWRSTS_* defines, essentially so that INACTIVE is >a valid state. > >That will eliminate the need for a special check for inactive in this >patch.
This is a chicken-egg problem. If you alter the PWRSTS_* defines, you need to change implementation of pwrdm_set_next_pwrst() as it would accept INACTIVE also, which is not supported by the code right now. > >Kevin > >>> >>>diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h >>>b/arch/arm/plat-omap/include/plat/powerdomain.h >>>index a1ecd47..c692472 100644 >>>--- a/arch/arm/plat-omap/include/plat/powerdomain.h >>>+++ b/arch/arm/plat-omap/include/plat/powerdomain.h >>>@@ -31,17 +31,17 @@ >>> #define PWRDM_MAX_PWRSTS 4 >>> >>> /* Powerdomain allowable state bitfields */ >>>-#define PWRSTS_OFF_ON ((1 << PWRDM_POWER_OFF) | \ >>>+#define PWRSTS_ON ((1 << PWRDM_POWER_INACTIVE) | \ >>> (1 << PWRDM_POWER_ON)) >>> >>>+#define PWRSTS_OFF_ON ((1 << PWRDM_POWER_OFF) >| PWRSTS_ON) >>>+ >>> #define PWRSTS_OFF_RET ((1 << PWRDM_POWER_OFF) | \ >>> (1 << PWRDM_POWER_RET)) >>> >>>-#define PWRSTS_RET_ON ((1 << PWRDM_POWER_RET) | \ >>>- (1 << PWRDM_POWER_ON)) >>>- >>>-#define PWRSTS_OFF_RET_ON (PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON)) >>>+#define PWRSTS_RET_ON ((1 << PWRDM_POWER_RET) >| PWRSTS_ON) >>> >>>+#define PWRSTS_OFF_RET_ON (PWRSTS_OFF_RET | PWRSTS_ON) >>> >>> /* Powerdomain flags */ >>> #define PWRDM_HAS_HDWR_SAR (1 << 0) /* hardware >>>save-and-restore support */ >>> >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
