Specify and document current PM_DEVICE behavior, including PM_DEVICE=n and device_deinit() #90275

bjarki-andreasen · 2025-05-21T10:36:56Z

This PR aims to document the status quo of device power management to provide a foundation for current driver development, and future refactoring discussions, since we need to agree on the current state before being able to discuss the future state :)

The PR also adds the missing counterpart to pm_device_driver_init().

JordanYates

What is the purpose of this function? It seems odd that it is running functions when CONFIG_PM_DEVICE=n but doesn't do anything when CONFIG_PM_DEVICE=y?

bjarki-andreasen · 2025-05-21T12:54:51Z

What is the purpose of this function? It seems odd that it is running functions when CONFIG_PM_DEVICE=n but doesn't do anything when CONFIG_PM_DEVICE=y?

Check out subsys/pm/device.c for now it simply ensures the device is not in use (suspended or off) to prevent deinit of a device which is in use. Even if it did nothing if CONFIG_PM_DEVICE=n, that would still be fine IMO, its there to allow device_deinit() to undo what device_init() did through pm_device_driver_init()

subsys/pm/device.c

JordanYates

For now, this is a simple check ensuring the device is not in use

What is stopping a final implementation from being implemented now? The function should have defined semantics, and they should be implemented properly when the function is introduced, not modified at some point in "the future".

bjarki-andreasen · 2025-05-22T11:55:22Z

For now, this is a simple check ensuring the device is not in use

What is stopping a final implementation from being implemented now? The function should have defined semantics, and they should be implemented properly when the function is introduced, not modified at some point in "the future".

This is all we need for now, I don't think there is a "final" implementation, its just a hook for doing whatever is required of PM at a given time :)

JordanYates · 2025-05-22T12:02:02Z

include/zephyr/pm/device.h

+		return rc;
+	}
+
+	rc = action_cb(dev, PM_DEVICE_ACTION_TURN_OFF);


This doesn't seem right, the device is not literally being turned off, just the software construct destroyed.
This would lead to GPIO's being put into incorrect states if used generically, which the PM API is.

Overloading the meaning of PM_DEVICE_ACTION_TURN_OFF is a bad idea IMO.

I'm okay with removing the call to TURN_OFF, given that all resources like GPIOs are released as part of device_deinit(), I think you are right that it makes little sense to act as if the driver is being turned off before being deinitialized.

Agree about it

include/zephyr/pm/device.h

bjarki-andreasen · 2025-05-27T13:11:22Z

@JordanYates could you revisit this one?

subsys/pm/device.c

JordanYates

It's still not clear to me how this helps to reliably implement the requirements of the device_deinit API.

zephyr/include/zephyr/device.h

Lines 880 to 885 in af46f62

    
            * When a device is de-initialized, it will release any resources it has 
        
            * acquired (e.g. pins, memory, clocks, DMA channels, etc.) and its status will 
        
            * be left as in its reset state. 
        
            * 
        
            * @warning It is the responsibility of the caller to ensure that the device is 
        
            * ready to be de-initialized.

It's on the caller to determine whether the device can be de-initialized, not the implementation.

Why is STATE_SUSPENDED any more likely to be the "reset state" of a device than STATE_ACTIVE? In many cases the suspend state is explicitly not the reset state of a device.

bjarki-andreasen · 2025-05-29T09:31:02Z

It's still not clear to me how this helps to reliably implement the requirements of the device_deinit API.

zephyr/include/zephyr/device.h

Lines 880 to 885 in af46f62

* When a device is de-initialized, it will release any resources it has

* acquired (e.g. pins, memory, clocks, DMA channels, etc.) and its status will

* be left as in its reset state.

*

* @warning It is the responsibility of the caller to ensure that the device is

* ready to be de-initialized.

It's on the caller to determine whether the device can be de-initialized, not the implementation.

This to my knowledge was intended to clearly indicate that there is no reference counting or other tracking of who is using the device built in to the device model (which there was with an early version of device_deinit()), not that no checks are allowed to make sure it makes sense. Otherwise surely

zephyr/kernel/device.c

Lines 150 to 152 in 600cae6

    
           if (!dev->state->initialized) { 
        
           	return -EPERM; 
        
           }

would not exist :) The function returns a value, it can fail, we are allowed to do additional sanity checking.

Why is STATE_SUSPENDED any more likely to be the "reset state" of a device than STATE_ACTIVE? In many cases the suspend state is explicitly not the reset state of a device.

If CONFIG_PM_DEVICE=n, state is not checked, instead it just calls the suspend action. If CONFIG_PM_DEVICE=y, then device has to be suspended, thus STATE_SUSPENDED or STATE_OFF are explicitly states from which the "reset state" can be safely entered. The only difference between STATE_SUSPENDED and STATE_OFF would be whether the hardware block lost power, and thus was implicitly reset.

JordanYates · 2025-05-29T12:09:30Z

As a concrete request, please document what the function is actually supposed to be doing beyond just @brief Deinit a device driver. It is easier to check that an intended behavior makes sense and that an implementation matches the intended behavior than to try and reason about how a particular implementation is supposed to fit into the larger device_deinit API based only on code.

include/zephyr/pm/device.h

danieldegrasse · 2025-07-02T22:52:37Z

@decsny can you take a look as well?

decsny · 2025-07-04T01:13:28Z

I trust that it's probably fine, I won't be able to review it properly this week, don't wait on me

aescolar · 2025-07-19T08:14:11Z

doc/releases/release-notes-4.2.rst

@@ -221,6 +221,7 @@ New APIs and options
    * :kconfig:option:`CONFIG_PM_DEVICE_RUNTIME_DEDICATED_WQ_PRIO`
    * :kconfig:option:`CONFIG_PM_DEVICE_RUNTIME_DEDICATED_WQ_INIT_PRIO`
    * :kconfig:option:`CONFIG_PM_DEVICE_RUNTIME_ASYNC`
+    * :c:func:`pm_device_driver_deinit`


should this modify the 4.3 release notes? or is it meant as a backport?
(Feel free to disregard this review if all it needs is a backport label)

Its not meant to be backported :) Release notes will be updated

Rewrite the docstring for pm_device_driver_init() to align with documentation, especially covering the case where CONFIG_PM_DEVICE=n Signed-off-by: Bjarki Arge Andreasen <[email protected]>

Introduce pm_device_driver_deinit() which is required to fully support device_deinit(). This function shall be called by drivers when device_deinit() is called. If PM_DEVICE is enabled, it will simply check whether the device is either SUSPENDED or OFF, this will prevent device_deinit() on a device which is currently in use, and ensure the device is in the correct state if device_init() is called later. If PM_DEVICE is not enabled, it will invoke the SUSPEND action which mirrors the pm_device_driver_init() behavior. Note that TURN_OFF action is not called since the device is deinitialized after this call. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

Extend test suite to cover pm_device_driver_deinit() resulting from call to device_deinit(). Signed-off-by: Bjarki Arge Andreasen <[email protected]>

- Specify "Device" Power Management Support in sections covering CONFIG_PM_DEVICE=y or n - Extend the enum pm_device_state docstrings to define and detail expectations of devices in each enum pm_device_state state. - Extend pm device driver code example with more concise comments and include pm_device_driver_init, pm_device_driver_deinit, and DEVICE_DT_INST_DEINIT_DEFINE() (device deinit feature) - Describe the device driver PM states used if PM_DEVICE is not enabled. - Note which states a device driver is initialized from and can be deinitialized in and why. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

Mention introduction of pm_device_driver_deinit() in the 4.3 release notes. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen · 2025-07-21T08:15:39Z

Rebased and moved mention of introduction of pm_device_driver_deinit from release notes 4.2 to 4.3

sonarqubecloud · 2025-07-21T08:18:17Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Addressed

bjarki-andreasen · 2025-07-23T09:34:59Z

ping @JordanYates @decsny

bjarki-andreasen requested review from JordanYates, ceolin, decsny and fabiobaltieri May 21, 2025 10:37

github-actions bot added the area: Power Management label May 21, 2025

github-actions bot requested review from nashif, teburd and tmleman May 21, 2025 10:37

github-actions bot assigned ceolin May 21, 2025

fabiobaltieri requested a review from keith-zephyr May 21, 2025 10:52

bjarki-andreasen force-pushed the pm-device-driver-deinit branch from 537633f to 4ea0b00 Compare May 21, 2025 10:54

JordanYates reviewed May 21, 2025

View reviewed changes

JordanYates reviewed May 22, 2025

View reviewed changes

subsys/pm/device.c Show resolved Hide resolved

JordanYates requested changes May 22, 2025

View reviewed changes

JordanYates reviewed May 22, 2025

View reviewed changes

bjarki-andreasen force-pushed the pm-device-driver-deinit branch from 4ea0b00 to 0f5a01c Compare May 22, 2025 12:20

ceolin reviewed May 25, 2025

View reviewed changes

include/zephyr/pm/device.h Outdated Show resolved Hide resolved

