{bp-19433} drivers/watchdog: fix capture automonitor notifier context

Documentation/applications/testing/drivertest/index.rst

@@ -2,4 +2,83 @@
 ``drivertest`` cmocka driver test
 ======================================
 
-This is a test for specific driver or chip based on cmocka.
+``drivertest`` is a collection of applications that exercise NuttX driver
+interfaces with the :doc:`cmocka </applications/testing/cmocka/index>` test
+framework.  It is not a single test runner.  Each test is registered as a
+separate application, such as ``cmocka_driver_rtc`` or
+``cmocka_driver_watchdog``, when its configuration dependencies are met.
+
+Configuration and use
+=====================
+
+Enable ``CONFIG_TESTING_CMOCKA`` and ``CONFIG_TESTING_DRIVER_TEST``.  The
+selected board configuration determines which driver test applications are
+built.  ``CONFIG_TESTING_DRIVER_TEST_PRIORITY`` and
+``CONFIG_TESTING_DRIVER_TEST_STACKSIZE`` set their common task priority and
+stack size.  The simple cmocka self-test and the oneshot test also require
+``CONFIG_TESTING_DRIVER_TEST_SIMPLE`` and ``CONFIG_TESTING_ONESHOT_TEST``,
+respectively.
+
+Run an enabled test from NSH by entering its application name.  Some tests
+require real or simulated devices and may change device state, wait for an
+interrupt, or reset the board.  Check the test-specific behavior before adding
+one to an unattended test run.
+
+Current test coverage
+=====================
+
+The available applications are grouped below.  Only applications whose
+dependencies are enabled are included in a given NuttX image.
+
+* Basic and storage tests: ``cmocka_driver_simple`` and
+  ``cmocka_driver_block``.
+* Time-related tests: ``cmocka_driver_rtc``, ``cmocka_driver_timer``,
+  ``cmocka_driver_oneshot``, ``cmocka_posix_timer``, and
+  ``cmocka_driver_watchdog``.
+* Peripheral and bus tests: ``cmocka_driver_pwm``, ``cmocka_driver_adc``,
+  ``cmocka_driver_i2c_spi``, ``cmocka_driver_i2c_write``,
+  ``cmocka_driver_i2c_read``, ``cmocka_driver_gpio``,
+  ``cmocka_driver_relay``, ``cmocka_driver_uart``, and
+  ``cmocka_driver_audio``.
+* Display and input tests: ``cmocka_driver_framebuffer``,
+  ``cmocka_driver_lcd``, and ``cmocka_driver_touchpanel``.
+* Power-management tests: ``cmocka_driver_cpufreq``,
+  ``cmocka_driver_regulator``, ``cmocka_driver_pm``,
+  ``cmocka_driver_pm_smp``, and ``cmocka_driver_pm_runtime``.
+* Arm MPS2 AN500 interrupt tests: ``cmocka_driver_mps2``,
+  ``cmocka_driver_mps2_zerointerrupt``, and
+  ``cmocka_driver_mps2_isr_signal``.
+
+Watchdog tests
+==============
+
+``cmocka_driver_watchdog`` is built when ``CONFIG_WATCHDOG`` is enabled with
+at least one of ``CONFIG_BOARDCTL_RESET_CAUSE`` or
+``CONFIG_WATCHDOG_TIMEOUT_NOTIFIER``.
+
+With ``CONFIG_BOARDCTL_RESET_CAUSE``, the ``-r`` option selects a hardware
+watchdog test case:
+
+* ``-r 0`` feeds the watchdog for a configured interval and then stops
+  feeding it.  The watchdog is expected to reset the board.
+* ``-r 1`` verifies that the watchdog resets the board while interrupts are
+  disabled.
+* ``-r 2`` verifies that the watchdog resets the board when an interrupt-level
+  watchdog callback does not return.
+* ``-r 3`` exercises the watchdog status, keepalive, capture, and stop
+  interfaces.  This case is not included on Armv7-A TrustZone builds.
+
+Cases 0 through 2 intentionally reset the target and use the reset cause from
+the preceding boot to check the result.  They must therefore be run with a
+board that reports reset causes correctly and should not be treated as normal
+non-destructive unit tests.  The application accepts ``-d`` for the device
+path, ``-o`` for the timeout, ``-t`` for the total ping interval, ``-l`` for
+the delay between pings, ``-a`` for the status tolerance, and ``-g`` to skip
+the status check.
+
+With ``CONFIG_WATCHDOG_TIMEOUT_NOTIFIER``, the application also runs two
+non-destructive notifier tests.  They verify callback priority ordering,
+duplicate registration, repeated delivery, unregister behavior, the selected
+automonitor action and payload, and concurrent register/notify/unregister
+operation.  These notifier tests do not require
+``CONFIG_BOARDCTL_RESET_CAUSE`` or a watchdog device node.

