RE: [EXTERNAL] Re: [PATCH] drivers: rtc: handle OTF clock changes

[Elad Nachman]

Hi Alexandre,

> 
> 
> From: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
> Sent: Wednesday, June 24, 2026 6:31 PM
> To: Elad Nachman <enachman@xxxxxxxxxxx>
> Cc: linux-rtc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: [EXTERNAL] Re: [PATCH] drivers: rtc: handle OTF clock changes
> 
> > From: Elad Nachman <mailto:enachman@xxxxxxxxxxx>
> >
> > When processing expired RTC events and rearming them, use now
> > instead of expiry to prevent endless loops.
> > Issue seen with Armada 385 SOC.
> 
> The loop is not endless, it may be long however. How do you reproduce

48 seconds are long enough to trigger RCU related watchdog which eventually causes NOS reboot...

> this? Or maybe the question is what is enabling PIE on your system?

Not only PIE but also UIE enablement leads to this behavior.
UIE is enabled by Chrony, and that is activated by SONIC NOS.
SONIC NOS was introduced a time modification test (forward/backward)
which triggers this issue, causing the following kernel soft lockup:

[  409.188746] rcu: blocking rcu_node structures (internal RCU debug):
[  436.242391] watchdog: BUG: soft lockup - CPU#1 stuck for 48s! [kworker/1:3:432]
[  436.250173] Kernel panic - not syncing: softlockup: hung tasks
[  436.256032] CPU: 1 PID: 432 Comm: kworker/1:3 Tainted: G           O L     6.1.0-29-2-armmp
[  436.272194] Workqueue: events rtc_timer_do_work
[  436.377383]  __irq_svc from _raw_spin_unlock_irqrestore+0x24/0x28
[  436.383510]  _raw_spin_unlock_irqrestore from __wake_up_common_lock+0x8c/0xc0
[  436.390685]  __wake_up_common_lock from __wake_up+0x20/0x28
[  436.396286]  __wake_up from rtc_handle_legacy_irq+0x58/0x6c
[  436.401890]  rtc_handle_legacy_irq from rtc_timer_do_work+0xe0/0x37c
[  436.408275]  rtc_timer_do_work from process_one_work+0x1f8/0x4b4
[  436.414317]  process_one_work from worker_thread+0x54/0x50c
[  436.419924]  worker_thread from kthread+0xd8/0xf4

> 
> Your patch breaks existing code because it will expect to get one event
> per elapsed period while your patch will cause events to be skipped.

I realize that, but the alternative of having soft lockup followed by reboot is worse in my opinion...

If you have any better idea, I am more than open to suggestions...

> 
> >
> > Fixes commit 6610e0893b8b ("RTC: Rework RTC code to use timerqueue for events")
> > Signed-off-by: Elad Nachman <mailto:enachman@xxxxxxxxxxx>
> > ---
> >  drivers/rtc/interface.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> > index 96626f8068f9..c32ef95a07d4 100644
> > --- a/drivers/rtc/interface.c
> > +++ b/drivers/rtc/interface.c
> > @@ -126,6 +126,7 @@ EXPORT_SYMBOL_GPL(rtc_read_time);
> >  int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
> >  {
> >  	int err, uie;
> > +	struct rtc_time new_tm;
> >
> >  	err = rtc_valid_tm(tm);
> >  	if (err != 0)
> > @@ -159,6 +160,17 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
> >  	else
> >  		err = -EINVAL;
> >
> > +	if (rtc && rtc->ops && rtc->ops->read_time) {
> > +		if (!rtc->ops->read_time(rtc->dev.parent, &new_tm)) {
> > +			pr_debug("new rtc time secs %d mins %d hours %d mday %d mon %d year %d way %d yday %d dst %d\n",
> > +					new_tm.tm_sec, new_tm.tm_min,
> > +					new_tm.tm_hour, new_tm.tm_mday,
> > +					new_tm.tm_mon, new_tm.tm_year,
> > +					new_tm.tm_wday, new_tm.tm_yday,
> > +					new_tm.tm_isdst);
> > +		}
> > +	}
> > +
> 
> 
> This is unrelated to the patch.

I can remove this easily once we find a solution to the main issue above...

> 
> >  	pm_stay_awake(rtc->dev.parent);
> >  	mutex_unlock(&rtc->ops_lock);
> >  	/* A timer might have just expired */
> > @@ -999,7 +1011,7 @@ void rtc_timer_do_work(struct work_struct *work)
> >  		trace_rtc_timer_fired(timer);
> >  		/* Re-add/fwd periodic timers */
> >  		if (ktime_to_ns(timer->period)) {
> > -			timer->node.expires = ktime_add(timer->node.expires,
> > +			timer->node.expires = ktime_add(now,
> >  							timer->period);
> >  			timer->enabled = 1;
> >  			timerqueue_add(&rtc->timerqueue, &timer->node);
> > --
> > 2.25.1
> >
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://urldefense.proofpoint.com/v2/url?u=https-3A__bootlin.com&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=eTeNTLEK5-TxXczjOcKPhANIFtlB9pP4lq9qhdlFrwQ&m=6PDPbjuCmjWGjRiUxkS5sPKd4lHU_NypV7pVhoFdBcLnksTSMiVrFYs7blEViGJR&s=IQM9cBpCUzoV4ONJnQs4yjzbjygO7zRYDW1jpJL3zWM&e=

Thanks,

Elad.