Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states
Jean, On Monday 21 May 2012 07:55 PM, Shilimkar, Santosh wrote: > Jean, > > On Mon, May 21, 2012 at 7:23 PM, Jean Pihet wrote: >> Hi Santosh, >> >> On Thu, May 17, 2012 at 12:04 PM, Santosh Shilimkar >> wrote: [...] >> What do you think? >> > I like the idea as mentioned. I just think the series should already > take care of memory state, logic state and quirk flags to see how > it looks overall. Will have one more fresh look at the series but if > you have subsequent WIP patches which can handle the other > parameters, please send that link. > I have scanned the series again. Somehow I still find myself not convinced with the approach. I found having PD OSWR CSWR state is the only change from this series helps to some extent. But if you look from PRCM point of view, they are already two distinct power domain states. Only bad part from programming perspective, is, it's needs to take care of additional bit to set and get PD state. If we actually support 'PWRDM_POWER_OSWRET" and 'PWRDM_POWER_CSWRET" as valid state then we can do everything without the functional power state series. I mean we can use 'omap_set_pwrdm_state()' as a single API for programming the PD, instead of duplicating these all over the place. OSWR by definition can be customised per power domain based on various supported logic and memory states.With this series, OSWR definition also become static and if that is agreed then we should cab just make that as a state like ON/OFF/INACTIVE. But I don't want to object this series if Paul is convinced and agrees with the approach. My view may be bit narrow but I am not convinced with the approach. Btw, ther are some real differences in PD INACTIVE state in OMAP3 and OMAP4+ devices. I had some discussion with Paul before on it. It needs to be clubbed with voltagedomain layer to properly represent. Some discussion was on the list [1] and some of that was off list with hardware designers. Relevant thread is here [1] Regards Santosh [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-February/041133.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
Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states
Jean,
On Mon, May 21, 2012 at 7:23 PM, Jean Pihet wrote:
> Hi Santosh,
>
> On Thu, May 17, 2012 at 12:04 PM, Santosh Shilimkar
> wrote:
>> Jean,
>> On Tuesday 08 May 2012 02:10 PM, Jean Pihet wrote:
>>> Paul,
>>>
>>> On Mon, May 7, 2012 at 11:28 AM, Paul Walmsley wrote:
Hi
On Wed, 18 Apr 2012, jean.pi...@newoldbits.com wrote:
> From: Jean Pihet
>
> Introduce functional (or logical) states for power domains and the
> API functions to read the power domains settings and to convert
> between the functional (i.e. logical) and the internal (or registers)
> values.
>
> OMAP2, OMAP3 and OMAP4 platforms are defining a conversion routine.
>
> In the new API the function omap_set_pwrdm_state takes the functional
> states as parameter; while at it the function is moved to the power
> domains code.
>
> The memory and logic states are not using the functional states, this
> comes as a subsequent patch.
>
> Signed-off-by: Jean Pihet
This patch results in several checkpatch warnings; please resolve them.
>>> Oops. Will check and update.
>>>
> ---
> arch/arm/mach-omap2/pm.c | 66 ---
> arch/arm/mach-omap2/powerdomain-common.c | 61 ++
> arch/arm/mach-omap2/powerdomain.c | 175
>
> arch/arm/mach-omap2/powerdomain.h | 21
> arch/arm/mach-omap2/powerdomain2xxx_3xxx.c | 5 +
> arch/arm/mach-omap2/powerdomain44xx.c | 2 +
> 6 files changed, 264 insertions(+), 66 deletions(-)
>
>>> ...
>>>
> +/*
> + * Functional (i.e. logical) to internal (i.e. registers)
> + * values for the power domains states
> + */
Please use kerneldoc-style function comments.
>>> Ok
>>>
> +int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
> +{
> + int ret;
> +
> + switch (func_pwrst) {
> + case PWRDM_FUNC_PWRST_ON:
> + ret = PWRDM_POWER_ON;
> + break;
> + case PWRDM_FUNC_PWRST_INACTIVE:
> + ret = PWRDM_POWER_INACTIVE;
> + break;
> + case PWRDM_FUNC_PWRST_CSWR:
> + case PWRDM_FUNC_PWRST_OSWR:
> + ret = PWRDM_POWER_RET;
> + break;
> + case PWRDM_FUNC_PWRST_OFF:
> + ret = PWRDM_POWER_OFF;
> + break;
> + default:
> + ret = -1;
> + }
> +
> + return ret;
> +}
At a quick glance, I don't think that this function is appropriate for all
OMAP2+ chips. For example, off-mode is not supported in our OMAP2xxx
kernels. And OMAP2xxx/3xxx do not support the INACTIVE powerstate. So
probably this function should differ by chip, and be located in the
powerdomain[2-4]*xx*.c files.
>>> I hope to make this function as generic as possible, hence the
>>> location (powerdomain-common.c). Some states are not programmed by the
>>> pwrdm_* functions since there forbidden by the contents of the
>>> pwrdm->pwrst field.
>>> Now if the need arises some platform specific function can be defined
>>> for conversion functions since the pwrdm->ops function pointers are
>>> used. I do not think it is needed now but it can easily be changed.
>>>
Also, what about the logic and memory bank power states? Shouldn't this
function pass those back as well?
>>> Cf. in the description "The memory and logic states are not using the
>>> functional states, this comes as a subsequent patch."
>>>
>> I don't see much difference between the functional power states to
>> actual power states with some cases here and there.
>>
>> + case PWRDM_FUNC_PWRST_CSWR:
>> + case PWRDM_FUNC_PWRST_OSWR:
>> + ret = PWRDM_POWER_RET;
> The whole patch series takes the logic state into account in the
> functional power state.
> Cf. in the description "The memory and logic states are not using the
> functional states, this comes as a subsequent patch."
>
Yeah. i understood that from comments.
>> This is not true and even if you add logic/memmort etc support,
>> I still think, we are just duplicating stuff between functional
>> vs actual power state.
> The idea is to provide a single function to program the power domains
> state, including the logic state.
> There is no duplication of code, just a simplification in the API. IMO
> there are too much calls to set and get the power domain states (power
> state, logical etc.) and most of the calls do not take the logical
> state into account.
> If you look the the code in pm.c and cpuidle.c you will notice
> that the code is simplified.
>
I noticed that but thought that simplification is not substantial.
>> If the idea is to have one single interface to program the power
>> domain states, so that Generic frameworks, or PM code movement to
>> driver/* directories is easi
Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states
Hi Santosh,
On Thu, May 17, 2012 at 12:04 PM, Santosh Shilimkar
wrote:
> Jean,
> On Tuesday 08 May 2012 02:10 PM, Jean Pihet wrote:
>> Paul,
>>
>> On Mon, May 7, 2012 at 11:28 AM, Paul Walmsley wrote:
>>> Hi
>>>
>>> On Wed, 18 Apr 2012, jean.pi...@newoldbits.com wrote:
>>>
From: Jean Pihet
Introduce functional (or logical) states for power domains and the
API functions to read the power domains settings and to convert
between the functional (i.e. logical) and the internal (or registers)
values.
OMAP2, OMAP3 and OMAP4 platforms are defining a conversion routine.
In the new API the function omap_set_pwrdm_state takes the functional
states as parameter; while at it the function is moved to the power
domains code.
The memory and logic states are not using the functional states, this
comes as a subsequent patch.
Signed-off-by: Jean Pihet
>>>
>>> This patch results in several checkpatch warnings; please resolve them.
>> Oops. Will check and update.
>>
>>>
---
arch/arm/mach-omap2/pm.c | 66 ---
arch/arm/mach-omap2/powerdomain-common.c | 61 ++
arch/arm/mach-omap2/powerdomain.c | 175
arch/arm/mach-omap2/powerdomain.h | 21
arch/arm/mach-omap2/powerdomain2xxx_3xxx.c | 5 +
arch/arm/mach-omap2/powerdomain44xx.c | 2 +
6 files changed, 264 insertions(+), 66 deletions(-)
>> ...
>>
+/*
+ * Functional (i.e. logical) to internal (i.e. registers)
+ * values for the power domains states
+ */
>>>
>>> Please use kerneldoc-style function comments.
>> Ok
>>
>>>
+int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
+{
+ int ret;
+
+ switch (func_pwrst) {
+ case PWRDM_FUNC_PWRST_ON:
+ ret = PWRDM_POWER_ON;
+ break;
+ case PWRDM_FUNC_PWRST_INACTIVE:
+ ret = PWRDM_POWER_INACTIVE;
+ break;
+ case PWRDM_FUNC_PWRST_CSWR:
+ case PWRDM_FUNC_PWRST_OSWR:
+ ret = PWRDM_POWER_RET;
+ break;
+ case PWRDM_FUNC_PWRST_OFF:
+ ret = PWRDM_POWER_OFF;
+ break;
+ default:
+ ret = -1;
+ }
+
+ return ret;
+}
>>>
>>> At a quick glance, I don't think that this function is appropriate for all
>>> OMAP2+ chips. For example, off-mode is not supported in our OMAP2xxx
>>> kernels. And OMAP2xxx/3xxx do not support the INACTIVE powerstate. So
>>> probably this function should differ by chip, and be located in the
>>> powerdomain[2-4]*xx*.c files.
>> I hope to make this function as generic as possible, hence the
>> location (powerdomain-common.c). Some states are not programmed by the
>> pwrdm_* functions since there forbidden by the contents of the
>> pwrdm->pwrst field.
>> Now if the need arises some platform specific function can be defined
>> for conversion functions since the pwrdm->ops function pointers are
>> used. I do not think it is needed now but it can easily be changed.
>>
>>> Also, what about the logic and memory bank power states? Shouldn't this
>>> function pass those back as well?
>> Cf. in the description "The memory and logic states are not using the
>> functional states, this comes as a subsequent patch."
>>
> I don't see much difference between the functional power states to
> actual power states with some cases here and there.
>
> + case PWRDM_FUNC_PWRST_CSWR:
> + case PWRDM_FUNC_PWRST_OSWR:
> + ret = PWRDM_POWER_RET;
The whole patch series takes the logic state into account in the
functional power state.
Cf. in the description "The memory and logic states are not using the
functional states, this comes as a subsequent patch."
> This is not true and even if you add logic/memmort etc support,
> I still think, we are just duplicating stuff between functional
> vs actual power state.
The idea is to provide a single function to program the power domains
state, including the logic state.
There is no duplication of code, just a simplification in the API. IMO
there are too much calls to set and get the power domain states (power
state, logical etc.) and most of the calls do not take the logical
state into account.
If you look the the code in pm.c and cpuidle.c you will notice
that the code is simplified.
> If the idea is to have one single interface to program the power
> domain states, so that Generic frameworks, or PM code movement to
> driver/* directories is easier, we may want think about alternative
> which is scalable.
>
> Just a wild thought...
>
> May be we can abstract all the necessary/used power domain programming
> entities inside a structure and export that. That should contain, power
> state, logic state, mem state, special quirk flags like SAR etc.
> b
Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states
Jean,
On Tuesday 08 May 2012 02:10 PM, Jean Pihet wrote:
> Paul,
>
> On Mon, May 7, 2012 at 11:28 AM, Paul Walmsley wrote:
>> Hi
>>
>> On Wed, 18 Apr 2012, jean.pi...@newoldbits.com wrote:
>>
>>> From: Jean Pihet
>>>
>>> Introduce functional (or logical) states for power domains and the
>>> API functions to read the power domains settings and to convert
>>> between the functional (i.e. logical) and the internal (or registers)
>>> values.
>>>
>>> OMAP2, OMAP3 and OMAP4 platforms are defining a conversion routine.
>>>
>>> In the new API the function omap_set_pwrdm_state takes the functional
>>> states as parameter; while at it the function is moved to the power
>>> domains code.
>>>
>>> The memory and logic states are not using the functional states, this
>>> comes as a subsequent patch.
>>>
>>> Signed-off-by: Jean Pihet
>>
>> This patch results in several checkpatch warnings; please resolve them.
> Oops. Will check and update.
>
>>
>>> ---
>>> arch/arm/mach-omap2/pm.c | 66 ---
>>> arch/arm/mach-omap2/powerdomain-common.c | 61 ++
>>> arch/arm/mach-omap2/powerdomain.c | 175
>>>
>>> arch/arm/mach-omap2/powerdomain.h | 21
>>> arch/arm/mach-omap2/powerdomain2xxx_3xxx.c |5 +
>>> arch/arm/mach-omap2/powerdomain44xx.c |2 +
>>> 6 files changed, 264 insertions(+), 66 deletions(-)
>>>
> ...
>
>>> +/*
>>> + * Functional (i.e. logical) to internal (i.e. registers)
>>> + * values for the power domains states
>>> + */
>>
>> Please use kerneldoc-style function comments.
> Ok
>
>>
>>> +int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
>>> +{
>>> + int ret;
>>> +
>>> + switch (func_pwrst) {
>>> + case PWRDM_FUNC_PWRST_ON:
>>> + ret = PWRDM_POWER_ON;
>>> + break;
>>> + case PWRDM_FUNC_PWRST_INACTIVE:
>>> + ret = PWRDM_POWER_INACTIVE;
>>> + break;
>>> + case PWRDM_FUNC_PWRST_CSWR:
>>> + case PWRDM_FUNC_PWRST_OSWR:
>>> + ret = PWRDM_POWER_RET;
>>> + break;
>>> + case PWRDM_FUNC_PWRST_OFF:
>>> + ret = PWRDM_POWER_OFF;
>>> + break;
>>> + default:
>>> + ret = -1;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>
>> At a quick glance, I don't think that this function is appropriate for all
>> OMAP2+ chips. For example, off-mode is not supported in our OMAP2xxx
>> kernels. And OMAP2xxx/3xxx do not support the INACTIVE powerstate. So
>> probably this function should differ by chip, and be located in the
>> powerdomain[2-4]*xx*.c files.
> I hope to make this function as generic as possible, hence the
> location (powerdomain-common.c). Some states are not programmed by the
> pwrdm_* functions since there forbidden by the contents of the
> pwrdm->pwrst field.
> Now if the need arises some platform specific function can be defined
> for conversion functions since the pwrdm->ops function pointers are
> used. I do not think it is needed now but it can easily be changed.
>
>> Also, what about the logic and memory bank power states? Shouldn't this
>> function pass those back as well?
> Cf. in the description "The memory and logic states are not using the
> functional states, this comes as a subsequent patch."
>
I don't see much difference between the functional power states to
actual power states with some cases here and there.
+ case PWRDM_FUNC_PWRST_CSWR:
+ case PWRDM_FUNC_PWRST_OSWR:
+ ret = PWRDM_POWER_RET;
This is not true and even if you add logic/memmort etc support,
I still think, we are just duplicating stuff between functional
vs actual power state.
If the idea is to have one single interface to program the power
domain states, so that Generic frameworks, or PM code movement to
driver/* directories is easier, we may want think about alternative
which is scalable.
Just a wild thought...
May be we can abstract all the necessary/used power domain programming
entities inside a structure and export that. That should contain, power
state, logic state, mem state, special quirk flags like SAR etc.
being structure is always expandable and an API taking structure as
a parameter need not change.
Regards
Santosh
--
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
Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states
Paul,
On Mon, May 7, 2012 at 11:28 AM, Paul Walmsley wrote:
> Hi
>
> On Wed, 18 Apr 2012, jean.pi...@newoldbits.com wrote:
>
>> From: Jean Pihet
>>
>> Introduce functional (or logical) states for power domains and the
>> API functions to read the power domains settings and to convert
>> between the functional (i.e. logical) and the internal (or registers)
>> values.
>>
>> OMAP2, OMAP3 and OMAP4 platforms are defining a conversion routine.
>>
>> In the new API the function omap_set_pwrdm_state takes the functional
>> states as parameter; while at it the function is moved to the power
>> domains code.
>>
>> The memory and logic states are not using the functional states, this
>> comes as a subsequent patch.
>>
>> Signed-off-by: Jean Pihet
>
> This patch results in several checkpatch warnings; please resolve them.
Oops. Will check and update.
>
>> ---
>> arch/arm/mach-omap2/pm.c | 66 ---
>> arch/arm/mach-omap2/powerdomain-common.c | 61 ++
>> arch/arm/mach-omap2/powerdomain.c | 175
>>
>> arch/arm/mach-omap2/powerdomain.h | 21
>> arch/arm/mach-omap2/powerdomain2xxx_3xxx.c | 5 +
>> arch/arm/mach-omap2/powerdomain44xx.c | 2 +
>> 6 files changed, 264 insertions(+), 66 deletions(-)
>>
...
>> +/*
>> + * Functional (i.e. logical) to internal (i.e. registers)
>> + * values for the power domains states
>> + */
>
> Please use kerneldoc-style function comments.
Ok
>
>> +int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
>> +{
>> + int ret;
>> +
>> + switch (func_pwrst) {
>> + case PWRDM_FUNC_PWRST_ON:
>> + ret = PWRDM_POWER_ON;
>> + break;
>> + case PWRDM_FUNC_PWRST_INACTIVE:
>> + ret = PWRDM_POWER_INACTIVE;
>> + break;
>> + case PWRDM_FUNC_PWRST_CSWR:
>> + case PWRDM_FUNC_PWRST_OSWR:
>> + ret = PWRDM_POWER_RET;
>> + break;
>> + case PWRDM_FUNC_PWRST_OFF:
>> + ret = PWRDM_POWER_OFF;
>> + break;
>> + default:
>> + ret = -1;
>> + }
>> +
>> + return ret;
>> +}
>
> At a quick glance, I don't think that this function is appropriate for all
> OMAP2+ chips. For example, off-mode is not supported in our OMAP2xxx
> kernels. And OMAP2xxx/3xxx do not support the INACTIVE powerstate. So
> probably this function should differ by chip, and be located in the
> powerdomain[2-4]*xx*.c files.
I hope to make this function as generic as possible, hence the
location (powerdomain-common.c). Some states are not programmed by the
pwrdm_* functions since there forbidden by the contents of the
pwrdm->pwrst field.
Now if the need arises some platform specific function can be defined
for conversion functions since the pwrdm->ops function pointers are
used. I do not think it is needed now but it can easily be changed.
> Also, what about the logic and memory bank power states? Shouldn't this
> function pass those back as well?
Cf. in the description "The memory and logic states are not using the
functional states, this comes as a subsequent patch."
>
>> +
>> +/*
>> + * Internal (i.e. registers) to functional (i.e. logical) values
>> + * for the power domains states
>> + */
>
> Please use kerneldoc-style function documentation.
Ok
>
>> +int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst)
>
> Probably best to use "func_pwrst" or "fpwrst" rather than simply "func"
> here.
Ok. I am for 'omap2_pwrdm_pwrst_to_fpwrst(...)'.
>
>> +{
>> + int ret;
>> +
>> + switch (pwrst) {
>> + case PWRDM_POWER_ON:
>> + ret = PWRDM_FUNC_PWRST_ON;
>> + break;
>> + case PWRDM_POWER_INACTIVE:
>> + ret = PWRDM_FUNC_PWRST_INACTIVE;
>> + break;
>> + case PWRDM_POWER_RET:
>> + /*
>> + * XXX warning: return OSWR in case of pd in RET and
>> + * logic in OFF
>> + */
>> + ret = PWRDM_FUNC_PWRST_CSWR;
>> + break;
>> + case PWRDM_POWER_OFF:
>> + ret = PWRDM_FUNC_PWRST_OFF;
>> + break;
>> + default:
>> + ret = -1;
>> + }
>> +
>> + return ret;
>> +}
>
> This function also needs to check the bank power states and logic power
> states to determine what the actual functional powerstate is.
Same remark as above + there is a comment about it in the code ("XXX warning").
>> +
>> diff --git a/arch/arm/mach-omap2/powerdomain.c
>> b/arch/arm/mach-omap2/powerdomain.c
>> index 319b277..718fa43 100644
>> --- a/arch/arm/mach-omap2/powerdomain.c
>> +++ b/arch/arm/mach-omap2/powerdomain.c
>> @@ -466,6 +466,136 @@ int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm)
>> }
>>
>> /**
>> + * pwrdm_func_to_pwrst - get the internal (i.e. registers) value mapped
>> + * to the functional state
>
> Probably best to use "func_pwrst" or "fpwrst" rather than simply "func"
> here.
Ok
>
>> + * @pwrdm: struct powe
Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states
Hi
On Wed, 18 Apr 2012, jean.pi...@newoldbits.com wrote:
> From: Jean Pihet
>
> Introduce functional (or logical) states for power domains and the
> API functions to read the power domains settings and to convert
> between the functional (i.e. logical) and the internal (or registers)
> values.
>
> OMAP2, OMAP3 and OMAP4 platforms are defining a conversion routine.
>
> In the new API the function omap_set_pwrdm_state takes the functional
> states as parameter; while at it the function is moved to the power
> domains code.
>
> The memory and logic states are not using the functional states, this
> comes as a subsequent patch.
>
> Signed-off-by: Jean Pihet
This patch results in several checkpatch warnings; please resolve them.
> ---
> arch/arm/mach-omap2/pm.c | 66 ---
> arch/arm/mach-omap2/powerdomain-common.c | 61 ++
> arch/arm/mach-omap2/powerdomain.c | 175
>
> arch/arm/mach-omap2/powerdomain.h | 21
> arch/arm/mach-omap2/powerdomain2xxx_3xxx.c |5 +
> arch/arm/mach-omap2/powerdomain44xx.c |2 +
> 6 files changed, 264 insertions(+), 66 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index 6918a13..8670c4b 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -74,10 +74,6 @@ static void __init omap2_init_processor_devices(void)
> }
> }
>
> -/* Types of sleep_switch used in omap_set_pwrdm_state */
> -#define FORCEWAKEUP_SWITCH 0
> -#define LOWPOWERSTATE_SWITCH 1
> -
> int __init omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused)
> {
> if (clkdm->flags & CLKDM_CAN_ENABLE_AUTO)
> @@ -89,68 +85,6 @@ int __init omap_pm_clkdms_setup(struct clockdomain *clkdm,
> void *unused)
> }
>
> /*
> - * This sets pwrdm state (other than mpu & core. Currently only ON &
> - * RET are supported.
> - */
> -int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst)
> -{
> - u8 curr_pwrst, next_pwrst;
> - int sleep_switch = -1, ret = 0, hwsup = 0;
> -
> - if (!pwrdm || IS_ERR(pwrdm))
> - return -EINVAL;
> -
> - mutex_lock(&pwrdm->lock);
> -
> - while (!(pwrdm->pwrsts & (1 << pwrst))) {
> - if (pwrst == PWRDM_POWER_OFF)
> - goto out;
> - pwrst--;
> - }
> -
> - next_pwrst = pwrdm_read_next_pwrst(pwrdm);
> - if (next_pwrst == pwrst)
> - goto out;
> -
> - curr_pwrst = pwrdm_read_pwrst(pwrdm);
> - if (curr_pwrst < PWRDM_POWER_ON) {
> - if ((curr_pwrst > pwrst) &&
> - (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
> - sleep_switch = LOWPOWERSTATE_SWITCH;
> - } else {
> - hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
> - clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> - sleep_switch = FORCEWAKEUP_SWITCH;
> - }
> - }
> -
> - ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
> - if (ret)
> - pr_err("%s: unable to set power state of powerdomain: %s\n",
> -__func__, pwrdm->name);
> -
> - switch (sleep_switch) {
> - case FORCEWAKEUP_SWITCH:
> - if (hwsup)
> - clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> - else
> - clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
> - break;
> - case LOWPOWERSTATE_SWITCH:
> - pwrdm_set_lowpwrstchange(pwrdm);
> - pwrdm_wait_transition(pwrdm);
> - pwrdm_state_switch(pwrdm);
> - break;
> - }
> -
> -out:
> - mutex_unlock(&pwrdm->lock);
> - return ret;
> -}
> -
> -
> -
> -/*
> * This API is to be called during init to set the various voltage
> * domains to the voltage as per the opp table. Typically we boot up
> * at the nominal voltage. So this function finds out the rate of
> diff --git a/arch/arm/mach-omap2/powerdomain-common.c
> b/arch/arm/mach-omap2/powerdomain-common.c
> index c0aeabf..098dc42 100644
> --- a/arch/arm/mach-omap2/powerdomain-common.c
> +++ b/arch/arm/mach-omap2/powerdomain-common.c
> @@ -108,3 +108,64 @@ u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank)
> return 0;
> }
>
> +/*
> + * Functional (i.e. logical) to internal (i.e. registers)
> + * values for the power domains states
> + */
Please use kerneldoc-style function comments.
> +int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
> +{
> + int ret;
> +
> + switch (func_pwrst) {
> + case PWRDM_FUNC_PWRST_ON:
> + ret = PWRDM_POWER_ON;
> + break;
> + case PWRDM_FUNC_PWRST_INACTIVE:
> + ret = PWRDM_POWER_INACTIVE;
> + break;
> + case PWRDM_FUNC_PWRST_CSWR:
> + case PWRDM_FUNC_PWRST_OSWR:
> + ret = PWRDM_POWER_RET;
> + break;
> + case PWRDM_FUNC_PWRST_OFF:
> + ret = PWRDM_POWER_O
[PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states
From: Jean Pihet
Introduce functional (or logical) states for power domains and the
API functions to read the power domains settings and to convert
between the functional (i.e. logical) and the internal (or registers)
values.
OMAP2, OMAP3 and OMAP4 platforms are defining a conversion routine.
In the new API the function omap_set_pwrdm_state takes the functional
states as parameter; while at it the function is moved to the power
domains code.
The memory and logic states are not using the functional states, this
comes as a subsequent patch.
Signed-off-by: Jean Pihet
---
arch/arm/mach-omap2/pm.c | 66 ---
arch/arm/mach-omap2/powerdomain-common.c | 61 ++
arch/arm/mach-omap2/powerdomain.c | 175
arch/arm/mach-omap2/powerdomain.h | 21
arch/arm/mach-omap2/powerdomain2xxx_3xxx.c |5 +
arch/arm/mach-omap2/powerdomain44xx.c |2 +
6 files changed, 264 insertions(+), 66 deletions(-)
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 6918a13..8670c4b 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -74,10 +74,6 @@ static void __init omap2_init_processor_devices(void)
}
}
-/* Types of sleep_switch used in omap_set_pwrdm_state */
-#define FORCEWAKEUP_SWITCH 0
-#define LOWPOWERSTATE_SWITCH 1
-
int __init omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused)
{
if (clkdm->flags & CLKDM_CAN_ENABLE_AUTO)
@@ -89,68 +85,6 @@ int __init omap_pm_clkdms_setup(struct clockdomain *clkdm,
void *unused)
}
/*
- * This sets pwrdm state (other than mpu & core. Currently only ON &
- * RET are supported.
- */
-int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst)
-{
- u8 curr_pwrst, next_pwrst;
- int sleep_switch = -1, ret = 0, hwsup = 0;
-
- if (!pwrdm || IS_ERR(pwrdm))
- return -EINVAL;
-
- mutex_lock(&pwrdm->lock);
-
- while (!(pwrdm->pwrsts & (1 << pwrst))) {
- if (pwrst == PWRDM_POWER_OFF)
- goto out;
- pwrst--;
- }
-
- next_pwrst = pwrdm_read_next_pwrst(pwrdm);
- if (next_pwrst == pwrst)
- goto out;
-
- curr_pwrst = pwrdm_read_pwrst(pwrdm);
- if (curr_pwrst < PWRDM_POWER_ON) {
- if ((curr_pwrst > pwrst) &&
- (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
- sleep_switch = LOWPOWERSTATE_SWITCH;
- } else {
- hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
- clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
- sleep_switch = FORCEWAKEUP_SWITCH;
- }
- }
-
- ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
- if (ret)
- pr_err("%s: unable to set power state of powerdomain: %s\n",
- __func__, pwrdm->name);
-
- switch (sleep_switch) {
- case FORCEWAKEUP_SWITCH:
- if (hwsup)
- clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
- else
- clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
- break;
- case LOWPOWERSTATE_SWITCH:
- pwrdm_set_lowpwrstchange(pwrdm);
- pwrdm_wait_transition(pwrdm);
- pwrdm_state_switch(pwrdm);
- break;
- }
-
-out:
- mutex_unlock(&pwrdm->lock);
- return ret;
-}
-
-
-
-/*
* This API is to be called during init to set the various voltage
* domains to the voltage as per the opp table. Typically we boot up
* at the nominal voltage. So this function finds out the rate of
diff --git a/arch/arm/mach-omap2/powerdomain-common.c
b/arch/arm/mach-omap2/powerdomain-common.c
index c0aeabf..098dc42 100644
--- a/arch/arm/mach-omap2/powerdomain-common.c
+++ b/arch/arm/mach-omap2/powerdomain-common.c
@@ -108,3 +108,64 @@ u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank)
return 0;
}
+/*
+ * Functional (i.e. logical) to internal (i.e. registers)
+ * values for the power domains states
+ */
+int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
+{
+ int ret;
+
+ switch (func_pwrst) {
+ case PWRDM_FUNC_PWRST_ON:
+ ret = PWRDM_POWER_ON;
+ break;
+ case PWRDM_FUNC_PWRST_INACTIVE:
+ ret = PWRDM_POWER_INACTIVE;
+ break;
+ case PWRDM_FUNC_PWRST_CSWR:
+ case PWRDM_FUNC_PWRST_OSWR:
+ ret = PWRDM_POWER_RET;
+ break;
+ case PWRDM_FUNC_PWRST_OFF:
+ ret = PWRDM_POWER_OFF;
+ break;
+ default:
+ ret = -1;
+ }
+
+ return ret;
+}
+
+/*
+ * Internal (i.e. registers) to functional (i.e. logical) values
+ * for the power domains states
+ */
+int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst)
+{
+ int ret;
+
+ switch (pwrst) {
+ case
[PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states
From: Jean Pihet
Introduce functional (or logical) states for power domains and the
API functions to read the power domains settings and to convert
between the functional (i.e. logical) and the internal (or registers)
values.
OMAP2, OMAP3 and OMAP4 platforms are defining a conversion routine.
In the new API the function omap_set_pwrdm_state takes the functional
states as parameter; while at it the function is moved to the power
domains code.
The memory and logic states are not using the functional states, this
comes as a subsequent patch.
Signed-off-by: Jean Pihet
---
arch/arm/mach-omap2/pm.c | 66 ---
arch/arm/mach-omap2/powerdomain-common.c | 61 ++
arch/arm/mach-omap2/powerdomain.c | 175
arch/arm/mach-omap2/powerdomain.h | 21
arch/arm/mach-omap2/powerdomain2xxx_3xxx.c |5 +
arch/arm/mach-omap2/powerdomain44xx.c |2 +
6 files changed, 264 insertions(+), 66 deletions(-)
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 6918a13..8670c4b 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -74,10 +74,6 @@ static void __init omap2_init_processor_devices(void)
}
}
-/* Types of sleep_switch used in omap_set_pwrdm_state */
-#define FORCEWAKEUP_SWITCH 0
-#define LOWPOWERSTATE_SWITCH 1
-
int __init omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused)
{
if (clkdm->flags & CLKDM_CAN_ENABLE_AUTO)
@@ -89,68 +85,6 @@ int __init omap_pm_clkdms_setup(struct clockdomain *clkdm,
void *unused)
}
/*
- * This sets pwrdm state (other than mpu & core. Currently only ON &
- * RET are supported.
- */
-int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst)
-{
- u8 curr_pwrst, next_pwrst;
- int sleep_switch = -1, ret = 0, hwsup = 0;
-
- if (!pwrdm || IS_ERR(pwrdm))
- return -EINVAL;
-
- mutex_lock(&pwrdm->lock);
-
- while (!(pwrdm->pwrsts & (1 << pwrst))) {
- if (pwrst == PWRDM_POWER_OFF)
- goto out;
- pwrst--;
- }
-
- next_pwrst = pwrdm_read_next_pwrst(pwrdm);
- if (next_pwrst == pwrst)
- goto out;
-
- curr_pwrst = pwrdm_read_pwrst(pwrdm);
- if (curr_pwrst < PWRDM_POWER_ON) {
- if ((curr_pwrst > pwrst) &&
- (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
- sleep_switch = LOWPOWERSTATE_SWITCH;
- } else {
- hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
- clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
- sleep_switch = FORCEWAKEUP_SWITCH;
- }
- }
-
- ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
- if (ret)
- pr_err("%s: unable to set power state of powerdomain: %s\n",
- __func__, pwrdm->name);
-
- switch (sleep_switch) {
- case FORCEWAKEUP_SWITCH:
- if (hwsup)
- clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
- else
- clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
- break;
- case LOWPOWERSTATE_SWITCH:
- pwrdm_set_lowpwrstchange(pwrdm);
- pwrdm_wait_transition(pwrdm);
- pwrdm_state_switch(pwrdm);
- break;
- }
-
-out:
- mutex_unlock(&pwrdm->lock);
- return ret;
-}
-
-
-
-/*
* This API is to be called during init to set the various voltage
* domains to the voltage as per the opp table. Typically we boot up
* at the nominal voltage. So this function finds out the rate of
diff --git a/arch/arm/mach-omap2/powerdomain-common.c
b/arch/arm/mach-omap2/powerdomain-common.c
index c0aeabf..098dc42 100644
--- a/arch/arm/mach-omap2/powerdomain-common.c
+++ b/arch/arm/mach-omap2/powerdomain-common.c
@@ -108,3 +108,64 @@ u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank)
return 0;
}
+/*
+ * Functional (i.e. logical) to internal (i.e. registers)
+ * values for the power domains states
+ */
+int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
+{
+ int ret;
+
+ switch (func_pwrst) {
+ case PWRDM_FUNC_PWRST_ON:
+ ret = PWRDM_POWER_ON;
+ break;
+ case PWRDM_FUNC_PWRST_INACTIVE:
+ ret = PWRDM_POWER_INACTIVE;
+ break;
+ case PWRDM_FUNC_PWRST_CSWR:
+ case PWRDM_FUNC_PWRST_OSWR:
+ ret = PWRDM_POWER_RET;
+ break;
+ case PWRDM_FUNC_PWRST_OFF:
+ ret = PWRDM_POWER_OFF;
+ break;
+ default:
+ ret = -1;
+ }
+
+ return ret;
+}
+
+/*
+ * Internal (i.e. registers) to functional (i.e. logical) values
+ * for the power domains states
+ */
+int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst)
+{
+ int ret;
+
+ switch (pwrst) {
+ case
