Re: [PATCH] OMAP3 powerdomains: remove RET from SGX power states list

[Kevin Hilman <khilman@deeprootsystems.com>]

Högander Jouni wrote:
> "ext Paul Walmsley" <paul@pwsan.com> writes:
> 
>> Hi Jouni,
>>
>> On Fri, 7 Nov 2008, Högander Jouni wrote:
>>
>>> What do you Paul think about patch below:
>> I'm okay with it, but one potential problem: won't this prevent the chip 
>> from entering retention, since SGX will be set to ON?
> 
> Yes, you are right here.
> 
>> Anyway, if you agree this is a problem, what do you think about adding a 
>> powerdomain flag that indicates that the powerdomain next power state 
>> should be set to OFF on initialization, and then testing that in 
>> set_pwrdm_state() ?
> 
> I wouldn't add any flags for this. The goal is finally to set all
> next_states as OFF until someone has set some constraint which
> prevents OFF usage. For now we need to use RET as default, because
> drivers are not supporting OFF mode. Do you agree this?
> 
> Easiest way here would be to add own hook for SGX in pwrdms_setup? One
> more strcmp("*_pwrdm, pwrdm->name) :)
> 
> What do you think?
> 

Personally, I don't like these if statements (or ifdefs) in pwrdms_setup 
or the off_mode_enable hook.  And I would like to see them all disappear.

I would rather see set_pwrdm_state() be smarter by simply not trying
to use a state that is not in its list of allowed states.

Kevin


>> - Paul
>>
>>
>>> From: Jouni Hogander <jouni.hogander@nokia.com>
>>> Date: Fri, 7 Nov 2008 16:50:51 +0200
>>> Subject: [PATCH] OMAP3: PM: Check that wanted state is supported by pwrdm in pwrdms_setup
>>>
>>> Check that wanted sleep state is supported by powerdomain. If it is
>>> not supported, then use next highest supported state.
>>>
>>> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
>>> ---
>>>  arch/arm/mach-omap2/pm34xx.c |   11 ++++++++++-
>>>  1 files changed, 10 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>>> index da098d2..d9959a8 100644
>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>> @@ -515,6 +515,7 @@ static void __init prcm_setup_regs(void)
>>>  static int __init pwrdms_setup(struct powerdomain *pwrdm)
>>>  {
>>>  	struct power_state *pwrst;
>>> +	u32 next_state = PWRDM_POWER_RET;
>>>  
>>>  	if (!pwrdm->pwrsts)
>>>  		return 0;
>>> @@ -523,12 +524,20 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm)
>>>  	if (!pwrst)
>>>  		return -ENOMEM;
>>>  	pwrst->pwrdm = pwrdm;
>>> -	pwrst->next_state = PWRDM_POWER_RET;
>>>  	list_add(&pwrst->node, &pwrst_list);
>>>  
>>>  	if (pwrdm_has_hdwr_sar(pwrdm))
>>>  		pwrdm_enable_hdwr_sar(pwrdm);
>>>  
>>> +	while (!(pwrdm->pwrsts & (1 << next_state))) {
>>> +		if (next_state > PWRDM_POWER_ON) {
>>> +			next_state = pwrdm_read_next_pwrst(pwrst->pwrdm);
>>> +			break;
>>> +		}
>>> +		next_state++;
>>> +	}
>>> +	pwrst->next_state = next_state;
>>> +
>>>  	return set_pwrdm_state(pwrst->pwrdm, pwrst->next_state);
>>>  }
>>>  
>>> -- 
>>> 1.6.0.1
>>>
>>>
>>
>> - Paul
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html