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

periph/timer: Change timer_init API to support arbitrary (integer) frequencies #4040

Merged
merged 39 commits into from Feb 15, 2016

Conversation

jnohlgard
Copy link
Member

This PR modifies the timer_init function to take a frequency parameter instead of another mostly unused us per tick or tick per us parameter.

The rationale is that with the previous API there is no way to request a 32768 Hz timer, which is a common frequency for RTC crystals.

The new timer_init function has the following declaration:

int timer_init(tim_t dev, unsigned long freq, void (*callback)(int));

Needs testing on all boards... sigh

Use tests/xtimer_msg, tests/xtimer_usleep_until etc for testing.
Things to look out for:

  • Timers running too fast/slow, it should be enough to run tests/xtimer_msg for couple of minutes.
  • Timers stopping after some time, it should be enough to run tests/xtimer_msg for a couple of minutes.
  • Make sure tests/xtimer_usleep_until can run to finish, the text Test complete. should be printed at the end if all goes well)

Testing checklist:now=253.098701 (0 hours 4 min), diff=0

  • airfy-beacon
  • arduino-due (repeated O.K.)
  • arduino-mega2560
  • avsextrem
  • cc2538dk (FAIL)
  • chronos
  • ek-lm4f120xl
  • f4vi1
  • fox
  • frdm-k64f
  • iotlab-m3
  • limifrog-v1
  • mbed_lpc1768
  • msb-430
  • msb-430h
  • msba2
  • msbiot
  • mulle
  • native
  • nrf51dongle
  • nrf52dk
  • nrf6310
  • nucleo-f091
  • nucleo-f303
  • nucleo-f334
  • nucleo-l1 , xtimer_usleep_until, xtimer_msg_receive_timeout ?
  • openmote
  • pba-d-01-kw2x
  • pca10000
  • pca10005
  • pttu
  • remote
  • saml21-xpro
  • samr21-xpro (repeated O.K.)
  • slwstk6220a
  • spark-core
  • stm32f0discovery, xtimer_drift, xtimer_msg_receive_timeout ?
  • stm32f3discovery
  • stm32f4discovery, xtimer_drift, xtimer_msg, xtimer_msg_receive_timeout ? - stm32f4: UART DMA causes lockup in xtimer_drift test #4716
  • telosb
  • udoo
  • weio
  • wsn430-v1_3b
  • wsn430-v1_4
  • yunjia-nrf51822
  • z1

@jnohlgard jnohlgard added Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Area: timers Area: timer subsystems labels Oct 3, 2015
@jnohlgard
Copy link
Member Author

