Skip to contentSkip to conversation

rtc: brcmstb-waketimer: support level alarm_irq

PATCH Applied Doug Berger 3 participants 4 messages Aug 30, 2023 – Oct 1, 2023

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 rtc: brcmstb-waketimer: support level alarm_irq

Some devices (e.g. BCM72112) use an alarm_irq interrupt that is connected to a level interrupt controller rather than an edge interrupt controller. In this case, the interrupt cannot be left enabled by the irq handler while preserving the hardware wake-up signal on wake capable devices or an interrupt storm will occur.

The alarm_expired flag is introduced to allow the disabling of the interrupt when an alarm expires and to support balancing the calls to disable_irq() and enable_irq() in accordance with the existing design.

Fixes 24304a87158a ("rtc: brcmstb-waketimer: allow use as non-wake alarm")
---
 drivers/rtc/rtc-brcmstb-waketimer.c | 47 ++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 5 deletions(-)
drivers/rtc/rtc-brcmstb-waketimer.c +42 -5
index 3cdc015692ca..1a65a4e0dc00 100644
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright © 2014-2017 Broadcom
+ * Copyright © 2014-2023 Broadcom
  */
 
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
@@ -34,6 +34,7 @@ struct brcmstb_waketmr {
 	u32 rate;
 	unsigned long rtc_alarm;
 	bool alarm_en;
+	bool alarm_expired;
 };
 
 #define BRCMSTB_WKTMR_EVENT		0x00
@@ -64,6 +65,11 @@ static inline void brcmstb_waketmr_clear_alarm(struct brcmstb_waketmr *timer)
 	writel_relaxed(reg - 1, timer->base + BRCMSTB_WKTMR_ALARM);
 	writel_relaxed(WKTMR_ALARM_EVENT, timer->base + BRCMSTB_WKTMR_EVENT);
 	(void)readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
+	if (timer->alarm_expired) {
+		timer->alarm_expired = false;
+		/* maintain call balance */
+		enable_irq(timer->alarm_irq);
+	}
 }
 
 static void brcmstb_waketmr_set_alarm(struct brcmstb_waketmr *timer,
@@ -105,10 +111,17 @@ static irqreturn_t brcmstb_alarm_irq(int irq, void *data)
 		return IRQ_HANDLED;
 
 	if (timer->alarm_en) {
-		if (!device_may_wakeup(timer->dev))
+		if (device_may_wakeup(timer->dev)) {
+			disable_irq_nosync(irq);
+			timer->alarm_expired = true;
+		} else {
 			writel_relaxed(WKTMR_ALARM_EVENT,
 				       timer->base + BRCMSTB_WKTMR_EVENT);
+		}
 		rtc_update_irq(timer->rtc, 1, RTC_IRQF | RTC_AF);
+	} else {
+		writel_relaxed(WKTMR_ALARM_EVENT,
+			       timer->base + BRCMSTB_WKTMR_EVENT);
 	}
 
 	return IRQ_HANDLED;
@@ -221,8 +234,14 @@ static int brcmstb_waketmr_alarm_enable(struct device *dev,
 		    !brcmstb_waketmr_is_pending(timer))
 			return -EINVAL;
 		timer->alarm_en = true;
-		if (timer->alarm_irq)
+		if (timer->alarm_irq) {
+			if (timer->alarm_expired) {
+				timer->alarm_expired = false;
+				/* maintain call balance */
+				enable_irq(timer->alarm_irq);
+			}
 			enable_irq(timer->alarm_irq);
+		}
 	} else if (!enabled && timer->alarm_en) {
 		if (timer->alarm_irq)
 			disable_irq(timer->alarm_irq);
@@ -352,6 +371,17 @@ static int brcmstb_waketmr_suspend(struct device *dev)
 	return brcmstb_waketmr_prepare_suspend(timer);
 }
 
+static int brcmstb_waketmr_suspend_noirq(struct device *dev)
+{
+	struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+
+	/* Catch any alarms occurring prior to noirq */
+	if (timer->alarm_expired && device_may_wakeup(dev))
+		return -EBUSY;
+
+	return 0;
+}
+
 static int brcmstb_waketmr_resume(struct device *dev)
 {
 	struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
@@ -368,10 +398,17 @@ static int brcmstb_waketmr_resume(struct device *dev)
 
 	return ret;
 }
+#else
+#define brcmstb_waketmr_suspend		NULL
+#define brcmstb_waketmr_suspend_noirq	NULL
+#define brcmstb_waketmr_resume		NULL
 #endif /* CONFIG_PM_SLEEP */
 
-static SIMPLE_DEV_PM_OPS(brcmstb_waketmr_pm_ops,
-			 brcmstb_waketmr_suspend, brcmstb_waketmr_resume);
+static const struct dev_pm_ops brcmstb_waketmr_pm_ops = {
+	.suspend	= brcmstb_waketmr_suspend,
+	.suspend_noirq	= brcmstb_waketmr_suspend_noirq,
+	.resume		= brcmstb_waketmr_resume,
+};
 
 static const __maybe_unused struct of_device_id brcmstb_waketmr_of_match[] = {
 	{ .compatible = "brcm,brcmstb-waketimer" },
Signature · 1 line
2.34.1
maintainer Reviewed-by on v1 1/1
#1 lore raw
Quoted 6 lines · Doug Berger

Some devices (e.g. BCM72112) use an alarm_irq interrupt that is connected to a level interrupt controller rather than an edge interrupt controller. In this case, the interrupt cannot be left enabled by the irq handler while preserving the hardware wake-up signal on wake capable devices or an interrupt storm will occur.

Quoted 7 lines

The alarm_expired flag is introduced to allow the disabling of the interrupt when an alarm expires and to support balancing the calls to disable_irq() and enable_irq() in accordance with the existing design.

Fixes 24304a87158a ("rtc: brcmstb-waketimer: allow use as non-wake alarm")
Signature · 1 line
Florian
maintainer
#2 lore raw
Quoted 12 lines · Florian Fainelli
Quoted 11 lines · Doug Berger

Some devices (e.g. BCM72112) use an alarm_irq interrupt that is connected to a level interrupt controller rather than an edge interrupt controller. In this case, the interrupt cannot be left enabled by the irq handler while preserving the hardware wake-up signal on wake capable devices or an interrupt storm will occur.

The alarm_expired flag is introduced to allow the disabling of the interrupt when an alarm expires and to support balancing the calls to disable_irq() and enable_irq() in accordance with the existing design.

Quoted 4 lines
Quoted 2 lines
Fixes 24304a87158a ("rtc: brcmstb-waketimer: allow use as non-wake alarm")

Alexandre, any chance you could apply this patch? Thanks!

Signature · 1 line
Florian

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

maintainer Applied
#3 lore raw
In reply to Doug Berger
Quoted 6 lines · Doug Berger

Some devices (e.g. BCM72112) use an alarm_irq interrupt that is connected to a level interrupt controller rather than an edge interrupt controller. In this case, the interrupt cannot be left enabled by the irq handler while preserving the hardware wake-up signal on wake capable devices or an interrupt storm will occur.

Quoted 6 lines

The alarm_expired flag is introduced to allow the disabling of the interrupt when an alarm expires and to support balancing the calls to disable_irq() and enable_irq() in accordance with the existing design.

[...]

Applied, thanks!

[1/1] rtc: brcmstb-waketimer: support level alarm_irq

      commit: e005a9b35b464be5b2e0194f717e90e7e496785d

Best regards,

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