linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states
@ 2012-09-12  9:55 Jean Pihet
  2012-09-12  9:55 ` [PATCH 1/7] ARM: OMAP2+: PM: introduce " Jean Pihet
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Jean Pihet @ 2012-09-12  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

Here is a re-spin after some comments and suggestions after review
and discussions.

Implement the functional states for the power domains:
- unify the API to use the functional states. The new API
  consists of the pwrdm_set*_fpwrst and pwrdm_read*_fpwrst
  functions and is the API to use to control the power domains
  power and logic states,
- reorganize the powerdomain API in internal and external parts,
  in powerdomain.h [1]
- protect the power domain state change by a lock in the
  functions that read and set the powerdomains next functional state,
- introduce the functional states for power domains power states and
  logic power states [2], and the conversion functions between the
  functional and internal states. The conversion functions are
  lightweight and generic. The power domains allowed states [3] are
  defined in the pwrsts and pwrsts_logic_ret fields of the struct
  powerdomain,
- program the logic power state of power domains from the functional
  states, in pwrdm_set*_fpwrst
- convert the OMAP2/3/4 PM code to use the updated API,
- provide the power domains statistics by functional states,
- provide ftrace tracepoints with the functional state,
- provide error logs in critical code, which makes the development
  easier.

Notes:
[1] the physical split of internal and external APIs into
   different header files is not part of this series, it comes as
   a separate patch set.
[2] the abstraction for functional power states provides a generic
   way of controlling the power domains states across all OMAP
   chipsets and revisions. The users of the functional power states
   are (1) the OMAP PM components: cpuidle, suspend, pmxxxx, clock*
   and (2) the per-device PM QoS constraints and the generic power
   domain frameworks. All those users require a generic abstraction
   of the power domain states while the low level power domain code
   has the knowledge of the internal settings (power, logic... states).
[3] About the power domains allowed states:
   Power domains have varied capabilities, as defined by the value of
   the pwrsts and pwrsts_logic_ret fields of the powerdomain struct.
   When reading or setting a low power state such as OFF/RET, a specific
   requested state may not be supported on the given power domain.
   In the states conversion functions a power or logic state is first
   looked for in the lower power states in order to maximize the power
   savings, then if not found in the higher power states. An iteration
   function is used, as suggested by Rajendra Nayak <rnayak@ti.com>
   This functionality brings consistency in the functional power states
   core code and acts as a guard against hardware implementations
   discrepancies, e.g. some power domains only support the RET logic
   state although reading the logic state registers (previous, current
   and next) always returns OFF. The DSS power domain on OMAP3 is an
   example.


Based on mainline kernel 3.6.0-rc4.

Tested on OMAP3 Beagleboard, with suspend and cpuidle in RET and
OFF modes.


History:
 v6:
 - rework to a lighter and generic conversion mechanism between the
   functional and the internal states (static internal functions
   are defined instead of using the pwrdm fops function pointers),
 - the functional power states API now has a function to set the next
   power state (pwrdm_set_next_fpwrst) and a function to program (i.e.
   set, apply the state and wait for completion) the power state
   (pwrdm_set_fpwrst),
 - When attempting a low power state such as OFF/RET, a specific
   requested state may not be supported on the given power domain.
   In the states conversion functions a power state is first looked
   for in the lower power states in order to maximize the power savings,
   then if not found in the higher power states. Only allowed states are
   returned by the conversion functions, as defined by the value of the
   pwrsts and pwrsts_logic_ret fields of the powerdomain struct.
 - added more error logging: check for NULL in pwrdm, better error
   messages using pr_err_ratelimited etc.

 v5:
 - complete rework after review and suggestions,
 - improved locking on next state read/write; spinlock instead of mutex
 - added more error logging in critical code,

 v4:
 - reworked the code after internal review and testing with OMAP3&4 device
   OFF,
 - fixed the tracepoints generation code,
 - introduce a function that returns power domains achievable functional
   states, in order to return a valid state for power domains that only
   support some of the power states. Although it has been tested OK the
   code is in RFC state.

 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: introduce power domains functional states
  ARM: OMAP2+: PM: add a lock to protect the powerdomains next state
  ARM: OMAP2+: PM: use the functional power states API
  ARM: OMAP2+: PM: use power domain functional state in stats counters
  ARM: OMAP2+: PM debug: trace the functional power domains states
  ARM: OMAP2+: PM: reorganize the powerdomain API in public and private
    parts

Nishanth Menon (1):
  ARM: OMAP2+: powerdomain: add error logs

 arch/arm/mach-omap2/cpuidle34xx.c         |   58 ++--
 arch/arm/mach-omap2/cpuidle44xx.c         |   24 +-
 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/pm.h                  |    1 -
 arch/arm/mach-omap2/pm24xx.c              |   14 +-
 arch/arm/mach-omap2/pm34xx.c              |   75 ++--
 arch/arm/mach-omap2/pm44xx.c              |   24 +-
 arch/arm/mach-omap2/powerdomain.c         |  594 ++++++++++++++++++++++++++---
 arch/arm/mach-omap2/powerdomain.h         |  137 +++++---
 12 files changed, 746 insertions(+), 299 deletions(-)

-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/7] ARM: OMAP2+: PM: introduce power domains functional states
  2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
@ 2012-09-12  9:55 ` Jean Pihet
  2012-09-12  9:55 ` [PATCH 2/7] ARM: OMAP2+: PM: add a lock to protect the powerdomains next state Jean Pihet
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Jean Pihet @ 2012-09-12  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

Introduce the functional states for power domains, which include
the power states and the logic states.
This patch provides the API functions to set and read the power
domains functional state and internal functions to convert between
the functional (i.e. logical) and the internal (or registers) values.

In the new API only the functions pwrdm_set_next_fpwrst and
pwrdm_set_next_fpwrst shall be used to change a power domain
target state, along with the associated PWRDM_FUNC_* and
PWRDM_LOGIC_* macros as the state parameters.

Note about the power domains allowed states:
Power domains have varied capabilities, as defined by the value of
the pwrsts and pwrsts_logic_ret fields of the powerdomain struct.
When reading or setting a low power state such as OFF/RET, a specific
requested state may not be supported on the given power domain.
In the states conversion functions a power or logic state is first
looked for in the lower power states in order to maximize the power
savings, then if not found in the higher power states. An iteration
function is used, as suggested by Rajendra Nayak <rnayak@ti.com>

This functionality brings consistency in the functional power states
core code and acts as a guard against hardware implementations
discrepancies, e.g. some power domains only support the RET logic
state although reading the logic state registers (previous, current
and next) always returns OFF. The DSS power domain on OMAP3 is an
example.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |  374 ++++++++++++++++++++++++++++++++++++-
 arch/arm/mach-omap2/powerdomain.h |   32 +++-
 2 files changed, 396 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 69b36e1..18c21fa 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -1,7 +1,7 @@
 /*
  * OMAP powerdomain control
  *
- * Copyright (C) 2007-2008, 2011 Texas Instruments, Inc.
+ * Copyright (C) 2007-2008, 2011-2012 Texas Instruments, Inc.
  * Copyright (C) 2007-2011 Nokia Corporation
  *
  * Written by Paul Walmsley
@@ -128,14 +128,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]++;
 	}
 }
@@ -199,6 +199,201 @@ static int _pwrdm_post_transition_cb(struct powerdomain *pwrdm, void *unused)
 	return 0;
 }
 
+/**
+ * Search down then up for a valid state from a list of allowed states.
+ * Used by states conversion functions (_pwrdm_fpwrst_to_*) to look for
+ * allowed power and logic states for a powerdomain.
+ *
+ * @pwrsts: list of allowed states, defined as a bitmask
+ * @pwrst: initial state to be used as a starting point
+ * @min: minimum (i.e. lowest consumption) allowed state
+ * @max: maximum (i.e. highest consumption) allowed state
+ *
+ * Returns the matching allowed state.
+ */
+static inline int _match_pwrst(u32 pwrsts, int pwrst, int min, int max)
+{
+	int found = 1, new_pwrst = pwrst;
+
+	/*
+	 * If the power domain does not allow any state programmation
+	 * return the max state which is always allowed
+	 */
+	if (!pwrsts)
+		return max;
+
+	/*
+	 * Search lower: if the requested state is not supported
+	 * try the lower states, stopping at the minimum allowed
+	 * state
+	 */
+	while (!(pwrsts & (1 << new_pwrst))) {
+		if (new_pwrst <= min) {
+			found = 0;
+			break;
+		}
+		new_pwrst--;
+	}
+
+	/*
+	 * Search higher: if no lower state found fallback to the higher
+	 * states, stopping@the maximum allowed state
+	 */
+	if (!found) {
+		new_pwrst = pwrst;
+		while (!(pwrsts & (1 << new_pwrst))) {
+			if (new_pwrst >= max) {
+				new_pwrst = max;
+				break;
+			}
+			new_pwrst++;
+		}
+	}
+
+	return new_pwrst;
+}
+
+/**
+ * _pwrdm_fpwrst_to_pwrst - Convert functional (i.e. logical) to
+ * internal (i.e. registers) values for the power domains states.
+ * @struct powerdomain * to convert the values for
+ * @fpwrst: functional power state
+ *
+ * Returns the internal power state value for the power domain, or
+ * -EINVAL in case of invalid parameters passed in.
+ */
+static int _pwrdm_fpwrst_to_pwrst(struct powerdomain *pwrdm, u8 fpwrst)
+{
+	int ret;
+
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
+		return -EINVAL;
+	}
+
+	switch (fpwrst) {
+	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:
+		pr_err_ratelimited("%s: powerdomain %s: bad fpwrst %0x\n",
+				   __func__, pwrdm->name, fpwrst);
+		return -EINVAL;
+	}
+
+	ret = _match_pwrst(pwrdm->pwrsts, ret,
+			   PWRDM_POWER_OFF,
+			   PWRDM_POWER_ON);
+
+	pr_debug("powerdomain %s: convert fpwrst %0x to pwrst %0x\n",
+		 pwrdm->name, fpwrst, ret);
+
+	return ret;
+}
+
+/**
+ * _pwrdm_fpwrst_to_logic_pwrst - Convert functional (i.e. logical) to
+ * internal (i.e. registers) values for the power domains logic states.
+ * @struct powerdomain * to convert the values for
+ * @pwrst: functional power state
+ *
+ * Returns the internal logic state value for the power domain, or
+ * -EINVAL in case of invalid parameters passed in.
+ */
+static int _pwrdm_fpwrst_to_logic_pwrst(struct powerdomain *pwrdm, u8 fpwrst)
+{
+	int ret;
+
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
+		return -EINVAL;
+	}
+
+	switch (fpwrst) {
+	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:
+		pr_err_ratelimited("%s: powerdomain %s: bad fpwrst %0x\n",
+				   __func__, pwrdm->name, fpwrst);
+		return -EINVAL;
+	}
+
+	ret = _match_pwrst(pwrdm->pwrsts_logic_ret, ret,
+			   PWRDM_LOGIC_MEM_PWRST_OFF,
+			   PWRDM_LOGIC_MEM_PWRST_RET);
+
+	pr_debug("powerdomain %s: convert fpwrst %0x to logic %0x pwrst\n",
+		 pwrdm->name, fpwrst, ret);
+
+	return ret;
+}
+
+/**
+ * _pwrdm_pwrst_to_fpwrst - Convert internal (i.e. registers) to
+ * functional (i.e. logical) values for the power domains states.
+ * @struct powerdomain * to convert the values for
+ * @pwrst: internal power state
+ *
+ * Returns the functional power state value for the power domain, or
+ * -EINVAL in case of invalid parameters passed in.
+ */
+static int _pwrdm_pwrst_to_fpwrst(struct powerdomain *pwrdm, u8 pwrst, u8 logic)
+{
+	int ret;
+
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
+		return -EINVAL;
+	}
+
+	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:
+		logic = _match_pwrst(pwrdm->pwrsts_logic_ret, logic,
+				     PWRDM_LOGIC_MEM_PWRST_OFF,
+				     PWRDM_LOGIC_MEM_PWRST_RET);
+
+		if (logic == PWRDM_LOGIC_MEM_PWRST_OFF)
+			ret = PWRDM_FUNC_PWRST_OSWR;
+		else
+			ret = PWRDM_FUNC_PWRST_CSWR;
+		break;
+	case PWRDM_POWER_OFF:
+		ret = PWRDM_FUNC_PWRST_OFF;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	pr_debug("powerdomain: convert pwrst (%0x,%0x) to fpwrst %0x\n",
+		 pwrst, logic, ret);
+
+	return ret;
+}
+
+
 /* Public functions */
 
 /**
@@ -464,6 +659,111 @@ int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm)
 	return pwrdm->banks;
 }
 
+/* Types of sleep_switch used in pwrdm_set_fpwrst */
+#define FORCEWAKEUP_SWITCH	0
+#define LOWPOWERSTATE_SWITCH	1
+
+/**
+ * pwrdm_set_fpwrst - program next powerdomain functional power state
+ * @pwrdm: struct powerdomain * to set
+ * @fpwrst: 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 pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
+{
+	u8 curr_pwrst, next_fpwrst;
+	int pwrst = _pwrdm_fpwrst_to_pwrst(pwrdm, fpwrst);
+	int logic = _pwrdm_fpwrst_to_logic_pwrst(pwrdm, fpwrst);
+	int sleep_switch = -1, ret = 0, hwsup = 0;
+
+	if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) {
+		pr_debug("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
+			 __func__, pwrdm, fpwrst);
+		return -EINVAL;
+	}
+
+	pr_debug("%s: set fpwrst %0x to pwrdm %s\n",
+		 __func__, fpwrst, pwrdm->name);
+
+	next_fpwrst = pwrdm_read_next_fpwrst(pwrdm);
+	if (next_fpwrst == fpwrst)
+		return ret;
+
+	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;
+		}
+	}
+
+	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",
+		       __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;
+	}
+
+	return ret;
+}
+
+/**
+ * pwrdm_set_next_fpwrst - set next powerdomain functional power state
+ * @pwrdm: struct powerdomain * to set
+ * @fpwrst: power domain functional state, one of the PWRDM_FUNC_* macros
+ *
+ * This programs the pwrdm next functional state
+ */
+int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
+			  enum pwrdm_func_state fpwrst)
+{
+	int pwrst = _pwrdm_fpwrst_to_pwrst(pwrdm, fpwrst);
+	int logic = _pwrdm_fpwrst_to_logic_pwrst(pwrdm, fpwrst);
+	int ret = 0;
+
+	if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) {
+		pr_debug("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
+			 __func__, pwrdm, fpwrst);
+		return -EINVAL;
+	}
+
+	pr_debug("%s: set fpwrst %0x to pwrdm %s\n",
+		 __func__, fpwrst, pwrdm->name);
+
+	ret = pwrdm_set_logic_retst(pwrdm, logic);
+	if (ret)
+		pr_err("%s: unable to set logic state %0x of powerdomain: %s\n",
+		       __func__, logic, pwrdm->name);
+
+	ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
+	if (ret)
+		pr_err("%s: unable to set power state %0x of powerdomain: %s\n",
+		       __func__, pwrst, pwrdm->name);
+
+	return ret;
+};
+
 /**
  * pwrdm_set_next_pwrst - set next powerdomain power state
  * @pwrdm: struct powerdomain * to set
@@ -482,6 +782,13 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	if (!pwrdm)
 		return -EINVAL;
 
+	/*
+	 * Do nothing in case the power domain does not have any
+	 * valid state, e.g. DPLL domains.
+	 */
+	if (!pwrdm->pwrsts)
+		return 0;
+
 	if (!(pwrdm->pwrsts & (1 << pwrst)))
 		return -EINVAL;
 
@@ -521,6 +828,22 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 }
 
 /**
+ * pwrdm_read_next_fpwrst - 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_fpwrst(struct powerdomain *pwrdm)
+{
+	int next_pwrst = pwrdm_read_next_pwrst(pwrdm);
+	int next_logic = pwrdm_read_logic_retst(pwrdm);
+
+	return _pwrdm_pwrst_to_fpwrst(pwrdm, next_pwrst, next_logic);
+}
+
+/**
  * pwrdm_read_pwrst - get current powerdomain power state
  * @pwrdm: struct powerdomain * to get power state
  *
@@ -546,6 +869,22 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 }
 
 /**
+ * pwrdm_read_fpwrst - 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_fpwrst(struct powerdomain *pwrdm)
+{
+	int pwrst = pwrdm_read_pwrst(pwrdm);
+	int logic = pwrdm_read_logic_pwrst(pwrdm);
+
+	return _pwrdm_pwrst_to_fpwrst(pwrdm, pwrst, logic);
+}
+
+/**
  * pwrdm_read_prev_pwrst - get previous powerdomain power state
  * @pwrdm: struct powerdomain * to get previous power state
  *
@@ -567,9 +906,25 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
 }
 
 /**
+ * pwrdm_read_prev_fpwrst - 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_fpwrst(struct powerdomain *pwrdm)
+{
+	int prev_pwrst = pwrdm_read_prev_pwrst(pwrdm);
+	int prev_logic = pwrdm_read_prev_logic_pwrst(pwrdm);
+
+	return _pwrdm_pwrst_to_fpwrst(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.
@@ -584,6 +939,13 @@ int pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst)
 	if (!pwrdm)
 		return -EINVAL;
 
+	/*
+	 * Do nothing in case the power domain does not have any
+	 * valid state, e.g. DPLL domains.
+	 */
+	if (!pwrdm->pwrsts_logic_ret)
+		return 0;
+
 	if (!(pwrdm->pwrsts_logic_ret & (1 << pwrst)))
 		return -EINVAL;
 
@@ -600,7 +962,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
@@ -637,7 +999,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 baee906..aa5de4f 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -1,7 +1,7 @@
 /*
  * OMAP2/3/4 powerdomain control
  *
- * Copyright (C) 2007-2008, 2010 Texas Instruments, Inc.
+ * Copyright (C) 2007-2008, 2010, 2012 Texas Instruments, Inc.
  * Copyright (C) 2007-2011 Nokia Corporation
  *
  * Paul Walmsley
@@ -9,9 +9,6 @@
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
- *
- * XXX This should be moved to the mach-omap2/ directory@the earliest
- * opportunity.
  */
 
 #ifndef __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_H
@@ -26,6 +23,26 @@
 
 #include "voltage.h"
 
+/* Powerdomain functional power states, used by the external API functions */
+enum pwrdm_func_state {
+	PWRDM_FUNC_PWRST_OFF		= 0x0,
+	PWRDM_FUNC_PWRST_OSWR,
+	PWRDM_FUNC_PWRST_CSWR,
+	PWRDM_FUNC_PWRST_INACTIVE,
+	PWRDM_FUNC_PWRST_ON,
+	PWRDM_MAX_FUNC_PWRSTS		/* Last value, used as the max value */
+};
+
+/*
+ * Powerdomains logic and memory functional power states,
+ * used by the external API functions
+ */
+enum pwrdm_logic_mem_state {
+	PWRDM_LOGIC_MEM_PWRST_OFF	= 0x0,
+	PWRDM_LOGIC_MEM_PWRST_RET,
+	PWRDM_MAX_LOGIC_MEM_PWRST	/* Last value, used as the max value */
+};
+
 /* Powerdomain basic power states */
 #define PWRDM_POWER_OFF		0x0
 #define PWRDM_POWER_RET		0x1
@@ -206,6 +223,13 @@ struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm);
 
 int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm);
 
+int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst);
+int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
+			  enum pwrdm_func_state fpwrst);
+int pwrdm_read_prev_fpwrst(struct powerdomain *pwrdm);
+int pwrdm_read_fpwrst(struct powerdomain *pwrdm);
+int pwrdm_read_next_fpwrst(struct powerdomain *pwrdm);
+
 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);
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/7] ARM: OMAP2+: PM: add a lock to protect the powerdomains next state
  2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
  2012-09-12  9:55 ` [PATCH 1/7] ARM: OMAP2+: PM: introduce " Jean Pihet
@ 2012-09-12  9:55 ` Jean Pihet
  2012-09-12  9:55 ` [PATCH 3/7] ARM: OMAP2+: PM: use the functional power states API Jean Pihet
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Jean Pihet @ 2012-09-12  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

pwrdm_set_fpwrst, pwrdm_set_next_fpwrst and pwrdm_read_next_fpwrst
are intented to be the only API to program and request the next
state of a power domain.
This patch protects the power domain next state settings and structs
from concurrent accesses by the use of a lock.

A spinlock is used since the API functions can be called from
atomic context (.i.e) either from cpuidle path or suspend path.

[ambresh at ti.com: reported the atomic context issue and suggested
to replace the initial mutex with a spinlock]

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Reported-by: Ambresh K <ambresh@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |   40 ++++++++++++++++++++++++++++++++----
 arch/arm/mach-omap2/powerdomain.h |    3 ++
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 18c21fa..f4b219f 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);
 
+	spin_lock_init(&pwrdm->lock);
 	list_add(&pwrdm->node, &pwrdm_list);
 
 	/* Initialize the powerdomain's state counter */
@@ -394,6 +395,22 @@ static int _pwrdm_pwrst_to_fpwrst(struct powerdomain *pwrdm, u8 pwrst, u8 logic)
 }
 
 
+/**
+ * _pwrdm_read_next_fpwrst - 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.
+ */
+static int _pwrdm_read_next_fpwrst(struct powerdomain *pwrdm)
+{
+	int next_pwrst = pwrdm_read_next_pwrst(pwrdm);
+	int next_logic = pwrdm_read_logic_retst(pwrdm);
+
+	return _pwrdm_pwrst_to_fpwrst(pwrdm, next_pwrst, next_logic);
+}
+
 /* Public functions */
 
 /**
@@ -677,6 +694,7 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
 	int pwrst = _pwrdm_fpwrst_to_pwrst(pwrdm, fpwrst);
 	int logic = _pwrdm_fpwrst_to_logic_pwrst(pwrdm, fpwrst);
 	int sleep_switch = -1, ret = 0, hwsup = 0;
+	unsigned long flags;
 
 	if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) {
 		pr_debug("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
@@ -687,9 +705,11 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
 	pr_debug("%s: set fpwrst %0x to pwrdm %s\n",
 		 __func__, fpwrst, pwrdm->name);
 
-	next_fpwrst = pwrdm_read_next_fpwrst(pwrdm);
+	spin_lock_irqsave(&pwrdm->lock, flags);
+
+	next_fpwrst = _pwrdm_read_next_fpwrst(pwrdm);
 	if (next_fpwrst == fpwrst)
-		return ret;
+		goto out;
 
 	curr_pwrst = pwrdm_read_pwrst(pwrdm);
 	if (curr_pwrst < PWRDM_POWER_ON) {
@@ -725,6 +745,8 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
 		break;
 	}
 
+out:
+	spin_unlock_irqrestore(&pwrdm->lock, flags);
 	return ret;
 }
 
@@ -741,6 +763,7 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 	int pwrst = _pwrdm_fpwrst_to_pwrst(pwrdm, fpwrst);
 	int logic = _pwrdm_fpwrst_to_logic_pwrst(pwrdm, fpwrst);
 	int ret = 0;
+	unsigned long flags;
 
 	if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) {
 		pr_debug("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
@@ -751,6 +774,8 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 	pr_debug("%s: set fpwrst %0x to pwrdm %s\n",
 		 __func__, fpwrst, pwrdm->name);
 
+	spin_lock_irqsave(&pwrdm->lock, flags);
+
 	ret = pwrdm_set_logic_retst(pwrdm, logic);
 	if (ret)
 		pr_err("%s: unable to set logic state %0x of powerdomain: %s\n",
@@ -761,6 +786,7 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 		pr_err("%s: unable to set power state %0x of powerdomain: %s\n",
 		       __func__, pwrst, pwrdm->name);
 
+	spin_unlock_irqrestore(&pwrdm->lock, flags);
 	return ret;
 };
 
@@ -837,10 +863,14 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
  */
 int pwrdm_read_next_fpwrst(struct powerdomain *pwrdm)
 {
-	int next_pwrst = pwrdm_read_next_pwrst(pwrdm);
-	int next_logic = pwrdm_read_logic_retst(pwrdm);
+	int ret;
+	unsigned long flags;
 
-	return _pwrdm_pwrst_to_fpwrst(pwrdm, next_pwrst, next_logic);
+	spin_lock_irqsave(&pwrdm->lock, flags);
+	ret = _pwrdm_read_next_fpwrst(pwrdm);
+	spin_unlock_irqrestore(&pwrdm->lock, flags);
+
+	return ret;
 }
 
 /**
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index aa5de4f..c3dc363 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -17,6 +17,7 @@
 #include <linux/types.h>
 #include <linux/list.h>
 
+#include <linux/spinlock.h>
 #include <linux/atomic.h>
 
 #include <plat/cpu.h>
@@ -109,6 +110,7 @@ struct powerdomain;
  * @pwrdm_clkdms: Clockdomains in this powerdomain
  * @node: list_head linking all powerdomains
  * @voltdm_node: list_head linking all powerdomains in a voltagedomain
+ * @lock: powerdomain next state registers protection lock
  * @pwrstctrl_offs: (AM33XX only) XXX_PWRSTCTRL reg offset from prcm_offs
  * @pwrstst_offs: (AM33XX only) XXX_PWRSTST reg offset from prcm_offs
  * @logicretstate_mask: (AM33XX only) mask for logic retention bitfield
@@ -142,6 +144,7 @@ struct powerdomain {
 	struct clockdomain *pwrdm_clkdms[PWRDM_MAX_CLKDMS];
 	struct list_head node;
 	struct list_head voltdm_node;
+	spinlock_t lock;
 	int state;
 	unsigned state_counter[PWRDM_MAX_PWRSTS];
 	unsigned ret_logic_off_counter;
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/7] ARM: OMAP2+: PM: use the functional power states API
  2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
  2012-09-12  9:55 ` [PATCH 1/7] ARM: OMAP2+: PM: introduce " Jean Pihet
  2012-09-12  9:55 ` [PATCH 2/7] ARM: OMAP2+: PM: add a lock to protect the powerdomains next state Jean Pihet
@ 2012-09-12  9:55 ` Jean Pihet
  2012-09-12  9:55 ` [PATCH 4/7] ARM: OMAP2+: PM: use power domain functional state in stats counters Jean Pihet
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Jean Pihet @ 2012-09-12  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

Use the functional power states as the API to control power
domains:
- use the PWRDM_FUNC_PWRST_* and PWRDM_LOGIC_MEM_PWRST_*
  macros for the power states and logic settings,
- the function pwrdm_set_next_fpwrst, which controls
  the power domains next power and logic settings, shall
  be used instead of pwrdm_set_next_pwrst to program the
  power domains next states,
- the function pwrdm_set_fpwrst, which programs the power
  domains power and logic settings, shall be used instead
  of omap_set_pwrdm_state.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c         |   58 +++++++++++-----------
 arch/arm/mach-omap2/cpuidle44xx.c         |   24 ++++------
 arch/arm/mach-omap2/omap-hotplug.c        |    2 +-
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |   39 ++++++++-------
 arch/arm/mach-omap2/pm24xx.c              |   14 ++---
 arch/arm/mach-omap2/pm34xx.c              |   75 +++++++++++++++--------------
 arch/arm/mach-omap2/pm44xx.c              |   24 ++++------
 arch/arm/mach-omap2/powerdomain.c         |    2 +-
 8 files changed, 114 insertions(+), 124 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index f2a49a4..4ca37d2 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -44,32 +44,32 @@ struct omap3_idle_statedata {
 
 static struct omap3_idle_statedata omap3_idle_data[] = {
 	{
-		.mpu_state = PWRDM_POWER_ON,
-		.core_state = PWRDM_POWER_ON,
+		.mpu_state = PWRDM_FUNC_PWRST_ON,
+		.core_state = PWRDM_FUNC_PWRST_ON,
 	},
 	{
-		.mpu_state = PWRDM_POWER_ON,
-		.core_state = PWRDM_POWER_ON,
+		.mpu_state = PWRDM_FUNC_PWRST_ON,
+		.core_state = PWRDM_FUNC_PWRST_ON,
 	},
 	{
-		.mpu_state = PWRDM_POWER_RET,
-		.core_state = PWRDM_POWER_ON,
+		.mpu_state = PWRDM_FUNC_PWRST_CSWR,
+		.core_state = PWRDM_FUNC_PWRST_ON,
 	},
 	{
-		.mpu_state = PWRDM_POWER_OFF,
-		.core_state = PWRDM_POWER_ON,
+		.mpu_state = PWRDM_FUNC_PWRST_OFF,
+		.core_state = PWRDM_FUNC_PWRST_ON,
 	},
 	{
-		.mpu_state = PWRDM_POWER_RET,
-		.core_state = PWRDM_POWER_RET,
+		.mpu_state = PWRDM_FUNC_PWRST_CSWR,
+		.core_state = PWRDM_FUNC_PWRST_CSWR,
 	},
 	{
-		.mpu_state = PWRDM_POWER_OFF,
-		.core_state = PWRDM_POWER_RET,
+		.mpu_state = PWRDM_FUNC_PWRST_OFF,
+		.core_state = PWRDM_FUNC_PWRST_CSWR,
 	},
 	{
-		.mpu_state = PWRDM_POWER_OFF,
-		.core_state = PWRDM_POWER_OFF,
+		.mpu_state = PWRDM_FUNC_PWRST_OFF,
+		.core_state = PWRDM_FUNC_PWRST_OFF,
 	},
 };
 
@@ -84,8 +84,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);
+	pwrdm_set_next_fpwrst(mpu_pd, mpu_state);
+	pwrdm_set_next_fpwrst(core_pd, core_state);
 
 	if (omap_irq_pending() || need_resched())
 		goto return_sleep_time;
@@ -100,7 +100,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 */
@@ -110,7 +110,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_fpwrst(mpu_pd) == PWRDM_FUNC_PWRST_OFF)
 		cpu_pm_exit();
 
 	/* Re-allow idle for C1 */
@@ -159,20 +159,20 @@ static int next_valid_state(struct cpuidle_device *dev,
 			    struct cpuidle_driver *drv, int index)
 {
 	struct omap3_idle_statedata *cx = &omap3_idle_data[index];
-	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 idx;
 	int next_index = 0; /* C1 is the default value */
 
 	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 */
@@ -218,7 +218,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	 * Use only C1 if CAM is active.
 	 * CAM does not have wakeup capability in OMAP3.
 	 */
-	if (pwrdm_read_pwrst(cam_pd) == PWRDM_POWER_ON)
+	if (pwrdm_read_fpwrst(cam_pd) == PWRDM_FUNC_PWRST_ON)
 		new_state_idx = drv->safe_state_index;
 	else
 		new_state_idx = next_valid_state(dev, drv, index);
@@ -234,7 +234,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	/* Program PER state */
 	cx = &omap3_idle_data[new_state_idx];
 	core_next_state = cx->core_state;
-	per_next_state = per_saved_state = pwrdm_read_next_pwrst(per_pd);
+	per_next_state = per_saved_state = pwrdm_read_next_fpwrst(per_pd);
 	if (new_state_idx == 0) {
 		/* In C1 do not allow PER state lower than CORE state */
 		if (per_next_state < core_next_state)
@@ -244,20 +244,20 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 		 * Prevent PER OFF if CORE is not in RETention or OFF as this
 		 * would disable PER wakeups completely.
 		 */
-		if ((per_next_state == PWRDM_POWER_OFF) &&
-		    (core_next_state > PWRDM_POWER_RET))
-			per_next_state = PWRDM_POWER_RET;
+		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);
+		pwrdm_set_next_fpwrst(per_pd, per_next_state);
 
 	ret = omap3_enter_idle(dev, drv, new_state_idx);
 
 	/* Restore original PER state if it was modified */
 	if (per_next_state != per_saved_state)
-		pwrdm_set_next_pwrst(per_pd, per_saved_state);
+		pwrdm_set_next_fpwrst(per_pd, per_saved_state);
 
 	return ret;
 }
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index 288bee6..617ff43 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -26,25 +26,21 @@
 /* Machine specific information */
 struct omap4_idle_statedata {
 	u32 cpu_state;
-	u32 mpu_logic_state;
 	u32 mpu_state;
 };
 
 static struct omap4_idle_statedata omap4_idle_data[] = {
 	{
-		.cpu_state = PWRDM_POWER_ON,
-		.mpu_state = PWRDM_POWER_ON,
-		.mpu_logic_state = PWRDM_POWER_RET,
+		.cpu_state = PWRDM_FUNC_PWRST_ON,
+		.mpu_state = PWRDM_FUNC_PWRST_ON,
 	},
 	{
-		.cpu_state = PWRDM_POWER_OFF,
-		.mpu_state = PWRDM_POWER_RET,
-		.mpu_logic_state = PWRDM_POWER_RET,
+		.cpu_state = PWRDM_FUNC_PWRST_OFF,
+		.mpu_state = PWRDM_FUNC_PWRST_CSWR,
 	},
 	{
-		.cpu_state = PWRDM_POWER_OFF,
-		.mpu_state = PWRDM_POWER_RET,
-		.mpu_logic_state = PWRDM_POWER_OFF,
+		.cpu_state = PWRDM_FUNC_PWRST_OFF,
+		.mpu_state = PWRDM_FUNC_PWRST_OSWR,
 	},
 };
 
@@ -91,7 +87,7 @@ static int omap4_enter_idle_coupled(struct cpuidle_device *dev,
 	 * out of coherency and in OFF mode.
 	 */
 	if (dev->cpu == 0 && cpumask_test_cpu(1, cpu_online_mask)) {
-		while (pwrdm_read_pwrst(cpu_pd[1]) != PWRDM_POWER_OFF) {
+		while (pwrdm_read_fpwrst(cpu_pd[1]) != PWRDM_FUNC_PWRST_OFF) {
 			cpu_relax();
 
 			/*
@@ -116,15 +112,13 @@ static int omap4_enter_idle_coupled(struct cpuidle_device *dev,
 	cpu_pm_enter();
 
 	if (dev->cpu == 0) {
-		pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
-		omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
+		pwrdm_set_fpwrst(mpu_pd, cx->mpu_state);
 
 		/*
 		 * 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_OSWR)
 				cpu_cluster_pm_enter();
 	}
 
diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
index 414083b..be53726 100644
--- a/arch/arm/mach-omap2/omap-hotplug.c
+++ b/arch/arm/mach-omap2/omap-hotplug.c
@@ -58,7 +58,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);
 
 		if (omap_secure_apis_support())
 			boot_cpu = omap_read_auxcoreboot0();
diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
index 637a1bd..f73c0c8 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);
+	pwrdm_set_next_fpwrst(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_fpwrst(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;
@@ -236,14 +237,15 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 		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 +261,10 @@ 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))
+	if (pwrdm_read_next_fpwrst(mpuss_pd) == PWRDM_FUNC_PWRST_OSWR)
 		save_state = 2;
 
 	cpu_clear_prev_logic_pwrst(cpu);
@@ -285,7 +286,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(NULL);
 
@@ -304,7 +305,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 +320,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 +355,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);
+	pwrdm_set_next_fpwrst(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 +372,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);
+	pwrdm_set_next_fpwrst(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 2edeffc..f012bb4 100644
--- a/arch/arm/mach-omap2/pm24xx.c
+++ b/arch/arm/mach-omap2/pm24xx.c
@@ -93,8 +93,7 @@ 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);
-	pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_RET);
+	pwrdm_set_next_fpwrst(mpu_pwrdm, PWRDM_FUNC_PWRST_CSWR);
 
 	/* Workaround to kill USB */
 	l = omap_ctrl_readl(OMAP2_CONTROL_DEVCONF0) | OMAP24XX_USBSTANDBYCTRL;
@@ -248,26 +247,25 @@ 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 */
-	pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET);
+	pwrdm_set_next_fpwrst(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);
+	pwrdm_set_next_fpwrst(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);
+	pwrdm_set_next_fpwrst(pwrdm, PWRDM_FUNC_PWRST_OFF);
 	clkdm_sleep(dsp_clkdm);
 
 	pwrdm = clkdm_get_pwrdm(gfx_clkdm);
-	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_OFF);
+	pwrdm_set_next_fpwrst(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 05bd8f0..5b6505f 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -111,7 +111,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_fpwrst(mpu_pwrdm);
 
 	if (omap_type() != OMAP2_DEVICE_TYPE_GP) {
 		/*
@@ -119,10 +119,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);
+		pwrdm_set_next_fpwrst(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);
+		pwrdm_set_next_fpwrst(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);
@@ -241,21 +241,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;
 	u32 sdrc_pwr = 0;
 
-	mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm);
+	mpu_next_state = pwrdm_read_next_fpwrst(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:
@@ -265,24 +267,25 @@ 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_fpwrst(neon_pwrdm) == PWRDM_FUNC_PWRST_ON)
+		pwrdm_set_next_fpwrst(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_fpwrst(per_pwrdm);
+	core_next_state = pwrdm_read_next_fpwrst(core_pwrdm);
 
 	pwrdm_pre_transition(NULL);
 
 	/* 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();
 		}
@@ -299,7 +302,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);
 
 	/*
@@ -318,19 +321,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_fpwrst(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);
@@ -340,7 +343,7 @@ void omap_sram_idle(void)
 	pwrdm_post_transition(NULL);
 
 	/* PER */
-	if (per_next_state < PWRDM_POWER_ON)
+	if (per_next_state < PWRDM_FUNC_PWRST_ON)
 		omap2_gpio_resume_after_idle();
 }
 
@@ -371,10 +374,10 @@ 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_fpwrst(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))
+		if (pwrdm_set_fpwrst(pwrst->pwrdm, pwrst->next_state))
 			goto restore;
 		if (pwrdm_clear_all_prev_pwrst(pwrst->pwrdm))
 			goto restore;
@@ -387,14 +390,14 @@ 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_fpwrst(pwrst->pwrdm);
 		if (state > pwrst->next_state) {
 			pr_info("Powerdomain (%s) didn't enter "
 				"target state %d\n",
 			       pwrst->pwrdm->name, pwrst->next_state);
 			ret = -1;
 		}
-		omap_set_pwrdm_state(pwrst->pwrdm, pwrst->saved_state);
+		pwrdm_set_fpwrst(pwrst->pwrdm, pwrst->saved_state);
 	}
 	if (ret)
 		pr_err("Could not enter target state in pm_suspend\n");
@@ -566,21 +569,21 @@ 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 {
 			pwrst->next_state = state;
 		}
-		omap_set_pwrdm_state(pwrst->pwrdm, pwrst->next_state);
+		pwrdm_set_fpwrst(pwrst->pwrdm, pwrst->next_state);
 	}
 }
 
@@ -619,13 +622,13 @@ 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))
 		pwrdm_enable_hdwr_sar(pwrdm);
 
-	return omap_set_pwrdm_state(pwrst->pwrdm, pwrst->next_state);
+	return pwrdm_set_fpwrst(pwrst->pwrdm, pwrst->next_state);
 }
 
 /*
diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c
index ea24174..6a38ef5 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;
 };
@@ -43,16 +42,12 @@ static int omap4_pm_suspend(void)
 	u32 cpu_id = smp_processor_id();
 
 	/* Save current powerdomain state */
-	list_for_each_entry(pwrst, &pwrst_list, node) {
-		pwrst->saved_state = pwrdm_read_next_pwrst(pwrst->pwrdm);
-		pwrst->saved_logic_state = pwrdm_read_logic_retst(pwrst->pwrdm);
-	}
+	list_for_each_entry(pwrst, &pwrst_list, node)
+		pwrst->saved_state = pwrdm_read_next_fpwrst(pwrst->pwrdm);
 
 	/* Set targeted power domain states by suspend */
-	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);
-	}
+	list_for_each_entry(pwrst, &pwrst_list, node)
+		pwrdm_set_fpwrst(pwrst->pwrdm, pwrst->next_state);
 
 	/*
 	 * For MPUSS to hit power domain retention(CSWR or OSWR),
@@ -63,19 +58,18 @@ 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_fpwrst(pwrst->pwrdm);
 		if (state > pwrst->next_state) {
 			pr_info("Powerdomain (%s) didn't enter "
 			       "target state %d\n",
 			       pwrst->pwrdm->name, pwrst->next_state);
 			ret = -1;
 		}
-		omap_set_pwrdm_state(pwrst->pwrdm, pwrst->saved_state);
-		pwrdm_set_logic_retst(pwrst->pwrdm, pwrst->saved_logic_state);
+		pwrdm_set_fpwrst(pwrst->pwrdm, pwrst->saved_state);
 	}
 	if (ret)
 		pr_crit("Could not enter target state in pm_suspend\n");
@@ -113,10 +107,10 @@ 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);
+	return pwrdm_set_fpwrst(pwrst->pwrdm, pwrst->next_state);
 }
 
 /**
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index f4b219f..82ae9e9 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -481,7 +481,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);
+		pwrdm_set_next_fpwrst(temp_p, PWRDM_FUNC_PWRST_ON);
 
 	return 0;
 }
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/7] ARM: OMAP2+: PM: use power domain functional state in stats counters
  2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
                   ` (2 preceding siblings ...)
  2012-09-12  9:55 ` [PATCH 3/7] ARM: OMAP2+: PM: use the functional power states API Jean Pihet
@ 2012-09-12  9:55 ` Jean Pihet
  2012-09-12 23:35   ` Kevin Hilman
  2012-09-12  9:55 ` [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states Jean Pihet
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Jean Pihet @ 2012-09-12  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

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..8eaa3f2 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_fpwrst(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_fpwrst(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 82ae9e9..267241f 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_fpwrst(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_fpwrst(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_fpwrst(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,
@@ -1414,7 +1414,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 c3dc363..a29caec 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -146,7 +146,7 @@ struct powerdomain {
 	struct list_head voltdm_node;
 	spinlock_t 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];
 
@@ -160,7 +160,7 @@ struct powerdomain {
 
 #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] 18+ messages in thread

* [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states
  2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
                   ` (3 preceding siblings ...)
  2012-09-12  9:55 ` [PATCH 4/7] ARM: OMAP2+: PM: use power domain functional state in stats counters Jean Pihet
@ 2012-09-12  9:55 ` Jean Pihet
  2012-09-12 23:47   ` Kevin Hilman
  2012-09-12  9:55 ` [PATCH 6/7] ARM: OMAP2+: powerdomain: add error logs Jean Pihet
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Jean Pihet @ 2012-09-12  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

Trace the power domain transitions using the functional power states,
which include the power and logic states.

While at it, fix the trace in the case a power domain did not hit
the desired state, as reported by Paul Walmsley.

Reported-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 267241f..2277ad3 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -144,7 +144,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm)
 static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 {
 
-	int prev, state, trace_state = 0;
+	int prev, next, state, trace_state;
 
 	if (pwrdm == NULL)
 		return -EINVAL;
@@ -165,10 +165,10 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 		 * If the power domain did not hit the desired state,
 		 * generate a trace event with both the desired and hit states
 		 */
-		if (state != prev) {
+		next = pwrdm_read_next_fpwrst(pwrdm);
+		if (next != prev) {
 			trace_state = (PWRDM_TRACE_STATES_FLAG |
-				       ((state & OMAP_POWERSTATE_MASK) << 8) |
-				       ((prev & OMAP_POWERSTATE_MASK) << 0));
+				       (next << 8) | (prev << 0));
 			trace_power_domain_target(pwrdm->name, trace_state,
 						  smp_processor_id());
 		}
@@ -723,6 +723,10 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
 		}
 	}
 
+	/* Trace the pwrdm desired target state */
+	trace_power_domain_target(pwrdm->name, next_fpwrst,
+				  smp_processor_id());
+
 	if (logic != pwrdm_read_logic_retst(pwrdm))
 		pwrdm_set_logic_retst(pwrdm, logic);
 
@@ -776,6 +780,10 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 
 	spin_lock_irqsave(&pwrdm->lock, flags);
 
+	/* Trace the pwrdm desired target state */
+	trace_power_domain_target(pwrdm->name, fpwrst,
+				  smp_processor_id());
+
 	ret = pwrdm_set_logic_retst(pwrdm, logic);
 	if (ret)
 		pr_err("%s: unable to set logic state %0x of powerdomain: %s\n",
@@ -821,13 +829,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
 
-	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
-		/* Trace the pwrdm desired target state */
-		trace_power_domain_target(pwrdm->name, pwrst,
-					  smp_processor_id());
-		/* Program the pwrdm desired target state */
+	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
 		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
-	}
 
 	return ret;
 }
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/7] ARM: OMAP2+: powerdomain: add error logs
  2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
                   ` (4 preceding siblings ...)
  2012-09-12  9:55 ` [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states Jean Pihet
@ 2012-09-12  9:55 ` Jean Pihet
  2012-09-12  9:55 ` [PATCH 7/7] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts Jean Pihet
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Jean Pihet @ 2012-09-12  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

From: Nishanth Menon <nm@ti.com>

Silent failure makes debug hard. So, provide rate limited error
messages in functional and oft-used code to prevent spam
when something goes wrong.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |  177 +++++++++++++++++++++++++++----------
 1 files changed, 130 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 2277ad3..37dfabf 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -19,6 +19,7 @@
 #include <linux/list.h>
 #include <linux/errno.h>
 #include <linux/string.h>
+#include <linux/ratelimit.h>
 #include <trace/events/power.h>
 
 #include "cm2xxx_3xxx.h"
@@ -146,8 +147,10 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 
 	int prev, next, state, trace_state;
 
-	if (pwrdm == NULL)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	state = pwrdm_read_fpwrst(pwrdm);
 
@@ -174,6 +177,8 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 		}
 		break;
 	default:
+		pr_err_ratelimited("%s: powerdomain %s: bad flag %0x\n",
+				   __func__, pwrdm->name, flag);
 		return -EINVAL;
 	}
 
@@ -385,6 +390,8 @@ static int _pwrdm_pwrst_to_fpwrst(struct powerdomain *pwrdm, u8 pwrst, u8 logic)
 		ret = PWRDM_FUNC_PWRST_OFF;
 		break;
 	default:
+		pr_err_ratelimited("%s: powerdomain %s: bad pwrst %0x\n",
+				   __func__, pwrdm->name, pwrst);
 		ret = -EINVAL;
 	}
 
@@ -697,8 +704,8 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
 	unsigned long flags;
 
 	if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) {
-		pr_debug("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
-			 __func__, pwrdm, fpwrst);
+		pr_err_ratelimited("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
+				   __func__, pwrdm, fpwrst);
 		return -EINVAL;
 	}
 
@@ -727,13 +734,17 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
 	trace_power_domain_target(pwrdm->name, next_fpwrst,
 				  smp_processor_id());
 
-	if (logic != pwrdm_read_logic_retst(pwrdm))
-		pwrdm_set_logic_retst(pwrdm, logic);
+	if (logic != pwrdm_read_logic_retst(pwrdm)) {
+		ret = pwrdm_set_logic_retst(pwrdm, logic);
+		if (ret)
+			pr_err_ratelimited("%s: unable to set logic state %0x of powerdomain %s, ret=%d\n",
+					   __func__, logic, pwrdm->name, ret);
+	}
 
 	ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
 	if (ret)
-		pr_err("%s: unable to set power state of powerdomain: %s\n",
-		       __func__, pwrdm->name);
+		pr_err_ratelimited("%s: unable to set power state %0x of powerdomain %s, ret=%d\n",
+				   __func__, pwrst, pwrdm->name, ret);
 
 	switch (sleep_switch) {
 	case FORCEWAKEUP_SWITCH:
@@ -770,8 +781,8 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 	unsigned long flags;
 
 	if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) {
-		pr_debug("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
-			 __func__, pwrdm, fpwrst);
+		pr_err_ratelimited("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
+				   __func__, pwrdm, fpwrst);
 		return -EINVAL;
 	}
 
@@ -786,13 +797,13 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 
 	ret = pwrdm_set_logic_retst(pwrdm, logic);
 	if (ret)
-		pr_err("%s: unable to set logic state %0x of powerdomain: %s\n",
-		       __func__, logic, pwrdm->name);
+		pr_err_ratelimited("%s: unable to set logic state %0x of powerdomain %s, ret=%d\n",
+				   __func__, logic, pwrdm->name, ret);
 
 	ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
 	if (ret)
-		pr_err("%s: unable to set power state %0x of powerdomain: %s\n",
-		       __func__, pwrst, pwrdm->name);
+		pr_err_ratelimited("%s: unable to set power state %0x of powerdomain %s, ret=%d\n",
+				   __func__, pwrst, pwrdm->name, ret);
 
 	spin_unlock_irqrestore(&pwrdm->lock, flags);
 	return ret;
@@ -813,8 +824,10 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	/*
 	 * Do nothing in case the power domain does not have any
@@ -823,8 +836,11 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	if (!pwrdm->pwrsts)
 		return 0;
 
-	if (!(pwrdm->pwrsts & (1 << pwrst)))
+	if (!(pwrdm->pwrsts & (1 << pwrst))) {
+		pr_err_ratelimited("%s: powerdomain %s: unsupported pwrst %0x\n",
+				   __func__, pwrdm->name, pwrst);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
@@ -847,8 +863,10 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_next_pwrst)
 		ret = arch_pwrdm->pwrdm_read_next_pwrst(pwrdm);
@@ -889,8 +907,10 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (pwrdm->pwrsts == PWRSTS_ON)
 		return PWRDM_POWER_ON;
@@ -929,8 +949,10 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_prev_pwrst)
 		ret = arch_pwrdm->pwrdm_read_prev_pwrst(pwrdm);
@@ -969,8 +991,10 @@ int pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	/*
 	 * Do nothing in case the power domain does not have any
@@ -979,8 +1003,11 @@ int pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst)
 	if (!pwrdm->pwrsts_logic_ret)
 		return 0;
 
-	if (!(pwrdm->pwrsts_logic_ret & (1 << pwrst)))
+	if (!(pwrdm->pwrsts_logic_ret & (1 << pwrst))) {
+		pr_err_ratelimited("%s: powerdomain %s: bad pwrst %0x\n",
+				   __func__, pwrdm->name, pwrst);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: setting next logic powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
@@ -1010,14 +1037,22 @@ int pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank, u8 pwrst)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return -EEXIST;
+	}
 
-	if (!(pwrdm->pwrsts_mem_on[bank] & (1 << pwrst)))
+	if (!(pwrdm->pwrsts_mem_on[bank] & (1 << pwrst))) {
+		pr_err_ratelimited("%s: powerdomain %s: bank %d bad pwrst %0x\n",
+				   __func__, pwrdm->name, bank, pwrst);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: setting next memory powerstate for domain %s "
 		 "bank %0x while pwrdm-ON to %0x\n", pwrdm->name, bank, pwrst);
@@ -1048,14 +1083,22 @@ int pwrdm_set_mem_retst(struct powerdomain *pwrdm, u8 bank, u8 pwrst)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return -EEXIST;
+	}
 
-	if (!(pwrdm->pwrsts_mem_ret[bank] & (1 << pwrst)))
+	if (!(pwrdm->pwrsts_mem_ret[bank] & (1 << pwrst))) {
+		pr_err_ratelimited("%s: powerdomain %s: bank %d bad pwrst %0x\n",
+				   __func__, pwrdm->name, bank, pwrst);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: setting next memory powerstate for domain %s "
 		 "bank %0x while pwrdm-RET to %0x\n", pwrdm->name, bank, pwrst);
@@ -1079,8 +1122,10 @@ int pwrdm_read_logic_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_logic_pwrst)
 		ret = arch_pwrdm->pwrdm_read_logic_pwrst(pwrdm);
@@ -1100,8 +1145,10 @@ int pwrdm_read_prev_logic_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_prev_logic_pwrst)
 		ret = arch_pwrdm->pwrdm_read_prev_logic_pwrst(pwrdm);
@@ -1121,8 +1168,10 @@ int pwrdm_read_logic_retst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_logic_retst)
 		ret = arch_pwrdm->pwrdm_read_logic_retst(pwrdm);
@@ -1144,11 +1193,16 @@ int pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8 bank)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return ret;
+	}
 
 	if (pwrdm->flags & PWRDM_HAS_MPU_QUIRK)
 		bank = 1;
@@ -1174,11 +1228,16 @@ int pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm, u8 bank)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return ret;
+	}
 
 	if (pwrdm->flags & PWRDM_HAS_MPU_QUIRK)
 		bank = 1;
@@ -1203,11 +1262,16 @@ int pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return ret;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_mem_retst)
 		ret = arch_pwrdm->pwrdm_read_mem_retst(pwrdm, bank);
@@ -1228,8 +1292,10 @@ int pwrdm_clear_all_prev_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
 	/*
 	 * XXX should get the powerdomain's current state here;
@@ -1260,11 +1326,16 @@ int pwrdm_enable_hdwr_sar(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (!(pwrdm->flags & PWRDM_HAS_HDWR_SAR))
+	if (!(pwrdm->flags & PWRDM_HAS_HDWR_SAR)) {
+		pr_err_ratelimited("%s: powerdomain %s: no HDSAR in flag %0x\n",
+				   __func__, pwrdm->name, pwrdm->flags);
 		return ret;
+	}
 
 	pr_debug("powerdomain: %s: setting SAVEANDRESTORE bit\n",
 		 pwrdm->name);
@@ -1290,11 +1361,16 @@ int pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (!(pwrdm->flags & PWRDM_HAS_HDWR_SAR))
+	if (!(pwrdm->flags & PWRDM_HAS_HDWR_SAR)) {
+		pr_err_ratelimited("%s: powerdomain %s: no HDSAR in flag %0x\n",
+				   __func__, pwrdm->name, pwrdm->flags);
 		return ret;
+	}
 
 	pr_debug("powerdomain: %s: clearing SAVEANDRESTORE bit\n",
 		 pwrdm->name);
@@ -1331,11 +1407,16 @@ int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
-	if (!(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE))
+	if (!(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
+		pr_err_ratelimited("%s: powerdomain %s:no lowpwrch in flag %0x\n",
+				   __func__, pwrdm->name, pwrdm->flags);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: %s: setting LOWPOWERSTATECHANGE bit\n",
 		 pwrdm->name);
@@ -1360,8 +1441,10 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_wait_transition)
 		ret = arch_pwrdm->pwrdm_wait_transition(pwrdm);
@@ -1406,14 +1489,14 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
  *
  * Context loss count is the sum of powerdomain off-mode counter, the
  * logic off counter and the per-bank memory off counter.  Returns negative
- * (and WARNs) upon error, otherwise, returns the context loss count.
+ * and report upon error, otherwise, returns the context loss count.
  */
 int pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
 {
 	int i, count;
 
 	if (!pwrdm) {
-		WARN(1, "powerdomain: %s: pwrdm is null\n", __func__);
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -ENODEV;
 	}
 
@@ -1452,8 +1535,8 @@ bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm)
 	int i;
 
 	if (IS_ERR_OR_NULL(pwrdm)) {
-		pr_debug("powerdomain: %s: invalid powerdomain pointer\n",
-			 __func__);
+		pr_err_ratelimited("powerdomain: %s: invalid pwrdm=%p\nr",
+				   __func__, pwrdm);
 		return 1;
 	}
 
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/7] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts
  2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
                   ` (5 preceding siblings ...)
  2012-09-12  9:55 ` [PATCH 6/7] ARM: OMAP2+: powerdomain: add error logs Jean Pihet
