Skip to content

apps/examples/watchdog: Introduce new watchdog example#7297

Open
seokhun-eom24 wants to merge 1 commit intoSamsung:masterfrom
seokhun-eom24:260421-watchdog-example
Open

apps/examples/watchdog: Introduce new watchdog example#7297
seokhun-eom24 wants to merge 1 commit intoSamsung:masterfrom
seokhun-eom24:260421-watchdog-example

Conversation

@seokhun-eom24
Copy link
Copy Markdown
Contributor

Introduce a new watchdog example, include Kconfig, Makefile and main source file.
This example demonstrates how to interact with the watchdog timer, providing commands for expiring the timer and pausing/resuming its operation.
To use this example add CONFIG_EXAMPLES_WATCHDOG=y to defconfig.

[Usage]

watchdog expire 30001       -> Expire after 30001ms
watchdog expire 30001 1000  -> Print status every 1000ms and expire after 30001ms
watchdog pause_resume       -> Test watchdog pause/resume functionality and print status

Introduce a new watchdog example, include Kconfig, Makefile and main source file.
This example demonstrates how to interact with the watchdog timer, providing commands for expiring the timer and pausing/resuming its operation.
To use this example add CONFIG_EXAMPLES_WATCHDOG=y to defconfig.

[Usage]
```
watchdog expire 30001       -> Expire after 30001ms
watchdog expire 30001 1000  -> Print status every 1000ms and expire after 30001ms
watchdog pause_resume       -> Test watchdog pause/resume functionality and print status
```
@seokhun-eom24
Copy link
Copy Markdown
Contributor Author

Automated nightly Codex PR review
PR: #7297 — apps/examples/watchdog: Introduce new watchdog example
PR URL: #7297
GitHub updatedAt: 2026-04-21T08:00:28Z
Refreshed (UTC): 2026-04-21 17:41:48 UTC
Refreshed (KST): 2026-04-22 02:41:48 KST
Comment model: single evolving Codex-authored PR comment

PR #7297 — apps/examples/watchdog: Introduce new watchdog example

This review done by codex. AI reviews can be inaccurate.
UTC: 2026-04-21 17:40 UTC
KST (UTC+9): 2026-04-22 02:40 KST


Repository: Samsung/TizenRT

Base → Head: masterpr/7297

HEAD Commit: ef57eb531796a598e917311b1000f7bcff719211

Scope: New apps/examples/watchdog example plus its Kconfig and build-registration files.


🔎 Review Summary

Category Status
Build / Compile Status ⚠️ Not run
Functional Correctness ❌ Blocking issues found
Validation Coverage ⚠️ Static review only

Final Verdict: ❗ Request Changes


🚨 Must-Fix Issues

1. `expire ` leaves the watchdog armed when status polling fails
Item Details
Location apps/examples/watchdog/watchdog_main.c:154
Severity High
Type Functional correctness / Safety

Problem
watchdog_wait_loop() returns ERROR as soon as WDIOC_GETSTATUS fails, and watchdog_run_expire() immediately propagates that error without stopping the watchdog or closing the descriptor. This only affects the polling mode (watchdog expire <timeout_ms> <status_period_ms>), because that is the only path that calls WDIOC_GETSTATUS in the loop.

Impact

  • The command can return control to TASH while the watchdog is still running.
  • The board can then reset unexpectedly a short time later even though the example already reported a command failure.
  • The file descriptor is also leaked on that path.

Evidence

  • apps/examples/watchdog/watchdog_main.c:154 returns ERROR immediately when watchdog_getstatus() fails inside the polling loop.
  • apps/examples/watchdog/watchdog_main.c:185 returns the loop result directly, with no WDIOC_STOP or close(fd) cleanup.
  • apps/examples/watchdog/watchdog_main.c:126 already defines a watchdog_stop() helper, so the omission is local to this path rather than a missing utility.

Required Fix
Files to edit:

  • apps/examples/watchdog/watchdog_main.cwatchdog_run_expire() / watchdog_wait_loop()

Change outline:

  • Keep the current "wait forever for reset" behavior when status_period_ms == 0.
  • On any polling-loop failure, stop the watchdog before returning to the shell, then close the descriptor.
  • Make the cleanup path explicit in watchdog_run_expire() so future error branches do not bypass it.

Example patch:

diff --git a/apps/examples/watchdog/watchdog_main.c b/apps/examples/watchdog/watchdog_main.c
@@
-static int watchdog_wait_loop(int fd, uint32_t status_period_ms)
+static int watchdog_wait_loop(int fd, uint32_t status_period_ms)
 {
@@
-               return ERROR;
+               return ERROR;
        }
@@
-       return watchdog_wait_loop(fd, status_period_ms);
+       ret = watchdog_wait_loop(fd, status_period_ms);
+       if (ret != OK) {
+               (void)watchdog_stop(fd);
+               close(fd);
+               return ERROR;
+       }
+
+       close(fd);
+       return OK;
 }

Inference:

  • The exact cleanup label structure is inferred, but the required behavior is not: the example must not return to the shell with an armed watchdog after a polling failure.

