| Message ID | 1435785330-7384-1-git-send-email-digetx@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 1 July 2015 at 22:15, Dmitry Osipenko <digetx@gmail.com> wrote: > Timer, running in periodic mode, can't be stopped or coming one-shot tick > won't be canceled because timer control code just doesn't handle timer > disabling. Fix it by checking enable bit and deleting timer if bit isn't set. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > hw/timer/arm_mptimer.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c > index 8b93b3c..917a10f 100644 > --- a/hw/timer/arm_mptimer.c > +++ b/hw/timer/arm_mptimer.c > @@ -127,6 +127,9 @@ static void timerblock_write(void *opaque, hwaddr addr, > tb->count = tb->load; > } > timerblock_reload(tb, 1); > + } else if (!(value & 1)) { > + /* Shutdown timer. */ > + timer_del(tb->timer); > } > break; > case 12: /* Interrupt status. */ Thanks; this does look like a bug. This change will mean we call timer_del() even if the timer was already disabled, though, so I think it would be slightly better to rearrange the existing logic something like this: if ((old & 1) != (value & 1)) { if (value & 1) { if (tb->count == 0 && (tb->control & 2)) { tb->count = tb->load; } timerblock_reload(tb, 1); } else { timer_del(tb->timer); } } thanks -- PMM
Hello Peter, thanks a lot for comment. 02.07.2015 12:27, Peter Maydell пишет: > Thanks; this does look like a bug. This change will mean we > call timer_del() even if the timer was already disabled, > though, so I think it would be slightly better to rearrange > the existing logic something like this: > > if ((old & 1) != (value & 1)) { > if (value & 1) { > if (tb->count == 0 && (tb->control & 2)) { > tb->count = tb->load; > } > timerblock_reload(tb, 1); > } else { > timer_del(tb->timer); > } > } > > thanks > -- PMM > Calling timer_del() twice is safe, but I agree that it would be better to avoid it. I'll send V2.
diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c index 8b93b3c..917a10f 100644 --- a/hw/timer/arm_mptimer.c +++ b/hw/timer/arm_mptimer.c @@ -127,6 +127,9 @@ static void timerblock_write(void *opaque, hwaddr addr, tb->count = tb->load; } timerblock_reload(tb, 1); + } else if (!(value & 1)) { + /* Shutdown timer. */ + timer_del(tb->timer); } break; case 12: /* Interrupt status. */
Timer, running in periodic mode, can't be stopped or coming one-shot tick won't be canceled because timer control code just doesn't handle timer disabling. Fix it by checking enable bit and deleting timer if bit isn't set. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- hw/timer/arm_mptimer.c | 3 +++ 1 file changed, 3 insertions(+)