ARM: OMAP2+: PM: introduce the power domains functional states

omap_set_pwrdm_state is intented to be the only API for changing a power domain state.

This patch protects the power domains settings and structs from concurrent accesses to the function by using a mutex.

Signed-off-by Jean Pihet<[email protected]>

---
  arch/arm/mach-omap2/pm.c          |    8 ++++++--
  arch/arm/mach-omap2/powerdomain.c |    1 +
  arch/arm/mach-omap2/powerdomain.h |    3 ++-
  3 files changed, 9 insertions(+), 3 deletions(-)

arch/arm/mach-omap2/pm.c +6 -2

index 9cb5ced..a05f00c 100644
@@ -100,15 +100,17 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst)
  	if (!pwrdm || IS_ERR(pwrdm))
  		return -EINVAL;
 
+	mutex_lock(&pwrdm->lock);
+
  	while (!(pwrdm->pwrsts&  (1<<  pwrst))) {
  		if (pwrst == PWRDM_POWER_OFF)
-			return ret;
+			goto out;
  		pwrst--;
  	}
 
  	next_pwrst = pwrdm_read_next_pwrst(pwrdm);
  	if (next_pwrst == pwrst)
-		return ret;
+		goto out;
 
  	curr_pwrst = pwrdm_read_pwrst(pwrdm);
  	if (curr_pwrst<  PWRDM_POWER_ON) {

@@ -141,6 +143,8 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst)
  		break;
  	}
 
+out:
+	mutex_unlock(&pwrdm->lock);
  	return ret;
  }
 

arch/arm/mach-omap2/powerdomain.c +1 -0

index 9611490..1641e72 100644
@@ -102,6 +102,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
  	INIT_LIST_HEAD(&pwrdm->voltdm_node);
  	voltdm_add_pwrdm(voltdm, pwrdm);
 
+	mutex_init(&pwrdm->lock);
  	list_add(&pwrdm->node,&pwrdm_list);
 
  	/* Initialize the powerdomain's state counter */

arch/arm/mach-omap2/powerdomain.h +2 -1

index 8f88d65..bab84fc 100644
@@ -19,7 +19,7 @@
 
  #include<linux/types.h>
  #include<linux/list.h>
-
+#include<linux/mutex.h>
  #include<linux/atomic.h>
 
  #include<plat/cpu.h>

@@ -116,6 +116,7 @@ struct powerdomain {
  	struct clockdomain *pwrdm_clkdms[PWRDM_MAX_CLKDMS];
  	struct list_head node;
  	struct list_head voltdm_node;
+	struct mutex lock;
  	int state;
  	unsigned state_counter[PWRDM_MAX_PWRSTS];
  	unsigned ret_logic_off_counter;

Introduce functional (or logical) states for power domains and the API functions to read the power domains settings and to convert between the functional (i.e. logical) and the internal (or registers) values.

OMAP2, OMAP3 and OMAP4 platforms are defining a conversion routine.

In the new API only the function omap_set_pwrdm_state shall be used to change a power domain target state, along with the associated PWRDM_FUNC_* macros as the state parameters.
While at it the function is moved to the power domains code.

The power domain API in powerdomain.h is split in the external and the internal parts; only the external API functions and defines shall be used by external code, the internal API is only to be used in powerdomain*.[ch] files.

The memory and logic states are not using the functional states, this comes as a subsequent patch.

Signed-off-by Jean Pihet <[email protected]>

---

?arch/arm/mach-omap2/pm.c ? ? ? ? ? ? ? ? ? | ? 66 -----------
?arch/arm/mach-omap2/pm.h ? ? ? ? ? ? ? ? ? | ? ?1 -
?arch/arm/mach-omap2/powerdomain-common.c ? | ? 71 +++++++++++
?arch/arm/mach-omap2/powerdomain.c ? ? ? ? ?| ?174 ++++++++++++++++++++++++++++
?arch/arm/mach-omap2/powerdomain.h ? ? ? ? ?| ? 77 +++++++++----
?arch/arm/mach-omap2/powerdomain2xxx_3xxx.c | ? ?5 +
?arch/arm/mach-omap2/powerdomain44xx.c ? ? ?| ? ?2 +

?7 files changed, 306 insertions(+), 90 deletions(-)

arch/arm/mach-omap2/pm.c +0 -4

index a05f00c..dfe702b 100644
@@ -74,10 +74,6 @@ static void __init omap2_init_processor_devices(void)
 ? ? ? ?}
 ?}
 
-/* Types of sleep_switch used in omap_set_pwrdm_state */
-#define FORCEWAKEUP_SWITCH ? ? 0
-#define LOWPOWERSTATE_SWITCH ? 1
-
 ?int __init omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused)
 ?{
 ? ? ? ?if (clkdm->flags & CLKDM_CAN_ENABLE_AUTO)

@@ -89,68 +85,6 @@ int __init omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused)
 ?}
 

?/*
- * This sets pwrdm state (other than mpu & core. Currently only ON &
- * RET are supported.
- */

arch/arm/mach-omap2/pm.c +0 -59

-int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst)
-{
- ? ? ? u8 curr_pwrst, next_pwrst;
- ? ? ? int sleep_switch = -1, ret = 0, hwsup = 0;
-
- ? ? ? if (!pwrdm || IS_ERR(pwrdm))
- ? ? ? ? ? ? ? return -EINVAL;
-
- ? ? ? mutex_lock(&pwrdm->lock);
-
- ? ? ? while (!(pwrdm->pwrsts & (1 << pwrst))) {
- ? ? ? ? ? ? ? if (pwrst == PWRDM_POWER_OFF)
- ? ? ? ? ? ? ? ? ? ? ? goto out;
- ? ? ? ? ? ? ? pwrst--;
- ? ? ? }
-
- ? ? ? next_pwrst = pwrdm_read_next_pwrst(pwrdm);
- ? ? ? if (next_pwrst == pwrst)
- ? ? ? ? ? ? ? goto out;
-
- ? ? ? curr_pwrst = pwrdm_read_pwrst(pwrdm);
- ? ? ? if (curr_pwrst < PWRDM_POWER_ON) {
- ? ? ? ? ? ? ? if ((curr_pwrst > pwrst) &&
- ? ? ? ? ? ? ? ? ? ? ? (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
- ? ? ? ? ? ? ? ? ? ? ? sleep_switch = LOWPOWERSTATE_SWITCH;
- ? ? ? ? ? ? ? } else {
- ? ? ? ? ? ? ? ? ? ? ? hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
- ? ? ? ? ? ? ? ? ? ? ? clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
- ? ? ? ? ? ? ? ? ? ? ? sleep_switch = FORCEWAKEUP_SWITCH;
- ? ? ? ? ? ? ? }
- ? ? ? }
-
- ? ? ? ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
- ? ? ? if (ret)
- ? ? ? ? ? ? ? pr_err("%s: unable to set power state of powerdomain: %s\n",
- ? ? ? ? ? ? ? ? ? ? ?__func__, pwrdm->name);
-
- ? ? ? switch (sleep_switch) {
- ? ? ? case FORCEWAKEUP_SWITCH:
- ? ? ? ? ? ? ? if (hwsup)
- ? ? ? ? ? ? ? ? ? ? ? clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
- ? ? ? ? ? ? ? else
- ? ? ? ? ? ? ? ? ? ? ? clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
- ? ? ? ? ? ? ? break;
- ? ? ? case LOWPOWERSTATE_SWITCH:
- ? ? ? ? ? ? ? pwrdm_set_lowpwrstchange(pwrdm);
- ? ? ? ? ? ? ? pwrdm_wait_transition(pwrdm);
- ? ? ? ? ? ? ? pwrdm_state_switch(pwrdm);
- ? ? ? ? ? ? ? break;
- ? ? ? }
-
-out:
- ? ? ? mutex_unlock(&pwrdm->lock);
- ? ? ? return ret;
-}
-
-
-
-/*

?* This API is to be called during init to set the various voltage ?* domains to the voltage as per the opp table. Typically we boot up ?* at the nominal voltage. So this function finds out the rate of

arch/arm/mach-omap2/pm.h +0 -1

index 7856489..5bc0848 100644
@@ -18,7 +18,6 @@
 ?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 omap3_idle_init(void);
 ?extern int omap4_idle_init(void);
 ?extern int omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused);

arch/arm/mach-omap2/powerdomain-common.c +71 -0

index c0aeabf..2973be7 100644
@@ -108,3 +108,74 @@ u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank)
 ? ? ? ?return 0;
 ?}
 
+/**
+ * omap2_pwrdm_func_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
+ * @func_pwrst: functional power state
+ *
+ * Returns the internal power state value for the power domain, or
+ * -EINVAL in case of invalid parameters passed in.
+ */
+int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
+{
+ ? ? ? int ret;
+
+ ? ? ? switch (func_pwrst) {
+ ? ? ? case PWRDM_FUNC_PWRST_ON:
+ ? ? ? ? ? ? ? ret = PWRDM_POWER_ON;
+ ? ? ? ? ? ? ? break;
+ ? ? ? case PWRDM_FUNC_PWRST_INACTIVE:
+ ? ? ? ? ? ? ? ret = PWRDM_POWER_INACTIVE;
+ ? ? ? ? ? ? ? break;
+ ? ? ? case PWRDM_FUNC_PWRST_CSWR:
+ ? ? ? case PWRDM_FUNC_PWRST_OSWR:
+ ? ? ? ? ? ? ? ret = PWRDM_POWER_RET;
+ ? ? ? ? ? ? ? break;
+ ? ? ? case PWRDM_FUNC_PWRST_OFF:
+ ? ? ? ? ? ? ? ret = PWRDM_POWER_OFF;
+ ? ? ? ? ? ? ? break;
+ ? ? ? default:
+ ? ? ? ? ? ? ? ret = -EINVAL;
+ ? ? ? }
+
+ ? ? ? return ret;
+}
+
+/**
+ * omap2_pwrdm_pwrst_to_func - 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.
+ */
+int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst)
+{
+ ? ? ? int ret;
+
+ ? ? ? switch (pwrst) {
+ ? ? ? case PWRDM_POWER_ON:
+ ? ? ? ? ? ? ? ret = PWRDM_FUNC_PWRST_ON;
+ ? ? ? ? ? ? ? break;
+ ? ? ? case PWRDM_POWER_INACTIVE:
+ ? ? ? ? ? ? ? ret = PWRDM_FUNC_PWRST_INACTIVE;
+ ? ? ? ? ? ? ? break;
+ ? ? ? case PWRDM_POWER_RET:
+ ? ? ? ? ? ? ? /*
+ ? ? ? ? ? ? ? ?* XXX warning: return OSWR in case of pd in RET and
+ ? ? ? ? ? ? ? ?* logic in OFF
+ ? ? ? ? ? ? ? ?*/
+ ? ? ? ? ? ? ? ret = PWRDM_FUNC_PWRST_CSWR;
+ ? ? ? ? ? ? ? break;
+ ? ? ? case PWRDM_POWER_OFF:
+ ? ? ? ? ? ? ? ret = PWRDM_FUNC_PWRST_OFF;
+ ? ? ? ? ? ? ? break;
+ ? ? ? default:
+ ? ? ? ? ? ? ? ret = -EINVAL;
+ ? ? ? }
+
+ ? ? ? return ret;
+}
+

arch/arm/mach-omap2/powerdomain.c +0 -0

index 1641e72..e79b5ae 100644
@@ -466,6 +466,135 @@ int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm)
 ?}
 

?/**
+ * pwrdm_func_to_pwrst - get the internal (i.e. registers) value mapped
+ * to the functional state
+ * @pwrdm: struct powerdomain * to query
+ * @func_pwrst: functional power state
+ *
+ * Convert the functional power state to the platform specific power
+ * domain state value.
+ * Returns the internal power domain state value or -EINVAL in case
+ * of invalid parameters passed in.
+ */

arch/arm/mach-omap2/powerdomain.c +119 -0

+int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
+{
+ ? ? ? int ret = func_pwrst;
+
+ ? ? ? if ((!pwrdm) || (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS))
+ ? ? ? ? ? ? ? return -EINVAL;
+
+ ? ? ? if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_pwrst)
+ ? ? ? ? ? ? ? ret = arch_pwrdm->pwrdm_func_to_pwrst(pwrdm, func_pwrst);
+
+ ? ? ? pr_debug("powerdomain: convert func %0x to pwrst %0x for %s\n",
+ ? ? ? ? ? ? ? ?func_pwrst, ret, pwrdm->name);
+
+ ? ? ? return ret;
+}
+
+/**
+ * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped
+ * to the internal state
+ * @pwrdm: struct powerdomain * to query
+ * @pwrst: internal power state
+ *
+ * Convert the internal power state to the power domain functional value.
+ * Returns the functional power domain state value or -EINVAL in case
+ * of invalid parameters passed in.
+ */
+int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst)
+{
+ ? ? ? int ret = pwrst;
+
+ ? ? ? if ((!pwrdm) || (pwrst >= PWRDM_MAX_PWRSTS))
+ ? ? ? ? ? ? ? return -EINVAL;
+
+ ? ? ? pr_debug("powerdomain: convert %0x pwrst to func for %s\n",
+ ? ? ? ? ? ? ? ?pwrst, pwrdm->name);
+
+ ? ? ? if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func)
+ ? ? ? ? ? ? ? ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst);
+
+ ? ? ? return ret;
+}
+
+/* Types of sleep_switch used in omap_set_pwrdm_state */
+#define FORCEWAKEUP_SWITCH ? ? 0
+#define LOWPOWERSTATE_SWITCH ? 1
+
+/**
+ * omap_set_pwrdm_state - program next powerdomain power state
+ * @pwrdm: struct powerdomain * to set
+ * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros
+ *
+ * This programs the pwrdm next functional state, sets the dependencies
+ * and waits for the state to be applied.
+ */
+int omap_set_pwrdm_state(struct powerdomain *pwrdm,
+ ? ? ? ? ? ? ? ? ? ? ? ?enum pwrdm_func_state func_pwrst)
+{
+ ? ? ? u8 curr_pwrst, next_pwrst;
+ ? ? ? int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst);
+ ? ? ? int sleep_switch = -1, ret = 0, hwsup = 0;
+
+ ? ? ? if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0)) {
+ ? ? ? ? ? ? ? pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n",
+ ? ? ? ? ? ? ? ? ? ? ? ?__func__, pwrdm, func_pwrst);
+ ? ? ? ? ? ? ? return -EINVAL;
+ ? ? ? }
+
+ ? ? ? pr_debug("%s: set func_pwrst %0x to pwrdm %s\n",
+ ? ? ? ? ? ? ? ?__func__, func_pwrst, pwrdm->name);
+
+ ? ? ? mutex_lock(&pwrdm->lock);
+
+ ? ? ? while (!(pwrdm->pwrsts & (1 << pwrst))) {
+ ? ? ? ? ? ? ? if (pwrst == PWRDM_POWER_OFF)
+ ? ? ? ? ? ? ? ? ? ? ? goto out;
+ ? ? ? ? ? ? ? pwrst--;
+ ? ? ? }
+
+ ? ? ? next_pwrst = pwrdm_read_next_pwrst(pwrdm);
+ ? ? ? if (next_pwrst == pwrst)
+ ? ? ? ? ? ? ? goto out;
+
+ ? ? ? curr_pwrst = pwrdm_read_pwrst(pwrdm);
+ ? ? ? if (curr_pwrst < PWRDM_POWER_ON) {
+ ? ? ? ? ? ? ? if ((curr_pwrst > pwrst) &&
+ ? ? ? ? ? ? ? ? ? ? ? (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
+ ? ? ? ? ? ? ? ? ? ? ? sleep_switch = LOWPOWERSTATE_SWITCH;
+ ? ? ? ? ? ? ? } else {
+ ? ? ? ? ? ? ? ? ? ? ? hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
+ ? ? ? ? ? ? ? ? ? ? ? clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
+ ? ? ? ? ? ? ? ? ? ? ? sleep_switch = FORCEWAKEUP_SWITCH;
+ ? ? ? ? ? ? ? }
+ ? ? ? }
+
+ ? ? ? ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
+ ? ? ? if (ret)
+ ? ? ? ? ? ? ? pr_err("%s: unable to set power state of powerdomain: %s\n",
+ ? ? ? ? ? ? ? ? ? ? ?__func__, pwrdm->name);
+
+ ? ? ? switch (sleep_switch) {
+ ? ? ? case FORCEWAKEUP_SWITCH:
+ ? ? ? ? ? ? ? if (hwsup)
+ ? ? ? ? ? ? ? ? ? ? ? clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+ ? ? ? ? ? ? ? else
+ ? ? ? ? ? ? ? ? ? ? ? clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
+ ? ? ? ? ? ? ? break;
+ ? ? ? case LOWPOWERSTATE_SWITCH:
+ ? ? ? ? ? ? ? pwrdm_set_lowpwrstchange(pwrdm);
+ ? ? ? ? ? ? ? pwrdm_wait_transition(pwrdm);
+ ? ? ? ? ? ? ? pwrdm_state_switch(pwrdm);
+ ? ? ? ? ? ? ? break;
+ ? ? ? }
+
+out:
+ ? ? ? mutex_unlock(&pwrdm->lock);
+ ? ? ? return ret;
+}
+
+/**

?* pwrdm_set_next_pwrst - set next powerdomain power state ?* @pwrdm: struct powerdomain * to set
?* @pwrst: one of the PWRDM_POWER_* macros

arch/arm/mach-omap2/powerdomain.c +0 -0

@@ -522,6 +651,21 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 ?}
 

?/**
+ * pwrdm_read_next_func_pwrst - get next powerdomain functional power state
+ * @pwrdm: struct powerdomain * to get power state
+ *
+ * Return the powerdomain @pwrdm's next functional power state.
+ * Returns -EINVAL if the powerdomain pointer is null or returns
+ * the next power state upon success.
+ */

