Hi Guys,
Here is another try to migrate clockevents to use set_dev_mode(). I have tried to fix all issues raised by Kevin/Preeti/Daniel. Also some updates are done to drivers to make sure they don't disable clockevents for unsupported modes, i.e. 'default' case.
Please do another round of review, it would be very helpful in getting these through tglx.
@Kevin: I have updated my branch again, please see if you do another round of automated tests for it.
Cover-letter for mainline:
A clockevent device can be stopped, or its events can be masked, if next event is expected at KTIME_MAX, i.e. no events are required for very long time.
This normally happens with NO_HZ_FULL (when tick-sched timer is removed and we don't have any more timers scheduled for long), not sure if it can happen otherwise.
If we don't STOP clockevent device, we are guaranteed to receive atleast one fake interrupt, as hrtimer_force_reprogram() isn't reprogramming the event device. Depending on particular implementation of clockevent device, this can be a fake interrupt at tick-rate.. (This was observed atleast on one implementation: arm_arch_timer.c).
A simple solution to get this fixed could be: update hrtimer_force_reprogram() to always reprogram clockevent device and update clockevent drivers to STOP generating events (or delay it to max time), when 'expires' is set to KTIME_MAX. But the drawback here is that every clockevent driver has to be hacked for this particular case and its very easy for new ones to miss this.
Thomas suggested to add an optional mode: CLOCK_EVT_MODE_ONESHOT_STOPPED (lkml.org/lkml/2014/5/9/508) to get tihs fixed.
This requires ->set_mode() callback to have capability to return 'success' or 'failure' for this new mode. Currently its return type is 'void'.
To change return type of set_mode() Thomas suggested to implement another callback: ->set_dev_mode(), with return type 'int'. We can then convert clockevent drivers to use this interface instead of existing ->set_mode() and then finally remove ->set_mode()'s support.
This patchset first adds another callback with return capability, set_dev_mode(), then it migrates all drivers one by one and finally removes earlier callback set_mode() when nobody is using it.
Viresh Kumar (8): clockevents: add ->set_dev_mode() to struct clock_event_device clockevents: arm: migrate to ->set_dev_mode() clockevents: mips: migrate to ->set_dev_mode() clockevents: sparc: migrate to ->set_dev_mode() clockevents: x86: migrate to ->set_dev_mode() clockevents: drivers/: migrate to ->set_dev_mode() clockevents: misc: migrate to ->set_dev_mode() clockevents: remove ->set_mode() from struct clock_event_device
arch/alpha/kernel/time.c | 32 ++++++++++++++++++---- arch/arc/kernel/time.c | 11 +++++--- arch/arm/common/timer-sp.c | 13 +++++---- arch/arm/kernel/smp_twd.c | 10 ++++--- arch/arm/mach-at91/at91rm9200_time.c | 10 +++++-- arch/arm/mach-at91/at91sam926x_time.c | 11 ++++---- arch/arm/mach-clps711x/common.c | 12 ++++----- arch/arm/mach-cns3xxx/core.c | 10 ++++--- arch/arm/mach-davinci/time.c | 7 +++-- arch/arm/mach-footbridge/dc21285-timer.c | 7 +++-- arch/arm/mach-gemini/time.c | 7 ++--- arch/arm/mach-imx/epit.c | 20 ++++++++------ arch/arm/mach-imx/time.c | 20 ++++++++------ arch/arm/mach-integrator/integrator_ap.c | 16 +++++------ arch/arm/mach-ixp4xx/common.c | 8 +++--- arch/arm/mach-ks8695/time.c | 22 ++++++++++----- arch/arm/mach-lpc32xx/timer.c | 11 ++++---- arch/arm/mach-mmp/time.c | 12 +++++---- arch/arm/mach-netx/time.c | 16 +++++------ arch/arm/mach-omap1/time.c | 7 +++-- arch/arm/mach-omap1/timer32k.c | 11 ++++---- arch/arm/mach-omap2/timer.c | 12 +++++---- arch/arm/mach-pxa/time.c | 8 +++--- arch/arm/mach-sa1100/time.c | 8 +++--- arch/arm/mach-spear/time.c | 20 +++++++------- arch/arm/mach-w90x900/time.c | 7 +++-- arch/arm/plat-iop/time.c | 8 +++--- arch/arm/plat-orion/time.c | 20 ++++++++++---- arch/avr32/kernel/time.c | 7 ++--- arch/blackfin/kernel/time-ts.c | 14 +++++++--- arch/c6x/platforms/timer64.c | 7 +++-- arch/hexagon/kernel/time.c | 11 +++++--- arch/m68k/platform/coldfire/pit.c | 7 +++-- arch/microblaze/kernel/timer.c | 7 +++-- arch/mips/alchemy/common/time.c | 14 ++++++++-- arch/mips/include/asm/cevt-r4k.h | 2 +- arch/mips/jazz/irq.c | 14 ++++++++-- arch/mips/jz4740/time.c | 9 ++++--- arch/mips/kernel/cevt-bcm1480.c | 11 +++++--- arch/mips/kernel/cevt-ds1287.c | 12 ++++++--- arch/mips/kernel/cevt-gic.c | 14 ++++++++-- arch/mips/kernel/cevt-gt641xx.c | 12 ++++++--- arch/mips/kernel/cevt-r4k.c | 14 ++++++++-- arch/mips/kernel/cevt-sb1250.c | 11 +++++--- arch/mips/kernel/cevt-smtc.c | 2 +- arch/mips/kernel/cevt-txx9.c | 12 ++++++--- arch/mips/loongson/common/cs5536/cs5536_mfgpt.c | 12 ++++----- arch/mips/ralink/cevt-rt3352.c | 13 +++++---- arch/mips/sgi-ip27/ip27-timer.c | 14 ++++++++-- arch/mips/sni/time.c | 9 ++++--- arch/mn10300/kernel/cevt-mn10300.c | 14 ++++++++-- arch/openrisc/kernel/time.c | 11 ++++---- arch/powerpc/kernel/time.c | 16 ++++++++--- arch/s390/kernel/time.c | 14 ++++++++-- arch/score/kernel/time.c | 8 +++--- arch/sh/kernel/localtimer.c | 15 +++++++++-- arch/sparc/kernel/time_32.c | 18 ++++++++----- arch/sparc/kernel/time_64.c | 11 ++++---- arch/tile/kernel/time.c | 16 ++++++++--- arch/um/kernel/time.c | 7 +++-- arch/unicore32/kernel/time.c | 8 +++--- arch/x86/kernel/apic/apic.c | 10 ++++--- arch/x86/kernel/hpet.c | 19 +++++++------ arch/x86/lguest/boot.c | 9 ++++--- arch/x86/platform/uv/uv_time.c | 10 ++++--- arch/x86/xen/time.c | 23 +++++++--------- arch/xtensa/kernel/time.c | 9 ++++--- drivers/clocksource/arm_arch_timer.c | 36 ++++++++++++++----------- drivers/clocksource/arm_global_timer.c | 9 ++++--- drivers/clocksource/bcm2835_timer.c | 8 +++--- drivers/clocksource/bcm_kona_timer.c | 10 ++++--- drivers/clocksource/cadence_ttc_timer.c | 7 +++-- drivers/clocksource/cs5535-clockevt.c | 21 +++++++++++---- drivers/clocksource/dummy_timer.c | 15 +++++++++-- drivers/clocksource/dw_apb_timer.c | 7 +++-- drivers/clocksource/em_sti.c | 11 +++++--- drivers/clocksource/exynos_mct.c | 23 ++++++++++------ drivers/clocksource/i8253.c | 8 ++++-- drivers/clocksource/metag_generic.c | 11 ++++---- drivers/clocksource/moxart_timer.c | 8 +++--- drivers/clocksource/mxs_timer.c | 17 ++++++------ drivers/clocksource/nomadik-mtu.c | 7 +++-- drivers/clocksource/qcom-timer.c | 12 ++++----- drivers/clocksource/samsung_pwm_timer.c | 13 ++++----- drivers/clocksource/sh_cmt.c | 9 ++++--- drivers/clocksource/sh_mtu2.c | 9 ++++--- drivers/clocksource/sh_tmu.c | 9 ++++--- drivers/clocksource/sun4i_timer.c | 9 ++++--- drivers/clocksource/tcb_clksrc.c | 11 +++++--- drivers/clocksource/tegra20_timer.c | 12 +++++---- drivers/clocksource/time-armada-370-xp.c | 20 +++++++++----- drivers/clocksource/time-efm32.c | 7 +++-- drivers/clocksource/time-orion.c | 17 +++++++++--- drivers/clocksource/timer-keystone.c | 9 ++++--- drivers/clocksource/timer-marco.c | 12 ++++++--- drivers/clocksource/timer-prima2.c | 10 +++---- drivers/clocksource/timer-sun5i.c | 10 ++++--- drivers/clocksource/timer-u300.c | 7 +++-- drivers/clocksource/vf_pit_timer.c | 12 ++++++--- drivers/clocksource/vt8500_timer.c | 8 +++--- drivers/clocksource/zevio-timer.c | 9 +++---- include/linux/clockchips.h | 4 +-- kernel/time/clockevents.c | 7 +++-- kernel/time/tick-broadcast-hrtimer.c | 11 +++++--- kernel/time/timer_list.c | 2 +- 105 files changed, 810 insertions(+), 419 deletions(-)
A clockevent device can be stopped, or its events can be masked, if next event is expected at KTIME_MAX, i.e. no events are required for very long time.
This normally happens with NO_HZ_FULL (when tick-sched timer is removed and we don't have any more timers scheduled for long), not sure if it can happen otherwise.
If we don't STOP clockevent device, we are guaranteed to receive atleast one fake interrupt, as hrtimer_force_reprogram() isn't reprogramming the event device. Depending on particular implementation of clockevent device, this can be a fake interrupt at tick-rate.. (This was observed atleast on one implementation: arm_arch_timer.c).
A simple solution to get this fixed could be: update hrtimer_force_reprogram() to always reprogram clockevent device and update clockevent drivers to STOP generating events (or delay it to max time), when 'expires' is set to KTIME_MAX. But the drawback here is that every clockevent driver has to be hacked for this particular case and its very easy for new ones to miss this.
Thomas suggested to add an optional mode: CLOCK_EVT_MODE_ONESHOT_STOPPED (lkml.org/lkml/2014/5/9/508) to get tihs fixed.
This requires ->set_mode() callback to have capability to return 'success' or 'failure' for this new mode. Currently its return type is 'void'.
To change return type of set_mode() Thomas suggested to implement another callback: ->set_dev_mode(), with return type 'int'. We can then convert clockevent drivers to use this interface instead of existing ->set_mode() and then finally remove ->set_mode()'s support.
This patch adds support for ->set_dev_mode() in clockevents core.
For now, just add a WARN_ON() to callers of ->set_dev_mode() as none of currently implemented modes are expected/allowed to fail.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- include/linux/clockchips.h | 5 ++++- kernel/time/clockevents.c | 17 ++++++++++++++--- kernel/time/timer_list.c | 5 ++++- 3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 2e4cb67..8ab1a86 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -81,7 +81,8 @@ enum clock_event_mode { * @mode: operating mode assigned by the management code * @features: features * @retries: number of forced programming retries - * @set_mode: set mode function + * @set_dev_mode: set dev mode function + * @set_mode: set mode function (deprecated, use set_dev_mode instead) * @broadcast: function to broadcast events * @min_delta_ticks: minimum delta value in ticks stored for reconfiguration * @max_delta_ticks: maximum delta value in ticks stored for reconfiguration @@ -109,6 +110,8 @@ struct clock_event_device { unsigned long retries;
void (*broadcast)(const struct cpumask *mask); + int (*set_dev_mode)(enum clock_event_mode mode, + struct clock_event_device *); void (*set_mode)(enum clock_event_mode mode, struct clock_event_device *); void (*suspend)(struct clock_event_device *); diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c index ad362c2..3cf7455 100644 --- a/kernel/time/clockevents.c +++ b/kernel/time/clockevents.c @@ -105,7 +105,14 @@ void clockevents_set_mode(struct clock_event_device *dev, enum clock_event_mode mode) { if (dev->mode != mode) { - dev->set_mode(mode, dev); + if (dev->set_dev_mode) { + /* WARN: Currently available modes shouldn't fail */ + if (WARN_ON(dev->set_dev_mode(mode, dev))) + return; + } else { + dev->set_mode(mode, dev); + } + dev->mode = mode;
/* @@ -446,8 +453,12 @@ int __clockevents_update_freq(struct clock_event_device *dev, u32 freq) if (dev->mode == CLOCK_EVT_MODE_ONESHOT) return clockevents_program_event(dev, dev->next_event, false);
- if (dev->mode == CLOCK_EVT_MODE_PERIODIC) - dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev); + if (dev->mode == CLOCK_EVT_MODE_PERIODIC) { + if (dev->set_dev_mode) + WARN_ON(dev->set_dev_mode(CLOCK_EVT_MODE_PERIODIC, dev)); + else + dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev); + }
return 0; } diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c index 61ed862..957a049 100644 --- a/kernel/time/timer_list.c +++ b/kernel/time/timer_list.c @@ -229,7 +229,10 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu) SEQ_printf(m, "\n");
SEQ_printf(m, " set_mode: "); - print_name_offset(m, dev->set_mode); + if (dev->set_dev_mode) + print_name_offset(m, dev->set_dev_mode); + else + print_name_offset(m, dev->set_mode); SEQ_printf(m, "\n");
SEQ_printf(m, " event_handler: ");
On Thu, May 22, 2014 at 06:38:29PM +0530, Viresh Kumar wrote:
A clockevent device can be stopped, or its events can be masked, if next event is expected at KTIME_MAX, i.e. no events are required for very long time.
This normally happens with NO_HZ_FULL (when tick-sched timer is removed and we don't have any more timers scheduled for long), not sure if it can happen otherwise.
This should also happen with NO_HZ_IDLE. It's rather a wide NO_HZ concern.
If we don't STOP clockevent device, we are guaranteed to receive atleast one fake interrupt, as hrtimer_force_reprogram() isn't reprogramming the event device. Depending on particular implementation of clockevent device, this can be a fake interrupt at tick-rate.. (This was observed atleast on one implementation: arm_arch_timer.c).
A simple solution to get this fixed could be: update hrtimer_force_reprogram() to always reprogram clockevent device and update clockevent drivers to STOP generating events (or delay it to max time), when 'expires' is set to KTIME_MAX. But the drawback here is that every clockevent driver has to be hacked for this particular case and its very easy for new ones to miss this.
Thomas suggested to add an optional mode: CLOCK_EVT_MODE_ONESHOT_STOPPED (lkml.org/lkml/2014/5/9/508) to get tihs fixed.
This requires ->set_mode() callback to have capability to return 'success' or 'failure' for this new mode. Currently its return type is 'void'.
To change return type of set_mode() Thomas suggested to implement another callback: ->set_dev_mode(), with return type 'int'. We can then convert clockevent drivers to use this interface instead of existing ->set_mode() and then finally remove ->set_mode()'s support.
This patch adds support for ->set_dev_mode() in clockevents core.
For now, just add a WARN_ON() to callers of ->set_dev_mode() as none of currently implemented modes are expected/allowed to fail.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
include/linux/clockchips.h | 5 ++++- kernel/time/clockevents.c | 17 ++++++++++++++--- kernel/time/timer_list.c | 5 ++++- 3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 2e4cb67..8ab1a86 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -81,7 +81,8 @@ enum clock_event_mode {
- @mode: operating mode assigned by the management code
- @features: features
- @retries: number of forced programming retries
- @set_mode: set mode function
- @set_dev_mode: set dev mode function
- @set_mode: set mode function (deprecated, use set_dev_mode instead)
- @broadcast: function to broadcast events
- @min_delta_ticks: minimum delta value in ticks stored for reconfiguration
- @max_delta_ticks: maximum delta value in ticks stored for reconfiguration
@@ -109,6 +110,8 @@ struct clock_event_device { unsigned long retries; void (*broadcast)(const struct cpumask *mask);
- int (*set_dev_mode)(enum clock_event_mode mode,
struct clock_event_device *);diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c index ad362c2..3cf7455 100644 --- a/kernel/time/clockevents.c +++ b/kernel/time/clockevents.c @@ -105,7 +105,14 @@ void clockevents_set_mode(struct clock_event_device *dev, enum clock_event_mode mode) { if (dev->mode != mode) {
dev->set_mode(mode, dev);
if (dev->set_dev_mode) {/* WARN: Currently available modes shouldn't fail */if (WARN_ON(dev->set_dev_mode(mode, dev)))
Rather use WARN_ON_ONCE(), in general it's much better. Also probably the error handling should be done in this patchset?
Thanks.
return;} else {dev->set_mode(mode, dev);}- dev->mode = mode;
/* @@ -446,8 +453,12 @@ int __clockevents_update_freq(struct clock_event_device *dev, u32 freq) if (dev->mode == CLOCK_EVT_MODE_ONESHOT) return clockevents_program_event(dev, dev->next_event, false);
- if (dev->mode == CLOCK_EVT_MODE_PERIODIC)
dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev);
- if (dev->mode == CLOCK_EVT_MODE_PERIODIC) {
if (dev->set_dev_mode)WARN_ON(dev->set_dev_mode(CLOCK_EVT_MODE_PERIODIC, dev));elsedev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev);- }
return 0; } diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c index 61ed862..957a049 100644 --- a/kernel/time/timer_list.c +++ b/kernel/time/timer_list.c @@ -229,7 +229,10 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu) SEQ_printf(m, "\n"); SEQ_printf(m, " set_mode: ");
- print_name_offset(m, dev->set_mode);
- if (dev->set_dev_mode)
print_name_offset(m, dev->set_dev_mode);- else
print_name_offset(m, dev->set_mode);SEQ_printf(m, " event_handler: "); -- 2.0.0.rc2
On 22 May 2014 21:04, Frederic Weisbecker fweisbec@gmail.com wrote:
On Thu, May 22, 2014 at 06:38:29PM +0530, Viresh Kumar wrote:
A clockevent device can be stopped, or its events can be masked, if next event is expected at KTIME_MAX, i.e. no events are required for very long time.
This normally happens with NO_HZ_FULL (when tick-sched timer is removed and we don't have any more timers scheduled for long), not sure if it can happen otherwise.
This should also happen with NO_HZ_IDLE. It's rather a wide NO_HZ concern.
How exactly? I would have seen these fake interrupts in normal boot if that's the case as we go in and out of idle again and again.. The reason I can think of why it isn't happening for NO_HZ is: clockevent devices are normally part of CPU block (Atleast on ARM), and when CPU goes into some power down state, the even device goes as well..
Though there is one way it can happen for NO_HZ_IDLE, probably Preeti or Daniel told me this earlier and I forgot to mention it. i.e. the case of external event device.
@@ -105,7 +105,14 @@ void clockevents_set_mode(struct clock_event_device *dev, enum clock_event_mode mode) { if (dev->mode != mode) {
dev->set_mode(mode, dev);
if (dev->set_dev_mode) {/* WARN: Currently available modes shouldn't fail */if (WARN_ON(dev->set_dev_mode(mode, dev)))Rather use WARN_ON_ONCE(), in general it's much better.
Sure..
Also probably the error handling should be done in this patchset?
What kind of error handling ? Do we need to return the error code? Thomas rather asked us to do error handling here only and I am not sure what else we can do here apart from a WARN.
On Thu, May 22, 2014 at 09:36:09PM +0530, Viresh Kumar wrote:
On 22 May 2014 21:04, Frederic Weisbecker fweisbec@gmail.com wrote:
On Thu, May 22, 2014 at 06:38:29PM +0530, Viresh Kumar wrote:
A clockevent device can be stopped, or its events can be masked, if next event is expected at KTIME_MAX, i.e. no events are required for very long time.
This normally happens with NO_HZ_FULL (when tick-sched timer is removed and we don't have any more timers scheduled for long), not sure if it can happen otherwise.
This should also happen with NO_HZ_IDLE. It's rather a wide NO_HZ concern.
How exactly? I would have seen these fake interrupts in normal boot if that's the case as we go in and out of idle again and again.. The reason I can think of why it isn't happening for NO_HZ is: clockevent devices are normally part of CPU block (Atleast on ARM), and when CPU goes into some power down state, the even device goes as well..
Ah right, the devices goes down. But since I'm not sure every arch or subarch does that, I'd rather consider the whole NO_HZ scenarios.
Thanks.
Though there is one way it can happen for NO_HZ_IDLE, probably Preeti or Daniel told me this earlier and I forgot to mention it. i.e. the case of external event device.
@@ -105,7 +105,14 @@ void clockevents_set_mode(struct clock_event_device *dev, enum clock_event_mode mode) { if (dev->mode != mode) {
dev->set_mode(mode, dev);
if (dev->set_dev_mode) {/* WARN: Currently available modes shouldn't fail */if (WARN_ON(dev->set_dev_mode(mode, dev)))Rather use WARN_ON_ONCE(), in general it's much better.
Sure..
Also probably the error handling should be done in this patchset?
What kind of error handling ? Do we need to return the error code? Thomas rather asked us to do error handling here only and I am not sure what else we can do here apart from a WARN.
On Thu, May 22, 2014 at 09:36:09PM +0530, Viresh Kumar wrote:
On 22 May 2014 21:04, Frederic Weisbecker fweisbec@gmail.com wrote:
On Thu, May 22, 2014 at 06:38:29PM +0530, Viresh Kumar wrote:
A clockevent device can be stopped, or its events can be masked, if next event is expected at KTIME_MAX, i.e. no events are required for very long time.
This normally happens with NO_HZ_FULL (when tick-sched timer is removed and we don't have any more timers scheduled for long), not sure if it can happen otherwise.
This should also happen with NO_HZ_IDLE. It's rather a wide NO_HZ concern.
How exactly? I would have seen these fake interrupts in normal boot if that's the case as we go in and out of idle again and again.. The reason I can think of why it isn't happening for NO_HZ is: clockevent devices are normally part of CPU block (Atleast on ARM), and when CPU goes into some power down state, the even device goes as well..
Though there is one way it can happen for NO_HZ_IDLE, probably Preeti or Daniel told me this earlier and I forgot to mention it. i.e. the case of external event device.
@@ -105,7 +105,14 @@ void clockevents_set_mode(struct clock_event_device *dev, enum clock_event_mode mode) { if (dev->mode != mode) {
dev->set_mode(mode, dev);
if (dev->set_dev_mode) {/* WARN: Currently available modes shouldn't fail */if (WARN_ON(dev->set_dev_mode(mode, dev)))Rather use WARN_ON_ONCE(), in general it's much better.
Sure..
Also probably the error handling should be done in this patchset?
What kind of error handling ? Do we need to return the error code? Thomas rather asked us to do error handling here only and I am not sure what else we can do here apart from a WARN.
So clockevents_set_mode() returns void so for now I don't know. But __clockevents_update_freq() can return errors.
On 23 May 2014 19:24, Frederic Weisbecker fweisbec@gmail.com wrote:
Also probably the error handling should be done in this patchset?
What kind of error handling ? Do we need to return the error code? Thomas rather asked us to do error handling here only and I am not sure what else we can do here apart from a WARN.
So clockevents_set_mode() returns void so for now I don't know. But __clockevents_update_freq() can return errors.
I tried that initially while you weren't cc'd and this is what I received.
On 21 May 2014 23:29, Kevin Hilman khilman@linaro.org wrote:
Viresh Kumar viresh.kumar@linaro.org writes:
int __clockevents_update_freq(struct clock_event_device *dev, u32 freq) {
int ret = 0;clockevents_config(dev, freq); if (dev->mode == CLOCK_EVT_MODE_ONESHOT) return clockevents_program_event(dev, dev->next_event, false);
if (dev->mode == CLOCK_EVT_MODE_PERIODIC)dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev);
if (dev->mode == CLOCK_EVT_MODE_PERIODIC) {if (dev->set_dev_mode) {ret = dev->set_dev_mode(CLOCK_EVT_MODE_PERIODIC, dev);WARN_ON(ret);} else {dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev);}}
return 0;
return ret;}
First, this change isn't mentioned in the changelog. But more importantly (as Daniel has mentioned), returning an error here has consequences.
Specifically, why would the caller of this function care that the specific clkevt mode was not implmented? As Thomas mentioned in his reviews, the clockevent code should handle the failures itself, not just pass them up and expect other layers to handle them.
Also, if PERIODIC mode can't fail today, why would we expect it to fail in the changed version?
On Fri, May 23, 2014 at 08:25:30PM +0530, Viresh Kumar wrote:
On 23 May 2014 19:24, Frederic Weisbecker fweisbec@gmail.com wrote:
Also probably the error handling should be done in this patchset?
What kind of error handling ? Do we need to return the error code? Thomas rather asked us to do error handling here only and I am not sure what else we can do here apart from a WARN.
So clockevents_set_mode() returns void so for now I don't know. But __clockevents_update_freq() can return errors.
I tried that initially while you weren't cc'd and this is what I received.
On 21 May 2014 23:29, Kevin Hilman khilman@linaro.org wrote:
Viresh Kumar viresh.kumar@linaro.org writes:
int __clockevents_update_freq(struct clock_event_device *dev, u32 freq) {
int ret = 0;clockevents_config(dev, freq); if (dev->mode == CLOCK_EVT_MODE_ONESHOT) return clockevents_program_event(dev, dev->next_event, false);
if (dev->mode == CLOCK_EVT_MODE_PERIODIC)dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev);
if (dev->mode == CLOCK_EVT_MODE_PERIODIC) {if (dev->set_dev_mode) {ret = dev->set_dev_mode(CLOCK_EVT_MODE_PERIODIC, dev);WARN_ON(ret);} else {dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev);}}
return 0;
return ret;}
First, this change isn't mentioned in the changelog. But more importantly (as Daniel has mentioned), returning an error here has consequences.
Specifically, why would the caller of this function care that the specific clkevt mode was not implmented? As Thomas mentioned in his reviews, the clockevent code should handle the failures itself, not just pass them up and expect other layers to handle them.
Also, if PERIODIC mode can't fail today, why would we expect it to fail in the changed version?
Hmm, ok lets just keep the WARN and see then :)
Clockevents core now supports ->set_dev_mode() (as a replacement to ->set_mode()), with capability to return error codes.
Migrate all ARM specific clockevent drivers to implement this new callback.
Drivers now return -ENOSYS when a unsupported mode is passed to their ->set_dev_mode() callbacks and return 0 on success.
Most of the changes are automated with help of Coccinelle (http://coccinelle.lip6.fr/) and the ones left are around the switch block which are handled manually.
Some drivers had a WARN()/BUG()/pr_err() for unsupported modes. These statements are removed now as proper error handling with a WARN_ON() is done at clockevents core.
Also it is made sure that drivers don't disable events for unsupported modes.
A simplified version of the semantic patch is:
@@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c);
@@ identifier setmode; @@ -void +int setmode(enum clock_event_mode, struct clock_event_device *);
@fixret@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c) { ... + return 0; }
@depends on fixret@ identifier ced; identifier fixret.setmode; @@ ... struct clock_event_device ced = { ..., -.set_mode +.set_dev_mode = setmode, };
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced->set_mode + ced->set_dev_mode = setmode
@depends on fixret@ identifier fixret.setmode; @@ { . -set_mode +set_dev_mode = setmode }
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced.set_mode + ced.set_dev_mode = setmode
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- arch/arm/common/timer-sp.c | 13 ++++++++----- arch/arm/kernel/smp_twd.c | 10 +++++++--- arch/arm/mach-at91/at91rm9200_time.c | 10 ++++++++-- arch/arm/mach-at91/at91sam926x_time.c | 11 ++++++----- arch/arm/mach-clps711x/common.c | 12 ++++++------ arch/arm/mach-cns3xxx/core.c | 10 +++++++--- arch/arm/mach-davinci/time.c | 7 +++++-- arch/arm/mach-footbridge/dc21285-timer.c | 7 +++++-- arch/arm/mach-gemini/time.c | 7 ++++--- arch/arm/mach-imx/epit.c | 20 ++++++++++++-------- arch/arm/mach-imx/time.c | 20 ++++++++++++-------- arch/arm/mach-integrator/integrator_ap.c | 16 ++++++++-------- arch/arm/mach-ixp4xx/common.c | 8 +++++--- arch/arm/mach-ks8695/time.c | 22 ++++++++++++++++------ arch/arm/mach-lpc32xx/timer.c | 11 +++++------ arch/arm/mach-mmp/time.c | 12 +++++++----- arch/arm/mach-netx/time.c | 16 +++++++--------- arch/arm/mach-omap1/time.c | 7 +++++-- arch/arm/mach-omap1/timer32k.c | 11 ++++++----- arch/arm/mach-omap2/timer.c | 12 +++++++----- arch/arm/mach-pxa/time.c | 8 +++++--- arch/arm/mach-sa1100/time.c | 8 +++++--- arch/arm/mach-spear/time.c | 20 +++++++++++--------- arch/arm/mach-w90x900/time.c | 7 +++++-- arch/arm/plat-iop/time.c | 8 +++++--- arch/arm/plat-orion/time.c | 20 +++++++++++++++----- 26 files changed, 192 insertions(+), 121 deletions(-)
diff --git a/arch/arm/common/timer-sp.c b/arch/arm/common/timer-sp.c index fd6bff0..8831e9e 100644 --- a/arch/arm/common/timer-sp.c +++ b/arch/arm/common/timer-sp.c @@ -127,31 +127,34 @@ static irqreturn_t sp804_timer_interrupt(int irq, void *dev_id) return IRQ_HANDLED; }
-static void sp804_set_mode(enum clock_event_mode mode, +static int sp804_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE;
- writel(ctrl, clkevt_base + TIMER_CTRL); - switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + writel(ctrl, clkevt_base + TIMER_CTRL); writel(clkevt_reload, clkevt_base + TIMER_LOAD); ctrl |= TIMER_CTRL_PERIODIC | TIMER_CTRL_ENABLE; break;
case CLOCK_EVT_MODE_ONESHOT: /* period set, and timer enabled in 'next_event' hook */ + writel(ctrl, clkevt_base + TIMER_CTRL); ctrl |= TIMER_CTRL_ONESHOT; break;
case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: - default: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; }
writel(ctrl, clkevt_base + TIMER_CTRL); + return 0; }
static int sp804_set_next_event(unsigned long next, @@ -168,7 +171,7 @@ static int sp804_set_next_event(unsigned long next, static struct clock_event_device sp804_clockevent = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ, - .set_mode = sp804_set_mode, + .set_dev_mode = sp804_set_mode, .set_next_event = sp804_set_next_event, .rating = 300, }; diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c index dfc3213..f45420d 100644 --- a/arch/arm/kernel/smp_twd.c +++ b/arch/arm/kernel/smp_twd.c @@ -36,7 +36,7 @@ static DEFINE_PER_CPU(bool, percpu_setup_called); static struct clock_event_device __percpu *twd_evt; static int twd_ppi;
-static void twd_set_mode(enum clock_event_mode mode, +static int twd_set_mode(enum clock_event_mode mode, struct clock_event_device *clk) { unsigned long ctrl; @@ -54,11 +54,15 @@ static void twd_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: - default: + case CLOCK_EVT_MODE_RESUME: ctrl = 0; + break; + default: + return -ENOSYS; }
writel_relaxed(ctrl, twd_base + TWD_TIMER_CONTROL); + return 0; }
static int twd_set_next_event(unsigned long evt, @@ -296,7 +300,7 @@ static void twd_timer_setup(void) clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP; clk->rating = 350; - clk->set_mode = twd_set_mode; + clk->set_dev_mode = twd_set_mode; clk->set_next_event = twd_set_next_event; clk->irq = twd_ppi; clk->cpumask = cpumask_of(cpu); diff --git a/arch/arm/mach-at91/at91rm9200_time.c b/arch/arm/mach-at91/at91rm9200_time.c index 7fd13ae..c052038 100644 --- a/arch/arm/mach-at91/at91rm9200_time.c +++ b/arch/arm/mach-at91/at91rm9200_time.c @@ -112,9 +112,11 @@ static struct clocksource clk32k = { .flags = CLOCK_SOURCE_IS_CONTINUOUS, };
-static void +static int clkevt32k_mode(enum clock_event_mode mode, struct clock_event_device *dev) { + int ret = 0; + /* Disable and flush pending timer interrupts */ at91_st_write(AT91_ST_IDR, AT91_ST_PITS | AT91_ST_ALMS); at91_st_read(AT91_ST_SR); @@ -138,8 +140,12 @@ clkevt32k_mode(enum clock_event_mode mode, struct clock_event_device *dev) case CLOCK_EVT_MODE_RESUME: irqmask = 0; break; + default: + /* irqmask retains value from last call */ + ret = -ENOSYS; } at91_st_write(AT91_ST_IER, irqmask); + return ret; }
static int @@ -177,7 +183,7 @@ static struct clock_event_device clkevt = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .rating = 150, .set_next_event = clkevt32k_next_event, - .set_mode = clkevt32k_mode, + .set_dev_mode = clkevt32k_mode, };
void __iomem *at91_st_base; diff --git a/arch/arm/mach-at91/at91sam926x_time.c b/arch/arm/mach-at91/at91sam926x_time.c index 0a9e2fc..9698eb6 100644 --- a/arch/arm/mach-at91/at91sam926x_time.c +++ b/arch/arm/mach-at91/at91sam926x_time.c @@ -83,7 +83,7 @@ static struct clocksource pit_clk = { /* * Clockevent device: interrupts every 1/HZ (== pit_cycles * MCK/16) */ -static void +static int pit_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) { switch (mode) { @@ -93,9 +93,6 @@ pit_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) pit_write(AT91_PIT_MR, (pit_cycle - 1) | AT91_PIT_PITEN | AT91_PIT_PITIEN); break; - case CLOCK_EVT_MODE_ONESHOT: - BUG(); - /* FALLTHROUGH */ case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_UNUSED: /* disable irq, leaving the clocksource active */ @@ -103,7 +100,11 @@ pit_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) break; case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + + return 0; }
static void at91sam926x_pit_suspend(struct clock_event_device *cedev) @@ -135,7 +136,7 @@ static struct clock_event_device pit_clkevt = { .features = CLOCK_EVT_FEAT_PERIODIC, .shift = 32, .rating = 100, - .set_mode = pit_clkevt_mode, + .set_dev_mode = pit_clkevt_mode, .suspend = at91sam926x_pit_suspend, .resume = at91sam926x_pit_resume, }; diff --git a/arch/arm/mach-clps711x/common.c b/arch/arm/mach-clps711x/common.c index aee81fa..fa7c090 100644 --- a/arch/arm/mach-clps711x/common.c +++ b/arch/arm/mach-clps711x/common.c @@ -69,30 +69,30 @@ static u64 notrace clps711x_sched_clock_read(void) return ~readw_relaxed(CLPS711X_VIRT_BASE + TC1D); }
-static void clps711x_clockevent_set_mode(enum clock_event_mode mode, +static int clps711x_clockevent_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { - disable_irq(IRQ_TC2OI); - switch (mode) { case CLOCK_EVT_MODE_PERIODIC: enable_irq(IRQ_TC2OI); break; - case CLOCK_EVT_MODE_ONESHOT: - /* Not supported */ case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_RESUME: + disable_irq(IRQ_TC2OI); /* Left event sources disabled, no more interrupts appear */ break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device clockevent_clps711x = { .name = "clps711x-clockevent", .rating = 300, .features = CLOCK_EVT_FEAT_PERIODIC, - .set_mode = clps711x_clockevent_set_mode, + .set_dev_mode = clps711x_clockevent_set_mode, };
static irqreturn_t clps711x_timer_interrupt(int irq, void *dev_id) diff --git a/arch/arm/mach-cns3xxx/core.c b/arch/arm/mach-cns3xxx/core.c index 2ae28a6..9746501 100644 --- a/arch/arm/mach-cns3xxx/core.c +++ b/arch/arm/mach-cns3xxx/core.c @@ -113,7 +113,7 @@ void cns3xxx_power_off(void) */ static void __iomem *cns3xxx_tmr1;
-static void cns3xxx_timer_set_mode(enum clock_event_mode mode, +static int cns3xxx_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *clk) { unsigned long ctrl = readl(cns3xxx_tmr1 + TIMER1_2_CONTROL_OFFSET); @@ -132,11 +132,15 @@ static void cns3xxx_timer_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: - default: + case CLOCK_EVT_MODE_RESUME: ctrl = 0; + break; + default: + return -ENOSYS; }
writel(ctrl, cns3xxx_tmr1 + TIMER1_2_CONTROL_OFFSET); + return 0; }
static int cns3xxx_timer_set_next_event(unsigned long evt, @@ -153,7 +157,7 @@ static int cns3xxx_timer_set_next_event(unsigned long evt, static struct clock_event_device cns3xxx_tmr1_clockevent = { .name = "cns3xxx timer1", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = cns3xxx_timer_set_mode, + .set_dev_mode = cns3xxx_timer_set_mode, .set_next_event = cns3xxx_timer_set_next_event, .rating = 350, .cpumask = cpu_all_mask, diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c index 24ad30f..7ab6a6c 100644 --- a/arch/arm/mach-davinci/time.c +++ b/arch/arm/mach-davinci/time.c @@ -303,7 +303,7 @@ static int davinci_set_next_event(unsigned long cycles, return 0; }
-static void davinci_set_mode(enum clock_event_mode mode, +static int davinci_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { struct timer_s *t = &timers[TID_CLOCKEVENT]; @@ -326,13 +326,16 @@ static void davinci_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device clockevent_davinci = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .set_next_event = davinci_set_next_event, - .set_mode = davinci_set_mode, + .set_dev_mode = davinci_set_mode, };
diff --git a/arch/arm/mach-footbridge/dc21285-timer.c b/arch/arm/mach-footbridge/dc21285-timer.c index bf7aa7d..c949daf 100644 --- a/arch/arm/mach-footbridge/dc21285-timer.c +++ b/arch/arm/mach-footbridge/dc21285-timer.c @@ -57,7 +57,7 @@ static int ckevt_dc21285_set_next_event(unsigned long delta, return 0; }
-static void ckevt_dc21285_set_mode(enum clock_event_mode mode, +static int ckevt_dc21285_set_mode(enum clock_event_mode mode, struct clock_event_device *c) { switch (mode) { @@ -74,7 +74,10 @@ static void ckevt_dc21285_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_SHUTDOWN: *CSR_TIMER1_CNTL = 0; break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device ckevt_dc21285 = { @@ -84,7 +87,7 @@ static struct clock_event_device ckevt_dc21285 = { .rating = 200, .irq = IRQ_TIMER1, .set_next_event = ckevt_dc21285_set_next_event, - .set_mode = ckevt_dc21285_set_mode, + .set_dev_mode = ckevt_dc21285_set_mode, };
static irqreturn_t timer1_interrupt(int irq, void *dev_id) diff --git a/arch/arm/mach-gemini/time.c b/arch/arm/mach-gemini/time.c index 0a63c4d..8fea6f3 100644 --- a/arch/arm/mach-gemini/time.c +++ b/arch/arm/mach-gemini/time.c @@ -59,7 +59,7 @@ static int gemini_timer_set_next_event(unsigned long cycles, return 0; }
-static void gemini_timer_set_mode(enum clock_event_mode mode, +static int gemini_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { u32 period = DIV_ROUND_CLOSEST(tick_rate, HZ); @@ -91,8 +91,9 @@ static void gemini_timer_set_mode(enum clock_event_mode mode, writel(cr, TIMER_CR(IO_ADDRESS(GEMINI_TIMER_BASE))); break; default: - break; + return -ENOSYS; } + return 0; }
/* Use TIMER2 as clock event */ @@ -101,7 +102,7 @@ static struct clock_event_device gemini_clockevent = { .rating = 300, /* Reasonably fast and accurate clock event */ .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .set_next_event = gemini_timer_set_next_event, - .set_mode = gemini_timer_set_mode, + .set_dev_mode = gemini_timer_set_mode, };
/* diff --git a/arch/arm/mach-imx/epit.c b/arch/arm/mach-imx/epit.c index 074b1a8..a2a8c3d 100644 --- a/arch/arm/mach-imx/epit.c +++ b/arch/arm/mach-imx/epit.c @@ -106,7 +106,7 @@ static int epit_set_next_event(unsigned long evt, return 0; }
-static void epit_set_mode(enum clock_event_mode mode, +static int epit_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { unsigned long flags; @@ -127,15 +127,9 @@ static void epit_set_mode(enum clock_event_mode mode, epit_irq_acknowledge(); }
- /* Remember timer mode */ - clockevent_mode = mode; local_irq_restore(flags);
switch (mode) { - case CLOCK_EVT_MODE_PERIODIC: - printk(KERN_ERR "epit_set_mode: Periodic mode is not " - "supported for i.MX EPIT\n"); - break; case CLOCK_EVT_MODE_ONESHOT: /* * Do not put overhead of interrupt enable/disable into @@ -152,7 +146,17 @@ static void epit_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: /* Left event sources disabled, no more interrupts appear */ break; + default: + /* Enable interrupts again */ + local_irq_save(flags); + epit_irq_enable(); + local_irq_restore(flags); + return -ENOSYS; } + + /* Remember timer mode */ + clockevent_mode = mode; + return 0; }
/* @@ -178,7 +182,7 @@ static struct irqaction epit_timer_irq = { static struct clock_event_device clockevent_epit = { .name = "epit", .features = CLOCK_EVT_FEAT_ONESHOT, - .set_mode = epit_set_mode, + .set_dev_mode = epit_set_mode, .set_next_event = epit_set_next_event, .rating = 200, }; diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c index 65222ea..c0a3159 100644 --- a/arch/arm/mach-imx/time.c +++ b/arch/arm/mach-imx/time.c @@ -179,7 +179,7 @@ static const char *clock_event_mode_label[] = { }; #endif /* DEBUG */
-static void mxc_set_mode(enum clock_event_mode mode, +static int mxc_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { unsigned long flags; @@ -212,15 +212,9 @@ static void mxc_set_mode(enum clock_event_mode mode, clock_event_mode_label[mode]); #endif /* DEBUG */
- /* Remember timer mode */ - clockevent_mode = mode; local_irq_restore(flags);
switch (mode) { - case CLOCK_EVT_MODE_PERIODIC: - printk(KERN_ERR"mxc_set_mode: Periodic mode is not " - "supported for i.MX\n"); - break; case CLOCK_EVT_MODE_ONESHOT: /* * Do not put overhead of interrupt enable/disable into @@ -237,7 +231,17 @@ static void mxc_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: /* Left event sources disabled, no more interrupts appear */ break; + default: + /* Enable interrupts again */ + local_irq_save(flags); + gpt_irq_enable(); + local_irq_restore(flags); + return -ENOSYS; } + + /* Remember timer mode */ + clockevent_mode = mode; + return ret; }
/* @@ -269,7 +273,7 @@ static struct irqaction mxc_timer_irq = { static struct clock_event_device clockevent_mxc = { .name = "mxc_timer1", .features = CLOCK_EVT_FEAT_ONESHOT, - .set_mode = mxc_set_mode, + .set_dev_mode = mxc_set_mode, .set_next_event = mx1_2_set_next_event, .rating = 200, }; diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c index dd0cc67..6d338e9 100644 --- a/arch/arm/mach-integrator/integrator_ap.c +++ b/arch/arm/mach-integrator/integrator_ap.c @@ -307,33 +307,33 @@ static irqreturn_t integrator_timer_interrupt(int irq, void *dev_id) return IRQ_HANDLED; }
-static void clkevt_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) +static int clkevt_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { u32 ctrl = readl(clkevt_base + TIMER_CTRL) & ~TIMER_CTRL_ENABLE;
- /* Disable timer */ - writel(ctrl, clkevt_base + TIMER_CTRL); - switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + writel(ctrl, clkevt_base + TIMER_CTRL); /* Enable the timer and start the periodic tick */ writel(timer_reload, clkevt_base + TIMER_LOAD); ctrl |= TIMER_CTRL_PERIODIC | TIMER_CTRL_ENABLE; - writel(ctrl, clkevt_base + TIMER_CTRL); break; case CLOCK_EVT_MODE_ONESHOT: + writel(ctrl, clkevt_base + TIMER_CTRL); /* Leave the timer disabled, .set_next_event will enable it */ ctrl &= ~TIMER_CTRL_PERIODIC; - writel(ctrl, clkevt_base + TIMER_CTRL); break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: - default: /* Just leave in disabled state */ break; + default: + return -ENOSYS; }
+ writel(ctrl, clkevt_base + TIMER_CTRL); + return 0; }
static int clkevt_set_next_event(unsigned long next, struct clock_event_device *evt) @@ -350,7 +350,7 @@ static int clkevt_set_next_event(unsigned long next, struct clock_event_device * static struct clock_event_device integrator_clockevent = { .name = "timer1", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = clkevt_set_mode, + .set_dev_mode = clkevt_set_mode, .set_next_event = clkevt_set_next_event, .rating = 300, }; diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c index fc4b7b2..05c050a 100644 --- a/arch/arm/mach-ixp4xx/common.c +++ b/arch/arm/mach-ixp4xx/common.c @@ -521,7 +521,7 @@ static int ixp4xx_set_next_event(unsigned long evt, return 0; }
-static void ixp4xx_set_mode(enum clock_event_mode mode, +static int ixp4xx_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { unsigned long opts = *IXP4XX_OSRT1 & IXP4XX_OST_RELOAD_MASK; @@ -544,19 +544,21 @@ static void ixp4xx_set_mode(enum clock_event_mode mode, opts |= IXP4XX_OST_ENABLE; break; case CLOCK_EVT_MODE_UNUSED: - default: osrt = opts = 0; break; + default: + return -ENOSYS; }
*IXP4XX_OSRT1 = osrt | opts; + return 0; }
static struct clock_event_device clockevent_ixp4xx = { .name = "ixp4xx timer1", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .rating = 200, - .set_mode = ixp4xx_set_mode, + .set_dev_mode = ixp4xx_set_mode, .set_next_event = ixp4xx_set_next_event, };
diff --git a/arch/arm/mach-ks8695/time.c b/arch/arm/mach-ks8695/time.c index a197874..8d2149c 100644 --- a/arch/arm/mach-ks8695/time.c +++ b/arch/arm/mach-ks8695/time.c @@ -54,14 +54,15 @@ /* Timer0 Timeout Counter Register */ #define T0TC_WATCHDOG (0xff) /* Enable watchdog mode */
-static void ks8695_set_mode(enum clock_event_mode mode, +static int ks8695_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { - u32 tmcon; + u32 tmcon, rate, half;
- if (mode == CLOCK_EVT_FEAT_PERIODIC) { - u32 rate = DIV_ROUND_CLOSEST(KS8695_CLOCK_RATE, HZ); - u32 half = DIV_ROUND_CLOSEST(rate, 2); + switch (mode) { + case CLOCK_EVT_MODE_PERIODIC: + rate = DIV_ROUND_CLOSEST(KS8695_CLOCK_RATE, HZ); + half = DIV_ROUND_CLOSEST(rate, 2);
/* Disable timer 1 */ tmcon = readl_relaxed(KS8695_TMR_VA + KS8695_TMCON); @@ -75,7 +76,16 @@ static void ks8695_set_mode(enum clock_event_mode mode, /* Re-enable timer1 */ tmcon |= TMCON_T1EN; writel_relaxed(tmcon, KS8695_TMR_VA + KS8695_TMCON); + break; + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: + case CLOCK_EVT_MODE_UNUSED: + break; + default: + return -ENOSYS; } + return 0; }
static int ks8695_set_next_event(unsigned long cycles, @@ -106,7 +116,7 @@ static struct clock_event_device clockevent_ks8695 = { .rating = 300, /* Reasonably fast and accurate clock event */ .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC, .set_next_event = ks8695_set_next_event, - .set_mode = ks8695_set_mode, + .set_dev_mode = ks8695_set_mode, };
/* diff --git a/arch/arm/mach-lpc32xx/timer.c b/arch/arm/mach-lpc32xx/timer.c index 4e58372..d57a84d 100644 --- a/arch/arm/mach-lpc32xx/timer.c +++ b/arch/arm/mach-lpc32xx/timer.c @@ -43,14 +43,10 @@ static int lpc32xx_clkevt_next_event(unsigned long delta, return 0; }
-static void lpc32xx_clkevt_mode(enum clock_event_mode mode, +static int lpc32xx_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) { switch (mode) { - case CLOCK_EVT_MODE_PERIODIC: - WARN_ON(1); - break; - case CLOCK_EVT_MODE_ONESHOT: case CLOCK_EVT_MODE_SHUTDOWN: /* @@ -64,7 +60,10 @@ static void lpc32xx_clkevt_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device lpc32xx_clkevt = { @@ -72,7 +71,7 @@ static struct clock_event_device lpc32xx_clkevt = { .features = CLOCK_EVT_FEAT_ONESHOT, .rating = 300, .set_next_event = lpc32xx_clkevt_next_event, - .set_mode = lpc32xx_clkevt_mode, + .set_dev_mode = lpc32xx_clkevt_mode, };
static irqreturn_t lpc32xx_timer_interrupt(int irq, void *dev_id) diff --git a/arch/arm/mach-mmp/time.c b/arch/arm/mach-mmp/time.c index 2756351..8491da9 100644 --- a/arch/arm/mach-mmp/time.c +++ b/arch/arm/mach-mmp/time.c @@ -124,24 +124,26 @@ static int timer_set_next_event(unsigned long delta, return 0; }
-static void timer_set_mode(enum clock_event_mode mode, +static int timer_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) { unsigned long flags;
- local_irq_save(flags); switch (mode) { case CLOCK_EVT_MODE_ONESHOT: case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: /* disable the matching interrupt */ + local_irq_save(flags); __raw_writel(0x00, mmp_timer_base + TMR_IER(0)); + local_irq_restore(flags); break; case CLOCK_EVT_MODE_RESUME: - case CLOCK_EVT_MODE_PERIODIC: break; + default: + return -ENOSYS; } - local_irq_restore(flags); + return 0; }
static struct clock_event_device ckevt = { @@ -149,7 +151,7 @@ static struct clock_event_device ckevt = { .features = CLOCK_EVT_FEAT_ONESHOT, .rating = 200, .set_next_event = timer_set_next_event, - .set_mode = timer_set_mode, + .set_dev_mode = timer_set_mode, };
static cycle_t clksrc_read(struct clocksource *cs) diff --git a/arch/arm/mach-netx/time.c b/arch/arm/mach-netx/time.c index 5fb2a59..32a6625 100644 --- a/arch/arm/mach-netx/time.c +++ b/arch/arm/mach-netx/time.c @@ -34,16 +34,14 @@ #define TIMER_CLOCKEVENT 0 #define TIMER_CLOCKSOURCE 1
-static void netx_set_mode(enum clock_event_mode mode, +static int netx_set_mode(enum clock_event_mode mode, struct clock_event_device *clk) { u32 tmode;
- /* disable timer */ - writel(0, NETX_GPIO_COUNTER_CTRL(TIMER_CLOCKEVENT)); - switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + writel(0, NETX_GPIO_COUNTER_CTRL(TIMER_CLOCKEVENT)); writel(NETX_LATCH, NETX_GPIO_COUNTER_MAX(TIMER_CLOCKEVENT)); tmode = NETX_GPIO_COUNTER_CTRL_RST_EN | NETX_GPIO_COUNTER_CTRL_IRQ_EN | @@ -51,23 +49,23 @@ static void netx_set_mode(enum clock_event_mode mode, break;
case CLOCK_EVT_MODE_ONESHOT: + writel(0, NETX_GPIO_COUNTER_CTRL(TIMER_CLOCKEVENT)); writel(0, NETX_GPIO_COUNTER_MAX(TIMER_CLOCKEVENT)); tmode = NETX_GPIO_COUNTER_CTRL_IRQ_EN | NETX_GPIO_COUNTER_CTRL_RUN; break;
- default: - WARN(1, "%s: unhandled mode %d\n", __func__, mode); - /* fall through */ - case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_RESUME: tmode = 0; break; + default: + return -ENOSYS; }
writel(tmode, NETX_GPIO_COUNTER_CTRL(TIMER_CLOCKEVENT)); + return 0; }
static int netx_set_next_event(unsigned long evt, @@ -81,7 +79,7 @@ static struct clock_event_device netx_clockevent = { .name = "netx-timer" __stringify(TIMER_CLOCKEVENT), .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .set_next_event = netx_set_next_event, - .set_mode = netx_set_mode, + .set_dev_mode = netx_set_mode, };
/* diff --git a/arch/arm/mach-omap1/time.c b/arch/arm/mach-omap1/time.c index a7588cf..b2bcd25 100644 --- a/arch/arm/mach-omap1/time.c +++ b/arch/arm/mach-omap1/time.c @@ -124,7 +124,7 @@ static int omap_mpu_set_next_event(unsigned long cycles, return 0; }
-static void omap_mpu_set_mode(enum clock_event_mode mode, +static int omap_mpu_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -139,14 +139,17 @@ static void omap_mpu_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device clockevent_mpu_timer1 = { .name = "mpu_timer1", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .set_next_event = omap_mpu_set_next_event, - .set_mode = omap_mpu_set_mode, + .set_dev_mode = omap_mpu_set_mode, };
static irqreturn_t omap_mpu_timer1_interrupt(int irq, void *dev_id) diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c index 107e7ab..c589402 100644 --- a/arch/arm/mach-omap1/timer32k.c +++ b/arch/arm/mach-omap1/timer32k.c @@ -119,11 +119,9 @@ static int omap_32k_timer_set_next_event(unsigned long delta, return 0; }
-static void omap_32k_timer_set_mode(enum clock_event_mode mode, +static int omap_32k_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { - omap_32k_timer_stop(); - switch (mode) { case CLOCK_EVT_MODE_PERIODIC: omap_32k_timer_start(OMAP_32K_TIMER_TICK_PERIOD); @@ -131,17 +129,20 @@ static void omap_32k_timer_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_ONESHOT: case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: - break; case CLOCK_EVT_MODE_RESUME: + omap_32k_timer_stop(); break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device clockevent_32k_timer = { .name = "32k-timer", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .set_next_event = omap_32k_timer_set_next_event, - .set_mode = omap_32k_timer_set_mode, + .set_dev_mode = omap_32k_timer_set_mode, };
static irqreturn_t omap_32k_timer_interrupt(int irq, void *dev_id) diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index b62de9f..7f13846 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -101,15 +101,14 @@ static int omap2_gp_timer_set_next_event(unsigned long cycles, return 0; }
-static void omap2_gp_timer_set_mode(enum clock_event_mode mode, +static int omap2_gp_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { u32 period;
- __omap_dm_timer_stop(&clkev, OMAP_TIMER_POSTED, clkev.rate); - switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + __omap_dm_timer_stop(&clkev, OMAP_TIMER_POSTED, clkev.rate); period = clkev.rate / HZ; period -= 1; /* Looks like we need to first set the load value separately */ @@ -120,19 +119,22 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode mode, 0xffffffff - period, OMAP_TIMER_POSTED); break; case CLOCK_EVT_MODE_ONESHOT: - break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: + __omap_dm_timer_stop(&clkev, OMAP_TIMER_POSTED, clkev.rate); break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device clockevent_gpt = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .rating = 300, .set_next_event = omap2_gp_timer_set_next_event, - .set_mode = omap2_gp_timer_set_mode, + .set_dev_mode = omap2_gp_timer_set_mode, };
static struct property device_disabled = { diff --git a/arch/arm/mach-pxa/time.c b/arch/arm/mach-pxa/time.c index fca174e..a4a8789 100644 --- a/arch/arm/mach-pxa/time.c +++ b/arch/arm/mach-pxa/time.c @@ -67,7 +67,7 @@ pxa_osmr0_set_next_event(unsigned long delta, struct clock_event_device *dev) return (signed)(next - oscr) <= MIN_OSCR_DELTA ? -ETIME : 0; }
-static void +static int pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) { switch (mode) { @@ -84,9 +84,11 @@ pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) break;
case CLOCK_EVT_MODE_RESUME: - case CLOCK_EVT_MODE_PERIODIC: break; + default: + return -ENOSYS; } + return 0; }
#ifdef CONFIG_PM @@ -130,7 +132,7 @@ static struct clock_event_device ckevt_pxa_osmr0 = { .features = CLOCK_EVT_FEAT_ONESHOT, .rating = 200, .set_next_event = pxa_osmr0_set_next_event, - .set_mode = pxa_osmr0_set_mode, + .set_dev_mode = pxa_osmr0_set_mode, .suspend = pxa_timer_suspend, .resume = pxa_timer_resume, }; diff --git a/arch/arm/mach-sa1100/time.c b/arch/arm/mach-sa1100/time.c index 1dea6cf..2282858 100644 --- a/arch/arm/mach-sa1100/time.c +++ b/arch/arm/mach-sa1100/time.c @@ -56,7 +56,7 @@ sa1100_osmr0_set_next_event(unsigned long delta, struct clock_event_device *c) return (signed)(next - oscr) <= MIN_OSCR_DELTA ? -ETIME : 0; }
-static void +static int sa1100_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *c) { switch (mode) { @@ -68,9 +68,11 @@ sa1100_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *c) break;
case CLOCK_EVT_MODE_RESUME: - case CLOCK_EVT_MODE_PERIODIC: break; + default: + return -ENOSYS; } + return 0; }
#ifdef CONFIG_PM @@ -109,7 +111,7 @@ static struct clock_event_device ckevt_sa1100_osmr0 = { .features = CLOCK_EVT_FEAT_ONESHOT, .rating = 200, .set_next_event = sa1100_osmr0_set_next_event, - .set_mode = sa1100_osmr0_set_mode, + .set_dev_mode = sa1100_osmr0_set_mode, .suspend = sa1100_timer_suspend, .resume = sa1100_timer_resume, }; diff --git a/arch/arm/mach-spear/time.c b/arch/arm/mach-spear/time.c index 26fda4e..8aa9ea5 100644 --- a/arch/arm/mach-spear/time.c +++ b/arch/arm/mach-spear/time.c @@ -66,7 +66,7 @@ static __iomem void *gpt_base; static struct clk *gpt_clk;
-static void clockevent_set_mode(enum clock_event_mode mode, +static int clockevent_set_mode(enum clock_event_mode mode, struct clock_event_device *clk_event_dev); static int clockevent_next_event(unsigned long evt, struct clock_event_device *clk_event_dev); @@ -98,12 +98,12 @@ static void __init spear_clocksource_init(void) static struct clock_event_device clkevt = { .name = "tmr0", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = clockevent_set_mode, + .set_dev_mode = clockevent_set_mode, .set_next_event = clockevent_next_event, .shift = 0, /* to be computed */ };
-static void clockevent_set_mode(enum clock_event_mode mode, +static int clockevent_set_mode(enum clock_event_mode mode, struct clock_event_device *clk_event_dev) { u32 period; @@ -112,10 +112,11 @@ static void clockevent_set_mode(enum clock_event_mode mode, /* stop the timer */ val = readw(gpt_base + CR(CLKEVT)); val &= ~CTRL_ENABLE; - writew(val, gpt_base + CR(CLKEVT));
switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + writew(val, gpt_base + CR(CLKEVT)); + period = clk_get_rate(gpt_clk) / HZ; period >>= CTRL_PRESCALER16; writew(period, gpt_base + LOAD(CLKEVT)); @@ -123,24 +124,25 @@ static void clockevent_set_mode(enum clock_event_mode mode, val = readw(gpt_base + CR(CLKEVT)); val &= ~CTRL_ONE_SHOT; val |= CTRL_ENABLE | CTRL_INT_ENABLE; - writew(val, gpt_base + CR(CLKEVT));
break; case CLOCK_EVT_MODE_ONESHOT: + writew(val, gpt_base + CR(CLKEVT)); + val = readw(gpt_base + CR(CLKEVT)); val |= CTRL_ONE_SHOT; - writew(val, gpt_base + CR(CLKEVT));
break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: - break; default: - pr_err("Invalid mode requested\n"); - break; + return -ENOSYS; } + + writew(val, gpt_base + CR(CLKEVT)); + return 0; }
static int clockevent_next_event(unsigned long cycles, diff --git a/arch/arm/mach-w90x900/time.c b/arch/arm/mach-w90x900/time.c index 9230d37..e4a8067 100644 --- a/arch/arm/mach-w90x900/time.c +++ b/arch/arm/mach-w90x900/time.c @@ -48,7 +48,7 @@
static unsigned int timer0_load;
-static void nuc900_clockevent_setmode(enum clock_event_mode mode, +static int nuc900_clockevent_setmode(enum clock_event_mode mode, struct clock_event_device *clk) { unsigned int val; @@ -70,9 +70,12 @@ static void nuc900_clockevent_setmode(enum clock_event_mode mode, case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; }
__raw_writel(val, REG_TCSR0); + return 0; }
static int nuc900_clockevent_setnextevent(unsigned long evt, @@ -92,7 +95,7 @@ static int nuc900_clockevent_setnextevent(unsigned long evt, static struct clock_event_device nuc900_clockevent_device = { .name = "nuc900-timer0", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = nuc900_clockevent_setmode, + .set_dev_mode = nuc900_clockevent_setmode, .set_next_event = nuc900_clockevent_setnextevent, .rating = 300, }; diff --git a/arch/arm/plat-iop/time.c b/arch/arm/plat-iop/time.c index 6ad65d8..a21a18c 100644 --- a/arch/arm/plat-iop/time.c +++ b/arch/arm/plat-iop/time.c @@ -77,7 +77,7 @@ static int iop_set_next_event(unsigned long delta,
static unsigned long ticks_per_jiffy;
-static void iop_set_mode(enum clock_event_mode mode, +static int iop_set_mode(enum clock_event_mode mode, struct clock_event_device *unused) { u32 tmr = read_tmr0(); @@ -98,12 +98,14 @@ static void iop_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_UNUSED: - default: tmr &= ~IOP_TMR_EN; break; + default: + return -ENOSYS; }
write_tmr0(tmr); + return 0; }
static struct clock_event_device iop_clockevent = { @@ -111,7 +113,7 @@ static struct clock_event_device iop_clockevent = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .rating = 300, .set_next_event = iop_set_next_event, - .set_mode = iop_set_mode, + .set_dev_mode = iop_set_mode, };
static irqreturn_t diff --git a/arch/arm/plat-orion/time.c b/arch/arm/plat-orion/time.c index 261258f..7dc5cc8 100644 --- a/arch/arm/plat-orion/time.c +++ b/arch/arm/plat-orion/time.c @@ -106,14 +106,15 @@ orion_clkevt_next_event(unsigned long delta, struct clock_event_device *dev) return 0; }
-static void +static int orion_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) { unsigned long flags; u32 u;
local_irq_save(flags); - if (mode == CLOCK_EVT_MODE_PERIODIC) { + switch (mode) { + case CLOCK_EVT_MODE_PERIODIC: /* * Setup timer to fire at 1/HZ intervals. */ @@ -132,7 +133,12 @@ orion_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) u = readl(timer_base + TIMER_CTRL_OFF); writel(u | TIMER1_EN | TIMER1_RELOAD_EN, timer_base + TIMER_CTRL_OFF); - } else { + break; + + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: /* * Disable timer. */ @@ -149,9 +155,13 @@ orion_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) * ACK pending timer interrupt. */ writel(bridge_timer1_clr_mask, bridge_base + BRIDGE_CAUSE_OFF); - + break; + default: + local_irq_restore(flags); + return -ENOSYS; } local_irq_restore(flags); + return 0; }
static struct clock_event_device orion_clkevt = { @@ -159,7 +169,7 @@ static struct clock_event_device orion_clkevt = { .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC, .rating = 300, .set_next_event = orion_clkevt_next_event, - .set_mode = orion_clkevt_mode, + .set_dev_mode = orion_clkevt_mode, };
static irqreturn_t orion_timer_interrupt(int irq, void *dev_id)
Viresh Kumar viresh.kumar@linaro.org writes:
Clockevents core now supports ->set_dev_mode() (as a replacement to ->set_mode()), with capability to return error codes.
Migrate all ARM specific clockevent drivers to implement this new callback.
Drivers now return -ENOSYS when a unsupported mode is passed to their ->set_dev_mode() callbacks and return 0 on success.
Most of the changes are automated with help of Coccinelle (http://coccinelle.lip6.fr/) and the ones left are around the switch block which are handled manually.
Some drivers had a WARN()/BUG()/pr_err() for unsupported modes. These statements are removed now as proper error handling with a WARN_ON() is done at clockevents core.
Also it is made sure that drivers don't disable events for unsupported modes.
Why wouldn't you want that? Most drivers are leaving events disabled for unsupported modes currently. Doing this seems to complicate things, and needlessly change/complicate current behavior, as well as make this patch more difficult to read/review.
In any case, I've given a quick skim over some of the driver changes below and found several (possible) issues.
This will be tricky to get right on all platforms, so IMO the changes should be minimal.
[...]
diff --git a/arch/arm/mach-clps711x/common.c b/arch/arm/mach-clps711x/common.c index aee81fa..fa7c090 100644 --- a/arch/arm/mach-clps711x/common.c +++ b/arch/arm/mach-clps711x/common.c @@ -69,30 +69,30 @@ static u64 notrace clps711x_sched_clock_read(void) return ~readw_relaxed(CLPS711X_VIRT_BASE + TC1D); } -static void clps711x_clockevent_set_mode(enum clock_event_mode mode, +static int clps711x_clockevent_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) {
- disable_irq(IRQ_TC2OI);
You probably noticed this, but this step is now skipped for _PERIODIC. Since the IRQ is immediately re-enabled, it's probably not a problem, but it *is* a functional change that should be noted.
switch (mode) { case CLOCK_EVT_MODE_PERIODIC: enable_irq(IRQ_TC2OI); break;
- case CLOCK_EVT_MODE_ONESHOT:
/* Not supported */
disable_irq(IRQ_TC2OI);- default:
return -ENOSYS;- return 0;
}
[...]
diff --git a/arch/arm/mach-cns3xxx/core.c b/arch/arm/mach-cns3xxx/core.c index 2ae28a6..9746501 100644 --- a/arch/arm/mach-cns3xxx/core.c +++ b/arch/arm/mach-cns3xxx/core.c @@ -113,7 +113,7 @@ void cns3xxx_power_off(void) */ static void __iomem *cns3xxx_tmr1; -static void cns3xxx_timer_set_mode(enum clock_event_mode mode, +static int cns3xxx_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *clk) { unsigned long ctrl = readl(cns3xxx_tmr1 + TIMER1_2_CONTROL_OFFSET); @@ -132,11 +132,15 @@ static void cns3xxx_timer_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN:
- default:
- case CLOCK_EVT_MODE_RESUME: ctrl = 0;
break;- default:
return -ENOSYS;writel(ctrl, cns3xxx_tmr1 + TIMER1_2_CONTROL_OFFSET);
Hmm, looks like the timer is left disabled for the unsupported modes, which is the opposite of what's done elsewhere.
- return 0;
}
[...]
diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c index 107e7ab..c589402 100644 --- a/arch/arm/mach-omap1/timer32k.c +++ b/arch/arm/mach-omap1/timer32k.c @@ -119,11 +119,9 @@ static int omap_32k_timer_set_next_event(unsigned long delta, return 0; } -static void omap_32k_timer_set_mode(enum clock_event_mode mode, +static int omap_32k_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) {
- omap_32k_timer_stop();
- switch (mode) { case CLOCK_EVT_MODE_PERIODIC: omap_32k_timer_start(OMAP_32K_TIMER_TICK_PERIOD);
Note that this is also a functional change because the timer doesn't get stopped before it is restarted in periodic mode. I can't remember if that's safe on OMAP1 (I think not), but it in any case, a functional change like should not be made without more thorough testing (IMO, this kind of functional change should not be part of this patch.)
@@ -131,17 +129,20 @@ static void omap_32k_timer_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_ONESHOT: case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN:
break;
omap_32k_timer_stop();- default:
return -ENOSYS;- return 0;
}
[...]
diff --git a/arch/arm/mach-pxa/time.c b/arch/arm/mach-pxa/time.c index fca174e..a4a8789 100644 --- a/arch/arm/mach-pxa/time.c +++ b/arch/arm/mach-pxa/time.c @@ -67,7 +67,7 @@ pxa_osmr0_set_next_event(unsigned long delta, struct clock_event_device *dev) return (signed)(next - oscr) <= MIN_OSCR_DELTA ? -ETIME : 0; } -static void +static int pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) { switch (mode) { @@ -84,9 +84,11 @@ pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) break; case CLOCK_EVT_MODE_RESUME:
- case CLOCK_EVT_MODE_PERIODIC:
Hmm, you're changing periodic mode from supported to unsupported. Why?
I see some others like this later also.
OK, I give up for now. I've found enough issues to convince myself that this needs some closer attention, and possibly a rethink about how to handle unsupported modes.
Kevin
On 23 May 2014 03:51, Kevin Hilman khilman@linaro.org wrote:
Viresh Kumar viresh.kumar@linaro.org writes:
Also it is made sure that drivers don't disable events for unsupported modes.
Why wouldn't you want that? Most drivers are leaving events disabled for unsupported modes currently.
You know why it took me 4 days to send this version because I wasn't able to decide what to do here..
So here is the problem: - There is no problem even after the current series as every driver is taking care of all currently available modes and so they would never goto default case..
- When we will send ONESHOT_STOPPED notification strange things might happen. Core would send ONESHOT_STOPPED notification and drivers would return -ENOSYS (unless they are updated to handle it). Additionally few of them would disable events, and core would never think that. It would still think drivers are running in the last configured mode as they returned error here..
Core wouldn't try to set mode again for them as it thinks they are still running in that mode.
For ONESHOT_STOPPED it may work without issues as core will call set_next_event() and they might enable events there..
But with another mode in future things might start to fail. Suppose we got a call for unsupported mode while we are in PERIODIC MODE. Driver will disable mode and return error, but core will expect it to generate events periodically.
So, I still feel these changes are required. But wasn't sure if I should send them separately before this series, with this series or after this one as a separate series.
Doing this seems to complicate things, and needlessly change/complicate current behavior, as well as make this patch more difficult to read/review.
So yes. Sending it here wasn't the best idea as anyway we aren't adding this bug in this series. Should have sent patches separately after this series.
In any case, I've given a quick skim over some of the driver changes below and found several (possible) issues.
Probably all related to above stuff only..
This will be tricky to get right on all platforms, so IMO the changes should be minimal.
Hmm, yes. Probably I will mention this problem in cover-letter and will mention that I will fix it separately before adding another mode.
diff --git a/arch/arm/mach-clps711x/common.c b/arch/arm/mach-clps711x/common.c index aee81fa..fa7c090 100644 --- a/arch/arm/mach-clps711x/common.c +++ b/arch/arm/mach-clps711x/common.c @@ -69,30 +69,30 @@ static u64 notrace clps711x_sched_clock_read(void) return ~readw_relaxed(CLPS711X_VIRT_BASE + TC1D); }
-static void clps711x_clockevent_set_mode(enum clock_event_mode mode, +static int clps711x_clockevent_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) {
disable_irq(IRQ_TC2OI);You probably noticed this, but this step is now skipped for _PERIODIC. Since the IRQ is immediately re-enabled, it's probably not a problem, but it *is* a functional change that should be noted.
Review for these is difficult and to mention them in patches is also difficult. There are so many drivers per patch.
diff --git a/arch/arm/mach-cns3xxx/core.c b/arch/arm/mach-cns3xxx/core.c
+static int cns3xxx_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *clk) { unsigned long ctrl = readl(cns3xxx_tmr1 + TIMER1_2_CONTROL_OFFSET); @@ -132,11 +132,15 @@ static void cns3xxx_timer_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN:
default:
case CLOCK_EVT_MODE_RESUME: ctrl = 0;break;default:return -ENOSYS; } writel(ctrl, cns3xxx_tmr1 + TIMER1_2_CONTROL_OFFSET);Hmm, looks like the timer is left disabled for the unsupported modes, which is the opposite of what's done elsewhere.
No. There is no writel() at top of this routine to disable events. We are doing it at then end for UNUSED/SHUTDOWN/RESUME but left it enabled for 'default case'..
return 0;}
diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c
+static int omap_32k_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) {
omap_32k_timer_stop();switch (mode) { case CLOCK_EVT_MODE_PERIODIC: omap_32k_timer_start(OMAP_32K_TIMER_TICK_PERIOD);Note that this is also a functional change because the timer doesn't get stopped before it is restarted in periodic mode. I can't remember if that's safe on OMAP1 (I think not), but it in any case, a functional change like should not be made without more thorough testing (IMO, this kind of functional change should not be part of this patch.)
I overlooked it. I thought it will work safely but probably it wouldn't.
Anyways, I am going to remove these kind of diffs now..
diff --git a/arch/arm/mach-pxa/time.c b/arch/arm/mach-pxa/time.c
pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) { switch (mode) { @@ -84,9 +84,11 @@ pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) break;
case CLOCK_EVT_MODE_RESUME:
case CLOCK_EVT_MODE_PERIODIC:Hmm, you're changing periodic mode from supported to unsupported. Why?
This should have been mentioned in logs, forgot it. Actually some drivers don't support these modes, like pxa doesn't support periodic mode, And they either have a BUG/WARN/pr_err or a simple 'break;' to handle these. I have dropped them as Thomas asked me to get rid of these BUG/WARN/pr_err etc in this series.
OK, I give up for now. I've found enough issues to convince myself that this needs some closer attention, and possibly a rethink about how to handle unsupported modes.
Yeah.. Let me try another series, it should be much easier to review that :)
Just to mention, I have kept another branch with such changes on top as I knew I may have to send them separately. Good, less work for me :)
Clockevents core now supports ->set_dev_mode() (as a replacement to ->set_mode()), with capability to return error codes.
Migrate all MIPS specific clockevent drivers to implement this new callback.
Drivers now return -ENOSYS when a unsupported mode is passed to their ->set_dev_mode() callbacks and return 0 on success.
Most of the changes are automated with help of Coccinelle (http://coccinelle.lip6.fr/) and the ones left are around the switch block which are handled manually.
Some drivers had a WARN()/BUG()/pr_err() for unsupported modes. These statements are removed now as proper error handling with a WARN_ON() is done at clockevents core.
Also it is made sure that drivers don't disable events for unsupported modes.
A simplified version of the semantic patch is:
@@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c);
@@ identifier setmode; @@ -void +int setmode(enum clock_event_mode, struct clock_event_device *);
@fixret@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c) { ... + return 0; }
@depends on fixret@ identifier ced; identifier fixret.setmode; @@ ... struct clock_event_device ced = { ..., -.set_mode +.set_dev_mode = setmode, };
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced->set_mode + ced->set_dev_mode = setmode
@depends on fixret@ identifier fixret.setmode; @@ { . -set_mode +set_dev_mode = setmode }
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced.set_mode + ced.set_dev_mode = setmode
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- arch/mips/alchemy/common/time.c | 14 ++++++++++++-- arch/mips/include/asm/cevt-r4k.h | 2 +- arch/mips/jazz/irq.c | 14 ++++++++++++-- arch/mips/jz4740/time.c | 9 ++++++--- arch/mips/kernel/cevt-bcm1480.c | 11 +++++++---- arch/mips/kernel/cevt-ds1287.c | 12 +++++++++--- arch/mips/kernel/cevt-gic.c | 14 ++++++++++++-- arch/mips/kernel/cevt-gt641xx.c | 12 +++++++++--- arch/mips/kernel/cevt-r4k.c | 14 ++++++++++++-- arch/mips/kernel/cevt-sb1250.c | 11 +++++++---- arch/mips/kernel/cevt-smtc.c | 2 +- arch/mips/kernel/cevt-txx9.c | 12 +++++++++--- arch/mips/loongson/common/cs5536/cs5536_mfgpt.c | 12 ++++++------ arch/mips/ralink/cevt-rt3352.c | 13 ++++++++----- arch/mips/sgi-ip27/ip27-timer.c | 14 ++++++++++++-- arch/mips/sni/time.c | 9 +++++---- 16 files changed, 128 insertions(+), 47 deletions(-)
diff --git a/arch/mips/alchemy/common/time.c b/arch/mips/alchemy/common/time.c index 93fa586..4af4ed2 100644 --- a/arch/mips/alchemy/common/time.c +++ b/arch/mips/alchemy/common/time.c @@ -70,9 +70,19 @@ static int au1x_rtcmatch2_set_next_event(unsigned long delta, return 0; }
-static void au1x_rtcmatch2_set_mode(enum clock_event_mode mode, +static int au1x_rtcmatch2_set_mode(enum clock_event_mode mode, struct clock_event_device *cd) { + switch (mode) { + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: + break; + default: + return -ENOSYS; + } + return 0; }
static irqreturn_t au1x_rtcmatch2_irq(int irq, void *dev_id) @@ -87,7 +97,7 @@ static struct clock_event_device au1x_rtcmatch2_clockdev = { .features = CLOCK_EVT_FEAT_ONESHOT, .rating = 1500, .set_next_event = au1x_rtcmatch2_set_next_event, - .set_mode = au1x_rtcmatch2_set_mode, + .set_dev_mode = au1x_rtcmatch2_set_mode, .cpumask = cpu_all_mask, };
diff --git a/arch/mips/include/asm/cevt-r4k.h b/arch/mips/include/asm/cevt-r4k.h index 65f9bdd..e4872df 100644 --- a/arch/mips/include/asm/cevt-r4k.h +++ b/arch/mips/include/asm/cevt-r4k.h @@ -21,7 +21,7 @@ DECLARE_PER_CPU(struct clock_event_device, mips_clockevent_device);
void mips_event_handler(struct clock_event_device *dev); int c0_compare_int_usable(void); -void mips_set_clock_mode(enum clock_event_mode, struct clock_event_device *); +int mips_set_clock_mode(enum clock_event_mode, struct clock_event_device *); irqreturn_t c0_compare_interrupt(int, void *);
extern struct irqaction c0_compare_irqaction; diff --git a/arch/mips/jazz/irq.c b/arch/mips/jazz/irq.c index e1ea4f6..1ceee92 100644 --- a/arch/mips/jazz/irq.c +++ b/arch/mips/jazz/irq.c @@ -110,10 +110,20 @@ asmlinkage void plat_irq_dispatch(void) } }
-static void r4030_set_mode(enum clock_event_mode mode, +static int r4030_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { + switch (mode) { + case CLOCK_EVT_MODE_PERIODIC: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: /* Nothing to do ... */ + break; + default: + return -ENOSYS; + } + return 0; }
struct clock_event_device r4030_clockevent = { @@ -121,7 +131,7 @@ struct clock_event_device r4030_clockevent = { .features = CLOCK_EVT_FEAT_PERIODIC, .rating = 300, .irq = JAZZ_TIMER_IRQ, - .set_mode = r4030_set_mode, + .set_dev_mode = r4030_set_mode, };
static irqreturn_t r4030_timer_interrupt(int irq, void *dev_id) diff --git a/arch/mips/jz4740/time.c b/arch/mips/jz4740/time.c index 5e430ce..2eb3fe1 100644 --- a/arch/mips/jz4740/time.c +++ b/arch/mips/jz4740/time.c @@ -57,7 +57,7 @@ static irqreturn_t jz4740_clockevent_irq(int irq, void *devid) return IRQ_HANDLED; }
-static void jz4740_clockevent_set_mode(enum clock_event_mode mode, +static int jz4740_clockevent_set_mode(enum clock_event_mode mode, struct clock_event_device *cd) { switch (mode) { @@ -72,9 +72,12 @@ static void jz4740_clockevent_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_SHUTDOWN: jz4740_timer_disable(TIMER_CLOCKEVENT); break; - default: + case CLOCK_EVT_MODE_UNUSED: break; + default: + return -ENOSYS; } + return 0; }
static int jz4740_clockevent_set_next(unsigned long evt, @@ -91,7 +94,7 @@ static struct clock_event_device jz4740_clockevent = { .name = "jz4740-timer", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .set_next_event = jz4740_clockevent_set_next, - .set_mode = jz4740_clockevent_set_mode, + .set_dev_mode = jz4740_clockevent_set_mode, .rating = 200, .irq = JZ4740_IRQ_TCU0, }; diff --git a/arch/mips/kernel/cevt-bcm1480.c b/arch/mips/kernel/cevt-bcm1480.c index 7976457..007fdb1 100644 --- a/arch/mips/kernel/cevt-bcm1480.c +++ b/arch/mips/kernel/cevt-bcm1480.c @@ -40,7 +40,7 @@ * The general purpose timer ticks at 1MHz independent if * the rest of the system */ -static void sibyte_set_mode(enum clock_event_mode mode, +static int sibyte_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { unsigned int cpu = smp_processor_id(); @@ -63,10 +63,13 @@ static void sibyte_set_mode(enum clock_event_mode mode, __raw_writeq(0, cfg); break;
- case CLOCK_EVT_MODE_UNUSED: /* shuddup gcc */ + case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_RESUME: - ; + break; + default: + return -ENOSYS; } + return 0; }
static int sibyte_next_event(unsigned long delta, struct clock_event_device *cd) @@ -130,7 +133,7 @@ void sb1480_clockevent_init(void) cd->irq = irq; cd->cpumask = cpumask_of(cpu); cd->set_next_event = sibyte_next_event; - cd->set_mode = sibyte_set_mode; + cd->set_dev_mode = sibyte_set_mode; clockevents_register_device(cd);
bcm1480_mask_irq(cpu, irq); diff --git a/arch/mips/kernel/cevt-ds1287.c b/arch/mips/kernel/cevt-ds1287.c index ff1f01b..f86642a 100644 --- a/arch/mips/kernel/cevt-ds1287.c +++ b/arch/mips/kernel/cevt-ds1287.c @@ -59,7 +59,7 @@ static int ds1287_set_next_event(unsigned long delta, return -EINVAL; }
-static void ds1287_set_mode(enum clock_event_mode mode, +static int ds1287_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { u8 val; @@ -72,14 +72,20 @@ static void ds1287_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_PERIODIC: val |= RTC_PIE; break; - default: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: val &= ~RTC_PIE; break; + default: + spin_unlock(&rtc_lock); + return -ENOSYS; }
CMOS_WRITE(val, RTC_REG_B);
spin_unlock(&rtc_lock); + return 0; }
static void ds1287_event_handler(struct clock_event_device *dev) @@ -90,7 +96,7 @@ static struct clock_event_device ds1287_clockevent = { .name = "ds1287", .features = CLOCK_EVT_FEAT_PERIODIC, .set_next_event = ds1287_set_next_event, - .set_mode = ds1287_set_mode, + .set_dev_mode = ds1287_set_mode, .event_handler = ds1287_event_handler, };
diff --git a/arch/mips/kernel/cevt-gic.c b/arch/mips/kernel/cevt-gic.c index 594cbbf..ae4b1ee 100644 --- a/arch/mips/kernel/cevt-gic.c +++ b/arch/mips/kernel/cevt-gic.c @@ -31,10 +31,20 @@ static int gic_next_event(unsigned long delta, struct clock_event_device *evt) return res; }
-void gic_set_clock_mode(enum clock_event_mode mode, +int gic_set_clock_mode(enum clock_event_mode mode, struct clock_event_device *evt) { + switch (mode) { + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: /* Nothing to do ... */ + break; + default: + return -ENOSYS; + } + return 0; }
irqreturn_t gic_compare_interrupt(int irq, void *dev_id) @@ -85,7 +95,7 @@ int gic_clockevent_init(void) cd->irq = irq; cd->cpumask = cpumask_of(cpu); cd->set_next_event = gic_next_event; - cd->set_mode = gic_set_clock_mode; + cd->set_dev_mode = gic_set_clock_mode; cd->event_handler = gic_event_handler;
clockevents_register_device(cd); diff --git a/arch/mips/kernel/cevt-gt641xx.c b/arch/mips/kernel/cevt-gt641xx.c index f069460..7c152ba 100644 --- a/arch/mips/kernel/cevt-gt641xx.c +++ b/arch/mips/kernel/cevt-gt641xx.c @@ -64,7 +64,7 @@ static int gt641xx_timer0_set_next_event(unsigned long delta, return 0; }
-static void gt641xx_timer0_set_mode(enum clock_event_mode mode, +static int gt641xx_timer0_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { u32 ctrl; @@ -81,13 +81,19 @@ static void gt641xx_timer0_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_ONESHOT: ctrl |= GT_TC_CONTROL_ENTC0_MSK; break; - default: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: break; + default: + raw_spin_unlock(>641xx_timer_lock); + return -ENOSYS; }
GT_WRITE(GT_TC_CONTROL_OFS, ctrl);
raw_spin_unlock(>641xx_timer_lock); + return 0; }
static void gt641xx_timer0_event_handler(struct clock_event_device *dev) @@ -99,7 +105,7 @@ static struct clock_event_device gt641xx_timer0_clockevent = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .irq = GT641XX_TIMER0_IRQ, .set_next_event = gt641xx_timer0_set_next_event, - .set_mode = gt641xx_timer0_set_mode, + .set_dev_mode = gt641xx_timer0_set_mode, .event_handler = gt641xx_timer0_event_handler, };
diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c index 50d3f5a..3ad0f42 100644 --- a/arch/mips/kernel/cevt-r4k.c +++ b/arch/mips/kernel/cevt-r4k.c @@ -38,10 +38,20 @@ static int mips_next_event(unsigned long delta,
#endif /* CONFIG_MIPS_MT_SMTC */
-void mips_set_clock_mode(enum clock_event_mode mode, +int mips_set_clock_mode(enum clock_event_mode mode, struct clock_event_device *evt) { + switch (mode) { + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: /* Nothing to do ... */ + break; + default: + return -ENOSYS; + } + return 0; }
DEFINE_PER_CPU(struct clock_event_device, mips_clockevent_device); @@ -207,7 +217,7 @@ int r4k_clockevent_init(void) cd->irq = irq; cd->cpumask = cpumask_of(cpu); cd->set_next_event = mips_next_event; - cd->set_mode = mips_set_clock_mode; + cd->set_dev_mode = mips_set_clock_mode; cd->event_handler = mips_event_handler;
#ifdef CONFIG_CEVT_GIC diff --git a/arch/mips/kernel/cevt-sb1250.c b/arch/mips/kernel/cevt-sb1250.c index 5ea6d6b..f7cf430 100644 --- a/arch/mips/kernel/cevt-sb1250.c +++ b/arch/mips/kernel/cevt-sb1250.c @@ -38,7 +38,7 @@ * The general purpose timer ticks at 1MHz independent if * the rest of the system */ -static void sibyte_set_mode(enum clock_event_mode mode, +static int sibyte_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { unsigned int cpu = smp_processor_id(); @@ -61,10 +61,13 @@ static void sibyte_set_mode(enum clock_event_mode mode, __raw_writeq(0, cfg); break;
- case CLOCK_EVT_MODE_UNUSED: /* shuddup gcc */ + case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_RESUME: - ; + break; + default: + return -ENOSYS; } + return 0; }
static int sibyte_next_event(unsigned long delta, struct clock_event_device *cd) @@ -129,7 +132,7 @@ void sb1250_clockevent_init(void) cd->irq = irq; cd->cpumask = cpumask_of(cpu); cd->set_next_event = sibyte_next_event; - cd->set_mode = sibyte_set_mode; + cd->set_dev_mode = sibyte_set_mode; clockevents_register_device(cd);
sb1250_mask_irq(cpu, irq); diff --git a/arch/mips/kernel/cevt-smtc.c b/arch/mips/kernel/cevt-smtc.c index b6cf0a6..0d41366 100644 --- a/arch/mips/kernel/cevt-smtc.c +++ b/arch/mips/kernel/cevt-smtc.c @@ -297,7 +297,7 @@ int smtc_clockevent_init(void) cd->irq = irq; cd->cpumask = cpumask_of(cpu); cd->set_next_event = mips_next_event; - cd->set_mode = mips_set_clock_mode; + cd->set_dev_mode = mips_set_clock_mode; cd->event_handler = mips_event_handler;
clockevents_register_device(cd); diff --git a/arch/mips/kernel/cevt-txx9.c b/arch/mips/kernel/cevt-txx9.c index 2ae0846..f227309 100644 --- a/arch/mips/kernel/cevt-txx9.c +++ b/arch/mips/kernel/cevt-txx9.c @@ -76,16 +76,16 @@ static void txx9tmr_stop_and_clear(struct txx9_tmr_reg __iomem *tmrptr) __raw_writel(0, &tmrptr->tisr); }
-static void txx9tmr_set_mode(enum clock_event_mode mode, +static int txx9tmr_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { struct txx9_clock_event_device *txx9_cd = container_of(evt, struct txx9_clock_event_device, cd); struct txx9_tmr_reg __iomem *tmrptr = txx9_cd->tmrptr;
- txx9tmr_stop_and_clear(tmrptr); switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + txx9tmr_stop_and_clear(tmrptr); __raw_writel(TXx9_TMITMR_TIIE | TXx9_TMITMR_TZCE, &tmrptr->itmr); /* start timer */ @@ -96,16 +96,22 @@ static void txx9tmr_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_UNUSED: + txx9tmr_stop_and_clear(tmrptr); __raw_writel(0, &tmrptr->itmr); break; case CLOCK_EVT_MODE_ONESHOT: + txx9tmr_stop_and_clear(tmrptr); __raw_writel(TXx9_TMITMR_TIIE, &tmrptr->itmr); break; case CLOCK_EVT_MODE_RESUME: + txx9tmr_stop_and_clear(tmrptr); __raw_writel(TIMER_CCD, &tmrptr->ccdr); __raw_writel(0, &tmrptr->itmr); break; + default: + return -ENOSYS; } + return 0; }
static int txx9tmr_set_next_event(unsigned long delta, @@ -128,7 +134,7 @@ static struct txx9_clock_event_device txx9_clock_event_device = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .rating = 200, - .set_mode = txx9tmr_set_mode, + .set_dev_mode = txx9tmr_set_mode, .set_next_event = txx9tmr_set_next_event, }, }; diff --git a/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c b/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c index c639b9d..846f4ab 100644 --- a/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c +++ b/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c @@ -52,7 +52,7 @@ void enable_mfgpt0_counter(void) } EXPORT_SYMBOL(enable_mfgpt0_counter);
-static void init_mfgpt_timer(enum clock_event_mode mode, +static int init_mfgpt_timer(enum clock_event_mode mode, struct clock_event_device *evt) { spin_lock(&mfgpt_lock); @@ -71,21 +71,21 @@ static void init_mfgpt_timer(enum clock_event_mode mode, disable_mfgpt0_counter(); break;
- case CLOCK_EVT_MODE_ONESHOT: - /* The oneshot mode have very high deviation, Not use it! */ - break; - case CLOCK_EVT_MODE_RESUME: /* Nothing to do here */ break; + default: + spin_unlock(&mfgpt_lock); + return -ENOSYS; } spin_unlock(&mfgpt_lock); + return 0; }
static struct clock_event_device mfgpt_clockevent = { .name = "mfgpt", .features = CLOCK_EVT_FEAT_PERIODIC, - .set_mode = init_mfgpt_timer, + .set_dev_mode = init_mfgpt_timer, .irq = CS5536_MFGPT_INTR, };
diff --git a/arch/mips/ralink/cevt-rt3352.c b/arch/mips/ralink/cevt-rt3352.c index 24bf057..c92b211 100644 --- a/arch/mips/ralink/cevt-rt3352.c +++ b/arch/mips/ralink/cevt-rt3352.c @@ -36,7 +36,7 @@ struct systick_device { int freq_scale; };
-static void systick_set_clock_mode(enum clock_event_mode mode, +static int systick_set_clock_mode(enum clock_event_mode mode, struct clock_event_device *evt);
static int systick_next_event(unsigned long delta, @@ -76,7 +76,7 @@ static struct systick_device systick = { .rating = 310, .features = CLOCK_EVT_FEAT_ONESHOT, .set_next_event = systick_next_event, - .set_mode = systick_set_clock_mode, + .set_dev_mode = systick_set_clock_mode, .event_handler = systick_event_handler, }, }; @@ -87,7 +87,7 @@ static struct irqaction systick_irqaction = { .dev_id = &systick.dev, };
-static void systick_set_clock_mode(enum clock_event_mode mode, +static int systick_set_clock_mode(enum clock_event_mode mode, struct clock_event_device *evt) { struct systick_device *sdev; @@ -110,10 +110,13 @@ static void systick_set_clock_mode(enum clock_event_mode mode, iowrite32(0, systick.membase + SYSTICK_CONFIG); break;
- default: - pr_err("%s: Unhandeled mips clock_mode\n", systick.dev.name); + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static void __init ralink_systick_init(struct device_node *np) diff --git a/arch/mips/sgi-ip27/ip27-timer.c b/arch/mips/sgi-ip27/ip27-timer.c index 1d97eab..5d5e90c 100644 --- a/arch/mips/sgi-ip27/ip27-timer.c +++ b/arch/mips/sgi-ip27/ip27-timer.c @@ -63,10 +63,20 @@ static int rt_next_event(unsigned long delta, struct clock_event_device *evt) return LOCAL_HUB_L(PI_RT_COUNT) >= cnt ? -ETIME : 0; }
-static void rt_set_mode(enum clock_event_mode mode, +static int rt_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { + switch (mode) { + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: /* Nothing to do ... */ + break; + default: + return -ENOSYS; + } + return 0; }
unsigned int rt_timer_irq; @@ -123,7 +133,7 @@ void hub_rt_clock_event_init(void) cd->irq = irq; cd->cpumask = cpumask_of(cpu); cd->set_next_event = rt_next_event; - cd->set_mode = rt_set_mode; + cd->set_dev_mode = rt_set_mode; clockevents_register_device(cd); }
diff --git a/arch/mips/sni/time.c b/arch/mips/sni/time.c index cf8ec56..ac666e5 100644 --- a/arch/mips/sni/time.c +++ b/arch/mips/sni/time.c @@ -14,7 +14,7 @@ #define SNI_COUNTER2_DIV 64 #define SNI_COUNTER0_DIV ((SNI_CLOCK_TICK_RATE / SNI_COUNTER2_DIV) / HZ)
-static void a20r_set_mode(enum clock_event_mode mode, +static int a20r_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -34,13 +34,14 @@ static void a20r_set_mode(enum clock_event_mode mode, wmb();
break; - case CLOCK_EVT_MODE_ONESHOT: case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: - break; case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device a20r_clockevent_device = { @@ -51,7 +52,7 @@ static struct clock_event_device a20r_clockevent_device = {
.rating = 300, .irq = SNI_A20R_IRQ_TIMER, - .set_mode = a20r_set_mode, + .set_dev_mode = a20r_set_mode, };
static irqreturn_t a20r_interrupt(int irq, void *dev_id)
Clockevents core now supports ->set_dev_mode() (as a replacement to ->set_mode()), with capability to return error codes.
Migrate all SPARC specific clockevent drivers to implement this new callback.
Drivers now return -ENOSYS when a unsupported mode is passed to their ->set_dev_mode() callbacks and return 0 on success.
Most of the changes are automated with help of Coccinelle (http://coccinelle.lip6.fr/) and the ones left are around the switch block which are handled manually.
Some drivers had a WARN()/BUG()/pr_err() for unsupported modes. These statements are removed now as proper error handling with a WARN_ON() is done at clockevents core.
Also it is made sure that drivers don't disable events for unsupported modes.
A simplified version of the semantic patch is:
@@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c);
@@ identifier setmode; @@ -void +int setmode(enum clock_event_mode, struct clock_event_device *);
@fixret@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c) { ... + return 0; }
@depends on fixret@ identifier ced; identifier fixret.setmode; @@ ... struct clock_event_device ced = { ..., -.set_mode +.set_dev_mode = setmode, };
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced->set_mode + ced->set_dev_mode = setmode
@depends on fixret@ identifier fixret.setmode; @@ { . -set_mode +set_dev_mode = setmode }
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced.set_mode + ced.set_dev_mode = setmode
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- arch/sparc/kernel/time_32.c | 18 ++++++++++++------ arch/sparc/kernel/time_64.c | 11 +++++------ 2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/arch/sparc/kernel/time_32.c b/arch/sparc/kernel/time_32.c index c4c27b0..4861c53 100644 --- a/arch/sparc/kernel/time_32.c +++ b/arch/sparc/kernel/time_32.c @@ -107,7 +107,7 @@ irqreturn_t notrace timer_interrupt(int dummy, void *dev_id) return IRQ_HANDLED; }
-static void timer_ce_set_mode(enum clock_event_mode mode, +static int timer_ce_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -118,10 +118,13 @@ static void timer_ce_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_SHUTDOWN: timer_ce_enabled = 0; break; - default: + case CLOCK_EVT_MODE_UNUSED: break; + default: + return -ENOSYS; } smp_mb(); + return 0; }
static __init void setup_timer_ce(void) @@ -133,7 +136,7 @@ static __init void setup_timer_ce(void) ce->name = "timer_ce"; ce->rating = 100; ce->features = CLOCK_EVT_FEAT_PERIODIC; - ce->set_mode = timer_ce_set_mode; + ce->set_dev_mode = timer_ce_set_mode; ce->cpumask = cpu_possible_mask; ce->shift = 32; ce->mult = div_sc(sparc_config.clock_rate, NSEC_PER_SEC, @@ -193,7 +196,7 @@ static __init int setup_timer_cs(void) }
#ifdef CONFIG_SMP -static void percpu_ce_setup(enum clock_event_mode mode, +static int percpu_ce_setup(enum clock_event_mode mode, struct clock_event_device *evt) { int cpu = __first_cpu(evt->cpumask); @@ -208,9 +211,12 @@ static void percpu_ce_setup(enum clock_event_mode mode, case CLOCK_EVT_MODE_UNUSED: sparc_config.load_profile_irq(cpu, 0); break; - default: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static int percpu_ce_set_next_event(unsigned long delta, @@ -234,7 +240,7 @@ void register_percpu_ce(int cpu) ce->name = "percpu_ce"; ce->rating = 200; ce->features = features; - ce->set_mode = percpu_ce_setup; + ce->set_dev_mode = percpu_ce_setup; ce->set_next_event = percpu_ce_set_next_event; ce->cpumask = cpumask_of(cpu); ce->shift = 32; diff --git a/arch/sparc/kernel/time_64.c b/arch/sparc/kernel/time_64.c index 3fddf64..1708edc 100644 --- a/arch/sparc/kernel/time_64.c +++ b/arch/sparc/kernel/time_64.c @@ -691,7 +691,7 @@ static int sparc64_next_event(unsigned long delta, return tick_ops->add_compare(delta) ? -ETIME : 0; }
-static void sparc64_timer_setup(enum clock_event_mode mode, +static int sparc64_timer_setup(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -703,16 +703,15 @@ static void sparc64_timer_setup(enum clock_event_mode mode, tick_ops->disable_irq(); break;
- case CLOCK_EVT_MODE_PERIODIC: - case CLOCK_EVT_MODE_UNUSED: - WARN_ON(1); - break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device sparc64_clockevent = { .features = CLOCK_EVT_FEAT_ONESHOT, - .set_mode = sparc64_timer_setup, + .set_dev_mode = sparc64_timer_setup, .set_next_event = sparc64_next_event, .rating = 100, .shift = 30,
Clockevents core now supports ->set_dev_mode() (as a replacement to ->set_mode()), with capability to return error codes.
Migrate all x86 specific clockevent drivers to implement this new callback.
Drivers now return -ENOSYS when a unsupported mode is passed to their ->set_dev_mode() callbacks and return 0 on success.
Most of the changes are automated with help of Coccinelle (http://coccinelle.lip6.fr/) and the ones left are around the switch block which are handled manually.
Some drivers had a WARN()/BUG()/pr_err() for unsupported modes. These statements are removed now as proper error handling with a WARN_ON() is done at clockevents core.
Also it is made sure that drivers don't disable events for unsupported modes.
A simplified version of the semantic patch is:
@@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c);
@@ identifier setmode; @@ -void +int setmode(enum clock_event_mode, struct clock_event_device *);
@fixret@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c) { ... + return 0; }
@depends on fixret@ identifier ced; identifier fixret.setmode; @@ ... struct clock_event_device ced = { ..., -.set_mode +.set_dev_mode = setmode, };
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced->set_mode + ced->set_dev_mode = setmode
@depends on fixret@ identifier fixret.setmode; @@ { . -set_mode +set_dev_mode = setmode }
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced.set_mode + ced.set_dev_mode = setmode
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- arch/x86/kernel/apic/apic.c | 10 +++++++--- arch/x86/kernel/hpet.c | 19 +++++++++++-------- arch/x86/lguest/boot.c | 9 +++++---- arch/x86/platform/uv/uv_time.c | 10 ++++++---- arch/x86/xen/time.c | 23 ++++++++++------------- 5 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index ad28db7..f89084e 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -495,7 +495,7 @@ static int lapic_next_deadline(unsigned long delta, /* * Setup the lapic timer in periodic or oneshot mode */ -static void lapic_timer_setup(enum clock_event_mode mode, +static int lapic_timer_setup(enum clock_event_mode mode, struct clock_event_device *evt) { unsigned long flags; @@ -503,7 +503,7 @@ static void lapic_timer_setup(enum clock_event_mode mode,
/* Lapic used as dummy for broadcast ? */ if (evt->features & CLOCK_EVT_FEAT_DUMMY) - return; + return 0;
local_irq_save(flags);
@@ -523,9 +523,13 @@ static void lapic_timer_setup(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: /* Nothing to do here */ break; + default: + local_irq_restore(flags); + return -ENOSYS; }
local_irq_restore(flags); + return 0; }
/* @@ -547,7 +551,7 @@ static struct clock_event_device lapic_clockevent = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP | CLOCK_EVT_FEAT_DUMMY, .shift = 32, - .set_mode = lapic_timer_setup, + .set_dev_mode = lapic_timer_setup, .set_next_event = lapic_next_event, .broadcast = lapic_timer_broadcast, .rating = 100, diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index 4177bfb..8cab472 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -228,7 +228,7 @@ static void hpet_reserve_platform_timers(unsigned int id) { } */ static unsigned long hpet_freq;
-static void hpet_legacy_set_mode(enum clock_event_mode mode, +static int hpet_legacy_set_mode(enum clock_event_mode mode, struct clock_event_device *evt); static int hpet_legacy_next_event(unsigned long delta, struct clock_event_device *evt); @@ -239,7 +239,7 @@ static int hpet_legacy_next_event(unsigned long delta, static struct clock_event_device hpet_clockevent = { .name = "hpet", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = hpet_legacy_set_mode, + .set_dev_mode = hpet_legacy_set_mode, .set_next_event = hpet_legacy_next_event, .irq = 0, .rating = 50, @@ -310,7 +310,7 @@ static void hpet_legacy_clockevent_register(void)
static int hpet_setup_msi_irq(unsigned int irq);
-static void hpet_set_mode(enum clock_event_mode mode, +static int hpet_set_mode(enum clock_event_mode mode, struct clock_event_device *evt, int timer) { unsigned int cfg, cmp, now; @@ -367,7 +367,10 @@ static void hpet_set_mode(enum clock_event_mode mode, } hpet_print_config(); break; + default: + return -ENOSYS; } + return 0; }
static int hpet_next_event(unsigned long delta, @@ -407,10 +410,10 @@ static int hpet_next_event(unsigned long delta, return res < HPET_MIN_CYCLES ? -ETIME : 0; }
-static void hpet_legacy_set_mode(enum clock_event_mode mode, +static int hpet_legacy_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { - hpet_set_mode(mode, evt, 0); + return hpet_set_mode(mode, evt, 0); }
static int hpet_legacy_next_event(unsigned long delta, @@ -462,11 +465,11 @@ void hpet_msi_read(struct hpet_dev *hdev, struct msi_msg *msg) msg->address_hi = 0; }
-static void hpet_msi_set_mode(enum clock_event_mode mode, +static int hpet_msi_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { struct hpet_dev *hdev = EVT_TO_HPET_DEV(evt); - hpet_set_mode(mode, evt, hdev->num); + return hpet_set_mode(mode, evt, hdev->num); }
static int hpet_msi_next_event(unsigned long delta, @@ -558,7 +561,7 @@ static void init_one_hpet_msi_clockevent(struct hpet_dev *hdev, int cpu) if (hdev->flags & HPET_DEV_PERI_CAP) evt->features |= CLOCK_EVT_FEAT_PERIODIC;
- evt->set_mode = hpet_msi_set_mode; + evt->set_dev_mode = hpet_msi_set_mode; evt->set_next_event = hpet_msi_next_event; evt->cpumask = cpumask_of(hdev->cpu);
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c index aae9413..54aa944 100644 --- a/arch/x86/lguest/boot.c +++ b/arch/x86/lguest/boot.c @@ -962,7 +962,7 @@ static int lguest_clockevent_set_next_event(unsigned long delta, return 0; }
-static void lguest_clockevent_set_mode(enum clock_event_mode mode, +static int lguest_clockevent_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -974,11 +974,12 @@ static void lguest_clockevent_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_ONESHOT: /* This is what we expect. */ break; - case CLOCK_EVT_MODE_PERIODIC: - BUG(); case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
/* This describes our primitive timer chip. */ @@ -986,7 +987,7 @@ static struct clock_event_device lguest_clockevent = { .name = "lguest", .features = CLOCK_EVT_FEAT_ONESHOT, .set_next_event = lguest_clockevent_set_next_event, - .set_mode = lguest_clockevent_set_mode, + .set_dev_mode = lguest_clockevent_set_mode, .rating = INT_MAX, .mult = 1, .shift = 0, diff --git a/arch/x86/platform/uv/uv_time.c b/arch/x86/platform/uv/uv_time.c index 5c86786..b9c822d 100644 --- a/arch/x86/platform/uv/uv_time.c +++ b/arch/x86/platform/uv/uv_time.c @@ -32,7 +32,7 @@
static cycle_t uv_read_rtc(struct clocksource *cs); static int uv_rtc_next_event(unsigned long, struct clock_event_device *); -static void uv_rtc_timer_setup(enum clock_event_mode, +static int uv_rtc_timer_setup(enum clock_event_mode, struct clock_event_device *);
static struct clocksource clocksource_uv = { @@ -50,7 +50,7 @@ static struct clock_event_device clock_event_device_uv = { .rating = 400, .irq = -1, .set_next_event = uv_rtc_next_event, - .set_mode = uv_rtc_timer_setup, + .set_dev_mode = uv_rtc_timer_setup, .event_handler = NULL, };
@@ -323,13 +323,12 @@ static int uv_rtc_next_event(unsigned long delta, /* * Setup the RTC timer in oneshot mode */ -static void uv_rtc_timer_setup(enum clock_event_mode mode, +static int uv_rtc_timer_setup(enum clock_event_mode mode, struct clock_event_device *evt) { int ced_cpu = cpumask_first(evt->cpumask);
switch (mode) { - case CLOCK_EVT_MODE_PERIODIC: case CLOCK_EVT_MODE_ONESHOT: case CLOCK_EVT_MODE_RESUME: /* Nothing to do here yet */ @@ -338,7 +337,10 @@ static void uv_rtc_timer_setup(enum clock_event_mode mode, case CLOCK_EVT_MODE_SHUTDOWN: uv_rtc_unset_timer(ced_cpu, 1); break; + default: + return -ENOSYS; } + return 0; }
static void uv_rtc_interrupt(void) diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c index 7b78f88..08952ed 100644 --- a/arch/x86/xen/time.c +++ b/arch/x86/xen/time.c @@ -274,15 +274,10 @@ static s64 get_abs_timeout(unsigned long delta) return xen_clocksource_read() + delta; }
-static void xen_timerop_set_mode(enum clock_event_mode mode, +static int xen_timerop_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { - case CLOCK_EVT_MODE_PERIODIC: - /* unsupported */ - WARN_ON(1); - break; - case CLOCK_EVT_MODE_ONESHOT: case CLOCK_EVT_MODE_RESUME: break; @@ -291,7 +286,10 @@ static void xen_timerop_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_SHUTDOWN: HYPERVISOR_set_timer_op(0); /* cancel timeout */ break; + default: + return -ENOSYS; } + return 0; }
static int xen_timerop_set_next_event(unsigned long delta, @@ -320,22 +318,18 @@ static const struct clock_event_device xen_timerop_clockevent = { .shift = 0, .rating = 500,
- .set_mode = xen_timerop_set_mode, + .set_dev_mode = xen_timerop_set_mode, .set_next_event = xen_timerop_set_next_event, };
-static void xen_vcpuop_set_mode(enum clock_event_mode mode, +static int xen_vcpuop_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { int cpu = smp_processor_id();
switch (mode) { - case CLOCK_EVT_MODE_PERIODIC: - WARN_ON(1); /* unsupported */ - break; - case CLOCK_EVT_MODE_ONESHOT: if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, cpu, NULL)) BUG(); @@ -349,7 +343,10 @@ static void xen_vcpuop_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static int xen_vcpuop_set_next_event(unsigned long delta, @@ -382,7 +379,7 @@ static const struct clock_event_device xen_vcpuop_clockevent = { .shift = 0, .rating = 500,
- .set_mode = xen_vcpuop_set_mode, + .set_dev_mode = xen_vcpuop_set_mode, .set_next_event = xen_vcpuop_set_next_event, };
Clockevents core now supports ->set_dev_mode() (as a replacement to ->set_mode()), with capability to return error codes.
Migrate all drivers in 'drivers/clocksource' to implement this new callback.
Drivers now return -ENOSYS when a unsupported mode is passed to their ->set_dev_mode() callbacks and return 0 on success.
Most of the changes are automated with help of Coccinelle (http://coccinelle.lip6.fr/) and the ones left are around the switch block which are handled manually.
Some drivers had a WARN()/BUG()/pr_err() for unsupported modes. These statements are removed now as proper error handling with a WARN_ON() is done at clockevents core.
Also it is made sure that drivers don't disable events for unsupported modes.
A simplified version of the semantic patch is:
@@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c);
@@ identifier setmode; @@ -void +int setmode(enum clock_event_mode, struct clock_event_device *);
@fixret@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c) { ... + return 0; }
@depends on fixret@ identifier ced; identifier fixret.setmode; @@ ... struct clock_event_device ced = { ..., -.set_mode +.set_dev_mode = setmode, };
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced->set_mode + ced->set_dev_mode = setmode
@depends on fixret@ identifier fixret.setmode; @@ { . -set_mode +set_dev_mode = setmode }
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced.set_mode + ced.set_dev_mode = setmode
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/clocksource/arm_arch_timer.c | 36 ++++++++++++++++++-------------- drivers/clocksource/arm_global_timer.c | 9 +++++--- drivers/clocksource/bcm2835_timer.c | 8 +++---- drivers/clocksource/bcm_kona_timer.c | 10 ++++++--- drivers/clocksource/cadence_ttc_timer.c | 7 +++++-- drivers/clocksource/cs5535-clockevt.c | 21 ++++++++++++++----- drivers/clocksource/dummy_timer.c | 15 +++++++++++-- drivers/clocksource/dw_apb_timer.c | 7 +++++-- drivers/clocksource/em_sti.c | 11 ++++++---- drivers/clocksource/exynos_mct.c | 23 +++++++++++++------- drivers/clocksource/i8253.c | 8 +++++-- drivers/clocksource/metag_generic.c | 11 +++++----- drivers/clocksource/moxart_timer.c | 8 ++++--- drivers/clocksource/mxs_timer.c | 17 ++++++++------- drivers/clocksource/nomadik-mtu.c | 7 +++++-- drivers/clocksource/qcom-timer.c | 12 +++++------ drivers/clocksource/samsung_pwm_timer.c | 13 ++++++------ drivers/clocksource/sh_cmt.c | 9 +++++--- drivers/clocksource/sh_mtu2.c | 9 +++++--- drivers/clocksource/sh_tmu.c | 9 +++++--- drivers/clocksource/sun4i_timer.c | 9 +++++--- drivers/clocksource/tcb_clksrc.c | 11 +++++++--- drivers/clocksource/tegra20_timer.c | 12 ++++++----- drivers/clocksource/time-armada-370-xp.c | 20 ++++++++++++------ drivers/clocksource/time-efm32.c | 7 +++++-- drivers/clocksource/time-orion.c | 17 +++++++++++---- drivers/clocksource/timer-keystone.c | 9 +++++--- drivers/clocksource/timer-marco.c | 12 +++++++---- drivers/clocksource/timer-prima2.c | 10 ++++----- drivers/clocksource/timer-sun5i.c | 10 ++++++--- drivers/clocksource/timer-u300.c | 7 +++++-- drivers/clocksource/vf_pit_timer.c | 12 ++++++++--- drivers/clocksource/vt8500_timer.c | 8 ++++--- drivers/clocksource/zevio-timer.c | 9 ++++---- 34 files changed, 261 insertions(+), 142 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 5163ec1..44f1eaa 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -179,7 +179,7 @@ static irqreturn_t arch_timer_handler_virt_mem(int irq, void *dev_id) return timer_handler(ARCH_TIMER_MEM_VIRT_ACCESS, evt); }
-static __always_inline void timer_set_mode(const int access, int mode, +static __always_inline int timer_set_mode(const int access, int mode, struct clock_event_device *clk) { unsigned long ctrl; @@ -190,33 +190,37 @@ static __always_inline void timer_set_mode(const int access, int mode, ctrl &= ~ARCH_TIMER_CTRL_ENABLE; arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk); break; - default: + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
-static void arch_timer_set_mode_virt(enum clock_event_mode mode, +static int arch_timer_set_mode_virt(enum clock_event_mode mode, struct clock_event_device *clk) { - timer_set_mode(ARCH_TIMER_VIRT_ACCESS, mode, clk); + return timer_set_mode(ARCH_TIMER_VIRT_ACCESS, mode, clk); }
-static void arch_timer_set_mode_phys(enum clock_event_mode mode, +static int arch_timer_set_mode_phys(enum clock_event_mode mode, struct clock_event_device *clk) { - timer_set_mode(ARCH_TIMER_PHYS_ACCESS, mode, clk); + return timer_set_mode(ARCH_TIMER_PHYS_ACCESS, mode, clk); }
-static void arch_timer_set_mode_virt_mem(enum clock_event_mode mode, +static int arch_timer_set_mode_virt_mem(enum clock_event_mode mode, struct clock_event_device *clk) { - timer_set_mode(ARCH_TIMER_MEM_VIRT_ACCESS, mode, clk); + return timer_set_mode(ARCH_TIMER_MEM_VIRT_ACCESS, mode, clk); }
-static void arch_timer_set_mode_phys_mem(enum clock_event_mode mode, +static int arch_timer_set_mode_phys_mem(enum clock_event_mode mode, struct clock_event_device *clk) { - timer_set_mode(ARCH_TIMER_MEM_PHYS_ACCESS, mode, clk); + return timer_set_mode(ARCH_TIMER_MEM_PHYS_ACCESS, mode, clk); }
static __always_inline void set_next_event(const int access, unsigned long evt, @@ -271,11 +275,11 @@ static void __arch_timer_setup(unsigned type, clk->cpumask = cpumask_of(smp_processor_id()); if (arch_timer_use_virtual) { clk->irq = arch_timer_ppi[VIRT_PPI]; - clk->set_mode = arch_timer_set_mode_virt; + clk->set_dev_mode = arch_timer_set_mode_virt; clk->set_next_event = arch_timer_set_next_event_virt; } else { clk->irq = arch_timer_ppi[PHYS_SECURE_PPI]; - clk->set_mode = arch_timer_set_mode_phys; + clk->set_dev_mode = arch_timer_set_mode_phys; clk->set_next_event = arch_timer_set_next_event_phys; } } else { @@ -284,17 +288,17 @@ static void __arch_timer_setup(unsigned type, clk->rating = 400; clk->cpumask = cpu_all_mask; if (arch_timer_mem_use_virtual) { - clk->set_mode = arch_timer_set_mode_virt_mem; + clk->set_dev_mode = arch_timer_set_mode_virt_mem; clk->set_next_event = arch_timer_set_next_event_virt_mem; } else { - clk->set_mode = arch_timer_set_mode_phys_mem; + clk->set_dev_mode = arch_timer_set_mode_phys_mem; clk->set_next_event = arch_timer_set_next_event_phys_mem; } }
- clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, clk); + clk->set_dev_mode(CLOCK_EVT_MODE_SHUTDOWN, clk);
clockevents_config_and_register(clk, arch_timer_rate, 0xf, 0x7fffffff); } @@ -457,7 +461,7 @@ static void arch_timer_stop(struct clock_event_device *clk) disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]); }
- clk->set_mode(CLOCK_EVT_MODE_UNUSED, clk); + clk->set_dev_mode(CLOCK_EVT_MODE_UNUSED, clk); }
static int arch_timer_cpu_notify(struct notifier_block *self, diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c index 0fc31d0..937e039 100644 --- a/drivers/clocksource/arm_global_timer.c +++ b/drivers/clocksource/arm_global_timer.c @@ -107,7 +107,7 @@ static void gt_compare_set(unsigned long delta, int periodic) writel(ctrl, gt_base + GT_CONTROL); }
-static void gt_clockevent_set_mode(enum clock_event_mode mode, +static int gt_clockevent_set_mode(enum clock_event_mode mode, struct clock_event_device *clk) { unsigned long ctrl; @@ -124,9 +124,12 @@ static void gt_clockevent_set_mode(enum clock_event_mode mode, GT_CONTROL_IRQ_ENABLE | GT_CONTROL_AUTO_INC); writel(ctrl, gt_base + GT_CONTROL); break; - default: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static int gt_clockevent_set_next_event(unsigned long evt, @@ -171,7 +174,7 @@ static int gt_clockevents_init(struct clock_event_device *clk) clk->name = "arm_global_timer"; clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU; - clk->set_mode = gt_clockevent_set_mode; + clk->set_dev_mode = gt_clockevent_set_mode; clk->set_next_event = gt_clockevent_set_next_event; clk->cpumask = cpumask_of(cpu); clk->rating = 300; diff --git a/drivers/clocksource/bcm2835_timer.c b/drivers/clocksource/bcm2835_timer.c index 26ed331..455b5dd 100644 --- a/drivers/clocksource/bcm2835_timer.c +++ b/drivers/clocksource/bcm2835_timer.c @@ -54,7 +54,7 @@ static u64 notrace bcm2835_sched_read(void) return readl_relaxed(system_clock); }
-static void bcm2835_time_set_mode(enum clock_event_mode mode, +static int bcm2835_time_set_mode(enum clock_event_mode mode, struct clock_event_device *evt_dev) { switch (mode) { @@ -64,9 +64,9 @@ static void bcm2835_time_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: break; default: - WARN(1, "%s: unhandled event mode %d\n", __func__, mode); - break; + return -ENOSYS; } + return 0; }
static int bcm2835_time_set_next_event(unsigned long event, @@ -129,7 +129,7 @@ static void __init bcm2835_timer_init(struct device_node *node) timer->evt.name = node->name; timer->evt.rating = 300; timer->evt.features = CLOCK_EVT_FEAT_ONESHOT; - timer->evt.set_mode = bcm2835_time_set_mode; + timer->evt.set_dev_mode = bcm2835_time_set_mode; timer->evt.set_next_event = bcm2835_time_set_next_event; timer->evt.cpumask = cpumask_of(0); timer->act.name = node->name; diff --git a/drivers/clocksource/bcm_kona_timer.c b/drivers/clocksource/bcm_kona_timer.c index 0595dc6..a3899f4 100644 --- a/drivers/clocksource/bcm_kona_timer.c +++ b/drivers/clocksource/bcm_kona_timer.c @@ -128,7 +128,7 @@ static int kona_timer_set_next_event(unsigned long clc, return 0; }
-static void kona_timer_set_mode(enum clock_event_mode mode, +static int kona_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *unused) { switch (mode) { @@ -137,16 +137,20 @@ static void kona_timer_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: - default: + case CLOCK_EVT_MODE_RESUME: kona_timer_disable_and_clear(timers.tmr_regs); + break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device kona_clockevent_timer = { .name = "timer 1", .features = CLOCK_EVT_FEAT_ONESHOT, .set_next_event = kona_timer_set_next_event, - .set_mode = kona_timer_set_mode + .set_dev_mode = kona_timer_set_mode };
static void __init kona_timer_clockevents_init(void) diff --git a/drivers/clocksource/cadence_ttc_timer.c b/drivers/clocksource/cadence_ttc_timer.c index 49fbe28..3d355ec 100644 --- a/drivers/clocksource/cadence_ttc_timer.c +++ b/drivers/clocksource/cadence_ttc_timer.c @@ -196,7 +196,7 @@ static int ttc_set_next_event(unsigned long cycles, * @mode: Mode to be set * @evt: Address of clock event instance **/ -static void ttc_set_mode(enum clock_event_mode mode, +static int ttc_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { struct ttc_timer_clockevent *ttce = to_ttc_timer_clkevent(evt); @@ -224,7 +224,10 @@ static void ttc_set_mode(enum clock_event_mode mode, __raw_writel(ctrl_reg, timer->base_addr + TTC_CNT_CNTRL_OFFSET); break; + default: + return -ENOSYS; } + return 0; }
static int ttc_rate_change_clocksource_cb(struct notifier_block *nb, @@ -428,7 +431,7 @@ static void __init ttc_setup_clockevent(struct clk *clk, ttcce->ce.name = "ttc_clockevent"; ttcce->ce.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT; ttcce->ce.set_next_event = ttc_set_next_event; - ttcce->ce.set_mode = ttc_set_mode; + ttcce->ce.set_dev_mode = ttc_set_mode; ttcce->ce.rating = 200; ttcce->ce.irq = irq; ttcce->ce.cpumask = cpu_possible_mask; diff --git a/drivers/clocksource/cs5535-clockevt.c b/drivers/clocksource/cs5535-clockevt.c index db21052..8d7c9da 100644 --- a/drivers/clocksource/cs5535-clockevt.c +++ b/drivers/clocksource/cs5535-clockevt.c @@ -77,15 +77,26 @@ static void start_timer(struct cs5535_mfgpt_timer *timer, uint16_t delta) MFGPT_SETUP_CNTEN | MFGPT_SETUP_CMP2); }
-static void mfgpt_set_mode(enum clock_event_mode mode, +static int mfgpt_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { - disable_timer(cs5535_event_clock); - - if (mode == CLOCK_EVT_MODE_PERIODIC) + switch (mode) { + case CLOCK_EVT_MODE_PERIODIC: + disable_timer(cs5535_event_clock); start_timer(cs5535_event_clock, MFGPT_PERIODIC); + break; + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: + disable_timer(cs5535_event_clock); + break; + default: + return -ENOSYS; + }
cs5535_tick_mode = mode; + return 0; }
static int mfgpt_next_event(unsigned long delta, struct clock_event_device *evt) @@ -97,7 +108,7 @@ static int mfgpt_next_event(unsigned long delta, struct clock_event_device *evt) static struct clock_event_device cs5535_clockevent = { .name = DRV_NAME, .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = mfgpt_set_mode, + .set_dev_mode = mfgpt_set_mode, .set_next_event = mfgpt_next_event, .rating = 250, }; diff --git a/drivers/clocksource/dummy_timer.c b/drivers/clocksource/dummy_timer.c index ad35725..2d0cc09 100644 --- a/drivers/clocksource/dummy_timer.c +++ b/drivers/clocksource/dummy_timer.c @@ -16,13 +16,24 @@
static DEFINE_PER_CPU(struct clock_event_device, dummy_timer_evt);
-static void dummy_timer_set_mode(enum clock_event_mode mode, +static int dummy_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { + switch (mode) { + case CLOCK_EVT_MODE_PERIODIC: + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: /* * Core clockevents code will call this when exchanging timer devices. * We don't need to do anything here. */ + break; + default: + return -ENOSYS; + } + return 0; }
static void dummy_timer_setup(void) @@ -35,7 +46,7 @@ static void dummy_timer_setup(void) CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DUMMY; evt->rating = 100; - evt->set_mode = dummy_timer_set_mode; + evt->set_dev_mode = dummy_timer_set_mode; evt->cpumask = cpumask_of(cpu);
clockevents_register_device(evt); diff --git a/drivers/clocksource/dw_apb_timer.c b/drivers/clocksource/dw_apb_timer.c index f3656a6b..15ac39c 100644 --- a/drivers/clocksource/dw_apb_timer.c +++ b/drivers/clocksource/dw_apb_timer.c @@ -110,7 +110,7 @@ static void apbt_enable_int(struct dw_apb_timer *timer) apbt_writel(timer, ctrl, APBTMR_N_CONTROL); }
-static void apbt_set_mode(enum clock_event_mode mode, +static int apbt_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { unsigned long ctrl; @@ -173,7 +173,10 @@ static void apbt_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: apbt_enable_int(&dw_ced->timer); break; + default: + return -ENOSYS; } + return 0; }
static int apbt_next_event(unsigned long delta, @@ -232,7 +235,7 @@ dw_apb_clockevent_init(int cpu, const char *name, unsigned rating, dw_ced->ced.min_delta_ns = clockevent_delta2ns(5000, &dw_ced->ced); dw_ced->ced.cpumask = cpumask_of(cpu); dw_ced->ced.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT; - dw_ced->ced.set_mode = apbt_set_mode; + dw_ced->ced.set_dev_mode = apbt_set_mode; dw_ced->ced.set_next_event = apbt_next_event; dw_ced->ced.irq = dw_ced->timer.irq; dw_ced->ced.rating = rating; diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c index 9d17083..34b9a01 100644 --- a/drivers/clocksource/em_sti.c +++ b/drivers/clocksource/em_sti.c @@ -251,7 +251,7 @@ static struct em_sti_priv *ced_to_em_sti(struct clock_event_device *ced) return container_of(ced, struct em_sti_priv, ced); }
-static void em_sti_clock_event_mode(enum clock_event_mode mode, +static int em_sti_clock_event_mode(enum clock_event_mode mode, struct clock_event_device *ced) { struct em_sti_priv *p = ced_to_em_sti(ced); @@ -275,9 +275,12 @@ static void em_sti_clock_event_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_UNUSED: em_sti_stop(p, USER_CLOCKEVENT); break; - default: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static int em_sti_clock_event_next(unsigned long delta, @@ -303,11 +306,11 @@ static void em_sti_register_clockevent(struct em_sti_priv *p) ced->rating = 200; ced->cpumask = cpu_possible_mask; ced->set_next_event = em_sti_clock_event_next; - ced->set_mode = em_sti_clock_event_mode; + ced->set_dev_mode = em_sti_clock_event_mode;
dev_info(&p->pdev->dev, "used for clock events\n");
- /* Register with dummy 1 Hz value, gets updated in ->set_mode() */ + /* Register with dummy 1 Hz value, gets updated in ->set_dev_mode() */ clockevents_config_and_register(ced, 1, 2, 0xffffffff); }
diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c index acf5a32..c3c6903 100644 --- a/drivers/clocksource/exynos_mct.c +++ b/drivers/clocksource/exynos_mct.c @@ -242,14 +242,14 @@ static int exynos4_comp_set_next_event(unsigned long cycles, return 0; }
-static void exynos4_comp_set_mode(enum clock_event_mode mode, +static int exynos4_comp_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { unsigned long cycles_per_jiffy; - exynos4_mct_comp0_stop();
switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + exynos4_mct_comp0_stop(); cycles_per_jiffy = (((unsigned long long) NSEC_PER_SEC / HZ * evt->mult) >> evt->shift); exynos4_mct_comp0_start(mode, cycles_per_jiffy); @@ -259,8 +259,12 @@ static void exynos4_comp_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: + exynos4_mct_comp0_stop(); break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device mct_comp_device = { @@ -268,7 +272,7 @@ static struct clock_event_device mct_comp_device = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .rating = 250, .set_next_event = exynos4_comp_set_next_event, - .set_mode = exynos4_comp_set_mode, + .set_dev_mode = exynos4_comp_set_mode, };
static irqreturn_t exynos4_mct_comp_isr(int irq, void *dev_id) @@ -344,16 +348,15 @@ static int exynos4_tick_set_next_event(unsigned long cycles, return 0; }
-static inline void exynos4_tick_set_mode(enum clock_event_mode mode, +static inline int exynos4_tick_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick); unsigned long cycles_per_jiffy;
- exynos4_mct_tick_stop(mevt); - switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + exynos4_mct_tick_stop(mevt); cycles_per_jiffy = (((unsigned long long) NSEC_PER_SEC / HZ * evt->mult) >> evt->shift); exynos4_mct_tick_start(cycles_per_jiffy, mevt); @@ -363,8 +366,12 @@ static inline void exynos4_tick_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: + exynos4_mct_tick_stop(mevt); break; + default: + return -ENOSYS; } + return 0; }
static int exynos4_mct_tick_clear(struct mct_clock_event_device *mevt) @@ -413,7 +420,7 @@ static int exynos4_local_timer_setup(struct clock_event_device *evt) evt->name = mevt->name; evt->cpumask = cpumask_of(cpu); evt->set_next_event = exynos4_tick_set_next_event; - evt->set_mode = exynos4_tick_set_mode; + evt->set_dev_mode = exynos4_tick_set_mode; evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT; evt->rating = 450;
@@ -440,7 +447,7 @@ static int exynos4_local_timer_setup(struct clock_event_device *evt)
static void exynos4_local_timer_stop(struct clock_event_device *evt) { - evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt); + evt->set_dev_mode(CLOCK_EVT_MODE_UNUSED, evt); if (mct_int_type == MCT_INT_SPI) free_irq(evt->irq, this_cpu_ptr(&percpu_mct_tick)); else diff --git a/drivers/clocksource/i8253.c b/drivers/clocksource/i8253.c index 14ee3ef..936da9c 100644 --- a/drivers/clocksource/i8253.c +++ b/drivers/clocksource/i8253.c @@ -105,7 +105,7 @@ int __init clocksource_i8253_init(void) * * This is also called after resume to bring the PIT into operation again. */ -static void init_pit_timer(enum clock_event_mode mode, +static int init_pit_timer(enum clock_event_mode mode, struct clock_event_device *evt) { raw_spin_lock(&i8253_lock); @@ -136,8 +136,12 @@ static void init_pit_timer(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: /* Nothing to do here */ break; + default: + raw_spin_unlock(&i8253_lock); + return -ENOSYS; } raw_spin_unlock(&i8253_lock); + return 0; }
/* @@ -162,7 +166,7 @@ static int pit_next_event(unsigned long delta, struct clock_event_device *evt) struct clock_event_device i8253_clockevent = { .name = "pit", .features = CLOCK_EVT_FEAT_PERIODIC, - .set_mode = init_pit_timer, + .set_dev_mode = init_pit_timer, .set_next_event = pit_next_event, };
diff --git a/drivers/clocksource/metag_generic.c b/drivers/clocksource/metag_generic.c index 9e4db41..501fff4 100644 --- a/drivers/clocksource/metag_generic.c +++ b/drivers/clocksource/metag_generic.c @@ -56,7 +56,7 @@ static int metag_timer_set_next_event(unsigned long delta, return 0; }
-static void metag_timer_set_mode(enum clock_event_mode mode, +static int metag_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -68,11 +68,10 @@ static void metag_timer_set_mode(enum clock_event_mode mode, /* We should disable the IRQ here */ break;
- case CLOCK_EVT_MODE_PERIODIC: - case CLOCK_EVT_MODE_UNUSED: - WARN_ON(1); - break; + default: + return -ENOSYS; }; + return 0; }
static cycle_t metag_clocksource_read(struct clocksource *cs) @@ -129,7 +128,7 @@ static void arch_timer_setup(unsigned int cpu) clk->rating = 200, clk->shift = 12, clk->irq = tbisig_map(TBID_SIGNUM_TRT), - clk->set_mode = metag_timer_set_mode, + clk->set_dev_mode = metag_timer_set_mode, clk->set_next_event = metag_timer_set_next_event,
clk->mult = div_sc(hwtimer_freq, NSEC_PER_SEC, clk->shift); diff --git a/drivers/clocksource/moxart_timer.c b/drivers/clocksource/moxart_timer.c index 5eb2c35..edae259 100644 --- a/drivers/clocksource/moxart_timer.c +++ b/drivers/clocksource/moxart_timer.c @@ -58,7 +58,7 @@ static void __iomem *base; static unsigned int clock_count_per_tick;
-static void moxart_clkevt_mode(enum clock_event_mode mode, +static int moxart_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *clk) { switch (mode) { @@ -73,10 +73,12 @@ static void moxart_clkevt_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: - default: writel(TIMER1_DISABLE, base + TIMER_CR); break; + default: + return -ENOSYS; } + return 0; }
static int moxart_clkevt_next_event(unsigned long cycles, @@ -98,7 +100,7 @@ static struct clock_event_device moxart_clockevent = { .name = "moxart_timer", .rating = 200, .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = moxart_clkevt_mode, + .set_dev_mode = moxart_clkevt_mode, .set_next_event = moxart_clkevt_next_event, };
diff --git a/drivers/clocksource/mxs_timer.c b/drivers/clocksource/mxs_timer.c index 445b68a..462623f 100644 --- a/drivers/clocksource/mxs_timer.c +++ b/drivers/clocksource/mxs_timer.c @@ -150,7 +150,7 @@ static const char *clock_event_mode_label[] const = { }; #endif /* DEBUG */
-static void mxs_set_mode(enum clock_event_mode mode, +static int mxs_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { /* Disable interrupt in timer module */ @@ -175,13 +175,7 @@ static void mxs_set_mode(enum clock_event_mode mode, clock_event_mode_label[mode]); #endif /* DEBUG */
- /* Remember timer mode */ - mxs_clockevent_mode = mode; - switch (mode) { - case CLOCK_EVT_MODE_PERIODIC: - pr_err("%s: Periodic mode is not implemented\n", __func__); - break; case CLOCK_EVT_MODE_ONESHOT: timrot_irq_enable(); break; @@ -190,13 +184,20 @@ static void mxs_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: /* Left event sources disabled, no more interrupts appear */ break; + default: + timrot_irq_enable(); + return -ENOSYS; } + + /* Remember timer mode */ + mxs_clockevent_mode = mode; + return 0; }
static struct clock_event_device mxs_clockevent_device = { .name = "mxs_timrot", .features = CLOCK_EVT_FEAT_ONESHOT, - .set_mode = mxs_set_mode, + .set_dev_mode = mxs_set_mode, .set_next_event = timrotv2_set_next_event, .rating = 200, }; diff --git a/drivers/clocksource/nomadik-mtu.c b/drivers/clocksource/nomadik-mtu.c index a709cfa..8c3d355 100644 --- a/drivers/clocksource/nomadik-mtu.c +++ b/drivers/clocksource/nomadik-mtu.c @@ -119,7 +119,7 @@ static void nmdk_clkevt_reset(void) } }
-static void nmdk_clkevt_mode(enum clock_event_mode mode, +static int nmdk_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) { switch (mode) { @@ -140,7 +140,10 @@ static void nmdk_clkevt_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static void nmdk_clksrc_reset(void) @@ -167,7 +170,7 @@ static struct clock_event_device nmdk_clkevt = { .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_DYNIRQ, .rating = 200, - .set_mode = nmdk_clkevt_mode, + .set_dev_mode = nmdk_clkevt_mode, .set_next_event = nmdk_clkevt_next, .resume = nmdk_clkevt_resume, }; diff --git a/drivers/clocksource/qcom-timer.c b/drivers/clocksource/qcom-timer.c index e807acf..1f2d55a 100644 --- a/drivers/clocksource/qcom-timer.c +++ b/drivers/clocksource/qcom-timer.c @@ -75,7 +75,7 @@ static int msm_timer_set_next_event(unsigned long cycles, return 0; }
-static void msm_timer_set_mode(enum clock_event_mode mode, +static int msm_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { u32 ctrl; @@ -85,16 +85,16 @@ static void msm_timer_set_mode(enum clock_event_mode mode,
switch (mode) { case CLOCK_EVT_MODE_RESUME: - case CLOCK_EVT_MODE_PERIODIC: - break; case CLOCK_EVT_MODE_ONESHOT: /* Timer is enabled in set_next_event */ - break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: break; + default: + return -ENOSYS; } writel_relaxed(ctrl, event_base + TIMER_ENABLE); + return 0; }
static struct clock_event_device __percpu *msm_evt; @@ -126,7 +126,7 @@ static int msm_local_timer_setup(struct clock_event_device *evt) evt->name = "msm_timer"; evt->features = CLOCK_EVT_FEAT_ONESHOT; evt->rating = 200; - evt->set_mode = msm_timer_set_mode; + evt->set_dev_mode = msm_timer_set_mode; evt->set_next_event = msm_timer_set_next_event; evt->cpumask = cpumask_of(cpu);
@@ -147,7 +147,7 @@ static int msm_local_timer_setup(struct clock_event_device *evt)
static void msm_local_timer_stop(struct clock_event_device *evt) { - evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt); + evt->set_dev_mode(CLOCK_EVT_MODE_UNUSED, evt); disable_percpu_irq(evt->irq); }
diff --git a/drivers/clocksource/samsung_pwm_timer.c b/drivers/clocksource/samsung_pwm_timer.c index 5645cfc..b085bd5 100644 --- a/drivers/clocksource/samsung_pwm_timer.c +++ b/drivers/clocksource/samsung_pwm_timer.c @@ -207,25 +207,26 @@ static int samsung_set_next_event(unsigned long cycles, return 0; }
-static void samsung_set_mode(enum clock_event_mode mode, +static int samsung_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { - samsung_time_stop(pwm.event_id); - switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + samsung_time_stop(pwm.event_id); samsung_time_setup(pwm.event_id, pwm.clock_count_per_tick - 1); samsung_time_start(pwm.event_id, true); break;
case CLOCK_EVT_MODE_ONESHOT: - break; - case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: + samsung_time_stop(pwm.event_id); break; + default: + return -ENOSYS; } + return 0; }
static void samsung_clockevent_resume(struct clock_event_device *cev) @@ -244,7 +245,7 @@ static struct clock_event_device time_event_device = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .rating = 200, .set_next_event = samsung_set_next_event, - .set_mode = samsung_set_mode, + .set_dev_mode = samsung_set_mode, .resume = samsung_clockevent_resume, };
diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c index 0b1836a..ed4e5d0 100644 --- a/drivers/clocksource/sh_cmt.c +++ b/drivers/clocksource/sh_cmt.c @@ -585,7 +585,7 @@ static void sh_cmt_clock_event_start(struct sh_cmt_priv *p, int periodic) sh_cmt_set_next(p, p->max_match_value); }
-static void sh_cmt_clock_event_mode(enum clock_event_mode mode, +static int sh_cmt_clock_event_mode(enum clock_event_mode mode, struct clock_event_device *ced) { struct sh_cmt_priv *p = ced_to_sh_cmt(ced); @@ -613,9 +613,12 @@ static void sh_cmt_clock_event_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_UNUSED: sh_cmt_stop(p, FLAG_CLOCKEVENT); break; - default: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static int sh_cmt_clock_event_next(unsigned long delta, @@ -661,7 +664,7 @@ static void sh_cmt_register_clockevent(struct sh_cmt_priv *p, ced->rating = rating; ced->cpumask = cpumask_of(0); ced->set_next_event = sh_cmt_clock_event_next; - ced->set_mode = sh_cmt_clock_event_mode; + ced->set_dev_mode = sh_cmt_clock_event_mode; ced->suspend = sh_cmt_clock_event_suspend; ced->resume = sh_cmt_clock_event_resume;
diff --git a/drivers/clocksource/sh_mtu2.c b/drivers/clocksource/sh_mtu2.c index e30d76e..3d51350 100644 --- a/drivers/clocksource/sh_mtu2.c +++ b/drivers/clocksource/sh_mtu2.c @@ -184,7 +184,7 @@ static struct sh_mtu2_priv *ced_to_sh_mtu2(struct clock_event_device *ced) return container_of(ced, struct sh_mtu2_priv, ced); }
-static void sh_mtu2_clock_event_mode(enum clock_event_mode mode, +static int sh_mtu2_clock_event_mode(enum clock_event_mode mode, struct clock_event_device *ced) { struct sh_mtu2_priv *p = ced_to_sh_mtu2(ced); @@ -210,9 +210,12 @@ static void sh_mtu2_clock_event_mode(enum clock_event_mode mode, sh_mtu2_disable(p); break; case CLOCK_EVT_MODE_SHUTDOWN: - default: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static void sh_mtu2_clock_event_suspend(struct clock_event_device *ced) @@ -237,7 +240,7 @@ static void sh_mtu2_register_clockevent(struct sh_mtu2_priv *p, ced->features = CLOCK_EVT_FEAT_PERIODIC; ced->rating = rating; ced->cpumask = cpumask_of(0); - ced->set_mode = sh_mtu2_clock_event_mode; + ced->set_dev_mode = sh_mtu2_clock_event_mode; ced->suspend = sh_mtu2_clock_event_suspend; ced->resume = sh_mtu2_clock_event_resume;
diff --git a/drivers/clocksource/sh_tmu.c b/drivers/clocksource/sh_tmu.c index ecd7b60..e8f9573 100644 --- a/drivers/clocksource/sh_tmu.c +++ b/drivers/clocksource/sh_tmu.c @@ -321,7 +321,7 @@ static void sh_tmu_clock_event_start(struct sh_tmu_priv *p, int periodic) } }
-static void sh_tmu_clock_event_mode(enum clock_event_mode mode, +static int sh_tmu_clock_event_mode(enum clock_event_mode mode, struct clock_event_device *ced) { struct sh_tmu_priv *p = ced_to_sh_tmu(ced); @@ -352,9 +352,12 @@ static void sh_tmu_clock_event_mode(enum clock_event_mode mode, sh_tmu_disable(p); break; case CLOCK_EVT_MODE_SHUTDOWN: - default: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static int sh_tmu_clock_event_next(unsigned long delta, @@ -393,7 +396,7 @@ static void sh_tmu_register_clockevent(struct sh_tmu_priv *p, ced->rating = rating; ced->cpumask = cpumask_of(0); ced->set_next_event = sh_tmu_clock_event_next; - ced->set_mode = sh_tmu_clock_event_mode; + ced->set_dev_mode = sh_tmu_clock_event_mode; ced->suspend = sh_tmu_clock_event_suspend; ced->resume = sh_tmu_clock_event_resume;
diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c index efb17c3..b755fcb 100644 --- a/drivers/clocksource/sun4i_timer.c +++ b/drivers/clocksource/sun4i_timer.c @@ -81,7 +81,7 @@ static void sun4i_clkevt_time_start(u8 timer, bool periodic) timer_base + TIMER_CTL_REG(timer)); }
-static void sun4i_clkevt_mode(enum clock_event_mode mode, +static int sun4i_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *clk) { switch (mode) { @@ -96,10 +96,13 @@ static void sun4i_clkevt_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: - default: + case CLOCK_EVT_MODE_RESUME: sun4i_clkevt_time_stop(0); break; + default: + return -ENOSYS; } + return 0; }
static int sun4i_clkevt_next_event(unsigned long evt, @@ -116,7 +119,7 @@ static struct clock_event_device sun4i_clockevent = { .name = "sun4i_tick", .rating = 350, .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = sun4i_clkevt_mode, + .set_dev_mode = sun4i_clkevt_mode, .set_next_event = sun4i_clkevt_next_event, };
diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c index 00fdd11..b6b23f1 100644 --- a/drivers/clocksource/tcb_clksrc.c +++ b/drivers/clocksource/tcb_clksrc.c @@ -91,7 +91,7 @@ static struct tc_clkevt_device *to_tc_clkevt(struct clock_event_device *clkevt) */ static u32 timer_clock;
-static void tc_mode(enum clock_event_mode m, struct clock_event_device *d) +static int tc_mode(enum clock_event_mode m, struct clock_event_device *d) { struct tc_clkevt_device *tcd = to_tc_clkevt(d); void __iomem *regs = tcd->regs; @@ -137,9 +137,14 @@ static void tc_mode(enum clock_event_mode m, struct clock_event_device *d) /* set_next_event() configures and starts the timer */ break;
- default: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static int tc_next_event(unsigned long delta, struct clock_event_device *d) @@ -160,7 +165,7 @@ static struct tc_clkevt_device clkevt = { /* Should be lower than at91rm9200's system timer */ .rating = 125, .set_next_event = tc_next_event, - .set_mode = tc_mode, + .set_dev_mode = tc_mode, }, };
diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c index d1869f0..e02194f 100644 --- a/drivers/clocksource/tegra20_timer.c +++ b/drivers/clocksource/tegra20_timer.c @@ -69,25 +69,27 @@ static int tegra_timer_set_next_event(unsigned long cycles, return 0; }
-static void tegra_timer_set_mode(enum clock_event_mode mode, +static int tegra_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { u32 reg;
- timer_writel(0, TIMER3_BASE + TIMER_PTV); - switch (mode) { case CLOCK_EVT_MODE_PERIODIC: + timer_writel(0, TIMER3_BASE + TIMER_PTV); reg = 0xC0000000 | ((1000000/HZ)-1); timer_writel(reg, TIMER3_BASE + TIMER_PTV); break; case CLOCK_EVT_MODE_ONESHOT: - break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: + timer_writel(0, TIMER3_BASE + TIMER_PTV); break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device tegra_clockevent = { @@ -95,7 +97,7 @@ static struct clock_event_device tegra_clockevent = { .rating = 300, .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC, .set_next_event = tegra_timer_set_next_event, - .set_mode = tegra_timer_set_mode, + .set_dev_mode = tegra_timer_set_mode, };
static u64 notrace tegra_read_sched_clock(void) diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c index 0451e62..8aa35c1 100644 --- a/drivers/clocksource/time-armada-370-xp.c +++ b/drivers/clocksource/time-armada-370-xp.c @@ -120,12 +120,12 @@ armada_370_xp_clkevt_next_event(unsigned long delta, return 0; }
-static void +static int armada_370_xp_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) { - if (mode == CLOCK_EVT_MODE_PERIODIC) { - + switch (mode) { + case CLOCK_EVT_MODE_PERIODIC: /* * Setup timer to fire at 1/HZ intervals. */ @@ -136,7 +136,11 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode, * Enable timer. */ local_timer_ctrl_clrset(0, TIMER0_RELOAD_EN | enable_mask); - } else { + break; + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: /* * Disable timer. */ @@ -146,7 +150,11 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode, * ACK pending timer interrupt. */ writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS); + break; + default: + return -ENOSYS; } + return 0; }
static int armada_370_xp_clkevt_irq; @@ -184,7 +192,7 @@ static int armada_370_xp_timer_setup(struct clock_event_device *evt) evt->shift = 32, evt->rating = 300, evt->set_next_event = armada_370_xp_clkevt_next_event, - evt->set_mode = armada_370_xp_clkevt_mode, + evt->set_dev_mode = armada_370_xp_clkevt_mode, evt->irq = armada_370_xp_clkevt_irq; evt->cpumask = cpumask_of(cpu);
@@ -196,7 +204,7 @@ static int armada_370_xp_timer_setup(struct clock_event_device *evt)
static void armada_370_xp_timer_stop(struct clock_event_device *evt) { - evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt); + evt->set_dev_mode(CLOCK_EVT_MODE_UNUSED, evt); disable_percpu_irq(evt->irq); }
diff --git a/drivers/clocksource/time-efm32.c b/drivers/clocksource/time-efm32.c index 1a6205b..6e5af98 100644 --- a/drivers/clocksource/time-efm32.c +++ b/drivers/clocksource/time-efm32.c @@ -48,7 +48,7 @@ struct efm32_clock_event_ddata { unsigned periodic_top; };
-static void efm32_clock_event_set_mode(enum clock_event_mode mode, +static int efm32_clock_event_set_mode(enum clock_event_mode mode, struct clock_event_device *evtdev) { struct efm32_clock_event_ddata *ddata = @@ -81,7 +81,10 @@ static void efm32_clock_event_set_mode(enum clock_event_mode mode,
case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static int efm32_clock_event_set_next_event(unsigned long evt, @@ -112,7 +115,7 @@ static struct efm32_clock_event_ddata clock_event_ddata = { .evtdev = { .name = "efm32 clockevent", .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_MODE_PERIODIC, - .set_mode = efm32_clock_event_set_mode, + .set_dev_mode = efm32_clock_event_set_mode, .set_next_event = efm32_clock_event_set_next_event, .rating = 200, }, diff --git a/drivers/clocksource/time-orion.c b/drivers/clocksource/time-orion.c index 0b3ce03..3a26ce9 100644 --- a/drivers/clocksource/time-orion.c +++ b/drivers/clocksource/time-orion.c @@ -60,21 +60,30 @@ static int orion_clkevt_next_event(unsigned long delta, return 0; }
-static void orion_clkevt_mode(enum clock_event_mode mode, +static int orion_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) { - if (mode == CLOCK_EVT_MODE_PERIODIC) { + switch (mode) { + case CLOCK_EVT_MODE_PERIODIC: /* setup and enable periodic timer at 1/HZ intervals */ writel(ticks_per_jiffy - 1, timer_base + TIMER1_RELOAD); writel(ticks_per_jiffy - 1, timer_base + TIMER1_VAL); atomic_io_modify(timer_base + TIMER_CTRL, TIMER1_RELOAD_EN | TIMER1_EN, TIMER1_RELOAD_EN | TIMER1_EN); - } else { + break; + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: /* disable timer */ atomic_io_modify(timer_base + TIMER_CTRL, TIMER1_RELOAD_EN | TIMER1_EN, 0); + break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device orion_clkevt = { @@ -83,7 +92,7 @@ static struct clock_event_device orion_clkevt = { .shift = 32, .rating = 300, .set_next_event = orion_clkevt_next_event, - .set_mode = orion_clkevt_mode, + .set_dev_mode = orion_clkevt_mode, };
static irqreturn_t orion_clkevt_irq_handler(int irq, void *dev_id) diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c index 0250354..722c7e3 100644 --- a/drivers/clocksource/timer-keystone.c +++ b/drivers/clocksource/timer-keystone.c @@ -141,7 +141,7 @@ static int keystone_set_next_event(unsigned long cycles, return keystone_timer_config(cycles, evt->mode); }
-static void keystone_set_mode(enum clock_event_mode mode, +static int keystone_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -153,9 +153,12 @@ static void keystone_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_ONESHOT: keystone_timer_disable(); break; - default: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static void __init keystone_timer_init(struct device_node *np) @@ -222,7 +225,7 @@ static void __init keystone_timer_init(struct device_node *np) /* setup clockevent */ event_dev->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT; event_dev->set_next_event = keystone_set_next_event; - event_dev->set_mode = keystone_set_mode; + event_dev->set_dev_mode = keystone_set_mode; event_dev->cpumask = cpu_all_mask; event_dev->owner = THIS_MODULE; event_dev->name = TIMER_NAME; diff --git a/drivers/clocksource/timer-marco.c b/drivers/clocksource/timer-marco.c index b52e1c0..ae493cc 100644 --- a/drivers/clocksource/timer-marco.c +++ b/drivers/clocksource/timer-marco.c @@ -114,18 +114,22 @@ static int sirfsoc_timer_set_next_event(unsigned long delta, return 0; }
-static void sirfsoc_timer_set_mode(enum clock_event_mode mode, +static int sirfsoc_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *ce) { switch (mode) { case CLOCK_EVT_MODE_ONESHOT: /* enable in set_next_event */ + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: + sirfsoc_timer_count_disable(smp_processor_id()); break; default: - break; + return -ENOSYS; }
- sirfsoc_timer_count_disable(smp_processor_id()); + return 0; }
static void sirfsoc_clocksource_suspend(struct clocksource *cs) @@ -190,7 +194,7 @@ static int sirfsoc_local_timer_setup(struct clock_event_device *ce) ce->name = "local_timer"; ce->features = CLOCK_EVT_FEAT_ONESHOT; ce->rating = 200; - ce->set_mode = sirfsoc_timer_set_mode; + ce->set_dev_mode = sirfsoc_timer_set_mode; ce->set_next_event = sirfsoc_timer_set_next_event; clockevents_calc_mult_shift(ce, MARCO_CLOCK_FREQ, 60); ce->max_delta_ns = clockevent_delta2ns(-2, ce); diff --git a/drivers/clocksource/timer-prima2.c b/drivers/clocksource/timer-prima2.c index 1a6b2d6..0b49c10 100644 --- a/drivers/clocksource/timer-prima2.c +++ b/drivers/clocksource/timer-prima2.c @@ -99,14 +99,11 @@ static int sirfsoc_timer_set_next_event(unsigned long delta, return next - now > delta ? -ETIME : 0; }
-static void sirfsoc_timer_set_mode(enum clock_event_mode mode, +static int sirfsoc_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *ce) { u32 val = readl_relaxed(sirfsoc_timer_base + SIRFSOC_TIMER_INT_EN); switch (mode) { - case CLOCK_EVT_MODE_PERIODIC: - WARN_ON(1); - break; case CLOCK_EVT_MODE_ONESHOT: writel_relaxed(val | BIT(0), sirfsoc_timer_base + SIRFSOC_TIMER_INT_EN); break; @@ -116,7 +113,10 @@ static void sirfsoc_timer_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static void sirfsoc_clocksource_suspend(struct clocksource *cs) @@ -144,7 +144,7 @@ static struct clock_event_device sirfsoc_clockevent = { .name = "sirfsoc_clockevent", .rating = 200, .features = CLOCK_EVT_FEAT_ONESHOT, - .set_mode = sirfsoc_timer_set_mode, + .set_dev_mode = sirfsoc_timer_set_mode, .set_next_event = sirfsoc_timer_set_next_event, };
diff --git a/drivers/clocksource/timer-sun5i.c b/drivers/clocksource/timer-sun5i.c index deebcd6..ce690f4 100644 --- a/drivers/clocksource/timer-sun5i.c +++ b/drivers/clocksource/timer-sun5i.c @@ -79,7 +79,7 @@ static void sun5i_clkevt_time_start(u8 timer, bool periodic) timer_base + TIMER_CTL_REG(timer)); }
-static void sun5i_clkevt_mode(enum clock_event_mode mode, +static int sun5i_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *clk) { switch (mode) { @@ -94,10 +94,14 @@ static void sun5i_clkevt_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN: - default: sun5i_clkevt_time_stop(0); break; + case CLOCK_EVT_MODE_RESUME: + break; + default: + return -ENOSYS; } + return 0; }
static int sun5i_clkevt_next_event(unsigned long evt, @@ -114,7 +118,7 @@ static struct clock_event_device sun5i_clockevent = { .name = "sun5i_tick", .rating = 340, .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = sun5i_clkevt_mode, + .set_dev_mode = sun5i_clkevt_mode, .set_next_event = sun5i_clkevt_next_event, };
diff --git a/drivers/clocksource/timer-u300.c b/drivers/clocksource/timer-u300.c index 5dcf756..000a865 100644 --- a/drivers/clocksource/timer-u300.c +++ b/drivers/clocksource/timer-u300.c @@ -192,7 +192,7 @@ struct u300_clockevent_data { * have oneshot timer active, the oneshot scheduling function * u300_set_next_event() is called immediately after. */ -static void u300_set_mode(enum clock_event_mode mode, +static int u300_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { struct u300_clockevent_data *cevdata = @@ -265,7 +265,10 @@ static void u300_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: /* Ignore this call */ break; + default: + return -ENOSYS; } + return 0; }
/* @@ -315,7 +318,7 @@ static struct u300_clockevent_data u300_clockevent_data = { .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .set_next_event = u300_set_next_event, - .set_mode = u300_set_mode, + .set_dev_mode = u300_set_mode, }, };
diff --git a/drivers/clocksource/vf_pit_timer.c b/drivers/clocksource/vf_pit_timer.c index a918bc4..a34e5c8 100644 --- a/drivers/clocksource/vf_pit_timer.c +++ b/drivers/clocksource/vf_pit_timer.c @@ -86,16 +86,22 @@ static int pit_set_next_event(unsigned long delta, return 0; }
-static void pit_set_mode(enum clock_event_mode mode, +static int pit_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { case CLOCK_EVT_MODE_PERIODIC: pit_set_next_event(cycle_per_jiffy, evt); break; - default: + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static irqreturn_t pit_timer_interrupt(int irq, void *dev_id) @@ -121,7 +127,7 @@ static irqreturn_t pit_timer_interrupt(int irq, void *dev_id) static struct clock_event_device clockevent_pit = { .name = "VF pit timer", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = pit_set_mode, + .set_dev_mode = pit_set_mode, .set_next_event = pit_set_next_event, .rating = 300, }; diff --git a/drivers/clocksource/vt8500_timer.c b/drivers/clocksource/vt8500_timer.c index 1098ed3..17492ed 100644 --- a/drivers/clocksource/vt8500_timer.c +++ b/drivers/clocksource/vt8500_timer.c @@ -88,12 +88,11 @@ static int vt8500_timer_set_next_event(unsigned long cycles, return 0; }
-static void vt8500_timer_set_mode(enum clock_event_mode mode, +static int vt8500_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { case CLOCK_EVT_MODE_RESUME: - case CLOCK_EVT_MODE_PERIODIC: break; case CLOCK_EVT_MODE_ONESHOT: case CLOCK_EVT_MODE_UNUSED: @@ -102,7 +101,10 @@ static void vt8500_timer_set_mode(enum clock_event_mode mode, regbase + TIMER_CTRL_VAL); writel(0, regbase + TIMER_IER_VAL); break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device clockevent = { @@ -110,7 +112,7 @@ static struct clock_event_device clockevent = { .features = CLOCK_EVT_FEAT_ONESHOT, .rating = 200, .set_next_event = vt8500_timer_set_next_event, - .set_mode = vt8500_timer_set_mode, + .set_dev_mode = vt8500_timer_set_mode, };
static irqreturn_t vt8500_timer_interrupt(int irq, void *dev_id) diff --git a/drivers/clocksource/zevio-timer.c b/drivers/clocksource/zevio-timer.c index 7ce4421..ce7246d 100644 --- a/drivers/clocksource/zevio-timer.c +++ b/drivers/clocksource/zevio-timer.c @@ -76,7 +76,7 @@ static int zevio_timer_set_event(unsigned long delta, return 0; }
-static void zevio_timer_set_mode(enum clock_event_mode mode, +static int zevio_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) { struct zevio_timer *timer = container_of(dev, struct zevio_timer, @@ -97,11 +97,10 @@ static void zevio_timer_set_mode(enum clock_event_mode mode, /* Stop timer */ writel(CNTL_STOP_TIMER, timer->timer1 + IO_CONTROL); break; - case CLOCK_EVT_MODE_PERIODIC: default: - /* Unsupported */ - break; + return -ENOSYS; } + return 0; }
static irqreturn_t zevio_timer_interrupt(int irq, void *dev_id) @@ -162,7 +161,7 @@ static int __init zevio_timer_add(struct device_node *node) if (timer->interrupt_regs && irqnr) { timer->clkevt.name = timer->clockevent_name; timer->clkevt.set_next_event = zevio_timer_set_event; - timer->clkevt.set_mode = zevio_timer_set_mode; + timer->clkevt.set_dev_mode = zevio_timer_set_mode; timer->clkevt.rating = 200; timer->clkevt.cpumask = cpu_all_mask; timer->clkevt.features = CLOCK_EVT_FEAT_ONESHOT;
Clockevents core now supports ->set_dev_mode() (as a replacement to ->set_mode()), with capability to return error codes.
This patch migrates clockevent drivers for all architectures that had a single clockevent driver (in order to limit patch count).
Drivers now return -ENOSYS when a unsupported mode is passed to their ->set_dev_mode() callbacks and return 0 on success.
Most of the changes are automated with help of Coccinelle (http://coccinelle.lip6.fr/) and the ones left are around the switch block which are handled manually.
Some drivers had a WARN()/BUG()/pr_err() for unsupported modes. These statements are removed now as proper error handling with a WARN_ON() is done at clockevents core.
Also it is made sure that drivers don't disable events for unsupported modes.
A simplified version of the semantic patch is:
@@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c);
@@ identifier setmode; @@ -void +int setmode(enum clock_event_mode, struct clock_event_device *);
@fixret@ identifier m,c,setmode; @@ -void +int setmode(enum clock_event_mode m, struct clock_event_device *c) { ... + return 0; }
@depends on fixret@ identifier ced; identifier fixret.setmode; @@ ... struct clock_event_device ced = { ..., -.set_mode +.set_dev_mode = setmode, };
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced->set_mode + ced->set_dev_mode = setmode
@depends on fixret@ identifier fixret.setmode; @@ { . -set_mode +set_dev_mode = setmode }
@depends on fixret@ expression ced; identifier fixret.setmode; @@ - ced.set_mode + ced.set_dev_mode = setmode
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- arch/alpha/kernel/time.c | 32 +++++++++++++++++++++++++++----- arch/arc/kernel/time.c | 11 +++++++---- arch/avr32/kernel/time.c | 7 ++++--- arch/blackfin/kernel/time-ts.c | 14 ++++++++++---- arch/c6x/platforms/timer64.c | 7 +++++-- arch/hexagon/kernel/time.c | 11 ++++++++--- arch/m68k/platform/coldfire/pit.c | 7 +++++-- arch/microblaze/kernel/timer.c | 7 +++++-- arch/mn10300/kernel/cevt-mn10300.c | 14 ++++++++++++-- arch/openrisc/kernel/time.c | 11 +++++------ arch/powerpc/kernel/time.c | 16 ++++++++++++---- arch/s390/kernel/time.c | 14 ++++++++++++-- arch/score/kernel/time.c | 8 ++++---- arch/sh/kernel/localtimer.c | 15 +++++++++++++-- arch/tile/kernel/time.c | 16 +++++++++++++--- arch/um/kernel/time.c | 7 +++++-- arch/unicore32/kernel/time.c | 8 +++++--- arch/xtensa/kernel/time.c | 9 +++++---- kernel/time/tick-broadcast-hrtimer.c | 11 ++++++++--- 19 files changed, 165 insertions(+), 60 deletions(-)
diff --git a/arch/alpha/kernel/time.c b/arch/alpha/kernel/time.c index ee39cee..97b8bac 100644 --- a/arch/alpha/kernel/time.c +++ b/arch/alpha/kernel/time.c @@ -104,11 +104,22 @@ rtc_timer_interrupt(int irq, void *dev) return IRQ_HANDLED; }
-static void +static int rtc_ce_set_mode(enum clock_event_mode mode, struct clock_event_device *ce) { /* The mode member of CE is updated in generic code. Since we only support periodic events, nothing to do. */ + switch (mode) { + case CLOCK_EVT_MODE_PERIODIC: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: + break; + + default: + return -ENOSYS; + } + return 0; }
static int @@ -129,7 +140,7 @@ init_rtc_clockevent(void) .features = CLOCK_EVT_FEAT_PERIODIC, .rating = 100, .cpumask = cpumask_of(cpu), - .set_mode = rtc_ce_set_mode, + .set_dev_mode = rtc_ce_set_mode, .set_next_event = rtc_ce_set_next_event, };
@@ -161,12 +172,23 @@ static struct clocksource qemu_cs = { * The QEMU alarm as a clock_event_device primitive. */
-static void +static int qemu_ce_set_mode(enum clock_event_mode mode, struct clock_event_device *ce) { /* The mode member of CE is updated for us in generic code. Just make sure that the event is disabled. */ - qemu_set_alarm_abs(0); + switch (mode) { + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: + qemu_set_alarm_abs(0); + break; + + default: + return -ENOSYS; + } + return 0; }
static int @@ -197,7 +219,7 @@ init_qemu_clockevent(void) .features = CLOCK_EVT_FEAT_ONESHOT, .rating = 400, .cpumask = cpumask_of(cpu), - .set_mode = qemu_ce_set_mode, + .set_dev_mode = qemu_ce_set_mode, .set_next_event = qemu_ce_set_next_event, };
diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c index 71c4252..5a9d348 100644 --- a/arch/arc/kernel/time.c +++ b/arch/arc/kernel/time.c @@ -163,7 +163,7 @@ static int arc_clkevent_set_next_event(unsigned long delta, return 0; }
-static void arc_clkevent_set_mode(enum clock_event_mode mode, +static int arc_clkevent_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) { switch (mode) { @@ -171,12 +171,15 @@ static void arc_clkevent_set_mode(enum clock_event_mode mode, arc_timer_event_setup(arc_get_core_freq() / HZ); break; case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: break; default: - break; + return -ENOSYS; }
- return; + return 0; }
static DEFINE_PER_CPU(struct clock_event_device, arc_clockevent_device) = { @@ -186,7 +189,7 @@ static DEFINE_PER_CPU(struct clock_event_device, arc_clockevent_device) = { .rating = 300, .irq = TIMER0_IRQ, /* hardwired, no need for resources */ .set_next_event = arc_clkevent_set_next_event, - .set_mode = arc_clkevent_set_mode, + .set_dev_mode = arc_clkevent_set_mode, };
static irqreturn_t timer_irq_handler(int irq, void *dev_id) diff --git a/arch/avr32/kernel/time.c b/arch/avr32/kernel/time.c index d0f771b..3f82011 100644 --- a/arch/avr32/kernel/time.c +++ b/arch/avr32/kernel/time.c @@ -80,7 +80,7 @@ static int comparator_next_event(unsigned long delta, return 0; }
-static void comparator_mode(enum clock_event_mode mode, +static int comparator_mode(enum clock_event_mode mode, struct clock_event_device *evdev) { switch (mode) { @@ -108,8 +108,9 @@ static void comparator_mode(enum clock_event_mode mode, } break; default: - BUG(); + return -ENOSYS; } + return 0; }
static struct clock_event_device comparator = { @@ -118,7 +119,7 @@ static struct clock_event_device comparator = { .shift = 16, .rating = 50, .set_next_event = comparator_next_event, - .set_mode = comparator_mode, + .set_dev_mode = comparator_mode, };
void read_persistent_clock(struct timespec *ts) diff --git a/arch/blackfin/kernel/time-ts.c b/arch/blackfin/kernel/time-ts.c index cb0a484..b1592a1 100644 --- a/arch/blackfin/kernel/time-ts.c +++ b/arch/blackfin/kernel/time-ts.c @@ -136,7 +136,7 @@ static int bfin_gptmr0_set_next_event(unsigned long cycles, return 0; }
-static void bfin_gptmr0_set_mode(enum clock_event_mode mode, +static int bfin_gptmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -173,7 +173,10 @@ static void bfin_gptmr0_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static void bfin_gptmr0_ack(void) @@ -217,7 +220,7 @@ static struct clock_event_device clockevent_gptmr0 = { .shift = 32, .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, .set_next_event = bfin_gptmr0_set_next_event, - .set_mode = bfin_gptmr0_set_mode, + .set_dev_mode = bfin_gptmr0_set_mode, };
static void __init bfin_gptmr0_clockevent_init(struct clock_event_device *evt) @@ -250,7 +253,7 @@ static int bfin_coretmr_set_next_event(unsigned long cycles, return 0; }
-static void bfin_coretmr_set_mode(enum clock_event_mode mode, +static int bfin_coretmr_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -279,7 +282,10 @@ static void bfin_coretmr_set_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
void bfin_coretmr_init(void) @@ -335,7 +341,7 @@ void bfin_coretmr_clockevent_init(void) evt->shift = 32; evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT; evt->set_next_event = bfin_coretmr_set_next_event; - evt->set_mode = bfin_coretmr_set_mode; + evt->set_dev_mode = bfin_coretmr_set_mode;
clock_tick = get_cclk() / TIME_SCALE; evt->mult = div_sc(clock_tick, NSEC_PER_SEC, evt->shift); diff --git a/arch/c6x/platforms/timer64.c b/arch/c6x/platforms/timer64.c index 3c73d74..3eef8fb 100644 --- a/arch/c6x/platforms/timer64.c +++ b/arch/c6x/platforms/timer64.c @@ -126,7 +126,7 @@ static int next_event(unsigned long delta, return 0; }
-static void set_clock_mode(enum clock_event_mode mode, +static int set_clock_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -146,14 +146,17 @@ static void set_clock_mode(enum clock_event_mode mode, break; case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device t64_clockevent_device = { .name = "TIMER64_EVT32_TIMER", .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC, .rating = 200, - .set_mode = set_clock_mode, + .set_dev_mode = set_clock_mode, .set_next_event = next_event, };
diff --git a/arch/hexagon/kernel/time.c b/arch/hexagon/kernel/time.c index 17fbf45..540b138 100644 --- a/arch/hexagon/kernel/time.c +++ b/arch/hexagon/kernel/time.c @@ -100,15 +100,20 @@ static int set_next_event(unsigned long delta, struct clock_event_device *evt) /* * Sets the mode (periodic, shutdown, oneshot, etc) of a timer. */ -static void set_mode(enum clock_event_mode mode, +static int set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { case CLOCK_EVT_MODE_SHUTDOWN: /* XXX implement me */ - default: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
#ifdef CONFIG_SMP @@ -125,7 +130,7 @@ static struct clock_event_device hexagon_clockevent_dev = { .rating = 400, .irq = RTOS_TIMER_INT, .set_next_event = set_next_event, - .set_mode = set_mode, + .set_dev_mode = set_mode, #ifdef CONFIG_SMP .broadcast = broadcast, #endif diff --git a/arch/m68k/platform/coldfire/pit.c b/arch/m68k/platform/coldfire/pit.c index 493b311..993561a 100644 --- a/arch/m68k/platform/coldfire/pit.c +++ b/arch/m68k/platform/coldfire/pit.c @@ -42,7 +42,7 @@ static u32 pit_cnt; * This is also called after resume to bring the PIT into operation again. */
-static void init_cf_pit_timer(enum clock_event_mode mode, +static int init_cf_pit_timer(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -72,7 +72,10 @@ static void init_cf_pit_timer(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: /* Nothing to do here */ break; + default: + return -ENOSYS; } + return 0; }
/* @@ -90,7 +93,7 @@ static int cf_pit_next_event(unsigned long delta, struct clock_event_device cf_pit_clockevent = { .name = "pit", .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = init_cf_pit_timer, + .set_dev_mode = init_cf_pit_timer, .set_next_event = cf_pit_next_event, .shift = 32, .irq = MCF_IRQ_PIT1, diff --git a/arch/microblaze/kernel/timer.c b/arch/microblaze/kernel/timer.c index dd96f0e..21c17db 100644 --- a/arch/microblaze/kernel/timer.c +++ b/arch/microblaze/kernel/timer.c @@ -121,7 +121,7 @@ static int xilinx_timer_set_next_event(unsigned long delta, return 0; }
-static void xilinx_timer_set_mode(enum clock_event_mode mode, +static int xilinx_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -142,7 +142,10 @@ static void xilinx_timer_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: pr_info("%s: resume\n", __func__); break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device clockevent_xilinx_timer = { @@ -151,7 +154,7 @@ static struct clock_event_device clockevent_xilinx_timer = { .shift = 8, .rating = 300, .set_next_event = xilinx_timer_set_next_event, - .set_mode = xilinx_timer_set_mode, + .set_dev_mode = xilinx_timer_set_mode, };
static inline void timer_ack(void) diff --git a/arch/mn10300/kernel/cevt-mn10300.c b/arch/mn10300/kernel/cevt-mn10300.c index 60f64ca..668906a 100644 --- a/arch/mn10300/kernel/cevt-mn10300.c +++ b/arch/mn10300/kernel/cevt-mn10300.c @@ -41,10 +41,20 @@ static int next_event(unsigned long delta, return 0; }
-static void set_clock_mode(enum clock_event_mode mode, +static int set_clock_mode(enum clock_event_mode mode, struct clock_event_device *evt) { + switch (mode) { + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: /* Nothing to do ... */ + break; + default: + return -ENOSYS; + } + return 0; }
static DEFINE_PER_CPU(struct clock_event_device, mn10300_clockevent_device); @@ -108,7 +118,7 @@ int __init init_clockevents(void)
cd->rating = 200; cd->cpumask = cpumask_of(smp_processor_id()); - cd->set_mode = set_clock_mode; + cd->set_dev_mode = set_clock_mode; cd->event_handler = event_handler; cd->set_next_event = next_event;
diff --git a/arch/openrisc/kernel/time.c b/arch/openrisc/kernel/time.c index 7c52e94..6844084 100644 --- a/arch/openrisc/kernel/time.c +++ b/arch/openrisc/kernel/time.c @@ -48,14 +48,10 @@ static int openrisc_timer_set_next_event(unsigned long delta, return 0; }
-static void openrisc_timer_set_mode(enum clock_event_mode mode, +static int openrisc_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { - case CLOCK_EVT_MODE_PERIODIC: - pr_debug(KERN_INFO "%s: periodic\n", __func__); - BUG(); - break; case CLOCK_EVT_MODE_ONESHOT: pr_debug(KERN_INFO "%s: oneshot\n", __func__); break; @@ -68,7 +64,10 @@ static void openrisc_timer_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_RESUME: pr_debug(KERN_INFO "%s: resume\n", __func__); break; + default: + return -ENOSYS; } + return 0; }
/* This is the clock event device based on the OR1K tick timer. @@ -82,7 +81,7 @@ static struct clock_event_device clockevent_openrisc_timer = { .features = CLOCK_EVT_FEAT_ONESHOT, .rating = 300, .set_next_event = openrisc_timer_set_next_event, - .set_mode = openrisc_timer_set_mode, + .set_dev_mode = openrisc_timer_set_mode, };
static inline void timer_ack(void) diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index 122a580..f552041 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -98,7 +98,7 @@ static struct clocksource clocksource_timebase = {
static int decrementer_set_next_event(unsigned long evt, struct clock_event_device *dev); -static void decrementer_set_mode(enum clock_event_mode mode, +static int decrementer_set_mode(enum clock_event_mode mode, struct clock_event_device *dev);
struct clock_event_device decrementer_clockevent = { @@ -106,7 +106,7 @@ struct clock_event_device decrementer_clockevent = { .rating = 200, .irq = 0, .set_next_event = decrementer_set_next_event, - .set_mode = decrementer_set_mode, + .set_dev_mode = decrementer_set_mode, .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP, }; EXPORT_SYMBOL(decrementer_clockevent); @@ -826,11 +826,19 @@ static int decrementer_set_next_event(unsigned long evt, return 0; }
-static void decrementer_set_mode(enum clock_event_mode mode, +static int decrementer_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) { - if (mode != CLOCK_EVT_MODE_ONESHOT) + switch (mode) { + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: decrementer_set_next_event(DECREMENTER_MAX, dev); + break; + default: + return -ENOSYS; + } + return 0; }
/* Interrupt handler for the timer broadcast IPI */ diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c index 386d37a..49e3812 100644 --- a/arch/s390/kernel/time.c +++ b/arch/s390/kernel/time.c @@ -116,9 +116,19 @@ static int s390_next_event(unsigned long delta, return 0; }
-static void s390_set_mode(enum clock_event_mode mode, +static int s390_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { + switch (mode) { + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: + break; + default: + return -ENOSYS; + } + return 0; }
/* @@ -144,7 +154,7 @@ void init_cpu_timer(void) cd->rating = 400; cd->cpumask = cpumask_of(cpu); cd->set_next_event = s390_next_event; - cd->set_mode = s390_set_mode; + cd->set_dev_mode = s390_set_mode;
clockevents_register_device(cd);
diff --git a/arch/score/kernel/time.c b/arch/score/kernel/time.c index f0a43aff..886bfdd 100644 --- a/arch/score/kernel/time.c +++ b/arch/score/kernel/time.c @@ -55,7 +55,7 @@ static int score_timer_set_next_event(unsigned long delta, return 0; }
-static void score_timer_set_mode(enum clock_event_mode mode, +static int score_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evdev) { switch (mode) { @@ -64,14 +64,14 @@ static void score_timer_set_mode(enum clock_event_mode mode, outl(SYSTEM_CLOCK/HZ, P_TIMER0_PRELOAD); outl(inl(P_TIMER0_CTRL) | TMR_ENABLE, P_TIMER0_CTRL); break; - case CLOCK_EVT_MODE_ONESHOT: case CLOCK_EVT_MODE_SHUTDOWN: case CLOCK_EVT_MODE_RESUME: case CLOCK_EVT_MODE_UNUSED: break; default: - BUG(); + return -ENOSYS; } + return 0; }
static struct clock_event_device score_clockevent = { @@ -79,7 +79,7 @@ static struct clock_event_device score_clockevent = { .features = CLOCK_EVT_FEAT_PERIODIC, .shift = 16, .set_next_event = score_timer_set_next_event, - .set_mode = score_timer_set_mode, + .set_dev_mode = score_timer_set_mode, };
void __init time_init(void) diff --git a/arch/sh/kernel/localtimer.c b/arch/sh/kernel/localtimer.c index 8bfc6df..7d48435 100644 --- a/arch/sh/kernel/localtimer.c +++ b/arch/sh/kernel/localtimer.c @@ -39,9 +39,20 @@ void local_timer_interrupt(void) irq_exit(); }
-static void dummy_timer_set_mode(enum clock_event_mode mode, +static int dummy_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *clk) { + switch (mode) { + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_PERIODIC: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: + break; + default: + return -ENOSYS; + } + return 0; }
void local_timer_setup(unsigned int cpu) @@ -54,7 +65,7 @@ void local_timer_setup(unsigned int cpu) CLOCK_EVT_FEAT_DUMMY; clk->rating = 400; clk->mult = 1; - clk->set_mode = dummy_timer_set_mode; + clk->set_dev_mode = dummy_timer_set_mode; clk->broadcast = smp_timer_broadcast; clk->cpumask = cpumask_of(cpu);
diff --git a/arch/tile/kernel/time.c b/arch/tile/kernel/time.c index 462dcd0..40d532b 100644 --- a/arch/tile/kernel/time.c +++ b/arch/tile/kernel/time.c @@ -140,10 +140,20 @@ static int tile_timer_set_next_event(unsigned long ticks, * Whenever anyone tries to change modes, we just mask interrupts * and wait for the next event to get set. */ -static void tile_timer_set_mode(enum clock_event_mode mode, +static int tile_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { - arch_local_irq_mask_now(INT_TILE_TIMER); + switch (mode) { + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_SHUTDOWN: + case CLOCK_EVT_MODE_RESUME: + arch_local_irq_mask_now(INT_TILE_TIMER); + break; + default: + return -ENOSYS; + } + return 0; }
/* @@ -157,7 +167,7 @@ static DEFINE_PER_CPU(struct clock_event_device, tile_timer) = { .rating = 100, .irq = -1, .set_next_event = tile_timer_set_next_event, - .set_mode = tile_timer_set_mode, + .set_dev_mode = tile_timer_set_mode, };
void setup_tile_timer(void) diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c index 117568d..fdcd4eb 100644 --- a/arch/um/kernel/time.c +++ b/arch/um/kernel/time.c @@ -22,7 +22,7 @@ void timer_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs) local_irq_restore(flags); }
-static void itimer_set_mode(enum clock_event_mode mode, +static int itimer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { @@ -38,7 +38,10 @@ static void itimer_set_mode(enum clock_event_mode mode,
case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
static int itimer_next_event(unsigned long delta, @@ -52,7 +55,7 @@ static struct clock_event_device itimer_clockevent = { .rating = 250, .cpumask = cpu_all_mask, .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, - .set_mode = itimer_set_mode, + .set_dev_mode = itimer_set_mode, .set_next_event = itimer_next_event, .shift = 32, .irq = 0, diff --git a/arch/unicore32/kernel/time.c b/arch/unicore32/kernel/time.c index d3824b2..c19f5a5 100644 --- a/arch/unicore32/kernel/time.c +++ b/arch/unicore32/kernel/time.c @@ -46,7 +46,7 @@ puv3_osmr0_set_next_event(unsigned long delta, struct clock_event_device *c) return (signed)(next - oscr) <= MIN_OSCR_DELTA ? -ETIME : 0; }
-static void +static int puv3_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *c) { switch (mode) { @@ -58,9 +58,11 @@ puv3_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *c) break;
case CLOCK_EVT_MODE_RESUME: - case CLOCK_EVT_MODE_PERIODIC: break; + default: + return -ENOSYS; } + return 0; }
static struct clock_event_device ckevt_puv3_osmr0 = { @@ -68,7 +70,7 @@ static struct clock_event_device ckevt_puv3_osmr0 = { .features = CLOCK_EVT_FEAT_ONESHOT, .rating = 200, .set_next_event = puv3_osmr0_set_next_event, - .set_mode = puv3_osmr0_set_mode, + .set_dev_mode = puv3_osmr0_set_mode, };
static cycle_t puv3_read_oscr(struct clocksource *cs) diff --git a/arch/xtensa/kernel/time.c b/arch/xtensa/kernel/time.c index 2a1823d..529f060 100644 --- a/arch/xtensa/kernel/time.c +++ b/arch/xtensa/kernel/time.c @@ -52,7 +52,7 @@ static struct clocksource ccount_clocksource = {
static int ccount_timer_set_next_event(unsigned long delta, struct clock_event_device *dev); -static void ccount_timer_set_mode(enum clock_event_mode mode, +static int ccount_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt); struct ccount_timer { struct clock_event_device evt; @@ -77,7 +77,7 @@ static int ccount_timer_set_next_event(unsigned long delta, return ret; }
-static void ccount_timer_set_mode(enum clock_event_mode mode, +static int ccount_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { struct ccount_timer *timer = @@ -104,8 +104,9 @@ static void ccount_timer_set_mode(enum clock_event_mode mode, timer->irq_enabled = 1; } default: - break; + return -ENOSYS; } + return 0; }
static irqreturn_t timer_interrupt(int irq, void *dev_id); @@ -126,7 +127,7 @@ void local_timer_setup(unsigned cpu) clockevent->features = CLOCK_EVT_FEAT_ONESHOT; clockevent->rating = 300; clockevent->set_next_event = ccount_timer_set_next_event; - clockevent->set_mode = ccount_timer_set_mode; + clockevent->set_dev_mode = ccount_timer_set_mode; clockevent->cpumask = cpumask_of(cpu); clockevent->irq = irq_create_mapping(NULL, LINUX_TIMER_INT); if (WARN(!clockevent->irq, "error: can't map timer irq")) diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c index eb682d5..de2e5e2 100644 --- a/kernel/time/tick-broadcast-hrtimer.c +++ b/kernel/time/tick-broadcast-hrtimer.c @@ -18,7 +18,7 @@
static struct hrtimer bctimer;
-static void bc_set_mode(enum clock_event_mode mode, +static int bc_set_mode(enum clock_event_mode mode, struct clock_event_device *bc) { switch (mode) { @@ -38,9 +38,14 @@ static void bc_set_mode(enum clock_event_mode mode, */ hrtimer_try_to_cancel(&bctimer); break; - default: + case CLOCK_EVT_MODE_ONESHOT: + case CLOCK_EVT_MODE_UNUSED: + case CLOCK_EVT_MODE_RESUME: break; + default: + return -ENOSYS; } + return 0; }
/* @@ -72,7 +77,7 @@ static int bc_set_next(ktime_t expires, struct clock_event_device *bc) }
static struct clock_event_device ce_broadcast_hrtimer = { - .set_mode = bc_set_mode, + .set_dev_mode = bc_set_mode, .set_next_ktime = bc_set_next, .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_KTIME |
All clockevent drivers are migrated to use the new callback ->set_dev_mode(). Get rid of the deprecated ->dev_mode() callback now.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- include/linux/clockchips.h | 3 --- kernel/time/clockevents.c | 18 +++++------------- kernel/time/timer_list.c | 5 +---- 3 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 8ab1a86..08b203e 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -82,7 +82,6 @@ enum clock_event_mode { * @features: features * @retries: number of forced programming retries * @set_dev_mode: set dev mode function - * @set_mode: set mode function (deprecated, use set_dev_mode instead) * @broadcast: function to broadcast events * @min_delta_ticks: minimum delta value in ticks stored for reconfiguration * @max_delta_ticks: maximum delta value in ticks stored for reconfiguration @@ -112,8 +111,6 @@ struct clock_event_device { void (*broadcast)(const struct cpumask *mask); int (*set_dev_mode)(enum clock_event_mode mode, struct clock_event_device *); - void (*set_mode)(enum clock_event_mode mode, - struct clock_event_device *); void (*suspend)(struct clock_event_device *); void (*resume)(struct clock_event_device *); unsigned long min_delta_ticks; diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c index 3cf7455..a0b6dd9 100644 --- a/kernel/time/clockevents.c +++ b/kernel/time/clockevents.c @@ -105,13 +105,9 @@ void clockevents_set_mode(struct clock_event_device *dev, enum clock_event_mode mode) { if (dev->mode != mode) { - if (dev->set_dev_mode) { - /* WARN: Currently available modes shouldn't fail */ - if (WARN_ON(dev->set_dev_mode(mode, dev))) - return; - } else { - dev->set_mode(mode, dev); - } + /* WARN: Currently available modes shouldn't fail */ + if (WARN_ON(dev->set_dev_mode(mode, dev))) + return;
dev->mode = mode;
@@ -453,12 +449,8 @@ int __clockevents_update_freq(struct clock_event_device *dev, u32 freq) if (dev->mode == CLOCK_EVT_MODE_ONESHOT) return clockevents_program_event(dev, dev->next_event, false);
- if (dev->mode == CLOCK_EVT_MODE_PERIODIC) { - if (dev->set_dev_mode) - WARN_ON(dev->set_dev_mode(CLOCK_EVT_MODE_PERIODIC, dev)); - else - dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev); - } + if (dev->mode == CLOCK_EVT_MODE_PERIODIC) + WARN_ON(dev->set_dev_mode(CLOCK_EVT_MODE_PERIODIC, dev));
return 0; } diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c index 957a049..8e43c9f 100644 --- a/kernel/time/timer_list.c +++ b/kernel/time/timer_list.c @@ -229,10 +229,7 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu) SEQ_printf(m, "\n");
SEQ_printf(m, " set_mode: "); - if (dev->set_dev_mode) - print_name_offset(m, dev->set_dev_mode); - else - print_name_offset(m, dev->set_mode); + print_name_offset(m, dev->set_dev_mode); SEQ_printf(m, "\n");
SEQ_printf(m, " event_handler: ");
Viresh Kumar viresh.kumar@linaro.org writes:
Hi Guys,
Here is another try to migrate clockevents to use set_dev_mode(). I have tried to fix all issues raised by Kevin/Preeti/Daniel. Also some updates are done to drivers to make sure they don't disable clockevents for unsupported modes, i.e. 'default' case.
Please do another round of review, it would be very helpful in getting these through tglx.
Overall a much better changelog, but some more comments below...
@Kevin: I have updated my branch again, please see if you do another round of automated tests for it.
Cover-letter for mainline:
A clockevent device can be stopped, or its events can be masked, if next event is expected at KTIME_MAX, i.e. no events are required for very long time.
The wording here is the wrong way around IMO. The way it's worded, it sounds like it's already the case. You're trying to make the case that this *should* happen, so make sure it's worded.
This normally happens with NO_HZ_FULL (when tick-sched timer is removed and we don't have any more timers scheduled for long), not sure if it can happen otherwise.
This isn't specific to NO_HZ_FULL, but will happen in regular tickless idle (NO_HZ_IDLE) also.
If we don't STOP clockevent device, we are guaranteed to receive atleast one fake interrupt, as hrtimer_force_reprogram() isn't reprogramming the event device.
I don't think "fake" is the right word. There's nothing fake about them. You might say unneeded or spurious instead.
Depending on particular implementation of clockevent device, this can be a fake interrupt at tick-rate.. (This was observed atleast on one implementation: arm_arch_timer.c).
I don't follow this part (or see the relevance to the series.) Was the continued spurious interrupt because that clockevent didn't implement one-shot mode, so was left in periodic mode?
A simple solution to get this fixed could be: update hrtimer_force_reprogram()
s/simple/simple (yet hacky)/ :)
to always reprogram clockevent device and update clockevent drivers to STOP generating events (or delay it to max time), when 'expires' is set to KTIME_MAX. But the drawback here is that every clockevent driver has to be hacked for this particular case and its very easy for new ones to miss this.
Thomas suggested to add an optional mode: CLOCK_EVT_MODE_ONESHOT_STOPPED
However, Thomas suggested...
(lkml.org/lkml/2014/5/9/508) to get tihs fixed.
s/get this fixed/solve this problem/
This requires ->set_mode() callback to have capability to return 'success' or 'failure' for this new mode.
Why? (yes, I know the answer, but it should be described here.)
Currently its return type is 'void'.
To change return type of set_mode() Thomas suggested to implement another callback: ->set_dev_mode(), with return type 'int'. We can then convert clockevent drivers to use this interface instead of existing ->set_mode() and then finally remove ->set_mode()'s support.
This patchset first adds another callback with return capability, set_dev_mode(), then it migrates all drivers one by one and finally removes earlier callback set_mode() when nobody is using it.
This is looking much better,
Kevin
On 23 May 2014 03:07, Kevin Hilman khilman@linaro.org wrote:
Overall a much better changelog, but some more comments below...
Thanks for spending more time on these :)
A clockevent device can be stopped, or its events can be masked, if next event is expected at KTIME_MAX, i.e. no events are required for very long time.
The wording here is the wrong way around IMO. The way it's worded, it sounds like it's already the case. You're trying to make the case that this *should* happen, so make sure it's worded.
Yeah, I thought of s/can/should in my first line while writing this but still did the mistake. Yes, what you are saying makes a lot of sense :)
This normally happens with NO_HZ_FULL (when tick-sched timer is removed and we don't have any more timers scheduled for long), not sure if it can happen otherwise.
This isn't specific to NO_HZ_FULL, but will happen in regular tickless idle (NO_HZ_IDLE) also.
Copying below from my reply to Frederic, he said the same thing :)
How exactly? I would have seen these fake interrupts in normal boot if that's the case as we go in and out of idle again and again.. The reason I can think of why it isn't happening for NO_HZ is: clockevent devices are normally part of CPU block (Atleast on ARM), and when CPU goes into some power down state, the even device goes as well..
Though there is one way it can happen for NO_HZ_IDLE, probably Preeti or Daniel told me this earlier and I forgot to mention it. i.e. the case of external event device.
-- This is new now :)
I still wasn't sure about the above case, though it sounds logical. And probably you guys can tell me if that's the case for some of the platforms atleast. Because as I said, it isn't happening for me with Exynos (dual A15) as timers are part of CPU block..
Anyway, kernel shouldn't make any assumptions here and should always try to stop clockevent when not in use..
If we don't STOP clockevent device, we are guaranteed to receive atleast one fake interrupt, as hrtimer_force_reprogram() isn't reprogramming the event device.
I don't think "fake" is the right word. There's nothing fake about them. You might say unneeded or spurious instead.
Spurious is the word, yes ..
Depending on particular implementation of clockevent device, this can be a fake interrupt at tick-rate.. (This was observed atleast on one implementation: arm_arch_timer.c).
I don't follow this part (or see the relevance to the series.) Was the continued spurious interrupt because that clockevent didn't implement one-shot mode, so was left in periodic mode?
Hmm, I should have given more details :(
drivers/clocksource/arm_arch_timer.c (driver for Exynos) is probably emulating ONESHOT mode from Periodic. And so we get continuous 'spurious' interrupts every 5 ms :)
A simple solution to get this fixed could be: update hrtimer_force_reprogram()
s/simple/simple (yet hacky)/ :)
/me nods..
to always reprogram clockevent device and update clockevent drivers to STOP generating events (or delay it to max time), when 'expires' is set to KTIME_MAX. But the drawback here is that every clockevent driver has to be hacked for this particular case and its very easy for new ones to miss this.
Thomas suggested to add an optional mode: CLOCK_EVT_MODE_ONESHOT_STOPPED
However, Thomas suggested...
Grrrr .. I realize that my English is really poor :)
(lkml.org/lkml/2014/5/9/508) to get tihs fixed.
s/get this fixed/solve this problem/
hmm.. thanks for looking at so minute levels ..
This requires ->set_mode() callback to have capability to return 'success' or 'failure' for this new mode.
Why? (yes, I know the answer, but it should be described here.)
Specifically for this I mentioned *optional* in above line and though these two together would make sense. Anyway, they didn't.. Will mention that..
Currently its return type is 'void'.
To change return type of set_mode() Thomas suggested to implement another callback: ->set_dev_mode(), with return type 'int'. We can then convert clockevent drivers to use this interface instead of existing ->set_mode() and then finally remove ->set_mode()'s support.
This patchset first adds another callback with return capability, set_dev_mode(), then it migrates all drivers one by one and finally removes earlier callback set_mode() when nobody is using it.
This is looking much better,
Finally :)
On Fri, May 23, 2014 at 10:15:59AM +0530, Viresh Kumar wrote:
On 23 May 2014 03:07, Kevin Hilman khilman@linaro.org wrote:
This isn't specific to NO_HZ_FULL, but will happen in regular tickless idle (NO_HZ_IDLE) also.
Copying below from my reply to Frederic, he said the same thing :)
How exactly? I would have seen these fake interrupts in normal boot if that's the case as we go in and out of idle again and again.. The reason I can think of why it isn't happening for NO_HZ is: clockevent devices are normally part of CPU block (Atleast on ARM), and when CPU goes into some power down state, the even device goes as well..
Though there is one way it can happen for NO_HZ_IDLE, probably Preeti or Daniel told me this earlier and I forgot to mention it. i.e. the case of external event device.
-- This is new now :)
I still wasn't sure about the above case, though it sounds logical. And probably you guys can tell me if that's the case for some of the platforms atleast. Because as I said, it isn't happening for me with Exynos (dual A15) as timers are part of CPU block..
Anyway, kernel shouldn't make any assumptions here and should always try to stop clockevent when not in use..
Yeah. I mean I have no idea how x86 behaves with this. Whether all possible clock source: HPET, APIC, etc.. are always disabled on low power mode in idle. Also is it possible that the spurious interrupt fires before we enter low power mode? Does it depend on the cpufreq or governors?
And there are many other archs to consider. So yeah I think we just shouldn't make any assumption here.
linaro-kernel@lists.linaro.org
-
Frederic Weisbecker -
Kevin Hilman -
Viresh Kumar