[PATCH RFC 0/3] alarmtimer: Choose the wakeup RTC per sleep state from the device tree

[PATCH RFC 0/3] alarmtimer: Choose the wakeup RTC per sleep state from the device tree

[Igor Velkov via B4 Relay]

alarmtimer binds to the first registered RTC that has an alarm and a
wakeup-capable parent and programs it on every suspend. On ODROID-HC4
that RTC is amlogic,meson-vrtc: a register the SoC firmware reads when
the application processors suspend. It wakes the board from deep sleep
only, and only when the firmware handles the alarm; it raises no
interrupt in Linux, so it never wakes the board from s2idle. The board's
pcf8563, whose interrupt line goes to a GPIO, registers later and is
never used.

Which RTC wakes the system depends on the sleep state and on the board's
firmware, so the series lets the board device tree say it per RTC:

- linux,wakeup-states: the sleep states an alarm of this RTC wakes the
  system from ("s2idle", "shallow", "deep" as in /sys/power/mem_sleep,
  plus "hibernate" and "off"), or "none".
- linux,alarmtimer-priority: a weight to pick among several RTCs that
  wake the system from the same state.

alarmtimer keeps each RTC that has an alarm and a wakeup-capable parent,
unless an RTC registered before it, with at least the same priority,
already wakes from every state it lists; each kept RTC gets its own
"alarmtimer" platform device under the RTC. alarmtimer_suspend() picks,
by pm_suspend_target_state, the RTC with the highest priority among
those listing that state, the first registered among equals, and
programs the alarm from that RTC's platform device only. The driver core
suspends a device before its ancestors (dpm_suspend() takes
dpm_prepared_list from the tail, in reverse registration order), so the
chosen RTC and its parent, which drives the hardware, are still awake
when the alarm is programmed.
Without the properties only the first RTC gets a platform device and
alarmtimer programs it on every suspend.

An RTC without linux,wakeup-states is taken to wake the system from
every state, so that adding the property to one RTC, e.g. from an
overlay, does not change how the others are used; a priority on the new
one settles which wins.

Hibernation wakeups (.freeze/.poweroff callbacks) are not part of this
series.

Patch 3 sets linux,wakeup-states on ODROID-HC4: "s2idle" for the
pcf8563 and "deep" for meson-vrtc, so that alarmtimer programs the
pcf8563 for s2idle and meson-vrtc for deep sleep. The patch applies to
the current tree, but the s2idle wakeup also needs the pcf8563
interrupt (GPIOX_5), which a patch under review adds [1].

Tested on ODROID-HC4 with v7.3.1, [1] and a BL301 that handles the vrtc
alarm, waking by a CLOCK_BOOTTIME_ALARM timer, once per state: s2idle
woke on the pcf8563 interrupt, deep sleep on meson-vrtc with the pcf8563
alarm left unset. Without the series the alarm went to meson-vrtc and
the board did not wake from s2idle.

Questions:

- Are "linux,wakeup-states" and "linux,alarmtimer-priority" acceptable
  names, or should they be generic (no linux, prefix), given that the
  states describe the hardware and the firmware rather than Linux?
- Is the device tree the right place for a capability that depends on
  the firmware the board runs? On HC4, meson-vrtc wakes from deep sleep
  only with a BL301 that handles its alarm.

The alarm limit of meson-vrtc also depends on the firmware, and the
kernel cannot learn it. Some API is needed to pass this information to
the kernel.

[1] https://lore.kernel.org/r/20261020-hc4-rtc-alarm-irq-v1-1-b2b8c68d54f5@iav.lv

Signed-off-by: Igor Velkov <iav@iav.lv>
---
Igor Velkov (3):
      dt-bindings: rtc: Add linux,wakeup-states and linux,alarmtimer-priority
      alarmtimer: Choose the wakeup RTC by sleep state and priority
      arm64: dts: amlogic: meson-sm1-odroid-hc4: Describe which RTC wakes the board

 .../bindings/rtc/amlogic,meson-vrtc.yaml           |   3 +
 .../devicetree/bindings/rtc/nxp,pcf8563.yaml       |   3 +
 Documentation/devicetree/bindings/rtc/rtc.yaml     |  29 ++++
 .../boot/dts/amlogic/meson-sm1-odroid-hc4.dts      |   5 +
 kernel/time/alarmtimer.c                           | 157 ++++++++++++++++++---
 5 files changed, 180 insertions(+), 17 deletions(-)
---
base-commit: 348f54c435bf3177baa78201952810b255d44b30
change-id: 20261104-b4-alarmtimer-rtc-priority-2e25131a2917

Best regards,
-- 
Igor Velkov <iav@iav.lv>

[PATCH RFC 1/3] dt-bindings: rtc: Add linux,wakeup-states and linux,alarmtimer-priority

[Igor Velkov via B4 Relay]

From: Igor Velkov <iav@iav.lv>

A board can have more than one RTC with an alarm that wakes the system,
for example an I2C RTC next to an RTC in the PMIC or the SoC firmware.
Which of them wakes the system depends on the sleep state and on the
board's firmware: amlogic,meson-vrtc is a register the firmware reads
when the application processors suspend, so it wakes the system from
deep sleep only, and whether an I2C RTC wakes the system from deep
sleep depends on the firmware handling its interrupt line.

Add two properties to describe this per RTC. linux,wakeup-states lists
the sleep states an alarm of the RTC wakes the system from, named as in
/sys/power/mem_sleep, plus hibernate and off, or none for an RTC whose
alarm wakes the system from no state. linux,alarmtimer-priority weights
the RTC among those that wake the system from the target state.

List both in nxp,pcf8563.yaml and amlogic,meson-vrtc.yaml, which do not
allow unlisted properties.

Signed-off-by: Igor Velkov <iav@iav.lv>
---
 .../bindings/rtc/amlogic,meson-vrtc.yaml           |  3 +++
 .../devicetree/bindings/rtc/nxp,pcf8563.yaml       |  3 +++
 Documentation/devicetree/bindings/rtc/rtc.yaml     | 29 ++++++++++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/amlogic,meson-vrtc.yaml b/Documentation/devicetree/bindings/rtc/amlogic,meson-vrtc.yaml
index a89865fa676a..bc4a1916f600 100644
--- a/Documentation/devicetree/bindings/rtc/amlogic,meson-vrtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/amlogic,meson-vrtc.yaml
@@ -30,6 +30,9 @@ properties:
   reg:
     maxItems: 1
 
+  linux,alarmtimer-priority: true
+  linux,wakeup-states: true
+
 required:
   - compatible
   - reg
diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf8563.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf8563.yaml
index 22909a96123e..5aabeb074c82 100644
--- a/Documentation/devicetree/bindings/rtc/nxp,pcf8563.yaml
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf8563.yaml
@@ -32,6 +32,9 @@ properties:
   interrupts:
     maxItems: 1
 
+  linux,alarmtimer-priority: true
+  linux,wakeup-states: true
+
   start-year: true
   wakeup-source: true
 
diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
index efb66df82782..3b4b8ff90ada 100644
--- a/Documentation/devicetree/bindings/rtc/rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
@@ -26,6 +26,35 @@ properties:
       0: not chargeable
       1: chargeable
 
+  linux,alarmtimer-priority:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Weight of this RTC for waking the system on alarmtimer alarms.
+      Among the RTCs that can wake the system from the target sleep
+      state, the one with the highest value is used; on a tie, the
+      operating system decides. An RTC without this property has
+      priority 0.
+
+  linux,wakeup-states:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description:
+      System sleep states an alarm of this RTC wakes the system from,
+      given the board and its firmware. s2idle, shallow and deep are the
+      states of /sys/power/mem_sleep, hibernate is suspend-to-disk and
+      off is the powered-off system, and none, given alone, means the
+      alarm wakes the system from no state. An RTC without this property
+      is taken to wake the system from every state.
+    oneOf:
+      - items:
+          enum:
+            - s2idle
+            - shallow
+            - deep
+            - hibernate
+            - "off"
+      - items:
+          - const: none
+
   quartz-load-femtofarads:
     description:
       The capacitive load of the quartz(x-tal), expressed in femto

-- 
2.43.0

[PATCH RFC 2/3] alarmtimer: Choose the wakeup RTC by sleep state and priority

[Igor Velkov via B4 Relay]

From: Igor Velkov <iav@iav.lv>

alarmtimer_rtc_add_device() registers a platform device under the first
RTC that has an alarm and a wakeup-capable parent, and alarmtimer_suspend()
programs that RTC. On ODROID-HC4 it is meson-vrtc, a register the firmware
reads when the application processors suspend, with no interrupt in
Linux, so an alarm set on it does not wake the board from s2idle; the
board's pcf8563 registers later and is never used.

Keep each such RTC that no earlier registered one with at least the same
priority already covers, each with its own platform device, and read
linux,wakeup-states and linux,alarmtimer-priority from its parent. In
alarmtimer_suspend() choose, among the RTCs whose linux,wakeup-states
contains pm_suspend_target_state, the one with the highest priority,
registered first among equals, and program the alarm from its platform
device only. That platform device is a child of its RTC, so the driver
core suspends it before the RTC. alarmtimer_resume() cancels the timer
from the platform device that started it.

An RTC without linux,wakeup-states qualifies for every state; without
linux,alarmtimer-priority its priority is 0. An RTC that an earlier
registered one with at least the same priority beats for every state gets
no platform device, so without the properties only the first RTC is kept.
alarmtimer does not use an RTC that lists no suspend state, as with
"none". hibernate and off count as no suspend state; nothing in this
series acts on them.

alarmtimer_get_rtcdev() returns the RTC with the highest priority.

Signed-off-by: Igor Velkov <iav@iav.lv>
---
 kernel/time/alarmtimer.c | 157 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 140 insertions(+), 17 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index ea5be5870e51..d7033fcd3178 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -19,9 +19,13 @@
 #include <linux/sched/signal.h>
 #include <linux/sched/debug.h>
 #include <linux/alarmtimer.h>
+#include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/posix-timers.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
 #include <linux/workqueue.h>
 #include <linux/freezer.h>
 #include <linux/compat.h>
@@ -60,13 +64,39 @@ static DEFINE_SPINLOCK(freezer_delta_lock);
 #ifdef CONFIG_RTC_CLASS
 /* rtc timer and device for setting alarm wakeups at suspend */
 static struct rtc_timer		rtctimer;
+static struct rtc_device	*rtctimer_rtc;
 static struct rtc_device	*rtcdev;
 static DEFINE_SPINLOCK(rtcdev_lock);
 
 /**
- * alarmtimer_get_rtcdev - Return selected rtcdevice
+ * struct alarmtimer_rtc - RTC that can wake the system
+ * @node:	Entry in alarmtimer_rtcs
+ * @rtc:	The RTC
+ * @priority:	linux,alarmtimer-priority of the RTC, 0 without the property
+ * @wakes_from:	Per suspend state, whether linux,wakeup-states lists it;
+ *		all set without the property
+ */
+struct alarmtimer_rtc {
+	struct list_head	node;
+	struct rtc_device	*rtc;
+	u32			priority;
+	bool			wakes_from[PM_SUSPEND_MAX];
+};
+
+/* Ordered by descending priority, registration order among equals */
+static LIST_HEAD(alarmtimer_rtcs);
+
+static const char * const alarmtimer_state_names[PM_SUSPEND_MAX] = {
+	[PM_SUSPEND_TO_IDLE] = "s2idle",
+	[PM_SUSPEND_STANDBY] = "shallow",
+	[PM_SUSPEND_MEM] = "deep",
+};
+
+/**
+ * alarmtimer_get_rtcdev - Return the preferred RTC
  *
- * This function returns the rtc device to use for wakealarms.
+ * This function returns the RTC with the highest linux,alarmtimer-priority.
+ * Which RTC alarmtimer_suspend() programs depends on the target sleep state.
  */
 struct rtc_device *alarmtimer_get_rtcdev(void)
 {
@@ -79,19 +109,95 @@ struct rtc_device *alarmtimer_get_rtcdev(void)
 }
 EXPORT_SYMBOL_GPL(alarmtimer_get_rtcdev);
 
