xiaoxiang781216 commented on code in PR #15324:
URL: https://github.com/apache/nuttx/pull/15324#discussion_r1899616478
##########
arch/renesas/src/rx65n/rx65n_rtc.c:
##########
@@ -916,7 +926,8 @@ int rx65n_rtc_setalarm(struct alm_setalarm_s *alminfo)
/* Is there already something waiting on the ALARM? */
- flags = enter_critical_section();
+ flags = spin_lock_irqsave(&g_rtc_lock);
+ sched_lock();
Review Comment:
why need sched lock/unlock
##########
sched/clock/clock_initialize.c:
##########
@@ -170,7 +173,20 @@ static void clock_inittime(FAR const struct timespec *tp)
clock_basetime(&g_basetime);
}
+#ifdef CONFIG_RTC_HIRES
Review Comment:
why add?
##########
arch/renesas/src/rx65n/rx65n_rtc.c:
##########
@@ -476,6 +480,9 @@ int up_rtc_gettime(struct timespec *tp)
uint8_t regval;
struct tm t;
+ flags = spin_lock_irqsave(&g_rtc_lock);
+ sched_lock();
Review Comment:
why need sched lock/unlock
##########
arch/arm/src/sam34/sam_rtc.c:
##########
@@ -654,7 +657,8 @@ int sam_rtc_setalarm(const struct timespec *tp, alarmcb_t
callback)
/* Is there already something waiting on the ALARM? */
- flags = enter_critical_section();
+ flags = spin_lock_irqsave(&g_rtc_lock);
+ sched_lock();
Review Comment:
why need sched lock
##########
arch/arm/src/sam34/sam_rtc.c:
##########
@@ -759,11 +764,14 @@ int up_rtc_gettime(struct timespec *tp)
{
/* This is a hack to emulate a high resolution rtc using the rtt */
+ irqstate_t flags;
uint32_t rtc_cal;
uint32_t rtc_tim;
uint32_t rtt_val;
struct tm t;
+ flags = spin_lock_irqsave(&g_rtc_lock);
Review Comment:
why need sched_lock
##########
arch/arm/src/stm32/stm32_eth.c:
##########
@@ -4318,11 +4323,13 @@ int up_rtc_gettime(struct timespec *tp)
*/
DEBUGASSERT(!g_rtc_enabled);
+ spin_unlock_irqrestore(&g_rtc_lock, flags);
Review Comment:
move before line 4325
##########
arch/x86_64/src/intel64/intel64_rtc.c:
##########
@@ -64,6 +65,10 @@
* Private Data
****************************************************************************/
+#ifdef CONFIG_RTC_HIRES
+static spinlock_t g_rtc_lock = SP_UNLOCKED;
Review Comment:
why need lcok
##########
sched/clock/clock_initialize.c:
##########
@@ -373,12 +400,18 @@ void clock_resynchronize(FAR struct timespec *rtc_diff)
/* Add the sleep time to correct system timer */
- g_system_ticks += SEC2TICK(rtc_diff->tv_sec);
- g_system_ticks += NSEC2TICK(rtc_diff->tv_nsec);
+ clock_t diff_ticks = SEC2TICK(rtc_diff->tv_sec) +
+ NSEC2TICK(rtc_diff->tv_nsec);
+
+#ifdef CONFIG_SYSTEM_TIME64
+ atomic64_fetch_add((FAR atomic64_t *)&g_system_ticks, diff_ticks);
Review Comment:
why protected by both atomic and spinlock
##########
sched/clock/clock_initialize.c:
##########
@@ -341,7 +355,20 @@ void clock_resynchronize(FAR struct timespec *rtc_diff)
* bias value that we need to use to correct the base time.
*/
+#ifdef CONFIG_RTC_HIRES
Review Comment:
why add this code snippets
##########
sched/clock/clock_initialize.c:
##########
@@ -398,6 +431,10 @@ void clock_timer(void)
{
/* Increment the per-tick system counter */
- g_system_ticks++;
Review Comment:
let's split thee bug fix to different patch
--
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: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
