Re: [PATCH 05/12] arm: omap3: am35x: Add PWROFF feature
On Fri, Apr 27, 2012 at 02:07:13PM -0700, Kevin Hilman wrote: Mark A. Greer mgr...@animalcreek.com writes: On Wed, Apr 11, 2012 at 03:46:20PM -0700, Kevin Hilman wrote: Hi Mark, Hi Kevin. Mark A. Greer mgr...@animalcreek.com writes: From: Mark A. Greer mgr...@animalcreek.com Typical OMAP3 SoCs have four power domain states: ON, INACTIVE, RETENTION, and OFF. The am35x family of SoCs has only two states: ON and INACTIVE. To distinguish which set of states the current device has, add the 'OMAP3_HAS_PWROFF' feature. When that feature/bit is set, the device supports the RETENTION and OFF states; otherwise, it doesn't. Signed-off-by: Mark A. Greer mgr...@animalcreek.com Paul has mentioned this already, but the same applies here: We shouldn't be using SoC-global feature flag for this. We already have per-pwrdm flags that indicate what states a given powerdomain suports (see .pwrsts field.) Wherever we have blind writes to next powerstates that assume support for RET/OFF is present, those should probably use a helper function from the powerdomain code that checks if that state is even supported. How about something like the patch below? Note: its not well tested; just RFC. Yes, your proposed patch looks right to me. I guess it's up to Paul Jean to see if they'd rather see this build on top of the Jean's functional power state work, or take this as a standalone fix. Kevin FYI, I just submitted the patch, http://www.spinics.net/lists/linux-omap/msg69066.html Mark -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] arm: omap3: am35x: Add PWROFF feature
Mark A. Greer mgr...@animalcreek.com writes:
On Wed, Apr 11, 2012 at 03:46:20PM -0700, Kevin Hilman wrote:
Hi Mark,
Hi Kevin.
Mark A. Greer mgr...@animalcreek.com writes:
From: Mark A. Greer mgr...@animalcreek.com
Typical OMAP3 SoCs have four power domain states: ON,
INACTIVE, RETENTION, and OFF. The am35x family of SoCs
has only two states: ON and INACTIVE. To distinguish which
set of states the current device has, add the 'OMAP3_HAS_PWROFF'
feature. When that feature/bit is set, the device supports the
RETENTION and OFF states; otherwise, it doesn't.
Signed-off-by: Mark A. Greer mgr...@animalcreek.com
Paul has mentioned this already, but the same applies here: We shouldn't
be using SoC-global feature flag for this. We already have per-pwrdm
flags that indicate what states a given powerdomain suports (see .pwrsts
field.)
Wherever we have blind writes to next powerstates that assume support
for RET/OFF is present, those should probably use a helper function from
the powerdomain code that checks if that state is even supported.
How about something like the patch below?
Note: its not well tested; just RFC.
Yes, your proposed patch looks right to me.
I guess it's up to Paul Jean to see if they'd rather see this build on
top of the Jean's functional power state work, or take this as a
standalone fix.
Kevin
Jean's work on functional powerstates will probably help here if you
really need to support INACTIVE. However, Paul may be right in that you
might just start with supporing ON only, and validate that module-level
wakups acutally work.
Yes, I think INACTIVE is a red herring so I'm going to stick with k.o
master branch for now (IOW, not base on Jean's patches). If you still
want me to base on his patches, just let me know.
Mark
--
From 32c54adb15c76396aeec809d38de4dde936b1e66 Mon Sep 17 00:00:00 2001
From: Mark A. Greer mgr...@animalcreek.com
Date: Mon, 23 Apr 2012 17:48:06 -0700
Subject: [PATCH] arm: omap: Use only valid power domain states
Some parts of the OMAP code assume that all power
domains support certain states (e.g., RET OFF).
This isn't always true (e.g., the am35x family of
SoC's) so those assumptions need to be removed.
Remove those assumptions by looking up the deepest
state that a power domain can be in whereever its
being blindly set. The 'max()' of the deepest
state and what the code formerly wanted to set it
to, is the correct state. If the code formerly
wanted to set it to PWRDM_POWER_OFF, then the
deepest possible state will be used (i.e., no
need to perform the 'max()').
The code still assumes that ON is a valid state.
Signed-off-by: Mark A. Greer mgr...@animalcreek.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 14 ++
arch/arm/mach-omap2/pm34xx.c | 15 +--
arch/arm/mach-omap2/powerdomain.c | 25 +
arch/arm/mach-omap2/powerdomain.h |2 ++
4 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 5358664..60aa0c3 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -175,19 +175,25 @@ static int next_valid_state(struct cpuidle_device *dev,
struct cpuidle_state_usage *curr_usage = dev-states_usage[index];
struct cpuidle_state *curr = drv-states[index];
struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr_usage);
- u32 mpu_deepest_state = PWRDM_POWER_RET;
- u32 core_deepest_state = PWRDM_POWER_RET;
+ u32 mpu_deepest_state, mpu_deepest_possible;
+ u32 core_deepest_state, core_deepest_possible;
int next_index = -1;
+ mpu_deepest_possible = pwrdm_get_deepest_state(mpu_pd);
+ mpu_deepest_state = max(mpu_deepest_possible, (u32)PWRDM_POWER_RET);
+
+ core_deepest_possible = pwrdm_get_deepest_state(core_pd);
+ core_deepest_state = max(core_deepest_possible, (u32)PWRDM_POWER_RET);
+
if (enable_off_mode) {
- mpu_deepest_state = PWRDM_POWER_OFF;
+ mpu_deepest_state = mpu_deepest_possible;
/*
* 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 = core_deepest_possible;
}
/* Check if current state is valid */
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index ec92676..7737bfb 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -614,12 +614,11 @@ void omap3_pm_off_mode_enable(int enable)
struct power_state *pwrst;
u32 state;
- if (enable)
- state = PWRDM_POWER_OFF;
Re: [PATCH 05/12] arm: omap3: am35x: Add PWROFF feature
On Wed, Apr 11, 2012 at 03:46:20PM -0700, Kevin Hilman wrote:
Hi Mark,
Hi Kevin.
Mark A. Greer mgr...@animalcreek.com writes:
From: Mark A. Greer mgr...@animalcreek.com
Typical OMAP3 SoCs have four power domain states: ON,
INACTIVE, RETENTION, and OFF. The am35x family of SoCs
has only two states: ON and INACTIVE. To distinguish which
set of states the current device has, add the 'OMAP3_HAS_PWROFF'
feature. When that feature/bit is set, the device supports the
RETENTION and OFF states; otherwise, it doesn't.
Signed-off-by: Mark A. Greer mgr...@animalcreek.com
Paul has mentioned this already, but the same applies here: We shouldn't
be using SoC-global feature flag for this. We already have per-pwrdm
flags that indicate what states a given powerdomain suports (see .pwrsts
field.)
Wherever we have blind writes to next powerstates that assume support
for RET/OFF is present, those should probably use a helper function from
the powerdomain code that checks if that state is even supported.
How about something like the patch below?
Note: its not well tested; just RFC.
Jean's work on functional powerstates will probably help here if you
really need to support INACTIVE. However, Paul may be right in that you
might just start with supporing ON only, and validate that module-level
wakups acutally work.
Yes, I think INACTIVE is a red herring so I'm going to stick with k.o
master branch for now (IOW, not base on Jean's patches). If you still
want me to base on his patches, just let me know.
Mark
--
From 32c54adb15c76396aeec809d38de4dde936b1e66 Mon Sep 17 00:00:00 2001
From: Mark A. Greer mgr...@animalcreek.com
Date: Mon, 23 Apr 2012 17:48:06 -0700
Subject: [PATCH] arm: omap: Use only valid power domain states
Some parts of the OMAP code assume that all power
domains support certain states (e.g., RET OFF).
This isn't always true (e.g., the am35x family of
SoC's) so those assumptions need to be removed.
Remove those assumptions by looking up the deepest
state that a power domain can be in whereever its
being blindly set. The 'max()' of the deepest
state and what the code formerly wanted to set it
to, is the correct state. If the code formerly
wanted to set it to PWRDM_POWER_OFF, then the
deepest possible state will be used (i.e., no
need to perform the 'max()').
The code still assumes that ON is a valid state.
Signed-off-by: Mark A. Greer mgr...@animalcreek.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 14 ++
arch/arm/mach-omap2/pm34xx.c | 15 +--
arch/arm/mach-omap2/powerdomain.c | 25 +
arch/arm/mach-omap2/powerdomain.h |2 ++
4 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 5358664..60aa0c3 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -175,19 +175,25 @@ static int next_valid_state(struct cpuidle_device *dev,
struct cpuidle_state_usage *curr_usage = dev-states_usage[index];
struct cpuidle_state *curr = drv-states[index];
struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr_usage);
- u32 mpu_deepest_state = PWRDM_POWER_RET;
- u32 core_deepest_state = PWRDM_POWER_RET;
+ u32 mpu_deepest_state, mpu_deepest_possible;
+ u32 core_deepest_state, core_deepest_possible;
int next_index = -1;
+ mpu_deepest_possible = pwrdm_get_deepest_state(mpu_pd);
+ mpu_deepest_state = max(mpu_deepest_possible, (u32)PWRDM_POWER_RET);
+
+ core_deepest_possible = pwrdm_get_deepest_state(core_pd);
+ core_deepest_state = max(core_deepest_possible, (u32)PWRDM_POWER_RET);
+
if (enable_off_mode) {
- mpu_deepest_state = PWRDM_POWER_OFF;
+ mpu_deepest_state = mpu_deepest_possible;
/*
* 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 = core_deepest_possible;
}
/* Check if current state is valid */
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index ec92676..7737bfb 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -614,12 +614,11 @@ void omap3_pm_off_mode_enable(int enable)
struct power_state *pwrst;
u32 state;
- if (enable)
- state = PWRDM_POWER_OFF;
- else
- state = PWRDM_POWER_RET;
-
list_for_each_entry(pwrst, pwrst_list, node) {
+ state = pwrdm_get_deepest_state(pwrst-pwrdm);
+ if (!enable)
+ state = max(state, (u32)PWRDM_POWER_RET);
+
[PATCH 05/12] arm: omap3: am35x: Add PWROFF feature
From: Mark A. Greer mgr...@animalcreek.com Typical OMAP3 SoCs have four power domain states: ON, INACTIVE, RETENTION, and OFF. The am35x family of SoCs has only two states: ON and INACTIVE. To distinguish which set of states the current device has, add the 'OMAP3_HAS_PWROFF' feature. When that feature/bit is set, the device supports the RETENTION and OFF states; otherwise, it doesn't. Signed-off-by: Mark A. Greer mgr...@animalcreek.com --- arch/arm/mach-omap2/id.c |7 +-- arch/arm/plat-omap/include/plat/cpu.h |8 +--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c index 9736049..b6508e5 100644 --- a/arch/arm/mach-omap2/id.c +++ b/arch/arm/mach-omap2/id.c @@ -245,7 +245,7 @@ void __init omap3xxx_check_features(void) omap_rev() == OMAP3430_REV_ES3_1_2) omap_features |= OMAP3_HAS_IO_CHAIN_CTRL; - omap_features |= OMAP3_HAS_SDRC; + omap_features |= OMAP3_HAS_SDRC | OMAP3_HAS_PWROFF; /* * am35x fixups: @@ -254,9 +254,12 @@ void __init omap3xxx_check_features(void) * OMAP3_CHECK_FEATURE() will interpret some of those zeroes to * mean that a feature is present even though it isn't so clear * the incorrectly set feature bits. +* - Indicate that am35x SoCs don't support the PWRDM_POWER_RET +* and PWRDM_POWER_OFF states by clearing OMAP3_HAS_PWROFF. */ if (cpu_is_omap3505() || cpu_is_omap3517()) - omap_features = ~(OMAP3_HAS_IVA | OMAP3_HAS_ISP); + omap_features = ~(OMAP3_HAS_IVA | OMAP3_HAS_ISP | + OMAP3_HAS_PWROFF); /* * TODO: Get additional info (where applicable) diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h index dc6a86b..c3f1a42 100644 --- a/arch/arm/plat-omap/include/plat/cpu.h +++ b/arch/arm/plat-omap/include/plat/cpu.h @@ -477,9 +477,10 @@ extern u32 omap_features; #define OMAP3_HAS_IO_WAKEUPBIT(6) #define OMAP3_HAS_SDRC BIT(7) #define OMAP3_HAS_IO_CHAIN_CTRLBIT(8) -#define OMAP4_HAS_MPU_1GHZ BIT(9) -#define OMAP4_HAS_MPU_1_2GHZ BIT(10) -#define OMAP4_HAS_MPU_1_5GHZ BIT(11) +#define OMAP3_HAS_PWROFF BIT(9) +#define OMAP4_HAS_MPU_1GHZ BIT(10) +#define OMAP4_HAS_MPU_1_2GHZ BIT(11) +#define OMAP4_HAS_MPU_1_5GHZ BIT(12) #define OMAP3_HAS_FEATURE(feat,flag) \ @@ -497,6 +498,7 @@ OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) OMAP3_HAS_FEATURE(sdrc, SDRC) OMAP3_HAS_FEATURE(io_chain_ctrl, IO_CHAIN_CTRL) +OMAP3_HAS_FEATURE(pwroff, PWROFF) /* * Runtime detection of OMAP4 features -- 1.7.9.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] arm: omap3: am35x: Add PWROFF feature
Hi Mark, Mark A. Greer mgr...@animalcreek.com writes: From: Mark A. Greer mgr...@animalcreek.com Typical OMAP3 SoCs have four power domain states: ON, INACTIVE, RETENTION, and OFF. The am35x family of SoCs has only two states: ON and INACTIVE. To distinguish which set of states the current device has, add the 'OMAP3_HAS_PWROFF' feature. When that feature/bit is set, the device supports the RETENTION and OFF states; otherwise, it doesn't. Signed-off-by: Mark A. Greer mgr...@animalcreek.com Paul has mentioned this already, but the same applies here: We shouldn't be using SoC-global feature flag for this. We already have per-pwrdm flags that indicate what states a given powerdomain suports (see .pwrsts field.) Wherever we have blind writes to next powerstates that assume support for RET/OFF is present, those should probably use a helper function from the powerdomain code that checks if that state is even supported. Jean's work on functional powerstates will probably help here if you really need to support INACTIVE. However, Paul may be right in that you might just start with supporing ON only, and validate that module-level wakups acutally work. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] arm: omap3: am35x: Add PWROFF feature
On Wed, Apr 11, 2012 at 03:46:20PM -0700, Kevin Hilman wrote: Hi Mark, Mark A. Greer mgr...@animalcreek.com writes: From: Mark A. Greer mgr...@animalcreek.com Typical OMAP3 SoCs have four power domain states: ON, INACTIVE, RETENTION, and OFF. The am35x family of SoCs has only two states: ON and INACTIVE. To distinguish which set of states the current device has, add the 'OMAP3_HAS_PWROFF' feature. When that feature/bit is set, the device supports the RETENTION and OFF states; otherwise, it doesn't. Signed-off-by: Mark A. Greer mgr...@animalcreek.com Paul has mentioned this already, but the same applies here: We shouldn't be using SoC-global feature flag for this. We already have per-pwrdm flags that indicate what states a given powerdomain suports (see .pwrsts field.) Wherever we have blind writes to next powerstates that assume support for RET/OFF is present, those should probably use a helper function from the powerdomain code that checks if that state is even supported. Okay, thanks Kevin. Jean's work on functional powerstates will probably help here if you really need to support INACTIVE. However, Paul may be right in that you might just start with supporing ON only, and validate that module-level wakups acutally work. Yeah, I'm going to do that as soon as I catch up the emails. I thinking more more that Paul is right and that we should just scrap the INACTIVE state and leave everything ON. Mark -- -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
