[PATCH 0/5][RFC] Use per cpuidle_device states

[PATCH 0/5][RFC] Use per cpuidle_device states

[Daniel Lezcano]

Since commit 46bcfad7a819bd17ac4e831b04405152d59784ab,
        cpuidle: Single/Global registration of idle states

we have a single registration for the cpuidle states which makes
sense. But now two new architectures are coming: tegra3 and big.LITTLE.

These architectures have different cpus with different caracteristics
for power saving. High load => powerfull processors, idle => small processors.

That implies different cpu latencies.

This patchset present a simple way to keep the current behavior as introduced
by Deepthi without breaking the drivers and add the possibility to specify a
per cpu states.

Daniel Lezcano (5):
  acpi : move the acpi_idle_driver variable declaration
  acpi : move cpuidle_device field out of the acpi_processor_power
    structure
  cpuidle : add a pointer for cpuidle_state in the cpuidle_device
  cpuidle : use per cpuidle device cpu states
  cpuidle : add cpuidle_register_states function

 drivers/acpi/processor_driver.c    |    2 +-
 drivers/acpi/processor_idle.c      |   25 ++++++++++++++++++-------
 drivers/cpuidle/cpuidle.c          |   24 +++++++++++++++++++++---
 drivers/cpuidle/governors/ladder.c |   18 +++++++++---------
 drivers/cpuidle/governors/menu.c   |    8 ++++----
 drivers/cpuidle/sysfs.c            |    3 +--
 include/acpi/processor.h           |    3 ---
 include/linux/cpuidle.h            |   11 ++++++++---
 8 files changed, 62 insertions(+), 32 deletions(-)

-- 
1.7.5.4

[PATCH 1/5] acpi : move the acpi_idle_driver variable declaration

[Daniel Lezcano]

This variable is only used in the in processor_driver.c.
This patch reduces the scope of the variable by moving it
to this file.

Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/acpi/processor_driver.c |    2 +-
 include/acpi/processor.h        |    1 -
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 7048b97..017b39d 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -113,7 +113,7 @@ static struct acpi_driver acpi_processor_driver = {
 
 DEFINE_PER_CPU(struct acpi_processor *, processors);
 EXPORT_PER_CPU_SYMBOL(processors);
-
+extern struct cpuidle_driver acpi_idle_driver;
 struct acpi_processor_errata errata __read_mostly;
 
 /* --------------------------------------------------------------------------
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index db427fa..8b2c39a 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -332,7 +332,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr,
 			      struct acpi_device *device);
 int acpi_processor_suspend(struct device *dev);
 int acpi_processor_resume(struct device *dev);
-extern struct cpuidle_driver acpi_idle_driver;
 
 /* in processor_thermal.c */
 int acpi_processor_get_limit_info(struct acpi_processor *pr);
-- 
1.7.5.4

Re: [PATCH 1/5] acpi : move the acpi_idle_driver variable declaration

[Deepthi Dharwar]

Hi Daniel,


On 07/25/2012 04:15 PM, Daniel Lezcano wrote:

> This variable is only used in the in processor_driver.c.
> This patch reduces the scope of the variable by moving it
> to this file.
>


This is true after applying your second patch .
Maybe you can check the sequencing of patches in the series.
As a thumb rule one should be able to build the kernel
by applying each and every patch in the series .
There would be a build break after applying this one as
acpi_idle_driver is still used in processor_idle.c by applying
just this patch.

Cheers,
Deepthi

 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/acpi/processor_driver.c |    2 +-
>  include/acpi/processor.h        |    1 -
>  2 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 7048b97..017b39d 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -113,7 +113,7 @@ static struct acpi_driver acpi_processor_driver = {
> 
>  DEFINE_PER_CPU(struct acpi_processor *, processors);
>  EXPORT_PER_CPU_SYMBOL(processors);
> -
> +extern struct cpuidle_driver acpi_idle_driver;
>  struct acpi_processor_errata errata __read_mostly;
> 
>  /* --------------------------------------------------------------------------
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index db427fa..8b2c39a 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -332,7 +332,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr,
>  			      struct acpi_device *device);
>  int acpi_processor_suspend(struct device *dev);
>  int acpi_processor_resume(struct device *dev);
> -extern struct cpuidle_driver acpi_idle_driver;
> 
>  /* in processor_thermal.c */
>  int acpi_processor_get_limit_info(struct acpi_processor *pr);

Re: [PATCH 1/5] acpi : move the acpi_idle_driver variable declaration

[Daniel Lezcano]

On 07/27/2012 07:46 AM, Deepthi Dharwar wrote:
> Hi Daniel,
> 
> 
> On 07/25/2012 04:15 PM, Daniel Lezcano wrote:
> 
>> This variable is only used in the in processor_driver.c.
>> This patch reduces the scope of the variable by moving it
>> to this file.
>>
> 
> 
> This is true after applying your second patch .
> Maybe you can check the sequencing of patches in the series.
> As a thumb rule one should be able to build the kernel
> by applying each and every patch in the series .

Thanks Deepthi. I think I already git bisect tested the patchset.
This patch does not break the build, the variable is defined in the
processor_driver.c file.



> There would be a build break after applying this one as
> acpi_idle_driver is still used in processor_idle.c by applying
> just this patch.
> 
> Cheers,
> Deepthi
> 
>  
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/acpi/processor_driver.c |    2 +-
>>  include/acpi/processor.h        |    1 -
>>  2 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>> index 7048b97..017b39d 100644
>> --- a/drivers/acpi/processor_driver.c
>> +++ b/drivers/acpi/processor_driver.c
>> @@ -113,7 +113,7 @@ static struct acpi_driver acpi_processor_driver = {
>>
>>  DEFINE_PER_CPU(struct acpi_processor *, processors);
>>  EXPORT_PER_CPU_SYMBOL(processors);
>> -
>> +extern struct cpuidle_driver acpi_idle_driver;
>>  struct acpi_processor_errata errata __read_mostly;
>>
>>  /* --------------------------------------------------------------------------
>> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
>> index db427fa..8b2c39a 100644
>> --- a/include/acpi/processor.h
>> +++ b/include/acpi/processor.h
>> @@ -332,7 +332,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr,
>>  			      struct acpi_device *device);
>>  int acpi_processor_suspend(struct device *dev);
>>  int acpi_processor_resume(struct device *dev);
>> -extern struct cpuidle_driver acpi_idle_driver;
>>
>>  /* in processor_thermal.c */
>>  int acpi_processor_get_limit_info(struct acpi_processor *pr);
> 
> 


-- 
 <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-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] acpi : move cpuidle_device field out of the acpi_processor_power structure

[Daniel Lezcano]

Currently we have the cpuidle_device field in the acpi_processor_power structure.
This adds a dependency in processor.h for cpuidle.h.

In order to be consistent with the rest of the drivers and for the per cpu states
coming right after this patch, this one move out of the acpi_processor_power
structure the cpuidle_device field.

Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/acpi/processor_idle.c |   25 ++++++++++++++++++-------
 include/acpi/processor.h      |    2 --
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 90582fb..62cc80f 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -79,6 +79,8 @@ module_param(bm_check_disable, uint, 0000);
 static unsigned int latency_factor __read_mostly = 2;
 module_param(latency_factor, uint, 0644);
 
+static DEFINE_PER_CPU(struct cpuidle_device, acpi_cpuidle_device);
+
 static int disabled_by_idle_boot_param(void)
 {
 	return boot_option_idle_override == IDLE_POLL ||
@@ -998,7 +1000,7 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr)
 	int i, count = CPUIDLE_DRIVER_STATE_START;
 	struct acpi_processor_cx *cx;
 	struct cpuidle_state_usage *state_usage;
-	struct cpuidle_device *dev = &pr->power.dev;
+	struct cpuidle_device *dev = &per_cpu(acpi_cpuidle_device, pr->id);
 
 	if (!pr->flags.power_setup_done)
 		return -EINVAL;
@@ -1130,6 +1132,7 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
 int acpi_processor_hotplug(struct acpi_processor *pr)
 {
 	int ret = 0;
+	struct cpuidle_device *dev = &per_cpu(acpi_cpuidle_device, pr->id);
 
 	if (disabled_by_idle_boot_param())
 		return 0;
@@ -1145,11 +1148,11 @@ int acpi_processor_hotplug(struct acpi_processor *pr)
 		return -ENODEV;
 
 	cpuidle_pause_and_lock();
-	cpuidle_disable_device(&pr->power.dev);
+	cpuidle_disable_device(dev);
 	acpi_processor_get_power_info(pr);
 	if (pr->flags.power) {
 		acpi_processor_setup_cpuidle_cx(pr);
-		ret = cpuidle_enable_device(&pr->power.dev);
+		ret = cpuidle_enable_device(dev);
 	}
 	cpuidle_resume_and_unlock();
 
@@ -1160,6 +1163,7 @@ int acpi_processor_cst_has_changed(struct acpi_processor *pr)
 {
 	int cpu;
 	struct acpi_processor *_pr;
+	struct cpuidle_device *dev;
 
 	if (disabled_by_idle_boot_param())
 		return 0;
@@ -1190,7 +1194,8 @@ int acpi_processor_cst_has_changed(struct acpi_processor *pr)
 			_pr = per_cpu(processors, cpu);
 			if (!_pr || !_pr->flags.power_setup_done)
 				continue;
-			cpuidle_disable_device(&_pr->power.dev);
+			dev = &per_cpu(acpi_cpuidle_device, cpu);
+			cpuidle_disable_device(dev);
 		}
 
 		/* Populate Updated C-state information */
@@ -1204,7 +1209,8 @@ int acpi_processor_cst_has_changed(struct acpi_processor *pr)
 			acpi_processor_get_power_info(_pr);
 			if (_pr->flags.power) {
 				acpi_processor_setup_cpuidle_cx(_pr);
-				cpuidle_enable_device(&_pr->power.dev);
+				dev = &per_cpu(acpi_cpuidle_device, cpu);
+				cpuidle_enable_device(dev);
 			}
 		}
 		put_online_cpus();
@@ -1221,6 +1227,7 @@ int __cpuinit acpi_processor_power_init(struct acpi_processor *pr,
 {
 	acpi_status status = 0;
 	int retval;
+	struct cpuidle_device *dev;
 	static int first_run;
 
 	if (disabled_by_idle_boot_param())
@@ -1270,7 +1277,9 @@ int __cpuinit acpi_processor_power_init(struct acpi_processor *pr,
 		 * must already be registered before registering device
 		 */
 		acpi_processor_setup_cpuidle_cx(pr);
-		retval = cpuidle_register_device(&pr->power.dev);
+
+		dev = &per_cpu(acpi_cpuidle_device, pr->id);
+		retval = cpuidle_register_device(dev);
 		if (retval) {
 			if (acpi_processor_registered == 0)
 				cpuidle_unregister_driver(&acpi_idle_driver);
@@ -1284,11 +1293,13 @@ int __cpuinit acpi_processor_power_init(struct acpi_processor *pr,
 int acpi_processor_power_exit(struct acpi_processor *pr,
 			      struct acpi_device *device)
 {
+	struct cpuidle_device *dev = &per_cpu(acpi_cpuidle_device, pr->id);
+
 	if (disabled_by_idle_boot_param())
 		return 0;
 
 	if (pr->flags.power) {
-		cpuidle_unregister_device(&pr->power.dev);
+		cpuidle_unregister_device(dev);
 		acpi_processor_registered--;
 		if (acpi_processor_registered == 0)
 			cpuidle_unregister_driver(&acpi_idle_driver);
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 8b2c39a..4d98ec8 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -3,7 +3,6 @@
 
 #include <linux/kernel.h>
 #include <linux/cpu.h>
-#include <linux/cpuidle.h>
 #include <linux/thermal.h>
 #include <asm/acpi.h>
 
@@ -64,7 +63,6 @@ struct acpi_processor_cx {
 };
 
 struct acpi_processor_power {
-	struct cpuidle_device dev;
 	struct acpi_processor_cx *state;
 	unsigned long bm_check_timestamp;
 	u32 default_state;
-- 
1.7.5.4

[PATCH 3/5][RFC] cpuidle : add a pointer for cpuidle_state in the cpuidle_device

[Daniel Lezcano]

This patch adds a pointer to the cpuidle_state array in the cpuidle_device
structure. When the cpuidle_device is initialized, the pointer is assigned
from the driver's cpuidle states array.

Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/cpuidle/cpuidle.c |    1 +
 include/linux/cpuidle.h   |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index d6a533e..42b1a8a 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -305,6 +305,7 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
 		return -EIO;
 	if (!dev->state_count)
 		dev->state_count = drv->state_count;
+	dev->states = drv->states;
 
 	if (dev->registered == 0) {
 		ret = __cpuidle_register_device(dev);
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 89dcd30..40a04a1 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -95,6 +95,7 @@ struct cpuidle_device {
 
 	int			last_residency;
 	int			state_count;
+	struct cpuidle_state    *states;
 	struct cpuidle_state_usage	states_usage[CPUIDLE_STATE_MAX];
 	struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX];
 
-- 
1.7.5.4

Re: [PATCH 3/5][RFC] cpuidle : add a pointer for cpuidle_state in the cpuidle_device

[Shilimkar, Santosh]

On Wed, Jul 25, 2012 at 12:46 PM, Daniel Lezcano
<daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
> This patch adds a pointer to the cpuidle_state array in the cpuidle_device
> structure. When the cpuidle_device is initialized, the pointer is assigned
> from the driver's cpuidle states array.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
Would be good to also mention the intention behind this change
in the change-log. The next patch make it clear but for this change
too, it should be added.

Feel free to add my ack if you need one.

Acked-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>

Re: [PATCH 3/5][RFC] cpuidle : add a pointer for cpuidle_state in the cpuidle_device

[Peter De Schrijver]

On Wed, Jul 25, 2012 at 12:46:00PM +0200, Daniel Lezcano wrote:
> This patch adds a pointer to the cpuidle_state array in the cpuidle_device
> structure. When the cpuidle_device is initialized, the pointer is assigned
> from the driver's cpuidle states array.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/cpuidle/cpuidle.c |    1 +
>  include/linux/cpuidle.h   |    1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index d6a533e..42b1a8a 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -305,6 +305,7 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
>  		return -EIO;
>  	if (!dev->state_count)
>  		dev->state_count = drv->state_count;
> +	dev->states = drv->states;

This should only be done when dev->state_count == 0 no?

Cheers,

Peter.

Re: [PATCH 3/5][RFC] cpuidle : add a pointer for cpuidle_state in the cpuidle_device

[Daniel Lezcano]

On 09/03/2012 05:16 PM, Peter De Schrijver wrote:
> On Wed, Jul 25, 2012 at 12:46:00PM +0200, Daniel Lezcano wrote:
>> This patch adds a pointer to the cpuidle_state array in the cpuidle_device
>> structure. When the cpuidle_device is initialized, the pointer is assigned
>> from the driver's cpuidle states array.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/cpuidle/cpuidle.c |    1 +
>>  include/linux/cpuidle.h   |    1 +
>>  2 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index d6a533e..42b1a8a 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -305,6 +305,7 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
>>  		return -EIO;
>>  	if (!dev->state_count)
>>  		dev->state_count = drv->state_count;
>> +	dev->states = drv->states;
> 
> This should only be done when dev->state_count == 0 no?

Right. In acpi/processor_idle.c, the state count for the device is
initialized and I overwritten this value in all the cases, but this not
right. I removed the initialization in acpi because it is pointless and
moved this line to do the 'if' block as you mentioned.

Thanks
  -- 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-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5][RFC] cpuidle : use per cpuidle device cpu states

[Daniel Lezcano]

We have the cpuidle states pointer stored in the cpuidle device
structure. This patch use this pointer instead of the driver's one.

Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/cpuidle/cpuidle.c          |    6 +++---
 drivers/cpuidle/governors/ladder.c |   18 +++++++++---------
 drivers/cpuidle/governors/menu.c   |    8 ++++----
 drivers/cpuidle/sysfs.c            |    3 +--
 4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 42b1a8a..199878a 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -45,7 +45,7 @@ static int __cpuidle_register_device(struct cpuidle_device *dev);
 static inline int cpuidle_enter(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv, int index)
 {
-	struct cpuidle_state *target_state = &drv->states[index];
+	struct cpuidle_state *target_state = &dev->states[index];
 	return target_state->enter(dev, drv, index);
 }
 
@@ -77,7 +77,7 @@ int cpuidle_play_dead(void)
 
 	/* Find lowest-power state that supports long-term idle */
 	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
-		struct cpuidle_state *s = &drv->states[i];
+		struct cpuidle_state *s = &dev->states[i];
 
 		if (s->power_usage < power_usage && s->enter_dead) {
 			power_usage = s->power_usage;
@@ -86,7 +86,7 @@ int cpuidle_play_dead(void)
 	}
 
 	if (dead_state != -1)
-		return drv->states[dead_state].enter_dead(dev, dead_state);
+		return dev->states[dead_state].enter_dead(dev, dead_state);
 
 	return -ENODEV;
 }
diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
index b6a09ea..b50cae3 100644
--- a/drivers/cpuidle/governors/ladder.c
+++ b/drivers/cpuidle/governors/ladder.c
@@ -79,17 +79,17 @@ static int ladder_select_state(struct cpuidle_driver *drv,
 
 	last_state = &ldev->states[last_idx];
 
-	if (drv->states[last_idx].flags & CPUIDLE_FLAG_TIME_VALID) {
+	if (dev->states[last_idx].flags & CPUIDLE_FLAG_TIME_VALID) {
 		last_residency = cpuidle_get_last_residency(dev) - \
-					 drv->states[last_idx].exit_latency;
+					 dev->states[last_idx].exit_latency;
 	}
 	else
 		last_residency = last_state->threshold.promotion_time + 1;
 
 	/* consider promotion */
-	if (last_idx < drv->state_count - 1 &&
+	if (last_idx < dev->state_count - 1 &&
 	    last_residency > last_state->threshold.promotion_time &&
-	    drv->states[last_idx + 1].exit_latency <= latency_req) {
+	    dev->states[last_idx + 1].exit_latency <= latency_req) {
 		last_state->stats.promotion_count++;
 		last_state->stats.demotion_count = 0;
 		if (last_state->stats.promotion_count >= last_state->threshold.promotion_count) {
@@ -100,11 +100,11 @@ static int ladder_select_state(struct cpuidle_driver *drv,
 
 	/* consider demotion */
 	if (last_idx > CPUIDLE_DRIVER_STATE_START &&
-	    drv->states[last_idx].exit_latency > latency_req) {
+	    dev->states[last_idx].exit_latency > latency_req) {
 		int i;
 
 		for (i = last_idx - 1; i > CPUIDLE_DRIVER_STATE_START; i--) {
-			if (drv->states[i].exit_latency <= latency_req)
+			if (dev->states[i].exit_latency <= latency_req)
 				break;
 		}
 		ladder_do_selection(ldev, last_idx, i);
@@ -140,8 +140,8 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
 
 	ldev->last_state_idx = CPUIDLE_DRIVER_STATE_START;
 
-	for (i = 0; i < drv->state_count; i++) {
-		state = &drv->states[i];
+	for (i = 0; i < dev->state_count; i++) {
+		state = &dev->states[i];
 		lstate = &ldev->states[i];
 
 		lstate->stats.promotion_count = 0;
@@ -150,7 +150,7 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
 		lstate->threshold.promotion_count = PROMOTION_COUNT;
 		lstate->threshold.demotion_count = DEMOTION_COUNT;
 
-		if (i < drv->state_count - 1)
+		if (i < dev->state_count - 1)
 			lstate->threshold.promotion_time = state->exit_latency;
 		if (i > 0)
 			lstate->threshold.demotion_time = state->exit_latency;
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 5b1f2c3..3a9a9bd 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -281,7 +281,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
 	 * unless the timer is happening really really soon.
 	 */
 	if (data->expected_us > 5 &&
-	    !drv->states[CPUIDLE_DRIVER_STATE_START].disabled &&
+	    !dev->states[CPUIDLE_DRIVER_STATE_START].disabled &&
 		dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable == 0)
 		data->last_state_idx = CPUIDLE_DRIVER_STATE_START;
 
@@ -289,8 +289,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
 	 * Find the idle state with the lowest power while satisfying
 	 * our constraints.
 	 */
-	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
-		struct cpuidle_state *s = &drv->states[i];
+	for (i = CPUIDLE_DRIVER_STATE_START; i < dev->state_count; i++) {
+		struct cpuidle_state *s = &dev->states[i];
 		struct cpuidle_state_usage *su = &dev->states_usage[i];
 
 		if (s->disabled || su->disable)
@@ -338,7 +338,7 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev)
 	struct menu_device *data = &__get_cpu_var(menu_devices);
 	int last_idx = data->last_state_idx;
 	unsigned int last_idle_us = cpuidle_get_last_residency(dev);
-	struct cpuidle_state *target = &drv->states[last_idx];
+	struct cpuidle_state *target = &dev->states[last_idx];
 	unsigned int measured_us;
 	u64 new_factor;
 
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 5f809e3..6b20929 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -363,14 +363,13 @@ int cpuidle_add_state_sysfs(struct cpuidle_device *device)
 {
 	int i, ret = -ENOMEM;
 	struct cpuidle_state_kobj *kobj;
-	struct cpuidle_driver *drv = cpuidle_get_driver();
 
 	/* state statistics */
 	for (i = 0; i < device->state_count; i++) {
 		kobj = kzalloc(sizeof(struct cpuidle_state_kobj), GFP_KERNEL);
 		if (!kobj)
 			goto error_state;
-		kobj->state = &drv->states[i];
+		kobj->state = &device->states[i];
 		kobj->state_usage = &device->states_usage[i];
 		init_completion(&kobj->kobj_unregister);
 
-- 
1.7.5.4

Re: [PATCH 4/5][RFC] cpuidle : use per cpuidle device cpu states

[Shilimkar, Santosh]

On Wed, Jul 25, 2012 at 12:46 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> We have the cpuidle states pointer stored in the cpuidle device
> structure. This patch use this pointer instead of the driver's one.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
Sorry for another comment on change log. Would be give a reference
about the commit which removed the feature(was present before), and
also some examples like say big.LITTLE, Tegra arch which needs the
per CPU latency tables.

Regards
Santosh

[PATCH 5/5][RFC] cpuidle : add cpuidle_register_states function

[Daniel Lezcano]

The tegra3 and big.LITTLE architecture have different cpu latencies.
This API allows to specify a different cpu latency for a specific cpu.

With the previous patches, we use the per cpuidle device states pointer,
this function overrides this pointer.

Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/cpuidle/cpuidle.c |   17 +++++++++++++++++
 include/linux/cpuidle.h   |   10 +++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 199878a..3b21b68 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -456,6 +456,23 @@ void cpuidle_unregister_device(struct cpuidle_device *dev)
 
 EXPORT_SYMBOL_GPL(cpuidle_unregister_device);
 
+int cpuidle_register_states(struct cpuidle_device *dev,
+			    struct cpuidle_state *states,
+			    int state_count)
+{
+	if (!dev || !states)
+		return -EINVAL;
+
+	if (state_count <= 0)
+		return -EINVAL;
+
+	dev->states = states;
+	dev->state_count = state_count;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cpuidle_register_state);
+
 #ifdef CONFIG_SMP
 
 static void smp_callback(void *v)
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 40a04a1..425200d 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -144,7 +144,9 @@ extern void cpuidle_driver_unref(void);
 extern void cpuidle_unregister_driver(struct cpuidle_driver *drv);
 extern int cpuidle_register_device(struct cpuidle_device *dev);
 extern void cpuidle_unregister_device(struct cpuidle_device *dev);
-
+extern int cpuidle_register_states(struct cpuidle_device *dev,
+				   struct cpuidle_state *states,
+				   int state_count);
 extern void cpuidle_pause_and_lock(void);
 extern void cpuidle_resume_and_unlock(void);
 extern void cpuidle_pause(void);
@@ -156,7 +158,6 @@ extern int cpuidle_wrap_enter(struct cpuidle_device *dev,
 				int (*enter)(struct cpuidle_device *dev,
 					struct cpuidle_driver *drv, int index));
 extern int cpuidle_play_dead(void);
-
 #else
 static inline void disable_cpuidle(void) { }
 static inline int cpuidle_idle_call(void) { return -ENODEV; }
@@ -169,7 +170,10 @@ static inline void cpuidle_unregister_driver(struct cpuidle_driver *drv) { }
 static inline int cpuidle_register_device(struct cpuidle_device *dev)
 {return -ENODEV; }
 static inline void cpuidle_unregister_device(struct cpuidle_device *dev) { }
-
+static inline  int cpuidle_register_states(struct cpuidle_device *dev,
+					   struct cpuidle_state *states,
+					   int state_count)
+{ return -ENODEV; }
 static inline void cpuidle_pause_and_lock(void) { }
 static inline void cpuidle_resume_and_unlock(void) { }
 static inline void cpuidle_pause(void) { }
-- 
1.7.5.4

Re: [PATCH 5/5][RFC] cpuidle : add cpuidle_register_states function

[Deepthi Dharwar]

On 07/25/2012 04:16 PM, Daniel Lezcano wrote:

> The tegra3 and big.LITTLE architecture have different cpu latencies.
> This API allows to specify a different cpu latency for a specific cpu.
> 
> With the previous patches, we use the per cpuidle device states pointer,
> this function overrides this pointer.
> 


May be add some more explanation and pointers to previous discussions,
as stated on the cover in the patch series.


> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>


Acked-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>

> ---
>  drivers/cpuidle/cpuidle.c |   17 +++++++++++++++++
>  include/linux/cpuidle.h   |   10 +++++++---
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 199878a..3b21b68 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -456,6 +456,23 @@ void cpuidle_unregister_device(struct cpuidle_device *dev)
> 
>  EXPORT_SYMBOL_GPL(cpuidle_unregister_device);
> 
> +int cpuidle_register_states(struct cpuidle_device *dev,
> +			    struct cpuidle_state *states,
> +			    int state_count)
> +{
> +	if (!dev || !states)
> +		return -EINVAL;
> +
> +	if (state_count <= 0)
> +		return -EINVAL;
> +
> +	dev->states = states;
> +	dev->state_count = state_count;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(cpuidle_register_state);
> +
>  #ifdef CONFIG_SMP
> 
>  static void smp_callback(void *v)
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 40a04a1..425200d 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -144,7 +144,9 @@ extern void cpuidle_driver_unref(void);
>  extern void cpuidle_unregister_driver(struct cpuidle_driver *drv);
>  extern int cpuidle_register_device(struct cpuidle_device *dev);
>  extern void cpuidle_unregister_device(struct cpuidle_device *dev);
> -
> +extern int cpuidle_register_states(struct cpuidle_device *dev,
> +				   struct cpuidle_state *states,
> +				   int state_count);
>  extern void cpuidle_pause_and_lock(void);
>  extern void cpuidle_resume_and_unlock(void);
>  extern void cpuidle_pause(void);
> @@ -156,7 +158,6 @@ extern int cpuidle_wrap_enter(struct cpuidle_device *dev,
>  				int (*enter)(struct cpuidle_device *dev,
>  					struct cpuidle_driver *drv, int index));
>  extern int cpuidle_play_dead(void);
> -
>  #else
>  static inline void disable_cpuidle(void) { }
>  static inline int cpuidle_idle_call(void) { return -ENODEV; }
> @@ -169,7 +170,10 @@ static inline void cpuidle_unregister_driver(struct cpuidle_driver *drv) { }
>  static inline int cpuidle_register_device(struct cpuidle_device *dev)
>  {return -ENODEV; }
>  static inline void cpuidle_unregister_device(struct cpuidle_device *dev) { }
> -
> +static inline  int cpuidle_register_states(struct cpuidle_device *dev,
> +					   struct cpuidle_state *states,
> +					   int state_count)
> +{ return -ENODEV; }
>  static inline void cpuidle_pause_and_lock(void) { }
>  static inline void cpuidle_resume_and_unlock(void) { }
>  static inline void cpuidle_pause(void) { }

Re: [PATCH 5/5][RFC] cpuidle : add cpuidle_register_states function

[Lorenzo Pieralisi]

Hi Daniel,

thanks for this patchset.

On Wed, Jul 25, 2012 at 11:46:02AM +0100, Daniel Lezcano wrote:
> The tegra3 and big.LITTLE architecture have different cpu latencies.
> This API allows to specify a different cpu latency for a specific cpu.
> 
> With the previous patches, we use the per cpuidle device states pointer,
> this function overrides this pointer.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/cpuidle/cpuidle.c |   17 +++++++++++++++++
>  include/linux/cpuidle.h   |   10 +++++++---
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 199878a..3b21b68 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -456,6 +456,23 @@ void cpuidle_unregister_device(struct cpuidle_device *dev)
>  
>  EXPORT_SYMBOL_GPL(cpuidle_unregister_device);
>  
> +int cpuidle_register_states(struct cpuidle_device *dev,
> +			    struct cpuidle_state *states,
> +			    int state_count)
> +{
> +	if (!dev || !states)
> +		return -EINVAL;
> +
> +	if (state_count <= 0)
> +		return -EINVAL;
> +
> +	dev->states = states;
> +	dev->state_count = state_count;

Is this function supposed to be called after cpuidle_device registration ?
I think so since at registration time the dev->states pointers are all
initialized to point to the driver state array, which is global and not
really what we want.

Unless this function is called on the cpu that requires swapping the state
pointer, I think it is unsafe to register a different state pointer
without a minimal level of locking (or disabling idle and renabling idle)
since the update of dev->states and dev->state_count is not atomic.
Maybe it is implicit but it should be documented somehow to define
cpuidle_register_states(...) proper usage.

Thanks,
Lorenzo

Re: [PATCH 5/5][RFC] cpuidle : add cpuidle_register_states function

[Daniel Lezcano]

On 08/10/2012 07:17 PM, Lorenzo Pieralisi wrote:
> Hi Daniel,
> 
> thanks for this patchset.
> 
> On Wed, Jul 25, 2012 at 11:46:02AM +0100, Daniel Lezcano wrote:
>> The tegra3 and big.LITTLE architecture have different cpu latencies.
>> This API allows to specify a different cpu latency for a specific cpu.
>>
>> With the previous patches, we use the per cpuidle device states pointer,
>> this function overrides this pointer.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/cpuidle/cpuidle.c |   17 +++++++++++++++++
>>  include/linux/cpuidle.h   |   10 +++++++---
>>  2 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 199878a..3b21b68 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -456,6 +456,23 @@ void cpuidle_unregister_device(struct cpuidle_device *dev)
>>  
>>  EXPORT_SYMBOL_GPL(cpuidle_unregister_device);
>>  
>> +int cpuidle_register_states(struct cpuidle_device *dev,
>> +			    struct cpuidle_state *states,
>> +			    int state_count)
>> +{
>> +	if (!dev || !states)
>> +		return -EINVAL;
>> +
>> +	if (state_count <= 0)
>> +		return -EINVAL;
>> +
>> +	dev->states = states;
>> +	dev->state_count = state_count;
> 
> Is this function supposed to be called after cpuidle_device registration ?
> I think so since at registration time the dev->states pointers are all
> initialized to point to the driver state array, which is global and not
> really what we want.
> 
> Unless this function is called on the cpu that requires swapping the state
> pointer, I think it is unsafe to register a different state pointer
> without a minimal level of locking (or disabling idle and renabling idle)
> since the update of dev->states and dev->state_count is not atomic.
> Maybe it is implicit but it should be documented somehow to define
> cpuidle_register_states(...) proper usage.

Hi Lorenzo,

Yes, you are right. I will add the cpuidle lock.

Thanks !

  -- 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-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5][RFC] cpuidle : add cpuidle_register_states function

[Peter De Schrijver]

On Wed, Jul 25, 2012 at 12:46:02PM +0200, Daniel Lezcano wrote:
> The tegra3 and big.LITTLE architecture have different cpu latencies.
> This API allows to specify a different cpu latency for a specific cpu.
> 
> With the previous patches, we use the per cpuidle device states pointer,
> this function overrides this pointer.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/cpuidle/cpuidle.c |   17 +++++++++++++++++
>  include/linux/cpuidle.h   |   10 +++++++---
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 199878a..3b21b68 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -456,6 +456,23 @@ void cpuidle_unregister_device(struct cpuidle_device *dev)
>  
>  EXPORT_SYMBOL_GPL(cpuidle_unregister_device);
>  
> +int cpuidle_register_states(struct cpuidle_device *dev,
> +			    struct cpuidle_state *states,
> +			    int state_count)
> +{
> +	if (!dev || !states)
> +		return -EINVAL;
> +
> +	if (state_count <= 0)
> +		return -EINVAL;
> +
> +	dev->states = states;
> +	dev->state_count = state_count;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(cpuidle_register_state);
> +
>  #ifdef CONFIG_SMP
>  

Looks good... apart from the fact that the function definition says
cpuidle_register_stateS and the exported symbol is cpuidle_register_state...

Cheers,

Peter.

Re: [PATCH 5/5][RFC] cpuidle : add cpuidle_register_states function

[Daniel Lezcano]

On 09/03/2012 03:22 PM, Peter De Schrijver wrote:
> On Wed, Jul 25, 2012 at 12:46:02PM +0200, Daniel Lezcano wrote:
>> The tegra3 and big.LITTLE architecture have different cpu latencies.
>> This API allows to specify a different cpu latency for a specific cpu.
>>
>> With the previous patches, we use the per cpuidle device states pointer,
>> this function overrides this pointer.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/cpuidle/cpuidle.c |   17 +++++++++++++++++
>>  include/linux/cpuidle.h   |   10 +++++++---
>>  2 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 199878a..3b21b68 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -456,6 +456,23 @@ void cpuidle_unregister_device(struct cpuidle_device *dev)
>>  
>>  EXPORT_SYMBOL_GPL(cpuidle_unregister_device);
>>  
>> +int cpuidle_register_states(struct cpuidle_device *dev,
>> +			    struct cpuidle_state *states,
>> +			    int state_count)
>> +{
>> +	if (!dev || !states)
>> +		return -EINVAL;
>> +
>> +	if (state_count <= 0)
>> +		return -EINVAL;
>> +
>> +	dev->states = states;
>> +	dev->state_count = state_count;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(cpuidle_register_state);
>> +
>>  #ifdef CONFIG_SMP
>>  
> 
> Looks good... apart from the fact that the function definition says
> cpuidle_register_stateS and the exported symbol is cpuidle_register_state...


Ok, fixed. Thanks !

  -- 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-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5][RFC] Use per cpuidle_device states

[Peter De Schrijver]

On Wed, Jul 25, 2012 at 12:45:57PM +0200, Daniel Lezcano wrote:
> Since commit 46bcfad7a819bd17ac4e831b04405152d59784ab,
>         cpuidle: Single/Global registration of idle states
> 
> we have a single registration for the cpuidle states which makes
> sense. But now two new architectures are coming: tegra3 and big.LITTLE.
> 
> These architectures have different cpus with different caracteristics
> for power saving. High load => powerfull processors, idle => small processors.
> 
> That implies different cpu latencies.
> 
> This patchset present a simple way to keep the current behavior as introduced
> by Deepthi without breaking the drivers and add the possibility to specify a
> per cpu states.
> 

Looks good! I will do some tests with this next week.

Cheers,

Peter.

Re: [PATCH 0/5][RFC] Use per cpuidle_device states

[Daniel Lezcano]

On 07/27/2012 01:28 PM, Peter De Schrijver wrote:
> On Wed, Jul 25, 2012 at 12:45:57PM +0200, Daniel Lezcano wrote:
>> Since commit 46bcfad7a819bd17ac4e831b04405152d59784ab,
>>         cpuidle: Single/Global registration of idle states
>>
>> we have a single registration for the cpuidle states which makes
>> sense. But now two new architectures are coming: tegra3 and big.LITTLE.
>>
>> These architectures have different cpus with different caracteristics
>> for power saving. High load => powerfull processors, idle => small processors.
>>
>> That implies different cpu latencies.
>>
>> This patchset present a simple way to keep the current behavior as introduced
>> by Deepthi without breaking the drivers and add the possibility to specify a
>> per cpu states.
>>
> Looks good! I will do some tests with this next week.

Hi Peter,

shall I consider adding your acked-by ?

Did you have time to test the patchset ?

Thanks
-- 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



_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 0/5][RFC] Use per cpuidle_device states

[Peter De Schrijver]

On Fri, Aug 31, 2012 at 11:19:15PM +0200, Daniel Lezcano wrote:
> On 07/27/2012 01:28 PM, Peter De Schrijver wrote:
> > On Wed, Jul 25, 2012 at 12:45:57PM +0200, Daniel Lezcano wrote:
> >> Since commit 46bcfad7a819bd17ac4e831b04405152d59784ab,
> >>         cpuidle: Single/Global registration of idle states
> >>
> >> we have a single registration for the cpuidle states which makes
> >> sense. But now two new architectures are coming: tegra3 and big.LITTLE.
> >>
> >> These architectures have different cpus with different caracteristics
> >> for power saving. High load => powerfull processors, idle => small processors.
> >>
> >> That implies different cpu latencies.
> >>
> >> This patchset present a simple way to keep the current behavior as introduced
> >> by Deepthi without breaking the drivers and add the possibility to specify a
> >> per cpu states.
> >>
> > Looks good! I will do some tests with this next week.
> 
> Hi Peter,
> 
> shall I consider adding your acked-by ?
> 

You can. I don't see any problem with the code as such (besides the race).

> Did you have time to test the patchset ?
> 

I haven't had time no :( I was on holiday most of august and had some other
things to take care of last week :(. Working on it now.

Cheers,

Peter.

Re: [PATCH 0/5][RFC] Use per cpuidle_device states

[Peter De Schrijver]

On Mon, Sep 03, 2012 at 01:05:11PM +0200, Peter De Schrijver wrote:
> On Fri, Aug 31, 2012 at 11:19:15PM +0200, Daniel Lezcano wrote:
> > On 07/27/2012 01:28 PM, Peter De Schrijver wrote:
> > > On Wed, Jul 25, 2012 at 12:45:57PM +0200, Daniel Lezcano wrote:
> > >> Since commit 46bcfad7a819bd17ac4e831b04405152d59784ab,
> > >>         cpuidle: Single/Global registration of idle states
> > >>
> > >> we have a single registration for the cpuidle states which makes
> > >> sense. But now two new architectures are coming: tegra3 and big.LITTLE.
> > >>
> > >> These architectures have different cpus with different caracteristics
> > >> for power saving. High load => powerfull processors, idle => small processors.
> > >>
> > >> That implies different cpu latencies.
> > >>
> > >> This patchset present a simple way to keep the current behavior as introduced
> > >> by Deepthi without breaking the drivers and add the possibility to specify a
> > >> per cpu states.
> > >>
> > > Looks good! I will do some tests with this next week.
> > 
> > Hi Peter,
> > 
> > shall I consider adding your acked-by ?
> > 
> 
> You can. I don't see any problem with the code as such (besides the race).
> 
> > Did you have time to test the patchset ?
> > 
> 
> I haven't had time no :( I was on holiday most of august and had some other
> things to take care of last week :(. Working on it now.
> 

I did some preliminary testing, besides the issues I posted earlier. It looks
all good here.

Cheers,

Peter.