From 60442a221a58f99ef6727f6dc8bbad3f8c29362f Mon Sep 17 00:00:00 2001 From: Jordan Yates Date: Fri, 4 Oct 2024 13:44:24 +1000 Subject: [PATCH] modules: segger: remove mutex locking Remove mutex locking in favour of the standard IRQ locking mechanism. The primary problem with the mutex implementation is that mutex locking is forbidden in ISR's. This means that any logging from an interrupt context (e.g. LOG_PANIC in an exception handler), will itself trigger another assertion due its attempt to use a mutex. Furthermore, mutexes are a relatively heavyweight locking scheme, which doesn't necessarily make sense in the context of extremely short locking periods that would be expected from RTT. This change aligns Zephyr with the default RTT locking scheme, which uses interrupt masking to perform access control. Resolves #79403. Signed-off-by: Jordan Yates --- drivers/serial/uart_rtt.c | 17 +---------------- modules/segger/Kconfig | 2 +- modules/segger/SEGGER_RTT_zephyr.c | 30 ------------------------------ west.yml | 2 +- 4 files changed, 3 insertions(+), 48 deletions(-) diff --git a/drivers/serial/uart_rtt.c b/drivers/serial/uart_rtt.c index 59044d87e60cb9..7ed234329b80d1 100644 --- a/drivers/serial/uart_rtt.c +++ b/drivers/serial/uart_rtt.c @@ -10,8 +10,6 @@ #define DT_DRV_COMPAT segger_rtt_uart -extern struct k_mutex rtt_term_mutex; - struct uart_rtt_config { void *up_buffer; size_t up_size; @@ -100,21 +98,8 @@ static int uart_rtt_tx(const struct device *dev, ARG_UNUSED(timeout); - /* RTT mutex cannot be claimed in ISRs */ - if (k_is_in_isr()) { - return -ENOTSUP; - } - - /* Claim the RTT lock */ - if (k_mutex_lock(&rtt_term_mutex, K_NO_WAIT) != 0) { - return -EBUSY; - } - /* Output the buffer */ - SEGGER_RTT_WriteNoLock(ch, buf, len); - - /* Return RTT lock */ - SEGGER_RTT_UNLOCK(); + SEGGER_RTT_Write(ch, buf, len); /* Send the TX complete callback */ if (data->callback) { diff --git a/modules/segger/Kconfig b/modules/segger/Kconfig index fd04ac99b0f82c..597b4ab9f32ce5 100644 --- a/modules/segger/Kconfig +++ b/modules/segger/Kconfig @@ -23,7 +23,7 @@ if USE_SEGGER_RTT config SEGGER_RTT_CUSTOM_LOCKING bool "Custom locking" help - Enable custom locking using a mutex. + Enable custom locking using Zephyr APIs. config SEGGER_RTT_MAX_NUM_UP_BUFFERS int "Maximum number of up-buffers" diff --git a/modules/segger/SEGGER_RTT_zephyr.c b/modules/segger/SEGGER_RTT_zephyr.c index b7b8e83e4abcb1..a673f15d3d69e2 100644 --- a/modules/segger/SEGGER_RTT_zephyr.c +++ b/modules/segger/SEGGER_RTT_zephyr.c @@ -9,41 +9,13 @@ #include #include "SEGGER_RTT.h" -/* - * Common mutex for locking access to terminal buffer. - * Note that SEGGER uses same lock macros for both SEGGER_RTT_Write and - * SEGGER_RTT_Read functions. Because of this we are not able generally - * separate up and down access using two mutexes until SEGGER library fix - * this. - * - * If sharing access cause performance problems, consider using another - * non terminal buffers. - */ - -K_MUTEX_DEFINE(rtt_term_mutex); - static int rtt_init(void) { - SEGGER_RTT_Init(); return 0; } -#ifdef CONFIG_MULTITHREADING - -void zephyr_rtt_mutex_lock(void) -{ - k_mutex_lock(&rtt_term_mutex, K_FOREVER); -} - -void zephyr_rtt_mutex_unlock(void) -{ - k_mutex_unlock(&rtt_term_mutex); -} - -#endif /* CONFIG_MULTITHREADING */ - unsigned int zephyr_rtt_irq_lock(void) { return irq_lock(); @@ -54,6 +26,4 @@ void zephyr_rtt_irq_unlock(unsigned int key) irq_unlock(key); } - - SYS_INIT(rtt_init, PRE_KERNEL_1, CONFIG_KERNEL_INIT_PRIORITY_OBJECTS); diff --git a/west.yml b/west.yml index b61f2b97afb79c..ba68f6d18ce002 100644 --- a/west.yml +++ b/west.yml @@ -317,7 +317,7 @@ manifest: path: modules/lib/picolibc revision: d492d5fa7c96918e37653f303028346bb0dd51a2 - name: segger - revision: b011c45b585e097d95d9cf93edf4f2e01588d3cd + revision: 798f95ea9304e5ed8165a661081443051f210733 path: modules/debug/segger groups: - debug