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.

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

Kinetis: hwtimer refactor #3146

Merged
merged 1 commit into from Jun 4, 2015

Conversation

jnohlgard
Copy link
Member

This PR reduces the number of lost ticks in the Kinetis hwtimer, see #3139
The number of lost ticks is reduced to 14 in a 900 second test run of tests/vtimer_msg, that corresponds to a timer error of less than 0.5 µs / sec. Lost ticks are not good, but at least at this rate the hwtimer_now clock is actually usable.

Note: these lost ticks that this PR is trying to reduce are purely in software, it does not matter if the RTC crystal is slow/fast or otherwise bad, as both LPTMR and RTT was running off the same crystal source during the test and should therefore count up at the same time.

  • Use hwtimer_set for hwtimer_set_absolute()
  • Collect hwtimer statistics with #if ENABLE_STATS
  • Assembler optimized functions for CNR handling
  • Correct another off-by-1, after counter reset in hwtimer_set
  • Defer CMR update from hwtimer_unset until ISR fires

@jnohlgard jnohlgard added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Platform: ARM Platform: This PR/issue effects ARM-based platforms CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 2, 2015
@jnohlgard jnohlgard added this to the Release 2015.06 milestone Jun 2, 2015
uint32_t tmp;
uint32_t cnr = LPTIMER_DEV->CNR;
uint16_t tmp;
uint16_t cnr = *((uint16_t*)(&LPTIMER_DEV->CNR));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really better that way? Does it make a difference if cpu compares 32 bit or 16 bit?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it does not AFAIK.
The above was a left over from an experiment when I tested if the memory access width would make a difference. I could not observe any difference other than the CNR not latching unless the write was 32 bit.
Changed back to 32 bit.

@jnohlgard
Copy link
Member Author

Addressed comments and cleaned up a little. Replaced some 16 bit vars with 32 bit.

DEBUG("cntr: %lu, cmr: %lu, diff: %lu\n", stimer.counter32b, stimer.cmr32b, stimer.diff);
uint32_t diff = stimer.diff;
uint32_t cnr = lptmr_update_cmr(diff);
++stimer.counter32b;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please change it to stimer.counter32b += 1; , just for readability and move the comment from L143:L144 here?

@jfischer-no
Copy link
Contributor

@gebart Tested on kw2x, ACK, please squash

 - Use hwtimer_set for hwtimer_set_absolute()
 - Collect hwtimer statistics with #if ENABLE_STATS
 - Assembler optimized functions for CNR handling
 - Correct off-by-1 after counter reset
 - Defer CMR update from hwtimer_unset until ISR fires
@jnohlgard jnohlgard force-pushed the pr/kinetis-hwtimer-refactor branch from e05c9ee to 65f088a Compare June 4, 2015 12:25
@jnohlgard
Copy link
Member Author

Squashed, updated the counter32b++ part with a comment

@jfischer-no
Copy link
Contributor

travis is happy, go

jfischer-no pushed a commit that referenced this pull request Jun 4, 2015
@jfischer-no jfischer-no merged commit 02bebcd into RIOT-OS:master Jun 4, 2015
@jnohlgard jnohlgard deleted the pr/kinetis-hwtimer-refactor branch June 25, 2015 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants