Skip to content

NXP drivers: pwm mcux sctimer: adds PM low power support #89384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+130 −21
Open

NXP drivers: pwm mcux sctimer: adds PM low power support #89384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5862b64
dts: arm: nxp: rw6xx: Add disabling power states to sctimer
alrodlim May 2, 2025
82afaaa
drivers: pwm_mcux_sctimer: Add PM support for low power modes
alrodlim May 2, 2025
b20e33f
tests: drivers: pwm: pwm_api: frdm_rw612: Enable standby power mode
alrodlim May 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion drivers/pwm/Kconfig.mcux_sctimer
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2021 NXP
# Copyright 2021, 2025 NXP
# SPDX-License-Identifier: Apache-2.0

config PWM_MCUX_SCTIMER
Expand All @@ -9,3 +9,12 @@ config PWM_MCUX_SCTIMER
select PINCTRL
help
Enable sctimer based pwm driver.

config PWM_MCUX_SCT_KEEP_STATE
bool "Retain output level after PWM is disabled"
default y
help
When enabled, the PWM driver will hold the inactive output level and
retain the power management lock even after the PWM signal is
stopped (pulse = 0). This prevents the pin from floating or losing state
in low power modes where the peripheral or pad may be disabled.
122 changes: 102 additions & 20 deletions drivers/pwm/pwm_mcux_sctimer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, NXP
* Copyright 2021, 2025 NXP
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -16,6 +16,9 @@

#include <zephyr/logging/log.h>

#include <zephyr/pm/policy.h>
#include <zephyr/pm/device.h>

LOG_MODULE_REGISTER(pwm_mcux_sctimer, CONFIG_PWM_LOG_LEVEL);

#define CHANNEL_COUNT FSL_FEATURE_SCT_NUMBER_OF_OUTPUTS
Expand All @@ -36,6 +39,11 @@ struct pwm_mcux_sctimer_data {
sctimer_pwm_signal_param_t channel[CHANNEL_COUNT];
uint32_t match_period;
uint32_t configured_chan;
/* This flag keeps track of active/idle channels
* to determine whether to allow low power mode
* or not (see PWM_MCUX_SCTIMER_RETAIN_INACTIVE_STATE).
*/
uint32_t active_chan;
};

/* Helper to setup channel that has not previously been configured for PWM */
Expand Down Expand Up @@ -75,9 +83,50 @@ static int mcux_sctimer_new_channel(const struct device *dev,

SCTIMER_StartTimer(config->base, kSCTIMER_Counter_U);
data->configured_chan++;
data->active_chan++;

return 0;
}

static void mcux_sctimer_pwm_update_channel(const struct device *dev, uint32_t channel,
uint8_t duty_cycle)
{
const struct pwm_mcux_sctimer_config *config = dev->config;
struct pwm_mcux_sctimer_data *data = dev->data;
SCT_Type *base = config->base;

SCTIMER_UpdatePwmDutycycle(base, channel, duty_cycle,
data->event_number[channel]);

if (duty_cycle > 0 && data->channel[channel].dutyCyclePercent == 0) {
data->active_chan++;
}

if (duty_cycle == 0 && data->channel[channel].dutyCyclePercent > 0) {
data->active_chan--;
}

data->channel[channel].dutyCyclePercent = duty_cycle;

if (data->active_chan == 0) {
/* No channels active. Stop the sctimer to manually set the output.
* Set the output to inactive state. Output for other channels
* already at the correct level. Just need to update the current one.
*/
SCTIMER_StopTimer(base, kSCTIMER_Counter_U);

if (data->channel[channel].level == kSCTIMER_HighTrue) {
base->OUTPUT &= ~(1UL << channel);
} else {
base->OUTPUT |= (1UL << channel);
}

#ifndef CONFIG_PWM_MCUX_SCT_KEEP_STATE
pm_policy_device_power_lock_put(dev);
#endif
}
}

static int mcux_sctimer_pwm_set_cycles(const struct device *dev,
uint32_t channel, uint32_t period_cycles,
uint32_t pulse_cycles, pwm_flags_t flags)
Expand Down Expand Up @@ -106,21 +155,10 @@ static int mcux_sctimer_pwm_set_cycles(const struct device *dev,
duty_cycle = 100 * pulse_cycles / period_cycles;

if (duty_cycle == 0 && data->configured_chan == 1) {
/* Only one channel is active. We can turn off the SCTimer
/* Only one channel is configured. We can turn off the SCTimer
* global counter.
*/
SCT_Type *base = config->base;

/* Stop timer so we can set output directly */
SCTIMER_StopTimer(base, kSCTIMER_Counter_U);

/* Set the output to inactive State */
if (data->channel[channel].level == kSCTIMER_HighTrue) {
base->OUTPUT &= ~(1UL << channel);
} else {
base->OUTPUT |= (1UL << channel);
}

mcux_sctimer_pwm_update_channel(dev, channel, duty_cycle);
return 0;
}

Expand All @@ -147,6 +185,10 @@ static int mcux_sctimer_pwm_set_cycles(const struct device *dev,
if (ret < 0) {
return ret;
}

/* Block from losing power while PWM output is enabled. */
pm_policy_device_power_lock_get(dev);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do we unblock?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't unblock since the PWM runs continuously once enabled and there is not API to cancel it. We don't want it to suddenly stop, especially if it's driving something critical.

Copy link
Member

@decsny decsny May 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes it just drives an LED.
I think a request of 0 width pulse is supposed to be interpreted as request to make it stop

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zephyr now supports a deinit API where user applications can call device_deinit() on a device. Would it make sense for us to add support for this API in this driver. We can release the power lock and deinit SCTimer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this seems like another case where only application knows its requirements for using the pwm and driver should not be making assumptions about it and controlling what states are entered primarily from it's unknowing perspective. @alxrey makes a good point that if the pwm is controlling a critical thing then it already must already have critical code which knows when it will be in use or not.

PWM, unlike uart or SPI or something, is not a transactional API, so the driver can't instrinsically know when an operation is "complete" and when it needs to be blocking power states due to a transaction in progress. Things like GPIO and PWM control only makes sense to do from app level, and only app can know when whatever it is doing is still in progress or not, or what the significance is.

As far as implementing deinit specifically for the purpose of allowing PM entry, I think that is completely the wrong architectural direction. AFAIK deinit is supposed to be the opposite of an init, ie return device to default/uninitialized state, which is NOT what we want to do just to enter PM. So first of all, Ideally the device remains initialized and nothing has to change to enter PM except any bare minimum. And secondly, it's not a good design paradigm to overload and couple the meaning and mechanisms of separate functionalities together.

Ping @anangl and @bjarki-andreasen to help resolve this question about what to do or not do regarding PWM device drivers locking or unlocking system power state transitions, is what makes sense as far as the roles and responsibilities of different components of the PM architecture and what makes sense from the perspective of PWM specifically

Copy link
Collaborator

@bjarki-andreasen bjarki-andreasen May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PM_DEVICE_RUNTIME has no such limitations :) See #84049 Drivers do indeed not know what the user wants, this is one of the reasons why we are working towards deprecating PM_SYSTEM_MANAGED in the first place. User calls pm_device_runtime_get() when user wants device to be operational. PM_DEVICE_RUNTIME automatically calls pm_policy_device_power_lock_get(), and makes sure a potential power domain, and its power domain etc. is resumed, before resuming device.

All a device driver needs to do, is become operational when PM_DEVICE_ACTION_RESUME is received. The driver does not need to know anything about locking power states, power domains, or do any self resuming/suspending internally based on best guesses of what the user wants. See #89031 and #89372

Copy link
Member

@dleach02 dleach02 May 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it matters what the application is doing. If the IP was configured by a higher layer to start doing PWM stuff, then the driver knows that it can't allow the power mode to go into a state that would disrupt the IP block. And as @bjarki-andreasen noted, setting the pulse to zero is to put the pin into an inactive state which is effectively telling the driver the higher layer is finished using the IP block for PWM functions... the driver can then unblock the PM.

It doesn't really matter what the application is doing though because the system won't go into PM if the application thread is still running. And if the application wants to wake up at some future time to actually do some PWM activity, then the PM system will have the information necessary to determine if there is enough time to go into a power mode before this timed event occurs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still believe the driver should be responsible for managing its own power state as it has full context of the hardware’s behavior/limitations. Shifting this responsibility to the user undermines the purpose of Zephyr's abstraction layers. Forcing users to manage individual power states also increases the risk of runtime bugs such as race conditions or inconsistent suspend/resume handling.

Here’s how this can be addressed:

  1. The driver calls pm_device_busy_set() when the device is active and clears it with pm_device_busy_clear() when idle. This should prevent the system power manager from suspending the device (needs confirmation).
  2. If the device loses state or behaves incorrectly in certain low power modes (e.g., PM3), the driver should call pm_policy_state_lock_get() to block entry into those modes.
  3. The driver should still allow the user to force a suspend if needed for example via pm_device_action_run(PM_DEVICE_ACTION_SUSPEND). The added complexity here is knowing when the system pm is calling suspend vs when the user is calling it, but I think (1) addresses this.

This gives users the flexibility to manage power at a higher level without burdening them with low level details.

As for the PWM specific case of whether to block low power modes during idle (i.e., constant output) states, this can be made configurable using a Kconfig option to accommodate different hardware or application needs.

Copy link
Member

@decsny decsny May 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still believe the driver should be responsible for managing its own power state as it has full context of the hardware’s behavior/limitations.

Drivers have never been responsible for managing their own power state and sounds like this is going to be even more true soon. The power state of devices is managed by the power subsystem. Drivers only react to the actions taken by the power subsystem, and give information about the state of their operation.

Shifting this responsibility to the user undermines the purpose of Zephyr's abstraction layers.

I don't see how this is the case. Also, this is not about just "shifting responsibility to the user", the goal here is to make as close to a one to one relationship as possible between knowledge of the system and responsibilities to the system for each component involved. What is ideal is if components that know something that others don't are responsible for what they know and capable to act on that knowledge. THAT is the purpose of the "abstraction layers". (although, this architecture, both in implementation and requirement, is much more complex and nuanced than simple layered abstraction).

Forcing users to manage individual power states also increases the risk of runtime bugs such as race conditions or inconsistent suspend/resume handling.

I don't see how that is the case. There is an atomic usage counter for a reason. If any part of the system is written to use a resource without releasing it, that's not a fault of the architecture, that's just a classic leakage bug. It's like malloc'ing memory without freeing it, which is usually on accident. This could happen in any part of the codebase, whether app level, subsystem level, driver level, anywhere.

Here’s how this can be addressed:

1. The driver calls pm_device_busy_set() when the device is active and clears it with pm_device_busy_clear() when idle. This should prevent the system power manager from suspending the device (needs confirmation).

That API is being removed soon according to power maintainers response to my question about it. It is redundant once this architecture becomes streamlined and system managed device PM is removed. So I don't think we should plan on using it anymore.

2. If the device loses state or behaves incorrectly in certain low power modes (e.g., PM3), the driver should call pm_policy_state_lock_get() to block entry into those modes.

That once again goes back to the question of whether or not a PWM device should be considered "in the middle of an operation" or not if it has had it's pulse width set to 0, and vice versa. To answer that, we really need @anangl (maintainer of PWM) or @henrikbrixandersen (collaborator of PWM) to clarify it. The specific questions are:

1. Should a PWM device outputting a non-zero pulse be allowed to be suspended by PM system?
2. Should a device outputting a 0 pulse be considered as disabled/off and not driving pin or having state?
3. Should the zephyr driver implementation make assumptions about whether or not the user wants to allow the PWM to be suspended or not, or about whether or not the system should transition modes based on the PWM state?.

Personally I think this is completely up to the app to know because 1) a 0 pulse width actually means that the pin is being driven at a constant inactive level, NOT that the PWM is not doing anything and 2) Only the app knows if the PWM being on is actually something that needs to block system PM transitions. For example, maybe it's just driving some RGB LED which turns off when the system goes to sleep, and that could be expected behavior. Why should we assume at driver level that the entire system needs blocked because the PWM is currently a certain width? Why should we assume that if someone passed in 0 width that they don't care about what the PWM is doing anymore? I would imagine that in most SOCs, if a pin is muxed to a peripheral and that peripheral drives the pin, if the peripheral loses power then the pin would be floating? But that's not the same thing as driving the pin at constant inactive level which is what the PWM API says. It would be great if a PWM maintainer clarified this.

3. The driver should still allow the user to force a suspend if needed for example via pm_device_action_run(PM_DEVICE_ACTION_SUSPEND). The added complexity here is knowing when the system pm is calling suspend vs when the user is calling it, but I think (1) addresses this.

The driver is not involved in this, and I am pretty sure that's how it already works. The source doesn't need to be known. And as far as I am understanding, this is also not exactly related to what we are talking about because the device suspend is not the issue here, it's the system mode transition.

This gives users the flexibility to manage power at a higher level without burdening them with low level details.

I actually think what you've described is more complex and detail leaking for a user than just calling a simple hardware agnostic reference count API. It's completely normal in OS to have reference counting specifically for this reason of different components not knowing about each other and not knowing what each other know. All of the APIs are implemented to just take a device pointer and user doesn't need to know any low level details. But having to call these APIs different ways depending on the situation like you're describing would be a result of leaking low level details.

As for the PWM specific case of whether to block low power modes during idle (i.e., constant output) states, this can be made configurable using a Kconfig option to accommodate different hardware or application needs.

That seems like a reasonable suggestion on paper, but adding configurability into the meaning and contract of APIs adds a lot of complexity to both the implementation and the usage of it, by an exponential amount. For example, if you add a configuration like that, now the amount of test cases (at the very least in configuration, but even potentially in code, depending on how the API is changed by the configurability) actually has to double. And each configuration added doubles it again, this is an exponential complexity. So if we already have a clean and unified way to do this at the app level which is agnostic completely to the resource (ie, we could be talking about pwm, uart, shell, display, the reference counting works the same no matter what the resource is), then I don't think it would be advantageous to start making configurations and changing how PM works for every different type of resource supported in zephyr. It would get unwieldy and confusing for users very fast, as well as adding exponential more maintenance effort for the kernel side developers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation and your points make sense. Just to clarify my earlier point, when I said drivers should manage their own power state, I was specifically referring to the ability to block low power states during critical operations (e.g., active transactions). Shifting this responsibility entirely to the application forces it to be aware of the driver’s internal timing and state, which I believe breaks the abstraction. That said, I understand this is a trade-off being made in favor of a more unified and flexible power management model.


} else if (data->event_number[channel] == EVENT_NOT_SET) {
/* We have already configured a PWM signal, but this channel
* has not been setup. We can only support this channel
Expand All @@ -168,7 +210,7 @@ static int mcux_sctimer_pwm_set_cycles(const struct device *dev,
*/
if (data->configured_chan != 1) {
LOG_ERR("Cannot change PWM period when multiple "
"channels active");
"channels configured");
return -ENOTSUP;
}

Expand All @@ -183,8 +225,7 @@ static int mcux_sctimer_pwm_set_cycles(const struct device *dev,
data->match_period = period_cycles;
} else {
/* Only duty cycle needs to be updated */
SCTIMER_UpdatePwmDutycycle(config->base, channel, duty_cycle,
data->event_number[channel]);
mcux_sctimer_pwm_update_channel(dev, channel, duty_cycle);
}

return 0;
Expand All @@ -207,7 +248,7 @@ static int mcux_sctimer_pwm_get_cycles_per_sec(const struct device *dev,
return 0;
}

static int mcux_sctimer_pwm_init(const struct device *dev)
static int mcux_sctimer_pwm_init_common(const struct device *dev)
{
const struct pwm_mcux_sctimer_config *config = dev->config;
struct pwm_mcux_sctimer_data *data = dev->data;
Expand Down Expand Up @@ -239,10 +280,51 @@ static int mcux_sctimer_pwm_init(const struct device *dev)
}
data->match_period = 0;
data->configured_chan = 0;
data->active_chan = 0;

return 0;
}

/* Note: This driver does not save and restore state when entering or exiting
* low power mode due to limitations in the SDK drivers. Users should ensure
* that the required configuration is reapplied after waking up from low power
* states.
*/
static int mcux_sctimer_pwm_driver_pm_action(const struct device *dev,
enum pm_device_action action)
{
const struct pwm_mcux_sctimer_config *config = dev->config;
int err = 0;

switch (action) {
case PM_DEVICE_ACTION_RESUME:
break;
case PM_DEVICE_ACTION_TURN_ON:
err = mcux_sctimer_pwm_init_common(dev);
break;
case PM_DEVICE_ACTION_SUSPEND:
break;
case PM_DEVICE_ACTION_TURN_OFF:
/* Reaching this point implies that all the PWM channels
* are in idle state and the SCTimer counter is disabled.
*/
err = pinctrl_apply_state(config->pincfg, PINCTRL_STATE_SLEEP);
break;
default:
err = -ENOTSUP;
}

return err;
}

static int mcux_sctimer_pwm_init(const struct device *dev)
{
/* The device init is done from the PM_DEVICE_ACTION_TURN_ON in
* the pm callback which is invoked by pm_device_driver_init.
*/
return pm_device_driver_init(dev, mcux_sctimer_pwm_driver_pm_action);
}

static DEVICE_API(pwm, pwm_mcux_sctimer_driver_api) = {
.set_cycles = mcux_sctimer_pwm_set_cycles,
.get_cycles_per_sec = mcux_sctimer_pwm_get_cycles_per_sec,
Expand All @@ -259,10 +341,10 @@ static DEVICE_API(pwm, pwm_mcux_sctimer_driver_api) = {
.clock_dev = DEVICE_DT_GET(DT_INST_CLOCKS_CTLR(n)), \
.clock_subsys = (clock_control_subsys_t)DT_INST_CLOCKS_CELL(n, name),\
}; \
\
PM_DEVICE_DT_INST_DEFINE(n, mcux_sctimer_pwm_driver_pm_action); \
DEVICE_DT_INST_DEFINE(n, \
mcux_sctimer_pwm_init, \
NULL, \
PM_DEVICE_DT_INST_GET(n), \
&pwm_mcux_sctimer_data_##n, \
&pwm_mcux_sctimer_config_##n, \
POST_KERNEL, CONFIG_PWM_INIT_PRIORITY, \
Expand Down
1 change: 1 addition & 0 deletions dts/arm/nxp/nxp_rw6xx_common.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@
status = "disabled";
prescaler = <8>;
#pwm-cells = <3>;
zephyr,disabling-power-states = <&suspend &standby>;
power-domains = <&power_mode3_domain>;
};

Expand Down
7 changes: 7 additions & 0 deletions tests/drivers/pwm/pwm_api/boards/frdm_rw612.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#
# Copyright 2025 NXP
#
# SPDX-License-Identifier Apache-2.0
#
CONFIG_PM=y
CONFIG_PWM_MCUX_SCT_KEEP_STATE=n
10 changes: 10 additions & 0 deletions tests/drivers/pwm/pwm_api/boards/frdm_rw612.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* Copyright 2025 NXP
*
* SPDX-License-Identifier: Apache-2.0
*/

/* Enable standby low power mode */
&standby {
status = "okay";
};