[01/15] timer: move enum definition out of ifdef section

Message ID 1364234140-514-2-git-send-email-daniel.lezcano@linaro.org
State New
Headers show

Commit Message

Daniel Lezcano March 25, 2013, 5:55 p.m.
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(-)

Comments

Santosh Shilimkar March 26, 2013, 4:36 a.m. | #1
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
Daniel Lezcano March 26, 2013, 9:55 a.m. | #2
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.

Patch

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