arch/arm/src/common/stm32/stm32_wwdg_m0_v1.c

@@ -299,7 +299,7 @@ static int stm32_interrupt(int irq, void *context, void *arg)
            * upon return.
            */
 
-          priv->handler(irq, context, arg);
+          priv->handler(irq, context, priv);
         }
 
       /* The EWI interrupt is cleared by writing '0' to the EWIF bit in the

arch/arm/src/common/stm32/stm32_wwdg_m3m4_v1.c

@@ -299,7 +299,7 @@ static int stm32_interrupt(int irq, void *context, void *arg)
            * upon return.
            */
 
-          priv->handler(irq, context, arg);
+          priv->handler(irq, context, priv);
         }
 
       /* The EWI interrupt is cleared by writing '0' to the EWIF bit in the

arch/arm/src/stm32h7/stm32_wwdg.c

@@ -298,7 +298,7 @@ static int stm32_interrupt(int irq, void *context, void *arg)
            * upon return.
            */
 
-          priv->handler(irq, context, arg);
+          priv->handler(irq, context, priv);
         }
 
       /* The EWI interrupt is cleared by writing '0' to the EWIF bit in the

boards/sim/sim/sim/configs/watchdog-notifier/defconfig

@@ -0,0 +1,77 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_NSH_CMDOPT_HEXDUMP is not set
+CONFIG_ALLOW_MIT_COMPONENTS=y
+CONFIG_ARCH="sim"
+CONFIG_ARCH_BOARD="sim"
+CONFIG_ARCH_BOARD_SIM=y
+CONFIG_ARCH_CHIP="sim"
+CONFIG_ARCH_SIM=y
+CONFIG_BOARDCTL_APP_SYMTAB=y
+CONFIG_BOARDCTL_POWEROFF=y
+CONFIG_BOARD_LOOPSPERMSEC=0
+CONFIG_BOOT_RUNFROMEXTSRAM=y
+CONFIG_BUILTIN=y
+CONFIG_DEBUG_ASSERTIONS=y
+CONFIG_DEBUG_ASSERTIONS_EXPRESSION=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DEV_GPIO=y
+CONFIG_DEV_LOOP=y
+CONFIG_ETC_FATDEVNO=2
+CONFIG_ETC_ROMFS=y
+CONFIG_ETC_ROMFSDEVNO=1
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_BINFS=y
+CONFIG_FS_FAT=y
+CONFIG_FS_HOSTFS=y
+CONFIG_FS_PROCFS=y
+CONFIG_FS_RAMMAP=y
+CONFIG_FS_ROMFS=y
+CONFIG_GPIO_LOWER_HALF=y
+CONFIG_IDLETHREAD_STACKSIZE=4096
+CONFIG_INIT_ENTRYPOINT="nsh_main"
+CONFIG_IOEXPANDER=y
+CONFIG_IOEXPANDER_DUMMY=y
+CONFIG_LIBC_ENVPATH=y
+CONFIG_LIBC_EXECFUNCS=y
+CONFIG_LIBC_LOCALE=y
+CONFIG_LIBC_LOCALE_CATALOG=y
+CONFIG_LIBC_LOCALE_GETTEXT=y
+CONFIG_LIBC_MAX_EXITFUNS=1
+CONFIG_LIBC_NUMBERED_ARGS=y
+CONFIG_NDEBUG=y
+CONFIG_NSH_BUILTIN_APPS=y
+CONFIG_NSH_FILE_APPS=y
+CONFIG_NSH_READLINE=y
+CONFIG_PATH_INITIAL="/bin"
+CONFIG_PIPES=y
+CONFIG_PSEUDOFS_ATTRIBUTES=y
+CONFIG_PSEUDOFS_FILE=y
+CONFIG_PSEUDOFS_SOFTLINKS=y
+CONFIG_READLINE_TABCOMPLETION=y
+CONFIG_RTC=y
+CONFIG_RTC_ARCH=y
+CONFIG_RTC_DRIVER=y
+CONFIG_SCHED_BACKTRACE=y
+CONFIG_SCHED_EVENTS=y
+CONFIG_SCHED_HAVE_PARENT=y
+CONFIG_SCHED_WAITPID=y
+CONFIG_SIM_HOSTFS=y
+CONFIG_SIM_WALLTIME_SIGNAL=y
+CONFIG_START_MONTH=6
+CONFIG_START_YEAR=2008
+CONFIG_SYSTEM_DUMPSTACK=y
+CONFIG_SYSTEM_NSH=y
+CONFIG_TESTING_CMOCKA=y
+CONFIG_TESTING_DRIVER_TEST=y
+CONFIG_WATCHDOG=y
+CONFIG_WATCHDOG_AUTOMONITOR=y
+CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE=y
+CONFIG_WATCHDOG_TIMEOUT_NOTIFIER=y

drivers/timers/watchdog.c

@@ -42,6 +42,7 @@
 #include <nuttx/panic_notifier.h>
 #include <nuttx/power/pm.h>
 #include <nuttx/mutex.h>
+#include <nuttx/spinlock.h>
 #include <nuttx/wdog.h>
 #include <nuttx/wqueue.h>
 #include <nuttx/timers/oneshot.h>
@@ -99,6 +100,9 @@ struct watchdog_upperhalf_s
   struct notifier_block nb;
 #endif
 #ifdef CONFIG_WATCHDOG_AUTOMONITOR
+#  if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+  FAR struct watchdog_upperhalf_s *capture_next;
+#  endif
 #  if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
   FAR struct oneshot_lowerhalf_s *oneshot;
 #  elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_TIMER)
@@ -153,24 +157,129 @@ static const struct file_operations g_wdogops =
 static ATOMIC_NOTIFIER_HEAD(g_watchdog_notifier_list);
 #endif
 
+#ifdef CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE
+/* Active capture automonitor instances.  Lower halves that pass an argument
+ * can identify their upper-half directly or through the lower-half pointer.
+ * A NULL argument is accepted only when this list contains one instance.
+ */
+
+static FAR struct watchdog_upperhalf_s *g_watchdog_capture_list;
+static spinlock_t g_watchdog_capture_lock = SP_UNLOCKED;
+#endif
+
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
-#if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+#ifdef CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE
+static void watchdog_capture_add(FAR struct watchdog_upperhalf_s *upper)
+{
+  irqstate_t flags = spin_lock_irqsave(&g_watchdog_capture_lock);
+
+  upper->capture_next = g_watchdog_capture_list;
+  g_watchdog_capture_list = upper;
+  spin_unlock_irqrestore(&g_watchdog_capture_lock, flags);
+}
+
+static void watchdog_capture_remove(FAR struct watchdog_upperhalf_s *upper)
+{
+  FAR struct watchdog_upperhalf_s **cursor;
+  irqstate_t flags = spin_lock_irqsave(&g_watchdog_capture_lock);
+
+  cursor = &g_watchdog_capture_list;
+  while (*cursor != NULL)
+    {
+      if (*cursor == upper)
+        {
+          *cursor = upper->capture_next;
+          upper->capture_next = NULL;
+          break;
+        }
+
+      cursor = &(*cursor)->capture_next;
+    }
+
+  spin_unlock_irqrestore(&g_watchdog_capture_lock, flags);
+}
+
+static FAR struct watchdog_upperhalf_s *
+watchdog_capture_find(FAR void *arg)
+{
+  FAR struct watchdog_upperhalf_s *upper;
+  FAR struct watchdog_upperhalf_s *match = NULL;
+  irqstate_t flags = spin_lock_irqsave(&g_watchdog_capture_lock);
+
+  if (arg == NULL)
+    {
+      for (upper = g_watchdog_capture_list; upper != NULL;
+           upper = upper->capture_next)
+        {
+          if (match != NULL)
+            {
+              /* More than one NULL-context instance cannot be identified. */
+
+              match = NULL;
+              break;
+            }
+
+          match = upper;
+        }
+    }
+  else
+    {
+      for (upper = g_watchdog_capture_list; upper != NULL;
+           upper = upper->capture_next)
+        {
+          if (arg == upper || arg == upper->lower)
+            {
+              match = upper;
+              break;
+            }
+        }
+    }
+
+  spin_unlock_irqrestore(&g_watchdog_capture_lock, flags);
+  return match;
+}
+
 static int watchdog_automonitor_capture(int irq, FAR void *context,
                                         FAR void *arg)
 {
-  FAR struct watchdog_upperhalf_s *upper = arg;
-  FAR struct watchdog_lowerhalf_s *lower = upper->lower;
+  FAR struct watchdog_upperhalf_s *upper = watchdog_capture_find(arg);
+  FAR struct watchdog_lowerhalf_s *lower;
+
+  /* A stop operation can race with a pending interrupt.  Do not dereference
+   * a removed association after automonitor has been stopped.
+   */
+
+  if (upper == NULL)
+    {
+      return 0;
+    }
+
+  lower = upper->lower;
 
   if (upper->monitor)
     {
+      /* Reload the hardware watchdog before dispatching optional
+       * notifications.  The EWI-to-reset interval is short, and notifier
+       * callbacks must not delay the keepalive operation.
+       */
+
       lower->ops->keepalive(lower);
+
+#ifdef CONFIG_WATCHDOG_TIMEOUT_NOTIFIER
+      /* The capture callback is entered from the watchdog timeout
+       * interrupt.  Notifier callbacks must be safe in interrupt context.
+       */
+
+      watchdog_automonitor_timeout();
+#endif
     }
 
   return 0;
 }
