Skip to content

Commit

Permalink
Merge #18632 #19031
Browse files Browse the repository at this point in the history
18632: tests/thread_float: do not overload slow MCUs with IRQs r=kaspar030 a=maribu

### Contribution description

If the regular context switches are triggered too fast, slow MCUs will be able to spent little time on actually progressing in the test. This will scale the IRQ rate with the CPU clock as a crude way too keep load within limits.

### Testing procedure

The unit test should now pass on the Microduino CoreRF

```
$ make BOARD=microduino-corerf AVRDUDE_PROGRAMMER=dragon_jtag -C tests/thread_float flash test
make: Entering directory '/home/maribu/Repos/software/RIOT/tests/thread_float'
Building application "tests_thread_float" for "microduino-corerf" with MCU "atmega128rfa1".
[...]
   text	  data	   bss	   dec	   hex	filename
  12834	   520	  3003	 16357	  3fe5	/home/maribu/Repos/software/RIOT/tests/thread_float/bin/microduino-corerf/tests_thread_float.elf
avrdude -c dragon_jtag -p m128rfa1  -U flash:w:/home/maribu/Repos/software/RIOT/tests/thread_float/bin/microduino-corerf/tests_thread_float.hex
[...]
Welcome to pyterm!
Type '/exit' to exit.
READY
s
START
main(): This is RIOT! (Version: 2022.10-devel-858-g18566-tests/thread_float)
THREADS CREATED

Context switch every 3125 µs
{ "threads": [{ "name": "idle", "stack_size": 192, "stack_used": 88 }]}
{ "threads": [{ "name": "main", "stack_size": 640, "stack_used": 220 }]}
THREAD t1 start
THREAD t2 start
THREAD t3 start
t1: 141.443770
t3: 141.466810
t1: 141.443770
t3: 141.466810
t1: 141.443770
t3: 141.466810
t1: 141.443770
t3: 141.466810
t1: 141.443770
t3: 141.466810
t1: 141.443770
t3: 141.466810
t1: 141.443770

make: Leaving directory '/home/maribu/Repos/software/RIOT/tests/thread_float'
```

(~~Note: The idle thread exiting is something that should never occur. I guess the culprit may be `cpu_switch_context_exit()` messing things up when the main thread exits. But that is not directly related to what this PR aims to fix. Adding a `thread_sleep()` at the end of `main()` does indeed prevent the idle thread from exiting.~~
Update: That's expected. The idle thread stats are printed on exit of the main thread, the idle thread does not actually exit.)

### Issues/PRs references

Fixes #16908 maybe?

19031: cpu/stm32/periph_timer: implement timer_set() r=benpicco a=maribu

### Contribution description

The fallback implementation of timer_set() in `drivers/periph_common` is known to fail on short relative sets. This adds a robust implementation.

### Testing procedure

Run `tests/periph_timer_short_relative_set` at least a few dozen times (or use #19030 to have a few dozen repetitions of the test case in a single run of the test application). It should now succeed.

### Issues/PRs references

None

Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
  • Loading branch information
bors[bot] and maribu authored Jan 4, 2023
3 parents 9f396d1 + 0835466 + b8cc222 commit 9cc4d9b
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 7 deletions.
5 changes: 5 additions & 0 deletions cpu/stm32/include/periph/cpu_timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ extern "C" {
*/
#define TIMER_CHANNEL_NUMOF (4U)

/**
* @brief The driver provides a relative set function
*/
#define PERIPH_TIMER_PROVIDES_SET

/**
* @brief Define a macro for accessing a timer channel
*/
Expand Down
41 changes: 41 additions & 0 deletions cpu/stm32/periph/timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include "cpu.h"
#include "periph/timer.h"
#include <sys/_intsup.h>

/**
* @brief Interrupt context for each configured timer
Expand Down Expand Up @@ -146,6 +147,46 @@ int timer_set_absolute(tim_t tim, int channel, unsigned int value)
return 0;
}

int timer_set(tim_t tim, int channel, unsigned int timeout)
{
if (channel >= (int)TIMER_CHANNEL_NUMOF) {
return -1;
}

unsigned irqstate = irq_disable();
set_oneshot(tim, channel);

/* clear spurious IRQs */
dev(tim)->SR &= ~(TIM_SR_CC1IF << channel);

unsigned value = (dev(tim)->CNT + timeout) & timer_config[tim].max;
TIM_CHAN(tim, channel) = value;

/* enable IRQ */
dev(tim)->DIER |= (TIM_DIER_CC1IE << channel);

#ifdef MODULE_PERIPH_TIMER_PERIODIC
if (dev(tim)->ARR == TIM_CHAN(tim, channel)) {
dev(tim)->ARR = timer_config[tim].max;
}
#endif

/* calculate time till timeout */
value = (value - dev(tim)->CNT) & timer_config[tim].max;

if (value > timeout) {
/* time till timeout is larger than requested --> timer already expired
* ==> let's make sure we have an IRQ pending :) */
dev(tim)->CR1 &= ~(TIM_CR1_CEN);
TIM_CHAN(tim, channel) = dev(tim)->CNT;
dev(tim)->CR1 |= TIM_CR1_CEN;
}

irq_restore(irqstate);

return 0;
}

#ifdef MODULE_PERIPH_TIMER_PERIODIC
int timer_set_periodic(tim_t tim, int channel, unsigned int value, uint8_t flags)
{
Expand Down
29 changes: 22 additions & 7 deletions tests/thread_float/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@
#include <stdio.h>
#include <string.h>

#include "thread.h"
#include "board.h"
#include "clk.h"
#include "macros/units.h"
#include "msg.h"
#include "periph_conf.h"
#include "thread.h"
#include "time_units.h"
#include "ztimer.h"
#include "timex.h"

static char t1_stack[THREAD_STACKSIZE_MAIN];
static char t2_stack[THREAD_STACKSIZE_MAIN];
Expand All @@ -36,15 +40,13 @@ static kernel_pid_t p1, p2, p3;

static ztimer_t timer;

#define OFFSET (100)

static mutex_t lock = MUTEX_INIT;

static void timer_cb(void *arg)
{
(void)arg;
uint32_t *timeout = arg;
thread_yield();
ztimer_set(ZTIMER_USEC, &timer, OFFSET);
ztimer_set(ZTIMER_USEC, &timer, *timeout);
}

static void *thread_1_2_3(void *_arg)
Expand Down Expand Up @@ -81,6 +83,16 @@ int main(void)
const char *t2_name = "t2";
const char *t3_name = "t3";

/* Let's not overwhelm boards by firing IRQs faster than they can handle and
* give them 50 billion CPU cycles per timeout.
*
* (Note: The `static` is required as this variable will be accessed from
* the ISR, which will occur even after the main thread has exited.) */
static uint32_t timeout = 0;
/* Note: It must be initialized dynamically, as coreclk() is not
* constant. */
timeout = 50000000000U / coreclk();

p1 = thread_create(t1_stack, sizeof(t1_stack), THREAD_PRIORITY_MAIN + 1,
THREAD_CREATE_WOUT_YIELD | THREAD_CREATE_STACKTEST,
thread_1_2_3, (void *)t1_name, t1_name);
Expand All @@ -92,8 +104,11 @@ int main(void)
thread_1_2_3, (void *)t3_name, t3_name);
puts("THREADS CREATED\n");

printf("Context switch every %" PRIu32 " µs\n", timeout);

timer.callback = timer_cb;
ztimer_set(ZTIMER_USEC, &timer, OFFSET);
timer.arg = &timeout;
ztimer_set(ZTIMER_USEC, &timer, timeout);

return 0;
}

0 comments on commit 9cc4d9b

Please sign in to comment.