[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1dfdd796-3ea3-4c7b-b0b6-cf8a50513e87@roeck-us.net>
Date: Thu, 24 Aug 2023 22:46:51 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: John Stultz <jstultz@...gle.com>
Cc: Alexandre Belloni <alexandre.belloni@...tlin.com>,
Alessandro Zummo <a.zummo@...ertech.it>,
Benson Leung <bleung@...omium.org>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Thomas Gleixner <tglx@...utronix.de>,
Stephen Boyd <sboyd@...nel.org>, linux-rtc@...r.kernel.org,
linux-kernel@...r.kernel.org,
Brian Norris <briannorris@...omium.org>
Subject: Re: [PATCH v2 2/7] rtc: alarmtimer: Use maximum alarm time offset
On Thu, Aug 24, 2023 at 08:52:44PM -0700, John Stultz wrote:
> On Thu, Aug 17, 2023 at 3:55 PM Guenter Roeck <linux@...ck-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 will result in brief
> > spurious wakeups from suspend, it is still much better than not suspending
> > at all.
> >
> > Cc: Brian Norris <briannorris@...omium.org>
> > Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> > ---
> > v2: Rename range_max_offset -> alarm_offset_max
> >
> > kernel/time/alarmtimer.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> > index 8d9f13d847f0..895e3a6d6444 100644
> > --- a/kernel/time/alarmtimer.c
> > +++ b/kernel/time/alarmtimer.c
> > @@ -290,6 +290,19 @@ 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 will wake up earlier than necessary and is expected
> > + * to go back to sleep if it has nothing to do.
> > + * It would be desirable to handle such early wakeups without fully
> > + * waking up the system, but it is unknown if this is even possible.
> > + */
> > + if (rtc->alarm_offset_max &&
> > + rtc->alarm_offset_max * MSEC_PER_SEC < ktime_to_ms(min))
> > + min = ms_to_ktime(rtc->alarm_offset_max * MSEC_PER_SEC);
>
> I don't really have an objection here, but I wonder if this would be
> better abstracted by a rtc_ function?
>
> ktime_t rtc_bound_ktime_interval(ktime interval)
Probably more like like rtc_bound_alarm_interval(),
but fine with me.
> {
> if (!rtc->alarm_offset_max)
> return interval;
> return ms_to_ktime(min(rtc->alarm_offset_max, ktime_to_ms(interval)));
alarm_offset_max is in seconds, so that would need some tweaking.
Guenter
> }
>
> (simple enough to throw into rtc.h maybe as an inline function?)
>
> Then the above would be tweaked to:
> min = rtc_bound_interval(min);
>
> thanks
> -john
Powered by blists - more mailing lists