diff mbox series

[1/6] pm: runtime: Simplify pm_runtime_get_if_active() usage

Message ID 20231115181840.1509046-2-sakari.ailus@linux.intel.com
State Superseded
Headers show
Series Small Runtime PM API changes | expand

Commit Message

Sakari Ailus Nov. 15, 2023, 6:18 p.m. UTC 
The majority of the callers currently using pm_runtime_get_if_active()
call it with ign_usage_count argument set to true. There's only one driver
using this feature (and natually implementation of
pm_runtime_get_if_in_use()).

To make this function more practical to use, remove the ign_usage_count
argument from the function. The main implementation is renamed as
__pm_runtime_get_conditional().

This function is expected to gain a large number of users in the future
--- camera sensor drivers using runtime autosuspend have a need to get the
device's usage_count conditionally if it is enabled. Most of these
currently use pm_runtime_get_if_in_use(), which is a bug.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/power/runtime_pm.rst      |  5 ++--
 drivers/base/power/runtime.c            |  9 ++++---
 drivers/gpu/drm/i915/intel_runtime_pm.c |  2 +-
 drivers/media/i2c/ccs/ccs-core.c        |  2 +-
 drivers/net/ipa/ipa_smp2p.c             |  2 +-
 drivers/pci/pci.c                       |  2 +-
 include/linux/pm_runtime.h              | 31 +++++++++++++++++++++----
 sound/hda/hdac_device.c                 |  2 +-
 8 files changed, 39 insertions(+), 16 deletions(-)

Comments

kernel test robot Nov. 15, 2023, 9:04 p.m. UTC | #1 
Hi Sakari,

kernel test robot noticed the following build errors:

[auto build test ERROR on 3e238417254bfdcc23fe207780b59cbb08656762]

url:    https://github.com/intel-lab-lkp/linux/commits/Sakari-Ailus/pm-runtime-Simplify-pm_runtime_get_if_active-usage/20231116-022051
base:   3e238417254bfdcc23fe207780b59cbb08656762
patch link:    https://lore.kernel.org/r/20231115181840.1509046-2-sakari.ailus%40linux.intel.com
patch subject: [PATCH 1/6] pm: runtime: Simplify pm_runtime_get_if_active() usage
config: i386-buildonly-randconfig-006-20231116 (https://download.01.org/0day-ci/archive/20231116/202311160402.sZooXBaB-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231116/202311160402.sZooXBaB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311160402.sZooXBaB-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_runtime_pm.c: In function '__intel_runtime_pm_get_if_active':
>> drivers/gpu/drm/i915/intel_runtime_pm.c:437:21: error: too many arguments to function '__pm_runtime_get_conditional'
     437 |                 if (__pm_runtime_get_conditional(rpm->kdev, ignore_usecount) <= 0)
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/i915/intel_runtime_pm.c:29:
   include/linux/pm_runtime.h:300:19: note: declared here
     300 | static inline int __pm_runtime_get_conditional(struct device *dev)
         |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/__pm_runtime_get_conditional +437 drivers/gpu/drm/i915/intel_runtime_pm.c

   404	
   405	/**
   406	 * __intel_runtime_pm_get_if_active - grab a runtime pm reference if device is active
   407	 * @rpm: the intel_runtime_pm structure
   408	 * @ignore_usecount: get a ref even if dev->power.usage_count is 0
   409	 *
   410	 * This function grabs a device-level runtime pm reference if the device is
   411	 * already active and ensures that it is powered up. It is illegal to try
   412	 * and access the HW should intel_runtime_pm_get_if_active() report failure.
   413	 *
   414	 * If @ignore_usecount is true, a reference will be acquired even if there is no
   415	 * user requiring the device to be powered up (dev->power.usage_count == 0).
   416	 * If the function returns false in this case then it's guaranteed that the
   417	 * device's runtime suspend hook has been called already or that it will be
   418	 * called (and hence it's also guaranteed that the device's runtime resume
   419	 * hook will be called eventually).
   420	 *
   421	 * Any runtime pm reference obtained by this function must have a symmetric
   422	 * call to intel_runtime_pm_put() to release the reference again.
   423	 *
   424	 * Returns: the wakeref cookie to pass to intel_runtime_pm_put(), evaluates
   425	 * as True if the wakeref was acquired, or False otherwise.
   426	 */
   427	static intel_wakeref_t __intel_runtime_pm_get_if_active(struct intel_runtime_pm *rpm,
   428								bool ignore_usecount)
   429	{
   430		if (IS_ENABLED(CONFIG_PM)) {
   431			/*
   432			 * In cases runtime PM is disabled by the RPM core and we get
   433			 * an -EINVAL return value we are not supposed to call this
   434			 * function, since the power state is undefined. This applies
   435			 * atm to the late/early system suspend/resume handlers.
   436			 */
 > 437			if (__pm_runtime_get_conditional(rpm->kdev, ignore_usecount) <= 0)
   438				return 0;
   439		}
   440	
   441		intel_runtime_pm_acquire(rpm, true);
   442	
   443		return track_intel_runtime_pm_wakeref(rpm);
   444	}
   445
diff mbox series

Patch

diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
index 65b86e487afe..da99379071a4 100644
--- a/Documentation/power/runtime_pm.rst
+++ b/Documentation/power/runtime_pm.rst
@@ -396,10 +396,9 @@  drivers/base/power/runtime.c and include/linux/pm_runtime.h:
       nonzero, increment the counter and return 1; otherwise return 0 without
       changing the counter
 
-  `int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count);`
+  `int pm_runtime_get_if_active(struct device *dev);`
     - return -EINVAL if 'power.disable_depth' is nonzero; otherwise, if the
-      runtime PM status is RPM_ACTIVE, and either ign_usage_count is true
-      or the device's usage_count is non-zero, increment the counter and
+      runtime PM status is RPM_ACTIVE, increment the counter and
       return 1; otherwise return 0 without changing the counter
 
   `void pm_runtime_put_noidle(struct device *dev);`
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 4545669cb973..8b56468eca9d 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1175,7 +1175,7 @@  int __pm_runtime_resume(struct device *dev, int rpmflags)
 EXPORT_SYMBOL_GPL(__pm_runtime_resume);
 
 /**
- * pm_runtime_get_if_active - Conditionally bump up device usage counter.
+ * __pm_runtime_get_conditional - Conditionally bump up device usage counter.
  * @dev: Device to handle.
  * @ign_usage_count: Whether or not to look at the current usage counter value.
  *
@@ -1195,8 +1195,11 @@  EXPORT_SYMBOL_GPL(__pm_runtime_resume);
  *
  * The caller is responsible for decrementing the runtime PM usage counter of
  * @dev after this function has returned a positive value for it.
+ *
+ * This function is not intended to be called by drivers, use
+ * pm_runtime_get_if_active() or pm_runtime_get_if_in_use() instead.
  */
-int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count)
+int __pm_runtime_get_conditional(struct device *dev, bool ign_usage_count)
 {
 	unsigned long flags;
 	int retval;
@@ -1217,7 +1220,7 @@  int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count)
 
 	return retval;
 }
-EXPORT_SYMBOL_GPL(pm_runtime_get_if_active);
+EXPORT_SYMBOL_GPL(__pm_runtime_get_conditional);
 
 /**
  * __pm_runtime_set_status - Set runtime PM status of a device.
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6d8e5e5c0cba..b163efe80975 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -434,7 +434,7 @@  static intel_wakeref_t __intel_runtime_pm_get_if_active(struct intel_runtime_pm
 		 * function, since the power state is undefined. This applies
 		 * atm to the late/early system suspend/resume handlers.
 		 */
-		if (pm_runtime_get_if_active(rpm->kdev, ignore_usecount) <= 0)
+		if (__pm_runtime_get_conditional(rpm->kdev, ignore_usecount) <= 0)
 			return 0;
 	}
 
diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 12e6f0a26fc8..45701a18c845 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -664,7 +664,7 @@  static int ccs_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	}
 
-	pm_status = pm_runtime_get_if_active(&client->dev, true);
+	pm_status = pm_runtime_get_if_active(&client->dev);
 	if (!pm_status)
 		return 0;
 
