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.
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 for the variable cpu which 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 | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 6634652..f9fd937 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -8,6 +8,20 @@ #ifndef _LINUX_CLOCKCHIPS_H #define _LINUX_CLOCKCHIPS_H
+/* Clock event notification values */ +enum clock_event_nofitiers { + CLOCK_EVT_NOTIFY_ADD, + CLOCK_EVT_NOTIFY_BROADCAST_ON, + CLOCK_EVT_NOTIFY_BROADCAST_OFF, + CLOCK_EVT_NOTIFY_BROADCAST_FORCE, + CLOCK_EVT_NOTIFY_BROADCAST_ENTER, + CLOCK_EVT_NOTIFY_BROADCAST_EXIT, + CLOCK_EVT_NOTIFY_SUSPEND, + CLOCK_EVT_NOTIFY_RESUME, + CLOCK_EVT_NOTIFY_CPU_DYING, + CLOCK_EVT_NOTIFY_CPU_DEAD, +}; + #ifdef CONFIG_GENERIC_CLOCKEVENTS_BUILD
#include <linux/clocksource.h> @@ -26,20 +40,6 @@ enum clock_event_mode { CLOCK_EVT_MODE_RESUME, };
-/* Clock event notification values */ -enum clock_event_nofitiers { - CLOCK_EVT_NOTIFY_ADD, - CLOCK_EVT_NOTIFY_BROADCAST_ON, - CLOCK_EVT_NOTIFY_BROADCAST_OFF, - CLOCK_EVT_NOTIFY_BROADCAST_FORCE, - CLOCK_EVT_NOTIFY_BROADCAST_ENTER, - CLOCK_EVT_NOTIFY_BROADCAST_EXIT, - CLOCK_EVT_NOTIFY_SUSPEND, - CLOCK_EVT_NOTIFY_RESUME, - CLOCK_EVT_NOTIFY_CPU_DYING, - CLOCK_EVT_NOTIFY_CPU_DEAD, -}; - /* * 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
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 | 31 +++++++++++++++++++++++++++++-- include/linux/cpuidle.h | 2 ++ 2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c index 422c7b6..8dfaaae 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,28 @@ 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 +56,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 +70,12 @@ 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;
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");
On 03/27/2013 11:22 AM, Daniel Lezcano wrote:
The initialization is done from the cpuidle framework.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
I believe as I am the author of this driver, you can assume it is acked-by myself :)
The initialization is done from the cpuidle framework.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Acked-by: Santosh Shilimkar santosh.shilimkar@ti.com --- 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;
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); }
Hi,
All 5 in this series applied to linux-pm.git/linux-next.
What am I supposed to do with patches [6-15/15] in your previous series?
Are you going to resend the ones that have been ACKed?
Rafael
On Wednesday, March 27, 2013 11:22:09 AM 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.
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 for the variable cpu which 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 | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 6634652..f9fd937 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -8,6 +8,20 @@ #ifndef _LINUX_CLOCKCHIPS_H #define _LINUX_CLOCKCHIPS_H +/* Clock event notification values */ +enum clock_event_nofitiers {
- CLOCK_EVT_NOTIFY_ADD,
- CLOCK_EVT_NOTIFY_BROADCAST_ON,
- CLOCK_EVT_NOTIFY_BROADCAST_OFF,
- CLOCK_EVT_NOTIFY_BROADCAST_FORCE,
- CLOCK_EVT_NOTIFY_BROADCAST_ENTER,
- CLOCK_EVT_NOTIFY_BROADCAST_EXIT,
- CLOCK_EVT_NOTIFY_SUSPEND,
- CLOCK_EVT_NOTIFY_RESUME,
- CLOCK_EVT_NOTIFY_CPU_DYING,
- CLOCK_EVT_NOTIFY_CPU_DEAD,
+};
#ifdef CONFIG_GENERIC_CLOCKEVENTS_BUILD #include <linux/clocksource.h> @@ -26,20 +40,6 @@ enum clock_event_mode { CLOCK_EVT_MODE_RESUME, }; -/* Clock event notification values */ -enum clock_event_nofitiers {
- CLOCK_EVT_NOTIFY_ADD,
- CLOCK_EVT_NOTIFY_BROADCAST_ON,
- CLOCK_EVT_NOTIFY_BROADCAST_OFF,
- CLOCK_EVT_NOTIFY_BROADCAST_FORCE,
- CLOCK_EVT_NOTIFY_BROADCAST_ENTER,
- CLOCK_EVT_NOTIFY_BROADCAST_EXIT,
- CLOCK_EVT_NOTIFY_SUSPEND,
- CLOCK_EVT_NOTIFY_RESUME,
- CLOCK_EVT_NOTIFY_CPU_DYING,
- CLOCK_EVT_NOTIFY_CPU_DEAD,
-};
/*
- 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
On 04/02/2013 01:24 AM, Rafael J. Wysocki wrote:
Hi,
All 5 in this series applied to linux-pm.git/linux-next.
What am I supposed to do with patches [6-15/15] in your previous series?
Are you going to resend the ones that have been ACKed?
No, please ignore them. After discussing, people want to see the init function in the cpuidle framework not in a generic ARM driver. So I am rewriting the patchset for that.
There are the cleanups before to apply [1], except [4/9] which was already fixed in the OMAP tree.
Thanks -- Daniel
[1] https://patchwork.kernel.org/patch/2365041/
On Wednesday, March 27, 2013 11:22:09 AM 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.
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 for the variable cpu which 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 | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 6634652..f9fd937 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -8,6 +8,20 @@ #ifndef _LINUX_CLOCKCHIPS_H #define _LINUX_CLOCKCHIPS_H +/* Clock event notification values */ +enum clock_event_nofitiers {
- CLOCK_EVT_NOTIFY_ADD,
- CLOCK_EVT_NOTIFY_BROADCAST_ON,
- CLOCK_EVT_NOTIFY_BROADCAST_OFF,
- CLOCK_EVT_NOTIFY_BROADCAST_FORCE,
- CLOCK_EVT_NOTIFY_BROADCAST_ENTER,
- CLOCK_EVT_NOTIFY_BROADCAST_EXIT,
- CLOCK_EVT_NOTIFY_SUSPEND,
- CLOCK_EVT_NOTIFY_RESUME,
- CLOCK_EVT_NOTIFY_CPU_DYING,
- CLOCK_EVT_NOTIFY_CPU_DEAD,
+};
#ifdef CONFIG_GENERIC_CLOCKEVENTS_BUILD #include <linux/clocksource.h> @@ -26,20 +40,6 @@ enum clock_event_mode { CLOCK_EVT_MODE_RESUME, }; -/* Clock event notification values */ -enum clock_event_nofitiers {
- CLOCK_EVT_NOTIFY_ADD,
- CLOCK_EVT_NOTIFY_BROADCAST_ON,
- CLOCK_EVT_NOTIFY_BROADCAST_OFF,
- CLOCK_EVT_NOTIFY_BROADCAST_FORCE,
- CLOCK_EVT_NOTIFY_BROADCAST_ENTER,
- CLOCK_EVT_NOTIFY_BROADCAST_EXIT,
- CLOCK_EVT_NOTIFY_SUSPEND,
- CLOCK_EVT_NOTIFY_RESUME,
- CLOCK_EVT_NOTIFY_CPU_DYING,
- CLOCK_EVT_NOTIFY_CPU_DEAD,
-};
/*
- 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
On Tuesday, April 02, 2013 06:43:38 AM Daniel Lezcano wrote:
On 04/02/2013 01:24 AM, Rafael J. Wysocki wrote:
Hi,
All 5 in this series applied to linux-pm.git/linux-next.
What am I supposed to do with patches [6-15/15] in your previous series?
Are you going to resend the ones that have been ACKed?
No, please ignore them. After discussing, people want to see the init function in the cpuidle framework not in a generic ARM driver. So I am rewriting the patchset for that.
There are the cleanups before to apply [1], except [4/9] which was already fixed in the OMAP tree.
OK, I'll ignore the rest of that patchset, then.
Thanks, Rafael
linaro-kernel@lists.linaro.org
-
Daniel Lezcano -
Rafael J. Wysocki