Skip to content

kinetis: Refactor PIT timer driver implementation#8933

Merged
astralien3000 merged 2 commits intoRIOT-OS:masterfrom
jnohlgard:pr/kinetis-pit-refactor
May 29, 2018
Merged

kinetis: Refactor PIT timer driver implementation#8933
astralien3000 merged 2 commits intoRIOT-OS:masterfrom
jnohlgard:pr/kinetis-pit-refactor

Conversation

@jnohlgard
Copy link
Copy Markdown
Member

@jnohlgard jnohlgard commented Apr 12, 2018

Contribution description

Cleaned up the PIT driver implementation significantly. Simplify the implementation by stopping/starting the prescaler channel instead of the counter. The refactor also fixes some random deadline misses in the old driver where the timer would occasionally overshoot by some 0.5-2 ms. The cause of the bug is unknown, just that the broken behaviour has not been observed with this refactored driver. The measurable driver runtime overhead seem to be lower in the refactored driver. Also fixed a bug where setting a new target on a stopped timer would modify the paused timer_read value.

Issues/PRs references

depends on #8928. Use #8531 for testing

@jnohlgard jnohlgard added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) 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 Area: timers Area: timer subsystems labels Apr 12, 2018
@jnohlgard jnohlgard added this to the Release 2018.07 milestone Apr 12, 2018
@jnohlgard jnohlgard force-pushed the pr/kinetis-pit-refactor branch 2 times, most recently from 84f7f84 to 8ba9abc Compare April 18, 2018 09:31
@jnohlgard
Copy link
Copy Markdown
Member Author

Rebased after #8928 was merged

@jnohlgard jnohlgard force-pushed the pr/kinetis-pit-refactor branch from 8ba9abc to 7e09861 Compare April 25, 2018 06:38
Joakim Nohlgård added 2 commits May 22, 2018 16:47
Stop prescaler instead of counter channel to avoid the need for saving
and restoring timeouts
@jnohlgard jnohlgard force-pushed the pr/kinetis-pit-refactor branch from ac35a9a to fb73067 Compare May 22, 2018 14:47
@tcschmidt
Copy link
Copy Markdown
Member

@aabadie can you perform an initial review?

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented May 25, 2018

I don't feel enough confident with kinetis to review this, but maybe @kYc0o or @astralien3000 could be of better help here.

Copy link
Copy Markdown
Member

@astralien3000 astralien3000 left a comment

Choose a reason for hiding this comment

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

I have not seen any problem here. Implementation choices are commented. Code is clean. I have no experience in this driver so that I can't comment the way it's done, but at this point I think we just need to test.

@astralien3000
Copy link
Copy Markdown
Member

I don't have my teensy right now. I will try to test as soon as possible.

@jnohlgard
Copy link
Copy Markdown
Member Author

I recommend #8531 for testing timer implementations

@astralien3000
Copy link
Copy Markdown
Member

Sorry for the long waiting. I tested xtimer_msg, periph_timer, and bench_timers (from #8531) on a teensy31. Everything is working fine. I also tested bench_timers without this PR, which was showing errors. So, ACK !

@astralien3000 astralien3000 merged commit b7a8ba7 into RIOT-OS:master May 29, 2018
@jnohlgard jnohlgard deleted the pr/kinetis-pit-refactor branch June 8, 2018 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: timers Area: timer subsystems 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: bug The issue reports a bug / The PR fixes a bug (including spelling errors) 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.

4 participants