"ext Kevin Hilman" <khilman@xxxxxxxxxxxxxxxxxxx> writes:
> Högander Jouni wrote:
>> "ext Paul Walmsley" <paul@xxxxxxxxx> 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.
Should we write all the states in off_mode_enable/pwrdms_setup
and then write them again in resource_refresh/CPUidle loop.
>
> 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.
Should we then just ignore possible error value from set_pwrdm_state?
Or consider unsupported PWRDM_ST as a not an error in set_pwrdm_state? Just
ignore it?
>
> Kevin
>
>
>>> - Paul
>>>
>>>
>>>> From: Jouni Hogander <jouni.hogander@xxxxxxxxx>
>>>> 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@xxxxxxxxx>
>>>> ---
>>>> 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
>>
>
--
Jouni Högander
--
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