This is to prepare the API for some xtimer changes (#3990) which will be necessary for low power operation on some platforms (Kinetis and others).

@jnohlgard jnohlgard added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 3, 2015
@jnohlgard jnohlgard force-pushed the pr/periph-timer-init-freq branch 2 times, most recently from 373df4d to 2f555c7 Compare October 4, 2015 09:54
@jnohlgard
Copy link
Member Author

Size seem to have gone up marginally on most platforms. The 2 kB save on Kinetis is due to the rewrite of the prescaler calculation.

I have no name!@a34090b6ee74:/data/riotbuild/riotproject/tests/periph_timer$ make info-buildsizes-diff OLDBIN=master NEWBIN=periph-timer-init-freq 
text    data    bss     dec     BOARD/BINDIRBASE

64      0       0       64      airfy-beacon
8936    128     2680    11744   master
9000    128     2680    11808   periph-timer-init-freq

8       0       0       8       arduino-due
9356    132     2716    12204   master
9364    132     2716    12212   periph-timer-init-freq

8       0       0       8       arduino-mega2560
7138    1056    757     8951    master
7146    1056    757     8959    periph-timer-init-freq

16      0       0       16      avsextrem
40352   2240    96057   138649  master
40368   2240    96057   138665  periph-timer-init-freq

-4      0       0       -4      cc2538dk
8984    128     2688    11800   master
8980    128     2688    11796   periph-timer-init-freq

14      0       0       14      chronos
8002    90      1050    9142    master
8016    90      1050    9156    periph-timer-init-freq

24      0       0       24      ek-lm4f120xl
8800    128     2680    11608   master
8824    128     2680    11632   periph-timer-init-freq

24      0       0       24      f4vi1
8996    132     2676    11804   master
9020    132     2676    11828   periph-timer-init-freq

12      0       0       12      fox
9252    128     2808    12188   master
9264    128     2808    12200   periph-timer-init-freq

-2016   0       0       -2016   frdm-k64f
10812   1156    4836    16804   master
8796    1156    4836    14788   periph-timer-init-freq

12      0       0       12      iotlab-m3
9276    128     2808    12212   master
9288    128     2808    12224   periph-timer-init-freq

8       0       0       8       limifrog-v1
8492    128     2688    11308   master
8500    128     2688    11316   periph-timer-init-freq

12      0       0       12      mbed_lpc1768
8576    128     2688    11392   master
8588    128     2688    11404   periph-timer-init-freq

14      0       0       14      msb-430
7640    18      1102    8760    master
7654    18      1102    8774    periph-timer-init-freq

14      0       0       14      msb-430h
7518    18      1102    8638    master
7532    18      1102    8652    periph-timer-init-freq

16      0       0       16      msba2
40008   2240    96057   138305  master
40024   2240    96057   138321  periph-timer-init-freq

24      0       0       24      msbiot
9292    132     2700    12124   master
9316    132     2700    12148   periph-timer-init-freq

-2060   0       0       -2060   mulle
12868   1180    4572    18620   master
10808   1180    4572    16560   periph-timer-init-freq

16      0       0       16      native
22930   396     47904   71230   master
22946   396     47904   71246   periph-timer-init-freq

64      0       0       64      nrf51dongle
8960    128     2680    11768   master
9024    128     2680    11832   periph-timer-init-freq

64      0       0       64      nrf6310
8936    128     2680    11744   master
9000    128     2680    11808   periph-timer-init-freq

12      0       0       12      nucleo-f091
8932    128     2688    11748   master
8944    128     2688    11760   periph-timer-init-freq

12      0       0       12      nucleo-f303
8908    128     2696    11732   master
8920    128     2696    11744   periph-timer-init-freq

12      0       0       12      nucleo-f334
8648    128     2672    11448   master
8660    128     2672    11460   periph-timer-init-freq

8       0       0       8       nucleo-l1
8368    128     2680    11176   master
8376    128     2680    11184   periph-timer-init-freq

-4      0       0       -4      openmote
8892    128     2688    11708   master
8888    128     2688    11704   periph-timer-init-freq

-2016   0       0       -2016   pba-d-01-kw2x
10880   1156    4220    16256   master
8864    1156    4220    14240   periph-timer-init-freq

64      0       0       64      pca10000
8960    128     2680    11768   master
9024    128     2680    11832   periph-timer-init-freq

64      0       0       64      pca10005
8940    128     2680    11748   master
9004    128     2680    11812   periph-timer-init-freq

16      0       0       16      pttu
40560   2240    96057   138857  master
40576   2240    96057   138873  periph-timer-init-freq

-4      0       0       -4      remote
9548    128     2688    12364   master
9544    128     2688    12360   periph-timer-init-freq

4       0       0       4       saml21-xpro
9088    128     2800    12016   master
9092    128     2800    12020   periph-timer-init-freq

12      0       0       12      samr21-xpro
9456    128     2816    12400   master
9468    128     2816    12412   periph-timer-init-freq

12      0       0       12      spark-core
9308    128     2808    12244   master
9320    128     2808    12256   periph-timer-init-freq

12      0       0       12      stm32f0discovery
8940    128     2688    11756   master
8952    128     2688    11768   periph-timer-init-freq

12      0       0       12      stm32f3discovery
8916    128     2696    11740   master
8928    128     2696    11752   periph-timer-init-freq

24      0       0       24      stm32f4discovery
9160    132     2692    11984   master
9184    132     2692    12008   periph-timer-init-freq

14      0       0       14      telosb
7584    14      1102    8700    master
7598    14      1102    8714    periph-timer-init-freq

8       0       0       8       udoo
9356    132     2716    12204   master
9364    132     2716    12212   periph-timer-init-freq

12      0       0       12      weio
8736    128     2672    11536   master
8748    128     2672    11548   periph-timer-init-freq

14      0       0       14      wsn430-v1_3b
7588    18      1102    8708    master
7602    18      1102    8722    periph-timer-init-freq

14      0       0       14      wsn430-v1_4
7588    18      1102    8708    master
7602    18      1102    8722    periph-timer-init-freq

64      0       0       64      yunjia-nrf51822
8924    128     2680    11732   master
8988    128     2680    11796   periph-timer-init-freq

14      0       0       14      z1
7312    14      1102    8428    master
7326    14      1102    8442    periph-timer-init-freq

@kaspar030
Copy link
Contributor

What do you think about adding a prescaler value? Just a signed int. Negative means "right shift the freq parameter by N", positive means "left shift".

Otherwise, the current signature allows only timers with 2^32 Hz, while many hardware timers can do a lot more.

@jnohlgard
Copy link
Member Author

@kaspar030 basically what you describe is encoding the frequency as a floating point value. Do you want me to update the signature to float freq?

@kaspar030
Copy link
Contributor

@gebart Wouldn't that introduce floating point code dependencies?

Do you have another idea on how to allow >4MHz timer speeds?

@jnohlgard
Copy link
Member Author

@kaspar030 that's not 4MHz, it's 4GHz that is the limit.

While I do have some radio hardware which uses a 48 GHz clock internally, it is mostly non-configurable, and very specific to its use case (measuring time of flight for radio waves). Can we extend this API further when we see a use case for general purpose 4+ GHz timers?

@kaspar030
Copy link
Contributor

kaspar030 commented Oct 7, 2015 via email

@jnohlgard
Copy link
Member Author

@kaspar030 is that an ACK for the API change?

@jnohlgard
Copy link
Member Author

rebased on latest master

@A-Paul
Copy link
Member

A-Paul commented Oct 8, 2015

I collected some HW on my desk and may test on arduino-due, nucleo-f091, stm32f0discovery.
Maybe nucleo-f303 and pba-d-01-kw2x next week.

Timers stopping after some time (especially in tests/xtimer_usleep_until)

Can you be more specific about some time? :-) I ran xtimer_msg for ~45min. Is that enouth?

@jnohlgard
Copy link
Member Author

Testing for a few minutes should be enough.

I saw a problem before where the xtimer_usleep_until test simply stopped after a few seconds, but that was a symptom of a timer running at a much higher speed than what xtimer believed it to be running at, in combination with some other messed up settings and possibly the xtimer bug that was identified and fixed yesterday by Kaspar and Daniel.

To test thoroughly we need to run it for at least 4300 seconds (until overflow), but I don't think this will be necessary for this API change. It should not affect the overflow behaviour.

@jnohlgard
Copy link
Member Author

@A-Paul ping!

@A-Paul
Copy link
Member

A-Paul commented Oct 13, 2015

On arduino-due, nucleo-f091, stm32f0discovery and pba-d-01-kw2x all tests/xtimer_{hang,remove,reset,usleep_until} succeeded. tests/xtimer_msg ran 300s with no drift on the first three.
On pba-d-01-kw2x I had -5s (sec=295). On a longer running test I had -146s after 7200s runtime, which is ~-2%. Don't know if this is seen as acceptable.

I had issues to get terminal connection with our nucleo-f303, so I couldn't test. Will try again tomorrow on campus.

@jnohlgard
Copy link
Member Author

@A-Paul thanks, pba-d-01-kw2x is not expected to be perfect with regards to drift. See #4065 for the refactored timer driver I have been working on, to which this API change is a prerequisite.

@A-Paul
Copy link
Member

A-Paul commented Oct 14, 2015

Also checked nucleo-f303. Everything is fine.

@jnohlgard jnohlgard added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Oct 21, 2015
@jnohlgard
Copy link
Member Author

  • rebased
  • fixed static test failure in avr/timer.c regarding unused uint32_t b when all timers are disabled.

@jnohlgard
Copy link
Member Author

Updated the nrf52 with the exact same change as the nrf51 (git show | patch -p3)

@jnohlgard
Copy link
Member Author

Travis is green.
Can someone do a quick review of the nrf52 which I added after the last ACK?

@jnohlgard
Copy link
Member Author

Should I squash this or leave it as separate commits for each CPU? The code will not be able to build for the intermediate commits, only after all commits are merged. But on the other hand, the squashed commit will be huge..

@haukepetersen
Copy link
Contributor

changes for the nrf52 look good. ACK

@jnohlgard
Copy link
Member Author

YAY!

jnohlgard pushed a commit that referenced this pull request Feb 15, 2016
periph/timer: Change timer_init API to support arbitrary (integer) frequencies
@jnohlgard jnohlgard merged commit e2f7ac7 into RIOT-OS:master Feb 15, 2016
@jnohlgard jnohlgard deleted the pr/periph-timer-init-freq branch February 15, 2016 22:36
@OlegHahm
Copy link
Member

Hooray! Congrats and thanks for the awesome work.

Btw with this PR we just crossed another frontier: 10,013 commits!

@haukepetersen
Copy link
Contributor

nice!

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 Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties PR-award-nominee Deprecated. Will be removed soon. Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants