* [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2012-12-12 8:31 ` Vaibhav Hiremath
2012-12-12 9:28 ` Santosh Shilimkar
2012-12-09 1:23 ` [PATCH 02/10] ARM: OMAP2+: clockdomain: add pwrdm_state_switch() call to clkdm_sleep() Paul Walmsley
` (9 subsequent siblings)
10 siblings, 2 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
Fix the following sparse warnings in the OMAP3/4 CPUIdle code:
arch/arm/mach-omap2/cpuidle34xx.c:272:1: warning: symbol 'omap3_idle_dev' was not declared. Should it be static?
arch/arm/mach-omap2/cpuidle34xx.c:274:23: warning: symbol 'omap3_idle_driver' was not declared. Should it be static?
arch/arm/mach-omap2/cpuidle44xx.c:164:1: warning: symbol 'omap4_idle_dev' was not declared. Should it be static?
arch/arm/mach-omap2/cpuidle44xx.c:166:23: warning: symbol 'omap4_idle_driver' was not declared. Should it be static?
Also fix the following checkpatch warnings:
WARNING: please, no space before tabs
#44: FILE: arch/arm/mach-omap2/cpuidle34xx.c:105:
+^I.name = ^I"omap3_idle",$
WARNING: please, no space before tabs
#45: FILE: arch/arm/mach-omap2/cpuidle34xx.c:106:
+^I.owner = ^ITHIS_MODULE,$
ERROR: code indent should use tabs where possible
#211: FILE: arch/arm/mach-omap2/cpuidle44xx.c:74:
+ /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */$
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
arch/arm/mach-omap2/cpuidle34xx.c | 14 +++++++++-----
arch/arm/mach-omap2/cpuidle44xx.c | 28 ++++++++++++++++------------
2 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index ad70220..e6215b5 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -53,6 +53,8 @@ struct omap3_idle_statedata {
*/
#define OMAP_CPUIDLE_CX_NO_CLKDM_IDLE BIT(0)
+static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
+
/*
* Prevent PER OFF if CORE is not in RETention or OFF as this would
* disable PER wakeups completely.
@@ -97,7 +99,7 @@ static struct omap3_idle_statedata omap3_idle_data[] = {
},
};
-static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
+/* Private functions */
static int __omap3_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
@@ -269,11 +271,11 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
return ret;
}
-DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
+static DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
-struct cpuidle_driver omap3_idle_driver = {
- .name = "omap3_idle",
- .owner = THIS_MODULE,
+static struct cpuidle_driver omap3_idle_driver = {
+ .name = "omap3_idle",
+ .owner = THIS_MODULE,
.states = {
{
.enter = omap3_enter_idle_bm,
@@ -336,6 +338,8 @@ struct cpuidle_driver omap3_idle_driver = {
.safe_state_index = 0,
};
+/* Public functions */
+
/**
* omap3_idle_init - Init routine for OMAP3 idle
*
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index 288bee6..d639aef 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -54,6 +54,8 @@ static struct clockdomain *cpu_clkdm[NR_CPUS];
static atomic_t abort_barrier;
static bool cpu_done[NR_CPUS];
+/* Private functions */
+
/**
* omap4_enter_idle_coupled_[simple/coupled] - OMAP4 cpuidle entry functions
* @dev: cpuidle device
@@ -161,9 +163,19 @@ fail:
return index;
}
-DEFINE_PER_CPU(struct cpuidle_device, omap4_idle_dev);
+/*
+ * For each cpu, setup the broadcast timer because local timers
+ * stops for the states above C1.
+ */
+static void omap_setup_broadcast_timer(void *arg)
+{
+ int cpu = smp_processor_id();
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
+}
+
+static DEFINE_PER_CPU(struct cpuidle_device, omap4_idle_dev);
-struct cpuidle_driver omap4_idle_driver = {
+static struct cpuidle_driver omap4_idle_driver = {
.name = "omap4_idle",
.owner = THIS_MODULE,
.en_core_tk_irqen = 1,
@@ -178,7 +190,7 @@ struct cpuidle_driver omap4_idle_driver = {
.desc = "MPUSS ON"
},
{
- /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */
+ /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */
.exit_latency = 328 + 440,
.target_residency = 960,
.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_COUPLED,
@@ -200,15 +212,7 @@ struct cpuidle_driver omap4_idle_driver = {
.safe_state_index = 0,
};
-/*
- * For each cpu, setup the broadcast timer because local timers
- * stops for the states above C1.
- */
-static void omap_setup_broadcast_timer(void *arg)
-{
- int cpu = smp_processor_id();
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
-}
+/* Public functions */
/**
* omap4_idle_init - Init routine for OMAP4 idle
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings
2012-12-09 1:23 ` [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings Paul Walmsley
@ 2012-12-12 8:31 ` Vaibhav Hiremath
2012-12-13 5:41 ` Paul Walmsley
2012-12-12 9:28 ` Santosh Shilimkar
1 sibling, 1 reply; 28+ messages in thread
From: Vaibhav Hiremath @ 2012-12-12 8:31 UTC (permalink / raw)
To: linux-arm-kernel
On 12/9/2012 6:53 AM, Paul Walmsley wrote:
> Fix the following sparse warnings in the OMAP3/4 CPUIdle code:
>
> arch/arm/mach-omap2/cpuidle34xx.c:272:1: warning: symbol 'omap3_idle_dev' was not declared. Should it be static?
> arch/arm/mach-omap2/cpuidle34xx.c:274:23: warning: symbol 'omap3_idle_driver' was not declared. Should it be static?
> arch/arm/mach-omap2/cpuidle44xx.c:164:1: warning: symbol 'omap4_idle_dev' was not declared. Should it be static?
> arch/arm/mach-omap2/cpuidle44xx.c:166:23: warning: symbol 'omap4_idle_driver' was not declared. Should it be static?
>
> Also fix the following checkpatch warnings:
>
> WARNING: please, no space before tabs
> #44: FILE: arch/arm/mach-omap2/cpuidle34xx.c:105:
> +^I.name = ^I"omap3_idle",$
>
> WARNING: please, no space before tabs
> #45: FILE: arch/arm/mach-omap2/cpuidle34xx.c:106:
> +^I.owner = ^ITHIS_MODULE,$
>
> ERROR: code indent should use tabs where possible
> #211: FILE: arch/arm/mach-omap2/cpuidle44xx.c:74:
> + /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */$
>
>
Paul,
I am using your "paul-linux-pwrdm_post_fpwrst_devel_a_3.9" branch, where
all the patches which you have posted are present (I believe so) and I
am getting following sparse warning -
CHECK arch/arm/mach-omap2/powerdomain.c
arch/arm/mach-omap2/powerdomain.c:811:2: warning: context imbalance in
'pwrdm_lock': unexpected unlock
arch/arm/mach-omap2/powerdomain.c:811:2: default context: wanted 1, got 0
include/linux/spinlock.h:340:2: warning: context problem in
'pwrdm_unlock': '_raw_spin_unlock_irqrestore' expected different context
include/linux/spinlock.h:340:2: context 'lock': wanted >= 1, got 0
arch/arm/mach-omap2/powerdomain.c:1130:14: warning: context problem in
'pwrdm_state_switch': 'pwrdm_unlock' expected different context
arch/arm/mach-omap2/powerdomain.c:1130:14: default context: wanted >=
1, got 0
arch/arm/mach-omap2/powerdomain.c:1295:14: warning: context problem in
'pwrdm_set_next_fpwrst': 'pwrdm_unlock' expected different context
arch/arm/mach-omap2/powerdomain.c:1295:14: default context: wanted >=
1, got 0
arch/arm/mach-omap2/powerdomain.c:1317:14: warning: context problem in
'pwrdm_read_next_fpwrst': 'pwrdm_unlock' expected different context
arch/arm/mach-omap2/powerdomain.c:1317:14: default context: wanted >=
1, got 0
arch/arm/mach-omap2/powerdomain.c:1382:14: warning: context problem in
'pwrdm_set_fpwrst': 'pwrdm_unlock' expected different context
arch/arm/mach-omap2/powerdomain.c:1382:14: default context: wanted >=
1, got 0
arch/arm/mach-omap2/powerdomain.c:1407:14: warning: context problem in
'pwrdm_read_fpwrst': 'pwrdm_unlock' expected different context
arch/arm/mach-omap2/powerdomain.c:1407:14: default context: wanted >=
1, got 0
arch/arm/mach-omap2/powerdomain.c:1432:14: warning: context problem in
'pwrdm_read_prev_fpwrst': 'pwrdm_unlock' expected different context
arch/arm/mach-omap2/powerdomain.c:1432:14: default context: wanted >=
1, got 0
arch/arm/mach-omap2/powerdomain.c:1505:14: warning: context problem in
'pwrdm_dbg_show_counter': 'pwrdm_unlock' expected different context
arch/arm/mach-omap2/powerdomain.c:1505:14: default context: wanted >=
1, got 0
arch/arm/mach-omap2/powerdomain.c:1542:14: warning: context problem in
'pwrdm_dbg_show_timer': 'pwrdm_unlock' expected different context
arch/arm/mach-omap2/powerdomain.c:1542:14: default context: wanted >=
1, got 0
CC arch/arm/mach-omap2/powerdomain.o
On the other hand, I have boot tested it on BeagleBone platform.
Thanks,
Vaibhav
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings
2012-12-12 8:31 ` Vaibhav Hiremath
@ 2012-12-13 5:41 ` Paul Walmsley
2012-12-13 5:55 ` Hiremath, Vaibhav
0 siblings, 1 reply; 28+ messages in thread
From: Paul Walmsley @ 2012-12-13 5:41 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 12 Dec 2012, Vaibhav Hiremath wrote:
> I am using your "paul-linux-pwrdm_post_fpwrst_devel_a_3.9" branch, where
> all the patches which you have posted are present (I believe so) and I
> am getting following sparse warning -
The branch name to use is:
"TEST_pwrdm_post_fpwrst_devel_a_3.9"
- Paul
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings
2012-12-13 5:41 ` Paul Walmsley
@ 2012-12-13 5:55 ` Hiremath, Vaibhav
2012-12-13 7:29 ` Paul Walmsley
0 siblings, 1 reply; 28+ messages in thread
From: Hiremath, Vaibhav @ 2012-12-13 5:55 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 13, 2012 at 11:11:49, Paul Walmsley wrote:
> On Wed, 12 Dec 2012, Vaibhav Hiremath wrote:
>
> > I am using your "paul-linux-pwrdm_post_fpwrst_devel_a_3.9" branch, where
> > all the patches which you have posted are present (I believe so) and I
> > am getting following sparse warning -
>
> The branch name to use is:
>
> "TEST_pwrdm_post_fpwrst_devel_a_3.9"
>
If I am correct, it only includes one additional patch (merge commit),
right???
commit d94831e0005fee743cefd28f4c20b7c435c71236
Merge: 3e885c6 80ab3b2
Author: Paul Walmsley <paul@pwsan.com>
Date: Sun Dec 9 13:06:51 2012 -0700
build fixes
Does this also fix sparse warnings?
Thanks,
Vaibhav
>
> - Paul
>
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings
2012-12-13 5:55 ` Hiremath, Vaibhav
@ 2012-12-13 7:29 ` Paul Walmsley
2012-12-13 9:18 ` Hiremath, Vaibhav
0 siblings, 1 reply; 28+ messages in thread
From: Paul Walmsley @ 2012-12-13 7:29 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 13 Dec 2012, Hiremath, Vaibhav wrote:
> On Thu, Dec 13, 2012 at 11:11:49, Paul Walmsley wrote:
> >
> > The branch name to use is:
> >
> > "TEST_pwrdm_post_fpwrst_devel_a_3.9"
>
> If I am correct, it only includes one additional patch (merge commit),
> right???
>
> commit d94831e0005fee743cefd28f4c20b7c435c71236
> Merge: 3e885c6 80ab3b2
> Author: Paul Walmsley <paul@pwsan.com>
> Date: Sun Dec 9 13:06:51 2012 -0700
>
> build fixes
>
>
>
> Does this also fix sparse warnings?
Just ran a quick sparse check on mach-omap2 at 3e885c6 and d94831e with
'make -j4 C=2 arch/arm/mach-omap2', and no warnings showed up. There were
some sparse issues that got fixed at an earlier point, though, so perhaps
you have an older copy of the branches somehow?
- Paul
paul at dusk:/kernel/kernel/current$ git log -1
commit d94831e0005fee743cefd28f4c20b7c435c71236
Merge: 3e885c6 80ab3b2
Author: Paul Walmsley <paul@pwsan.com>
Date: Sun Dec 9 13:06:51 2012 -0700
build fixes
paul at dusk:/kernel/kernel/current$ make C=2 -j4 arch/arm/mach-omap2/
CHK include/generated/uapi/linux/version.h
CHECK scripts/mod/empty.c
CHK include/generated/utsrelease.h
UPD include/generated/utsrelease.h
make[1]: `include/generated/mach-types.h' is up to date.
CC kernel/bounds.s
GEN include/generated/bounds.h
CC arch/arm/kernel/asm-offsets.s
GEN include/generated/asm-offsets.h
CALL scripts/checksyscalls.sh
CHECK arch/arm/mach-omap2/id.c
CHECK arch/arm/mach-omap2/io.c
CHECK arch/arm/mach-omap2/control.c
CHECK arch/arm/mach-omap2/mux.c
CC arch/arm/mach-omap2/id.o
CC arch/arm/mach-omap2/control.o
CC arch/arm/mach-omap2/io.o
CC arch/arm/mach-omap2/mux.o
CHECK arch/arm/mach-omap2/devices.c
CHECK arch/arm/mach-omap2/serial.c
CHECK arch/arm/mach-omap2/gpmc.c
CC arch/arm/mach-omap2/devices.o
CC arch/arm/mach-omap2/serial.o
CC arch/arm/mach-omap2/gpmc.o
CHECK arch/arm/mach-omap2/timer.c
CHECK arch/arm/mach-omap2/pm.c
CC arch/arm/mach-omap2/timer.o
CC arch/arm/mach-omap2/pm.o
CHECK arch/arm/mach-omap2/common.c
CC arch/arm/mach-omap2/common.o
CHECK arch/arm/mach-omap2/gpio.c
CC arch/arm/mach-omap2/gpio.o
CHECK arch/arm/mach-omap2/dma.c
CC arch/arm/mach-omap2/dma.o
CHECK arch/arm/mach-omap2/wd_timer.c
CHECK arch/arm/mach-omap2/display.c
CHECK arch/arm/mach-omap2/i2c.c
CC arch/arm/mach-omap2/wd_timer.o
CC arch/arm/mach-omap2/i2c.o
CC arch/arm/mach-omap2/display.o
CHECK arch/arm/mach-omap2/hdq1w.c
CC arch/arm/mach-omap2/hdq1w.o
CHECK arch/arm/mach-omap2/omap_hwmod.c
CHECK arch/arm/mach-omap2/omap_device.c
CC arch/arm/mach-omap2/omap_hwmod.o
CC arch/arm/mach-omap2/omap_device.o
CHECK arch/arm/mach-omap2/irq.c
CHECK arch/arm/mach-omap2/omap_hwmod_common_data.c
CC arch/arm/mach-omap2/irq.o
CC arch/arm/mach-omap2/omap_hwmod_common_data.o
CHECK arch/arm/mach-omap2/omap-secure.c
CC arch/arm/mach-omap2/omap-secure.o
CHECK arch/arm/mach-omap2/prm44xx.c
CHECK arch/arm/mach-omap2/mcbsp.c
CC arch/arm/mach-omap2/prm44xx.o
CC arch/arm/mach-omap2/mcbsp.o
CHECK arch/arm/mach-omap2/omap_twl.c
CC arch/arm/mach-omap2/omap_twl.o
CHECK arch/arm/mach-omap2/sdrc.c
CC arch/arm/mach-omap2/sdrc.o
CHECK arch/arm/mach-omap2/omap-smp.c
CHECK arch/arm/mach-omap2/omap-hotplug.c
CC arch/arm/mach-omap2/omap-smp.o
CC arch/arm/mach-omap2/omap-hotplug.o
CHECK arch/arm/mach-omap2/omap4-common.c
CC arch/arm/mach-omap2/omap4-common.o
CHECK arch/arm/mach-omap2/omap-wakeupgen.c
AS arch/arm/mach-omap2/sram242x.o
AS arch/arm/mach-omap2/sram243x.o
CC arch/arm/mach-omap2/omap-wakeupgen.o
AS arch/arm/mach-omap2/sram34xx.o
CHECK arch/arm/mach-omap2/omap2-restart.c
CHECK arch/arm/mach-omap2/omap3-restart.c
CC arch/arm/mach-omap2/omap2-restart.o
CC arch/arm/mach-omap2/omap3-restart.o
CHECK arch/arm/mach-omap2/mux2420.c
CC arch/arm/mach-omap2/mux2420.o
CHECK arch/arm/mach-omap2/mux2430.c
CHECK arch/arm/mach-omap2/mux34xx.c
CHECK arch/arm/mach-omap2/mux44xx.c
CC arch/arm/mach-omap2/mux2430.o
CC arch/arm/mach-omap2/mux34xx.o
CC arch/arm/mach-omap2/mux44xx.o
CHECK arch/arm/mach-omap2/sdrc2xxx.c
CHECK arch/arm/mach-omap2/opp.c
CC arch/arm/mach-omap2/sdrc2xxx.o
CC arch/arm/mach-omap2/opp.o
CHECK arch/arm/mach-omap2/opp3xxx_data.c
CHECK arch/arm/mach-omap2/opp4xxx_data.c
CC arch/arm/mach-omap2/opp3xxx_data.o
CC arch/arm/mach-omap2/opp4xxx_data.o
CHECK arch/arm/mach-omap2/pm24xx.c
AS arch/arm/mach-omap2/sleep24xx.o
CHECK arch/arm/mach-omap2/pm34xx.c
AS arch/arm/mach-omap2/sleep34xx.o
CHECK arch/arm/mach-omap2/pm44xx.c
CC arch/arm/mach-omap2/pm24xx.o
CHECK arch/arm/mach-omap2/omap-mpuss-lowpower.c
CC arch/arm/mach-omap2/pm34xx.o
CC arch/arm/mach-omap2/pm44xx.o
CC arch/arm/mach-omap2/omap-mpuss-lowpower.o
CHECK arch/arm/mach-omap2/pm-debug.c
CHECK arch/arm/mach-omap2/prm_common.c
CC arch/arm/mach-omap2/pm-debug.o
CHECK arch/arm/mach-omap2/cm_common.c
CC arch/arm/mach-omap2/prm_common.o
CC arch/arm/mach-omap2/cm_common.o
CHECK arch/arm/mach-omap2/prm2xxx_3xxx.c
CC arch/arm/mach-omap2/prm2xxx_3xxx.o
CHECK arch/arm/mach-omap2/prm2xxx.c
CHECK arch/arm/mach-omap2/cm2xxx.c
CC arch/arm/mach-omap2/prm2xxx.o
CC arch/arm/mach-omap2/cm2xxx.o
CHECK arch/arm/mach-omap2/prm3xxx.c
CC arch/arm/mach-omap2/prm3xxx.o
CHECK arch/arm/mach-omap2/cm3xxx.c
CC arch/arm/mach-omap2/cm3xxx.o
CHECK arch/arm/mach-omap2/vc3xxx_data.c
CHECK arch/arm/mach-omap2/vp3xxx_data.c
CC arch/arm/mach-omap2/vc3xxx_data.o
CC arch/arm/mach-omap2/vp3xxx_data.o
CHECK arch/arm/mach-omap2/prm33xx.c
CC arch/arm/mach-omap2/prm33xx.o
CHECK arch/arm/mach-omap2/cm33xx.c
CHECK arch/arm/mach-omap2/cminst44xx.c
CC arch/arm/mach-omap2/cm33xx.o
CC arch/arm/mach-omap2/cminst44xx.o
CHECK arch/arm/mach-omap2/cm44xx.c
CC arch/arm/mach-omap2/cm44xx.o
CHECK arch/arm/mach-omap2/prcm_mpu44xx.c
CC arch/arm/mach-omap2/prcm_mpu44xx.o
CHECK arch/arm/mach-omap2/prminst44xx.c
CHECK arch/arm/mach-omap2/vc44xx_data.c
CC arch/arm/mach-omap2/prminst44xx.o
CHECK arch/arm/mach-omap2/vp44xx_data.c
CC arch/arm/mach-omap2/vc44xx_data.o
CHECK arch/arm/mach-omap2/voltage.c
CC arch/arm/mach-omap2/vp44xx_data.o
CC arch/arm/mach-omap2/voltage.o
CHECK arch/arm/mach-omap2/vc.c
CC arch/arm/mach-omap2/vc.o
CHECK arch/arm/mach-omap2/vp.c
CHECK arch/arm/mach-omap2/voltagedomains2xxx_data.c
CC arch/arm/mach-omap2/voltagedomains2xxx_data.o
CC arch/arm/mach-omap2/vp.o
CHECK arch/arm/mach-omap2/voltagedomains3xxx_data.c
CC arch/arm/mach-omap2/voltagedomains3xxx_data.o
CHECK arch/arm/mach-omap2/voltagedomains44xx_data.c
CHECK arch/arm/mach-omap2/voltagedomains33xx_data.c
CC arch/arm/mach-omap2/voltagedomains33xx_data.o
CC arch/arm/mach-omap2/voltagedomains44xx_data.o
CHECK arch/arm/mach-omap2/powerdomain.c
CC arch/arm/mach-omap2/powerdomain.o
CHECK arch/arm/mach-omap2/powerdomain-common.c
CC arch/arm/mach-omap2/powerdomain-common.o
CHECK arch/arm/mach-omap2/powerdomains2xxx_data.c
CC arch/arm/mach-omap2/powerdomains2xxx_data.o
CHECK arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c
CC arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.o
CHECK arch/arm/mach-omap2/powerdomains3xxx_data.c
CHECK arch/arm/mach-omap2/powerdomains44xx_data.c
CC arch/arm/mach-omap2/powerdomains3xxx_data.o
CHECK arch/arm/mach-omap2/powerdomains33xx_data.c
CC arch/arm/mach-omap2/powerdomains33xx_data.o
CC arch/arm/mach-omap2/powerdomains44xx_data.o
CHECK arch/arm/mach-omap2/clockdomain.c
CC arch/arm/mach-omap2/clockdomain.o
CHECK arch/arm/mach-omap2/clockdomains2xxx_3xxx_data.c
CC arch/arm/mach-omap2/clockdomains2xxx_3xxx_data.o
CHECK arch/arm/mach-omap2/clockdomains2420_data.c
CHECK arch/arm/mach-omap2/clockdomains2430_data.c
CC arch/arm/mach-omap2/clockdomains2420_data.o
CC arch/arm/mach-omap2/clockdomains2430_data.o
CHECK arch/arm/mach-omap2/clockdomains3xxx_data.c
CHECK arch/arm/mach-omap2/clockdomains44xx_data.c
CC arch/arm/mach-omap2/clockdomains3xxx_data.o
CHECK arch/arm/mach-omap2/clockdomains33xx_data.c
CC arch/arm/mach-omap2/clockdomains44xx_data.o
CC arch/arm/mach-omap2/clockdomains33xx_data.o
CHECK arch/arm/mach-omap2/clock.c
CHECK arch/arm/mach-omap2/clock_common_data.c
CC arch/arm/mach-omap2/clock_common_data.o
CC arch/arm/mach-omap2/clock.o
CHECK arch/arm/mach-omap2/clkt_dpll.c
CHECK arch/arm/mach-omap2/clkt_clksel.c
CC arch/arm/mach-omap2/clkt_dpll.o
CHECK arch/arm/mach-omap2/clock2xxx.c
CC arch/arm/mach-omap2/clkt_clksel.o
CC arch/arm/mach-omap2/clock2xxx.o
CHECK arch/arm/mach-omap2/clkt2xxx_sys.c
CC arch/arm/mach-omap2/clkt2xxx_sys.o
CHECK arch/arm/mach-omap2/clkt2xxx_dpllcore.c
CC arch/arm/mach-omap2/clkt2xxx_dpllcore.o
CHECK arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c
CC arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.o
CHECK arch/arm/mach-omap2/clkt2xxx_apll.c
CC arch/arm/mach-omap2/clkt2xxx_apll.o
CHECK arch/arm/mach-omap2/clkt2xxx_osc.c
CC arch/arm/mach-omap2/clkt2xxx_osc.o
CHECK arch/arm/mach-omap2/clkt2xxx_dpll.c
CC arch/arm/mach-omap2/clkt2xxx_dpll.o
CHECK arch/arm/mach-omap2/clkt_iclk.c
CC arch/arm/mach-omap2/clkt_iclk.o
CHECK arch/arm/mach-omap2/clock2420_data.c
CHECK arch/arm/mach-omap2/clock2430.c
CHECK arch/arm/mach-omap2/clock2430_data.c
CC arch/arm/mach-omap2/clock2430.o
CC arch/arm/mach-omap2/clock2420_data.o
CC arch/arm/mach-omap2/clock2430_data.o
CHECK arch/arm/mach-omap2/clock3xxx.c
CC arch/arm/mach-omap2/clock3xxx.o
CHECK arch/arm/mach-omap2/clock34xx.c
CC arch/arm/mach-omap2/clock34xx.o
CHECK arch/arm/mach-omap2/clkt34xx_dpll3m2.c
CHECK arch/arm/mach-omap2/clock3517.c
CHECK arch/arm/mach-omap2/clock36xx.c
CC arch/arm/mach-omap2/clock3517.o
CC arch/arm/mach-omap2/clkt34xx_dpll3m2.o
CC arch/arm/mach-omap2/clock36xx.o
CHECK arch/arm/mach-omap2/dpll3xxx.c
CC arch/arm/mach-omap2/dpll3xxx.o
CHECK arch/arm/mach-omap2/clock3xxx_data.c
CHECK arch/arm/mach-omap2/clock44xx_data.c
CHECK arch/arm/mach-omap2/dpll44xx.c
CC arch/arm/mach-omap2/clock3xxx_data.o
CC arch/arm/mach-omap2/dpll44xx.o
CC arch/arm/mach-omap2/clock44xx_data.o
CHECK arch/arm/mach-omap2/clock33xx_data.c
CHECK arch/arm/mach-omap2/opp2420_data.c
CC arch/arm/mach-omap2/opp2420_data.o
CHECK arch/arm/mach-omap2/opp2430_data.c
CHECK arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
CC arch/arm/mach-omap2/clock33xx_data.o
CC arch/arm/mach-omap2/opp2430_data.o
CHECK arch/arm/mach-omap2/omap_hwmod_2xxx_3xxx_ipblock_data.c
CC arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.o
CC arch/arm/mach-omap2/omap_hwmod_2xxx_3xxx_ipblock_data.o
CHECK arch/arm/mach-omap2/omap_hwmod_2xxx_interconnect_data.c
CC arch/arm/mach-omap2/omap_hwmod_2xxx_interconnect_data.o
CHECK arch/arm/mach-omap2/omap_hwmod_2xxx_3xxx_interconnect_data.c
CHECK arch/arm/mach-omap2/omap_hwmod_2420_data.c
CHECK arch/arm/mach-omap2/omap_hwmod_2430_data.c
CC arch/arm/mach-omap2/omap_hwmod_2xxx_3xxx_interconnect_data.o
CC arch/arm/mach-omap2/omap_hwmod_2420_data.o
CC arch/arm/mach-omap2/omap_hwmod_2430_data.o
CHECK arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
CC arch/arm/mach-omap2/omap_hwmod_3xxx_data.o
CHECK arch/arm/mach-omap2/omap_hwmod_33xx_data.c
CHECK arch/arm/mach-omap2/omap_hwmod_44xx_data.c
CHECK arch/arm/mach-omap2/pmu.c
CC arch/arm/mach-omap2/omap_hwmod_33xx_data.o
CC arch/arm/mach-omap2/omap_hwmod_44xx_data.o
CC arch/arm/mach-omap2/pmu.o
CHECK arch/arm/mach-omap2/msdi.c
CC arch/arm/mach-omap2/msdi.o
CHECK arch/arm/mach-omap2/board-generic.c
CHECK arch/arm/mach-omap2/board-h4.c
CC arch/arm/mach-omap2/board-generic.o
CHECK arch/arm/mach-omap2/board-2430sdp.c
CC arch/arm/mach-omap2/board-h4.o
CC arch/arm/mach-omap2/board-2430sdp.o
CHECK arch/arm/mach-omap2/board-apollon.c
CHECK arch/arm/mach-omap2/board-omap3beagle.c
CC arch/arm/mach-omap2/board-apollon.o
CC arch/arm/mach-omap2/board-omap3beagle.o
CHECK arch/arm/mach-omap2/board-devkit8000.c
CC arch/arm/mach-omap2/board-devkit8000.o
CHECK arch/arm/mach-omap2/board-ldp.c
CHECK arch/arm/mach-omap2/board-omap3logic.c
CC arch/arm/mach-omap2/board-ldp.o
CC arch/arm/mach-omap2/board-omap3logic.o
CHECK arch/arm/mach-omap2/board-overo.c
CC arch/arm/mach-omap2/board-overo.o
CHECK arch/arm/mach-omap2/board-omap3evm.c
CC arch/arm/mach-omap2/board-omap3evm.o
CHECK arch/arm/mach-omap2/board-omap3pandora.c
CHECK arch/arm/mach-omap2/board-3430sdp.c
CC arch/arm/mach-omap2/board-omap3pandora.o
CC arch/arm/mach-omap2/board-3430sdp.o
CHECK arch/arm/mach-omap2/board-n8x0.c
CC arch/arm/mach-omap2/board-n8x0.o
CHECK arch/arm/mach-omap2/board-rm680.c
CC arch/arm/mach-omap2/board-rm680.o
CHECK arch/arm/mach-omap2/sdram-nokia.c
CHECK arch/arm/mach-omap2/board-rx51.c
CC arch/arm/mach-omap2/sdram-nokia.o
CC arch/arm/mach-omap2/board-rx51.o
CHECK arch/arm/mach-omap2/board-rx51-peripherals.c
CC arch/arm/mach-omap2/board-rx51-peripherals.o
CHECK arch/arm/mach-omap2/board-rx51-video.c
CC arch/arm/mach-omap2/board-rx51-video.o
CHECK arch/arm/mach-omap2/board-zoom-peripherals.c
CHECK arch/arm/mach-omap2/board-zoom.c
CC arch/arm/mach-omap2/board-zoom.o
CC arch/arm/mach-omap2/board-zoom-peripherals.o
CHECK arch/arm/mach-omap2/board-zoom-display.c
CHECK arch/arm/mach-omap2/board-zoom-debugboard.c
CC arch/arm/mach-omap2/board-zoom-display.o
CC arch/arm/mach-omap2/board-zoom-debugboard.o
CHECK arch/arm/mach-omap2/board-3630sdp.c
CHECK arch/arm/mach-omap2/board-cm-t35.c
CC arch/arm/mach-omap2/board-3630sdp.o
CHECK arch/arm/mach-omap2/board-cm-t3517.c
CC arch/arm/mach-omap2/board-cm-t35.o
CC arch/arm/mach-omap2/board-cm-t3517.o
CHECK arch/arm/mach-omap2/board-igep0020.c
CHECK arch/arm/mach-omap2/board-omap3touchbook.c
CC arch/arm/mach-omap2/board-igep0020.o
CC arch/arm/mach-omap2/board-omap3touchbook.o
CHECK arch/arm/mach-omap2/board-4430sdp.c
CHECK arch/arm/mach-omap2/board-omap4panda.c
CC arch/arm/mach-omap2/board-4430sdp.o
CC arch/arm/mach-omap2/board-omap4panda.o
CHECK arch/arm/mach-omap2/board-am3517evm.c
CHECK arch/arm/mach-omap2/board-omap3stalker.c
CC arch/arm/mach-omap2/board-am3517evm.o
CC arch/arm/mach-omap2/board-omap3stalker.o
CHECK arch/arm/mach-omap2/board-ti8168evm.c
CC arch/arm/mach-omap2/board-ti8168evm.o
CHECK arch/arm/mach-omap2/board-flash.c
CC arch/arm/mach-omap2/board-flash.o
CHECK arch/arm/mach-omap2/hsmmc.c
CHECK arch/arm/mach-omap2/usb-musb.c
CC arch/arm/mach-omap2/hsmmc.o
CHECK arch/arm/mach-omap2/omap_phy_internal.c
CC arch/arm/mach-omap2/usb-musb.o
CC arch/arm/mach-omap2/omap_phy_internal.o
CHECK arch/arm/mach-omap2/usb-tusb6010.c
CHECK arch/arm/mach-omap2/usb-host.c
CC arch/arm/mach-omap2/usb-tusb6010.o
CC arch/arm/mach-omap2/usb-host.o
CHECK arch/arm/mach-omap2/gpmc-onenand.c
CC arch/arm/mach-omap2/gpmc-onenand.o
CHECK arch/arm/mach-omap2/gpmc-nand.c
CHECK arch/arm/mach-omap2/gpmc-smc91x.c
CC arch/arm/mach-omap2/gpmc-smc91x.o
CC arch/arm/mach-omap2/gpmc-nand.o
CHECK arch/arm/mach-omap2/gpmc-smsc911x.c
CC arch/arm/mach-omap2/gpmc-smsc911x.o
CHECK arch/arm/mach-omap2/common-board-devices.c
CHECK arch/arm/mach-omap2/twl-common.c
CC arch/arm/mach-omap2/common-board-devices.o
CC arch/arm/mach-omap2/twl-common.o
LD arch/arm/mach-omap2/built-in.o
paul at dusk:/kernel/kernel/current$
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings
2012-12-13 7:29 ` Paul Walmsley
@ 2012-12-13 9:18 ` Hiremath, Vaibhav
0 siblings, 0 replies; 28+ messages in thread
From: Hiremath, Vaibhav @ 2012-12-13 9:18 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 13, 2012 at 12:59:49, Paul Walmsley wrote:
> On Thu, 13 Dec 2012, Hiremath, Vaibhav wrote:
>
> > On Thu, Dec 13, 2012 at 11:11:49, Paul Walmsley wrote:
> > >
> > > The branch name to use is:
> > >
> > > "TEST_pwrdm_post_fpwrst_devel_a_3.9"
> >
> > If I am correct, it only includes one additional patch (merge commit),
> > right???
> >
> > commit d94831e0005fee743cefd28f4c20b7c435c71236
> > Merge: 3e885c6 80ab3b2
> > Author: Paul Walmsley <paul@pwsan.com>
> > Date: Sun Dec 9 13:06:51 2012 -0700
> >
> > build fixes
> >
> >
> >
> > Does this also fix sparse warnings?
>
> Just ran a quick sparse check on mach-omap2 at 3e885c6 and d94831e with
> 'make -j4 C=2 arch/arm/mach-omap2', and no warnings showed up. There were
> some sparse issues that got fixed at an earlier point, though, so perhaps
> you have an older copy of the branches somehow?
>
Ignore my earlier comment on sparse warning, it was my wrong sparse tool
which was culprit here. I am working on remote server, which has sparse
present inside /usr/bin, and somehow my default config using that one
instead of what I was building.
I fixed this by changing the PATH, and now I do not see any issues/warnings.
Thanks,
Vaibhav
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings
2012-12-09 1:23 ` [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings Paul Walmsley
2012-12-12 8:31 ` Vaibhav Hiremath
@ 2012-12-12 9:28 ` Santosh Shilimkar
1 sibling, 0 replies; 28+ messages in thread
From: Santosh Shilimkar @ 2012-12-12 9:28 UTC (permalink / raw)
To: linux-arm-kernel
On Sunday 09 December 2012 02:23 AM, Paul Walmsley wrote:
> Fix the following sparse warnings in the OMAP3/4 CPUIdle code:
>
> arch/arm/mach-omap2/cpuidle34xx.c:272:1: warning: symbol 'omap3_idle_dev' was not declared. Should it be static?
> arch/arm/mach-omap2/cpuidle34xx.c:274:23: warning: symbol 'omap3_idle_driver' was not declared. Should it be static?
> arch/arm/mach-omap2/cpuidle44xx.c:164:1: warning: symbol 'omap4_idle_dev' was not declared. Should it be static?
> arch/arm/mach-omap2/cpuidle44xx.c:166:23: warning: symbol 'omap4_idle_driver' was not declared. Should it be static?
>
> Also fix the following checkpatch warnings:
>
> WARNING: please, no space before tabs
> #44: FILE: arch/arm/mach-omap2/cpuidle34xx.c:105:
> +^I.name = ^I"omap3_idle",$
>
> WARNING: please, no space before tabs
> #45: FILE: arch/arm/mach-omap2/cpuidle34xx.c:106:
> +^I.owner = ^ITHIS_MODULE,$
>
> ERROR: code indent should use tabs where possible
> #211: FILE: arch/arm/mach-omap2/cpuidle44xx.c:74:
> + /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */$
>
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@ti.com>
> ---
Acked-by: Santosh Shilimkar at ti.com
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 02/10] ARM: OMAP2+: clockdomain: add pwrdm_state_switch() call to clkdm_sleep()
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
2012-12-09 1:23 ` [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2012-12-09 1:23 ` [PATCH 03/10] ARM: OMAP2xxx: PM: clean up some crufty powerstate programming code Paul Walmsley
` (8 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
In clkdm_sleep(), the powerdomain should be eligible to switch power
states right after the call to the low-level clockdomain sleep
function. We should have been tracking that with the
pwrdm_state_switch() code, but we weren't, for whatever reason. Fix that.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/clockdomain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 64e5046..18f65fd 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -763,6 +763,7 @@ int clkdm_sleep(struct clockdomain *clkdm)
spin_lock_irqsave(&clkdm->lock, flags);
clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
ret = arch_clkdm->clkdm_sleep(clkdm);
+ ret |= pwrdm_state_switch(clkdm->pwrdm.ptr);
spin_unlock_irqrestore(&clkdm->lock, flags);
return ret;
}
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 03/10] ARM: OMAP2xxx: PM: clean up some crufty powerstate programming code
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
2012-12-09 1:23 ` [PATCH 01/10] ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings Paul Walmsley
2012-12-09 1:23 ` [PATCH 02/10] ARM: OMAP2+: clockdomain: add pwrdm_state_switch() call to clkdm_sleep() Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2012-12-09 1:23 ` [PATCH 04/10] ARM: OMAP2: PM/powerdomain: drop unnecessary pwrdm_wait_transition() Paul Walmsley
` (7 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
Don't attempt to put clockdomains to sleep; this should be handled by the
clock framework. It should be enough to program the next-power-state,
and then let the code in omap_pm_clkdms_setup() deal with the rest.
Start out by programming the MPU and CORE powerdomains to stay ON.
Then control the MPU and CORE powerdomain states directly in
omap2_enter_full_retention() and omap2_enter_mpu_retention(). Not the
most optimal way to do it, but certainly is the most conservative until
OMAP2xxx PM is working again.
Get rid of the open-coded PM_PWSTCTRL_MPU writes in
omap2_enter_mpu_retention(); use the powerdomain code instead.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
---
arch/arm/mach-omap2/pm24xx.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
index 83815dd..4c06d8a 100644
--- a/arch/arm/mach-omap2/pm24xx.c
+++ b/arch/arm/mach-omap2/pm24xx.c
@@ -89,11 +89,7 @@ static int omap2_enter_full_retention(void)
omap2_prm_write_mod_reg(0xffffffff, CORE_MOD, OMAP24XX_PM_WKST2);
omap2_prm_write_mod_reg(0xffffffff, WKUP_MOD, PM_WKST);
- /*
- * Set MPU powerdomain's next power state to RETENTION;
- * preserve logic state during retention
- */
- pwrdm_set_logic_retst(mpu_pwrdm, PWRDM_POWER_RET);
+ pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET);
pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_RET);
/* Workaround to kill USB */
@@ -136,6 +132,9 @@ no_sleep:
/* Mask future PRCM-to-MPU interrupts */
omap2_prm_write_mod_reg(0x0, OCP_MOD, OMAP2_PRCM_IRQSTATUS_MPU_OFFSET);
+ pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_ON);
+ pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_ON);
+
return 0;
}
@@ -185,17 +184,16 @@ static void omap2_enter_mpu_retention(void)
omap2_prm_write_mod_reg(0xffffffff, WKUP_MOD, PM_WKST);
/* Try to enter MPU retention */
- omap2_prm_write_mod_reg((0x01 << OMAP_POWERSTATE_SHIFT) |
- OMAP_LOGICRETSTATE_MASK,
- MPU_MOD, OMAP2_PM_PWSTCTRL);
+ pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_RET);
+
} else {
/* Block MPU retention */
-
- omap2_prm_write_mod_reg(OMAP_LOGICRETSTATE_MASK, MPU_MOD,
- OMAP2_PM_PWSTCTRL);
+ pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_ON);
}
omap2_sram_idle();
+
+ pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_ON);
}
static int omap2_can_sleep(void)
@@ -250,25 +248,17 @@ static void __init prcm_setup_regs(void)
for (i = 0; i < num_mem_banks; i++)
pwrdm_set_mem_retst(core_pwrdm, i, PWRDM_POWER_RET);
- /* Set CORE powerdomain's next power state to RETENTION */
- pwrdm_set_next_pwrst(core_pwrdm, PWRDM_POWER_RET);
+ pwrdm_set_logic_retst(core_pwrdm, PWRDM_POWER_RET);
- /*
- * Set MPU powerdomain's next power state to RETENTION;
- * preserve logic state during retention
- */
pwrdm_set_logic_retst(mpu_pwrdm, PWRDM_POWER_RET);
- pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_RET);
/* Force-power down DSP, GFX powerdomains */
pwrdm = clkdm_get_pwrdm(dsp_clkdm);
pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_OFF);
- clkdm_sleep(dsp_clkdm);
pwrdm = clkdm_get_pwrdm(gfx_clkdm);
pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_OFF);
- clkdm_sleep(gfx_clkdm);
/* Enable hardware-supervised idle for all clkdms */
clkdm_for_each(omap_pm_clkdms_setup, NULL);
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 04/10] ARM: OMAP2: PM/powerdomain: drop unnecessary pwrdm_wait_transition()
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
` (2 preceding siblings ...)
2012-12-09 1:23 ` [PATCH 03/10] ARM: OMAP2xxx: PM: clean up some crufty powerstate programming code Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2012-12-09 1:23 ` [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code Paul Walmsley
` (6 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
Drop an unnecessary pwrdm_wait_transition() from mach-omap2/pm.c -
it's called by the subsequent pwrdm_state_switch().
Also get rid of pwrdm_wait_transition() in the powerdomain code - there's
no longer any need to export this function.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
arch/arm/mach-omap2/pm.c | 1 -
arch/arm/mach-omap2/powerdomain.c | 30 +++++-------------------------
arch/arm/mach-omap2/powerdomain.h | 2 --
3 files changed, 5 insertions(+), 28 deletions(-)
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 331478f..cc8ed0f 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -139,7 +139,6 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 pwrst)
break;
case LOWPOWERSTATE_SWITCH:
pwrdm_set_lowpwrstchange(pwrdm);
- pwrdm_wait_transition(pwrdm);
pwrdm_state_switch(pwrdm);
break;
}
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 92388c0..97b3881 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -112,7 +112,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
for (i = 0; i < pwrdm->banks; i++)
pwrdm->ret_mem_off_counter[i] = 0;
- pwrdm_wait_transition(pwrdm);
+ arch_pwrdm->pwrdm_wait_transition(pwrdm);
pwrdm->state = pwrdm_read_pwrst(pwrdm);
pwrdm->state_counter[pwrdm->state] = 1;
@@ -950,34 +950,14 @@ int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
return ret;
}
-/**
- * pwrdm_wait_transition - wait for powerdomain power transition to finish
- * @pwrdm: struct powerdomain * to wait for
- *
- * If the powerdomain @pwrdm is in the process of a state transition,
- * spin until it completes the power transition, or until an iteration
- * bailout value is reached. Returns -EINVAL if the powerdomain
- * pointer is null, -EAGAIN if the bailout value was reached, or
- * returns 0 upon success.
- */
-int pwrdm_wait_transition(struct powerdomain *pwrdm)
-{
- int ret = -EINVAL;
-
- if (!pwrdm)
- return -EINVAL;
-
- if (arch_pwrdm && arch_pwrdm->pwrdm_wait_transition)
- ret = arch_pwrdm->pwrdm_wait_transition(pwrdm);
-
- return ret;
-}
-
int pwrdm_state_switch(struct powerdomain *pwrdm)
{
int ret;
- ret = pwrdm_wait_transition(pwrdm);
+ if (!pwrdm || !arch_pwrdm)
+ return -EINVAL;
+
+ ret = arch_pwrdm->pwrdm_wait_transition(pwrdm);
if (!ret)
ret = _pwrdm_state_switch(pwrdm, PWRDM_STATE_NOW);
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 5277d56eb..7c1534b 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -225,8 +225,6 @@ 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);
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
` (3 preceding siblings ...)
2012-12-09 1:23 ` [PATCH 04/10] ARM: OMAP2: PM/powerdomain: drop unnecessary pwrdm_wait_transition() Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2012-12-12 9:31 ` Jean Pihet
` (2 more replies)
2012-12-09 1:23 ` [PATCH 07/10] ARM: OMAP2xxx: CM: remove autodep handling Paul Walmsley
` (5 subsequent siblings)
10 siblings, 3 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
Move omap_set_pwrdm_state() from the PM code to the powerdomain code,
and refactor it to split it up into several functions. A subsequent patch
will rename it to conform with the existing powerdomain function names.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Jean Pihet <jean.pihet@newoldbits.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
arch/arm/mach-omap2/pm.c | 61 --------------------
arch/arm/mach-omap2/pm.h | 1
arch/arm/mach-omap2/powerdomain.c | 112 +++++++++++++++++++++++++++----------
arch/arm/mach-omap2/powerdomain.h | 3 +
4 files changed, 85 insertions(+), 92 deletions(-)
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index cc8ed0f..2e2a897 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -76,10 +76,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) &&
@@ -92,63 +88,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_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.c b/arch/arm/mach-omap2/powerdomain.c
index 97b3881..05f00660 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -921,35 +921,6 @@ bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm)
return (pwrdm && pwrdm->flags & PWRDM_HAS_HDWR_SAR) ? 1 : 0;
}
-/**
- * pwrdm_set_lowpwrstchange - Request a low power state change
- * @pwrdm: struct powerdomain *
- *
- * Allows a powerdomain to transtion to a lower power sleep state
- * from an existing sleep state without waking up the powerdomain.
- * Returns -EINVAL if the powerdomain pointer is null or if the
- * powerdomain does not support LOWPOWERSTATECHANGE, or returns 0
- * upon success.
- */
-int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
-{
- int ret = -EINVAL;
-
- if (!pwrdm)
- return -EINVAL;
-
- if (!(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE))
- return -EINVAL;
-
- pr_debug("powerdomain: %s: setting LOWPOWERSTATECHANGE bit\n",
- pwrdm->name);
-
- if (arch_pwrdm && arch_pwrdm->pwrdm_set_lowpwrstchange)
- ret = arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
-
- return ret;
-}
-
int pwrdm_state_switch(struct powerdomain *pwrdm)
{
int ret;
@@ -984,6 +955,89 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
return 0;
}
+/* Types of sleep_switch used in omap_set_pwrdm_state */
+#define ALREADYACTIVE_SWITCH 0
+#define FORCEWAKEUP_SWITCH 1
+#define LOWPOWERSTATE_SWITCH 2
+
+static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
+ u8 pwrst, bool *hwsup)
+{
+ u8 curr_pwrst, sleep_switch;
+
+ curr_pwrst = pwrdm_read_pwrst(pwrdm);
+ if (curr_pwrst < PWRDM_POWER_ON) {
+ if (curr_pwrst > pwrst &&
+ pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
+ arch_pwrdm->pwrdm_set_lowpwrstchange) {
+ sleep_switch = LOWPOWERSTATE_SWITCH;
+ } else {
+ *hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
+ clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
+ sleep_switch = FORCEWAKEUP_SWITCH;
+ }
+ } else {
+ sleep_switch = ALREADYACTIVE_SWITCH;
+ }
+
+ return sleep_switch;
+}
+
+static void _pwrdm_restore_clkdm_state(struct powerdomain *pwrdm,
+ u8 sleep_switch, bool hwsup)
+{
+ 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:
+ if (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
+ arch_pwrdm->pwrdm_set_lowpwrstchange)
+ arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
+ pwrdm_state_switch(pwrdm);
+ break;
+ }
+}
+
+/*
+ * This sets pwrdm state (other than mpu & core. Currently only ON &
+ * RET are supported.
+ */
+int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
+{
+ u8 next_pwrst, sleep_switch;
+ int ret = 0;
+ bool hwsup = false;
+
+ 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;
+
+ sleep_switch = _pwrdm_save_clkdm_state_and_activate(pwrdm, pwrst,
+ &hwsup);
+
+ ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
+ if (ret)
+ pr_err("%s: unable to set power state of powerdomain: %s\n",
+ __func__, pwrdm->name);
+
+ _pwrdm_restore_clkdm_state(pwrdm, sleep_switch, hwsup);
+
+ return ret;
+}
+
/**
* pwrdm_get_context_loss_count - get powerdomain's context loss count
* @pwrdm: struct powerdomain * to wait for
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 7c1534b..1edb3b7 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -228,10 +228,11 @@ 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_set_lowpwrstchange(struct powerdomain *pwrdm);
int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
+extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 state);
+
extern void omap242x_powerdomains_init(void);
extern void omap243x_powerdomains_init(void);
extern void omap3xxx_powerdomains_init(void);
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code
2012-12-09 1:23 ` [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code Paul Walmsley
@ 2012-12-12 9:31 ` Jean Pihet
2013-01-29 20:59 ` Paul Walmsley
2012-12-12 10:21 ` Vaibhav Hiremath
2012-12-12 10:31 ` Jean Pihet
2 siblings, 1 reply; 28+ messages in thread
From: Jean Pihet @ 2012-12-12 9:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
On Sun, Dec 9, 2012 at 2:23 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Move omap_set_pwrdm_state() from the PM code to the powerdomain code,
> and refactor it to split it up into several functions. A subsequent patch
> will rename it to conform with the existing powerdomain function names.
>
Glad to see this rather cryptic function getting a rewrite. It had never
been clear what the function is doing so I think it owes some more comments.
More comments below.
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Jean Pihet <jean.pihet@newoldbits.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> ---
> arch/arm/mach-omap2/pm.c | 61 --------------------
> arch/arm/mach-omap2/pm.h | 1
> arch/arm/mach-omap2/powerdomain.c | 112
> +++++++++++++++++++++++++++----------
> arch/arm/mach-omap2/powerdomain.h | 3 +
> 4 files changed, 85 insertions(+), 92 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index cc8ed0f..2e2a897 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -76,10 +76,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) &&
> @@ -92,63 +88,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_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.c
> b/arch/arm/mach-omap2/powerdomain.c
> index 97b3881..05f00660 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -921,35 +921,6 @@ bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm)
> return (pwrdm && pwrdm->flags & PWRDM_HAS_HDWR_SAR) ? 1 : 0;
> }
>
> -/**
> - * pwrdm_set_lowpwrstchange - Request a low power state change
> - * @pwrdm: struct powerdomain *
> - *
> - * Allows a powerdomain to transtion to a lower power sleep state
> - * from an existing sleep state without waking up the powerdomain.
> - * Returns -EINVAL if the powerdomain pointer is null or if the
> - * powerdomain does not support LOWPOWERSTATECHANGE, or returns 0
> - * upon success.
> - */
>
Can this kerneldoc be reused in the new code?
-int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
> -{
> - int ret = -EINVAL;
> -
> - if (!pwrdm)
> - return -EINVAL;
> -
> - if (!(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE))
> - return -EINVAL;
> -
> - pr_debug("powerdomain: %s: setting LOWPOWERSTATECHANGE bit\n",
> - pwrdm->name);
> -
> - if (arch_pwrdm && arch_pwrdm->pwrdm_set_lowpwrstchange)
> - ret = arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
> -
> - return ret;
> -}
> -
> int pwrdm_state_switch(struct powerdomain *pwrdm)
> {
> int ret;
> @@ -984,6 +955,89 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
> return 0;
> }
>
> +/* Types of sleep_switch used in omap_set_pwrdm_state */
> +#define ALREADYACTIVE_SWITCH 0
> +#define FORCEWAKEUP_SWITCH 1
> +#define LOWPOWERSTATE_SWITCH 2
>
Could you add some more documentation here?
What is the goal of the function, what does it return etc. ?
> +
> +static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
> + u8 pwrst, bool *hwsup)
+{
> + u8 curr_pwrst, sleep_switch;
> +
> + curr_pwrst = pwrdm_read_pwrst(pwrdm);
> + if (curr_pwrst < PWRDM_POWER_ON) {
> + if (curr_pwrst > pwrst &&
> + pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
> + arch_pwrdm->pwrdm_set_lowpwrstchange) {
> + sleep_switch = LOWPOWERSTATE_SWITCH;
> + } else {
> + *hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
> + clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> + sleep_switch = FORCEWAKEUP_SWITCH;
> + }
> + } else {
> + sleep_switch = ALREADYACTIVE_SWITCH;
> + }
> +
> + return sleep_switch;
> +}
> +
>
Same here
> +static void _pwrdm_restore_clkdm_state(struct powerdomain *pwrdm,
> + u8 sleep_switch, bool hwsup)
> +{
> + 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:
> + if (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
> + arch_pwrdm->pwrdm_set_lowpwrstchange)
> + arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
> + pwrdm_state_switch(pwrdm);
> + break;
> + }
> +}
> +
> +/*
> + * This sets pwrdm state (other than mpu & core. Currently only ON &
> + * RET are supported.
> + */
>
Same here.
Is this one correct?
Regards,
Jean
> +int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
> +{
> + u8 next_pwrst, sleep_switch;
> + int ret = 0;
> + bool hwsup = false;
> +
> + 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;
> +
> + sleep_switch = _pwrdm_save_clkdm_state_and_activate(pwrdm, pwrst,
> + &hwsup);
> +
> + ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
> + if (ret)
> + pr_err("%s: unable to set power state of powerdomain:
> %s\n",
> + __func__, pwrdm->name);
> +
> + _pwrdm_restore_clkdm_state(pwrdm, sleep_switch, hwsup);
> +
> + return ret;
> +}
> +
> /**
> * pwrdm_get_context_loss_count - get powerdomain's context loss count
> * @pwrdm: struct powerdomain * to wait for
> diff --git a/arch/arm/mach-omap2/powerdomain.h
> b/arch/arm/mach-omap2/powerdomain.h
> index 7c1534b..1edb3b7 100644
> --- a/arch/arm/mach-omap2/powerdomain.h
> +++ b/arch/arm/mach-omap2/powerdomain.h
> @@ -228,10 +228,11 @@ 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_set_lowpwrstchange(struct powerdomain *pwrdm);
> int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
> bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
>
> +extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 state);
> +
> extern void omap242x_powerdomains_init(void);
> extern void omap243x_powerdomains_init(void);
> extern void omap3xxx_powerdomains_init(void);
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121212/3e4e3a15/attachment-0001.html>
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code
2012-12-12 9:31 ` Jean Pihet
@ 2013-01-29 20:59 ` Paul Walmsley
0 siblings, 0 replies; 28+ messages in thread
From: Paul Walmsley @ 2013-01-29 20:59 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
(redacted irrelevant code sections)
On Wed, 12 Dec 2012, Jean Pihet wrote:
> On Sun, Dec 9, 2012 at 2:23 AM, Paul Walmsley <paul@pwsan.com> wrote:
>
> > -/**
> > - * pwrdm_set_lowpwrstchange - Request a low power state change
> > - * @pwrdm: struct powerdomain *
> > - *
> > - * Allows a powerdomain to transtion to a lower power sleep state
> > - * from an existing sleep state without waking up the powerdomain.
> > - * Returns -EINVAL if the powerdomain pointer is null or if the
> > - * powerdomain does not support LOWPOWERSTATECHANGE, or returns 0
> > - * upon success.
> > - */
> >
>
> Can this kerneldoc be reused in the new code?
Not directly, since the function is being removed. But I've added some
LOWPOWERSTATECHANGE-related documentation in powerdomain.h in the updated
patch (below).
> > @@ -984,6 +955,89 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
> > return 0;
> > }
> >
> > +/* Types of sleep_switch used in omap_set_pwrdm_state */
> > +#define ALREADYACTIVE_SWITCH 0
> > +#define FORCEWAKEUP_SWITCH 1
> > +#define LOWPOWERSTATE_SWITCH 2
> >
>
> Could you add some more documentation here?
> What is the goal of the function, what does it return etc. ?
I've added some related kerneldoc to omap_set_pwrdm_state().
> > +
> > +static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
> > + u8 pwrst, bool *hwsup)
>
> Same here
>
> > +static void _pwrdm_restore_clkdm_state(struct powerdomain *pwrdm,
> > + u8 sleep_switch, bool hwsup)
>
> Same here.
I've added kerneldoc for both of these functions.
> > +/*
> > + * This sets pwrdm state (other than mpu & core. Currently only ON &
> > + * RET are supported.
> > + */
> >
> Is this one correct?
This is just a cut and paste from the previous comments in the file. I
agree that it is not helpful, so I've added kerneldoc for
omap_set_pwrdm_state().
The updated patch follows.
- Paul
From: Paul Walmsley <paul@pwsan.com>
Date: Tue, 29 Jan 2013 13:45:09 -0700
Subject: [PATCH] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to
powerdomain code
Move omap_set_pwrdm_state() from the PM code to the powerdomain code,
and refactor it to split it up into several functions. A subsequent patch
will rename it to conform with the existing powerdomain function names.
This version includes some additional documentation, based on a
suggestion from Jean Pihet. It also modifies omap_set_pwrdm_state()
to not bail out early unless both the powerdomain current power state
and the next power state are equal. (Previously it would terminate
early if the next power state was equal to the target power state,
which was insufficiently rigorous.)
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Jean Pihet <jean.pihet@newoldbits.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Tero Kristo <t-kristo@ti.com>
---
arch/arm/mach-omap2/pm.c | 61 --------------
arch/arm/mach-omap2/pm.h | 1 -
arch/arm/mach-omap2/powerdomain.c | 161 ++++++++++++++++++++++++++++++-------
arch/arm/mach-omap2/powerdomain.h | 13 ++-
4 files changed, 144 insertions(+), 92 deletions(-)
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index f18afc9..48d6d5d 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -108,10 +108,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) &&
@@ -124,63 +120,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_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 c22503b..7bdd22a 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.c b/arch/arm/mach-omap2/powerdomain.c
index 97b3881..65c3464 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -42,6 +42,16 @@ enum {
PWRDM_STATE_PREV,
};
+/*
+ * Types of sleep_switch used internally in omap_set_pwrdm_state()
+ * and its associated static functions
+ *
+ * XXX Better documentation is needed here
+ */
+#define ALREADYACTIVE_SWITCH 0
+#define FORCEWAKEUP_SWITCH 1
+#define LOWPOWERSTATE_SWITCH 2
+#define ERROR_SWITCH 3
/* pwrdm_list contains all registered struct powerdomains */
static LIST_HEAD(pwrdm_list);
@@ -200,6 +210,80 @@ static int _pwrdm_post_transition_cb(struct powerdomain *pwrdm, void *unused)
return 0;
}
+/**
+ * _pwrdm_save_clkdm_state_and_activate - prepare for power state change
+ * @pwrdm: struct powerdomain * to operate on
+ * @curr_pwrst: current power state of @pwrdm
+ * @pwrst: power state to switch to
+ * @hwsup: ptr to a bool to return whether the clkdm is hardware-supervised
+ *
+ * Determine whether the powerdomain needs to be turned on before
+ * attempting to switch power states. Called by
+ * omap_set_pwrdm_state(). NOTE that if the powerdomain contains
+ * multiple clockdomains, this code assumes that the first clockdomain
+ * supports software-supervised wakeup mode - potentially a problem.
+ * Returns the power state switch mode currently in use (see the
+ * "Types of sleep_switch" comment above).
+ */
+static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
+ u8 curr_pwrst, u8 pwrst,
+ bool *hwsup)
+{
+ u8 sleep_switch;
+
+ if (curr_pwrst < 0) {
+ WARN_ON(1);
+ sleep_switch = ERROR_SWITCH;
+ } else if (curr_pwrst < PWRDM_POWER_ON) {
+ if (curr_pwrst > pwrst &&
+ pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
+ arch_pwrdm->pwrdm_set_lowpwrstchange) {
+ sleep_switch = LOWPOWERSTATE_SWITCH;
+ } else {
+ *hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
+ clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
+ sleep_switch = FORCEWAKEUP_SWITCH;
+ }
+ } else {
+ sleep_switch = ALREADYACTIVE_SWITCH;
+ }
+
+ return sleep_switch;
+}
+
+/**
+ * _pwrdm_restore_clkdm_state - restore the clkdm hwsup state after pwrst change
+ * @pwrdm: struct powerdomain * to operate on
+ * @sleep_switch: return value from _pwrdm_save_clkdm_state_and_activate()
+ * @hwsup: should @pwrdm's first clockdomain be set to hardware-supervised mode?
+ *
+ * Restore the clockdomain state perturbed by
+ * _pwrdm_save_clkdm_state_and_activate(), and call the power state
+ * bookkeeping code. Called by omap_set_pwrdm_state(). NOTE that if
+ * the powerdomain contains multiple clockdomains, this assumes that
+ * the first associated clockdomain supports either
+ * hardware-supervised idle control in the register, or
+ * software-supervised sleep. No return value.
+ */
+static void _pwrdm_restore_clkdm_state(struct powerdomain *pwrdm,
+ u8 sleep_switch, bool hwsup)
+{
+ 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:
+ if (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
+ arch_pwrdm->pwrdm_set_lowpwrstchange)
+ arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
+ pwrdm_state_switch(pwrdm);
+ break;
+ }
+}
+
/* Public functions */
/**
@@ -921,35 +1005,6 @@ bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm)
return (pwrdm && pwrdm->flags & PWRDM_HAS_HDWR_SAR) ? 1 : 0;
}
-/**
- * pwrdm_set_lowpwrstchange - Request a low power state change
- * @pwrdm: struct powerdomain *
- *
- * Allows a powerdomain to transtion to a lower power sleep state
- * from an existing sleep state without waking up the powerdomain.
- * Returns -EINVAL if the powerdomain pointer is null or if the
- * powerdomain does not support LOWPOWERSTATECHANGE, or returns 0
- * upon success.
- */
-int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
-{
- int ret = -EINVAL;
-
- if (!pwrdm)
- return -EINVAL;
-
- if (!(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE))
- return -EINVAL;
-
- pr_debug("powerdomain: %s: setting LOWPOWERSTATECHANGE bit\n",
- pwrdm->name);
-
- if (arch_pwrdm && arch_pwrdm->pwrdm_set_lowpwrstchange)
- ret = arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
-
- return ret;
-}
-
int pwrdm_state_switch(struct powerdomain *pwrdm)
{
int ret;
@@ -985,6 +1040,54 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
}
/**
+ * omap_set_pwrdm_state - change a powerdomain's current power state
+ * @pwrdm: struct powerdomain * to change the power state of
+ * @pwrst: power state to change to
+ *
+ * Change the current hardware power state of the powerdomain
+ * represented by @pwrdm to the power state represented by @pwrst.
+ * Returns -EINVAL if @pwrdm is null or invalid or if the
+ * powerdomain's current power state could not be read, or returns 0
+ * upon success or if @pwrdm does not support @pwrst or any
+ * lower-power state. XXX Should not return 0 if the @pwrdm does not
+ * support @pwrst or any lower-power state: this should be an error.
+ */
+int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
+{
+ u8 curr_pwrst, next_pwrst, sleep_switch;
+ int ret = 0;
+ bool hwsup = false;
+
+ if (!pwrdm || IS_ERR(pwrdm))
+ return -EINVAL;
+
+ while (!(pwrdm->pwrsts & (1 << pwrst))) {
+ if (pwrst == PWRDM_POWER_OFF)
+ return ret;
+ pwrst--;
+ }
+
+ curr_pwrst = pwrdm_read_pwrst(pwrdm);
+ next_pwrst = pwrdm_read_next_pwrst(pwrdm);
+ if (curr_pwrst == pwrst && next_pwrst == pwrst)
+ return ret;
+
+ sleep_switch = _pwrdm_save_clkdm_state_and_activate(pwrdm, curr_pwrst,
+ pwrst, &hwsup);
+ if (sleep_switch == ERROR_SWITCH)
+ return -EINVAL;
+
+ ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
+ if (ret)
+ pr_err("%s: unable to set power state of powerdomain: %s\n",
+ __func__, pwrdm->name);
+
+ _pwrdm_restore_clkdm_state(pwrdm, sleep_switch, hwsup);
+
+ return ret;
+}
+
+/**
* pwrdm_get_context_loss_count - get powerdomain's context loss count
* @pwrdm: struct powerdomain * to wait for
*
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 7c1534b..93e7df8 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -162,6 +162,16 @@ struct powerdomain {
* @pwrdm_disable_hdwr_sar: Disable Hardware Save-Restore feature for a pd
* @pwrdm_set_lowpwrstchange: Enable pd transitions from a shallow to deep sleep
* @pwrdm_wait_transition: Wait for a pd state transition to complete
+ *
+ * Regarding @pwrdm_set_lowpwrstchange: On the OMAP2 and 3-family
+ * chips, a powerdomain's power state is not allowed to directly
+ * transition from one low-power state (e.g., CSWR) to another
+ * low-power state (e.g., OFF) without first waking up the
+ * powerdomain. This wastes energy. So OMAP4 chips support the
+ * ability to transition a powerdomain power state directly from one
+ * low-power state to another. The function pointed to by
+ * @pwrdm_set_lowpwrstchange is intended to configure the OMAP4
+ * hardware powerdomain state machine to enable this feature.
*/
struct pwrdm_ops {
int (*pwrdm_set_next_pwrst)(struct powerdomain *pwrdm, u8 pwrst);
@@ -228,10 +238,11 @@ 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_set_lowpwrstchange(struct powerdomain *pwrdm);
int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
+extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 state);
+
extern void omap242x_powerdomains_init(void);
extern void omap243x_powerdomains_init(void);
extern void omap3xxx_powerdomains_init(void);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code
2012-12-09 1:23 ` [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code Paul Walmsley
2012-12-12 9:31 ` Jean Pihet
@ 2012-12-12 10:21 ` Vaibhav Hiremath
2013-01-09 17:43 ` Russell King - ARM Linux
2012-12-12 10:31 ` Jean Pihet
2 siblings, 1 reply; 28+ messages in thread
From: Vaibhav Hiremath @ 2012-12-12 10:21 UTC (permalink / raw)
To: linux-arm-kernel
Minor comment below,
On 12/9/2012 6:53 AM, Paul Walmsley wrote:
> Move omap_set_pwrdm_state() from the PM code to the powerdomain code,
> and refactor it to split it up into several functions. A subsequent patch
> will rename it to conform with the existing powerdomain function names.
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Jean Pihet <jean.pihet@newoldbits.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> ---
> arch/arm/mach-omap2/pm.c | 61 --------------------
> arch/arm/mach-omap2/pm.h | 1
> arch/arm/mach-omap2/powerdomain.c | 112 +++++++++++++++++++++++++++----------
> arch/arm/mach-omap2/powerdomain.h | 3 +
> 4 files changed, 85 insertions(+), 92 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index cc8ed0f..2e2a897 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -76,10 +76,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) &&
> @@ -92,63 +88,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))
You can use IS_ERR_OR_NULL here.
Thanks,
Vaibhav
> - 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_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.c b/arch/arm/mach-omap2/powerdomain.c
> index 97b3881..05f00660 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -921,35 +921,6 @@ bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm)
> return (pwrdm && pwrdm->flags & PWRDM_HAS_HDWR_SAR) ? 1 : 0;
> }
>
> -/**
> - * pwrdm_set_lowpwrstchange - Request a low power state change
> - * @pwrdm: struct powerdomain *
> - *
> - * Allows a powerdomain to transtion to a lower power sleep state
> - * from an existing sleep state without waking up the powerdomain.
> - * Returns -EINVAL if the powerdomain pointer is null or if the
> - * powerdomain does not support LOWPOWERSTATECHANGE, or returns 0
> - * upon success.
> - */
> -int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
> -{
> - int ret = -EINVAL;
> -
> - if (!pwrdm)
> - return -EINVAL;
> -
> - if (!(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE))
> - return -EINVAL;
> -
> - pr_debug("powerdomain: %s: setting LOWPOWERSTATECHANGE bit\n",
> - pwrdm->name);
> -
> - if (arch_pwrdm && arch_pwrdm->pwrdm_set_lowpwrstchange)
> - ret = arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
> -
> - return ret;
> -}
> -
> int pwrdm_state_switch(struct powerdomain *pwrdm)
> {
> int ret;
> @@ -984,6 +955,89 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
> return 0;
> }
>
> +/* Types of sleep_switch used in omap_set_pwrdm_state */
> +#define ALREADYACTIVE_SWITCH 0
> +#define FORCEWAKEUP_SWITCH 1
> +#define LOWPOWERSTATE_SWITCH 2
> +
> +static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
> + u8 pwrst, bool *hwsup)
> +{
> + u8 curr_pwrst, sleep_switch;
> +
> + curr_pwrst = pwrdm_read_pwrst(pwrdm);
> + if (curr_pwrst < PWRDM_POWER_ON) {
> + if (curr_pwrst > pwrst &&
> + pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
> + arch_pwrdm->pwrdm_set_lowpwrstchange) {
> + sleep_switch = LOWPOWERSTATE_SWITCH;
> + } else {
> + *hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
> + clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> + sleep_switch = FORCEWAKEUP_SWITCH;
> + }
> + } else {
> + sleep_switch = ALREADYACTIVE_SWITCH;
> + }
> +
> + return sleep_switch;
> +}
> +
> +static void _pwrdm_restore_clkdm_state(struct powerdomain *pwrdm,
> + u8 sleep_switch, bool hwsup)
> +{
> + 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:
> + if (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
> + arch_pwrdm->pwrdm_set_lowpwrstchange)
> + arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
> + pwrdm_state_switch(pwrdm);
> + break;
> + }
> +}
> +
> +/*
> + * This sets pwrdm state (other than mpu & core. Currently only ON &
> + * RET are supported.
> + */
> +int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
> +{
> + u8 next_pwrst, sleep_switch;
> + int ret = 0;
> + bool hwsup = false;
> +
> + 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;
> +
> + sleep_switch = _pwrdm_save_clkdm_state_and_activate(pwrdm, pwrst,
> + &hwsup);
> +
> + ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
> + if (ret)
> + pr_err("%s: unable to set power state of powerdomain: %s\n",
> + __func__, pwrdm->name);
> +
> + _pwrdm_restore_clkdm_state(pwrdm, sleep_switch, hwsup);
> +
> + return ret;
> +}
> +
> /**
> * pwrdm_get_context_loss_count - get powerdomain's context loss count
> * @pwrdm: struct powerdomain * to wait for
> diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
> index 7c1534b..1edb3b7 100644
> --- a/arch/arm/mach-omap2/powerdomain.h
> +++ b/arch/arm/mach-omap2/powerdomain.h
> @@ -228,10 +228,11 @@ 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_set_lowpwrstchange(struct powerdomain *pwrdm);
> int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
> bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
>
> +extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 state);
> +
> extern void omap242x_powerdomains_init(void);
> extern void omap243x_powerdomains_init(void);
> extern void omap3xxx_powerdomains_init(void);
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code
2012-12-12 10:21 ` Vaibhav Hiremath
@ 2013-01-09 17:43 ` Russell King - ARM Linux
0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2013-01-09 17:43 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 12, 2012 at 03:51:49PM +0530, Vaibhav Hiremath wrote:
> > -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))
>
> You can use IS_ERR_OR_NULL here.
As this is removing code...
However, if you pay attention to linux-kernel or linux-arm-kernel, you will
notice that I've sent a patch deprecating IS_ERR_OR_NULL() and I'm starting
to remove its use. IS_ERR_OR_NULL() creates more problems than it solves
because people don't think about what the hell they're doing with it, or
even whether it's appropriate to use it.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code
2012-12-09 1:23 ` [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code Paul Walmsley
2012-12-12 9:31 ` Jean Pihet
2012-12-12 10:21 ` Vaibhav Hiremath
@ 2012-12-12 10:31 ` Jean Pihet
2 siblings, 0 replies; 28+ messages in thread
From: Jean Pihet @ 2012-12-12 10:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
-resending in plain text only, sorry about that-
On Sun, Dec 9, 2012 at 2:23 AM, Paul Walmsley <paul@pwsan.com> wrote:
>
> Move omap_set_pwrdm_state() from the PM code to the powerdomain code,
> and refactor it to split it up into several functions. A subsequent patch
> will rename it to conform with the existing powerdomain function names.
Glad to see this rather cryptic function getting a rewrite. It had
never been clear what the function is doing so I think it owes some
more comments.
More comments below.
>
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Jean Pihet <jean.pihet@newoldbits.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> ---
> arch/arm/mach-omap2/pm.c | 61 --------------------
> arch/arm/mach-omap2/pm.h | 1
> arch/arm/mach-omap2/powerdomain.c | 112 +++++++++++++++++++++++++++----------
> arch/arm/mach-omap2/powerdomain.h | 3 +
> 4 files changed, 85 insertions(+), 92 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index cc8ed0f..2e2a897 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -76,10 +76,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) &&
> @@ -92,63 +88,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_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.c b/arch/arm/mach-omap2/powerdomain.c
> index 97b3881..05f00660 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -921,35 +921,6 @@ bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm)
> return (pwrdm && pwrdm->flags & PWRDM_HAS_HDWR_SAR) ? 1 : 0;
> }
>
> -/**
> - * pwrdm_set_lowpwrstchange - Request a low power state change
> - * @pwrdm: struct powerdomain *
> - *
> - * Allows a powerdomain to transtion to a lower power sleep state
> - * from an existing sleep state without waking up the powerdomain.
> - * Returns -EINVAL if the powerdomain pointer is null or if the
> - * powerdomain does not support LOWPOWERSTATECHANGE, or returns 0
> - * upon success.
> - */
Can this kerneldoc be reused in the new code?
Could you add some more documentation here?
What is the goal of the function, what does it return etc. ?
>
> +
> +static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
> + u8 pwrst, bool *hwsup)
>
> +{
> + u8 curr_pwrst, sleep_switch;
> +
> + curr_pwrst = pwrdm_read_pwrst(pwrdm);
> + if (curr_pwrst < PWRDM_POWER_ON) {
> + if (curr_pwrst > pwrst &&
> + pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
> + arch_pwrdm->pwrdm_set_lowpwrstchange) {
> + sleep_switch = LOWPOWERSTATE_SWITCH;
> + } else {
> + *hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
> + clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> + sleep_switch = FORCEWAKEUP_SWITCH;
> + }
> + } else {
> + sleep_switch = ALREADYACTIVE_SWITCH;
> + }
> +
> + return sleep_switch;
> +}
> +
Same here
>
> +static void _pwrdm_restore_clkdm_state(struct powerdomain *pwrdm,
> + u8 sleep_switch, bool hwsup)
> +{
> + 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:
> + if (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
> + arch_pwrdm->pwrdm_set_lowpwrstchange)
> + arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
> + pwrdm_state_switch(pwrdm);
> + break;
> + }
> +}
> +
> +/*
> + * This sets pwrdm state (other than mpu & core. Currently only ON &
> + * RET are supported.
> + */
Same here.
Is this one correct?
Regards,
Jean
+int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
+{
+ u8 next_pwrst, sleep_switch;
+ int ret = 0;
+ bool hwsup = false;
+
+ 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;
+
+ sleep_switch = _pwrdm_save_clkdm_state_and_
>
> activate(pwrdm, pwrst,
> + &hwsup);
> +
> + ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
> + if (ret)
> + pr_err("%s: unable to set power state of powerdomain: %s\n",
> + __func__, pwrdm->name);
> +
> + _pwrdm_restore_clkdm_state(pwrdm, sleep_switch, hwsup);
> +
> + return ret;
> +}
> +
> /**
> * pwrdm_get_context_loss_count - get powerdomain's context loss count
> * @pwrdm: struct powerdomain * to wait for
> diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
> index 7c1534b..1edb3b7 100644
> --- a/arch/arm/mach-omap2/powerdomain.h
> +++ b/arch/arm/mach-omap2/powerdomain.h
> @@ -228,10 +228,11 @@ 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_set_lowpwrstchange(struct powerdomain *pwrdm);
> int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
> bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
>
> +extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 state);
> +
> extern void omap242x_powerdomains_init(void);
> extern void omap243x_powerdomains_init(void);
> extern void omap3xxx_powerdomains_init(void);
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 07/10] ARM: OMAP2xxx: CM: remove autodep handling
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
` (4 preceding siblings ...)
2012-12-09 1:23 ` [PATCH 05/10] ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2012-12-09 1:23 ` [PATCH 06/10] ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock Paul Walmsley
` (4 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
There's no need to preserve the autodep handling code in
mach-omap2/cm2xxx.c, since no autodeps are defined for these chips.
Hopefully they'll never be needed, but if in some future case they are,
this code can be added back in.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/cm2xxx.c | 33 ++++-----------------------------
1 file changed, 4 insertions(+), 29 deletions(-)
diff --git a/arch/arm/mach-omap2/cm2xxx.c b/arch/arm/mach-omap2/cm2xxx.c
index db65069..6774a53 100644
--- a/arch/arm/mach-omap2/cm2xxx.c
+++ b/arch/arm/mach-omap2/cm2xxx.c
@@ -273,9 +273,6 @@ int omap2xxx_cm_wait_module_ready(s16 prcm_mod, u8 idlest_id, u8 idlest_shift)
static void omap2xxx_clkdm_allow_idle(struct clockdomain *clkdm)
{
- if (atomic_read(&clkdm->usecount) > 0)
- _clkdm_add_autodeps(clkdm);
-
omap2xxx_cm_clkdm_enable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
}
@@ -284,9 +281,6 @@ static void omap2xxx_clkdm_deny_idle(struct clockdomain *clkdm)
{
omap2xxx_cm_clkdm_disable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
-
- if (atomic_read(&clkdm->usecount) > 0)
- _clkdm_del_autodeps(clkdm);
}
static int omap2xxx_clkdm_clk_enable(struct clockdomain *clkdm)
@@ -298,18 +292,8 @@ static int omap2xxx_clkdm_clk_enable(struct clockdomain *clkdm)
hwsup = omap2xxx_cm_is_clkdm_in_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
-
- if (hwsup) {
- /* Disable HW transitions when we are changing deps */
- omap2xxx_cm_clkdm_disable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
- clkdm->clktrctrl_mask);
- _clkdm_add_autodeps(clkdm);
- omap2xxx_cm_clkdm_enable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
- clkdm->clktrctrl_mask);
- } else {
- if (clkdm->flags & CLKDM_CAN_FORCE_WAKEUP)
- omap2xxx_clkdm_wakeup(clkdm);
- }
+ if (!hwsup && clkdm->flags & CLKDM_CAN_FORCE_WAKEUP)
+ omap2xxx_clkdm_wakeup(clkdm);
return 0;
}
@@ -324,17 +308,8 @@ static int omap2xxx_clkdm_clk_disable(struct clockdomain *clkdm)
hwsup = omap2xxx_cm_is_clkdm_in_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
- if (hwsup) {
- /* Disable HW transitions when we are changing deps */
- omap2xxx_cm_clkdm_disable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
- clkdm->clktrctrl_mask);
- _clkdm_del_autodeps(clkdm);
- omap2xxx_cm_clkdm_enable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
- clkdm->clktrctrl_mask);
- } else {
- if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP)
- omap2xxx_clkdm_sleep(clkdm);
- }
+ if (!hwsup && clkdm->flags & CLKDM_CAN_FORCE_SLEEP)
+ omap2xxx_clkdm_sleep(clkdm);
return 0;
}
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 06/10] ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
` (5 preceding siblings ...)
2012-12-09 1:23 ` [PATCH 07/10] ARM: OMAP2xxx: CM: remove autodep handling Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2012-12-12 9:41 ` Jean Pihet
2012-12-12 10:28 ` Jean Pihet
2012-12-09 1:23 ` [PATCH 08/10] ARM: OMAP2+: clockdomain: work on wkdep/sleepdep functions Paul Walmsley
` (3 subsequent siblings)
10 siblings, 2 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
Add a per-powerdomain spinlock. Use that instead of the clockdomain
spinlock. Add pwrdm_lock()/pwrdm_unlock() functions to allow other
code to acquire or release the powerdomain spinlock without reaching
directly into the struct powerdomain.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Jean Pihet <jean.pihet@newoldbits.com>
---
arch/arm/mach-omap2/clockdomain-powerdomain.h | 22 +++
arch/arm/mach-omap2/clockdomain.c | 161 +++++++++++++++++--------
arch/arm/mach-omap2/clockdomain.h | 6 +
arch/arm/mach-omap2/powerdomain-clockdomain.h | 27 ++++
arch/arm/mach-omap2/powerdomain.c | 56 ++++++++-
arch/arm/mach-omap2/powerdomain.h | 11 +-
6 files changed, 219 insertions(+), 64 deletions(-)
create mode 100644 arch/arm/mach-omap2/clockdomain-powerdomain.h
create mode 100644 arch/arm/mach-omap2/powerdomain-clockdomain.h
diff --git a/arch/arm/mach-omap2/clockdomain-powerdomain.h b/arch/arm/mach-omap2/clockdomain-powerdomain.h
new file mode 100644
index 0000000..8beee2d
--- /dev/null
+++ b/arch/arm/mach-omap2/clockdomain-powerdomain.h
@@ -0,0 +1,22 @@
+/*
+ * OMAP clockdomain framework functions for use only by the powerdomain code
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ * Paul Walmsley
+ *
+ * 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.
+ */
+
+#ifndef __ARCH_ARM_MACH_OMAP2_CLOCKDOMAIN_POWERDOMAIN_H
+#define __ARCH_ARM_MACH_OMAP2_CLOCKDOMAIN_POWERDOMAIN_H
+
+#include "clockdomain.h"
+
+extern void clkdm_allow_idle_nolock(struct clockdomain *clkdm);
+extern void clkdm_deny_idle_nolock(struct clockdomain *clkdm);
+extern int clkdm_wakeup_nolock(struct clockdomain *clkdm);
+extern int clkdm_sleep_nolock(struct clockdomain *clkdm);
+
+#endif
diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 18f65fd..2142dab 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -30,6 +30,7 @@
#include "soc.h"
#include "clock.h"
#include "clockdomain.h"
+#include "powerdomain-clockdomain.h"
/* clkdm_list contains all registered struct clockdomains */
static LIST_HEAD(clkdm_list);
@@ -91,8 +92,6 @@ static int _clkdm_register(struct clockdomain *clkdm)
pwrdm_add_clkdm(pwrdm, clkdm);
- spin_lock_init(&clkdm->lock);
-
pr_debug("clockdomain: registered %s\n", clkdm->name);
return 0;
@@ -733,18 +732,17 @@ int clkdm_clear_all_sleepdeps(struct clockdomain *clkdm)
}
/**
- * clkdm_sleep - force clockdomain sleep transition
+ * clkdm_sleep_nolock - force clockdomain sleep transition (lockless)
* @clkdm: struct clockdomain *
*
* Instruct the CM to force a sleep transition on the specified
- * clockdomain @clkdm. Returns -EINVAL if @clkdm is NULL or if
- * clockdomain does not support software-initiated sleep; 0 upon
- * success.
+ * clockdomain @clkdm. Only for use by the powerdomain code. Returns
+ * -EINVAL if @clkdm is NULL or if clockdomain does not support
+ * software-initiated sleep; 0 upon success.
*/
-int clkdm_sleep(struct clockdomain *clkdm)
+int clkdm_sleep_nolock(struct clockdomain *clkdm)
{
int ret;
- unsigned long flags;
if (!clkdm)
return -EINVAL;
@@ -760,27 +758,45 @@ int clkdm_sleep(struct clockdomain *clkdm)
pr_debug("clockdomain: forcing sleep on %s\n", clkdm->name);
- spin_lock_irqsave(&clkdm->lock, flags);
clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
ret = arch_clkdm->clkdm_sleep(clkdm);
- ret |= pwrdm_state_switch(clkdm->pwrdm.ptr);
- spin_unlock_irqrestore(&clkdm->lock, flags);
+ ret |= pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
+
return ret;
}
/**
- * clkdm_wakeup - force clockdomain wakeup transition
+ * clkdm_sleep - force clockdomain sleep transition
* @clkdm: struct clockdomain *
*
- * Instruct the CM to force a wakeup transition on the specified
- * clockdomain @clkdm. Returns -EINVAL if @clkdm is NULL or if the
- * clockdomain does not support software-controlled wakeup; 0 upon
+ * Instruct the CM to force a sleep transition on the specified
+ * clockdomain @clkdm. Returns -EINVAL if @clkdm is NULL or if
+ * clockdomain does not support software-initiated sleep; 0 upon
* success.
*/
-int clkdm_wakeup(struct clockdomain *clkdm)
+int clkdm_sleep(struct clockdomain *clkdm)
+{
+ int ret;
+
+ pwrdm_lock(clkdm->pwrdm.ptr);
+ ret = clkdm_sleep_nolock(clkdm);
+ pwrdm_unlock(clkdm->pwrdm.ptr);
+
+ return ret;
+}
+
+/**
+ * clkdm_wakeup_nolock - force clockdomain wakeup transition (lockless)
+ * @clkdm: struct clockdomain *
+ *
+ * Instruct the CM to force a wakeup transition on the specified
+ * clockdomain @clkdm. Only for use by the powerdomain code. Returns
+ * -EINVAL if @clkdm is NULL or if the clockdomain does not support
+ * software-controlled wakeup; 0 upon success.
+ */
+int clkdm_wakeup_nolock(struct clockdomain *clkdm)
{
int ret;
- unsigned long flags;
if (!clkdm)
return -EINVAL;
@@ -796,28 +812,46 @@ int clkdm_wakeup(struct clockdomain *clkdm)
pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
- spin_lock_irqsave(&clkdm->lock, flags);
clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
ret = arch_clkdm->clkdm_wakeup(clkdm);
- ret |= pwrdm_state_switch(clkdm->pwrdm.ptr);
- spin_unlock_irqrestore(&clkdm->lock, flags);
+ ret |= pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
+
return ret;
}
/**
- * clkdm_allow_idle - enable hwsup idle transitions for clkdm
+ * clkdm_wakeup - force clockdomain wakeup transition
* @clkdm: struct clockdomain *
*
- * Allow the hardware to automatically switch the clockdomain @clkdm into
- * active or idle states, as needed by downstream clocks. If the
+ * Instruct the CM to force a wakeup transition on the specified
+ * clockdomain @clkdm. Returns -EINVAL if @clkdm is NULL or if the
+ * clockdomain does not support software-controlled wakeup; 0 upon
+ * success.
+ */
+int clkdm_wakeup(struct clockdomain *clkdm)
+{
+ int ret;
+
+ pwrdm_lock(clkdm->pwrdm.ptr);
+ ret = clkdm_wakeup_nolock(clkdm);
+ pwrdm_unlock(clkdm->pwrdm.ptr);
+
+ return ret;
+}
+
+/**
+ * clkdm_allow_idle_nolock - enable hwsup idle transitions for clkdm
+ * @clkdm: struct clockdomain *
+ *
+ * Allow the hardware to automatically switch the clockdomain @clkdm
+ * into active or idle states, as needed by downstream clocks. If the
* clockdomain has any downstream clocks enabled in the clock
* framework, wkdep/sleepdep autodependencies are added; this is so
- * device drivers can read and write to the device. No return value.
+ * device drivers can read and write to the device. Only for use by
+ * the powerdomain code. No return value.
*/
-void clkdm_allow_idle(struct clockdomain *clkdm)
+void clkdm_allow_idle_nolock(struct clockdomain *clkdm)
{
- unsigned long flags;
-
if (!clkdm)
return;
@@ -833,11 +867,26 @@ void clkdm_allow_idle(struct clockdomain *clkdm)
pr_debug("clockdomain: enabling automatic idle transitions for %s\n",
clkdm->name);
- spin_lock_irqsave(&clkdm->lock, flags);
clkdm->_flags |= _CLKDM_FLAG_HWSUP_ENABLED;
arch_clkdm->clkdm_allow_idle(clkdm);
- pwrdm_state_switch(clkdm->pwrdm.ptr);
- spin_unlock_irqrestore(&clkdm->lock, flags);
+ pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
+}
+
+/**
+ * clkdm_allow_idle - enable hwsup idle transitions for clkdm
+ * @clkdm: struct clockdomain *
+ *
+ * Allow the hardware to automatically switch the clockdomain @clkdm into
+ * active or idle states, as needed by downstream clocks. If the
+ * clockdomain has any downstream clocks enabled in the clock
+ * framework, wkdep/sleepdep autodependencies are added; this is so
+ * device drivers can read and write to the device. No return value.
+ */
+void clkdm_allow_idle(struct clockdomain *clkdm)
+{
+ pwrdm_lock(clkdm->pwrdm.ptr);
+ clkdm_allow_idle_nolock(clkdm);
+ pwrdm_unlock(clkdm->pwrdm.ptr);
}
/**
@@ -847,12 +896,11 @@ void clkdm_allow_idle(struct clockdomain *clkdm)
* Prevent the hardware from automatically switching the clockdomain
* @clkdm into inactive or idle states. If the clockdomain has
* downstream clocks enabled in the clock framework, wkdep/sleepdep
- * autodependencies are removed. No return value.
+ * autodependencies are removed. Only for use by the powerdomain
+ * code. No return value.
*/
-void clkdm_deny_idle(struct clockdomain *clkdm)
+void clkdm_deny_idle_nolock(struct clockdomain *clkdm)
{
- unsigned long flags;
-
if (!clkdm)
return;
@@ -868,11 +916,25 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
pr_debug("clockdomain: disabling automatic idle transitions for %s\n",
clkdm->name);
- spin_lock_irqsave(&clkdm->lock, flags);
clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
arch_clkdm->clkdm_deny_idle(clkdm);
- pwrdm_state_switch(clkdm->pwrdm.ptr);
- spin_unlock_irqrestore(&clkdm->lock, flags);
+ pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
+}
+
+/**
+ * clkdm_deny_idle - disable hwsup idle transitions for clkdm
+ * @clkdm: struct clockdomain *
+ *
+ * Prevent the hardware from automatically switching the clockdomain
+ * @clkdm into inactive or idle states. If the clockdomain has
+ * downstream clocks enabled in the clock framework, wkdep/sleepdep
+ * autodependencies are removed. No return value.
+ */
+void clkdm_deny_idle(struct clockdomain *clkdm)
+{
+ pwrdm_lock(clkdm->pwrdm.ptr);
+ clkdm_deny_idle_nolock(clkdm);
+ pwrdm_unlock(clkdm->pwrdm.ptr);
}
/**
@@ -889,14 +951,11 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
bool clkdm_in_hwsup(struct clockdomain *clkdm)
{
bool ret;
- unsigned long flags;
if (!clkdm)
return false;
- spin_lock_irqsave(&clkdm->lock, flags);
ret = (clkdm->_flags & _CLKDM_FLAG_HWSUP_ENABLED) ? true : false;
- spin_unlock_irqrestore(&clkdm->lock, flags);
return ret;
}
@@ -922,12 +981,10 @@ bool clkdm_missing_idle_reporting(struct clockdomain *clkdm)
static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
{
- unsigned long flags;
-
if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_enable)
return -EINVAL;
- spin_lock_irqsave(&clkdm->lock, flags);
+ pwrdm_lock(clkdm->pwrdm.ptr);
/*
* For arch's with no autodeps, clkcm_clk_enable
@@ -935,13 +992,13 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
* enabled, so the clkdm can be force woken up.
*/
if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps) {
- spin_unlock_irqrestore(&clkdm->lock, flags);
+ pwrdm_unlock(clkdm->pwrdm.ptr);
return 0;
}
arch_clkdm->clkdm_clk_enable(clkdm);
- pwrdm_state_switch(clkdm->pwrdm.ptr);
- spin_unlock_irqrestore(&clkdm->lock, flags);
+ pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
+ pwrdm_unlock(clkdm->pwrdm.ptr);
pr_debug("clockdomain: %s: enabled\n", clkdm->name);
@@ -950,27 +1007,25 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
{
- unsigned long flags;
-
if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_disable)
return -EINVAL;
- spin_lock_irqsave(&clkdm->lock, flags);
+ pwrdm_lock(clkdm->pwrdm.ptr);
if (atomic_read(&clkdm->usecount) == 0) {
- spin_unlock_irqrestore(&clkdm->lock, flags);
+ pwrdm_unlock(clkdm->pwrdm.ptr);
WARN_ON(1); /* underflow */
return -ERANGE;
}
if (atomic_dec_return(&clkdm->usecount) > 0) {
- spin_unlock_irqrestore(&clkdm->lock, flags);
+ pwrdm_unlock(clkdm->pwrdm.ptr);
return 0;
}
arch_clkdm->clkdm_clk_disable(clkdm);
- pwrdm_state_switch(clkdm->pwrdm.ptr);
- spin_unlock_irqrestore(&clkdm->lock, flags);
+ pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
+ pwrdm_unlock(clkdm->pwrdm.ptr);
pr_debug("clockdomain: %s: disabled\n", clkdm->name);
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index bc42446..e7f1b4b 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -15,7 +15,6 @@
#define __ARCH_ARM_MACH_OMAP2_CLOCKDOMAIN_H
#include <linux/init.h>
-#include <linux/spinlock.h>
#include "powerdomain.h"
#include "clock.h"
@@ -139,7 +138,6 @@ struct clockdomain {
struct clkdm_dep *sleepdep_srcs;
atomic_t usecount;
struct list_head node;
- spinlock_t lock;
};
/**
@@ -196,12 +194,16 @@ int clkdm_del_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2);
int clkdm_read_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2);
int clkdm_clear_all_sleepdeps(struct clockdomain *clkdm);
+void clkdm_allow_idle_nolock(struct clockdomain *clkdm);
void clkdm_allow_idle(struct clockdomain *clkdm);
+void clkdm_deny_idle_nolock(struct clockdomain *clkdm);
void clkdm_deny_idle(struct clockdomain *clkdm);
bool clkdm_in_hwsup(struct clockdomain *clkdm);
bool clkdm_missing_idle_reporting(struct clockdomain *clkdm);
+int clkdm_wakeup_nolock(struct clockdomain *clkdm);
int clkdm_wakeup(struct clockdomain *clkdm);
+int clkdm_sleep_nolock(struct clockdomain *clkdm);
int clkdm_sleep(struct clockdomain *clkdm);
int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk);
diff --git a/arch/arm/mach-omap2/powerdomain-clockdomain.h b/arch/arm/mach-omap2/powerdomain-clockdomain.h
new file mode 100644
index 0000000..e404eec
--- /dev/null
+++ b/arch/arm/mach-omap2/powerdomain-clockdomain.h
@@ -0,0 +1,27 @@
+/*
+ * OMAP powerdomain functions only for use by the OMAP clockdomain code
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ * Paul Walmsley
+ *
+ * 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.
+ *
+ * The point of this file is to try to prevent other code outside of
+ * clockdomain.c from calling the functions listed herein. Yes, this
+ * means you.
+ */
+
+#ifndef __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_CLOCKDOMAIN_H
+#define __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_CLOCKDOMAIN_H
+
+#include <linux/types.h>
+
+#include "powerdomain.h"
+
+extern void pwrdm_lock(struct powerdomain *pwrdm);
+extern void pwrdm_unlock(struct powerdomain *pwrdm);
+extern int pwrdm_state_switch_nolock(struct powerdomain *pwrdm);
+
+#endif
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 05f00660..2a5f15b 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -19,6 +19,7 @@
#include <linux/list.h>
#include <linux/errno.h>
#include <linux/string.h>
+#include <linux/spinlock.h>
#include <trace/events/power.h>
#include "cm2xxx_3xxx.h"
@@ -31,6 +32,7 @@
#include "powerdomain.h"
#include "clockdomain.h"
+#include "clockdomain-powerdomain.h"
#include "soc.h"
#include "pm.h"
@@ -101,6 +103,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
pwrdm->voltdm.ptr = voltdm;
INIT_LIST_HEAD(&pwrdm->voltdm_node);
voltdm_add_pwrdm(voltdm, pwrdm);
+ spin_lock_init(&pwrdm->_lock);
list_add(&pwrdm->node, &pwrdm_list);
@@ -276,6 +279,30 @@ int pwrdm_complete_init(void)
}
/**
+ * pwrdm_lock - acquire a Linux spinlock on a powerdomain
+ * @pwrdm: struct powerdomain * to lock
+ *
+ * Acquire the powerdomain spinlock on @pwrdm. No return value.
+ */
+void pwrdm_lock(struct powerdomain *pwrdm)
+ __acquires(&pwrdm->_lock)
+{
+ spin_lock_irqsave(&pwrdm->_lock, pwrdm->_lock_flags);
+}
+
+/**
+ * pwrdm_unlock - release a Linux spinlock on a powerdomain
+ * @pwrdm: struct powerdomain * to unlock
+ *
+ * Release the powerdomain spinlock on @pwrdm. No return value.
+ */
+void pwrdm_unlock(struct powerdomain *pwrdm)
+ __releases(&pwrdm->_lock)
+{
+ spin_unlock_irqrestore(&pwrdm->_lock, pwrdm->_lock_flags);
+}
+
+/**
* pwrdm_lookup - look up a powerdomain by name, return a pointer
* @name: name of powerdomain
*
@@ -921,7 +948,7 @@ bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm)
return (pwrdm && pwrdm->flags & PWRDM_HAS_HDWR_SAR) ? 1 : 0;
}
-int pwrdm_state_switch(struct powerdomain *pwrdm)
+int pwrdm_state_switch_nolock(struct powerdomain *pwrdm)
{
int ret;
@@ -935,6 +962,17 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)
return ret;
}
+int __deprecated pwrdm_state_switch(struct powerdomain *pwrdm)
+{
+ int ret;
+
+ pwrdm_lock(pwrdm);
+ ret = pwrdm_state_switch_nolock(pwrdm);
+ pwrdm_unlock(pwrdm);
+
+ return ret;
+}
+
int pwrdm_pre_transition(struct powerdomain *pwrdm)
{
if (pwrdm)
@@ -973,7 +1011,7 @@ static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
sleep_switch = LOWPOWERSTATE_SWITCH;
} else {
*hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
- clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
+ clkdm_wakeup_nolock(pwrdm->pwrdm_clkdms[0]);
sleep_switch = FORCEWAKEUP_SWITCH;
}
} else {
@@ -989,15 +1027,15 @@ static void _pwrdm_restore_clkdm_state(struct powerdomain *pwrdm,
switch (sleep_switch) {
case FORCEWAKEUP_SWITCH:
if (hwsup)
- clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+ clkdm_allow_idle_nolock(pwrdm->pwrdm_clkdms[0]);
else
- clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
+ clkdm_sleep_nolock(pwrdm->pwrdm_clkdms[0]);
break;
case LOWPOWERSTATE_SWITCH:
if (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
arch_pwrdm->pwrdm_set_lowpwrstchange)
arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
- pwrdm_state_switch(pwrdm);
+ pwrdm_state_switch_nolock(pwrdm);
break;
}
}
@@ -1021,9 +1059,13 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
pwrst--;
}
+ pwrdm_lock(pwrdm);
+
next_pwrst = pwrdm_read_next_pwrst(pwrdm);
- if (next_pwrst == pwrst)
+ if (next_pwrst == pwrst) {
+ pwrdm_unlock(pwrdm);
return ret;
+ }
sleep_switch = _pwrdm_save_clkdm_state_and_activate(pwrdm, pwrst,
&hwsup);
@@ -1035,6 +1077,8 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
_pwrdm_restore_clkdm_state(pwrdm, sleep_switch, hwsup);
+ pwrdm_unlock(pwrdm);
+
return ret;
}
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 1edb3b7..83b4892 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -19,8 +19,7 @@
#include <linux/types.h>
#include <linux/list.h>
-
-#include <linux/atomic.h>
+#include <linux/spinlock.h>
#include "voltage.h"
@@ -103,6 +102,8 @@ struct powerdomain;
* @state_counter:
* @timer:
* @state_timer:
+ * @_lock: spinlock used to serialize powerdomain and some clockdomain ops
+ * @_lock_flags: stored flags when @_lock is taken
*
* @prcm_partition possible values are defined in mach-omap2/prcm44xx.h.
*/
@@ -127,7 +128,8 @@ struct powerdomain {
unsigned state_counter[PWRDM_MAX_PWRSTS];
unsigned ret_logic_off_counter;
unsigned ret_mem_off_counter[PWRDM_MAX_MEM_BANKS];
-
+ spinlock_t _lock;
+ unsigned long _lock_flags;
const u8 pwrstctrl_offs;
const u8 pwrstst_offs;
const u32 logicretstate_mask;
@@ -225,6 +227,7 @@ 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_nolock(struct powerdomain *pwrdm);
int pwrdm_state_switch(struct powerdomain *pwrdm);
int pwrdm_pre_transition(struct powerdomain *pwrdm);
int pwrdm_post_transition(struct powerdomain *pwrdm);
@@ -252,5 +255,7 @@ extern u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank);
extern struct powerdomain wkup_omap2_pwrdm;
extern struct powerdomain gfx_omap2_pwrdm;
+extern void pwrdm_lock(struct powerdomain *pwrdm);
+extern void pwrdm_unlock(struct powerdomain *pwrdm);
#endif
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 06/10] ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock
2012-12-09 1:23 ` [PATCH 06/10] ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock Paul Walmsley
@ 2012-12-12 9:41 ` Jean Pihet
2013-01-29 21:13 ` Paul Walmsley
2012-12-12 10:28 ` Jean Pihet
1 sibling, 1 reply; 28+ messages in thread
From: Jean Pihet @ 2012-12-12 9:41 UTC (permalink / raw)
To: linux-arm-kernel
Paul,
On Sun, Dec 9, 2012 at 2:23 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Add a per-powerdomain spinlock. Use that instead of the clockdomain
> spinlock. Add pwrdm_lock()/pwrdm_unlock() functions to allow other
> code to acquire or release the powerdomain spinlock without reaching
> directly into the struct powerdomain.
>
Since clockdomains are part of powerdomains it seems weird for the
clockdomain code to take a powerdoamin lock.
Is there a reason why the powerdomain could not take the lock before
calling the clockdomain functions?
Also, are the lock and nolock version the clockdomain function needed?
Regards,
Jean
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Jean Pihet <jean.pihet@newoldbits.com>
> ---
> arch/arm/mach-omap2/clockdomain-powerdomain.h | 22 +++
> arch/arm/mach-omap2/clockdomain.c | 161
> +++++++++++++++++--------
> arch/arm/mach-omap2/clockdomain.h | 6 +
> arch/arm/mach-omap2/powerdomain-clockdomain.h | 27 ++++
> arch/arm/mach-omap2/powerdomain.c | 56 ++++++++-
> arch/arm/mach-omap2/powerdomain.h | 11 +-
> 6 files changed, 219 insertions(+), 64 deletions(-)
> create mode 100644 arch/arm/mach-omap2/clockdomain-powerdomain.h
> create mode 100644 arch/arm/mach-omap2/powerdomain-clockdomain.h
>
> diff --git a/arch/arm/mach-omap2/clockdomain-powerdomain.h
> b/arch/arm/mach-omap2/clockdomain-powerdomain.h
> new file mode 100644
> index 0000000..8beee2d
> --- /dev/null
> +++ b/arch/arm/mach-omap2/clockdomain-powerdomain.h
> @@ -0,0 +1,22 @@
> +/*
> + * OMAP clockdomain framework functions for use only by the powerdomain
> code
> + *
> + * Copyright (C) 2012 Texas Instruments, Inc.
> + * Paul Walmsley
> + *
> + * 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.
> + */
> +
> +#ifndef __ARCH_ARM_MACH_OMAP2_CLOCKDOMAIN_POWERDOMAIN_H
> +#define __ARCH_ARM_MACH_OMAP2_CLOCKDOMAIN_POWERDOMAIN_H
> +
> +#include "clockdomain.h"
> +
> +extern void clkdm_allow_idle_nolock(struct clockdomain *clkdm);
> +extern void clkdm_deny_idle_nolock(struct clockdomain *clkdm);
> +extern int clkdm_wakeup_nolock(struct clockdomain *clkdm);
> +extern int clkdm_sleep_nolock(struct clockdomain *clkdm);
> +
> +#endif
> diff --git a/arch/arm/mach-omap2/clockdomain.c
> b/arch/arm/mach-omap2/clockdomain.c
> index 18f65fd..2142dab 100644
> --- a/arch/arm/mach-omap2/clockdomain.c
> +++ b/arch/arm/mach-omap2/clockdomain.c
> @@ -30,6 +30,7 @@
> #include "soc.h"
> #include "clock.h"
> #include "clockdomain.h"
> +#include "powerdomain-clockdomain.h"
>
> /* clkdm_list contains all registered struct clockdomains */
> static LIST_HEAD(clkdm_list);
> @@ -91,8 +92,6 @@ static int _clkdm_register(struct clockdomain *clkdm)
>
> pwrdm_add_clkdm(pwrdm, clkdm);
>
> - spin_lock_init(&clkdm->lock);
> -
> pr_debug("clockdomain: registered %s\n", clkdm->name);
>
> return 0;
> @@ -733,18 +732,17 @@ int clkdm_clear_all_sleepdeps(struct clockdomain
> *clkdm)
> }
>
> /**
> - * clkdm_sleep - force clockdomain sleep transition
> + * clkdm_sleep_nolock - force clockdomain sleep transition (lockless)
> * @clkdm: struct clockdomain *
> *
> * Instruct the CM to force a sleep transition on the specified
> - * clockdomain @clkdm. Returns -EINVAL if @clkdm is NULL or if
> - * clockdomain does not support software-initiated sleep; 0 upon
> - * success.
> + * clockdomain @clkdm. Only for use by the powerdomain code. Returns
> + * -EINVAL if @clkdm is NULL or if clockdomain does not support
> + * software-initiated sleep; 0 upon success.
> */
> -int clkdm_sleep(struct clockdomain *clkdm)
> +int clkdm_sleep_nolock(struct clockdomain *clkdm)
> {
> int ret;
> - unsigned long flags;
>
> if (!clkdm)
> return -EINVAL;
> @@ -760,27 +758,45 @@ int clkdm_sleep(struct clockdomain *clkdm)
>
> pr_debug("clockdomain: forcing sleep on %s\n", clkdm->name);
>
> - spin_lock_irqsave(&clkdm->lock, flags);
> clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
> ret = arch_clkdm->clkdm_sleep(clkdm);
> - ret |= pwrdm_state_switch(clkdm->pwrdm.ptr);
> - spin_unlock_irqrestore(&clkdm->lock, flags);
> + ret |= pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
> +
> return ret;
> }
>
> /**
> - * clkdm_wakeup - force clockdomain wakeup transition
> + * clkdm_sleep - force clockdomain sleep transition
> * @clkdm: struct clockdomain *
> *
> - * Instruct the CM to force a wakeup transition on the specified
> - * clockdomain @clkdm. Returns -EINVAL if @clkdm is NULL or if the
> - * clockdomain does not support software-controlled wakeup; 0 upon
> + * Instruct the CM to force a sleep transition on the specified
> + * clockdomain @clkdm. Returns -EINVAL if @clkdm is NULL or if
> + * clockdomain does not support software-initiated sleep; 0 upon
> * success.
> */
> -int clkdm_wakeup(struct clockdomain *clkdm)
> +int clkdm_sleep(struct clockdomain *clkdm)
> +{
> + int ret;
> +
> + pwrdm_lock(clkdm->pwrdm.ptr);
> + ret = clkdm_sleep_nolock(clkdm);
> + pwrdm_unlock(clkdm->pwrdm.ptr);
> +
> + return ret;
> +}
> +
> +/**
> + * clkdm_wakeup_nolock - force clockdomain wakeup transition (lockless)
> + * @clkdm: struct clockdomain *
> + *
> + * Instruct the CM to force a wakeup transition on the specified
> + * clockdomain @clkdm. Only for use by the powerdomain code. Returns
> + * -EINVAL if @clkdm is NULL or if the clockdomain does not support
> + * software-controlled wakeup; 0 upon success.
> + */
> +int clkdm_wakeup_nolock(struct clockdomain *clkdm)
> {
> int ret;
> - unsigned long flags;
>
> if (!clkdm)
> return -EINVAL;
> @@ -796,28 +812,46 @@ int clkdm_wakeup(struct clockdomain *clkdm)
>
> pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
>
> - spin_lock_irqsave(&clkdm->lock, flags);
> clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
> ret = arch_clkdm->clkdm_wakeup(clkdm);
> - ret |= pwrdm_state_switch(clkdm->pwrdm.ptr);
> - spin_unlock_irqrestore(&clkdm->lock, flags);
> + ret |= pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
> +
> return ret;
> }
>
> /**
> - * clkdm_allow_idle - enable hwsup idle transitions for clkdm
> + * clkdm_wakeup - force clockdomain wakeup transition
> * @clkdm: struct clockdomain *
> *
> - * Allow the hardware to automatically switch the clockdomain @clkdm into
> - * active or idle states, as needed by downstream clocks. If the
> + * Instruct the CM to force a wakeup transition on the specified
> + * clockdomain @clkdm. Returns -EINVAL if @clkdm is NULL or if the
> + * clockdomain does not support software-controlled wakeup; 0 upon
> + * success.
> + */
> +int clkdm_wakeup(struct clockdomain *clkdm)
> +{
> + int ret;
> +
> + pwrdm_lock(clkdm->pwrdm.ptr);
> + ret = clkdm_wakeup_nolock(clkdm);
> + pwrdm_unlock(clkdm->pwrdm.ptr);
> +
> + return ret;
> +}
> +
> +/**
> + * clkdm_allow_idle_nolock - enable hwsup idle transitions for clkdm
> + * @clkdm: struct clockdomain *
> + *
> + * Allow the hardware to automatically switch the clockdomain @clkdm
> + * into active or idle states, as needed by downstream clocks. If the
> * clockdomain has any downstream clocks enabled in the clock
> * framework, wkdep/sleepdep autodependencies are added; this is so
> - * device drivers can read and write to the device. No return value.
> + * device drivers can read and write to the device. Only for use by
> + * the powerdomain code. No return value.
> */
> -void clkdm_allow_idle(struct clockdomain *clkdm)
> +void clkdm_allow_idle_nolock(struct clockdomain *clkdm)
> {
> - unsigned long flags;
> -
> if (!clkdm)
> return;
>
> @@ -833,11 +867,26 @@ void clkdm_allow_idle(struct clockdomain *clkdm)
> pr_debug("clockdomain: enabling automatic idle transitions for
> %s\n",
> clkdm->name);
>
> - spin_lock_irqsave(&clkdm->lock, flags);
> clkdm->_flags |= _CLKDM_FLAG_HWSUP_ENABLED;
> arch_clkdm->clkdm_allow_idle(clkdm);
> - pwrdm_state_switch(clkdm->pwrdm.ptr);
> - spin_unlock_irqrestore(&clkdm->lock, flags);
> + pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
> +}
> +
> +/**
> + * clkdm_allow_idle - enable hwsup idle transitions for clkdm
> + * @clkdm: struct clockdomain *
> + *
> + * Allow the hardware to automatically switch the clockdomain @clkdm into
> + * active or idle states, as needed by downstream clocks. If the
> + * clockdomain has any downstream clocks enabled in the clock
> + * framework, wkdep/sleepdep autodependencies are added; this is so
> + * device drivers can read and write to the device. No return value.
> + */
> +void clkdm_allow_idle(struct clockdomain *clkdm)
> +{
> + pwrdm_lock(clkdm->pwrdm.ptr);
> + clkdm_allow_idle_nolock(clkdm);
> + pwrdm_unlock(clkdm->pwrdm.ptr);
> }
>
> /**
> @@ -847,12 +896,11 @@ void clkdm_allow_idle(struct clockdomain *clkdm)
> * Prevent the hardware from automatically switching the clockdomain
> * @clkdm into inactive or idle states. If the clockdomain has
> * downstream clocks enabled in the clock framework, wkdep/sleepdep
> - * autodependencies are removed. No return value.
> + * autodependencies are removed. Only for use by the powerdomain
> + * code. No return value.
> */
> -void clkdm_deny_idle(struct clockdomain *clkdm)
> +void clkdm_deny_idle_nolock(struct clockdomain *clkdm)
> {
> - unsigned long flags;
> -
> if (!clkdm)
> return;
>
> @@ -868,11 +916,25 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
> pr_debug("clockdomain: disabling automatic idle transitions for
> %s\n",
> clkdm->name);
>
> - spin_lock_irqsave(&clkdm->lock, flags);
> clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
> arch_clkdm->clkdm_deny_idle(clkdm);
> - pwrdm_state_switch(clkdm->pwrdm.ptr);
> - spin_unlock_irqrestore(&clkdm->lock, flags);
> + pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
> +}
> +
> +/**
> + * clkdm_deny_idle - disable hwsup idle transitions for clkdm
> + * @clkdm: struct clockdomain *
> + *
> + * Prevent the hardware from automatically switching the clockdomain
> + * @clkdm into inactive or idle states. If the clockdomain has
> + * downstream clocks enabled in the clock framework, wkdep/sleepdep
> + * autodependencies are removed. No return value.
> + */
> +void clkdm_deny_idle(struct clockdomain *clkdm)
> +{
> + pwrdm_lock(clkdm->pwrdm.ptr);
> + clkdm_deny_idle_nolock(clkdm);
> + pwrdm_unlock(clkdm->pwrdm.ptr);
> }
>
> /**
> @@ -889,14 +951,11 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
> bool clkdm_in_hwsup(struct clockdomain *clkdm)
> {
> bool ret;
> - unsigned long flags;
>
> if (!clkdm)
> return false;
>
> - spin_lock_irqsave(&clkdm->lock, flags);
> ret = (clkdm->_flags & _CLKDM_FLAG_HWSUP_ENABLED) ? true : false;
> - spin_unlock_irqrestore(&clkdm->lock, flags);
>
> return ret;
> }
> @@ -922,12 +981,10 @@ bool clkdm_missing_idle_reporting(struct clockdomain
> *clkdm)
>
> static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
> {
> - unsigned long flags;
> -
> if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_enable)
> return -EINVAL;
>
> - spin_lock_irqsave(&clkdm->lock, flags);
> + pwrdm_lock(clkdm->pwrdm.ptr);
>
> /*
> * For arch's with no autodeps, clkcm_clk_enable
> @@ -935,13 +992,13 @@ static int _clkdm_clk_hwmod_enable(struct
> clockdomain *clkdm)
> * enabled, so the clkdm can be force woken up.
> */
> if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps) {
> - spin_unlock_irqrestore(&clkdm->lock, flags);
> + pwrdm_unlock(clkdm->pwrdm.ptr);
> return 0;
> }
>
> arch_clkdm->clkdm_clk_enable(clkdm);
> - pwrdm_state_switch(clkdm->pwrdm.ptr);
> - spin_unlock_irqrestore(&clkdm->lock, flags);
> + pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
> + pwrdm_unlock(clkdm->pwrdm.ptr);
>
> pr_debug("clockdomain: %s: enabled\n", clkdm->name);
>
> @@ -950,27 +1007,25 @@ static int _clkdm_clk_hwmod_enable(struct
> clockdomain *clkdm)
>
> static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
> {
> - unsigned long flags;
> -
> if (!clkdm || !arch_clkdm || !arch_clkdm->clkdm_clk_disable)
> return -EINVAL;
>
> - spin_lock_irqsave(&clkdm->lock, flags);
> + pwrdm_lock(clkdm->pwrdm.ptr);
>
> if (atomic_read(&clkdm->usecount) == 0) {
> - spin_unlock_irqrestore(&clkdm->lock, flags);
> + pwrdm_unlock(clkdm->pwrdm.ptr);
> WARN_ON(1); /* underflow */
> return -ERANGE;
> }
>
> if (atomic_dec_return(&clkdm->usecount) > 0) {
> - spin_unlock_irqrestore(&clkdm->lock, flags);
> + pwrdm_unlock(clkdm->pwrdm.ptr);
> return 0;
> }
>
> arch_clkdm->clkdm_clk_disable(clkdm);
> - pwrdm_state_switch(clkdm->pwrdm.ptr);
> - spin_unlock_irqrestore(&clkdm->lock, flags);
> + pwrdm_state_switch_nolock(clkdm->pwrdm.ptr);
> + pwrdm_unlock(clkdm->pwrdm.ptr);
>
> pr_debug("clockdomain: %s: disabled\n", clkdm->name);
>
> diff --git a/arch/arm/mach-omap2/clockdomain.h
> b/arch/arm/mach-omap2/clockdomain.h
> index bc42446..e7f1b4b 100644
> --- a/arch/arm/mach-omap2/clockdomain.h
> +++ b/arch/arm/mach-omap2/clockdomain.h
> @@ -15,7 +15,6 @@
> #define __ARCH_ARM_MACH_OMAP2_CLOCKDOMAIN_H
>
> #include <linux/init.h>
> -#include <linux/spinlock.h>
>
> #include "powerdomain.h"
> #include "clock.h"
> @@ -139,7 +138,6 @@ struct clockdomain {
> struct clkdm_dep *sleepdep_srcs;
> atomic_t usecount;
> struct list_head node;
> - spinlock_t lock;
> };
>
> /**
> @@ -196,12 +194,16 @@ int clkdm_del_sleepdep(struct clockdomain *clkdm1,
> struct clockdomain *clkdm2);
> int clkdm_read_sleepdep(struct clockdomain *clkdm1, struct clockdomain
> *clkdm2);
> int clkdm_clear_all_sleepdeps(struct clockdomain *clkdm);
>
> +void clkdm_allow_idle_nolock(struct clockdomain *clkdm);
> void clkdm_allow_idle(struct clockdomain *clkdm);
> +void clkdm_deny_idle_nolock(struct clockdomain *clkdm);
> void clkdm_deny_idle(struct clockdomain *clkdm);
> bool clkdm_in_hwsup(struct clockdomain *clkdm);
> bool clkdm_missing_idle_reporting(struct clockdomain *clkdm);
>
> +int clkdm_wakeup_nolock(struct clockdomain *clkdm);
> int clkdm_wakeup(struct clockdomain *clkdm);
> +int clkdm_sleep_nolock(struct clockdomain *clkdm);
> int clkdm_sleep(struct clockdomain *clkdm);
>
> int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk);
> diff --git a/arch/arm/mach-omap2/powerdomain-clockdomain.h
> b/arch/arm/mach-omap2/powerdomain-clockdomain.h
> new file mode 100644
> index 0000000..e404eec
> --- /dev/null
> +++ b/arch/arm/mach-omap2/powerdomain-clockdomain.h
> @@ -0,0 +1,27 @@
> +/*
> + * OMAP powerdomain functions only for use by the OMAP clockdomain code
> + *
> + * Copyright (C) 2012 Texas Instruments, Inc.
> + * Paul Walmsley
> + *
> + * 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.
> + *
> + * The point of this file is to try to prevent other code outside of
> + * clockdomain.c from calling the functions listed herein. Yes, this
> + * means you.
> + */
> +
> +#ifndef __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_CLOCKDOMAIN_H
> +#define __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_CLOCKDOMAIN_H
> +
> +#include <linux/types.h>
> +
> +#include "powerdomain.h"
> +
> +extern void pwrdm_lock(struct powerdomain *pwrdm);
> +extern void pwrdm_unlock(struct powerdomain *pwrdm);
> +extern int pwrdm_state_switch_nolock(struct powerdomain *pwrdm);
> +
> +#endif
> diff --git a/arch/arm/mach-omap2/powerdomain.c
> b/arch/arm/mach-omap2/powerdomain.c
> index 05f00660..2a5f15b 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -19,6 +19,7 @@
> #include <linux/list.h>
> #include <linux/errno.h>
> #include <linux/string.h>
> +#include <linux/spinlock.h>
> #include <trace/events/power.h>
>
> #include "cm2xxx_3xxx.h"
> @@ -31,6 +32,7 @@
>
> #include "powerdomain.h"
> #include "clockdomain.h"
> +#include "clockdomain-powerdomain.h"
>
> #include "soc.h"
> #include "pm.h"
> @@ -101,6 +103,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
> pwrdm->voltdm.ptr = voltdm;
> INIT_LIST_HEAD(&pwrdm->voltdm_node);
> voltdm_add_pwrdm(voltdm, pwrdm);
> + spin_lock_init(&pwrdm->_lock);
>
> list_add(&pwrdm->node, &pwrdm_list);
>
> @@ -276,6 +279,30 @@ int pwrdm_complete_init(void)
> }
>
> /**
> + * pwrdm_lock - acquire a Linux spinlock on a powerdomain
> + * @pwrdm: struct powerdomain * to lock
> + *
> + * Acquire the powerdomain spinlock on @pwrdm. No return value.
> + */
> +void pwrdm_lock(struct powerdomain *pwrdm)
> + __acquires(&pwrdm->_lock)
> +{
> + spin_lock_irqsave(&pwrdm->_lock, pwrdm->_lock_flags);
> +}
> +
> +/**
> + * pwrdm_unlock - release a Linux spinlock on a powerdomain
> + * @pwrdm: struct powerdomain * to unlock
> + *
> + * Release the powerdomain spinlock on @pwrdm. No return value.
> + */
> +void pwrdm_unlock(struct powerdomain *pwrdm)
> + __releases(&pwrdm->_lock)
> +{
> + spin_unlock_irqrestore(&pwrdm->_lock, pwrdm->_lock_flags);
> +}
> +
> +/**
> * pwrdm_lookup - look up a powerdomain by name, return a pointer
> * @name: name of powerdomain
> *
> @@ -921,7 +948,7 @@ bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm)
> return (pwrdm && pwrdm->flags & PWRDM_HAS_HDWR_SAR) ? 1 : 0;
> }
>
> -int pwrdm_state_switch(struct powerdomain *pwrdm)
> +int pwrdm_state_switch_nolock(struct powerdomain *pwrdm)
> {
> int ret;
>
> @@ -935,6 +962,17 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)
> return ret;
> }
>
> +int __deprecated pwrdm_state_switch(struct powerdomain *pwrdm)
> +{
> + int ret;
> +
> + pwrdm_lock(pwrdm);
> + ret = pwrdm_state_switch_nolock(pwrdm);
> + pwrdm_unlock(pwrdm);
> +
> + return ret;
> +}
> +
> int pwrdm_pre_transition(struct powerdomain *pwrdm)
> {
> if (pwrdm)
> @@ -973,7 +1011,7 @@ static u8 _pwrdm_save_clkdm_state_and_activate(struct
> powerdomain *pwrdm,
> sleep_switch = LOWPOWERSTATE_SWITCH;
> } else {
> *hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
> - clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> + clkdm_wakeup_nolock(pwrdm->pwrdm_clkdms[0]);
> sleep_switch = FORCEWAKEUP_SWITCH;
> }
> } else {
> @@ -989,15 +1027,15 @@ static void _pwrdm_restore_clkdm_state(struct
> powerdomain *pwrdm,
> switch (sleep_switch) {
> case FORCEWAKEUP_SWITCH:
> if (hwsup)
> - clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> + clkdm_allow_idle_nolock(pwrdm->pwrdm_clkdms[0]);
> else
> - clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
> + clkdm_sleep_nolock(pwrdm->pwrdm_clkdms[0]);
> break;
> case LOWPOWERSTATE_SWITCH:
> if (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
> arch_pwrdm->pwrdm_set_lowpwrstchange)
> arch_pwrdm->pwrdm_set_lowpwrstchange(pwrdm);
> - pwrdm_state_switch(pwrdm);
> + pwrdm_state_switch_nolock(pwrdm);
> break;
> }
> }
> @@ -1021,9 +1059,13 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm,
> u8 pwrst)
> pwrst--;
> }
>
> + pwrdm_lock(pwrdm);
> +
> next_pwrst = pwrdm_read_next_pwrst(pwrdm);
> - if (next_pwrst == pwrst)
> + if (next_pwrst == pwrst) {
> + pwrdm_unlock(pwrdm);
> return ret;
> + }
>
> sleep_switch = _pwrdm_save_clkdm_state_and_activate(pwrdm, pwrst,
> &hwsup);
> @@ -1035,6 +1077,8 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm,
> u8 pwrst)
>
> _pwrdm_restore_clkdm_state(pwrdm, sleep_switch, hwsup);
>
> + pwrdm_unlock(pwrdm);
> +
> return ret;
> }
>
> diff --git a/arch/arm/mach-omap2/powerdomain.h
> b/arch/arm/mach-omap2/powerdomain.h
> index 1edb3b7..83b4892 100644
> --- a/arch/arm/mach-omap2/powerdomain.h
> +++ b/arch/arm/mach-omap2/powerdomain.h
> @@ -19,8 +19,7 @@
>
> #include <linux/types.h>
> #include <linux/list.h>
> -
> -#include <linux/atomic.h>
> +#include <linux/spinlock.h>
>
> #include "voltage.h"
>
> @@ -103,6 +102,8 @@ struct powerdomain;
> * @state_counter:
> * @timer:
> * @state_timer:
> + * @_lock: spinlock used to serialize powerdomain and some clockdomain ops
> + * @_lock_flags: stored flags when @_lock is taken
> *
> * @prcm_partition possible values are defined in mach-omap2/prcm44xx.h.
> */
> @@ -127,7 +128,8 @@ struct powerdomain {
> unsigned state_counter[PWRDM_MAX_PWRSTS];
> unsigned ret_logic_off_counter;
> unsigned ret_mem_off_counter[PWRDM_MAX_MEM_BANKS];
> -
> + spinlock_t _lock;
> + unsigned long _lock_flags;
> const u8 pwrstctrl_offs;
> const u8 pwrstst_offs;
> const u32 logicretstate_mask;
> @@ -225,6 +227,7 @@ 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_nolock(struct powerdomain *pwrdm);
> int pwrdm_state_switch(struct powerdomain *pwrdm);
> int pwrdm_pre_transition(struct powerdomain *pwrdm);
> int pwrdm_post_transition(struct powerdomain *pwrdm);
> @@ -252,5 +255,7 @@ extern u32 omap2_pwrdm_get_mem_bank_stst_mask(u8 bank);
> extern struct powerdomain wkup_omap2_pwrdm;
> extern struct powerdomain gfx_omap2_pwrdm;
>
> +extern void pwrdm_lock(struct powerdomain *pwrdm);
> +extern void pwrdm_unlock(struct powerdomain *pwrdm);
>
> #endif
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121212/174fb6f6/attachment-0001.html>
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 06/10] ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock
2012-12-12 9:41 ` Jean Pihet
@ 2013-01-29 21:13 ` Paul Walmsley
0 siblings, 0 replies; 28+ messages in thread
From: Paul Walmsley @ 2013-01-29 21:13 UTC (permalink / raw)
To: linux-arm-kernel
Hi
On Wed, 12 Dec 2012, Jean Pihet wrote:
> On Sun, Dec 9, 2012 at 2:23 AM, Paul Walmsley <paul@pwsan.com> wrote:
>
> > Add a per-powerdomain spinlock. Use that instead of the clockdomain
> > spinlock. Add pwrdm_lock()/pwrdm_unlock() functions to allow other
> > code to acquire or release the powerdomain spinlock without reaching
> > directly into the struct powerdomain.
> >
> Since clockdomains are part of powerdomains it seems weird for the
> clockdomain code to take a powerdoamin lock.
Why?
> Is there a reason why the powerdomain could not take the lock before
> calling the clockdomain functions?
Do you mean "is there a reason why the powerdomain _code_ could not take
the lock"? If so, the reason is that code other than the powerdomain code
calls the clkdm_* functions directly, without calling any powerdomain
functions first. So there's really no other place to take the lock unless
the callers are updated to take the powerdomain lock themselves. That
seems like something to avoid if the caller doesn't have any other
relationship to the powerdomain code.
> Also, are the lock and nolock version the clockdomain function needed?
Did you have a different solution in mind? The two versions are used for
code that needs to be called from two contexts: the first with the
powerdomain's lock already held; the second needing to acquire the
powerdomain's lock to avoid racing against other PRCM code.
- Paul
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 06/10] ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock
2012-12-09 1:23 ` [PATCH 06/10] ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock Paul Walmsley
2012-12-12 9:41 ` Jean Pihet
@ 2012-12-12 10:28 ` Jean Pihet
1 sibling, 0 replies; 28+ messages in thread
From: Jean Pihet @ 2012-12-12 10:28 UTC (permalink / raw)
To: linux-arm-kernel
Paul,
-resending in plain text only, sorry about that-
On Sun, Dec 9, 2012 at 2:23 AM, Paul Walmsley <paul@pwsan.com> wrote:
>
> Add a per-powerdomain spinlock. Use that instead of the clockdomain
> spinlock. Add pwrdm_lock()/pwrdm_unlock() functions to allow other
> code to acquire or release the powerdomain spinlock without reaching
> directly into the struct powerdomain.
Since clockdomains are part of powerdomains it seems weird for the
clockdomain code to take a powerdoamin lock.
Is there a reason why the powerdomain could not take the lock before
calling the clockdomain functions?
Also, are the lock and nolock version the clockdomain function needed?
Regards,
Jean
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 08/10] ARM: OMAP2+: clockdomain: work on wkdep/sleepdep functions
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
` (6 preceding siblings ...)
2012-12-09 1:23 ` [PATCH 06/10] ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2012-12-09 1:23 ` [PATCH 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints Paul Walmsley
` (2 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
Split the clkdm_(add|del)_(wk|sleep)dep() functions into lockless and
locking versions -- this will be needed in a subsequent patch. Also,
while here, remove the leading underscore, since these are not
currently static functions. And for functions that have
kerneldoc-style comment blocks, but which are missing the initial
'/**' tag, fix the tag to indicate that they are kerneldoc.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/clockdomain.c | 394 ++++++++++++++++++++++---------------
arch/arm/mach-omap2/clockdomain.h | 5
arch/arm/mach-omap2/cm3xxx.c | 8 -
3 files changed, 240 insertions(+), 167 deletions(-)
diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 2142dab..9d5b69f 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -120,7 +120,7 @@ static struct clkdm_dep *_clkdm_deps_lookup(struct clockdomain *clkdm,
return cd;
}
-/*
+/**
* _autodep_lookup - resolve autodep clkdm names to clkdm pointers; store
* @autodep: struct clkdm_autodep * to resolve
*
@@ -152,88 +152,202 @@ static void _autodep_lookup(struct clkdm_autodep *autodep)
autodep->clkdm.ptr = clkdm;
}
-/*
- * _clkdm_add_autodeps - add auto sleepdeps/wkdeps to clkdm upon clock enable
- * @clkdm: struct clockdomain *
+/**
+ * _resolve_clkdm_deps() - resolve clkdm_names in @clkdm_deps to clkdms
+ * @clkdm: clockdomain that we are resolving dependencies for
+ * @clkdm_deps: ptr to array of struct clkdm_deps to resolve
*
- * Add the "autodep" sleep & wakeup dependencies to clockdomain 'clkdm'
- * in hardware-supervised mode. Meant to be called from clock framework
- * when a clock inside clockdomain 'clkdm' is enabled. No return value.
+ * Iterates through @clkdm_deps, looking up the struct clockdomain named by
+ * clkdm_name and storing the clockdomain pointer in the struct clkdm_dep.
+ * No return value.
+ */
+static void _resolve_clkdm_deps(struct clockdomain *clkdm,
+ struct clkdm_dep *clkdm_deps)
+{
+ struct clkdm_dep *cd;
+
+ for (cd = clkdm_deps; cd && cd->clkdm_name; cd++) {
+ if (cd->clkdm)
+ continue;
+ cd->clkdm = _clkdm_lookup(cd->clkdm_name);
+
+ WARN(!cd->clkdm, "clockdomain: %s: could not find clkdm %s while resolving dependencies - should never happen",
+ clkdm->name, cd->clkdm_name);
+ }
+}
+
+/**
+ * _clkdm_add_wkdep - add a wakeup dependency from clkdm2 to clkdm1 (lockless)
+ * @clkdm1: wake this struct clockdomain * up (dependent)
+ * @clkdm2: when this struct clockdomain * wakes up (source)
*
- * XXX autodeps are deprecated and should be removed at the earliest
- * opportunity
+ * When the clockdomain represented by @clkdm2 wakes up, wake up
+ * @clkdm1. Implemented in hardware on the OMAP, this feature is
+ * designed to reduce wakeup latency of the dependent clockdomain @clkdm1.
+ * Returns -EINVAL if presented with invalid clockdomain pointers,
+ * -ENOENT if @clkdm2 cannot wake up clkdm1 in hardware, or 0 upon
+ * success.
*/
-void _clkdm_add_autodeps(struct clockdomain *clkdm)
+static int _clkdm_add_wkdep(struct clockdomain *clkdm1,
+ struct clockdomain *clkdm2)
{
- struct clkdm_autodep *autodep;
+ struct clkdm_dep *cd;
+ int ret = 0;
- if (!autodeps || clkdm->flags & CLKDM_NO_AUTODEPS)
- return;
+ if (!clkdm1 || !clkdm2)
+ return -EINVAL;
- for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {
- if (IS_ERR(autodep->clkdm.ptr))
- continue;
+ cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
+ if (IS_ERR(cd))
+ ret = PTR_ERR(cd);
- pr_debug("clockdomain: %s: adding %s sleepdep/wkdep\n",
- clkdm->name, autodep->clkdm.ptr->name);
+ if (!arch_clkdm || !arch_clkdm->clkdm_add_wkdep)
+ ret = -EINVAL;
- clkdm_add_sleepdep(clkdm, autodep->clkdm.ptr);
- clkdm_add_wkdep(clkdm, autodep->clkdm.ptr);
+ if (ret) {
+ pr_debug("clockdomain: hardware cannot set/clear wake up of %s when %s wakes up\n",
+ clkdm1->name, clkdm2->name);
+ return ret;
+ }
+
+ if (atomic_inc_return(&cd->wkdep_usecount) == 1) {
+ pr_debug("clockdomain: hardware will wake up %s when %s wakes up\n",
+ clkdm1->name, clkdm2->name);
+
+ ret = arch_clkdm->clkdm_add_wkdep(clkdm1, clkdm2);
}
+
+ return ret;
}
-/*
- * _clkdm_add_autodeps - remove auto sleepdeps/wkdeps from clkdm
- * @clkdm: struct clockdomain *
+/**
+ * _clkdm_del_wkdep - remove a wakeup dep from clkdm2 to clkdm1 (lockless)
+ * @clkdm1: wake this struct clockdomain * up (dependent)
+ * @clkdm2: when this struct clockdomain * wakes up (source)
*
- * Remove the "autodep" sleep & wakeup dependencies from clockdomain 'clkdm'
- * in hardware-supervised mode. Meant to be called from clock framework
- * when a clock inside clockdomain 'clkdm' is disabled. No return value.
+ * Remove a wakeup dependency causing @clkdm1 to wake up when @clkdm2
+ * wakes up. Returns -EINVAL if presented with invalid clockdomain
+ * pointers, -ENOENT if @clkdm2 cannot wake up clkdm1 in hardware, or
+ * 0 upon success.
+ */
+static int _clkdm_del_wkdep(struct clockdomain *clkdm1,
+ struct clockdomain *clkdm2)
+{
+ struct clkdm_dep *cd;
+ int ret = 0;
+
+ if (!clkdm1 || !clkdm2)
+ return -EINVAL;
+
+ cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
+ if (IS_ERR(cd))
+ ret = PTR_ERR(cd);
+
+ if (!arch_clkdm || !arch_clkdm->clkdm_del_wkdep)
+ ret = -EINVAL;
+
+ if (ret) {
+ pr_debug("clockdomain: hardware cannot set/clear wake up of %s when %s wakes up\n",
+ clkdm1->name, clkdm2->name);
+ return ret;
+ }
+
+ if (atomic_dec_return(&cd->wkdep_usecount) == 0) {
+ pr_debug("clockdomain: hardware will no longer wake up %s after %s wakes up\n",
+ clkdm1->name, clkdm2->name);
+
+ ret = arch_clkdm->clkdm_del_wkdep(clkdm1, clkdm2);
+ }
+
+ return ret;
+}
+
+/**
+ * _clkdm_add_sleepdep - add a sleep dependency from clkdm2 to clkdm1 (lockless)
+ * @clkdm1: prevent this struct clockdomain * from sleeping (dependent)
+ * @clkdm2: when this struct clockdomain * is active (source)
*
- * XXX autodeps are deprecated and should be removed at the earliest
- * opportunity
+ * Prevent @clkdm1 from automatically going inactive (and then to
+ * retention or off) if @clkdm2 is active. Returns -EINVAL if
+ * presented with invalid clockdomain pointers or called on a machine
+ * that does not support software-configurable hardware sleep
+ * dependencies, -ENOENT if the specified dependency cannot be set in
+ * hardware, or 0 upon success.
*/
-void _clkdm_del_autodeps(struct clockdomain *clkdm)
+static int _clkdm_add_sleepdep(struct clockdomain *clkdm1,
+ struct clockdomain *clkdm2)
{
- struct clkdm_autodep *autodep;
+ struct clkdm_dep *cd;
+ int ret = 0;
- if (!autodeps || clkdm->flags & CLKDM_NO_AUTODEPS)
- return;
+ if (!clkdm1 || !clkdm2)
+ return -EINVAL;
- for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {
- if (IS_ERR(autodep->clkdm.ptr))
- continue;
+ cd = _clkdm_deps_lookup(clkdm2, clkdm1->sleepdep_srcs);
+ if (IS_ERR(cd))
+ ret = PTR_ERR(cd);
- pr_debug("clockdomain: %s: removing %s sleepdep/wkdep\n",
- clkdm->name, autodep->clkdm.ptr->name);
+ if (!arch_clkdm || !arch_clkdm->clkdm_add_sleepdep)
+ ret = -EINVAL;
- clkdm_del_sleepdep(clkdm, autodep->clkdm.ptr);
- clkdm_del_wkdep(clkdm, autodep->clkdm.ptr);
+ if (ret) {
+ pr_debug("clockdomain: hardware cannot set/clear sleep dependency affecting %s from %s\n",
+ clkdm1->name, clkdm2->name);
+ return ret;
}
+
+ if (atomic_inc_return(&cd->sleepdep_usecount) == 1) {
+ pr_debug("clockdomain: will prevent %s from sleeping if %s is active\n",
+ clkdm1->name, clkdm2->name);
+
+ ret = arch_clkdm->clkdm_add_sleepdep(clkdm1, clkdm2);
+ }
+
+ return ret;
}
/**
- * _resolve_clkdm_deps() - resolve clkdm_names in @clkdm_deps to clkdms
- * @clkdm: clockdomain that we are resolving dependencies for
- * @clkdm_deps: ptr to array of struct clkdm_deps to resolve
+ * _clkdm_del_sleepdep - remove a sleep dep from clkdm2 to clkdm1 (lockless)
+ * @clkdm1: prevent this struct clockdomain * from sleeping (dependent)
+ * @clkdm2: when this struct clockdomain * is active (source)
*
- * Iterates through @clkdm_deps, looking up the struct clockdomain named by
- * clkdm_name and storing the clockdomain pointer in the struct clkdm_dep.
- * No return value.
+ * Allow @clkdm1 to automatically go inactive (and then to retention or
+ * off), independent of the activity state of @clkdm2. Returns -EINVAL
+ * if presented with invalid clockdomain pointers or called on a machine
+ * that does not support software-configurable hardware sleep dependencies,
+ * -ENOENT if the specified dependency cannot be cleared in hardware, or
+ * 0 upon success.
*/
-static void _resolve_clkdm_deps(struct clockdomain *clkdm,
- struct clkdm_dep *clkdm_deps)
+static int _clkdm_del_sleepdep(struct clockdomain *clkdm1,
+ struct clockdomain *clkdm2)
{
struct clkdm_dep *cd;
+ int ret = 0;
- for (cd = clkdm_deps; cd && cd->clkdm_name; cd++) {
- if (cd->clkdm)
- continue;
- cd->clkdm = _clkdm_lookup(cd->clkdm_name);
+ if (!clkdm1 || !clkdm2)
+ return -EINVAL;
- WARN(!cd->clkdm, "clockdomain: %s: could not find clkdm %s while resolving dependencies - should never happen",
- clkdm->name, cd->clkdm_name);
+ cd = _clkdm_deps_lookup(clkdm2, clkdm1->sleepdep_srcs);
+ if (IS_ERR(cd))
+ ret = PTR_ERR(cd);
+
+ if (!arch_clkdm || !arch_clkdm->clkdm_del_sleepdep)
+ ret = -EINVAL;
+
+ if (ret) {
+ pr_debug("clockdomain: hardware cannot set/clear sleep dependency affecting %s from %s\n",
+ clkdm1->name, clkdm2->name);
+ return ret;
}
+
+ if (atomic_dec_return(&cd->sleepdep_usecount) == 0) {
+ pr_debug("clockdomain: will no longer prevent %s from sleeping if %s is active\n",
+ clkdm1->name, clkdm2->name);
+
+ ret = arch_clkdm->clkdm_del_sleepdep(clkdm1, clkdm2);
+ }
+
+ return ret;
}
/* Public functions */
@@ -453,33 +567,7 @@ struct powerdomain *clkdm_get_pwrdm(struct clockdomain *clkdm)
*/
int clkdm_add_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
{
- struct clkdm_dep *cd;
- int ret = 0;
-
- if (!clkdm1 || !clkdm2)
- return -EINVAL;
-
- cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
- if (IS_ERR(cd))
- ret = PTR_ERR(cd);
-
- if (!arch_clkdm || !arch_clkdm->clkdm_add_wkdep)
- ret = -EINVAL;
-
- if (ret) {
- pr_debug("clockdomain: hardware cannot set/clear wake up of %s when %s wakes up\n",
- clkdm1->name, clkdm2->name);
- return ret;
- }
-
- if (atomic_inc_return(&cd->wkdep_usecount) == 1) {
- pr_debug("clockdomain: hardware will wake up %s when %s wakes up\n",
- clkdm1->name, clkdm2->name);
-
- ret = arch_clkdm->clkdm_add_wkdep(clkdm1, clkdm2);
- }
-
- return ret;
+ return _clkdm_add_wkdep(clkdm1, clkdm2);
}
/**
@@ -494,33 +582,7 @@ int clkdm_add_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
*/
int clkdm_del_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
{
- struct clkdm_dep *cd;
- int ret = 0;
-
- if (!clkdm1 || !clkdm2)
- return -EINVAL;
-
- cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
- if (IS_ERR(cd))
- ret = PTR_ERR(cd);
-
- if (!arch_clkdm || !arch_clkdm->clkdm_del_wkdep)
- ret = -EINVAL;
-
- if (ret) {
- pr_debug("clockdomain: hardware cannot set/clear wake up of %s when %s wakes up\n",
- clkdm1->name, clkdm2->name);
- return ret;
- }
-
- if (atomic_dec_return(&cd->wkdep_usecount) == 0) {
- pr_debug("clockdomain: hardware will no longer wake up %s after %s wakes up\n",
- clkdm1->name, clkdm2->name);
-
- ret = arch_clkdm->clkdm_del_wkdep(clkdm1, clkdm2);
- }
-
- return ret;
+ return _clkdm_del_wkdep(clkdm1, clkdm2);
}
/**
@@ -597,33 +659,7 @@ int clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
*/
int clkdm_add_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
{
- struct clkdm_dep *cd;
- int ret = 0;
-
- if (!clkdm1 || !clkdm2)
- return -EINVAL;
-
- cd = _clkdm_deps_lookup(clkdm2, clkdm1->sleepdep_srcs);
- if (IS_ERR(cd))
- ret = PTR_ERR(cd);
-
- if (!arch_clkdm || !arch_clkdm->clkdm_add_sleepdep)
- ret = -EINVAL;
-
- if (ret) {
- pr_debug("clockdomain: hardware cannot set/clear sleep dependency affecting %s from %s\n",
- clkdm1->name, clkdm2->name);
- return ret;
- }
-
- if (atomic_inc_return(&cd->sleepdep_usecount) == 1) {
- pr_debug("clockdomain: will prevent %s from sleeping if %s is active\n",
- clkdm1->name, clkdm2->name);
-
- ret = arch_clkdm->clkdm_add_sleepdep(clkdm1, clkdm2);
- }
-
- return ret;
+ return _clkdm_add_sleepdep(clkdm1, clkdm2);
}
/**
@@ -640,33 +676,7 @@ int clkdm_add_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
*/
int clkdm_del_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
{
- struct clkdm_dep *cd;
- int ret = 0;
-
- if (!clkdm1 || !clkdm2)
- return -EINVAL;
-
- cd = _clkdm_deps_lookup(clkdm2, clkdm1->sleepdep_srcs);
- if (IS_ERR(cd))
- ret = PTR_ERR(cd);
-
- if (!arch_clkdm || !arch_clkdm->clkdm_del_sleepdep)
- ret = -EINVAL;
-
- if (ret) {
- pr_debug("clockdomain: hardware cannot set/clear sleep dependency affecting %s from %s\n",
- clkdm1->name, clkdm2->name);
- return ret;
- }
-
- if (atomic_dec_return(&cd->sleepdep_usecount) == 0) {
- pr_debug("clockdomain: will no longer prevent %s from sleeping if %s is active\n",
- clkdm1->name, clkdm2->name);
-
- ret = arch_clkdm->clkdm_del_sleepdep(clkdm1, clkdm2);
- }
-
- return ret;
+ return _clkdm_del_sleepdep(clkdm1, clkdm2);
}
/**
@@ -977,6 +987,68 @@ bool clkdm_missing_idle_reporting(struct clockdomain *clkdm)
return (clkdm->flags & CLKDM_MISSING_IDLE_REPORTING) ? true : false;
}
+/* Public autodep handling functions (deprecated) */
+
+/**
+ * clkdm_add_autodeps - add auto sleepdeps/wkdeps to clkdm upon clock enable
+ * @clkdm: struct clockdomain *
+ *
+ * Add the "autodep" sleep & wakeup dependencies to clockdomain 'clkdm'
+ * in hardware-supervised mode. Meant to be called from clock framework
+ * when a clock inside clockdomain 'clkdm' is enabled. No return value.
+ *
+ * XXX autodeps are deprecated and should be removed at the earliest
+ * opportunity
+ */
+void clkdm_add_autodeps(struct clockdomain *clkdm)
+{
+ struct clkdm_autodep *autodep;
+
+ if (!autodeps || clkdm->flags & CLKDM_NO_AUTODEPS)
+ return;
+
+ for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {
+ if (IS_ERR(autodep->clkdm.ptr))
+ continue;
+
+ pr_debug("clockdomain: %s: adding %s sleepdep/wkdep\n",
+ clkdm->name, autodep->clkdm.ptr->name);
+
+ _clkdm_add_sleepdep(clkdm, autodep->clkdm.ptr);
+ _clkdm_add_wkdep(clkdm, autodep->clkdm.ptr);
+ }
+}
+
+/**
+ * clkdm_del_autodeps - remove auto sleepdeps/wkdeps from clkdm
+ * @clkdm: struct clockdomain *
+ *
+ * Remove the "autodep" sleep & wakeup dependencies from clockdomain 'clkdm'
+ * in hardware-supervised mode. Meant to be called from clock framework
+ * when a clock inside clockdomain 'clkdm' is disabled. No return value.
+ *
+ * XXX autodeps are deprecated and should be removed at the earliest
+ * opportunity
+ */
+void clkdm_del_autodeps(struct clockdomain *clkdm)
+{
+ struct clkdm_autodep *autodep;
+
+ if (!autodeps || clkdm->flags & CLKDM_NO_AUTODEPS)
+ return;
+
+ for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {
+ if (IS_ERR(autodep->clkdm.ptr))
+ continue;
+
+ pr_debug("clockdomain: %s: removing %s sleepdep/wkdep\n",
+ clkdm->name, autodep->clkdm.ptr->name);
+
+ _clkdm_del_sleepdep(clkdm, autodep->clkdm.ptr);
+ _clkdm_del_wkdep(clkdm, autodep->clkdm.ptr);
+ }
+}
+
/* Clockdomain-to-clock/hwmod framework interface code */
static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index e7f1b4b..50c3cd8 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -216,8 +216,9 @@ extern void __init omap243x_clockdomains_init(void);
extern void __init omap3xxx_clockdomains_init(void);
extern void __init am33xx_clockdomains_init(void);
extern void __init omap44xx_clockdomains_init(void);
-extern void _clkdm_add_autodeps(struct clockdomain *clkdm);
-extern void _clkdm_del_autodeps(struct clockdomain *clkdm);
+
+extern void clkdm_add_autodeps(struct clockdomain *clkdm);
+extern void clkdm_del_autodeps(struct clockdomain *clkdm);
extern struct clkdm_ops omap2_clkdm_operations;
extern struct clkdm_ops omap3_clkdm_operations;
diff --git a/arch/arm/mach-omap2/cm3xxx.c b/arch/arm/mach-omap2/cm3xxx.c
index c2086f2..b94af4c 100644
--- a/arch/arm/mach-omap2/cm3xxx.c
+++ b/arch/arm/mach-omap2/cm3xxx.c
@@ -210,7 +210,7 @@ static int omap3xxx_clkdm_wakeup(struct clockdomain *clkdm)
static void omap3xxx_clkdm_allow_idle(struct clockdomain *clkdm)
{
if (atomic_read(&clkdm->usecount) > 0)
- _clkdm_add_autodeps(clkdm);
+ clkdm_add_autodeps(clkdm);
omap3xxx_cm_clkdm_enable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
@@ -222,7 +222,7 @@ static void omap3xxx_clkdm_deny_idle(struct clockdomain *clkdm)
clkdm->clktrctrl_mask);
if (atomic_read(&clkdm->usecount) > 0)
- _clkdm_del_autodeps(clkdm);
+ clkdm_del_autodeps(clkdm);
}
static int omap3xxx_clkdm_clk_enable(struct clockdomain *clkdm)
@@ -250,7 +250,7 @@ static int omap3xxx_clkdm_clk_enable(struct clockdomain *clkdm)
/* Disable HW transitions when we are changing deps */
omap3xxx_cm_clkdm_disable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
- _clkdm_add_autodeps(clkdm);
+ clkdm_add_autodeps(clkdm);
omap3xxx_cm_clkdm_enable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
} else {
@@ -287,7 +287,7 @@ static int omap3xxx_clkdm_clk_disable(struct clockdomain *clkdm)
/* Disable HW transitions when we are changing deps */
omap3xxx_cm_clkdm_disable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
- _clkdm_del_autodeps(clkdm);
+ clkdm_del_autodeps(clkdm);
omap3xxx_cm_clkdm_enable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
} else {
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
` (7 preceding siblings ...)
2012-12-09 1:23 ` [PATCH 08/10] ARM: OMAP2+: clockdomain: work on wkdep/sleepdep functions Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2012-12-12 10:21 ` Vaibhav Hiremath
2012-12-26 6:31 ` Bedia, Vaibhav
2012-12-09 1:23 ` [PATCH 10/10] ARM: OMAP2+: powerdomain: fix whitespace, improve flag comments Paul Walmsley
2013-01-04 13:07 ` [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Tero Kristo
10 siblings, 2 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
The atomic usecounts seem to be confusing, and are no longer needed
since the operations that they are attached to really should take
place under lock. Replace the atomic counters with simple integers,
protected by the enclosing powerdomain spinlock.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
arch/arm/mach-omap2/clockdomain.c | 88 +++++++++++++++++++++++++++++++-----
arch/arm/mach-omap2/clockdomain.h | 6 +-
arch/arm/mach-omap2/cm3xxx.c | 6 +-
arch/arm/mach-omap2/cminst44xx.c | 2 -
arch/arm/mach-omap2/pm-debug.c | 6 +-
arch/arm/mach-omap2/pm.c | 3 +
arch/arm/mach-omap2/prm2xxx_3xxx.c | 3 +
7 files changed, 88 insertions(+), 26 deletions(-)
diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 9d5b69f..ed8ac2f 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -210,7 +210,8 @@ static int _clkdm_add_wkdep(struct clockdomain *clkdm1,
return ret;
}
- if (atomic_inc_return(&cd->wkdep_usecount) == 1) {
+ cd->wkdep_usecount++;
+ if (cd->wkdep_usecount == 1) {
pr_debug("clockdomain: hardware will wake up %s when %s wakes up\n",
clkdm1->name, clkdm2->name);
@@ -252,7 +253,8 @@ static int _clkdm_del_wkdep(struct clockdomain *clkdm1,
return ret;
}
- if (atomic_dec_return(&cd->wkdep_usecount) == 0) {
+ cd->wkdep_usecount--;
+ if (cd->wkdep_usecount == 0) {
pr_debug("clockdomain: hardware will no longer wake up %s after %s wakes up\n",
clkdm1->name, clkdm2->name);
@@ -296,7 +298,8 @@ static int _clkdm_add_sleepdep(struct clockdomain *clkdm1,
return ret;
}
- if (atomic_inc_return(&cd->sleepdep_usecount) == 1) {
+ cd->sleepdep_usecount++;
+ if (cd->sleepdep_usecount == 1) {
pr_debug("clockdomain: will prevent %s from sleeping if %s is active\n",
clkdm1->name, clkdm2->name);
@@ -340,7 +343,8 @@ static int _clkdm_del_sleepdep(struct clockdomain *clkdm1,
return ret;
}
- if (atomic_dec_return(&cd->sleepdep_usecount) == 0) {
+ cd->sleepdep_usecount--;
+ if (cd->sleepdep_usecount == 0) {
pr_debug("clockdomain: will no longer prevent %s from sleeping if %s is active\n",
clkdm1->name, clkdm2->name);
@@ -567,7 +571,21 @@ struct powerdomain *clkdm_get_pwrdm(struct clockdomain *clkdm)
*/
int clkdm_add_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
{
- return _clkdm_add_wkdep(clkdm1, clkdm2);
+ struct clkdm_dep *cd;
+ int ret;
+
+ if (!clkdm1 || !clkdm2)
+ return -EINVAL;
+
+ cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
+ if (IS_ERR(cd))
+ return PTR_ERR(cd);
+
+ pwrdm_lock(cd->clkdm->pwrdm.ptr);
+ ret = _clkdm_add_wkdep(clkdm1, clkdm2);
+ pwrdm_unlock(cd->clkdm->pwrdm.ptr);
+
+ return ret;
}
/**
@@ -582,7 +600,21 @@ int clkdm_add_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
*/
int clkdm_del_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
{
- return _clkdm_del_wkdep(clkdm1, clkdm2);
+ struct clkdm_dep *cd;
+ int ret;
+
+ if (!clkdm1 || !clkdm2)
+ return -EINVAL;
+
+ cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
+ if (IS_ERR(cd))
+ return PTR_ERR(cd);
+
+ pwrdm_lock(cd->clkdm->pwrdm.ptr);
+ ret = _clkdm_del_wkdep(clkdm1, clkdm2);
+ pwrdm_unlock(cd->clkdm->pwrdm.ptr);
+
+ return ret;
}
/**
@@ -620,7 +652,7 @@ int clkdm_read_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
return ret;
}
- /* XXX It's faster to return the atomic wkdep_usecount */
+ /* XXX It's faster to return the wkdep_usecount */
return arch_clkdm->clkdm_read_wkdep(clkdm1, clkdm2);
}
@@ -659,7 +691,21 @@ int clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
*/
int clkdm_add_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
{
- return _clkdm_add_sleepdep(clkdm1, clkdm2);
+ struct clkdm_dep *cd;
+ int ret;
+
+ if (!clkdm1 || !clkdm2)
+ return -EINVAL;
+
+ cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
+ if (IS_ERR(cd))
+ return PTR_ERR(cd);
+
+ pwrdm_lock(cd->clkdm->pwrdm.ptr);
+ ret = _clkdm_add_sleepdep(clkdm1, clkdm2);
+ pwrdm_unlock(cd->clkdm->pwrdm.ptr);
+
+ return ret;
}
/**
@@ -676,7 +722,21 @@ int clkdm_add_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
*/
int clkdm_del_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
{
- return _clkdm_del_sleepdep(clkdm1, clkdm2);
+ struct clkdm_dep *cd;
+ int ret;
+
+ if (!clkdm1 || !clkdm2)
+ return -EINVAL;
+
+ cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
+ if (IS_ERR(cd))
+ return PTR_ERR(cd);
+
+ pwrdm_lock(cd->clkdm->pwrdm.ptr);
+ ret = _clkdm_del_sleepdep(clkdm1, clkdm2);
+ pwrdm_unlock(cd->clkdm->pwrdm.ptr);
+
+ return ret;
}
/**
@@ -716,7 +776,7 @@ int clkdm_read_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
return ret;
}
- /* XXX It's faster to return the atomic sleepdep_usecount */
+ /* XXX It's faster to return the sleepdep_usecount */
return arch_clkdm->clkdm_read_sleepdep(clkdm1, clkdm2);
}
@@ -1063,7 +1123,8 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
* should be called for every clock instance or hwmod that is
* enabled, so the clkdm can be force woken up.
*/
- if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps) {
+ clkdm->usecount++;
+ if (clkdm->usecount > 1 && autodeps) {
pwrdm_unlock(clkdm->pwrdm.ptr);
return 0;
}
@@ -1084,13 +1145,14 @@ static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
pwrdm_lock(clkdm->pwrdm.ptr);
- if (atomic_read(&clkdm->usecount) == 0) {
+ if (clkdm->usecount == 0) {
pwrdm_unlock(clkdm->pwrdm.ptr);
WARN_ON(1); /* underflow */
return -ERANGE;
}
- if (atomic_dec_return(&clkdm->usecount) > 0) {
+ clkdm->usecount--;
+ if (clkdm->usecount > 0) {
pwrdm_unlock(clkdm->pwrdm.ptr);
return 0;
}
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 50c3cd8..2da3765 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -91,8 +91,8 @@ struct clkdm_autodep {
struct clkdm_dep {
const char *clkdm_name;
struct clockdomain *clkdm;
- atomic_t wkdep_usecount;
- atomic_t sleepdep_usecount;
+ s16 wkdep_usecount;
+ s16 sleepdep_usecount;
};
/* Possible flags for struct clockdomain._flags */
@@ -136,7 +136,7 @@ struct clockdomain {
const u16 clkdm_offs;
struct clkdm_dep *wkdep_srcs;
struct clkdm_dep *sleepdep_srcs;
- atomic_t usecount;
+ int usecount;
struct list_head node;
};
diff --git a/arch/arm/mach-omap2/cm3xxx.c b/arch/arm/mach-omap2/cm3xxx.c
index b94af4c..9061c30 100644
--- a/arch/arm/mach-omap2/cm3xxx.c
+++ b/arch/arm/mach-omap2/cm3xxx.c
@@ -186,7 +186,7 @@ static int omap3xxx_clkdm_clear_all_sleepdeps(struct clockdomain *clkdm)
continue; /* only happens if data is erroneous */
mask |= 1 << cd->clkdm->dep_bit;
- atomic_set(&cd->sleepdep_usecount, 0);
+ cd->sleepdep_usecount = 0;
}
omap2_cm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs,
OMAP3430_CM_SLEEPDEP);
@@ -209,7 +209,7 @@ static int omap3xxx_clkdm_wakeup(struct clockdomain *clkdm)
static void omap3xxx_clkdm_allow_idle(struct clockdomain *clkdm)
{
- if (atomic_read(&clkdm->usecount) > 0)
+ if (clkdm->usecount > 0)
clkdm_add_autodeps(clkdm);
omap3xxx_cm_clkdm_enable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
@@ -221,7 +221,7 @@ static void omap3xxx_clkdm_deny_idle(struct clockdomain *clkdm)
omap3xxx_cm_clkdm_disable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
clkdm->clktrctrl_mask);
- if (atomic_read(&clkdm->usecount) > 0)
+ if (clkdm->usecount > 0)
clkdm_del_autodeps(clkdm);
}
diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
index 7f9a464..f0290f5 100644
--- a/arch/arm/mach-omap2/cminst44xx.c
+++ b/arch/arm/mach-omap2/cminst44xx.c
@@ -393,7 +393,7 @@ static int omap4_clkdm_clear_all_wkup_sleep_deps(struct clockdomain *clkdm)
continue; /* only happens if data is erroneous */
mask |= 1 << cd->clkdm->dep_bit;
- atomic_set(&cd->wkdep_usecount, 0);
+ cd->wkdep_usecount = 0;
}
omap4_cminst_clear_inst_reg_bits(mask, clkdm->prcm_partition,
diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c
index 3cf4fdf..806a06b 100644
--- a/arch/arm/mach-omap2/pm-debug.c
+++ b/arch/arm/mach-omap2/pm-debug.c
@@ -84,10 +84,8 @@ static int clkdm_dbg_show_counter(struct clockdomain *clkdm, void *user)
strncmp(clkdm->name, "dpll", 4) == 0)
return 0;
- seq_printf(s, "%s->%s (%d)", clkdm->name,
- clkdm->pwrdm.ptr->name,
- atomic_read(&clkdm->usecount));
- seq_printf(s, "\n");
+ seq_printf(s, "%s->%s (%d)\n", clkdm->name, clkdm->pwrdm.ptr->name,
+ clkdm->usecount);
return 0;
}
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 2e2a897..6b7cb7c 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -78,11 +78,12 @@ static void __init omap2_init_processor_devices(void)
int __init omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused)
{
+ /* XXX The usecount test is racy */
if ((clkdm->flags & CLKDM_CAN_ENABLE_AUTO) &&
!(clkdm->flags & CLKDM_MISSING_IDLE_REPORTING))
clkdm_allow_idle(clkdm);
else if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP &&
- atomic_read(&clkdm->usecount) == 0)
+ clkdm->usecount == 0)
clkdm_sleep(clkdm);
return 0;
}
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
index a3e121f..947f6ad 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
@@ -210,6 +210,7 @@ int omap2_clkdm_read_wkdep(struct clockdomain *clkdm1,
PM_WKDEP, (1 << clkdm2->dep_bit));
}
+/* XXX Caller must hold the clkdm's powerdomain lock */
int omap2_clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
{
struct clkdm_dep *cd;
@@ -221,7 +222,7 @@ int omap2_clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
/* PRM accesses are slow, so minimize them */
mask |= 1 << cd->clkdm->dep_bit;
- atomic_set(&cd->wkdep_usecount, 0);
+ cd->wkdep_usecount = 0;
}
omap2_prm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs,
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints
2012-12-09 1:23 ` [PATCH 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints Paul Walmsley
@ 2012-12-12 10:21 ` Vaibhav Hiremath
2012-12-26 6:31 ` Bedia, Vaibhav
1 sibling, 0 replies; 28+ messages in thread
From: Vaibhav Hiremath @ 2012-12-12 10:21 UTC (permalink / raw)
To: linux-arm-kernel
On 12/9/2012 6:53 AM, Paul Walmsley wrote:
> The atomic usecounts seem to be confusing, and are no longer needed
> since the operations that they are attached to really should take
> place under lock. Replace the atomic counters with simple integers,
> protected by the enclosing powerdomain spinlock.
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> ---
> arch/arm/mach-omap2/clockdomain.c | 88 +++++++++++++++++++++++++++++++-----
> arch/arm/mach-omap2/clockdomain.h | 6 +-
> arch/arm/mach-omap2/cm3xxx.c | 6 +-
> arch/arm/mach-omap2/cminst44xx.c | 2 -
> arch/arm/mach-omap2/pm-debug.c | 6 +-
> arch/arm/mach-omap2/pm.c | 3 +
> arch/arm/mach-omap2/prm2xxx_3xxx.c | 3 +
> 7 files changed, 88 insertions(+), 26 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
> index 9d5b69f..ed8ac2f 100644
> --- a/arch/arm/mach-omap2/clockdomain.c
> +++ b/arch/arm/mach-omap2/clockdomain.c
> @@ -210,7 +210,8 @@ static int _clkdm_add_wkdep(struct clockdomain *clkdm1,
> return ret;
> }
>
> - if (atomic_inc_return(&cd->wkdep_usecount) == 1) {
> + cd->wkdep_usecount++;
> + if (cd->wkdep_usecount == 1) {
> pr_debug("clockdomain: hardware will wake up %s when %s wakes up\n",
> clkdm1->name, clkdm2->name);
>
> @@ -252,7 +253,8 @@ static int _clkdm_del_wkdep(struct clockdomain *clkdm1,
> return ret;
> }
>
> - if (atomic_dec_return(&cd->wkdep_usecount) == 0) {
> + cd->wkdep_usecount--;
> + if (cd->wkdep_usecount == 0) {
> pr_debug("clockdomain: hardware will no longer wake up %s after %s wakes up\n",
> clkdm1->name, clkdm2->name);
>
> @@ -296,7 +298,8 @@ static int _clkdm_add_sleepdep(struct clockdomain *clkdm1,
> return ret;
> }
>
> - if (atomic_inc_return(&cd->sleepdep_usecount) == 1) {
> + cd->sleepdep_usecount++;
> + if (cd->sleepdep_usecount == 1) {
> pr_debug("clockdomain: will prevent %s from sleeping if %s is active\n",
> clkdm1->name, clkdm2->name);
>
> @@ -340,7 +343,8 @@ static int _clkdm_del_sleepdep(struct clockdomain *clkdm1,
> return ret;
> }
>
> - if (atomic_dec_return(&cd->sleepdep_usecount) == 0) {
> + cd->sleepdep_usecount--;
> + if (cd->sleepdep_usecount == 0) {
> pr_debug("clockdomain: will no longer prevent %s from sleeping if %s is active\n",
> clkdm1->name, clkdm2->name);
>
> @@ -567,7 +571,21 @@ struct powerdomain *clkdm_get_pwrdm(struct clockdomain *clkdm)
> */
> int clkdm_add_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> {
> - return _clkdm_add_wkdep(clkdm1, clkdm2);
> + struct clkdm_dep *cd;
> + int ret;
> +
> + if (!clkdm1 || !clkdm2)
> + return -EINVAL;
> +
> + cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
> + if (IS_ERR(cd))
> + return PTR_ERR(cd);
> +
> + pwrdm_lock(cd->clkdm->pwrdm.ptr);
> + ret = _clkdm_add_wkdep(clkdm1, clkdm2);
> + pwrdm_unlock(cd->clkdm->pwrdm.ptr);
> +
> + return ret;
> }
>
> /**
> @@ -582,7 +600,21 @@ int clkdm_add_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> */
> int clkdm_del_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> {
> - return _clkdm_del_wkdep(clkdm1, clkdm2);
> + struct clkdm_dep *cd;
> + int ret;
> +
> + if (!clkdm1 || !clkdm2)
> + return -EINVAL;
> +
> + cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
> + if (IS_ERR(cd))
> + return PTR_ERR(cd);
> +
> + pwrdm_lock(cd->clkdm->pwrdm.ptr);
> + ret = _clkdm_del_wkdep(clkdm1, clkdm2);
> + pwrdm_unlock(cd->clkdm->pwrdm.ptr);
> +
> + return ret;
> }
>
> /**
> @@ -620,7 +652,7 @@ int clkdm_read_wkdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> return ret;
> }
>
> - /* XXX It's faster to return the atomic wkdep_usecount */
> + /* XXX It's faster to return the wkdep_usecount */
> return arch_clkdm->clkdm_read_wkdep(clkdm1, clkdm2);
> }
>
> @@ -659,7 +691,21 @@ int clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
> */
> int clkdm_add_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> {
> - return _clkdm_add_sleepdep(clkdm1, clkdm2);
> + struct clkdm_dep *cd;
> + int ret;
> +
> + if (!clkdm1 || !clkdm2)
> + return -EINVAL;
> +
> + cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
> + if (IS_ERR(cd))
> + return PTR_ERR(cd);
> +
> + pwrdm_lock(cd->clkdm->pwrdm.ptr);
> + ret = _clkdm_add_sleepdep(clkdm1, clkdm2);
> + pwrdm_unlock(cd->clkdm->pwrdm.ptr);
> +
> + return ret;
> }
>
> /**
> @@ -676,7 +722,21 @@ int clkdm_add_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> */
> int clkdm_del_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> {
> - return _clkdm_del_sleepdep(clkdm1, clkdm2);
> + struct clkdm_dep *cd;
> + int ret;
> +
> + if (!clkdm1 || !clkdm2)
> + return -EINVAL;
> +
> + cd = _clkdm_deps_lookup(clkdm2, clkdm1->wkdep_srcs);
> + if (IS_ERR(cd))
> + return PTR_ERR(cd);
> +
> + pwrdm_lock(cd->clkdm->pwrdm.ptr);
> + ret = _clkdm_del_sleepdep(clkdm1, clkdm2);
> + pwrdm_unlock(cd->clkdm->pwrdm.ptr);
> +
> + return ret;
> }
>
> /**
> @@ -716,7 +776,7 @@ int clkdm_read_sleepdep(struct clockdomain *clkdm1, struct clockdomain *clkdm2)
> return ret;
> }
>
> - /* XXX It's faster to return the atomic sleepdep_usecount */
> + /* XXX It's faster to return the sleepdep_usecount */
> return arch_clkdm->clkdm_read_sleepdep(clkdm1, clkdm2);
> }
>
> @@ -1063,7 +1123,8 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
> * should be called for every clock instance or hwmod that is
> * enabled, so the clkdm can be force woken up.
> */
> - if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps) {
> + clkdm->usecount++;
> + if (clkdm->usecount > 1 && autodeps) {
> pwrdm_unlock(clkdm->pwrdm.ptr);
> return 0;
> }
> @@ -1084,13 +1145,14 @@ static int _clkdm_clk_hwmod_disable(struct clockdomain *clkdm)
>
> pwrdm_lock(clkdm->pwrdm.ptr);
>
> - if (atomic_read(&clkdm->usecount) == 0) {
> + if (clkdm->usecount == 0) {
> pwrdm_unlock(clkdm->pwrdm.ptr);
> WARN_ON(1); /* underflow */
> return -ERANGE;
> }
>
> - if (atomic_dec_return(&clkdm->usecount) > 0) {
> + clkdm->usecount--;
> + if (clkdm->usecount > 0) {
> pwrdm_unlock(clkdm->pwrdm.ptr);
> return 0;
> }
> diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
> index 50c3cd8..2da3765 100644
> --- a/arch/arm/mach-omap2/clockdomain.h
> +++ b/arch/arm/mach-omap2/clockdomain.h
> @@ -91,8 +91,8 @@ struct clkdm_autodep {
> struct clkdm_dep {
> const char *clkdm_name;
> struct clockdomain *clkdm;
> - atomic_t wkdep_usecount;
> - atomic_t sleepdep_usecount;
> + s16 wkdep_usecount;
> + s16 sleepdep_usecount;
> };
>
> /* Possible flags for struct clockdomain._flags */
> @@ -136,7 +136,7 @@ struct clockdomain {
> const u16 clkdm_offs;
> struct clkdm_dep *wkdep_srcs;
> struct clkdm_dep *sleepdep_srcs;
> - atomic_t usecount;
> + int usecount;
> struct list_head node;
> };
>
> diff --git a/arch/arm/mach-omap2/cm3xxx.c b/arch/arm/mach-omap2/cm3xxx.c
> index b94af4c..9061c30 100644
> --- a/arch/arm/mach-omap2/cm3xxx.c
> +++ b/arch/arm/mach-omap2/cm3xxx.c
> @@ -186,7 +186,7 @@ static int omap3xxx_clkdm_clear_all_sleepdeps(struct clockdomain *clkdm)
> continue; /* only happens if data is erroneous */
>
> mask |= 1 << cd->clkdm->dep_bit;
> - atomic_set(&cd->sleepdep_usecount, 0);
> + cd->sleepdep_usecount = 0;
> }
> omap2_cm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs,
> OMAP3430_CM_SLEEPDEP);
> @@ -209,7 +209,7 @@ static int omap3xxx_clkdm_wakeup(struct clockdomain *clkdm)
>
> static void omap3xxx_clkdm_allow_idle(struct clockdomain *clkdm)
> {
> - if (atomic_read(&clkdm->usecount) > 0)
> + if (clkdm->usecount > 0)
> clkdm_add_autodeps(clkdm);
>
> omap3xxx_cm_clkdm_enable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
> @@ -221,7 +221,7 @@ static void omap3xxx_clkdm_deny_idle(struct clockdomain *clkdm)
> omap3xxx_cm_clkdm_disable_hwsup(clkdm->pwrdm.ptr->prcm_offs,
> clkdm->clktrctrl_mask);
>
> - if (atomic_read(&clkdm->usecount) > 0)
> + if (clkdm->usecount > 0)
> clkdm_del_autodeps(clkdm);
> }
>
> diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
> index 7f9a464..f0290f5 100644
> --- a/arch/arm/mach-omap2/cminst44xx.c
> +++ b/arch/arm/mach-omap2/cminst44xx.c
> @@ -393,7 +393,7 @@ static int omap4_clkdm_clear_all_wkup_sleep_deps(struct clockdomain *clkdm)
> continue; /* only happens if data is erroneous */
>
> mask |= 1 << cd->clkdm->dep_bit;
> - atomic_set(&cd->wkdep_usecount, 0);
> + cd->wkdep_usecount = 0;
> }
>
> omap4_cminst_clear_inst_reg_bits(mask, clkdm->prcm_partition,
> diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c
> index 3cf4fdf..806a06b 100644
> --- a/arch/arm/mach-omap2/pm-debug.c
> +++ b/arch/arm/mach-omap2/pm-debug.c
> @@ -84,10 +84,8 @@ static int clkdm_dbg_show_counter(struct clockdomain *clkdm, void *user)
> strncmp(clkdm->name, "dpll", 4) == 0)
> return 0;
>
> - seq_printf(s, "%s->%s (%d)", clkdm->name,
> - clkdm->pwrdm.ptr->name,
> - atomic_read(&clkdm->usecount));
> - seq_printf(s, "\n");
> + seq_printf(s, "%s->%s (%d)\n", clkdm->name, clkdm->pwrdm.ptr->name,
> + clkdm->usecount);
>
> return 0;
> }
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index 2e2a897..6b7cb7c 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -78,11 +78,12 @@ static void __init omap2_init_processor_devices(void)
>
> int __init omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused)
> {
> + /* XXX The usecount test is racy */
> if ((clkdm->flags & CLKDM_CAN_ENABLE_AUTO) &&
> !(clkdm->flags & CLKDM_MISSING_IDLE_REPORTING))
> clkdm_allow_idle(clkdm);
> else if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP &&
> - atomic_read(&clkdm->usecount) == 0)
> + clkdm->usecount == 0)
> clkdm_sleep(clkdm);
> return 0;
> }
> diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
> index a3e121f..947f6ad 100644
> --- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
> +++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
> @@ -210,6 +210,7 @@ int omap2_clkdm_read_wkdep(struct clockdomain *clkdm1,
> PM_WKDEP, (1 << clkdm2->dep_bit));
> }
>
> +/* XXX Caller must hold the clkdm's powerdomain lock */
Isn't this comment applicable for all the internal api's
omap2_clkdm_clear_all_wkdeps,
omap3xxx_clkdm_add_sleepdep,
omap3xxx_clkdm_del_sleepdep,
omap3xxx_clkdm_read_sleepdep,
omap3xxx_clkdm_clear_all_sleepdeps,
> int omap2_clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
Why this function is non-static? typo??? May be I am missing something.
It was static before, but api became public by below commit -
ARM: OMAP2/3: clockdomain/PRM/CM: move the low-level clockdomain
functions into PRM/CM
Thanks,
Vaibhav
> {
> struct clkdm_dep *cd;
> @@ -221,7 +222,7 @@ int omap2_clkdm_clear_all_wkdeps(struct clockdomain *clkdm)
>
> /* PRM accesses are slow, so minimize them */
> mask |= 1 << cd->clkdm->dep_bit;
> - atomic_set(&cd->wkdep_usecount, 0);
> + cd->wkdep_usecount = 0;
> }
>
> omap2_prm_clear_mod_reg_bits(mask, clkdm->pwrdm.ptr->prcm_offs,
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints
2012-12-09 1:23 ` [PATCH 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints Paul Walmsley
2012-12-12 10:21 ` Vaibhav Hiremath
@ 2012-12-26 6:31 ` Bedia, Vaibhav
1 sibling, 0 replies; 28+ messages in thread
From: Bedia, Vaibhav @ 2012-12-26 6:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
On Sun, Dec 09, 2012 at 06:53:43, Paul Walmsley wrote:
> The atomic usecounts seem to be confusing, and are no longer needed
> since the operations that they are attached to really should take
> place under lock. Replace the atomic counters with simple integers,
> protected by the enclosing powerdomain spinlock.
[...]
>
> @@ -1063,7 +1123,8 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain *clkdm)
> * should be called for every clock instance or hwmod that is
> * enabled, so the clkdm can be force woken up.
> */
> - if ((atomic_inc_return(&clkdm->usecount) > 1) && autodeps) {
> + clkdm->usecount++;
> + if (clkdm->usecount > 1 && autodeps) {
> pwrdm_unlock(clkdm->pwrdm.ptr);
> return 0;
> }
This is not directly related to this patch but something I noticed when I enabled
the various debug prints.
if the clkdm->usecount is > 1, there is still a call to arch_clkdm->clkdm_clk_enable().
Won't the usecount >1 guarantee that the clkdm is in the right state and the PRCM
access can be skipped?
Regards,
Vaibhav
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 10/10] ARM: OMAP2+: powerdomain: fix whitespace, improve flag comments
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
` (8 preceding siblings ...)
2012-12-09 1:23 ` [PATCH 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints Paul Walmsley
@ 2012-12-09 1:23 ` Paul Walmsley
2013-01-04 13:07 ` [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Tero Kristo
10 siblings, 0 replies; 28+ messages in thread
From: Paul Walmsley @ 2012-12-09 1:23 UTC (permalink / raw)
To: linux-arm-kernel
Fix some whitespace problems introduced by commit
da03ce65b5431245b9cd20db3edaaa6b9f5c8dc1 ("OMAP3: powerdomain data:
add voltage domains"). Also, improve the documentation for the struct
powerdomain.flags field.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
arch/arm/mach-omap2/powerdomain.h | 26 +++++++------
arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c | 4 +-
arch/arm/mach-omap2/powerdomains2xxx_data.c | 8 ++--
arch/arm/mach-omap2/powerdomains3xxx_data.c | 44 +++++++++++-----------
4 files changed, 42 insertions(+), 40 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 83b4892..842ba46 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -43,18 +43,20 @@
#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
- * in MEM bank 1 position. This is
- * true for OMAP3430
- */
-#define PWRDM_HAS_LOWPOWERSTATECHANGE (1 << 2) /*
- * support to transition from a
- * sleep state to a lower sleep
- * state without waking up the
- * powerdomain
- */
+/*
+ * Powerdomain flags (struct powerdomain.flags)
+ *
+ * PWRDM_HAS_HDWR_SAR - powerdomain has hardware save-and-restore support
+ *
+ * PWRDM_HAS_MPU_QUIRK - MPU pwr domain has MEM bank 0 bits in MEM
+ * bank 1 position. This is true for OMAP3430
+ *
+ * PWRDM_HAS_LOWPOWERSTATECHANGE - can transition from a sleep state
+ * to a lower sleep state without waking up the powerdomain
+ */
+#define PWRDM_HAS_HDWR_SAR BIT(0)
+#define PWRDM_HAS_MPU_QUIRK BIT(1)
+#define PWRDM_HAS_LOWPOWERSTATECHANGE BIT(2)
/*
* Number of memory banks that are power-controllable. On OMAP4430, the
diff --git a/arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c b/arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c
index d3a5399..7b946f1 100644
--- a/arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c
+++ b/arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c
@@ -54,12 +54,12 @@ struct powerdomain gfx_omap2_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
struct powerdomain wkup_omap2_pwrdm = {
.name = "wkup_pwrdm",
.prcm_offs = WKUP_MOD,
.pwrsts = PWRSTS_ON,
- .voltdm = { .name = "wakeup" },
+ .voltdm = { .name = "wakeup" },
};
diff --git a/arch/arm/mach-omap2/powerdomains2xxx_data.c b/arch/arm/mach-omap2/powerdomains2xxx_data.c
index 01abc1e..578eef8 100644
--- a/arch/arm/mach-omap2/powerdomains2xxx_data.c
+++ b/arch/arm/mach-omap2/powerdomains2xxx_data.c
@@ -38,7 +38,7 @@ static struct powerdomain dsp_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON,
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain mpu_24xx_pwrdm = {
@@ -53,7 +53,7 @@ static struct powerdomain mpu_24xx_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON,
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain core_24xx_pwrdm = {
@@ -72,7 +72,7 @@ static struct powerdomain core_24xx_pwrdm = {
[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
[2] = PWRSTS_OFF_RET_ON, /* MEM3ONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
@@ -94,7 +94,7 @@ static struct powerdomain mdm_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
/*
diff --git a/arch/arm/mach-omap2/powerdomains3xxx_data.c b/arch/arm/mach-omap2/powerdomains3xxx_data.c
index 8b23d23..f0e14e9ef 100644
--- a/arch/arm/mach-omap2/powerdomains3xxx_data.c
+++ b/arch/arm/mach-omap2/powerdomains3xxx_data.c
@@ -50,7 +50,7 @@ static struct powerdomain iva2_pwrdm = {
[2] = PWRSTS_OFF_ON,
[3] = PWRSTS_ON,
},
- .voltdm = { .name = "mpu_iva" },
+ .voltdm = { .name = "mpu_iva" },
};
static struct powerdomain mpu_3xxx_pwrdm = {
@@ -66,7 +66,7 @@ static struct powerdomain mpu_3xxx_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_OFF_ON,
},
- .voltdm = { .name = "mpu_iva" },
+ .voltdm = { .name = "mpu_iva" },
};
static struct powerdomain mpu_am35x_pwrdm = {
@@ -82,7 +82,7 @@ static struct powerdomain mpu_am35x_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON,
},
- .voltdm = { .name = "mpu_iva" },
+ .voltdm = { .name = "mpu_iva" },
};
/*
@@ -109,7 +109,7 @@ static struct powerdomain core_3xxx_pre_es3_1_pwrdm = {
[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain core_3xxx_es3_1_pwrdm = {
@@ -131,7 +131,7 @@ static struct powerdomain core_3xxx_es3_1_pwrdm = {
[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain core_am35x_pwrdm = {
@@ -148,7 +148,7 @@ static struct powerdomain core_am35x_pwrdm = {
[0] = PWRSTS_ON, /* MEM1ONSTATE */
[1] = PWRSTS_ON, /* MEM2ONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain dss_pwrdm = {
@@ -163,7 +163,7 @@ static struct powerdomain dss_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain dss_am35x_pwrdm = {
@@ -178,7 +178,7 @@ static struct powerdomain dss_am35x_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
/*
@@ -199,7 +199,7 @@ static struct powerdomain sgx_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain sgx_am35x_pwrdm = {
@@ -214,7 +214,7 @@ static struct powerdomain sgx_am35x_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain cam_pwrdm = {
@@ -229,7 +229,7 @@ static struct powerdomain cam_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain per_pwrdm = {
@@ -244,7 +244,7 @@ static struct powerdomain per_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain per_am35x_pwrdm = {
@@ -259,13 +259,13 @@ static struct powerdomain per_am35x_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain emu_pwrdm = {
.name = "emu_pwrdm",
.prcm_offs = OMAP3430_EMU_MOD,
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain neon_pwrdm = {
@@ -273,7 +273,7 @@ static struct powerdomain neon_pwrdm = {
.prcm_offs = OMAP3430_NEON_MOD,
.pwrsts = PWRSTS_OFF_RET_ON,
.pwrsts_logic_ret = PWRSTS_RET,
- .voltdm = { .name = "mpu_iva" },
+ .voltdm = { .name = "mpu_iva" },
};
static struct powerdomain neon_am35x_pwrdm = {
@@ -281,7 +281,7 @@ static struct powerdomain neon_am35x_pwrdm = {
.prcm_offs = OMAP3430_NEON_MOD,
.pwrsts = PWRSTS_ON,
.pwrsts_logic_ret = PWRSTS_ON,
- .voltdm = { .name = "mpu_iva" },
+ .voltdm = { .name = "mpu_iva" },
};
static struct powerdomain usbhost_pwrdm = {
@@ -303,37 +303,37 @@ static struct powerdomain usbhost_pwrdm = {
.pwrsts_mem_on = {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain dpll1_pwrdm = {
.name = "dpll1_pwrdm",
.prcm_offs = MPU_MOD,
- .voltdm = { .name = "mpu_iva" },
+ .voltdm = { .name = "mpu_iva" },
};
static struct powerdomain dpll2_pwrdm = {
.name = "dpll2_pwrdm",
.prcm_offs = OMAP3430_IVA2_MOD,
- .voltdm = { .name = "mpu_iva" },
+ .voltdm = { .name = "mpu_iva" },
};
static struct powerdomain dpll3_pwrdm = {
.name = "dpll3_pwrdm",
.prcm_offs = PLL_MOD,
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain dpll4_pwrdm = {
.name = "dpll4_pwrdm",
.prcm_offs = PLL_MOD,
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
static struct powerdomain dpll5_pwrdm = {
.name = "dpll5_pwrdm",
.prcm_offs = PLL_MOD,
- .voltdm = { .name = "core" },
+ .voltdm = { .name = "core" },
};
/* As powerdomains are added or removed above, this list must also be changed */
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9
2012-12-09 1:23 [PATCH 00/10] ARM: OMAP2+: second set of PM fixes and cleanup for 3.9 Paul Walmsley
` (9 preceding siblings ...)
2012-12-09 1:23 ` [PATCH 10/10] ARM: OMAP2+: powerdomain: fix whitespace, improve flag comments Paul Walmsley
@ 2013-01-04 13:07 ` Tero Kristo
10 siblings, 0 replies; 28+ messages in thread
From: Tero Kristo @ 2013-01-04 13:07 UTC (permalink / raw)
To: linux-arm-kernel
Hi Paul,
I just tested this + next two powerdomain series from you on omap3beagle
+ omap4panda boards. This set seems fine, for the code I don't have any
major complaints either, thus you can add my ack if you like. I'll send
comments for the next two sets soon also.
-Tero
On Sat, 2012-12-08 at 18:23 -0700, Paul Walmsley wrote:
> Several more OMAP2+ power management fixes, optimizations, and
> cleanup, intended for 3.9. This series is also a prerequisite for the
> functional powerdomain conversion series.
>
> I'll plan to bundle these patches together into a pull request with the first
> set of 3.9 PM patches, sent earlier.
>
> - Paul
>
> ---
>
> vmlinux object size
> (delta in bytes from TEST_pm_cleanup_fixes_a_3.9 (8f5e20850396fc60fd4ee01f586705033902bb53)):
> text data bss total kernel
> +460 -368 0 +92 am33xx_only
> +200 -464 0 -264 n800_multi_omap2xxx
> +192 -144 0 +48 n800_only_a
> 0 0 0 0 omap1_defconfig
> 0 0 0 0 omap1_defconfig_1510innovator_only
> 0 0 0 0 omap1_defconfig_5912osk_only
> +296 -1816 0 -1520 omap2plus_defconfig
> +272 -176 0 +96 omap2plus_defconfig_2430sdp_only
> +232 -1816 0 -1584 omap2plus_defconfig_cpupm
> +280 -1520 0 -1240 omap2plus_defconfig_no_pm
> +4388 -1016 0 +3372 omap2plus_defconfig_omap2_4_only
> +468 -952 0 -484 omap2plus_defconfig_omap3_4_only
> +700 -584 -144 -28 rmk_omap3430_ldp_allnoconfig
> +452 -648 0 -196 rmk_omap3430_ldp_oldconfig
> +700 -584 -144 -28 rmk_omap4430_sdp_allnoconfig
> +424 -328 0 +96 rmk_omap4430_sdp_oldconfig
>
> Boot-time memory difference
> (delta in bytes from TEST_pm_cleanup_fixes_a_3.9 (8f5e20850396fc60fd4ee01f586705033902bb53))
> avail rsrvd high freed board kconfig
> 4k -4k . . 2430sdp omap2plus_defconfig
> 4k -4k . . 3517evm omap2plus_defconfig
> 4k -4k . . 3530es3beagle omap2plus_defconfig
> 4k -4k . . 3730beaglexm omap2plus_defconfig
> 4k -4k . . 37xxevm omap2plus_defconfig
> 4k -4k . . 4430es2panda omap2plus_defconfig
> 4k -4k . . cmt3517 omap2plus_defconfig
>
> Paul Walmsley (10):
> ARM: OMAP3/4: cpuidle: fix sparse and checkpatch warnings
> ARM: OMAP2+: clockdomain: add pwrdm_state_switch() call to clkdm_sleep()
> ARM: OMAP2xxx: PM: clean up some crufty powerstate programming code
> ARM: OMAP2: PM/powerdomain: drop unnecessary pwrdm_wait_transition()
> ARM: OMAP2+: PM/powerdomain: move omap_set_pwrdm_state() to powerdomain code
> ARM: OMAP2+: powerdomain/clockdomain: add a per-powerdomain spinlock
> ARM: OMAP2xxx: CM: remove autodep handling
> ARM: OMAP2+: clockdomain: work on wkdep/sleepdep functions
> ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints
> ARM: OMAP2+: powerdomain: fix whitespace, improve flag comments
>
>
> arch/arm/mach-omap2/clockdomain-powerdomain.h | 22 +
> arch/arm/mach-omap2/clockdomain.c | 550 +++++++++++++++-------
> arch/arm/mach-omap2/clockdomain.h | 17 -
> arch/arm/mach-omap2/cm2xxx.c | 33 -
> arch/arm/mach-omap2/cm3xxx.c | 14 -
> arch/arm/mach-omap2/cminst44xx.c | 2
> arch/arm/mach-omap2/cpuidle34xx.c | 14 -
> arch/arm/mach-omap2/cpuidle44xx.c | 28 +
> arch/arm/mach-omap2/pm-debug.c | 6
> arch/arm/mach-omap2/pm.c | 65 ---
> arch/arm/mach-omap2/pm.h | 1
> arch/arm/mach-omap2/pm24xx.c | 30 -
> arch/arm/mach-omap2/powerdomain-clockdomain.h | 27 +
> arch/arm/mach-omap2/powerdomain.c | 176 +++++--
> arch/arm/mach-omap2/powerdomain.h | 42 +-
> arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c | 4
> arch/arm/mach-omap2/powerdomains2xxx_data.c | 8
> arch/arm/mach-omap2/powerdomains3xxx_data.c | 44 +-
> arch/arm/mach-omap2/prm2xxx_3xxx.c | 3
> 19 files changed, 661 insertions(+), 425 deletions(-)
> create mode 100644 arch/arm/mach-omap2/clockdomain-powerdomain.h
> create mode 100644 arch/arm/mach-omap2/powerdomain-clockdomain.h
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread