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.
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..
diff --git a/arch/arm/mach-clps711x/common.c b/arch/arm/mach-clps711x/common.c index aee81fa..69ada1e 100644 --- a/arch/arm/mach-clps711x/common.c +++ b/arch/arm/mach-clps711x/common.c @@ -69,7 +69,7 @@ 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); @@ -78,21 +78,22 @@ static void clps711x_clockevent_set_mode(enum clock_event_mode 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: /* Left event sources disabled, no more interrupts appear */ break;
default:return -ENOSYS; }return 0;If oneshot is not supported, the mode is never called to the driver?
Yes.
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:Why periodic is removed here?
See below..
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,
PERIODIC isn't supported and it would never be called.
.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-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:Same here?
See below..
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,
Same here.