Validation Method

  • Force WDIOC_GETSTATUS to fail after WDIOC_START on a test board or temporary lower-half fault injection.
  • Run watchdog expire 30001 1000 and confirm the command returns ERROR without a delayed watchdog reset afterward.
  • Re-run the same flow under lsof/fd tracing or shell-level descriptor checks if available to confirm the descriptor is closed.
2. `pause_resume` reports success even if `WDIOC_STOP` fails at the end
Item Details
Location apps/examples/watchdog/watchdog_main.c:250
Severity High
Type Functional correctness / Safety

Problem
The success path in watchdog_run_pause_resume() calls watchdog_stop(fd) and ignores its return value. If the lower-half refuses WDIOC_STOP, the function still closes the descriptor and returns OK.

Impact

  • The sample can claim that the pause/resume test passed even though the watchdog is still active.
  • A late watchdog reset can occur after the command exits, which is especially confusing because the output path looks like a successful demonstration.
  • This hides the exact failure that users need in order to diagnose board-specific watchdog behavior.

Evidence

  • apps/examples/watchdog/watchdog_main.c:250 calls watchdog_stop(fd); without checking the result.
  • apps/examples/watchdog/watchdog_main.c:126 through :136 already defines watchdog_stop() to print the failure and return ERROR.
  • Earlier error paths in the same function do treat watchdog-stop as part of cleanup, so the unchecked final call is inconsistent with the rest of the function.

Required Fix
Files to edit:

  • apps/examples/watchdog/watchdog_main.cwatchdog_run_pause_resume()

Change outline:

  • Check the return value of the final watchdog_stop(fd) call.
  • Propagate ERROR if the stop fails, after closing the descriptor.
  • Prefer one shared cleanup label so the final close/return behavior stays consistent across all exit paths.

Example patch:

diff --git a/apps/examples/watchdog/watchdog_main.c b/apps/examples/watchdog/watchdog_main.c
@@
-       watchdog_stop(fd);
-       close(fd);
-       return OK;
+       ret = watchdog_stop(fd);
+       close(fd);
+       return (ret == OK) ? OK : ERROR;
 }

Inference:

  • A cleanup label is optional. The required behavior is that the command must fail visibly when the watchdog cannot be stopped.

Validation Method

  • Run watchdog pause_resume on a board or fault-injected lower-half that rejects WDIOC_STOP.
  • Confirm the command returns ERROR instead of OK, and that the printed failure comes from watchdog_stop().
  • On a normal board, confirm the command still returns OK and no delayed reset occurs after the shell prompt returns.

🟡 Nice-to-Have Improvements

1. Restrict or document `pause_resume` for boards that actually implement the extra watchdog ioctls
Item Details
Location apps/examples/watchdog/Kconfig:9, apps/examples/watchdog/watchdog_main.c:222
Severity Medium
Type Validation / Maintainability

Problem
The example is selectable for any CONFIG_WATCHDOG=y build, but WDIOC_PAUSE / WDIOC_RESUME are only implemented by a subset of lower-half drivers in this tree. Static grep in this PR checkout only found those ioctls in the Ameba/Armino watchdog drivers plus this new example.

Impact

  • The sample advertises a generic pause_resume subcommand even on boards where it will always fail with -ENOSYS.
  • That weakens the usefulness of the example outside the primary Realtek/Beken targets.

Recommended Action

  • Either narrow the feature documentation to the boards that support WDIOC_PAUSE / WDIOC_RESUME, or make the help text explicitly state that pause_resume is optional and driver-dependent.
  • If broader support is intended, add a board-capability guard before advertising the subcommand.

Expected Output

  • Help text and Kconfig help that match the actual board support matrix.
  • A quick validation matrix covering at least one supporting target and one non-supporting target.

✅ Notable Improvements

Notable Improvements

✔ Uses the configured watchdog device path instead of hard-coding /dev/watchdog0

  • watchdog_open_device() opens CONFIG_WATCHDOG_DEVPATH, which matches existing TizenRT watchdog examples and board boot initialization.
  • That keeps the sample aligned with per-board configuration instead of baking in one device node assumption.

✔ Follows the standard example-app registration pattern

  • The PR adds Kconfig, Kconfig_ENTRY, Make.defs, and Makefile in the expected apps/examples/<name> layout.
  • That makes the new example consistent with the surrounding examples tree and easy to enable from existing app build flows.

🧾 Final Assessment

Must-Fix Summary

  • The new example has two watchdog-lifecycle cleanup gaps: polling-mode failures can return with the watchdog still armed, and the pause/resume success path ignores a failed stop.

Nice-to-Have Summary

  • The pause_resume subcommand should be documented or guarded as an optional capability rather than implied as universal across all watchdog-enabled boards.

Residual Risk

  • I did not run a board build or execute the example, so compile status and runtime behavior remain based on static code review only.

📌 Final Verdict

❗ Request Changes

The app scaffolding is fine, but the current example can leave a live watchdog behind on failure paths and can misreport success after an unsuccessful stop. Those lifecycle issues should be fixed before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant