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()/empty-implementation for unsupported modes. These statements and unsupported modes are removed now as proper error handling with a WARN_ON() is done at clockevents core.
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 | 9 ++++++--- arch/arm/kernel/smp_twd.c | 10 +++++++--- arch/arm/mach-at91/at91rm9200_time.c | 7 +++++-- arch/arm/mach-at91/at91sam926x_time.c | 8 ++++++-- arch/arm/mach-clps711x/common.c | 7 +++++-- 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 | 7 +++++-- arch/arm/mach-imx/time.c | 7 +++++-- arch/arm/mach-integrator/integrator_ap.c | 9 +++++---- arch/arm/mach-ixp4xx/common.c | 8 +++++--- arch/arm/mach-ks8695/time.c | 22 ++++++++++++++++------ arch/arm/mach-lpc32xx/timer.c | 7 +++++-- arch/arm/mach-mmp/time.c | 11 +++++++---- arch/arm/mach-netx/time.c | 11 +++++------ arch/arm/mach-omap1/time.c | 7 +++++-- arch/arm/mach-omap1/timer32k.c | 7 +++++-- arch/arm/mach-omap2/timer.c | 7 +++++-- arch/arm/mach-pxa/time.c | 7 +++++-- arch/arm/mach-sa1100/time.c | 7 +++++-- arch/arm/mach-spear/time.c | 10 +++++----- arch/arm/mach-w90x900/time.c | 7 +++++-- arch/arm/plat-iop/time.c | 8 +++++--- arch/arm/plat-orion/time.c | 20 +++++++++++++++----- 26 files changed, 158 insertions(+), 76 deletions(-)
diff --git a/arch/arm/common/timer-sp.c b/arch/arm/common/timer-sp.c index fd6bff0..9ed3845 100644 --- a/arch/arm/common/timer-sp.c +++ b/arch/arm/common/timer-sp.c @@ -127,7 +127,7 @@ 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; @@ -147,11 +147,14 @@ static void sp804_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_UNUSED: case CLOCK_EVT_MODE_SHUTDOWN:
- default:
- case CLOCK_EVT_MODE_RESUME: break;
- default:
}return -ENOSYS;
You missed some previous comments.
Note that the early 'return' changes the *functional* behavior of this code. e.g. it used to do 2 writes to this register, but now it does one. Assuming this is well-behaved hardware, it's probably fine but we just don't know.
As I mentioned in an earlier review, to preserve the *behavior* of the current code used to disable timers, you should not mess with any of the register accesses.
Instead, add an 'int ret = 0' at the beginning, 'ret = -ENOSYS' in the default case and 'return ret' at the end.
This not only makes the code easier to review for those who are not intimately familiar with each of these clockevent devices, but it's much more obvious on inspection that you are not changing the *behavior* of the code.
The same problem exists on several of the following changes, so I stopped reviewing after I saw 3-4 of them.
Kevin