<[email protected]> 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.

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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to