Skip to content

drivers/watchdog: fix capture automonitor notifier context#19433

Open
Zepp-Hanzj wants to merge 2 commits into
apache:masterfrom
Zepp-Hanzj:fix/watchdog-capture-automonitor-notifier
Open

drivers/watchdog: fix capture automonitor notifier context#19433
Zepp-Hanzj wants to merge 2 commits into
apache:masterfrom
Zepp-Hanzj:fix/watchdog-capture-automonitor-notifier

Conversation

@Zepp-Hanzj

Copy link
Copy Markdown
Contributor

Summary

Fix watchdog capture-based automonitor handling for lower-half drivers that invoke the capture callback with a NULL argument, and make timeout notifier delivery match the documented automonitor contract.

Changes

  • Associate the active capture automonitor upper-half with the generic watchdog driver when the lower-half does not pass callback context.
  • Clear the association when automonitor stops and safely ignore a pending capture interrupt after stop.
  • Perform the hardware keepalive before invoking timeout notifiers, preserving the short EWI-to-reset window.
  • Call watchdog_automonitor_timeout() from the capture interrupt path when timeout notifiers are enabled.
  • Standardize notifier delivery to WATCHDOG_NOTIFIER_ACTION with data == NULL.
  • Document interrupt-context restrictions for notifier callbacks.
  • Add an independent simulator defconfig that registers only cmocka_driver_watchdog.
  • Include reproducible hardware and simulator validation evidence.

Design limitation

The fallback context is required only for lower-halves that call the capture handler with arg == NULL. Such a lower-half can support at most one active capture automonitor instance. Lower-halves that provide a per-instance callback argument remain instance-safe. This limitation is documented in the source and validation report; supporting multiple NULL-context lower-halves would require an instance-level callback/context interface in each affected lower-half.

Validation

Simulator

  • Notifier ordering and duplicate-registration behavior.
  • Repeated timeout notifications.
  • Unregister behavior.
  • Correct action and NULL data.
  • Concurrent register/timeout/unregister race.
  • Capture-stop pending-interrupt guard (validated with a temporary injection test; the injection hook was removed after validation).
  • All supported automonitor backends built and exercised: capture, watchdog, worker, oneshot, timer, and idle.

Observed cmocka result:

[==========] tests: 2 test(s) run.
[  PASSED  ] 2 test(s).

STM32F407 hardware

  • Real WWDG EWI -> capture -> hardware keepalive -> notifier path.
  • Repeated notifier output with matching capture/monitor/timeout counters.
  • No-feed WWDG reset loop, confirmed by a new NSH banner after reset.
  • Long-running capture/notifier execution without HardFault or abnormal rapid reset.
  • WWDG WDIOC_STOP limitation recorded: STM32 WWDG cannot be stopped after it has been started, so a stop-race test cannot use the hardware stop ioctl as a success criterion.

Evidence

Relationship to nuttx-apps

The companion test-only change is submitted separately in nuttx-apps as fix/watchdog-notifier-tests. Merge this core PR before enabling the companion test configuration.

Checklist

  • Core implementation change is isolated from board support and flashing scripts.
  • Documentation and validation evidence are included.
  • git diff --check passes.
  • Simulator and STM32 validation completed.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

arduino-mega2560

  • flash: .text +30 B (+0.0%, 67,248 B / 262,144 B, total: 26% used)

esp32-devkitc

  • ROM: .flash.text +8 B (+0.0%, 122,716 B / 4,194,272 B, total: 3% used)
  • irom0_0_seg: .flash.text +8 B (+0.0%, 87,084 B / 3,342,304 B, total: 3% used)

mirtoo

  • kseg0_progmem: .text +20 B (+0.0%, 67,152 B / 131,072 B, total: 51% used)

qemu-armv8a

  • Code: .text.posix_spawnattr_init -16 B, .text.task_spawn +36 B (+0.0%, 315,374 B)

qemu-intel64

  • Code: .text +48 B (+0.0%, 8,655,445 B)

rx65n-rsk2mb

  • ROM: .text +16 B (+0.0%, 85,656 B / 2,097,152 B, total: 4% used)

s698pm-dkit

  • Code: .text +736 B (+0.2%, 358,096 B)

stm32-nucleo-f103rb

  • flash: .text +20 B (+0.1%, 33,792 B / 131,072 B, total: 26% used)
    No memory changes detected for:
  • hifive1-revb

acassis
acassis previously approved these changes Jul 14, 2026
Comment thread docs/watchdog_automonitor_validation.md Outdated
Comment thread docs/validation-evidence/watchdog/04-stm32-reset-cycle-0714.log Outdated
Comment thread drivers/timers/watchdog.c Outdated
@Zepp-Hanzj Zepp-Hanzj force-pushed the fix/watchdog-capture-automonitor-notifier branch from ffd7875 to 5c1719b Compare July 14, 2026 14:35
@github-actions github-actions Bot added Size: M The size of the change in this PR is medium and removed Size: L The size of the change in this PR is large labels Jul 14, 2026
@Zepp-Hanzj Zepp-Hanzj force-pushed the fix/watchdog-capture-automonitor-notifier branch from 5c1719b to 2f117b0 Compare July 14, 2026 14:37
@Zepp-Hanzj

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I updated the branch:

  • Removed the Chinese validation document and all UART log/screenshot files from the source change. Validation results remain summarized here, without adding generated evidence artifacts to the repository.
  • Reformatted the new watchdog comments to pass nxstyle.
  • Added the required Signed-off-by trailer.
  • The capture fallback remains intentional for lower-halves that invoke the callback with arg == NULL; the source comment now explicitly documents that this is a single-active-instance compatibility path. Lower-halves that can pass callback context remain instance-safe. Fixing every affected lower-half would not address existing generic callers that provide no context.

