From 2e00c2b35710d858814fb4162755e5bfea267fa1 Mon Sep 17 00:00:00 2001 From: Rahix <rahix@rahix.de> Date: Sun, 21 Jul 2019 14:49:42 +0200 Subject: [PATCH] feat(epicardium): Add a lock for i2c Multiple tasks might access the i2c bus simultaneously. To prevent race conditions, this patch introduces a lock which tasks should acquire before accessing the bus. Signed-off-by: Rahix <rahix@rahix.de> --- epicardium/main.c | 1 + epicardium/modules/bhi.c | 21 ++++++++++++++------- epicardium/modules/i2c.c | 33 +++++++++++++++++++++++++++++++++ epicardium/modules/meson.build | 1 + epicardium/modules/modules.h | 5 +++++ epicardium/modules/pmic.c | 17 +++++++++++++---- 6 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 epicardium/modules/i2c.c diff --git a/epicardium/main.c b/epicardium/main.c index e6ac3ea9..b7ab3419 100644 --- a/epicardium/main.c +++ b/epicardium/main.c @@ -60,6 +60,7 @@ int main(void) fatfs_init(); api_interrupt_init(); stream_init(); + i2c_init(); LOG_INFO("startup", "Initializing tasks ..."); diff --git a/epicardium/modules/bhi.c b/epicardium/modules/bhi.c index 5e05e672..79ce0a72 100644 --- a/epicardium/modules/bhi.c +++ b/epicardium/modules/bhi.c @@ -115,7 +115,8 @@ int epic_bhi160_enable_sensor( return -ENODEV; } - if (xSemaphoreTake(bhi160_mutex, LOCK_WAIT) == pdTRUE) { + if (i2c_lock() == 0 && + xSemaphoreTake(bhi160_mutex, LOCK_WAIT) == pdTRUE) { struct stream_info *stream = &bhi160_streams[sensor_type]; stream->item_size = bhi160_lookup_data_size(sensor_type); /* TODO: Sanity check length */ @@ -138,6 +139,7 @@ int epic_bhi160_enable_sensor( 0, config->dynamic_range /* dynamic range is sensor dependent */ ); + i2c_unlock(); xSemaphoreGive(bhi160_mutex); } else { return -EBUSY; @@ -153,13 +155,15 @@ int epic_bhi160_disable_sensor(enum bhi160_sensor_type sensor_type) return -ENODEV; } - if (xSemaphoreTake(bhi160_mutex, LOCK_WAIT) == pdTRUE) { + if (i2c_lock() == 0 && + xSemaphoreTake(bhi160_mutex, LOCK_WAIT) == pdTRUE) { struct stream_info *stream = &bhi160_streams[sensor_type]; stream_deregister(bhi160_lookup_sd(sensor_type), stream); vQueueDelete(stream->queue); stream->queue = NULL; bhy_disable_virtual_sensor(vs_id, VS_WAKEUP); + i2c_unlock(); xSemaphoreGive(bhi160_mutex); } else { return -EBUSY; @@ -233,7 +237,8 @@ static int bhi160_fetch_fifo(void) /* Number of bytes left in BHI160's FIFO buffer */ uint16_t bytes_left_in_fifo = 1; - if (xSemaphoreTake(bhi160_mutex, LOCK_WAIT) != pdTRUE) { + if (i2c_lock() < 0 || + xSemaphoreTake(bhi160_mutex, LOCK_WAIT) != pdTRUE) { return -EBUSY; } @@ -282,6 +287,7 @@ static int bhi160_fetch_fifo(void) } xSemaphoreGive(bhi160_mutex); + i2c_unlock(); return 0; } @@ -310,8 +316,8 @@ void vBhi160Task(void *pvParameters) bhi160_mutex = xSemaphoreCreateMutexStatic(&bhi160_mutex_data); /* Take Mutex during initialization, just in case */ - if (xSemaphoreTake(bhi160_mutex, 0) != pdTRUE) { - LOG_CRIT("bhi160", "Failed to acquire BHI160 mutex!"); + if (i2c_lock() < 0 || xSemaphoreTake(bhi160_mutex, 0) != pdTRUE) { + LOG_CRIT("bhi160", "Failed to acquire BHI160/i2c mutex!"); vTaskDelay(portMAX_DELAY); } @@ -336,8 +342,7 @@ void vBhi160Task(void *pvParameters) vTaskDelay(portMAX_DELAY); } - /* Wait for first two interrupts */ - ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(100)); + /* Wait for first interrupt */ ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(100)); /* Remap axes to match card10 layout */ @@ -356,6 +361,8 @@ void vBhi160Task(void *pvParameters) xSemaphoreGive(bhi160_mutex); + i2c_unlock(); + /* ----------------------------------------- */ while (1) { diff --git a/epicardium/modules/i2c.c b/epicardium/modules/i2c.c new file mode 100644 index 00000000..f7ba63e3 --- /dev/null +++ b/epicardium/modules/i2c.c @@ -0,0 +1,33 @@ +#include "modules/log.h" +#include "modules/modules.h" + +#include "FreeRTOS.h" +#include "task.h" +#include "semphr.h" + +#include <errno.h> + +static StaticSemaphore_t i2c_mutex_data; +static SemaphoreHandle_t i2c_mutex; + +int i2c_init(void) +{ + i2c_mutex = xSemaphoreCreateMutexStatic(&i2c_mutex_data); + return 0; +} + +int i2c_lock(void) +{ + if (xSemaphoreTake(i2c_mutex, pdMS_TO_TICKS(100)) != pdTRUE) { + LOG_WARN("i2c", "Mutex is busy"); + return -EBUSY; + } + return 0; +} + +void i2c_unlock(void) +{ + if (xSemaphoreGive(i2c_mutex) != pdTRUE) { + LOG_ERR("i2c", "Mutex was not acquired correctly"); + } +} diff --git a/epicardium/modules/meson.build b/epicardium/modules/meson.build index e59aeaee..0732576c 100644 --- a/epicardium/modules/meson.build +++ b/epicardium/modules/meson.build @@ -3,6 +3,7 @@ module_sources = files( 'display.c', 'fatfs.c', 'fatfs_fileops.c', + 'i2c.c', 'leds.c', 'log.c', 'pmic.c', diff --git a/epicardium/modules/modules.h b/epicardium/modules/modules.h index f09caa39..d1e92dcd 100644 --- a/epicardium/modules/modules.h +++ b/epicardium/modules/modules.h @@ -26,6 +26,11 @@ void vPmicTask(void *pvParameters); #define BHI160_MUTEX_WAIT_MS 50 void vBhi160Task(void *pvParameters); +/* ---------- I2C ---------------------------------------------------------- */ +int i2c_init(void); +int i2c_lock(void); +void i2c_unlock(void); + // Forces an unlock of the display. Only to be used in epicardium void disp_forcelock(); #endif /* MODULES_H */ diff --git a/epicardium/modules/pmic.c b/epicardium/modules/pmic.c index a41b38aa..64f6526f 100644 --- a/epicardium/modules/pmic.c +++ b/epicardium/modules/pmic.c @@ -27,9 +27,9 @@ void pmic_interrupt_callback(void *_) void vPmicTask(void *pvParameters) { - int count = 0; + int count = 0; portTickType delay = portMAX_DELAY; - pmic_task_id = xTaskGetCurrentTaskHandle(); + pmic_task_id = xTaskGetCurrentTaskHandle(); while (1) { ulTaskNotifyTake(pdTRUE, delay); @@ -43,7 +43,13 @@ void vPmicTask(void *pvParameters) MAX77650_setSFT_RST(0x2); } + while (i2c_lock() < 0) { + LOG_WARN("pmic", "Failed to acquire i2c. Retrying."); + vTaskDelay(pdMS_TO_TICKS(500)); + } + uint8_t int_flag = MAX77650_getINT_GLBL(); + i2c_unlock(); if (int_flag & MAX77650_INT_nEN_F) { /* Button was pressed */ @@ -62,8 +68,11 @@ void vPmicTask(void *pvParameters) /* TODO: Remove when all interrupts are handled */ if (int_flag & ~(MAX77650_INT_nEN_F | MAX77650_INT_nEN_R)) { - LOG_WARN("pmic", "Unhandled PMIC Interrupt: %x", - int_flag); + LOG_WARN( + "pmic", + "Unhandled PMIC Interrupt: %x", + int_flag + ); } if (delay != portMAX_DELAY) { -- GitLab