All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/2] rtc-cmos: Clear ACPI-driven alarms upon resume
@ 2016-09-19 23:12 ` Gabriele Mazzotta
  0 siblings, 0 replies; 8+ messages in thread
From: Gabriele Mazzotta @ 2016-09-19 23:12 UTC (permalink / raw)
  To: alexandre.belloni, a.zummo
  Cc: rtc-linux, linux-kernel, matthew.garrett, Gabriele Mazzotta

Currently ACPI-driven alarms are not cleared when they wake the
system. As consequence, expired alarms must be manually cleared to
program a new alarm. Fix this by correctly handling ACPI-driven
alarms.

More specifically, the ACPI specification [1] provides for two
alternative implementations of the RTC. Depending on the
implementation, the driver either clear the alarm from the resume
callback or from ACPI interrupt handler:

 - The platform has the RTC wakeup status fixed in hardware
   (ACPI_FADT_FIXED_RTC is 0). In this case the driver can determine
   if the RTC was the reason of the wakeup from the resume callback
   by reading the RTC status register.

 - The platform has no fixed hardware feature event bits. In this
   case a GPE is used to wake the system and the driver clears the
   alarm from its handler.

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

Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/rtc/rtc-cmos.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 1dec52f..fd7ec0a 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -900,6 +900,9 @@ static inline int cmos_poweroff(struct device *dev)
 
 #ifdef	CONFIG_PM_SLEEP
 
+static void cmos_check_acpi_rtc_status(struct device *dev,
+				       unsigned char *rtc_control);
+
 static int cmos_resume(struct device *dev)
 {
 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
@@ -939,6 +942,9 @@ static int cmos_resume(struct device *dev)
 			tmp &= ~RTC_AIE;
 			hpet_mask_rtc_irq_bit(RTC_AIE);
 		} while (mask & RTC_AIE);
+
+		if (tmp & RTC_AIE)
+			cmos_check_acpi_rtc_status(dev, &tmp);
 	}
 	spin_unlock_irq(&rtc_lock);
 
@@ -976,6 +982,22 @@ static SIMPLE_DEV_PM_OPS(cmos_pm_ops, cmos_suspend, cmos_resume);
 static u32 rtc_handler(void *context)
 {
 	struct device *dev = context;
+	struct cmos_rtc *cmos = dev_get_drvdata(dev);
+	unsigned char rtc_control;
+	unsigned char rtc_intr;
+
+	spin_lock_irq(&rtc_lock);
+	if (!cmos_rtc.suspend_ctrl)
+		rtc_control = cmos_rtc.suspend_ctrl;
+	else
+		rtc_control = CMOS_READ(RTC_CONTROL);
+	if (rtc_control & RTC_AIE) {
+		cmos_rtc.suspend_ctrl &= ~RTC_AIE;
+		CMOS_WRITE(rtc_control, RTC_CONTROL);
+		rtc_intr = CMOS_READ(RTC_INTR_FLAGS);
+		rtc_update_irq(cmos->rtc, 1, rtc_intr);
+	}
+	spin_unlock_irq(&rtc_lock);
 
 	pm_wakeup_event(dev, 0);
 	acpi_clear_event(ACPI_EVENT_RTC);
@@ -1042,12 +1064,39 @@ static void cmos_wake_setup(struct device *dev)
 	device_init_wakeup(dev, 1);
 }
 
+static void cmos_check_acpi_rtc_status(struct device *dev,
+				       unsigned char *rtc_control)
+{
+	struct cmos_rtc *cmos = dev_get_drvdata(dev);
+	acpi_event_status rtc_status;
+	acpi_status status;
+
+	if (acpi_gbl_FADT.flags & ACPI_FADT_FIXED_RTC)
+		return;
+
+	status = acpi_get_event_status(ACPI_EVENT_RTC, &rtc_status);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "Could not get RTC status\n");
+	} else if (rtc_status & ACPI_EVENT_FLAG_SET) {
+		unsigned char mask;
+		*rtc_control &= ~RTC_AIE;
+		CMOS_WRITE(*rtc_control, RTC_CONTROL);
+		mask = CMOS_READ(RTC_INTR_FLAGS);
+		rtc_update_irq(cmos->rtc, 1, mask);
+	}
+}
+
 #else
 
 static void cmos_wake_setup(struct device *dev)
 {
 }
 
+static void cmos_check_acpi_rtc_status(struct device *dev,
+				       unsigned char *rtc_control)
+{
+}
+
 #endif
 
 #ifdef	CONFIG_PNP
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [rtc-linux] [PATCH v4 1/2] rtc-cmos: Clear ACPI-driven alarms upon resume
@ 2016-09-19 23:12 ` Gabriele Mazzotta
  0 siblings, 0 replies; 8+ messages in thread
From: Gabriele Mazzotta @ 2016-09-19 23:12 UTC (permalink / raw)
  To: alexandre.belloni, a.zummo
  Cc: rtc-linux, linux-kernel, matthew.garrett, Gabriele Mazzotta

Currently ACPI-driven alarms are not cleared when they wake the
system. As consequence, expired alarms must be manually cleared to
program a new alarm. Fix this by correctly handling ACPI-driven
alarms.

More specifically, the ACPI specification [1] provides for two
alternative implementations of the RTC. Depending on the
implementation, the driver either clear the alarm from the resume
callback or from ACPI interrupt handler:

 - The platform has the RTC wakeup status fixed in hardware
   (ACPI_FADT_FIXED_RTC is 0). In this case the driver can determine
   if the RTC was the reason of the wakeup from the resume callback
   by reading the RTC status register.

 - The platform has no fixed hardware feature event bits. In this
   case a GPE is used to wake the system and the driver clears the
   alarm from its handler.

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

Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/rtc/rtc-cmos.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 1dec52f..fd7ec0a 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -900,6 +900,9 @@ static inline int cmos_poweroff(struct device *dev)
 
 #ifdef	CONFIG_PM_SLEEP
 
+static void cmos_check_acpi_rtc_status(struct device *dev,
+				       unsigned char *rtc_control);
+
 static int cmos_resume(struct device *dev)
 {
 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
@@ -939,6 +942,9 @@ static int cmos_resume(struct device *dev)
 			tmp &= ~RTC_AIE;
 			hpet_mask_rtc_irq_bit(RTC_AIE);
 		} while (mask & RTC_AIE);
+
+		if (tmp & RTC_AIE)
+			cmos_check_acpi_rtc_status(dev, &tmp);
 	}
 	spin_unlock_irq(&rtc_lock);
 
@@ -976,6 +982,22 @@ static SIMPLE_DEV_PM_OPS(cmos_pm_ops, cmos_suspend, cmos_resume);
 static u32 rtc_handler(void *context)
 {
 	struct device *dev = context;
+	struct cmos_rtc *cmos = dev_get_drvdata(dev);
+	unsigned char rtc_control;
+	unsigned char rtc_intr;
+
+	spin_lock_irq(&rtc_lock);
+	if (!cmos_rtc.suspend_ctrl)
+		rtc_control = cmos_rtc.suspend_ctrl;
+	else
+		rtc_control = CMOS_READ(RTC_CONTROL);
+	if (rtc_control & RTC_AIE) {
+		cmos_rtc.suspend_ctrl &= ~RTC_AIE;
+		CMOS_WRITE(rtc_control, RTC_CONTROL);
+		rtc_intr = CMOS_READ(RTC_INTR_FLAGS);
+		rtc_update_irq(cmos->rtc, 1, rtc_intr);
+	}
+	spin_unlock_irq(&rtc_lock);
 
 	pm_wakeup_event(dev, 0);
 	acpi_clear_event(ACPI_EVENT_RTC);
@@ -1042,12 +1064,39 @@ static void cmos_wake_setup(struct device *dev)
 	device_init_wakeup(dev, 1);
 }
 