arch/arm/mach-omap2/powerdomain.c +8 -0

+int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm)
+{
+ ? ? ? int ret = pwrdm_read_next_pwrst(pwrdm);
+
+ ? ? ? return pwrdm_pwrst_to_func(pwrdm, ret);
+}
+
+/**

?* pwrdm_read_pwrst - get current powerdomain power state ?* @pwrdm: struct powerdomain * to get power state ?*

arch/arm/mach-omap2/powerdomain.c +0 -0

@@ -543,6 +687,21 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 ?}
 

?/**
+ * pwrdm_read_func_pwrst - get current powerdomain functional power state
+ * @pwrdm: struct powerdomain * to get power state
+ *
+ * Return the powerdomain @pwrdm's current functional power state.
+ * Returns -EINVAL if the powerdomain pointer is null or returns
+ * the current power state upon success.
+ */

arch/arm/mach-omap2/powerdomain.c +8 -0

+int pwrdm_read_func_pwrst(struct powerdomain *pwrdm)
+{
+ ? ? ? int ret = pwrdm_read_pwrst(pwrdm);
+
+ ? ? ? return pwrdm_pwrst_to_func(pwrdm, ret);
+}
+
+/**

?* pwrdm_read_prev_pwrst - get previous powerdomain power state ?* @pwrdm: struct powerdomain * to get previous power state ?*

arch/arm/mach-omap2/powerdomain.c +0 -0

@@ -564,6 +723,21 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
 ?}
 

?/**
+ * pwrdm_read_prev_func_pwrst - get previous powerdomain functional power state
+ * @pwrdm: struct powerdomain * to get previous power state
+ *
+ * Return the powerdomain @pwrdm's previous functional power state.
+ * Returns -EINVAL if the powerdomain pointer is null or returns the
+ * previous power state upon success.
+ */

arch/arm/mach-omap2/powerdomain.c +8 -0

+int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm)
+{
+ ? ? ? int ret = pwrdm_read_prev_pwrst(pwrdm);
+
+ ? ? ? return pwrdm_pwrst_to_func(pwrdm, ret);
+}
+
+/**

?* pwrdm_set_logic_retst - set powerdomain logic power state upon retention ?* @pwrdm: struct powerdomain * to set
?* @pwrst: one of the PWRDM_POWER_* macros

arch/arm/mach-omap2/powerdomain.h +0 -0

index bab84fc..0404f9f 100644
@@ -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 at the earliest
- * opportunity.
?*/

?#ifndef __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_H

arch/arm/mach-omap2/powerdomain.h +0 -0

@@ -26,7 +23,51 @@
 

?#include "voltage.h"

arch/arm/mach-omap2/powerdomain.h +3 -1

-/* Powerdomain basic power states */
+/***************************************************************
+ * External API
+ ***************************************************************/

arch/arm/mach-omap2/powerdomain.h +32 -0

+
+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);
+
+struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm);
+
+int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm);
+
+int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm);
+int pwrdm_read_func_pwrst(struct powerdomain *pwrdm);
+int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm);
+extern int omap_set_pwrdm_state(struct powerdomain *pwrdm,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? enum pwrdm_func_state func_pwrst);
+
+extern void omap242x_powerdomains_init(void);
+extern void omap243x_powerdomains_init(void);
+extern void omap3xxx_powerdomains_init(void);
+extern void omap44xx_powerdomains_init(void);
+
+
+/***************************************************************
+ * Internal code, only used in powerdomain*.[ch]
+ ***************************************************************/
+
+/* Powerdomain internal power states, internal use only */

?#define PWRDM_POWER_OFF ? ? ? ? ? ? ? ?0x0
?#define PWRDM_POWER_RET ? ? ? ? ? ? ? ?0x1
?#define PWRDM_POWER_INACTIVE ? 0x2

arch/arm/mach-omap2/powerdomain.h +0 -1

@@ -45,7 +86,6 @@
 ?#define PWRSTS_RET_ON ? ? ? ? ?(PWRSTS_RET | PWRSTS_ON)
 ?#define PWRSTS_OFF_RET_ON ? ? ?(PWRSTS_OFF_RET | PWRSTS_ON)
 
-

?/* Powerdomain flags */
?#define PWRDM_HAS_HDWR_SAR ? ? (1 << 0) /* hardware save-and-restore support */ ?#define PWRDM_HAS_MPU_QUIRK ? ?(1 << 1) /* MPU pwr domain has MEM bank 0 bits

arch/arm/mach-omap2/powerdomain.h +0 -0

@@ -74,9 +114,6 @@

?/* XXX A completely arbitrary number. What is reasonable here? */ ?#define PWRDM_TRANSITION_BAILOUT 100000

arch/arm/mach-omap2/powerdomain.h +0 -3

-struct clockdomain;
-struct powerdomain;
-

?/**
?* struct powerdomain - OMAP powerdomain
?* @name: Powerdomain name

arch/arm/mach-omap2/powerdomain.h +0 -0

@@ -130,6 +167,10 @@ struct powerdomain {
 

?/**
?* struct pwrdm_ops - Arch specific function implementations
+ * @pwrdm_func_to_pwrst: Convert the pd functional power state to
+ * ?the internal state
+ * @pwrdm_pwrst_to_func: Convert the pd internal power state to
+ * ?the functional state
?* @pwrdm_set_next_pwrst: Set the target power state for a pd ?* @pwrdm_read_next_pwrst: Read the target power state set for a pd ?* @pwrdm_read_pwrst: Read the current power state of a pd

arch/arm/mach-omap2/powerdomain.h +0 -0

@@ -150,6 +191,8 @@ struct powerdomain {

?* @pwrdm_wait_transition: Wait for a pd state transition to complete ?*/
?struct pwrdm_ops {
+ ? ? ? int ? ? (*pwrdm_func_to_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst);
+ ? ? ? int ? ? (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst); ? ? ? ?int ? ? (*pwrdm_set_next_pwrst)(struct powerdomain *pwrdm, u8 pwrst); ? ? ? ?int ? ? (*pwrdm_read_next_pwrst)(struct powerdomain *pwrdm); ? ? ? ?int ? ? (*pwrdm_read_pwrst)(struct powerdomain *pwrdm);

arch/arm/mach-omap2/powerdomain.h +3 -15

@@ -174,21 +217,11 @@ 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_next_pwrst(struct powerdomain *pwrdm, u8 pwrst);
 ?int pwrdm_read_next_pwrst(struct powerdomain *pwrdm);

@@ -196,6 +229,9 @@ 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 omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst);
+int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst);
+
 ?int pwrdm_set_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);

@@ -220,11 +256,6 @@ 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 omap44xx_powerdomains_init(void);
-
 ?extern struct pwrdm_ops omap2_pwrdm_operations;
 ?extern struct pwrdm_ops omap3_pwrdm_operations;
 ?extern struct pwrdm_ops omap4_pwrdm_operations;

arch/arm/mach-omap2/powerdomain2xxx_3xxx.c +0 -0

index 0f0a9f1..79c1293 100644
@@ -26,6 +26,7 @@
 
 

?/* Common functions across OMAP2 and OMAP3 */
+
?static int omap2_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst) ?{
? ? ? ?omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,

arch/arm/mach-omap2/powerdomain2xxx_3xxx.c +4 -0

@@ -210,6 +211,8 @@ static int omap3_pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm)
 ?}
 
 ?struct pwrdm_ops omap2_pwrdm_operations = {
+ ? ? ? .pwrdm_func_to_pwrst ? ?= omap2_pwrdm_func_to_pwrst,
+ ? ? ? .pwrdm_pwrst_to_func ? ?= omap2_pwrdm_pwrst_to_func,
 ? ? ? ?.pwrdm_set_next_pwrst ? = omap2_pwrdm_set_next_pwrst,
 ? ? ? ?.pwrdm_read_next_pwrst ?= omap2_pwrdm_read_next_pwrst,
 ? ? ? ?.pwrdm_read_pwrst ? ? ? = omap2_pwrdm_read_pwrst,

@@ -222,6 +225,8 @@ struct pwrdm_ops omap2_pwrdm_operations = {
 ?};
 
 ?struct pwrdm_ops omap3_pwrdm_operations = {
+ ? ? ? .pwrdm_func_to_pwrst ? ?= omap2_pwrdm_func_to_pwrst,
+ ? ? ? .pwrdm_pwrst_to_func ? ?= omap2_pwrdm_pwrst_to_func,
 ? ? ? ?.pwrdm_set_next_pwrst ? = omap2_pwrdm_set_next_pwrst,
 ? ? ? ?.pwrdm_read_next_pwrst ?= omap2_pwrdm_read_next_pwrst,
 ? ? ? ?.pwrdm_read_pwrst ? ? ? = omap2_pwrdm_read_pwrst,

arch/arm/mach-omap2/powerdomain44xx.c +2 -1

index 601325b..538b528 100644
@@ -209,6 +209,8 @@ static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm)
 ?}
 
 ?struct pwrdm_ops omap4_pwrdm_operations = {
+ ? ? ? .pwrdm_func_to_pwrst ? ?= omap2_pwrdm_func_to_pwrst,
+ ? ? ? .pwrdm_pwrst_to_func ? ?= omap2_pwrdm_pwrst_to_func,
 ? ? ? ?.pwrdm_set_next_pwrst ? = omap4_pwrdm_set_next_pwrst,
 ? ? ? ?.pwrdm_read_next_pwrst ?= omap4_pwrdm_read_next_pwrst,
 ? ? ? ?.pwrdm_read_pwrst ? ? ? = omap4_pwrdm_read_pwrst,
--

1.7.7.6

arch/arm/mach-omap2/powerdomain-common.c +0 -0

@@ -108,3 +108,74 @@ u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank)
        return 0;
 }
 

arch/arm/mach-omap2/powerdomain-common.c +11 -0

+               break;
+       case PWRDM_FUNC_PWRST_OFF:
+               ret = PWRDM_POWER_OFF;
+               break;
+       default:
+               ret = -EINVAL;
+       }
+
+       return ret;
+}
+

arch/arm/mach-omap2/powerdomain-common.c +5 -0

+{
+       int ret;
+
+       switch (pwrst) {
+       case PWRDM_POWER_ON:

arch/arm/mach-omap2/powerdomain-common.c +12 -0

+               ret = PWRDM_FUNC_PWRST_CSWR;
+               break;
+       case PWRDM_POWER_OFF:
+               ret = PWRDM_FUNC_PWRST_OFF;
+               break;
+       default:
+               ret = -EINVAL;
+       }
+
+       return ret;
+}
+

arch/arm/mach-omap2/powerdomain.c +1 -0

index 1641e72..e79b5ae 100644
@@ -466,6 +466,135 @@ int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm)
 }
 
 /**
+ * pwrdm_func_to_pwrst - get the internal (i.e. registers) value mapped

arch/arm/mach-omap2/powerdomain.c +8 -0

+
+       pr_debug("powerdomain: convert func %0x to pwrst %0x for %s\n",
+                func_pwrst, ret, pwrdm->name);
+
+       return ret;
+}
+
+/**

arch/arm/mach-omap2/powerdomain.c +23 -0

+
+       return ret;
+}
+
+/* Types of sleep_switch used in omap_set_pwrdm_state */
+#define FORCEWAKEUP_SWITCH     0
+#define LOWPOWERSTATE_SWITCH   1
+
+/**
+ * omap_set_pwrdm_state - program next powerdomain power state
+ * @pwrdm: struct powerdomain * to set
+ * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros
+ *
+ * This programs the pwrdm next functional state, sets the dependencies
+ * and waits for the state to be applied.
+ */
+int omap_set_pwrdm_state(struct powerdomain *pwrdm,
+                        enum pwrdm_func_state func_pwrst)
+{
+       u8 curr_pwrst, next_pwrst;
+       int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst);
+       int sleep_switch = -1, ret = 0, hwsup = 0;
+

arch/arm/mach-omap2/powerdomain.c +41 -0

+       next_pwrst = pwrdm_read_next_pwrst(pwrdm);
+       if (next_pwrst == pwrst)
+               goto out;
+
+       curr_pwrst = pwrdm_read_pwrst(pwrdm);
+       if (curr_pwrst < PWRDM_POWER_ON) {
+               if ((curr_pwrst > pwrst) &&
+                       (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
+                       sleep_switch = LOWPOWERSTATE_SWITCH;
+               } else {
+                       hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
+                       clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
+                       sleep_switch = FORCEWAKEUP_SWITCH;
+               }
+       }
+
+       ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
+       if (ret)
+               pr_err("%s: unable to set power state of powerdomain: %s\n",
+                      __func__, pwrdm->name);
+
+       switch (sleep_switch) {
+       case FORCEWAKEUP_SWITCH:
+               if (hwsup)
+                       clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+               else
+                       clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
+               break;
+       case LOWPOWERSTATE_SWITCH:
+               pwrdm_set_lowpwrstchange(pwrdm);
+               pwrdm_wait_transition(pwrdm);
+               pwrdm_state_switch(pwrdm);
+               break;
+       }
+
+out:
+       mutex_unlock(&pwrdm->lock);
+       return ret;
+}
+
+/**
  * pwrdm_set_next_pwrst - set next powerdomain power state
  * @pwrdm: struct powerdomain * to set
  * @pwrst: one of the PWRDM_POWER_* macros

@@ -522,6 +651,21 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 }

arch/arm/mach-omap2/powerdomain.c +7 -0

+{
+       int ret = pwrdm_read_next_pwrst(pwrdm);
+
+       return pwrdm_pwrst_to_func(pwrdm, ret);
+}
+
+/**
  * pwrdm_read_pwrst - get current powerdomain power state
  * @pwrdm: struct powerdomain * to get power state
  *

@@ -543,6 +687,21 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 }

arch/arm/mach-omap2/powerdomain.c +2 -0

+
+/**
  * pwrdm_read_prev_pwrst - get previous powerdomain power state
  * @pwrdm: struct powerdomain * to get previous power state
  *

@@ -564,6 +723,21 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
 }

arch/arm/mach-omap2/powerdomain.c +2 -0

+
+/**
  * pwrdm_set_logic_retst - set powerdomain logic power state upon retention
  * @pwrdm: struct powerdomain * to set
  * @pwrst: one of the PWRDM_POWER_* macros

arch/arm/mach-omap2/powerdomain.h +30 -4

index bab84fc..0404f9f 100644
@@ -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 at the earliest
- * opportunity.
  */
 
 #ifndef __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_H

@@ -26,7 +23,51 @@
 
 #include "voltage.h"
 
-/* Powerdomain basic power states */
+/***************************************************************
+ * External API
+ ***************************************************************/
+
+/* 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 */
+};
+
+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);
+
+struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm);
+
+int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm);
+
+int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm);
+int pwrdm_read_func_pwrst(struct powerdomain *pwrdm);

arch/arm/mach-omap2/powerdomain.h +9 -4

+/***************************************************************
+ * Internal code, only used in powerdomain*.[ch]
+ ***************************************************************/
+
+/* Powerdomain internal power states, internal use only */
 #define PWRDM_POWER_OFF                0x0
 #define PWRDM_POWER_RET                0x1
 #define PWRDM_POWER_INACTIVE   0x2

@@ -45,7 +86,6 @@
 #define PWRSTS_RET_ON          (PWRSTS_RET | PWRSTS_ON)
 #define PWRSTS_OFF_RET_ON      (PWRSTS_OFF_RET | PWRSTS_ON)
 
-
 /* Powerdomain flags */
 #define PWRDM_HAS_HDWR_SAR     (1 << 0) /* hardware save-and-restore support */
 #define PWRDM_HAS_MPU_QUIRK    (1 << 1) /* MPU pwr domain has MEM bank 0 bits

@@ -74,9 +114,6 @@
 /* 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

@@ -130,6 +167,10 @@ struct powerdomain {
 
 /**
  * struct pwrdm_ops - Arch specific function implementations
+ * @pwrdm_func_to_pwrst: Convert the pd functional power state to
+ *  the internal state
+ * @pwrdm_pwrst_to_func: Convert the pd internal power state to
+ *  the functional state
  * @pwrdm_set_next_pwrst: Set the target power state for a pd
  * @pwrdm_read_next_pwrst: Read the target power state set for a pd
  * @pwrdm_read_pwrst: Read the current power state of a pd

arch/arm/mach-omap2/powerdomain.h +3 -15

        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);

@@ -174,21 +217,11 @@ 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_next_pwrst(struct powerdomain *pwrdm, u8 pwrst);
 int pwrdm_read_next_pwrst(struct powerdomain *pwrdm);

@@ -196,6 +229,9 @@ 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 omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst);
+int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst);
+
 int pwrdm_set_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);

@@ -220,11 +256,6 @@ 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 omap44xx_powerdomains_init(void);
-
 extern struct pwrdm_ops omap2_pwrdm_operations;
 extern struct pwrdm_ops omap3_pwrdm_operations;
 extern struct pwrdm_ops omap4_pwrdm_operations;

arch/arm/mach-omap2/powerdomain2xxx_3xxx.c +5 -0

index 0f0a9f1..79c1293 100644
@@ -26,6 +26,7 @@
 
 
 /* Common functions across OMAP2 and OMAP3 */
+
 static int omap2_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
        omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,

@@ -210,6 +211,8 @@ static int omap3_pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm)
 }
 
 struct pwrdm_ops omap2_pwrdm_operations = {
+       .pwrdm_func_to_pwrst    = omap2_pwrdm_func_to_pwrst,
+       .pwrdm_pwrst_to_func    = omap2_pwrdm_pwrst_to_func,
        .pwrdm_set_next_pwrst   = omap2_pwrdm_set_next_pwrst,
        .pwrdm_read_next_pwrst  = omap2_pwrdm_read_next_pwrst,
        .pwrdm_read_pwrst       = omap2_pwrdm_read_pwrst,

@@ -222,6 +225,8 @@ struct pwrdm_ops omap2_pwrdm_operations = {
 };
 
 struct pwrdm_ops omap3_pwrdm_operations = {
+       .pwrdm_func_to_pwrst    = omap2_pwrdm_func_to_pwrst,
+       .pwrdm_pwrst_to_func    = omap2_pwrdm_pwrst_to_func,
        .pwrdm_set_next_pwrst   = omap2_pwrdm_set_next_pwrst,
        .pwrdm_read_next_pwrst  = omap2_pwrdm_read_next_pwrst,
        .pwrdm_read_pwrst       = omap2_pwrdm_read_pwrst,

arch/arm/mach-omap2/powerdomain44xx.c +0 -0

index 601325b..538b528 100644

On Thu, Jun 14, 2012 at 9:53 AM, Jean Pihet <[email protected]> wrote:
[..]

Quoted 1 line

+int pwrdm_read_func_pwrst(struct powerdomain *pwrdm)

should return enum pwrdm_func_state

Review

Quoted 5 lines

(quoted diff) +5 -0

+{
+       int ret = pwrdm_read_pwrst(pwrdm);
+
+       return pwrdm_pwrst_to_func(pwrdm, ret);
+}

my Crib about the above apis are lack of logic power state handling :( which forces code like cpuidle to use different apis for logic power state and force them to use these apis just for pwrst.

Review

Quoted 6 lines

(quoted diff) +6 -0

+extern void omap242x_powerdomains_init(void);
+extern void omap243x_powerdomains_init(void);
+extern void omap3xxx_powerdomains_init(void);
+extern void omap44xx_powerdomains_init(void);
+
+

We should split the following out to a separate header which is not to be used by anyone else other than powerdomain*.[ch]

Quoted 3 lines

*/
struct pwrdm_ops {
+ int (*pwrdm_func_to_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst);

should take in enum pwrdm_func_state

Quoted 1 line

+ int (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst);

should return enum pwrdm_func_state

otherwise, this is a nice concept

my Crib about the above apis are lack of logic power state handling:( which forces code like cpuidle to use different apis for logic power state and force them to use these apis just for pwrst.

Quoted 4 lines · Nishanth

my Crib about the above apis are lack of logic power state handling:( which forces code like cpuidle to use different apis for logic power state and force them to use these apis just for pwrst.

Have you looked at an alternate approach that was proposed here..
https://patchwork.kernel.org/patch/1160431/

Quoted 7 lines · Rajendra Nayak

Quoted 4 lines · Nishanth

my Crib about the above apis are lack of logic power state handling:( which forces code like cpuidle to use different apis for logic power state and force them to use these apis just for pwrst.

Quoted 3 lines

Have you looked at an alternate approach that was proposed here..
https://patchwork.kernel.org/patch/1160431/

Santosh pointed me to the thread offline. This is indeed a much better approach IMHO than having 3 conflicting options inside powerdomain framework.

Regards,
Nishanth Menon

Quoted 3 lines

Santosh pointed me to the thread offline. This is indeed a much better approach IMHO than having 3 conflicting options inside powerdomain framework.

After looking at the code and having sent my comments, I like it ... mainly because it is really similar to my proposal ;-p Can you elaborate more on 'having 3 conflicting options inside powerdomain framework'?

Here are the main differences in the implementation:
- the RFC code provides a _private header file, which forces the external users (cpuidle, pmXXXX.c etc.),

- the RFC code still uses the same function names while my code renames them to '*_func_*'. This makes the code look more complicated than it really is.

On Thu, Jun 14, 2012 at 9:53 AM, Jean Pihet <[email protected]> wrote:
[..]

Quoted 6 lines

arch/arm/mach-omap2/powerdomain-common.c +0 -0

@@ -108,3 +108,74 @@ u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank)
        return 0;
 }
 

arch/arm/mach-omap2/powerdomain-common.c u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank) · line 120

Quoted 10 lines

arch/arm/mach-omap2/powerdomain-common.c +10 -0

+/**
+ * omap2_pwrdm_func_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
+ * @func_pwrst: functional power state
+ *
+ * Returns the internal power state value for the power domain, or
+ * -EINVAL in case of invalid parameters passed in.
+ */
+int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)

Unless we handle logicpwst and pwst here, it makes it hard for non arch/arm/mach-omap2/powerdomain* files to use this properly.

Quoted 4 lines

arch/arm/mach-omap2/powerdomain-common.c +3 -0

+{
+       int ret;
+

arch/arm/mach-omap2/powerdomain-common.c u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank) · line 133

Quoted 10 lines

arch/arm/mach-omap2/powerdomain-common.c +10 -0

+       switch (func_pwrst) {
+       case PWRDM_FUNC_PWRST_ON:
+               ret = PWRDM_POWER_ON;
+               break;
+       case PWRDM_FUNC_PWRST_INACTIVE:
+               ret = PWRDM_POWER_INACTIVE;
+               break;
+       case PWRDM_FUNC_PWRST_CSWR:
+       case PWRDM_FUNC_PWRST_OSWR:
+               ret = PWRDM_POWER_RET;

logic power state?

Quoted 11 lines

arch/arm/mach-omap2/powerdomain-common.c +11 -0

+               break;
+       case PWRDM_FUNC_PWRST_OFF:
+               ret = PWRDM_POWER_OFF;
+               break;
+       default:
+               ret = -EINVAL;
+       }
+
+       return ret;
+}
+

arch/arm/mach-omap2/powerdomain-common.c u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank) · line 154

Quoted 10 lines

arch/arm/mach-omap2/powerdomain-common.c +10 -0

+/**
+ * omap2_pwrdm_pwrst_to_func - 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.
+ */
+int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst)

Same here as well..

Quoted 5 lines

arch/arm/mach-omap2/powerdomain-common.c +5 -0

+{
+       int ret;
+
+       switch (pwrst) {
+       case PWRDM_POWER_ON:

arch/arm/mach-omap2/powerdomain-common.c u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank) · line 169

Quoted 10 lines

arch/arm/mach-omap2/powerdomain-common.c +10 -0

+               ret = PWRDM_FUNC_PWRST_ON;
+               break;
+       case PWRDM_POWER_INACTIVE:
+               ret = PWRDM_FUNC_PWRST_INACTIVE;
+               break;
+       case PWRDM_POWER_RET:
+               /*
+                * XXX warning: return OSWR in case of pd in RET and
+                * logic in OFF
+                */

We need to be able to pass in logic pwst to be able to make that determination.

Quoted 19 lines

arch/arm/mach-omap2/powerdomain-common.c +12 -0

+               ret = PWRDM_FUNC_PWRST_CSWR;
+               break;
+       case PWRDM_POWER_OFF:
+               ret = PWRDM_FUNC_PWRST_OFF;
+               break;
+       default:
+               ret = -EINVAL;
+       }
+
+       return ret;
+}
+

arch/arm/mach-omap2/powerdomain.c +0 -0

index 1641e72..e79b5ae 100644
@@ -466,6 +466,135 @@ int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm)
 }
 

arch/arm/mach-omap2/powerdomain.c int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm) · line 479

Quoted 12 lines

arch/arm/mach-omap2/powerdomain.c +11 -0

 /**
+ * pwrdm_func_to_pwrst - get the internal (i.e. registers) value mapped
+ * to the functional state
+ * @pwrdm: struct powerdomain * to query
+ * @func_pwrst: functional power state
+ *
+ * Convert the functional power state to the platform specific power
+ * domain state value.
+ * Returns the internal power domain state value or -EINVAL in case
+ * of invalid parameters passed in.
+ */
+int pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)

should take enum pwrdm_func_state func_pwrst

arch/arm/mach-omap2/powerdomain.c int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm) · line 487

Quoted 9 lines

arch/arm/mach-omap2/powerdomain.c +8 -0

+{
+       int ret = func_pwrst;
+
+       if ((!pwrdm) || (func_pwrst >= PWRDM_MAX_FUNC_PWRSTS))
+               return -EINVAL;
+
+       if (arch_pwrdm && arch_pwrdm->pwrdm_func_to_pwrst)
+               ret = arch_pwrdm->pwrdm_func_to_pwrst(pwrdm, func_pwrst);

Should we not return an error if the function pointer is not available for the arch?
on platforms without func_pwrst pointer converter routing hooked on, we return func_pwrst back to caller, which expects pwrst!

Quoted 12 lines

arch/arm/mach-omap2/powerdomain.c +11 -0

+
+       pr_debug("powerdomain: convert func %0x to pwrst %0x for %s\n",
+                func_pwrst, ret, pwrdm->name);
+
+       return ret;
+}
+
+/**
+ * pwrdm_pwrst_to_func - get the functional (i.e. logical) value mapped
+ * to the internal state
+ * @pwrdm: struct powerdomain * to query

arch/arm/mach-omap2/powerdomain.c int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm) · line 505

Quoted 7 lines

arch/arm/mach-omap2/powerdomain.c +7 -0

+ * @pwrst: internal power state
+ *
+ * Convert the internal power state to the power domain functional value.
+ * Returns the functional power domain state value or -EINVAL in case
+ * of invalid parameters passed in.
+ */
+int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst)

should return enum pwrdm_func_state

arch/arm/mach-omap2/powerdomain.c int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm) · line 516

Quoted 12 lines

arch/arm/mach-omap2/powerdomain.c +11 -0

+{
+       int ret = pwrst;
+
+       if ((!pwrdm) || (pwrst >= PWRDM_MAX_PWRSTS))
+               return -EINVAL;
+
+       pr_debug("powerdomain: convert %0x pwrst to func for %s\n",
+                pwrst, pwrdm->name);
+
+       if (arch_pwrdm && arch_pwrdm->pwrdm_pwrst_to_func)
+               ret = arch_pwrdm->pwrdm_pwrst_to_func(pwrdm, pwrst);

same in reverse here.

Quoted 26 lines

arch/arm/mach-omap2/powerdomain.c +26 -0

+
+       return ret;
+}
+
+/* Types of sleep_switch used in omap_set_pwrdm_state */
+#define FORCEWAKEUP_SWITCH     0
+#define LOWPOWERSTATE_SWITCH   1
+
+/**
+ * omap_set_pwrdm_state - program next powerdomain power state
+ * @pwrdm: struct powerdomain * to set
+ * @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros
+ *
+ * This programs the pwrdm next functional state, sets the dependencies
+ * and waits for the state to be applied.
+ */
+int omap_set_pwrdm_state(struct powerdomain *pwrdm,
+                        enum pwrdm_func_state func_pwrst)
+{
+       u8 curr_pwrst, next_pwrst;
+       int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst);
+       int sleep_switch = -1, ret = 0, hwsup = 0;
+
+       if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0)) {
+               pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n",
+                        __func__, pwrdm, func_pwrst);

arch/arm/mach-omap2/powerdomain.c int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm) · line 549

Quoted 7 lines

arch/arm/mach-omap2/powerdomain.c +7 -0

+               return -EINVAL;
+       }
+
+       pr_debug("%s: set func_pwrst %0x to pwrdm %s\n",
+                __func__, func_pwrst, pwrdm->name);
+
+       mutex_lock(&pwrdm->lock);

This should be a spinlock (previous patch i believe) - hurts cpuidle otherwise.

arch/arm/mach-omap2/powerdomain.c int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm) · line 556

Quoted 8 lines

arch/arm/mach-omap2/powerdomain.c +7 -0

+
+       while (!(pwrdm->pwrsts & (1 << pwrst))) {
+               if (pwrst == PWRDM_POWER_OFF)
+                       goto out;
+               pwrst--;
+       }
+

if we like to handle a single API for OSWR and CSWR, then it needs to handle logic power state as well here.

Quoted 46 lines

arch/arm/mach-omap2/powerdomain.c +41 -0

+       next_pwrst = pwrdm_read_next_pwrst(pwrdm);
+       if (next_pwrst == pwrst)
+               goto out;
+
+       curr_pwrst = pwrdm_read_pwrst(pwrdm);
+       if (curr_pwrst < PWRDM_POWER_ON) {
+               if ((curr_pwrst > pwrst) &&
+                       (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
+                       sleep_switch = LOWPOWERSTATE_SWITCH;
+               } else {
+                       hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
+                       clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
+                       sleep_switch = FORCEWAKEUP_SWITCH;
+               }
+       }
+
+       ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
+       if (ret)
+               pr_err("%s: unable to set power state of powerdomain: %s\n",
+                      __func__, pwrdm->name);
+
+       switch (sleep_switch) {
+       case FORCEWAKEUP_SWITCH:
+               if (hwsup)
+                       clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+               else
+                       clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
+               break;
+       case LOWPOWERSTATE_SWITCH:
+               pwrdm_set_lowpwrstchange(pwrdm);
+               pwrdm_wait_transition(pwrdm);
+               pwrdm_state_switch(pwrdm);
+               break;
+       }
+
+out:
+       mutex_unlock(&pwrdm->lock);
+       return ret;
+}
+
+/**
  * pwrdm_set_next_pwrst - set next powerdomain power state
  * @pwrdm: struct powerdomain * to set
  * @pwrst: one of the PWRDM_POWER_* macros

@@ -522,6 +651,21 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 }

arch/arm/mach-omap2/powerdomain.c int pwrdm_read_next_pwrst(struct powerdomain *pwrdm) · line 661

Quoted 10 lines

arch/arm/mach-omap2/powerdomain.c +8 -0

 
 /**
+ * pwrdm_read_next_func_pwrst - get next powerdomain functional power state
+ * @pwrdm: struct powerdomain * to get power state
+ *
+ * Return the powerdomain @pwrdm's next functional power state.
+ * Returns -EINVAL if the powerdomain pointer is null or returns
+ * the next power state upon success.
+ */
+int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm)

should return enum pwrdm_func_state

Quoted 12 lines

arch/arm/mach-omap2/powerdomain.c +7 -0

+{
+       int ret = pwrdm_read_next_pwrst(pwrdm);
+
+       return pwrdm_pwrst_to_func(pwrdm, ret);
+}
+
+/**
  * pwrdm_read_pwrst - get current powerdomain power state
  * @pwrdm: struct powerdomain * to get power state
  *

@@ -543,6 +687,21 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 }

arch/arm/mach-omap2/powerdomain.c int pwrdm_read_pwrst(struct powerdomain *pwrdm) · line 697

Quoted 10 lines

arch/arm/mach-omap2/powerdomain.c +8 -0

 
 /**
+ * pwrdm_read_func_pwrst - get current powerdomain functional power state
+ * @pwrdm: struct powerdomain * to get power state
+ *
+ * Return the powerdomain @pwrdm's current functional power state.
+ * Returns -EINVAL if the powerdomain pointer is null or returns
+ * the current power state upon success.
+ */
+int pwrdm_read_func_pwrst(struct powerdomain *pwrdm)

should return enum pwrdm_func_state

arch/arm/mach-omap2/powerdomain.c int pwrdm_read_pwrst(struct powerdomain *pwrdm) · line 702

Quoted 5 lines

arch/arm/mach-omap2/powerdomain.c +5 -0

+{
+       int ret = pwrdm_read_pwrst(pwrdm);
+
+       return pwrdm_pwrst_to_func(pwrdm, ret);
+}

my Crib about the above apis are lack of logic power state handling :( which forces code like cpuidle to use different apis for logic power state and force them to use these apis just for pwrst.

Quoted 7 lines

arch/arm/mach-omap2/powerdomain.c +2 -0

+
+/**
  * pwrdm_read_prev_pwrst - get previous powerdomain power state
  * @pwrdm: struct powerdomain * to get previous power state
  *

@@ -564,6 +723,21 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
 }

arch/arm/mach-omap2/powerdomain.c int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm) · line 733

Quoted 10 lines

arch/arm/mach-omap2/powerdomain.c +8 -0

 
 /**
+ * pwrdm_read_prev_func_pwrst - get previous powerdomain functional power state
+ * @pwrdm: struct powerdomain * to get previous power state
+ *
+ * Return the powerdomain @pwrdm's previous functional power state.
+ * Returns -EINVAL if the powerdomain pointer is null or returns the
+ * previous power state upon success.
+ */
+int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm)

should return enum pwrdm_func_state

arch/arm/mach-omap2/powerdomain.c int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm) · line 738

Quoted 5 lines

arch/arm/mach-omap2/powerdomain.c +5 -0

+{
+       int ret = pwrdm_read_prev_pwrst(pwrdm);
+
+       return pwrdm_pwrst_to_func(pwrdm, ret);
+}

again here.

Quoted 54 lines

arch/arm/mach-omap2/powerdomain.c +2 -0

+
+/**
  * pwrdm_set_logic_retst - set powerdomain logic power state upon retention
  * @pwrdm: struct powerdomain * to set
  * @pwrst: one of the PWRDM_POWER_* macros

arch/arm/mach-omap2/powerdomain.h +30 -4

index bab84fc..0404f9f 100644
@@ -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 at the earliest
- * opportunity.
  */
 
 #ifndef __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_H

@@ -26,7 +23,51 @@
 
 #include "voltage.h"
 
-/* Powerdomain basic power states */
+/***************************************************************
+ * External API
+ ***************************************************************/
+
+/* 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 */
+};
+
+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);
+
+struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm);
+
+int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm);
+
+int pwrdm_read_prev_func_pwrst(struct powerdomain *pwrdm);
+int pwrdm_read_func_pwrst(struct powerdomain *pwrdm);

arch/arm/mach-omap2/powerdomain.h line 65

Quoted 10 lines

arch/arm/mach-omap2/powerdomain.h +10 -0

+int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm);
+extern int omap_set_pwrdm_state(struct powerdomain *pwrdm,
+                               enum pwrdm_func_state func_pwrst);
+
+extern void omap242x_powerdomains_init(void);
+extern void omap243x_powerdomains_init(void);
+extern void omap3xxx_powerdomains_init(void);
+extern void omap44xx_powerdomains_init(void);
+
+

We should split the following out to a separate header which is not to be used by anyone else other than powerdomain*.[ch]

