Re: [PATCHv6 9/9] OMAP3: PM: Added support for suspending to INACTIVE state

[Kevin Hilman <khilman@deeprootsystems.com>]

<Tero.Kristo@nokia.com> writes:

>  
>
>>-----Original Message-----
>>From: ext Kevin Hilman [mailto:khilman@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.Kristo@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.

OK, I see the chicken-egg problem now.  

You're original version is ok with me.

Thanks,

Kevin