From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Lukas Wunner <lukas@wunner.de>
Cc: Linux PM <linux-pm@vger.kernel.org>,
Jonathan Corbet <corbet@lwn.net>,
Linux Documentation <linux-doc@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Subject: Re: [PATCH 0/3] PM / docs: linux/pm.h kerneldocs update and conversion of two docs to reST
Date: Thu, 02 Feb 2017 01:06:59 +0100 [thread overview]
Message-ID: <21248446.r6y1HENFuA@aspire.rjw.lan> (raw)
In-Reply-To: <20170109013439.GA23364@wunner.de>
On Monday, January 09, 2017 02:34:39 AM Lukas Wunner wrote:
> On Fri, Jan 06, 2017 at 02:38:13AM +0100, Rafael J. Wysocki wrote:
> > I sent patches [1-2/3] previosly a couple of weeks ago and there have not
> > been any comments since then, so either they are fine by everybody or the
> > timing was particularly bad and no one had the time to look at them.
>
> So far I was only able to peruse the "Device Power Management Data Types"
> section (which is generated from include/linux/pm.h) and came across the
> following:
>
> The description for the ->prepare hook says:
>
> If the transition is a suspend to memory or standby (that
> is, not related to hibernation), the return value of @prepare() may be
> used to indicate to the PM core to leave the device in runtime suspend
> if applicable.
>
> Maybe I'm missing something
You are. Please see device_prepare().
> but in the places where the direct_complete
> flag is calculated (e.g. in pci_dev_keep_suspended()) or where it's checked,
> I don't see that we're differentiating anywhere whether we're going through
> a suspend-to-RAM versus suspend-to-disk transition. So in the above snippet,
> the portion "If the transition is a suspend to memory or standby (that is,
> not related to hibernation)" seems wrong and should probably be removed.
>
> I know you're not touching this paragraph in the present commits, it's just
> something that caught my eye while going over the rendered output.
>
>
> Furthermore, the description for the ->freeze hook says:
>
> Analogous to @suspend(), but it should not enable the device to signal
> wakeup events or change its power state.
>
> However looking at the PCI core it looks like this constraint isn't
> satisfied, pci_dev_keep_suspended() (which gets called from ->prepare)
> disables PME only if:
>
> if (pm_runtime_suspended(dev) && pci_dev->current_state < PCI_D3cold &&
> !device_may_wakeup(dev))
> __pci_pme_active(pci_dev, false);
>
> Shouldn't this be something like:
>
> if (pm_runtime_suspended(dev) && pci_dev->current_state < PCI_D3cold &&
> (!device_may_wakeup(dev) || (system_entering_hibernation() &&
> system_state != SYSTEM_POWER_OFF)))
> __pci_pme_active(pci_dev, false);
>
> So that PME is disabled before entering the freeze phase for direct_complete
> devices, but not before entering the poweroff phase.
It should be safe to leave PME as is for the non-poweroff phases of hibernation,
which is why the code is the way it is.
Besides, this comment is about ->freeze, not about ->prepare. :-)
Thanks,
Rafael
prev parent reply other threads:[~2017-02-02 0:11 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-06 1:38 [PATCH 0/3] PM / docs: linux/pm.h kerneldocs update and conversion of two docs to reST Rafael J. Wysocki
2017-01-06 1:39 ` [Resend][PATCH 1/3] PM / core: Update kerneldoc comments in pm.h Rafael J. Wysocki
2017-01-06 1:41 ` [Resend][PATCH 2/3] PM / core / docs: Convert sleep states API document to reST Rafael J. Wysocki
2017-01-10 1:41 ` Lukas Wunner
2017-01-11 2:58 ` Rafael J. Wysocki
2017-01-06 1:44 ` [PATCH 3/3] PM / sleep / docs: Convert PM notifiers " Rafael J. Wysocki
2017-01-09 1:34 ` [PATCH 0/3] PM / docs: linux/pm.h kerneldocs update and conversion of two docs " Lukas Wunner
2017-02-02 0:06 ` Rafael J. Wysocki [this message]
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=21248446.r6y1HENFuA@aspire.rjw.lan \
--to=rjw@rjwysocki.net \
--cc=corbet@lwn.net \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=mchehab@osg.samsung.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).