Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers/timers/watchdog: add watchdog timer notifier chain #15387

Merged
merged 1 commit into from
Jan 12, 2025

Conversation

Jiaqi-YP7
Copy link
Contributor

Add support for watchdog timer notifer chain so that users
can customize the callback function when the watchdog timer
times out which enabled by Auto-monitor.

Summary

Some watchdog timers support an interrupt on timeout and allow
the user to perform some custom processing actions instead of
immediately sending a reset signal. However, when watchdog
timer is managed by auto-monitor, users can no longer register
the timeout callback function implemented by users for watchdog
timer through ioctl.

Impact

We provide CONFIG_WATCHDOG_TIMEOUT_NOTIFIER to enable
or disable watchdog timer notifier chain, This option depends
on CONFIG_WATCHDOG_AUTOMONITOR.

After the watchdog timer notifier chain is enabled, user can put the
watchdog_automonitor_timeout function in the timeout interrupt handler
of the watchdog timer, for example

//wdt_handler is the watchdog interrupt handler function
int wdt_handler(void)
{
  watchdog_automonitor_timeout();
  return OK;
}

This function will call the function registered on the wdt notifier chain
when the watchdog timer times out.

To register the user-defined timeout callback function on the wdt
notifier chain, users should perform the following steps:

step 1:

Include necessary header files

#include <nuttx/wdt_notifier.h>

step 2:

Define the necessary notifier_block

struct notifier_block g_wdt_nb;

step 3:

Define the function to be registered. In The following example,
"The callback function registered by the user is triggered!" is
printed when the watchdog timer times out.

int user_wdt_timeout_callback(FAR struct notifier_block *nb, unsigned long action,
                         FAR void *data)
{
  UNUSED(data);
  UNUSED(nb);
  UNUSED(action);
  printf("The callback function registered by the user is triggered!\n");
  return 0;
}

step 4:

Register user-defined callback functions with wdt_notifier_chain_register

g_wdt_nb.notifier_call = user_wdt_timeout_callback;
wdt_notifier_chain_register(&g_wdt_nb);

*step 5(not necessary):

If users need to unregister the callback function that has been registered
on the watchdog timer notifier chain.

wdt_notifier_chain_unregister(&g_wdt_nb);

Testing

I verified this feature on a private platform where the WDT could support
an interrupt signal instead of a reset signal when times out.

In the test we need to register the custom timeout callback function as
described in Impact. We can add these changes in
nuttx-apps/examples/watchdog/watchdog_main.C,
this example can make watchdog timeout.

I registered The callback function shown in Impact and it just prints
"The callback function registered by the user is triggered!"
After we execute the modified watchdog_example, when the watchdog
times out, the log will show as follows:

wdog_main: timeout=1000_timeleft=497
NO ping elapsed=2012
The callback function registered by the user is triggered!

@github-actions github-actions bot added Area: Drivers Drivers issues Area: OS Components OS Components issues Size: M The size of the change in this PR is medium labels Dec 30, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 30, 2024

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although there are a few minor points that could be improved.

Strengths:

  • Clear Summary: Explains the problem (lack of custom timeout handling with auto-monitor), the solution (notifier chain), and the affected functional area.
  • Detailed Impact: Covers most impact areas and provides clear instructions for using the new feature. The step-by-step guide with code examples is excellent.
  • Testing Description: Explains the test setup and provides relevant log output demonstrating the feature's functionality.

Areas for Potential Improvement:

  • Impact - Build: While you mention CONFIG_WATCHDOG_TIMEOUT_NOTIFIER, explicitly state whether this is a new Kconfig option and if any dependencies are added (besides the stated dependency on CONFIG_WATCHDOG_AUTOMONITOR). For instance: "A new Kconfig option, CONFIG_WATCHDOG_TIMEOUT_NOTIFIER, is added. This option is only available if CONFIG_WATCHDOG_AUTOMONITOR is enabled."
  • Impact - Documentation: Even if minimal, mention where documentation updates are made (e.g., "Updated the watchdog documentation in docs/").
  • Testing - Before Change Logs: While you describe the expected behavior before the change (no custom callback possible), include actual log output showing this. This strengthens the evidence that the change has the intended effect. A simple log snippet showing the watchdog timing out and resetting (without the custom message) would suffice.
  • Testing - Targets: Be more specific about the target platform used for testing. Instead of "a private platform," provide details like architecture and board if possible (even if partially redacted for confidentiality). Example: "Tested on a private platform based on ARM Cortex-M4 (board details redacted)." This helps reviewers understand the context of the testing.
  • Conciseness: The "Does this PR meet the NuttX Requirements?" section can be shortened. A simple "Yes" would suffice, as the rest of the description already provides the necessary details.

By addressing these minor points, the PR will be even stronger and easier for reviewers to assess. Overall, it's a well-structured and informative PR submission.

drivers/timers/Make.defs Outdated Show resolved Hide resolved
drivers/timers/wdt_notifier.c Outdated Show resolved Hide resolved
include/nuttx/wdt_notifier.h Outdated Show resolved Hide resolved
include/nuttx/wdt_notifier.h Outdated Show resolved Hide resolved
include/nuttx/wdt_notifier.h Outdated Show resolved Hide resolved
include/nuttx/wdt_notifier.h Outdated Show resolved Hide resolved
drivers/timers/wdt_notifier.c Outdated Show resolved Hide resolved
include/nuttx/wdt_notifier.h Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the Area: OS Components OS Components issues label Dec 31, 2024
include/nuttx/timers/watchdog.h Outdated Show resolved Hide resolved
drivers/timers/watchdog_notifier.c Outdated Show resolved Hide resolved
drivers/timers/watchdog_notifier.c Outdated Show resolved Hide resolved
include/nuttx/timers/watchdog.h Outdated Show resolved Hide resolved
drivers/timers/watchdog.c Show resolved Hide resolved
drivers/timers/watchdog.c Outdated Show resolved Hide resolved
@xiaoxiang781216 xiaoxiang781216 force-pushed the add-wdt-notifier branch 3 times, most recently from 643a269 to f1607b9 Compare January 11, 2025 16:04
Add support for watchdog timer notifer chain so that users
can customize the callback function when the watchdog timer
times out which enabled by Auto-monitor

Signed-off-by: yaojiaqi <yaojiaqi@lixiang.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 43797ea into apache:master Jan 12, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Drivers Drivers issues 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