mcxw7x: Add Power Management support #96891

mmahadevan108 · 2025-10-01T22:39:48Z

Add support for power management states

Signed-off-by: Mahesh Mahadevan [email protected]

decsny · 2025-10-01T23:25:17Z

dts/arm/nxp/nxp_mcxw7x_common.dtsi

+			/* This is corresponding to entering the "sleep" mode
+			 * of the MCXW without any futher configurations. This is most simple
+			 * state and is just entered by doing WFI mostly.
+			 */
+			sleep: sleep {
+				compatible = "zephyr,power-state";
+				power-state-name = "runtime-idle";
+				min-residency-us = <100>;
+				exit-latency-us = <0>;
+			};
+			/* This is corresponding to "sleep mode" with extra
+			 * optimization by gating the clocks of certain peripherals,
+			 * also the subdomains voltage levels could be configured differently
+			 * Note it is "suspend-to-idle" for zephyr due to some peripherals may
+			 * lose operation if their clock is gated, even though it is same
+			 * hardware power mode.
+			 */
+			sleep_optimized: sleep-optimized {
+				compatible = "zephyr,power-state";
+				power-state-name = "suspend-to-idle";
+				substate-id = <0>;
+				min-residency-us = <500>;
+				exit-latency-us = <10>;
+			};
+			/* It is possible to do more configuring of the hardware in addition
+			 * to just enter deep sleep, the peripherals in core domain
+			 * can be power gated instead of just low power retention,
+			 * and different peripheral in wakeup domain can be put in low power
+			 * instead of remaining operational. So this is "standby" due to
+			 * allowing configure of power gating peripherals which could lose state
+			 */
+			deep_sleep_optimized: deep-sleep-optimized {
+				compatible = "zephyr,power-state";
+				power-state-name = "standby";
+				min-residency-us = <1000>;
+				exit-latency-us = <11>;
+			};


we need to align about these states, the comments i provided you internally that you seem to have kept here were just brainstorms, this "optimized" state idea , is not complete yet without domains and device runtime PM enablement of the device on this platform, the idea was about system configuration per app, to show that a zephyr PM state doesnt need to map 1:1 with hardware SOC modes

decsny · 2025-10-01T23:26:17Z

soc/nxp/mcx/mcxw/mcxw7xx/CMakeLists.txt

+zephyr_sources_ifdef(CONFIG_PM
+  power.c
+  )
+


nit, do on one line

decsny · 2025-10-01T23:27:11Z

soc/nxp/mcx/mcxw/mcxw7xx/Kconfig.defconfig

 	default 96000000 if CORTEX_M_SYSTICK
+	default $(dt_node_int_prop_int,$(DT_LPTMR_PATH),clock-frequency) if MCUX_LPTMR_TIMER


are you sure it's getting set right, I thought the first value will take precedence here, and I don't see wheere would "CORTEX_M_SYSTICK" be unset

CORTEX_M_SYSTICK is disabled when LPTMR is enabled.

where does that happen?

https://github.com/zephyrproject-rtos/zephyr/blob/main/soc/nxp/mcx/mcxw/Kconfig.defconfig#L8-L9

decsny · 2025-10-01T23:27:41Z

soc/nxp/mcx/mcxw/mcxw7xx/power.c

+#ifdef CONFIG_DEBUG
+	CMC_EnableDebugOperation(CMC0, true);
+#else
+	CMC_EnableDebugOperation(CMC0, false);
+#endif


Suggested change

#ifdef CONFIG_DEBUG

CMC_EnableDebugOperation(CMC0, true);

#else

CMC_EnableDebugOperation(CMC0, false);

#endif

CMC_EnableDebugOperation(CMC0, IS_ENABLED(CONFIG_DEBUG));

decsny · 2025-10-01T23:31:31Z

soc/nxp/mcx/mcxw/mcxw7xx/soc.c


+#if CONFIG_PM
+	nxp_mcxw7x_power_init();
+	set_spc_configuration();


this function i dont see why it wouldnt just be called or even just put inline in the power init function, don't know why it is going in soc.c

decsny · 2025-10-01T23:32:01Z

soc/nxp/mcx/mcxw/mcxw7xx/power.c