+static void cmos_check_acpi_rtc_status(struct device *dev,
+				       unsigned char *rtc_control)
+{
+	struct cmos_rtc *cmos = dev_get_drvdata(dev);
+	acpi_event_status rtc_status;
+	acpi_status status;
+
+	if (acpi_gbl_FADT.flags & ACPI_FADT_FIXED_RTC)
+		return;
+
+	status = acpi_get_event_status(ACPI_EVENT_RTC, &rtc_status);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "Could not get RTC status\n");
+	} else if (rtc_status & ACPI_EVENT_FLAG_SET) {
+		unsigned char mask;
+		*rtc_control &= ~RTC_AIE;
+		CMOS_WRITE(*rtc_control, RTC_CONTROL);
+		mask = CMOS_READ(RTC_INTR_FLAGS);
+		rtc_update_irq(cmos->rtc, 1, mask);
+	}
+}
+
 #else
 
 static void cmos_wake_setup(struct device *dev)
 {
 }
 
+static void cmos_check_acpi_rtc_status(struct device *dev,
+				       unsigned char *rtc_control)
+{
+}
+
 #endif
 
 #ifdef	CONFIG_PNP
-- 
2.9.3

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/2] rtc-cmos: Restore alarm after resume
  2016-09-19 23:12 ` [rtc-linux] " Gabriele Mazzotta
@ 2016-09-19 23:12   ` Gabriele Mazzotta
  -1 siblings, 0 replies; 8+ messages in thread
From: Gabriele Mazzotta @ 2016-09-19 23:12 UTC (permalink / raw)
  To: alexandre.belloni, a.zummo
  Cc: rtc-linux, linux-kernel, matthew.garrett, Gabriele Mazzotta

Some platform firmware may interfere with the RTC alarm over suspend,
resulting in the kernel and hardware having different ideas about system
state but also potentially causing problems with firmware that assumes the
OS will clean this case up.  This patch restores the RTC alarm on resume
to ensure that kernel and hardware are in sync.

The case we've seen is Intel Rapid Start, which is a firmware-mediated
feature that automatically transitions systems from suspend-to-RAM to
suspend-to-disk without OS involvement.  It does this by setting the RTC
alarm and a flag that indicates that on wake it should perform the
transition rather than re-starting the OS.  However, if the OS has set a
wakeup alarm that would wake the machine earlier, it refuses to overwrite
it and allows the system to wake instead.

This fails in the following situation:

1) User configures Intel Rapid Start to transition after (say) 15
minutes
2) User suspends to RAM. Firmware sets the wakeup alarm for 15 minutes
in the future
3) User resumes after 5 minutes. Firmware does not reset the alarm, and
as such it is still set for 10 minutes in the future
4) User suspends after 5 minutes. Firmware notices that the alarm is set
for 5 minutes in the future, which is less than the 15 minute transition
threshold. It therefore assumes that the user wants the machine to wake
in 5 minutes
5) System resumes after 5 minutes

The worst case scenario here is that the user may have put the system in a
bag between (4) and (5), resulting in it running in a confined space and
potentially overheating.  This seems reasonably important.  The Rapid
Start support code got added in 3.11, but it can be configured in the
firmware regardless of kernel support.

Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/rtc/rtc-cmos.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index fd7ec0a..6a25776 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -62,6 +62,8 @@ struct cmos_rtc {
 	u8			day_alrm;
 	u8			mon_alrm;
 	u8			century;
+
+	struct rtc_wkalrm	saved_wkalrm;
 };
 
 /* both platform and pnp busses use negative numbers for invalid irqs */
@@ -880,6 +882,8 @@ static int cmos_suspend(struct device *dev)
 			enable_irq_wake(cmos->irq);
 	}
 
+	cmos_read_alarm(dev, &cmos->saved_wkalrm);
+
 	dev_dbg(dev, "suspend%s, ctrl %02x\n",
 			(tmp & RTC_AIE) ? ", alarm may wake" : "",
 			tmp);
@@ -900,6 +904,22 @@ static inline int cmos_poweroff(struct device *dev)
 
 #ifdef	CONFIG_PM_SLEEP
 
+static void cmos_check_wkalrm(struct device *dev)
+{
+	struct cmos_rtc *cmos = dev_get_drvdata(dev);
+	struct rtc_wkalrm current_alarm;
+	time64_t t_current_expires;
+	time64_t t_saved_expires;
+
+	cmos_read_alarm(dev, &current_alarm);
+	t_current_expires = rtc_tm_to_time64(&current_alarm.time);
+	t_saved_expires = rtc_tm_to_time64(&cmos->saved_wkalrm.time);
+	if (t_current_expires != t_saved_expires ||
+	    cmos->saved_wkalrm.enabled != current_alarm.enabled) {
+		cmos_set_alarm(dev, &cmos->saved_wkalrm);
+	}
+}
+
 static void cmos_check_acpi_rtc_status(struct device *dev,
 				       unsigned char *rtc_control);
 
@@ -916,6 +936,9 @@ static int cmos_resume(struct device *dev)
 		cmos->enabled_wake = 0;
 	}
 
+	/* The BIOS might have changed the alarm, restore it */
+	cmos_check_wkalrm(dev);
+
 	spin_lock_irq(&rtc_lock);
 	tmp = cmos->suspend_ctrl;
 	cmos->suspend_ctrl = 0;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [rtc-linux] [PATCH v4 2/2] rtc-cmos: Restore alarm after resume
@ 2016-09-19 23:12   ` Gabriele Mazzotta
  0 siblings, 0 replies; 8+ messages in thread
From: Gabriele Mazzotta @ 2016-09-19 23:12 UTC (permalink / raw)
  To: alexandre.belloni, a.zummo
  Cc: rtc-linux, linux-kernel, matthew.garrett, Gabriele Mazzotta

Some platform firmware may interfere with the RTC alarm over suspend,
resulting in the kernel and hardware having different ideas about system
state but also potentially causing problems with firmware that assumes the
OS will clean this case up.  This patch restores the RTC alarm on resume
to ensure that kernel and hardware are in sync.

The case we've seen is Intel Rapid Start, which is a firmware-mediated
feature that automatically transitions systems from suspend-to-RAM to
suspend-to-disk without OS involvement.  It does this by setting the RTC
alarm and a flag that indicates that on wake it should perform the
transition rather than re-starting the OS.  However, if the OS has set a
wakeup alarm that would wake the machine earlier, it refuses to overwrite
it and allows the system to wake instead.

This fails in the following situation:

1) User configures Intel Rapid Start to transition after (say) 15
minutes
2) User suspends to RAM. Firmware sets the wakeup alarm for 15 minutes
in the future
3) User resumes after 5 minutes. Firmware does not reset the alarm, and
as such it is still set for 10 minutes in the future
4) User suspends after 5 minutes. Firmware notices that the alarm is set
for 5 minutes in the future, which is less than the 15 minute transition
threshold. It therefore assumes that the user wants the machine to wake
in 5 minutes
5) System resumes after 5 minutes

The worst case scenario here is that the user may have put the system in a
bag between (4) and (5), resulting in it running in a confined space and
potentially overheating.  This seems reasonably important.  The Rapid
Start support code got added in 3.11, but it can be configured in the
firmware regardless of kernel support.

Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/rtc/rtc-cmos.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index fd7ec0a..6a25776 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -62,6 +62,8 @@ struct cmos_rtc {
 	u8			day_alrm;
 	u8			mon_alrm;
 	u8			century;
+
+	struct rtc_wkalrm	saved_wkalrm;
 };
 
 /* both platform and pnp busses use negative numbers for invalid irqs */
@@ -880,6 +882,8 @@ static int cmos_suspend(struct device *dev)
 			enable_irq_wake(cmos->irq);
 	}
 
+	cmos_read_alarm(dev, &cmos->saved_wkalrm);
+
 	dev_dbg(dev, "suspend%s, ctrl %02x\n",
 			(tmp & RTC_AIE) ? ", alarm may wake" : "",
 			tmp);
@@ -900,6 +904,22 @@ static inline int cmos_poweroff(struct device *dev)
 
 #ifdef	CONFIG_PM_SLEEP
 
+static void cmos_check_wkalrm(struct device *dev)
+{
+	struct cmos_rtc *cmos = dev_get_drvdata(dev);
+	struct rtc_wkalrm current_alarm;
+	time64_t t_current_expires;
+	time64_t t_saved_expires;
+
+	cmos_read_alarm(dev, &current_alarm);
+	t_current_expires = rtc_tm_to_time64(&current_alarm.time);
+	t_saved_expires = rtc_tm_to_time64(&cmos->saved_wkalrm.time);
+	if (t_current_expires != t_saved_expires ||
+	    cmos->saved_wkalrm.enabled != current_alarm.enabled) {
+		cmos_set_alarm(dev, &cmos->saved_wkalrm);
+	}
+}
+
 static void cmos_check_acpi_rtc_status(struct device *dev,
 				       unsigned char *rtc_control);
 
@@ -916,6 +936,9 @@ static int cmos_resume(struct device *dev)
 		cmos->enabled_wake = 0;
 	}
 
+	/* The BIOS might have changed the alarm, restore it */
+	cmos_check_wkalrm(dev);
+
 	spin_lock_irq(&rtc_lock);
 	tmp = cmos->suspend_ctrl;
 	cmos->suspend_ctrl = 0;
-- 
2.9.3

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] rtc-cmos: Clear ACPI-driven alarms upon resume
  2016-09-19 23:12 ` [rtc-linux] " Gabriele Mazzotta
@ 2016-09-21 20:07   ` Alexandre Belloni
  -1 siblings, 0 replies; 8+ messages in thread
From: Alexandre Belloni @ 2016-09-21 20:07 UTC (permalink / raw)
  To: Gabriele Mazzotta; +Cc: a.zummo, rtc-linux, linux-kernel, matthew.garrett

Hi,

Thank you for your perseverance.

On 20/09/2016 at 01:12:43 +0200, Gabriele Mazzotta wrote :
> Currently ACPI-driven alarms are not cleared when they wake the
> system. As consequence, expired alarms must be manually cleared to
> program a new alarm. Fix this by correctly handling ACPI-driven
> alarms.
> 
> More specifically, the ACPI specification [1] provides for two
> alternative implementations of the RTC. Depending on the
> implementation, the driver either clear the alarm from the resume
> callback or from ACPI interrupt handler:
> 
>  - The platform has the RTC wakeup status fixed in hardware
>    (ACPI_FADT_FIXED_RTC is 0). In this case the driver can determine
>    if the RTC was the reason of the wakeup from the resume callback
>    by reading the RTC status register.
> 
>  - The platform has no fixed hardware feature event bits. In this
>    case a GPE is used to wake the system and the driver clears the
>    alarm from its handler.
> 
> [1] http://www.acpi.info/DOWNLOADS/ACPI_5_Errata%20A.pdf
> 
> Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> ---
>  drivers/rtc/rtc-cmos.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)

Applied, with the following cosmetic change:
@@ -982,13 +982,11 @@ static u32 rtc_handler(void *context)
 {
        struct device *dev = context;
        struct cmos_rtc *cmos = dev_get_drvdata(dev);
-       unsigned char rtc_control;
+       unsigned char rtc_control = 0;
        unsigned char rtc_intr;
 
        spin_lock_irq(&rtc_lock);
-       if (!cmos_rtc.suspend_ctrl)
-               rtc_control = cmos_rtc.suspend_ctrl;
-       else
+       if (cmos_rtc.suspend_ctrl)
                rtc_control = CMOS_READ(RTC_CONTROL);
        if (rtc_control & RTC_AIE) {
                cmos_rtc.suspend_ctrl &= ~RTC_AIE;



-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [rtc-linux] Re: [PATCH v4 1/2] rtc-cmos: Clear ACPI-driven alarms upon resume
@ 2016-09-21 20:07   ` Alexandre Belloni
  0 siblings, 0 replies; 8+ messages in thread
