Re: [PATCH v2 1/2] rtc-cmos: Ensure no expired alarm is left enabled after resume

[Gabriele Mazzotta]

On 01/09/2016 00:58, Gabriele Mazzotta wrote:
> Some BIOSes, such as the one of the Dell XPS13 9333, wake the system
> when an alarm goes off without informing the OS. If any of the

A clarification on this first sentence. I was looking at the ACPI
specification [1] and it seems that there are two optional bits
that can be used to determine if the RTC alarm was the reason of
the resume. However, the value reported can be inaccurate when
resuming from S4 (there's an additional flag to know if the value
is accurate or not), so a fallback mechanism such as the one of
this patch is probably still needed.

I will have a better look at this.

Regards,
Gabriele

[1] http://www.acpi.info/DOWNLOADS/ACPI_5_Errata%20A.pdf

> RTC_IRQMASK bits is set when this happens and hpet is enabled,
> the alarm is not automatically cleared at resume. As consequence,
> the user must manually clear the alarm writing 0 to wakealarm
> before being able to set a new alarm. Ensure this never happens.
> 
> Signed-off-by: Gabriele Mazzotta <gabriele.mzt@xxxxxxxxx>
> ---
>  drivers/rtc/rtc-cmos.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> index 1dec52f..62d5b33 100644
> --- a/drivers/rtc/rtc-cmos.c
> +++ b/drivers/rtc/rtc-cmos.c
> @@ -900,10 +900,32 @@ static inline int cmos_poweroff(struct device *dev)
>  
>  #ifdef	CONFIG_PM_SLEEP
>  
> +static bool cmos_is_wkalrm_expired(struct device *dev)
> +{
> +	struct cmos_rtc *cmos = dev_get_drvdata(dev);
> +	struct rtc_wkalrm alarm;
> +	struct rtc_time now;
> +	time64_t t_now;
> +	time64_t t_expires;
> +	int ret;
> +
> +	ret = rtc_read_alarm(cmos->rtc, &alarm);
> +	if (ret) {
> +		dev_err(dev, "Could not read alarm: err=%d\n", ret);
> +		return false;
> +	}
> +	t_expires = rtc_tm_to_time64(&alarm.time);
> +	cmos_read_time(dev, &now);
> +	t_now = rtc_tm_to_time64(&now);
> +
> +	return t_expires <= t_now;
> +}
> +
>  static int cmos_resume(struct device *dev)
>  {
>  	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
>  	unsigned char tmp;
> +	bool is_wkalrm_expired;
>  
>  	if (cmos->enabled_wake) {
>  		if (cmos->wake_off)
> @@ -913,6 +935,8 @@ static int cmos_resume(struct device *dev)
>  		cmos->enabled_wake = 0;
>  	}
>  
> +	is_wkalrm_expired = cmos_is_wkalrm_expired(dev);
> +
>  	spin_lock_irq(&rtc_lock);
>  	tmp = cmos->suspend_ctrl;
>  	cmos->suspend_ctrl = 0;
> @@ -939,6 +963,17 @@ static int cmos_resume(struct device *dev)
>  			tmp &= ~RTC_AIE;
>  			hpet_mask_rtc_irq_bit(RTC_AIE);
>  		} while (mask & RTC_AIE);
> +
> +		/*
> +		 * If RTC_AIE is set and we have an alarm set to go off in the
> +		 * past, then the BIOS woke the system when the alarm went off
> +		 * and we now have to clear it.
> +		 */
> +		if ((tmp & RTC_AIE) && is_wkalrm_expired) {
> +			rtc_update_irq(cmos->rtc, 1, mask);
> +			tmp &= ~RTC_AIE;
> +			CMOS_WRITE(tmp, RTC_CONTROL);
> +		}
>  	}
>  	spin_unlock_irq(&rtc_lock);
>  
>