From c0b71385fc1b29923533146d398a45a0a1599126 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Wed, 3 Sep 2025 21:03:18 +0300 Subject: [PATCH] rtos: remove remains of old XTOS spinlock.h implementation Remove old XTOS spinlock code, including the legacy DEBUG_LOCKS and DEBUG_LOGS_VERBOSE Kconfig options. The posix implementation of spinlock.h is simplified. The operations were no-ops already before, but there were multiple layers of abstractions left in the code. Remove all of these. Signed-off-by: Kai Vehmanen --- Kconfig.sof | 16 --- posix/include/rtos/spinlock.h | 144 ++------------------ posix/include/rtos/wait.h | 4 - src/CMakeLists.txt | 2 +- src/arch/host/include/arch/spinlock.h | 27 ---- src/include/sof/debug/debug.h | 4 - src/spinlock.c | 46 ------- test/cmocka/src/common_mocks.c | 14 -- test/cmocka/src/lib/alloc/CMakeLists.txt | 2 - test/cmocka/src/lib/fast-get/CMakeLists.txt | 1 - 10 files changed, 13 insertions(+), 247 deletions(-) delete mode 100644 src/arch/host/include/arch/spinlock.h delete mode 100644 src/spinlock.c diff --git a/Kconfig.sof b/Kconfig.sof index 8a2cf6a1c833..be11a5249ab4 100644 --- a/Kconfig.sof +++ b/Kconfig.sof @@ -176,22 +176,6 @@ config DEBUG_MEMORY_USAGE_SCAN This feature does not affect standard memory operations, especially allocation and deallocation. -config DEBUG_LOCKS - bool "Spinlock debug" - default n - help - It adds additional information to the spinlocks about - the current user of the lock. Also executes panic - on deadlock. - -config DEBUG_LOCKS_VERBOSE - bool "Spinlock verbose debug" - depends on DEBUG_LOCKS - default n - help - In addition to DEBUG_LOCKS it also adds spinlock traces - every time the lock is acquired. - config DEBUG_IPC_COUNTERS bool "IPC counters" depends on CAVS diff --git a/posix/include/rtos/spinlock.h b/posix/include/rtos/spinlock.h index beb20ab6e847..22580246519f 100644 --- a/posix/include/rtos/spinlock.h +++ b/posix/include/rtos/spinlock.h @@ -13,125 +13,12 @@ #ifndef __POSIX_RTOS_SPINLOCK_H__ #define __POSIX_RTOS_SPINLOCK_H__ -#include -typedef uint32_t k_spinlock_key_t; -#include -#include - #include -/* - * Lock debugging provides a simple interface to debug deadlocks. The rmbox - * trace output will show an output :- - * - * 0xd70 [41.306406] delta [0.359638] lock eal - * 0xd80 [41.306409] delta [0.000002] value 0x00000000000001b7 - * 0xd90 [41.306411] delta [0.000002] value 0x0000000000000001 - * 0xda0 [41.306413] delta [0.000002] value 0x0000000001000348 - * - * "eal" indicates we are holding a lock with interrupts OFF. The next value - * is the line number of where the lock was acquired. The second number is the - * number of other locks held whilst this lock is held and the subsequent - * numbers list each lock and the line number of it's holder. e.g. to find - * the locks :- - * - * grep -rn lock --include *.c | grep 840 (search for lock at line 0x348) - * src/drivers/dw-dma.c:840: spinlock_init(&dma->lock); - * - * grep -rn lock --include *.c | grep 439 - * src/lib/alloc.c:439: k_spin_lock_irq(&memmap.lock, flags); - * - * Every lock entry and exit shows LcE and LcX in trace alongside the lock - * line numbers in hex. e.g. - * - * 0xfd60 [11032.730567] delta [0.000004] lock LcE - * 0xfd70 [11032.730569] delta [0.000002] value 0x00000000000000ae - * - * Deadlock can be confirmed in rmbox :- - * - * Debug log: - * debug: 0x0 (00) = 0xdead0007 (-559087609) |....| - * .... - * Error log: - * using 19.20MHz timestamp clock - * 0xc30 [26.247240] delta [26.245851] lock DED - * 0xc40 [26.247242] delta [0.000002] value 0x00000000000002b4 - * 0xc50 [26.247244] delta [0.000002] value 0x0000000000000109 - * - * DED means deadlock has been detected and the DSP is now halted. The first - * value after DEA is the line number where deadlock occurs and the second - * number is the line number where the lock is allocated. These can be grepped - * like above. - */ - -#if CONFIG_DEBUG_LOCKS - -#include -#include -#include -#include +struct k_spinlock { +}; -#define DBG_LOCK_USERS 8 -#define DBG_LOCK_TRIES 10000 - -extern uint32_t lock_dbg_atomic; -extern uint32_t lock_dbg_user[DBG_LOCK_USERS]; - -extern struct tr_ctx sl_tr; - -/* panic on deadlock */ -#define spin_try_lock_dbg(lock, line) \ - do { \ - int __tries; \ - for (__tries = DBG_LOCK_TRIES; __tries > 0; __tries--) { \ - if (arch_try_lock(lock)) \ - break; /* lock acquired */ \ - } \ - if (__tries == 0) { \ - tr_err_atomic(&sl_tr, "DED"); \ - tr_err_atomic(&sl_tr, "line: %d", line); \ - tr_err_atomic(&sl_tr, "user: %d", (lock)->user); \ - panic(SOF_IPC_PANIC_DEADLOCK); /* lock not acquired */ \ - } \ - } while (0) - -#if CONFIG_DEBUG_LOCKS_VERBOSE -#define spin_lock_log(lock, line) \ - do { \ - if (lock_dbg_atomic) { \ - int __i = 0; \ - int __count = lock_dbg_atomic >= DBG_LOCK_USERS \ - ? DBG_LOCK_USERS : lock_dbg_atomic; \ - tr_err_atomic(&sl_tr, "eal"); \ - tr_err_atomic(&sl_tr, "line: %d", line); \ - tr_err_atomic(&sl_tr, "dbg_atomic: %d", lock_dbg_atomic); \ - for (__i = 0; __i < __count; __i++) { \ - tr_err_atomic(&sl_tr, "value: %d", \ - (lock_dbg_atomic << 24) | \ - lock_dbg_user[__i]); \ - } \ - } \ - } while (0) - -#define spin_lock_dbg(line) \ - do { \ - tr_info(&sl_tr, "LcE"); \ - tr_info(&sl_tr, "line: %d", line); \ - } while (0) - -#define spin_unlock_dbg(line) \ - do { \ - tr_info(&sl_tr, "LcX"); \ - tr_info(&sl_tr, "line: %d", line); \ - } while (0) - -#else /* CONFIG_DEBUG_LOCKS_VERBOSE */ -#define spin_lock_log(lock, line) do {} while (0) -#define spin_lock_dbg(line) do {} while (0) -#define spin_unlock_dbg(line) do {} while (0) -#endif /* CONFIG_DEBUG_LOCKS_VERBOSE */ - -#else /* CONFIG_DEBUG_LOCKS */ +typedef uint32_t k_spinlock_key_t; #define trace_lock(__e) do {} while (0) #define tracev_lock(__e) do {} while (0) @@ -139,25 +26,18 @@ extern struct tr_ctx sl_tr; #define spin_lock_dbg(line) do {} while (0) #define spin_unlock_dbg(line) do {} while (0) -#endif /* CONFIG_DEBUG_LOCKS */ +#define k_spinlock_init(lock) do {} while (0) -/* all SMP spinlocks need init, nothing todo on UP */ -static inline void _spinlock_init(struct k_spinlock *lock, int line) +static inline k_spinlock_key_t k_spin_lock(struct k_spinlock *l) { - arch_spinlock_init(lock); -#if CONFIG_DEBUG_LOCKS - lock->user = line; -#endif + return 0; } -#define k_spinlock_init(lock) _spinlock_init(lock, __LINE__) - -/* disables all IRQ sources and takes lock - enter atomic context */ -k_spinlock_key_t _k_spin_lock_irq(struct k_spinlock *lock); -#define k_spin_lock(lock) _k_spin_lock_irq(lock) - -/* re-enables current IRQ sources and releases lock - leave atomic context */ -void _k_spin_unlock_irq(struct k_spinlock *lock, k_spinlock_key_t key, int line); -#define k_spin_unlock(lock, key) _k_spin_unlock_irq(lock, key, __LINE__) +static inline void k_spin_unlock(struct k_spinlock *l, + k_spinlock_key_t key) +{ + (void)l; + (void)key; +} #endif /* __POSIX_RTOS_SPINLOCK_H__ */ diff --git a/posix/include/rtos/wait.h b/posix/include/rtos/wait.h index b6874888a763..c6cd029ec6b6 100644 --- a/posix/include/rtos/wait.h +++ b/posix/include/rtos/wait.h @@ -30,10 +30,6 @@ static inline void wait_for_interrupt(int level) LOG_MODULE_DECLARE(wait, CONFIG_SOF_LOG_LEVEL); tr_dbg(&wait_tr, "WFE"); -#if CONFIG_DEBUG_LOCKS - if (lock_dbg_atomic) - tr_err_atomic(&wait_tr, "atm"); -#endif platform_wait_for_interrupt(level); tr_dbg(&wait_tr, "WFX"); } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 29d1ad54a8bb..e1455a75bd7e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -11,7 +11,7 @@ add_subdirectory(module) if(CONFIG_SAMPLES) add_subdirectory(samples) endif() -add_local_sources(sof spinlock.c) + add_subdirectory(drivers) if (CONFIG_TRACE) diff --git a/src/arch/host/include/arch/spinlock.h b/src/arch/host/include/arch/spinlock.h deleted file mode 100644 index 360cb72bdb1f..000000000000 --- a/src/arch/host/include/arch/spinlock.h +++ /dev/null @@ -1,27 +0,0 @@ -/* SPDX-License-Identifier: BSD-3-Clause - * - * Copyright(c) 2016 Intel Corporation. All rights reserved. - * - * Author: Liam Girdwood - */ - -/* TODO: this needs fixed as part of the "host does not need rtos headers work" */ -#ifdef __POSIX_RTOS_SPINLOCK_H__ - -#ifndef __ARCH_SPINLOCK_H__ -#define __ARCH_SPINLOCK_H__ - -struct k_spinlock { -}; - -static inline void arch_spinlock_init(struct k_spinlock *lock) {} -static inline void arch_spin_lock(struct k_spinlock *lock) {} -static inline void arch_spin_unlock(struct k_spinlock *lock) {} - -#endif /* __ARCH_SPINLOCK_H__ */ - -#else - -#error "This file shouldn't be included from outside of sof/spinlock.h" - -#endif /* __SOF_SPINLOCK_H__ */ diff --git a/src/include/sof/debug/debug.h b/src/include/sof/debug/debug.h index 8103da3012b8..76eec7062de3 100644 --- a/src/include/sof/debug/debug.h +++ b/src/include/sof/debug/debug.h @@ -33,8 +33,6 @@ #define DEBUG_SET_FW_READY_FLAGS \ ( \ SOF_IPC_INFO_BUILD | \ - (IS_ENABLED(CONFIG_DEBUG_LOCKS) ? SOF_IPC_INFO_LOCKS : 0) | \ - (IS_ENABLED(CONFIG_DEBUG_LOCKS_VERBOSE) ? SOF_IPC_INFO_LOCKSV : 0) | \ (IS_ENABLED(CONFIG_GDB_DEBUG) ? SOF_IPC_INFO_GDB : 0) \ ) @@ -124,8 +122,6 @@ #define DEBUG_SET_FW_READY_FLAGS \ ( \ - (IS_ENABLED(CONFIG_DEBUG_LOCKS) ? SOF_IPC_INFO_LOCKS : 0) | \ - (IS_ENABLED(CONFIG_DEBUG_LOCKS_VERBOSE) ? SOF_IPC_INFO_LOCKSV : 0) | \ (IS_ENABLED(CONFIG_GDB_DEBUG) ? SOF_IPC_INFO_GDB : 0) \ ) diff --git a/src/spinlock.c b/src/spinlock.c deleted file mode 100644 index 52e80a29ff42..000000000000 --- a/src/spinlock.c +++ /dev/null @@ -1,46 +0,0 @@ -// SPDX-License-Identifier: BSD-3-Clause -// -// Copyright(c) 2020 Intel Corporation. All rights reserved. -// -// Author: Tomasz Lauda - -#include -#if CONFIG_DEBUG_LOCKS -#include -#endif -#include - -#include - -#if CONFIG_DEBUG_LOCKS - -SOF_DEFINE_REG_UUID(spinlock); - -DECLARE_TR_CTX(sl_tr, SOF_UUID(spinlock_uuid), LOG_LEVEL_INFO); - -#endif - -#ifndef __ZEPHYR__ -k_spinlock_key_t _k_spin_lock_irq(struct k_spinlock *lock) -{ - k_spinlock_key_t key = interrupt_global_disable(); -#if CONFIG_DEBUG_LOCKS - lock_dbg_atomic++; -#endif - arch_spin_lock(lock); -#if CONFIG_DEBUG_LOCKS - if (lock_dbg_atomic < DBG_LOCK_USERS) - lock_dbg_user[lock_dbg_atomic - 1] = (lock)->user; -#endif - return key; -} - -void _k_spin_unlock_irq(struct k_spinlock *lock, k_spinlock_key_t key, int line) -{ - arch_spin_unlock(lock); -#if CONFIG_DEBUG_LOCKS - lock_dbg_atomic--; -#endif - interrupt_global_enable(key); -} -#endif diff --git a/test/cmocka/src/common_mocks.c b/test/cmocka/src/common_mocks.c index 3d844b803268..cb116c5f8908 100644 --- a/test/cmocka/src/common_mocks.c +++ b/test/cmocka/src/common_mocks.c @@ -146,20 +146,6 @@ volatile void * WEAK task_context_get(void) return NULL; } -uint32_t WEAK _k_spin_lock_irq(struct k_spinlock *lock) -{ - (void)lock; - - return 0; -} - -void WEAK _k_spin_unlock_irq(struct k_spinlock *lock, uint32_t flags, int line) -{ - (void)lock; - (void)flags; - (void)line; -} - uint64_t WEAK platform_timer_get(struct timer *timer) { (void)timer; diff --git a/test/cmocka/src/lib/alloc/CMakeLists.txt b/test/cmocka/src/lib/alloc/CMakeLists.txt index 4b7649599435..c9ba91a9a1d7 100644 --- a/test/cmocka/src/lib/alloc/CMakeLists.txt +++ b/test/cmocka/src/lib/alloc/CMakeLists.txt @@ -5,7 +5,6 @@ if(BUILD_UNIT_TESTS_HOST) alloc.c ${PROJECT_SOURCE_DIR}/src/lib/alloc.c ${PROJECT_SOURCE_DIR}/src/platform/library/lib/memory.c - ${PROJECT_SOURCE_DIR}/src/spinlock.c ) else() if(CONFIG_CAVS) @@ -15,7 +14,6 @@ else() alloc.c ${PROJECT_SOURCE_DIR}/src/lib/alloc.c ${PROJECT_SOURCE_DIR}/src/debug/panic.c - ${PROJECT_SOURCE_DIR}/src/spinlock.c ${MEMORY_FILE} ) endif() diff --git a/test/cmocka/src/lib/fast-get/CMakeLists.txt b/test/cmocka/src/lib/fast-get/CMakeLists.txt index 0ca1a6c2f86f..640f821cf4f8 100644 --- a/test/cmocka/src/lib/fast-get/CMakeLists.txt +++ b/test/cmocka/src/lib/fast-get/CMakeLists.txt @@ -5,7 +5,6 @@ cmocka_test(fast-get-tests ${PROJECT_SOURCE_DIR}/zephyr/lib/fast-get.c ${PROJECT_SOURCE_DIR}/src/lib/alloc.c ${PROJECT_SOURCE_DIR}/src/platform/library/lib/memory.c - ${PROJECT_SOURCE_DIR}/src/spinlock.c ) target_link_libraries(fast-get-tests PRIVATE "-Wl,--wrap=rzalloc,--wrap=rmalloc,--wrap=rfree")