[PATCH v2] rtc: pcf85063: add 1 Hz update interrupt (UIE) support

[PATCH v2] rtc: pcf85063: add 1 Hz update interrupt (UIE) support

[A. Sverdlin]

From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

The PCF85063A/RV8263 have no dedicated 1 Hz update interrupt, but their
countdown timer can be sourced from a 1 Hz clock (TCF = 1 Hz, T = 1) to
generate a periodic interrupt on the INT pin. Use it to offload UIE to
the hardware, following the pattern of other RTC drivers: the timer is
enabled from set_alarm()/alarm_irq_enable() whenever uie_rtctimer is
active, and the IRQ handler reports RTC_UF when the timer flag (TF) is
set.

RTC_FEATURE_UPDATE_INTERRUPT is now only cleared when no usable interrupt
is wired, preserving the polling emulation for those configurations.

Co-developed-by: Adrian Freihofer <adrian.freihofer@siemens.com>
Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
Changelog (addressed Sashiko comments):
- preserve AF bit in pcf85063_set_timer_1hz()
- faster and less racy (regarding AF flag) pcf85063_rtc_handle_irq()

 drivers/rtc/rtc-pcf85063.c | 100 ++++++++++++++++++++++++++++++++++---
 1 file changed, 92 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
index 01e209d88f5f3..e10e6aaa37d52 100644
--- a/drivers/rtc/rtc-pcf85063.c
+++ b/drivers/rtc/rtc-pcf85063.c
@@ -44,6 +44,7 @@
 #define PCF85063_REG_CTRL2		0x01
 #define PCF85063_CTRL2_AF		BIT(6)
 #define PCF85063_CTRL2_AIE		BIT(7)
+#define PCF85063_CTRL2_TF		BIT(3)
 
 #define PCF85063_REG_OFFSET		0x02
 #define PCF85063_OFFSET_SIGN_BIT	6	/* 2's complement sign bit */
@@ -63,6 +64,14 @@
 #define PCF85063_REG_ALM_S		0x0b
 #define PCF85063_AEN			BIT(7)
 
+#define PCF85063_REG_TIMER_VALUE	0x10
+#define PCF85063_REG_TIMER_MODE		0x11
+#define PCF85063_TIMER_MODE_TI_TP	BIT(0)
+#define PCF85063_TIMER_MODE_TIE		BIT(1)
+#define PCF85063_TIMER_MODE_TE		BIT(2)
+#define PCF85063_TIMER_MODE_TCF_MASK	GENMASK(4, 3)
+#define PCF85063_TIMER_MODE_TCF_1HZ	(2 << 3)
+
 struct pcf85063_config {
 	struct regmap_config regmap;
 	unsigned has_alarms:1;
@@ -188,12 +197,65 @@ static int pcf85063_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	return 0;
 }
 
+/*
+ * The chip has no dedicated 1 Hz update interrupt. Drive the countdown timer
+ * at 1 Hz to emulate it, so the RTC core can offload UIE to the hardware
+ * instead of polling. A free-running timer is left untouched to preserve its
+ * phase across the per-second re-arming done by the core.
+ */
+static int pcf85063_set_timer_1hz(struct pcf85063 *pcf85063, bool enable)
+{
+	unsigned int mask = PCF85063_TIMER_MODE_TCF_MASK |
+			    PCF85063_TIMER_MODE_TIE |
+			    PCF85063_TIMER_MODE_TI_TP |
+			    PCF85063_TIMER_MODE_TE;
+	unsigned int mode = 0;
+	unsigned int cur;
+	int ret;
+
+	if (enable)
+		mode = PCF85063_TIMER_MODE_TCF_1HZ | PCF85063_TIMER_MODE_TIE |
+		       PCF85063_TIMER_MODE_TI_TP | PCF85063_TIMER_MODE_TE;
+
+	ret = regmap_read(pcf85063->regmap, PCF85063_REG_TIMER_MODE, &cur);
+	if (ret)
+		return ret;
+
+	if ((cur & mask) == mode)
+		return 0;
+
+	/* Stop the counter before changing its reload value. */
+	ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_TIMER_MODE,
+				 PCF85063_TIMER_MODE_TE, 0);
+	if (ret)
+		return ret;
+
+	if (enable) {
+		ret = regmap_write(pcf85063->regmap, PCF85063_REG_TIMER_VALUE, 1);
+		if (ret)
+			return ret;
+
+		/* Clear TF but preserve AF */
+		ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
+					 PCF85063_CTRL2_TF | PCF85063_CTRL2_AF, PCF85063_CTRL2_AF);
+		if (ret)
+			return ret;
+	}
+
+	return regmap_update_bits(pcf85063->regmap, PCF85063_REG_TIMER_MODE,
+				  mask, mode);
+}
+
 static int pcf85063_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
 	struct pcf85063 *pcf85063 = dev_get_drvdata(dev);
 	u8 buf[5];
 	int ret;
 
+	ret = pcf85063_set_timer_1hz(pcf85063, pcf85063->rtc->uie_rtctimer.enabled);
+	if (ret)
+		return ret;
+
 	buf[0] = bin2bcd(alrm->time.tm_sec);
 	buf[1] = bin2bcd(alrm->time.tm_min);
 	buf[2] = bin2bcd(alrm->time.tm_hour);
@@ -212,22 +274,31 @@ static int pcf85063_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 	return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
 				  PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF,
-				  alrm->enabled ? PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF : PCF85063_CTRL2_AF);
+				  pcf85063->rtc->aie_timer.enabled ?
+				  PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF : PCF85063_CTRL2_AF);
 }
 
 static int pcf85063_rtc_alarm_irq_enable(struct device *dev,
 					 unsigned int enabled)
 {
 	struct pcf85063 *pcf85063 = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pcf85063_set_timer_1hz(pcf85063, pcf85063->rtc->uie_rtctimer.enabled);
+	if (ret)
+		return ret;
 
 	return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
 				  PCF85063_CTRL2_AIE,
-				  enabled ? PCF85063_CTRL2_AIE : 0);
+				  pcf85063->rtc->aie_timer.enabled ?
+				  PCF85063_CTRL2_AIE : 0);
 }
 
 static irqreturn_t pcf85063_rtc_handle_irq(int irq, void *dev_id)
 {
 	struct pcf85063 *pcf85063 = dev_id;
+	unsigned long events = RTC_IRQF;
+	irqreturn_t ret = IRQ_NONE;
 	unsigned int val;
 	int err;
 
@@ -236,14 +307,26 @@ static irqreturn_t pcf85063_rtc_handle_irq(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	if (val & PCF85063_CTRL2_AF) {
-		rtc_update_irq(pcf85063->rtc, 1, RTC_IRQF | RTC_AF);
-		regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
-				   PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF,
-				   0);
-		return IRQ_HANDLED;
+		events |= RTC_AF;
+		val &= ~(PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF);
+		ret = IRQ_HANDLED;
+	} else {
+		/* Preserve AF if it comes after the above regmap_read() */
+		val |= PCF85063_CTRL2_AF;
+	}
+
+	if (val & PCF85063_CTRL2_TF) {
+		events |= RTC_UF;
+		val &= ~PCF85063_CTRL2_TF;
+		ret = IRQ_HANDLED;
+	}
+
+	if (ret == IRQ_HANDLED) {
+		regmap_write(pcf85063->regmap, PCF85063_REG_CTRL2, val);
+		rtc_update_irq(pcf85063->rtc, 1, events);
 	}
 
-	return IRQ_NONE;
+	return ret;
 }
 
 static int pcf85063_read_offset(struct device *dev, long *offset)
@@ -607,6 +690,7 @@ static int pcf85063_probe(struct device *dev, struct regmap *regmap, int irq,
 			dev_warn(&pcf85063->rtc->dev,
 				 "unable to request IRQ, alarms disabled\n");
 		} else {
+			set_bit(RTC_FEATURE_UPDATE_INTERRUPT, pcf85063->rtc->features);
 			set_bit(RTC_FEATURE_ALARM, pcf85063->rtc->features);
 			device_init_wakeup(dev, true);
 			err = dev_pm_set_wake_irq(dev, irq);
-- 
2.55.0

Re: [PATCH v2] rtc: pcf85063: add 1 Hz update interrupt (UIE) support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Read-modify-write operations on CTRL2 inadvertently clear Write-0-to-Clear (W0C) TF and AF interrupt flags, causing dropped interrupts.
- [High] A non-atomic read-modify-write sequence in the IRQ handler causes a data race that overwrites concurrent register updates.

Pre-existing issues:
- [High] Requesting the threaded IRQ before registering the RTC device creates a devres ordering inversion leading to a Use-After-Free.
- [High] Missing of_clk_del_provider() cleanup causes a Use-After-Free of the OF clock provider on driver unbind.
--

--- Patch [1]: [PATCH v2] rtc: pcf85063: add 1 Hz update interrupt (UIE) support ---
commit 79dacfbe2c0fd3c5e8261fd4569fedb1fe4b32a5
Author: Alexander Sverdlin <alexander.sverdlin@siemens.com>

rtc: pcf85063: add 1 Hz update interrupt (UIE) support
    
The PCF85063A/RV8263 have no dedicated 1 Hz update interrupt, but their
countdown timer can be sourced from a 1 Hz clock to generate a periodic
interrupt. This patch offloads UIE to the hardware by enabling this timer
when uie_rtctimer is active, reporting RTC_UF when the timer flag is set.

> diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
> --- a/drivers/rtc/rtc-pcf85063.c
> +++ b/drivers/rtc/rtc-pcf85063.c

[ ... ]

> @@ -212,22 +274,31 @@ static int pcf85063_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  
>  	return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
>  				  PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF,
> -				  alrm->enabled ? PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF : PCF85063_CTRL2_AF);
> +				  pcf85063->rtc->aie_timer.enabled ?
> +				  PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF : PCF85063_CTRL2_AF);
>  }

[Severity: High]
Does this code inadvertently drop the TF interrupt flag when updating the
alarm?

If PCF85063_CTRL2_TF is a write-0-to-clear bit, excluding it from the
regmap_update_bits() mask means it will be written back as 0 if it was
already set in the register, clearing any pending timer interrupt.

[ ... ]

> @@ -274,22 +287,31 @@ static int pcf85063_rtc_alarm_irq_enable(struct device *dev,
>  					 unsigned int enabled)
>  {
>  	struct pcf85063 *pcf85063 = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pcf85063_set_timer_1hz(pcf85063, pcf85063->rtc->uie_rtctimer.enabled);
> +	if (ret)
> +		return ret;
>  
>  	return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
>  				  PCF85063_CTRL2_AIE,
> -				  enabled ? PCF85063_CTRL2_AIE : 0);
> +				  pcf85063->rtc->aie_timer.enabled ?
> +				  PCF85063_CTRL2_AIE : 0);
>  }

[Severity: High]
Could this function also clear pending interrupts?

Similar to pcf85063_rtc_set_alarm(), both AF and TF are excluded from the
mask here. If they are write-0-to-clear flags, regmap_update_bits() will
write them back as read (0), neutralizing any concurrent assertions.

[ ... ]

> @@ -236,14 +307,26 @@ static irqreturn_t pcf85063_rtc_handle_irq(int irq, void *dev_id)
>  {
>  	struct pcf85063 *pcf85063 = dev_id;
> +	unsigned long events = RTC_IRQF;
> +	irqreturn_t ret = IRQ_NONE;
>  	unsigned int val;
>  	int err;
>  
>  	err = regmap_read(pcf85063->regmap, PCF85063_REG_CTRL2, &val);
>  	if (err)
>  		return IRQ_NONE;
>  
>  	if (val & PCF85063_CTRL2_AF) {
> -		rtc_update_irq(pcf85063->rtc, 1, RTC_IRQF | RTC_AF);
> -		regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
> -				   PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF,
> -				   0);
> -		return IRQ_HANDLED;
> +		events |= RTC_AF;
> +		val &= ~(PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF);
> +		ret = IRQ_HANDLED;
> +	} else {
> +		/* Preserve AF if it comes after the above regmap_read() */
> +		val |= PCF85063_CTRL2_AF;
> +	}
> +
> +	if (val & PCF85063_CTRL2_TF) {
> +		events |= RTC_UF;
> +		val &= ~PCF85063_CTRL2_TF;
> +		ret = IRQ_HANDLED;
> +	}

[Severity: High]
Does this code risk clearing an unhandled UIE interrupt if TF asserts
after the register read?

If TF asserts post-read, the read value will have it as 0. Because there is
no else branch to preserve TF (like the one preserving AF), the subsequent
regmap_write() will commit a 0 for TF, clearing the flag before it is
handled.

> +	if (ret == IRQ_HANDLED) {
> +		regmap_write(pcf85063->regmap, PCF85063_REG_CTRL2, val);
> +		rtc_update_irq(pcf85063->rtc, 1, events);
>  	}
>  
> -	return IRQ_NONE;
> +	return ret;
>  }

[Severity: High]
Is there a data race in this read-modify-write sequence?

pcf85063_rtc_handle_irq() drops the regmap lock between regmap_read() and
regmap_write(). If a process-context change happens in this unlocked window,
such as AIE toggling via pcf85063_rtc_alarm_irq_enable(), will it be
overwritten and lost when the IRQ handler commits its stale read data?

[ ... ]

> @@ -607,6 +690,7 @@ static int pcf85063_probe(struct device *dev, struct regmap *regmap, int irq,
>  		err = devm_request_threaded_irq(dev, irq,
>  						NULL, pcf85063_rtc_handle_irq,
>  						irqflags | IRQF_ONESHOT,
>  						"pcf85063", pcf85063);
>  		if (err) {
>  			dev_warn(&pcf85063->rtc->dev,
>  				 "unable to request IRQ, alarms disabled\n");
>  		} else {
> +			set_bit(RTC_FEATURE_UPDATE_INTERRUPT, pcf85063->rtc->features);
>  			set_bit(RTC_FEATURE_ALARM, pcf85063->rtc->features);

[Severity: High]
This is a pre-existing issue, but does the devres allocation order here cause
a use-after-free on teardown?

Earlier in pcf85063_probe(), devm_request_threaded_irq() is called before
devm_rtc_register_device(). During teardown, rtc_device_unregister() runs
first, but the IRQ remains active.

If the timer fires at this time, pcf85063_rtc_handle_irq() executes and
rtc_update_irq() re-schedules the irqwork. Devres will subsequently free the
rtc structure, leaving the system workqueue to dereference freed memory.

[ ... ]

>  	nvmem_cfg.priv = pcf85063->regmap;
>  	devm_rtc_nvmem_register(pcf85063->rtc, &nvmem_cfg);
>  
>  #ifdef CONFIG_COMMON_CLK
>  	/* register clk in common clk framework */
>  	pcf85063_clkout_register_clk(pcf85063);
>  #endif
>  
>  	return devm_rtc_register_device(pcf85063->rtc);
>  }

[Severity: High]
This is a pre-existing issue, but is there a missing cleanup for the OF clock
provider?

In pcf85063_clkout_register_clk(), of_clk_add_provider() is invoked but there
appears to be no matching of_clk_del_provider(). When devres frees the clock
structure during driver unbind, does the global OF list retain a dangling
pointer, triggering a use-after-free on the next device tree clock
resolution?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820112437.3715237-1-alexander.sverdlin@siemens.com?part=1

Re: [PATCH v2] rtc: pcf85063: add 1 Hz update interrupt (UIE) support

[Sverdlin, Alexander]

Hi all,

On Thu, 2026-08-20 at 11:39 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> 
> New issues:
> - [High] Read-modify-write operations on CTRL2 inadvertently clear Write-0-to-Clear (W0C) TF and AF interrupt flags, causing dropped interrupts.
> - [High] A non-atomic read-modify-write sequence in the IRQ handler causes a data race that overwrites concurrent register updates.
> 
> Pre-existing issues:
> - [High] Requesting the threaded IRQ before registering the RTC device creates a devres ordering inversion leading to a Use-After-Free.
> - [High] Missing of_clk_del_provider() cleanup causes a Use-After-Free of the OF clock provider on driver unbind.
> --
> 
> --- Patch [1]: [PATCH v2] rtc: pcf85063: add 1 Hz update interrupt (UIE) support ---
> commit 79dacfbe2c0fd3c5e8261fd4569fedb1fe4b32a5
> Author: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> rtc: pcf85063: add 1 Hz update interrupt (UIE) support
>     
> The PCF85063A/RV8263 have no dedicated 1 Hz update interrupt, but their
> countdown timer can be sourced from a 1 Hz clock to generate a periodic
> interrupt. This patch offloads UIE to the hardware by enabling this timer
> when uie_rtctimer is active, reporting RTC_UF when the timer flag is set.
> 
> > diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
> > --- a/drivers/rtc/rtc-pcf85063.c
> > +++ b/drivers/rtc/rtc-pcf85063.c
> 
> [ ... ]
> 
> > @@ -212,22 +274,31 @@ static int pcf85063_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> >  
> >  	return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
> >  				  PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF,
> > -				  alrm->enabled ? PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF : PCF85063_CTRL2_AF);
> > +				  pcf85063->rtc->aie_timer.enabled ?
> > +				  PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF : PCF85063_CTRL2_AF);
> >  }
> 
> [Severity: High]
> Does this code inadvertently drop the TF interrupt flag when updating the
> alarm?
> 
> If PCF85063_CTRL2_TF is a write-0-to-clear bit, excluding it from the
> regmap_update_bits() mask means it will be written back as 0 if it was
> already set in the register, clearing any pending timer interrupt.

