Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sys/vtimer: Fix two vtimer issues (hwtimer tick conversion). #2515

Merged
merged 2 commits into from
Jun 1, 2015
Merged

sys/vtimer: Fix two vtimer issues (hwtimer tick conversion). #2515

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
645ba51
sys/vtimer: Fix two vtimer issues (hwtimer tick conversion).
Mar 2, 2015
2ee6ff0
sys/vtimer: Fix longterm_tick_timer handling.
Mar 4, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 14 additions & 22 deletions sys/vtimer/vtimer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ static volatile int in_callback = false;
static int hwtimer_id = -1;
static uint32_t hwtimer_next_absolute;

static uint32_t seconds = 0;

static inline priority_queue_node_t *timer_get_node(vtimer_t *timer)
{
if (!timer) {
Expand Down Expand Up @@ -117,25 +115,20 @@ static int update_shortterm(void)
/* short term part of the next vtimer */
hwtimer_next_absolute = shortterm_priority_queue_root.first->priority;

uint32_t next = hwtimer_next_absolute;
uint32_t next = hwtimer_next_absolute + longterm_tick_start;

/* current short term time */
uint32_t now = HWTIMER_TICKS_TO_US(hwtimer_now());

/* make sure the longterm_tick_timer does not get truncated */
if (node_get_timer(shortterm_priority_queue_root.first)->action != vtimer_callback_tick) {
/* the next vtimer to schedule is the long term tick */
/* it has a shortterm offset of longterm_tick_start */
next += longterm_tick_start;
}
uint32_t now_ticks = hwtimer_now();
uint32_t now = HWTIMER_TICKS_TO_US(now_ticks);

if((next - HWTIMER_TICKS_TO_US(VTIMER_THRESHOLD) - now) > MICROSECONDS_PER_TICK ) {
DEBUG("truncating next (next - HWTIMER_TICKS_TO_US(VTIMER_THRESHOLD) - now): %lu\n", (next - HWTIMER_TICKS_TO_US(VTIMER_THRESHOLD) - now));
next = now + HWTIMER_TICKS_TO_US(VTIMER_BACKOFF);
}

DEBUG("update_shortterm: Set hwtimer to %" PRIu32 " (now=%lu)\n", next, HWTIMER_TICKS_TO_US(hwtimer_now()));
hwtimer_id = hwtimer_set_absolute(HWTIMER_TICKS(next), vtimer_callback, NULL);
DEBUG("update_shortterm: Set hwtimer to %" PRIu32 " (now=%lu)\n", next, now);
uint32_t next_ticks = now_ticks + HWTIMER_TICKS(next - now);
hwtimer_id = hwtimer_set_absolute(next_ticks, vtimer_callback, NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand why the original implementation used hwtimer_set_absolute since they are setting a relative time. hwtimer_set is semantically more correct for this operation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's also what i thought that the set_relative functions may be more appropriate here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus it is the only usage of hwtimer_set_absolute() - would be great to get rid of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're going to change this, @lightblu?


return 0;
}
Expand All @@ -145,16 +138,16 @@ void vtimer_callback_tick(vtimer_t *timer)
(void) timer;

DEBUG("vtimer_callback_tick().\n");
seconds += SECONDS_PER_TICK;

longterm_tick_start = longterm_tick_timer.absolute.microseconds;
longterm_tick_timer.absolute.microseconds += MICROSECONDS_PER_TICK;
longterm_tick_start += MICROSECONDS_PER_TICK;
longterm_tick_timer.absolute.seconds += SECONDS_PER_TICK;
longterm_tick_timer.absolute.microseconds = MICROSECONDS_PER_TICK; // Should never change, just for clarity.
set_shortterm(&longterm_tick_timer);

while (longterm_priority_queue_root.first) {
vtimer_t *timer = node_get_timer(longterm_priority_queue_root.first);

if (timer->absolute.seconds == seconds) {
if (timer->absolute.seconds == longterm_tick_timer.absolute.seconds) {
priority_queue_remove_head(&longterm_priority_queue_root);
set_shortterm(timer);
}
Expand Down Expand Up @@ -262,7 +255,7 @@ static int vtimer_set(vtimer_t *timer)
}

unsigned state = disableIRQ();
if (timer->absolute.seconds != seconds) {
if (timer->absolute.seconds != longterm_tick_timer.absolute.seconds) {
/* we're long-term */
DEBUG("vtimer_set(): setting long_term\n");
result = set_longterm(timer);
Expand All @@ -286,10 +279,10 @@ static int vtimer_set(vtimer_t *timer)

void vtimer_now(timex_t *out)
{
uint32_t us = HWTIMER_TICKS_TO_US(hwtimer_now() - longterm_tick_start);
uint32_t us_per_s = 1000ul * 1000ul;
uint32_t us = HWTIMER_TICKS_TO_US(hwtimer_now()) - longterm_tick_start;
static const uint32_t us_per_s = 1000ul * 1000ul;

out->seconds = seconds + us / us_per_s;
out->seconds = longterm_tick_timer.absolute.seconds + us / us_per_s;
out->microseconds = us % us_per_s;
}

Expand Down Expand Up @@ -317,7 +310,6 @@ int vtimer_init(void)
{
DEBUG("vtimer_init().\n");
unsigned state = disableIRQ();
seconds = 0;

longterm_tick_start = 0;

Expand Down