| Message ID | 20250305101038.9933-2-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v2] rtc: rzn1: implement one-second accuracy for alarms | expand |
Hello, On 05/03/2025 11:08:16+0100, Wolfram Sang wrote: > The hardware alarm only supports one-minute accuracy which is coarse and > disables UIE usage. Use the 1-second interrupt to achieve per-second > accuracy. It is activated once we hit the per-minute alarm. The new > feature is optional. When there is no 1-second interrupt, old behaviour > with per-minute accuracy is used as before. With this feature, all tests > of 'rtctest' are successfully passed. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > Tested with the Renesas RZ/N1D board. Besides 'rtctest', I did some > manual testing with 'rtc' on top trying to stresstest corner cases. > > Looking forward to comments. AFAICS, this is the first driver trying to > overcome the per-minute limitation using 1-second interrupts. > 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? > Change since v1: > * consider 1s interrupt when setting the alarm->enabled flag > > drivers/rtc/rtc-rzn1.c | 108 ++++++++++++++++++++++++++++++++++------- > 1 file changed, 91 insertions(+), 17 deletions(-) > > diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c > index cb220807d925..eeb9612a666f 100644 > --- a/drivers/rtc/rtc-rzn1.c > +++ b/drivers/rtc/rtc-rzn1.c > @@ -19,6 +19,7 @@ > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/rtc.h> > +#include <linux/spinlock.h> > > #define RZN1_RTC_CTL0 0x00 > #define RZN1_RTC_CTL0_SLSB_SUBU 0 > @@ -27,6 +28,7 @@ > #define RZN1_RTC_CTL0_CE BIT(7) > > #define RZN1_RTC_CTL1 0x04 > +#define RZN1_RTC_CTL1_1SE BIT(3) > #define RZN1_RTC_CTL1_ALME BIT(4) > > #define RZN1_RTC_CTL2 0x08 > @@ -58,6 +60,13 @@ > struct rzn1_rtc { > struct rtc_device *rtcdev; > void __iomem *base; > + /* > + * Protects access to RZN1_RTC_CTL1 reg. rtc_lock with threaded_irqs > + * would introduce race conditions when switching interrupts because > + * of potential sleeps > + */ > + spinlock_t ctl1_access_lock; > + struct rtc_time tm_alarm; > }; > > static void rzn1_rtc_get_time_snapshot(struct rzn1_rtc *rtc, struct rtc_time *tm) > @@ -135,8 +144,38 @@ static int rzn1_rtc_set_time(struct device *dev, struct rtc_time *tm) > static irqreturn_t rzn1_rtc_alarm_irq(int irq, void *dev_id) > { > struct rzn1_rtc *rtc = dev_id; > + u32 ctl1, set_irq_bits = 0; > + > + if (rtc->tm_alarm.tm_sec == 0) > + rtc_update_irq(rtc->rtcdev, 1, RTC_AF | RTC_IRQF); > + else > + /* Switch to 1s interrupts */ > + set_irq_bits = RZN1_RTC_CTL1_1SE; > > - rtc_update_irq(rtc->rtcdev, 1, RTC_AF | RTC_IRQF); > + guard(spinlock)(&rtc->ctl1_access_lock); > + > + ctl1 = readl(rtc->base + RZN1_RTC_CTL1); > + ctl1 &= ~RZN1_RTC_CTL1_ALME; > + ctl1 |= set_irq_bits; > + writel(ctl1, rtc->base + RZN1_RTC_CTL1); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t rzn1_rtc_1s_irq(int irq, void *dev_id) > +{ > + struct rzn1_rtc *rtc = dev_id; > + u32 ctl1; > + > + if (readl(rtc->base + RZN1_RTC_SECC) == bin2bcd(rtc->tm_alarm.tm_sec)) { > + guard(spinlock)(&rtc->ctl1_access_lock); > + > + ctl1 = readl(rtc->base + RZN1_RTC_CTL1); > + ctl1 &= ~RZN1_RTC_CTL1_1SE; > + writel(ctl1, rtc->base + RZN1_RTC_CTL1); > + > + rtc_update_irq(rtc->rtcdev, 1, RTC_AF | RTC_IRQF); > + } > > return IRQ_HANDLED; > } > @@ -144,14 +183,38 @@ static irqreturn_t rzn1_rtc_alarm_irq(int irq, void *dev_id) > static int rzn1_rtc_alarm_irq_enable(struct device *dev, unsigned int enable) > { > struct rzn1_rtc *rtc = dev_get_drvdata(dev); > - u32 ctl1 = readl(rtc->base + RZN1_RTC_CTL1); > + struct rtc_time *tm = &rtc->tm_alarm, tm_now; > + u32 ctl1; > + int ret; > > - if (enable) > - ctl1 |= RZN1_RTC_CTL1_ALME; > - else > - ctl1 &= ~RZN1_RTC_CTL1_ALME; > + guard(spinlock_irqsave)(&rtc->ctl1_access_lock); > > - writel(ctl1, rtc->base + RZN1_RTC_CTL1); > + ctl1 = readl(rtc->base + RZN1_RTC_CTL1); > + > + if (enable) { > + /* > + * Use alarm interrupt if alarm time is at least a minute away > + * or less than a minute but in the next minute. Otherwise use > + * 1 second interrupt to wait for the proper second > + */ > + do { > + ctl1 &= ~(RZN1_RTC_CTL1_ALME | RZN1_RTC_CTL1_1SE); > + > + ret = rzn1_rtc_read_time(dev, &tm_now); > + if (ret) > + return ret; > + > + if (rtc_tm_sub(tm, &tm_now) > 59 || tm->tm_min != tm_now.tm_min) > + ctl1 |= RZN1_RTC_CTL1_ALME; > + else > + ctl1 |= RZN1_RTC_CTL1_1SE; > + > + writel(ctl1, rtc->base + RZN1_RTC_CTL1); > + } while (readl(rtc->base + RZN1_RTC_SECC) != bin2bcd(tm_now.tm_sec)); > + } else { > + ctl1 &= ~(RZN1_RTC_CTL1_ALME | RZN1_RTC_CTL1_1SE); > + writel(ctl1, rtc->base + RZN1_RTC_CTL1); > + } > > return 0; > } > @@ -185,7 +248,7 @@ static int rzn1_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) > } > > ctl1 = readl(rtc->base + RZN1_RTC_CTL1); > - alrm->enabled = !!(ctl1 & RZN1_RTC_CTL1_ALME); > + alrm->enabled = !!(ctl1 & (RZN1_RTC_CTL1_ALME | RZN1_RTC_CTL1_1SE)); > > return 0; > } > @@ -216,6 +279,8 @@ static int rzn1_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) > writel(bin2bcd(tm->tm_hour), rtc->base + RZN1_RTC_ALH); > writel(BIT(wday), rtc->base + RZN1_RTC_ALW); > > + rtc->tm_alarm = alrm->time; > + > rzn1_rtc_alarm_irq_enable(dev, alrm->enabled); > > return 0; > @@ -304,7 +369,7 @@ static const struct rtc_class_ops rzn1_rtc_ops = { > static int rzn1_rtc_probe(struct platform_device *pdev) > { > struct rzn1_rtc *rtc; > - int alarm_irq; > + int irq; > int ret; > > rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL); > @@ -317,9 +382,9 @@ static int rzn1_rtc_probe(struct platform_device *pdev) > if (IS_ERR(rtc->base)) > return dev_err_probe(&pdev->dev, PTR_ERR(rtc->base), "Missing reg\n"); > > - alarm_irq = platform_get_irq(pdev, 0); > - if (alarm_irq < 0) > - return alarm_irq; > + irq = platform_get_irq_byname(pdev, "alarm"); > + if (irq < 0) > + return irq; > > rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev); > if (IS_ERR(rtc->rtcdev)) > @@ -329,8 +394,6 @@ static int rzn1_rtc_probe(struct platform_device *pdev) > rtc->rtcdev->range_max = RTC_TIMESTAMP_END_2099; > rtc->rtcdev->alarm_offset_max = 7 * 86400; > rtc->rtcdev->ops = &rzn1_rtc_ops; > - set_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->rtcdev->features); > - clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtcdev->features); > > ret = devm_pm_runtime_enable(&pdev->dev); > if (ret < 0) > @@ -349,13 +412,24 @@ static int rzn1_rtc_probe(struct platform_device *pdev) > /* Disable all interrupts */ > writel(0, rtc->base + RZN1_RTC_CTL1); > > - ret = devm_request_irq(&pdev->dev, alarm_irq, rzn1_rtc_alarm_irq, 0, > - dev_name(&pdev->dev), rtc); > + spin_lock_init(&rtc->ctl1_access_lock); > + > + ret = devm_request_irq(&pdev->dev, irq, rzn1_rtc_alarm_irq, 0, "RZN1 RTC Alarm", rtc); > if (ret) { > - dev_err(&pdev->dev, "RTC timer interrupt not available\n"); > + dev_err(&pdev->dev, "RTC alarm interrupt not available\n"); > goto dis_runtime_pm; > } > > + irq = platform_get_irq_byname_optional(pdev, "pps"); > + if (irq >= 0) > + ret = devm_request_irq(&pdev->dev, irq, rzn1_rtc_1s_irq, 0, "RZN1 RTC 1s", rtc); > + > + if (irq < 0 || ret) { > + set_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->rtcdev->features); > + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtcdev->features); > + 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 ;) 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; https://elixir.bootlin.com/linux/v6.13.5/source/drivers/rtc/rtc-rv8803.c#L439 Like I said, this is a bit convoluted but there are only a few cases so I didn't bother hiding this behind a proper API.
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
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
Hi Alexandre, > Again I'm fine with the patch as is, I just wanted to point out that the > complexity may not be needed. Thank you for pointing this out. I would like this patch to go in, still. I agree that the code is probably a tad more complex. The concept, however, seems more straightforward to me. Because RTC core now works best with an alarm timer with second accuracy. So, let's provide that, so the timerqueue can do all it needs to do. Plus, you never know what customers do in their application space. I prefer to be feature complete. Thanks for the discussion, I got a bit more insight now. Wolfram
> Yes, you followed the code correctly, I have a series that is removing > RTC_UF that I didn't send yet. Please CC me when you send this. I am interested.
Hi Alexandre, On Mon, Mar 10, 2025 at 09:16:58AM +0100, Wolfram Sang wrote: > > > Yes, you followed the code correctly, I have a series that is removing > > RTC_UF that I didn't send yet. > > Please CC me when you send this. I am interested. Do you also have a series pending simplifying handling of 'max_user_freq'? AFAICS this is totally HW independent now, meaning we can just deal with the constant max value in the core and remove messing with with it in the drivers. If you don't have such a series, I am willing to work on this. All the best, Wolfram
diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c index cb220807d925..eeb9612a666f 100644 --- a/drivers/rtc/rtc-rzn1.c +++ b/drivers/rtc/rtc-rzn1.c @@ -19,6 +19,7 @@ #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/rtc.h> +#include <linux/spinlock.h> #define RZN1_RTC_CTL0 0x00 #define RZN1_RTC_CTL0_SLSB_SUBU 0 @@ -27,6 +28,7 @@ #define RZN1_RTC_CTL0_CE BIT(7) #define RZN1_RTC_CTL1 0x04 +#define RZN1_RTC_CTL1_1SE BIT(3) #define RZN1_RTC_CTL1_ALME BIT(4) #define RZN1_RTC_CTL2 0x08 @@ -58,6 +60,13 @@ struct rzn1_rtc { struct rtc_device *rtcdev; void __iomem *base; + /* + * Protects access to RZN1_RTC_CTL1 reg. rtc_lock with threaded_irqs + * would introduce race conditions when switching interrupts because + * of potential sleeps + */ + spinlock_t ctl1_access_lock; + struct rtc_time tm_alarm; }; static void rzn1_rtc_get_time_snapshot(struct rzn1_rtc *rtc, struct rtc_time *tm) @@ -135,8 +144,38 @@ static int rzn1_rtc_set_time(struct device *dev, struct rtc_time *tm) static irqreturn_t rzn1_rtc_alarm_irq(int irq, void *dev_id) { struct rzn1_rtc *rtc = dev_id; + u32 ctl1, set_irq_bits = 0; + + if (rtc->tm_alarm.tm_sec == 0) + rtc_update_irq(rtc->rtcdev, 1, RTC_AF | RTC_IRQF); + else + /* Switch to 1s interrupts */ + set_irq_bits = RZN1_RTC_CTL1_1SE; - rtc_update_irq(rtc->rtcdev, 1, RTC_AF | RTC_IRQF); + guard(spinlock)(&rtc->ctl1_access_lock); + + ctl1 = readl(rtc->base + RZN1_RTC_CTL1); + ctl1 &= ~RZN1_RTC_CTL1_ALME; + ctl1 |= set_irq_bits; + writel(ctl1, rtc->base + RZN1_RTC_CTL1); + + return IRQ_HANDLED; +} + +static irqreturn_t rzn1_rtc_1s_irq(int irq, void *dev_id) +{ + struct rzn1_rtc *rtc = dev_id; + u32 ctl1; + + if (readl(rtc->base + RZN1_RTC_SECC) == bin2bcd(rtc->tm_alarm.tm_sec)) { + guard(spinlock)(&rtc->ctl1_access_lock); + + ctl1 = readl(rtc->base + RZN1_RTC_CTL1); + ctl1 &= ~RZN1_RTC_CTL1_1SE; + writel(ctl1, rtc->base + RZN1_RTC_CTL1); + + rtc_update_irq(rtc->rtcdev, 1, RTC_AF | RTC_IRQF); + } return IRQ_HANDLED; } @@ -144,14 +183,38 @@ static irqreturn_t rzn1_rtc_alarm_irq(int irq, void *dev_id) static int rzn1_rtc_alarm_irq_enable(struct device *dev, unsigned int enable) { struct rzn1_rtc *rtc = dev_get_drvdata(dev); - u32 ctl1 = readl(rtc->base + RZN1_RTC_CTL1); + struct rtc_time *tm = &rtc->tm_alarm, tm_now; + u32 ctl1; + int ret; - if (enable) - ctl1 |= RZN1_RTC_CTL1_ALME; - else - ctl1 &= ~RZN1_RTC_CTL1_ALME; + guard(spinlock_irqsave)(&rtc->ctl1_access_lock); - writel(ctl1, rtc->base + RZN1_RTC_CTL1); + ctl1 = readl(rtc->base + RZN1_RTC_CTL1); + + if (enable) { + /* + * Use alarm interrupt if alarm time is at least a minute away + * or less than a minute but in the next minute. Otherwise use + * 1 second interrupt to wait for the proper second + */ + do { + ctl1 &= ~(RZN1_RTC_CTL1_ALME | RZN1_RTC_CTL1_1SE); + + ret = rzn1_rtc_read_time(dev, &tm_now); + if (ret) + return ret; + + if (rtc_tm_sub(tm, &tm_now) > 59 || tm->tm_min != tm_now.tm_min) + ctl1 |= RZN1_RTC_CTL1_ALME; + else + ctl1 |= RZN1_RTC_CTL1_1SE; + + writel(ctl1, rtc->base + RZN1_RTC_CTL1); + } while (readl(rtc->base + RZN1_RTC_SECC) != bin2bcd(tm_now.tm_sec)); + } else { + ctl1 &= ~(RZN1_RTC_CTL1_ALME | RZN1_RTC_CTL1_1SE); + writel(ctl1, rtc->base + RZN1_RTC_CTL1); + } return 0; } @@ -185,7 +248,7 @@ static int rzn1_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) } ctl1 = readl(rtc->base + RZN1_RTC_CTL1); - alrm->enabled = !!(ctl1 & RZN1_RTC_CTL1_ALME); + alrm->enabled = !!(ctl1 & (RZN1_RTC_CTL1_ALME | RZN1_RTC_CTL1_1SE)); return 0; } @@ -216,6 +279,8 @@ static int rzn1_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) writel(bin2bcd(tm->tm_hour), rtc->base + RZN1_RTC_ALH); writel(BIT(wday), rtc->base + RZN1_RTC_ALW); + rtc->tm_alarm = alrm->time; + rzn1_rtc_alarm_irq_enable(dev, alrm->enabled); return 0; @@ -304,7 +369,7 @@ static const struct rtc_class_ops rzn1_rtc_ops = { static int rzn1_rtc_probe(struct platform_device *pdev) { struct rzn1_rtc *rtc; - int alarm_irq; + int irq; int ret; rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL); @@ -317,9 +382,9 @@ static int rzn1_rtc_probe(struct platform_device *pdev) if (IS_ERR(rtc->base)) return dev_err_probe(&pdev->dev, PTR_ERR(rtc->base), "Missing reg\n"); - alarm_irq = platform_get_irq(pdev, 0); - if (alarm_irq < 0) - return alarm_irq; + irq = platform_get_irq_byname(pdev, "alarm"); + if (irq < 0) + return irq; rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev); if (IS_ERR(rtc->rtcdev)) @@ -329,8 +394,6 @@ static int rzn1_rtc_probe(struct platform_device *pdev) rtc->rtcdev->range_max = RTC_TIMESTAMP_END_2099; rtc->rtcdev->alarm_offset_max = 7 * 86400; rtc->rtcdev->ops = &rzn1_rtc_ops; - set_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->rtcdev->features); - clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtcdev->features); ret = devm_pm_runtime_enable(&pdev->dev); if (ret < 0) @@ -349,13 +412,24 @@ static int rzn1_rtc_probe(struct platform_device *pdev) /* Disable all interrupts */ writel(0, rtc->base + RZN1_RTC_CTL1); - ret = devm_request_irq(&pdev->dev, alarm_irq, rzn1_rtc_alarm_irq, 0, - dev_name(&pdev->dev), rtc); + spin_lock_init(&rtc->ctl1_access_lock); + + ret = devm_request_irq(&pdev->dev, irq, rzn1_rtc_alarm_irq, 0, "RZN1 RTC Alarm", rtc); if (ret) { - dev_err(&pdev->dev, "RTC timer interrupt not available\n"); + dev_err(&pdev->dev, "RTC alarm interrupt not available\n"); goto dis_runtime_pm; } + irq = platform_get_irq_byname_optional(pdev, "pps"); + if (irq >= 0) + ret = devm_request_irq(&pdev->dev, irq, rzn1_rtc_1s_irq, 0, "RZN1 RTC 1s", rtc); + + if (irq < 0 || ret) { + set_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->rtcdev->features); + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtcdev->features); + dev_warn(&pdev->dev, "RTC pps interrupt not available. Alarm has only minute accuracy\n"); + } + ret = devm_rtc_register_device(rtc->rtcdev); if (ret) goto dis_runtime_pm;
The hardware alarm only supports one-minute accuracy which is coarse and disables UIE usage. Use the 1-second interrupt to achieve per-second accuracy. It is activated once we hit the per-minute alarm. The new feature is optional. When there is no 1-second interrupt, old behaviour with per-minute accuracy is used as before. With this feature, all tests of 'rtctest' are successfully passed. Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- Tested with the Renesas RZ/N1D board. Besides 'rtctest', I did some manual testing with 'rtc' on top trying to stresstest corner cases. Looking forward to comments. AFAICS, this is the first driver trying to overcome the per-minute limitation using 1-second interrupts. Change since v1: * consider 1s interrupt when setting the alarm->enabled flag drivers/rtc/rtc-rzn1.c | 108 ++++++++++++++++++++++++++++++++++------- 1 file changed, 91 insertions(+), 17 deletions(-)