Merge branch 'rahix/interrupts' into 'master'

Dispatch interrupts asynchroneously

See merge request !377

Documentation/overview.rst

@@ -14,31 +14,34 @@ Epicardium
 Epicardium is based on `FreeRTOS <https://www.freertos.org/>`_.  There are a
 number of tasks that will have been keeping card10 running.  These are:
 
-+-------------------+-------------------------------+----------+-------------------------------------------+
++--------------------+-------------------------------+----------+-------------------------------------------+
 | Name               | ID Global                     | Priority | Description                               |
-+===================+===============================+==========+===========================================+
++====================+===============================+==========+===========================================+
 | `vPmicTask`_       | ``pmic_task_id`` (static)     | +4       | Power Management (and Reset Button)       |
-+-------------------+-------------------------------+----------+-------------------------------------------+
++--------------------+-------------------------------+----------+-------------------------------------------+
 | `vLifecycleTask`_  | ``lifecycle_task`` (static)   | +3       | Control of the payload running on core 1. |
-+-------------------+-------------------------------+----------+-------------------------------------------+
++--------------------+-------------------------------+----------+-------------------------------------------+
 | `vBleTask`_        | ``ble_task_id`` (static)      | +3       | Bluetooth Low Energy Stack                |
-+-------------------+-------------------------------+----------+-------------------------------------------+
++--------------------+-------------------------------+----------+-------------------------------------------+
 | `vSerialTask`_     | ``serial_task_id``            | +3       | Serial Output via UART/CDC-ACM/BLE        |
-+-------------------+-------------------------------+----------+-------------------------------------------+
++--------------------+-------------------------------+----------+-------------------------------------------+
 | `vApiDispatcher`_  | ``dispatcher_task_id``        | +2       | Epicardium API dispatcher                 |
-+-------------------+-------------------------------+----------+-------------------------------------------+
++--------------------+-------------------------------+----------+-------------------------------------------+
+| `vInterruptsTask`_ | ``interrupts_task`` (static)  | +2       | Interrupt dispatcher worker               |
++--------------------+-------------------------------+----------+-------------------------------------------+
 | `vLedTask`_        | -/-                           | +1       | LED Animations                            |
-+-------------------+-------------------------------+----------+-------------------------------------------+
++--------------------+-------------------------------+----------+-------------------------------------------+
 | `vMAX30001Task`_   | ``max30001_task_id`` (static) | +1       | `MAX30001`_ ECG driver                    |
-+-------------------+-------------------------------+----------+-------------------------------------------+
++--------------------+-------------------------------+----------+-------------------------------------------+
 | `vBhi160Task`_     | ``bhi160_task_id`` (static)   | +1       | `BHI160`_ sensor fusion driver            |
-+-------------------+-------------------------------+----------+-------------------------------------------+
++--------------------+-------------------------------+----------+-------------------------------------------+
 
 .. _vPmicTask: https://git.card10.badge.events.ccc.de/card10/firmware/blob/master/epicardium/modules/pmic.c#L281
 .. _vLifecycleTask: https://git.card10.badge.events.ccc.de/card10/firmware/blob/master/epicardium/modules/lifecycle.c#L361
 .. _vBleTask: https://git.card10.badge.events.ccc.de/card10/firmware/blob/master/epicardium/ble/ble.c#L237
 .. _vSerialTask: https://git.card10.badge.events.ccc.de/card10/firmware/blob/master/epicardium/modules/serial.c#L289
 .. _vApiDispatcher: https://git.card10.badge.events.ccc.de/card10/firmware/blob/master/epicardium/modules/dispatcher.c#L25
+.. _vInterruptsTask: https://git.card10.badge.events.ccc.de/card10/firmware/blob/master/epicardium/modules/interrupts.c#L119
 .. _vLedTask: https://git.card10.badge.events.ccc.de/card10/firmware/blob/master/epicardium/modules/personal_state.c#L58
 .. _vMAX30001Task: https://git.card10.badge.events.ccc.de/card10/firmware/blob/master/epicardium/modules/max30001.c#L378
 .. _vBhi160Task: https://git.card10.badge.events.ccc.de/card10/firmware/blob/master/epicardium/modules/bhi.c#L419

epicardium/api/control.c

 #include "epicardium.h"
 #include "api/dispatcher.h"
