Hello, I assume I can ignore this patch as this seems to be only for testing I'm not even sure why this is needed as this completely breaks setting the alarm time. On 05/05/2022 09:58:14+0800, Zhang Rui wrote: > Automated suspend/resume testing uses the RTC for wakeup. > A short rtcwake period is desirable, so that more suspend/resume > cycles can be completed, while the machine is available for testing. > > But if too short a wake interval is specified, the event can occur, > while still suspending, and then no event wakes the suspended system > until the user notices that testing has stalled, and manually intervenes. > > Here we add a hook to the rtc-cmos driver to > a) remove the alarm timer in the beginning of suspend, if there is any > b) arm the wakeup in PM notifier callback, which is in the very late stage > before the system really suspends > The remaining part of suspend is usually measured under 10 ms, > and so arming the timer at this point could be as fast as the minimum > time of 1-second. > > But there is a 2nd race. The RTC has 1-second granularity, and unless > you are timing the timer with a higher resolution timer, > there is no telling if the current time + 1 will occur immediately, > or a full second in the future. And so 2-seconds is the safest minimum: > > Run 1,000 s2idle cycles with (max of) 2 second wakeup period: > > # echo 2 > /sys/module/rtc_cmos/parameters/rtc_wake_override_sec > # sleepgraph.py -m freeze -multi 1000 0 -skiphtml -gzip > > Clear the timer override, to not interfere with subsequent > real use of the machine's suspend/resume feature: > > # echo 0 > /sys/module/rtc_cmos/parameters/rtc_wake_override_sec > > Originally-by: Len Brown <len.brown@xxxxxxxxx> > Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx> > Tested-by: Sumeet Pawnikar <sumeet.r.pawnikar@xxxxxxxxx> > --- > drivers/rtc/interface.c | 1 + > drivers/rtc/rtc-cmos.c | 45 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c > index 9edd662c69ac..fb93aa2dc99c 100644 > --- a/drivers/rtc/interface.c > +++ b/drivers/rtc/interface.c > @@ -1020,6 +1020,7 @@ void rtc_timer_cancel(struct rtc_device *rtc, struct rtc_timer *timer) > rtc_timer_remove(rtc, timer); > mutex_unlock(&rtc->ops_lock); > } > +EXPORT_SYMBOL_GPL(rtc_timer_cancel); > > /** > * rtc_read_offset - Read the amount of rtc offset in parts per billion > diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c > index 7c006c2b125f..9590c40fa9d8 100644 > --- a/drivers/rtc/rtc-cmos.c > +++ b/drivers/rtc/rtc-cmos.c > @@ -32,6 +32,7 @@ > #include <linux/init.h> > #include <linux/interrupt.h> > #include <linux/spinlock.h> > +#include <linux/suspend.h> > #include <linux/platform_device.h> > #include <linux/log2.h> > #include <linux/pm.h> > @@ -70,6 +71,9 @@ static inline int cmos_use_acpi_alarm(void) > } > #endif > > +static int rtc_wake_override_sec; > +module_param(rtc_wake_override_sec, int, 0644); > + > struct cmos_rtc { > struct rtc_device *rtc; > struct device *dev; > @@ -89,6 +93,7 @@ struct cmos_rtc { > u8 century; > > struct rtc_wkalrm saved_wkalrm; > + struct notifier_block pm_nb; > }; > > /* both platform and pnp busses use negative numbers for invalid irqs */ > @@ -744,6 +749,42 @@ static irqreturn_t cmos_interrupt(int irq, void *p) > return IRQ_NONE; > } > > +static int cmos_pm_notify(struct notifier_block *nb, unsigned long mode, void *_unused) > +{ > + struct cmos_rtc *cmos = container_of(nb, struct cmos_rtc, pm_nb); > + struct rtc_device *rtc = cmos->rtc; > + unsigned long now; > + struct rtc_wkalrm alm; > + > + if (rtc_wake_override_sec == 0) > + return NOTIFY_OK; > + > + switch (mode) { > + case PM_SUSPEND_PREPARE: > + /* > + * Cancel the timer to make sure it won't fire > + * before rtc is rearmed later. > + */ > + rtc_timer_cancel(rtc, &rtc->aie_timer); > + break; > + case PM_SUSPEND_LATE: > + if (rtc_read_time(rtc, &alm.time)) > + return NOTIFY_BAD; > + > + now = rtc_tm_to_time64(&alm.time); > + memset(&alm, 0, sizeof(alm)); > + rtc_time64_to_tm(now + rtc_wake_override_sec, &alm.time); > + alm.enabled = true; > + if (rtc_set_alarm(rtc, &alm)) > + return NOTIFY_BAD; > + if (cmos->wake_on) > + cmos->wake_on(cmos->dev); > + break; > + } > + > + return NOTIFY_OK; > +} > + > #ifdef CONFIG_PNP > #define INITSECTION > > @@ -937,6 +978,9 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) > nvmem_cfg.size, > use_hpet_alarm() ? ", hpet irqs" : ""); > > + cmos_rtc.pm_nb.notifier_call = cmos_pm_notify; > + register_pm_notifier(&cmos_rtc.pm_nb); > + > return 0; > > cleanup2: > @@ -965,6 +1009,7 @@ static void cmos_do_remove(struct device *dev) > struct cmos_rtc *cmos = dev_get_drvdata(dev); > struct resource *ports; > > + unregister_pm_notifier(&cmos_rtc.pm_nb); > cmos_do_shutdown(cmos->irq); > > if (is_valid_irq(cmos->irq)) { > -- > 2.17.1 > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com