Re: [PATCH v2] rtc: rzn1: implement one-second accuracy for alarms

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 2322 bytes --]

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.

> > +		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?

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]