drivers: pwm: nrfx: Add runtime PM and fix case when PWM is not disabled when not used #78759
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zephyrbot
requested review from
henrikbrixandersen,
jaz1-nordic,
magp-nordic and
nika-nordic
nordic-krch
force-pushed
the
pwm_rt_pm
branch
2 times, most recently
from
640e38c to
6ae0a4b
Compare
nika-nordic
previously approved these changes
Sep 30, 2024
anangl
requested changes
Oct 1, 2024
nordic-krch
force-pushed
the
pwm_rt_pm
branch
3 times, most recently
from
921a432 to
16d8e42
Compare
Use NRFX_FOREACH_PRESENT macro to iterate over all PWM instances and create device only for those enabled in the devicetree. This approach removes need of changing driver code when new instance id is added. Signed-off-by: Krzysztof Chruściński <[email protected]>
mstasiaknordic
approved these changes
Oct 4, 2024
nordic-krch
added a commit
to nordic-krch/fw-nrfconnect-zephyr
that referenced
this pull request
Oct 8, 2024
Use NRFX_FOREACH_PRESENT macro to iterate over all PWM instances and create device only for those enabled in the devicetree. This approach removes need of changing driver code when new instance id is added. Upstream PR: zephyrproject-rtos/zephyr#78759 Signed-off-by: Krzysztof Chruściński <[email protected]>
nordic-krch
added a commit
to nordic-krch/fw-nrfconnect-zephyr
that referenced
this pull request
Oct 8, 2024
Shim was not correctly disabling PWM when it was not used. Task STOP was triggered but PWM->ENABLE remained set which caused increased current. Added interrupt and enabled event handler in the nrfx driver to allow disabling PWM on STOPPED event. Upstream PR: zephyrproject-rtos/zephyr#78759 Signed-off-by: Krzysztof Chruściński <[email protected]>
nordic-krch
added a commit
to nordic-krch/fw-nrfconnect-zephyr
that referenced
this pull request
Oct 8, 2024
Rework PM handling to use pm_device_driver_init(). Shim is not using put and get internally as there is no api that disables or stops pwm so it is hard to determine when to put the device. There are cases when PWM peripheral is stopped but PWM is still active because duty cycle is 100% or 0% and pin is driven by GPIO and not PWM. If user want to use runtime PM with PWM it is possible and getting the device will initialize internal data and putting will suspend by forcing PWM stop if used and setting pins to sleep state. However, from power consumption perspective it is enough to set 0% or 100% duty cycle on all channels. Upstream PR: zephyrproject-rtos/zephyr#78759 Signed-off-by: Krzysztof Chruściński <[email protected]>
nordic-krch
added a commit
to nordic-krch/fw-nrfconnect-zephyr
that referenced
this pull request
Oct 9, 2024
Use NRFX_FOREACH_PRESENT macro to iterate over all PWM instances and create device only for those enabled in the devicetree. This approach removes need of changing driver code when new instance id is added. Upstream PR: zephyrproject-rtos/zephyr#78759 Signed-off-by: Krzysztof Chruściński <[email protected]>
nordic-krch
added a commit
to nordic-krch/fw-nrfconnect-zephyr
that referenced
this pull request
Oct 9, 2024
Shim was not correctly disabling PWM when it was not used. Task STOP was triggered but PWM->ENABLE remained set which caused increased current. Added interrupt and enabled event handler in the nrfx driver to allow disabling PWM on STOPPED event. Upstream PR: zephyrproject-rtos/zephyr#78759 Signed-off-by: Krzysztof Chruściński <[email protected]>
nordic-krch
added a commit
to nordic-krch/fw-nrfconnect-zephyr
that referenced
this pull request
Oct 9, 2024
Rework PM handling to use pm_device_driver_init(). Shim is not using put and get internally as there is no api that disables or stops pwm so it is hard to determine when to put the device. There are cases when PWM peripheral is stopped but PWM is still active because duty cycle is 100% or 0% and pin is driven by GPIO and not PWM. If user want to use runtime PM with PWM it is possible and getting the device will initialize internal data and putting will suspend by forcing PWM stop if used and setting pins to sleep state. However, from power consumption perspective it is enough to set 0% or 100% duty cycle on all channels. Upstream PR: zephyrproject-rtos/zephyr#78759 Signed-off-by: Krzysztof Chruściński <[email protected]>
anangl
reviewed
Oct 9, 2024
nordic-krch
force-pushed
the
pwm_rt_pm
branch
2 times, most recently
from
c1dfbbf to
0ef0e0c
Compare
Shim was not correctly disabling PWM when it was not used. Task STOP was triggered but PWM->ENABLE remained set which caused increased current. Added interrupt and enabled event handler in the nrfx driver to allow disabling PWM on STOPPED event. Signed-off-by: Krzysztof Chruściński <[email protected]>
anangl
reviewed
Oct 10, 2024
nordic-krch
force-pushed
the
pwm_rt_pm
branch
from
06037a4 to
9f76255
Compare
anangl
reviewed
Oct 10, 2024
drivers/pwm/pwm_nrfx.c
Outdated
|
|
||
| default: | ||
| return -ENOTSUP; | ||
| if (IS_ENABLED(CONFIG_PM_DEVICE)) { |
There was a problem hiding this comment.
One last thing, this should rather be CONFIG_PM_DEVICE_RUNTIME, because only then the device is expected to be initialized as suspended (when the runtime PM is auto-enabled for it) but the SUSPEND action is not called, right?
There was a problem hiding this comment.
yes, when runtime PM is off pins will be changed to default because resume action will be called always so it's redundant to set sleep state.
Rework PM handling to use pm_device_driver_init(). Shim is not using put and get internally as there is no api that disables or stops pwm so it is hard to determine when to put the device. There are cases when PWM peripheral is stopped but PWM is still active because duty cycle is 100% or 0% and pin is driven by GPIO and not PWM. If user want to use runtime PM with PWM it is possible and getting the device will initialize internal data and putting will suspend by forcing PWM stop if used and setting pins to sleep state. However, from power consumption perspective it is enough to set 0% or 100% duty cycle on all channels. Signed-off-by: Krzysztof Chruściński <[email protected]>
nordic-krch
force-pushed
the
pwm_rt_pm
branch
from
9f76255 to
7bd2631
Compare
anangl
approved these changes
Oct 10, 2024
nordic-krch
added a commit
to nordic-krch/fw-nrfconnect-zephyr
that referenced
this pull request
Oct 11, 2024
Use NRFX_FOREACH_PRESENT macro to iterate over all PWM instances and create device only for those enabled in the devicetree. This approach removes need of changing driver code when new instance id is added. Upstream PR: zephyrproject-rtos/zephyr#78759 Signed-off-by: Krzysztof Chruściński <[email protected]>
nordic-krch
added a commit
to nordic-krch/fw-nrfconnect-zephyr
that referenced
this pull request
Oct 11, 2024
Shim was not correctly disabling PWM when it was not used. Task STOP was triggered but PWM->ENABLE remained set which caused increased current. Added interrupt and enabled event handler in the nrfx driver to allow disabling PWM on STOPPED event. Upstream PR: zephyrproject-rtos/zephyr#78759 Signed-off-by: Krzysztof Chruściński <[email protected]>
nordic-krch
added a commit
to nordic-krch/fw-nrfconnect-zephyr
that referenced
this pull request
Oct 11, 2024
Rework PM handling to use pm_device_driver_init(). Shim is not using put and get internally as there is no api that disables or stops pwm so it is hard to determine when to put the device. There are cases when PWM peripheral is stopped but PWM is still active because duty cycle is 100% or 0% and pin is driven by GPIO and not PWM. If user want to use runtime PM with PWM it is possible and getting the device will initialize internal data and putting will suspend by forcing PWM stop if used and setting pins to sleep state. However, from power consumption perspective it is enough to set 0% or 100% duty cycle on all channels. Upstream PR: zephyrproject-rtos/zephyr#78759 Signed-off-by: Krzysztof Chruściński <[email protected]>
nika-nordic
approved these changes
Oct 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PWM not disabled when not used
Shim was not correctly disabling PWM when it was not used. Task STOP was triggered but PWM->ENABLE remained set which caused increased current. Added interrupt and enabled event handler in the nrfx driver to allow disabling PWM on STOPPED event.
Add runtime PM support
Rework PM handling to use pm_device_driver_init(). Shim is not using put and get internally as there is no api that disables or stops pwm so it is hard to determine when to put the device. There are cases when PWM peripheral is stopped but PWM is still active because duty cycle is 100% or 0% and pin is driven by GPIO and not PWM.
If user want to use runtime PM with PWM it is possible and getting the device will initialize internal data and putting will suspend by forcing PWM stop if used and setting pins to sleep state. However, from power consumption perspective it is enough to set 0% or 100% duty cycle on all channels.