+
 #elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
 static void
 watchdog_automonitor_oneshot(FAR struct oneshot_lowerhalf_s *oneshot,
@@ -267,6 +376,7 @@ watchdog_automonitor_start(FAR struct watchdog_upperhalf_s *upper)
   if (!upper->monitor)
     {
 #  if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+      watchdog_capture_add(upper);
       lower->ops->capture(lower, watchdog_automonitor_capture);
 #  elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
       struct timespec ts =
@@ -311,6 +421,7 @@ static void watchdog_automonitor_stop(FAR struct watchdog_upperhalf_s *upper)
       lower->ops->stop(lower);
 #  if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
       lower->ops->capture(lower, NULL);
+      watchdog_capture_remove(upper);
 #  elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
       ONESHOT_CANCEL(upper->oneshot, NULL);
 #  elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_TIMER)
@@ -762,7 +873,12 @@ void watchdog_notifier_chain_unregister(FAR struct notifier_block *nb)
 
 void watchdog_automonitor_timeout(void)
 {
-  atomic_notifier_call_chain(&g_watchdog_notifier_list, action, data);
+  /* The action identifies the automonitor keepalive mechanism selected at
+   * build time.  There is no source-specific payload for this event.
+   */
+
+  atomic_notifier_call_chain(&g_watchdog_notifier_list,
+                             WATCHDOG_NOTIFIER_ACTION, NULL);
 }
 #endif /* CONFIG_WATCHDOG_TIMEOUT_NOTIFIER */
 

include/nuttx/timers/watchdog.h

@@ -258,9 +258,12 @@ void watchdog_notifier_chain_unregister(FAR struct notifier_block *nb);
  * Name: watchdog_automonitor_timeout
  *
  * Description:
- *   This function can be called in the watchdog timeout interrupt handler.
- *   If so, callbacks on the watchdog timer notify chain are called when the
- *   watchdog timer times out.
+ *   Notify callbacks registered on the watchdog timeout notifier chain.
+ *   The action identifies the automonitor keepalive mechanism selected by
+ *   CONFIG_WATCHDOG_AUTOMONITOR_BY_*.  The callback data argument is NULL.
+ *
+ *   This function is intended to be called from a watchdog timeout interrupt
+ *   handler.  Callback functions must obey interrupt-context restrictions.
  *
  ****************************************************************************/