Quoted 38 lines

arch/arm/mach-omap2/powerdomain.h +9 -4

+/***************************************************************
+ * Internal code, only used in powerdomain*.[ch]
+ ***************************************************************/
+
+/* Powerdomain internal power states, internal use only */
 #define PWRDM_POWER_OFF                0x0
 #define PWRDM_POWER_RET                0x1
 #define PWRDM_POWER_INACTIVE   0x2

@@ -45,7 +86,6 @@
 #define PWRSTS_RET_ON          (PWRSTS_RET | PWRSTS_ON)
 #define PWRSTS_OFF_RET_ON      (PWRSTS_OFF_RET | PWRSTS_ON)
 
-
 /* Powerdomain flags */
 #define PWRDM_HAS_HDWR_SAR     (1 << 0) /* hardware save-and-restore support */
 #define PWRDM_HAS_MPU_QUIRK    (1 << 1) /* MPU pwr domain has MEM bank 0 bits

@@ -74,9 +114,6 @@
 /* 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

@@ -130,6 +167,10 @@ struct powerdomain {
 
 /**
  * struct pwrdm_ops - Arch specific function implementations
+ * @pwrdm_func_to_pwrst: Convert the pd functional power state to
+ *  the internal state
+ * @pwrdm_pwrst_to_func: Convert the pd internal power state to
+ *  the functional state
  * @pwrdm_set_next_pwrst: Set the target power state for a pd
  * @pwrdm_read_next_pwrst: Read the target power state set for a pd
  * @pwrdm_read_pwrst: Read the current power state of a pd

arch/arm/mach-omap2/powerdomain.h struct powerdomain { · line 194

Quoted 4 lines

arch/arm/mach-omap2/powerdomain.h +1 -0

@@ -150,6 +191,8 @@ struct powerdomain {
  * @pwrdm_wait_transition: Wait for a pd state transition to complete
  */
 struct pwrdm_ops {
+       int     (*pwrdm_func_to_pwrst)(struct powerdomain *pwrdm, u8 func_pwrst);

should take in enum pwrdm_func_state

arch/arm/mach-omap2/powerdomain.h struct powerdomain { · line 195

Quoted 1 line

arch/arm/mach-omap2/powerdomain.h +1 -0

+       int     (*pwrdm_pwrst_to_func)(struct powerdomain *pwrdm, u8 func_pwrst);

should return enum pwrdm_func_state

Quoted 84 lines

arch/arm/mach-omap2/powerdomain.h +3 -15

        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);

@@ -174,21 +217,11 @@ 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_next_pwrst(struct powerdomain *pwrdm, u8 pwrst);
 int pwrdm_read_next_pwrst(struct powerdomain *pwrdm);

@@ -196,6 +229,9 @@ 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 omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst);
+int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst);
+
 int pwrdm_set_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);

@@ -220,11 +256,6 @@ 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 omap44xx_powerdomains_init(void);
-
 extern struct pwrdm_ops omap2_pwrdm_operations;
 extern struct pwrdm_ops omap3_pwrdm_operations;
 extern struct pwrdm_ops omap4_pwrdm_operations;

arch/arm/mach-omap2/powerdomain2xxx_3xxx.c +5 -0

index 0f0a9f1..79c1293 100644
@@ -26,6 +26,7 @@
 
 
 /* Common functions across OMAP2 and OMAP3 */
+
 static int omap2_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
        omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,

@@ -210,6 +211,8 @@ static int omap3_pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm)
 }
 
 struct pwrdm_ops omap2_pwrdm_operations = {
+       .pwrdm_func_to_pwrst    = omap2_pwrdm_func_to_pwrst,
+       .pwrdm_pwrst_to_func    = omap2_pwrdm_pwrst_to_func,
        .pwrdm_set_next_pwrst   = omap2_pwrdm_set_next_pwrst,
        .pwrdm_read_next_pwrst  = omap2_pwrdm_read_next_pwrst,
        .pwrdm_read_pwrst       = omap2_pwrdm_read_pwrst,

@@ -222,6 +225,8 @@ struct pwrdm_ops omap2_pwrdm_operations = {
 };
 
 struct pwrdm_ops omap3_pwrdm_operations = {
+       .pwrdm_func_to_pwrst    = omap2_pwrdm_func_to_pwrst,
+       .pwrdm_pwrst_to_func    = omap2_pwrdm_pwrst_to_func,
        .pwrdm_set_next_pwrst   = omap2_pwrdm_set_next_pwrst,
        .pwrdm_read_next_pwrst  = omap2_pwrdm_read_next_pwrst,
        .pwrdm_read_pwrst       = omap2_pwrdm_read_pwrst,

arch/arm/mach-omap2/powerdomain44xx.c +0 -0

index 601325b..538b528 100644
@@ -209,6 +209,8 @@ static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm)
 }
 

arch/arm/mach-omap2/powerdomain44xx.c static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm) · line 216

Quoted 6 lines

arch/arm/mach-omap2/powerdomain44xx.c +2 -0

 struct pwrdm_ops omap4_pwrdm_operations = {
+       .pwrdm_func_to_pwrst    = omap2_pwrdm_func_to_pwrst,
+       .pwrdm_pwrst_to_func    = omap2_pwrdm_pwrst_to_func,
        .pwrdm_set_next_pwrst   = omap4_pwrdm_set_next_pwrst,
        .pwrdm_read_next_pwrst  = omap4_pwrdm_read_next_pwrst,
        .pwrdm_read_pwrst       = omap4_pwrdm_read_pwrst,

otherwise, this is a nice concept

--
Regards,
Nishanth Menon

Quoted 3 lines

my Crib about the above apis are lack of logic power state handling :( which forces code like cpuidle to use different apis for logic power state and force them to use these apis just for pwrst.

Please look at the rest of the series.

Note: the patch is in RFC state because the state machine for setting the next power domain states needs more discussion. Validated on OMAP3&4 with cpuidle and suspend/resume, though.

Power domains have varied capabilities. When attempting a low power state such as OFF/RET, a specific min requested state may not be supported on the power domain. This is because a combination of system power states where the parent PD's state is not in line with expectation can result in system instabilities.

This patch provides a function that returns the achievable functional power state for a power domain and its use by omap_set_pwrdm_state. The achievable 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.

Inspired from Tero's work on OMAP4 device OFF support and generalized to the functional power states.

Signed-off-by Jean Pihet <[email protected]>

Cc Tero Kristo <[email protected]>

---
?arch/arm/mach-omap2/powerdomain.c | ?156 +++++++++++++++++++++++++++++++------ ?arch/arm/mach-omap2/powerdomain.h | ? ?1 +
?2 files changed, 134 insertions(+), 23 deletions(-)

arch/arm/mach-omap2/powerdomain.c +84 -0

index ce2685a..f94174b 100644
@@ -554,6 +554,108 @@ int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst, u8 logic)
 ? ? ? ?return ret;
 ?}
 