From: Alexandre Belloni @ 2016-09-21 20:07 UTC (permalink / raw)
  To: Gabriele Mazzotta; +Cc: a.zummo, rtc-linux, linux-kernel, matthew.garrett

Hi,

Thank you for your perseverance.

On 20/09/2016 at 01:12:43 +0200, Gabriele Mazzotta wrote :
> Currently ACPI-driven alarms are not cleared when they wake the
> system. As consequence, expired alarms must be manually cleared to
> program a new alarm. Fix this by correctly handling ACPI-driven
> alarms.
> 
> More specifically, the ACPI specification [1] provides for two
> alternative implementations of the RTC. Depending on the
> implementation, the driver either clear the alarm from the resume
> callback or from ACPI interrupt handler:
> 
>  - The platform has the RTC wakeup status fixed in hardware
>    (ACPI_FADT_FIXED_RTC is 0). In this case the driver can determine
>    if the RTC was the reason of the wakeup from the resume callback
>    by reading the RTC status register.
> 
>  - The platform has no fixed hardware feature event bits. In this
>    case a GPE is used to wake the system and the driver clears the
>    alarm from its handler.
> 
> [1] http://www.acpi.info/DOWNLOADS/ACPI_5_Errata%20A.pdf
> 
> Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> ---
>  drivers/rtc/rtc-cmos.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)