+static struct alarmtimer_rtc *alarmtimer_rtc_for_state(suspend_state_t state)
+{
+	struct alarmtimer_rtc *ar;
+
+	lockdep_assert_held(&rtcdev_lock);
+
+	list_for_each_entry(ar, &alarmtimer_rtcs, node)
+		if (ar->wakes_from[state])
+			return ar;
+
+	return NULL;
+}
+
+static bool alarmtimer_rtc_covers(const struct alarmtimer_rtc *a,
+				  const struct alarmtimer_rtc *b)
+{
+	suspend_state_t state;
+
+	for (state = PM_SUSPEND_MIN; state < PM_SUSPEND_MAX; state++)
+		if (b->wakes_from[state] && !a->wakes_from[state])
+			return false;
+
+	return true;
+}
+
+/*
+ * Place for @ar in alarmtimer_rtcs, or NULL when an RTC ahead of it would be
+ * chosen over it for every suspend state.
+ */
+static struct list_head *alarmtimer_rtc_position(const struct alarmtimer_rtc *ar)
+{
+	struct alarmtimer_rtc *other;
+	struct list_head *pos;
+
+	lockdep_assert_held(&rtcdev_lock);
+
+	list_for_each(pos, &alarmtimer_rtcs) {
+		other = list_entry(pos, struct alarmtimer_rtc, node);
+		if (other->priority < ar->priority)
+			break;
+		if (alarmtimer_rtc_covers(other, ar))
+			return NULL;
+	}
+
+	return pos;
+}
+
 static int alarmtimer_rtc_add_device(struct device *dev)
 {
 	struct rtc_device *rtc = to_rtc_device(dev);
+	struct device *parent = rtc->dev.parent;
 	struct platform_device *pdev;
+	bool listed, wakes = false;
+	struct alarmtimer_rtc *ar;
+	struct list_head *pos;
+	suspend_state_t state;
 	int ret = 0;
 
-	if (rtcdev)
-		return -EBUSY;
-
 	if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
 		return -1;
-	if (!device_may_wakeup(rtc->dev.parent))
+	if (!device_may_wakeup(parent))
+		return -1;
+
+	ar = kzalloc_obj(*ar);
+	if (!ar)
+		return -ENOMEM;
+
+	ar->rtc = rtc;
+	device_property_read_u32(parent, "linux,alarmtimer-priority",
+				 &ar->priority);
+	listed = device_property_present(parent, "linux,wakeup-states");
+	for (state = PM_SUSPEND_MIN; state < PM_SUSPEND_MAX; state++) {
+		ar->wakes_from[state] = !listed ||
+			device_property_match_string(parent, "linux,wakeup-states",
+						     alarmtimer_state_names[state]) >= 0;
+		wakes |= ar->wakes_from[state];
+	}
+	/* Wakes from no suspend state: "none", or only hibernate and off */
+	if (!wakes) {
+		kfree(ar);
 		return -1;
+	}
+
+	scoped_guard(spinlock_irqsave, &rtcdev_lock)
+		pos = alarmtimer_rtc_position(ar);
+	if (!pos) {
+		kfree(ar);
+		return -EBUSY;
+	}
 
 	pdev = platform_device_register_data(dev, "alarmtimer",
 					     PLATFORM_DEVID_AUTO, NULL, 0);
@@ -99,17 +205,22 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 		device_init_wakeup(&pdev->dev, true);
 
 	scoped_guard(spinlock_irqsave, &rtcdev_lock) {
-		if (!IS_ERR(pdev) && !rtcdev && try_module_get(rtc->owner)) {
-			rtcdev = rtc;
+		pos = IS_ERR(pdev) ? NULL : alarmtimer_rtc_position(ar);
+		if (pos && try_module_get(rtc->owner)) {
+			list_add_tail(&ar->node, pos);
+			rtcdev = list_first_entry(&alarmtimer_rtcs,
+						  struct alarmtimer_rtc, node)->rtc;
 			/* hold a reference so it doesn't go away */
 			get_device(dev);
 			pdev = NULL;
+			ar = NULL;
 		} else {
 			ret = -1;
 		}
 	}
 
 	platform_device_unregister(pdev);
+	kfree(ar);
 	return ret;
 }
 
@@ -208,20 +319,30 @@ EXPORT_SYMBOL_GPL(alarm_expires_remaining);
 #ifdef CONFIG_RTC_CLASS
 /**
  * alarmtimer_suspend - Suspend time callback
- * @dev: unused
+ * @dev: platform device registered under the RTC
  *
  * When we are going into suspend, we look through the bases
  * to see which is the soonest timer to expire. We then
  * set an rtc timer to fire that far into the future, which
  * will wake us from suspend.
+ *
+ * Each RTC that can wake the system has its own platform device. Only the
+ * platform device under the RTC chosen for the target sleep state programs
+ * the alarm.
  */
 static int alarmtimer_suspend(struct device *dev)
 {
+	struct rtc_device *rtc = to_rtc_device(dev->parent);
 	ktime_t min, now, expires;
-	struct rtc_device *rtc;
+	struct alarmtimer_rtc *ar;
 	struct rtc_time tm;
 	int i, ret, type;
 
+	scoped_guard(spinlock_irqsave, &rtcdev_lock)
+		ar = alarmtimer_rtc_for_state(pm_suspend_target_state);
+	if (ar && ar->rtc != rtc)
+		return 0;
+
 	scoped_guard(spinlock_irqsave, &freezer_delta_lock) {
 		min = freezer_delta;
 		expires = freezer_expires;
@@ -229,9 +350,8 @@ static int alarmtimer_suspend(struct device *dev)
 		freezer_delta = 0;
 	}
 
-	rtc = alarmtimer_get_rtcdev();
-	/* If we have no rtcdev, just return */
-	if (!rtc)
+	/* No RTC wakes the system from this state */
+	if (!ar)
 		return 0;
 
 	/* Find the soonest timer to expire */
@@ -266,7 +386,9 @@ static int alarmtimer_suspend(struct device *dev)
 	trace_alarmtimer_suspend(expires, type);
 
 	/* Setup an rtc timer to fire that far in the future */
-	rtc_timer_cancel(rtc, &rtctimer);
+	if (rtctimer_rtc)
+		rtc_timer_cancel(rtctimer_rtc, &rtctimer);
+	rtctimer_rtc = rtc;
 	rtc_read_time(rtc, &tm);
 	now = rtc_tm_to_ktime(tm);
 
@@ -291,11 +413,12 @@ static int alarmtimer_suspend(struct device *dev)
 
 static int alarmtimer_resume(struct device *dev)
 {
-	struct rtc_device *rtc;
+	struct rtc_device *rtc = to_rtc_device(dev->parent);
 
-	rtc = alarmtimer_get_rtcdev();
-	if (rtc)
+	if (rtc == rtctimer_rtc) {
 		rtc_timer_cancel(rtc, &rtctimer);
+		rtctimer_rtc = NULL;
+	}
 	return 0;
 }
 

-- 
2.43.0

[PATCH RFC 3/3] arm64: dts: amlogic: meson-sm1-odroid-hc4: Describe which RTC wakes the board

[Igor Velkov via B4 Relay]

From: Igor Velkov <iav@iav.lv>

The board has two RTCs with an alarm. The pcf8563 interrupt line goes
to GPIOX_5, so once that interrupt is described [1], its alarm wakes the
board from s2idle but not from deep sleep. meson-vrtc is a register the
SoC firmware reads when the application processors suspend, so it wakes
the board from deep sleep only, and only with a BL301 that handles its
alarm.

List the states per RTC, so that alarmtimer programs the pcf8563 for
s2idle and meson-vrtc for deep sleep.

Link: https://lore.kernel.org/r/20261020-hc4-rtc-alarm-irq-v1-1-b2b8c68d54f5@iav.lv [1]
Signed-off-by: Igor Velkov <iav@iav.lv>
---
 arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts
index 3ece30a0a1ff..935dd31572cb 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts
@@ -106,6 +106,7 @@ rtc: rtc@51 {
 		compatible = "nxp,pcf8563";
 		reg = <0x51>;
 		wakeup-source;
+		linux,wakeup-states = "s2idle";
 	};
 };
 
@@ -142,3 +143,7 @@ &usb {
 	phys = <&usb2_phy0>, <&usb2_phy1>;
 	phy-names = "usb2-phy0", "usb2-phy1";
 };
+
+&vrtc {
+	linux,wakeup-states = "deep";
+};

-- 
2.43.0

Re: [PATCH RFC 0/3] alarmtimer: Choose the wakeup RTC per sleep state from the device tree

[Neil Armstrong]

On 11/4/26 06:54, Igor Velkov via B4 Relay wrote:
> alarmtimer binds to the first registered RTC that has an alarm and a
> wakeup-capable parent and programs it on every suspend. On ODROID-HC4
> that RTC is amlogic,meson-vrtc: a register the SoC firmware reads when
> the application processors suspend. It wakes the board from deep sleep
> only, and only when the firmware handles the alarm; it raises no
> interrupt in Linux, so it never wakes the board from s2idle. The board's
> pcf8563, whose interrupt line goes to a GPIO, registers later and is
> never used.
> 
> Which RTC wakes the system depends on the sleep state and on the board's
> firmware, so the series lets the board device tree say it per RTC:
> 
> - linux,wakeup-states: the sleep states an alarm of this RTC wakes the
>    system from ("s2idle", "shallow", "deep" as in /sys/power/mem_sleep,
>    plus "hibernate" and "off"), or "none".
> - linux,alarmtimer-priority: a weight to pick among several RTCs that
>    wake the system from the same state.

This sounds bulky, perhaps it would be much simpler if we just added
all the available RTCs which has RTC_FEATURE_ALARM and can wakeup ?

Just switch from 1 single to multiple RTCs in a linked list and you're
done, no DT & bindings changes.

Neil

> 
> alarmtimer keeps each RTC that has an alarm and a wakeup-capable parent,
> unless an RTC registered before it, with at least the same priority,
> already wakes from every state it lists; each kept RTC gets its own
> "alarmtimer" platform device under the RTC. alarmtimer_suspend() picks,
> by pm_suspend_target_state, the RTC with the highest priority among
> those listing that state, the first registered among equals, and
> programs the alarm from that RTC's platform device only. The driver core
> suspends a device before its ancestors (dpm_suspend() takes
> dpm_prepared_list from the tail, in reverse registration order), so the
> chosen RTC and its parent, which drives the hardware, are still awake
> when the alarm is programmed.
> Without the properties only the first RTC gets a platform device and
> alarmtimer programs it on every suspend.
> 
> An RTC without linux,wakeup-states is taken to wake the system from
> every state, so that adding the property to one RTC, e.g. from an
> overlay, does not change how the others are used; a priority on the new
> one settles which wins.
> 
> Hibernation wakeups (.freeze/.poweroff callbacks) are not part of this
> series.
> 
> Patch 3 sets linux,wakeup-states on ODROID-HC4: "s2idle" for the
> pcf8563 and "deep" for meson-vrtc, so that alarmtimer programs the
> pcf8563 for s2idle and meson-vrtc for deep sleep. The patch applies to
> the current tree, but the s2idle wakeup also needs the pcf8563
> interrupt (GPIOX_5), which a patch under review adds [1].
> 
> Tested on ODROID-HC4 with v7.3.1, [1] and a BL301 that handles the vrtc
> alarm, waking by a CLOCK_BOOTTIME_ALARM timer, once per state: s2idle
> woke on the pcf8563 interrupt, deep sleep on meson-vrtc with the pcf8563
> alarm left unset. Without the series the alarm went to meson-vrtc and
> the board did not wake from s2idle.
> 
> Questions:
> 
> - Are "linux,wakeup-states" and "linux,alarmtimer-priority" acceptable
>    names, or should they be generic (no linux, prefix), given that the
>    states describe the hardware and the firmware rather than Linux?
> - Is the device tree the right place for a capability that depends on
>    the firmware the board runs? On HC4, meson-vrtc wakes from deep sleep
>    only with a BL301 that handles its alarm.
> 
> The alarm limit of meson-vrtc also depends on the firmware, and the
> kernel cannot learn it. Some API is needed to pass this information to
> the kernel.
> 
> [1] https://lore.kernel.org/r/20261020-hc4-rtc-alarm-irq-v1-1-b2b8c68d54f5@iav.lv
> 
> Signed-off-by: Igor Velkov <iav@iav.lv>
> ---
> Igor Velkov (3):
>        dt-bindings: rtc: Add linux,wakeup-states and linux,alarmtimer-priority
>        alarmtimer: Choose the wakeup RTC by sleep state and priority
>        arm64: dts: amlogic: meson-sm1-odroid-hc4: Describe which RTC wakes the board
> 
>   .../bindings/rtc/amlogic,meson-vrtc.yaml           |   3 +
>   .../devicetree/bindings/rtc/nxp,pcf8563.yaml       |   3 +
>   Documentation/devicetree/bindings/rtc/rtc.yaml     |  29 ++++
>   .../boot/dts/amlogic/meson-sm1-odroid-hc4.dts      |   5 +
>   kernel/time/alarmtimer.c                           | 157 ++++++++++++++++++---
>   5 files changed, 180 insertions(+), 17 deletions(-)
> ---
> base-commit: 348f54c435bf3177baa78201952810b255d44b30
> change-id: 20261104-b4-alarmtimer-rtc-priority-2e25131a2917
> 
> Best regards,

Re: [PATCH RFC 0/3] alarmtimer: Choose the wakeup RTC per sleep state from the device tree

[Igor Velkov]

On 11/4/26, Neil Armstrong wrote:
> This sounds bulky, perhaps it would be much simpler if we just added
> all the available RTCs which has RTC_FEATURE_ALARM and can wakeup ?
>
> Just switch from 1 single to multiple RTCs in a linked list and you're
> done, no DT & bindings changes.

A list it is, and v2 drops linux,alarmtimer-priority.

Arming every RTC makes the coarsest one fire first: pcf8563 has no
alarm seconds, so its alarm comes up to 59 s early, and an RTC with
alarm_offset_max wakes earlier still. The board wakes before the timer
expires, and the next alarm may come seconds later: while it is still
resuming, which takes seconds with disks and PCIe, or right after it
is suspended again. Drivers then go through suspend and resume at
short intervals. Arming one RTC per sleep state avoids that, and which
RTC wakes from which state is board knowledge, so v2 keeps
linux,wakeup-states.

--
Igor Velkov

Re: [PATCH RFC 2/3] alarmtimer: Choose the wakeup RTC by sleep state and priority

[Thomas Gleixner]

On Wed, Nov 04 2026 at 05:54, Igor Velkov via B4 Relay wrote:
> alarmtimer_rtc_add_device() registers a platform device under the first
> RTC that has an alarm and a wakeup-capable parent, and alarmtimer_suspend()
> programs that RTC. On ODROID-HC4 it is meson-vrtc, a register the firmware
> reads when the application processors suspend, with no interrupt in
> Linux, so an alarm set on it does not wake the board from s2idle; the
> board's pcf8563 registers later and is never used.

I'm impressed how creative hardware/firmware people can be...

FYI, the patch fails to apply against

     git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers/core

and violates the tip maintainer coding style requirements

    https://docs.kernel.org/process/maintainer-tip.html

all over the place.

> +/*
> + * Place for @ar in alarmtimer_rtcs, or NULL when an RTC ahead of it would be
> + * chosen over it for every suspend state.
> + */
> +static struct list_head *alarmtimer_rtc_position(const struct alarmtimer_rtc *ar)
> +{
> +	struct alarmtimer_rtc *other;
> +	struct list_head *pos;
> +
> +	lockdep_assert_held(&rtcdev_lock);
> +
> +	list_for_each(pos, &alarmtimer_rtcs) {
> +		other = list_entry(pos, struct alarmtimer_rtc, node);
> +		if (other->priority < ar->priority)
> +			break;

What's this priority check for? Nothing uses it and we are _not_
designing for hypothetical use cases where a platform has a gazillion of
RTCs. It's bad enough that we have to support per suspend state wakeup
sources in the first place.

> +	ar = kzalloc_obj(*ar);
> +	if (!ar)
> +		return -ENOMEM;
> +
> +	ar->rtc = rtc;
> +	device_property_read_u32(parent, "linux,alarmtimer-priority",
> +				 &ar->priority);
> +	listed = device_property_present(parent, "linux,wakeup-states");
> +	for (state = PM_SUSPEND_MIN; state < PM_SUSPEND_MAX; state++) {
> +		ar->wakes_from[state] = !listed ||
> +			device_property_match_string(parent, "linux,wakeup-states",
> +						     alarmtimer_state_names[state]) >= 0;
> +		wakes |= ar->wakes_from[state];
> +	}

This is unreadable garbage and list management is the completely wrong
approach.

You have three suspend states to care about (s2idle, standby, mem) and
you ideally have a RTC device assigned to each state. Whether that's a
single device or different ones does not matter:

struct rtc_wakeup {
	struct rtc_dev		*rtc[PM_SUSPEND_MAX];
        struct rtc_dev		*active_rtc;
        struct rct_timer	timer;
        unsigned long		assigned;
};

static struct rtc_wakeup wakeup_ctl;

register()
	...
        // Reads state list or returns STATES_ALL (0xE) if no list property available
        unsigned long states = parse_wakeup_states(dev);

        // Remove already assigned states
        states &= ~wakeup_ctl.assigned;
        if (!states)
        	return -EBUSY;

  	pdev = platform_device_register_data(dev, "alarmtimer",
  					     PLATFORM_DEVID_AUTO, NULL, 0);

	if (IS_ERR(pdev))
        	return PTR_ERR(pdev);

	if (!try_module_get(rtc->owner)) {
	  	platform_device_unregister(pdev);
        	return -ESRCH;
        }

        get_device(dev);
	device_init_wakeup(&pdev->dev, true);

        for_each_set_bit(state, &states, PM_SUSPEND_MAX)
        	wakeup_ctl.rtcs[state] = dev;

	WRITE_ONCE(wakeup_ctl.assigned, wakeup_ctl.assigned | states);
        return 0;

No allocations, straight forward comprehensible code.

That requires to rework alarmtimer_get_rtcdev() to

bool alarmtimer_has_rtcdev()
{
	return !!READ_ONCE(wakeup_ctl.assigned);
}

But that's fine because with that change nothing needs
alarmtimer_get_rtcdev() outside of the core code anymore.

>  static int alarmtimer_suspend(struct device *dev)
>  {
> +	struct rtc_device *rtc = to_rtc_device(dev->parent);
>  	ktime_t min, now, expires;
> -	struct rtc_device *rtc;
> +	struct alarmtimer_rtc *ar;
>  	struct rtc_time tm;
>  	int i, ret, type;
>  
> +	scoped_guard(spinlock_irqsave, &rtcdev_lock)
> +		ar = alarmtimer_rtc_for_state(pm_suspend_target_state);
> +	if (ar && ar->rtc != rtc)
> +		return 0;

...

> +	/* No RTC wakes the system from this state */
> +	if (!ar)
>  		return 0;

In the upstream code this condition is bogus because if there is a
platform device then there must be a rtc device as well.

Though I understand that with multiple devices you need to somehow
handle the freezer_delta/expires magic even if there is no RTC device
for the current suspend state.

But with the above that's truly incomprehensible and after reading it
three times I still can't convince myself that it is even halfways
correct.

The other thing is that if there is a non-supported state your hackery
fails (contrary to the current code which has the pointless condition
for rtcdev) to evaluate whether there is an alarm armed at all.

If there is an alarm armed then it's sensible to reject the suspend
attempt for the unsupported state. You modeled this after your use case
which does not use the non-supported states, but that's far from being
generally correct, no?

Trying to handle this with magic hacks is not going to fly, but this
can be simply done by avoiding the multi-platform device creation and
just creating one disconnected pseudo device:

struct rtc_wakeup {
	struct rtc_dev		*rtc[PM_SUSPEND_MAX];
        struct platform_device	*pdev;
        struct rtc_dev		*active_rtc;
        struct rct_timer	timer;
        unsigned long		assigned;
};

static struct rtc_wakeup wakeup_ctl;

static bool wakeup_create_pdev(void)
{
	if (!IS_ERR_OR_NULL(wakeup_ctl.pdev))
		return true;

	wakeup_ctl.pdev = platform_device_register_data(NULL, "alarmtimer",
							PLATFORM_DEVID_AUTO, NULL, 0);
	if (IS_ERR(wakeup_ctl.pdev))
        	return false;

	device_init_wakeup(&wakeup_ctl->pdev->dev, true);
        return true;
}

register()
	...
        // Reads state list or returns STATES_ALL (0xE) if no list property available
        unsigned long states = parse_wakeup_states(dev);

        // Remove already assigned states
        states &= ~wakeup_ctl.assigned;
        if (!states)
        	return -EBUSY;

	if (!try_module_get(rtc->owner))
        	return -ESRCH;

	if (!wakeup_create_pdev()) {
        	module_put(rtc->owner);
        	return -ENODEV;
	}                

        get_device(dev);

        for_each_set_bit(state, &states, PM_SUSPEND_MAX)
        	wakeup_ctl.rtcs[state] = dev;

	WRITE_ONCE(wakeup_ctl.assigned, wakeup_ctl.assigned | states);
        return 0;

Then all of this falls into place:

static int alarmtimer_suspend(struct device *unused)
{
	scoped_guard(spinlock_irqsave, &freezer_delta_lock) {
		min = freezer_delta;
		expires = freezer_expires;
		type = freezer_alarmtype;
		freezer_delta = 0;
	}

        // Evaluate the pending alarms
        ...
        
      	if (!alarm_armed)
               	return 0;

	struct rtc_device *rtc = wakeup_ctl.rtcs[pm_suspend_target_state];

        // Handle the case of an unsupported state
	if (!rtc) {
          	pm_wakeup_event(dev, ....);
                return -E$TOBEDEFINED;
        }

        ....

	wakeup_ctl.active_rtc = rtc;
}

and

static int alarmtimer_resume(struct device *unused)
{
	if (!wakeup_ctl.active_rtc)
        	return;

	rtc_timer_cancel(wakeup_ctl.active_rtc, &wakeup_ctl.timer);
	wakeup_ctl.active_rtc = NULL;
}

So this gets _ONE_ suspend() and _ONE_ resume() call and nothing more
and everything falls in place and is comprehensible, no?

One user space visible issue here might be that user space cannot
determine which RTC device is responsible for wakeup. So this needs some
investigation whether user space actually searches for
/sys/.../alarmtimer.xxx

I doubt it, but who the hell knows.

Though the same problem exists when user space suddenly finds more than
one alarmtimer device...

While staring at that code long enough I really got disgusted by this
freezer handling nonsense. That's a horrible hack and want's to be fixed
up _before_ we add multi-rtc wakeup support. The proper thing to do is
to integrate this into the wakeup_ctl infrastructure:

struct freezer_wakeup {
	struct alarm		alarm;
        ktime_t			expires;
};

struct rtc_wakeup {
	struct rtc_dev		*rtc[PM_SUSPEND_MAX];
        struct platform_device	*pdev;
        struct rtc_dev		*active_rtc;
        struct rct_timer	timer;
  	struct freezer_wakeup	freezer[ALARM_NUMTYPE];
        unsigned long		assigned;
};

static struct rtc_wakeup wakeup_ctl;

And obviously initialize that:

static void alarmtimer_freezer_fn(struct alarm *alarm, ktime_t now) { }

static __init ....()
{
	struct freezer_wakeup *f = wakeup_ctl.freezer;

	for (unsigned int t = 0; t < ALARM_NUMTYPE; t++, f++)
		alarm_init(&f->alarm, t, alarmtimer_freezer_fn);
}

But then this whole freezerset() thing boils down to:

static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
{
	if (WARN_ON_ONCE(type >= ALARM_NUMTYPE))
        	return;

	struct freezer_wakeup *f = wakeup_ctl.freezer + type;

	guard(spinlock_irqsave)(&freezer_delta_lock);
	if (f->expires > absexp)
		f->expires = absexp;       
}

And the suspend path gets:

static void alarmtimer_arm_freezer(void)
{
        guard(spinlock_irqsave)(&freezer_delta_lock);

	struct freezer_wakeup *f = wakeup_ctl.freezer;

	for (unsigned int t = 0; t < ALARM_NUMTYPE; t++, f++) {
        	if (f->expires == KTIME_MAX)
                	contine;
                alarm_start_timer(&f->expires, f->alarm, false)
                f->expires = KTIME_MAX;
        }
}

static ktime_t alarmtimer_getnext(struct alarm_base *base)
{
	guard(spinlock_irqsave)(&base->lock);

	struct timerqueue_node *next = timerqueue_getnext(&base->timerqueue);

        return next ? next->expires : KTIME_MAX;
}

static int alarmtimer_suspend(struct device *unused)
{
	alarmtimer_arm_freezer();
        ...

	struct alarm_base *base = alarm_bases;
	ktime_t expires_rel = KTIME_MAX;
        enum alarmtimer_type type;

	for (unsigned int t = 0; t < ALARM_NUMTYPE; t++, base++) {
		ktime_t expabs = alarmtimer_getnext(base);

		if (expabs == KTIME_MAX)
                	continue;

		ktime_t exprel = ktime_sub(expires, base->get_ktime());

		if (exprel < expires_rel) {
                	expires_rel = exprel;
                        type = t;
                }
	}

        // Remove the freezer timers from the queue
	alarmtimer_disarm_freezer();

	// Stop right here if no alarm set
        if (expires_rel == KTIME_MAX)
		return 0;
        ...
}

I'm sure that I missed something there, but you get the general idea.

Thanks,

        tglx

Re: [PATCH RFC 2/3] alarmtimer: Choose the wakeup RTC by sleep state and priority

[Igor Velkov]

On Wed, Nov 11 2026 at 00:14, Thomas Gleixner wrote:
> So this gets _ONE_ suspend() and _ONE_ resume() call and nothing more
> and everything falls in place and is comprehensible, no?

Thanks, that is much simpler. v2 will first replace the freezer_delta
handling as you sketched, then add the per-state wakeup_ctl with one
pseudo device and no priority, and reject suspend into a state without
an RTC while an alarm is armed. It will be based on tip timers/core and
follow the tip style. I'll also check whether user space looks up the
alarmtimer device.

Igor