From c45e4ac440781ebce826ce3f964f8d13a7fff147 Mon Sep 17 00:00:00 2001 From: hujun5 Date: Thu, 1 Aug 2024 15:15:43 +0800 Subject: [PATCH] boardcrtl: change BOARDCTL_TESTSET to BOARDIOC_SPINLOCK reason: BOARDIOC_SPINLOCK can support the combined semantics of disabling interrupts (irq), trylock, and spinlock. Signed-off-by: hujun5 --- boards/Kconfig | 5 +- boards/boardctl.c | 73 ++++++++++++++++++++-------- include/sys/boardctl.h | 35 ++++++++++--- libs/libc/pthread/Kconfig | 2 +- libs/libc/pthread/pthread_spinlock.c | 54 +++++++++----------- 5 files changed, 105 insertions(+), 64 deletions(-) diff --git a/boards/Kconfig b/boards/Kconfig index 15c4586444..090a0aa602 100644 --- a/boards/Kconfig +++ b/boards/Kconfig @@ -4815,12 +4815,11 @@ config BOARDCTL_USBDEVCTRL ---help--- Enables support for the BOARDIOC_USBDEV_CONTROL boardctl() command. -config BOARDCTL_TESTSET - bool "Architecture-specific test/set operation" +config BOARDCTL_SPINLOCK + bool "spinlock specific operation" default n ---help--- Enables support for the BOARDIOC_SPINLOCK boardctl() command. - Architecture specific logic must provide up_testset() interface. config BOARDCTL_IRQ_AFFINITY bool "Set an IRQ affinity to CPUs by software" diff --git a/boards/boardctl.c b/boards/boardctl.c index c6cae7ba41..788de6a6d5 100644 --- a/boards/boardctl.c +++ b/boards/boardctl.c @@ -53,10 +53,6 @@ # include #endif -#ifdef CONFIG_BOARDCTL_TESTSET -# include -#endif - #if defined(CONFIG_BUILD_PROTECTED) && defined(CONFIG_BUILTIN) # include #endif @@ -344,7 +340,7 @@ static inline int boardctl_pmctrl(FAR struct boardioc_pm_ctrl_s *ctrl) int boardctl(unsigned int cmd, uintptr_t arg) { - int ret; + int ret = OK; switch (cmd) { @@ -586,7 +582,6 @@ int boardctl(unsigned int cmd, uintptr_t arg) DEBUGASSERT(symdesc != NULL); exec_setsymtab(symdesc->symtab, symdesc->nsymbols); - ret = OK; } break; #endif @@ -608,7 +603,6 @@ int boardctl(unsigned int cmd, uintptr_t arg) DEBUGASSERT(symdesc != NULL); modlib_setsymtab(symdesc->symtab, symdesc->nsymbols); - ret = OK; } break; #endif @@ -643,7 +637,6 @@ int boardctl(unsigned int cmd, uintptr_t arg) DEBUGASSERT(builtin != NULL); builtin_setlist(builtin->builtins, builtin->count); #endif - ret = OK; } break; #endif @@ -789,27 +782,65 @@ int boardctl(unsigned int cmd, uintptr_t arg) #endif /* CONFIG_NXTERM */ -#ifdef CONFIG_BOARDCTL_TESTSET - /* CMD: BOARDIOC_TESTSET - * DESCRIPTION: Access architecture-specific up_testset() operation - * ARG: A pointer to a write-able spinlock object. On - * success the preceding spinlock state is returned: - * 0=unlocked, 1=locked. - * CONFIGURATION: CONFIG_BOARDCTL_TESTSET - * DEPENDENCIES: Architecture-specific logic provides up_testset() +#ifdef CONFIG_BOARDCTL_SPINLOCK + /* CMD: BOARDIOC_SPINLOCK + * DESCRIPTION: Access spinlock specific operation + * ARG: A pointer to a write-able boardioc_spinlock_s + * object. + * CONFIGURATION: CONFIG_BOARDCTL_SPINLOCK + * DEPENDENCIES: spinlock specific logic */ - case BOARDIOC_TESTSET: + case BOARDIOC_SPINLOCK: { - volatile FAR spinlock_t *lock = (volatile FAR spinlock_t *)arg; + FAR struct boardioc_spinlock_s *spinlock = + (FAR struct boardioc_spinlock_s *)arg; + FAR volatile spinlock_t *lock = spinlock->lock; + FAR irqstate_t *flags = spinlock->flags; - if (lock == NULL) + if (spinlock->action == BOARDIOC_SPINLOCK_LOCK) { - ret = -EINVAL; + if (flags != NULL) + { + *flags = up_irq_save(); + } + + if (lock != NULL) + { + spin_lock(lock); + } + } + else if (spinlock->action == BOARDIOC_SPINLOCK_TRYLOCK) + { + if (flags != NULL) + { + *flags = up_irq_save(); + } + + if (!spin_trylock(lock)) + { + ret = -EBUSY; + if (flags != NULL) + { + up_irq_restore(*flags); + } + } + } + else if (spinlock->action == BOARDIOC_SPINLOCK_UNLOCK) + { + if (flags != NULL) + { + up_irq_restore(*flags); + } + + if (lock != NULL) + { + spin_unlock(lock); + } } else { - ret = up_testset(lock) == SP_LOCKED ? 1 : 0; + ret = -EINVAL; } } break; diff --git a/include/sys/boardctl.h b/include/sys/boardctl.h index 25ac632a05..8674608b9d 100644 --- a/include/sys/boardctl.h +++ b/include/sys/boardctl.h @@ -42,6 +42,10 @@ # include #endif +#ifdef CONFIG_BOARDCTL_SPINLOCK +# include +#endif + #ifdef CONFIG_BOARDCTL /**************************************************************************** @@ -173,13 +177,12 @@ * CONFIGURATION: CONFIG_NXTERM * DEPENDENCIES: Base NX terminal logic provides nxterm_ioctl_tap() * - * CMD: BOARDIOC_TESTSET - * DESCRIPTION: Access architecture-specific up_testset() operation - * ARG: A pointer to a write-able spinlock object. On success - * the preceding spinlock state is returned: 0=unlocked, - * 1=locked. - * CONFIGURATION: CONFIG_BOARDCTL_TESTSET - * DEPENDENCIES: Architecture-specific logic provides up_testset() + * CMD: BOARDIOC_SPINLOCK + * DESCRIPTION: spinlock specific operation + * ARG: A pointer to a write-able boardioc_spinlock_s + * + * CONFIGURATION: CONFIG_BOARDCTL_SPINLOCK + * DEPENDENCIES: spinlock specific logic * * CMD: BOARDIOC_RESET_CAUSE * DESCRIPTION: Get the cause of last-time board reset @@ -204,7 +207,7 @@ #define BOARDIOC_VNC_START _BOARDIOC(0x000e) #define BOARDIOC_NXTERM _BOARDIOC(0x000f) #define BOARDIOC_NXTERM_IOCTL _BOARDIOC(0x0010) -#define BOARDIOC_TESTSET _BOARDIOC(0x0011) +#define BOARDIOC_SPINLOCK _BOARDIOC(0x0011) #define BOARDIOC_UNIQUEKEY _BOARDIOC(0x0012) #define BOARDIOC_SWITCH_BOOT _BOARDIOC(0x0013) #define BOARDIOC_BOOT_IMAGE _BOARDIOC(0x0014) @@ -280,6 +283,22 @@ struct boardioc_romdisk_s }; #endif +#ifdef CONFIG_BOARDCTL_SPINLOCK +enum boardioc_spinlock_e +{ + BOARDIOC_SPINLOCK_LOCK = 0, /* call up_irq_save or/and spin_lock */ + BOARDIOC_SPINLOCK_TRYLOCK = 1, /* call up_irq_save or/and spin_trylock */ + BOARDIOC_SPINLOCK_UNLOCK = 2, /* call up_irq_restore or/and spin_unlock */ +}; + +struct boardioc_spinlock_s +{ + enum boardioc_spinlock_e action; /* see enum boardioc_spinlock_e */ + FAR irqstate_t *flags; /* whether we need to disable int */ + FAR volatile spinlock_t *lock; /* whether we need to call spinlock */ +}; +#endif + /* In order to full describe a symbol table, a vector containing the address * of the symbol table and the number of elements in the symbol table is * required. diff --git a/libs/libc/pthread/Kconfig b/libs/libc/pthread/Kconfig index 19cbff73c9..70aad3ea3b 100644 --- a/libs/libc/pthread/Kconfig +++ b/libs/libc/pthread/Kconfig @@ -10,7 +10,7 @@ config PTHREAD_SPINLOCKS bool "pthread spinlock support" default n depends on SPINLOCK && (BUILD_FLAT || BOARDCTL) - select BOARDCTL_TESTSET if !BUILD_FLAT + select BOARDCTL_SPINLOCK ---help--- Enable support for pthread spinlocks. diff --git a/libs/libc/pthread/pthread_spinlock.c b/libs/libc/pthread/pthread_spinlock.c index e4c14bfeb6..9b0f422cea 100644 --- a/libs/libc/pthread/pthread_spinlock.c +++ b/libs/libc/pthread/pthread_spinlock.c @@ -163,6 +163,7 @@ int pthread_spin_destroy(pthread_spinlock_t *lock) int pthread_spin_lock(pthread_spinlock_t *lock) { + struct boardioc_spinlock_s spinlock; pthread_t me = pthread_self(); int ret; @@ -176,22 +177,10 @@ int pthread_spin_lock(pthread_spinlock_t *lock) return EDEADLOCK; } - /* Loop until we successfully take the spinlock (i.e., until the previous - * state of the spinlock was SP_UNLOCKED). - * NOTE that the test/set operation is performed via boardctl() to avoid a - * variety of issues. An option might be to move the implementation of - * up_testset() to libs/libc/machine. - */ - - do - { -#ifdef CONFIG_BUILD_FLAT - ret = up_testset(&lock->sp_lock) == SP_LOCKED ? 1 : 0; -#else - ret = boardctl(BOARDIOC_TESTSET, (uintptr_t)&lock->sp_lock); -#endif - } - while (ret == 1); + spinlock.action = BOARDIOC_SPINLOCK_LOCK; + spinlock.lock = &lock->sp_lock; + spinlock.flags = NULL; + ret = boardctl(BOARDIOC_SPINLOCK, (uintptr_t)&spinlock); /* Check for success (previous state was SP_UNLOCKED) */ @@ -233,6 +222,7 @@ int pthread_spin_lock(pthread_spinlock_t *lock) int pthread_spin_trylock(pthread_spinlock_t *lock) { + struct boardioc_spinlock_s spinlock; pthread_t me = pthread_self(); int ret; @@ -249,21 +239,18 @@ int pthread_spin_trylock(pthread_spinlock_t *lock) { /* Perform the test/set operation via boardctl() */ - ret = boardctl(BOARDIOC_TESTSET, (uintptr_t)&lock->sp_lock); - switch (ret) + spinlock.action = BOARDIOC_SPINLOCK_TRYLOCK; + spinlock.lock = &lock->sp_lock; + spinlock.flags = NULL; + ret = boardctl(BOARDIOC_SPINLOCK, (uintptr_t)&spinlock); + if (ret == 0) /* Previously unlocked. We hold the spinlock */ { - case 0: /* Previously unlocked. We hold the spinlock */ - lock->sp_holder = me; - break; - - case 1: /* Previously locked. We did not get the spinlock */ - ret = EBUSY; - break; - - default: - DEBUGASSERT(ret < 0); - ret = -ret; - break; + lock->sp_holder = me; + } + else /* Previously locked. We did not get the spinlock */ + { + DEBUGASSERT(ret < 0); + ret = -ret; } } @@ -301,6 +288,7 @@ int pthread_spin_trylock(pthread_spinlock_t *lock) int pthread_spin_unlock(pthread_spinlock_t *lock) { + struct boardioc_spinlock_s spinlock; pthread_t me = pthread_self(); DEBUGASSERT(lock != NULL && @@ -319,7 +307,11 @@ int pthread_spin_unlock(pthread_spinlock_t *lock) /* Release the lock */ lock->sp_holder = IMPOSSIBLE_THREAD; - spin_initialize(&lock->sp_lock, SP_UNLOCKED); + spinlock.action = BOARDIOC_SPINLOCK_UNLOCK; + spinlock.lock = &lock->sp_lock; + spinlock.flags = NULL; + boardctl(BOARDIOC_SPINLOCK, (uintptr_t)&spinlock); + return OK; }