ARM: OMAP3/4 : cpuidle34xx and cpuidle44xx cleanups
v2 cover ARM: OMAP3/4 : cpuidle34xx and cpuidle44xx cleanups
This patchset makes some cleanup on these cpuidle drivers and consolidate the code across both architecture.
Tested on OMAP3 (igepV2).
Partially tested on OMAP4 (pandaboard), without offlining the cpu1.
V2 : Fixed a couple of typos in the patch description
V1 : Initial Post
Daniel Lezcano (17):
ARM: OMAP4: cpuidle - Remove unused valid field
ARM: OMAP4: cpuidle - Declare the states with the driver declaration
ARM: OMAP4: cpuidle - Remove the cpuidle_params_table table
ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration
ARM: OMAP4: cpuidle - Initialize omap4_idle_data at compile time
ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot
time
ARM: OMAP3: cpuidle - remove rx51 cpuidle parameters table
ARM: OMAP3: define cpuidle statically
ARM: OMAP3: cpuidle - remove the 'valid' field
ARM: OMAP3: cpuidle - remove cpuidle_params_table
ARM: OMAP3: define statically the omap3_idle_data
ARM: OMAP3: cpuidle - use omap3_idle_data directly
ARM: OMAP3 : cpuidle - simplify next_valid_state
ARM: OMAP3: set omap3_idle_data as static
ARM: OMAP3/4: consolidate cpuidle Makefile
ARM: OMAP3: cpuidle - set global variables static
arch/arm/mach-omap2/Makefile | 11 +- arch/arm/mach-omap2/board-rx51.c | 38 +++--- arch/arm/mach-omap2/cpuidle34xx.c | 299 +++++++++++++++---------------------- arch/arm/mach-omap2/cpuidle44xx.c | 135 +++++++---------- arch/arm/mach-omap2/pm.h | 38 ++--- 5 files changed, 219 insertions(+), 302 deletions(-)
Signature · 1 line
1.7.5.4
PATCH v2 1/17 ARM: OMAP4: cpuidle - Remove unused valid field
The 'valid' field is never used in the code, let's remove it.
--- arch/arm/mach-omap2/cpuidle44xx.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
arch/arm/mach-omap2/cpuidle44xx.c +3 -6
@@ -29,16 +29,15 @@ struct omap4_idle_statedata {u32 cpu_state;u32 mpu_logic_state;u32 mpu_state; -u8 valid; };static struct cpuidle_params cpuidle_params_table[] = {/* C1 - CPU0 ON + CPU1 ON + MPU ON */ - {.exit_latency =2 +2 , .target_residency =5 , .valid =1 }, + {.exit_latency =2 +2 , .target_residency =5 },/* C2- CPU0 OFF + CPU1 OFF + MPU CSWR */ - {.exit_latency =328 +440 , .target_residency =960 , .valid =1 }, + {.exit_latency =328 +440 , .target_residency =960 },/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ - {.exit_latency =460 +518 , .target_residency =1100 , .valid =1 }, + {.exit_latency =460 +518 , .target_residency =1100 }, };#define OMAP4_NUM_STATES ARRAY_SIZE(cpuidle_params_table)
@@ -155,7 +154,6 @@ static inline struct omap4_idle_statedata *_fill_cstate_usage(struct omap4_idle_statedata *cx = &omap4_idle_data[idx];struct cpuidle_state_usage *state_usage = &dev->states_usage[idx]; - cx->valid = cpuidle_params_table[idx].valid;cpuidle_set_statedata (state_usage, cx);return cx;
@@ -191,7 +189,6 @@ int __init omap4_idle_init(void)_fill_cstate (drv,0 ,"MPUSS ON" ); drv->safe_state_index =0 ; cx =_fill_cstate_usage (dev,0 ); - cx->valid =1 ;/* C1 is always valid */ cx->cpu_state =PWRDM_POWER_ON ; cx->mpu_state =PWRDM_POWER_ON ; cx->mpu_logic_state =PWRDM_POWER_RET ;
Signature · 1 line
1.7.5.4
PATCH v2 2/17 ARM: OMAP4: cpuidle - Declare the states with the driver declaration
In reply to Daniel LezcanoThe cpuidle API allows to declare statically the states in the driver structure. Let's use it.
We do no longer need the fill_cstate function called at runtime and by the way adding more instructions at boot time.
--- arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++--------------- 1 files changed, 33 insertions(+), 24 deletions(-)
arch/arm/mach-omap2/cpuidle44xx.c +33 -24
@@ -132,21 +132,39 @@ struct cpuidle_driver omap4_idle_driver = { .name = "omap4_idle", .owner = THIS_MODULE, .en_core_tk_irqen = 1, + .states = { + { + /* C1 - CPU0 ON + CPU1 ON + MPU ON */ + .exit_latency = 2 + 2, + .target_residency = 5, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C1", + .desc = "MPUSS ON" + }, + { + /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ + .exit_latency = 328 + 440, + .target_residency = 960, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C2", + .desc = "MPUSS CSWR", + }, + { + /* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ + .exit_latency = 460 + 518, + .target_residency = 1100, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C3", + .desc = "MPUSS OSWR", + }, + }, + .state_count = OMAP4_NUM_STATES, + .safe_state_index = 0, }; -static inline void _fill_cstate (struct cpuidle_driver *drv, -int idx,const char *descr) -{ -struct cpuidle_state *state = &drv->states[idx]; - - state->exit_latency = cpuidle_params_table[idx].exit_latency; - state->target_residency = cpuidle_params_table[idx].target_residency; - state->flags =CPUIDLE_FLAG_TIME_VALID ; - state->enter = omap4_enter_idle; -sprintf (state->name,"C%d" , idx +1 ); -strncpy (state->desc, descr,CPUIDLE_DESC_LEN ); -} - static inline struct omap4_idle_statedata *_fill_cstate_usage( struct cpuidle_device *dev, int idx)
@@ -180,37 +198,28 @@ int __init omap4_idle_init(void)if ((!mpu_pd) || (!cpu0_pd) || (!cpu1_pd))return -ENODEV ; - - drv->safe_state_index =-1 ; dev = &per_cpu (omap4_idle_dev, cpu_id); dev->cpu = cpu_id; -/* C1 - CPU0 ON + CPU1 ON + MPU ON */ -_fill_cstate (drv,0 ,"MPUSS ON" ); - drv->safe_state_index =0 ; cx =_fill_cstate_usage (dev,0 ); cx->cpu_state =PWRDM_POWER_ON ; cx->mpu_state =PWRDM_POWER_ON ; cx->mpu_logic_state =PWRDM_POWER_RET ; -/* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ -_fill_cstate (drv,1 ,"MPUSS CSWR" ); cx =_fill_cstate_usage (dev,1 ); cx->cpu_state =PWRDM_POWER_OFF ; cx->mpu_state =PWRDM_POWER_RET ; cx->mpu_logic_state =PWRDM_POWER_RET ; -/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ -_fill_cstate (drv,2 ,"MPUSS OSWR" ); cx =_fill_cstate_usage (dev,2 ); cx->cpu_state =PWRDM_POWER_OFF ; cx->mpu_state =PWRDM_POWER_RET ; cx->mpu_logic_state =PWRDM_POWER_OFF ; - drv->state_count =OMAP4_NUM_STATES ;cpuidle_register_driver (&omap4_idle_driver); - dev->state_count =OMAP4_NUM_STATES ; + dev->state_count = drv->state_count; +if (cpuidle_register_device (dev)) {pr_err ("%s: CPUidle register device failed\n" , __func__);return -EIO ;
Signature · 1 line
1.7.5.4
PATCH v2 2/17 ARM: OMAP4: cpuidle - Declare the states with the driver declaration
Quoted 40 lines · Daniel Lezcano
The cpuidle API allows to declare statically the states in the driver structure. Let's use it.
We do no longer need thefill_cstatefunction called at runtime and by the way adding more instructions at boot time.Signed-off-by Daniel Lezcano <[email protected]>Reviewed-by Jean Pihet <[email protected]>Reviewed-by Santosh Shilimkar <[email protected]>--- arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++--------------- 1 files changed, 33 insertions(+), 24 deletions(-)arch/arm/mach-omap2/cpuidle44xx.c +20
-0@@ -132,21 +132,39 @@ struct cpuidle_driver omap4_idle_driver = { .name = "omap4_idle", .owner = THIS_MODULE, .en_core_tk_irqen = 1, + .states = { + { + /* C1 - CPU0 ON + CPU1 ON + MPU ON */ + .exit_latency = 2 + 2, + .target_residency = 5, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C1", + .desc = "MPUSS ON" + }, + { + /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ + .exit_latency = 328 + 440, + .target_residency = 960, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C2", + .desc = "MPUSS CSWR", + }, + {
arch/arm/mach-omap2/cpuidle44xx.c struct cpuidle_driver omap4_idle_driver = { · line 164
Quoted 10 lines
arch/arm/mach-omap2/cpuidle44xx.c +10
-0+/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ + .exit_latency = 460 + 518, + .target_residency = 1100, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C3", + .desc = "MPUSS OSWR", + }, + }, + .state_count = OMAP4_NUM_STATES,
I think you can drop OMAP4_NUM_STATES here, and just use:
.state_count = ARRAY_SIZE(omap4_idle_driver.states),
Then drop OMAP4_NUM_STATES all together in patch 3.
Kevin
PATCH v2 2/17 ARM: OMAP4: cpuidle - Declare the states with the driver declaration
Quoted 55 lines · Kevin Hilman
Quoted 40 lines · Daniel Lezcano
The cpuidle API allows to declare statically the states in the driver structure. Let's use it.
We do no longer need thefill_cstatefunction called at runtime and by the way adding more instructions at boot time.Signed-off-by Daniel Lezcano<[email protected]>Reviewed-by Jean Pihet<[email protected]>Reviewed-by Santosh Shilimkar<[email protected]>--- arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++--------------- 1 files changed, 33 insertions(+), 24 deletions(-)arch/arm/mach-omap2/cpuidle44xx.c +20
-0@@ -132,21 +132,39 @@ struct cpuidle_driver omap4_idle_driver = { .name = "omap4_idle", .owner = THIS_MODULE, .en_core_tk_irqen = 1, + .states = { + { + /* C1 - CPU0 ON + CPU1 ON + MPU ON */ + .exit_latency = 2 + 2, + .target_residency = 5, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C1", + .desc = "MPUSS ON" + }, + { + /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ + .exit_latency = 328 + 440, + .target_residency = 960, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C2", + .desc = "MPUSS CSWR", + }, + {arch/arm/mach-omap2/cpuidle44xx.c struct cpuidle_driver omap4_idle_driver = { · line 164
Quoted 10 lines
arch/arm/mach-omap2/cpuidle44xx.c +10
-0+/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ + .exit_latency = 460 + 518, + .target_residency = 1100, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C3", + .desc = "MPUSS OSWR", + }, + }, + .state_count = OMAP4_NUM_STATES,I think you can drop
OMAP4_NUM_STATEShere, and just use:
Quoted 3 lines
.state_count =
ARRAY_SIZE(omap4_idle_driver.states),Then drop
OMAP4_NUM_STATESall together in patch 3.
Ok.
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
PATCH v2 2/17 ARM: OMAP4: cpuidle - Declare the states with the driver declaration
Quoted 58 lines · Daniel Lezcano
Quoted 57 lines · Kevin Hilman
Quoted 42 lines · Daniel Lezcano
The cpuidle API allows to declare statically the states in the driver structure. Let's use it.
We do no longer need thefill_cstatefunction called at runtime and by the way adding more instructions at boot time.Signed-off-by Daniel Lezcano<[email protected]>Reviewed-by Jean Pihet<[email protected]>Reviewed-by Santosh Shilimkar<[email protected]>--- arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++--------------- 1 files changed, 33 insertions(+), 24 deletions(-)arch/arm/mach-omap2/cpuidle44xx.c +20
-0@@ -132,21 +132,39 @@ struct cpuidle_driver omap4_idle_driver = { .name = "omap4_idle", .owner = THIS_MODULE, .en_core_tk_irqen = 1, + .states = { + { + /* C1 - CPU0 ON + CPU1 ON + MPU ON */ + .exit_latency = 2 + 2, + .target_residency = 5, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C1", + .desc = "MPUSS ON" + }, + { + /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ + .exit_latency = 328 + 440, + .target_residency = 960, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C2", + .desc = "MPUSS CSWR", + }, + {arch/arm/mach-omap2/cpuidle44xx.c struct cpuidle_driver omap4_idle_driver = { · line 164
Quoted 10 lines
arch/arm/mach-omap2/cpuidle44xx.c +10
-0+/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ + .exit_latency = 460 + 518, + .target_residency = 1100, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C3", + .desc = "MPUSS OSWR", + }, + }, + .state_count = OMAP4_NUM_STATES,I think you can drop
OMAP4_NUM_STATEShere, and just use:
Quoted 5 lines
Quoted 3 lines
.state_count =
ARRAY_SIZE(omap4_idle_driver.states),Then drop
OMAP4_NUM_STATESall together in patch 3.Ok.
I said 'ok' but it is not :)
omap4_idle_driver.states has a fixed length which is CPUIDLE_STATE_MAX (8). We need to define it manually as 3 for now.
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro:<http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
PATCH v2 2/17 ARM: OMAP4: cpuidle - Declare the states with the driver declaration
Quoted 61 lines · Daniel Lezcano
Quoted 60 lines · Daniel Lezcano
Quoted 59 lines · Kevin Hilman
Quoted 42 lines · Daniel Lezcano
The cpuidle API allows to declare statically the states in the driver structure. Let's use it.
We do no longer need thefill_cstatefunction called at runtime and by the way adding more instructions at boot time.Signed-off-by Daniel Lezcano<[email protected]>Reviewed-by Jean Pihet<[email protected]>Reviewed-by Santosh Shilimkar<[email protected]>--- arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++--------------- 1 files changed, 33 insertions(+), 24 deletions(-)arch/arm/mach-omap2/cpuidle44xx.c +20
-0@@ -132,21 +132,39 @@ struct cpuidle_driver omap4_idle_driver = { .name = "omap4_idle", .owner = THIS_MODULE, .en_core_tk_irqen = 1, + .states = { + { + /* C1 - CPU0 ON + CPU1 ON + MPU ON */ + .exit_latency = 2 + 2, + .target_residency = 5, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C1", + .desc = "MPUSS ON" + }, + { + /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ + .exit_latency = 328 + 440, + .target_residency = 960, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C2", + .desc = "MPUSS CSWR", + }, + {arch/arm/mach-omap2/cpuidle44xx.c struct cpuidle_driver omap4_idle_driver = { · line 164
Quoted 10 lines
arch/arm/mach-omap2/cpuidle44xx.c +10
-0+/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ + .exit_latency = 460 + 518, + .target_residency = 1100, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C3", + .desc = "MPUSS OSWR", + }, + }, + .state_count = OMAP4_NUM_STATES,I think you can drop
OMAP4_NUM_STATEShere, and just use:.state_count =
ARRAY_SIZE(omap4_idle_driver.states),
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Then drop
OMAP4_NUM_STATESall together in patch 3.Ok.
I said 'ok' but it is not :)
omap4_idle_driver.states has a fixed length which is
CPUIDLE_STATE_MAX(8). We need to define it manually as 3 for now.
I don't see the connection between the two.
Why can't you use ARRAY_SIZE(), and just have an error check later in the init to see if state_count > max.
Kevin
PATCH v2 2/17 ARM: OMAP4: cpuidle - Declare the states with the driver declaration
Quoted 69 lines · Kevin Hilman
Quoted 67 lines · Daniel Lezcano
Quoted 58 lines · Daniel Lezcano
Quoted 57 lines · Kevin Hilman
Quoted 42 lines · Daniel Lezcano
The cpuidle API allows to declare statically the states in the driver structure. Let's use it.
We do no longer need thefill_cstatefunction called at runtime and by the way adding more instructions at boot time.Signed-off-by Daniel Lezcano<[email protected]>Reviewed-by Jean Pihet<[email protected]>Reviewed-by Santosh Shilimkar<[email protected]>--- arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++--------------- 1 files changed, 33 insertions(+), 24 deletions(-)arch/arm/mach-omap2/cpuidle44xx.c +20
-0@@ -132,21 +132,39 @@ struct cpuidle_driver omap4_idle_driver = { .name = "omap4_idle", .owner = THIS_MODULE, .en_core_tk_irqen = 1, + .states = { + { + /* C1 - CPU0 ON + CPU1 ON + MPU ON */ + .exit_latency = 2 + 2, + .target_residency = 5, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C1", + .desc = "MPUSS ON" + }, + { + /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ + .exit_latency = 328 + 440, + .target_residency = 960, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C2", + .desc = "MPUSS CSWR", + }, + {arch/arm/mach-omap2/cpuidle44xx.c struct cpuidle_driver omap4_idle_driver = { · line 164
Quoted 10 lines
arch/arm/mach-omap2/cpuidle44xx.c +10
-0+/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ + .exit_latency = 460 + 518, + .target_residency = 1100, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C3", + .desc = "MPUSS OSWR", + }, + }, + .state_count = OMAP4_NUM_STATES,I think you can drop
OMAP4_NUM_STATEShere, and just use:Quoted 5 lines
Quoted 3 lines
.state_count =
ARRAY_SIZE(omap4_idle_driver.states),Then drop
OMAP4_NUM_STATESall together in patch 3.Ok.
I said 'ok' but it is not :)
Quoted 7 lines
Quoted 2 lines
omap4_idle_driver.states has a fixed length which is
CPUIDLE_STATE_MAX(8). We need to define it manually as 3 for now.I don't see the connection between the two.
Why can't you use
ARRAY_SIZE(), and just have an error check later in the init to see ifstate_count> max.
Maybe I misunderstood but you say:
.state_count = ARRAY_SIZE(omap4_idle_driver.states),
As in the cpuidle structure, the state array is CPUIDLE_STATE_MAX, ARRAY_SIZE will always return 8, not the number of the initialized states.
Anyway, I modified the patchset to use ARRAY_SIZE on the omap4_idle_data array, so we have:
.state_count = ARRAY_SIZE(omap4_idle_data),
-- Daniel
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
PATCH v2 2/17 ARM: OMAP4: cpuidle - Declare the states with the driver declaration
Quoted 79 lines · Daniel Lezcano
Quoted 69 lines · Kevin Hilman
Quoted 67 lines · Daniel Lezcano
Quoted 58 lines · Daniel Lezcano
Quoted 57 lines · Kevin Hilman
Quoted 42 lines · Daniel Lezcano
The cpuidle API allows to declare statically the states in the driver structure. Let's use it.
We do no longer need thefill_cstatefunction called at runtime and by the way adding more instructions at boot time.Signed-off-by Daniel Lezcano<[email protected]>Reviewed-by Jean Pihet<[email protected]>Reviewed-by Santosh Shilimkar<[email protected]>--- arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++--------------- 1 files changed, 33 insertions(+), 24 deletions(-)arch/arm/mach-omap2/cpuidle44xx.c +20
-0@@ -132,21 +132,39 @@ struct cpuidle_driver omap4_idle_driver = { .name = "omap4_idle", .owner = THIS_MODULE, .en_core_tk_irqen = 1, + .states = { + { + /* C1 - CPU0 ON + CPU1 ON + MPU ON */ + .exit_latency = 2 + 2, + .target_residency = 5, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C1", + .desc = "MPUSS ON" + }, + { + /* C2 - CPU0 OFF + CPU1 OFF + MPU CSWR */ + .exit_latency = 328 + 440, + .target_residency = 960, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C2", + .desc = "MPUSS CSWR", + }, + {arch/arm/mach-omap2/cpuidle44xx.c struct cpuidle_driver omap4_idle_driver = { · line 164
Quoted 10 lines
arch/arm/mach-omap2/cpuidle44xx.c +10
-0+/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ + .exit_latency = 460 + 518, + .target_residency = 1100, + .flags = CPUIDLE_FLAG_TIME_VALID, + .enter = omap4_enter_idle, + .name = "C3", + .desc = "MPUSS OSWR", + }, + }, + .state_count = OMAP4_NUM_STATES,I think you can drop
OMAP4_NUM_STATEShere, and just use:Quoted 5 lines
Quoted 3 lines
.state_count =
ARRAY_SIZE(omap4_idle_driver.states),Then drop
OMAP4_NUM_STATESall together in patch 3.Ok.
I said 'ok' but it is not :)
Quoted 7 lines
Quoted 2 lines
omap4_idle_driver.states has a fixed length which is
CPUIDLE_STATE_MAX(8). We need to define it manually as 3 for now.I don't see the connection between the two.
Why can't you use
ARRAY_SIZE(), and just have an error check later in the init to see ifstate_count> max.
Quoted 7 lines
Maybe I misunderstood but you say:
.state_count =
ARRAY_SIZE(omap4_idle_driver.states),As in the cpuidle structure, the state array is
CPUIDLE_STATE_MAX,ARRAY_SIZEwill always return 8, not the number of the initialized states.
Oh, I see now.
Quoted 4 lines
Anyway, I modified the patchset to use
ARRAY_SIZEon theomap4_idle_dataarray, so we have:.state_count =
ARRAY_SIZE(omap4_idle_data),
Great, that should work.
Thanks,
Kevin
PATCH v2 3/17 ARM: OMAP4: cpuidle - Remove the cpuidle_params_table table
In reply to Daniel LezcanoWe do not longer need this table as we defined the values in the driver states.
--- arch/arm/mach-omap2/cpuidle44xx.c | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-)
arch/arm/mach-omap2/cpuidle44xx.c +1 -10
@@ -31,16 +31,7 @@ struct omap4_idle_statedata {u32 mpu_state; }; -static struct cpuidle_params cpuidle_params_table[] = { -/* C1 - CPU0 ON + CPU1 ON + MPU ON */ - {.exit_latency =2 +2 , .target_residency =5 }, -/* C2- CPU0 OFF + CPU1 OFF + MPU CSWR */ - {.exit_latency =328 +440 , .target_residency =960 }, -/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ - {.exit_latency =460 +518 , .target_residency =1100 }, -}; - -#define OMAP4_NUM_STATES ARRAY_SIZE(cpuidle_params_table) +#define OMAP4_NUM_STATES 3struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES ];static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;
Signature · 1 line
1.7.5.4
PATCH v2 4/17 ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration
In reply to Daniel Lezcano--- arch/arm/mach-omap2/cpuidle44xx.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/cpuidle44xx.c +1 -1
@@ -33,7 +33,7 @@ struct omap4_idle_statedata {#define OMAP4_NUM_STATES 3 -struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES ]; +static struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES ];static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;/**
Signature · 1 line
1.7.5.4
PATCH v2 4/17 ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration
Quoted 3 lines · Daniel Lezcano
Signed-off-by Daniel Lezcano <[email protected]>Reviewed-by Jean Pihet <[email protected]>Reviewed-by Santosh Shilimkar <[email protected]>
Missing changelog.
Kevin
Quoted 17 lines
--- arch/arm/mach-omap2/cpuidle44xx.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)arch/arm/mach-omap2/cpuidle44xx.c +1
-1@@ -33,7 +33,7 @@ struct omap4_idle_statedata {#define OMAP4_NUM_STATES 3 -struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES ]; +static struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES ];static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;/**
PATCH v2 5/17 ARM: OMAP4: cpuidle - Initialize omap4_idle_data at compile time
In reply to Daniel Lezcano--- arch/arm/mach-omap2/cpuidle44xx.c | 19 ++++++++++++++++++- 1 files changed, 18 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/cpuidle44xx.c +18 -1
@@ -33,7 +33,24 @@ struct omap4_idle_statedata {#define OMAP4_NUM_STATES 3 -static struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES ]; +static struct omap4_idle_statedata omap4_idle_data[] = { + { + .cpu_state =PWRDM_POWER_ON , + .mpu_state =PWRDM_POWER_ON , + .mpu_logic_state =PWRDM_POWER_RET , + }, + { + .cpu_state =PWRDM_POWER_OFF , + .mpu_state =PWRDM_POWER_RET , + .mpu_logic_state =PWRDM_POWER_RET , + }, + { + .cpu_state =PWRDM_POWER_OFF , + .mpu_state =PWRDM_POWER_RET , + .mpu_logic_state =PWRDM_POWER_OFF , + }, +}; +static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;/**
Signature · 1 line
1.7.5.4
PATCH v2 6/17 ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
In reply to Daniel LezcanoWe are storing the 'omap4_idle_data' in the private data field of the cpuidle device. As we are using this variable only in this file, that does not really make sense. Let's use the global variable directly instead dereferencing pointers in an idle critical loop.
Also, that simplfies the code.
--- arch/arm/mach-omap2/cpuidle44xx.c | 15 ++++----------- 1 files changed, 4 insertions(+), 11 deletions(-)
arch/arm/mach-omap2/cpuidle44xx.c +4 -11
@@ -24,7 +24,7 @@ #ifdef CONFIG_CPU_IDLE -/* Machine specific information to be recorded in the C-state driver_data */ +/* Machine specific information */ struct omap4_idle_statedata { u32 cpu_state; u32 mpu_logic_state;
@@ -67,8 +67,7 @@ static int omap4_enter_idle(struct cpuidle_device *dev, struct cpuidle_driver *drv, int index) { -struct omap4_idle_statedata *cx = -cpuidle_get_statedata (&dev->states_usage[index]); +struct omap4_idle_statedata *cx = &omap4_idle_data[index];u32 cpu1_state;int cpu_id =smp_processor_id ();
@@ -85,7 +84,7 @@ static int omap4_enter_idle(struct cpuidle_device *dev, cpu1_state =pwrdm_read_pwrst (cpu1_pd);if (cpu1_state !=PWRDM_POWER_OFF ) { index = drv->safe_state_index; - cx =cpuidle_get_statedata (&dev->states_usage[index]); + cx = &omap4_idle_data[index]; }if (index >0 )
@@ -178,15 +177,9 @@ static inline struct omap4_idle_statedata *_fill_cstate_usage( int idx) {struct omap4_idle_statedata *cx = &omap4_idle_data[idx]; -struct cpuidle_state_usage *state_usage = &dev->states_usage[idx]; - -cpuidle_set_statedata (state_usage, cx); -return cx; } - -/** * omap4_idle_init - Init routine for OMAP4 idle *
@@ -196,8 +189,8 @@ static inline struct omap4_idle_statedata *_fill_cstate_usage(int __init omap4_idle_init (void ) {struct omap4_idle_statedata *cx; -struct cpuidle_device *dev;struct cpuidle_driver *drv = &omap4_idle_driver; +struct cpuidle_device *dev;unsigned int cpu_id =0 ; mpu_pd =pwrdm_lookup ("mpu_pwrdm" );
Signature · 1 line
1.7.5.4
PATCH v2 6/17 ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
Quoted 4 lines · Daniel Lezcano
We are storing the '
omap4_idle_data' in the private data field of the cpuidle device. As we are using this variable only in this file, that does not really make sense. Let's use the global variable directly instead dereferencing pointers in an idle critical loop.
Did you notice a performance impact before this change?
Quoted 1 line
Also, that simplfies the code.
possibly, but at the expense of clean abstractions which IMO helps readability.
Unless there is a real performance hit here (which I doubt), I'd prefer to leave this as is.
Kevin
PATCH v2 6/17 ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
Quoted 8 lines · Kevin Hilman
Quoted 4 lines · Daniel Lezcano
We are storing the '
omap4_idle_data' in the private data field of the cpuidle device. As we are using this variable only in this file, that does not really make sense. Let's use the global variable directly instead dereferencing pointers in an idle critical loop.Did you notice a performance impact before this change?
No, I didn't and I don't think I will be able to measure it. But by experience, when dereferencing a pointer that could leads to a cache miss and impact the performances. That may not be the case here, so I won't argue with that as I won't able to prove it :)
Quoted 6 lines
Quoted 1 line
Also, that simplfies the code.
possibly, but at the expense of clean abstractions which IMO helps readability.
Unless there is a real performance hit here (which I doubt), I'd prefer to leave this as is.
There are two reasons of this change. We are storing 'state_count' times a pointer in a private structure for state_usage but the information is already available in the file and easily accessible.
Also, this is set in the fill_cstate function I am removing to let all the initialization to be done at compile time.
Furthermore, I don't get why we have a 'driver data' area in a structure which is dedicated for the states statistics, IMHO that does not help understanding. My mid-term cleanup is to kill the 'driver_data' field in the cpuidle core because I don't think it is at the right place.
An idea I had for consolidate all the cpuidle driver was to use the containerof macro to define the architecture specific structure and declare inside this structure the cpuidle driver and the devices. It is similar on how are implemented the 'routes' for the network stack or the cgroup subsystems, there is a core engine handling generic structure which a contained by the specific structure related to the engine's user. That helps a lot for readability.
Well this is an idea but before I have to unify the cpuidle drivers code to make it clear what is doable or not.
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
PATCH v2 7/17 ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot time
In reply to Daniel LezcanoWe initialized it at compile time, no need to do that at boot time.
--- arch/arm/mach-omap2/cpuidle44xx.c | 26 +------------------------- 1 files changed, 1 insertions(+), 25 deletions(-)
arch/arm/mach-omap2/cpuidle44xx.c +1 -25
@@ -33,7 +33,7 @@ struct omap4_idle_statedata {#define OMAP4_NUM_STATES 3 -static struct omap4_idle_statedata omap4_idle_data[] = { +static struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES ] = { { .cpu_state =PWRDM_POWER_ON , .mpu_state =PWRDM_POWER_ON ,
@@ -172,14 +172,6 @@ struct cpuidle_driver omap4_idle_driver = { .safe_state_index = 0, }; -static inline struct omap4_idle_statedata *_fill_cstate_usage ( -struct cpuidle_device *dev, -int idx) -{ -struct omap4_idle_statedata *cx = &omap4_idle_data[idx]; -return cx; -} -/** * omap4_idle_init - Init routine for OMAP4 idle *
@@ -188,7 +180,6 @@ static inline struct omap4_idle_statedata *_fill_cstate_usage(*/ int __init omap4_idle_init (void ) { -struct omap4_idle_statedata *cx;struct cpuidle_driver *drv = &omap4_idle_driver;struct cpuidle_device *dev;unsigned int cpu_id =0 ;
@@ -202,21 +193,6 @@ int __init omap4_idle_init(void) dev = &per_cpu (omap4_idle_dev, cpu_id); dev->cpu = cpu_id; - cx =_fill_cstate_usage (dev,0 ); - cx->cpu_state =PWRDM_POWER_ON ; - cx->mpu_state =PWRDM_POWER_ON ; - cx->mpu_logic_state =PWRDM_POWER_RET ; - - cx =_fill_cstate_usage (dev,1 ); - cx->cpu_state =PWRDM_POWER_OFF ; - cx->mpu_state =PWRDM_POWER_RET ; - cx->mpu_logic_state =PWRDM_POWER_RET ; - - cx =_fill_cstate_usage (dev,2 ); - cx->cpu_state =PWRDM_POWER_OFF ; - cx->mpu_state =PWRDM_POWER_RET ; - cx->mpu_logic_state =PWRDM_POWER_OFF ; -cpuidle_register_driver (&omap4_idle_driver); dev->state_count = drv->state_count;
Signature · 1 line
1.7.5.4
PATCH v2 7/17 ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot time
Quoted 15 lines · Daniel Lezcano
We initialized it at compile time, no need to do that at boot time.
Signed-off-by Daniel Lezcano <[email protected]>Reviewed-by Jean Pihet <[email protected]>Reviewed-by Santosh Shilimkar <[email protected]>--- arch/arm/mach-omap2/cpuidle44xx.c | 26 +------------------------- 1 files changed, 1 insertions(+), 25 deletions(-)arch/arm/mach-omap2/cpuidle44xx.c +0
-0
arch/arm/mach-omap2/cpuidle44xx.c struct omap4_idle_statedata { · line 36
Quoted 5 lines
arch/arm/mach-omap2/cpuidle44xx.c +1
-1@@ -33,7 +33,7 @@ struct omap4_idle_statedata {#define OMAP4_NUM_STATES 3 -static struct omap4_idle_statedata omap4_idle_data[] = { +static struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES ] = {
You removed OMAP4_NUM_STATES usage in patch 5, why add it back here?
Kevin
PATCH v2 8/17 ARM: OMAP3: cpuidle - remove rx51 cpuidle parameters table
In reply to Daniel LezcanoAs suggested, this table is an optimized version for rx51 and we remove it in order to consolidate the cpuidle code between omap3 and omap4, we remove this specific data definition which is used to override the default omap3 latencies but at the cost of extra code and complexity.
In order to not lose the values which probably took time to be measured, the table is converted into a comment with an array description.
--- arch/arm/mach-omap2/board-rx51.c | 38 +++++++++++++++++------------------- arch/arm/mach-omap2/cpuidle34xx.c | 17 ---------------- arch/arm/mach-omap2/pm.h | 9 -------- 3 files changed, 18 insertions(+), 46 deletions(-)
arch/arm/mach-omap2/board-rx51.c +18 -20
@@ -59,25 +59,24 @@ static struct platform_device leds_gpio = { };/* -* cpuidle C-states definition override from the default values. -* The 'exit_latency' field is the sum of sleep and wake-up latencies. -*/ -static struct cpuidle_params rx51_cpuidle_params[] = { -/* C1 */ - {110 +162 ,5 ,1 }, -/* C2 */ - {106 +180 ,309 ,1 }, -/* C3 */ - {107 +410 ,46057 ,0 }, -/* C4 */ - {121 +3374 ,46057 ,0 }, -/* C5 */ - {855 +1146 ,46057 ,1 }, -/* C6 */ - {7580 +4134 ,484329 ,0 }, -/* C7 */ - {7505 +15274 ,484329 ,1 }, -}; +* cpuidle C-states definition for rx51. +* +* The 'exit_latency' field is the sum of sleep +* and wake-up latencies. + +--------------------------------------------- +| state | exit_latency | target_residency | +--------------------------------------------- +| C1 | 110 + 162 | 5 | +| C2 | 106 + 180 | 309 | +| C3 | 107 + 410 | 46057 | +| C4 | 121 + 3374 | 46057 | +| C5 | 855 + 1146 | 46057 | +| C6 | 7580 + 4134 | 484329 | +| C7 | 7505 + 15274 | 484329 | +--------------------------------------------- + +*/ extern void __init rx51_peripherals_init (void );
@@ -98,7 +97,6 @@ static void __init rx51_init(void)struct omap_sdrc_params *sdrc_params;omap3_mux_init (board_mux,OMAP_PACKAGE_CBB ); -omap3_pm_init_cpuidle (rx51_cpuidle_params);omap_serial_init (); sdrc_params =nokia_get_sdram_timings ();
arch/arm/mach-omap2/cpuidle34xx.c +0 -17
@@ -298,23 +298,6 @@ select_state: DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev); -void omap3_pm_init_cpuidle (struct cpuidle_params *cpuidle_board_params) -{ -int i; - -if (!cpuidle_board_params) -return ; - -for (i =0 ; i <OMAP3_NUM_STATES ; i++) { - cpuidle_params_table[i].valid = cpuidle_board_params[i].valid; - cpuidle_params_table[i].exit_latency = - cpuidle_board_params[i].exit_latency; - cpuidle_params_table[i].target_residency = - cpuidle_board_params[i].target_residency; - } -return ; -} -struct cpuidle_driver omap3_idle_driver = { .name ="omap3_idle" , .owner =THIS_MODULE ,
arch/arm/mach-omap2/pm.h +0 -9
@@ -50,15 +50,6 @@ struct cpuidle_params {u8 valid;/* validates the C-state */ }; -#if defined(CONFIG_PM ) && defined(CONFIG_CPU_IDLE ) -extern void omap3_pm_init_cpuidle (struct cpuidle_params *cpuidle_board_params); -#else -static -inline void omap3_pm_init_cpuidle (struct cpuidle_params *cpuidle_board_params) -{ -} -#endif -extern int omap3_pm_get_suspend_state (struct powerdomain *pwrdm);extern int omap3_pm_set_suspend_state (struct powerdomain *pwrdm,int state);
Signature · 1 line
1.7.5.4
PATCH v2 9/17 ARM: OMAP3: define cpuidle statically
In reply to Daniel LezcanoUse the new cpuidle API and define in the driver the states.
--- arch/arm/mach-omap2/cpuidle34xx.c | 86 +++++++++++++++++++++++++----------- 1 files changed, 60 insertions(+), 26 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +60 -26
@@ -301,23 +301,68 @@ DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);struct cpuidle_driver omap3_idle_driver = { .name ="omap3_idle" , .owner =THIS_MODULE , + .states = { + { + .enter = omap3_enter_idle, + .exit_latency =2 +2 , + .target_residency =5 , + .flags =CPUIDLE_FLAG_TIME_VALID , + .name ="C1" , + .desc ="MPU ON + CORE ON" , + }, + { + .enter = omap3_enter_idle_bm, + .exit_latency =10 +10 , + .target_residency =30 , + .flags =CPUIDLE_FLAG_TIME_VALID , + .name ="C2" , + .desc ="MPU ON + CORE ON" , + }, + { + .enter = omap3_enter_idle_bm, + .exit_latency =50 +50 , + .target_residency =300 , + .flags =CPUIDLE_FLAG_TIME_VALID , + .name ="C3" , + .desc ="MPU RET + CORE ON" , + }, + { + .enter = omap3_enter_idle_bm, + .exit_latency =1500 +1800 , + .target_residency =4000 , + .flags =CPUIDLE_FLAG_TIME_VALID , + .name ="C4" , + .desc ="MPU OFF + CORE ON" , + }, + { + .enter = omap3_enter_idle_bm, + .exit_latency =2500 +7500 , + .target_residency =12000 , + .flags =CPUIDLE_FLAG_TIME_VALID , + .name ="C5" , + .desc ="MPU RET + CORE RET" , + }, + { + .enter = omap3_enter_idle_bm, + .exit_latency =3000 +8500 , + .target_residency =15000 , + .flags =CPUIDLE_FLAG_TIME_VALID , + .name ="C6" , + .desc ="MPU OFF + CORE RET" , + }, + { + .enter = omap3_enter_idle_bm, + .exit_latency =10000 +30000 , + .target_residency =30000 , + .flags =CPUIDLE_FLAG_TIME_VALID , + .name ="C7" , + .desc ="MPU OFF + CORE OFF" , + }, + }, + .state_count =OMAP3_NUM_STATES , + .safe_state_index =0 , }; -/* Helper to fill the C-state common data*/ -static inline void _fill_cstate (struct cpuidle_driver *drv, -int idx,const char *descr) -{ -struct cpuidle_state *state = &drv->states[idx]; - - state->exit_latency = cpuidle_params_table[idx].exit_latency; - state->target_residency = cpuidle_params_table[idx].target_residency; - state->flags =CPUIDLE_FLAG_TIME_VALID ; - state->enter = omap3_enter_idle_bm; -sprintf (state->name,"C%d" , idx +1 ); -strncpy (state->desc, descr,CPUIDLE_DESC_LEN ); - -} -/* Helper to register the driver_data */ static inline struct omap3_idle_statedata *_fill_cstate_usage( struct cpuidle_device *dev,
@@ -350,50 +395,40 @@ int __init omap3_idle_init(void) cam_pd =pwrdm_lookup ("cam_pwrdm" ); - drv->safe_state_index =-1 ; dev = &per_cpu (omap3_idle_dev,smp_processor_id ());/* C1 . MPU WFI + Core active */ -_fill_cstate (drv,0 ,"MPU ON + CORE ON" ); - (&drv->states[0 ])->enter = omap3_enter_idle; - drv->safe_state_index =0 ; cx =_fill_cstate_usage (dev,0 ); cx->valid =1 ;/* C1 is always valid */ cx->mpu_state =PWRDM_POWER_ON ; cx->core_state =PWRDM_POWER_ON ;/* C2 . MPU WFI + Core inactive */ -_fill_cstate (drv,1 ,"MPU ON + CORE ON" ); cx =_fill_cstate_usage (dev,1 ); cx->mpu_state =PWRDM_POWER_ON ; cx->core_state =PWRDM_POWER_ON ;/* C3 . MPU CSWR + Core inactive */ -_fill_cstate (drv,2 ,"MPU RET + CORE ON" ); cx =_fill_cstate_usage (dev,2 ); cx->mpu_state =PWRDM_POWER_RET ; cx->core_state =PWRDM_POWER_ON ;/* C4 . MPU OFF + Core inactive */ -_fill_cstate (drv,3 ,"MPU OFF + CORE ON" ); cx =_fill_cstate_usage (dev,3 ); cx->mpu_state =PWRDM_POWER_OFF ; cx->core_state =PWRDM_POWER_ON ;/* C5 . MPU RET + Core RET */ -_fill_cstate (drv,4 ,"MPU RET + CORE RET" ); cx =_fill_cstate_usage (dev,4 ); cx->mpu_state =PWRDM_POWER_RET ; cx->core_state =PWRDM_POWER_RET ;/* C6 . MPU OFF + Core RET */ -_fill_cstate (drv,5 ,"MPU OFF + CORE RET" ); cx =_fill_cstate_usage (dev,5 ); cx->mpu_state =PWRDM_POWER_OFF ; cx->core_state =PWRDM_POWER_RET ;/* C7 . MPU OFF + Core OFF */ -_fill_cstate (drv,6 ,"MPU OFF + CORE OFF" ); cx =_fill_cstate_usage (dev,6 );/* * Erratum i583: implementation for ES rev < Es1.2 on 3630. We cannot
@@ -411,7 +446,6 @@ int __init omap3_idle_init(void) drv->state_count =OMAP3_NUM_STATES ;cpuidle_register_driver (&omap3_idle_driver); - dev->state_count =OMAP3_NUM_STATES ;if (cpuidle_register_device (dev)) {printk (KERN_ERR "%s: CPUidle register device failed\n" , __func__);
Signature · 1 line
1.7.5.4
PATCH v2 10/17 ARM: OMAP3: cpuidle - remove the 'valid' field
In reply to Daniel LezcanoWith the previous changes all the states are valid, except the last state which can be handled by decreasing the number of states.
--- arch/arm/mach-omap2/cpuidle34xx.c | 12 +++--------- 1 files changed, 3 insertions(+), 9 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +3 -9
@@ -67,7 +67,6 @@ static struct cpuidle_params cpuidle_params_table[] = {struct omap3_idle_statedata {u32 mpu_state;u32 core_state; -u8 valid; };struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES ];
@@ -191,8 +190,7 @@ static int next_valid_state(struct cpuidle_device *dev, }/* Check if current state is valid */ -if ((cx->valid) && - (cx->mpu_state >= mpu_deepest_state) && +if ((cx->mpu_state >= mpu_deepest_state) && (cx->core_state >= core_deepest_state)) {return index; }else {
@@ -216,8 +214,7 @@ static int next_valid_state(struct cpuidle_device *dev, idx--;for (; idx >=0 ; idx--) { cx =cpuidle_get_statedata (&dev->states_usage[idx]); -if ((cx->valid) && - (cx->mpu_state >= mpu_deepest_state) && +if ((cx->mpu_state >= mpu_deepest_state) && (cx->core_state >= core_deepest_state)) { next_index = idx;break ;
@@ -371,7 +368,6 @@ static inline struct omap3_idle_statedata *_fill_cstate_usage(struct omap3_idle_statedata *cx = &omap3_idle_data[idx];struct cpuidle_state_usage *state_usage = &dev->states_usage[idx]; - cx->valid = cpuidle_params_table[idx].valid;cpuidle_set_statedata (state_usage, cx);return cx;
@@ -399,7 +395,6 @@ int __init omap3_idle_init(void)/* C1 . MPU WFI + Core active */ cx =_fill_cstate_usage (dev,0 ); - cx->valid =1 ;/* C1 is always valid */ cx->mpu_state =PWRDM_POWER_ON ; cx->core_state =PWRDM_POWER_ON ;
@@ -436,14 +431,13 @@ int __init omap3_idle_init(void)* We disable C7 state as a result. */ if (IS_PM34XX_ERRATUM (PM_SDRC_WAKEUP_ERRATUM_i583)) { - cx->valid =0 ; + drv->state_count =OMAP3_NUM_STATES -1 ;pr_warn ("%s: core off state C7 disabled due to i583\n" , __func__); } cx->mpu_state =PWRDM_POWER_OFF ; cx->core_state =PWRDM_POWER_OFF ; - drv->state_count =OMAP3_NUM_STATES ;cpuidle_register_driver (&omap3_idle_driver);if (cpuidle_register_device (dev)) {
Signature · 1 line
1.7.5.4
PATCH v2 10/17 ARM: OMAP3: cpuidle - remove the 'valid' field
Quoted 59 lines · Daniel Lezcano
With the previous changes all the states are valid, except the last state which can be handled by decreasing the number of states.
Signed-off-by Daniel Lezcano <[email protected]>Reviewed-by Jean Pihet <[email protected]>--- arch/arm/mach-omap2/cpuidle34xx.c | 12 +++--------- 1 files changed, 3 insertions(+), 9 deletions(-)arch/arm/mach-omap2/cpuidle34xx.c +2
-7@@ -67,7 +67,6 @@ static struct cpuidle_params cpuidle_params_table[] = {struct omap3_idle_statedata {u32 mpu_state;u32 core_state; -u8 valid; };struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES ];@@ -191,8 +190,7 @@ static int next_valid_state(struct cpuidle_device *dev, }/* Check if current state is valid */ -if ((cx->valid) && - (cx->mpu_state >= mpu_deepest_state) && +if ((cx->mpu_state >= mpu_deepest_state) && (cx->core_state >= core_deepest_state)) {return index; }else {@@ -216,8 +214,7 @@ static int next_valid_state(struct cpuidle_device *dev, idx--;for (; idx >=0 ; idx--) { cx =cpuidle_get_statedata (&dev->states_usage[idx]); -if ((cx->valid) && - (cx->mpu_state >= mpu_deepest_state) && +if ((cx->mpu_state >= mpu_deepest_state) && (cx->core_state >= core_deepest_state)) { next_index = idx;break ;@@ -371,7 +368,6 @@ static inline struct omap3_idle_statedata *_fill_cstate_usage(struct omap3_idle_statedata *cx = &omap3_idle_data[idx];struct cpuidle_state_usage *state_usage = &dev->states_usage[idx]; - cx->valid = cpuidle_params_table[idx].valid;cpuidle_set_statedata (state_usage, cx);return cx;@@ -399,7 +395,6 @@ int __init omap3_idle_init(void)/* C1 . MPU WFI + Core active */ cx =_fill_cstate_usage (dev,0 ); - cx->valid =1 ;/* C1 is always valid */ cx->mpu_state =PWRDM_POWER_ON ; cx->core_state =PWRDM_POWER_ON ;
arch/arm/mach-omap2/cpuidle34xx.c int __init omap3_idle_init(void) · line 436
Quoted 7 lines
arch/arm/mach-omap2/cpuidle34xx.c +1
-1@@ -436,14 +431,13 @@ int __init omap3_idle_init(void)* We disable C7 state as a result. */ if (IS_PM34XX_ERRATUM (PM_SDRC_WAKEUP_ERRATUM_i583)) { - cx->valid =0 ; + drv->state_count =OMAP3_NUM_STATES -1 ;pr_warn ("%s: core off state C7 disabled due to i583\n" , __func__);
I'm not too particular about this one, but it might be cleaner to just remove this check all together. This errata already has a check in next_valid_state() so strictly speaking, it's not needed here.
Kevin
Quoted 8 lines
arch/arm/mach-omap2/cpuidle34xx.c +0
-1} cx->mpu_state =PWRDM_POWER_OFF ; cx->core_state =PWRDM_POWER_OFF ; - drv->state_count =OMAP3_NUM_STATES ;cpuidle_register_driver (&omap3_idle_driver);if (cpuidle_register_device (dev)) {
PATCH v2 10/17 ARM: OMAP3: cpuidle - remove the 'valid' field
Quoted 9 lines · Kevin Hilman
Quoted 7 lines · Daniel Lezcano
With the previous changes all the states are valid, except the last state which can be handled by decreasing the number of states.
Signed-off-by Daniel Lezcano<[email protected]>Reviewed-by Jean Pihet<[email protected]>---
[ ... ]
Quoted 9 lines
arch/arm/mach-omap2/cpuidle34xx.c int __init omap3_idle_init(void) · line 436
Quoted 5 lines
if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) {arch/arm/mach-omap2/cpuidle34xx.c +1
-1- cx->valid =0 ; + drv->state_count =OMAP3_NUM_STATES -1 ;pr_warn ("%s: core off state C7 disabled due to i583\n" , __func__);I'm not too particular about this one, but it might be cleaner to just remove this check all together. This errata already has a check in
next_valid_state()so strictly speaking, it's not needed here.
Yes, right. Thanks for pointing this.
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
PATCH v2 11/17 ARM: OMAP3: cpuidle - remove cpuidle_params_table
In reply to Daniel LezcanoWe do not longer need the ''cpuidle_params_table' array as we defined the states in the driver and we checked they are all valid.
We also remove the structure definition as it is no longer used.
--- arch/arm/mach-omap2/cpuidle34xx.c | 28 +++------------------------- arch/arm/mach-omap2/pm.h | 12 ------------ 2 files changed, 3 insertions(+), 37 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +3 -25
@@ -38,36 +38,14 @@#ifdef CONFIG_CPU_IDLE -/* -* The latencies/thresholds for various C states have -* to be configured from the respective board files. -* These are some default values (which might not provide -* the best power savings) used on boards which do not -* pass these details from the board file. -*/ -static struct cpuidle_params cpuidle_params_table[] = { -/* C1 */ - {2 +2 ,5 ,1 }, -/* C2 */ - {10 +10 ,30 ,1 }, -/* C3 */ - {50 +50 ,300 ,1 }, -/* C4 */ - {1500 +1800 ,4000 ,1 }, -/* C5 */ - {2500 +7500 ,12000 ,1 }, -/* C6 */ - {3000 +8500 ,15000 ,1 }, -/* C7 */ - {10000 +30000 ,300000 ,1 }, -}; -#define OMAP3_NUM_STATES ARRAY_SIZE(cpuidle_params_table) -/* Mach specific information to be recorded in the C-state driver_data */ struct omap3_idle_statedata {u32 mpu_state;u32 core_state; }; + +#define OMAP3_NUM_STATES 7 +struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES ];struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
arch/arm/mach-omap2/pm.h +0 -12
@@ -38,18 +38,6 @@ static inline int omap4_opp_init(void) }#endif -/* -* cpuidle mach specific parameters -* -* The board code can override the default C-states definition using -* omap3_pm_init_cpuidle -*/ -struct cpuidle_params { -u32 exit_latency;/* exit_latency = sleep + wake-up latencies */ -u32 target_residency; -u8 valid;/* validates the C-state */ -}; -extern int omap3_pm_get_suspend_state (struct powerdomain *pwrdm);extern int omap3_pm_set_suspend_state (struct powerdomain *pwrdm,int state);
Signature · 1 line
1.7.5.4
PATCH v2 11/17 ARM: OMAP3: cpuidle - remove cpuidle_params_table
Quoted 44 lines · Daniel Lezcano
We do not longer need the ''
cpuidle_params_table' array as we defined the states in the driver and we checked they are all valid.We also remove the structure definition as it is no longer used.
Signed-off-by Daniel Lezcano <[email protected]>Reviewed-by Jean Pihet <[email protected]>--- arch/arm/mach-omap2/cpuidle34xx.c | 28 +++------------------------- arch/arm/mach-omap2/pm.h | 12 ------------ 2 files changed, 3 insertions(+), 37 deletions(-)arch/arm/mach-omap2/cpuidle34xx.c +0
-23@@ -38,36 +38,14 @@#ifdef CONFIG_CPU_IDLE -/* -* The latencies/thresholds for various C states have -* to be configured from the respective board files. -* These are some default values (which might not provide -* the best power savings) used on boards which do not -* pass these details from the board file. -*/ -static struct cpuidle_params cpuidle_params_table[] = { -/* C1 */ - {2 +2 ,5 ,1 }, -/* C2 */ - {10 +10 ,30 ,1 }, -/* C3 */ - {50 +50 ,300 ,1 }, -/* C4 */ - {1500 +1800 ,4000 ,1 }, -/* C5 */ - {2500 +7500 ,12000 ,1 }, -/* C6 */ - {3000 +8500 ,15000 ,1 }, -/* C7 */ - {10000 +30000 ,300000 ,1 }, -};
arch/arm/mach-omap2/cpuidle34xx.c line 48
Quoted 10 lines
arch/arm/mach-omap2/cpuidle34xx.c +3
-2-#define OMAP3_NUM_STATES ARRAY_SIZE(cpuidle_params_table) -/* Mach specific information to be recorded in the C-state driver_data */ struct omap3_idle_statedata {u32 mpu_state;u32 core_state; }; + +#define OMAP3_NUM_STATES 7 +
Similar comment as for OMAP4. You should be able to git rid of this all together now.
Kevin
PATCH v2 12/17 ARM: OMAP3: define statically the omap3_idle_data
In reply to Daniel Lezcano--- arch/arm/mach-omap2/cpuidle34xx.c | 31 ++++++++++++++++++++++++++++++- 1 files changed, 30 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +30 -1
@@ -46,7 +46,36 @@ struct omap3_idle_statedata {#define OMAP3_NUM_STATES 7 -struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES ]; +struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES ] = { + { + .mpu_state =PWRDM_POWER_ON , + .core_state =PWRDM_POWER_ON , + }, + { + .mpu_state =PWRDM_POWER_ON , + .core_state =PWRDM_POWER_ON , + }, + { + .mpu_state =PWRDM_POWER_RET , + .core_state =PWRDM_POWER_ON , + }, + { + .mpu_state =PWRDM_POWER_OFF , + .core_state =PWRDM_POWER_ON , + }, + { + .mpu_state =PWRDM_POWER_RET , + .core_state =PWRDM_POWER_RET , + }, + { + .mpu_state =PWRDM_POWER_OFF , + .core_state =PWRDM_POWER_RET , + }, + { + .mpu_state =PWRDM_POWER_OFF , + .core_state =PWRDM_POWER_OFF , + }, +};struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
Signature · 1 line
1.7.5.4
PATCH v2 13/17 ARM: OMAP3: cpuidle - use omap3_idle_data directly
In reply to Daniel LezcanoWe are storing the 'omap3_idle_data' in the private data field of the cpuidle device. As we are using this variable only in this file, that does not really make sense. Let's use the global variable directly instead dereferencing pointers in an idle critical loop.
As the table is initialized statically, let's remove the initialization at startup too.
Also, that simplfies the code.
--- arch/arm/mach-omap2/cpuidle34xx.c | 65 ++++-------------------------------- 1 files changed, 8 insertions(+), 57 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +8 -57
@@ -97,8 +97,7 @@ static int __omap3_enter_idle(struct cpuidle_device *dev, struct cpuidle_driver *drv, int index) { -struct omap3_idle_statedata *cx = -cpuidle_get_statedata (&dev->states_usage[index]); +struct omap3_idle_statedata *cx = &omap3_idle_data[index];u32 mpu_state = cx->mpu_state, core_state = cx->core_state;local_fiq_disable ();
@@ -178,9 +177,8 @@ static int next_valid_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, int index) { -struct cpuidle_state_usage *curr_usage = &dev->states_usage[index];struct cpuidle_state *curr = &drv->states[index]; -struct omap3_idle_statedata *cx =cpuidle_get_statedata (curr_usage); +struct omap3_idle_statedata *cx = &omap3_idle_data[index];u32 mpu_deepest_state =PWRDM_POWER_RET ;u32 core_deepest_state =PWRDM_POWER_RET ;int next_index =-1 ;
@@ -220,7 +218,7 @@ static int next_valid_state(struct cpuidle_device *dev,*/ idx--;for (; idx >=0 ; idx--) { - cx =cpuidle_get_statedata (&dev->states_usage[idx]); + cx = &omap3_idle_data[idx];if ((cx->mpu_state >= mpu_deepest_state) && (cx->core_state >= core_deepest_state)) { next_index = idx;
@@ -277,7 +275,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,* Prevent PER off if CORE is not in retention or off as this * would disable PER wakeups completely. */ - cx =cpuidle_get_statedata (&dev->states_usage[index]); + cx = &omap3_idle_data[index]; core_next_state = cx->core_state; per_next_state = per_saved_state =pwrdm_read_next_pwrst (per_pd); if ((per_next_state == PWRDM_POWER_OFF) &&
@@ -367,19 +365,6 @@ struct cpuidle_driver omap3_idle_driver = { .safe_state_index = 0, }; -/* Helper to register the driver_data */ -static inline struct omap3_idle_statedata *_fill_cstate_usage ( -struct cpuidle_device *dev, -int idx) -{ -struct omap3_idle_statedata *cx = &omap3_idle_data[idx]; -struct cpuidle_state_usage *state_usage = &dev->states_usage[idx]; - -cpuidle_set_statedata (state_usage, cx); - -return cx; -} -/** * omap3_idle_init - Init routine for OMAP3 idle *
@@ -390,48 +375,12 @@ int __init omap3_idle_init(void) {struct cpuidle_device *dev;struct cpuidle_driver *drv = &omap3_idle_driver; -struct omap3_idle_statedata *cx; mpu_pd =pwrdm_lookup ("mpu_pwrdm" ); core_pd =pwrdm_lookup ("core_pwrdm" ); per_pd =pwrdm_lookup ("per_pwrdm" ); cam_pd =pwrdm_lookup ("cam_pwrdm" ); - - dev = &per_cpu (omap3_idle_dev,smp_processor_id ()); - -/* C1 . MPU WFI + Core active */ - cx =_fill_cstate_usage (dev,0 ); - cx->mpu_state =PWRDM_POWER_ON ; - cx->core_state =PWRDM_POWER_ON ; - -/* C2 . MPU WFI + Core inactive */ - cx =_fill_cstate_usage (dev,1 ); - cx->mpu_state =PWRDM_POWER_ON ; - cx->core_state =PWRDM_POWER_ON ; - -/* C3 . MPU CSWR + Core inactive */ - cx =_fill_cstate_usage (dev,2 ); - cx->mpu_state =PWRDM_POWER_RET ; - cx->core_state =PWRDM_POWER_ON ; - -/* C4 . MPU OFF + Core inactive */ - cx =_fill_cstate_usage (dev,3 ); - cx->mpu_state =PWRDM_POWER_OFF ; - cx->core_state =PWRDM_POWER_ON ; - -/* C5 . MPU RET + Core RET */ - cx =_fill_cstate_usage (dev,4 ); - cx->mpu_state =PWRDM_POWER_RET ; - cx->core_state =PWRDM_POWER_RET ; - -/* C6 . MPU OFF + Core RET */ - cx =_fill_cstate_usage (dev,5 ); - cx->mpu_state =PWRDM_POWER_OFF ; - cx->core_state =PWRDM_POWER_RET ; - -/* C7 . MPU OFF + Core OFF */ - cx =_fill_cstate_usage (dev,6 );/* * Erratum i583: implementation for ES rev < Es1.2 on 3630. We cannot * enable OFF mode in a stable form for previous revisions.
@@ -442,11 +391,13 @@ int __init omap3_idle_init(void)pr_warn ("%s: core off state C7 disabled due to i583\n" , __func__); } - cx->mpu_state =PWRDM_POWER_OFF ; - cx->core_state =PWRDM_POWER_OFF ;cpuidle_register_driver (&omap3_idle_driver); + dev = &per_cpu (omap3_idle_dev,smp_processor_id ()); + dev->cpu =0 ; + dev->state_count = drv->state_count; +if (cpuidle_register_device (dev)) {printk (KERN_ERR "%s: CPUidle register device failed\n" , __func__);
Signature · 1 line
1.7.5.4
PATCH v2 14/17 ARM: OMAP3 : cpuidle - simplify next_valid_state
In reply to Daniel LezcanoSimplify the indentation by removing the useless 'else' statement. Remove the first loop for the 'idx' search as we have it already with the 'index' passed as parameter.
--- arch/arm/mach-omap2/cpuidle34xx.c | 53 +++++++++++++----------------------- 1 files changed, 19 insertions(+), 34 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +19 -34
@@ -174,13 +174,12 @@ static inline int omap3_enter_idle(struct cpuidle_device *dev,* if it satisfies the enable_off_mode condition. */ static int next_valid_state (struct cpuidle_device *dev, -struct cpuidle_driver *drv, -int index) +struct cpuidle_driver *drv,int index) { -struct cpuidle_state *curr = &drv->states[index];struct omap3_idle_statedata *cx = &omap3_idle_data[index];u32 mpu_deepest_state =PWRDM_POWER_RET ;u32 core_deepest_state =PWRDM_POWER_RET ; +int idx;int next_index =-1 ;if (enable_off_mode) {
@@ -196,42 +195,28 @@ static int next_valid_state(struct cpuidle_device *dev,/* Check if current state is valid */ if ((cx->mpu_state >= mpu_deepest_state) && - (cx->core_state >= core_deepest_state)) { + (cx->core_state >= core_deepest_state))return index; - }else { -int idx =OMAP3_NUM_STATES -1 ; - -/* Reach the current state starting at highest C-state */ -for (; idx >=0 ; idx--) { -if (&drv->states[idx] == curr) { - next_index = idx; -break ; - } - } - -/* Should never hit this condition */ -WARN_ON (next_index ==-1 ); -/* -* Drop to next valid state. -* Start search from the next (lower) state. -*/ - idx--; -for (; idx >=0 ; idx--) { - cx = &omap3_idle_data[idx]; -if ((cx->mpu_state >= mpu_deepest_state) && - (cx->core_state >= core_deepest_state)) { - next_index = idx; -break ; - } +/* +* Drop to next valid state. +* Start search from the next (lower) state. +*/ +for (idx = index -1 ; idx >=0 ; idx--) { + cx = &omap3_idle_data[idx]; +if ((cx->mpu_state >= mpu_deepest_state) && + (cx->core_state >= core_deepest_state)) { + next_index = idx; +break ; } -/* -* C1 is always valid. -* So, no need to check for 'next_index == -1' outside -* this loop. -*/ } +/* +* C1 is always valid. +* So, no need to check for 'next_index == -1' outside +* this loop. +*/ +return next_index; }
Signature · 1 line
1.7.5.4
PATCH v2 15/17 ARM: OMAP3: set omap3_idle_data as static
In reply to Daniel Lezcano--- arch/arm/mach-omap2/cpuidle34xx.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +1 -1
@@ -46,7 +46,7 @@ struct omap3_idle_statedata {#define OMAP3_NUM_STATES 7 -struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES ] = { +static struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES ] = { { .mpu_state =PWRDM_POWER_ON , .core_state =PWRDM_POWER_ON ,
Signature · 1 line
1.7.5.4
PATCH v2 16/17 ARM: OMAP3/4: consolidate cpuidle Makefile
In reply to Daniel Lezcano--- arch/arm/mach-omap2/Makefile | 11 +++++++---- arch/arm/mach-omap2/cpuidle34xx.c | 8 -------- arch/arm/mach-omap2/cpuidle44xx.c | 8 -------- arch/arm/mach-omap2/pm.h | 17 +++++++++++++++-- 4 files changed, 22 insertions(+), 22 deletions(-)
arch/arm/mach-omap2/Makefile +7 -4
@@ -64,10 +64,8 @@ endif ifeq ($(CONFIG_PM),y) obj-$(CONFIG_ARCH_OMAP2) += pm24xx.o obj-$(CONFIG_ARCH_OMAP2) += sleep24xx.o -obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o \ - cpuidle34xx.o -obj-$(CONFIG_ARCH_OMAP4) += pm44xx.o omap-mpuss-lowpower.o \ - cpuidle44xx.o +obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o +obj-$(CONFIG_ARCH_OMAP4) += pm44xx.o omap-mpuss-lowpower.o obj-$(CONFIG_PM_DEBUG) += pm-debug.o obj-$(CONFIG_OMAP_SMARTREFLEX) += sr_device.o smartreflex.o obj-$(CONFIG_OMAP_SMARTREFLEX_CLASS3) += smartreflex-class3.o
@@ -81,6 +79,11 @@ endif endif +ifeq ($(CONFIG_CPU_IDLE),y) +obj-$(CONFIG_ARCH_OMAP3) += cpuidle34xx.o +obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o +endif + # PRCM obj-y += prm_common.o obj-$(CONFIG_ARCH_OMAP2) += prcm.o cm2xxx_3xxx.o prm2xxx_3xxx.o
arch/arm/mach-omap2/cpuidle34xx.c +0 -8
@@ -36,8 +36,6 @@#include "control.h" #include "common.h" -#ifdef CONFIG_CPU_IDLE -/* Mach specific information to be recorded in the C-state driver_data */ struct omap3_idle_statedata {u32 mpu_state;
@@ -391,9 +389,3 @@ int __init omap3_idle_init(void)return 0 ; } -#else -int __init omap3_idle_init (void ) -{ -return 0 ; -} -#endif /* CONFIG_CPU_IDLE */
arch/arm/mach-omap2/cpuidle44xx.c +0 -8
@@ -22,8 +22,6 @@#include "pm.h" #include "prm.h" -#ifdef CONFIG_CPU_IDLE -/* Machine specific information */ struct omap4_idle_statedata {u32 cpu_state;
@@ -204,9 +202,3 @@ int __init omap4_idle_init(void)return 0 ; } -#else -int __init omap4_idle_init (void ) -{ -return 0 ; -} -#endif /* CONFIG_CPU_IDLE */
arch/arm/mach-omap2/pm.h +15 -2
@@ -15,12 +15,25 @@#include "powerdomain.h" +#ifdef CONFIG_CPU_IDLE +extern int __init omap3_idle_init (void ); +extern int __init omap4_idle_init (void ); +#else +static inline int omap3_idle_init (void ) +{ +return 0 ; +} + +static inline int omap4_idle_init (void ) +{ +return 0 ; +} +#endif +extern void *omap3_secure_ram_storage;extern void omap3_pm_off_mode_enable (int );extern void omap_sram_idle (void );extern int omap_set_pwrdm_state (struct powerdomain *pwrdm,u32 state); -extern int omap3_idle_init (void ); -extern int omap4_idle_init (void );extern int omap_pm_clkdms_setup (struct clockdomain *clkdm,void *unused);extern int (*omap_pm_suspend)(void );
Signature · 1 line
1.7.5.4
PATCH v2 17/17 ARM: OMAP3: cpuidle - set global variables static
In reply to Daniel Lezcanoand check the powerdomain lookup is successful.
--- arch/arm/mach-omap2/cpuidle34xx.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +4 -1
@@ -75,7 +75,7 @@ static struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES] = { }, }; -struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd; +static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;static int _cpuidle_allow_idle (struct powerdomain *pwrdm,struct clockdomain *clkdm)
@@ -364,6 +364,9 @@ int __init omap3_idle_init(void) per_pd =pwrdm_lookup ("per_pwrdm" ); cam_pd =pwrdm_lookup ("cam_pwrdm" ); +if (!mpu_pd || !core_pd || !per_pd || !cam_pd) +return -ENODEV ; +/* * Erratum i583: implementation for ES rev < Es1.2 on 3630. We cannot * enable OFF mode in a stable form for previous revisions.
Signature · 1 line
1.7.5.4
v2 cover ARM: OMAP3/4 : cpuidle34xx and cpuidle44xx cleanups
In reply to Daniel LezcanoQuoted 2 lines · Daniel Lezcano
This patchset makes some cleanup on these cpuidle drivers and consolidate the code across both architecture.
Thanks for this really nice cleanup. I have some comments on specific patches, but here's some general comments:
Some minor comments:
First, please be sure all patches have a descriptive changelog. Yes, even simple ones need changelogs. In particular, maintainers are looking for the "why" of a patch, not just the "what" or "how".
Second, this series introduced a couple sparse warnings:
/work/kernel/omap/pm/arch/arm/mach-omap2/cpuidle44xx.c:134:1: warning: symbol 'omap4_idle_dev' was not declared. Should it be static? /work/kernel/omap/pm/arch/arm/mach-omap2/cpuidle44xx.c:136:23: warning: symbol 'omap4_idle_driver' was not declared. Should it be static?
Thanks,
Kevin
v2 cover ARM: OMAP3/4 : cpuidle34xx and cpuidle44xx cleanups
Quoted 10 lines · Kevin Hilman
Quoted 2 lines · Daniel Lezcano
This patchset makes some cleanup on these cpuidle drivers and consolidate the code across both architecture.
Thanks for this really nice cleanup. I have some comments on specific patches, but here's some general comments:
Some minor comments:
Quoted 8 lines
First, please be sure all patches have a descriptive changelog. Yes, even simple ones need changelogs. In particular, maintainers are looking for the "why" of a patch, not just the "what" or "how".
Second, this series introduced a couple sparse warnings:
/work/kernel/omap/pm/arch/arm/mach-omap2/cpuidle44xx.c:134:1: warning: symbol '
omap4_idle_dev' was not declared. Should it be static? /work/kernel/omap/pm/arch/arm/mach-omap2/cpuidle44xx.c:136:23: warning: symbol 'omap4_idle_driver' was not declared. Should it be static?
Ok, I probably missed it. I will fix that it in the next version.
Thanks for reviewing.
-- Daniel
Signature · 5 lines
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog