diff mbox

[7/7] ARM: OMAP2+: PM: reorganize the powerdomain API in public and private parts

Message ID 1347443732-7411-8-git-send-email-j-pihet@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jean Pihet Sept. 12, 2012, 9:55 a.m. UTC 
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(-)

Comments

Kevin Hilman Sept. 13, 2012, 12:11 a.m. UTC | #1 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jean Pihet Sept. 13, 2012, 7:29 a.m. UTC | #2 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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