Re: [PATCH 0/7] rtc: Add support for limited alarm timer offsets

[Alexandre Belloni]

Hello,

On 16/08/2023 06:39:29-0700, Guenter Roeck wrote:
> Some alarm timers are based on time offsets, not on absolute times.
> In some situations, the amount of time that can be scheduled in the
> future is limited. This may result in a refusal to suspend the system,
> causing substantial battery drain.
> 
> This problem was previously observed on a Chromebook using the cros_ec
> rtc driver. EC variants on some older Chromebooks only support 24 hours
> of alarm time in the future. To work around the problem on affected
> Chromebooks, code to limit the maximum alarm time was added to the cros_ec
> rtc driver with commit f27efee66370 ("rtc: cros-ec: Limit RTC alarm range
> if needed"). The problem is now seen again on a system using the cmos
> RTC driver on hardware limited to 24 hours of alarm time, so a more
> generic solution is needed.
> 
> Some RTC drivers remedy the situation by setting the alarm time to the
> maximum supported time if a request for an out-of-range timeout is made.
> This is not really desirable since it may result in unexpected early
> wakeups. It would be even more undesirable to change the behavior
> of existing widely used drivers such as the cmos RTC driver.
> 
> The existing range_max variable in struct rtc_device can not be used
> to determine the maximum time offset supported by an rtc chip since
> it describes the maximum absolute time supported by the chip, not the
> maximum time offset that can be set for alarms.
> 
> To reduce the impact of this problem, introduce a new variable
> rtc_time_offset in struct rtc_device to let RTC drivers report the maximum
> supported alarm time offset. The code setting alarm timers can then
> decide if it wants to reject setting alarm timers to a larger value, if it
> wants to implement recurring alarms until the actually requested alarm
> time is met, or if it wants to accept the limited alarm time. Use the new
> variable to limit the alarm timer range.
> 
> The series is intended to solve the problem with minimal changes in the
> rtc core and in affected drivers.
> 
> An alternative I had considered was to have the alarmtimer code guess the
> maximum timeout supported by the rtc hardware. I discarded it as less
> desirable since it had to retry repeatedly depending on rtc limitations.
> This often resulted in error messages by the rtc driver.  On top of that,
> it was all but impossible to support rtc chips such as tps6586x which
> can only support wake alarms up to 16,383 seconds in the future.
> 
> The first patch of the series adds support for providing the maximum
> supported time offset to the rtc core. The second patch uses that value
> in the alarmtimer code to set the maximum wake-up time from system suspend.
> Subsequent patches add support for reporting the maximum alarm timer offset
> to a subset of affected drivers.
> 
> Previous discussion:
>     https://lore.kernel.org/lkml/Y19AdIntJZGnBh%2Fy@xxxxxxxxxx/T/#mc06d206d5bdb77c613712148818934b4f5640de5
> 

I'm fine with the series, however, this doesn't solve the issue for RTCs
that have an absolute limit on the alarm (as opposed to an offset to the
current time/date).



> ----------------------------------------------------------------
> Guenter Roeck (7):
>       rtc: Add support for limited alarm timer offsets
>       rtc: alarmtimer: Use maximum alarm time offset
>       rtc: cros-ec: Detect and report supported alarm window size
>       rtc: cmos: Report supported alarm limit to rtc infrastructure
>       rtc: tps6586x: Report maximum alarm limit to rtc core
>       rtc: ds1305: Report maximum alarm limit to rtc core
>       rtc: rzn1: Report maximum alarm limit to rtc core
> 
>  drivers/rtc/rtc-cmos.c     | 11 +++++++++++
>  drivers/rtc/rtc-cros-ec.c  | 38 +++++++++++++++++++++++---------------
>  drivers/rtc/rtc-ds1305.c   |  3 ++-
>  drivers/rtc/rtc-rzn1.c     |  1 +
>  drivers/rtc/rtc-tps6586x.c |  1 +
>  include/linux/rtc.h        |  1 +
>  kernel/time/alarmtimer.c   | 13 +++++++++++++
>  7 files changed, 52 insertions(+), 16 deletions(-)

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