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 <[email protected]>

Author: hujun260

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"

boards/boardctl.c

@@ -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
@@ -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;

include/sys/boardctl.h

@@ -42,6 +42,10 @@
 #  include <nuttx/nx/nxterm.h>
 #endif
 
+#ifdef CONFIG_BOARDCTL_SPINLOCK
+#  include <nuttx/spinlock.h>
+#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.

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.
 

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;
 }