diff --git a/epicardium/modules/bhi.c b/epicardium/modules/bhi.c index 4e38306e4e6616f36b48f1ad6f411ad3d3377cff..1332c12ea0c3164b56087d45c4099fa21e894ec4 100644 --- a/epicardium/modules/bhi.c +++ b/epicardium/modules/bhi.c @@ -135,7 +135,7 @@ int epic_bhi160_enable_sensor( return -ENODEV; } - result = hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)); + result = hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY); if (result < 0) { return result; } @@ -192,7 +192,7 @@ int epic_bhi160_disable_sensor(enum bhi160_sensor_type sensor_type) return -ENODEV; } - result = hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)); + result = hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY); if (result < 0) { return result; } @@ -347,7 +347,7 @@ static int bhi160_fetch_fifo(void) /* Number of bytes left in BHI160's FIFO buffer */ uint16_t bytes_left_in_fifo = 1; - result = hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)); + result = hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY); if (result < 0) { return result; } @@ -433,7 +433,7 @@ void vBhi160Task(void *pvParameters) */ vTaskDelay(pdMS_TO_TICKS(3)); - int lockret = hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)); + int lockret = hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY); if (lockret < 0) { LOG_CRIT("bhi160", "Failed to acquire I2C lock!"); vTaskDelay(portMAX_DELAY); @@ -469,7 +469,7 @@ void vBhi160Task(void *pvParameters) /* Wait for first interrupt */ hwlock_release(HWLOCK_I2C); ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(100)); - lockret = hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)); + lockret = hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY); if (lockret < 0) { LOG_CRIT("bhi160", "Failed to acquire I2C lock!"); vTaskDelay(portMAX_DELAY); diff --git a/epicardium/modules/bme680.c b/epicardium/modules/bme680.c index d70f9938e86f8b06371edb5ae14dd5b05aa050bd..caadf68b9fa5ba9f2a0d5395db610a56c1237410 100644 --- a/epicardium/modules/bme680.c +++ b/epicardium/modules/bme680.c @@ -43,7 +43,7 @@ int epic_bme680_init() return 0; } - if (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)) < 0) { + if (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { return -EBUSY; } @@ -110,7 +110,7 @@ int epic_bme680_deinit() return 0; } - if (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)) < 0) { + if (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { return -EBUSY; } @@ -133,7 +133,7 @@ int epic_bme680_read_sensors(struct bme680_sensor_data *data) return -EINVAL; } - if (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)) < 0) { + if (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { return -EBUSY; } @@ -152,7 +152,7 @@ int epic_bme680_read_sensors(struct bme680_sensor_data *data) */ hwlock_release(HWLOCK_I2C); vTaskDelay(pdMS_TO_TICKS(profile_dur)); - if (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)) < 0) { + if (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { return -EBUSY; } diff --git a/epicardium/modules/buttons.c b/epicardium/modules/buttons.c index 14e9e613719e3e5279f811393afdfaf843025430..6afbdf9c1513f7e5014d132b9d1eaefebf04da09 100644 --- a/epicardium/modules/buttons.c +++ b/epicardium/modules/buttons.c @@ -17,7 +17,7 @@ uint8_t epic_buttons_read(uint8_t mask) { uint8_t ret = 0; if (portexpander_detected() && (mask & 0x7)) { - if (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(100)) < 0) { + if (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { LOG_ERR("buttons", "Can't acquire I2C bus"); return 0; } diff --git a/epicardium/modules/gpio.c b/epicardium/modules/gpio.c index 4af356ee843b1268dec103a301b7bf9a766a3115..872aaee7a1d31b9f3c341ea2c0b67e3323b8a4ce 100644 --- a/epicardium/modules/gpio.c +++ b/epicardium/modules/gpio.c @@ -137,7 +137,7 @@ int epic_gpio_read_pin(uint8_t pin) } else if (cfg->func == GPIO_FUNC_IN) { return GPIO_InGet(cfg) != 0; } else if (cfg->func == GPIO_FUNC_ALT1) { - int rc = hwlock_acquire(HWLOCK_ADC, pdMS_TO_TICKS(10)); + int rc = hwlock_acquire_timeout(HWLOCK_ADC, portMAX_DELAY); if (!rc) { ADC_StartConvert(s_adc_channels[pin], 0, 0); uint16_t value; diff --git a/epicardium/modules/hw-lock.c b/epicardium/modules/hw-lock.c index ce88d921748302edd849eea0b923e6c61eab8dec..47929fe06cf80c263168be59de1265aa5c87daec 100644 --- a/epicardium/modules/hw-lock.c +++ b/epicardium/modules/hw-lock.c @@ -1,79 +1,78 @@ #include "modules/log.h" #include "modules/modules.h" +#include "modules/mutex.h" #include "FreeRTOS.h" #include "task.h" -#include "semphr.h" #include <errno.h> -static StaticSemaphore_t hwlock_mutex_data[_HWLOCK_MAX]; -static SemaphoreHandle_t hwlock_mutex[_HWLOCK_MAX]; -/* Which task is holding the lock */ -static TaskHandle_t hwlock_tasks[_HWLOCK_MAX]; +static struct mutex hwlock_mutex[_HWLOCK_MAX] = { { 0 } }; void hwlock_init(void) { for (int i = 0; i < _HWLOCK_MAX; i++) { - hwlock_mutex[i] = - xSemaphoreCreateMutexStatic(&hwlock_mutex_data[i]); + /* + * TODO: mutex_create() names these all these mutexes + * "&hwlock_mutex[i]" which is not helpful at all. We + * should somehow rename them to the actual hwlock names. + */ + mutex_create(&hwlock_mutex[i]); } } -int hwlock_acquire(enum hwlock_periph p, TickType_t wait) +void hwlock_acquire(enum hwlock_periph p) { - if (p >= _HWLOCK_MAX) { - return -EINVAL; + assert(p < _HWLOCK_MAX); + mutex_lock(&hwlock_mutex[p]); +} + +int hwlock_acquire_nonblock(enum hwlock_periph p) +{ + assert(p < _HWLOCK_MAX); + if (mutex_trylock(&hwlock_mutex[p])) { + return 0; + } else { + return -EBUSY; } - TaskHandle_t task = xTaskGetCurrentTaskHandle(); +} - if (xSemaphoreTake(hwlock_mutex[p], wait) != pdTRUE) { - /* Don't print warnings for 0 wait acquires */ - if (wait == 0) { - return -EBUSY; - } +int hwlock_acquire_timeout(enum hwlock_periph p, TickType_t wait) +{ + assert(p < _HWLOCK_MAX); + /* + * Check for code still defining a timeout. It will be ignored in the + * new implementation but a warning is emitted so someone hopefully + * eventually fixes the offending code ... + * + * At some point, the signature of this function should be changed. + * Alternatively it could be split into two, similar to the mutex API. + */ + if (wait != 0 && wait != portMAX_DELAY) { LOG_WARN( "hwlock", - "Lock %u is busy! Held by \"%s\" and attempted to accquire by \"%s\"", + "Attempting to lock %d with a timeout (from %p).", p, - pcTaskGetName(hwlock_tasks[p]), - pcTaskGetName(task) - ); - LOG_DEBUG( - "hwlock", - "...attempted to lock from pc %p", __builtin_return_address(0) ); - return -EBUSY; } - hwlock_tasks[p] = task; + /* Non-blocking lock attempt */ + if (wait == 0) { + if (mutex_trylock(&hwlock_mutex[p]) == true) { + return 0; + } else { + return -EBUSY; + } + } + mutex_lock(&hwlock_mutex[p]); return 0; } -int hwlock_release(enum hwlock_periph p) +void hwlock_release(enum hwlock_periph p) { - int ret = 0; - - if (p >= _HWLOCK_MAX) { - return -EINVAL; - } - - if (hwlock_tasks[p] != xTaskGetCurrentTaskHandle()) { - LOG_ERR("hwlock", - "Lock %u is released by task \"%s\" while it was acquired by \"%s\"", - p, - pcTaskGetName(NULL), - pcTaskGetName(hwlock_tasks[p])); - ret = -EACCES; - } - - if (xSemaphoreGive(hwlock_mutex[p]) != pdTRUE) { - LOG_ERR("hwlock", "Lock %u not released correctly.", p); - ret = -EINVAL; - } - - return ret; + assert(p < _HWLOCK_MAX); + mutex_unlock(&hwlock_mutex[p]); } diff --git a/epicardium/modules/leds.c b/epicardium/modules/leds.c index ad08487d10d02b61e192cf6414aa4126d02f264f..3c474ebfb6081f30aae1a7724abda44d9aa3aa67 100644 --- a/epicardium/modules/leds.c +++ b/epicardium/modules/leds.c @@ -13,7 +13,7 @@ static void do_update() { - while (hwlock_acquire(HWLOCK_LED, pdMS_TO_TICKS(1)) < 0) { + while (hwlock_acquire_timeout(HWLOCK_LED, portMAX_DELAY) < 0) { vTaskDelay(pdMS_TO_TICKS(1)); } @@ -96,7 +96,7 @@ void epic_leds_dim_top(uint8_t value) { leds_set_dim_top(value); if (personal_state_enabled() == 0) { - while (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(1)) < 0) { + while (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { vTaskDelay(pdMS_TO_TICKS(1)); } @@ -110,7 +110,7 @@ void epic_leds_dim_bottom(uint8_t value) { leds_set_dim_bottom(value); if (personal_state_enabled() == 0) { - while (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(1)) < 0) { + while (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { vTaskDelay(pdMS_TO_TICKS(1)); } @@ -122,7 +122,7 @@ void epic_leds_dim_bottom(uint8_t value) void epic_leds_set_rocket(int led, uint8_t value) { - while (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(1)) < 0) { + while (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { vTaskDelay(pdMS_TO_TICKS(1)); } @@ -135,7 +135,7 @@ void epic_leds_set_rocket(int led, uint8_t value) int epic_leds_get_rocket(int led) { int ret = 0; - while (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(1)) < 0) { + while (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { vTaskDelay(pdMS_TO_TICKS(1)); } @@ -146,7 +146,7 @@ int epic_leds_get_rocket(int led) void epic_set_flashlight(bool power) { - while (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(1)) < 0) { + while (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { vTaskDelay(pdMS_TO_TICKS(1)); } @@ -162,7 +162,7 @@ void epic_leds_update(void) void epic_leds_set_powersave(bool eco) { - while (hwlock_acquire(HWLOCK_I2C, pdMS_TO_TICKS(1)) < 0) { + while (hwlock_acquire_timeout(HWLOCK_I2C, portMAX_DELAY) < 0) { vTaskDelay(pdMS_TO_TICKS(1)); } diff --git a/epicardium/modules/light_sensor.c b/epicardium/modules/light_sensor.c index de20b6e2b21c20e269f8fb19b8dde76b6a13ffdb..a0f997ac1d88e1b1e60822ea1309f06879b3ee65 100644 --- a/epicardium/modules/light_sensor.c +++ b/epicardium/modules/light_sensor.c @@ -29,7 +29,7 @@ static int light_sensor_init() uint16_t epic_light_sensor_read() { - if (hwlock_acquire(HWLOCK_ADC, pdMS_TO_TICKS(1000)) != 0) { + if (hwlock_acquire_timeout(HWLOCK_ADC, portMAX_DELAY) != 0) { return 0; } @@ -42,7 +42,7 @@ uint16_t epic_light_sensor_read() static void readAdcCallback() { - if (hwlock_acquire(HWLOCK_ADC, 0) != 0) { + if (hwlock_acquire_timeout(HWLOCK_ADC, 0) != 0) { /* Can't do much about this here ... Retry next time */ return; } @@ -57,7 +57,7 @@ int epic_light_sensor_run() { int ret = 0; - if (hwlock_acquire(HWLOCK_ADC, pdMS_TO_TICKS(500)) != 0) { + if (hwlock_acquire_timeout(HWLOCK_ADC, portMAX_DELAY) != 0) { return -EBUSY; } diff --git a/epicardium/modules/max30001.c b/epicardium/modules/max30001.c index fd78a77bd14d578ef125a847c781d84684047f09..5c2ff10f8965e7a9e28c30ab51bbc00fd0af0c45 100644 --- a/epicardium/modules/max30001.c +++ b/epicardium/modules/max30001.c @@ -9,7 +9,6 @@ #include "FreeRTOS.h" #include "task.h" -#include "semphr.h" #include "queue.h" #include "api/interrupt-sender.h" @@ -17,9 +16,7 @@ #include "modules/log.h" #include "modules/modules.h" #include "modules/stream.h" - -/* Ticks to wait when trying to acquire lock */ -#define LOCK_WAIT pdMS_TO_TICKS(MAX30001_MUTEX_WAIT_MS) +#include "modules/mutex.h" /* Interrupt Pin */ static const gpio_cfg_t max30001_interrupt_pin = { @@ -36,8 +33,7 @@ static const gpio_cfg_t analog_switch = { static TaskHandle_t max30001_task_id = NULL; /* MAX30001 Mutex */ -static StaticSemaphore_t max30001_mutex_data; -static SemaphoreHandle_t max30001_mutex = NULL; +static struct mutex max30001_mutex = { 0 }; /* Stream */ static struct stream_info max30001_stream; @@ -54,15 +50,8 @@ int epic_max30001_enable_sensor(struct max30001_sensor_config *config) { int result = 0; - result = hwlock_acquire(HWLOCK_SPI_ECG, pdMS_TO_TICKS(100)); - if (result < 0) { - return result; - } - - if (xSemaphoreTake(max30001_mutex, LOCK_WAIT) != pdTRUE) { - result = -EBUSY; - goto out_free_spi; - } + mutex_lock(&max30001_mutex); + hwlock_acquire(HWLOCK_SPI_ECG); struct stream_info *stream = &max30001_stream; ; @@ -97,9 +86,8 @@ int epic_max30001_enable_sensor(struct max30001_sensor_config *config) result = SD_MAX30001_ECG; out_free_both: - xSemaphoreGive(max30001_mutex); -out_free_spi: hwlock_release(HWLOCK_SPI_ECG); + mutex_unlock(&max30001_mutex); return result; } @@ -107,15 +95,8 @@ int epic_max30001_disable_sensor(void) { int result = 0; - result = hwlock_acquire(HWLOCK_SPI_ECG, pdMS_TO_TICKS(100)); - if (result < 0) { - return result; - } - - if (xSemaphoreTake(max30001_mutex, LOCK_WAIT) != pdTRUE) { - result = -EBUSY; - goto out_free_spi; - } + mutex_lock(&max30001_mutex); + hwlock_acquire(HWLOCK_SPI_ECG); struct stream_info *stream = &max30001_stream; result = stream_deregister(SD_MAX30001_ECG, stream); @@ -134,9 +115,8 @@ int epic_max30001_disable_sensor(void) result = 0; out_free_both: - xSemaphoreGive(max30001_mutex); -out_free_spi: hwlock_release(HWLOCK_SPI_ECG); + mutex_unlock(&max30001_mutex); return result; } @@ -298,15 +278,8 @@ static int max30001_fetch_fifo(void) { int result = 0; - result = hwlock_acquire(HWLOCK_SPI_ECG, pdMS_TO_TICKS(100)); - if (result < 0) { - return result; - } - - if (xSemaphoreTake(max30001_mutex, LOCK_WAIT) != pdTRUE) { - result = -EBUSY; - goto out_free_spi; - } + mutex_lock(&max30001_mutex); + hwlock_acquire(HWLOCK_SPI_ECG); uint32_t ecgFIFO, readECGSamples, ETAG[32], status; int16_t ecgSample[32]; @@ -344,9 +317,8 @@ static int max30001_fetch_fifo(void) max30001_handle_samples(ecgSample, readECGSamples); } - xSemaphoreGive(max30001_mutex); -out_free_spi: hwlock_release(HWLOCK_SPI_ECG); + mutex_unlock(&max30001_mutex); return result; } @@ -369,24 +341,15 @@ static void max300001_interrupt_callback(void *_) void max30001_mutex_init(void) { - max30001_mutex = xSemaphoreCreateMutexStatic(&max30001_mutex_data); + mutex_create(&max30001_mutex); } void vMAX30001Task(void *pvParameters) { max30001_task_id = xTaskGetCurrentTaskHandle(); - int lockret = hwlock_acquire(HWLOCK_SPI_ECG, pdMS_TO_TICKS(100)); - if (lockret < 0) { - LOG_CRIT("max30001", "Failed to acquire SPI lock!"); - vTaskDelay(portMAX_DELAY); - } - - /* Take Mutex during initialization, just in case */ - if (xSemaphoreTake(max30001_mutex, 0) != pdTRUE) { - LOG_CRIT("max30001", "Failed to acquire MAX30001 mutex!"); - vTaskDelay(portMAX_DELAY); - } + mutex_lock(&max30001_mutex); + hwlock_acquire(HWLOCK_SPI_ECG); /* Install interrupt callback */ GPIO_Config(&max30001_interrupt_pin); @@ -407,20 +370,15 @@ void vMAX30001Task(void *pvParameters) GPIO_Config(&analog_switch); GPIO_OutClr(&analog_switch); // Wrist - xSemaphoreGive(max30001_mutex); hwlock_release(HWLOCK_SPI_ECG); + mutex_unlock(&max30001_mutex); /* ----------------------------------------- */ while (1) { if (max30001_sensor_active) { int ret = max30001_fetch_fifo(); - if (ret == -EBUSY) { - LOG_WARN( - "max30001", "Could not acquire mutex?" - ); - continue; - } else if (ret < 0) { + if (ret < 0) { LOG_ERR("max30001", "Unknown error: %d", -ret); } } diff --git a/epicardium/modules/modules.h b/epicardium/modules/modules.h index 745ecec230be10e8a97ebd0b181b72eaa8e2fc28..567a2a82bb98341451c5ea3c5b847a05e63b1951 100644 --- a/epicardium/modules/modules.h +++ b/epicardium/modules/modules.h @@ -97,8 +97,10 @@ enum hwlock_periph { _HWLOCK_MAX, }; -int hwlock_acquire(enum hwlock_periph p, TickType_t wait); -int hwlock_release(enum hwlock_periph p); +int hwlock_acquire_timeout(enum hwlock_periph p, TickType_t wait); +void hwlock_acquire(enum hwlock_periph p); +int hwlock_acquire_nonblock(enum hwlock_periph p); +void hwlock_release(enum hwlock_periph p); /* ---------- Display ------------------------------------------------------ */ /* Forces an unlock of the display. Only to be used in Epicardium */ @@ -110,12 +112,10 @@ void disp_forcelock(); void vBhi160Task(void *pvParameters); /* ---------- MAX30001 ----------------------------------------------------- */ -#define MAX30001_MUTEX_WAIT_MS 50 void vMAX30001Task(void *pvParameters); void max30001_mutex_init(void); /* ---------- GPIO --------------------------------------------------------- */ -#define MAX30001_MUTEX_WAIT_MS 50 extern gpio_cfg_t gpio_configs[]; diff --git a/epicardium/modules/personal_state.c b/epicardium/modules/personal_state.c index e1990f2b8d80fee987676af3b937e35d0faca51f..08028b58b8c31acf98a3ae269018579ad43985f5 100644 --- a/epicardium/modules/personal_state.c +++ b/epicardium/modules/personal_state.c @@ -31,7 +31,7 @@ int epic_personal_state_set(uint8_t state, bool persistent) personal_state_persistent = persistent; if (was_enabled && !_personal_state_enabled) { - while (hwlock_acquire(HWLOCK_LED, pdMS_TO_TICKS(1)) < 0) { + while (hwlock_acquire_timeout(HWLOCK_LED, portMAX_DELAY) < 0) { vTaskDelay(pdMS_TO_TICKS(1)); } @@ -60,8 +60,8 @@ void vLedTask(void *pvParameters) const int led_animation_rate = 1000 / 25; /* 25Hz -> 40ms*/ while (1) { if (_personal_state_enabled) { - while (hwlock_acquire(HWLOCK_LED, pdMS_TO_TICKS(1)) < - 0) { + while (hwlock_acquire_timeout( + HWLOCK_LED, portMAX_DELAY) < 0) { vTaskDelay(pdMS_TO_TICKS(1)); } diff --git a/epicardium/modules/pmic.c b/epicardium/modules/pmic.c index 748b1d154e6161ffb4f55516d982d14e49ac311a..f9deaff7c4d4ad142b362d90dfe1f1be2bbc319f 100644 --- a/epicardium/modules/pmic.c +++ b/epicardium/modules/pmic.c @@ -18,7 +18,7 @@ #include <stdio.h> #include <string.h> -#define LOCK_WAIT pdMS_TO_TICKS(1000) +#define LOCK_WAIT portMAX_DELAY /* Task ID for the pmic handler */ static TaskHandle_t pmic_task_id = NULL; @@ -53,12 +53,12 @@ int pmic_read_amux(enum pmic_amux_signal sig, float *result) return -EINVAL; } - int adc_ret = hwlock_acquire(HWLOCK_ADC, LOCK_WAIT); + int adc_ret = hwlock_acquire_timeout(HWLOCK_ADC, LOCK_WAIT); if (adc_ret < 0) { ret = adc_ret; goto done; } - i2c_ret = hwlock_acquire(HWLOCK_I2C, LOCK_WAIT); + i2c_ret = hwlock_acquire_timeout(HWLOCK_I2C, LOCK_WAIT); if (i2c_ret < 0) { ret = i2c_ret; goto done; @@ -75,7 +75,7 @@ int pmic_read_amux(enum pmic_amux_signal sig, float *result) hwlock_release(HWLOCK_I2C); vTaskDelay(pdMS_TO_TICKS(5)); - i2c_ret = hwlock_acquire(HWLOCK_I2C, LOCK_WAIT); + i2c_ret = hwlock_acquire_timeout(HWLOCK_I2C, LOCK_WAIT); if (i2c_ret < 0) { ret = i2c_ret; goto done; @@ -138,7 +138,7 @@ done: */ static uint8_t pmic_poll_interrupts(void) { - while (hwlock_acquire(HWLOCK_I2C, LOCK_WAIT) < 0) { + while (hwlock_acquire_timeout(HWLOCK_I2C, LOCK_WAIT) < 0) { LOG_WARN("pmic", "Failed to acquire I2C. Retrying ..."); vTaskDelay(pdMS_TO_TICKS(100)); }