Re: [PATCH 7/7] rtc: cmos: Add suspend/resume endurance testing hook

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

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@intel.com>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> Tested-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com>
> ---
>  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