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/sam3: reworked timer driver #4891

Merged
merged 4 commits into from
Feb 27, 2016
Merged

Conversation

haukepetersen
Copy link
Contributor

While searching for the bug that was failing the periph_timer test, I decided to go for a hard cleanup. I decided to make only 3 channels per timer available for the following reasons:

  • not using 3 channels saves energy
  • this simplifies the code quite a bit
  • removes some error-prone code (e.g. the disabling of interrupts in two steps)
  • the high-level timer system in RIOT (xtimer) needs only a single channel anyway

Further I reduced the amount of defined timers for the udoo and arudino-due to 2, as normally 1 is used and 2 are idle, so now only 1 being unused should be enough for use cases where one wants to use an additional timer...

So let the code size speak:

tests/periph_timer, master:
   text    data     bss     dec     hex 
   8908     132    2700   11740    2ddc

this branch:
   text    data     bss     dec     hex 
   8320     132    2708   11160    2b98

-> saves 588 byte and ~300 lines of code...

@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: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties labels Feb 24, 2016
@haukepetersen haukepetersen added this to the Release 2016.03 milestone Feb 24, 2016
@haukepetersen haukepetersen added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 26, 2016
@kYc0o
Copy link
Contributor

kYc0o commented Feb 26, 2016

I like the code, periph/timer test passed, all periph/xtimer_* tests passed, so just wait for travis and go?

@haukepetersen
Copy link
Contributor Author

yes, once you give your explicit ACK and Travis is green this can be merged :-)

@haukepetersen
Copy link
Contributor Author

Found a last-minute unclean line in the driver, the timer_irq_enable/disable implementations were broken: once you would define the used timers in another order, you would not enable/disable the correct interrupts. Fixed this -> @kYc0o: would you look at this and re-ack? Thx

@kYc0o
Copy link
Contributor

kYc0o commented Feb 26, 2016

wow! yes, you're right, I really missed that. Yes, so travis should restart?

@haukepetersen
Copy link
Contributor Author

it already went over the latest commit and is happy. Do you ack?

@kYc0o
Copy link
Contributor

kYc0o commented Feb 27, 2016

yes! ACK and merge ;)

kYc0o added a commit that referenced this pull request Feb 27, 2016
@kYc0o kYc0o merged commit 30e4266 into RIOT-OS:master Feb 27, 2016
@haukepetersen haukepetersen deleted the opt_sam3_timer branch February 29, 2016 13:39
@dondonprad
Copy link

may i ask for Makefile this application?

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 Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties 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: 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.

4 participants