[PATCH 1/2] rtc: Add UIE handling ability to rtc_class_ops

[PATCH 1/2] rtc: Add UIE handling ability to rtc_class_ops

[Csókás, Bence]

Currently, Update Interrupt Enable is performed by emulating
it with either polling, or alarm interrupts. Some RTCs, however,
can directly provide Update Interrupts.

Signed-off-by: "Csókás, Bence" <csokas.bence@prolan.hu>
---
 drivers/rtc/interface.c | 8 ++++++++
 include/linux/rtc.h     | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 1b63111cdda2..9e1eac441c54 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -581,6 +581,14 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
 #endif
 	}
 
+	if (rtc->ops && rtc->ops->update_irq_enable) {
+		err = rtc->ops->update_irq_enable(rtc->dev.parent, enabled);
+		if (!err)
+			rtc->uie_rtctimer.enabled = enabled;
+		if (err != -EINVAL)
+			goto out;
+	}
+
 	if (enabled) {
 		struct rtc_time tm;
 		ktime_t now, onesec;
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 5f8e438a0312..63dad8dfe278 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -68,6 +68,7 @@ struct rtc_class_ops {
 	int (*set_offset)(struct device *, long offset);
 	int (*param_get)(struct device *, struct rtc_param *param);
 	int (*param_set)(struct device *, struct rtc_param *param);
+	int (*update_irq_enable)(struct device *, unsigned int enabled);
 };
 
 struct rtc_device;
-- 
2.34.1

Re: [PATCH 1/2] rtc: Add UIE handling ability to rtc_class_ops

[Alexandre Belloni]

Hello,

On 28/05/2024 18:13:14+0200, Csókás, Bence wrote:
> Currently, Update Interrupt Enable is performed by emulating
> it with either polling, or alarm interrupts. Some RTCs, however,
> can directly provide Update Interrupts.
> 

This has been removed from the kernel 13 years ago. What is your use
case to reintroduce it?


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

Re: [PATCH 1/2] rtc: Add UIE handling ability to rtc_class_ops

[Csókás Bence]

Hi!

On 5/28/24 19:56, Alexandre Belloni wrote:
> Hello,
> 
> On 28/05/2024 18:13:14+0200, Csókás, Bence wrote:
>> Currently, Update Interrupt Enable is performed by emulating
>> it with either polling, or alarm interrupts. Some RTCs, however,
>> can directly provide Update Interrupts.
>>
> 
> This has been removed from the kernel 13 years ago. What is your use
> case to reintroduce it?

The rationale was in the cover letter. Currently, the RTC core uses the 
alarm system to implement UI, but this causes lost updates on PCF2129. 
However, it has built-in Update Interrupt capabilities in the form of 
the Second Interrupt, therefore we should just use that.

So my question would instead be: what was the rationale of removing it 
in the first place?

Bence

Re: [PATCH 1/2] rtc: Add UIE handling ability to rtc_class_ops

[Alexandre Belloni]

On 30/05/2024 11:59:47+0200, Csókás Bence wrote:
> Hi!
> 
> On 5/28/24 19:56, Alexandre Belloni wrote:
> > Hello,
> > 
> > On 28/05/2024 18:13:14+0200, Csókás, Bence wrote:
> > > Currently, Update Interrupt Enable is performed by emulating
> > > it with either polling, or alarm interrupts. Some RTCs, however,
> > > can directly provide Update Interrupts.
> > > 
> > 
> > This has been removed from the kernel 13 years ago. What is your use
> > case to reintroduce it?
> 
> The rationale was in the cover letter. Currently, the RTC core uses the
> alarm system to implement UI, but this causes lost updates on PCF2129.
> However, it has built-in Update Interrupt capabilities in the form of the
> Second Interrupt, therefore we should just use that.
> 
> So my question would instead be: what was the rationale of removing it in
> the first place?

Well, you didn't answer my question which is why do you need UIE and
especially UIE from the RTC? Using the timer emulation is probably what
you want because it is in sync with your system time.


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

Re: [PATCH 1/2] rtc: Add UIE handling ability to rtc_class_ops

[Csókás Bence]

Hi!

On 5/30/24 12:35, Alexandre Belloni wrote:
> Well, you didn't answer my question which is why do you need UIE and
> especially UIE from the RTC? Using the timer emulation is probably what
> you want because it is in sync with your system time.

We set up `chrony` to use UI events as a time reference. Upon startup, 
`chrony` opens the RTC with UIE_ON, reads the initial RTC value on the 
first transition, then keeps the system clock in sync with the RTC using 
the interrupt signal. So we want to synchronize the system time to UIE, 
not the other way around.

 >> So my question would instead be: what was the rationale of removing 
it in
 >> the first place?

I was genuinely asking, because I couldn't find the original letter, 
only subsequent ones from 2-4 years ago, removing bits and pieces from 
individual RTC device drivers.

Bence