+void nxp_mcxw7x_power_init(void)
+{
+	/* Enable LPTMR0 as wakeup source */
+	NXP_ENABLE_WAKEUP_SIGNAL(0);
+}


seems like the SPC code can just go in here

decsny · 2025-10-01T23:32:50Z

soc/nxp/mcx/mcxw/mcxw7xx/soc.c

+}
+#endif
+
+extern void nxp_mcxw7x_power_init(void);


should be declared in soc.h with a mock ie #else #define nxp_mcxw7x_power_init(...) #endif

decsny · 2025-10-01T23:36:22Z

soc/nxp/mcx/mcxw/mcxw7xx/soc.h

+#include <fsl_wuu.h>

 #define PORT_MUX_GPIO kPORT_MuxAsGpio

 #define nbu_handler RF_IMU0_IRQHandler

+#undef NXP_ENABLE_WAKEUP_SIGNAL
+#define NXP_ENABLE_WAKEUP_SIGNAL(sig) WUU_SetInternalWakeUpModulesConfig(WUU0,\
+				      sig, kWUU_InternalModuleInterrupt);


as discussed previously, we should not be adding hal headers into soc.h anymore, instead this macro should evaluate to a wrapper function declared in this file and defined in power.c

decsny · 2025-10-01T23:38:14Z

soc/nxp/mcx/mcxw/mcxw7xx/power.c

+	/* Set PRIMASK */
+	__disable_irq();
+	/* Set BASEPRI to 0 */
+	irq_unlock(0);


should not be necessary, already done by arch code last time I checked.

I think this is needed. We can discuss where you see this implemented.

zephyr/kernel/idle.c

Line 51 in 4ef1163

(void) arch_irq_lock();

I found this as well: https://github.com/zephyrproject-rtos/zephyr/blob/main/arch/arm/core/cortex_m/cpu_idle.c#L71-L89

ZhaoxiangJin · 2025-10-02T01:16:18Z

soc/nxp/mcx/mcxw/mcxw7xx/soc.h


+#undef NXP_ENABLE_WAKEUP_SIGNAL
+#define NXP_ENABLE_WAKEUP_SIGNAL(sig) WUU_SetInternalWakeUpModulesConfig(WUU0,\
+				      sig, kWUU_InternalModuleInterrupt);


Hello @bjarki-andreasen, it seems that the management of wakeup sources in zephyr is currently quite fragmented? Some wakeup sources are managed through device drivers, while others are managed at the SoC level (soc.c/.h). I would like to ask the power management maintainer if there are any plans to introduce a standardized/systematized wakeup source management system. Thanks.

