[PATCH] rtc: omap: fail rtc-only suspend without a power-off RTC

[PATCH] rtc: omap: fail rtc-only suspend without a power-off RTC

[Rosen Penev]

omap_rtc_power_off_program() dereferences omap_rtc_power_off_rtc
unconditionally, but that pointer is only set when an RTC with the
"system-power-controller" property probed while pm_power_off was still
free, and it is cleared again when that RTC is removed.

pm33xx uses the function for rtc-only suspend whenever off mode is
enabled and an "rtc0" class device exists, which guarantees none of
that. Without the property, with another driver owning pm_power_off,
or with a different RTC registered as rtc0, suspending to mem
dereferences NULL in the cpu_suspend() finisher.

Return -ENODEV when no power-off RTC is set up, and propagate it from
am33xx_rtc_only_idle(). A non-zero return from the finisher makes
cpu_suspend() abort, so pm33xx reports a suspend failure instead of
oopsing.

Fixes: 5a99ae0092fe ("soc: ti: pm33xx: AM437X: Add rtc_only with ddr in self-refresh support")
Assisted-by: LLM
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/rtc/rtc-omap.c  | 11 +++++++++++
 drivers/soc/ti/pm33xx.c |  8 +++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index 18492165a19f..540232d6f26c 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -407,6 +407,9 @@ static struct omap_rtc *omap_rtc_power_off_rtc;
  * omap_rtc_power_off_program: Set the pmic power off sequence. The RTC
  * generates pmic_pwr_enable control, which can be used to control an external
  * PMIC.
+ *
+ * Return: 0 on success, or -ENODEV if no RTC was registered as the system
+ * power controller.
  */
 int omap_rtc_power_off_program(void)
 {
@@ -416,6 +419,14 @@ int omap_rtc_power_off_program(void)
 	int seconds;
 	u32 val;
 
+	/*
+	 * Only set when an RTC with "system-power-controller" took over
+	 * pm_power_off. pm33xx calls this for rtc-only suspend whenever an
+	 * rtc0 exists, which does not guarantee that.
+	 */
+	if (!rtc)
+		return -ENODEV;
+
 	rtc->type->unlock(rtc);
 	/* enable pmic_power_en control */
 	val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
diff --git a/drivers/soc/ti/pm33xx.c b/drivers/soc/ti/pm33xx.c
index 82979122417d..02ebf5a6f4b5 100644
--- a/drivers/soc/ti/pm33xx.c
+++ b/drivers/soc/ti/pm33xx.c
@@ -174,7 +174,13 @@ static struct wkup_m3_wakeup_src rtc_wake_src(void)
 
 static int am33xx_rtc_only_idle(unsigned long wfi_flags)
 {
-	omap_rtc_power_off_program();
+	int ret;
+
+	/* A non-zero return makes cpu_suspend() abort the suspend. */
+	ret = omap_rtc_power_off_program();
+	if (ret)
+		return ret;
+
 	am33xx_do_wfi_sram(wfi_flags);
 	return 0;
 }
-- 
2.56.0

Re: [PATCH] rtc: omap: fail rtc-only suspend without a power-off RTC

[sashiko-bot]

> omap_rtc_power_off_program() dereferences omap_rtc_power_off_rtc
> unconditionally, but that pointer is only set when an RTC with the
> "system-power-controller" property probed while pm_power_off was still
> free, and it is cleared again when that RTC is removed.
> 
> pm33xx uses the function for rtc-only suspend whenever off mode is
> enabled and an "rtc0" class device exists, which guarantees none of
> that. Without the property, with another driver owning pm_power_off,
> or with a different RTC registered as rtc0, suspending to mem
> dereferences NULL in the cpu_suspend() finisher.
> 
> Return -ENODEV when no power-off RTC is set up, and propagate it from
> am33xx_rtc_only_idle(). A non-zero return from the finisher makes
> cpu_suspend() abort, so pm33xx reports a suspend failure instead of
> oopsing.
> 
> Fixes: 5a99ae0092fe ("soc: ti: pm33xx: AM437X: Add rtc_only with ddr in self-refresh support")
> Assisted-by: LLM
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004215843.396867-1-rosenp@gmail.com?part=1