On 07/03/2025 07:55:33+0100, Wolfram Sang wrote:
> Hi Alexandre,
>
> thank you for replying even though you are on holidays.
>
> > What I'm really wondering about is the use case. What is expected here?
> > I guess that would be so you could go back to sleep between each 1s
> > interrupt? Does this actually happen and does it actually save any power
> > versus waking up early and waiting for the timer to actually elapse?
>
> There is no specific use case and it is not about saving power. My
> customer wants this IP core fully supported. And it seemed strange that
> UIE is not supported even though there is an 1s interrupt. The primary
> intention was to support that. And my digging in the RTC subsystem made
> me think this is all handled via the regular alarm timerqueue. So, I
> added second granularity to the alarms so the timerqueue can be used for
> UIE. Giving the alarms a higher resolution was a neat side effect. What
> is wrong about that? Are wakeups from deep sleep states the only use
> case for RTC alarms? Can it not be that some other tool just wants an
> interrupt at some second? I assumed so, but actually, I dunno.
There is nothing wrong with your approach, I'm fine with the complexity
if you are ok with it. I don't think any application would use RTC
alarms as timers. The main use cases for RTCs are:
- set the initial system time, for this UIE is actually important to be
able to set the time with some accuracy.
- wake up the system from any sleep mode, usually th sleep will be
fairly long and waking up early because of the minute resolution is
fine as the system will then wait for the actual timer to elapse this
timer being based on the system time instead of the RTC time.
>
> > > + dev_warn(&pdev->dev, "RTC pps interrupt not available. Alarm has only minute accuracy\n");
> >
> > Is this message really necessary? I remember someone giving a talk about
> > how we should avoid adding countless strings to the kernel ;)
>
> Can be argued.
>
> > I'm on holidays and didn't reply to your previous email. The way to
> > support UIE while keeping the alarm at 1 minute resolution would be to
> > look at which timer is enabled.
> >
> > The rv8803 driver does:
> >
> > if (alrm->enabled) {
> > if (rv8803->rtc->uie_rtctimer.enabled)
> > rv8803->ctrl |= RV8803_CTRL_UIE;
> > if (rv8803->rtc->aie_timer.enabled)
> > rv8803->ctrl |= RV8803_CTRL_AIE;
>
> I totally believe you it works, but I am still not entirely sure why. I
> have no problems following the code until rtc_timer_enqueue(). After
> then, I see __rtc_set_alarm() being used again. Does it work because the
> actual alarm time is set but basically discarded for UIE? And the next
> interrupt is just used to be the right one, matching either UIE or
> regular alarm depending what is next in the timerqueue? So, basically
> the flags RTC_UF and RTC_AF are not really used anymore? I don't find
> specific RTC_UF handling in the core?
Yes, you followed the code correctly, I have a series that is removing
RTC_UF that I didn't send yet.
Again I'm fine with the patch as is, I just wanted to point out that the
complexity may not be needed.
Regards,
Alexandre Belloni
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
