* [RFC/PATCH v2 0/6] ARM: OMAP2+: PM: introduce the power domains functional states
@ 2012-04-11 20:46 jean.pihet
2012-04-11 20:46 ` [PATCH 1/6] ARM: OMAP2+: PM: protect the power domain state change by a mutex jean.pihet
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: jean.pihet @ 2012-04-11 20:46 UTC (permalink / raw)
To: ldc_pm_all, linux-omap, paul, b-cousson, khilman; +Cc: Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
Implement the functional states for the power domains:
- protect the power domain state change by a mutex in
omap_set_pwrdm_state,
- introduce the functional states for power domains power states and
logic power states, and the conversion functions between the
functional and internal states,
- unify the API to use the functional states,
- program the logic power state of power domains from the functional
states, in omap_set_pwrdm_state
- convert the OMAP2/3/4 PM code to use the updated API,
- provide the power domains statistics by functional states.
Tested on OMAP3 Beagleboard, with suspend and cpuidle in RET and
OFF modes.
Jean Pihet (6):
ARM: OMAP2+: PM: protect the power domain state change by a mutex
ARM: OMAP2+: PM: introduce power domains functional states
ARM: OMAP2+: PM: use the functional power states API
ARM: OMAP2+: PM: introduce power domains logic and memory functional
states
ARM: OMAP2+: PM: use the functional power states API for logic and
memory
ARM: OMAP2+: PM: use power domain functional state in stats counters
arch/arm/mach-omap2/cpuidle34xx.c | 61 ++++----
arch/arm/mach-omap2/cpuidle44xx.c | 28 ++--
arch/arm/mach-omap2/omap-hotplug.c | 2 +-
arch/arm/mach-omap2/omap-mpuss-lowpower.c | 39 +++---
arch/arm/mach-omap2/pm-debug.c | 15 +-
arch/arm/mach-omap2/pm.c | 62 -------
arch/arm/mach-omap2/pm24xx.c | 14 +-
arch/arm/mach-omap2/pm34xx.c | 80 +++++-----
arch/arm/mach-omap2/pm44xx.c | 15 +--
arch/arm/mach-omap2/powerdomain-common.c | 85 ++++++++++
arch/arm/mach-omap2/powerdomain.c | 244 ++++++++++++++++++++++++++--
arch/arm/mach-omap2/powerdomain.h | 42 +++++-
arch/arm/mach-omap2/powerdomain2xxx_3xxx.c | 7 +
arch/arm/mach-omap2/powerdomain44xx.c | 3 +
14 files changed, 489 insertions(+), 208 deletions(-)
--
1.7.7.6
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/6] ARM: OMAP2+: PM: protect the power domain state change by a mutex 2012-04-11 20:46 [RFC/PATCH v2 0/6] ARM: OMAP2+: PM: introduce the power domains functional states jean.pihet @ 2012-04-11 20:46 ` jean.pihet 2012-04-11 20:46 ` [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states jean.pihet ` (4 subsequent siblings) 5 siblings, 0 replies; 21+ messages in thread From: jean.pihet @ 2012-04-11 20:46 UTC (permalink / raw) To: ldc_pm_all, linux-omap, paul, b-cousson, khilman; +Cc: Jean Pihet From: Jean Pihet <j-pihet@ti.com> Signed-off-by: Jean Pihet <j-pihet@ti.com> --- arch/arm/mach-omap2/pm.c | 8 ++++++-- arch/arm/mach-omap2/powerdomain.c | 1 + arch/arm/mach-omap2/powerdomain.h | 3 ++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c index d0c1c96..6918a13 100644 --- a/arch/arm/mach-omap2/pm.c +++ b/arch/arm/mach-omap2/pm.c @@ -100,15 +100,17 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst) if (!pwrdm || IS_ERR(pwrdm)) return -EINVAL; + mutex_lock(&pwrdm->lock); + while (!(pwrdm->pwrsts & (1 << pwrst))) { if (pwrst == PWRDM_POWER_OFF) - return ret; + goto out; pwrst--; } next_pwrst = pwrdm_read_next_pwrst(pwrdm); if (next_pwrst == pwrst) - return ret; + goto out; curr_pwrst = pwrdm_read_pwrst(pwrdm); if (curr_pwrst < PWRDM_POWER_ON) { @@ -141,6 +143,8 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst) break; } +out: + mutex_unlock(&pwrdm->lock); return ret; } diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 96ad3dbe..319b277 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -102,6 +102,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm) INIT_LIST_HEAD(&pwrdm->voltdm_node); voltdm_add_pwrdm(voltdm, pwrdm); + mutex_init(&pwrdm->lock); list_add(&pwrdm->node, &pwrdm_list); /* Initialize the powerdomain's state counter */ diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h index 0d72a8a..6c6567d 100644 --- a/arch/arm/mach-omap2/powerdomain.h +++ b/arch/arm/mach-omap2/powerdomain.h @@ -19,7 +19,7 @@ #include <linux/types.h> #include <linux/list.h> - +#include <linux/mutex.h> #include <linux/atomic.h> #include <plat/cpu.h> @@ -116,6 +116,7 @@ struct powerdomain { struct clockdomain *pwrdm_clkdms[PWRDM_MAX_CLKDMS]; struct list_head node; struct list_head voltdm_node; + struct mutex lock; int state; unsigned state_counter[PWRDM_MAX_PWRSTS]; unsigned ret_logic_off_counter; -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states 2012-04-11 20:46 [RFC/PATCH v2 0/6] ARM: OMAP2+: PM: introduce the power domains functional states jean.pihet 2012-04-11 20:46 ` [PATCH 1/6] ARM: OMAP2+: PM: protect the power domain state change by a mutex jean.pihet @ 2012-04-11 20:46 ` jean.pihet 2012-04-11 20:46 ` [PATCH 3/6] ARM: OMAP2+: PM: use the functional power states API jean.pihet ` (3 subsequent siblings) 5 siblings, 0 replies; 21+ messages in thread From: jean.pihet @ 2012-04-11 20:46 UTC (permalink / raw) To: ldc_pm_all, linux-omap, paul, b-cousson, khilman; +Cc: Jean Pihet From: Jean Pihet <j-pihet@ti.com> 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 <j-pihet@ti.com> --- 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 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; +} + 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 + * @pwrdm: struct powerdomain * to query + * @func_pwrst: functional power state + * + * Convert the functional power state to the platform specific power + * domain state value. + * Returns the internal power domain state value or -EINVAL in case + * of invalid parameters passed in. + */ +int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) +{ + int ret = func_pwrst; + + if ((!pwrdm)|| (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS)) + return -EINVAL; + + pr_debug("powerdomain: convert %0x func to pwrst for %s\n", + func_pwrst, pwrdm->name); + + if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_pwrst) { + ret = arch_pwrdm->pwrdm_func_to_pwrst(pwrdm, func_pwrst); + } + + return ret; +} + +/** + * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped + * to the internal state + * @pwrdm: struct powerdomain * to query + * @pwrst: internal power state + * + * Convert the internal power state to the power domain functional value. + * Returns the functional power domain state value or -EINVAL in case + * of invalid parameters passed in. + */ +int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) +{ + int ret = pwrst; + + if ((!pwrdm)|| (pwrst >= PWRDM_MAX_PWRSTS)) + return -EINVAL; + + pr_debug("powerdomain: convert %0x pwrst to func for %s\n", + pwrst, pwrdm->name); + + if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func) { + ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst); + } + + return ret; +} + +/* Types of sleep_switch used in omap_set_pwrdm_state */ +#define FORCEWAKEUP_SWITCH 0 +#define LOWPOWERSTATE_SWITCH 1 + +/** + * omap_set_pwrdm_state - program next powerdomain power state + * @pwrdm: struct powerdomain * to set + * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros + * + * This programs the pwrdm next functional state, sets the dependencies + * and waits for the state to be applied. + */ +int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) +{ + u8 curr_pwrst, next_pwrst; + int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst); + int sleep_switch = -1, ret = 0, hwsup = 0; + + if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0)) { + pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n", + __func__, pwrdm, func_pwrst); + return -EINVAL; + } + + pr_debug("%s: set func_pwrst %0x to pwrdm %s\n", + __func__, func_pwrst, pwrdm->name); + + 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; +} + +/** * pwrdm_set_next_pwrst - set next powerdomain power state * @pwrdm: struct powerdomain * to set * @pwrst: one of the PWRDM_POWER_* macros @@ -522,6 +652,21 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm) } /** + * pwrdm_read_next_func_pwrst - get next powerdomain functional power state + * @pwrdm: struct powerdomain * to get power state + * + * Return the powerdomain @pwrdm's next functional power state. + * Returns -EINVAL if the powerdomain pointer is null or returns + * the next power state upon success. + */ +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm) +{ + int ret = pwrdm_read_next_pwrst(pwrdm); + + return pwrdm_pwrst_to_func(pwrdm, ret); +} + +/** * pwrdm_read_pwrst - get current powerdomain power state * @pwrdm: struct powerdomain * to get power state * @@ -543,6 +688,21 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm) } /** + * pwrdm_read_func_pwrst - get current powerdomain functional power state + * @pwrdm: struct powerdomain * to get power state + * + * Return the powerdomain @pwrdm's current functional power state. + * Returns -EINVAL if the powerdomain pointer is null or returns + * the current power state upon success. + */ +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm) +{ + int ret = pwrdm_read_pwrst(pwrdm); + + return pwrdm_pwrst_to_func(pwrdm, ret); +} + +/** * pwrdm_read_prev_pwrst - get previous powerdomain power state * @pwrdm: struct powerdomain * to get previous power state * @@ -564,6 +724,21 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm) } /** + * pwrdm_read_prev_func_pwrst - get previous powerdomain functional power state + * @pwrdm: struct powerdomain * to get previous power state + * + * Return the powerdomain @pwrdm's previous functional power state. + * Returns -EINVAL if the powerdomain pointer is null or returns the + * previous power state upon success. + */ +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm) +{ + int ret = pwrdm_read_prev_pwrst(pwrdm); + + return pwrdm_pwrst_to_func(pwrdm, ret); +} + +/** * pwrdm_set_logic_retst - set powerdomain logic power state upon retention * @pwrdm: struct powerdomain * to set * @pwrst: one of the PWRDM_POWER_* macros diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h index 6c6567d..460af0e 100644 --- a/arch/arm/mach-omap2/powerdomain.h +++ b/arch/arm/mach-omap2/powerdomain.h @@ -45,6 +45,14 @@ #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) #define PWRSTS_OFF_RET_ON (PWRSTS_OFF_RET | PWRSTS_ON) +/* Powerdomain functional power states, used by the external API functions */ +#define PWRDM_FUNC_PWRST_OFF 0x0 +#define PWRDM_FUNC_PWRST_OSWR 0x1 +#define PWRDM_FUNC_PWRST_CSWR 0x2 +#define PWRDM_FUNC_PWRST_INACTIVE 0x3 +#define PWRDM_FUNC_PWRST_ON 0x4 + +#define PWRDM_MAX_FUNC_PWRSTS 5 /* Powerdomain flags */ #define PWRDM_HAS_HDWR_SAR (1 << 0) /* hardware save-and-restore support */ @@ -130,6 +138,10 @@ struct powerdomain { /** * struct pwrdm_ops - Arch specific function implementations + * @pwrdm_func_to_pwrst: Convert the pd functional power state to + * the internal state + * @pwrdm_pwrst_to_func: Convert the pd internal power state to + * the functional state * @pwrdm_set_next_pwrst: Set the target power state for a pd * @pwrdm_read_next_pwrst: Read the target power state set for a pd * @pwrdm_read_pwrst: Read the current power state of a pd @@ -150,6 +162,8 @@ struct powerdomain { * @pwrdm_wait_transition: Wait for a pd state transition to complete */ struct pwrdm_ops { + int (*pwrdm_func_to_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst); + int (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst); int (*pwrdm_set_next_pwrst)(struct powerdomain *pwrdm, u8 pwrst); int (*pwrdm_read_next_pwrst)(struct powerdomain *pwrdm); int (*pwrdm_read_pwrst)(struct powerdomain *pwrdm); @@ -190,6 +204,13 @@ struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm); int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm); +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm); +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm); +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm); + +int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst); +int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst); + int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst); int pwrdm_read_next_pwrst(struct powerdomain *pwrdm); int pwrdm_read_pwrst(struct powerdomain *pwrdm); diff --git a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c index 0f0a9f1..79c1293 100644 --- a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c +++ b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c @@ -26,6 +26,7 @@ /* Common functions across OMAP2 and OMAP3 */ + static int omap2_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst) { omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK, @@ -210,6 +211,8 @@ static int omap3_pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm) } struct pwrdm_ops omap2_pwrdm_operations = { + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, @@ -222,6 +225,8 @@ struct pwrdm_ops omap2_pwrdm_operations = { }; struct pwrdm_ops omap3_pwrdm_operations = { + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, diff --git a/arch/arm/mach-omap2/powerdomain44xx.c b/arch/arm/mach-omap2/powerdomain44xx.c index 601325b..538b528 100644 --- a/arch/arm/mach-omap2/powerdomain44xx.c +++ b/arch/arm/mach-omap2/powerdomain44xx.c @@ -209,6 +209,8 @@ static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm) } struct pwrdm_ops omap4_pwrdm_operations = { + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap4_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap4_pwrdm_read_next_pwrst, .pwrdm_read_pwrst = omap4_pwrdm_read_pwrst, -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/6] ARM: OMAP2+: PM: use the functional power states API 2012-04-11 20:46 [RFC/PATCH v2 0/6] ARM: OMAP2+: PM: introduce the power domains functional states jean.pihet 2012-04-11 20:46 ` [PATCH 1/6] ARM: OMAP2+: PM: protect the power domain state change by a mutex jean.pihet 2012-04-11 20:46 ` [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states jean.pihet @ 2012-04-11 20:46 ` jean.pihet 2012-04-11 20:46 ` [PATCH 4/6] ARM: OMAP2+: PM: introduce power domains logic and memory functional states jean.pihet ` (2 subsequent siblings) 5 siblings, 0 replies; 21+ messages in thread From: jean.pihet @ 2012-04-11 20:46 UTC (permalink / raw) To: ldc_pm_all, linux-omap, paul, b-cousson, khilman; +Cc: Jean Pihet From: Jean Pihet <j-pihet@ti.com> Use the functional power states as the API to control power domains. The omap_set_pwrdm_state function shall be used instead of pwrdm_set_next_pwrst to control the power domains next states. Note: the internal code for power domains state management still uses the internal power states. Signed-off-by: Jean Pihet <j-pihet@ti.com> --- arch/arm/mach-omap2/cpuidle34xx.c | 61 +++++++++++----------- arch/arm/mach-omap2/cpuidle44xx.c | 25 +++++---- arch/arm/mach-omap2/omap-hotplug.c | 2 +- arch/arm/mach-omap2/omap-mpuss-lowpower.c | 43 +++++++++------- arch/arm/mach-omap2/pm24xx.c | 10 ++-- arch/arm/mach-omap2/pm34xx.c | 80 +++++++++++++++-------------- arch/arm/mach-omap2/pm44xx.c | 8 ++-- arch/arm/mach-omap2/powerdomain.c | 2 +- 8 files changed, 120 insertions(+), 111 deletions(-) diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c index 5358664..538aea5 100644 --- a/arch/arm/mach-omap2/cpuidle34xx.c +++ b/arch/arm/mach-omap2/cpuidle34xx.c @@ -97,8 +97,8 @@ static int __omap3_enter_idle(struct cpuidle_device *dev, local_fiq_disable(); - pwrdm_set_next_pwrst(mpu_pd, mpu_state); - pwrdm_set_next_pwrst(core_pd, core_state); + omap_set_pwrdm_state(mpu_pd, mpu_state); + omap_set_pwrdm_state(core_pd, core_state); if (omap_irq_pending() || need_resched()) goto return_sleep_time; @@ -113,7 +113,7 @@ static int __omap3_enter_idle(struct cpuidle_device *dev, * Call idle CPU PM enter notifier chain so that * VFP context is saved. */ - if (mpu_state == PWRDM_POWER_OFF) + if (mpu_state == PWRDM_FUNC_PWRST_OFF) cpu_pm_enter(); /* Execute ARM wfi */ @@ -123,7 +123,7 @@ static int __omap3_enter_idle(struct cpuidle_device *dev, * Call idle CPU PM enter notifier chain to restore * VFP context. */ - if (pwrdm_read_prev_pwrst(mpu_pd) == PWRDM_POWER_OFF) + if (pwrdm_read_prev_func_pwrst(mpu_pd) == PWRDM_FUNC_PWRST_OFF) cpu_pm_exit(); /* Re-allow idle for C1 */ @@ -175,19 +175,19 @@ static int next_valid_state(struct cpuidle_device *dev, struct cpuidle_state_usage *curr_usage = &dev->states_usage[index]; struct cpuidle_state *curr = &drv->states[index]; struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr_usage); - u32 mpu_deepest_state = PWRDM_POWER_RET; - u32 core_deepest_state = PWRDM_POWER_RET; + u32 mpu_deepest_state = PWRDM_FUNC_PWRST_CSWR; + u32 core_deepest_state = PWRDM_FUNC_PWRST_CSWR; int next_index = -1; if (enable_off_mode) { - mpu_deepest_state = PWRDM_POWER_OFF; + mpu_deepest_state = PWRDM_FUNC_PWRST_OFF; /* * Erratum i583: valable for ES rev < Es1.2 on 3630. * CORE OFF mode is not supported in a stable form, restrict * instead the CORE state to RET. */ if (!IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) - core_deepest_state = PWRDM_POWER_OFF; + core_deepest_state = PWRDM_FUNC_PWRST_OFF; } /* Check if current state is valid */ @@ -247,7 +247,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev, int index) { int new_state_idx; - u32 core_next_state, per_next_state = 0, per_saved_state = 0, cam_state; + u32 core_next_state, per_next_state = 0, per_saved_state = 0; struct omap3_idle_statedata *cx; int ret; @@ -255,8 +255,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev, * Prevent idle completely if CAM is active. * CAM does not have wakeup capability in OMAP3. */ - cam_state = pwrdm_read_pwrst(cam_pd); - if (cam_state == PWRDM_POWER_ON) { + if (pwrdm_read_func_pwrst(cam_pd) == PWRDM_FUNC_PWRST_ON) { new_state_idx = drv->safe_state_index; goto select_state; } @@ -275,14 +274,14 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev, */ cx = cpuidle_get_statedata(&dev->states_usage[index]); core_next_state = cx->core_state; - per_next_state = per_saved_state = pwrdm_read_next_pwrst(per_pd); - if ((per_next_state == PWRDM_POWER_OFF) && - (core_next_state > PWRDM_POWER_RET)) - per_next_state = PWRDM_POWER_RET; + per_next_state = per_saved_state = pwrdm_read_next_func_pwrst(per_pd); + if (per_next_state == PWRDM_FUNC_PWRST_OFF || + core_next_state > PWRDM_FUNC_PWRST_CSWR) + per_next_state = PWRDM_FUNC_PWRST_CSWR; /* Are we changing PER target state? */ if (per_next_state != per_saved_state) - pwrdm_set_next_pwrst(per_pd, per_next_state); + omap_set_pwrdm_state(per_pd, per_next_state); new_state_idx = next_valid_state(dev, drv, index); @@ -291,7 +290,7 @@ select_state: /* Restore original PER state if it was modified */ if (per_next_state != per_saved_state) - pwrdm_set_next_pwrst(per_pd, per_saved_state); + omap_set_pwrdm_state(per_pd, per_saved_state); return ret; } @@ -376,38 +375,38 @@ int __init omap3_idle_init(void) drv->safe_state_index = 0; cx = _fill_cstate_usage(dev, 0); cx->valid = 1; /* C1 is always valid */ - cx->mpu_state = PWRDM_POWER_ON; - cx->core_state = PWRDM_POWER_ON; + cx->mpu_state = PWRDM_FUNC_PWRST_ON; + cx->core_state = PWRDM_FUNC_PWRST_ON; /* C2 . MPU WFI + Core inactive */ _fill_cstate(drv, 1, "MPU ON + CORE ON"); cx = _fill_cstate_usage(dev, 1); - cx->mpu_state = PWRDM_POWER_ON; - cx->core_state = PWRDM_POWER_ON; + cx->mpu_state = PWRDM_FUNC_PWRST_ON; + cx->core_state = PWRDM_FUNC_PWRST_ON; /* C3 . MPU CSWR + Core inactive */ _fill_cstate(drv, 2, "MPU RET + CORE ON"); cx = _fill_cstate_usage(dev, 2); - cx->mpu_state = PWRDM_POWER_RET; - cx->core_state = PWRDM_POWER_ON; + cx->mpu_state = PWRDM_FUNC_PWRST_CSWR; + cx->core_state = PWRDM_FUNC_PWRST_ON; /* C4 . MPU OFF + Core inactive */ _fill_cstate(drv, 3, "MPU OFF + CORE ON"); cx = _fill_cstate_usage(dev, 3); - cx->mpu_state = PWRDM_POWER_OFF; - cx->core_state = PWRDM_POWER_ON; + cx->mpu_state = PWRDM_FUNC_PWRST_OFF; + cx->core_state = PWRDM_FUNC_PWRST_ON; /* C5 . MPU RET + Core RET */ _fill_cstate(drv, 4, "MPU RET + CORE RET"); cx = _fill_cstate_usage(dev, 4); - cx->mpu_state = PWRDM_POWER_RET; - cx->core_state = PWRDM_POWER_RET; + cx->mpu_state = PWRDM_FUNC_PWRST_CSWR; + cx->core_state = PWRDM_FUNC_PWRST_CSWR; /* C6 . MPU OFF + Core RET */ _fill_cstate(drv, 5, "MPU OFF + CORE RET"); cx = _fill_cstate_usage(dev, 5); - cx->mpu_state = PWRDM_POWER_OFF; - cx->core_state = PWRDM_POWER_RET; + cx->mpu_state = PWRDM_FUNC_PWRST_OFF; + cx->core_state = PWRDM_FUNC_PWRST_CSWR; /* C7 . MPU OFF + Core OFF */ _fill_cstate(drv, 6, "MPU OFF + CORE OFF"); @@ -422,8 +421,8 @@ int __init omap3_idle_init(void) pr_warn("%s: core off state C7 disabled due to i583\n", __func__); } - cx->mpu_state = PWRDM_POWER_OFF; - cx->core_state = PWRDM_POWER_OFF; + cx->mpu_state = PWRDM_FUNC_PWRST_OFF; + cx->core_state = PWRDM_FUNC_PWRST_OFF; drv->state_count = OMAP3_NUM_STATES; cpuidle_register_driver(&omap3_idle_driver); diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c index f386cbe..1daa0c1 100644 --- a/arch/arm/mach-omap2/cpuidle44xx.c +++ b/arch/arm/mach-omap2/cpuidle44xx.c @@ -75,8 +75,8 @@ static int omap4_enter_idle(struct cpuidle_device *dev, * Update dev->last_state so that governor stats reflects right * data. */ - cpu1_state = pwrdm_read_pwrst(cpu1_pd); - if (cpu1_state != PWRDM_POWER_OFF) { + cpu1_state = pwrdm_read_func_pwrst(cpu1_pd); + if (cpu1_state != PWRDM_FUNC_PWRST_OFF) { index = drv->safe_state_index; cx = cpuidle_get_statedata(&dev->states_usage[index]); } @@ -88,7 +88,7 @@ static int omap4_enter_idle(struct cpuidle_device *dev, * Call idle CPU PM enter notifier chain so that * VFP and per CPU interrupt context is saved. */ - if (cx->cpu_state == PWRDM_POWER_OFF) + if (cx->cpu_state == PWRDM_FUNC_PWRST_OFF) cpu_pm_enter(); pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state); @@ -98,8 +98,9 @@ static int omap4_enter_idle(struct cpuidle_device *dev, * Call idle CPU cluster PM enter notifier chain * to save GIC and wakeupgen context. */ - if ((cx->mpu_state == PWRDM_POWER_RET) && - (cx->mpu_logic_state == PWRDM_POWER_OFF)) + if (((cx->mpu_state == PWRDM_FUNC_PWRST_CSWR) || + (cx->mpu_state == PWRDM_FUNC_PWRST_OSWR)) && + (cx->mpu_logic_state == PWRDM_POWER_OFF)) cpu_cluster_pm_enter(); omap4_enter_lowpower(dev->cpu, cx->cpu_state); @@ -109,7 +110,7 @@ static int omap4_enter_idle(struct cpuidle_device *dev, * VFP and per CPU IRQ context. Only CPU0 state is * considered since CPU1 is managed by CPU hotplug. */ - if (pwrdm_read_prev_pwrst(cpu0_pd) == PWRDM_POWER_OFF) + if (pwrdm_read_prev_func_pwrst(cpu0_pd) == PWRDM_FUNC_PWRST_OFF) cpu_pm_exit(); /* @@ -192,22 +193,22 @@ int __init omap4_idle_init(void) drv->safe_state_index = 0; cx = _fill_cstate_usage(dev, 0); cx->valid = 1; /* C1 is always valid */ - cx->cpu_state = PWRDM_POWER_ON; - cx->mpu_state = PWRDM_POWER_ON; + cx->cpu_state = PWRDM_FUNC_PWRST_ON; + cx->mpu_state = PWRDM_FUNC_PWRST_ON; cx->mpu_logic_state = PWRDM_POWER_RET; /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ _fill_cstate(drv, 1, "MPUSS CSWR"); cx = _fill_cstate_usage(dev, 1); - cx->cpu_state = PWRDM_POWER_OFF; - cx->mpu_state = PWRDM_POWER_RET; + cx->cpu_state = PWRDM_FUNC_PWRST_OFF; + cx->mpu_state = PWRDM_FUNC_PWRST_CSWR; cx->mpu_logic_state = PWRDM_POWER_RET; /* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ _fill_cstate(drv, 2, "MPUSS OSWR"); cx = _fill_cstate_usage(dev, 2); - cx->cpu_state = PWRDM_POWER_OFF; - cx->mpu_state = PWRDM_POWER_RET; + cx->cpu_state = PWRDM_FUNC_PWRST_OFF; + cx->mpu_state = PWRDM_FUNC_PWRST_OSWR; cx->mpu_logic_state = PWRDM_POWER_OFF; drv->state_count = OMAP4_NUM_STATES; diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c index 56c345b..cb7a7eb 100644 --- a/arch/arm/mach-omap2/omap-hotplug.c +++ b/arch/arm/mach-omap2/omap-hotplug.c @@ -50,7 +50,7 @@ void __ref platform_cpu_die(unsigned int cpu) /* * Enter into low power state */ - omap4_hotplug_cpu(cpu, PWRDM_POWER_OFF); + omap4_hotplug_cpu(cpu, PWRDM_FUNC_PWRST_OFF); this_cpu = smp_processor_id(); if (omap_read_auxcoreboot0() == this_cpu) { /* diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c index 13670aa..dd94a0e 100644 --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c @@ -86,14 +86,14 @@ static inline void set_cpu_wakeup_addr(unsigned int cpu_id, u32 addr) } /* - * Set the CPUx powerdomain's previous power state + * Set the CPUx powerdomain's next functional power state */ static inline void set_cpu_next_pwrst(unsigned int cpu_id, unsigned int power_state) { struct omap4_cpu_pm_info *pm_info = &per_cpu(omap4_pm_info, cpu_id); - pwrdm_set_next_pwrst(pm_info->pwrdm, power_state); + omap_set_pwrdm_state(pm_info->pwrdm, power_state); } /* @@ -103,7 +103,7 @@ static inline unsigned int read_cpu_prev_pwrst(unsigned int cpu_id) { struct omap4_cpu_pm_info *pm_info = &per_cpu(omap4_pm_info, cpu_id); - return pwrdm_read_prev_pwrst(pm_info->pwrdm); + return pwrdm_read_prev_func_pwrst(pm_info->pwrdm); } /* @@ -125,14 +125,15 @@ static void scu_pwrst_prepare(unsigned int cpu_id, unsigned int cpu_state) u32 scu_pwr_st; switch (cpu_state) { - case PWRDM_POWER_RET: + case PWRDM_FUNC_PWRST_OSWR: + case PWRDM_FUNC_PWRST_CSWR: scu_pwr_st = SCU_PM_DORMANT; break; - case PWRDM_POWER_OFF: + case PWRDM_FUNC_PWRST_OFF: scu_pwr_st = SCU_PM_POWEROFF; break; - case PWRDM_POWER_ON: - case PWRDM_POWER_INACTIVE: + case PWRDM_FUNC_PWRST_ON: + case PWRDM_FUNC_PWRST_INACTIVE: default: scu_pwr_st = SCU_PM_NORMAL; break; @@ -231,19 +232,21 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state) { unsigned int save_state = 0; unsigned int wakeup_cpu; + int mpuss_state; if (omap_rev() == OMAP4430_REV_ES1_0) return -ENXIO; switch (power_state) { - case PWRDM_POWER_ON: - case PWRDM_POWER_INACTIVE: + case PWRDM_FUNC_PWRST_ON: + case PWRDM_FUNC_PWRST_INACTIVE: save_state = 0; break; - case PWRDM_POWER_OFF: + case PWRDM_FUNC_PWRST_OFF: save_state = 1; break; - case PWRDM_POWER_RET: + case PWRDM_FUNC_PWRST_OSWR: + case PWRDM_FUNC_PWRST_CSWR: default: /* * CPUx CSWR is invalid hardware state. Also CPUx OSWR @@ -259,11 +262,13 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state) /* * Check MPUSS next state and save interrupt controller if needed. - * In MPUSS OSWR or device OFF, interrupt controller contest is lost. + * In MPUSS OSWR or device OFF, interrupt controller context is lost. */ mpuss_clear_prev_logic_pwrst(); - if ((pwrdm_read_next_pwrst(mpuss_pd) == PWRDM_POWER_RET) && - (pwrdm_read_logic_retst(mpuss_pd) == PWRDM_POWER_OFF)) + mpuss_state = pwrdm_read_next_func_pwrst(mpuss_pd); + if (((mpuss_state == PWRDM_FUNC_PWRST_CSWR) || + (mpuss_state == PWRDM_FUNC_PWRST_OSWR)) && + (pwrdm_read_logic_retst(mpuss_pd) == PWRDM_POWER_OFF)) save_state = 2; cpu_clear_prev_logic_pwrst(cpu); @@ -285,7 +290,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state) * domain transition */ wakeup_cpu = smp_processor_id(); - set_cpu_next_pwrst(wakeup_cpu, PWRDM_POWER_ON); + set_cpu_next_pwrst(wakeup_cpu, PWRDM_FUNC_PWRST_ON); pwrdm_post_transition(); @@ -304,7 +309,7 @@ int __cpuinit omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state) if (omap_rev() == OMAP4430_REV_ES1_0) return -ENXIO; - if (power_state == PWRDM_POWER_OFF) + if (power_state == PWRDM_FUNC_PWRST_OFF) cpu_state = 1; clear_cpu_prev_pwrst(cpu); @@ -319,7 +324,7 @@ int __cpuinit omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state) */ omap4_finish_suspend(cpu_state); - set_cpu_next_pwrst(cpu, PWRDM_POWER_ON); + set_cpu_next_pwrst(cpu, PWRDM_FUNC_PWRST_ON); return 0; } @@ -354,7 +359,7 @@ int __init omap4_mpuss_init(void) cpu_clear_prev_logic_pwrst(0); /* Initialise CPU0 power domain state to ON */ - pwrdm_set_next_pwrst(pm_info->pwrdm, PWRDM_POWER_ON); + omap_set_pwrdm_state(pm_info->pwrdm, PWRDM_FUNC_PWRST_ON); pm_info = &per_cpu(omap4_pm_info, 0x1); pm_info->scu_sar_addr = sar_base + SCU_OFFSET1; @@ -371,7 +376,7 @@ int __init omap4_mpuss_init(void) cpu_clear_prev_logic_pwrst(1); /* Initialise CPU1 power domain state to ON */ - pwrdm_set_next_pwrst(pm_info->pwrdm, PWRDM_POWER_ON); + omap_set_pwrdm_state(pm_info->pwrdm, PWRDM_FUNC_PWRST_ON); mpuss_pd = pwrdm_lookup("mpu_pwrdm"); if (!mpuss_pd) { diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c index 95442b6..bb84348 100644 --- a/arch/arm/mach-omap2/pm24xx.c +++ b/arch/arm/mach-omap2/pm24xx.c @@ -94,7 +94,7 @@ static int omap2_enter_full_retention(void) * preserve logic state during retention */ pwrdm_set_logic_retst(mpu_pwrdm, PWRDM_POWER_RET); - pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_RET); + omap_set_pwrdm_state(mpu_pwrdm, PWRDM_FUNC_PWRST_CSWR); /* Workaround to kill USB */ l = omap_ctrl_readl(OMAP2_CONTROL_DEVCONF0) | OMAP24XX_USBSTANDBYCTRL; @@ -254,23 +254,23 @@ static void __init prcm_setup_regs(void) pwrdm_set_mem_retst(core_pwrdm, i, PWRDM_POWER_RET); /* Set CORE powerdomain's next power state to RETENTION */ - pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET); + omap_set_pwrdm_state(core_pwrdm, PWRDM_FUNC_PWRST_CSWR); /* * Set MPU powerdomain's next power state to RETENTION; * preserve logic state during retention */ pwrdm_set_logic_retst(mpu_pwrdm, PWRDM_POWER_RET); - pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_RET); + omap_set_pwrdm_state(mpu_pwrdm, PWRDM_FUNC_PWRST_CSWR); /* Force-power down DSP, GFX powerdomains */ pwrdm = clkdm_get_pwrdm(dsp_clkdm); - pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_OFF); + omap_set_pwrdm_state(pwrdm, PWRDM_FUNC_PWRST_OFF); clkdm_sleep(dsp_clkdm); pwrdm = clkdm_get_pwrdm(gfx_clkdm); - pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_OFF); + omap_set_pwrdm_state(pwrdm, PWRDM_FUNC_PWRST_OFF); clkdm_sleep(gfx_clkdm); /* Enable hardware-supervised idle for all clkdms */ diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c index 703bd10..ed73ffc 100644 --- a/arch/arm/mach-omap2/pm34xx.c +++ b/arch/arm/mach-omap2/pm34xx.c @@ -139,7 +139,7 @@ static void omap3_core_restore_context(void) static void omap3_save_secure_ram_context(void) { u32 ret; - int mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm); + int mpu_next_state = pwrdm_read_next_func_pwrst(mpu_pwrdm); if (omap_type() != OMAP2_DEVICE_TYPE_GP) { /* @@ -147,10 +147,10 @@ static void omap3_save_secure_ram_context(void) * otherwise the WFI executed inside the ROM code * will hang the system. */ - pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_ON); + omap_set_pwrdm_state(mpu_pwrdm, PWRDM_FUNC_PWRST_ON); ret = _omap_save_secure_sram((u32 *) __pa(omap3_secure_ram_storage)); - pwrdm_set_next_pwrst(mpu_pwrdm, mpu_next_state); + omap_set_pwrdm_state(mpu_pwrdm, mpu_next_state); /* Following is for error tracking, it should not happen */ if (ret) { pr_err("save_secure_sram() returns %08x\n", ret); @@ -269,21 +269,23 @@ void omap_sram_idle(void) /* save_state = 2 => Only L2 lost */ /* save_state = 3 => L1, L2 and logic lost */ int save_state = 0; - int mpu_next_state = PWRDM_POWER_ON; - int per_next_state = PWRDM_POWER_ON; - int core_next_state = PWRDM_POWER_ON; + int mpu_next_state = PWRDM_FUNC_PWRST_ON; + int per_next_state = PWRDM_FUNC_PWRST_ON; + int core_next_state = PWRDM_FUNC_PWRST_ON; int per_going_off; int core_prev_state, per_prev_state; u32 sdrc_pwr = 0; - mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm); + mpu_next_state = pwrdm_read_next_func_pwrst(mpu_pwrdm); switch (mpu_next_state) { - case PWRDM_POWER_ON: - case PWRDM_POWER_RET: + case PWRDM_FUNC_PWRST_ON: + case PWRDM_FUNC_PWRST_INACTIVE: + case PWRDM_FUNC_PWRST_CSWR: /* No need to save context */ save_state = 0; break; - case PWRDM_POWER_OFF: + case PWRDM_FUNC_PWRST_OSWR: + case PWRDM_FUNC_PWRST_OFF: save_state = 3; break; default: @@ -293,16 +295,17 @@ void omap_sram_idle(void) } /* NEON control */ - if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON) - pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state); + if (pwrdm_read_func_pwrst(neon_pwrdm) == PWRDM_FUNC_PWRST_ON) + omap_set_pwrdm_state(neon_pwrdm, mpu_next_state); /* Enable IO-PAD and IO-CHAIN wakeups */ - per_next_state = pwrdm_read_next_pwrst(per_pwrdm); - core_next_state = pwrdm_read_next_pwrst(core_pwrdm); + per_next_state = pwrdm_read_next_func_pwrst(per_pwrdm); + core_next_state = pwrdm_read_next_func_pwrst(core_pwrdm); if (omap3_has_io_wakeup() && - (per_next_state < PWRDM_POWER_ON || - core_next_state < PWRDM_POWER_ON)) { - omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD, PM_WKEN); + (per_next_state < PWRDM_FUNC_PWRST_ON || + core_next_state < PWRDM_FUNC_PWRST_ON)) { + omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD, + PM_WKEN); if (omap3_has_io_chain_ctrl()) omap3_enable_io_chain(); } @@ -310,14 +313,15 @@ void omap_sram_idle(void) pwrdm_pre_transition(); /* PER */ - if (per_next_state < PWRDM_POWER_ON) { - per_going_off = (per_next_state == PWRDM_POWER_OFF) ? 1 : 0; + if (per_next_state < PWRDM_FUNC_PWRST_ON) { + per_going_off = (per_next_state == PWRDM_FUNC_PWRST_OFF) ? + 1 : 0; omap2_gpio_prepare_for_idle(per_going_off); } /* CORE */ - if (core_next_state < PWRDM_POWER_ON) { - if (core_next_state == PWRDM_POWER_OFF) { + if (core_next_state < PWRDM_FUNC_PWRST_ON) { + if (core_next_state == PWRDM_FUNC_PWRST_OFF) { omap3_core_save_context(); omap3_cm_save_context(); } @@ -334,7 +338,7 @@ void omap_sram_idle(void) if (cpu_is_omap3430() && omap_rev() >= OMAP3430_REV_ES3_0 && (omap_type() == OMAP2_DEVICE_TYPE_EMU || omap_type() == OMAP2_DEVICE_TYPE_SEC) && - core_next_state == PWRDM_POWER_OFF) + core_next_state == PWRDM_FUNC_PWRST_OFF) sdrc_pwr = sdrc_read_reg(SDRC_POWER); /* @@ -353,19 +357,19 @@ void omap_sram_idle(void) if (cpu_is_omap3430() && omap_rev() >= OMAP3430_REV_ES3_0 && (omap_type() == OMAP2_DEVICE_TYPE_EMU || omap_type() == OMAP2_DEVICE_TYPE_SEC) && - core_next_state == PWRDM_POWER_OFF) + core_next_state == PWRDM_FUNC_PWRST_OFF) sdrc_write_reg(sdrc_pwr, SDRC_POWER); /* CORE */ - if (core_next_state < PWRDM_POWER_ON) { - core_prev_state = pwrdm_read_prev_pwrst(core_pwrdm); - if (core_prev_state == PWRDM_POWER_OFF) { + if (core_next_state < PWRDM_FUNC_PWRST_ON) { + core_prev_state = pwrdm_read_prev_func_pwrst(core_pwrdm); + if (core_prev_state == PWRDM_FUNC_PWRST_OFF) { omap3_core_restore_context(); omap3_cm_restore_context(); omap3_sram_restore_context(); omap2_sms_restore_context(); } - if (core_next_state == PWRDM_POWER_OFF) + if (core_next_state == PWRDM_FUNC_PWRST_OFF) omap2_prm_clear_mod_reg_bits(OMAP3430_AUTO_OFF_MASK, OMAP3430_GR_MOD, OMAP3_PRM_VOLTCTRL_OFFSET); @@ -375,15 +379,15 @@ void omap_sram_idle(void) pwrdm_post_transition(); /* PER */ - if (per_next_state < PWRDM_POWER_ON) { - per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm); + if (per_next_state < PWRDM_FUNC_PWRST_ON) { + per_prev_state = pwrdm_read_prev_func_pwrst(per_pwrdm); omap2_gpio_resume_after_idle(); } /* Disable IO-PAD and IO-CHAIN wakeup */ if (omap3_has_io_wakeup() && - (per_next_state < PWRDM_POWER_ON || - core_next_state < PWRDM_POWER_ON)) { + (per_next_state < PWRDM_FUNC_PWRST_ON || + core_next_state < PWRDM_FUNC_PWRST_ON)) { omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD, PM_WKEN); if (omap3_has_io_chain_ctrl()) @@ -420,7 +424,7 @@ static int omap3_pm_suspend(void) /* Read current next_pwrsts */ list_for_each_entry(pwrst, &pwrst_list, node) - pwrst->saved_state = pwrdm_read_next_pwrst(pwrst->pwrdm); + pwrst->saved_state = pwrdm_read_next_func_pwrst(pwrst->pwrdm); /* Set ones wanted by suspend */ list_for_each_entry(pwrst, &pwrst_list, node) { if (omap_set_pwrdm_state(pwrst->pwrdm, pwrst->next_state)) @@ -436,7 +440,7 @@ static int omap3_pm_suspend(void) restore: /* Restore next_pwrsts */ list_for_each_entry(pwrst, &pwrst_list, node) { - state = pwrdm_read_prev_pwrst(pwrst->pwrdm); + state = pwrdm_read_prev_func_pwrst(pwrst->pwrdm); if (state > pwrst->next_state) { pr_info("Powerdomain (%s) didn't enter " "target state %d\n", @@ -610,15 +614,15 @@ void omap3_pm_off_mode_enable(int enable) u32 state; if (enable) - state = PWRDM_POWER_OFF; + state = PWRDM_FUNC_PWRST_OFF; else - state = PWRDM_POWER_RET; + state = PWRDM_FUNC_PWRST_CSWR; list_for_each_entry(pwrst, &pwrst_list, node) { if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583) && pwrst->pwrdm == core_pwrdm && - state == PWRDM_POWER_OFF) { - pwrst->next_state = PWRDM_POWER_RET; + state == PWRDM_FUNC_PWRST_OFF) { + pwrst->next_state = PWRDM_FUNC_PWRST_CSWR; pr_warn("%s: Core OFF disabled due to errata i583\n", __func__); } else { @@ -663,7 +667,7 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused) if (!pwrst) return -ENOMEM; pwrst->pwrdm = pwrdm; - pwrst->next_state = PWRDM_POWER_RET; + pwrst->next_state = PWRDM_FUNC_PWRST_CSWR; list_add(&pwrst->node, &pwrst_list); if (pwrdm_has_hdwr_sar(pwrdm)) diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c index 8856253..897fdee 100644 --- a/arch/arm/mach-omap2/pm44xx.c +++ b/arch/arm/mach-omap2/pm44xx.c @@ -44,7 +44,7 @@ static int omap4_pm_suspend(void) /* Save current powerdomain state */ list_for_each_entry(pwrst, &pwrst_list, node) { - pwrst->saved_state = pwrdm_read_next_pwrst(pwrst->pwrdm); + pwrst->saved_state = pwrdm_read_next_func_pwrst(pwrst->pwrdm); pwrst->saved_logic_state = pwrdm_read_logic_retst(pwrst->pwrdm); } @@ -63,11 +63,11 @@ static int omap4_pm_suspend(void) * domain CSWR is not supported by hardware. * More details can be found in OMAP4430 TRM section 4.3.4.2. */ - omap4_enter_lowpower(cpu_id, PWRDM_POWER_OFF); + omap4_enter_lowpower(cpu_id, PWRDM_FUNC_PWRST_OFF); /* Restore next powerdomain state */ list_for_each_entry(pwrst, &pwrst_list, node) { - state = pwrdm_read_prev_pwrst(pwrst->pwrdm); + state = pwrdm_read_prev_func_pwrst(pwrst->pwrdm); if (state > pwrst->next_state) { pr_info("Powerdomain (%s) didn't enter " "target state %d\n", @@ -113,7 +113,7 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused) return -ENOMEM; pwrst->pwrdm = pwrdm; - pwrst->next_state = PWRDM_POWER_RET; + pwrst->next_state = PWRDM_FUNC_PWRST_CSWR; list_add(&pwrst->node, &pwrst_list); return omap_set_pwrdm_state(pwrst->pwrdm, pwrst->next_state); diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 718fa43..28d5599 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -270,7 +270,7 @@ int pwrdm_complete_init(void) return -EACCES; list_for_each_entry(temp_p, &pwrdm_list, node) - pwrdm_set_next_pwrst(temp_p, PWRDM_POWER_ON); + omap_set_pwrdm_state(temp_p, PWRDM_FUNC_PWRST_ON); return 0; } -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/6] ARM: OMAP2+: PM: introduce power domains logic and memory functional states 2012-04-11 20:46 [RFC/PATCH v2 0/6] ARM: OMAP2+: PM: introduce the power domains functional states jean.pihet ` (2 preceding siblings ...) 2012-04-11 20:46 ` [PATCH 3/6] ARM: OMAP2+: PM: use the functional power states API jean.pihet @ 2012-04-11 20:46 ` jean.pihet 2012-04-11 20:46 ` [PATCH 5/6] ARM: OMAP2+: PM: use the functional power states API for logic and memory jean.pihet 2012-04-11 20:46 ` [PATCH 6/6] ARM: OMAP2+: PM: use power domain functional state in stats counters jean.pihet 5 siblings, 0 replies; 21+ messages in thread From: jean.pihet @ 2012-04-11 20:46 UTC (permalink / raw) To: ldc_pm_all, linux-omap, paul, b-cousson, khilman; +Cc: Jean Pihet From: Jean Pihet <j-pihet@ti.com> In the new API the functions pwrdm_*_logic_* and pwrdm_*_mem_* take the functional states as parameter. OMAP2, OMAP3 and OMAP4 platforms are defining a conversion routine between the functional state and the logic power state. The power domains logic state is now derived from the functional state and the logic state is programmed from omap_set_pwrdm_state. Signed-off-by: Jean Pihet <j-pihet@ti.com> --- arch/arm/mach-omap2/powerdomain-common.c | 36 ++++++++++-- arch/arm/mach-omap2/powerdomain.c | 80 +++++++++++++++++++++------ arch/arm/mach-omap2/powerdomain.h | 20 ++++++- arch/arm/mach-omap2/powerdomain2xxx_3xxx.c | 2 + arch/arm/mach-omap2/powerdomain44xx.c | 1 + 5 files changed, 112 insertions(+), 27 deletions(-) diff --git a/arch/arm/mach-omap2/powerdomain-common.c b/arch/arm/mach-omap2/powerdomain-common.c index 098dc42..ecaf405 100644 --- a/arch/arm/mach-omap2/powerdomain-common.c +++ b/arch/arm/mach-omap2/powerdomain-common.c @@ -138,10 +138,35 @@ int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) } /* + * Functional (i.e. logical) to internal (i.e. registers) + * values for the power domains logic states + */ +int omap2_pwrdm_func_to_logic_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) +{ + int ret; + + switch (func_pwrst) { + case PWRDM_FUNC_PWRST_ON: + case PWRDM_FUNC_PWRST_INACTIVE: + case PWRDM_FUNC_PWRST_CSWR: + ret = PWRDM_LOGIC_MEM_PWRST_RET; + break; + case PWRDM_FUNC_PWRST_OSWR: + case PWRDM_FUNC_PWRST_OFF: + ret = PWRDM_LOGIC_MEM_PWRST_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 omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst, u8 logic) { int ret; @@ -153,11 +178,10 @@ int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) 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; + if (logic == PWRDM_LOGIC_MEM_PWRST_RET) + ret = PWRDM_FUNC_PWRST_CSWR; + else + ret = PWRDM_FUNC_PWRST_OSWR; break; case PWRDM_POWER_OFF: ret = PWRDM_FUNC_PWRST_OFF; diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 28d5599..3faac62 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -129,14 +129,14 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm) prev_logic_pwrst = pwrdm_read_prev_logic_pwrst(pwrdm); if ((pwrdm->pwrsts_logic_ret == PWRSTS_OFF_RET) && - (prev_logic_pwrst == PWRDM_POWER_OFF)) + (prev_logic_pwrst == PWRDM_LOGIC_MEM_PWRST_OFF)) pwrdm->ret_logic_off_counter++; for (i = 0; i < pwrdm->banks; i++) { prev_mem_pwrst = pwrdm_read_prev_mem_pwrst(pwrdm, i); if ((pwrdm->pwrsts_mem_ret[i] == PWRSTS_OFF_RET) && - (prev_mem_pwrst == PWRDM_POWER_OFF)) + (prev_mem_pwrst == PWRDM_LOGIC_MEM_PWRST_OFF)) pwrdm->ret_mem_off_counter[i]++; } } @@ -494,27 +494,64 @@ int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) } /** + * pwrdm_func_to_logic_pwrst - get the internal (i.e. registers) value + * of the logic state mapped to the functional state + * @pwrdm: struct powerdomain * to query + * @func_pwrst: functional power state + * + * Convert the functional power state to the platform specific power + * domain logic state value. + * Returns the internal power domain logic state value or -EINVAL in + * case of invalid parameters passed in. + */ +int pwrdm_func_to_logic_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) +{ + int ret = func_pwrst; + + if ((!pwrdm)|| (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS)) + return -EINVAL; + + pr_debug("powerdomain: convert %0x func to logic pwrst for %s\n", + func_pwrst, pwrdm->name); + + if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_logic_pwrst) { + ret = arch_pwrdm->pwrdm_func_to_logic_pwrst(pwrdm, func_pwrst); + } + + return ret; +} + +/** * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped * to the internal state * @pwrdm: struct powerdomain * to query * @pwrst: internal power state + * @logic: internal logic state * - * Convert the internal power state to the power domain functional value. + * Convert the internal power state and logic power state to the + * power domain functional value. * Returns the functional power domain state value or -EINVAL in case * of invalid parameters passed in. */ -int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) +int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst, u8 logic) { int ret = pwrst; - if ((!pwrdm)|| (pwrst >= PWRDM_MAX_PWRSTS)) + if ((!pwrdm) || (pwrst >= PWRDM_MAX_PWRSTS) || + (logic >= PWRDM_MAX_LOGIC_MEM_PWRST)) return -EINVAL; - pr_debug("powerdomain: convert %0x pwrst to func for %s\n", - pwrst, pwrdm->name); + pr_debug("powerdomain: convert (%0x,%0x) pwrst to func for %s\n", + pwrst, logic, pwrdm->name); + + while (!(pwrdm->pwrsts_logic_ret & (1 << logic))) { + if (logic == PWRDM_LOGIC_MEM_PWRST_RET) + break; + logic++; + } if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func) { - ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst); + ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst, logic); } return ret; @@ -536,9 +573,10 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) { u8 curr_pwrst, next_pwrst; int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst); + int logic = pwrdm_func_to_logic_pwrst(pwrdm, func_pwrst); int sleep_switch = -1, ret = 0, hwsup = 0; - if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0)) { + if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) { pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n", __func__, pwrdm, func_pwrst); return -EINVAL; @@ -571,6 +609,9 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) } } + if (logic != pwrdm_read_logic_retst(pwrdm)) + pwrdm_set_logic_retst(pwrdm, logic); + ret = pwrdm_set_next_pwrst(pwrdm, pwrst); if (ret) pr_err("%s: unable to set power state of powerdomain: %s\n", @@ -661,9 +702,10 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm) */ int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm) { - int ret = pwrdm_read_next_pwrst(pwrdm); + int next_pwrst = pwrdm_read_next_pwrst(pwrdm); + int next_logic = pwrdm_read_logic_retst(pwrdm); - return pwrdm_pwrst_to_func(pwrdm, ret); + return pwrdm_pwrst_to_func(pwrdm, next_pwrst, next_logic); } /** @@ -697,9 +739,10 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm) */ int pwrdm_read_func_pwrst(struct powerdomain *pwrdm) { - int ret = pwrdm_read_pwrst(pwrdm); + int pwrst = pwrdm_read_pwrst(pwrdm); + int logic = pwrdm_read_logic_pwrst(pwrdm); - return pwrdm_pwrst_to_func(pwrdm, ret); + return pwrdm_pwrst_to_func(pwrdm, pwrst, logic); } /** @@ -733,15 +776,16 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm) */ int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm) { - int ret = pwrdm_read_prev_pwrst(pwrdm); + int prev_pwrst = pwrdm_read_prev_pwrst(pwrdm); + int prev_logic = pwrdm_read_prev_logic_pwrst(pwrdm); - return pwrdm_pwrst_to_func(pwrdm, ret); + return pwrdm_pwrst_to_func(pwrdm, prev_pwrst, prev_logic); } /** * pwrdm_set_logic_retst - set powerdomain logic power state upon retention * @pwrdm: struct powerdomain * to set - * @pwrst: one of the PWRDM_POWER_* macros + * @pwrst: one of the PWRDM_LOGIC_MEM_PWRST_* macros * * Set the next power state @pwrst that the logic portion of the * powerdomain @pwrdm will enter when the powerdomain enters retention. @@ -772,7 +816,7 @@ int pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst) * pwrdm_set_mem_onst - set memory power state while powerdomain ON * @pwrdm: struct powerdomain * to set * @bank: memory bank number to set (0-3) - * @pwrst: one of the PWRDM_POWER_* macros + * @pwrst: one of the PWRDM_LOGIC_MEM_PWRST_* macros * * Set the next power state @pwrst that memory bank @bank of the * powerdomain @pwrdm will enter when the powerdomain enters the ON @@ -809,7 +853,7 @@ int pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank, u8 pwrst) * pwrdm_set_mem_retst - set memory power state while powerdomain in RET * @pwrdm: struct powerdomain * to set * @bank: memory bank number to set (0-3) - * @pwrst: one of the PWRDM_POWER_* macros + * @pwrst: one of the PWRDM_LOGIC_MEM_PWRST_* macros * * Set the next power state @pwrst that memory bank @bank of the * powerdomain @pwrdm will enter when the powerdomain enters the diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h index 460af0e..4e5ce15 100644 --- a/arch/arm/mach-omap2/powerdomain.h +++ b/arch/arm/mach-omap2/powerdomain.h @@ -54,6 +54,15 @@ #define PWRDM_MAX_FUNC_PWRSTS 5 +/* + * Powerdomains logic and memory functional power states, + * used by the external API functions + */ +#define PWRDM_LOGIC_MEM_PWRST_OFF 0x0 +#define PWRDM_LOGIC_MEM_PWRST_RET 0x1 + +#define PWRDM_MAX_LOGIC_MEM_PWRST 2 + /* Powerdomain flags */ #define PWRDM_HAS_HDWR_SAR (1 << 0) /* hardware save-and-restore support */ #define PWRDM_HAS_MPU_QUIRK (1 << 1) /* MPU pwr domain has MEM bank 0 bits @@ -140,7 +149,9 @@ struct powerdomain { * struct pwrdm_ops - Arch specific function implementations * @pwrdm_func_to_pwrst: Convert the pd functional power state to * the internal state - * @pwrdm_pwrst_to_func: Convert the pd internal power state to + * @pwrdm_func_to_logic_pwrst: Convert the pd functional power state + * to the internal logic state + * @pwrdm_pwrst_to_func: Convert the pd internal and logic power state to * the functional state * @pwrdm_set_next_pwrst: Set the target power state for a pd * @pwrdm_read_next_pwrst: Read the target power state set for a pd @@ -163,7 +174,9 @@ struct powerdomain { */ struct pwrdm_ops { int (*pwrdm_func_to_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst); - int (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst); + int (*pwrdm_func_to_logic_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst); + int (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst, + u8 logic); int (*pwrdm_set_next_pwrst)(struct powerdomain *pwrdm, u8 pwrst); int (*pwrdm_read_next_pwrst)(struct powerdomain *pwrdm); int (*pwrdm_read_pwrst)(struct powerdomain *pwrdm); @@ -209,7 +222,8 @@ int pwrdm_read_func_pwrst(struct powerdomain *pwrdm); int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm); int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst); -int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst); +int omap2_pwrdm_func_to_logic_pwrst(struct powerdomain *pwrdm, u8 func_pwrst); +int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst, u8 logic); int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst); int pwrdm_read_next_pwrst(struct powerdomain *pwrdm); diff --git a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c index 79c1293..6a3ea3d 100644 --- a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c +++ b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c @@ -212,6 +212,7 @@ static int omap3_pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm) struct pwrdm_ops omap2_pwrdm_operations = { .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_func_to_logic_pwrst = omap2_pwrdm_func_to_logic_pwrst, .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, @@ -226,6 +227,7 @@ struct pwrdm_ops omap2_pwrdm_operations = { struct pwrdm_ops omap3_pwrdm_operations = { .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_func_to_logic_pwrst = omap2_pwrdm_func_to_logic_pwrst, .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, diff --git a/arch/arm/mach-omap2/powerdomain44xx.c b/arch/arm/mach-omap2/powerdomain44xx.c index 538b528..030d10c 100644 --- a/arch/arm/mach-omap2/powerdomain44xx.c +++ b/arch/arm/mach-omap2/powerdomain44xx.c @@ -210,6 +210,7 @@ static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm) struct pwrdm_ops omap4_pwrdm_operations = { .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_func_to_logic_pwrst = omap2_pwrdm_func_to_logic_pwrst, .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap4_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap4_pwrdm_read_next_pwrst, -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/6] ARM: OMAP2+: PM: use the functional power states API for logic and memory 2012-04-11 20:46 [RFC/PATCH v2 0/6] ARM: OMAP2+: PM: introduce the power domains functional states jean.pihet ` (3 preceding siblings ...) 2012-04-11 20:46 ` [PATCH 4/6] ARM: OMAP2+: PM: introduce power domains logic and memory functional states jean.pihet @ 2012-04-11 20:46 ` jean.pihet 2012-04-11 20:46 ` [PATCH 6/6] ARM: OMAP2+: PM: use power domain functional state in stats counters jean.pihet 5 siblings, 0 replies; 21+ messages in thread From: jean.pihet @ 2012-04-11 20:46 UTC (permalink / raw) To: ldc_pm_all, linux-omap, paul, b-cousson, khilman; +Cc: Jean Pihet From: Jean Pihet <j-pihet@ti.com> Since the power domains logic state is derived from the power domain functional state and is now programmed from omap_set_pwrdm_state, the calls to pwrdm_set_logic_retst are removed and the mpu_logic_state field is removed from the private data of the cpuidle and suspend code. Signed-off-by: Jean Pihet <j-pihet@ti.com> --- arch/arm/mach-omap2/cpuidle44xx.c | 9 +-------- arch/arm/mach-omap2/omap-mpuss-lowpower.c | 6 +----- arch/arm/mach-omap2/pm24xx.c | 4 +--- arch/arm/mach-omap2/pm44xx.c | 7 +------ 4 files changed, 4 insertions(+), 22 deletions(-) diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c index 1daa0c1..9d99696 100644 --- a/arch/arm/mach-omap2/cpuidle44xx.c +++ b/arch/arm/mach-omap2/cpuidle44xx.c @@ -27,7 +27,6 @@ /* Machine specific information to be recorded in the C-state driver_data */ struct omap4_idle_statedata { u32 cpu_state; - u32 mpu_logic_state; u32 mpu_state; u8 valid; }; @@ -91,16 +90,13 @@ static int omap4_enter_idle(struct cpuidle_device *dev, if (cx->cpu_state == PWRDM_FUNC_PWRST_OFF) cpu_pm_enter(); - pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state); omap_set_pwrdm_state(mpu_pd, cx->mpu_state); /* * Call idle CPU cluster PM enter notifier chain * to save GIC and wakeupgen context. */ - if (((cx->mpu_state == PWRDM_FUNC_PWRST_CSWR) || - (cx->mpu_state == PWRDM_FUNC_PWRST_OSWR)) && - (cx->mpu_logic_state == PWRDM_POWER_OFF)) + if (cx->mpu_state == PWRDM_FUNC_PWRST_OSWR) cpu_cluster_pm_enter(); omap4_enter_lowpower(dev->cpu, cx->cpu_state); @@ -195,21 +191,18 @@ int __init omap4_idle_init(void) cx->valid = 1; /* C1 is always valid */ cx->cpu_state = PWRDM_FUNC_PWRST_ON; cx->mpu_state = PWRDM_FUNC_PWRST_ON; - cx->mpu_logic_state = PWRDM_POWER_RET; /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ _fill_cstate(drv, 1, "MPUSS CSWR"); cx = _fill_cstate_usage(dev, 1); cx->cpu_state = PWRDM_FUNC_PWRST_OFF; cx->mpu_state = PWRDM_FUNC_PWRST_CSWR; - cx->mpu_logic_state = PWRDM_POWER_RET; /* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ _fill_cstate(drv, 2, "MPUSS OSWR"); cx = _fill_cstate_usage(dev, 2); cx->cpu_state = PWRDM_FUNC_PWRST_OFF; cx->mpu_state = PWRDM_FUNC_PWRST_OSWR; - cx->mpu_logic_state = PWRDM_POWER_OFF; drv->state_count = OMAP4_NUM_STATES; cpuidle_register_driver(&omap4_idle_driver); diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c index dd94a0e..c79cc0f 100644 --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c @@ -232,7 +232,6 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state) { unsigned int save_state = 0; unsigned int wakeup_cpu; - int mpuss_state; if (omap_rev() == OMAP4430_REV_ES1_0) return -ENXIO; @@ -265,10 +264,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state) * In MPUSS OSWR or device OFF, interrupt controller context is lost. */ mpuss_clear_prev_logic_pwrst(); - mpuss_state = pwrdm_read_next_func_pwrst(mpuss_pd); - if (((mpuss_state == PWRDM_FUNC_PWRST_CSWR) || - (mpuss_state == PWRDM_FUNC_PWRST_OSWR)) && - (pwrdm_read_logic_retst(mpuss_pd) == PWRDM_POWER_OFF)) + if (pwrdm_read_next_func_pwrst(mpuss_pd) == PWRDM_FUNC_PWRST_OSWR) save_state = 2; cpu_clear_prev_logic_pwrst(cpu); diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c index bb84348..fbadc37 100644 --- a/arch/arm/mach-omap2/pm24xx.c +++ b/arch/arm/mach-omap2/pm24xx.c @@ -93,7 +93,6 @@ static int omap2_enter_full_retention(void) * Set MPU powerdomain's next power state to RETENTION; * preserve logic state during retention */ - pwrdm_set_logic_retst(mpu_pwrdm, PWRDM_POWER_RET); omap_set_pwrdm_state(mpu_pwrdm, PWRDM_FUNC_PWRST_CSWR); /* Workaround to kill USB */ @@ -251,7 +250,7 @@ static void __init prcm_setup_regs(void) */ num_mem_banks = pwrdm_get_mem_bank_count(core_pwrdm); for (i = 0; i < num_mem_banks; i++) - pwrdm_set_mem_retst(core_pwrdm, i, PWRDM_POWER_RET); + pwrdm_set_mem_retst(core_pwrdm, i, PWRDM_LOGIC_MEM_PWRST_RET); /* Set CORE powerdomain's next power state to RETENTION */ omap_set_pwrdm_state(core_pwrdm, PWRDM_FUNC_PWRST_CSWR); @@ -260,7 +259,6 @@ static void __init prcm_setup_regs(void) * Set MPU powerdomain's next power state to RETENTION; * preserve logic state during retention */ - pwrdm_set_logic_retst(mpu_pwrdm, PWRDM_POWER_RET); omap_set_pwrdm_state(mpu_pwrdm, PWRDM_FUNC_PWRST_CSWR); /* Force-power down DSP, GFX powerdomains */ diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c index 897fdee..6c3caf3 100644 --- a/arch/arm/mach-omap2/pm44xx.c +++ b/arch/arm/mach-omap2/pm44xx.c @@ -28,7 +28,6 @@ struct power_state { u32 next_state; #ifdef CONFIG_SUSPEND u32 saved_state; - u32 saved_logic_state; #endif struct list_head node; }; @@ -45,14 +44,11 @@ static int omap4_pm_suspend(void) /* Save current powerdomain state */ list_for_each_entry(pwrst, &pwrst_list, node) { pwrst->saved_state = pwrdm_read_next_func_pwrst(pwrst->pwrdm); - pwrst->saved_logic_state = pwrdm_read_logic_retst(pwrst->pwrdm); } /* Set targeted power domain states by suspend */ - list_for_each_entry(pwrst, &pwrst_list, node) { + list_for_each_entry(pwrst, &pwrst_list, node) omap_set_pwrdm_state(pwrst->pwrdm, pwrst->next_state); - pwrdm_set_logic_retst(pwrst->pwrdm, PWRDM_POWER_OFF); - } /* * For MPUSS to hit power domain retention(CSWR or OSWR), @@ -75,7 +71,6 @@ static int omap4_pm_suspend(void) ret = -1; } omap_set_pwrdm_state(pwrst->pwrdm, pwrst->saved_state); - pwrdm_set_logic_retst(pwrst->pwrdm, pwrst->saved_logic_state); } if (ret) pr_crit("Could not enter target state in pm_suspend\n"); -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 6/6] ARM: OMAP2+: PM: use power domain functional state in stats counters 2012-04-11 20:46 [RFC/PATCH v2 0/6] ARM: OMAP2+: PM: introduce the power domains functional states jean.pihet ` (4 preceding siblings ...) 2012-04-11 20:46 ` [PATCH 5/6] ARM: OMAP2+: PM: use the functional power states API for logic and memory jean.pihet @ 2012-04-11 20:46 ` jean.pihet 5 siblings, 0 replies; 21+ messages in thread From: jean.pihet @ 2012-04-11 20:46 UTC (permalink / raw) To: ldc_pm_all, linux-omap, paul, b-cousson, khilman; +Cc: Jean Pihet From: Jean Pihet <j-pihet@ti.com> The PM code uses some counters to keep track of the power domains transitions, in order to provide the information to drivers (in pwrdm_get_context_loss_count) and to expose the information to sysfs for debug purpose. This patch provides the information for each functional state. Signed-off-by: Jean Pihet <j-pihet@ti.com> --- arch/arm/mach-omap2/pm-debug.c | 15 ++++++++------- arch/arm/mach-omap2/powerdomain.c | 12 ++++++------ arch/arm/mach-omap2/powerdomain.h | 4 ++-- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c index 814bcd9..13c00fb 100644 --- a/arch/arm/mach-omap2/pm-debug.c +++ b/arch/arm/mach-omap2/pm-debug.c @@ -53,9 +53,10 @@ enum { DEBUG_FILE_TIMERS, }; -static const char pwrdm_state_names[][PWRDM_MAX_PWRSTS] = { +static const char pwrdm_state_names[][PWRDM_MAX_FUNC_PWRSTS] = { "OFF", - "RET", + "OSWR", + "CSWR", "INA", "ON" }; @@ -102,13 +103,13 @@ static int pwrdm_dbg_show_counter(struct powerdomain *pwrdm, void *user) strncmp(pwrdm->name, "dpll", 4) == 0) return 0; - if (pwrdm->state != pwrdm_read_pwrst(pwrdm)) + if (pwrdm->state != pwrdm_read_func_pwrst(pwrdm)) printk(KERN_ERR "pwrdm state mismatch(%s) %d != %d\n", - pwrdm->name, pwrdm->state, pwrdm_read_pwrst(pwrdm)); + pwrdm->name, pwrdm->state, pwrdm_read_func_pwrst(pwrdm)); seq_printf(s, "%s (%s)", pwrdm->name, pwrdm_state_names[pwrdm->state]); - for (i = 0; i < PWRDM_MAX_PWRSTS; i++) + for (i = 0; i < PWRDM_MAX_FUNC_PWRSTS; i++) seq_printf(s, ",%s:%d", pwrdm_state_names[i], pwrdm->state_counter[i]); @@ -137,7 +138,7 @@ static int pwrdm_dbg_show_timer(struct powerdomain *pwrdm, void *user) seq_printf(s, "%s (%s)", pwrdm->name, pwrdm_state_names[pwrdm->state]); - for (i = 0; i < 4; i++) + for (i = 0; i < PWRDM_MAX_FUNC_PWRSTS; i++) seq_printf(s, ",%s:%lld", pwrdm_state_names[i], pwrdm->state_timer[i]); @@ -211,7 +212,7 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *dir) t = sched_clock(); - for (i = 0; i < 4; i++) + for (i = 0; i < PWRDM_MAX_FUNC_PWRSTS; i++) pwrdm->state_timer[i] = 0; pwrdm->timer = t; diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 3faac62..3f78f16 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -106,7 +106,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm) list_add(&pwrdm->node, &pwrdm_list); /* Initialize the powerdomain's state counter */ - for (i = 0; i < PWRDM_MAX_PWRSTS; i++) + for (i = 0; i < PWRDM_MAX_FUNC_PWRSTS; i++) pwrdm->state_counter[i] = 0; pwrdm->ret_logic_off_counter = 0; @@ -114,7 +114,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm) pwrdm->ret_mem_off_counter[i] = 0; pwrdm_wait_transition(pwrdm); - pwrdm->state = pwrdm_read_pwrst(pwrdm); + pwrdm->state = pwrdm_read_func_pwrst(pwrdm); pwrdm->state_counter[pwrdm->state] = 1; pr_debug("powerdomain: registered %s\n", pwrdm->name); @@ -149,17 +149,17 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag) if (pwrdm == NULL) return -EINVAL; - state = pwrdm_read_pwrst(pwrdm); + state = pwrdm_read_func_pwrst(pwrdm); switch (flag) { case PWRDM_STATE_NOW: prev = pwrdm->state; break; case PWRDM_STATE_PREV: - prev = pwrdm_read_prev_pwrst(pwrdm); + prev = pwrdm_read_prev_func_pwrst(pwrdm); if (pwrdm->state != prev) pwrdm->state_counter[prev]++; - if (prev == PWRDM_POWER_RET) + if (prev == PWRDM_FUNC_PWRST_OSWR) _update_logic_membank_counters(pwrdm); /* * If the power domain did not hit the desired state, @@ -1240,7 +1240,7 @@ int pwrdm_get_context_loss_count(struct powerdomain *pwrdm) return -ENODEV; } - count = pwrdm->state_counter[PWRDM_POWER_OFF]; + count = pwrdm->state_counter[PWRDM_FUNC_PWRST_OFF]; count += pwrdm->ret_logic_off_counter; for (i = 0; i < pwrdm->banks; i++) diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h index 4e5ce15..9249d42 100644 --- a/arch/arm/mach-omap2/powerdomain.h +++ b/arch/arm/mach-omap2/powerdomain.h @@ -135,13 +135,13 @@ struct powerdomain { struct list_head voltdm_node; struct mutex lock; int state; - unsigned state_counter[PWRDM_MAX_PWRSTS]; + unsigned state_counter[PWRDM_MAX_FUNC_PWRSTS]; unsigned ret_logic_off_counter; unsigned ret_mem_off_counter[PWRDM_MAX_MEM_BANKS]; #ifdef CONFIG_PM_DEBUG s64 timer; - s64 state_timer[PWRDM_MAX_PWRSTS]; + s64 state_timer[PWRDM_MAX_FUNC_PWRSTS]; #endif }; -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 0/6] ARM: OMAP2+: PM: introduce the power domains functional states
@ 2012-04-18 12:39 jean.pihet
2012-04-18 12:39 ` jean.pihet at newoldbits.com
0 siblings, 1 reply; 21+ messages in thread
From: jean.pihet @ 2012-04-18 12:39 UTC (permalink / raw)
To: linux-omap, paul, b-cousson, khilman, linux-arm-kernel; +Cc: Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
Implement the functional states for the power domains:
- protect the power domain state change by a mutex in
omap_set_pwrdm_state,
- introduce the functional states for power domains power states and
logic power states, and the conversion functions between the
functional and internal states,
- unify the API to use the functional states,
- program the logic power state of power domains from the functional
states, in omap_set_pwrdm_state
- convert the OMAP2/3/4 PM code to use the updated API,
- provide the power domains statistics by functional states.
Tested on OMAP3 Beagleboard, with suspend and cpuidle in RET and
OFF modes.
History:
v3:
- fix a bug in OMAP3 cpuidle which prevented the IO wake-ups in PER
v2:
- add the logic power states,
- provide the power domains statistics by functional states
v1:
- initial implementation, in RFC state
Jean Pihet (6):
ARM: OMAP2+: PM: protect the power domain state change by a mutex
ARM: OMAP2+: PM: introduce power domains functional states
ARM: OMAP2+: PM: use the functional power states API
ARM: OMAP2+: PM: introduce power domains logic and memory functional
states
ARM: OMAP2+: PM: use the functional power states API for logic and
memory
ARM: OMAP2+: PM: use power domain functional state in stats counters
arch/arm/mach-omap2/cpuidle34xx.c | 61 ++++----
arch/arm/mach-omap2/cpuidle44xx.c | 28 ++--
arch/arm/mach-omap2/omap-hotplug.c | 2 +-
arch/arm/mach-omap2/omap-mpuss-lowpower.c | 39 +++---
arch/arm/mach-omap2/pm-debug.c | 15 +-
arch/arm/mach-omap2/pm.c | 62 -------
arch/arm/mach-omap2/pm24xx.c | 14 +-
arch/arm/mach-omap2/pm34xx.c | 80 +++++-----
arch/arm/mach-omap2/pm44xx.c | 15 +--
arch/arm/mach-omap2/powerdomain-common.c | 85 ++++++++++
arch/arm/mach-omap2/powerdomain.c | 244 ++++++++++++++++++++++++++--
arch/arm/mach-omap2/powerdomain.h | 42 +++++-
arch/arm/mach-omap2/powerdomain2xxx_3xxx.c | 7 +
arch/arm/mach-omap2/powerdomain44xx.c | 3 +
14 files changed, 489 insertions(+), 208 deletions(-)
--
1.7.7.6
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states 2012-04-18 12:39 [PATCH v3 0/6] ARM: OMAP2+: PM: introduce the power domains functional states jean.pihet @ 2012-04-18 12:39 ` jean.pihet at newoldbits.com 0 siblings, 0 replies; 21+ messages in thread From: jean.pihet @ 2012-04-18 12:39 UTC (permalink / raw) To: linux-omap, paul, b-cousson, khilman, linux-arm-kernel; +Cc: Jean Pihet From: Jean Pihet <j-pihet@ti.com> 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 <j-pihet@ti.com> --- 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 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; +} + 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 + * @pwrdm: struct powerdomain * to query + * @func_pwrst: functional power state + * + * Convert the functional power state to the platform specific power + * domain state value. + * Returns the internal power domain state value or -EINVAL in case + * of invalid parameters passed in. + */ +int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) +{ + int ret = func_pwrst; + + if ((!pwrdm)|| (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS)) + return -EINVAL; + + pr_debug("powerdomain: convert %0x func to pwrst for %s\n", + func_pwrst, pwrdm->name); + + if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_pwrst) { + ret = arch_pwrdm->pwrdm_func_to_pwrst(pwrdm, func_pwrst); + } + + return ret; +} + +/** + * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped + * to the internal state + * @pwrdm: struct powerdomain * to query + * @pwrst: internal power state + * + * Convert the internal power state to the power domain functional value. + * Returns the functional power domain state value or -EINVAL in case + * of invalid parameters passed in. + */ +int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) +{ + int ret = pwrst; + + if ((!pwrdm)|| (pwrst >= PWRDM_MAX_PWRSTS)) + return -EINVAL; + + pr_debug("powerdomain: convert %0x pwrst to func for %s\n", + pwrst, pwrdm->name); + + if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func) { + ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst); + } + + return ret; +} + +/* Types of sleep_switch used in omap_set_pwrdm_state */ +#define FORCEWAKEUP_SWITCH 0 +#define LOWPOWERSTATE_SWITCH 1 + +/** + * omap_set_pwrdm_state - program next powerdomain power state + * @pwrdm: struct powerdomain * to set + * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros + * + * This programs the pwrdm next functional state, sets the dependencies + * and waits for the state to be applied. + */ +int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) +{ + u8 curr_pwrst, next_pwrst; + int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst); + int sleep_switch = -1, ret = 0, hwsup = 0; + + if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0)) { + pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n", + __func__, pwrdm, func_pwrst); + return -EINVAL; + } + + pr_debug("%s: set func_pwrst %0x to pwrdm %s\n", + __func__, func_pwrst, pwrdm->name); + + 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; +} + +/** * pwrdm_set_next_pwrst - set next powerdomain power state * @pwrdm: struct powerdomain * to set * @pwrst: one of the PWRDM_POWER_* macros @@ -522,6 +652,21 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm) } /** + * pwrdm_read_next_func_pwrst - get next powerdomain functional power state + * @pwrdm: struct powerdomain * to get power state + * + * Return the powerdomain @pwrdm's next functional power state. + * Returns -EINVAL if the powerdomain pointer is null or returns + * the next power state upon success. + */ +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm) +{ + int ret = pwrdm_read_next_pwrst(pwrdm); + + return pwrdm_pwrst_to_func(pwrdm, ret); +} + +/** * pwrdm_read_pwrst - get current powerdomain power state * @pwrdm: struct powerdomain * to get power state * @@ -543,6 +688,21 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm) } /** + * pwrdm_read_func_pwrst - get current powerdomain functional power state + * @pwrdm: struct powerdomain * to get power state + * + * Return the powerdomain @pwrdm's current functional power state. + * Returns -EINVAL if the powerdomain pointer is null or returns + * the current power state upon success. + */ +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm) +{ + int ret = pwrdm_read_pwrst(pwrdm); + + return pwrdm_pwrst_to_func(pwrdm, ret); +} + +/** * pwrdm_read_prev_pwrst - get previous powerdomain power state * @pwrdm: struct powerdomain * to get previous power state * @@ -564,6 +724,21 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm) } /** + * pwrdm_read_prev_func_pwrst - get previous powerdomain functional power state + * @pwrdm: struct powerdomain * to get previous power state + * + * Return the powerdomain @pwrdm's previous functional power state. + * Returns -EINVAL if the powerdomain pointer is null or returns the + * previous power state upon success. + */ +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm) +{ + int ret = pwrdm_read_prev_pwrst(pwrdm); + + return pwrdm_pwrst_to_func(pwrdm, ret); +} + +/** * pwrdm_set_logic_retst - set powerdomain logic power state upon retention * @pwrdm: struct powerdomain * to set * @pwrst: one of the PWRDM_POWER_* macros diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h index 6c6567d..460af0e 100644 --- a/arch/arm/mach-omap2/powerdomain.h +++ b/arch/arm/mach-omap2/powerdomain.h @@ -45,6 +45,14 @@ #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) #define PWRSTS_OFF_RET_ON (PWRSTS_OFF_RET | PWRSTS_ON) +/* Powerdomain functional power states, used by the external API functions */ +#define PWRDM_FUNC_PWRST_OFF 0x0 +#define PWRDM_FUNC_PWRST_OSWR 0x1 +#define PWRDM_FUNC_PWRST_CSWR 0x2 +#define PWRDM_FUNC_PWRST_INACTIVE 0x3 +#define PWRDM_FUNC_PWRST_ON 0x4 + +#define PWRDM_MAX_FUNC_PWRSTS 5 /* Powerdomain flags */ #define PWRDM_HAS_HDWR_SAR (1 << 0) /* hardware save-and-restore support */ @@ -130,6 +138,10 @@ struct powerdomain { /** * struct pwrdm_ops - Arch specific function implementations + * @pwrdm_func_to_pwrst: Convert the pd functional power state to + * the internal state + * @pwrdm_pwrst_to_func: Convert the pd internal power state to + * the functional state * @pwrdm_set_next_pwrst: Set the target power state for a pd * @pwrdm_read_next_pwrst: Read the target power state set for a pd * @pwrdm_read_pwrst: Read the current power state of a pd @@ -150,6 +162,8 @@ struct powerdomain { * @pwrdm_wait_transition: Wait for a pd state transition to complete */ struct pwrdm_ops { + int (*pwrdm_func_to_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst); + int (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst); int (*pwrdm_set_next_pwrst)(struct powerdomain *pwrdm, u8 pwrst); int (*pwrdm_read_next_pwrst)(struct powerdomain *pwrdm); int (*pwrdm_read_pwrst)(struct powerdomain *pwrdm); @@ -190,6 +204,13 @@ struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm); int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm); +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm); +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm); +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm); + +int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst); +int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst); + int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst); int pwrdm_read_next_pwrst(struct powerdomain *pwrdm); int pwrdm_read_pwrst(struct powerdomain *pwrdm); diff --git a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c index 0f0a9f1..79c1293 100644 --- a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c +++ b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c @@ -26,6 +26,7 @@ /* Common functions across OMAP2 and OMAP3 */ + static int omap2_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst) { omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK, @@ -210,6 +211,8 @@ static int omap3_pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm) } struct pwrdm_ops omap2_pwrdm_operations = { + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, @@ -222,6 +225,8 @@ struct pwrdm_ops omap2_pwrdm_operations = { }; struct pwrdm_ops omap3_pwrdm_operations = { + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, diff --git a/arch/arm/mach-omap2/powerdomain44xx.c b/arch/arm/mach-omap2/powerdomain44xx.c index 601325b..538b528 100644 --- a/arch/arm/mach-omap2/powerdomain44xx.c +++ b/arch/arm/mach-omap2/powerdomain44xx.c @@ -209,6 +209,8 @@ static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm) } struct pwrdm_ops omap4_pwrdm_operations = { + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap4_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap4_pwrdm_read_next_pwrst, .pwrdm_read_pwrst = omap4_pwrdm_read_pwrst, -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states @ 2012-04-18 12:39 ` jean.pihet at newoldbits.com 0 siblings, 0 replies; 21+ messages in thread From: jean.pihet at newoldbits.com @ 2012-04-18 12:39 UTC (permalink / raw) To: linux-arm-kernel From: Jean Pihet <j-pihet@ti.com> 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 <j-pihet@ti.com> --- 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 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; +} + 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 + * @pwrdm: struct powerdomain * to query + * @func_pwrst: functional power state + * + * Convert the functional power state to the platform specific power + * domain state value. + * Returns the internal power domain state value or -EINVAL in case + * of invalid parameters passed in. + */ +int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) +{ + int ret = func_pwrst; + + if ((!pwrdm)|| (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS)) + return -EINVAL; + + pr_debug("powerdomain: convert %0x func to pwrst for %s\n", + func_pwrst, pwrdm->name); + + if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_pwrst) { + ret = arch_pwrdm->pwrdm_func_to_pwrst(pwrdm, func_pwrst); + } + + return ret; +} + +/** + * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped + * to the internal state + * @pwrdm: struct powerdomain * to query + * @pwrst: internal power state + * + * Convert the internal power state to the power domain functional value. + * Returns the functional power domain state value or -EINVAL in case + * of invalid parameters passed in. + */ +int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) +{ + int ret = pwrst; + + if ((!pwrdm)|| (pwrst >= PWRDM_MAX_PWRSTS)) + return -EINVAL; + + pr_debug("powerdomain: convert %0x pwrst to func for %s\n", + pwrst, pwrdm->name); + + if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func) { + ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst); + } + + return ret; +} + +/* Types of sleep_switch used in omap_set_pwrdm_state */ +#define FORCEWAKEUP_SWITCH 0 +#define LOWPOWERSTATE_SWITCH 1 + +/** + * omap_set_pwrdm_state - program next powerdomain power state + * @pwrdm: struct powerdomain * to set + * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros + * + * This programs the pwrdm next functional state, sets the dependencies + * and waits for the state to be applied. + */ +int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) +{ + u8 curr_pwrst, next_pwrst; + int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst); + int sleep_switch = -1, ret = 0, hwsup = 0; + + if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0)) { + pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n", + __func__, pwrdm, func_pwrst); + return -EINVAL; + } + + pr_debug("%s: set func_pwrst %0x to pwrdm %s\n", + __func__, func_pwrst, pwrdm->name); + + 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; +} + +/** * pwrdm_set_next_pwrst - set next powerdomain power state * @pwrdm: struct powerdomain * to set * @pwrst: one of the PWRDM_POWER_* macros @@ -522,6 +652,21 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm) } /** + * pwrdm_read_next_func_pwrst - get next powerdomain functional power state + * @pwrdm: struct powerdomain * to get power state + * + * Return the powerdomain @pwrdm's next functional power state. + * Returns -EINVAL if the powerdomain pointer is null or returns + * the next power state upon success. + */ +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm) +{ + int ret = pwrdm_read_next_pwrst(pwrdm); + + return pwrdm_pwrst_to_func(pwrdm, ret); +} + +/** * pwrdm_read_pwrst - get current powerdomain power state * @pwrdm: struct powerdomain * to get power state * @@ -543,6 +688,21 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm) } /** + * pwrdm_read_func_pwrst - get current powerdomain functional power state + * @pwrdm: struct powerdomain * to get power state + * + * Return the powerdomain @pwrdm's current functional power state. + * Returns -EINVAL if the powerdomain pointer is null or returns + * the current power state upon success. + */ +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm) +{ + int ret = pwrdm_read_pwrst(pwrdm); + + return pwrdm_pwrst_to_func(pwrdm, ret); +} + +/** * pwrdm_read_prev_pwrst - get previous powerdomain power state * @pwrdm: struct powerdomain * to get previous power state * @@ -564,6 +724,21 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm) } /** + * pwrdm_read_prev_func_pwrst - get previous powerdomain functional power state + * @pwrdm: struct powerdomain * to get previous power state + * + * Return the powerdomain @pwrdm's previous functional power state. + * Returns -EINVAL if the powerdomain pointer is null or returns the + * previous power state upon success. + */ +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm) +{ + int ret = pwrdm_read_prev_pwrst(pwrdm); + + return pwrdm_pwrst_to_func(pwrdm, ret); +} + +/** * pwrdm_set_logic_retst - set powerdomain logic power state upon retention * @pwrdm: struct powerdomain * to set * @pwrst: one of the PWRDM_POWER_* macros diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h index 6c6567d..460af0e 100644 --- a/arch/arm/mach-omap2/powerdomain.h +++ b/arch/arm/mach-omap2/powerdomain.h @@ -45,6 +45,14 @@ #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) #define PWRSTS_OFF_RET_ON (PWRSTS_OFF_RET | PWRSTS_ON) +/* Powerdomain functional power states, used by the external API functions */ +#define PWRDM_FUNC_PWRST_OFF 0x0 +#define PWRDM_FUNC_PWRST_OSWR 0x1 +#define PWRDM_FUNC_PWRST_CSWR 0x2 +#define PWRDM_FUNC_PWRST_INACTIVE 0x3 +#define PWRDM_FUNC_PWRST_ON 0x4 + +#define PWRDM_MAX_FUNC_PWRSTS 5 /* Powerdomain flags */ #define PWRDM_HAS_HDWR_SAR (1 << 0) /* hardware save-and-restore support */ @@ -130,6 +138,10 @@ struct powerdomain { /** * struct pwrdm_ops - Arch specific function implementations + * @pwrdm_func_to_pwrst: Convert the pd functional power state to + * the internal state + * @pwrdm_pwrst_to_func: Convert the pd internal power state to + * the functional state * @pwrdm_set_next_pwrst: Set the target power state for a pd * @pwrdm_read_next_pwrst: Read the target power state set for a pd * @pwrdm_read_pwrst: Read the current power state of a pd @@ -150,6 +162,8 @@ struct powerdomain { * @pwrdm_wait_transition: Wait for a pd state transition to complete */ struct pwrdm_ops { + int (*pwrdm_func_to_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst); + int (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst); int (*pwrdm_set_next_pwrst)(struct powerdomain *pwrdm, u8 pwrst); int (*pwrdm_read_next_pwrst)(struct powerdomain *pwrdm); int (*pwrdm_read_pwrst)(struct powerdomain *pwrdm); @@ -190,6 +204,13 @@ struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm); int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm); +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm); +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm); +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm); + +int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst); +int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst); + int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst); int pwrdm_read_next_pwrst(struct powerdomain *pwrdm); int pwrdm_read_pwrst(struct powerdomain *pwrdm); diff --git a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c index 0f0a9f1..79c1293 100644 --- a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c +++ b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c @@ -26,6 +26,7 @@ /* Common functions across OMAP2 and OMAP3 */ + static int omap2_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst) { omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK, @@ -210,6 +211,8 @@ static int omap3_pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm) } struct pwrdm_ops omap2_pwrdm_operations = { + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, @@ -222,6 +225,8 @@ struct pwrdm_ops omap2_pwrdm_operations = { }; struct pwrdm_ops omap3_pwrdm_operations = { + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, diff --git a/arch/arm/mach-omap2/powerdomain44xx.c b/arch/arm/mach-omap2/powerdomain44xx.c index 601325b..538b528 100644 --- a/arch/arm/mach-omap2/powerdomain44xx.c +++ b/arch/arm/mach-omap2/powerdomain44xx.c @@ -209,6 +209,8 @@ static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm) } struct pwrdm_ops omap4_pwrdm_operations = { + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, .pwrdm_set_next_pwrst = omap4_pwrdm_set_next_pwrst, .pwrdm_read_next_pwrst = omap4_pwrdm_read_next_pwrst, .pwrdm_read_pwrst = omap4_pwrdm_read_pwrst, -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states 2012-04-18 12:39 ` jean.pihet at newoldbits.com @ 2012-05-07 9:28 ` Paul Walmsley -1 siblings, 0 replies; 21+ messages in thread From: Paul Walmsley @ 2012-05-07 9:28 UTC (permalink / raw) To: jean.pihet; +Cc: linux-omap, b-cousson, khilman, linux-arm-kernel, Jean Pihet Hi On Wed, 18 Apr 2012, jean.pihet@newoldbits.com wrote: > From: Jean Pihet <j-pihet@ti.com> > > 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 <j-pihet@ti.com> 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_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. Also, what about the logic and memory bank power states? Shouldn't this function pass those back as well? > + > +/* > + * Internal (i.e. registers) to functional (i.e. logical) values > + * for the power domains states > + */ Please use kerneldoc-style function documentation. > +int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) Probably best to use "func_pwrst" or "fpwrst" rather than simply "func" here. > +{ > + 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. > + > 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. > + * @pwrdm: struct powerdomain * to query > + * @func_pwrst: functional power state > + * > + * Convert the functional power state to the platform specific power > + * domain state value. > + * Returns the internal power domain state value or -EINVAL in case > + * of invalid parameters passed in. > + */ > +int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) > +{ > + int ret = func_pwrst; > + > + if ((!pwrdm)|| (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS)) > + return -EINVAL; > + > + pr_debug("powerdomain: convert %0x func to pwrst for %s\n", > + func_pwrst, pwrdm->name); > + > + if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_pwrst) { > + ret = arch_pwrdm->pwrdm_func_to_pwrst(pwrdm, func_pwrst); > + } > + > + return ret; > +} > + > +/** > + * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped > + * to the internal state > + * @pwrdm: struct powerdomain * to query > + * @pwrst: internal power state > + * > + * Convert the internal power state to the power domain functional value. > + * Returns the functional power domain state value or -EINVAL in case > + * of invalid parameters passed in. > + */ > +int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) > +{ > + int ret = pwrst; > + > + if ((!pwrdm)|| (pwrst >= PWRDM_MAX_PWRSTS)) > + return -EINVAL; > + > + pr_debug("powerdomain: convert %0x pwrst to func for %s\n", > + pwrst, pwrdm->name); > + > + if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func) { > + ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst); > + } > + > + return ret; > +} > + > +/* Types of sleep_switch used in omap_set_pwrdm_state */ > +#define FORCEWAKEUP_SWITCH 0 > +#define LOWPOWERSTATE_SWITCH 1 > + > +/** > + * omap_set_pwrdm_state - program next powerdomain power state > + * @pwrdm: struct powerdomain * to set > + * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros > + * > + * This programs the pwrdm next functional state, sets the dependencies > + * and waits for the state to be applied. > + */ > +int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) This should presumably get renamed to match the other function names in this file, to be something like pwrdm_set_func_pwrst() or something similar. > +{ > + u8 curr_pwrst, next_pwrst; > + int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst); > + int sleep_switch = -1, ret = 0, hwsup = 0; > + > + if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0)) { > + pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n", > + __func__, pwrdm, func_pwrst); > + return -EINVAL; > + } > + > + pr_debug("%s: set func_pwrst %0x to pwrdm %s\n", > + __func__, func_pwrst, pwrdm->name); > + > + 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; > +} > + > +/** > * pwrdm_set_next_pwrst - set next powerdomain power state > * @pwrdm: struct powerdomain * to set > * @pwrst: one of the PWRDM_POWER_* macros > @@ -522,6 +652,21 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm) > } > > /** > + * pwrdm_read_next_func_pwrst - get next powerdomain functional power state > + * @pwrdm: struct powerdomain * to get power state > + * > + * Return the powerdomain @pwrdm's next functional power state. > + * Returns -EINVAL if the powerdomain pointer is null or returns > + * the next power state upon success. > + */ > +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm) > +{ > + int ret = pwrdm_read_next_pwrst(pwrdm); > + > + return pwrdm_pwrst_to_func(pwrdm, ret); > +} > + > +/** > * pwrdm_read_pwrst - get current powerdomain power state > * @pwrdm: struct powerdomain * to get power state > * > @@ -543,6 +688,21 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm) > } > > /** > + * pwrdm_read_func_pwrst - get current powerdomain functional power state > + * @pwrdm: struct powerdomain * to get power state > + * > + * Return the powerdomain @pwrdm's current functional power state. > + * Returns -EINVAL if the powerdomain pointer is null or returns > + * the current power state upon success. > + */ > +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm) > +{ > + int ret = pwrdm_read_pwrst(pwrdm); > + > + return pwrdm_pwrst_to_func(pwrdm, ret); > +} > + > +/** > * pwrdm_read_prev_pwrst - get previous powerdomain power state > * @pwrdm: struct powerdomain * to get previous power state > * > @@ -564,6 +724,21 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm) > } > > /** > + * pwrdm_read_prev_func_pwrst - get previous powerdomain functional power state > + * @pwrdm: struct powerdomain * to get previous power state > + * > + * Return the powerdomain @pwrdm's previous functional power state. > + * Returns -EINVAL if the powerdomain pointer is null or returns the > + * previous power state upon success. > + */ > +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm) > +{ > + int ret = pwrdm_read_prev_pwrst(pwrdm); > + > + return pwrdm_pwrst_to_func(pwrdm, ret); > +} > + > +/** > * pwrdm_set_logic_retst - set powerdomain logic power state upon retention > * @pwrdm: struct powerdomain * to set > * @pwrst: one of the PWRDM_POWER_* macros > diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h > index 6c6567d..460af0e 100644 > --- a/arch/arm/mach-omap2/powerdomain.h > +++ b/arch/arm/mach-omap2/powerdomain.h > @@ -45,6 +45,14 @@ > #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) > #define PWRSTS_OFF_RET_ON (PWRSTS_OFF_RET | PWRSTS_ON) > > +/* Powerdomain functional power states, used by the external API functions */ > +#define PWRDM_FUNC_PWRST_OFF 0x0 > +#define PWRDM_FUNC_PWRST_OSWR 0x1 > +#define PWRDM_FUNC_PWRST_CSWR 0x2 > +#define PWRDM_FUNC_PWRST_INACTIVE 0x3 > +#define PWRDM_FUNC_PWRST_ON 0x4 > + > +#define PWRDM_MAX_FUNC_PWRSTS 5 > > /* Powerdomain flags */ > #define PWRDM_HAS_HDWR_SAR (1 << 0) /* hardware save-and-restore support */ > @@ -130,6 +138,10 @@ struct powerdomain { > > /** > * struct pwrdm_ops - Arch specific function implementations > + * @pwrdm_func_to_pwrst: Convert the pd functional power state to > + * the internal state > + * @pwrdm_pwrst_to_func: Convert the pd internal power state to > + * the functional state > * @pwrdm_set_next_pwrst: Set the target power state for a pd > * @pwrdm_read_next_pwrst: Read the target power state set for a pd > * @pwrdm_read_pwrst: Read the current power state of a pd > @@ -150,6 +162,8 @@ struct powerdomain { > * @pwrdm_wait_transition: Wait for a pd state transition to complete > */ > struct pwrdm_ops { > + int (*pwrdm_func_to_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst); > + int (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst); > int (*pwrdm_set_next_pwrst)(struct powerdomain *pwrdm, u8 pwrst); > int (*pwrdm_read_next_pwrst)(struct powerdomain *pwrdm); > int (*pwrdm_read_pwrst)(struct powerdomain *pwrdm); > @@ -190,6 +204,13 @@ struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm); > > int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm); > > +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm); > +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm); > +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm); > + > +int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst); > +int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst); > + > int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst); > int pwrdm_read_next_pwrst(struct powerdomain *pwrdm); > int pwrdm_read_pwrst(struct powerdomain *pwrdm); > diff --git a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c > index 0f0a9f1..79c1293 100644 > --- a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c > +++ b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c > @@ -26,6 +26,7 @@ > > > /* Common functions across OMAP2 and OMAP3 */ > + > static int omap2_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst) > { > omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK, > @@ -210,6 +211,8 @@ static int omap3_pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm) > } > > struct pwrdm_ops omap2_pwrdm_operations = { > + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, > + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, > .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, > .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, > .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, > @@ -222,6 +225,8 @@ struct pwrdm_ops omap2_pwrdm_operations = { > }; > > struct pwrdm_ops omap3_pwrdm_operations = { > + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, > + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, > .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, > .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, > .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, > diff --git a/arch/arm/mach-omap2/powerdomain44xx.c b/arch/arm/mach-omap2/powerdomain44xx.c > index 601325b..538b528 100644 > --- a/arch/arm/mach-omap2/powerdomain44xx.c > +++ b/arch/arm/mach-omap2/powerdomain44xx.c > @@ -209,6 +209,8 @@ static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm) > } > > struct pwrdm_ops omap4_pwrdm_operations = { > + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, > + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, > .pwrdm_set_next_pwrst = omap4_pwrdm_set_next_pwrst, > .pwrdm_read_next_pwrst = omap4_pwrdm_read_next_pwrst, > .pwrdm_read_pwrst = omap4_pwrdm_read_pwrst, > -- > 1.7.7.6 > - Paul ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states @ 2012-05-07 9:28 ` Paul Walmsley 0 siblings, 0 replies; 21+ messages in thread From: Paul Walmsley @ 2012-05-07 9:28 UTC (permalink / raw) To: linux-arm-kernel Hi On Wed, 18 Apr 2012, jean.pihet at newoldbits.com wrote: > From: Jean Pihet <j-pihet@ti.com> > > 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 <j-pihet@ti.com> 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_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. Also, what about the logic and memory bank power states? Shouldn't this function pass those back as well? > + > +/* > + * Internal (i.e. registers) to functional (i.e. logical) values > + * for the power domains states > + */ Please use kerneldoc-style function documentation. > +int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) Probably best to use "func_pwrst" or "fpwrst" rather than simply "func" here. > +{ > + 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. > + > 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. > + * @pwrdm: struct powerdomain * to query > + * @func_pwrst: functional power state > + * > + * Convert the functional power state to the platform specific power > + * domain state value. > + * Returns the internal power domain state value or -EINVAL in case > + * of invalid parameters passed in. > + */ > +int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) > +{ > + int ret = func_pwrst; > + > + if ((!pwrdm)|| (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS)) > + return -EINVAL; > + > + pr_debug("powerdomain: convert %0x func to pwrst for %s\n", > + func_pwrst, pwrdm->name); > + > + if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_pwrst) { > + ret = arch_pwrdm->pwrdm_func_to_pwrst(pwrdm, func_pwrst); > + } > + > + return ret; > +} > + > +/** > + * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped > + * to the internal state > + * @pwrdm: struct powerdomain * to query > + * @pwrst: internal power state > + * > + * Convert the internal power state to the power domain functional value. > + * Returns the functional power domain state value or -EINVAL in case > + * of invalid parameters passed in. > + */ > +int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) > +{ > + int ret = pwrst; > + > + if ((!pwrdm)|| (pwrst >= PWRDM_MAX_PWRSTS)) > + return -EINVAL; > + > + pr_debug("powerdomain: convert %0x pwrst to func for %s\n", > + pwrst, pwrdm->name); > + > + if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func) { > + ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst); > + } > + > + return ret; > +} > + > +/* Types of sleep_switch used in omap_set_pwrdm_state */ > +#define FORCEWAKEUP_SWITCH 0 > +#define LOWPOWERSTATE_SWITCH 1 > + > +/** > + * omap_set_pwrdm_state - program next powerdomain power state > + * @pwrdm: struct powerdomain * to set > + * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros > + * > + * This programs the pwrdm next functional state, sets the dependencies > + * and waits for the state to be applied. > + */ > +int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) This should presumably get renamed to match the other function names in this file, to be something like pwrdm_set_func_pwrst() or something similar. > +{ > + u8 curr_pwrst, next_pwrst; > + int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst); > + int sleep_switch = -1, ret = 0, hwsup = 0; > + > + if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0)) { > + pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n", > + __func__, pwrdm, func_pwrst); > + return -EINVAL; > + } > + > + pr_debug("%s: set func_pwrst %0x to pwrdm %s\n", > + __func__, func_pwrst, pwrdm->name); > + > + 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; > +} > + > +/** > * pwrdm_set_next_pwrst - set next powerdomain power state > * @pwrdm: struct powerdomain * to set > * @pwrst: one of the PWRDM_POWER_* macros > @@ -522,6 +652,21 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm) > } > > /** > + * pwrdm_read_next_func_pwrst - get next powerdomain functional power state > + * @pwrdm: struct powerdomain * to get power state > + * > + * Return the powerdomain @pwrdm's next functional power state. > + * Returns -EINVAL if the powerdomain pointer is null or returns > + * the next power state upon success. > + */ > +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm) > +{ > + int ret = pwrdm_read_next_pwrst(pwrdm); > + > + return pwrdm_pwrst_to_func(pwrdm, ret); > +} > + > +/** > * pwrdm_read_pwrst - get current powerdomain power state > * @pwrdm: struct powerdomain * to get power state > * > @@ -543,6 +688,21 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm) > } > > /** > + * pwrdm_read_func_pwrst - get current powerdomain functional power state > + * @pwrdm: struct powerdomain * to get power state > + * > + * Return the powerdomain @pwrdm's current functional power state. > + * Returns -EINVAL if the powerdomain pointer is null or returns > + * the current power state upon success. > + */ > +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm) > +{ > + int ret = pwrdm_read_pwrst(pwrdm); > + > + return pwrdm_pwrst_to_func(pwrdm, ret); > +} > + > +/** > * pwrdm_read_prev_pwrst - get previous powerdomain power state > * @pwrdm: struct powerdomain * to get previous power state > * > @@ -564,6 +724,21 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm) > } > > /** > + * pwrdm_read_prev_func_pwrst - get previous powerdomain functional power state > + * @pwrdm: struct powerdomain * to get previous power state > + * > + * Return the powerdomain @pwrdm's previous functional power state. > + * Returns -EINVAL if the powerdomain pointer is null or returns the > + * previous power state upon success. > + */ > +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm) > +{ > + int ret = pwrdm_read_prev_pwrst(pwrdm); > + > + return pwrdm_pwrst_to_func(pwrdm, ret); > +} > + > +/** > * pwrdm_set_logic_retst - set powerdomain logic power state upon retention > * @pwrdm: struct powerdomain * to set > * @pwrst: one of the PWRDM_POWER_* macros > diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h > index 6c6567d..460af0e 100644 > --- a/arch/arm/mach-omap2/powerdomain.h > +++ b/arch/arm/mach-omap2/powerdomain.h > @@ -45,6 +45,14 @@ > #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) > #define PWRSTS_OFF_RET_ON (PWRSTS_OFF_RET | PWRSTS_ON) > > +/* Powerdomain functional power states, used by the external API functions */ > +#define PWRDM_FUNC_PWRST_OFF 0x0 > +#define PWRDM_FUNC_PWRST_OSWR 0x1 > +#define PWRDM_FUNC_PWRST_CSWR 0x2 > +#define PWRDM_FUNC_PWRST_INACTIVE 0x3 > +#define PWRDM_FUNC_PWRST_ON 0x4 > + > +#define PWRDM_MAX_FUNC_PWRSTS 5 > > /* Powerdomain flags */ > #define PWRDM_HAS_HDWR_SAR (1 << 0) /* hardware save-and-restore support */ > @@ -130,6 +138,10 @@ struct powerdomain { > > /** > * struct pwrdm_ops - Arch specific function implementations > + * @pwrdm_func_to_pwrst: Convert the pd functional power state to > + * the internal state > + * @pwrdm_pwrst_to_func: Convert the pd internal power state to > + * the functional state > * @pwrdm_set_next_pwrst: Set the target power state for a pd > * @pwrdm_read_next_pwrst: Read the target power state set for a pd > * @pwrdm_read_pwrst: Read the current power state of a pd > @@ -150,6 +162,8 @@ struct powerdomain { > * @pwrdm_wait_transition: Wait for a pd state transition to complete > */ > struct pwrdm_ops { > + int (*pwrdm_func_to_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst); > + int (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst); > int (*pwrdm_set_next_pwrst)(struct powerdomain *pwrdm, u8 pwrst); > int (*pwrdm_read_next_pwrst)(struct powerdomain *pwrdm); > int (*pwrdm_read_pwrst)(struct powerdomain *pwrdm); > @@ -190,6 +204,13 @@ struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm); > > int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm); > > +int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm); > +int pwrdm_read_func_pwrst(struct powerdomain *pwrdm); > +int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm); > + > +int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst); > +int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst); > + > int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst); > int pwrdm_read_next_pwrst(struct powerdomain *pwrdm); > int pwrdm_read_pwrst(struct powerdomain *pwrdm); > diff --git a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c > index 0f0a9f1..79c1293 100644 > --- a/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c > +++ b/arch/arm/mach-omap2/powerdomain2xxx_3xxx.c > @@ -26,6 +26,7 @@ > > > /* Common functions across OMAP2 and OMAP3 */ > + > static int omap2_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst) > { > omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK, > @@ -210,6 +211,8 @@ static int omap3_pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm) > } > > struct pwrdm_ops omap2_pwrdm_operations = { > + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, > + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, > .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, > .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, > .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, > @@ -222,6 +225,8 @@ struct pwrdm_ops omap2_pwrdm_operations = { > }; > > struct pwrdm_ops omap3_pwrdm_operations = { > + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, > + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, > .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst, > .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst, > .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst, > diff --git a/arch/arm/mach-omap2/powerdomain44xx.c b/arch/arm/mach-omap2/powerdomain44xx.c > index 601325b..538b528 100644 > --- a/arch/arm/mach-omap2/powerdomain44xx.c > +++ b/arch/arm/mach-omap2/powerdomain44xx.c > @@ -209,6 +209,8 @@ static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm) > } > > struct pwrdm_ops omap4_pwrdm_operations = { > + .pwrdm_func_to_pwrst = omap2_pwrdm_func_to_pwrst, > + .pwrdm_pwrst_to_func = omap2_pwrdm_pwrst_to_func, > .pwrdm_set_next_pwrst = omap4_pwrdm_set_next_pwrst, > .pwrdm_read_next_pwrst = omap4_pwrdm_read_next_pwrst, > .pwrdm_read_pwrst = omap4_pwrdm_read_pwrst, > -- > 1.7.7.6 > - Paul ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states 2012-05-07 9:28 ` Paul Walmsley @ 2012-05-08 8:40 ` Jean Pihet -1 siblings, 0 replies; 21+ messages in thread From: Jean Pihet @ 2012-05-08 8:40 UTC (permalink / raw) To: Paul Walmsley Cc: linux-omap, b-cousson, khilman, linux-arm-kernel, Jean Pihet Paul, On Mon, May 7, 2012 at 11:28 AM, Paul Walmsley <paul@pwsan.com> wrote: > Hi > > On Wed, 18 Apr 2012, jean.pihet@newoldbits.com wrote: > >> From: Jean Pihet <j-pihet@ti.com> >> >> 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 <j-pihet@ti.com> > > 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 powerdomain * to query >> + * @func_pwrst: functional power state >> + * >> + * Convert the functional power state to the platform specific power >> + * domain state value. >> + * Returns the internal power domain state value or -EINVAL in case >> + * of invalid parameters passed in. >> + */ >> +int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) >> +{ >> + int ret = func_pwrst; >> + >> + if ((!pwrdm)|| (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS)) >> + return -EINVAL; >> + >> + pr_debug("powerdomain: convert %0x func to pwrst for %s\n", >> + func_pwrst, pwrdm->name); >> + >> + if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_pwrst) { >> + ret = arch_pwrdm->pwrdm_func_to_pwrst(pwrdm, func_pwrst); >> + } >> + >> + return ret; >> +} >> + >> +/** >> + * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped >> + * to the internal state >> + * @pwrdm: struct powerdomain * to query >> + * @pwrst: internal power state >> + * >> + * Convert the internal power state to the power domain functional value. >> + * Returns the functional power domain state value or -EINVAL in case >> + * of invalid parameters passed in. >> + */ >> +int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) >> +{ >> + int ret = pwrst; >> + >> + if ((!pwrdm)|| (pwrst >= PWRDM_MAX_PWRSTS)) >> + return -EINVAL; >> + >> + pr_debug("powerdomain: convert %0x pwrst to func for %s\n", >> + pwrst, pwrdm->name); >> + >> + if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func) { >> + ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst); >> + } >> + >> + return ret; >> +} >> + >> +/* Types of sleep_switch used in omap_set_pwrdm_state */ >> +#define FORCEWAKEUP_SWITCH 0 >> +#define LOWPOWERSTATE_SWITCH 1 >> + >> +/** >> + * omap_set_pwrdm_state - program next powerdomain power state >> + * @pwrdm: struct powerdomain * to set >> + * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros >> + * >> + * This programs the pwrdm next functional state, sets the dependencies >> + * and waits for the state to be applied. >> + */ >> +int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) > > This should presumably get renamed to match the other function names in > this file, to be something like pwrdm_set_func_pwrst() or something > similar. Ok that makes sense. ... > > > - Paul Thanks, Jean -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states @ 2012-05-08 8:40 ` Jean Pihet 0 siblings, 0 replies; 21+ messages in thread From: Jean Pihet @ 2012-05-08 8:40 UTC (permalink / raw) To: linux-arm-kernel Paul, On Mon, May 7, 2012 at 11:28 AM, Paul Walmsley <paul@pwsan.com> wrote: > Hi > > On Wed, 18 Apr 2012, jean.pihet at newoldbits.com wrote: > >> From: Jean Pihet <j-pihet@ti.com> >> >> 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 <j-pihet@ti.com> > > 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 powerdomain * to query >> + * @func_pwrst: functional power state >> + * >> + * Convert the functional power state to the platform specific power >> + * domain state value. >> + * Returns the internal power domain state value or -EINVAL in case >> + * of invalid parameters passed in. >> + */ >> +int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst) >> +{ >> + ? ? int ret = func_pwrst; >> + >> + ? ? if ((!pwrdm)|| (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS)) >> + ? ? ? ? ? ? return -EINVAL; >> + >> + ? ? pr_debug("powerdomain: convert %0x func to pwrst for %s\n", >> + ? ? ? ? ? ? ?func_pwrst, pwrdm->name); >> + >> + ? ? if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_pwrst) { >> + ? ? ? ? ? ? ret = arch_pwrdm->pwrdm_func_to_pwrst(pwrdm, func_pwrst); >> + ? ? } >> + >> + ? ? return ret; >> +} >> + >> +/** >> + * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped >> + * to the internal state >> + * @pwrdm: struct powerdomain * to query >> + * @pwrst: internal power state >> + * >> + * Convert the internal power state to the power domain functional value. >> + * Returns the functional power domain state value or -EINVAL in case >> + * of invalid parameters passed in. >> + */ >> +int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst) >> +{ >> + ? ? int ret = pwrst; >> + >> + ? ? if ((!pwrdm)|| (pwrst >= PWRDM_MAX_PWRSTS)) >> + ? ? ? ? ? ? return -EINVAL; >> + >> + ? ? pr_debug("powerdomain: convert %0x pwrst to func for %s\n", >> + ? ? ? ? ? ? ?pwrst, pwrdm->name); >> + >> + ? ? if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func) { >> + ? ? ? ? ? ? ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst); >> + ? ? } >> + >> + ? ? return ret; >> +} >> + >> +/* Types of sleep_switch used in omap_set_pwrdm_state */ >> +#define FORCEWAKEUP_SWITCH ? 0 >> +#define LOWPOWERSTATE_SWITCH 1 >> + >> +/** >> + * omap_set_pwrdm_state - program next powerdomain power state >> + * @pwrdm: struct powerdomain * to set >> + * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros >> + * >> + * This programs the pwrdm next functional state, sets the dependencies >> + * and waits for the state to be applied. >> + */ >> +int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) > > This should presumably get renamed to match the other function names in > this file, to be something like pwrdm_set_func_pwrst() or something > similar. Ok that makes sense. ... > > > - Paul Thanks, Jean ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states 2012-05-08 8:40 ` Jean Pihet @ 2012-05-17 10:04 ` Santosh Shilimkar -1 siblings, 0 replies; 21+ messages in thread From: Santosh Shilimkar @ 2012-05-17 10:04 UTC (permalink / raw) To: Jean Pihet Cc: Paul Walmsley, linux-omap, b-cousson, khilman, linux-arm-kernel, Jean Pihet Jean, On Tuesday 08 May 2012 02:10 PM, Jean Pihet wrote: > Paul, > > On Mon, May 7, 2012 at 11:28 AM, Paul Walmsley <paul@pwsan.com> wrote: >> Hi >> >> On Wed, 18 Apr 2012, jean.pihet@newoldbits.com wrote: >> >>> From: Jean Pihet <j-pihet@ti.com> >>> >>> 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 <j-pihet@ti.com> >> >> 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states @ 2012-05-17 10:04 ` Santosh Shilimkar 0 siblings, 0 replies; 21+ messages in thread From: Santosh Shilimkar @ 2012-05-17 10:04 UTC (permalink / raw) To: linux-arm-kernel Jean, On Tuesday 08 May 2012 02:10 PM, Jean Pihet wrote: > Paul, > > On Mon, May 7, 2012 at 11:28 AM, Paul Walmsley <paul@pwsan.com> wrote: >> Hi >> >> On Wed, 18 Apr 2012, jean.pihet at newoldbits.com wrote: >> >>> From: Jean Pihet <j-pihet@ti.com> >>> >>> 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 <j-pihet@ti.com> >> >> 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states 2012-05-17 10:04 ` Santosh Shilimkar @ 2012-05-21 13:53 ` Jean Pihet -1 siblings, 0 replies; 21+ messages in thread From: Jean Pihet @ 2012-05-21 13:53 UTC (permalink / raw) To: Santosh Shilimkar Cc: Paul Walmsley, linux-omap, b-cousson, khilman, linux-arm-kernel, Jean Pihet Hi Santosh, On Thu, May 17, 2012 at 12:04 PM, Santosh Shilimkar <santosh.shilimkar@ti.com> 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 <paul@pwsan.com> wrote: >>> Hi >>> >>> On Wed, 18 Apr 2012, jean.pihet@newoldbits.com wrote: >>> >>>> From: Jean Pihet <j-pihet@ti.com> >>>> >>>> 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 <j-pihet@ti.com> >>> >>> 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 pmxxxx.c and cpuidlexxxx.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. > being structure is always expandable and an API taking structure as > a parameter need not change. That is the goal. This patch series uses the date from the platform specific power domain structs and abstracts those in the API. So in summary I would say that the power domain code callers do benefit from the API simplification and the abstraction of thr power domains parameters: - cpuidle: the logic state field is removed from the structs; the power domain state checks are simplified, - per-device PM QoS: the latency parameters are specified for every functional power state, - pmxxxx.c: the suspend/resume code is simplified; the power domain state checks are simplified. > > Regards > Santosh > What do you think? Thanks, Jean -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states @ 2012-05-21 13:53 ` Jean Pihet 0 siblings, 0 replies; 21+ messages in thread From: Jean Pihet @ 2012-05-21 13:53 UTC (permalink / raw) To: linux-arm-kernel Hi Santosh, On Thu, May 17, 2012 at 12:04 PM, Santosh Shilimkar <santosh.shilimkar@ti.com> 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 <paul@pwsan.com> wrote: >>> Hi >>> >>> On Wed, 18 Apr 2012, jean.pihet at newoldbits.com wrote: >>> >>>> From: Jean Pihet <j-pihet@ti.com> >>>> >>>> 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 <j-pihet@ti.com> >>> >>> 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 pmxxxx.c and cpuidlexxxx.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. > being structure is always expandable and an API taking structure as > a parameter need not change. That is the goal. This patch series uses the date from the platform specific power domain structs and abstracts those in the API. So in summary I would say that the power domain code callers do benefit from the API simplification and the abstraction of thr power domains parameters: - cpuidle: the logic state field is removed from the structs; the power domain state checks are simplified, - per-device PM QoS: the latency parameters are specified for every functional power state, - pmxxxx.c: the suspend/resume code is simplified; the power domain state checks are simplified. > > Regards > Santosh > What do you think? Thanks, Jean ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states 2012-05-21 13:53 ` Jean Pihet @ 2012-05-21 14:25 ` Shilimkar, Santosh -1 siblings, 0 replies; 21+ messages in thread From: Shilimkar, Santosh @ 2012-05-21 14:25 UTC (permalink / raw) To: Jean Pihet Cc: Paul Walmsley, linux-omap, b-cousson, khilman, linux-arm-kernel, Jean Pihet Jean, On Mon, May 21, 2012 at 7:23 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote: > Hi Santosh, > > On Thu, May 17, 2012 at 12:04 PM, Santosh Shilimkar > <santosh.shilimkar@ti.com> 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 <paul@pwsan.com> wrote: >>>> Hi >>>> >>>> On Wed, 18 Apr 2012, jean.pihet@newoldbits.com wrote: >>>> >>>>> From: Jean Pihet <j-pihet@ti.com> >>>>> >>>>> 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 <j-pihet@ti.com> >>>> >>>> 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 pmxxxx.c and cpuidlexxxx.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 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. > That is the goal. This patch series uses the date from the platform > specific power domain structs and abstracts those in the API. > Good to know that we are on same page. > So in summary I would say that the power domain code callers do > benefit from the API simplification and the abstraction of thr power > domains parameters: I agree. I am just worried about this 1 API because we have many used programmable nobs. For example the API assumes all the power domain same and while programming the powerdomain deeper state, does clockdomain wakeup and sleep transition sequence. Such a process is not needed for domain like CPU/MPU PD's because they are powerdown them self. > - cpuidle: the logic state field is removed from the structs; the > power domain state checks are simplified, > - per-device PM QoS: the latency parameters are specified for every > functional power state, > - pmxxxx.c: the suspend/resume code is simplified; the power domain > state checks are simplified. > QOS does need functional power state especially since CSWR and OSWR has such a huge difference in terms of latency. >> >> Regards >> Santosh >> > > 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. Regards Santosh -- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states @ 2012-05-21 14:25 ` Shilimkar, Santosh 0 siblings, 0 replies; 21+ messages in thread From: Shilimkar, Santosh @ 2012-05-21 14:25 UTC (permalink / raw) To: linux-arm-kernel Jean, On Mon, May 21, 2012 at 7:23 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote: > Hi Santosh, > > On Thu, May 17, 2012 at 12:04 PM, Santosh Shilimkar > <santosh.shilimkar@ti.com> 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 <paul@pwsan.com> wrote: >>>> Hi >>>> >>>> On Wed, 18 Apr 2012, jean.pihet at newoldbits.com wrote: >>>> >>>>> From: Jean Pihet <j-pihet@ti.com> >>>>> >>>>> 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 <j-pihet@ti.com> >>>> >>>> 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 pmxxxx.c and cpuidlexxxx.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 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. > That is the goal. This patch series uses the date from the platform > specific power domain structs and abstracts those in the API. > Good to know that we are on same page. > So in summary I would say that the power domain code callers do > benefit from the API simplification and the abstraction of thr power > domains parameters: I agree. I am just worried about this 1 API because we have many used programmable nobs. For example the API assumes all the power domain same and while programming the powerdomain deeper state, does clockdomain wakeup and sleep transition sequence. Such a process is not needed for domain like CPU/MPU PD's because they are powerdown them self. > - cpuidle: the logic state field is removed from the structs; the > power domain state checks are simplified, > - per-device PM QoS: the latency parameters are specified for every > functional power state, > - pmxxxx.c: the suspend/resume code is simplified; the power domain > state checks are simplified. > QOS does need functional power state especially since CSWR and OSWR has such a huge difference in terms of latency. >> >> Regards >> Santosh >> > > 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. Regards Santosh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states 2012-05-21 14:25 ` Shilimkar, Santosh @ 2012-05-23 6:39 ` Santosh Shilimkar -1 siblings, 0 replies; 21+ messages in thread From: Santosh Shilimkar @ 2012-05-23 6:39 UTC (permalink / raw) To: Jean Pihet, Paul Walmsley Cc: linux-omap, b-cousson, khilman, linux-arm-kernel, Jean Pihet Jean, On Monday 21 May 2012 07:55 PM, Shilimkar, Santosh wrote: > Jean, > > On Mon, May 21, 2012 at 7:23 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote: >> Hi Santosh, >> >> On Thu, May 17, 2012 at 12:04 PM, Santosh Shilimkar >> <santosh.shilimkar@ti.com> 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states @ 2012-05-23 6:39 ` Santosh Shilimkar 0 siblings, 0 replies; 21+ messages in thread From: Santosh Shilimkar @ 2012-05-23 6:39 UTC (permalink / raw) To: linux-arm-kernel Jean, On Monday 21 May 2012 07:55 PM, Shilimkar, Santosh wrote: > Jean, > > On Mon, May 21, 2012 at 7:23 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote: >> Hi Santosh, >> >> On Thu, May 17, 2012 at 12:04 PM, Santosh Shilimkar >> <santosh.shilimkar@ti.com> 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2012-05-23 6:39 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-04-11 20:46 [RFC/PATCH v2 0/6] ARM: OMAP2+: PM: introduce the power domains functional states jean.pihet 2012-04-11 20:46 ` [PATCH 1/6] ARM: OMAP2+: PM: protect the power domain state change by a mutex jean.pihet 2012-04-11 20:46 ` [PATCH 2/6] ARM: OMAP2+: PM: introduce power domains functional states jean.pihet 2012-04-11 20:46 ` [PATCH 3/6] ARM: OMAP2+: PM: use the functional power states API jean.pihet 2012-04-11 20:46 ` [PATCH 4/6] ARM: OMAP2+: PM: introduce power domains logic and memory functional states jean.pihet 2012-04-11 20:46 ` [PATCH 5/6] ARM: OMAP2+: PM: use the functional power states API for logic and memory jean.pihet 2012-04-11 20:46 ` [PATCH 6/6] ARM: OMAP2+: PM: use power domain functional state in stats counters jean.pihet 2012-04-18 12:39 [PATCH v3 0/6] ARM: OMAP2+: PM: introduce the power domains functional states jean.pihet 2012-04-18 12:39 ` [PATCH 2/6] ARM: OMAP2+: PM: introduce " jean.pihet 2012-04-18 12:39 ` jean.pihet at newoldbits.com 2012-05-07 9:28 ` Paul Walmsley 2012-05-07 9:28 ` Paul Walmsley 2012-05-08 8:40 ` Jean Pihet 2012-05-08 8:40 ` Jean Pihet 2012-05-17 10:04 ` Santosh Shilimkar 2012-05-17 10:04 ` Santosh Shilimkar 2012-05-21 13:53 ` Jean Pihet 2012-05-21 13:53 ` Jean Pihet 2012-05-21 14:25 ` Shilimkar, Santosh 2012-05-21 14:25 ` Shilimkar, Santosh 2012-05-23 6:39 ` Santosh Shilimkar 2012-05-23 6:39 ` Santosh Shilimkar
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.