Page 1 of 1

GPTimer migration IDF 5.0 to newer

Posted: Thu Apr 18, 2024 6:32 am
by LSitar
Hello,
#ESP32-S3 #ESP-IDF-5.2.1 #custom-board

I worked on IDF master from 2023-02-28 (SHA 94c87a91060ff6e5962ec19e7152fc402e6c704c), so it is master between ESP-IDF Release v5.0.1 (2023-02-17) and Pre-release v5.1-beta1 (2023-05-25).
Now I try to port the project to some newer IDF tag, v5.1 at first, now I'm on v5.2.1 but the problem with GPTimer is the same.

I see the GPTimer got an internal FSM. That caused errors in my app, since I stopped a non-started gptimer etc. but this was easy to correct. It's similar to https://github.com/espressif/esp-idf/issues/13486 .

But my app still causes panic in GPTimer ISR and its non regular, like race condition.

The app uses GPTimer to set a few GPIOs with strict timing and sequence. A variable is pre-set with flags and then the timer decides in interrupt which GPIO to set and re-configures itself for next alarm interrupt.

Code: Select all

#include <stdatomic.h>
#include "driver/gptimer.h"
#include "hal/gpio_ll.h"

static gptimer_handle_t trigger_timer_handle;
static _Atomic uint32_t trigger_in_progress;

static bool IRAM_ATTR trigger_timer_callback(gptimer_handle_t timer, const gptimer_alarm_event_data_t* edata, void* user_data)
{
	// stop timer immediately
	gptimer_stop(timer);

	bool timer_restart = false;
	gptimer_alarm_config_t alarm_config;

	// THE TRIGGER CHECK SEQUENCE CORRESPONDS TO TRIGGER PRIORITY
	if (trigger_in_progress == 0)
	{
		gpio_ll_set_level(&GPIO, GPIO_FLAG1, 0);
		gpio_ll_set_level(&GPIO, GPIO_FLAG2, 0);
		gpio_ll_set_level(&GPIO, GPIO_FLAG3, 0);
	}
	else if (BIT_IS_SET(trigger_in_progress, FLAG1))
	{
		gpio_ll_set_level(&GPIO, GPIO_FLAG1, 1);
		BIT_CLEAR(trigger_in_progress, FLAG1);

		// set timer action depending on remaining triggers
		if (BIT_IS_SET(trigger_in_progress, FLAG2))
		{
			alarm_config.alarm_count = SOME_INTERVAL_1_US;
		}
		else if (BIT_IS_SET(trigger_in_progress, FLAG3))
		{
			alarm_config.alarm_count = SOME_INTERVAL_2_US;
		}
		else
		{ // no other on-triggers to set, configure timer for triggers off
			alarm_config.alarm_count = SOME_INTERVAL_3_US;
		}
		timer_restart = true;
	}
	else if (BIT_IS_SET(trigger_in_progress, FLAG3 | FLAG2))
	{
		if (BIT_IS_SET(trigger_in_progress, FLAG2))
		{
			gpio_ll_set_level(&GPIO, GPIO_FLAG2, 1);
			BIT_CLEAR(trigger_in_progress, FLAG2);
		}
		
		if (BIT_IS_SET(trigger_in_progress, FLAG3))
		{
			gpio_ll_set_level(&GPIO, GPIO_FLAG3, 1);
			BIT_CLEAR(trigger_in_progress, FLAG3);
		}

		// configure timer for triggers off
		BIT_SET(trigger_in_progress, IU_TRIG_LOAD_OFF);
		alarm_config.alarm_count = SOME_INTERVAL_4_US;
		timer_restart = true;
	}
	else
	{
		trigger_in_progress = 0;
		alarm_config.alarm_count = SOME_INTERVAL_5_US;
		timer_restart = true;
	}

	if (timer_restart)
	{
		alarm_config.flags.auto_reload_on_alarm = 0;
		ESP_ERROR_CHECK(gptimer_set_alarm_action(trigger_timer_handle, &alarm_config));
		ESP_ERROR_CHECK(gptimer_set_raw_count(trigger_timer_handle, 0));
		ESP_ERROR_CHECK(gptimer_start(trigger_timer_handle));
	}
	return pdFALSE; // return whether we need to yield at the end of ISR
}

void trigger_timer_init(void)
{
	gptimer_config_t gptimer_config = {
		.clk_src           = GPTIMER_CLK_SRC_DEFAULT,
		.direction         = GPTIMER_COUNT_UP,
		.resolution_hz     = 1000 * 1000,
		.flags.intr_shared = 0,
	};

	gptimer_new_timer(&gptimer_config, &trigger_timer_handle);	

	gptimer_event_callbacks_t callbacks = {
		.on_alarm = trigger_timer_callback,
	};

	gptimer_register_event_callbacks(trigger_timer_handle, &callbacks, NULL);
	gptimer_enable(trigger_timer_handle); // enable but not start yet
	return;
}

bool Trigger_by_timer(uint32_t trig_flags)
{
	bool error = false;
	
	if (trigger_in_progress == 0)
	{
		trigger_in_progress = trig_flags;

		// fire the timer, triggers will be set in ISR
		gptimer_alarm_config_t alarm_config = {
			.alarm_count                = 10, // tick = 1 us
			.flags.auto_reload_on_alarm = 0,
		};

		error |= gptimer_set_alarm_action(iu_timer_handle, &alarm_config);
		error |= gptimer_set_raw_count(iu_timer_handle, 0);
		error |= gptimer_start(iu_timer_handle);
	}
	else
	{
		error = true;
	}
	return error;
}
So now I suspect, is the GPTimer FSM free of race conditions? I caught such situation:
The gptimer_start() is called with some small time to alarm, and just when it releases lock the ISR is served, before state is changed from GPTIMER_FSM_RUN_WAIT to GPTIMER_FSM_RUN. So in ISR the gptimer_stop() reports wrong state, it expects GPTIMER_FSM_RUN that is yet to set by gptimer_start().
The screenshot is taken on v5.2.1 but these functions didn't changed since v5.1

According to Programming Guide, these functions I use are guaranteed to be thread safe. And there was also some note, like "alarm time from past will be triggered immediately", so it seems that my app is correct.
https://docs.espressif.com/projects/esp ... ead-safety

So in gptimer_start() and gptimer_stop(), shouldn't the state write atomic_store(&timer->fsm, XXX); be within CRITICAL_SAFE section a few lines before? Or maybe the state update should be before firing timer?

When I let the program execute without breaks on the error msg, it panics on 2 consecutive gptimer_default_isr() which I have no idea now how to debug or gather more information. Both ISRs seem to be stack'ed when calling my callback function, on gptimer.c line 542 `if (on_alarm_cb(timer, &edata, timer->user_ctx)) {`

Code: Select all

Thread #11 [meas_iu_task] 1070259856 (Name: meas_iu_task, State: Running @CPU1) (Suspended : Signal : SIGTRAP:Trace/breakpoint trap)	
	_init() at 0x40374400	
	_xt_panic() at panic_handler_asm.S:59 0x4037ae2e	
	0x40040024	
	gptimer_default_isr() at gptimer.c:543 0x4037b205	
	gptimer_default_isr() at gptimer.c:543 0x4037b205	
	_xt_lowint1() at xtensa_vectors.S:1 240 0x403779b4	
	0x400559e0	
	vPortClearInterruptMaskFromISR() at portmacro.h:564 0x4037ea26	
	vPortExitCritical() at port.c:504 0x4037ea26	
	vPortExitCriticalSafe() at portmacro.h:600 0x42035774	
	gptimer_start() at gptimer.c:353 0x42035774	
	Trigger_by_timer() at trigger.c:314 0x4201202d	
	<...more frames...>	
I'll apreciate any ideas or hints.

Re: GPTimer migration IDF 5.0 to newer

Posted: Mon Nov 04, 2024 8:12 am
by LSitar