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

From: Guenter Roeck
Date: Wed Aug 23 2023 - 23:27:31 EST

On Thu, Aug 24, 2023 at 12:51:29AM +0200, Alexandre Belloni wrote:
> Hello,
>
> On 23/08/2023 09:50:47-0700, Guenter Roeck wrote:
> > Hi Alexandre,
> >
> > On Thu, Aug 17, 2023 at 03:55:31PM -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.
> > >
> > > Some RTC alarm 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.
> > >
> > > To reduce the impact of this problem, let RTC drivers report the maximum
> > > supported alarm timer 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.
> > >
> > > Only introduce the necessary variable into struct rtc_device.
> > > Code to set and use the variable will follow with subsequent patches.
> > >
> > > Cc: Brian Norris <briannorris@xxxxxxxxxxxx>
> > > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> >
> > I guess it is a bit late to get the series into v6.6, but would it be
> > possible to apply it to a -next branch to get some more test coverage ?
> >
>
> I'm probably going to take 1 and 3-7 for 6.6 once I get a reliable
> internet access. I can't take 2/7 without a review or ack from the time
> maintainers.
>
Ok, makes sense.

> > Either case, do you have any additional comments / feedback ?
> >
>
> The main issue that remains is that after 2/7, the rtc_device structure
> is not opaque anymore to its user as alarmtimer_suspend now directly
> accesses one of the members. But I'd have to find which RTCs have an
> absolute limit so we can design a proper API. I may also decide that it
> is good enough to require that the alarm range must cover the registered
> RTC range.
>
What exactly do you have in mind ? For our use case, we need
alarmtimer_suspend() to either sleep for the requested time, or as long
as possible. Would an API function returning the alarm limit address
your concerns ?

Regarding relative/absolute, it is a mixed bag. Below is what I found
for the alphabetically first ~40 drivers. Note that "absolute" in the
configuration column means that the alarm time is specified as absolute
value (e.g, at 1am), not that the limit is absolute or relative to the
current time. The limit column gives a hint if the limit is relative
or absolute. In many cases the alarm is configured as absolute value
but limited to day/week/month/year in the future.

On a side note, struct rtc_device isn't exactly opaque even today.
Both alarmtimer.c and ntp.c already access some of its members.

Thanks,
Guenter

---
driver configuration limit comments
rtc-88pm80x.c relative 24 hours silently adjusted
rtc-88pm860x.c relative 32 bit seconds unchecked
rtc-ab8500.c absolute ~2032 unchecked
rtc-ab-b5ze-s3.c relative 1 month -EINVAL
rtc-ab-eoz9.c absolute 1 month month & year ignored (1 month + 1 day -> fires after 1 day)
rtc-abx80x.c absolute 1 year year ignored (1 year + 1 day -> fires after 1 day)
rtc-ac100.c absolute 2069 -EINVAL
rtc-armada38x.c absolute 2106 unchecked
rtc-as3722.c absolute unlimited unchecked
rtc-asm9260.c absolute unlimited unchecked
rtc-at91rm9200.c absolute 1 year year ignored (1 year + 1 day -> fires after 1 day)
rtc-at91sam9.c relative 32 bit seconds unchecked
rtc-bd70528.c absolute 2099 unchecked
rtc-brcmstb-waketimer.c absolute 2106 unchecked
rtc-cadence.c absolute 2999 unchecked
rtc-cmos.c absolute day/month/year -EINVAL if out of range
rtc-cpcap.c absolute ~2059 in days since 1970, day masked, unchecked (2060 -> 1971)
rtc-cros-ec.c relative 24h or max -EINVAL
rtc-da9052.c absolute 2063 unchecked, year masked
rtc-da9055.c absolute 2133 ? unchecked, year masked
rtc-da9063.c absolute 2063 unchecked, year masked
rtc-digicolor.c relative 32-bit seconds unchecked
rtc-ds1286.c absolute 24 hours unchecked, month & year ignored
rtc-ds1305.c absolute 24 hours -EINVAL
rtc-ds1307.c absolute 1 year unchecked, year ignored
rtc-ds1343.c absolute 1 month unchecked, month / year ignored
rtc-ds1374.c relative ~194 days unchecked, masked (195 days -> fires after 1 day)
rtc-ds1511.c absolute 1 month unchecked, month/year ignored
rtc-ds1553.c absolute 1 month unchecked, month/year ignored
rtc-ds1685.c absolute 1 month unchecked, month/year ignored
rtc-ds3232.c absolute 1 month unchecked, month/year ignored
rtc-efi.c absolute not specified several years
rtc-fm3130.c absolute 1 year year ignored
rtc-fsl-ftm-alarm.c relative 262 seconds -ERANGE
rtc-goldfish.c absolute ~500 years unchecked
rtc-hym8563.c absolute 1 month unchecked, month/year ignored
rtc-imxdi.c abolute 2106 unchecked (32 bit seconds since 1970)
rtc-imx-sc.c absolute unknown Value passed to SCU which determines if valid
rtc-isl1208.c absolute 1 year unchecked, year ignored
rtc-jz4740.c absolute 2106 unchecked (32 bit seconds since 1970)