+/**
+ * pwrdm_get_achievable_func_pwrst() - Provide achievable functional state
+ * @pwrdm: struct powerdomain * to set
+ * @func_pwrst: minimum functional state we would like to hit
+ * (one of the PWRDM_FUNC_* macros)
+ *
+ * Power domains have varied capabilities. When attempting a low power
+ * state such as OFF/RET, a specific min requested state may not be
+ * supported on the power domain. This is because a combination
+ * of system power states where the parent PD's state is not in line
+ * with expectation can result in system instabilities.
+ *
+ * The achievable 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.
+ *
+ * Returns the achievable functional power state, or -EINVAL in case of
+ * invalid parameters.
+ */
+int pwrdm_get_achievable_func_pwrst(struct powerdomain *pwrdm, u8 func_pwrst)
+{
+ ? ? ? int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst);
+ ? ? ? int logic = pwrdm_func_to_logic_pwrst(pwrdm, func_pwrst);
+ ? ? ? int new_func_pwrst, new_pwrst, new_logic;
+ ? ? ? int found;
+
+ ? ? ? if (!pwrdm || IS_ERR(pwrdm)) {
+ ? ? ? ? ? ? ? pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n",
+ ? ? ? ? ? ? ? ? ? ? ? ?__func__, pwrdm, func_pwrst);
+ ? ? ? ? ? ? ? return -EINVAL;
+ ? ? ? }
+
+ ? ? ? if ((pwrst < 0) || (logic < 0)) {
+ ? ? ? ? ? ? ? pr_debug("%s: invalid params for pwrdm %s, func_pwrst=%0x\n",
+ ? ? ? ? ? ? ? ? ? ? ? ?__func__, pwrdm->name, func_pwrst);
+ ? ? ? ? ? ? ? return PWRDM_FUNC_PWRST_ON;
+ ? ? ? }
+
+ ? ? ? /*
+ ? ? ? ?* Power state: if the requested state is not supported,
+ ? ? ? ?* try the lower power states.
+ ? ? ? ?*/
+ ? ? ? found = 1;
+ ? ? ? new_pwrst = pwrst;
+ ? ? ? while (!(pwrdm->pwrsts & (1 << new_pwrst))) {
+ ? ? ? ? ? ? ? if (new_pwrst == PWRDM_POWER_OFF) {
+ ? ? ? ? ? ? ? ? ? ? ? found = 0;
+ ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? new_pwrst--;
+ ? ? ? }
+
+ ? ? ? /*
+ ? ? ? ?* If no lower power state found, fallback to the higher
+ ? ? ? ?* power states.
+ ? ? ? ?* PWRDM_POWER_ON is always valid.
+ ? ? ? ?*/
+ ? ? ? if (!found) {
+ ? ? ? ? ? ? ? new_pwrst = pwrst;
+ ? ? ? ? ? ? ? while (!(pwrdm->pwrsts & (1 << new_pwrst))) {
+ ? ? ? ? ? ? ? ? ? ? ? if (new_pwrst == PWRDM_POWER_ON)
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? ? ? ? ? new_pwrst++;
+ ? ? ? ? ? ? ? }
+ ? ? ? }
+
+ ? ? ? /*
+ ? ? ? ?* Logic RET state: if the requested state is not supported,
+ ? ? ? ?* try the lower logic states.
+ ? ? ? ?*/
+ ? ? ? found = 1;
+ ? ? ? new_logic = logic;
+ ? ? ? while (!(pwrdm->pwrsts_logic_ret & (1 << new_logic))) {
+ ? ? ? ? ? ? ? if (new_logic == PWRDM_LOGIC_MEM_PWRST_OFF) {
+ ? ? ? ? ? ? ? ? ? ? ? found = 0;
+ ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? new_logic--;
+ ? ? ? }
+
+ ? ? ? /*
+ ? ? ? ?* If no lower logic state found, fallback to the higher
+ ? ? ? ?* logic states.
+ ? ? ? ?* PWRDM_LOGIC_MEM_PWRST_RET is always valid.

arch/arm/mach-omap2/powerdomain.c +8 -0

+ ? ? ? new_func_pwrst = pwrdm_pwrst_to_func(pwrdm, new_pwrst, new_logic);
+
+ ? ? ? pr_debug("%s(%s, func_pwrst=%0x) returns %0x\n", __func__,
+ ? ? ? ? ? ? ? ?pwrdm->name, func_pwrst, new_func_pwrst);
+
+ ? ? ? return new_func_pwrst;
+}
+

?/* Types of sleep_switch used in omap_set_pwrdm_state */ ?#define FORCEWAKEUP_SWITCH ? ? 0
?#define LOWPOWERSTATE_SWITCH ? 1

arch/arm/mach-omap2/powerdomain.c +0 -0

@@ -563,36 +665,32 @@ int pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst, u8 logic)

?* @pwrdm: struct powerdomain * to set
?* @func_pwrst: power domain functional state, one of the PWRDM_FUNC_* macros ?*
- * This programs the pwrdm next functional state, sets the dependencies
- * and waits for the state to be applied.
+ * This looks for the more suited (or achievable) next functional power
+ * state, programs it, sets the dependencies and waits for the state to
+ * be applied to the power domain.
?*/
?int omap_set_pwrdm_state(struct powerdomain *pwrdm,
? ? ? ? ? ? ? ? ? ? ? ? enum pwrdm_func_state func_pwrst)
?{
- ? ? ? u8 curr_pwrst, next_pwrst;
- ? ? ? int pwrst = pwrdm_func_to_pwrst(pwrdm, func_pwrst);
- ? ? ? int logic = pwrdm_func_to_logic_pwrst(pwrdm, func_pwrst); ? ? ? ?int sleep_switch = -1, ret = 0, hwsup = 0;
+ ? ? ? int new_func_pwrst, next_func_pwrst, pwrst, logic;
+ ? ? ? u8 curr_pwrst;

- ? ? ? if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) {
- ? ? ? ? ? ? ? pr_debug("%s: invalid params: pwrdm=%p, func_pwrst=%0x\n",
- ? ? ? ? ? ? ? ? ? ? ? ?__func__, pwrdm, func_pwrst);
+ ? ? ? if (!pwrdm || IS_ERR(pwrdm)) {
+ ? ? ? ? ? ? ? pr_debug("%s: invalid params: pwrdm=%p\n", __func__, pwrdm); ? ? ? ? ? ? ? ?return -EINVAL;
? ? ? ?}

- ? ? ? pr_debug("%s: set func_pwrst %0x to pwrdm %s\n",
- ? ? ? ? ? ? ? ?__func__, func_pwrst, pwrdm->name);
+ ? ? ? pr_debug("%s(%s, func_pwrst=%0x)\n", __func__, pwrdm->name, func_pwrst);

? ? ? ?mutex_lock(&pwrdm->lock);

- ? ? ? while (!(pwrdm->pwrsts & (1 << pwrst))) {
- ? ? ? ? ? ? ? if (pwrst == PWRDM_POWER_OFF)
- ? ? ? ? ? ? ? ? ? ? ? goto out;
- ? ? ? ? ? ? ? pwrst--;
- ? ? ? }
+ ? ? ? new_func_pwrst = pwrdm_get_achievable_func_pwrst(pwrdm, func_pwrst);
+ ? ? ? pwrst = pwrdm_func_to_pwrst(pwrdm, new_func_pwrst);
+ ? ? ? logic = pwrdm_func_to_logic_pwrst(pwrdm, new_func_pwrst);

- ? ? ? next_pwrst = pwrdm_read_next_pwrst(pwrdm);
- ? ? ? if (next_pwrst == pwrst)
+ ? ? ? next_func_pwrst = pwrdm_read_next_func_pwrst(pwrdm);
+ ? ? ? if (new_func_pwrst == next_func_pwrst)
? ? ? ? ? ? ? ?goto out;

? ? ? ?curr_pwrst = pwrdm_read_pwrst(pwrdm);

arch/arm/mach-omap2/powerdomain.c +18 -6

@@ -607,13 +705,25 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm,
 ? ? ? ? ? ? ? ?}
 ? ? ? ?}
 
- ? ? ? if (logic != pwrdm_read_logic_retst(pwrdm))
- ? ? ? ? ? ? ? pwrdm_set_logic_retst(pwrdm, logic);
+ ? ? ? pr_debug("%s: set func_pwrst %0x (%0x,%0x) to pwrdm %s\n",
+ ? ? ? ? ? ? ? ?__func__, new_func_pwrst, pwrst, logic, pwrdm->name);
+
+ ? ? ? /* Trace the pwrdm desired target state */
+ ? ? ? trace_power_domain_target(pwrdm->name, new_func_pwrst,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? smp_processor_id());
+
+ ? ? ? /* Program next power state */
+ ? ? ? if (pwrst != pwrdm_func_to_pwrst(pwrdm, next_func_pwrst)) {
+ ? ? ? ? ? ? ? ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
+ ? ? ? ? ? ? ? if (ret)
+ ? ? ? ? ? ? ? ? ? ? ? pr_err("%s: unable to set power state of powerdomain: %s\n",
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?__func__, pwrdm->name);
+ ? ? ? }
 
- ? ? ? ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
- ? ? ? if (ret)
- ? ? ? ? ? ? ? pr_err("%s: unable to set power state of powerdomain: %s\n",
- ? ? ? ? ? ? ? ? ? ? ?__func__, pwrdm->name);
+ ? ? ? /* Program RET logic state */
+ ? ? ? if ((pwrst == PWRDM_POWER_RET) &&
+ ? ? ? ? ? (logic != pwrdm_func_to_logic_pwrst(pwrdm, next_func_pwrst)))
+ ? ? ? ? ? ? ? pwrdm_set_logic_retst(pwrdm, logic);
 
 ? ? ? ?switch (sleep_switch) {

? ? ? ?case FORCEWAKEUP_SWITCH:

arch/arm/mach-omap2/powerdomain.h +1 -1

index d08c25d..9dd68ab 100644
@@ -247,6 +247,7 @@ int pwrdm_clear_all_prev_pwrst(struct powerdomain *pwrdm);
 ?int omap2_pwrdm_func_to_pwrst(struct powerdomain *pwrdm, u8 func_pwrst);
 ?int omap2_pwrdm_func_to_logic_pwrst(struct powerdomain *pwrdm, u8 func_pwrst);
 ?int omap2_pwrdm_pwrst_to_func(struct powerdomain *pwrdm, u8 pwrst, u8 logic);
+int pwrdm_get_achievable_func_pwrst(struct powerdomain *pwrdm, u8 func_pwrst);
 
 ?int pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst);
 ?int pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank, u8 pwrst);
--

1.7.7.6

"
1. The while loops need optimization. - This can definitely be simplified and made non-risky; this also needs some more error handling

2. About the power domains state machine: you have only one power state to move in and out of: it is called ON. If you do ON->CSWR->ON etc. attempting to program ON->CSWR->OSWR is NOT supported in OMAP and is an invitation for production team to sit and debug for a while before finding the use of invalid power states
"

Point 2 is a good point. The solution would be to implement a state machine in this function and/or omap_set_pwrdm_state, possibly using a platform specific handling function.