Applied, with the following cosmetic change:
@@ -982,13 +982,11 @@ static u32 rtc_handler(void *context)
 {
        struct device *dev = context;
        struct cmos_rtc *cmos = dev_get_drvdata(dev);
-       unsigned char rtc_control;
+       unsigned char rtc_control = 0;
        unsigned char rtc_intr;
 
        spin_lock_irq(&rtc_lock);
-       if (!cmos_rtc.suspend_ctrl)
-               rtc_control = cmos_rtc.suspend_ctrl;
-       else
+       if (cmos_rtc.suspend_ctrl)
                rtc_control = CMOS_READ(RTC_CONTROL);
        if (rtc_control & RTC_AIE) {
                cmos_rtc.suspend_ctrl &= ~RTC_AIE;



-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] rtc-cmos: Restore alarm after resume
  2016-09-19 23:12   ` [rtc-linux] " Gabriele Mazzotta
@ 2016-09-21 20:09     ` Alexandre Belloni
  -1 siblings, 0 replies; 8+ messages in thread
From: Alexandre Belloni @ 2016-09-21 20:09 UTC (permalink / raw)
  To: Gabriele Mazzotta; +Cc: a.zummo, rtc-linux, linux-kernel, matthew.garrett

On 20/09/2016 at 01:12:44 +0200, Gabriele Mazzotta wrote :
> Some platform firmware may interfere with the RTC alarm over suspend,
> resulting in the kernel and hardware having different ideas about system
> state but also potentially causing problems with firmware that assumes the
> OS will clean this case up.  This patch restores the RTC alarm on resume
> to ensure that kernel and hardware are in sync.
> 
> The case we've seen is Intel Rapid Start, which is a firmware-mediated
> feature that automatically transitions systems from suspend-to-RAM to
> suspend-to-disk without OS involvement.  It does this by setting the RTC
> alarm and a flag that indicates that on wake it should perform the
> transition rather than re-starting the OS.  However, if the OS has set a
> wakeup alarm that would wake the machine earlier, it refuses to overwrite
> it and allows the system to wake instead.
> 
> This fails in the following situation:
> 
> 1) User configures Intel Rapid Start to transition after (say) 15
> minutes
> 2) User suspends to RAM. Firmware sets the wakeup alarm for 15 minutes
> in the future
> 3) User resumes after 5 minutes. Firmware does not reset the alarm, and
> as such it is still set for 10 minutes in the future
> 4) User suspends after 5 minutes. Firmware notices that the alarm is set
> for 5 minutes in the future, which is less than the 15 minute transition
> threshold. It therefore assumes that the user wants the machine to wake
> in 5 minutes
> 5) System resumes after 5 minutes
> 
> The worst case scenario here is that the user may have put the system in a
> bag between (4) and (5), resulting in it running in a confined space and
> potentially overheating.  This seems reasonably important.  The Rapid
> Start support code got added in 3.11, but it can be configured in the
> firmware regardless of kernel support.
> 
> Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> ---
>  drivers/rtc/rtc-cmos.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [rtc-linux] Re: [PATCH v4 2/2] rtc-cmos: Restore alarm after resume
@ 2016-09-21 20:09     ` Alexandre Belloni
  0 siblings, 0 replies; 8+ messages in thread
From: Alexandre Belloni @ 2016-09-21 20:09 UTC (permalink / raw)
  To: Gabriele Mazzotta; +Cc: a.zummo, rtc-linux, linux-kernel, matthew.garrett

On 20/09/2016 at 01:12:44 +0200, Gabriele Mazzotta wrote :
> Some platform firmware may interfere with the RTC alarm over suspend,
> resulting in the kernel and hardware having different ideas about system
> state but also potentially causing problems with firmware that assumes the
> OS will clean this case up.  This patch restores the RTC alarm on resume
> to ensure that kernel and hardware are in sync.
> 
> The case we've seen is Intel Rapid Start, which is a firmware-mediated
> feature that automatically transitions systems from suspend-to-RAM to
> suspend-to-disk without OS involvement.  It does this by setting the RTC
> alarm and a flag that indicates that on wake it should perform the
> transition rather than re-starting the OS.  However, if the OS has set a
> wakeup alarm that would wake the machine earlier, it refuses to overwrite
> it and allows the system to wake instead.
> 
> This fails in the following situation:
> 
> 1) User configures Intel Rapid Start to transition after (say) 15
> minutes
> 2) User suspends to RAM. Firmware sets the wakeup alarm for 15 minutes
> in the future
> 3) User resumes after 5 minutes. Firmware does not reset the alarm, and
> as such it is still set for 10 minutes in the future
> 4) User suspends after 5 minutes. Firmware notices that the alarm is set
> for 5 minutes in the future, which is less than the 15 minute transition
> threshold. It therefore assumes that the user wants the machine to wake
> in 5 minutes
> 5) System resumes after 5 minutes
> 
> The worst case scenario here is that the user may have put the system in a
> bag between (4) and (5), resulting in it running in a confined space and
> potentially overheating.  This seems reasonably important.  The Rapid
> Start support code got added in 3.11, but it can be configured in the
> firmware regardless of kernel support.
> 
> Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> ---
>  drivers/rtc/rtc-cmos.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-09-21 20:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-19 23:12 [PATCH v4 1/2] rtc-cmos: Clear ACPI-driven alarms upon resume Gabriele Mazzotta
2016-09-19 23:12 ` [rtc-linux] " Gabriele Mazzotta
2016-09-19 23:12 ` [PATCH v4 2/2] rtc-cmos: Restore alarm after resume Gabriele Mazzotta
2016-09-19 23:12   ` [rtc-linux] " Gabriele Mazzotta
2016-09-21 20:09   ` Alexandre Belloni
2016-09-21 20:09     ` [rtc-linux] " Alexandre Belloni
2016-09-21 20:07 ` [PATCH v4 1/2] rtc-cmos: Clear ACPI-driven alarms upon resume Alexandre Belloni
2016-09-21 20:07   ` [rtc-linux] " Alexandre Belloni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.