Re: [PATCH 2/2] rtc: alarmtimer: Use maximum alarm time offset

[John Stultz]

On Fri, Sep 15, 2023 at 8:22 AM Guenter Roeck <linux@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>

thanks
-john