* [PATCH 01/15] timer: move enum definition out of ifdef section
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-26 4:36 ` Santosh Shilimkar
2013-03-25 17:55 ` [PATCH 02/15] cpuidle: initialize the broadcast timer framework Daniel Lezcano
` (14 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
The next patch will setup automatically the broadcast timer for
the different cpuidle driver when one idle state stops its timer.
This will be part of the generic code.
But some ARM boards, like s3c64xx, uses cpuidle but without the
CONFIG_GENERIC_CLOCKEVENTS_BUILD set. Hence the cpuidle framework
will be compiled with the code supposed to be generic, that is
with clockevents_notify and the different enum, but will fail to
compile because the enum is not defined.
Also the function clockevents_notify is a noop macro, this is fine
except the usual code is:
int cpu = smp_processor_id();
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
and that raises a warning telling the variable 'cpu' is not used.
Move the clock_event_nofitiers enum definition out of the
CONFIG_GENERIC_CLOCKEVENTS_BUILD section to prevent a compilation
error when these are used in the code.
Change the clockevents_notify macro to a static inline noop function
to prevent a compilation warning.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
include/linux/clockchips.h | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 6634652..ba661e9 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -8,15 +8,6 @@
#ifndef _LINUX_CLOCKCHIPS_H
#define _LINUX_CLOCKCHIPS_H
-#ifdef CONFIG_GENERIC_CLOCKEVENTS_BUILD
-
-#include <linux/clocksource.h>
-#include <linux/cpumask.h>
-#include <linux/ktime.h>
-#include <linux/notifier.h>
-
-struct clock_event_device;
-
/* Clock event mode commands */
enum clock_event_mode {
CLOCK_EVT_MODE_UNUSED = 0,
@@ -40,6 +31,15 @@ enum clock_event_nofitiers {
CLOCK_EVT_NOTIFY_CPU_DEAD,
};
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BUILD
+
+#include <linux/clocksource.h>
+#include <linux/cpumask.h>
+#include <linux/ktime.h>
+#include <linux/notifier.h>
+
+struct clock_event_device;
+
/*
* Clock event features
*/
@@ -173,7 +173,7 @@ extern int tick_receive_broadcast(void);
#ifdef CONFIG_GENERIC_CLOCKEVENTS
extern void clockevents_notify(unsigned long reason, void *arg);
#else
-# define clockevents_notify(reason, arg) do { } while (0)
+static inline void clockevents_notify(unsigned long reason, void *arg) {}
#endif
#else /* CONFIG_GENERIC_CLOCKEVENTS_BUILD */
@@ -181,7 +181,7 @@ extern void clockevents_notify(unsigned long reason, void *arg);
static inline void clockevents_suspend(void) {}
static inline void clockevents_resume(void) {}
-#define clockevents_notify(reason, arg) do { } while (0)
+static inline void clockevents_notify(unsigned long reason, void *arg) {}
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 01/15] timer: move enum definition out of ifdef section
2013-03-25 17:55 ` [PATCH 01/15] timer: move enum definition out of ifdef section Daniel Lezcano
@ 2013-03-26 4:36 ` Santosh Shilimkar
2013-03-26 9:55 ` Daniel Lezcano
0 siblings, 1 reply; 45+ messages in thread
From: Santosh Shilimkar @ 2013-03-26 4:36 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 25 March 2013 11:25 PM, Daniel Lezcano wrote:
> The next patch will setup automatically the broadcast timer for
> the different cpuidle driver when one idle state stops its timer.
> This will be part of the generic code.
>
> But some ARM boards, like s3c64xx, uses cpuidle but without the
> CONFIG_GENERIC_CLOCKEVENTS_BUILD set. Hence the cpuidle framework
> will be compiled with the code supposed to be generic, that is
> with clockevents_notify and the different enum, but will fail to
> compile because the enum is not defined.
>
> Also the function clockevents_notify is a noop macro, this is fine
> except the usual code is:
>
> int cpu = smp_processor_id();
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
>
> and that raises a warning telling the variable 'cpu' is not used.
>
> Move the clock_event_nofitiers enum definition out of the
> CONFIG_GENERIC_CLOCKEVENTS_BUILD section to prevent a compilation
> error when these are used in the code.
>
> Change the clockevents_notify macro to a static inline noop function
> to prevent a compilation warning.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
So why can't s3c64xx enable "CONFIG_GENERIC_CLOCKEVENTS_BUILD" if they
wants to use broadcast notfiers ? May be am missing something.
Regards,
Santosh
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 01/15] timer: move enum definition out of ifdef section
2013-03-26 4:36 ` Santosh Shilimkar
@ 2013-03-26 9:55 ` Daniel Lezcano
0 siblings, 0 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-26 9:55 UTC (permalink / raw)
To: linux-arm-kernel
On 03/26/2013 05:36 AM, Santosh Shilimkar wrote:
> On Monday 25 March 2013 11:25 PM, Daniel Lezcano wrote:
>> The next patch will setup automatically the broadcast timer for
>> the different cpuidle driver when one idle state stops its timer.
>> This will be part of the generic code.
>>
>> But some ARM boards, like s3c64xx, uses cpuidle but without the
>> CONFIG_GENERIC_CLOCKEVENTS_BUILD set. Hence the cpuidle framework
>> will be compiled with the code supposed to be generic, that is
>> with clockevents_notify and the different enum, but will fail to
>> compile because the enum is not defined.
>>
>> Also the function clockevents_notify is a noop macro, this is fine
>> except the usual code is:
>>
>> int cpu = smp_processor_id();
>> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
>>
>> and that raises a warning telling the variable 'cpu' is not used.
>>
>> Move the clock_event_nofitiers enum definition out of the
>> CONFIG_GENERIC_CLOCKEVENTS_BUILD section to prevent a compilation
>> error when these are used in the code.
>>
>> Change the clockevents_notify macro to a static inline noop function
>> to prevent a compilation warning.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>
> So why can't s3c64xx enable "CONFIG_GENERIC_CLOCKEVENTS_BUILD" if they
> wants to use broadcast notfiers ? May be am missing something.
They won't use broadcast notifiers but the cpuidle code
'drivers/cpuidle/cpuidle.c' has a generic code using it. It does not
compile because an enum definition is inside the block
CONFIG_GENERIC_CLOCKEVENTS_BUILD.
...
if (drv->states[next_state].flags & CPUIDLE_FLAG_TIMER_STOP)
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER,
&dev->cpu);
...
clockevents_notify will be a noop (this is ok), but
CLOCK_EVT_NOTIFY_BROADCAST_ENTER won't be defined, so error at compile time.
--
<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] 45+ messages in thread
* [PATCH 02/15] cpuidle: initialize the broadcast timer framework
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
2013-03-25 17:55 ` [PATCH 01/15] timer: move enum definition out of ifdef section Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-26 4:39 ` Santosh Shilimkar
2013-03-25 17:55 ` [PATCH 03/15] cpuidle: ux500: remove timer broadcast initialization Daniel Lezcano
` (13 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
The commit 89878baa73f0f1c679355006bd8632e5d78f96c2 introduced
the CPUIDLE_FLAG_TIMER_STOP flag where we specify a specific idle
state stops the local timer.
Now use this flag to check at init time if one state will need
the broadcast timer and, in this case, setup the broadcast timer
framework. That prevents multiple code duplication in the drivers.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/cpuidle/driver.c | 35 +++++++++++++++++++++++++++++++++--
include/linux/cpuidle.h | 2 ++
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
index 422c7b6..476ce0c 100644
--- a/drivers/cpuidle/driver.c
+++ b/drivers/cpuidle/driver.c
@@ -11,6 +11,8 @@
#include <linux/mutex.h>
#include <linux/module.h>
#include <linux/cpuidle.h>
+#include <linux/cpumask.h>
+#include <linux/clockchips.h>
#include "cpuidle.h"
@@ -19,9 +21,30 @@ DEFINE_SPINLOCK(cpuidle_driver_lock);
static void __cpuidle_set_cpu_driver(struct cpuidle_driver *drv, int cpu);
static struct cpuidle_driver * __cpuidle_get_cpu_driver(int cpu);
-static void __cpuidle_driver_init(struct cpuidle_driver *drv)
+static void cpuidle_setup_broadcast_timer(void *arg)
{
+ int cpu = smp_processor_id();
+ clockevents_notify((long)(arg), &cpu);
+}
+
+static void __cpuidle_driver_init(struct cpuidle_driver *drv, int cpu)
+{
+ int i;
+
drv->refcnt = 0;
+
+ for (i = drv->state_count - 1; i >= 0 ; i--) {
+
+ if (!(drv->states[i].flags & CPUIDLE_FLAG_TIMER_STOP))
+ continue;
+
+ drv->bctimer = 1;
+
+ on_each_cpu_mask(get_cpu_mask(cpu), cpuidle_setup_broadcast_timer,
+ (void *)CLOCK_EVT_NOTIFY_BROADCAST_ON, 1);
+
+ break;
+ }
}
static int __cpuidle_register_driver(struct cpuidle_driver *drv, int cpu)
@@ -35,7 +58,7 @@ static int __cpuidle_register_driver(struct cpuidle_driver *drv, int cpu)
if (__cpuidle_get_cpu_driver(cpu))
return -EBUSY;
- __cpuidle_driver_init(drv);
+ __cpuidle_driver_init(drv, cpu);
__cpuidle_set_cpu_driver(drv, cpu);
@@ -49,6 +72,14 @@ static void __cpuidle_unregister_driver(struct cpuidle_driver *drv, int cpu)
if (!WARN_ON(drv->refcnt > 0))
__cpuidle_set_cpu_driver(NULL, cpu);
+
+ if (drv->bctimer) {
+
+ drv->bctimer = 0;
+
+ on_each_cpu_mask(get_cpu_mask(cpu), cpuidle_setup_broadcast_timer,
+ (void *)CLOCK_EVT_NOTIFY_BROADCAST_OFF, 1);
+ }
}
#ifdef CONFIG_CPU_IDLE_MULTIPLE_DRIVERS
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index a837b33..fc3e580 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -107,6 +107,8 @@ struct cpuidle_driver {
/* set to 1 to use the core cpuidle time keeping (for all states). */
unsigned int en_core_tk_irqen:1;
+ /* used by the cpuidle framework to setup the broadcast timer */
+ unsigned int bctimer:1;
/* states array must be ordered in decreasing power consumption */
struct cpuidle_state states[CPUIDLE_STATE_MAX];
int state_count;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 02/15] cpuidle: initialize the broadcast timer framework
2013-03-25 17:55 ` [PATCH 02/15] cpuidle: initialize the broadcast timer framework Daniel Lezcano
@ 2013-03-26 4:39 ` Santosh Shilimkar
0 siblings, 0 replies; 45+ messages in thread
From: Santosh Shilimkar @ 2013-03-26 4:39 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 25 March 2013 11:25 PM, Daniel Lezcano wrote:
> The commit 89878baa73f0f1c679355006bd8632e5d78f96c2 introduced
> the CPUIDLE_FLAG_TIMER_STOP flag where we specify a specific idle
> state stops the local timer.
>
> Now use this flag to check at init time if one state will need
> the broadcast timer and, in this case, setup the broadcast timer
> framework. That prevents multiple code duplication in the drivers.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/cpuidle/driver.c | 35 +++++++++++++++++++++++++++++++++--
> include/linux/cpuidle.h | 2 ++
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
Make sense. Minor comments.
> diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
> index 422c7b6..476ce0c 100644
> --- a/drivers/cpuidle/driver.c
> +++ b/drivers/cpuidle/driver.c
> @@ -11,6 +11,8 @@
> #include <linux/mutex.h>
> #include <linux/module.h>
> #include <linux/cpuidle.h>
> +#include <linux/cpumask.h>
> +#include <linux/clockchips.h>
>
> #include "cpuidle.h"
>
> @@ -19,9 +21,30 @@ DEFINE_SPINLOCK(cpuidle_driver_lock);
> static void __cpuidle_set_cpu_driver(struct cpuidle_driver *drv, int cpu);
> static struct cpuidle_driver * __cpuidle_get_cpu_driver(int cpu);
>
> -static void __cpuidle_driver_init(struct cpuidle_driver *drv)
> +static void cpuidle_setup_broadcast_timer(void *arg)
> {
> + int cpu = smp_processor_id();
> + clockevents_notify((long)(arg), &cpu);
> +}
> +
> +static void __cpuidle_driver_init(struct cpuidle_driver *drv, int cpu)
> +{
> + int i;
> +
Un-necessary extra line..
> drv->refcnt = 0;
> +
> + for (i = drv->state_count - 1; i >= 0 ; i--) {
> +
here as well
> + if (!(drv->states[i].flags & CPUIDLE_FLAG_TIMER_STOP))
> + continue;
> +
> + drv->bctimer = 1;
> +
ditto
> + on_each_cpu_mask(get_cpu_mask(cpu), cpuidle_setup_broadcast_timer,
> + (void *)CLOCK_EVT_NOTIFY_BROADCAST_ON, 1);
> +
ditto
> + break;
> + }
> }
>
> static int __cpuidle_register_driver(struct cpuidle_driver *drv, int cpu)
> @@ -35,7 +58,7 @@ static int __cpuidle_register_driver(struct cpuidle_driver *drv, int cpu)
> if (__cpuidle_get_cpu_driver(cpu))
> return -EBUSY;
>
> - __cpuidle_driver_init(drv);
> + __cpuidle_driver_init(drv, cpu);
>
> __cpuidle_set_cpu_driver(drv, cpu);
>
> @@ -49,6 +72,14 @@ static void __cpuidle_unregister_driver(struct cpuidle_driver *drv, int cpu)
>
> if (!WARN_ON(drv->refcnt > 0))
> __cpuidle_set_cpu_driver(NULL, cpu);
> +
> + if (drv->bctimer) {
> +
> + drv->bctimer = 0;
> +
> +
no need of extra line here
on_each_cpu_mask(get_cpu_mask(cpu), cpuidle_setup_broadcast_timer,
> + (void *)CLOCK_EVT_NOTIFY_BROADCAST_OFF, 1);
> + }
> }
>
> #ifdef CONFIG_CPU_IDLE_MULTIPLE_DRIVERS
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index a837b33..fc3e580 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -107,6 +107,8 @@ struct cpuidle_driver {
>
> /* set to 1 to use the core cpuidle time keeping (for all states). */
> unsigned int en_core_tk_irqen:1;
> + /* used by the cpuidle framework to setup the broadcast timer */
> + unsigned int bctimer:1;
> /* states array must be ordered in decreasing power consumption */
> struct cpuidle_state states[CPUIDLE_STATE_MAX];
> int state_count;
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 03/15] cpuidle: ux500: remove timer broadcast initialization
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
2013-03-25 17:55 ` [PATCH 01/15] timer: move enum definition out of ifdef section Daniel Lezcano
2013-03-25 17:55 ` [PATCH 02/15] cpuidle: initialize the broadcast timer framework Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-27 14:46 ` Linus Walleij
2013-03-25 17:55 ` [PATCH 04/15] cpuidle: OMAP4: " Daniel Lezcano
` (12 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
The initialization is done from the cpuidle framework.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-ux500/cpuidle.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/arch/arm/mach-ux500/cpuidle.c b/arch/arm/mach-ux500/cpuidle.c
index 6d0c4b6..1b16d9e 100644
--- a/arch/arm/mach-ux500/cpuidle.c
+++ b/arch/arm/mach-ux500/cpuidle.c
@@ -11,7 +11,6 @@
#include <linux/module.h>
#include <linux/cpuidle.h>
-#include <linux/clockchips.h>
#include <linux/spinlock.h>
#include <linux/atomic.h>
#include <linux/smp.h>
@@ -112,16 +111,6 @@ static struct cpuidle_driver ux500_idle_driver = {
.state_count = 2,
};
-/*
- * For each cpu, setup the broadcast timer because we will
- * need to migrate the timers for the states >= ApIdle.
- */
-static void ux500_setup_broadcast_timer(void *arg)
-{
- int cpu = smp_processor_id();
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
-}
-
int __init ux500_idle_init(void)
{
int ret, cpu;
@@ -131,13 +120,6 @@ int __init ux500_idle_init(void)
prcmu_enable_wakeups(PRCMU_WAKEUP(ARM) | PRCMU_WAKEUP(RTC) |
PRCMU_WAKEUP(ABB));
- /*
- * Configure the timer broadcast for each cpu, that must
- * be done from the cpu context, so we use a smp cross
- * call with 'on_each_cpu'.
- */
- on_each_cpu(ux500_setup_broadcast_timer, NULL, 1);
-
ret = cpuidle_register_driver(&ux500_idle_driver);
if (ret) {
printk(KERN_ERR "failed to register ux500 idle driver\n");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 04/15] cpuidle: OMAP4: remove timer broadcast initialization
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (2 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 03/15] cpuidle: ux500: remove timer broadcast initialization Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-26 4:41 ` Santosh Shilimkar
2013-03-25 17:55 ` [PATCH 05/15] cpuidle: imx6: " Daniel Lezcano
` (11 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
The initialization is done from the cpuidle framework.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-omap2/cpuidle44xx.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index fe0e025..f4b1b23 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -14,7 +14,6 @@
#include <linux/cpuidle.h>
#include <linux/cpu_pm.h>
#include <linux/export.h>
-#include <linux/clockchips.h>
#include <asm/proc-fns.h>
@@ -158,16 +157,6 @@ fail:
return index;
}
-/*
- * For each cpu, setup the broadcast timer because local timers
- * stops for the states above C1.
- */
-static void omap_setup_broadcast_timer(void *arg)
-{
- int cpu = smp_processor_id();
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
-}
-
static DEFINE_PER_CPU(struct cpuidle_device, omap4_idle_dev);
static struct cpuidle_driver omap4_idle_driver = {
@@ -233,9 +222,6 @@ int __init omap4_idle_init(void)
if (!cpu_clkdm[0] || !cpu_clkdm[1])
return -ENODEV;
- /* Configure the broadcast timer on each cpu */
- on_each_cpu(omap_setup_broadcast_timer, NULL, 1);
-
for_each_cpu(cpu_id, cpu_online_mask) {
dev = &per_cpu(omap4_idle_dev, cpu_id);
dev->cpu = cpu_id;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 05/15] cpuidle: imx6: remove timer broadcast initialization
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (3 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 04/15] cpuidle: OMAP4: " Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-26 7:25 ` Shawn Guo
2013-03-26 14:28 ` Shawn Guo
2013-03-25 17:55 ` [PATCH 06/15] ARM: cpuidle: remove useless declaration Daniel Lezcano
` (10 subsequent siblings)
15 siblings, 2 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
The initialization is done from the cpuidle framework.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-imx/cpuidle-imx6q.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
index 5ae22f7..a783a63 100644
--- a/arch/arm/mach-imx/cpuidle-imx6q.c
+++ b/arch/arm/mach-imx/cpuidle-imx6q.c
@@ -6,7 +6,6 @@
* published by the Free Software Foundation.
*/
-#include <linux/clockchips.h>
#include <linux/cpuidle.h>
#include <linux/module.h>
#include <asm/cpuidle.h>
@@ -43,17 +42,6 @@ done:
return index;
}
-/*
- * For each cpu, setup the broadcast timer because local timer
- * stops for the states other than WFI.
- */
-static void imx6q_setup_broadcast_timer(void *arg)
-{
- int cpu = smp_processor_id();
-
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
-}
-
static struct cpuidle_driver imx6q_cpuidle_driver = {
.name = "imx6q_cpuidle",
.owner = THIS_MODULE,
@@ -84,8 +72,5 @@ int __init imx6q_cpuidle_init(void)
/* Set chicken bit to get a reliable WAIT mode support */
imx6q_set_chicken_bit();
- /* Configure the broadcast timer on each cpu */
- on_each_cpu(imx6q_setup_broadcast_timer, NULL, 1);
-
return imx_cpuidle_init(&imx6q_cpuidle_driver);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 05/15] cpuidle: imx6: remove timer broadcast initialization
2013-03-25 17:55 ` [PATCH 05/15] cpuidle: imx6: " Daniel Lezcano
@ 2013-03-26 7:25 ` Shawn Guo
2013-03-26 9:43 ` Daniel Lezcano
2013-03-26 14:28 ` Shawn Guo
1 sibling, 1 reply; 45+ messages in thread
From: Shawn Guo @ 2013-03-26 7:25 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 25, 2013 at 06:55:30PM +0100, Daniel Lezcano wrote:
> The initialization is done from the cpuidle framework.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> arch/arm/mach-imx/cpuidle-imx6q.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
> index 5ae22f7..a783a63 100644
> --- a/arch/arm/mach-imx/cpuidle-imx6q.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6q.c
> @@ -6,7 +6,6 @@
> * published by the Free Software Foundation.
> */
>
> -#include <linux/clockchips.h>
Removing this causes the compile error below.
CC arch/arm/mach-imx/cpuidle-imx6q.o
arch/arm/mach-imx/cpuidle-imx6q.c: In function ?imx6q_enter_wait?:
arch/arm/mach-imx/cpuidle-imx6q.c:25:2: error: implicit declaration of function ?clockevents_notify? [-Werror=implicit-function-declaration]
arch/arm/mach-imx/cpuidle-imx6q.c:25:21: error: ?CLOCK_EVT_NOTIFY_BROADCAST_ENTER? undeclared (first use in this function)
arch/arm/mach-imx/cpuidle-imx6q.c:25:21: note: each undeclared identifier is reported only once for each function it appears in
arch/arm/mach-imx/cpuidle-imx6q.c:45:21: error: ?CLOCK_EVT_NOTIFY_BROADCAST_EXIT? undeclared (first use in this function)
cc1: some warnings being treated as errors
make[2]: *** [arch/arm/mach-imx/cpuidle-imx6q.o] Error 1
Shawn
> #include <linux/cpuidle.h>
> #include <linux/module.h>
> #include <asm/cpuidle.h>
> @@ -43,17 +42,6 @@ done:
> return index;
> }
>
> -/*
> - * For each cpu, setup the broadcast timer because local timer
> - * stops for the states other than WFI.
> - */
> -static void imx6q_setup_broadcast_timer(void *arg)
> -{
> - int cpu = smp_processor_id();
> -
> - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
> -}
> -
> static struct cpuidle_driver imx6q_cpuidle_driver = {
> .name = "imx6q_cpuidle",
> .owner = THIS_MODULE,
> @@ -84,8 +72,5 @@ int __init imx6q_cpuidle_init(void)
> /* Set chicken bit to get a reliable WAIT mode support */
> imx6q_set_chicken_bit();
>
> - /* Configure the broadcast timer on each cpu */
> - on_each_cpu(imx6q_setup_broadcast_timer, NULL, 1);
> -
> return imx_cpuidle_init(&imx6q_cpuidle_driver);
> }
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH 05/15] cpuidle: imx6: remove timer broadcast initialization
2013-03-26 7:25 ` Shawn Guo
@ 2013-03-26 9:43 ` Daniel Lezcano
2013-03-26 12:40 ` Shawn Guo
0 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-26 9:43 UTC (permalink / raw)
To: linux-arm-kernel
On 03/26/2013 08:25 AM, Shawn Guo wrote:
> On Mon, Mar 25, 2013 at 06:55:30PM +0100, Daniel Lezcano wrote:
>> The initialization is done from the cpuidle framework.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>> arch/arm/mach-imx/cpuidle-imx6q.c | 15 ---------------
>> 1 file changed, 15 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
>> index 5ae22f7..a783a63 100644
>> --- a/arch/arm/mach-imx/cpuidle-imx6q.c
>> +++ b/arch/arm/mach-imx/cpuidle-imx6q.c
>> @@ -6,7 +6,6 @@
>> * published by the Free Software Foundation.
>> */
>>
>> -#include <linux/clockchips.h>
>
> Removing this causes the compile error below.
>
> CC arch/arm/mach-imx/cpuidle-imx6q.o
> arch/arm/mach-imx/cpuidle-imx6q.c: In function ?imx6q_enter_wait?:
> arch/arm/mach-imx/cpuidle-imx6q.c:25:2: error: implicit declaration of function ?clockevents_notify? [-Werror=implicit-function-declaration]
> arch/arm/mach-imx/cpuidle-imx6q.c:25:21: error: ?CLOCK_EVT_NOTIFY_BROADCAST_ENTER? undeclared (first use in this function)
> arch/arm/mach-imx/cpuidle-imx6q.c:25:21: note: each undeclared identifier is reported only once for each function it appears in
> arch/arm/mach-imx/cpuidle-imx6q.c:45:21: error: ?CLOCK_EVT_NOTIFY_BROADCAST_EXIT? undeclared (first use in this function)
> cc1: some warnings being treated as errors
> make[2]: *** [arch/arm/mach-imx/cpuidle-imx6q.o] Error 1
Hi Shawn,
actually this is because the patch applies on top of:
commit 12e849504b861470e5bf991f79c0be71b97c8dac
Author: Daniel Lezcano <daniel.lezcano@linaro.org>
Date: Thu Mar 21 12:21:33 2013 +0000
cpuidle / imx6 : use CPUIDLE_FLAG_TIMER_STOP flag
Use the CPUIDLE_FLAG_TIMER_STOP and let the cpuidle framework
to handle the CLOCK_EVT_NOTIFY_BROADCAST_ENTER/EXIT when entering
this state.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
--
<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] 45+ messages in thread* [PATCH 05/15] cpuidle: imx6: remove timer broadcast initialization
2013-03-26 9:43 ` Daniel Lezcano
@ 2013-03-26 12:40 ` Shawn Guo
2013-03-26 12:53 ` Daniel Lezcano
0 siblings, 1 reply; 45+ messages in thread
From: Shawn Guo @ 2013-03-26 12:40 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 26, 2013 at 10:43:29AM +0100, Daniel Lezcano wrote:
> Hi Shawn,
>
> actually this is because the patch applies on top of:
>
Ah, ok. It will be helpful for people who want to test the series on
their platform, if you can publish a branch for that.
Shawn
> commit 12e849504b861470e5bf991f79c0be71b97c8dac
> Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> Date: Thu Mar 21 12:21:33 2013 +0000
>
> cpuidle / imx6 : use CPUIDLE_FLAG_TIMER_STOP flag
>
> Use the CPUIDLE_FLAG_TIMER_STOP and let the cpuidle framework
> to handle the CLOCK_EVT_NOTIFY_BROADCAST_ENTER/EXIT when entering
> this state.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 05/15] cpuidle: imx6: remove timer broadcast initialization
2013-03-26 12:40 ` Shawn Guo
@ 2013-03-26 12:53 ` Daniel Lezcano
2013-03-26 13:06 ` Rafael J. Wysocki
0 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-26 12:53 UTC (permalink / raw)
To: linux-arm-kernel
On 03/26/2013 01:40 PM, Shawn Guo wrote:
> On Tue, Mar 26, 2013 at 10:43:29AM +0100, Daniel Lezcano wrote:
>> Hi Shawn,
>>
>> actually this is because the patch applies on top of:
>>
> Ah, ok. It will be helpful for people who want to test the series on
> their platform, if you can publish a branch for that.
Yes, sure.
It is in Rafael's tree.
https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git
in linux-next branch.
Thanks for the review
-- Daniel
>> commit 12e849504b861470e5bf991f79c0be71b97c8dac
>> Author: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Date: Thu Mar 21 12:21:33 2013 +0000
>>
>> cpuidle / imx6 : use CPUIDLE_FLAG_TIMER_STOP flag
>>
>> Use the CPUIDLE_FLAG_TIMER_STOP and let the cpuidle framework
>> to handle the CLOCK_EVT_NOTIFY_BROADCAST_ENTER/EXIT when entering
>> this state.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
--
<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] 45+ messages in thread
* [PATCH 05/15] cpuidle: imx6: remove timer broadcast initialization
2013-03-26 12:53 ` Daniel Lezcano
@ 2013-03-26 13:06 ` Rafael J. Wysocki
0 siblings, 0 replies; 45+ messages in thread
From: Rafael J. Wysocki @ 2013-03-26 13:06 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, March 26, 2013 01:53:34 PM Daniel Lezcano wrote:
> On 03/26/2013 01:40 PM, Shawn Guo wrote:
> > On Tue, Mar 26, 2013 at 10:43:29AM +0100, Daniel Lezcano wrote:
> >> Hi Shawn,
> >>
> >> actually this is because the patch applies on top of:
> >>
> > Ah, ok. It will be helpful for people who want to test the series on
> > their platform, if you can publish a branch for that.
>
> Yes, sure.
>
> It is in Rafael's tree.
>
> https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git
>
> in linux-next branch.
IOW, the patchset is based on the current linux-next. :-)
Thanks,
Rafael
> >> commit 12e849504b861470e5bf991f79c0be71b97c8dac
> >> Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> Date: Thu Mar 21 12:21:33 2013 +0000
> >>
> >> cpuidle / imx6 : use CPUIDLE_FLAG_TIMER_STOP flag
> >>
> >> Use the CPUIDLE_FLAG_TIMER_STOP and let the cpuidle framework
> >> to handle the CLOCK_EVT_NOTIFY_BROADCAST_ENTER/EXIT when entering
> >> this state.
> >>
> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 05/15] cpuidle: imx6: remove timer broadcast initialization
2013-03-25 17:55 ` [PATCH 05/15] cpuidle: imx6: " Daniel Lezcano
2013-03-26 7:25 ` Shawn Guo
@ 2013-03-26 14:28 ` Shawn Guo
1 sibling, 0 replies; 45+ messages in thread
From: Shawn Guo @ 2013-03-26 14:28 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 25, 2013 at 06:55:30PM +0100, Daniel Lezcano wrote:
> The initialization is done from the cpuidle framework.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> arch/arm/mach-imx/cpuidle-imx6q.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
> index 5ae22f7..a783a63 100644
> --- a/arch/arm/mach-imx/cpuidle-imx6q.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6q.c
> @@ -6,7 +6,6 @@
> * published by the Free Software Foundation.
> */
>
> -#include <linux/clockchips.h>
> #include <linux/cpuidle.h>
> #include <linux/module.h>
> #include <asm/cpuidle.h>
> @@ -43,17 +42,6 @@ done:
> return index;
> }
>
> -/*
> - * For each cpu, setup the broadcast timer because local timer
> - * stops for the states other than WFI.
> - */
> -static void imx6q_setup_broadcast_timer(void *arg)
> -{
> - int cpu = smp_processor_id();
> -
> - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
> -}
> -
> static struct cpuidle_driver imx6q_cpuidle_driver = {
> .name = "imx6q_cpuidle",
> .owner = THIS_MODULE,
> @@ -84,8 +72,5 @@ int __init imx6q_cpuidle_init(void)
> /* Set chicken bit to get a reliable WAIT mode support */
> imx6q_set_chicken_bit();
>
> - /* Configure the broadcast timer on each cpu */
> - on_each_cpu(imx6q_setup_broadcast_timer, NULL, 1);
> -
> return imx_cpuidle_init(&imx6q_cpuidle_driver);
> }
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 06/15] ARM: cpuidle: remove useless declaration
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (4 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 05/15] cpuidle: imx6: " Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-25 17:55 ` [PATCH 07/15] ARM: cpuidle: add init/exit routine Daniel Lezcano
` (9 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
The noop functions code is not necessary because the header file is
included in files which are compiled when CONFIG_CPU_IDLE is on.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/include/asm/cpuidle.h | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
index 2fca60a..7367787 100644
--- a/arch/arm/include/asm/cpuidle.h
+++ b/arch/arm/include/asm/cpuidle.h
@@ -1,13 +1,8 @@
#ifndef __ASM_ARM_CPUIDLE_H
#define __ASM_ARM_CPUIDLE_H
-#ifdef CONFIG_CPU_IDLE
extern int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
- struct cpuidle_driver *drv, int index);
-#else
-static inline int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
- struct cpuidle_driver *drv, int index) { return -ENODEV; }
-#endif
+ struct cpuidle_driver *drv, int index);
/* Common ARM WFI state */
#define ARM_CPUIDLE_WFI_STATE_PWR(p) {\
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (5 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 06/15] ARM: cpuidle: remove useless declaration Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-25 18:10 ` Andrew Lunn
2013-03-25 17:55 ` [PATCH 08/15] ARM: ux500: cpuidle: use init/exit common routine Daniel Lezcano
` (8 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
The init and exit routine for most of the drivers are the same,
that is register the driver and register the device.
Provide a common function to do that in the cpuidle driver for ARM,
so we can get rid of a lot of code duplication in the different SOC
cpuidle drivers.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/include/asm/cpuidle.h | 4 +++
arch/arm/kernel/cpuidle.c | 57 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
index 7367787..83a38ac 100644
--- a/arch/arm/include/asm/cpuidle.h
+++ b/arch/arm/include/asm/cpuidle.h
@@ -4,6 +4,10 @@
extern int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index);
+extern int arm_cpuidle_init(struct cpuidle_driver *drv);
+
+extern void arm_cpuidle_exit(struct cpuidle_driver *drv);
+
/* Common ARM WFI state */
#define ARM_CPUIDLE_WFI_STATE_PWR(p) {\
.enter = arm_cpuidle_simple_enter,\
diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
index 89545f6..13cfe3e 100644
--- a/arch/arm/kernel/cpuidle.c
+++ b/arch/arm/kernel/cpuidle.c
@@ -9,13 +9,68 @@
* http://www.gnu.org/copyleft/gpl.html
*/
+#include <linux/module.h>
#include <linux/cpuidle.h>
#include <asm/proc-fns.h>
+static DEFINE_PER_CPU(struct cpuidle_device, cpuidle_device);
+
int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
- struct cpuidle_driver *drv, int index)
+ struct cpuidle_driver *drv, int index)
{
cpu_do_idle();
return index;
}
+
+int __init arm_cpuidle_init(struct cpuidle_driver *drv)
+{
+ int ret, cpu;
+ struct cpuidle_device *device;
+
+ ret = cpuidle_register_driver(drv);
+ if (ret) {
+ printk(KERN_ERR "failed to register idle driver '%s'\n",
+ drv->name);
+ return ret;
+ }
+
+ for_each_online_cpu(cpu) {
+
+ device = &per_cpu(cpuidle_device, cpu);
+ device->cpu = cpu;
+ ret = cpuidle_register_device(device);
+ if (ret) {
+ printk(KERN_ERR "Failed to register cpuidle "
+ "device for cpu%d\n", cpu);
+ goto out_unregister;
+ }
+ }
+
+out:
+ return ret;
+
+out_unregister:
+ for_each_online_cpu(cpu) {
+ device = &per_cpu(cpuidle_device, cpu);
+ cpuidle_unregister_device(device);
+ }
+
+ cpuidle_unregister_driver(drv);
+ goto out;
+}
+EXPORT_SYMBOL_GPL(arm_cpuidle_init);
+
+void __exit arm_cpuidle_exit(struct cpuidle_driver *drv)
+{
+ int cpu;
+ struct cpuidle_device *device;
+
+ for_each_online_cpu(cpu) {
+ device = &per_cpu(cpuidle_device, cpu);
+ cpuidle_unregister_device(device);
+ }
+
+ cpuidle_unregister_driver(drv);
+}
+EXPORT_SYMBOL_GPL(arm_cpuidle_exit);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 17:55 ` [PATCH 07/15] ARM: cpuidle: add init/exit routine Daniel Lezcano
@ 2013-03-25 18:10 ` Andrew Lunn
2013-03-25 18:33 ` Daniel Lezcano
0 siblings, 1 reply; 45+ messages in thread
From: Andrew Lunn @ 2013-03-25 18:10 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 25, 2013 at 06:55:32PM +0100, Daniel Lezcano wrote:
> The init and exit routine for most of the drivers are the same,
> that is register the driver and register the device.
>
> Provide a common function to do that in the cpuidle driver for ARM,
> so we can get rid of a lot of code duplication in the different SOC
> cpuidle drivers.
Hi Daniel
Please could you add a comment in the code about which piece is
specific to ARM, because its not obvious to me. Its not like there is
a reference to WFI for example. It looks like this code could go in
drivers/cpuidle/cpuidle.c
Thanks
Andrew
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> arch/arm/include/asm/cpuidle.h | 4 +++
> arch/arm/kernel/cpuidle.c | 57 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
> index 7367787..83a38ac 100644
> --- a/arch/arm/include/asm/cpuidle.h
> +++ b/arch/arm/include/asm/cpuidle.h
> @@ -4,6 +4,10 @@
> extern int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int index);
>
> +extern int arm_cpuidle_init(struct cpuidle_driver *drv);
> +
> +extern void arm_cpuidle_exit(struct cpuidle_driver *drv);
> +
> /* Common ARM WFI state */
> #define ARM_CPUIDLE_WFI_STATE_PWR(p) {\
> .enter = arm_cpuidle_simple_enter,\
> diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
> index 89545f6..13cfe3e 100644
> --- a/arch/arm/kernel/cpuidle.c
> +++ b/arch/arm/kernel/cpuidle.c
> @@ -9,13 +9,68 @@
> * http://www.gnu.org/copyleft/gpl.html
> */
>
> +#include <linux/module.h>
> #include <linux/cpuidle.h>
> #include <asm/proc-fns.h>
>
> +static DEFINE_PER_CPU(struct cpuidle_device, cpuidle_device);
> +
> int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> - struct cpuidle_driver *drv, int index)
> + struct cpuidle_driver *drv, int index)
> {
> cpu_do_idle();
>
> return index;
> }
> +
> +int __init arm_cpuidle_init(struct cpuidle_driver *drv)
> +{
> + int ret, cpu;
> + struct cpuidle_device *device;
> +
> + ret = cpuidle_register_driver(drv);
> + if (ret) {
> + printk(KERN_ERR "failed to register idle driver '%s'\n",
> + drv->name);
> + return ret;
> + }
> +
> + for_each_online_cpu(cpu) {
> +
> + device = &per_cpu(cpuidle_device, cpu);
> + device->cpu = cpu;
> + ret = cpuidle_register_device(device);
> + if (ret) {
> + printk(KERN_ERR "Failed to register cpuidle "
> + "device for cpu%d\n", cpu);
> + goto out_unregister;
> + }
> + }
> +
> +out:
> + return ret;
> +
> +out_unregister:
> + for_each_online_cpu(cpu) {
> + device = &per_cpu(cpuidle_device, cpu);
> + cpuidle_unregister_device(device);
> + }
> +
> + cpuidle_unregister_driver(drv);
> + goto out;
> +}
> +EXPORT_SYMBOL_GPL(arm_cpuidle_init);
> +
> +void __exit arm_cpuidle_exit(struct cpuidle_driver *drv)
> +{
> + int cpu;
> + struct cpuidle_device *device;
> +
> + for_each_online_cpu(cpu) {
> + device = &per_cpu(cpuidle_device, cpu);
> + cpuidle_unregister_device(device);
> + }
> +
> + cpuidle_unregister_driver(drv);
> +}
> +EXPORT_SYMBOL_GPL(arm_cpuidle_exit);
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 18:10 ` Andrew Lunn
@ 2013-03-25 18:33 ` Daniel Lezcano
2013-03-25 19:09 ` Andrew Lunn
2013-03-25 20:28 ` Nicolas Pitre
0 siblings, 2 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 18:33 UTC (permalink / raw)
To: linux-arm-kernel
On 03/25/2013 07:10 PM, Andrew Lunn wrote:
> On Mon, Mar 25, 2013 at 06:55:32PM +0100, Daniel Lezcano wrote:
>> The init and exit routine for most of the drivers are the same,
>> that is register the driver and register the device.
>>
>> Provide a common function to do that in the cpuidle driver for ARM,
>> so we can get rid of a lot of code duplication in the different SOC
>> cpuidle drivers.
>
> Hi Daniel
>
> Please could you add a comment in the code about which piece is
> specific to ARM, because its not obvious to me. Its not like there is
> a reference to WFI for example. It looks like this code could go in
> drivers/cpuidle/cpuidle.c
Yes, I agree. At the first glance, the code, as it is, could go in this
file but more ARM specific code will be moved to this ARM generic code
driver like device tree description and couple idle states. The init
function would be more arch specific then.
For this reason, I think it is reasonable to move to
arm/kernel/cpuidle.c rather than drivers/cpuidle/cpuidle.c first.
In the future, when all the ARM cpuidle driver will be fully
consolidated, that will be easier to identify the common parts across
the different arch and then move them to the generic framework.
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>> arch/arm/include/asm/cpuidle.h | 4 +++
>> arch/arm/kernel/cpuidle.c | 57 +++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
>> index 7367787..83a38ac 100644
>> --- a/arch/arm/include/asm/cpuidle.h
>> +++ b/arch/arm/include/asm/cpuidle.h
>> @@ -4,6 +4,10 @@
>> extern int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
>> struct cpuidle_driver *drv, int index);
>>
>> +extern int arm_cpuidle_init(struct cpuidle_driver *drv);
>> +
>> +extern void arm_cpuidle_exit(struct cpuidle_driver *drv);
>> +
>> /* Common ARM WFI state */
>> #define ARM_CPUIDLE_WFI_STATE_PWR(p) {\
>> .enter = arm_cpuidle_simple_enter,\
>> diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
>> index 89545f6..13cfe3e 100644
>> --- a/arch/arm/kernel/cpuidle.c
>> +++ b/arch/arm/kernel/cpuidle.c
>> @@ -9,13 +9,68 @@
>> * http://www.gnu.org/copyleft/gpl.html
>> */
>>
>> +#include <linux/module.h>
>> #include <linux/cpuidle.h>
>> #include <asm/proc-fns.h>
>>
>> +static DEFINE_PER_CPU(struct cpuidle_device, cpuidle_device);
>> +
>> int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
>> - struct cpuidle_driver *drv, int index)
>> + struct cpuidle_driver *drv, int index)
>> {
>> cpu_do_idle();
>>
>> return index;
>> }
>> +
>> +int __init arm_cpuidle_init(struct cpuidle_driver *drv)
>> +{
>> + int ret, cpu;
>> + struct cpuidle_device *device;
>> +
>> + ret = cpuidle_register_driver(drv);
>> + if (ret) {
>> + printk(KERN_ERR "failed to register idle driver '%s'\n",
>> + drv->name);
>> + return ret;
>> + }
>> +
>> + for_each_online_cpu(cpu) {
>> +
>> + device = &per_cpu(cpuidle_device, cpu);
>> + device->cpu = cpu;
>> + ret = cpuidle_register_device(device);
>> + if (ret) {
>> + printk(KERN_ERR "Failed to register cpuidle "
>> + "device for cpu%d\n", cpu);
>> + goto out_unregister;
>> + }
>> + }
>> +
>> +out:
>> + return ret;
>> +
>> +out_unregister:
>> + for_each_online_cpu(cpu) {
>> + device = &per_cpu(cpuidle_device, cpu);
>> + cpuidle_unregister_device(device);
>> + }
>> +
>> + cpuidle_unregister_driver(drv);
>> + goto out;
>> +}
>> +EXPORT_SYMBOL_GPL(arm_cpuidle_init);
>> +
>> +void __exit arm_cpuidle_exit(struct cpuidle_driver *drv)
>> +{
>> + int cpu;
>> + struct cpuidle_device *device;
>> +
>> + for_each_online_cpu(cpu) {
>> + device = &per_cpu(cpuidle_device, cpu);
>> + cpuidle_unregister_device(device);
>> + }
>> +
>> + cpuidle_unregister_driver(drv);
>> +}
>> +EXPORT_SYMBOL_GPL(arm_cpuidle_exit);
>> --
>> 1.7.9.5
>>
--
<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] 45+ messages in thread* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 18:33 ` Daniel Lezcano
@ 2013-03-25 19:09 ` Andrew Lunn
2013-03-25 19:20 ` Daniel Lezcano
2013-03-25 20:28 ` Nicolas Pitre
1 sibling, 1 reply; 45+ messages in thread
From: Andrew Lunn @ 2013-03-25 19:09 UTC (permalink / raw)
To: linux-arm-kernel
> > Please could you add a comment in the code about which piece is
> > specific to ARM, because its not obvious to me. Its not like there is
> > a reference to WFI for example. It looks like this code could go in
> > drivers/cpuidle/cpuidle.c
>
> Yes, I agree. At the first glance, the code, as it is, could go in this
> file but more ARM specific code will be moved to this ARM generic code
> driver like device tree description and couple idle states. The init
> function would be more arch specific then.
Hi Daniel
There was a discussion about device tree bindings when i posted
the kirkwood cpuidle driver, now in drivers/cpuidle/cpuidle-kirkwood.c.
The conclusion was that pseudo devices, like cpuidle, do not have DT
bindings. They can check of_machine_is_compatible(), like
cpuidle-calxeda.c does, or they are platform drivers, which is what
cpuidle-kirkwood.c is.
Even if DT binding was allowed, it again should not be ARM specific.
Are coupled idle states ARM specific?
Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 19:09 ` Andrew Lunn
@ 2013-03-25 19:20 ` Daniel Lezcano
2013-03-25 19:31 ` Amit Kucheria
2013-03-25 21:42 ` Andrew Lunn
0 siblings, 2 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 19:20 UTC (permalink / raw)
To: linux-arm-kernel
On 03/25/2013 08:09 PM, Andrew Lunn wrote:
>>> Please could you add a comment in the code about which piece is
>>> specific to ARM, because its not obvious to me. Its not like there is
>>> a reference to WFI for example. It looks like this code could go in
>>> drivers/cpuidle/cpuidle.c
>>
>> Yes, I agree. At the first glance, the code, as it is, could go in this
>> file but more ARM specific code will be moved to this ARM generic code
>> driver like device tree description and couple idle states. The init
>> function would be more arch specific then.
>
> Hi Daniel
>
> There was a discussion about device tree bindings when i posted
> the kirkwood cpuidle driver, now in drivers/cpuidle/cpuidle-kirkwood.c.
>
> The conclusion was that pseudo devices, like cpuidle, do not have DT
> bindings. They can check of_machine_is_compatible(), like
> cpuidle-calxeda.c does, or they are platform drivers, which is what
> cpuidle-kirkwood.c is.
>
> Even if DT binding was allowed, it again should not be ARM specific.
If the DT binding was allowed, I *may* not be ARM specific but will
certainly used only by the ARM drivers as the x86 platform uses ACPI or
static tables.
> Are coupled idle states ARM specific?
Well the code is not arch specific but today the idle coupling is ARM
specific because it is the only arch using this kind of synchronization.
There is also a last man standing algorithm common to ux500 and imx
(maybe exynos soon) I would like to merge into this ARM driver.
AFAIK, the cluster shutdown is managed by the hardware on x86 so there
is no need for 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] 45+ messages in thread
* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 19:20 ` Daniel Lezcano
@ 2013-03-25 19:31 ` Amit Kucheria
2013-03-25 19:46 ` Daniel Lezcano
2013-03-25 21:42 ` Andrew Lunn
1 sibling, 1 reply; 45+ messages in thread
From: Amit Kucheria @ 2013-03-25 19:31 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 26, 2013 at 12:50 AM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> On 03/25/2013 08:09 PM, Andrew Lunn wrote:
>>>> Please could you add a comment in the code about which piece is
>>>> specific to ARM, because its not obvious to me. Its not like there is
>>>> a reference to WFI for example. It looks like this code could go in
>>>> drivers/cpuidle/cpuidle.c
>>>
>>> Yes, I agree. At the first glance, the code, as it is, could go in this
>>> file but more ARM specific code will be moved to this ARM generic code
>>> driver like device tree description and couple idle states. The init
>>> function would be more arch specific then.
>>
>> Hi Daniel
>>
>> There was a discussion about device tree bindings when i posted
>> the kirkwood cpuidle driver, now in drivers/cpuidle/cpuidle-kirkwood.c.
>>
>> The conclusion was that pseudo devices, like cpuidle, do not have DT
>> bindings. They can check of_machine_is_compatible(), like
>> cpuidle-calxeda.c does, or they are platform drivers, which is what
>> cpuidle-kirkwood.c is.
>>
>> Even if DT binding was allowed, it again should not be ARM specific.
>
> If the DT binding was allowed, I *may* not be ARM specific but will
> certainly used only by the ARM drivers as the x86 platform uses ACPI or
> static tables.
>
>> Are coupled idle states ARM specific?
>
> Well the code is not arch specific but today the idle coupling is ARM
> specific because it is the only arch using this kind of synchronization.
> There is also a last man standing algorithm common to ux500 and imx
> (maybe exynos soon) I would like to merge into this ARM driver.
Nico has developed a last man standing algorithm[1] for big.LITTLE
TC2. That too needs to be considered during this consolidation. While
it was developed for multi-cluster configurations, I don't see what it
shouldn't work here too.
[1] http://lwn.net/Articles/539082/
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 19:31 ` Amit Kucheria
@ 2013-03-25 19:46 ` Daniel Lezcano
0 siblings, 0 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 19:46 UTC (permalink / raw)
To: linux-arm-kernel
On 03/25/2013 08:31 PM, Amit Kucheria wrote:
> On Tue, Mar 26, 2013 at 12:50 AM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>> On 03/25/2013 08:09 PM, Andrew Lunn wrote:
>>>>> Please could you add a comment in the code about which piece is
>>>>> specific to ARM, because its not obvious to me. Its not like there is
>>>>> a reference to WFI for example. It looks like this code could go in
>>>>> drivers/cpuidle/cpuidle.c
>>>>
>>>> Yes, I agree. At the first glance, the code, as it is, could go in this
>>>> file but more ARM specific code will be moved to this ARM generic code
>>>> driver like device tree description and couple idle states. The init
>>>> function would be more arch specific then.
>>>
>>> Hi Daniel
>>>
>>> There was a discussion about device tree bindings when i posted
>>> the kirkwood cpuidle driver, now in drivers/cpuidle/cpuidle-kirkwood.c.
>>>
>>> The conclusion was that pseudo devices, like cpuidle, do not have DT
>>> bindings. They can check of_machine_is_compatible(), like
>>> cpuidle-calxeda.c does, or they are platform drivers, which is what
>>> cpuidle-kirkwood.c is.
>>>
>>> Even if DT binding was allowed, it again should not be ARM specific.
>>
>> If the DT binding was allowed, I *may* not be ARM specific but will
>> certainly used only by the ARM drivers as the x86 platform uses ACPI or
>> static tables.
>>
>>> Are coupled idle states ARM specific?
>>
>> Well the code is not arch specific but today the idle coupling is ARM
>> specific because it is the only arch using this kind of synchronization.
>> There is also a last man standing algorithm common to ux500 and imx
>> (maybe exynos soon) I would like to merge into this ARM driver.
>
> Nico has developed a last man standing algorithm[1] for big.LITTLE
> TC2. That too needs to be considered during this consolidation. While
> it was developed for multi-cluster configurations, I don't see what it
> shouldn't work here too.
>
> [1] http://lwn.net/Articles/539082/
I had it in mind when answering but I didn't mention it.
Thanks Amit for the pointer.
-- 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] 45+ messages in thread
* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 19:20 ` Daniel Lezcano
2013-03-25 19:31 ` Amit Kucheria
@ 2013-03-25 21:42 ` Andrew Lunn
2013-03-25 22:09 ` Daniel Lezcano
1 sibling, 1 reply; 45+ messages in thread
From: Andrew Lunn @ 2013-03-25 21:42 UTC (permalink / raw)
To: linux-arm-kernel
> If the DT binding was allowed, I *may* not be ARM specific but will
> certainly used only by the ARM drivers as the x86 platform uses ACPI or
> static tables.
And powerpc? Its powerpc that created DT, as far as i understand.
arch/powerpc/platforms/pseries/processor_idle.c
static int pseries_idle_devices_init(void)
{
int i;
struct cpuidle_driver *drv = &pseries_idle_driver;
struct cpuidle_device *dev;
pseries_cpuidle_devices = alloc_percpu(struct cpuidle_device);
if (pseries_cpuidle_devices == NULL)
return -ENOMEM;
for_each_possible_cpu(i) {
dev = per_cpu_ptr(pseries_cpuidle_devices, i);
dev->state_count = drv->state_count;
dev->cpu = i;
if (cpuidle_register_device(dev)) {
printk(KERN_DEBUG \
"cpuidle_register_device %d failed!\n", i);
return -EIO;
}
}
return 0;
}
This looks pretty similar to the code you are consolidating. Can your
'ARM' code be made to work on powerpc?
Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 21:42 ` Andrew Lunn
@ 2013-03-25 22:09 ` Daniel Lezcano
0 siblings, 0 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 22:09 UTC (permalink / raw)
To: linux-arm-kernel
On 03/25/2013 10:42 PM, Andrew Lunn wrote:
>> If the DT binding was allowed, I *may* not be ARM specific but will
>> certainly used only by the ARM drivers as the x86 platform uses ACPI or
>> static tables.
>
> And powerpc? Its powerpc that created DT, as far as i understand.
>
> arch/powerpc/platforms/pseries/processor_idle.c
>
> static int pseries_idle_devices_init(void)
> {
> int i;
> struct cpuidle_driver *drv = &pseries_idle_driver;
> struct cpuidle_device *dev;
>
> pseries_cpuidle_devices = alloc_percpu(struct cpuidle_device);
> if (pseries_cpuidle_devices == NULL)
> return -ENOMEM;
>
> for_each_possible_cpu(i) {
> dev = per_cpu_ptr(pseries_cpuidle_devices, i);
> dev->state_count = drv->state_count;
> dev->cpu = i;
> if (cpuidle_register_device(dev)) {
> printk(KERN_DEBUG \
> "cpuidle_register_device %d failed!\n", i);
> return -EIO;
> }
> }
>
> return 0;
> }
>
>
> This looks pretty similar to the code you are consolidating. Can your
> 'ARM' code be made to work on powerpc?
Yes, it is very similar. I am aware of this code and the cpuidle code of
all others archs but for now there are *18* cpuidle drivers for ARM I
would like to consolidate in priority into a single one. When all of
them will be factored out, I will recheck with the other arch. That will
be easier to consolidate four archs: x86, arm, sh and powerpc.
May be in the meantime, someone will cleanup the non-ARM drivers and
make my life easier :)
In any case, I keep in mind there are other arch using cpuidle.
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 18:33 ` Daniel Lezcano
2013-03-25 19:09 ` Andrew Lunn
@ 2013-03-25 20:28 ` Nicolas Pitre
2013-03-25 21:53 ` Daniel Lezcano
1 sibling, 1 reply; 45+ messages in thread
From: Nicolas Pitre @ 2013-03-25 20:28 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 25 Mar 2013, Daniel Lezcano wrote:
> On 03/25/2013 07:10 PM, Andrew Lunn wrote:
> > On Mon, Mar 25, 2013 at 06:55:32PM +0100, Daniel Lezcano wrote:
> >> The init and exit routine for most of the drivers are the same,
> >> that is register the driver and register the device.
> >>
> >> Provide a common function to do that in the cpuidle driver for ARM,
> >> so we can get rid of a lot of code duplication in the different SOC
> >> cpuidle drivers.
> >
> > Hi Daniel
> >
> > Please could you add a comment in the code about which piece is
> > specific to ARM, because its not obvious to me. Its not like there is
> > a reference to WFI for example. It looks like this code could go in
> > drivers/cpuidle/cpuidle.c
>
> Yes, I agree. At the first glance, the code, as it is, could go in this
> file but more ARM specific code will be moved to this ARM generic code
> driver like device tree description and couple idle states. The init
> function would be more arch specific then.
>
> For this reason, I think it is reasonable to move to
> arm/kernel/cpuidle.c rather than drivers/cpuidle/cpuidle.c first.
No.
Please move things under drivers/cpuidle/ upfront.
We do want drivers to be gathered according to their purpose and
subsystem, not according to the architecture they belong to.
This is a simple maintenance optimization to do so.
The reason is when someone wishes to improve the subsystem then that
someone who might know nothing about the ARM or other architectures
won't have to look for obscure drivers buried into arch specific
directories. When things are gathered under a common directory it is
then much easier to perform wide ranging changes to a subsystem.
And for those who do know the ARM architecture and wish to modify the
ARM driver it is not that hard to locate the driver once.
The "downside" for you might be that the activity under drivers/cpuidle/
is more closely scrutinized by more people. But that isn't a bad thing
either.
> In the future, when all the ARM cpuidle driver will be fully
> consolidated, that will be easier to identify the common parts across
> the different arch and then move them to the generic framework.
Nothing prevents you from doing that consolidation work right in
drivers/cpuidle/.
Nicolas
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 20:28 ` Nicolas Pitre
@ 2013-03-25 21:53 ` Daniel Lezcano
2013-03-25 21:57 ` Nicolas Pitre
0 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 21:53 UTC (permalink / raw)
To: linux-arm-kernel
On 03/25/2013 09:28 PM, Nicolas Pitre wrote:
> On Mon, 25 Mar 2013, Daniel Lezcano wrote:
>
>> On 03/25/2013 07:10 PM, Andrew Lunn wrote:
>>> On Mon, Mar 25, 2013 at 06:55:32PM +0100, Daniel Lezcano wrote:
>>>> The init and exit routine for most of the drivers are the same,
>>>> that is register the driver and register the device.
>>>>
>>>> Provide a common function to do that in the cpuidle driver for ARM,
>>>> so we can get rid of a lot of code duplication in the different SOC
>>>> cpuidle drivers.
>>>
>>> Hi Daniel
>>>
>>> Please could you add a comment in the code about which piece is
>>> specific to ARM, because its not obvious to me. Its not like there is
>>> a reference to WFI for example. It looks like this code could go in
>>> drivers/cpuidle/cpuidle.c
>>
>> Yes, I agree. At the first glance, the code, as it is, could go in this
>> file but more ARM specific code will be moved to this ARM generic code
>> driver like device tree description and couple idle states. The init
>> function would be more arch specific then.
>>
>> For this reason, I think it is reasonable to move to
>> arm/kernel/cpuidle.c rather than drivers/cpuidle/cpuidle.c first.
>
> No.
>
> Please move things under drivers/cpuidle/ upfront.
>
> We do want drivers to be gathered according to their purpose and
> subsystem, not according to the architecture they belong to.
>
> This is a simple maintenance optimization to do so.
>
> The reason is when someone wishes to improve the subsystem then that
> someone who might know nothing about the ARM or other architectures
> won't have to look for obscure drivers buried into arch specific
> directories. When things are gathered under a common directory it is
> then much easier to perform wide ranging changes to a subsystem.
>
> And for those who do know the ARM architecture and wish to modify the
> ARM driver it is not that hard to locate the driver once.
>
> The "downside" for you might be that the activity under drivers/cpuidle/
> is more closely scrutinized by more people. But that isn't a bad thing
> either.
>
>> In the future, when all the ARM cpuidle driver will be fully
>> consolidated, that will be easier to identify the common parts across
>> the different arch and then move them to the generic framework.
>
> Nothing prevents you from doing that consolidation work right in
> drivers/cpuidle/.
I fully agree with you but I think there is a misunderstanding.
The idea is to consolidate the ARM code in the ARM cpuidle driver which
is mostly empty. The init/exit functions could falsely lead someone to
think they could be moved in the generic framework but IMO it is too
soon because the code consolidation will bring some arch specificity and
in this case we will going back and forth. I want to avoid that.
Let me consolidate the ARM driver, cleanup the headers to split the arch
specific code from the rest and then move this driver to
drivers/cpuidle. This is just a question of a week.
--
<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] 45+ messages in thread
* [PATCH 07/15] ARM: cpuidle: add init/exit routine
2013-03-25 21:53 ` Daniel Lezcano
@ 2013-03-25 21:57 ` Nicolas Pitre
0 siblings, 0 replies; 45+ messages in thread
From: Nicolas Pitre @ 2013-03-25 21:57 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 25 Mar 2013, Daniel Lezcano wrote:
> On 03/25/2013 09:28 PM, Nicolas Pitre wrote:
> > On Mon, 25 Mar 2013, Daniel Lezcano wrote:
> >
> >> In the future, when all the ARM cpuidle driver will be fully
> >> consolidated, that will be easier to identify the common parts across
> >> the different arch and then move them to the generic framework.
> >
> > Nothing prevents you from doing that consolidation work right in
> > drivers/cpuidle/.
>
> I fully agree with you but I think there is a misunderstanding.
>
> The idea is to consolidate the ARM code in the ARM cpuidle driver which
> is mostly empty. The init/exit functions could falsely lead someone to
> think they could be moved in the generic framework but IMO it is too
> soon because the code consolidation will bring some arch specificity and
> in this case we will going back and forth. I want to avoid that.
>
> Let me consolidate the ARM driver, cleanup the headers to split the arch
> specific code from the rest and then move this driver to
> drivers/cpuidle. This is just a question of a week.
OK.
Nicolas
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 08/15] ARM: ux500: cpuidle: use init/exit common routine
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (6 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 07/15] ARM: cpuidle: add init/exit routine Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-27 14:47 ` Linus Walleij
2013-03-25 17:55 ` [PATCH 09/15] ARM: at91: " Daniel Lezcano
` (7 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
Remove the duplicate code and use the arm cpuidle driver's common code
for initialization.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-ux500/cpuidle.c | 32 +-------------------------------
1 file changed, 1 insertion(+), 31 deletions(-)
diff --git a/arch/arm/mach-ux500/cpuidle.c b/arch/arm/mach-ux500/cpuidle.c
index 1b16d9e..ee81d22 100644
--- a/arch/arm/mach-ux500/cpuidle.c
+++ b/arch/arm/mach-ux500/cpuidle.c
@@ -21,7 +21,6 @@
static atomic_t master = ATOMIC_INIT(0);
static DEFINE_SPINLOCK(master_lock);
-static DEFINE_PER_CPU(struct cpuidle_device, ux500_cpuidle_device);
static inline int ux500_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)
@@ -113,40 +112,11 @@ static struct cpuidle_driver ux500_idle_driver = {
int __init ux500_idle_init(void)
{
- int ret, cpu;
- struct cpuidle_device *device;
-
/* Configure wake up reasons */
prcmu_enable_wakeups(PRCMU_WAKEUP(ARM) | PRCMU_WAKEUP(RTC) |
PRCMU_WAKEUP(ABB));
- ret = cpuidle_register_driver(&ux500_idle_driver);
- if (ret) {
- printk(KERN_ERR "failed to register ux500 idle driver\n");
- return ret;
- }
-
- for_each_online_cpu(cpu) {
- device = &per_cpu(ux500_cpuidle_device, cpu);
- device->cpu = cpu;
- ret = cpuidle_register_device(device);
- if (ret) {
- printk(KERN_ERR "Failed to register cpuidle "
- "device for cpu%d\n", cpu);
- goto out_unregister;
- }
- }
-out:
- return ret;
-
-out_unregister:
- for_each_online_cpu(cpu) {
- device = &per_cpu(ux500_cpuidle_device, cpu);
- cpuidle_unregister_device(device);
- }
-
- cpuidle_unregister_driver(&ux500_idle_driver);
- goto out;
+ return arm_cpuidle_init(&ux500_idle_driver);
}
device_initcall(ux500_idle_init);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 09/15] ARM: at91: cpuidle: use init/exit common routine
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (7 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 08/15] ARM: ux500: cpuidle: use init/exit common routine Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-26 8:48 ` Nicolas Ferre
2013-03-25 17:55 ` [PATCH 10/15] ARM: OMAP3: " Daniel Lezcano
` (6 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
Remove the duplicate code and use the arm cpuidle driver's common code
for initialization.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-at91/cpuidle.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index 0c63815..0aa2c0f 100644
--- a/arch/arm/mach-at91/cpuidle.c
+++ b/arch/arm/mach-at91/cpuidle.c
@@ -27,8 +27,6 @@
#define AT91_MAX_STATES 2
-static DEFINE_PER_CPU(struct cpuidle_device, at91_cpuidle_device);
-
/* Actual code that puts the SoC in different idle states */
static int at91_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
@@ -61,20 +59,9 @@ static struct cpuidle_driver at91_idle_driver = {
};
/* Initialize CPU idle by registering the idle states */
-static int at91_init_cpuidle(void)
+static int __init at91_init_cpuidle(void)
{
- struct cpuidle_device *device;
-
- device = &per_cpu(at91_cpuidle_device, smp_processor_id());
- device->state_count = AT91_MAX_STATES;
-
- cpuidle_register_driver(&at91_idle_driver);
-
- if (cpuidle_register_device(device)) {
- printk(KERN_ERR "at91_init_cpuidle: Failed registering\n");
- return -EIO;
- }
- return 0;
+ return arm_cpuidle_init(&at91_idle_driver);
}
device_initcall(at91_init_cpuidle);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 09/15] ARM: at91: cpuidle: use init/exit common routine
2013-03-25 17:55 ` [PATCH 09/15] ARM: at91: " Daniel Lezcano
@ 2013-03-26 8:48 ` Nicolas Ferre
0 siblings, 0 replies; 45+ messages in thread
From: Nicolas Ferre @ 2013-03-26 8:48 UTC (permalink / raw)
To: linux-arm-kernel
On 03/25/2013 06:55 PM, Daniel Lezcano :
> Remove the duplicate code and use the arm cpuidle driver's common code
> for initialization.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
No objection for at91:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> arch/arm/mach-at91/cpuidle.c | 17 ++---------------
> 1 file changed, 2 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
> index 0c63815..0aa2c0f 100644
> --- a/arch/arm/mach-at91/cpuidle.c
> +++ b/arch/arm/mach-at91/cpuidle.c
> @@ -27,8 +27,6 @@
>
> #define AT91_MAX_STATES 2
>
> -static DEFINE_PER_CPU(struct cpuidle_device, at91_cpuidle_device);
> -
> /* Actual code that puts the SoC in different idle states */
> static int at91_enter_idle(struct cpuidle_device *dev,
> struct cpuidle_driver *drv,
> @@ -61,20 +59,9 @@ static struct cpuidle_driver at91_idle_driver = {
> };
>
> /* Initialize CPU idle by registering the idle states */
> -static int at91_init_cpuidle(void)
> +static int __init at91_init_cpuidle(void)
> {
> - struct cpuidle_device *device;
> -
> - device = &per_cpu(at91_cpuidle_device, smp_processor_id());
> - device->state_count = AT91_MAX_STATES;
> -
> - cpuidle_register_driver(&at91_idle_driver);
> -
> - if (cpuidle_register_device(device)) {
> - printk(KERN_ERR "at91_init_cpuidle: Failed registering\n");
> - return -EIO;
> - }
> - return 0;
> + return arm_cpuidle_init(&at91_idle_driver);
> }
>
> device_initcall(at91_init_cpuidle);
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 10/15] ARM: OMAP3: cpuidle: use init/exit common routine
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (8 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 09/15] ARM: at91: " Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-25 17:55 ` [PATCH 11/15] ARM: s3c64xx: " Daniel Lezcano
` (5 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
Remove the duplicate code and use the arm cpuidle driver's common code
for initialization.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-omap2/cpuidle34xx.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 80392fc..b5a0ba1 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -26,6 +26,7 @@
#include <linux/cpuidle.h>
#include <linux/export.h>
#include <linux/cpu_pm.h>
+#include <asm/cpuidle.h>
#include "powerdomain.h"
#include "clockdomain.h"
@@ -271,8 +272,6 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
return ret;
}
-static DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
-
static struct cpuidle_driver omap3_idle_driver = {
.name = "omap3_idle",
.owner = THIS_MODULE,
@@ -348,8 +347,6 @@ static struct cpuidle_driver omap3_idle_driver = {
*/
int __init omap3_idle_init(void)
{
- struct cpuidle_device *dev;
-
mpu_pd = pwrdm_lookup("mpu_pwrdm");
core_pd = pwrdm_lookup("core_pwrdm");
per_pd = pwrdm_lookup("per_pwrdm");
@@ -358,16 +355,5 @@ int __init omap3_idle_init(void)
if (!mpu_pd || !core_pd || !per_pd || !cam_pd)
return -ENODEV;
- cpuidle_register_driver(&omap3_idle_driver);
-
- dev = &per_cpu(omap3_idle_dev, smp_processor_id());
- dev->cpu = 0;
-
- if (cpuidle_register_device(dev)) {
- printk(KERN_ERR "%s: CPUidle register device failed\n",
- __func__);
- return -EIO;
- }
-
- return 0;
+ return arm_cpuidle_init(&omap3_idle_driver);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 11/15] ARM: s3c64xx: cpuidle: use init/exit common routine
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (9 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 10/15] ARM: OMAP3: " Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-25 17:55 ` [PATCH 12/15] ARM: tegra1: " Daniel Lezcano
` (4 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
Remove the duplicate code and use the arm cpuidle driver's common code
for initialization.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-s3c64xx/cpuidle.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/cpuidle.c b/arch/arm/mach-s3c64xx/cpuidle.c
index ead5fab..ce56142 100644
--- a/arch/arm/mach-s3c64xx/cpuidle.c
+++ b/arch/arm/mach-s3c64xx/cpuidle.c
@@ -16,6 +16,7 @@
#include <linux/export.h>
#include <linux/time.h>
+#include <asm/cpuidle.h>
#include <asm/proc-fns.h>
#include <mach/map.h>
@@ -40,8 +41,6 @@ static int s3c64xx_enter_idle(struct cpuidle_device *dev,
return index;
}
-static DEFINE_PER_CPU(struct cpuidle_device, s3c64xx_cpuidle_device);
-
static struct cpuidle_driver s3c64xx_cpuidle_driver = {
.name = "s3c64xx_cpuidle",
.owner = THIS_MODULE,
@@ -61,16 +60,6 @@ static struct cpuidle_driver s3c64xx_cpuidle_driver = {
static int __init s3c64xx_init_cpuidle(void)
{
- int ret;
-
- cpuidle_register_driver(&s3c64xx_cpuidle_driver);
-
- ret = cpuidle_register_device(&s3c64xx_cpuidle_device);
- if (ret) {
- pr_err("Failed to register cpuidle device: %d\n", ret);
- return ret;
- }
-
- return 0;
+ return arm_cpuidle_init(&s3c64xx_cpuidle_driver);
}
device_initcall(s3c64xx_init_cpuidle);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 12/15] ARM: tegra1: cpuidle: use init/exit common routine
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (10 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 11/15] ARM: s3c64xx: " Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-25 17:55 ` [PATCH 13/15] ARM: shmobile: pm: fix init sections Daniel Lezcano
` (3 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
Remove the duplicate code and use the arm cpuidle driver's common code
for initialization.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-tegra/cpuidle-tegra114.c | 27 +--------------------------
1 file changed, 1 insertion(+), 26 deletions(-)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
index 0f4e8c4..771f6e8 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
@@ -30,32 +30,7 @@ static struct cpuidle_driver tegra_idle_driver = {
},
};
-static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
-
int __init tegra114_cpuidle_init(void)
{
- int ret;
- unsigned int cpu;
- struct cpuidle_device *dev;
- struct cpuidle_driver *drv = &tegra_idle_driver;
-
- ret = cpuidle_register_driver(&tegra_idle_driver);
- if (ret) {
- pr_err("CPUidle driver registration failed\n");
- return ret;
- }
-
- for_each_possible_cpu(cpu) {
- dev = &per_cpu(tegra_idle_device, cpu);
- dev->cpu = cpu;
-
- dev->state_count = drv->state_count;
- ret = cpuidle_register_device(dev);
- if (ret) {
- pr_err("CPU%u: CPUidle device registration failed\n",
- cpu);
- return ret;
- }
- }
- return 0;
+ return arm_cpuidle_init(&tegra_idle_driver);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 13/15] ARM: shmobile: pm: fix init sections
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (11 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 12/15] ARM: tegra1: " Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-27 14:09 ` Simon Horman
2013-03-25 17:55 ` [PATCH 14/15] ARM: shmobile: cpuidle: remove useless WFI function Daniel Lezcano
` (2 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
Add the __init section for the functions which are called
at init time.
Signed-off-by: Daniel Lezcano <daniel.linaro.org>
---
arch/arm/mach-shmobile/cpuidle.c | 4 ++--
arch/arm/mach-shmobile/pm-sh7372.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
index 9e05026..068f9ca 100644
--- a/arch/arm/mach-shmobile/cpuidle.c
+++ b/arch/arm/mach-shmobile/cpuidle.c
@@ -36,12 +36,12 @@ static struct cpuidle_driver shmobile_cpuidle_default_driver = {
static struct cpuidle_driver *cpuidle_drv = &shmobile_cpuidle_default_driver;
-void shmobile_cpuidle_set_driver(struct cpuidle_driver *drv)
+void __init shmobile_cpuidle_set_driver(struct cpuidle_driver *drv)
{
cpuidle_drv = drv;
}
-int shmobile_cpuidle_init(void)
+int __init shmobile_cpuidle_init(void)
{
struct cpuidle_device *dev = &shmobile_cpuidle_dev;
diff --git a/arch/arm/mach-shmobile/pm-sh7372.c b/arch/arm/mach-shmobile/pm-sh7372.c
index a0826a4..f6b14ca 100644
--- a/arch/arm/mach-shmobile/pm-sh7372.c
+++ b/arch/arm/mach-shmobile/pm-sh7372.c
@@ -450,12 +450,12 @@ static struct cpuidle_driver sh7372_cpuidle_driver = {
},
};
-static void sh7372_cpuidle_init(void)
+static void __init sh7372_cpuidle_init(void)
{
shmobile_cpuidle_set_driver(&sh7372_cpuidle_driver);
}
#else
-static void sh7372_cpuidle_init(void) {}
+static void __init sh7372_cpuidle_init(void) {}
#endif
#ifdef CONFIG_SUSPEND
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 13/15] ARM: shmobile: pm: fix init sections
2013-03-25 17:55 ` [PATCH 13/15] ARM: shmobile: pm: fix init sections Daniel Lezcano
@ 2013-03-27 14:09 ` Simon Horman
0 siblings, 0 replies; 45+ messages in thread
From: Simon Horman @ 2013-03-27 14:09 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 25, 2013 at 06:55:38PM +0100, Daniel Lezcano wrote:
> Add the __init section for the functions which are called
> at init time.
>
> Signed-off-by: Daniel Lezcano <daniel.linaro.org>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> arch/arm/mach-shmobile/cpuidle.c | 4 ++--
> arch/arm/mach-shmobile/pm-sh7372.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
> index 9e05026..068f9ca 100644
> --- a/arch/arm/mach-shmobile/cpuidle.c
> +++ b/arch/arm/mach-shmobile/cpuidle.c
> @@ -36,12 +36,12 @@ static struct cpuidle_driver shmobile_cpuidle_default_driver = {
>
> static struct cpuidle_driver *cpuidle_drv = &shmobile_cpuidle_default_driver;
>
> -void shmobile_cpuidle_set_driver(struct cpuidle_driver *drv)
> +void __init shmobile_cpuidle_set_driver(struct cpuidle_driver *drv)
> {
> cpuidle_drv = drv;
> }
>
> -int shmobile_cpuidle_init(void)
> +int __init shmobile_cpuidle_init(void)
> {
> struct cpuidle_device *dev = &shmobile_cpuidle_dev;
>
> diff --git a/arch/arm/mach-shmobile/pm-sh7372.c b/arch/arm/mach-shmobile/pm-sh7372.c
> index a0826a4..f6b14ca 100644
> --- a/arch/arm/mach-shmobile/pm-sh7372.c
> +++ b/arch/arm/mach-shmobile/pm-sh7372.c
> @@ -450,12 +450,12 @@ static struct cpuidle_driver sh7372_cpuidle_driver = {
> },
> };
>
> -static void sh7372_cpuidle_init(void)
> +static void __init sh7372_cpuidle_init(void)
> {
> shmobile_cpuidle_set_driver(&sh7372_cpuidle_driver);
> }
> #else
> -static void sh7372_cpuidle_init(void) {}
> +static void __init sh7372_cpuidle_init(void) {}
> #endif
>
> #ifdef CONFIG_SUSPEND
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 14/15] ARM: shmobile: cpuidle: remove useless WFI function
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (12 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 13/15] ARM: shmobile: pm: fix init sections Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-27 14:09 ` Simon Horman
2013-03-25 17:55 ` [PATCH 15/15] ARM: shmobile: cpuidle: use init/exit common routine Daniel Lezcano
2013-03-25 20:27 ` [PATCH 00/15] ARM: cpuidle: code consolidation Rob Herring
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
Remove the shmobile_enter_wfi function which is the same as the
common WFI enter function from the arm cpuidle driver defined
with the ARM_CPUIDLE_WFI_STATE macro.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-shmobile/cpuidle.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
index 068f9ca..c872ae8 100644
--- a/arch/arm/mach-shmobile/cpuidle.c
+++ b/arch/arm/mach-shmobile/cpuidle.c
@@ -16,20 +16,12 @@
#include <asm/cpuidle.h>
#include <asm/io.h>
-int shmobile_enter_wfi(struct cpuidle_device *dev, struct cpuidle_driver *drv,
- int index)
-{
- cpu_do_idle();
- return 0;
-}
-
static struct cpuidle_device shmobile_cpuidle_dev;
static struct cpuidle_driver shmobile_cpuidle_default_driver = {
.name = "shmobile_cpuidle",
.owner = THIS_MODULE,
.en_core_tk_irqen = 1,
.states[0] = ARM_CPUIDLE_WFI_STATE,
- .states[0].enter = shmobile_enter_wfi,
.safe_state_index = 0, /* C1 */
.state_count = 1,
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 14/15] ARM: shmobile: cpuidle: remove useless WFI function
2013-03-25 17:55 ` [PATCH 14/15] ARM: shmobile: cpuidle: remove useless WFI function Daniel Lezcano
@ 2013-03-27 14:09 ` Simon Horman
0 siblings, 0 replies; 45+ messages in thread
From: Simon Horman @ 2013-03-27 14:09 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 25, 2013 at 06:55:39PM +0100, Daniel Lezcano wrote:
> Remove the shmobile_enter_wfi function which is the same as the
> common WFI enter function from the arm cpuidle driver defined
> with the ARM_CPUIDLE_WFI_STATE macro.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> arch/arm/mach-shmobile/cpuidle.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
> index 068f9ca..c872ae8 100644
> --- a/arch/arm/mach-shmobile/cpuidle.c
> +++ b/arch/arm/mach-shmobile/cpuidle.c
> @@ -16,20 +16,12 @@
> #include <asm/cpuidle.h>
> #include <asm/io.h>
>
> -int shmobile_enter_wfi(struct cpuidle_device *dev, struct cpuidle_driver *drv,
> - int index)
> -{
> - cpu_do_idle();
> - return 0;
> -}
> -
> static struct cpuidle_device shmobile_cpuidle_dev;
> static struct cpuidle_driver shmobile_cpuidle_default_driver = {
> .name = "shmobile_cpuidle",
> .owner = THIS_MODULE,
> .en_core_tk_irqen = 1,
> .states[0] = ARM_CPUIDLE_WFI_STATE,
> - .states[0].enter = shmobile_enter_wfi,
> .safe_state_index = 0, /* C1 */
> .state_count = 1,
> };
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 15/15] ARM: shmobile: cpuidle: use init/exit common routine
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (13 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 14/15] ARM: shmobile: cpuidle: remove useless WFI function Daniel Lezcano
@ 2013-03-25 17:55 ` Daniel Lezcano
2013-03-27 14:10 ` Simon Horman
2013-03-25 20:27 ` [PATCH 00/15] ARM: cpuidle: code consolidation Rob Herring
15 siblings, 1 reply; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
Remove the duplicate code and use the arm cpuidle driver's common code
for initialization.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
arch/arm/mach-shmobile/cpuidle.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
index c872ae8..9744d88 100644
--- a/arch/arm/mach-shmobile/cpuidle.c
+++ b/arch/arm/mach-shmobile/cpuidle.c
@@ -16,7 +16,6 @@
#include <asm/cpuidle.h>
#include <asm/io.h>
-static struct cpuidle_device shmobile_cpuidle_dev;
static struct cpuidle_driver shmobile_cpuidle_default_driver = {
.name = "shmobile_cpuidle",
.owner = THIS_MODULE,
@@ -35,12 +34,5 @@ void __init shmobile_cpuidle_set_driver(struct cpuidle_driver *drv)
int __init shmobile_cpuidle_init(void)
{
- struct cpuidle_device *dev = &shmobile_cpuidle_dev;
-
- cpuidle_register_driver(cpuidle_drv);
-
- dev->state_count = cpuidle_drv->state_count;
- cpuidle_register_device(dev);
-
- return 0;
+ return arm_cpuidle_init(cpuidle_drv);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 15/15] ARM: shmobile: cpuidle: use init/exit common routine
2013-03-25 17:55 ` [PATCH 15/15] ARM: shmobile: cpuidle: use init/exit common routine Daniel Lezcano
@ 2013-03-27 14:10 ` Simon Horman
0 siblings, 0 replies; 45+ messages in thread
From: Simon Horman @ 2013-03-27 14:10 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 25, 2013 at 06:55:40PM +0100, Daniel Lezcano wrote:
> Remove the duplicate code and use the arm cpuidle driver's common code
> for initialization.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> arch/arm/mach-shmobile/cpuidle.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
> index c872ae8..9744d88 100644
> --- a/arch/arm/mach-shmobile/cpuidle.c
> +++ b/arch/arm/mach-shmobile/cpuidle.c
> @@ -16,7 +16,6 @@
> #include <asm/cpuidle.h>
> #include <asm/io.h>
>
> -static struct cpuidle_device shmobile_cpuidle_dev;
> static struct cpuidle_driver shmobile_cpuidle_default_driver = {
> .name = "shmobile_cpuidle",
> .owner = THIS_MODULE,
> @@ -35,12 +34,5 @@ void __init shmobile_cpuidle_set_driver(struct cpuidle_driver *drv)
>
> int __init shmobile_cpuidle_init(void)
> {
> - struct cpuidle_device *dev = &shmobile_cpuidle_dev;
> -
> - cpuidle_register_driver(cpuidle_drv);
> -
> - dev->state_count = cpuidle_drv->state_count;
> - cpuidle_register_device(dev);
> -
> - return 0;
> + return arm_cpuidle_init(cpuidle_drv);
> }
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 00/15] ARM: cpuidle: code consolidation
2013-03-25 17:55 [PATCH 00/15] ARM: cpuidle: code consolidation Daniel Lezcano
` (14 preceding siblings ...)
2013-03-25 17:55 ` [PATCH 15/15] ARM: shmobile: cpuidle: use init/exit common routine Daniel Lezcano
@ 2013-03-25 20:27 ` Rob Herring
2013-03-25 21:55 ` Daniel Lezcano
15 siblings, 1 reply; 45+ messages in thread
From: Rob Herring @ 2013-03-25 20:27 UTC (permalink / raw)
To: linux-arm-kernel
On 03/25/2013 12:55 PM, Daniel Lezcano wrote:
> The flag CPUIDLE_FLAG_TIMER_STOP has been introduced in the commit
> 89878baa73f0f1c679355006bd8632e5d78f96c2.
>
> The flag tells the cpuidle framework the local timer will stop in the
> idle state.
>
> It is now easy to know if the cpuidle driver will use or not the broadcast
> timer by looking at the different states for this flag and then setup
> the broadcast timer consequently.
>
> When we remove the timer initialization duplicated code in the different
> drivers, we have most of the drivers with the same init function. This
> init function is changed to be generic and moved in the ARM cpuidle driver
> and used from the drivers. That cleanups code and removes a lot of annoying
> duplicated code.
>
> There is still some modification in OMAP4, tegra2, tegra3 and imx, especially
> around the coupled idle states, but we are more and more closer to a common
> squeleton for all the ARM drivers.
>
> Daniel Lezcano (15):
> timer: move enum definition out of ifdef section
> cpuidle: initialize the broadcast timer framework
> cpuidle: ux500: remove timer broadcast initialization
> cpuidle: OMAP4: remove timer broadcast initialization
> cpuidle: imx6: remove timer broadcast initialization
> ARM: cpuidle: remove useless declaration
> ARM: cpuidle: add init/exit routine
> ARM: ux500: cpuidle: use init/exit common routine
> ARM: at91: cpuidle: use init/exit common routine
> ARM: OMAP3: cpuidle: use init/exit common routine
> ARM: s3c64xx: cpuidle: use init/exit common routine
> ARM: tegra1: cpuidle: use init/exit common routine
> ARM: shmobile: pm: fix init sections
> ARM: shmobile: cpuidle: remove useless WFI function
> ARM: shmobile: cpuidle: use init/exit common routine
>
> arch/arm/include/asm/cpuidle.h | 11 +++---
> arch/arm/kernel/cpuidle.c | 57 +++++++++++++++++++++++++++++++-
> arch/arm/mach-at91/cpuidle.c | 17 ++--------
> arch/arm/mach-imx/cpuidle-imx6q.c | 15 ---------
> arch/arm/mach-omap2/cpuidle34xx.c | 18 ++--------
> arch/arm/mach-omap2/cpuidle44xx.c | 14 --------
> arch/arm/mach-s3c64xx/cpuidle.c | 15 ++-------
> arch/arm/mach-shmobile/cpuidle.c | 22 ++----------
> arch/arm/mach-shmobile/pm-sh7372.c | 4 +--
> arch/arm/mach-tegra/cpuidle-tegra114.c | 27 +--------------
> arch/arm/mach-ux500/cpuidle.c | 50 +---------------------------
> drivers/cpuidle/driver.c | 35 ++++++++++++++++++--
> include/linux/clockchips.h | 22 ++++++------
> include/linux/cpuidle.h | 2 ++
> 14 files changed, 120 insertions(+), 189 deletions(-)
What happened to the Calxeda driver?
Rob
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH 00/15] ARM: cpuidle: code consolidation
2013-03-25 20:27 ` [PATCH 00/15] ARM: cpuidle: code consolidation Rob Herring
@ 2013-03-25 21:55 ` Daniel Lezcano
0 siblings, 0 replies; 45+ messages in thread
From: Daniel Lezcano @ 2013-03-25 21:55 UTC (permalink / raw)
To: linux-arm-kernel
On 03/25/2013 09:27 PM, Rob Herring wrote:
> On 03/25/2013 12:55 PM, Daniel Lezcano wrote:
>> The flag CPUIDLE_FLAG_TIMER_STOP has been introduced in the commit
>> 89878baa73f0f1c679355006bd8632e5d78f96c2.
>>
>> The flag tells the cpuidle framework the local timer will stop in the
>> idle state.
>>
>> It is now easy to know if the cpuidle driver will use or not the broadcast
>> timer by looking at the different states for this flag and then setup
>> the broadcast timer consequently.
>>
>> When we remove the timer initialization duplicated code in the different
>> drivers, we have most of the drivers with the same init function. This
>> init function is changed to be generic and moved in the ARM cpuidle driver
>> and used from the drivers. That cleanups code and removes a lot of annoying
>> duplicated code.
>>
>> There is still some modification in OMAP4, tegra2, tegra3 and imx, especially
>> around the coupled idle states, but we are more and more closer to a common
>> squeleton for all the ARM drivers.
>>
>> Daniel Lezcano (15):
>> timer: move enum definition out of ifdef section
>> cpuidle: initialize the broadcast timer framework
>> cpuidle: ux500: remove timer broadcast initialization
>> cpuidle: OMAP4: remove timer broadcast initialization
>> cpuidle: imx6: remove timer broadcast initialization
>> ARM: cpuidle: remove useless declaration
>> ARM: cpuidle: add init/exit routine
>> ARM: ux500: cpuidle: use init/exit common routine
>> ARM: at91: cpuidle: use init/exit common routine
>> ARM: OMAP3: cpuidle: use init/exit common routine
>> ARM: s3c64xx: cpuidle: use init/exit common routine
>> ARM: tegra1: cpuidle: use init/exit common routine
>> ARM: shmobile: pm: fix init sections
>> ARM: shmobile: cpuidle: remove useless WFI function
>> ARM: shmobile: cpuidle: use init/exit common routine
>>
>> arch/arm/include/asm/cpuidle.h | 11 +++---
>> arch/arm/kernel/cpuidle.c | 57 +++++++++++++++++++++++++++++++-
>> arch/arm/mach-at91/cpuidle.c | 17 ++--------
>> arch/arm/mach-imx/cpuidle-imx6q.c | 15 ---------
>> arch/arm/mach-omap2/cpuidle34xx.c | 18 ++--------
>> arch/arm/mach-omap2/cpuidle44xx.c | 14 --------
>> arch/arm/mach-s3c64xx/cpuidle.c | 15 ++-------
>> arch/arm/mach-shmobile/cpuidle.c | 22 ++----------
>> arch/arm/mach-shmobile/pm-sh7372.c | 4 +--
>> arch/arm/mach-tegra/cpuidle-tegra114.c | 27 +--------------
>> arch/arm/mach-ux500/cpuidle.c | 50 +---------------------------
>> drivers/cpuidle/driver.c | 35 ++++++++++++++++++--
>> include/linux/clockchips.h | 22 ++++++------
>> include/linux/cpuidle.h | 2 ++
>> 14 files changed, 120 insertions(+), 189 deletions(-)
>
> What happened to the Calxeda driver?
On the way...
--
<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] 45+ messages in thread