From 91b8abfb4d52bbf0e7b9a22147026c21276f3dac Mon Sep 17 00:00:00 2001
From: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
Date: Thu, 2 Dec 2021 12:31:06 +0100
Subject: [PATCH] pm: device_runtime: suspend device on enable

The pm_device_runtime_enable did not suspend devices, so it assumed that
the device was in a physically suspended state. This change makes sure
that device is left in a suspended state if the device is initially
active.

Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
---
 doc/guides/pm/device_runtime.rst              | 19 ++++++++++++-------
 include/pm/device_runtime.h                   |  6 ++++++
 subsys/pm/device_runtime.c                    |  8 ++++++++
 tests/subsys/pm/device_runtime_api/src/main.c |  8 ++++++++
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/doc/guides/pm/device_runtime.rst b/doc/guides/pm/device_runtime.rst
index 16a40928213..561113fc777 100644
--- a/doc/guides/pm/device_runtime.rst
+++ b/doc/guides/pm/device_runtime.rst
@@ -142,23 +142,28 @@ special synchronization is required.
 
 To enable device runtime power management on a device, the driver needs to call
 :c:func:`pm_device_runtime_enable` at initialization time. Note that this
-function will put the device into the :c:enumerator:`PM_DEVICE_STATE_SUSPENDED`
-state. In case this does not reflect the actual device state, the init function
-must perform the necessary operations to suspend the device.
+function will suspend the device if its state is
+:c:enumerator:`PM_DEVICE_STATE_ACTIVE`. In case the device is physically
+suspended, the init function should call
+:c:func:`pm_device_runtime_init_suspended` before calling
+:c:func:`pm_device_runtime_enable`.
 
 .. code-block:: c
 
     /* device driver initialization function */
     static int mydev_init(const struct device *dev)
     {
+        int ret;
         ...
-        /* make sure the device physically is suspended */
+
+        /* OPTIONAL: mark device as suspended if it is physically suspended */
+        pm_device_runtime_init_suspended(dev);
+
         /* enable device runtime power management */
         ret = pm_device_runtime_enable(dev);
-        if (ret < 0) {
-                return ret;
+        if ((ret < 0) && (ret != -ENOSYS)) {
+            return ret;
         }
-        ...
     }
 
 Assuming an example device driver that implements an ``operation`` API call, the
diff --git a/include/pm/device_runtime.h b/include/pm/device_runtime.h
index 5424d2ae654..c01bfce167f 100644
--- a/include/pm/device_runtime.h
+++ b/include/pm/device_runtime.h
@@ -25,6 +25,9 @@ extern "C" {
 /**
  * @brief Enable device runtime PM
  *
+ * This function will enable runtime PM on the given device. If the device is
+ * in #PM_DEVICE_STATE_ACTIVE state, the device will be suspended.
+ *
  * @funcprops \pre_kernel_ok
  *
  * @param dev Device instance.
@@ -32,6 +35,9 @@ extern "C" {
  * @retval 0 If the device runtime PM is enabled successfully.
  * @retval -EPERM If device has power state locked.
  * @retval -ENOSYS If the functionality is not available.
+ * @retval -errno Other negative errno, result of suspending the device.
+ *
+ * @see pm_device_runtime_init_suspended()
  */
 int pm_device_runtime_enable(const struct device *dev);
 
diff --git a/subsys/pm/device_runtime.c b/subsys/pm/device_runtime.c
index 8562158c1f2..e55871bd33f 100644
--- a/subsys/pm/device_runtime.c
+++ b/subsys/pm/device_runtime.c
@@ -188,6 +188,14 @@ int pm_device_runtime_enable(const struct device *dev)
 		pm->dev = dev;
 		k_work_init_delayable(&pm->work, runtime_suspend_work);
 	}
+
+	if (pm->state == PM_DEVICE_STATE_ACTIVE) {
+		ret = pm->action_cb(pm->dev, PM_DEVICE_ACTION_SUSPEND);
+		if (ret < 0) {
+			goto unlock;
+		}
+	}
+
 	pm->state = PM_DEVICE_STATE_SUSPENDED;
 	pm->usage = 0U;
 
diff --git a/tests/subsys/pm/device_runtime_api/src/main.c b/tests/subsys/pm/device_runtime_api/src/main.c
index 35d2c53b20a..558b510e304 100644
--- a/tests/subsys/pm/device_runtime_api/src/main.c
+++ b/tests/subsys/pm/device_runtime_api/src/main.c
@@ -35,6 +35,7 @@ static void get_runner(void *arg1, void *arg2, void *arg3)
 static void test_api_setup(void)
 {
 	int ret;
+	enum pm_device_state state;
 
 	/* check API always returns 0 when runtime PM is disabled */
 	ret = pm_device_runtime_get(dev);
@@ -47,6 +48,13 @@ static void test_api_setup(void)
 	/* enable runtime PM */
 	ret = pm_device_runtime_enable(dev);
 	zassert_equal(ret, 0, NULL);
+
+	(void)pm_device_state_get(dev, &state);
+	zassert_equal(state, PM_DEVICE_STATE_SUSPENDED, NULL);
+
+	/* enabling again should succeed (no-op) */
+	ret = pm_device_runtime_enable(dev);
+	zassert_equal(ret, 0, NULL);
 }
 
 static void test_api_teardown(void)