diff --git a/drivers/net/ipa/ipa_smp2p.c b/drivers/net/ipa/ipa_smp2p.c
index 5620dc271fac..cbf3d4761ce3 100644
--- a/drivers/net/ipa/ipa_smp2p.c
+++ b/drivers/net/ipa/ipa_smp2p.c
@@ -92,7 +92,7 @@  static void ipa_smp2p_notify(struct ipa_smp2p *smp2p)
 		return;
 
 	dev = &smp2p->ipa->pdev->dev;
-	smp2p->power_on = pm_runtime_get_if_active(dev, true) > 0;
+	smp2p->power_on = pm_runtime_get_if_active(dev) > 0;
 
 	/* Signal whether the IPA power is enabled */
 	mask = BIT(smp2p->enabled_bit);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 59c01d68c6d5..d8d4abbc936f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2439,7 +2439,7 @@  static void pci_pme_list_scan(struct work_struct *work)
 			 * If the device is in a low power state it
 			 * should not be polled either.
 			 */
-			pm_status = pm_runtime_get_if_active(dev, true);
+			pm_status = pm_runtime_get_if_active(dev);
 			if (!pm_status)
 				continue;
 
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 7c9b35448563..810330bb802b 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -72,7 +72,8 @@  extern int pm_runtime_force_resume(struct device *dev);
 extern int __pm_runtime_idle(struct device *dev, int rpmflags);
 extern int __pm_runtime_suspend(struct device *dev, int rpmflags);
 extern int __pm_runtime_resume(struct device *dev, int rpmflags);
-extern int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count);
+extern int __pm_runtime_get_conditional(struct device *dev,
+					bool ign_usage_count);
 extern int pm_schedule_suspend(struct device *dev, unsigned int delay);
 extern int __pm_runtime_set_status(struct device *dev, unsigned int status);
 extern int pm_runtime_barrier(struct device *dev);
@@ -94,16 +95,33 @@  extern void pm_runtime_release_supplier(struct device_link *link);
 
 extern int devm_pm_runtime_enable(struct device *dev);
 
+/**
+ * pm_runtime_get_if_active - Bump up runtime PM usage counter if the device is
+ *			      in active state
+ * @dev: Target device.
+ *
+ * Increment the runtime PM usage counter of @dev if its runtime PM status is
+ * %RPM_ACTIVE, in which case it returns 1. If the device is in a different
+ * state, 0 is returned. -EINVAL is returned if runtime PM is disabled for the
+ * device.
+ */
+static inline int pm_runtime_get_if_active(struct device *dev)
+{
+	return __pm_runtime_get_conditional(dev, true);
+}
+
 /**
  * pm_runtime_get_if_in_use - Conditionally bump up runtime PM usage counter.
  * @dev: Target device.
  *
  * Increment the runtime PM usage counter of @dev if its runtime PM status is
- * %RPM_ACTIVE and its runtime PM usage counter is greater than 0.
+ * %RPM_ACTIVE and its runtime PM usage counter is greater than 0, in which case
+ * it returns 1. If the device is in a different state or its usage_count is 0,
+ * 0 is returned. -EINVAL is returned if runtime PM is disabled for the device.
  */
 static inline int pm_runtime_get_if_in_use(struct device *dev)
 {
-	return pm_runtime_get_if_active(dev, false);
+	return __pm_runtime_get_conditional(dev, false);
 }
 
 /**
@@ -275,8 +293,11 @@  static inline int pm_runtime_get_if_in_use(struct device *dev)
 {
 	return -EINVAL;
 }
-static inline int pm_runtime_get_if_active(struct device *dev,
-					   bool ign_usage_count)
+static inline int pm_runtime_get_if_active(struct device *dev)
+{
+	return -EINVAL;
+}
+static inline int __pm_runtime_get_conditional(struct device *dev)
 {
 	return -EINVAL;
 }
diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
index bbf7bcdb449a..0a9223c18d77 100644
--- a/sound/hda/hdac_device.c
+++ b/sound/hda/hdac_device.c
@@ -611,7 +611,7 @@  EXPORT_SYMBOL_GPL(snd_hdac_power_up_pm);
 int snd_hdac_keep_power_up(struct hdac_device *codec)
 {
 	if (!atomic_inc_not_zero(&codec->in_pm)) {
-		int ret = pm_runtime_get_if_active(&codec->dev, true);
+		int ret = pm_runtime_get_if_active(&codec->dev);
 		if (!ret)
 			return -1;
 		if (ret < 0)