linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] rtc: alarmtimer: Use maximum alarm time offset
@ 2023-09-15 15:22 Guenter Roeck
  2023-09-15 15:22 ` [PATCH v2 1/2] rtc: Add API function to return alarm time bound by rtc limit Guenter Roeck
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Guenter Roeck @ 2023-09-15 15:22 UTC (permalink / raw)
  To: Alexandre Belloni, John Stultz
  Cc: Alessandro Zummo, Thomas Gleixner, Stephen Boyd, linux-rtc,
	linux-kernel, Brian Norris, Guenter Roeck

Some userspace applications use timerfd_create() to request wakeups after
a long period of time. For example, a backup application may request a
wakeup once per week. This is perfectly fine as long as the system does
not try to suspend. However, if the system tries to suspend and the
system's RTC does not support the required alarm timeout, the suspend
operation will fail with an error such as

rtc_cmos 00:01: Alarms can be up to one day in the future
PM: dpm_run_callback(): platform_pm_suspend+0x0/0x4a returns -22
alarmtimer alarmtimer.4.auto: platform_pm_suspend+0x0/0x4a returned -22 after 117 usecs
PM: Device alarmtimer.4.auto failed to suspend: error -22

This results in a refusal to suspend the system, causing substantial
battery drain on affected systems.

To fix the problem, use the maximum alarm time offset as reported by rtc
drivers to set the maximum alarm time. While this may result in early
wakeups from suspend, it is still much better than not suspending at all.

This patch series standardizes system behavior if the requested alarm
timeout is larger than the alarm timeout supported by the rtc chip.
Currently, in this situation, the rtc driver will do one of the following.
- It may return an error.
- It may limit the alarm timeout to the maximum supported by the rtc chip.
- It may mask the timeout by the maximum alarm timeout supported by the RTC
  chip (i.e. a requested timeout of 1 day + 1 minute may result in a 1
  minute timeout).

With this series in place, if the rtc driver reports the maximum alarm
timeout supported by the rtc chip, the system will always limit the alarm
timeout to the maximum supported by the rtc chip.

The first patch of the series adds support for an API function which returns
the maximum of the requested alarm timeout and the alarm timeout supported
by the RTC chip. The second patch uses that value in the alarmtimer code
to set the maximum wake-up time from system suspend.

Version 1 of the series added support for storing the maximum alarm timeout
to the rtc core. This series is based on the original series, most of which
is now in the upstream kernel. It adds an API function to the rtc core and
uses that function in the alarm timer code to set the alarm time. It
replaces patch 2/7 ("rtc: alarmtimer: Use maximum alarm time offset") of
the original series.

v1:
    <fixme>

----------------------------------------------------------------
Guenter Roeck (2):
      rtc: Add API function to return alarm time bound by rtc limit
      rtc: alarmtimer: Use maximum alarm time offset

 include/linux/rtc.h      | 17 +++++++++++++++++
 kernel/time/alarmtimer.c | 11 +++++++++++
 2 files changed, 28 insertions(+)

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

* [PATCH v2 1/2] rtc: Add API function to return alarm time bound by rtc limit
  2023-09-15 15:22 [PATCH v2 0/2] rtc: alarmtimer: Use maximum alarm time offset Guenter Roeck
@ 2023-09-15 15:22 ` Guenter Roeck
  2023-09-15 15:22 ` [PATCH 2/2] rtc: alarmtimer: Use maximum alarm time offset Guenter Roeck
  2023-10-30  0:44 ` [PATCH v2 0/2] " Alexandre Belloni
  2 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2023-09-15 15:22 UTC (permalink / raw)
  To: Alexandre Belloni, John Stultz
  Cc: Alessandro Zummo, Thomas Gleixner, Stephen Boyd, linux-rtc,
	linux-kernel, Brian Norris, Guenter Roeck

Add rtc_bound_alarmtime() to return the requested alarm timeout
bound by the maxmum alarm timeout that is supported by a given rtc.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Added patch to provide API function

 include/linux/rtc.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 4c0bcbeb1f00..5f8e438a0312 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -225,6 +225,23 @@ static inline bool is_leap_year(unsigned int year)
 	return (!(year % 4) && (year % 100)) || !(year % 400);
 }
 
+/**
+ * rtc_bound_alarmtime() - Return alarm time bound by rtc limit
+ * @rtc: Pointer to rtc device structure
+ * @requested: Requested alarm timeout
+ *
+ * Return: Alarm timeout bound by maximum alarm time supported by rtc.
+ */
+static inline ktime_t rtc_bound_alarmtime(struct rtc_device *rtc,
+					  ktime_t requested)
+{
+	if (rtc->alarm_offset_max &&
+	    rtc->alarm_offset_max * MSEC_PER_SEC < ktime_to_ms(requested))
+		return ms_to_ktime(rtc->alarm_offset_max * MSEC_PER_SEC);
+
+	return requested;
+}
+
 #define devm_rtc_register_device(device) \
 	__devm_rtc_register_device(THIS_MODULE, device)
 
-- 
2.39.2


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

* [PATCH 2/2] rtc: alarmtimer: Use maximum alarm time offset
  2023-09-15 15:22 [PATCH v2 0/2] rtc: alarmtimer: Use maximum alarm time offset Guenter Roeck
  2023-09-15 15:22 ` [PATCH v2 1/2] rtc: Add API function to return alarm time bound by rtc limit Guenter Roeck
@ 2023-09-15 15:22 ` Guenter Roeck
  2023-09-15 17:42   ` John Stultz
  2023-10-30  0:44 ` [PATCH v2 0/2] " Alexandre Belloni
  2 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2023-09-15 15:22 UTC (permalink / raw)
  To: Alexandre Belloni, John Stultz
  Cc: Alessandro Zummo, Thomas Gleixner, Stephen Boyd, linux-rtc,
	linux-kernel, Brian Norris, Guenter Roeck

Some userspace applications use timerfd_create() to request wakeups after
a long period of time. For example, a backup application may request a
wakeup once per week. This is perfectly fine as long as the system does
not try to suspend. However, if the system tries to suspend and the
system's RTC does not support the required alarm timeout, the suspend
operation will fail with an error such as

rtc_cmos 00:01: Alarms can be up to one day in the future
PM: dpm_run_callback(): platform_pm_suspend+0x0/0x4a returns -22
alarmtimer alarmtimer.4.auto: platform_pm_suspend+0x0/0x4a returned -22 after 117 usecs
PM: Device alarmtimer.4.auto failed to suspend: error -22

This results in a refusal to suspend the system, causing substantial
battery drain on affected systems.

To fix the problem, use the maximum alarm time offset as reported by rtc
drivers to set the maximum alarm time. While this may result in early
wakeups from suspend, it is still much better than not suspending at all.

This patch standardizes system behavior if the requested alarm timeout
is larger than the alarm timeout supported by the rtc chip. Currently,
in this situation, the rtc driver will do one of the following.
- It may return an error.
- It may limit the alarm timeout to the maximum supported by the rtc chip.
- It may mask the timeout by the maximum alarm timeout supported by the RTC
  chip (i.e. a requested timeout of 1 day + 1 minute may result in a 1
  minute timeout).

With this patch in place, if the rtc driver reports the maximum alarm
timeout supported by the rtc chip, the system will always limit the alarm
timeout to the maximum supported by the rtc chip.

Cc: Brian Norris <briannorris@chromium.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Use API function instead of accessing rtc core internals directly.
    Modify comment in code per feedback received.

 kernel/time/alarmtimer.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 8d9f13d847f0..0dc68372efd0 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -290,6 +290,17 @@ static int alarmtimer_suspend(struct device *dev)
 	rtc_timer_cancel(rtc, &rtctimer);
 	rtc_read_time(rtc, &tm);
 	now = rtc_tm_to_ktime(tm);
+
+	/*
+	 * If the RTC alarm timer only supports a limited time offset, set the
+	 * alarm time to the maximum supported value.
+	 * The system may wake up earlier (possibly much earlier) than expected
+	 * when the alarmtimer runs. This is the best the kernel can do if
+	 * the alarmtimer exceeds the time that the rtc device can be programmed
+	 * for.
+	 */
+	min = rtc_bound_alarmtime(rtc, min);
+
 	now = ktime_add(now, min);
 
 	/* Set alarm, if in the past reject suspend briefly to handle */
-- 
2.39.2


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

* Re: [PATCH 2/2] rtc: alarmtimer: Use maximum alarm time offset
  2023-09-15 15:22 ` [PATCH 2/2] rtc: alarmtimer: Use maximum alarm time offset Guenter Roeck
@ 2023-09-15 17:42   ` John Stultz
  0 siblings, 0 replies; 6+ messages in thread
From: John Stultz @ 2023-09-15 17:42 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Alexandre Belloni, Alessandro Zummo, Thomas Gleixner,
	Stephen Boyd, linux-rtc, linux-kernel, Brian Norris

On Fri, Sep 15, 2023 at 8:22 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> Some userspace applications use timerfd_create() to request wakeups after
> a long period of time. For example, a backup application may request a
> wakeup once per week. This is perfectly fine as long as the system does
> not try to suspend. However, if the system tries to suspend and the
> system's RTC does not support the required alarm timeout, the suspend
> operation will fail with an error such as
>
> rtc_cmos 00:01: Alarms can be up to one day in the future
> PM: dpm_run_callback(): platform_pm_suspend+0x0/0x4a returns -22
> alarmtimer alarmtimer.4.auto: platform_pm_suspend+0x0/0x4a returned -22 after 117 usecs
> PM: Device alarmtimer.4.auto failed to suspend: error -22
>
> This results in a refusal to suspend the system, causing substantial
> battery drain on affected systems.
>
> To fix the problem, use the maximum alarm time offset as reported by rtc
> drivers to set the maximum alarm time. While this may result in early
> wakeups from suspend, it is still much better than not suspending at all.
>
> This patch standardizes system behavior if the requested alarm timeout
> is larger than the alarm timeout supported by the rtc chip. Currently,
> in this situation, the rtc driver will do one of the following.
> - It may return an error.
> - It may limit the alarm timeout to the maximum supported by the rtc chip.
> - It may mask the timeout by the maximum alarm timeout supported by the RTC
>   chip (i.e. a requested timeout of 1 day + 1 minute may result in a 1
>   minute timeout).
>
> With this patch in place, if the rtc driver reports the maximum alarm
> timeout supported by the rtc chip, the system will always limit the alarm
> timeout to the maximum supported by the rtc chip.
>
> Cc: Brian Norris <briannorris@chromium.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Use API function instead of accessing rtc core internals directly.
>     Modify comment in code per feedback received.
>
>  kernel/time/alarmtimer.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> index 8d9f13d847f0..0dc68372efd0 100644
> --- a/kernel/time/alarmtimer.c
> +++ b/kernel/time/alarmtimer.c
> @@ -290,6 +290,17 @@ static int alarmtimer_suspend(struct device *dev)
>         rtc_timer_cancel(rtc, &rtctimer);
>         rtc_read_time(rtc, &tm);
>         now = rtc_tm_to_ktime(tm);
> +
> +       /*
> +        * If the RTC alarm timer only supports a limited time offset, set the
> +        * alarm time to the maximum supported value.
> +        * The system may wake up earlier (possibly much earlier) than expected
> +        * when the alarmtimer runs. This is the best the kernel can do if
> +        * the alarmtimer exceeds the time that the rtc device can be programmed
> +        * for.
> +        */
> +       min = rtc_bound_alarmtime(rtc, min);
> +

Thanks for all the iterations on this Guenter! This looks nice.

Acked-by: John Stultz <jstultz@google.com>

thanks
-john

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

* Re: [PATCH v2 0/2] rtc: alarmtimer: Use maximum alarm time offset
  2023-09-15 15:22 [PATCH v2 0/2] rtc: alarmtimer: Use maximum alarm time offset Guenter Roeck
  2023-09-15 15:22 ` [PATCH v2 1/2] rtc: Add API function to return alarm time bound by rtc limit Guenter Roeck
  2023-09-15 15:22 ` [PATCH 2/2] rtc: alarmtimer: Use maximum alarm time offset Guenter Roeck
@ 2023-10-30  0:44 ` Alexandre Belloni
  2023-10-30  1:00   ` Alexandre Belloni
  2 siblings, 1 reply; 6+ messages in thread
From: Alexandre Belloni @ 2023-10-30  0:44 UTC (permalink / raw)
  To: John Stultz, Guenter Roeck
  Cc: Alessandro Zummo, Thomas Gleixner, Stephen Boyd, linux-rtc,
	linux-kernel, Brian Norris


On Fri, 15 Sep 2023 08:22:36 -0700, Guenter Roeck wrote:
> Some userspace applications use timerfd_create() to request wakeups after
> a long period of time. For example, a backup application may request a
> wakeup once per week. This is perfectly fine as long as the system does
> not try to suspend. However, if the system tries to suspend and the
> system's RTC does not support the required alarm timeout, the suspend
> operation will fail with an error such as
> 
> [...]

Applied, thanks!

[1/2] rtc: Add API function to return alarm time bound by rtc limit
      commit: b0790dc7419f334574fc5416690913ab4c9e9ba5
[2/2] rtc: alarmtimer: Use maximum alarm time offset
      commit: f628128dfe77f6e475507798b0f7ed25831ae893

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

* Re: [PATCH v2 0/2] rtc: alarmtimer: Use maximum alarm time offset
  2023-10-30  0:44 ` [PATCH v2 0/2] " Alexandre Belloni
@ 2023-10-30  1:00   ` Alexandre Belloni
  0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2023-10-30  1:00 UTC (permalink / raw)
  To: John Stultz, Guenter Roeck
  Cc: Alessandro Zummo, Thomas Gleixner, Stephen Boyd, linux-rtc,
	linux-kernel, Brian Norris

On 30/10/2023 01:44:55+0100, Alexandre Belloni wrote:
> 
> On Fri, 15 Sep 2023 08:22:36 -0700, Guenter Roeck wrote:
> > Some userspace applications use timerfd_create() to request wakeups after
> > a long period of time. For example, a backup application may request a
> > wakeup once per week. This is perfectly fine as long as the system does
> > not try to suspend. However, if the system tries to suspend and the
> > system's RTC does not support the required alarm timeout, the suspend
> > operation will fail with an error such as
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/2] rtc: Add API function to return alarm time bound by rtc limit
>       commit: b0790dc7419f334574fc5416690913ab4c9e9ba5
> [2/2] rtc: alarmtimer: Use maximum alarm time offset
>       commit: f628128dfe77f6e475507798b0f7ed25831ae893
> 

Actually, this is already in tip but I didn't get the notification.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

end of thread, other threads:[~2023-10-30  1:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-15 15:22 [PATCH v2 0/2] rtc: alarmtimer: Use maximum alarm time offset Guenter Roeck
2023-09-15 15:22 ` [PATCH v2 1/2] rtc: Add API function to return alarm time bound by rtc limit Guenter Roeck
2023-09-15 15:22 ` [PATCH 2/2] rtc: alarmtimer: Use maximum alarm time offset Guenter Roeck
2023-09-15 17:42   ` John Stultz
2023-10-30  0:44 ` [PATCH v2 0/2] " Alexandre Belloni
2023-10-30  1:00   ` Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).