Messages in this thread | | | Subject | Re: [PATCH] clocksource/drivers/fttmr010: fix set_next_event handler | From | Daniel Lezcano <> | Date | Thu, 20 Sep 2018 01:16:32 +0200 |
| |
On 20/09/2018 00:13, Tao Ren wrote: > Currently, the aspeed MATCH1 register is updated to <current_count - > cycles> in set_next_event handler, with the assumption that COUNT > register value is preserved when the timer is disabled and it continues > decrementing after the timer is enabled. But the assumption is wrong: > RELOAD register is loaded into COUNT register when the aspeed timer is > enabled, which means the next event may be delayed because timer > interrupt won't be generated until <0xFFFFFFFF - current_count + > cycles>. > > The problem can be fixed by updating RELOAD register to <cycles>, and > COUNT register will be re-loaded when the timer is enabled and interrupt > is generated when COUNT register overflows. > > The test result on Facebook Backpack-CMM BMC hardware (AST2500) shows > the issue is fixed: without the patch, usleep(100) suspends the process > for several milliseconds (and sometimes even over 40 milliseconds); > after applying the fix, usleep(100) takes averagely 240 microseconds to > return under the same workload level. > > Signed-off-by: Tao Ren <taoren@fb.com>
Linus, any comments ?
> --- > drivers/clocksource/timer-fttmr010.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/clocksource/timer-fttmr010.c b/drivers/clocksource/timer-fttmr010.c > index c020038ebfab..cf93f6419b51 100644 > --- a/drivers/clocksource/timer-fttmr010.c > +++ b/drivers/clocksource/timer-fttmr010.c > @@ -130,13 +130,17 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, > cr &= ~fttmr010->t1_enable_val; > writel(cr, fttmr010->base + TIMER_CR); > > - /* Setup the match register forward/backward in time */ > - cr = readl(fttmr010->base + TIMER1_COUNT); > - if (fttmr010->count_down) > - cr -= cycles; > - else > - cr += cycles; > - writel(cr, fttmr010->base + TIMER1_MATCH1); > + if (fttmr010->count_down) { > + /* > + * ASPEED Timer Controller will load TIMER1_LOAD register > + * into TIMER1_COUNT register when the timer is re-enabled. > + */ > + writel(cycles, fttmr010->base + TIMER1_LOAD); > + } else { > + /* Setup the match register forward in time */ > + cr = readl(fttmr010->base + TIMER1_COUNT); > + writel(cr + cycles, fttmr010->base + TIMER1_MATCH1); > + } > > /* Start */ > cr = readl(fttmr010->base + TIMER_CR); >
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |