cpu/efm32/timer: add support for LETIMER #13357

benemorius · 2020-02-13T05:10:30Z

Contribution description

This PR adds support for low energy timers to efm32. The implementation supports one LETIMER running at 32kHz which (IIRC) is a limitation imposed by the peripheral hardware.

Testing procedure

Compile with CFLAGS="-DEFM32_USE_LETIMER=1" and confirm that the letimer works as intended and with CFLAGS="-DEFM32_USE_LETIMER=0" and confirm that the regular timer still works as intended.

chrysn · 2020-02-13T09:04:19Z

As @dylad so kindly pointed out, this is almost the same as my #13344.

On first glance, your approach looks superior to mine, so I'll try this out and review it.

benemorius · 2020-02-13T09:17:16Z

Sorry I didn't notice the duplication. I did this some while back and I didn't check recently enough to make sure it was still undone. Two days. That's some coincidental timing.

chrysn

Looks good in a first review run, with some comments.

I don't know the PM layer well enough to say much there.

Tested so far with the micropython example on an stk3700, will later run more (especially timer-related; which of tests/* are generally recommended here?) on this board and the sltb001a.

boards/ikea-tradfri/include/periph_conf.h

cpu/efm32/include/periph_cpu.h

cpu/efm32/periph/timer.c

chrysn · 2020-02-13T10:59:11Z

cpu/efm32/periph/timer.c

+                LETIMER_IntEnable(tim, LETIMER_IEN_COMP0);
+                break;
+            case 1:
+                LETIMER_IntClear(tim, LETIMER_IFC_COMP1);


Looks straightforward enough, so take this more as a question of curiosity rather than a reviewer comment: Is this covered anywhere in tests?

chrysn · 2020-02-13T14:28:56Z

Concerning testing: I've run the periph_timer test as make -C tests/periph_timer BOARD=stk3700 all flash test, and that worked after adding my board to the BOARDS_TIMER_32kHz list in the test's Makefile -- kind of to be expected when the timers can only do that frequency. In practice, that list should be extended by whichever boards wind up having the LETIMER as a default, evaluating both channels.

With CFLAGS=-DEFM32_USE_LETIMER=0, three channels were tested for TIMER0, but did not succeed -- but then again, neither does it on master as far as I can see without getting my branches tangled, will investigate further to confirm.

chrysn · 2020-02-13T15:52:10Z

Correction: make -C tests/periph_timer BOARD=stk3700 TIMER_SPEED=32768 all flash test does work on master (that particular speed was a leftover from other tests, 28000 works just as 2000 or 1000000 does), whereas the same prefixed with CFLAGS=-DEFM32_USE_LETIMER=0 doesn't on this branch -- unlike on master, it ends with

TIMER_0: set channel 2 to 15000
TIMER_0: starting
Timeout in expect script at "child.expect('TEST SUCCEEDED')" (tests/periph_timer/tests/01-run.py:21)

I've tried stepping through the individual variants, but failed to find a precise culprit that way. I had suspicions about the power manager briefly, but even without the top commit, the test fails when EFM32_USE_LETIMER=0.

chrysn · 2020-02-13T16:12:58Z

In happier news, I've tested this with the sltb001a (programming which is a pain for different reasons: an openocd 0.11 would be direly due). With the default (EFM32_USE_LETIMER=1) setting, it passes tests/periph_timer, and builts micropython with time.sleep() just fine.

benemorius · 2020-02-14T07:20:10Z

With CFLAGS=-DEFM32_USE_LETIMER=0, three channels were tested for TIMER0, but did not succeed

Whoops, very sorry for the trouble. I just pushed a fix for that. It helps a lot if you actually start the timer...

chrysn · 2020-02-14T07:34:30Z

Yes, that makes both EFM32_USE_LETIMER=[01] pass tests/periph_timer on stk3700 and sltb001a, the letimer=0 version also with TIMER_SPEED 2000 or 1000000.

(One could argue about the necessity to keep the prescaler running, but when it runs I figure it contributes little to the total power consumption; that can still be tuned later.)

benemorius · 2020-03-02T06:40:38Z

Sorry for the delay. I finally made the drive to Ikea for a Tradfri platform I can test on. I'll get back to this soon.

cpu/efm32/periph/timer.c

boards/ikea-tradfri/include/board.h

cpu/efm32/periph/timer.c

basilfx · 2020-06-16T19:51:22Z

I gave this PR a quick test on the STK3600, SLSTK3401a and SLSTK3402a and that seems to work. I tried the xtimer example and some xtimer tests.

I think, with Kconfig, we can make the whole LETIMER even optional (as an optimization), just like I did with the LEUART driver. But that is definitely for later.

basilfx · 2020-06-16T19:52:03Z

@benemorius The requested changes are small. Feel free to rebase + squash.

benemorius · 2020-06-17T03:54:00Z

I guess I didn't recall travis was that stringent on documentation. I'll get that fixed up soon.

benemorius · 2020-06-17T23:57:14Z

Nevermind, I just botched the doxygen while adapting for #13900. It should be good now.

basilfx · 2020-06-24T13:42:06Z

@benemorius I think that part of your last commit should be squashed with the first commit (that whole #ifndef EFM32_USE_LETIMER that was added in the first commit, but removed in the last commit).

benemorius · 2020-07-11T22:56:19Z

Rebased to include the new CI checks.

benpicco

Code looks good and from what I gathered this works - I think this PR was simply forgotten.

fjmolinas · 2020-08-11T15:18:22Z

This PR broke tests/periph_timer on slstk3402a EDIT: or there is a missing configuration change for this board

2020-08-11 17:04:42,923 # main(): This is RIOT! (Version: 2020.10-devel-689-g57946c-pr/periph/timer_max)
2020-08-11 17:04:42,923 #
2020-08-11 17:04:42,924 # Test for peripheral TIMERs
2020-08-11 17:04:42,924 #
2020-08-11 17:04:42,932 # Available timers: 1
2020-08-11 17:04:42,932 #
2020-08-11 17:04:42,933 # Testing TIMER_0:
2020-08-11 17:04:42,934 # TIMER_0: initialization successful
2020-08-11 17:04:42,934 # TIMER_0: stopped
2020-08-11 17:04:42,935 # TIMER_0: set channel 0 to 5000
2020-08-11 17:04:42,937 # TIMER_0: set channel 1 to 10000
2020-08-11 17:04:42,937 # TIMER_0: starting
2020-08-11 17:06:30,324 # TIMER_0: channel 0 fired at SW count 429468906 - init: 429468906
2020-08-11 17:06:30,327 # TIMER_0: channel 1 fired at SW count 429488896 - diff:    19990
2020-08-11 17:06:30,327 #
2020-08-11 17:06:30,328 # TEST SUCCEEDED
2020-08-11 17:15:51,885 # Help: Press s to start test, r to print it is ready

benemorius · 2020-08-12T21:42:04Z

@fjmolinas I'm confused so far. Is this with EFM32_USE_LETIMER 1 or 0?

Could this have anything to do with LETIMER being countdown-only and therefore having the value inverted when read and written?

fjmolinas · 2020-08-13T06:54:02Z

@fjmolinas I'm confused so far. Is this with EFM32_USE_LETIMER 1 or 0?

Could this have anything to do with LETIMER being countdown-only and therefore having the value inverted when read and written?

Its the default, I think that is EFM32_USE_LETIMER 0

fjmolinas · 2020-08-13T06:54:43Z

boards/ikea-tradfri/include/board.h

 * @{
 */
+#ifdef EFM32_USE_LETIMER


This check is strange, since it uses #Ifdef if 0 or 1 this statement will evaluate as true.

Will this be enough to fix the timer on slstk3402a?

hmm no, something else must be off..

fjmolinas · 2020-08-13T06:55:05Z

boards/ikea-tradfri/include/periph_conf.h

 * @{
 */
+#if EFM32_USE_LETIMER


IMO IS_ACTIVE should be used here.

fjmolinas · 2020-08-18T12:27:08Z

cpu/efm32/periph/timer.c

+        if (!(tim->STATUS & TIMER_STATUS_RUNNING)) {
+            pm_block(EFM32_TIMER_PM_BLOCKER);
+        }
+        TIMER_Enable(timer_config[dev].timer.dev, true);


Toggling the prescaler timer fixed the issue for the test.

fjmolinas · 2020-08-20T07:55:28Z

@fjmolinas I'm confused so far. Is this with EFM32_USE_LETIMER 1 or 0?

Could this have anything to do with LETIMER being countdown-only and therefore having the value inverted when read and written

@benemorius can you take a look at 32e112d in #14780. That fixed the issue for me.

chrysn mentioned this pull request Feb 13, 2020

[WIP] efm32: add option to use LETIMER as a timer #13344

Closed

chrysn reviewed Feb 13, 2020

View reviewed changes

benpicco added Area: cpu Area: CPU/MCU ports Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Feb 13, 2020

benemorius requested review from basilfx and MrKevinWeiss as code owners March 21, 2020 22:37

basilfx reviewed Jun 16, 2020

View reviewed changes

cpu/efm32/periph/timer.c Show resolved Hide resolved

benemorius force-pushed the pr/efm32-letimer branch from e3a095a to 79e944b Compare June 17, 2020 01:51

benemorius force-pushed the pr/efm32-letimer branch from 79e944b to 181e983 Compare June 17, 2020 23:56

basilfx added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 24, 2020

benemorius force-pushed the pr/efm32-letimer branch from 181e983 to c463af9 Compare June 26, 2020 02:59

benemorius added 3 commits July 10, 2020 20:44

cpu/efm32/timer: add support for LETIMER

651a3bf

cpu/efm32/timer: add pm blockers

37a6cc6

boards: efm32 boards: add support for LETIMER

754d790

benemorius force-pushed the pr/efm32-letimer branch from c463af9 to 754d790 Compare July 11, 2020 01:44

benpicco approved these changes Aug 6, 2020

View reviewed changes

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 7, 2020

benpicco merged commit cc1ffc8 into RIOT-OS:master Aug 7, 2020

fjmolinas reviewed Aug 13, 2020

View reviewed changes

boards/ikea-tradfri/include/periph_conf.h

* @{

*/

#if EFM32_USE_LETIMER

Copy link

Contributor

fjmolinas Aug 13, 2020

Choose a reason for hiding this comment

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

IMO IS_ACTIVE should be used here.

fjmolinas reviewed Aug 18, 2020

View reviewed changes

fjmolinas mentioned this pull request Aug 18, 2020

cpu/efm32/timer: cleanups and improvements #14780

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cpu/efm32/timer: add support for LETIMER #13357

cpu/efm32/timer: add support for LETIMER #13357

benemorius commented Feb 13, 2020

chrysn commented Feb 13, 2020

benemorius commented Feb 13, 2020

chrysn left a comment

chrysn Feb 13, 2020

chrysn commented Feb 13, 2020

chrysn commented Feb 13, 2020

chrysn commented Feb 13, 2020

benemorius commented Feb 14, 2020

chrysn commented Feb 14, 2020

benemorius commented Mar 2, 2020

basilfx commented Jun 16, 2020

basilfx commented Jun 16, 2020

benemorius commented Jun 17, 2020

benemorius commented Jun 17, 2020

basilfx commented Jun 24, 2020

benemorius commented Jul 11, 2020

benpicco left a comment

fjmolinas commented Aug 11, 2020 •

edited

benemorius commented Aug 12, 2020

fjmolinas commented Aug 13, 2020

fjmolinas Aug 13, 2020

benpicco Aug 18, 2020

fjmolinas Aug 18, 2020

fjmolinas Aug 13, 2020

fjmolinas Aug 18, 2020

fjmolinas commented Aug 20, 2020

cpu/efm32/timer: add support for LETIMER #13357

cpu/efm32/timer: add support for LETIMER #13357

Conversation

benemorius commented Feb 13, 2020

Contribution description

Testing procedure

chrysn commented Feb 13, 2020

benemorius commented Feb 13, 2020

chrysn left a comment

Choose a reason for hiding this comment

chrysn Feb 13, 2020

Choose a reason for hiding this comment

chrysn commented Feb 13, 2020

chrysn commented Feb 13, 2020

chrysn commented Feb 13, 2020

benemorius commented Feb 14, 2020

chrysn commented Feb 14, 2020

benemorius commented Mar 2, 2020

basilfx commented Jun 16, 2020

basilfx commented Jun 16, 2020

benemorius commented Jun 17, 2020

benemorius commented Jun 17, 2020

basilfx commented Jun 24, 2020

benemorius commented Jul 11, 2020

benpicco left a comment

Choose a reason for hiding this comment

fjmolinas commented Aug 11, 2020 • edited

benemorius commented Aug 12, 2020

fjmolinas commented Aug 13, 2020

fjmolinas Aug 13, 2020

Choose a reason for hiding this comment

benpicco Aug 18, 2020

Choose a reason for hiding this comment

fjmolinas Aug 18, 2020

Choose a reason for hiding this comment

fjmolinas Aug 13, 2020

Choose a reason for hiding this comment

fjmolinas Aug 18, 2020

Choose a reason for hiding this comment

fjmolinas commented Aug 20, 2020

fjmolinas commented Aug 11, 2020 •

edited