power_domain: Internal SoC PM state change

[mmahadevan108]

System managed device power management only invokes PM_DEVICE_ACTION_SUSPEND and PM_DEVICE_ACTION_RESUME actions on the device drivers.
This changes allows the user to specify a property per power state called zephyr,pm-turn-on-off-action. This will make the system managed power management framework to invoke PM_DEVICE_ACTION_TURN_OFF and PM_DEVICE_ACTION_TURN_ON actions on the device drivers.
This allows device drivers to take additional power saving steps for certain low power states.

[gmarull]

these actions are meant to notify devices about being turned off or being turned on, please, check https://docs.zephyrproject.org/latest/services/pm/power_domain.html

[mmahadevan108]

This change is to allow devices and SoC's that do not use a power domain to receive TURN_ON/OFF actions. Its a property per power state that is in the users control.
If there is a power domain driver then this property can be left as false.
Do we need the TURN_ON/OFF actions to always go through a power-domain driver?

[JordanYates]

Do we need the TURN_ON/OFF actions to always go through a power-domain driver?

TURN_ON and TURN_OFF are defined in terms of power (voltage) being physically applied or removed from the device in question, not as a lower power sleep state. Device drivers cannot properly implement these actions is they don't have a common meaning across use cases.

[ceolin]

Do we need the TURN_ON/OFF actions to always go through a power-domain driver?

TURN_ON and TURN_OFF are defined in terms of power (voltage) being physically applied or removed from the device in question, not as a lower power sleep state. Device drivers cannot properly implement these actions is they don't have a common meaning across use cases.

Yep that is correct. But it is possible that depending on power state that the SoC sleeps, internal devices my have power removed.

But just to be clear, I completely agree, TURN_ON/OFF must be used for when power is applied and removed only, not for different power savings level.

[ceolin]

Maybe the nomenclature is ambiguous ? If this action is only used in cases where a power state cuts of device power, I don't see a problem.

[mmahadevan108]

We have SoC low power states where the IP blocks lose state and need to be reconfigured on exit from low power modes. SUSPEND and RESUME does not work in this and the device driver needs to receive a different action. I am using TURN_ON as that seemed an appropriate one.
Do you suggest we add a different action?

[JordanYates]

We have SoC low power states where the IP blocks lose state and need to be reconfigured on exit from low power modes.

I don't use system PM, is there a link somewhere between the IP blocks that do lose state and the power state with zephyr,pm-turn-on-off-action? Or will this result in the action being run on all devices?

Do you suggest we add a different action?

This is tricky, because adding an extra state means you have to fit it into the existing state transitions.

I think the current TURN_ON and TURN_OFF states are doing double duty to signify both the voltage applied (GPIO configuration needed) and device state (Reconfiguration needed) actions. Which mostly makes sense, because for external devices these two tend to be linked. If the device is powered it can hold state, if it is not it cannot.

Are these IP blocks losing state because they are being powered down, or for some other reason (i.e. clocks disabled)?

[mmahadevan108]

We have SoC low power states where the IP blocks lose state and need to be reconfigured on exit from low power modes.

I don't use system PM, is there a link somewhere between the IP blocks that do lose state and the power state with zephyr,pm-turn-on-off-action? Or will this result in the action being run on all devices?

This action will be run on all devices. If for some reason the device is already TURNED_OFF, then the system will ignore and move on to the next device. I could add logic to skip calling the TURN_OFF action on devices that are registered with a power-domain driver.

Do you suggest we add a different action?

This is tricky, because adding an extra state means you have to fit it into the existing state transitions.

I think the current TURN_ON and TURN_OFF states are doing double duty to signify both the voltage applied (GPIO configuration needed) and device state (Reconfiguration needed) actions. Which mostly makes sense, because for external devices these two tend to be linked. If the device is powered it can hold state, if it is not it cannot.

Are these IP blocks losing state because they are being powered down, or for some other reason (i.e. clocks disabled)?

They are being power down by the SoC.

[JordanYates]

They are being power down by the SoC.

Then I think TURN_ON and TURN_OFF can be used with no problems.

This action will be run on all devices. If for some reason the device is already TURNED_OFF, then the system will ignore and move on to the next device. I could add logic to skip calling the TURN_OFF action on devices that are registered with a power-domain driver.

This is a problem. Your suggested solution is not sufficient because that would also run the action on external devices that are not on a power domain. IMO there needs to be a setup where the actions are run ONLY on the IP blocks that are being powered down, not relying on the PM subsystem to discard actions.

[ceolin]

They are being power down by the SoC.

Then I think TURN_ON and TURN_OFF can be used with no problems.

Yep !

This action will be run on all devices. If for some reason the device is already TURNED_OFF, then the system will ignore and move on to the next device. I could add logic to skip calling the TURN_OFF action on devices that are registered with a power-domain driver.

This is a problem. Your suggested solution is not sufficient because that would also run the action on external devices that are not on a power domain. IMO there needs to be a setup where the actions are run ONLY on the IP blocks that are being powered down, not relying on the PM subsystem to discard actions.

Agree, this action should run only in devices that are losing power. @mmahadevan108 you can use the property added in this commit 02a14d7. It tells which power states cause power loss in a device. You may leverage it and trigger on/off in devices using it.

[mmahadevan108]

They are being power down by the SoC.

Then I think TURN_ON and TURN_OFF can be used with no problems.

Yep !

This action will be run on all devices. If for some reason the device is already TURNED_OFF, then the system will ignore and move on to the next device. I could add logic to skip calling the TURN_OFF action on devices that are registered with a power-domain driver.

This is a problem. Your suggested solution is not sufficient because that would also run the action on external devices that are not on a power domain. IMO there needs to be a setup where the actions are run ONLY on the IP blocks that are being powered down, not relying on the PM subsystem to discard actions.

Agree, this action should run only in devices that are losing power. @mmahadevan108 you can use the property added in this commit 02a14d7. It tells which power states cause power loss in a device. You may leverage it and trigger on/off in devices using it.

@ceolin, the zephyr,disabling-power-states will disable Device PM for a particular power state and I do not want that to happen. We do want device PM to be called, however with a different action.

[mmahadevan108]

They are being power down by the SoC.

Then I think TURN_ON and TURN_OFF can be used with no problems.

Great, this is what this PR is proposing.

This action will be run on all devices. If for some reason the device is already TURNED_OFF, then the system will ignore and move on to the next device. I could add logic to skip calling the TURN_OFF action on devices that are registered with a power-domain driver.

This is a problem. Your suggested solution is not sufficient because that would also run the action on external devices that are not on a power domain. IMO there needs to be a setup where the actions are run ONLY on the IP blocks that are being powered down, not relying on the PM subsystem to discard actions.

In this proposal, it is assumed that for certain power states all devices except the one's marked for wakeup are powered down.

[mmahadevan108]

@JordanYates @ceolin @gmarull please let me know your thoughts/suggestions on this PR.

[JordanYates]

In this proposal, it is assumed that for certain power states all devices except the one's marked for wakeup are powered down.

That is an incorrect assumption, you can't make any statements about the power state of devices that are external to the SoC.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[ceolin]

Minor revisions made by me. I believe this aligns with our desired direction, which is to map all devices to their respective power rails. It is logical to have a separate power rail for devices within the same SoC power domain.

[ceolin]

@JordanYates, I can pull some parts of this PR into this to show usage. Would that work? #84945

https://github.com/zephyrproject-rtos/zephyr/tree/main/tests/subsys/pm/power_states_api demonstrates that it is quite simple to test the PM state API in CI. I would prefer a copy of that test which adds in your power domain driver, and then does some basic validation that the devices are put into the correct states as the SoC PM states transition.

That provides the advantage of also testing that the implementation works, not just that the code compiles.

Agree !

[mmahadevan108]

@JordanYates @ceolin . One thing with this new approach of using a Power Domain Driver is that every node in the SoC dtsi file would need to be updated to add the power-domains property.
I think the issue highlighted with the original approach was it was calling TURN_ON and TURN_OFF actions on every device. This can be avoided by adding a pm_device_on_power_domain check and skip devices that are managed by a Power Domain Driver. The TURN_ON/OFF would still get called on the PD driver as there is no way I know to identify it.
Do you think this approach would work?

[JordanYates]

I think the issue highlighted with the original approach was it was calling TURN_ON and TURN_OFF actions on every device. This can be avoided by adding a pm_device_on_power_domain check and skip devices that are managed by a Power Domain Driver. The TURN_ON/OFF would still get called on the PD driver as there is no way I know to identify it.
Do you think this approach would work?

No, that would still run actions on external devices that are not on a power domain (permanently powered).

[JordanYates]

One thing with this new approach of using a Power Domain Driver is that every node in the SoC dtsi file would need to be updated to add the power-domains property.

That sounds like a reasonable requirement, there needs to be some link between devices and the power states they work in, where else would you put it?

[mmahadevan108]

main/tests/subsys/pm/power_states_api demonstrates that it is quite simple to test the PM state API in CI. I would prefer a copy of that test which adds in your power domain driver, and then does some basic validation that the devices are put into the correct states as the SoC PM states transition.

That provides the advantage of also testing that the implementation works, not just that the code compiles.

New test added.

[mmahadevan108]

@gmarull , could you kindly revisit this PR.

[mmahadevan108]

I think the issue highlighted with the original approach was it was calling TURN_ON and TURN_OFF actions on every device. This can be avoided by adding a pm_device_on_power_domain check and skip devices that are managed by a Power Domain Driver. The TURN_ON/OFF would still get called on the PD driver as there is no way I know to identify it.
Do you think this approach would work?

No, that would still run actions on external devices that are not on a power domain (permanently powered).

Thank you. For my reference and understanding, can you point me to some of these devices

[JordanYates]

Thank you. For my reference and understanding, can you point me to some of these devices

Pick practically any sensor driver, and find an instance of it in a board file without power-domains = <>;.
As a random example,

zephyr/boards/nordic/thingy53/thingy53_nrf5340_common.dtsi

Line 221 in 759e31d

bme688: bme688@76 {

[ceolin]

@mmahadevan108 thanks for putting it together. I tried the test and the failure does not seem to be a test problem. I noticed one additional tick in the timeout that is making the kernel call twice the suspend function instead of one. I am checking why this is happening.

[mmahadevan108]

@mmahadevan108 thanks for putting it together. I tried the test and the failure does not seem to be a test problem. I noticed one additional tick in the timeout that is making the kernel call twice the suspend function instead of one. I am checking why this is happening.

Thanks @ceolin, I have added a change to address that issue.

[mmahadevan108]

@ceolin @JordanYates @gmarull , thank you for your reviews. I think I have addressed your review comments. Can you please take a look one more time.

stale review

[ceolin]

@JordanYates can you take another look please ?