-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Device runtime support for uart ns16550 and power domain support for siwx91x soc #94851
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.
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
base: main
Are you sure you want to change the base?
Conversation
smalae
commented
Aug 22, 2025
- Added power domain node in siwg917.dtsi.
- Updated UART device nodes to reference the newly added power domain.
- Implemented the pm_action function to manage power domain transitions for the SoC.
- Enable the pm device runtime driver support for the uart_ns16550 driver (only for devices that have an associated power domain enabled).
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
2163c44
to
9f199b7
Compare
@bjarki-andreasen @fabiobaltieri, this PR addresses key challenges identified in the #94375 PR by introducing a power domain for the siwx91x SoC. It implements both turn_on and turn_off scenarios, ensuring that only devices associated with a power domain reconfigure their drivers upon wakeup. This aligns with the intended use case. Please provide your feedback. |
9f199b7
to
9614fbf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice progress!
dts/arm/silabs/siwg917.dtsi
Outdated
}; | ||
|
||
siwx91x_soc_pd: siwx91x_soc_pd { | ||
compatible = "power-domain"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PM DEVICE RUNTIME is required for power domain power management, so the zephyr,pm-device-runtime-auto should be added to the the uart and power domain :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"I'm encountering unexpected behavior after enabling CONFIG_PM_DEVICE_RUNTIME and adding zephyr,pm-device-runtime-auto to the UART node. The device isn't transitioning into low power mode as expected, and it appears to be stuck at the k_msleep() call. I'm currently investigating the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue is resolved now.
case PM_DEVICE_ACTION_TURN_OFF: | ||
break; | ||
default: | ||
ret = -ENOTSUP; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return -ENOTSUP;
here, return 0;
at the end and drop ret
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
24a91cf
to
a160b0c
Compare
Done |
drivers/serial/uart_ns16550.c
Outdated
case PM_DEVICE_ACTION_TURN_ON: | ||
return uart_ns16550_configure(dev, uart_cfg); | ||
case PM_DEVICE_ACTION_TURN_OFF: | ||
ret = clock_control_off(dev_cfg->clock_dev, dev_cfg->clock_subsys); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we have to consider the case where clock_dev == NULL
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, we should not perform any operations in 'PM_DEVICE_ACTION_TURN_OFF'. I've already handled that in my latest changes. But let me know if that can be handled differently.
@smalae rebase needed again (due to |
6ebd6e2
to
2cb5f60
Compare
@bjarki-andreasen @dcpleung could you have another look? Thanks. |
1. Added siwx91x power domain node in siwg917.dtsi 2. Updated UART device nodes to reference the newly added power domain. 3. Implemented power domain driver to manage power domain transitions for the SoC. Signed-off-by: Sai Santhosh Malae <[email protected]>
This commit enables the pm device runtime driver support for the uart_ns16550 driver (only for devices that have an associated power domain enabled). Signed-off-by: Sai Santhosh Malae <[email protected]>
Removed CMSIS references from wiseconnect CMakeLists as they are no longer required with UART device runtime PM. Keeping them adds unnecessary compilation overhead. Signed-off-by: Sai Santhosh Malae <[email protected]>
Update hal_silabs version to latest commit: edbd418670ad8c7cad629a09df2ceee0bc23cf6d Signed-off-by: Sai Santhosh Malae <[email protected]>
2cb5f60
to
a490bcb
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, tested in in a couple platforms using this driver (it82002aw and rts5912) and they still work. Thanks.