04.07.2015 22:36, Peter Crosthwaite пишет:
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;
Code, you are suggesting, is logically equal to my patch. Your variant looks
cleaner, I'll switch to it. Thanks.
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.
Sorry, I don't see where tb->count == 0 got dropped. I think you missed that
timerblock_reload() checks for tb->count == 0.
To clarify:
Timer is stopped in two cases:
1) timer was shutdown; (old & 1) == 0, tb->count may be unequal to 0 (if timer
was shutdown while it was ticking).
2) one-shot was completed; (old & 1) == 1, tb->count = 0
On timer enabling we don't need to do anything if either one-shot or periodic
count is currently in fly, because timerblock_tick() would correctly handle mode
change, otherwise we re-load tb->count in case of periodic mode or starting with
whatever(left after periodic shutdown or after loading 'count' via load
register) tb->count in case of one-shot.
BTW, timer pausing is not implemented and timer can be in two states only:
running and stopped. This leads to opportunity to convert arm_mptimer to use
ptimer, I'd like to do it after fixing current issues.
> 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.
>
Well... yeah, it may worth squashing them. I'll do it.
Thanks for review.
--
Dmitry