bjarki-andreasen force-pushed the pm-device-driver-deinit branch from 0f5a01c to 4a6a1d6 Compare May 25, 2025 18:55

ceolin previously approved these changes May 26, 2025

View reviewed changes

tobiwan88 reviewed May 28, 2025

View reviewed changes

subsys/pm/device.c Show resolved Hide resolved

tobiwan88 approved these changes May 28, 2025

View reviewed changes

JordanYates requested changes May 29, 2025

View reviewed changes

bjarki-andreasen mentioned this pull request May 29, 2025

Add device init/de-init functionality #84394

Merged

decsny reviewed May 30, 2025

View reviewed changes

include/zephyr/pm/device.h Show resolved Hide resolved

bjarki-andreasen dismissed ceolin’s stale review via a755e72 June 3, 2025 15:48

ceolin previously approved these changes Jul 1, 2025

View reviewed changes

bjarki-andreasen dismissed ceolin’s stale review via 5e0d6a2 July 1, 2025 12:16

bjarki-andreasen force-pushed the pm-device-driver-deinit branch from d6b30b7 to 5e0d6a2 Compare July 1, 2025 12:16

carlescufi requested review from nashif, tobiwan88 and ceolin July 1, 2025 14:24

nashif previously approved these changes Jul 2, 2025

View reviewed changes

ceolin previously approved these changes Jul 2, 2025

View reviewed changes

bjarki-andreasen removed this from the v4.2.0 milestone Jul 4, 2025

kartben added this to the v4.3.0 milestone Jul 4, 2025

aescolar previously requested changes Jul 19, 2025

View reviewed changes

bjarki-andreasen added 4 commits July 21, 2025 10:06

pm: device: extend pm_device_driver_init() description

274c267

Rewrite the docstring for pm_device_driver_init() to align with documentation, especially covering the case where CONFIG_PM_DEVICE=n Signed-off-by: Bjarki Arge Andreasen <[email protected]>

tests: pm: device_driver_init: extend to support device_deinit()

d51090e

Extend test suite to cover pm_device_driver_deinit() resulting from call to device_deinit(). Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen force-pushed the pm-device-driver-deinit branch from 5e0d6a2 to cc8e50b Compare July 21, 2025 08:14

doc: releases: 4.3: mention pm_device_driver_deinit()

2814832

Mention introduction of pm_device_driver_deinit() in the 4.3 release notes. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen dismissed stale reviews from nashif and ceolin via 2814832 July 21, 2025 08:14

bjarki-andreasen force-pushed the pm-device-driver-deinit branch from cc8e50b to 2814832 Compare July 21, 2025 08:14

bjarki-andreasen requested review from nashif and ceolin July 23, 2025 09:34

decsny approved these changes Jul 23, 2025

View reviewed changes

ceolin approved these changes Jul 26, 2025

View reviewed changes

	* When a device is de-initialized, it will release any resources it has
	* acquired (e.g. pins, memory, clocks, DMA channels, etc.) and its status will
	* be left as in its reset state.
	*
	* @warning It is the responsibility of the caller to ensure that the device is
	* ready to be de-initialized.

Specify and document current PM_DEVICE behavior, including PM_DEVICE=n and device_deinit() #90275

Are you sure you want to change the base?

Specify and document current PM_DEVICE behavior, including PM_DEVICE=n and device_deinit() #90275

Conversation

bjarki-andreasen commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JordanYates left a comment

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen commented May 21, 2025 • edited by carlescufi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

JordanYates left a comment

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JordanYates May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ceolin May 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bjarki-andreasen commented May 27, 2025

Uh oh!

Uh oh!

JordanYates left a comment

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JordanYates commented May 29, 2025

Uh oh!

Uh oh!

danieldegrasse commented Jul 2, 2025

Uh oh!

decsny commented Jul 4, 2025

Uh oh!

aescolar Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen commented Jul 21, 2025

Uh oh!

sonarqubecloud bot commented Jul 21, 2025

Quality Gate passed

Uh oh!

bjarki-andreasen commented Jul 23, 2025

Uh oh!

Uh oh!

bjarki-andreasen commented May 21, 2025 •

edited

Loading

bjarki-andreasen commented May 21, 2025 •

edited by carlescufi

Loading

bjarki-andreasen commented May 22, 2025 •

edited

Loading

JordanYates May 22, 2025 •

edited

Loading

bjarki-andreasen May 22, 2025 •

edited

Loading

bjarki-andreasen commented May 29, 2025 •

edited

Loading

aescolar Jul 19, 2025 •

edited

Loading