From: Sukrut Bellary sbellary@baylibre.com
[ Upstream commit 47fe74098f3dadba2f9cc1e507d813a4aa93f5f3 ]
Don't put the l4ls clk domain to sleep in case of standby. Since CM3 PM FW[1](ti-v4.1.y) doesn't wake-up/enable the l4ls clk domain upon wake-up, CM3 PM FW fails to wake-up the MPU.
[1] https://git.ti.com/cgit/processor-firmware/ti-amx3-cm3-pm-firmware/
Signed-off-by: Sukrut Bellary sbellary@baylibre.com Tested-by: Judith Mendez jm@ti.com Link: https://lore.kernel.org/r/20250318230042.3138542-2-sbellary@baylibre.com Signed-off-by: Kevin Hilman khilman@baylibre.com Signed-off-by: Sasha Levin sashal@kernel.org ---
**YES** This commit should be backported to stable kernel trees. Here's my detailed analysis: ## Analysis of the Bug Fix **Critical Power Management Issue**: This commit fixes a fundamental system hang condition where the AM33xx SoC cannot resume from standby mode. The l4ls (L4 Low Speed) clock domain is being incorrectly put to sleep during standby, which prevents the CM3 PM firmware from communicating with the MPU to wake it up. ## Code Changes Analysis **1. Minimal and Targeted Changes**: - Adds a single new flag `CLKDM_STANDBY_FORCE_WAKEUP` in clockdomain.h - Applies this flag only to the l4ls clock domain in clockdomains33xx_data.c - Modifies am33xx_clkdm_clk_disable() to check for standby state and avoid sleeping this specific domain **2. Conservative Approach**: The fix only prevents the problematic behavior (putting l4ls to sleep during standby) rather than changing the overall power management architecture. It follows the existing pattern already documented in the codebase for similar GFX_L4LS workarounds. **3. Well- Contained Risk**: - Changes are limited to AM33xx-specific code paths - No changes to external APIs or device tree bindings - The conditional check `pm_suspend_target_state == PM_SUSPEND_STANDBY` ensures the change only affects standby mode ## Backport Justification **Meets Stable Tree Criteria**: 1. **Important Bug Fix**: Resolves critical system hang during suspend/resume 2. **Minimal Risk**: Surgical change with very low regression potential 3. **No New Features**: Pure bug fix with no architectural changes 4. **Platform-Specific**: Contained to AM33xx SoCs, no cross-platform impact **High Impact for Users**: This affects fundamental system reliability for devices using AM335x chips (BeagleBone, industrial IoT systems). The current bug makes standby mode unusable, requiring users to avoid this power saving feature entirely. **Low Regression Risk**: The only potential side effect is marginally higher standby power consumption from keeping one clock domain awake, which is acceptable given the critical nature of the fix. This is exactly the type of targeted, low-risk fix that stable trees are designed for - addressing a critical functional issue with minimal code changes and clear benefit to users.
arch/arm/mach-omap2/clockdomain.h | 1 + arch/arm/mach-omap2/clockdomains33xx_data.c | 2 +- arch/arm/mach-omap2/cm33xx.c | 14 +++++++++++++- 3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h index 68550b23c938d..eb6ca2ea80679 100644 --- a/arch/arm/mach-omap2/clockdomain.h +++ b/arch/arm/mach-omap2/clockdomain.h @@ -48,6 +48,7 @@ #define CLKDM_NO_AUTODEPS (1 << 4) #define CLKDM_ACTIVE_WITH_MPU (1 << 5) #define CLKDM_MISSING_IDLE_REPORTING (1 << 6) +#define CLKDM_STANDBY_FORCE_WAKEUP BIT(7)
#define CLKDM_CAN_HWSUP (CLKDM_CAN_ENABLE_AUTO | CLKDM_CAN_DISABLE_AUTO) #define CLKDM_CAN_SWSUP (CLKDM_CAN_FORCE_SLEEP | CLKDM_CAN_FORCE_WAKEUP) diff --git a/arch/arm/mach-omap2/clockdomains33xx_data.c b/arch/arm/mach-omap2/clockdomains33xx_data.c index b4d5144df4454..c53df9d42ecf8 100644 --- a/arch/arm/mach-omap2/clockdomains33xx_data.c +++ b/arch/arm/mach-omap2/clockdomains33xx_data.c @@ -27,7 +27,7 @@ static struct clockdomain l4ls_am33xx_clkdm = { .pwrdm = { .name = "per_pwrdm" }, .cm_inst = AM33XX_CM_PER_MOD, .clkdm_offs = AM33XX_CM_PER_L4LS_CLKSTCTRL_OFFSET, - .flags = CLKDM_CAN_SWSUP, + .flags = CLKDM_CAN_SWSUP | CLKDM_STANDBY_FORCE_WAKEUP, };
static struct clockdomain l3s_am33xx_clkdm = { diff --git a/arch/arm/mach-omap2/cm33xx.c b/arch/arm/mach-omap2/cm33xx.c index ac4882ebdca33..be84c6750026e 100644 --- a/arch/arm/mach-omap2/cm33xx.c +++ b/arch/arm/mach-omap2/cm33xx.c @@ -28,6 +28,9 @@ #include "cm-regbits-34xx.h" #include "cm-regbits-33xx.h" #include "prm33xx.h" +#if IS_ENABLED(CONFIG_SUSPEND) +#include <linux/suspend.h> +#endif
/* * CLKCTRL_IDLEST_*: possible values for the CM_*_CLKCTRL.IDLEST bitfield: @@ -336,8 +339,17 @@ static int am33xx_clkdm_clk_disable(struct clockdomain *clkdm) { bool hwsup = false;
+#if IS_ENABLED(CONFIG_SUSPEND) + /* + * In case of standby, Don't put the l4ls clk domain to sleep. + * Since CM3 PM FW doesn't wake-up/enable the l4ls clk domain + * upon wake-up, CM3 PM FW fails to wake-up th MPU. + */ + if (pm_suspend_target_state == PM_SUSPEND_STANDBY && + (clkdm->flags & CLKDM_STANDBY_FORCE_WAKEUP)) + return 0; +#endif hwsup = am33xx_cm_is_clkdm_in_hwsup(clkdm->cm_inst, clkdm->clkdm_offs); - if (!hwsup && (clkdm->flags & CLKDM_CAN_FORCE_SLEEP)) am33xx_clkdm_sleep(clkdm);