anchao commented on code in PR #13410:
URL: https://github.com/apache/nuttx/pull/13410#discussion_r1757767217
##########
include/nuttx/wdog.h:
##########
@@ -173,8 +174,107 @@ int wd_start(FAR struct wdog_s *wdog, sclock_t delay,
*
****************************************************************************/
-int wd_start_absolute(FAR struct wdog_s *wdog, clock_t ticks,
- wdentry_t wdentry, wdparm_t arg);
+int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks,
+ wdentry_t wdentry, wdparm_t arg);
+
+/****************************************************************************
+ * Name: wd_start_abstime
+ *
+ * Description:
+ * This function adds a watchdog timer to the active timer queue. The
+ * specified watchdog function at 'wdentry' will be called from the
+ * interrupt level after the specified number of ticks has reached.
+ * Watchdog timers may be started from the interrupt level.
+ *
+ * Watchdog timers execute in the address environment that was in effect
+ * when wd_start() is called.
+ *
+ * Watchdog timers execute only once.
+ *
+ * To replace either the timeout delay or the function to be executed,
+ * call wd_start again with the same wdog; only the most recent wdStart()
+ * on a given watchdog ID has any effect.
+ *
+ * Input Parameters:
+ * wdog - Watchdog ID
+ * abstime - Absolute time with struct timespec
+ * wdentry - Function to call on timeout
+ * arg - Parameter to pass to wdentry.
+ *
+ * NOTE: The parameter must be of type wdparm_t.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success; a negated errno value is return 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.
+ *
+ ****************************************************************************/
+
+#define wd_start_abstime(wdog, abstime, wdentry, arg) \
+ wd_start_abstick(wdog, clock_time2ticks(abstime), wdentry, arg)
+
+/****************************************************************************
+ * Name: wd_start_realtime
+ *
+ * Description:
+ * This function adds a watchdog timer to the active timer queue. The
+ * specified watchdog function at 'wdentry' will be called from the
+ * interrupt level after the specified number of ticks has reached.
+ * Watchdog timers may be started from the interrupt level.
+ *
+ * Watchdog timers execute in the address environment that was in effect
+ * when wd_start() is called.
+ *
+ * Watchdog timers execute only once.
+ *
+ * To replace either the timeout delay or the function to be executed,
+ * call wd_start again with the same wdog; only the most recent wdStart()
+ * on a given watchdog ID has any effect.
+ *
+ * Input Parameters:
+ * wdog - Watchdog ID
+ * realtime - Realtime with struct timespec
+ * wdentry - Function to call on timeout
+ * arg - Parameter to pass to wdentry.
+ *
+ * NOTE: The parameter must be of type wdparm_t.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success; a negated errno value is return 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.
+ *
+ ****************************************************************************/
+
+static inline int wd_start_realtime(FAR struct wdog_s *wdog,
+ FAR const struct timespec *realtime,
+ wdentry_t wdentry,
+ wdparm_t arg)
+{
+#ifdef CONFIG_CLOCK_TIMEKEEPING
+ irqstate_t flags;
+ clock_t ticks;
+ int ret;
+
+ flags = enter_critical_section();
Review Comment:
no need to hold critical section, already protected by nxclock_gettime
##########
sched/signal/sig_notification.c:
##########
@@ -135,6 +133,23 @@ int nxsig_notification(pid_t pid, FAR struct sigevent
*event,
memcpy(&info.si_value, &event->sigev_value, sizeof(union sigval));
+ /* Used only by POSIX timer. Notice that it is UNSAFE, unless
+ * we GUARANTEE that event->sigev_notify_thread_id is valid.
+ */
+
+ if (event->sigev_notify & SIGEV_THREAD_ID)
+ {
+ rtcb = nxsched_get_tcb(event->sigev_notify_thread_id);
+ if (rtcb != NULL)
+ {
+ return nxsig_tcbdispatch(rtcb, &info);
+ }
+ else
+ {
+ return -ENOENT;
+ }
Review Comment:
```suggestion
pid = event->sigev_notify_thread_id;
```
##########
sched/timer/timer_settime.c:
##########
@@ -96,11 +96,32 @@ static inline void timer_signotify(FAR struct posix_timer_s
*timer)
static inline void timer_restart(FAR struct posix_timer_s *timer,
wdparm_t itimer)
{
+ clock_t ticks;
+ sclock_t delay;
+
/* If this is a repetitive timer, then restart the watchdog */
if (timer->pt_delay)
{
- wd_start(&timer->pt_wdog, timer->pt_delay, timer_timeout, itimer);
+ /* Check whether next expected time is reached */
+
+ ticks = clock_systime_ticks();
+ timer->pt_overrun = 0;
+
+ for (; ; )
+ {
+ timer->pt_expected += timer->pt_delay;
+ delay = timer->pt_expected - ticks;
+ if (delay > 0)
Review Comment:
```suggestion
if (timer->pt_expected > ticks)
```
##########
sched/timer/timer_create.c:
##########
@@ -158,6 +158,7 @@ int timer_create(clockid_t clockid, FAR struct sigevent
*evp,
FAR timer_t *timerid)
{
FAR struct posix_timer_s *ret;
+ FAR struct tcb_s *tcb = nxsched_self();
Review Comment:
```suggestion
FAR struct tcb_s *tcb = this_task();
```
##########
sched/timer/timer_settime.c:
##########
@@ -96,11 +96,32 @@ static inline void timer_signotify(FAR struct posix_timer_s
*timer)
static inline void timer_restart(FAR struct posix_timer_s *timer,
wdparm_t itimer)
{
+ clock_t ticks;
+ sclock_t delay;
+
/* If this is a repetitive timer, then restart the watchdog */
if (timer->pt_delay)
{
- wd_start(&timer->pt_wdog, timer->pt_delay, timer_timeout, itimer);
+ /* Check whether next expected time is reached */
+
+ ticks = clock_systime_ticks();
+ timer->pt_overrun = 0;
+
+ for (; ; )
Review Comment:
Is it possible busy loop here?
--
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]