This fixes a couple of different problems, that can cause RTC (alarm) irqs to be missing when generating UIE interrupts.
The first commit fixes a long-standing problem, which has been documented in a comment since 2010. This fixes a race that could cause UIE irqs to stop being generated, which was easily reproduced by timing the use of RTC_UIE_ON ioctl with the seconds tick in the RTC.
The last commit ensures that RTC (alarm) irqs are enabled whenever RTC_UIE_ON ioctl is used.
The driver specific commits avoids kernel warnings about unbalanced enable_irq/disable_irq, which gets triggered on first RTC_UIE_ON with the last commit. Before this series, the same warning should be seen on initial RTC_AIE_ON with those drivers.
Signed-off-by: Esben Haabendal esben@geanix.com --- Esben Haabendal (6): rtc: interface: Fix long-standing race when setting alarm rtc: isl12022: Fix initial enable_irq/disable_irq balance rtc: cpcap: Fix initial enable_irq/disable_irq balance rtc: st-lpc: Fix initial enable_irq/disable_irq balance rtc: tps6586x: Fix initial enable_irq/disable_irq balance rtc: interface: Ensure alarm irq is enabled when UIE is enabled
drivers/rtc/interface.c | 27 +++++++++++++++++++++++++++ drivers/rtc/rtc-cpcap.c | 1 + drivers/rtc/rtc-isl12022.c | 1 + drivers/rtc/rtc-st-lpc.c | 1 + drivers/rtc/rtc-tps6586x.c | 1 + 5 files changed, 31 insertions(+) --- base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37 change-id: 20241203-rtc-uie-irq-fixes-f2838782d0f8
Best regards,
As described in the old comment dating back to commit 6610e0893b8b ("RTC: Rework RTC code to use timerqueue for events") from 2010, we have been living with a race window when setting alarm with an expiry in the near future (i.e. next second). With 1 second resolution, it can happen that the second ticks after the check for the timer having expired, but before the alarm is actually set. When this happen, no alarm IRQ is generated, at least not with some RTC chips (isl12022 is an example of this).
With UIE RTC timer being implemented on top of alarm irq, being re-armed every second, UIE will occasionally fail to work, as an alarm irq lost due to this race will stop the re-arming loop.
For now, I have limited the additional expiry check to only be done for alarms set to next seconds. I expect it should be good enough, although I don't know if we can now for sure that systems with loads could end up causing the same problems for alarms set 2 seconds or even longer in the future.
I haven't been able to reproduce the problem with this check in place.
Cc: stable@vger.kernel.org Signed-off-by: Esben Haabendal esben@geanix.com --- drivers/rtc/interface.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c index aaf76406cd7d7d2cfd5479fc1fc892fcb5efbb6b..e365e8fd166db31f8b44fac9fb923d36881b1394 100644 --- a/drivers/rtc/interface.c +++ b/drivers/rtc/interface.c @@ -443,6 +443,29 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm) else err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
+ /* + * Check for potential race described above. If the waiting for next + * second, and the second just ticked since the check above, either + * + * 1) It ticked after the alarm was set, and an alarm irq should be + * generated. + * + * 2) It ticked before the alarm was set, and alarm irq most likely will + * not be generated. + * + * While we cannot easily check for which of these two scenarios we + * are in, we can return -ETIME to signal that the timer has already + * expired, which is true in both cases. + */ + if ((scheduled - now) <= 1) { + err = __rtc_read_time(rtc, &tm); + if (err) + return err; + now = rtc_tm_to_time64(&tm); + if (scheduled <= now) + return -ETIME; + } + trace_rtc_set_alarm(rtc_tm_to_time64(&alarm->time), err); return err; }
When setting a normal alarm, user-space is responsible for using RTC_AIE_ON/RTC_AIE_OFF to control if alarm irq should be enabled.
But when RTC_UIE_ON is used, interrupts must be so that the requested irq events are generated. When RTC_UIE_OFF is used, alarm irq is disabled if there are no other alarms queued, so this commit brings symmetry to that.
Signed-off-by: Esben Haabendal esben@geanix.com Cc: stable@vger.kernel.org --- drivers/rtc/interface.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c index e365e8fd166db31f8b44fac9fb923d36881b1394..39db12f267cc627febb78e67400aaf8fc3301b0c 100644 --- a/drivers/rtc/interface.c +++ b/drivers/rtc/interface.c @@ -617,6 +617,10 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled) rtc->uie_rtctimer.node.expires = ktime_add(now, onesec); rtc->uie_rtctimer.period = ktime_set(1, 0); err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer); + if (!err && rtc->ops && rtc->ops->alarm_irq_enable) + err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1); + if (err) + goto out; } else { rtc_timer_remove(rtc, &rtc->uie_rtctimer); }
Esben Haabendal esben@geanix.com writes:
This fixes a couple of different problems, that can cause RTC (alarm) irqs to be missing when generating UIE interrupts.
The first commit fixes a long-standing problem, which has been documented in a comment since 2010. This fixes a race that could cause UIE irqs to stop being generated, which was easily reproduced by timing the use of RTC_UIE_ON ioctl with the seconds tick in the RTC.
The last commit ensures that RTC (alarm) irqs are enabled whenever RTC_UIE_ON ioctl is used.
The driver specific commits avoids kernel warnings about unbalanced enable_irq/disable_irq, which gets triggered on first RTC_UIE_ON with the last commit. Before this series, the same warning should be seen on initial RTC_AIE_ON with those drivers.
I don't have access to hardware using cpcap, st-lpc or tps6586x rtc drivers, so I have not been able to test those 3 patches.
/Esben
Signed-off-by: Esben Haabendal esben@geanix.com
Esben Haabendal (6): rtc: interface: Fix long-standing race when setting alarm rtc: isl12022: Fix initial enable_irq/disable_irq balance rtc: cpcap: Fix initial enable_irq/disable_irq balance rtc: st-lpc: Fix initial enable_irq/disable_irq balance rtc: tps6586x: Fix initial enable_irq/disable_irq balance rtc: interface: Ensure alarm irq is enabled when UIE is enabled
drivers/rtc/interface.c | 27 +++++++++++++++++++++++++++ drivers/rtc/rtc-cpcap.c | 1 + drivers/rtc/rtc-isl12022.c | 1 + drivers/rtc/rtc-st-lpc.c | 1 + drivers/rtc/rtc-tps6586x.c | 1 + 5 files changed, 31 insertions(+)
base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37 change-id: 20241203-rtc-uie-irq-fixes-f2838782d0f8
Best regards,
linux-stable-mirror@lists.linaro.org
-
Esben Haabendal