Dac pm3 support #90240
Dac pm3 support #90240
Conversation
|
Hello @JairGudino, and thank you very much for your first pull request to the Zephyr project! |
github-actions
bot
added
platform: NXP Drivers
github-actions
bot
requested review from
dbaluta,
decsny,
dleach02,
manuargue,
martinjaeger,
mmahadevan108 and
Raymond0225
There was a problem hiding this comment.
Hi @JairGudino ,
Thank you for contributing this. We need to squash your PR into a single commit. And clean up the white space and compliance issues.
There was a problem hiding this comment.
Update copyright year:
Copyright 2023, 2025 NXP
drivers/dac/dac_mcux_gau.c
Outdated
| pm_policy_state_lock_get(PM_STATE_STANDBY, PM_ALL_SUBSTATES); //Lock the PM MODE 3 | ||
| pm_policy_state_lock_get(PM_STATE_SUSPEND_TO_IDLE, PM_ALL_SUBSTATES); //Lock the PM MODE 2 | ||
| pm_policy_state_lock_get(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES); //Lock the PM MODE 1 |
There was a problem hiding this comment.
Use pm_policy_device_power_lock_get() like PR #88722. That method is more portable and locks power modes specified in the devicetree, rather than hard-coding the power modes here.
drivers/dac/dac_mcux_gau.c
Outdated
| pm_policy_state_lock_put(PM_STATE_STANDBY, PM_ALL_SUBSTATES); //Lock the PM MODE 3 | ||
| pm_policy_state_lock_put(PM_STATE_SUSPEND_TO_IDLE, PM_ALL_SUBSTATES); //Lock the PM MODE 2 | ||
| pm_policy_state_lock_put(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES); //Lock the PM MODE 1 |
There was a problem hiding this comment.
Use pm_policy_device_power_lock_put(). I'm also concerned about the logic here. The PM lock_get() and lock_put() APIs use counters to track how many times lock_get() was called. The RW61x DAC has two channels. This driver is getting the lock in nxp_gau_dac_channel_setup(), and releasing the lock in nxp_gau_deinit(). If the app configures both channels, nxp_gau_dac_channel_setup() is called twice, right? Does that mean nxp_gau_deinit() must be called twice to release the lock? I'm not sure that is supported.
drivers/dac/dac_mcux_gau.c
Outdated
| { static const struct dac_channel_cfg dac_ch_cfg_pm = { | ||
| .channel_id = DAC_CHANNEL_ID_RESTORE, | ||
| .resolution = DAC_RESOLUTION_RESTORE, | ||
| .buffered = true | ||
| }; |
There was a problem hiding this comment.
Where is dac_ch_cfg_pm used?
drivers/dac/dac_mcux_gau.c
Outdated
| break; | ||
| case PM_DEVICE_ACTION_SUSPEND: | ||
| break; |
There was a problem hiding this comment.
nit: we can remove these 2 breaks, and collapse these 3 actions.
drivers/dac/dac_mcux_gau.c
Outdated
| case PM_DEVICE_ACTION_TURN_OFF: | ||
| break; | ||
| case PM_DEVICE_ACTION_TURN_ON: | ||
| nxp_gau_dac_init_common(dev); |
There was a problem hiding this comment.
indent to align with break;
drivers: actuator: dac_mcux_gau: Add PM3 support PM3 support to the rw61x in the dac driver added
JairGudino
force-pushed
the
DAC_PM3_SUPPORT
branch
from
9b5c9b4 to
2e12ac6
Compare
|
PM3 support to the DAC driver, these changes prevent the pm3 state when the dac is enabled and free the power management states after the dac deinit