Patchwork [resend] rtc-cmos: report wakeup event on ACPI RTC alarm

login
register
mail settings
Submitter Daniel Drake
Date May 9, 2012, 11:18 p.m.
Message ID <20120509231845.0301C9D401E@zog.reactivated.net>
Download mbox | patch
Permalink /patch/158059/
State New
Headers show

Comments

Daniel Drake - May 9, 2012, 11:18 p.m.
When the ACPI-driven RTC alarm wakes the system, report it as a wakeup
event. This allows userspace to determine that the reason for system
wakeup was RTC alarm.

Signed-off-by: Daniel Drake <dsd@laptop.org>
---
 drivers/rtc/rtc-cmos.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Resending after 3 weeks with no response.
Rafael J. Wysocki - May 10, 2012, 8:06 p.m.
On Thursday, May 10, 2012, Daniel Drake wrote:
> When the ACPI-driven RTC alarm wakes the system, report it as a wakeup
> event. This allows userspace to determine that the reason for system
> wakeup was RTC alarm.
> 
> Signed-off-by: Daniel Drake <dsd@laptop.org>

Len, John, any objections to this or may I take it for v3.5?

Rafael


> ---
>  drivers/rtc/rtc-cmos.c |    9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> Resending after 3 weeks with no response.
> 
> diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> index 7d5f56e..4267789 100644
> --- a/drivers/rtc/rtc-cmos.c
> +++ b/drivers/rtc/rtc-cmos.c
> @@ -910,14 +910,17 @@ static inline int cmos_poweroff(struct device *dev)
>  
>  static u32 rtc_handler(void *context)
>  {
> +	struct device *dev = context;
> +
> +	pm_wakeup_event(dev, 0);
>  	acpi_clear_event(ACPI_EVENT_RTC);
>  	acpi_disable_event(ACPI_EVENT_RTC, 0);
>  	return ACPI_INTERRUPT_HANDLED;
>  }
>  
> -static inline void rtc_wake_setup(void)
> +static inline void rtc_wake_setup(struct device *dev)
>  {
> -	acpi_install_fixed_event_handler(ACPI_EVENT_RTC, rtc_handler, NULL);
> +	acpi_install_fixed_event_handler(ACPI_EVENT_RTC, rtc_handler, dev);
>  	/*
>  	 * After the RTC handler is installed, the Fixed_RTC event should
>  	 * be disabled. Only when the RTC alarm is set will it be enabled.
> @@ -950,7 +953,7 @@ cmos_wake_setup(struct device *dev)
>  	if (acpi_disabled)
>  		return;
>  
> -	rtc_wake_setup();
> +	rtc_wake_setup(dev);
>  	acpi_rtc_info.wake_on = rtc_wake_on;
>  	acpi_rtc_info.wake_off = rtc_wake_off;
>  
>
John Stultz - May 22, 2012, 12:38 a.m.
On 05/10/2012 01:06 PM, Rafael J. Wysocki wrote:
> On Thursday, May 10, 2012, Daniel Drake wrote:
>> When the ACPI-driven RTC alarm wakes the system, report it as a wakeup
>> event. This allows userspace to determine that the reason for system
>> wakeup was RTC alarm.
>>
>> Signed-off-by: Daniel Drake<dsd@laptop.org>
> Len, John, any objections to this or may I take it for v3.5?
Seems reasonable. Although could this maybe be done more generically at 
the rtc infrastructure level instead of in the rtc-cmos driver?

thanks
-john
Rafael J. Wysocki - May 22, 2012, 3:12 p.m.
On Tuesday, May 22, 2012, John Stultz wrote:
> On 05/10/2012 01:06 PM, Rafael J. Wysocki wrote:
> > On Thursday, May 10, 2012, Daniel Drake wrote:
> >> When the ACPI-driven RTC alarm wakes the system, report it as a wakeup
> >> event. This allows userspace to determine that the reason for system
> >> wakeup was RTC alarm.
> >>
> >> Signed-off-by: Daniel Drake<dsd@laptop.org>
> > Len, John, any objections to this or may I take it for v3.5?
> Seems reasonable. Although could this maybe be done more generically at 
> the rtc infrastructure level instead of in the rtc-cmos driver?

I think it could, but it can be moved to the RTC layer later too.

Thanks,
Rafael
Daniel Drake - May 28, 2012, 10:51 p.m.
On Tue, May 22, 2012 at 9:12 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> Seems reasonable. Although could this maybe be done more generically at
>> the rtc infrastructure level instead of in the rtc-cmos driver?
>
> I think it could, but it can be moved to the RTC layer later too.

I don't think there is much you can win by making it generic - the
event has to be caught via the ACPI subsystem.

Instead of calling pm_wakeup_event() directly you could add a generic
RTC function to do the exact same thing, but I don't see the need to
hide wakeup event generation in a RTC-level API.

Rafael, is this OK for 3.5?

Thanks,
Daniel

Patch

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 7d5f56e..4267789 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -910,14 +910,17 @@  static inline int cmos_poweroff(struct device *dev)
 
 static u32 rtc_handler(void *context)
 {
+	struct device *dev = context;
+
+	pm_wakeup_event(dev, 0);
 	acpi_clear_event(ACPI_EVENT_RTC);
 	acpi_disable_event(ACPI_EVENT_RTC, 0);
 	return ACPI_INTERRUPT_HANDLED;
 }
 
-static inline void rtc_wake_setup(void)
+static inline void rtc_wake_setup(struct device *dev)
 {
-	acpi_install_fixed_event_handler(ACPI_EVENT_RTC, rtc_handler, NULL);
+	acpi_install_fixed_event_handler(ACPI_EVENT_RTC, rtc_handler, dev);
 	/*
 	 * After the RTC handler is installed, the Fixed_RTC event should
 	 * be disabled. Only when the RTC alarm is set will it be enabled.
@@ -950,7 +953,7 @@  cmos_wake_setup(struct device *dev)
 	if (acpi_disabled)
 		return;
 
-	rtc_wake_setup();
+	rtc_wake_setup(dev);
 	acpi_rtc_info.wake_on = rtc_wake_on;
 	acpi_rtc_info.wake_off = rtc_wake_off;