The companion apps branch was also updated with nxstyle/cmake-format fixes and Signed-off-by trailers.

@Zepp-Hanzj Zepp-Hanzj force-pushed the fix/watchdog-capture-automonitor-notifier branch from 2f117b0 to 5c56366 Compare July 14, 2026 14:46
@xiaoxiang781216

Copy link
Copy Markdown
Contributor

Thanks for the review. I updated the branch:

  • The capture fallback remains intentional for lower-halves that invoke the callback with arg == NULL; the source comment now explicitly documents that this is a single-active-instance compatibility path. Lower-halves that can pass callback context remain instance-safe. Fixing every affected lower-half would not address existing generic callers that provide no context.

but the change breaks the system which may have more than one watchdog hardware

@Zepp-Hanzj Zepp-Hanzj force-pushed the fix/watchdog-capture-automonitor-notifier branch from 5c56366 to 1ab7e2f Compare July 15, 2026 06:42
@Zepp-Hanzj

Copy link
Copy Markdown
Contributor Author

The multi-watchdog issue has been addressed in the latest commit 1ab7e2fa3e.

Changes:

  • Removed the single global g_watchdog_capture_upper association.
  • Added an active capture-instance mapping keyed by the callback argument. It accepts either the upper-half pointer used by existing lower-halves or the lower-half pointer used by the updated STM32 WWDG lower-halves.
  • Updated STM32 M0, STM32 M3/M4, and STM32H7 WWDG interrupt paths to pass their own lower-half instance as the callback argument.
  • A legacy lower-half that supplies arg == NULL is only accepted when exactly one capture automonitor instance is active. If multiple NULL-context instances are active, the generic code does not select an arbitrary watchdog, preventing cross-instance keepalive corruption.
  • Removed the single-instance global-pointer behavior that could make one watchdog service another watchdog.

The commit also includes a complete commit body, Signed-off-by: hanzhijian <hanzhijian@zepp.com>, and Assisted-by: OpenAI Codex. Local simulator build and commit-message checks pass; CI has been restarted by the branch update.

@github-actions github-actions Bot added the Arch: arm Issues related to ARM (32-bit) architecture label Jul 15, 2026
jerpelea
jerpelea previously approved these changes Jul 15, 2026
Comment thread drivers/timers/watchdog.c Outdated
Comment thread drivers/timers/watchdog.c Outdated
Comment thread drivers/timers/watchdog.c Outdated
Comment thread drivers/timers/watchdog.c Outdated
@Zepp-Hanzj Zepp-Hanzj force-pushed the fix/watchdog-capture-automonitor-notifier branch from 1ab7e2f to 575fe70 Compare July 15, 2026 07:29
@Zepp-Hanzj

Copy link
Copy Markdown
Contributor Author

Addressed the latest review comments in commit 575fe70:

  • changed the new capture-automonitor guards to #ifdef CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE;
  • replaced the global critical-section protection with a dedicated spinlock_t;
  • separated NULL-context lookup from explicit upper/lower-half context lookup;
  • moved the lower-half declaration into the function definition section to match NuttX declaration style.

The simulator build completes successfully and git diff --check is clean. CI has been restarted for the new head and is currently pending.

@Zepp-Hanzj Zepp-Hanzj force-pushed the fix/watchdog-capture-automonitor-notifier branch from 575fe70 to 090605a Compare July 15, 2026 08:43
@Zepp-Hanzj

Copy link
Copy Markdown
Contributor Author

The failed Build run was caused by the new sim/watchdog-notifier defconfig not being normalized. testbuild.sh rewrote the file during CI and treated the working-tree change as a failure; this was not a watchdog compile error.

I regenerated the defconfig with make savedefconfig, amended the PR, and pushed commit 090605a2ce. Local simulator build and git diff --check pass. CI has been restarted for the normalized configuration.

Provide per-instance capture automonitor lookup for watchdog lower halves
that pass callback context, and avoid selecting an unrelated watchdog when
legacy lower halves provide no context. Update STM32 WWDG lower halves to pass
their instance context so multiple watchdog devices remain distinguishable.

Signed-off-by: hanzhijian <hanzhijian@zepp.com>
Assisted-by: OpenAI Codex
@Zepp-Hanzj Zepp-Hanzj force-pushed the fix/watchdog-capture-automonitor-notifier branch from 090605a to c9701dc Compare July 15, 2026 11:10
Explain how drivertest applications are selected and run, list the current test categories, and document the watchdog reset and notifier test behavior.

Assisted-by: OpenAI Codex
Signed-off-by: hanzhijian <hanzhijian@zepp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: arm Issues related to ARM (32-bit) architecture Area: Drivers Drivers issues Board: simulator Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants