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

cpu/atmega2560: reworked timer implementation #5254

Merged
merged 4 commits into from
Apr 8, 2016

Conversation

haukepetersen
Copy link
Contributor

fixes also #5252

The timers were kind of broken (at least did not function correctly). This implementation should now work fine.

NOTE: the periph timer is not able to run at 1MHz (missing prescaler for that), so the tests/periph_timer test needs to be adjusted to run the timer either with 2MHz or better with 250khz.

Also the xtimer tests will most probably fail, as the xtimer requires the low-level timer to run at 1MHZ.

@haukepetersen haukepetersen 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: AVR Platform: This PR/issue effects AVR-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 6, 2016
@haukepetersen haukepetersen added this to the Release 2016.04 milestone Apr 6, 2016
@kYc0o
Copy link
Contributor

kYc0o commented Apr 6, 2016

Small murdock complain...

xtimer_shift_on_compare:arduino-mega2560:
Building application "xtimer_shift_on_compare" for "arduino-mega2560" with MCU "atmega2560".

/data/riotbuild/tests/xtimer_shift_on_compare/main.c: In function 'main':
/data/riotbuild/tests/xtimer_shift_on_compare/main.c:58:38: error: '_high_cnt' undeclared (first use in this function)
                 a = _lltimer_now() | _high_cnt;

@kaspar030
Copy link
Contributor

xtimer requires the low-level timer to run at 1MHZ.

Not true, it currently requires 1MHz or a frequency that can be reached by dividing with a power of two.

@haukepetersen
Copy link
Contributor Author

yapp, you are right, the XTIMER_USEC_TO_TICKS(1000000ul) macro confused me for a second...

@kYc0o
Copy link
Contributor

kYc0o commented Apr 7, 2016

So we need to provide _high_cnt by calculating it using xtimer_shift_on_compare is it?

@kaspar030
Copy link
Contributor

No, XTIMER_SHIFT needs to be set, probably to "2" (divide by 4), when setting the periph timer to 250khz.

XTIMER_SHIFT_ON_COMPARE probably also has to be set. It masks bits when reading two timer values in a row. Some MCU's (the atmega is a candidate) are not fast enough to return two times the same value, so some bits have to be ignored.

@haukepetersen
Copy link
Contributor Author

fixed doxygen issue and added missint XTIMER_MASK definition to the arduino-mega2560.

@kYc0o
Copy link
Contributor

kYc0o commented Apr 7, 2016

Nice! ACK and go!

@kYc0o
Copy link
Contributor

kYc0o commented Apr 7, 2016

Oh, squash first?

@haukepetersen
Copy link
Contributor Author

IMHO no need for squashing, as each commit contains a logical group of changes -> go

@haukepetersen haukepetersen merged commit 553d18a into RIOT-OS:master Apr 8, 2016
@haukepetersen haukepetersen deleted the opt_atmega_timer branch April 8, 2016 08:39
@cgundogan cgundogan added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Apr 15, 2016
@cgundogan
Copy link
Member

backport needed

@cgundogan cgundogan removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Apr 15, 2016
@haukepetersen haukepetersen added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Apr 18, 2016
@cgundogan cgundogan removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Apr 18, 2016
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: AVR Platform: This PR/issue effects AVR-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation 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

4 participants