On Thu, Jul 2, 2015 at 3:52 PM, Dmitry Osipenko <dig...@gmail.com> wrote:
> Timer won't start periodic ticking if ONE-SHOT -> PERIODIC mode change
> happened
> after one-shot tick was completed. Fix it by starting ticking only if timer
> was
> disabled previously and isn't ticking right now.
>
> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
> ---
> hw/timer/arm_mptimer.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
> index e230063..58e17c4 100644
> --- a/hw/timer/arm_mptimer.c
> +++ b/hw/timer/arm_mptimer.c
> @@ -122,11 +122,16 @@ static void timerblock_write(void *opaque, hwaddr addr,
> case 8: /* Control. */
> old = tb->control;
> tb->control = value;
> - if ((old & 1) == (value & 1)) {
> + /* Don't do anything if timer already disabled. */
> + if (((old & 1) == 0) && ((value & 1) == 0)) {
Your do-nothing code paths are now inconsistent between the 0 and 1
cases. I think this if can be consolidated with:
if (value & 1) {
if ((old & 1) && (tb->count != 0)) {
break;
}
if (tb->control & 2) {
...
}
tb_reload();
} else if (old & 1) {
timer_del();
}
break;
I think it's ok to squash patches 1 and 2. and just make a note in the
commit message of the multiple issues. It's hard to split this without
churning.
> break;
> }
> if (value & 1) {
> - if (tb->count == 0 && (tb->control & 2)) {
> + /* Don't do anything if timer already ticking. */
> + if (((old & 1) != 0) && (tb->count != 0)) {
> + break;
> + }
> + if (tb->control & 2) {
You have dropped the tb->count == 0 which looks like another bugfix
again. It looks like you are making sure the load value is loaded
regardless of timer run-state. If this is correct it just needs a note
in commit message.
Regards,
Peter
> tb->count = tb->load;
> }
> timerblock_reload(tb, 1);
> --
> 2.4.4
>
>