-#include "api/interrupt-sender.h"
 #include "modules/log.h"
 
 #include "card10.h"
@@ -10,6 +9,7 @@
 #include "tmr.h"
 
 static void __core1_init(void);
+extern void interrupt_trigger_sync(api_int_id_t id);
 
 struct core1_info {
 	/* Location of core1's interrupt vector table */
@@ -206,8 +206,13 @@ void core1_boot(void)
 
 void core1_trigger_reset(void)
 {
-	/* Signal core 1 that we intend to load a new payload. */
-	api_interrupt_trigger(EPIC_INT_RESET);
+	/*
+	 * Signal core 1 that we intend to load a new payload.
+	 *
+	 * This needs to be synchroneous because otherwise we will deadlock
+	 * (Lifecycle task busy-spins and interrupt can never get dispatched).
+	 */
+	interrupt_trigger_sync(EPIC_INT_RESET);
 }
 
 void core1_wait_ready(void)

epicardium/api/interrupt-sender.c

 #include "api/interrupt-sender.h"
 #include "api/common.h"
 #include "tmr_utils.h"
-
-static bool int_enabled[EPIC_INT_NUM];
-
-int api_interrupt_trigger(api_int_id_t id)
-{
-	if (id >= EPIC_INT_NUM) {
-		return -EINVAL;
-	}
-
-	if (int_enabled[id]) {
-		while (API_CALL_MEM->int_id != (api_int_id_t)(-1))
-			;
-
-		API_CALL_MEM->int_id = id;
-		TMR_TO_Start(MXC_TMR5, 1, 0);
-	}
-	return 0;
-}
+#include <assert.h>
 
 void api_interrupt_init(void)
 {
 	API_CALL_MEM->int_id = (-1);
-
-	for (int i = 0; i < EPIC_INT_NUM; i++) {
-		int_enabled[i] = false;
 }
 
-	/* Reset interrupt is always enabled */
-	int_enabled[EPIC_INT_RESET] = true;
-}
-
-int epic_interrupt_enable(api_int_id_t int_id)
+bool api_interrupt_is_ready(void)
 {
-	if (int_id >= EPIC_INT_NUM) {
-		return -EINVAL;
+	return API_CALL_MEM->int_id == (api_int_id_t)(-1);
 }
 
-	int_enabled[int_id] = true;
-	return 0;
-}
-
-int epic_interrupt_disable(api_int_id_t int_id)
+void api_interrupt_trigger(api_int_id_t id)
 {
-	if (int_id >= EPIC_INT_NUM || int_id == EPIC_INT_RESET) {
-		return -EINVAL;
-	}
+	assert(API_CALL_MEM->int_id == (api_int_id_t)(-1));
 
-	int_enabled[int_id] = false;
-	return 0;
+	API_CALL_MEM->int_id = id;
+	TMR_TO_Start(MXC_TMR5, 1, 0);
 }

epicardium/api/interrupt-sender.h

@@ -2,4 +2,5 @@
 #include "api/common.h"
 
 void api_interrupt_init(void);
-int api_interrupt_trigger(api_int_id_t id);
+bool api_interrupt_is_ready(void);
+void api_interrupt_trigger(api_int_id_t id);

epicardium/epicardium.h

@@ -158,6 +158,10 @@ typedef uint32_t api_int_id_t;
  * (masked/unmasked) using :c:func:`epic_interrupt_enable` and
  * :c:func:`epic_interrupt_disable`.
  *
+ * These interrupts work similar to hardware interrupts:  You will only get a
+ * single interrupt, even if multiple events occured since the ISR last ran
+ * (*this behavior is new since version 1.16*).
+ *
  * .. warning::
  *
  *    Never attempt to call the API from inside an ISR.  This might trigger an

epicardium/main.c

@@ -133,6 +133,16 @@ int main(void)
 		    &dispatcher_task_id) != pdPASS) {
 		panic("Failed to create %s task!", "API Dispatcher");
 	}