@ZhaoxiangJin it was a discussion a lot the last few years. but what you should know here is in our case that we intend to manage all of them by device driver. this soc.h define idea is to have a common api across all nxp platform that the nxp driver can call. but yes, this approach is just how nxp does it, but I think it will work for us, and is not exposed actually to client code. issue only would arise if NXP platform for some reason used some non-nxp driver for an soc peripheral or vice versa (which, is not impossible, since some IP can come from third party designer that is on many different vendor SOC, but usually still there are changes and they write their own driver pretending it's not third party is what I have noticed)

the way that is common in all of zephyr to declare/configure if something is a wakeup is by putting the wakeup-source property on it's node in DT.

ZhaoxiangJin · 2025-10-02T02:05:19Z

soc/nxp/mcx/mcxw/mcxw7xx/power.c

+void nxp_mcxw7x_power_init(void)
+{
+	/* Enable LPTMR0 as wakeup source */
+	NXP_ENABLE_WAKEUP_SIGNAL(0);


The mcxw7x has many wakeup sources. Users may need GPIO wake up, RTC wake up, or LPTMR wake up. However, based on the current approach, users cannot configure them. They can only refer to the SDK implementation or the SoC RM to know how to enable the wakeup sources they need here. Does this go against the Zephyr concept of configure hardware features based on the devicetree?

Currently, Zephyr does not have a wakeup source management subsystem. My view is that we need to describe the WUU information in the wakup source peripheral node, and then configure the WUU in the driver based on the information in the device tree.
As a SoC vendor, we need to enable all these wakeup sources in SoC.dts and driver. The user can use /delete-property/ is-wakeup-source; to remove wakeup sources they don't need in the board.dts or application.dts.

The mcxw7x has many wakeup sources. Users may need GPIO wake up, RTC wake up, or LPTMR wake up. However, based on the current approach, users cannot configure them. They can only refer to the SDK implementation or the SoC RM to know how to enable the wakeup sources they need here. Does this go against the Zephyr concept of configure hardware features based on the devicetree?

I think the reason mahesh put this here is because the lptmr in this case is chosen as the kernel timer when CONFIG_PM=y (hence building this function), so it is a system requirement that it should be a wakeup source, otherwise the system doesn't function. So it should not be left up to user configurability in this case. The LPTMR though can also be used just as a normal counter (not system timer) and in that case the wakeup-source property for configuration would be relevant and the counter driver should be responsible to call this macro to configure the wakeup source conditionally. But for system timer, it is not conditional, it is mandatory.

But, speaking of this line, @mmahadevan108 , this 0 is a magic number, is there not some identifier from the SDK that can mean LPTMR0 wakeup signal?

Currently, Zephyr does not have a wakeup source management subsystem. My view is that we need to describe the WUU information in the wakup source peripheral node, and then configure the WUU in the driver based on the information in the device tree. As a SoC vendor, we need to enable all these wakeup sources in SoC.dts and driver. The user can use /delete-property/ is-wakeup-source; to remove wakeup sources they don't need in the board.dts or application.dts.

No, I don't agree to this approach, nothing should be a wakeup by default, except the system timer. User should be explicit about what causes wakeup for their system, not try to figure out all the ones they need to disable, that is just annoying.

As for the discussion about how to do generic wakeup sources in zephyr, I think the current paradigm is well understood enough and sufficient , and we should avoid over architecting and causing churn across the whole project unless we really decide its actually needed, not just for pedantic reasons. But in an ideal world, probably each node would be putting some controller+data style info using phandle arrays in DT similar to the resets and interrupts and clocks property, about how to configure the wakeups, especially if one device can have different wakeup sources. But I think we should cross that bridge when we come to it, not just for the sake of it.

I can move this to the driver level. I have created PR #96996 for such cases where the wakeup signal is not mapped to the interrupt number.

decsny · 2025-10-04T01:30:20Z

soc/nxp/mcx/mcxw/mcxw7xx/soc.c

+void mcxw7xx_set_wakeup(int32_t sig)
+{
+	WUU_SetInternalWakeUpModulesConfig(WUU0, sig, kWUU_InternalModuleInterrupt);
+}
+


should go in power.c

Power.c gets compiled only when PM is enabled. This could get called without PM enabled as well.

Why would you be configuring wake up from low power modes if you don't enter them?

Some drivers may not have CONFIG_PM wrapper added. I don't see a negative to putting this in soc.c

decsny · 2025-10-04T01:30:53Z

soc/nxp/mcx/mcxw/mcxw7xx/soc.h

+#if CONFIG_PM
+extern void nxp_mcxw7x_power_init(void);
+#endif


this is not what extern is for, remove that, and declare a mock

Whats wrong with having an extern?

decsny · 2025-10-04T01:31:47Z

soc/nxp/mcx/mcxw/mcxw7xx/soc.h

 #define nbu_handler RF_IMU0_IRQHandler

+#undef NXP_ENABLE_WAKEUP_SIGNAL
+extern void mcxw7xx_set_wakeup(int32_t sig);


extern is redundant

Was seeing warning without it, so seems needed.

decsny · 2025-10-04T01:32:51Z

soc/nxp/mcx/mcxw/mcxw7xx/soc.c

+#if CONFIG_PM
+	nxp_mcxw7x_power_init();
+#endif


Had a discussion with @DerekSnell about this and we felt it best we make the changes to power only for PM as we want some of the voltage detects to stay enabled and the low voltage config is needed only for low power modes.

decsny · 2025-10-04T01:33:29Z

soc/nxp/mcx/mcxw/mcxw7xx/Kconfig.defconfig

 	default 96000000 if CORTEX_M_SYSTICK
+	default $(dt_node_int_prop_int,$(DT_LPTMR_PATH),clock-frequency) if MCUX_LPTMR_TIMER


where does that happen?

dts/arm/nxp/nxp_mcxw7x_common.dtsi

decsny · 2025-10-04T01:44:52Z

soc/nxp/mcx/mcxw/mcxw7xx/power.c

+	/* Set PRIMASK */
+	__disable_irq();
+	/* Set BASEPRI to 0 */
+	irq_unlock(0);


zephyr/kernel/idle.c

Line 51 in 4ef1163

(void) arch_irq_lock();

decsny · 2025-10-04T01:50:48Z

soc/nxp/mcx/mcxw/mcxw7xx/power.c

+			/* Set MAIN_CORE and MAIN_WAKE power domain into sleep mode. */
+			config.clock_mode  = kCMC_GateAllSystemClocksEnterLowPowerMode;
+			config.main_domain = kCMC_SleepMode;
+			config.wake_domain = kCMC_SleepMode;
+			CMC_EnterLowPowerMode(CMC0, &config);


we should not be setting domain states in the SOC power driver, this is backwards. the state of the domains should be handled by domain drivers and the result will affect what actual SOC state is selected in the first place based on device runtime PM constraints. cc @bjarki-andreasen

Optimally the devicetree would contain the power domains which manage the main and wake domains, if anything needs them to be resumed, the power domains would be resumed directly (thus not in sleep mode) and the sleep mode which can only be entered if they are suspended, would have been blocked by the drivers/power domains with pm_policy_state_lock_get() or similar.

That said, some hardware is just quirky, it may make no sense to manage these power domains independently, which is maybe hinted at by the CMC_EnterLowPowerMode(CMC0, &config); taking one config which configures all of them together, so this may just be the way to enter one particular sleep mode.

Add support for power management states Signed-off-by: Mahesh Mahadevan <[email protected]>

Add bindings for the power related modules. Use the bindings Kconfig to pull in SDK drivers for cmc, spc, vbat and wuu. Signed-off-by: Mahesh Mahadevan <[email protected]>

Add PM support for MCXW SoC's Signed-off-by: Mahesh Mahadevan <[email protected]>

sonarqubecloud · 2025-10-06T21:31:17Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mmahadevan108 requested a review from decsny October 1, 2025 22:39

mmahadevan108 force-pushed the Power_MCXW71 branch from 69dda9d to e3a628e Compare October 1, 2025 22:56

decsny requested changes Oct 1, 2025

View reviewed changes

ZhaoxiangJin reviewed Oct 2, 2025

View reviewed changes

mmahadevan108 force-pushed the Power_MCXW71 branch 2 times, most recently from 5ea2df0 to 1c69397 Compare October 3, 2025 22:24

decsny requested changes Oct 4, 2025

View reviewed changes

mmahadevan108 added 3 commits October 6, 2025 16:04

dts: mcxw7x: Add Power Management support

731fbf1

Add support for power management states Signed-off-by: Mahesh Mahadevan <[email protected]>

modules: hal_nxp: Pull in SDK cmc, spc, vbat, wuu drivers

3ce637d

Add bindings for the power related modules. Use the bindings Kconfig to pull in SDK drivers for cmc, spc, vbat and wuu. Signed-off-by: Mahesh Mahadevan <[email protected]>

soc: mcxw: Add Power Management support

924806f

Add PM support for MCXW SoC's Signed-off-by: Mahesh Mahadevan <[email protected]>

mmahadevan108 force-pushed the Power_MCXW71 branch from 1c69397 to 924806f Compare October 6, 2025 21:05

		default 96000000 if CORTEX_M_SYSTICK
		default $(dt_node_int_prop_int,$(DT_LPTMR_PATH),clock-frequency) if MCUX_LPTMR_TIMER

mcxw7x: Add Power Management support #96891

Are you sure you want to change the base?

mcxw7x: Add Power Management support #96891

Conversation

mmahadevan108 commented Oct 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

decsny Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZhaoxiangJin Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

decsny Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

decsny Oct 2, 2025 •

edited

Loading

ZhaoxiangJin Oct 2, 2025 •

edited

Loading

decsny Oct 2, 2025 •

edited

Loading