Skip to content

Commit

Permalink
boardcrtl: change BOARDCTL_TESTSET to BOARDIOC_SPINLOCK
Browse files Browse the repository at this point in the history
reason:
BOARDIOC_SPINLOCK can support the combined semantics
of disabling interrupts (irq), trylock, and spinlock.

Signed-off-by: hujun5 <hujun5@xiaomi.com>
  • Loading branch information
hujun260 committed Sep 6, 2024
1 parent f084685 commit 1e84233
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 64 deletions.
5 changes: 2 additions & 3 deletions boards/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -4805,12 +4805,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"
Expand Down
73 changes: 52 additions & 21 deletions boards/boardctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@
# include <nuttx/usb/composite.h>
#endif

#ifdef CONFIG_BOARDCTL_TESTSET
# include <nuttx/spinlock.h>
#endif

#if defined(CONFIG_BUILD_PROTECTED) && defined(CONFIG_BUILTIN)
# include <nuttx/lib/builtin.h>
#endif
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
35 changes: 27 additions & 8 deletions include/sys/boardctl.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@
# include <nuttx/nx/nxterm.h>
#endif

#ifdef CONFIG_BOARDCTL_SPINLOCK
# include <nuttx/spinlock.h>
#endif

#ifdef CONFIG_BOARDCTL

/****************************************************************************
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion libs/libc/pthread/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
54 changes: 23 additions & 31 deletions libs/libc/pthread/pthread_spinlock.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) */

Expand Down Expand Up @@ -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;

Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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 &&
Expand All @@ -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;
}

Expand Down

0 comments on commit 1e84233

Please sign in to comment.