On Wed, May 28, 2014 at 10:05:54PM +0530, Viresh Kumar wrote:
On 28 May 2014 19:55, Frederic Weisbecker fweisbec@gmail.com wrote:
On Fri, May 23, 2014 at 08:55:04PM +0530, Viresh Kumar wrote:
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;So your coccinelle script makes sure that all current modes are supported in the case enumeration, right?
Coccinelle script doesn't as it doesn't have very good support for switch blocks. I had a chat with Julia Lawall about it.
This was made sure manually though that all supported modes are present in the switch block and we should never hit the default case with these patches.
Ok looks all good then!
default:return -ENOSYS; } writel(ctrl, clkevt_base + TIMER_CTRL);Otherwise we would have a functional change here as unsupported mode won't do the writel() anymore.
Correct.
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;And here you replace the implementation's BUG() with the generic WARN_ON(), right?
Correct and this is mentioned in changelog..
Yeah I'm growing with bad habits :)
Thanks.