+	/* Interrupts */
+	if (xTaskCreate(
+		    vInterruptsTask,
+		    (const char *)"Interrupt Dispatcher",
+		    configMINIMAL_STACK_SIZE,
+		    NULL,
+		    tskIDLE_PRIORITY + 2,
+		    NULL) != pdPASS) {
+		panic("Failed to create %s task!", "Interrupt Dispatcher");
+	}
 
 	/* BLE */
 	if (ble_shall_start()) {

epicardium/modules/bhi.c

@@ -11,7 +11,6 @@
 #include "task.h"
 #include "queue.h"
 
-#include "api/interrupt-sender.h"
 #include "epicardium.h"
 #include "modules/log.h"
 #include "modules/modules.h"
@@ -320,7 +319,7 @@ bhi160_handle_packet(bhy_data_type_t data_type, bhy_data_generic_t *sensor_data)
 		}
 
 		if (wakeup) {
-			api_interrupt_trigger(epic_int);
+			interrupt_trigger(epic_int);
 		}
 		break;
 	default:

epicardium/modules/hardware.c

 #include "epicardium.h"
 
 #include "api/dispatcher.h"
-#include "api/interrupt-sender.h"
 #include "usb/epc_usb.h"
 #include "modules/filesystem.h"
 #include "modules/log.h"
@@ -171,7 +170,7 @@ int hardware_early_init(void)
 	/*
 	 * API Dispatcher & API Interrupts
 	 */
-	api_interrupt_init();
+	interrupt_init();
 	api_dispatcher_init();
 
 	/*
@@ -237,7 +236,7 @@ int hardware_reset(void)
 	/*
 	 * API Dispatcher & API Interrupts
 	 */
-	api_interrupt_init();
+	interrupt_init();
 	api_dispatcher_init();
 
 	/*

epicardium/modules/interrupts.c

+#include "modules/mutex.h"
+#include "modules/log.h"
+#include "epicardium.h"
+#include "api/interrupt-sender.h"
+#include <assert.h>
+
+struct interrupt_priv {
+	/* Whether this interrupt can be triggered */
+	bool int_enabled[EPIC_INT_NUM];
+	/* Whether this interrupt is waiting to be delivered */
+	bool int_pending[EPIC_INT_NUM];
+	/* Whether any interrupts are currently waiting to be triggered */
+	bool has_pending;
+};
+
+static struct interrupt_priv interrupt_data;
+static struct mutex interrupt_mutex;
+static TaskHandle_t interrupts_task;
+
+void interrupt_trigger(api_int_id_t id)
+{
+	assert(id < EPIC_INT_NUM);
+
+	mutex_lock(&interrupt_mutex);
+
+	if (interrupt_data.int_enabled[id]) {
+		interrupt_data.int_pending[id] = true;
+		interrupt_data.has_pending     = true;
+		mutex_unlock(&interrupt_mutex);
+		xTaskNotifyGive(interrupts_task);
+	} else {
+		mutex_unlock(&interrupt_mutex);
+	}
+}
+
+void interrupt_trigger_sync(api_int_id_t id)
+{
+	assert(id < EPIC_INT_NUM);
+
+	mutex_lock(&interrupt_mutex);
+	if (!interrupt_data.int_enabled[id])
+		goto out;
+
+	while (!api_interrupt_is_ready())
+		;
+
+	api_interrupt_trigger(id);
+out:
+	mutex_unlock(&interrupt_mutex);
+}
+
+/*
+ * This function solely exists because of that one use of interrupts that breaks
+ * the rules:  The RTC ALARM interrupt is triggered from a hardware ISR where
+ * interrupt_trigger_sync() won't work because it needs to lock a mutex.
+ *
+ * DO NOT USE THIS FUNCTION IN ANY NEW CODE.
+ */
+void __attribute__((deprecated)) interrupt_trigger_unsafe(api_int_id_t id)
+{
+	assert(id < EPIC_INT_NUM);
+
+	if (!interrupt_data.int_enabled[id])
+		return;
+
+	while (!api_interrupt_is_ready())
+		;
+
+	api_interrupt_trigger(id);
+}
+
+static void interrupt_set_enabled(api_int_id_t id, bool enabled)
+{
+	assert(id < EPIC_INT_NUM);
+
+	mutex_lock(&interrupt_mutex);
+	interrupt_data.int_enabled[id] = enabled;
+	mutex_unlock(&interrupt_mutex);
+}
+
+void interrupt_init(void)
+{
+	if (interrupt_mutex.name == NULL)
+		mutex_create(&interrupt_mutex);
+
+	api_interrupt_init();
+
+	/* Reset all irqs to disabled */
+	for (size_t i = 0; i < EPIC_INT_NUM; i++) {
+		interrupt_set_enabled(i, false);
+	}
+
+	/* Reset interrupt is always enabled */
+	interrupt_set_enabled(EPIC_INT_RESET, true);
+}
+
+/* Epic-calls {{{ */
+int epic_interrupt_enable(api_int_id_t int_id)
+{
+	if (int_id >= EPIC_INT_NUM) {
+		return -EINVAL;
+	}
+
+	interrupt_set_enabled(int_id, true);
+	return 0;
+}
+
+int epic_interrupt_disable(api_int_id_t int_id)
+{
+	if (int_id >= EPIC_INT_NUM || int_id == EPIC_INT_RESET) {
+		return -EINVAL;
+	}
+
+	interrupt_set_enabled(int_id, false);
+	return 0;
+}
+/* }}} */
+
+void vInterruptsTask(void *pvParameters)
+{
+	interrupts_task = xTaskGetCurrentTaskHandle();
+
+	while (true) {
+		mutex_lock(&interrupt_mutex);
+
+		if (!interrupt_data.has_pending) {
+			/* Wait for a wakeup event from interrupt_trigger() */
+			mutex_unlock(&interrupt_mutex);
+			ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
+			mutex_lock(&interrupt_mutex);
+		}
+
+		while (!api_interrupt_is_ready()) {
+			mutex_unlock(&interrupt_mutex);
+			vTaskDelay(pdMS_TO_TICKS(5));
+			mutex_lock(&interrupt_mutex);
+		}
+
+		api_int_id_t current_irq = EPIC_INT_NUM;
+		for (size_t i = 0; i < EPIC_INT_NUM; i++) {
+			if (interrupt_data.int_pending[i]) {
+				current_irq                   = i;
+				interrupt_data.int_pending[i] = false;
+				break;
+			}
+		}
+
+		if (current_irq == EPIC_INT_NUM) {
+			interrupt_data.has_pending = false;
+		} else if (interrupt_data.int_enabled[current_irq]) {
+			api_interrupt_trigger(current_irq);
+		}
+
+		mutex_unlock(&interrupt_mutex);
+	}
+}

epicardium/modules/lifecycle.c

@@ -4,7 +4,6 @@
 #include "modules/config.h"
 #include "modules/mutex.h"
 #include "api/dispatcher.h"
-#include "api/interrupt-sender.h"
 #include "l0der/l0der.h"
 
 #include "card10.h"
@@ -355,9 +354,17 @@ void vLifecycleTask(void *pvParameters)
 
 	mutex_unlock(&core1_mutex);
 
-	/* If `main.py` exists, start it.  Otherwise, start `menu.py`. */
-	if (epic_exec("main.py") < 0) {
-		return_to_menu();
+	/*
+	 * If `main.py` exists, start it.  Otherwise, start `menu.py`.
+	 *
+	 * We are not using epic_exec() & return_to_menu() here because those
+	 * trigger a reset which is undesirable during startup.
+	 */
+	mutex_lock(&core1_mutex);
+	int ret = load_sync("main.py", false);
+	mutex_unlock(&core1_mutex);
+	if (ret < 0) {
+		load_menu(false);
 	}
 
 	hardware_init();

epicardium/modules/max30001.c

@@ -11,7 +11,6 @@
 #include "task.h"
 #include "queue.h"
 
-#include "api/interrupt-sender.h"
 #include "epicardium.h"
 #include "modules/log.h"
 #include "modules/modules.h"
@@ -141,7 +140,7 @@ static void max30001_handle_samples(int16_t *sensor_data, int16_t n)
 			LOG_WARN("max30001", "queue full");
 		}
 	}
-	api_interrupt_trigger(EPIC_INT_MAX30001_ECG);
+	interrupt_trigger(EPIC_INT_MAX30001_ECG);
 }
 
 /***** Functions *****/

epicardium/modules/max86150.c

@@ -13,7 +13,6 @@
 #include "task.h"
 #include "queue.h"
 
-#include "api/interrupt-sender.h"
 #include "modules/modules.h"
 
 static const gpio_cfg_t max86150_interrupt_pin = {
@@ -140,7 +139,10 @@ static int max86150_handle_sample(struct max86150_sensor_data *data)
 		LOG_WARN("max86150", "queue full");
 		return -EIO;
 	}
-	return api_interrupt_trigger(EPIC_INT_MAX86150);
+
+	interrupt_trigger(EPIC_INT_MAX86150);
+
+	return 0;
 }
 
 static int max86150_fetch_fifo(void)

epicardium/modules/meson.build

@@ -9,6 +9,7 @@ module_sources = files(
   'gpio.c',
   'hardware.c',
   'hw-lock.c',
+  'interrupts.c',
   'leds.c',
   'lifecycle.c',
   'light_sensor.c',

epicardium/modules/modules.h

@@ -4,6 +4,7 @@
 #include "FreeRTOS.h"
 #include "gpio.h"
 #include "modules/mutex.h"
+#include "epicardium.h"
 
 #include <stdint.h>
 #include <stdbool.h>
@@ -27,6 +28,15 @@ int hardware_reset(void);
 void vLifecycleTask(void *pvParameters);
 void return_to_menu(void);
 
+/* ---------- Interrupts --------------------------------------------------- */
+void interrupt_init(void);
+void interrupt_trigger(api_int_id_t id);
+void interrupt_trigger_sync(api_int_id_t id);
+void interrupt_trigger_unsafe(api_int_id_t id) __attribute__((deprecated(
+	"interrupt_trigger_unsafe() is racy and only exists for legacy code."
+)));
+void vInterruptsTask(void *pvParameters);
+
 /* ---------- Serial ------------------------------------------------------- */
 #define SERIAL_READ_BUFFER_SIZE 128
 #define SERIAL_WRITE_STREAM_BUFFER_SIZE 512

epicardium/modules/rtc.c

 #include "epicardium.h"
 #include "modules/log.h"
-#include "api/interrupt-sender.h"
+#include "modules/modules.h"
 
 #include "FreeRTOS.h"
 #include "task.h"
@@ -84,19 +84,23 @@ void epic_rtc_set_milliseconds(uint64_t milliseconds)
 	monotonic_offset += diff;
 }
 
+/* We need to use interrupt_trigger_unsafe() here */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 void RTC_IRQHandler(void)
 {
 	int flags = RTC_GetFlags();
 
 	if (flags & MXC_F_RTC_CTRL_ALDF) {
 		RTC_ClearFlags(MXC_F_RTC_CTRL_ALDF);
-		api_interrupt_trigger(EPIC_INT_RTC_ALARM);
+		interrupt_trigger_unsafe(EPIC_INT_RTC_ALARM);
 	} else {
 		LOG_WARN("rtc", "Unknown IRQ caught!");
 		/* Disable IRQ so it does not retrigger */
 		NVIC_DisableIRQ(RTC_IRQn);
 	}
 }
+#pragma GCC diagnostic pop
 
 int epic_rtc_schedule_alarm(uint32_t timestamp)
 {
@@ -107,7 +111,7 @@ int epic_rtc_schedule_alarm(uint32_t timestamp)
 	 * immediately.
 	 */
 	if (epic_rtc_get_seconds() >= timestamp) {
-		api_interrupt_trigger(EPIC_INT_RTC_ALARM);
+		interrupt_trigger(EPIC_INT_RTC_ALARM);
 		return 0;
 	}
 

epicardium/modules/serial.c

 #include "epicardium.h"
-#include "api/interrupt-sender.h"
 #include "modules/log.h"
 #include "modules/modules.h"
 
@@ -280,7 +279,7 @@ void serial_enqueue_char(char chr)
 {
 	if (chr == 0x3) {
 		/* Control-C */
-		api_interrupt_trigger(EPIC_INT_CTRL_C);
+		interrupt_trigger(EPIC_INT_CTRL_C);
 	}
 
 	if (xQueueSend(read_queue, &chr, 100) == errQUEUE_FULL) {
@@ -288,7 +287,7 @@ void serial_enqueue_char(char chr)
 		vTaskDelay(portTICK_PERIOD_MS * 50);
 	}
 
-	api_interrupt_trigger(EPIC_INT_UART_RX);
+	interrupt_trigger(EPIC_INT_UART_RX);
 }
 
 void vSerialTask(void *pvParameters)