will be fixed in v3

> > @@ -274,22 +287,31 @@ static int pcf85063_rtc_alarm_irq_enable(struct device *dev,
> >  					 unsigned int enabled)
> >  {
> >  	struct pcf85063 *pcf85063 = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = pcf85063_set_timer_1hz(pcf85063, pcf85063->rtc->uie_rtctimer.enabled);
> > +	if (ret)
> > +		return ret;
> >  
> >  	return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
> >  				  PCF85063_CTRL2_AIE,
> > -				  enabled ? PCF85063_CTRL2_AIE : 0);
> > +				  pcf85063->rtc->aie_timer.enabled ?
> > +				  PCF85063_CTRL2_AIE : 0);
> >  }
> 
> [Severity: High]
> Could this function also clear pending interrupts?
> 
> Similar to pcf85063_rtc_set_alarm(), both AF and TF are excluded from the
> mask here. If they are write-0-to-clear flags, regmap_update_bits() will
> write them back as read (0), neutralizing any concurrent assertions.

will be fixed in v3

> > @@ -236,14 +307,26 @@ static irqreturn_t pcf85063_rtc_handle_irq(int irq, void *dev_id)
> >  {
> >  	struct pcf85063 *pcf85063 = dev_id;
> > +	unsigned long events = RTC_IRQF;
> > +	irqreturn_t ret = IRQ_NONE;
> >  	unsigned int val;
> >  	int err;
> >  
> >  	err = regmap_read(pcf85063->regmap, PCF85063_REG_CTRL2, &val);
> >  	if (err)
> >  		return IRQ_NONE;
> >  
> >  	if (val & PCF85063_CTRL2_AF) {
> > -		rtc_update_irq(pcf85063->rtc, 1, RTC_IRQF | RTC_AF);
> > -		regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
> > -				   PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF,
> > -				   0);
> > -		return IRQ_HANDLED;
> > +		events |= RTC_AF;
> > +		val &= ~(PCF85063_CTRL2_AIE | PCF85063_CTRL2_AF);
> > +		ret = IRQ_HANDLED;
> > +	} else {
> > +		/* Preserve AF if it comes after the above regmap_read() */
> > +		val |= PCF85063_CTRL2_AF;
> > +	}
> > +
> > +	if (val & PCF85063_CTRL2_TF) {
> > +		events |= RTC_UF;
> > +		val &= ~PCF85063_CTRL2_TF;
> > +		ret = IRQ_HANDLED;
> > +	}
> 
> [Severity: High]
> Does this code risk clearing an unhandled UIE interrupt if TF asserts
> after the register read?
> 
> If TF asserts post-read, the read value will have it as 0. Because there is
> no else branch to preserve TF (like the one preserving AF), the subsequent
> regmap_write() will commit a 0 for TF, clearing the flag before it is
> handled.

will be fixed in v3

> > +	if (ret == IRQ_HANDLED) {
> > +		regmap_write(pcf85063->regmap, PCF85063_REG_CTRL2, val);
> > +		rtc_update_irq(pcf85063->rtc, 1, events);
> >  	}
> >  
> > -	return IRQ_NONE;
> > +	return ret;
> >  }
> 
> [Severity: High]
> Is there a data race in this read-modify-write sequence?
> 
> pcf85063_rtc_handle_irq() drops the regmap lock between regmap_read() and
> regmap_write(). If a process-context change happens in this unlocked window,
> such as AIE toggling via pcf85063_rtc_alarm_irq_enable(), will it be
> overwritten and lost when the IRQ handler commits its stale read data?

This is pre-existing issue, will fix is in a separate patch preceeding v3

> > @@ -607,6 +690,7 @@ static int pcf85063_probe(struct device *dev, struct regmap *regmap, int irq,
> >  		err = devm_request_threaded_irq(dev, irq,
> >  						NULL, pcf85063_rtc_handle_irq,
> >  						irqflags | IRQF_ONESHOT,
> >  						"pcf85063", pcf85063);
> >  		if (err) {
> >  			dev_warn(&pcf85063->rtc->dev,
> >  				 "unable to request IRQ, alarms disabled\n");
> >  		} else {
> > +			set_bit(RTC_FEATURE_UPDATE_INTERRUPT, pcf85063->rtc->features);
> >  			set_bit(RTC_FEATURE_ALARM, pcf85063->rtc->features);
> 
> [Severity: High]
> This is a pre-existing issue, but does the devres allocation order here cause
> a use-after-free on teardown?
> 
> Earlier in pcf85063_probe(), devm_request_threaded_irq() is called before
> devm_rtc_register_device(). During teardown, rtc_device_unregister() runs
> first, but the IRQ remains active.
> 
> If the timer fires at this time, pcf85063_rtc_handle_irq() executes and
> rtc_update_irq() re-schedules the irqwork. Devres will subsequently free the
> rtc structure, leaving the system workqueue to dereference freed memory.

Seems to be false-positive:

rtc_device_unregister() does not free the struct rtc_device; it only drops the
cdev and clears rtc->ops. The object is reference-counted and freed only by the
devm_rtc_release_device() action (put_device()), which is registered first and
therefore runs last.

devres teardown is thus:

1. devm_rtc_unregister_device() — clears ops, removes cdev; does not free.
2. free_irq() — synchronizes with the threaded handler.
3. devm_rtc_release_device() — put_device() -> rtc_device_release(), which runs
cancel_work_sync(&rtc->irqwork) before kfree().
So in the only live-IRQ window between 1 and 2, rtc_update_irq() just schedules
irqwork on a still-valid object; step 2 stops the handler and step 3 flushes the
work before freeing.

> >  	nvmem_cfg.priv = pcf85063->regmap;
> >  	devm_rtc_nvmem_register(pcf85063->rtc, &nvmem_cfg);
> >  
> >  #ifdef CONFIG_COMMON_CLK
> >  	/* register clk in common clk framework */
> >  	pcf85063_clkout_register_clk(pcf85063);
> >  #endif
> >  
> >  	return devm_rtc_register_device(pcf85063->rtc);
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but is there a missing cleanup for the OF clock
> provider?
> 
> In pcf85063_clkout_register_clk(), of_clk_add_provider() is invoked but there
> appears to be no matching of_clk_del_provider(). When devres frees the clock
> structure during driver unbind, does the global OF list retain a dangling
> pointer, triggering a use-after-free on the next device tree clock
> resolution?

will fix is in a separate patch preceeding v3

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com