Received: by 2002:a05:6a10:a0d1:0:0:0:0 with SMTP id j17csp189140pxa; Tue, 4 Aug 2020 03:07:06 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzMYe1E5xq82iYCvnPX5R5K//ykio4p7PKX1/g1MPs4lgVTGLK0LmydYujFVEcmtGy1imJB X-Received: by 2002:a17:906:1911:: with SMTP id a17mr19542357eje.431.1596535626546; Tue, 04 Aug 2020 03:07:06 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1596535626; cv=none; d=google.com; s=arc-20160816; b=d1/bj/w+4wrCJVhR1B9Zp+pltTXMpjSV2IL1wx/F3lLYcIUFJqivLG9s9oPV//XvhF n3OlOJ7owuygr2WC0CFCJdw7JI4dln0/amqEtOSOVK99+QkyMHqOgRv/jwT4PTAiiRnH YvlXyCf4HZrDPHciUqzeLTcD0RhaIO9A929MWozxDPxwTphhuTNikvN67+n7zp5uDDta Ni9GZHohcIMC9vD5NvvJLAacojtwFnPI5ftK+OKFLVZKeSzagupIgayoGVWGmjS6xAif fMC3AcSBB9WRUbx5u/KH7aJy4tJVus6pVyqKPaktHgEzoBmtaOz5MHPzfYKb+hQKEPhP VWKw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version; bh=7SpZpAeU3AgAf8NDtR2fQzh8H7YzuPFd8ga8GTyjj8M=; b=OHuLvpGKirrKANTDFqb2cn1Wf17oRn6VTXOr4qvc/it+2saBvNReXbrefGfoipeboJ CscSvgyZaAJL132Wm1rgdkglu5StiwK06zrwgc3ZEPD1eBp/M3TuLh2uOJE/N3WDaYVu 3IdLMAxqqhn98WhZSiz8KvTr+tgQzxvflELiUis/HY6GDY7GhWOaNLddrO56iL1m5Dkb qjxIlfL/zXHM6P1OnyfVuwji8RHWQg0oOKUmopdrDgxRtQ9WKh0TH57mTns30xws0bER zbcEgiYV6LuYjbHh6Q1hrSJzOEgIyyjO0EZHngrouFcXKf4PKl9s0UqZDpGUgGFBRG37 SaQw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id o89si3647643eda.223.2020.08.04.03.06.43; Tue, 04 Aug 2020 03:07:06 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730142AbgHDKDc (ORCPT + 99 others); Tue, 4 Aug 2020 06:03:32 -0400 Received: from mail-oi1-f195.google.com ([209.85.167.195]:47094 "EHLO mail-oi1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730132AbgHDKDb (ORCPT ); Tue, 4 Aug 2020 06:03:31 -0400 Received: by mail-oi1-f195.google.com with SMTP id v13so19213284oiv.13; Tue, 04 Aug 2020 03:03:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=7SpZpAeU3AgAf8NDtR2fQzh8H7YzuPFd8ga8GTyjj8M=; b=uC4a9c50znuWsCauIUOiCw9OgL4zhtQOK8EpCj+U2+nDIGTO2bEMBSt5SQN7/wzYEB vxoJzseDY3LJaaDLENTrj+an7YAgoo3L4NVbdWnZLxs6wHpkwoi5XOuw4KFxF02OEF60 zoH1xFDhadlwXQ9R4F025nsRbx3OK/lMSzW+Fp8/+4MQ5MwXT2OKmv0dHIABdkrAsH00 pcFvFnVpPUM8EDN8M5gCsUtotP5FlHE9h37XKtpZO3iiLFACPEuRobQdRw5zd7OUaq3g v1RBSU16coaQMFY3OUFkN+8eqPkv3wTHZsB+iY8af8UzWHfE7hXW/BgSVRjpGB1JM7Kr o8DA== X-Gm-Message-State: AOAM53299/1v3N8LJDAJnP+fR2cPUa1JUQk9bpF9CrQQiCCCl4mckx/k 2nCkqA5TVxeSJyCNN6lBW8JPui5qxmduz5E1bt4= X-Received: by 2002:a54:4f14:: with SMTP id e20mr2791008oiy.103.1596535409346; Tue, 04 Aug 2020 03:03:29 -0700 (PDT) MIME-Version: 1.0 References: <2672940.cHDmkauF2A@kreacher> <20200803230323.GA13316@paasikivi.fi.intel.com> In-Reply-To: <20200803230323.GA13316@paasikivi.fi.intel.com> From: "Rafael J. Wysocki" Date: Tue, 4 Aug 2020 12:03:15 +0200 Message-ID: Subject: Re: [PATCH] PM: runtime: Add kerneldoc comments to multiple helpers To: Sakari Ailus Cc: "Rafael J. Wysocki" , Linux PM , LKML , Alan Stern , Ulf Hansson , Wolfram Sang Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sakari, On Tue, Aug 4, 2020 at 1:03 AM Sakari Ailus wrote: > > Hi Rafael, > > One more comment below. > > On Fri, Jul 31, 2020 at 07:03:26PM +0200, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > > > Add kerneldoc comments to multiple PM-runtime helper functions > > defined as static inline wrappers around lower-level routines to > > provide quick reference decumentation of their behavior. > > > > Some of them are similar to each other with subtle differences only > > and the behavior of some of them may appear as counter-intuitive, so > > clarify all that to avoid confusion. > > > > Signed-off-by: Rafael J. Wysocki > > --- > > include/linux/pm_runtime.h | 246 +++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 246 insertions(+) > > > > Index: linux-pm/include/linux/pm_runtime.h > > =================================================================== > > --- linux-pm.orig/include/linux/pm_runtime.h > > +++ linux-pm/include/linux/pm_runtime.h > > @@ -60,58 +60,151 @@ extern void pm_runtime_put_suppliers(str > > extern void pm_runtime_new_link(struct device *dev); > > extern void pm_runtime_drop_link(struct device *dev); > > > > +/** > > + * 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. > > + */ > > static inline int pm_runtime_get_if_in_use(struct device *dev) > > { > > return pm_runtime_get_if_active(dev, false); > > } > > > > +/** > > + * pm_suspend_ignore_children - Set runtime PM behavior regarding children. > > + * @dev: Target device. > > + * @enable: Whether or not to ignore possible dependencies on children. > > + * > > + * The dependencies of @dev on its children will not be taken into account by > > + * the runtime PM framework going forward if @enable is %true, or they will > > + * be taken into account otherwise. > > + */ > > static inline void pm_suspend_ignore_children(struct device *dev, bool enable) > > { > > dev->power.ignore_children = enable; > > } > > > > +/** > > + * pm_runtime_get_noresume - Bump up runtime PM usage counter of a device. > > + * @dev: Target device. > > + */ > > static inline void pm_runtime_get_noresume(struct device *dev) > > { > > atomic_inc(&dev->power.usage_count); > > } > > > > +/** > > + * pm_runtime_put_noidle - Drop runtime PM usage counter of a device. > > + * @dev: Target device. > > + * > > + * Decrement the runtime PM usage counter of @dev unless it is 0 already. > > + */ > > static inline void pm_runtime_put_noidle(struct device *dev) > > { > > atomic_add_unless(&dev->power.usage_count, -1, 0); > > } > > > > +/** > > + * pm_runtime_suspended - Check whether or not a device is runtime-suspended. > > + * @dev: Target device. > > + * > > + * Return %true if runtime PM is enabled for @dev and its runtime PM status is > > + * %RPM_SUSPENDED, or %false otherwise. > > + * > > + * Note that the return value of this function can only be trusted if it is > > + * called under the runtime PM lock of @dev or under conditions in which > > + * runtime PM cannot be either disabled or enabled for @dev and its runtime PM > > + * status cannot change. > > + */ > > static inline bool pm_runtime_suspended(struct device *dev) > > { > > return dev->power.runtime_status == RPM_SUSPENDED > > && !dev->power.disable_depth; > > } > > > > +/** > > + * pm_runtime_active - Check whether or not a device is runtime-active. > > + * @dev: Target device. > > + * > > + * Return %true if runtime PM is enabled for @dev and its runtime PM status is > > + * %RPM_ACTIVE, or %false otherwise. > > + * > > + * Note that the return value of this function can only be trusted if it is > > + * called under the runtime PM lock of @dev or under conditions in which > > + * runtime PM cannot be either disabled or enabled for @dev and its runtime PM > > + * status cannot change. > > + */ > > static inline bool pm_runtime_active(struct device *dev) > > { > > return dev->power.runtime_status == RPM_ACTIVE > > || dev->power.disable_depth; > > } > > > > +/** > > + * pm_runtime_status_suspended - Check if runtime PM status is "suspended". > > + * @dev: Target device. > > + * > > + * Return %true if the runtime PM status of @dev is %RPM_SUSPENDED, or %false > > + * otherwise, regardless of whether or not runtime PM has been enabled for @dev. > > + * > > + * Note that the return value of this function can only be trusted if it is > > + * called under the runtime PM lock of @dev or under conditions in which the > > + * runtime PM status of @dev cannot change. > > + */ > > static inline bool pm_runtime_status_suspended(struct device *dev) > > { > > return dev->power.runtime_status == RPM_SUSPENDED; > > } > > > > +/** > > + * pm_runtime_enabled - Check if runtime PM is enabled. > > + * @dev: Target device. > > + * > > + * Return %true if runtime PM is enabled for @dev or %false otherwise. > > + * > > + * Note that the return value of this function can only be trusted if it is > > + * called under the runtime PM lock of @dev or under conditions in which > > + * runtime PM cannot be either disabled or enabled for @dev. > > + */ > > static inline bool pm_runtime_enabled(struct device *dev) > > { > > return !dev->power.disable_depth; > > } > > > > +/** > > + * pm_runtime_has_no_callbacks - Check if runtime PM callbacks may be present. > > + * @dev: Target device. > > + * > > + * Return %true if @dev is a special device without runtime PM callbacks or > > + * %false otherwise. > > + */ > > static inline bool pm_runtime_has_no_callbacks(struct device *dev) > > { > > return dev->power.no_callbacks; > > } > > > > +/** > > + * pm_runtime_mark_last_busy - Update the last access time of a device. > > + * @dev: Target device. > > + * > > + * Update the last access time of @dev used by the runtime PM autosuspend > > + * mechanism to the current time as returned by ktime_get_mono_fast_ns(). > > + */ > > static inline void pm_runtime_mark_last_busy(struct device *dev) > > { > > WRITE_ONCE(dev->power.last_busy, ktime_get_mono_fast_ns()); > > } > > > > +/** > > + * pm_runtime_is_irq_safe - Check if runtime PM can work in interrupt context. > > + * @dev: Target device. > > + * > > + * Return %true if @dev has been marked as an "IRQ-safe" device (with respect > > + * to runtime PM), in which case its runtime PM callabcks can be expected to > > + * work correctly when invoked from interrupt handlers. > > + */ > > static inline bool pm_runtime_is_irq_safe(struct device *dev) > > { > > return dev->power.irq_safe; > > @@ -191,97 +284,250 @@ static inline void pm_runtime_drop_link( > > > > #endif /* !CONFIG_PM */ > > > > +/** > > + * pm_runtime_idle - Conditionally set up autosuspend of a device or suspend it. > > + * @dev: Target device. > > + * > > + * Invoke the "idle check" callback of @dev and, depending on its return value, > > + * set up autosuspend of @dev or suspend it (depending on whether or not > > + * autosuspend has been enabled for it). > > + */ > > static inline int pm_runtime_idle(struct device *dev) > > { > > return __pm_runtime_idle(dev, 0); > > } > > > > +/** > > + * pm_runtime_suspend - Suspend a device synchronously. > > + * @dev: Target device. > > + */ > > static inline int pm_runtime_suspend(struct device *dev) > > { > > return __pm_runtime_suspend(dev, 0); > > } > > > > +/** > > + * pm_runtime_autosuspend - Set up autosuspend of a device or suspend it. > > + * @dev: Target device. > > + * > > + * Set up autosuspend of @dev or suspend it (depending on whether or not > > + * autosuspend is enabled for it) without engaging its "idle check" callback. > > + */ > > static inline int pm_runtime_autosuspend(struct device *dev) > > { > > return __pm_runtime_suspend(dev, RPM_AUTO); > > } > > > > +/** > > + * pm_runtime_resume - Resume a device synchronously. > > + * @dev: Target device. > > + */ > > static inline int pm_runtime_resume(struct device *dev) > > { > > return __pm_runtime_resume(dev, 0); > > } > > > > +/** > > + * pm_request_idle - Queue up "idle check" execution for a device. > > + * @dev: Target device. > > + * > > + * Queue up a work item to run an equivalent of pm_runtime_idle() for @dev > > + * asynchronously. > > + */ > > static inline int pm_request_idle(struct device *dev) > > { > > return __pm_runtime_idle(dev, RPM_ASYNC); > > } > > > > +/** > > + * pm_request_resume - Queue up runtime-resume of a device. > > + * @dev: Target device. > > + */ > > static inline int pm_request_resume(struct device *dev) > > { > > return __pm_runtime_resume(dev, RPM_ASYNC); > > } > > > > +/** > > + * pm_request_autosuspend - Queue up autosuspend of a device. > > + * @dev: Target device. > > + * > > + * Queue up a work item to run an equivalent pm_runtime_autosuspend() for @dev > > + * asynchronously. > > + */ > > static inline int pm_request_autosuspend(struct device *dev) > > { > > return __pm_runtime_suspend(dev, RPM_ASYNC | RPM_AUTO); > > } > > > > +/** > > + * pm_runtime_get - Bump up usage counter and queue up resume of a device. > > + * @dev: Target device. > > + * > > + * Bump up the runtime PM usage counter of @dev and queue up a work item to > > + * carry out runtime-resume of it. > > + */ > > static inline int pm_runtime_get(struct device *dev) > > { > > return __pm_runtime_resume(dev, RPM_GET_PUT | RPM_ASYNC); > > } > > > > +/** > > + * pm_runtime_get_sync - Bump up usage counter of a device and resume it. > > + * @dev: Target device. > > + * > > + * Bump up the runtime PM usage counter of @dev and carry out runtime-resume of > > + * it synchronously. > > + * > > + * The possible return values of this function are the same as for > > + * pm_runtime_resume() and the runtime PM usage counter of @dev remains > > I think it's fine to refer to return values of pm_runtime_resume(), but the > return values of pm_runtime_resume() are not documented above. Yes, because it is a wrapper around __pm_runtime_resume() and the return values of that should be documented in its kerneldoc. Documenting them in two places would require manual synchronization of the kerneldoc comments on every future update of them. > Could you add them? I'm going to update the __pm_runtime_resume() kerneldoc to list all of them. > Reviewed-by: Sakari Ailus Thank you!