<tero.kri...@nokia.com> writes:
>
>
>>-----Original Message-----
>>From: ext Kevin Hilman [mailto:khil...@deeprootsystems.com]
>>Sent: 02 March, 2010 01:17
>>To: Kristo Tero (Nokia-D/Tampere)
>>Cc: linux-omap@vger.kernel.org
>>Subject: Re: [PATCHv6 1/9] OMAP3: PM: Added support functions
>>for omap3 pwrdm handling
>>
>>Tero Kristo <tero.kri...@nokia.com> writes:
>>
>>> From: Tero Kristo <tero.kri...@nokia.com>
>>>
>>> Added omap3_pwrdm_set_next_pwrst and
>>omap3_pwrdm_read_next_pwrst. These
>>> functions add support for INACTIVE and ON states to the standard OMAP
>>> powerdomain functions, and add caching logic for the next
>>state. These
>>> functions are used in subsequent patches to simplify the
>>logic of the idle
>>> loop.
>>>
>>> Signed-off-by: Tero Kristo <tero.kri...@nokia.com>
>>> ---
>>> arch/arm/mach-omap2/pm.h | 2 +
>>> arch/arm/mach-omap2/pm34xx.c | 60
>>+++++++++++++++++++++++++++++++++++++++++-
>>> 2 files changed, 61 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
>>> index 75aa685..1d9a740 100644
>>> --- a/arch/arm/mach-omap2/pm.h
>>> +++ b/arch/arm/mach-omap2/pm.h
>>> @@ -67,6 +67,8 @@ static inline void omap3_pm_init_vc(struct
>>prm_setup_vc *setup_vc)
>>>
>>> extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
>>> extern int omap3_pm_set_suspend_state(struct powerdomain
>>*pwrdm, int state);
>>> +extern int omap3_pwrdm_set_next_pwrst(struct powerdomain
>>*pwrdm, u8 pwrst);
>>> +extern int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm);
>>>
>>> extern u32 wakeup_timer_seconds;
>>> extern struct omap_dm_timer *gptimer_wakeup;
>>> diff --git a/arch/arm/mach-omap2/pm34xx.c
>>b/arch/arm/mach-omap2/pm34xx.c
>>> index 1359ec9..f20d3d8 100644
>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>> @@ -576,6 +576,64 @@ int omap3_can_sleep(void)
>>> return 1;
>>> }
>>>
>>> +struct powerdomain_data {
>>> + u8 next_state;
>>> + u8 init_done;
>>> +};
>>> +
>>> +static struct powerdomain_data mpu_pwrdm_data;
>>> +static struct powerdomain_data core_pwrdm_data;
>>> +static struct powerdomain_data neon_pwrdm_data;
>>> +
>>> +static struct powerdomain_data *get_pwrdm_data(struct
>>powerdomain *pwrdm)
>>> +{
>>> + if (pwrdm == mpu_pwrdm)
>>> + return &mpu_pwrdm_data;
>>> + else if (pwrdm == core_pwrdm)
>>> + return &core_pwrdm_data;
>>> + else if (pwrdm == neon_pwrdm)
>>> + return &neon_pwrdm_data;
>>> + return NULL;
>>> +}
>>> +
>>> +int omap3_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
>>
>>I think this func should be documented along the lines of the
>>changelog.
>>It should describe the caching and the fact that 'INACTIVE' is a state
>>that can be read, but not written.
>
> Ok, will do the change.
>
>>> +{
>>> + struct powerdomain_data *data = get_pwrdm_data(pwrdm);
>>> + u8 prg_pwrst;
>>> +
>>> + if (!data)
>>> + return pwrdm_set_next_pwrst(pwrdm, pwrst);
>>> +
>>> + if (!data->init_done)
>>> + data->init_done = 1;
>>
>>Not sure I follow the purpose of the 'init_done' flag here,
>>but I'm having
>>an averse reaction to it.
>
> init_done actually means if the cache is valid or not. If not, we ignore the
> current cached state. Another way of doing this would be to initialize all
> values at proper point during boot by reading from HW, or just put the HW
> reset values to the cache during compile time. The cache validity bit is
> needed in the read_next_pwrst code also below.
I'd rather see this done using init-time reads from the HW.
Kevin
>>
>>> + else if (data->next_state == pwrst)
>>> + return 0;
>>> +
>>> + if (pwrst == PWRDM_POWER_INACTIVE)
>>> + prg_pwrst = PWRDM_POWER_ON;
>>> + else
>>> + prg_pwrst = pwrst;
>>> +
>>> + pwrdm_set_next_pwrst(pwrdm, prg_pwrst);
>>> +
>>> + if (pwrst == PWRDM_POWER_ON)
>>> + omap2_clkdm_deny_idle(pwrdm->pwrdm_clkdms[0]);
>>> + else
>>> + omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>>> +
>>> + data->next_state = pwrst;
>>> + return 0;
>>> +}
>>> +
>>> +int omap3_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
>>> +{
>>> + struct powerdomain_data *data = get_pwrdm_data(pwrdm);
>>> +
>>> + if (!data || !data->init_done)
>>> + return pwrdm_read_next_pwrst(pwrdm);
>>> + return data->next_state;
>>> +}
>>> +
>>> /* This sets pwrdm state (other than mpu & core. Currently only ON &
>>> * RET are supported. Function is assuming that clkdm doesn't have
>>> * hw_sup mode enabled. */
>>> @@ -604,7 +662,7 @@ int set_pwrdm_state(struct powerdomain
>>*pwrdm, u32 state)
>>> pwrdm_wait_transition(pwrdm);
>>> }
>>>
>>> - ret = pwrdm_set_next_pwrst(pwrdm, state);
>>> + ret = omap3_pwrdm_set_next_pwrst(pwrdm, state);
>>> if (ret) {
>>> printk(KERN_ERR "Unable to set state of
>>powerdomain: %s\n",
>>> pwrdm->name);
>>> --
>>> 1.5.4.3
>>>
>>> --
>>> 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
>>
--
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
