[PATCH 0/6] rtc: brcmstb-waketimer: add RTC alarm irq

[PATCH 0/6] rtc: brcmstb-waketimer: add RTC alarm irq

[Doug Berger]

Support is added for an interrupt that can be triggered from the
brcmstb-waketimer hardware while the system is awake.

This interrupt allows the driver to pass the rtctest selftest.

Doug Berger (6):
  rtc: brcmstb-waketimer: introduce WKTMR_ALARM_EVENT flag
  rtc: brcmstb-waketimer: non-functional code changes
  rtc: brcmstb-waketimer: compensate for lack of wktmr disable
  rtc: brcmstb-waketimer: rename irq to wake_irq
  dt-bindings: rtc: brcm,brcmstb-waketimer: add alarm interrupt
  rtc: brcmstb-waketimer: allow use as non-wake alarm

 .../bindings/rtc/brcm,brcmstb-waketimer.yaml  |  22 ++-
 drivers/rtc/rtc-brcmstb-waketimer.c           | 152 +++++++++++++-----
 2 files changed, 130 insertions(+), 44 deletions(-)

-- 
2.25.1

[PATCH 1/6] rtc: brcmstb-waketimer: introduce WKTMR_ALARM_EVENT flag

[Doug Berger]

This commit defines bit 0 as the bit of interest within the
BRCMSTB_WKTMR_EVENT register to make the implementation more
readable.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/rtc/rtc-brcmstb-waketimer.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
index c74130e8f496..fbeb8be6664b 100644
--- a/drivers/rtc/rtc-brcmstb-waketimer.c
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -34,6 +34,7 @@ struct brcmstb_waketmr {
 };
 
 #define BRCMSTB_WKTMR_EVENT		0x00
+#define  WKTMR_ALARM_EVENT		BIT(0)
 #define BRCMSTB_WKTMR_COUNTER		0x04
 #define BRCMSTB_WKTMR_ALARM		0x08
 #define BRCMSTB_WKTMR_PRESCALER		0x0C
@@ -41,9 +42,17 @@ struct brcmstb_waketmr {
 
 #define BRCMSTB_WKTMR_DEFAULT_FREQ	27000000
 
+static inline bool brcmstb_waketmr_is_pending(struct brcmstb_waketmr *timer)
+{
+	u32 reg;
+
+	reg = readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
+	return !!(reg & WKTMR_ALARM_EVENT);
+}
+
 static inline void brcmstb_waketmr_clear_alarm(struct brcmstb_waketmr *timer)
 {
-	writel_relaxed(1, timer->base + BRCMSTB_WKTMR_EVENT);
+	writel_relaxed(WKTMR_ALARM_EVENT, timer->base + BRCMSTB_WKTMR_EVENT);
 	(void)readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
 }
 
@@ -147,7 +156,6 @@ static int brcmstb_waketmr_getalarm(struct device *dev,
 {
 	struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
 	time64_t sec;
-	u32 reg;
 
 	sec = readl_relaxed(timer->base + BRCMSTB_WKTMR_ALARM);
 	if (sec != 0) {
@@ -156,8 +164,7 @@ static int brcmstb_waketmr_getalarm(struct device *dev,
 		rtc_time64_to_tm(sec, &alarm->time);
 	}
 
-	reg = readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
-	alarm->pending = !!(reg & 1);
+	alarm->pending = brcmstb_waketmr_is_pending(timer);
 
 	return 0;
 }
-- 
2.25.1

[PATCH 2/6] rtc: brcmstb-waketimer: non-functional code changes

[Doug Berger]

These changes are not intended to affect functionality, but
simplify the source code. They are performed here to simplify
review and reduce confusion with other changes in this set.

Since set_alarm includes the alarm_irq_enable functionality call
it directly from that function for simplicity (even though it
does nothing at the moment). The order of the declarations is
changed to prevent the need for a prototype.

The function device_init_wakeup() is used to replace the
functions device_set_wakeup_capable() and device_wakeup_enable()
since it is equivalent.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/rtc/rtc-brcmstb-waketimer.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
index fbeb8be6664b..582c23793550 100644
--- a/drivers/rtc/rtc-brcmstb-waketimer.c
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -169,6 +169,16 @@ static int brcmstb_waketmr_getalarm(struct device *dev,
 	return 0;
 }
 
+/*
+ * Does not do much but keep the RTC class happy. We always support
+ * alarms.
+ */
+static int brcmstb_waketmr_alarm_enable(struct device *dev,
+					unsigned int enabled)
+{
+	return 0;
+}
+
 static int brcmstb_waketmr_setalarm(struct device *dev,
 				     struct rtc_wkalrm *alarm)
 {
@@ -182,17 +192,7 @@ static int brcmstb_waketmr_setalarm(struct device *dev,
 
 	brcmstb_waketmr_set_alarm(timer, sec);
 
-	return 0;
-}
-
-/*
- * Does not do much but keep the RTC class happy. We always support
- * alarms.
- */
-static int brcmstb_waketmr_alarm_enable(struct device *dev,
-					unsigned int enabled)
-{
-	return 0;
+	return brcmstb_waketmr_alarm_enable(dev, alarm->enabled);
 }
 
 static const struct rtc_class_ops brcmstb_waketmr_ops = {
@@ -228,8 +228,7 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev)
 	 * Set wakeup capability before requesting wakeup interrupt, so we can
 	 * process boot-time "wakeups" (e.g., from S5 soft-off)
 	 */
-	device_set_wakeup_capable(dev, true);
-	device_wakeup_enable(dev);
+	device_init_wakeup(dev, true);
 
 	timer->irq = platform_get_irq(pdev, 0);
 	if (timer->irq < 0)
-- 
2.25.1

[PATCH 3/6] rtc: brcmstb-waketimer: compensate for lack of wktmr disable

[Doug Berger]

Since the WKTMR hardware block cannot be disabled it is necessary
for the driver to accommodate for associated timing hazards. This
commit targets the following possibilities:

A possible race between clearing a wktmr event and the alarm expiring
is made one-sided by setting the alarm to its maximum value before
clearing the event.

Programming alarm values close to the current time may not trigger
events if the counter advances while the alarm is being programmed.
After programming an alarm, a check is made to ensure that it is
either in the future or an expiration event is pending.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/rtc/rtc-brcmstb-waketimer.c | 52 +++++++++++++++++++----------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
index 582c23793550..c791e92532b8 100644
--- a/drivers/rtc/rtc-brcmstb-waketimer.c
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -31,6 +31,8 @@ struct brcmstb_waketmr {
 	struct notifier_block reboot_notifier;
 	struct clk *clk;
 	u32 rate;
+	unsigned long rtc_alarm;
+	bool alarm_en;
 };
 
 #define BRCMSTB_WKTMR_EVENT		0x00
@@ -52,6 +54,11 @@ static inline bool brcmstb_waketmr_is_pending(struct brcmstb_waketmr *timer)
 
 static inline void brcmstb_waketmr_clear_alarm(struct brcmstb_waketmr *timer)
 {
+	u32 reg;
+
+	timer->alarm_en = false;
+	reg = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
+	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);
 }
@@ -59,12 +66,22 @@ static inline void brcmstb_waketmr_clear_alarm(struct brcmstb_waketmr *timer)
 static void brcmstb_waketmr_set_alarm(struct brcmstb_waketmr *timer,
 				      unsigned int secs)
 {
+	unsigned int now;
+
 	brcmstb_waketmr_clear_alarm(timer);
 
 	/* Make sure we are actually counting in seconds */
 	writel_relaxed(timer->rate, timer->base + BRCMSTB_WKTMR_PRESCALER);
 
-	writel_relaxed(secs + 1, timer->base + BRCMSTB_WKTMR_ALARM);
+	writel_relaxed(secs, timer->base + BRCMSTB_WKTMR_ALARM);
+	now = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
+
+	while ((int)(secs - now) <= 0 &&
+		!brcmstb_waketmr_is_pending(timer)) {
+		secs = now + 1;
+		writel_relaxed(secs, timer->base + BRCMSTB_WKTMR_ALARM);
+		now = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
+	}
 }
 
 static irqreturn_t brcmstb_waketmr_irq(int irq, void *data)
@@ -155,27 +172,30 @@ static int brcmstb_waketmr_getalarm(struct device *dev,
 				    struct rtc_wkalrm *alarm)
 {
 	struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
-	time64_t sec;
 
-	sec = readl_relaxed(timer->base + BRCMSTB_WKTMR_ALARM);
-	if (sec != 0) {
-		/* Alarm is enabled */
-		alarm->enabled = 1;
-		rtc_time64_to_tm(sec, &alarm->time);
-	}
+	alarm->enabled = timer->alarm_en;
+	rtc_time64_to_tm(timer->rtc_alarm, &alarm->time);
 
 	alarm->pending = brcmstb_waketmr_is_pending(timer);
 
 	return 0;
 }
 
-/*
- * Does not do much but keep the RTC class happy. We always support
- * alarms.
- */
 static int brcmstb_waketmr_alarm_enable(struct device *dev,
 					unsigned int enabled)
 {
+	struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+
+	if (enabled && !timer->alarm_en) {
+		if ((int)(readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER) -
+		    readl_relaxed(timer->base + BRCMSTB_WKTMR_ALARM)) >= 0 &&
+		    !brcmstb_waketmr_is_pending(timer))
+			return -EINVAL;
+		timer->alarm_en = true;
+	} else if (!enabled && timer->alarm_en) {
+		timer->alarm_en = false;
+	}
+
 	return 0;
 }
 
@@ -183,14 +203,10 @@ static int brcmstb_waketmr_setalarm(struct device *dev,
 				     struct rtc_wkalrm *alarm)
 {
 	struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
-	time64_t sec;
 
-	if (alarm->enabled)
-		sec = rtc_tm_to_time64(&alarm->time);
-	else
-		sec = 0;
+	timer->rtc_alarm = rtc_tm_to_time64(&alarm->time);
 
-	brcmstb_waketmr_set_alarm(timer, sec);
+	brcmstb_waketmr_set_alarm(timer, timer->rtc_alarm);
 
 	return brcmstb_waketmr_alarm_enable(dev, alarm->enabled);
 }
-- 
2.25.1

[PATCH 4/6] rtc: brcmstb-waketimer: rename irq to wake_irq

[Doug Berger]

In preparation for adding a second interrupt to service RTC
interrupts, the existing interrupt is renamed from the generic
'irq' to 'wake_irq' to more clearly convey its role.

It is also converted to an unsigned int.

Finally, the driver message that outputs the IRQ number when
registered is removed since devm_rtc_register_device() already
provides a report of registration and the interrupts can be
found in /proc/interrupts.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/rtc/rtc-brcmstb-waketimer.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
index c791e92532b8..e25f9fcd6ed1 100644
--- a/drivers/rtc/rtc-brcmstb-waketimer.c
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -27,7 +27,7 @@ struct brcmstb_waketmr {
 	struct rtc_device *rtc;
 	struct device *dev;
 	void __iomem *base;
-	int irq;
+	unsigned int wake_irq;
 	struct notifier_block reboot_notifier;
 	struct clk *clk;
 	u32 rate;
@@ -117,7 +117,7 @@ static int brcmstb_waketmr_prepare_suspend(struct brcmstb_waketmr *timer)
 	int ret = 0;
 
 	if (device_may_wakeup(dev)) {
-		ret = enable_irq_wake(timer->irq);
+		ret = enable_irq_wake(timer->wake_irq);
 		if (ret) {
 			dev_err(dev, "failed to enable wake-up interrupt\n");
 			return ret;
@@ -246,9 +246,10 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev)
 	 */
 	device_init_wakeup(dev, true);
 
-	timer->irq = platform_get_irq(pdev, 0);
-	if (timer->irq < 0)
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0)
 		return -ENODEV;
+	timer->wake_irq = (unsigned int)ret;
 
 	timer->clk = devm_clk_get(dev, NULL);
 	if (!IS_ERR(timer->clk)) {
@@ -263,7 +264,7 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev)
 		timer->clk = NULL;
 	}
 
-	ret = devm_request_irq(dev, timer->irq, brcmstb_waketmr_irq, 0,
+	ret = devm_request_irq(dev, timer->wake_irq, brcmstb_waketmr_irq, 0,
 			       "brcmstb-waketimer", timer);
 	if (ret < 0)
 		goto err_clk;
@@ -278,8 +279,6 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_notifier;
 
-	dev_info(dev, "registered, with irq %d\n", timer->irq);
-
 	return 0;
 
 err_notifier:
@@ -317,7 +316,7 @@ static int brcmstb_waketmr_resume(struct device *dev)
 	if (!device_may_wakeup(dev))
 		return 0;
 
-	ret = disable_irq_wake(timer->irq);
+	ret = disable_irq_wake(timer->wake_irq);
 
 	brcmstb_waketmr_clear_alarm(timer);
 
-- 
2.25.1

[PATCH 5/6] dt-bindings: rtc: brcm,brcmstb-waketimer: add alarm interrupt

[Doug Berger]

A second interrupt can optionally be specified for this device
to be used for generating RTC alarm interrupts.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 .../bindings/rtc/brcm,brcmstb-waketimer.yaml  | 22 ++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.yaml b/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.yaml
index 9fe079917a98..a9199f299a68 100644
--- a/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.yaml
+++ b/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.yaml
@@ -11,7 +11,8 @@ maintainers:
 
 description:
   The Broadcom STB wake-up timer provides a 27Mhz resolution timer, with the
-  ability to wake up the system from low-power suspend/standby modes.
+  ability to wake up the system from low-power suspend/standby modes and
+  optionally generate RTC alarm interrupts.
 
 allOf:
   - $ref: "rtc.yaml#"
@@ -24,8 +25,14 @@ properties:
     maxItems: 1
 
   interrupts:
-    description: the TIMER interrupt
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: the TIMER interrupt
+      - description: the ALARM interrupt
+    description:
+      The TIMER interrupt wakes the system from low-power suspend/standby modes.
+      An ALARM interrupt may be specified to interrupt the CPU when an RTC alarm
+      is enabled.
 
   clocks:
     description: clock reference in the 27MHz domain
@@ -42,3 +49,12 @@ examples:
         interrupt-parent = <&aon_pm_l2_intc>;
         clocks = <&upg_fixed>;
     };
+
+  - |
+    rtc@f041a080 {
+        compatible = "brcm,brcmstb-waketimer";
+        reg = <0xf041a080 0x14>;
+        interrupts-extended = <&aon_pm_l2_intc 0x04>,
+                              <&upg_aux_aon_intr2_intc 0x08>;
+        clocks = <&upg_fixed>;
+    };
-- 
2.25.1

[PATCH 6/6] rtc: brcmstb-waketimer: allow use as non-wake alarm

[Doug Berger]

The wake interrupt only fires when the system is in a suspend
state. Fortunately we have another interrupt that fires in a
non-suspend state at the L2 controller UPG_AUX_AON. Add support
for this interrupt line so we can use the alarm in a non-wake
context.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/rtc/rtc-brcmstb-waketimer.c | 55 +++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
index e25f9fcd6ed1..1efa81cecc27 100644
--- a/drivers/rtc/rtc-brcmstb-waketimer.c
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -28,6 +28,7 @@ struct brcmstb_waketmr {
 	struct device *dev;
 	void __iomem *base;
 	unsigned int wake_irq;
+	unsigned int alarm_irq;
 	struct notifier_block reboot_notifier;
 	struct clk *clk;
 	u32 rate;
@@ -56,6 +57,8 @@ static inline void brcmstb_waketmr_clear_alarm(struct brcmstb_waketmr *timer)
 {
 	u32 reg;
 
+	if (timer->alarm_en && timer->alarm_irq)
+		disable_irq(timer->alarm_irq);
 	timer->alarm_en = false;
 	reg = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
 	writel_relaxed(reg - 1, timer->base + BRCMSTB_WKTMR_ALARM);
@@ -88,7 +91,25 @@ static irqreturn_t brcmstb_waketmr_irq(int irq, void *data)
 {
 	struct brcmstb_waketmr *timer = data;
 
-	pm_wakeup_event(timer->dev, 0);
+	if (!timer->alarm_irq)
+		pm_wakeup_event(timer->dev, 0);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t brcmstb_alarm_irq(int irq, void *data)
+{
+	struct brcmstb_waketmr *timer = data;
+
+	/* Ignore spurious interrupts */
+	if (!brcmstb_waketmr_is_pending(timer))
+		return IRQ_HANDLED;
+
+	if (timer->alarm_en) {
+		if (!device_may_wakeup(timer->dev))
+			writel_relaxed(WKTMR_ALARM_EVENT,
+				       timer->base + BRCMSTB_WKTMR_EVENT);
+		rtc_update_irq(timer->rtc, 1, RTC_IRQF | RTC_AF);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -114,7 +135,7 @@ static void wktmr_read(struct brcmstb_waketmr *timer,
 static int brcmstb_waketmr_prepare_suspend(struct brcmstb_waketmr *timer)
 {
 	struct device *dev = timer->dev;
-	int ret = 0;
+	int ret;
 
 	if (device_may_wakeup(dev)) {
 		ret = enable_irq_wake(timer->wake_irq);
@@ -122,9 +143,17 @@ static int brcmstb_waketmr_prepare_suspend(struct brcmstb_waketmr *timer)
 			dev_err(dev, "failed to enable wake-up interrupt\n");
 			return ret;
 		}
+		if (timer->alarm_en && timer->alarm_irq) {
+			ret = enable_irq_wake(timer->alarm_irq);
+			if (ret) {
+				dev_err(dev, "failed to enable rtc interrupt\n");
+				disable_irq_wake(timer->wake_irq);
+				return ret;
+			}
+		}
 	}
 
-	return ret;
+	return 0;
 }
 
 /* If enabled as a wakeup-source, arm the timer when powering off */
@@ -192,7 +221,11 @@ static int brcmstb_waketmr_alarm_enable(struct device *dev,
 		    !brcmstb_waketmr_is_pending(timer))
 			return -EINVAL;
 		timer->alarm_en = true;
+		if (timer->alarm_irq)
+			enable_irq(timer->alarm_irq);
 	} else if (!enabled && timer->alarm_en) {
+		if (timer->alarm_irq)
+			disable_irq(timer->alarm_irq);
 		timer->alarm_en = false;
 	}
 
@@ -269,6 +302,19 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk;
 
+	brcmstb_waketmr_clear_alarm(timer);
+
+	/* Attempt to initialize non-wake irq */
+	ret = platform_get_irq(pdev, 1);
+	if (ret > 0) {
+		timer->alarm_irq = (unsigned int)ret;
+		ret = devm_request_irq(dev, timer->alarm_irq, brcmstb_alarm_irq,
+				       IRQF_NO_AUTOEN, "brcmstb-waketimer-rtc",
+				       timer);
+		if (ret < 0)
+			timer->alarm_irq = 0;
+	}
+
 	timer->reboot_notifier.notifier_call = brcmstb_waketmr_reboot;
 	register_reboot_notifier(&timer->reboot_notifier);
 
@@ -317,6 +363,8 @@ static int brcmstb_waketmr_resume(struct device *dev)
 		return 0;
 
 	ret = disable_irq_wake(timer->wake_irq);
+	if (timer->alarm_en && timer->alarm_irq)
+		disable_irq_wake(timer->alarm_irq);
 
 	brcmstb_waketmr_clear_alarm(timer);
 
@@ -346,4 +394,5 @@ module_platform_driver(brcmstb_waketmr_driver);
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Brian Norris");
 MODULE_AUTHOR("Markus Mayer");
+MODULE_AUTHOR("Doug Berger");
 MODULE_DESCRIPTION("Wake-up timer driver for STB chips");
-- 
2.25.1

Re: [PATCH 5/6] dt-bindings: rtc: brcm,brcmstb-waketimer: add alarm interrupt

[Krzysztof Kozlowski]

On 20/01/2023 20:01, Doug Berger wrote:
> A second interrupt can optionally be specified for this device
> to be used for generating RTC alarm interrupts.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> ---
>  .../bindings/rtc/brcm,brcmstb-waketimer.yaml  | 22 ++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.yaml b/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.yaml
> index 9fe079917a98..a9199f299a68 100644
> --- a/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.yaml
> +++ b/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.yaml
> @@ -11,7 +11,8 @@ maintainers:
>  
>  description:
>    The Broadcom STB wake-up timer provides a 27Mhz resolution timer, with the
> -  ability to wake up the system from low-power suspend/standby modes.
> +  ability to wake up the system from low-power suspend/standby modes and
> +  optionally generate RTC alarm interrupts.
>  
>  allOf:
>    - $ref: "rtc.yaml#"
> @@ -24,8 +25,14 @@ properties:
>      maxItems: 1
>  
>    interrupts:
> -    description: the TIMER interrupt
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: the TIMER interrupt
> +      - description: the ALARM interrupt
> +    description:
> +      The TIMER interrupt wakes the system from low-power suspend/standby modes.
> +      An ALARM interrupt may be specified to interrupt the CPU when an RTC alarm
> +      is enabled.
>  
>    clocks:
>      description: clock reference in the 27MHz domain
> @@ -42,3 +49,12 @@ examples:
>          interrupt-parent = <&aon_pm_l2_intc>;
>          clocks = <&upg_fixed>;
>      };
> +
> +  - |
> +    rtc@f041a080 {
> +        compatible = "brcm,brcmstb-waketimer";
> +        reg = <0xf041a080 0x14>;
> +        interrupts-extended = <&aon_pm_l2_intc 0x04>,
> +                              <&upg_aux_aon_intr2_intc 0x08>;
> +        clocks = <&upg_fixed>;

Change of number of interrupts does not really justify new example. You
can squash it with previous example or just skip it.

Best regards,
Krzysztof

Re: [PATCH 1/6] rtc: brcmstb-waketimer: introduce WKTMR_ALARM_EVENT flag

[Florian Fainelli]

On 1/20/23 11:01, Doug Berger wrote:
> This commit defines bit 0 as the bit of interest within the
> BRCMSTB_WKTMR_EVENT register to make the implementation more
> readable.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 2/6] rtc: brcmstb-waketimer: non-functional code changes

[Florian Fainelli]

On 1/20/23 11:01, Doug Berger wrote:
> These changes are not intended to affect functionality, but
> simplify the source code. They are performed here to simplify
> review and reduce confusion with other changes in this set.
> 
> Since set_alarm includes the alarm_irq_enable functionality call
> it directly from that function for simplicity (even though it
> does nothing at the moment). The order of the declarations is
> changed to prevent the need for a prototype.
> 
> The function device_init_wakeup() is used to replace the
> functions device_set_wakeup_capable() and device_wakeup_enable()
> since it is equivalent.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 3/6] rtc: brcmstb-waketimer: compensate for lack of wktmr disable

[Florian Fainelli]

On 1/20/23 11:01, Doug Berger wrote:
> Since the WKTMR hardware block cannot be disabled it is necessary
> for the driver to accommodate for associated timing hazards. This
> commit targets the following possibilities:
> 
> A possible race between clearing a wktmr event and the alarm expiring
> is made one-sided by setting the alarm to its maximum value before
> clearing the event.
> 
> Programming alarm values close to the current time may not trigger
> events if the counter advances while the alarm is being programmed.
> After programming an alarm, a check is made to ensure that it is
> either in the future or an expiration event is pending.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 4/6] rtc: brcmstb-waketimer: rename irq to wake_irq

[Florian Fainelli]

On 1/20/23 11:01, Doug Berger wrote:
> In preparation for adding a second interrupt to service RTC
> interrupts, the existing interrupt is renamed from the generic
> 'irq' to 'wake_irq' to more clearly convey its role.
> 
> It is also converted to an unsigned int.
> 
> Finally, the driver message that outputs the IRQ number when
> registered is removed since devm_rtc_register_device() already
> provides a report of registration and the interrupts can be
> found in /proc/interrupts.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 6/6] rtc: brcmstb-waketimer: allow use as non-wake alarm

[Florian Fainelli]

On 1/20/23 11:01, Doug Berger wrote:
> The wake interrupt only fires when the system is in a suspend
> state. Fortunately we have another interrupt that fires in a
> non-suspend state at the L2 controller UPG_AUX_AON. Add support
> for this interrupt line so we can use the alarm in a non-wake
> context.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: (subset) [PATCH 0/6] rtc: brcmstb-waketimer: add RTC alarm irq

[Alexandre Belloni]

On Fri, 20 Jan 2023 11:01:41 -0800, Doug Berger wrote:
> Support is added for an interrupt that can be triggered from the
> brcmstb-waketimer hardware while the system is awake.
> 
> This interrupt allows the driver to pass the rtctest selftest.
> 
> Doug Berger (6):
>   rtc: brcmstb-waketimer: introduce WKTMR_ALARM_EVENT flag
>   rtc: brcmstb-waketimer: non-functional code changes
>   rtc: brcmstb-waketimer: compensate for lack of wktmr disable
>   rtc: brcmstb-waketimer: rename irq to wake_irq
>   dt-bindings: rtc: brcm,brcmstb-waketimer: add alarm interrupt
>   rtc: brcmstb-waketimer: allow use as non-wake alarm
> 
> [...]

Applied, thanks!

[1/6] rtc: brcmstb-waketimer: introduce WKTMR_ALARM_EVENT flag
      commit: 90226f6b17a3edcb0bddaf2f16991861c99d6a15
[2/6] rtc: brcmstb-waketimer: non-functional code changes
      commit: 2cd98b22c1443d1f2921a371baee658da184868e
[3/6] rtc: brcmstb-waketimer: compensate for lack of wktmr disable
      commit: 516ae02c38ff3ae867f9b19fa050f78157e2bdae
[4/6] rtc: brcmstb-waketimer: rename irq to wake_irq
      commit: eae258edcb8705932c9e5c61a99f91d8235f688b

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

Re: (subset) [PATCH 0/6] rtc: brcmstb-waketimer: add RTC alarm irq

[Florian Fainelli]

On 1/23/23 15:08, 'Alexandre Belloni' via BCM-KERNEL-FEEDBACK-LIST,PDL 
wrote:
> 
> On Fri, 20 Jan 2023 11:01:41 -0800, Doug Berger wrote:
>> Support is added for an interrupt that can be triggered from the
>> brcmstb-waketimer hardware while the system is awake.
>>
>> This interrupt allows the driver to pass the rtctest selftest.
>>
>> Doug Berger (6):
>>    rtc: brcmstb-waketimer: introduce WKTMR_ALARM_EVENT flag
>>    rtc: brcmstb-waketimer: non-functional code changes
>>    rtc: brcmstb-waketimer: compensate for lack of wktmr disable
>>    rtc: brcmstb-waketimer: rename irq to wake_irq
>>    dt-bindings: rtc: brcm,brcmstb-waketimer: add alarm interrupt
>>    rtc: brcmstb-waketimer: allow use as non-wake alarm
>>
>> [...]
> 
> Applied, thanks!
> 
> [1/6] rtc: brcmstb-waketimer: introduce WKTMR_ALARM_EVENT flag
>        commit: 90226f6b17a3edcb0bddaf2f16991861c99d6a15
> [2/6] rtc: brcmstb-waketimer: non-functional code changes
>        commit: 2cd98b22c1443d1f2921a371baee658da184868e
> [3/6] rtc: brcmstb-waketimer: compensate for lack of wktmr disable
>        commit: 516ae02c38ff3ae867f9b19fa050f78157e2bdae
> [4/6] rtc: brcmstb-waketimer: rename irq to wake_irq
>        commit: eae258edcb8705932c9e5c61a99f91d8235f688b

That was quick, how about patch 6? It does not actually have a 
dependency on the Device Tree binding (patch 5) and the second interrupt 
is looked up by index.
-- 
Florian

Re: (subset) [PATCH 0/6] rtc: brcmstb-waketimer: add RTC alarm irq

[Alexandre Belloni]

On 24/01/2023 09:42:19-0800, Florian Fainelli wrote:
> On 1/23/23 15:08, 'Alexandre Belloni' via BCM-KERNEL-FEEDBACK-LIST,PDL
> wrote:
> > 
> > On Fri, 20 Jan 2023 11:01:41 -0800, Doug Berger wrote:
> > > Support is added for an interrupt that can be triggered from the
> > > brcmstb-waketimer hardware while the system is awake.
> > > 
> > > This interrupt allows the driver to pass the rtctest selftest.
> > > 
> > > Doug Berger (6):
> > >    rtc: brcmstb-waketimer: introduce WKTMR_ALARM_EVENT flag
> > >    rtc: brcmstb-waketimer: non-functional code changes
> > >    rtc: brcmstb-waketimer: compensate for lack of wktmr disable
> > >    rtc: brcmstb-waketimer: rename irq to wake_irq
> > >    dt-bindings: rtc: brcm,brcmstb-waketimer: add alarm interrupt
> > >    rtc: brcmstb-waketimer: allow use as non-wake alarm
> > > 
> > > [...]
> > 
> > Applied, thanks!
> > 
> > [1/6] rtc: brcmstb-waketimer: introduce WKTMR_ALARM_EVENT flag
> >        commit: 90226f6b17a3edcb0bddaf2f16991861c99d6a15
> > [2/6] rtc: brcmstb-waketimer: non-functional code changes
> >        commit: 2cd98b22c1443d1f2921a371baee658da184868e
> > [3/6] rtc: brcmstb-waketimer: compensate for lack of wktmr disable
> >        commit: 516ae02c38ff3ae867f9b19fa050f78157e2bdae
> > [4/6] rtc: brcmstb-waketimer: rename irq to wake_irq
> >        commit: eae258edcb8705932c9e5c61a99f91d8235f688b
> 
> That was quick, how about patch 6? It does not actually have a dependency on
> the Device Tree binding (patch 5) and the second interrupt is looked up by
> index.

My understanding is that if I take it, then the feature will not be
documented. I keep that as an incentive to send v2 ;)


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