* [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
The 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.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++---------------
1 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index ee0bc50..6d86b59 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
The 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.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++---------------
1 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index ee0bc50..6d86b59 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread[parent not found: <1333570371-1389-3-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-09 22:37 ` Kevin Hilman
-1 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 22:37 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw,
patches-QSEj5FYQhm4dnm+yROfE0A, tony-4v6yS6AI5VpBDgjK7y7TUQ,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
> The 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.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
> ---
> arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++---------------
> 1 files changed, 33 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
> index ee0bc50..6d86b59 100644
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -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,
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
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
@ 2012-04-09 22:37 ` Kevin Hilman
0 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 22:37 UTC (permalink / raw)
To: linux-arm-kernel
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> The 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.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Jean Pihet <j-pihet@ti.com>
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
> arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++---------------
> 1 files changed, 33 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
> index ee0bc50..6d86b59 100644
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -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,
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
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
2012-04-09 22:37 ` Kevin Hilman
@ 2012-04-19 13:58 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-19 13:58 UTC (permalink / raw)
To: Kevin Hilman
Cc: santosh.shilimkar, jean.pihet, tony, rob.lee, linux-omap,
linaro-dev, linux-arm-kernel, patches
On 04/10/2012 12:37 AM, Kevin Hilman wrote:
> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>
>> The 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.
>>
>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>> ---
>> arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++---------------
>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
>> index ee0bc50..6d86b59 100644
>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>> @@ -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,
>
> 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.
Ok.
--
<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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
@ 2012-04-19 13:58 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-19 13:58 UTC (permalink / raw)
To: linux-arm-kernel
On 04/10/2012 12:37 AM, Kevin Hilman wrote:
> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>
>> The 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.
>>
>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>> ---
>> arch/arm/mach-omap2/cpuidle44xx.c | 57 +++++++++++++++++++++---------------
>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
>> index ee0bc50..6d86b59 100644
>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>> @@ -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,
>
> 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.
Ok.
--
<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
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
2012-04-19 13:58 ` Daniel Lezcano
@ 2012-04-23 14:06 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-23 14:06 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linaro-dev, patches, tony, linux-omap, linux-arm-kernel
On 04/19/2012 03:58 PM, Daniel Lezcano wrote:
> On 04/10/2012 12:37 AM, Kevin Hilman wrote:
>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>
>>> The 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.
>>>
>>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>> ---
>>> arch/arm/mach-omap2/cpuidle44xx.c | 57
>>> +++++++++++++++++++++---------------
>>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
>>> b/arch/arm/mach-omap2/cpuidle44xx.c
>>> index ee0bc50..6d86b59 100644
>>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>>> @@ -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,
>>
>> 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.
>
> 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.
--
<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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
@ 2012-04-23 14:06 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-23 14:06 UTC (permalink / raw)
To: linux-arm-kernel
On 04/19/2012 03:58 PM, Daniel Lezcano wrote:
> On 04/10/2012 12:37 AM, Kevin Hilman wrote:
>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>
>>> The 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.
>>>
>>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>> ---
>>> arch/arm/mach-omap2/cpuidle44xx.c | 57
>>> +++++++++++++++++++++---------------
>>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
>>> b/arch/arm/mach-omap2/cpuidle44xx.c
>>> index ee0bc50..6d86b59 100644
>>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>>> @@ -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,
>>
>> 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.
>
> 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.
--
<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
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
2012-04-23 14:06 ` Daniel Lezcano
@ 2012-04-23 17:08 ` Kevin Hilman
-1 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-23 17:08 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: tony, linux-omap, linaro-dev, linux-arm-kernel, patches
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> On 04/19/2012 03:58 PM, Daniel Lezcano wrote:
>> On 04/10/2012 12:37 AM, Kevin Hilman wrote:
>>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>>
>>>> The 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.
>>>>
>>>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>>>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>>>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>>> ---
>>>> arch/arm/mach-omap2/cpuidle44xx.c | 57
>>>> +++++++++++++++++++++---------------
>>>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
>>>> b/arch/arm/mach-omap2/cpuidle44xx.c
>>>> index ee0bc50..6d86b59 100644
>>>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>>>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>>>> @@ -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,
>>>
>>> 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.
>>
>> 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
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
@ 2012-04-23 17:08 ` Kevin Hilman
0 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-23 17:08 UTC (permalink / raw)
To: linux-arm-kernel
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> On 04/19/2012 03:58 PM, Daniel Lezcano wrote:
>> On 04/10/2012 12:37 AM, Kevin Hilman wrote:
>>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>>
>>>> The 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.
>>>>
>>>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>>>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>>>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>>> ---
>>>> arch/arm/mach-omap2/cpuidle44xx.c | 57
>>>> +++++++++++++++++++++---------------
>>>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
>>>> b/arch/arm/mach-omap2/cpuidle44xx.c
>>>> index ee0bc50..6d86b59 100644
>>>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>>>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>>>> @@ -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,
>>>
>>> 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.
>>
>> 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
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
2012-04-23 17:08 ` Kevin Hilman
@ 2012-04-23 22:11 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-23 22:11 UTC (permalink / raw)
To: Kevin Hilman; +Cc: tony, linux-omap, linaro-dev, linux-arm-kernel, patches
On 04/23/2012 07:08 PM, Kevin Hilman wrote:
> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>
>> On 04/19/2012 03:58 PM, Daniel Lezcano wrote:
>>> On 04/10/2012 12:37 AM, Kevin Hilman wrote:
>>>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>>>
>>>>> The 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.
>>>>>
>>>>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>>>>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>>>>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>>>> ---
>>>>> arch/arm/mach-omap2/cpuidle44xx.c | 57
>>>>> +++++++++++++++++++++---------------
>>>>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
>>>>> b/arch/arm/mach-omap2/cpuidle44xx.c
>>>>> index ee0bc50..6d86b59 100644
>>>>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>>>>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>>>>> @@ -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,
>>>>
>>>> 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.
>>>
>>> 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.
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
--
<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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
@ 2012-04-23 22:11 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-23 22:11 UTC (permalink / raw)
To: linux-arm-kernel
On 04/23/2012 07:08 PM, Kevin Hilman wrote:
> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>
>> On 04/19/2012 03:58 PM, Daniel Lezcano wrote:
>>> On 04/10/2012 12:37 AM, Kevin Hilman wrote:
>>>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>>>
>>>>> The 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.
>>>>>
>>>>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>>>>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>>>>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>>>> ---
>>>>> arch/arm/mach-omap2/cpuidle44xx.c | 57
>>>>> +++++++++++++++++++++---------------
>>>>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
>>>>> b/arch/arm/mach-omap2/cpuidle44xx.c
>>>>> index ee0bc50..6d86b59 100644
>>>>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>>>>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>>>>> @@ -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,
>>>>
>>>> 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.
>>>
>>> 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.
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
--
<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
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
2012-04-23 22:11 ` Daniel Lezcano
@ 2012-04-24 0:27 ` Kevin Hilman
-1 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-24 0:27 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: tony, linux-omap, linaro-dev, linux-arm-kernel, patches
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> On 04/23/2012 07:08 PM, Kevin Hilman wrote:
>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>
>>> On 04/19/2012 03:58 PM, Daniel Lezcano wrote:
>>>> On 04/10/2012 12:37 AM, Kevin Hilman wrote:
>>>>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>>>>
>>>>>> The 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.
>>>>>>
>>>>>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>>>>>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>>>>>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>>>>> ---
>>>>>> arch/arm/mach-omap2/cpuidle44xx.c | 57
>>>>>> +++++++++++++++++++++---------------
>>>>>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
>>>>>> b/arch/arm/mach-omap2/cpuidle44xx.c
>>>>>> index ee0bc50..6d86b59 100644
>>>>>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>>>>>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>>>>>> @@ -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,
>>>>>
>>>>> 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.
>>>>
>>>> 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.
>
>
> 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.
Oh, I see now.
> 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),
Great, that should work.
Thanks,
Kevin
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 02/17][V2] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
@ 2012-04-24 0:27 ` Kevin Hilman
0 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-24 0:27 UTC (permalink / raw)
To: linux-arm-kernel
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> On 04/23/2012 07:08 PM, Kevin Hilman wrote:
>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>
>>> On 04/19/2012 03:58 PM, Daniel Lezcano wrote:
>>>> On 04/10/2012 12:37 AM, Kevin Hilman wrote:
>>>>> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>>>>>
>>>>>> The 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.
>>>>>>
>>>>>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>>>>>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>>>>>> Reviewed-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>>>>> ---
>>>>>> arch/arm/mach-omap2/cpuidle44xx.c | 57
>>>>>> +++++++++++++++++++++---------------
>>>>>> 1 files changed, 33 insertions(+), 24 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
>>>>>> b/arch/arm/mach-omap2/cpuidle44xx.c
>>>>>> index ee0bc50..6d86b59 100644
>>>>>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>>>>>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>>>>>> @@ -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,
>>>>>
>>>>> 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.
>>>>
>>>> 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.
>
>
> 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.
Oh, I see now.
> 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),
Great, that should work.
Thanks,
Kevin
^ permalink raw reply [flat|nested] 66+ messages in thread
* [PATCH 03/17][V2] ARM: OMAP4: cpuidle - Remove the cpuidle_params_table table
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
We do not longer need this table as we defined the values
in the driver states.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle44xx.c | 11 +----------
1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index 6d86b59..cdd7932 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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 3
struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES];
static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 03/17][V2] ARM: OMAP4: cpuidle - Remove the cpuidle_params_table table
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
We do not longer need this table as we defined the values
in the driver states.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
arch/arm/mach-omap2/cpuidle44xx.c | 11 +----------
1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index 6d86b59..cdd7932 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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 3
struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES];
static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread
* [PATCH 04/17][V2] ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle44xx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index cdd7932..fb0f76e 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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;
/**
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 04/17][V2] ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
arch/arm/mach-omap2/cpuidle44xx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index cdd7932..fb0f76e 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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;
/**
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* Re: [PATCH 04/17][V2] ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-09 22:38 ` Kevin Hilman
-1 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 22:38 UTC (permalink / raw)
To: Daniel Lezcano
Cc: santosh.shilimkar, jean.pihet, tony, rob.lee, linux-omap,
linaro-dev, linux-arm-kernel, patches
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Jean Pihet <j-pihet@ti.com>
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Missing changelog.
Kevin
> ---
> arch/arm/mach-omap2/cpuidle44xx.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
> index cdd7932..fb0f76e 100644
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -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;
>
> /**
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 04/17][V2] ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration
@ 2012-04-09 22:38 ` Kevin Hilman
0 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 22:38 UTC (permalink / raw)
To: linux-arm-kernel
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Jean Pihet <j-pihet@ti.com>
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Missing changelog.
Kevin
> ---
> arch/arm/mach-omap2/cpuidle44xx.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
> index cdd7932..fb0f76e 100644
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -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;
>
> /**
^ permalink raw reply [flat|nested] 66+ messages in thread
* [PATCH 05/17][V2] ARM: OMAP4: cpuidle - Initialize omap4_idle_data at compile time
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle44xx.c | 19 ++++++++++++++++++-
1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index fb0f76e..5b20115 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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;
/**
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 05/17][V2] ARM: OMAP4: cpuidle - Initialize omap4_idle_data at compile time
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
arch/arm/mach-omap2/cpuidle44xx.c | 19 ++++++++++++++++++-
1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index fb0f76e..5b20115 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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;
/**
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread
* [PATCH 06/17][V2] ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
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.
Also, that simplfies the code.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle44xx.c | 15 ++++-----------
1 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index 5b20115..b82f9fe 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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");
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 06/17][V2] ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
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.
Also, that simplfies the code.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
arch/arm/mach-omap2/cpuidle44xx.c | 15 ++++-----------
1 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index 5b20115..b82f9fe 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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");
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread[parent not found: <1333570371-1389-7-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 06/17][V2] ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-09 22:56 ` Kevin Hilman
-1 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 22:56 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw,
patches-QSEj5FYQhm4dnm+yROfE0A, tony-4v6yS6AI5VpBDgjK7y7TUQ,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
> 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?
> 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
^ permalink raw reply [flat|nested] 66+ messages in thread
* [PATCH 06/17][V2] ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
@ 2012-04-09 22:56 ` Kevin Hilman
0 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 22:56 UTC (permalink / raw)
To: linux-arm-kernel
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> 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?
> 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
^ permalink raw reply [flat|nested] 66+ messages in thread
* Re: [PATCH 06/17][V2] ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
2012-04-09 22:56 ` Kevin Hilman
@ 2012-04-19 14:49 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-19 14:49 UTC (permalink / raw)
To: Kevin Hilman
Cc: santosh.shilimkar, jean.pihet, tony, linux-omap,
linux-arm-kernel, rob.lee, linaro-dev
On 04/10/2012 12:56 AM, Kevin Hilman wrote:
> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>
>> 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 :)
>> 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.
--
<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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 66+ messages in thread
* [PATCH 06/17][V2] ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly
@ 2012-04-19 14:49 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-19 14:49 UTC (permalink / raw)
To: linux-arm-kernel
On 04/10/2012 12:56 AM, Kevin Hilman wrote:
> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>
>> 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 :)
>> 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.
--
<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
^ permalink raw reply [flat|nested] 66+ messages in thread
* [PATCH 07/17][V2] ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot time
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
We initialized it at compile time, no need to do that at boot
time.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle44xx.c | 26 +-------------------------
1 files changed, 1 insertions(+), 25 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index b82f9fe..b0dd220 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 07/17][V2] ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot time
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
We initialized it at compile time, no need to do that at boot
time.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
arch/arm/mach-omap2/cpuidle44xx.c | 26 +-------------------------
1 files changed, 1 insertions(+), 25 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index b82f9fe..b0dd220 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -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;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread[parent not found: <1333570371-1389-8-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 07/17][V2] ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot time
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-09 22:40 ` Kevin Hilman
-1 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 22:40 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw,
patches-QSEj5FYQhm4dnm+yROfE0A, tony-4v6yS6AI5VpBDgjK7y7TUQ,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
> We initialized it at compile time, no need to do that at boot
> time.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
> ---
> arch/arm/mach-omap2/cpuidle44xx.c | 26 +-------------------------
> 1 files changed, 1 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
> index b82f9fe..b0dd220 100644
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -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
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 07/17][V2] ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot time
@ 2012-04-09 22:40 ` Kevin Hilman
0 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 22:40 UTC (permalink / raw)
To: linux-arm-kernel
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> We initialized it at compile time, no need to do that at boot
> time.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Jean Pihet <j-pihet@ti.com>
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
> arch/arm/mach-omap2/cpuidle44xx.c | 26 +-------------------------
> 1 files changed, 1 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
> index b82f9fe..b0dd220 100644
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -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
^ permalink raw reply [flat|nested] 66+ messages in thread
* [PATCH 08/17][V2] ARM: OMAP3: cpuidle - remove rx51 cpuidle parameters table
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
As 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.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
---
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(-)
diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
index 27f01f0..2da92a6 100644
--- a/arch/arm/mach-omap2/board-rx51.c
+++ b/arch/arm/mach-omap2/board-rx51.c
@@ -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();
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 5358664..3519a8b 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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,
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 36fa90b..5646b80 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -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);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 08/17][V2] ARM: OMAP3: cpuidle - remove rx51 cpuidle parameters table
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
As 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.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
---
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(-)
diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
index 27f01f0..2da92a6 100644
--- a/arch/arm/mach-omap2/board-rx51.c
+++ b/arch/arm/mach-omap2/board-rx51.c
@@ -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();
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 5358664..3519a8b 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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,
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 36fa90b..5646b80 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -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);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread
* [PATCH 09/17][V2] ARM: OMAP3: define cpuidle statically
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
Use the new cpuidle API and define in the driver the states.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle34xx.c | 86 +++++++++++++++++++++++++-----------
1 files changed, 60 insertions(+), 26 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 3519a8b..11a2c23 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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__);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 09/17][V2] ARM: OMAP3: define cpuidle statically
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
Use the new cpuidle API and define in the driver the states.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/cpuidle34xx.c | 86 +++++++++++++++++++++++++-----------
1 files changed, 60 insertions(+), 26 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 3519a8b..11a2c23 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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__);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread
* [PATCH 10/17][V2] ARM: OMAP3: cpuidle - remove the 'valid' field
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
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 <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle34xx.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 11a2c23..3f46e45 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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)) {
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 10/17][V2] ARM: OMAP3: cpuidle - remove the 'valid' field
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
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 <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/cpuidle34xx.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 11a2c23..3f46e45 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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)) {
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* Re: [PATCH 10/17][V2] ARM: OMAP3: cpuidle - remove the 'valid' field
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-09 23:13 ` Kevin Hilman
-1 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 23:13 UTC (permalink / raw)
To: Daniel Lezcano
Cc: santosh.shilimkar, jean.pihet, tony, linux-omap,
linux-arm-kernel, rob.lee, linaro-dev, patches
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> 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 <daniel.lezcano@linaro.org>
> Reviewed-by: Jean Pihet <j-pihet@ti.com>
> ---
> arch/arm/mach-omap2/cpuidle34xx.c | 12 +++---------
> 1 files changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 11a2c23..3f46e45 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -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__);
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
> }
> 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)) {
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 10/17][V2] ARM: OMAP3: cpuidle - remove the 'valid' field
@ 2012-04-09 23:13 ` Kevin Hilman
0 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 23:13 UTC (permalink / raw)
To: linux-arm-kernel
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> 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 <daniel.lezcano@linaro.org>
> Reviewed-by: Jean Pihet <j-pihet@ti.com>
> ---
> arch/arm/mach-omap2/cpuidle34xx.c | 12 +++---------
> 1 files changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 11a2c23..3f46e45 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -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__);
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
> }
> 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)) {
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [PATCH 10/17][V2] ARM: OMAP3: cpuidle - remove the 'valid' field
2012-04-09 23:13 ` Kevin Hilman
@ 2012-04-19 14:02 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-19 14:02 UTC (permalink / raw)
To: Kevin Hilman
Cc: santosh.shilimkar, jean.pihet, tony, linux-omap,
linux-arm-kernel, rob.lee, linaro-dev, patches
On 04/10/2012 01:13 AM, Kevin Hilman wrote:
> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>
>> 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<daniel.lezcano@linaro.org>
>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>> ---
[ ... ]
>> 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.
Yes, right. Thanks for pointing this.
--
<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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 10/17][V2] ARM: OMAP3: cpuidle - remove the 'valid' field
@ 2012-04-19 14:02 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-19 14:02 UTC (permalink / raw)
To: linux-arm-kernel
On 04/10/2012 01:13 AM, Kevin Hilman wrote:
> Daniel Lezcano<daniel.lezcano@linaro.org> writes:
>
>> 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<daniel.lezcano@linaro.org>
>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>> ---
[ ... ]
>> 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.
Yes, right. Thanks for pointing this.
--
<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
^ permalink raw reply [flat|nested] 66+ messages in thread
* [PATCH 11/17][V2] ARM: OMAP3: cpuidle - remove cpuidle_params_table
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
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 <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle34xx.c | 28 +++-------------------------
arch/arm/mach-omap2/pm.h | 12 ------------
2 files changed, 3 insertions(+), 37 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 3f46e45..cdf1b8f 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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;
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 5646b80..7856489 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -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);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 11/17][V2] ARM: OMAP3: cpuidle - remove cpuidle_params_table
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
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 <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/cpuidle34xx.c | 28 +++-------------------------
arch/arm/mach-omap2/pm.h | 12 ------------
2 files changed, 3 insertions(+), 37 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 3f46e45..cdf1b8f 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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;
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 5646b80..7856489 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -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);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* Re: [PATCH 11/17][V2] ARM: OMAP3: cpuidle - remove cpuidle_params_table
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-09 23:12 ` Kevin Hilman
-1 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 23:12 UTC (permalink / raw)
To: Daniel Lezcano
Cc: santosh.shilimkar, jean.pihet, tony, linux-omap, linaro-dev,
linux-arm-kernel, patches
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> 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 <daniel.lezcano@linaro.org>
> Reviewed-by: Jean Pihet <j-pihet@ti.com>
> ---
> arch/arm/mach-omap2/cpuidle34xx.c | 28 +++-------------------------
> arch/arm/mach-omap2/pm.h | 12 ------------
> 2 files changed, 3 insertions(+), 37 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 3f46e45..cdf1b8f 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -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
> +
Similar comment as for OMAP4. You should be able to git rid of this all
together now.
Kevin
^ permalink raw reply [flat|nested] 66+ messages in thread* [PATCH 11/17][V2] ARM: OMAP3: cpuidle - remove cpuidle_params_table
@ 2012-04-09 23:12 ` Kevin Hilman
0 siblings, 0 replies; 66+ messages in thread
From: Kevin Hilman @ 2012-04-09 23:12 UTC (permalink / raw)
To: linux-arm-kernel
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> 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 <daniel.lezcano@linaro.org>
> Reviewed-by: Jean Pihet <j-pihet@ti.com>
> ---
> arch/arm/mach-omap2/cpuidle34xx.c | 28 +++-------------------------
> arch/arm/mach-omap2/pm.h | 12 ------------
> 2 files changed, 3 insertions(+), 37 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 3f46e45..cdf1b8f 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -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
> +
Similar comment as for OMAP4. You should be able to git rid of this all
together now.
Kevin
^ permalink raw reply [flat|nested] 66+ messages in thread
* [PATCH 14/17][V2] ARM: OMAP3 : cpuidle - simplify next_valid_state
2012-04-04 20:12 ` Daniel Lezcano
@ 2012-04-04 20:12 ` Daniel Lezcano
-1 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: santosh.shilimkar-l0cyMroinI0, jean.pihet-OTs2U3NB0ngRmelmmXo44Q,
khilman-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
patches-QSEj5FYQhm4dnm+yROfE0A
Simplify 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.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Jean Pihet <j-pihet-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-omap2/cpuidle34xx.c | 53 +++++++++++++-----------------------
1 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index e4738eb..35a1471 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread* [PATCH 14/17][V2] ARM: OMAP3 : cpuidle - simplify next_valid_state
@ 2012-04-04 20:12 ` Daniel Lezcano
0 siblings, 0 replies; 66+ messages in thread
From: Daniel Lezcano @ 2012-04-04 20:12 UTC (permalink / raw)
To: linux-arm-kernel
Simplify 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.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/cpuidle34xx.c | 53 +++++++++++++-----------------------
1 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index e4738eb..35a1471 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -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;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 66+ messages in thread