All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: "Linux PM" <linux-pm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Ulf Hansson" <ulf.hansson@linaro.org>,
	"Lukas Wunner" <lukas@wunner.de>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: Re: [PATCH v1] PM: runtime: docs: Update pm_runtime_allow/forbid() documentation
Date: Wed, 22 Oct 2025 13:44:20 -0700	[thread overview]
Message-ID: <aPlCJB3nzSbpO-S2@google.com> (raw)
In-Reply-To: <12780841.O9o76ZdvQC@rafael.j.wysocki>

Hi Rafael,

On Wed, Oct 22, 2025 at 10:26:23PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Drop confusing descriptions of pm_runtime_allow() and pm_runtime_forbid()
> from Documentation/power/runtime_pm.rst and update the kerneldoc comments
> of these functions to better explain their purpose.
> 
> Link: https://lore.kernel.org/linux-pm/08976178-298f-79d9-1d63-cff5a4e56cc3@linux.intel.com/
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  Documentation/power/runtime_pm.rst |   10 ----------
>  drivers/base/power/runtime.c       |   17 +++++++++++++----
>  2 files changed, 13 insertions(+), 14 deletions(-)
> 
> --- a/Documentation/power/runtime_pm.rst
> +++ b/Documentation/power/runtime_pm.rst
> @@ -480,16 +480,6 @@ drivers/base/power/runtime.c and include
>    `bool pm_runtime_status_suspended(struct device *dev);`
>      - return true if the device's runtime PM status is 'suspended'
>  
> -  `void pm_runtime_allow(struct device *dev);`
> -    - set the power.runtime_auto flag for the device and decrease its usage
> -      counter (used by the /sys/devices/.../power/control interface to
> -      effectively allow the device to be power managed at run time)
> -
> -  `void pm_runtime_forbid(struct device *dev);`
> -    - unset the power.runtime_auto flag for the device and increase its usage
> -      counter (used by the /sys/devices/.../power/control interface to
> -      effectively prevent the device from being power managed at run time)
> -

It feels a little odd just to strip 2 of the APIs from this doc, while
the rest remain. I'm not too familiar with ReST, nor with kerneldoc
integration, but would something like this work as a replacement?

.. kernel-doc:: drivers/base/power/runtime.c
   :export:

.. kernel-doc:: include/linux/pm_runtime.h

>    `void pm_runtime_no_callbacks(struct device *dev);`
>      - set the power.no_callbacks flag for the device and remove the runtime
>        PM attributes from /sys/devices/.../power (or prevent them from being
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -1664,9 +1664,12 @@ EXPORT_SYMBOL_GPL(devm_pm_runtime_get_no
>   * pm_runtime_forbid - Block runtime PM of a device.
>   * @dev: Device to handle.
>   *
> - * Increase the device's usage count and clear its power.runtime_auto flag,
> - * so that it cannot be suspended at run time until pm_runtime_allow() is called
> - * for it.
> + * Resume @dev if already suspended and block runtime suspend of @dev in such
> + * a way that it can be unblocked via the /sys/devices/.../power/control
> + * interface, or otherwise by calling pm_runtime_allow().
> + *
> + * Calling this function many times in a row has the same effect as calling it
> + * once.
>   */
>  void pm_runtime_forbid(struct device *dev)
>  {
> @@ -1687,7 +1690,13 @@ EXPORT_SYMBOL_GPL(pm_runtime_forbid);
>   * pm_runtime_allow - Unblock runtime PM of a device.
>   * @dev: Device to handle.
>   *
> - * Decrease the device's usage count and set its power.runtime_auto flag.
> + * Unblock runtime suspend of @dev after it has been blocked by
> + * pm_runtime_forbid() (for instance, if it has been blocked via the
> + * /sys/devices/.../power/control interface), check if @dev can be
> + * suspended and suspend it in that case.
> + *
> + * Calling this function many times in a row has the same effect as calling it
> + * once.
>   */
>  void pm_runtime_allow(struct device *dev)
>  {

The rewording looks helpful, as it's much more API-user-oriented now. So
this looks good to me as-is, even if there are other potential
improvements to make:

Reviewed-by: Brian Norris <briannorris@chromium.org>

Thanks!

  reply	other threads:[~2025-10-22 20:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-22 20:26 [PATCH v1] PM: runtime: docs: Update pm_runtime_allow/forbid() documentation Rafael J. Wysocki
2025-10-22 20:44 ` Brian Norris [this message]
2025-10-23 10:17   ` Rafael J. Wysocki
2025-10-23 10:14 ` Ulf Hansson
2025-10-23 13:16 ` Ilpo Järvinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPlCJB3nzSbpO-S2@google.com \
    --to=briannorris@chromium.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=rafael@kernel.org \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.