xiaoxiang781216 commented on code in PR #17295:
URL: https://github.com/apache/nuttx/pull/17295#discussion_r2508094586
##########
include/nuttx/wdog.h:
##########
@@ -89,6 +89,7 @@ struct wdog_s
#ifdef CONFIG_PIC
FAR void *picbase; /* PIC base address */
#endif
+ clock_t delay;
Review Comment:
This field increases the size of wdog by 25%(24bytes to 32bytes, 32bit
arch/64bit clock_t), but the benefit is very small since most driver already
keep the timeout value either by macro or variable.
@Fix-Point consider add this field, but drop it finally to save the size.
##########
include/nuttx/wdog.h:
##########
@@ -143,6 +143,40 @@ extern "C"
int wd_start(FAR struct wdog_s *wdog, clock_t delay,
wdentry_t wdentry, wdparm_t arg);
+/****************************************************************************
+ * Name: wd_reload
+ *
+ * Description:
+ * This function restarts (reloads) an already-started watchdog timer
+ * with a new delay time, without modifying its existing handler function
+ * or argument.
+ *
+ * It behaves similarly to wd_start(), but is typically used when the
+ * watchdog has already been started and needs to be rearmed before it
+ * expires. The same watchdog function (wdog->func) will be called from
+ * the interrupt level after the specified number of ticks has elapsed.
+ *
+ * Watchdog timers may be started or reloaded from the interrupt level.
+ * Watchdog timers execute in the address environment that was in effect
+ * when wd_reload() is called.
+ * Watchdog timers execute only once.
+ *
+ * Input Parameters:
+ * wdog - Watchdog ID
+ * delay - Delay count in clock ticks
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success; a negated errno value is returned to
+ * indicate the nature of any failure.
+ *
+ * Assumptions:
+ * The watchdog routine runs in the context of the timer interrupt handler
+ * and is subject to all ISR restrictions.
+ *
+ ****************************************************************************/
+
+int wd_reload(FAR struct wdog_s *wdog, clock_t delay);
Review Comment:
> By the way, to enable this convenient API (wd_restart()), a new delay
member was added to the watchdog structure. Although this introduces a small
memory overhead, it provides the following benefits:
>
> 1. It can be reused by the wd_reload() API, making it easier for users
to create accurate periodic events.
>
> 2. It offers additional debugging information in case the watchdog
fails to operate as expected.
It isn't good adding field into wdog_s to increase it's size, please see two
period wdog implemented by @Fix-Point to understand it:
https://github.com/apache/nuttx/pull/15937 and
https://github.com/apache/nuttx/pull/16231.
##########
sched/wdog/wd_start.c:
##########
@@ -400,6 +400,43 @@ int wd_start(FAR struct wdog_s *wdog, clock_t delay,
wdentry, arg);
}
+/****************************************************************************
+ * Name: wd_restart
+ *
+ * Description:
+ * This function restarts (reloads) an already-started watchdog timer
+ * with a new delay time, without modifying its existing handler function
+ * or argument.
+ *
+ * It behaves similarly to wd_start(), but is typically used when the
+ * watchdog has already been started and needs to be rearmed before it
+ * expires. The same watchdog function (wdog->func) will be called from
+ * the interrupt level after the specified number of ticks has elapsed.
+ *
+ * Watchdog timers may be started or reloaded from the interrupt level.
+ * Watchdog timers execute in the address environment that was in effect
+ * when wd_restart() is called.
+ * Watchdog timers execute only once.
+ *
+ * Input Parameters:
+ * wdog - Watchdog ID
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success; a negated errno value is returned to
+ * indicate the nature of any failure.
+ *
+ * Assumptions:
+ * The watchdog routine runs in the context of the timer interrupt handler
+ * and is subject to all ISR restrictions.
+ *
+ ****************************************************************************/
+
+int wd_restart(FAR struct wdog_s *wdog)
+{
+ return wd_start_abstick(wdog, clock_restartdelay2abstick(wdog->delay),
Review Comment:
still need add 1tick to delay
##########
sched/wdog/wd_start.c:
##########
@@ -400,6 +400,43 @@ int wd_start(FAR struct wdog_s *wdog, clock_t delay,
wdentry, arg);
}
+/****************************************************************************
+ * Name: wd_restart
+ *
+ * Description:
+ * This function restarts (reloads) an already-started watchdog timer
+ * with a new delay time, without modifying its existing handler function
+ * or argument.
+ *
+ * It behaves similarly to wd_start(), but is typically used when the
+ * watchdog has already been started and needs to be rearmed before it
+ * expires. The same watchdog function (wdog->func) will be called from
+ * the interrupt level after the specified number of ticks has elapsed.
+ *
+ * Watchdog timers may be started or reloaded from the interrupt level.
+ * Watchdog timers execute in the address environment that was in effect
+ * when wd_restart() is called.
+ * Watchdog timers execute only once.
+ *
+ * Input Parameters:
+ * wdog - Watchdog ID
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success; a negated errno value is returned to
+ * indicate the nature of any failure.
+ *
+ * Assumptions:
+ * The watchdog routine runs in the context of the timer interrupt handler
+ * and is subject to all ISR restrictions.
+ *
+ ****************************************************************************/
+
+int wd_restart(FAR struct wdog_s *wdog)
Review Comment:
let's pass delay from argument to save wdog_s size
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