@ 2012-09-12  9:55 ` Jean Pihet
  2012-09-13  0:11   ` Kevin Hilman
  2012-09-13  0:34 ` [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Kevin Hilman
  2012-09-26 21:28 ` Paul Walmsley
  8 siblings, 1 reply; 18+ messages in thread
From: Jean Pihet @ 2012-09-12  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

The newly added code for functional power states re-defines the
API to query and control the power domains settings.

The API is now split in the following parts in powerdomain.h:
- the public or external API, to be used by external PM components:
  cpuidle, suspend, pmxxxx, clock* etc.
- the private or internal API, to be used by the low level PM code
  only: powerdomain*, pm-debug, hwmod, voltage, clockdomainxxxx.

The function omap_set_pwrdm_state is not used anymore and so is
removed.

No functional change is introduced by this patch.

Note: the API reorganization in a public and private header files
is not part of this patch, this comes as a subsequent clean-up
patch series.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/pm.c          |   62 --------------------
 arch/arm/mach-omap2/pm.h          |    1 -
 arch/arm/mach-omap2/powerdomain.h |  112 +++++++++++++++++++++----------------
 3 files changed, 63 insertions(+), 112 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 9cb5ced..dfe702b 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,64 +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;
-
-	while (!(pwrdm->pwrsts & (1 << pwrst))) {
-		if (pwrst == PWRDM_POWER_OFF)
-			return ret;
-		pwrst--;
-	}
-
-	next_pwrst = pwrdm_read_next_pwrst(pwrdm);
-	if (next_pwrst == pwrst)
-		return ret;
-
-	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;
-	}
-
-	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/pm.h b/arch/arm/mach-omap2/pm.h
index 686137d..707e9cb 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -33,7 +33,6 @@ static inline int omap4_idle_init(void)
 extern void *omap3_secure_ram_storage;
 extern void omap3_pm_off_mode_enable(int);
 extern void omap_sram_idle(void);
-extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state);
 extern int omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused);
 extern int (*omap_pm_suspend)(void);
 
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index a29caec..dcd2315 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -24,6 +24,11 @@
 
 #include "voltage.h"
 
+/***************************************************************
+ * External API, to be used by external PM components: cpuidle,
+ * suspend, pmxxxx, clock* etc.
+ ***************************************************************/
+
 /* Powerdomain functional power states, used by the external API functions */
 enum pwrdm_func_state {
 	PWRDM_FUNC_PWRST_OFF		= 0x0,
@@ -44,6 +49,64 @@ enum pwrdm_logic_mem_state {
 	PWRDM_MAX_LOGIC_MEM_PWRST	/* Last value, used as the max value */
 };
 
+struct clockdomain;
+struct powerdomain;
+
+struct powerdomain *pwrdm_lookup(const char *name);
+
+int pwrdm_for_each(int (*fn)(struct powerdomain *pwrdm, void *user),
+			void *user);
+int pwrdm_for_each_nolock(int (*fn)(struct powerdomain *pwrdm, void *user),
+			void *user);
+
+int pwrdm_add_clkdm(struct powerdomain *pwrdm, struct clockdomain *clkdm);
+int pwrdm_del_clkdm(struct powerdomain *pwrdm, struct clockdomain *clkdm);
+int pwrdm_for_each_clkdm(struct powerdomain *pwrdm,
+			 int (*fn)(struct powerdomain *pwrdm,
+				   struct clockdomain *clkdm));
+struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm);
+
+int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm);
+
+int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst);
+int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
+			  enum pwrdm_func_state fpwrst);
+int pwrdm_read_prev_fpwrst(struct powerdomain *pwrdm);
+int pwrdm_read_fpwrst(struct powerdomain *pwrdm);
+int pwrdm_read_next_fpwrst(struct powerdomain *pwrdm);
+
+int pwrdm_clear_all_prev_pwrst(struct powerdomain *pwrdm);
+
+int pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank, u8 pwrst);
+int pwrdm_set_mem_retst(struct powerdomain *pwrdm, u8 bank, u8 pwrst);
+
+int pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8 bank);
+int pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm, u8 bank);
+int pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank);
+
+int pwrdm_enable_hdwr_sar(struct powerdomain *pwrdm);
+int pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm);
+bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm);
+
+int pwrdm_state_switch(struct powerdomain *pwrdm);
+int pwrdm_pre_transition(struct powerdomain *pwrdm);
+int pwrdm_post_transition(struct powerdomain *pwrdm);
+
+int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
+bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
+
+extern void omap242x_powerdomains_init(void);
+extern void omap243x_powerdomains_init(void);
+extern void omap3xxx_powerdomains_init(void);
+extern void am33xx_powerdomains_init(void);
+extern void omap44xx_powerdomains_init(void);
+
+
+/***************************************************************
+ * Internal API, to be included by the low level PM code only:
+ * powerdomain*, pm-debug, hwmod, voltage, clockdomainxxxx
+ ***************************************************************/
+
 /* Powerdomain basic power states */
 #define PWRDM_POWER_OFF		0x0
 #define PWRDM_POWER_RET		0x1
@@ -92,9 +155,6 @@ enum pwrdm_logic_mem_state {
 /* XXX A completely arbitrary number. What is reasonable here? */
 #define PWRDM_TRANSITION_BAILOUT 100000
 
-struct clockdomain;
-struct powerdomain;
-
 /**
  * struct powerdomain - OMAP powerdomain
  * @name: Powerdomain name
@@ -210,64 +270,19 @@ int pwrdm_register_platform_funcs(struct pwrdm_ops *custom_funcs);
 int pwrdm_register_pwrdms(struct powerdomain **pwrdm_list);
 int pwrdm_complete_init(void);
 
-struct powerdomain *pwrdm_lookup(const char *name);
-
-int pwrdm_for_each(int (*fn)(struct powerdomain *pwrdm, void *user),
-			void *user);
-int pwrdm_for_each_nolock(int (*fn)(struct powerdomain *pwrdm, void *user),
-			void *user);
-
-int pwrdm_add_clkdm(struct powerdomain *pwrdm, struct clockdomain *clkdm);
-int pwrdm_del_clkdm(struct powerdomain *pwrdm, struct clockdomain *clkdm);
-int pwrdm_for_each_clkdm(struct powerdomain *pwrdm,
-			 int (*fn)(struct powerdomain *pwrdm,
-				   struct clockdomain *clkdm));
-struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm);
-
-int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm);
-
-int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst);
-int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
-			  enum pwrdm_func_state fpwrst);
-int pwrdm_read_prev_fpwrst(struct powerdomain *pwrdm);
-int pwrdm_read_fpwrst(struct powerdomain *pwrdm);
-int pwrdm_read_next_fpwrst(struct powerdomain *pwrdm);
-
 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);
 int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm);
-int pwrdm_clear_all_prev_pwrst(struct powerdomain *pwrdm);
 
 int pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst);
-int pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank, u8 pwrst);
-int pwrdm_set_mem_retst(struct powerdomain *pwrdm, u8 bank, u8 pwrst);
 
 int pwrdm_read_logic_pwrst(struct powerdomain *pwrdm);
 int pwrdm_read_prev_logic_pwrst(struct powerdomain *pwrdm);
 int pwrdm_read_logic_retst(struct powerdomain *pwrdm);
-int pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8 bank);
-int pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm, u8 bank);
-int pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank);
-
-int pwrdm_enable_hdwr_sar(struct powerdomain *pwrdm);
-int pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm);
-bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm);
-
 int pwrdm_wait_transition(struct powerdomain *pwrdm);
 
-int pwrdm_state_switch(struct powerdomain *pwrdm);
-int pwrdm_pre_transition(struct powerdomain *pwrdm);
-int pwrdm_post_transition(struct powerdomain *pwrdm);
 int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm);
-int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
-bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
-
-extern void omap242x_powerdomains_init(void);
-extern void omap243x_powerdomains_init(void);
-extern void omap3xxx_powerdomains_init(void);
-extern void am33xx_powerdomains_init(void);
-extern void omap44xx_powerdomains_init(void);
 
 extern struct pwrdm_ops omap2_pwrdm_operations;
 extern struct pwrdm_ops omap3_pwrdm_operations;
@@ -282,5 +297,4 @@ extern u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank);
 extern struct powerdomain wkup_omap2_pwrdm;
 extern struct powerdomain gfx_omap2_pwrdm;
 
-
 #endif
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/7] ARM: OMAP2+: PM: use power domain functional state in stats counters
  2012-09-12  9:55 ` [PATCH 4/7] ARM: OMAP2+: PM: use power domain functional state in stats counters Jean Pihet
@ 2012-09-12 23:35   ` Kevin Hilman
  0 siblings, 0 replies; 18+ messages in thread
From: Kevin Hilman @ 2012-09-12 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

Jean Pihet <jean.pihet@newoldbits.com> writes:

> 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.

nit: this is part of debugfs, not sysfs

Otherwise, patch is fine.

> This patch provides the information for each functional state.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>

Kevin

> ---
>  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..8eaa3f2 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_fpwrst(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_fpwrst(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 82ae9e9..267241f 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_fpwrst(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_fpwrst(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_fpwrst(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,
> @@ -1414,7 +1414,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 c3dc363..a29caec 100644
> --- a/arch/arm/mach-omap2/powerdomain.h
> +++ b/arch/arm/mach-omap2/powerdomain.h
> @@ -146,7 +146,7 @@ struct powerdomain {
>  	struct list_head voltdm_node;
>  	spinlock_t 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];
>  
> @@ -160,7 +160,7 @@ struct powerdomain {
>  
>  #ifdef CONFIG_PM_DEBUG
>  	s64 timer;
> -	s64 state_timer[PWRDM_MAX_PWRSTS];
> +	s64 state_timer[PWRDM_MAX_FUNC_PWRSTS];
>  #endif
>  };

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states
  2012-09-12  9:55 ` [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states Jean Pihet
@ 2012-09-12 23:47   ` Kevin Hilman
  2012-09-13  7:26     ` Jean Pihet
  2012-09-13  7:31     ` Jean Pihet
  0 siblings, 2 replies; 18+ messages in thread
From: Kevin Hilman @ 2012-09-12 23:47 UTC (permalink / raw)
  To: linux-arm-kernel

Jean Pihet <jean.pihet@newoldbits.com> writes:

> Trace the power domain transitions using the functional power states,
> which include the power and logic states.

Just to be clear, this means that a trace will only contain functional
power state changes, not logical ones, correct?

> While at it, fix the trace in the case a power domain did not hit
> the desired state, as reported by Paul Walmsley.

What was broken here?  needs a bit more description.  To me it sounds
like a fix that should be a separate patch.

> Reported-by: Paul Walmsley <paul@pwsan.com>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
>  arch/arm/mach-omap2/powerdomain.c |   23 +++++++++++++----------
>  1 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index 267241f..2277ad3 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -144,7 +144,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm)
>  static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
>  {
>  
> -	int prev, state, trace_state = 0;
> +	int prev, next, state, trace_state;
>  
>  	if (pwrdm == NULL)
>  		return -EINVAL;
> @@ -165,10 +165,10 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
>  		 * If the power domain did not hit the desired state,
>  		 * generate a trace event with both the desired and hit states
>  		 */
> -		if (state != prev) {
> +		next = pwrdm_read_next_fpwrst(pwrdm);
> +		if (next != prev) {
>  			trace_state = (PWRDM_TRACE_STATES_FLAG |
> -				       ((state & OMAP_POWERSTATE_MASK) << 8) |
> -				       ((prev & OMAP_POWERSTATE_MASK) << 0));
> +				       (next << 8) | (prev << 0));
>  			trace_power_domain_target(pwrdm->name, trace_state,
>  						  smp_processor_id());
>  		}
> @@ -723,6 +723,10 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
>  		}
>  	}
>  
> +	/* Trace the pwrdm desired target state */
> +	trace_power_domain_target(pwrdm->name, next_fpwrst,
> +				  smp_processor_id());

Use of smp_processor_id() here will require the same care as pointed out
by Roger Quadros in [PATCH] perf: Use raw_smp_processor_id insted of
smp_processor_id.

Kevin

>  	if (logic != pwrdm_read_logic_retst(pwrdm))
>  		pwrdm_set_logic_retst(pwrdm, logic);
>  
> @@ -776,6 +780,10 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
>  
>  	spin_lock_irqsave(&pwrdm->lock, flags);
>  
> +	/* Trace the pwrdm desired target state */
> +	trace_power_domain_target(pwrdm->name, fpwrst,
> +				  smp_processor_id());
> +
>  	ret = pwrdm_set_logic_retst(pwrdm, logic);
>  	if (ret)
>  		pr_err("%s: unable to set logic state %0x of powerdomain: %s\n",
> @@ -821,13 +829,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
>  	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
>  		 pwrdm->name, pwrst);
>  
> -	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
> -		/* Trace the pwrdm desired target state */
> -		trace_power_domain_target(pwrdm->name, pwrst,
> -					  smp_processor_id());
> -		/* Program the pwrdm desired target state */
> +	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
>  		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
> -	}
>  
>  	return ret;
>  }

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 7/7] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts
  2012-09-12  9:55 ` [PATCH 7/7] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts Jean Pihet
@ 2012-09-13  0:11   ` Kevin Hilman
  2012-09-13  7:29     ` Jean Pihet
  0 siblings, 1 reply; 18+ messages in thread
From: Kevin Hilman @ 2012-09-13  0:11 UTC (permalink / raw)
  To: linux-arm-kernel

Jean Pihet <jean.pihet@newoldbits.com> writes:

> The newly added code for functional power states re-defines the
> API to query and control the power domains settings.
>
> The API is now split in the following parts in powerdomain.h:
> - the public or external API, to be used by external PM components:
>   cpuidle, suspend, pmxxxx, clock* etc.
> - the private or internal API, to be used by the low level PM code
>   only: powerdomain*, pm-debug, hwmod, voltage, clockdomainxxxx.
>
> The function omap_set_pwrdm_state is not used anymore and so is
> removed.
>
> No functional change is introduced by this patch.
>
> Note: the API reorganization in a public and private header files
> is not part of this patch, this comes as a subsequent clean-up
> patch series.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>

In addition to reorganizing the API, I suspect there are a handful of
out-of-tree hacks, er, users, that will are using the internal state
names, as well as the functions that should now only be internal.

As part of the subsequent cleanup series, it would it make sense to add
a '_' prefix to the internal names as well to catch unintentional use of
internal APIs?

Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states
  2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
                   ` (6 preceding siblings ...)
  2012-09-12  9:55 ` [PATCH 7/7] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts Jean Pihet
@ 2012-09-13  0:34 ` Kevin Hilman
  2012-09-13  7:04   ` Jean Pihet
  2012-09-26 21:28 ` Paul Walmsley
  8 siblings, 1 reply; 18+ messages in thread
From: Kevin Hilman @ 2012-09-13  0:34 UTC (permalink / raw)
  To: linux-arm-kernel

Jean Pihet <jean.pihet@newoldbits.com> writes:

> Here is a re-spin after some comments and suggestions after review
> and discussions.
>
> Implement the functional states for the power domains:
> - unify the API to use the functional states. The new API
>   consists of the pwrdm_set*_fpwrst and pwrdm_read*_fpwrst
>   functions and is the API to use to control the power domains
>   power and logic states,
> - reorganize the powerdomain API in internal and external parts,
>   in powerdomain.h [1]
> - protect the power domain state change by a lock in the
>   functions that read and set the powerdomains next functional state,
> - introduce the functional states for power domains power states and
>   logic power states [2], and the conversion functions between the
>   functional and internal states. The conversion functions are
>   lightweight and generic. The power domains allowed states [3] are
>   defined in the pwrsts and pwrsts_logic_ret fields of the struct
>   powerdomain,
> - program the logic power state of power domains from the functional
>   states, in pwrdm_set*_fpwrst
> - convert the OMAP2/3/4 PM code to use the updated API,
> - provide the power domains statistics by functional states,
> - provide ftrace tracepoints with the functional state,
> - provide error logs in critical code, which makes the development
>   easier.

I just gave this series a round of PM testing.  I tested retention
and off in idle & suspend, with and without CPUidle on 3430/n900,
3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda (though only
MPU/CPU ret/off is supported for OMAP4 in mainline.)

All PM tests passed with flying colors.  Nice!

Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states
  2012-09-13  0:34 ` [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Kevin Hilman
@ 2012-09-13  7:04   ` Jean Pihet
  2012-09-18 16:51     ` Jean Pihet
  0 siblings, 1 reply; 18+ messages in thread
From: Jean Pihet @ 2012-09-13  7:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 13, 2012 at 2:34 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Jean Pihet <jean.pihet@newoldbits.com> writes:
>
>> Here is a re-spin after some comments and suggestions after review
>> and discussions.
>>
>> Implement the functional states for the power domains:
>> - unify the API to use the functional states. The new API
>>   consists of the pwrdm_set*_fpwrst and pwrdm_read*_fpwrst
>>   functions and is the API to use to control the power domains
>>   power and logic states,
>> - reorganize the powerdomain API in internal and external parts,
>>   in powerdomain.h [1]
>> - protect the power domain state change by a lock in the
>>   functions that read and set the powerdomains next functional state,
>> - introduce the functional states for power domains power states and
>>   logic power states [2], and the conversion functions between the
>>   functional and internal states. The conversion functions are
>>   lightweight and generic. The power domains allowed states [3] are
>>   defined in the pwrsts and pwrsts_logic_ret fields of the struct
>>   powerdomain,
>> - program the logic power state of power domains from the functional
>>   states, in pwrdm_set*_fpwrst
>> - convert the OMAP2/3/4 PM code to use the updated API,
>> - provide the power domains statistics by functional states,
>> - provide ftrace tracepoints with the functional state,
>> - provide error logs in critical code, which makes the development
>>   easier.
>
> I just gave this series a round of PM testing.  I tested retention
> and off in idle & suspend, with and without CPUidle on 3430/n900,
> 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda (though only
> MPU/CPU ret/off is supported for OMAP4 in mainline.)
>
> All PM tests passed with flying colors.  Nice!
Great!

Thanks a lot Kevin for testing

Jean

>
> Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states
  2012-09-12 23:47   ` Kevin Hilman
@ 2012-09-13  7:26     ` Jean Pihet
  2012-09-13  7:31     ` Jean Pihet
  1 sibling, 0 replies; 18+ messages in thread
From: Jean Pihet @ 2012-09-13  7:26 UTC (permalink / raw)
  To: linux-arm-kernel

HI Kevin,

On Thu, Sep 13, 2012 at 1:47 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Jean Pihet <jean.pihet@newoldbits.com> writes:
>
>> Trace the power domain transitions using the functional power states,
>> which include the power and logic states.
>
> Just to be clear, this means that a trace will only contain functional
> power state changes, not logical ones, correct?
Correct! The trace reports functional states, while pr_err and
pr_debug statements (added by patch 6/7) are present for hardcore
debugging on the functional and internal states.

>
>> While at it, fix the trace in the case a power domain did not hit
>> the desired state, as reported by Paul Walmsley.
>
> What was broken here?  needs a bit more description.
Ok will do

> To me it sounds
> like a fix that should be a separate patch.
I kept the fix in this patch since it matches $SUBJECT. Can be split
if needed though.

Thanks for reviewing!
Jean

>
>> Reported-by: Paul Walmsley <paul@pwsan.com>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> ---
>>  arch/arm/mach-omap2/powerdomain.c |   23 +++++++++++++----------
>>  1 files changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
>> index 267241f..2277ad3 100644
>> --- a/arch/arm/mach-omap2/powerdomain.c
>> +++ b/arch/arm/mach-omap2/powerdomain.c
>> @@ -144,7 +144,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm)
>>  static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
>>  {
>>
>> -     int prev, state, trace_state = 0;
>> +     int prev, next, state, trace_state;
>>
>>       if (pwrdm == NULL)
>>               return -EINVAL;
>> @@ -165,10 +165,10 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
>>                * If the power domain did not hit the desired state,
>>                * generate a trace event with both the desired and hit states
>>                */
>> -             if (state != prev) {
>> +             next = pwrdm_read_next_fpwrst(pwrdm);
>> +             if (next != prev) {
>>                       trace_state = (PWRDM_TRACE_STATES_FLAG |
>> -                                    ((state & OMAP_POWERSTATE_MASK) << 8) |
>> -                                    ((prev & OMAP_POWERSTATE_MASK) << 0));
>> +                                    (next << 8) | (prev << 0));
>>                       trace_power_domain_target(pwrdm->name, trace_state,
>>                                                 smp_processor_id());
>>               }
>> @@ -723,6 +723,10 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
>>               }
>>       }
>>
>> +     /* Trace the pwrdm desired target state */
>> +     trace_power_domain_target(pwrdm->name, next_fpwrst,
>> +                               smp_processor_id());
>
> Use of smp_processor_id() here will require the same care as pointed out
> by Roger Quadros in [PATCH] perf: Use raw_smp_processor_id insted of
> smp_processor_id.
>
> Kevin
>
>>       if (logic != pwrdm_read_logic_retst(pwrdm))
>>               pwrdm_set_logic_retst(pwrdm, logic);
>>
>> @@ -776,6 +780,10 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
>>
>>       spin_lock_irqsave(&pwrdm->lock, flags);
>>
>> +     /* Trace the pwrdm desired target state */
>> +     trace_power_domain_target(pwrdm->name, fpwrst,
>> +                               smp_processor_id());
>> +
>>       ret = pwrdm_set_logic_retst(pwrdm, logic);
>>       if (ret)
>>               pr_err("%s: unable to set logic state %0x of powerdomain: %s\n",
>> @@ -821,13 +829,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
>>       pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
>>                pwrdm->name, pwrst);
>>
>> -     if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
>> -             /* Trace the pwrdm desired target state */
>> -             trace_power_domain_target(pwrdm->name, pwrst,
>> -                                       smp_processor_id());
>> -             /* Program the pwrdm desired target state */
>> +     if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
>>               ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
>> -     }
>>
>>       return ret;
>>  }

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 7/7] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts
  2012-09-13  0:11   ` Kevin Hilman
@ 2012-09-13  7:29     ` Jean Pihet
  0 siblings, 0 replies; 18+ messages in thread
From: Jean Pihet @ 2012-09-13  7:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 13, 2012 at 2:11 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Jean Pihet <jean.pihet@newoldbits.com> writes:
>
>> The newly added code for functional power states re-defines the
>> API to query and control the power domains settings.
>>
>> The API is now split in the following parts in powerdomain.h:
>> - the public or external API, to be used by external PM components:
>>   cpuidle, suspend, pmxxxx, clock* etc.
>> - the private or internal API, to be used by the low level PM code
>>   only: powerdomain*, pm-debug, hwmod, voltage, clockdomainxxxx.
>>
>> The function omap_set_pwrdm_state is not used anymore and so is
>> removed.
>>
>> No functional change is introduced by this patch.
>>
>> Note: the API reorganization in a public and private header files
>> is not part of this patch, this comes as a subsequent clean-up
>> patch series.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>
> In addition to reorganizing the API, I suspect there are a handful of
> out-of-tree hacks, er, users, that will are using the internal state
> names, as well as the functions that should now only be internal.
The API clean-up series (planned after this one is in the queue) will
sort out the public vs private APIs using different header files and
static functions.

> As part of the subsequent cleanup series, it would it make sense to add
> a '_' prefix to the internal names as well to catch unintentional use of
> internal APIs?
Sure.

>
> Kevin

Thanks,
Jean

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states
  2012-09-12 23:47   ` Kevin Hilman
  2012-09-13  7:26     ` Jean Pihet
@ 2012-09-13  7:31     ` Jean Pihet
  1 sibling, 0 replies; 18+ messages in thread
From: Jean Pihet @ 2012-09-13  7:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 13, 2012 at 1:47 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Use of smp_processor_id() here will require the same care as pointed out
> by Roger Quadros in [PATCH] perf: Use raw_smp_processor_id insted of
> smp_processor_id.
BTW it looks like get_cpu and put_cpu is the way to go, as pointed out
by Russell.

Jean

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states
  2012-09-13  7:04   ` Jean Pihet
@ 2012-09-18 16:51     ` Jean Pihet
  0 siblings, 0 replies; 18+ messages in thread
From: Jean Pihet @ 2012-09-18 16:51 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Paul,

On Thu, Sep 13, 2012 at 9:04 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
> On Thu, Sep 13, 2012 at 2:34 AM, Kevin Hilman
> <khilman@deeprootsystems.com> wrote:
>> Jean Pihet <jean.pihet@newoldbits.com> writes:
>>
>>> Here is a re-spin after some comments and suggestions after review
>>> and discussions.
Did you have a chance to look at the series?

Regards,
Jean

>>>
>>> Implement the functional states for the power domains:
>>> - unify the API to use the functional states. The new API
>>>   consists of the pwrdm_set*_fpwrst and pwrdm_read*_fpwrst
>>>   functions and is the API to use to control the power domains
>>>   power and logic states,
>>> - reorganize the powerdomain API in internal and external parts,
>>>   in powerdomain.h [1]
>>> - protect the power domain state change by a lock in the
>>>   functions that read and set the powerdomains next functional state,
>>> - introduce the functional states for power domains power states and
>>>   logic power states [2], and the conversion functions between the
>>>   functional and internal states. The conversion functions are
>>>   lightweight and generic. The power domains allowed states [3] are
>>>   defined in the pwrsts and pwrsts_logic_ret fields of the struct
>>>   powerdomain,
>>> - program the logic power state of power domains from the functional
>>>   states, in pwrdm_set*_fpwrst
>>> - convert the OMAP2/3/4 PM code to use the updated API,
>>> - provide the power domains statistics by functional states,
>>> - provide ftrace tracepoints with the functional state,
>>> - provide error logs in critical code, which makes the development
>>>   easier.
>>
>> I just gave this series a round of PM testing.  I tested retention
>> and off in idle & suspend, with and without CPUidle on 3430/n900,
>> 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda (though only
>> MPU/CPU ret/off is supported for OMAP4 in mainline.)
>>
>> All PM tests passed with flying colors.  Nice!
> Great!
>
> Thanks a lot Kevin for testing
>
> Jean
>
>>
>> Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states
  2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
                   ` (7 preceding siblings ...)
  2012-09-13  0:34 ` [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Kevin Hilman
@ 2012-09-26 21:28 ` Paul Walmsley
  8 siblings, 0 replies; 18+ messages in thread
From: Paul Walmsley @ 2012-09-26 21:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Jean,

On Wed, 12 Sep 2012, Jean Pihet wrote:

> Here is a re-spin after some comments and suggestions after review and 
> discussions.

Thanks Jean, I'll take it from here, probably with some changes which I 
will respond to the individual patches, probably for 3.8.


- Paul

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-09-26 21:28 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-12  9:55 [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Jean Pihet
2012-09-12  9:55 ` [PATCH 1/7] ARM: OMAP2+: PM: introduce " Jean Pihet
2012-09-12  9:55 ` [PATCH 2/7] ARM: OMAP2+: PM: add a lock to protect the powerdomains next state Jean Pihet
2012-09-12  9:55 ` [PATCH 3/7] ARM: OMAP2+: PM: use the functional power states API Jean Pihet
2012-09-12  9:55 ` [PATCH 4/7] ARM: OMAP2+: PM: use power domain functional state in stats counters Jean Pihet
2012-09-12 23:35   ` Kevin Hilman
2012-09-12  9:55 ` [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states Jean Pihet
2012-09-12 23:47   ` Kevin Hilman
2012-09-13  7:26     ` Jean Pihet
2012-09-13  7:31     ` Jean Pihet
2012-09-12  9:55 ` [PATCH 6/7] ARM: OMAP2+: powerdomain: add error logs Jean Pihet
2012-09-12  9:55 ` [PATCH 7/7] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts Jean Pihet
2012-09-13  0:11   ` Kevin Hilman
2012-09-13  7:29     ` Jean Pihet
2012-09-13  0:34 ` [PATCH v6 0/7] ARM: OMAP2+: PM: introduce the power domains functional states Kevin Hilman
2012-09-13  7:04   ` Jean Pihet
2012-09-18 16:51     ` Jean Pihet
2012-09-26 21:28 ` Paul Walmsley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).