On 26 May 2014 11:43, Preeti U Murthy preeti@linux.vnet.ibm.com wrote:
Ah ok! I see now. I mistook the changelog. I assumed that these drivers are returning success in default case although they are disabling the clock device and that we now intend to change that to return an error code. Sorry about this!
Now I see that it is an issue. Now we are going to hit the default case and get returned an error code, but with disabled clock device which is unexpected.Hmmm..this needs some thought.
Gud that we are on the same page now :)
I have already fixed it locally, not sure if that's the best approach though.
clockevent: drivers: don't disable events for unsupported modes
Few clockevent drivers have disabled clock events as soon as we enter their ->set_dev_mode() callbacks and so they stay disabled for unsupported modes as well.
All clockevent drivers are handling modes in ->set_dev_mode() with help of a 'switch' block. For the currently available modes none of them would ever get down to the 'default case' of switch block.
And so we would never hit the BUG where we failed to change mode and still disabled clock events.
But as soon as any new mode would be introduced in clockevents core, some of these might behave unexpectedly. Like, core may expect events to be enabled as call to ->set_dev_mode() returned failure and may not try enabling events again.
This patch tries to make sure such drivers don't left events disabled for unsupported modes.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- arch/arm/common/timer-sp.c | 4 ++-- arch/arm/mach-at91/at91rm9200_time.c | 7 +++++-- arch/arm/mach-clps711x/common.c | 3 +-- arch/arm/mach-imx/epit.c | 9 +++++++-- arch/arm/mach-imx/time.c | 9 +++++++-- arch/arm/mach-integrator/integrator_ap.c | 12 ++++++------ arch/arm/mach-netx/time.c | 5 ++--- arch/arm/mach-omap1/timer32k.c | 5 ++--- arch/arm/mach-omap2/timer.c | 5 ++--- arch/arm/mach-spear/time.c | 10 ++++++---- arch/mips/kernel/cevt-txx9.c | 5 ++++- drivers/clocksource/cs5535-clockevt.c | 4 ++-- drivers/clocksource/exynos_mct.c | 7 ++++--- drivers/clocksource/mxs_timer.c | 7 ++++--- drivers/clocksource/samsung_pwm_timer.c | 6 ++---- drivers/clocksource/tegra20_timer.c | 5 ++--- drivers/clocksource/timer-marco.c | 2 +- 17 files changed, 59 insertions(+), 46 deletions(-)
diff --git a/arch/arm/common/timer-sp.c b/arch/arm/common/timer-sp.c index 9ed3845..8831e9e 100644 --- a/arch/arm/common/timer-sp.c +++ b/arch/arm/common/timer-sp.c @@ -132,16 +132,16 @@ static int sp804_set_mode(enum clock_event_mode mode, { 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;
... .. .
And this goes on for the drivers mentioned above.
Anyway, it was broken before this patch as well and so I will try getting that fixed once this set is in.
Have we reached a state to start getting reivewed-by's ? :)