[PATCH 0/2] rtc: renesas-rtca3: Fix IRQ initialization and teardown ordering

[PATCH 0/2] rtc: renesas-rtca3: Fix IRQ initialization and teardown ordering

[Myeonghun Pak]

The RTCA3 alarm handler passes priv->rtc_dev to rtc_update_irq().
Registering managed IRQs before allocating the managed RTC lets devres
drop the RTC allocation reference before freeing and synchronizing the
IRQs. Masking the hardware sources in remove also does not wait for an
interrupt handler that has already entered.

Initialize the RTC before requesting IRQs so its allocation remains
valid until managed IRQ teardown on removal and probe failure. Then
stop both interrupt sources and synchronously disable both Linux IRQs
in remove, before managed RTC unregistration. Serialize removal with
RTC operations and reject later alarm enable or setup requests.

Compile-tested with Clang 22.1.2, W=1 (arm64). No runtime testing.
One existing FIELD_GET comparison warning also occurs when compiling
the unmodified driver with the same flags. No new compiler warnings.

Myeonghun Pak (2):
  rtc: renesas-rtca3: Initialize the RTC before requesting interrupts
  rtc: renesas-rtca3: Join interrupt handlers before unregistering the RTC

 drivers/rtc/rtc-renesas-rtca3.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

[PATCH 1/2] rtc: renesas-rtca3: Initialize the RTC before requesting interrupts

[Myeonghun Pak]

The alarm handler passes priv->rtc_dev to rtc_update_irq(), but the RTC
allocation is managed after the interrupt requests. Devres can therefore
drop the RTC's allocation reference before freeing and synchronizing the
alarm IRQ. Masking alarm interrupts in remove does not wait for a handler
that has already entered, and a pending alarm flag can still be handled.

Allocate the RTC before requesting the IRQs so that the allocation
reference remains valid until the IRQs have been freed and synchronized
on both removal and probe failure. Initialize its operations and time
range before requesting the IRQs as well, so an early alarm interrupt
cannot queue RTC work before the operations are available.

This issue was found by static analysis.

Fixes: d4488377609e ("rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC")
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Compile-tested with Clang 22.1.2, W=1 (arm64). No runtime testing.

One existing FIELD_GET comparison warning also occurs when compiling
the unmodified driver with the same flags. No new compiler warnings.

 drivers/rtc/rtc-renesas-rtca3.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
index b3875d041de5..c304ed60edc6 100644
--- a/drivers/rtc/rtc-renesas-rtca3.c
+++ b/drivers/rtc/rtc-renesas-rtca3.c
@@ -765,12 +765,6 @@ static int rtca3_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to setup the RTC!\n");
 
-	ret = rtca3_request_irqs(pdev, priv);
-	if (ret)
-		return ret;
-
-	device_init_wakeup(&pdev->dev, true);
-
 	priv->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
 	if (IS_ERR(priv->rtc_dev))
 		return PTR_ERR(priv->rtc_dev);
@@ -779,6 +773,12 @@ static int rtca3_probe(struct platform_device *pdev)
 	priv->rtc_dev->range_min = RTC_TIMESTAMP_BEGIN_2000;
 	priv->rtc_dev->range_max = RTC_TIMESTAMP_END_2099;
 
+	ret = rtca3_request_irqs(pdev, priv);
+	if (ret)
+		return ret;
+
+	device_init_wakeup(&pdev->dev, true);
+
 	return devm_rtc_register_device(priv->rtc_dev);
 }
 

Re: [PATCH 1/2] rtc: renesas-rtca3: Initialize the RTC before requesting interrupts

[sashiko-bot]

> The alarm handler passes priv->rtc_dev to rtc_update_irq(), but the RTC
> allocation is managed after the interrupt requests. Devres can therefore
> drop the RTC's allocation reference before freeing and synchronizing the
> alarm IRQ. Masking alarm interrupts in remove does not wait for a handler
> that has already entered, and a pending alarm flag can still be handled.
> 
> Allocate the RTC before requesting the IRQs so that the allocation
> reference remains valid until the IRQs have been freed and synchronized
> on both removal and probe failure. Initialize its operations and time
> range before requesting the IRQs as well, so an early alarm interrupt
> cannot queue RTC work before the operations are available.
> 
> This issue was found by static analysis.
> 
> Fixes: d4488377609e ("rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC")
> Assisted-by: LLM
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011225657.rtca3-irq-ordering.0@gmail.com?part=1

[PATCH 2/2] rtc: renesas-rtca3: Join interrupt handlers before unregistering the RTC

[Myeonghun Pak]

Masking AIE and PIE in remove does not wait for an interrupt handler
that has already entered. Managed RTC unregistration follows remove,
so such a handler can still report an alarm after the RTC operations
have been cleared.

Save the periodic IRQ number and disable both Linux IRQs synchronously
in remove. Wait for RTC operations under ops_lock before masking the
sources, and reject subsequent alarm setup and enable requests. This
prevents a concurrent alarm setup from re-enabling AIE or PIE after
removal masks them.

Drop the private spinlock before disabling the IRQs so that a handler
waiting for it can finish. Neither handler takes ops_lock. The managed
IRQ actions still release the IRQ resources during devres unwind.

This issue was found by source review.

Assisted-by: LLM
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Compile-tested with Clang 22.1.2, W=1 (arm64). No runtime testing.

One existing FIELD_GET comparison warning also occurs when compiling
the unmodified driver with the same flags. No new compiler warnings.

 drivers/rtc/rtc-renesas-rtca3.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
index c304ed60edc6..9b177a131c27 100644
--- a/drivers/rtc/rtc-renesas-rtca3.c
+++ b/drivers/rtc/rtc-renesas-rtca3.c
@@ -121,6 +121,8 @@ struct rtca3_ppb_per_cycle {
  * @lock: device lock
  * @ppb: ppb per cycle for each the available adjustment modes
  * @wakeup_irq: wakeup IRQ
+ * @period_irq: periodic IRQ
+ * @stopping: removal has started, protected by lock
  */
 struct rtca3_priv {
 	void __iomem *base;
@@ -131,6 +133,8 @@ struct rtca3_priv {
 	spinlock_t lock;
 	struct rtca3_ppb_per_cycle ppb;
 	int wakeup_irq;
+	int period_irq;
+	bool stopping;
 };
 
 static void rtca3_byte_update_bits(struct rtca3_priv *priv, u8 off, u8 mask, u8 val)
@@ -351,6 +355,9 @@ static int rtca3_alarm_irq_enable(struct device *dev, unsigned int enabled)
 
 	guard(spinlock_irqsave)(&priv->lock);
 
+	if (priv->stopping)
+		return -ENODEV;
+
 	return rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE, enabled);
 }
 
@@ -392,6 +399,9 @@ static int rtca3_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
 	int ret;
 
 	scoped_guard(spinlock_irqsave, &priv->lock) {
+		if (priv->stopping)
+			return -ENODEV;
+
 		tmp = readb(priv->base + RTCA3_RCR2);
 		if (!(tmp & RTCA3_RCR2_START))
 			return -EPERM;
@@ -684,6 +694,7 @@ static int rtca3_request_irqs(struct platform_device *pdev, struct rtca3_priv *p
 	ret = devm_request_irq(dev, irq, rtca3_periodic_handler, 0, "rtca3-period", priv);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to request period IRQ!\n");
+	priv->period_irq = irq;
 
 	/*
 	 * Driver doesn't implement carry handler. Just get the IRQ here
@@ -786,13 +797,18 @@ static void rtca3_remove(struct platform_device *pdev)
 {
 	struct rtca3_priv *priv = platform_get_drvdata(pdev);
 
-	guard(spinlock_irqsave)(&priv->lock);
+	/* Wait for RTC operations, including an alarm setup, to finish first. */
+	guard(mutex)(&priv->rtc_dev->ops_lock);
 
-	/*
-	 * Disable alarm, periodic interrupts. The RTC device cannot
-	 * power up the system.
-	 */
-	rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
+	scoped_guard(spinlock_irqsave, &priv->lock) {
+		priv->stopping = true;
+		/* Disable alarm and periodic interrupt sources. */
+		rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
+	}
+
+	/* Neither handler takes ops_lock. Do not hold priv->lock while waiting. */
+	disable_irq(priv->wakeup_irq);
+	disable_irq(priv->period_irq);
 }
 
 static int rtca3_suspend(struct device *dev)

Re: [PATCH 2/2] rtc: renesas-rtca3: Join interrupt handlers before unregistering the RTC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rtc: renesas-rtca3: mixing goto and scoped_guard in rtca3_set_alarm()
--

commit 847b7d6e6378d6cee33935541f2c60ff982bf01d
Author: Myeonghun Pak <mhun512@gmail.com>

rtc: renesas-rtca3: Join interrupt handlers before unregistering the RTC

This commit addresses an issue where interrupt handlers might still run
after the RTC operations have been cleared. It tracks a stopping state
to reject setup requests and disables IRQs synchronously during removal.

> diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
> index c304ed60edc6c..9b177a131c272 100644
> --- a/drivers/rtc/rtc-renesas-rtca3.c
> +++ b/drivers/rtc/rtc-renesas-rtca3.c
[ ... ]
> @@ -392,6 +399,9 @@ static int rtca3_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
>  	int ret;
>  
>  	scoped_guard(spinlock_irqsave, &priv->lock) {
> +		if (priv->stopping)
> +			return -ENODEV;
> +

[Severity: Low]
Does this mix goto-based error handling and scope-based cleanup helpers in
the same function?

According to include/linux/cleanup.h and subsystem guidelines, usage of
"goto" and cleanup helpers should never be mixed in the same function. All
resources that need a "goto" cleanup should either be converted to
scope-based cleanup or remain manually managed.

In this case, rtca3_set_alarm() uses
scoped_guard(spinlock_irqsave, &priv->lock) here, but later relies on:

    if (ret)
        goto setup_failed;

which violates the cleanup helper guidelines and creates confusing ownership
and control flow semantics during alarm setup failures.

>  		tmp = readb(priv->base + RTCA3_RCR2);
>  		if (!(tmp & RTCA3_RCR2_START))
>  			return -EPERM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011225657.rtca3-irq-ordering.0@gmail.com?part=2