GUIDINGLI commented on code in PR #17065:
URL: https://github.com/apache/nuttx/pull/17065#discussion_r2384753813
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -424,16 +427,22 @@ static clock_t nxsched_timer_start(clock_t ticks, clock_t
interval)
interval = interval <= (CONFIG_TIMER_ADJUST_USEC / USEC_PER_TICK) ? 0 :
interval - (CONFIG_TIMER_ADJUST_USEC / USEC_PER_TICK);
-#ifdef CONFIG_SCHED_TICKLESS_ALARM
+#ifdef CONFIG_HRTIMER
+ ret = hrtimer_start(&g_hrtimer_systick_timer,
+ NSEC_PER_TICK * (ticks + interval),
Review Comment:
as we use HRtimer, but upper half still use tick, we should ensue all the
flow to HIGH-Resolution, are we ?
Take a example:
The user call usleep() ---- HIGH-Resolution
change to ticks in wd_start --- LOW-Resolution
nxsched_timer_start tick ---- LOW-Resolution
hrtimer_start ns ---- HIGH-Resolution
Driver tick ---- LOW-Resolution
##########
drivers/timers/arch_alarm.c:
##########
@@ -136,22 +138,32 @@ static void oneshot_callback(FAR struct
oneshot_lowerhalf_s *lower,
/****************************************************************************
* Public Functions
****************************************************************************/
+#ifdef CONFIG_HRTIMER
+void up_alarm_set_hrtimer_lowerhalf(FAR struct hrtimer_lowerhalf_s *lower)
+{
+ hrtimer_upper_set_lower(&g_default_hrtimer_upperhalf, lower);
+ hrtimer_upper_start(&g_default_hrtimer_upperhalf);
+}
+#endif
void up_alarm_set_lowerhalf(FAR struct oneshot_lowerhalf_s *lower)
{
-#ifdef CONFIG_SCHED_TICKLESS
+#ifdef CONFIG_HRTIMER
Review Comment:
HRtimer and wdog, these two should exist in a coexisting relationship rather
than an either-or one.
How about provide a framework:
Keep the underlying driver unchanged, reuse a hardware timer, and add the
hrtimer processing flow in arch_alarm/arch_timer/wdog
##########
arch/tricore/src/common/tricore_systimer.c:
##########
@@ -356,15 +444,24 @@ static int tricore_systimer_interrupt(int irq, void
*context, void *arg)
*
****************************************************************************/
-struct oneshot_lowerhalf_s *
-tricore_systimer_initialize(volatile void *tbase, int irq, uint64_t freq)
Review Comment:
why remove the return oneshot_lowerhalf_s, if so, this timer can only used
for system tick, can't for others, like
cpuload in nxsched_oneshot_extclk()
##########
include/nuttx/timers/hrtimer.h:
##########
@@ -0,0 +1,131 @@
+/****************************************************************************
+ * include/nuttx/timers/hrtimer.h
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_TIMERS_HRTIMER_H
+#define __INCLUDE_NUTTX_TIMERS_HRTIMER_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/compiler.h>
+#include <nuttx/clock.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <assert.h>
+#include <errno.h>
+
+#ifdef CONFIG_HRTIMER
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Method access helper macros **********************************************/
+
+/* Set the expiration time (in absolute clock ticks).
+ * Argument: expiration time
+ */
+
+#define HRTIMER_SETEXPIRE(l,t) \
+ ((l)->ops->setexpire ? (l)->ops->setexpire(l,t) : -ENOTSUP)
+
+/* Get the current time (in clock ticks).
+ * Argument: Ignored
+ */
+
+#define HRTIMER_CURRENT(l) \
+ ((l)->ops->current ? (l)->ops->current(l) : -ENOTSUP)
+
+/* Start the high-resolution timer.
+ * Argument: Ignored
+ */
+
+#define HRTIMER_START(l) \
+ ((l)->ops->start ? (l)->ops->start(l) : -ENOTSUP)
+
+/* Trigger the timer immediately (force expiration).
+ * Argument: Ignored
+ */
+
+#define HRTIMER_TRIGGER(l) \
+ ((l)->ops->trigger ? (l)->ops->trigger(l) : -ENOTSUP)
+
+#define HRTIMER_USEC2TIME(l, us) \
+ (((clock_t)(us)) * ((l)->freq) / USEC_PER_SEC)
+
+#define HRTIMER_NSEC2TIME(l, ns) \
+ (((clock_t)(ns)) * ((l)->freq) / NSEC_PER_SEC)
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* Forward declaration */
+
+struct hrtimer_lowerhalf_s;
+
+/* This structure defines the operations provided by the "lower-half" HRTIMER
+ * driver to the "upper-half" driver.
+ */
+
+struct hrtimer_ops_s
+{
+ /* Set the timer expiration time (absolute time in clock ticks). */
+
+ CODE int (*setexpire)(FAR struct hrtimer_lowerhalf_s *lower,
+ clock_t expiration_time);
Review Comment:
HRtimer means the HIGH resolution, if here still use clock_t which unit is
"tick", where is the "HIGH Resolution" ? why not use timespec or ns ?
--
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]
