From: Lukas Wunner <lukas@wunner.de>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
Yinghai Lu <yinghai@kernel.org>,
"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
Mika Westerberg <mika.westerberg@linux.intel.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] PCI: pciehp: Don't enable PME on runtime suspend
Date: Tue, 7 Feb 2017 07:26:59 +0100 [thread overview]
Message-ID: <20170207062659.GC791@wunner.de> (raw)
In-Reply-To: <1518967.ikNpzX2uft@aspire.rjw.lan>
On Mon, Feb 06, 2017 at 10:52:12PM +0100, Rafael J. Wysocki wrote:
> On Monday, February 06, 2017 10:20:41 PM Lukas Wunner wrote:
> > On Mon, Feb 06, 2017 at 11:54:05AM -0600, Bjorn Helgaas wrote:
> > > On Mon, Feb 06, 2017 at 06:54:37AM +0100, Lukas Wunner wrote:
> > > > Since commit 68db9bc81436 ("PCI: pciehp: Add runtime PM support for PCIe
> > > > hotplug ports") we runtime suspend a hotplug port to D3hot when all its
> > > > children are runtime suspended or none are present.
> > > >
> > > > When runtime suspending the port the PCI core automatically enables PME:
> > > > pci_pm_runtime_suspend()
> > > > pci_finish_runtime_suspend()
> > > > __pci_enable_wake()
> > > >
> > > > According to the PCI Express Base Specification, section 6.7.3.4:
> > > > "Note that PME and Hot-Plug Event interrupts (when both are
> > > > implemented) always share the same MSI or MSI-X vector [...]
> > > > If wake generation is required by the associated form factor
> > > > specification, a hot-plug capable Downstream Port must support
> > > > generation of a wakeup event (using the PME mechanism) on hotplug
> > > > events that occur when the system is in a sleep state or the Port
> > > > is in device state D1, D2, or D3Hot."
> > > >
> > > > Thus, if the port is runtime suspended even though it is still occupied,
> > > > it may immediately be woken by a PME interrupt.
> > >
> > > The spec goes on to say that a wakeup event should be generated when
> > > all three of these conditions occur:
> > >
> > > - status register for an enabled [hotplug] event transitions from
> > > not set to set
> > >
> > > - Port is in D1, D2, or D3hot,
> > >
> > > - PME_En is set
> > >
> > > I think you're saying that if we put a hotplug-capable port that
> > > controls an occupied slot into D3hot, the port may immediately
> > > generate a wakeup PME.
> > >
> > > What is the hotplug event that causes generation of this wakeup event?
> >
> > If you had read all e-mails in this thread or looked at the bugzilla
> > entry I've created, you wouldn't have to ask this question.
> >
> > I think it's disappointing that you're asking me to jump through
> > various hoops like creating a bugzilla entry, as well as threatening
> > to revert my patch, but are unwilling to even look at the bugzilla
> > entry or read the entire thread. It is equally disappointing that
> > the reporter of the regression was unwilling or unable to provide
> > dmesg output for both machines so that we've got no real idea what
> > we're dealing with.
> >
> > It's a Link Up event.
> >
> > Which doesn't make sense of course because per the spec PME is only
> > supposed to be signaled when the Link Status *changes* from 0 to 1,
>
> BTW, where exactly did you find this bit in the spec?
PCIe Base Spec, section 6.7.3.4:
"For form factors that support wake generation, a wakeup event must be
generated if all three of the following conditions occur:
* The status register for an enabled event transitions from not set to set
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* The Port is in device state D1, D2, or D3Hot, and
* The PME_En bit in the Port's Power Management Control/Status register is
set"
My point was that the Link Status shouldn't arbitrarily change from 0 to 1
after powering off the slot.
Best regards,
Lukas
next prev parent reply other threads:[~2017-02-07 6:25 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-06 5:54 [PATCH v2] PCI: pciehp: Don't enable PME on runtime suspend Lukas Wunner
2017-02-06 11:56 ` Rafael J. Wysocki
2017-02-06 17:54 ` Bjorn Helgaas
2017-02-06 21:20 ` Lukas Wunner
2017-02-06 21:27 ` Rafael J. Wysocki
2017-02-06 21:52 ` Rafael J. Wysocki
2017-02-07 6:26 ` Lukas Wunner [this message]
2017-02-07 16:15 ` Rafael J. Wysocki
2017-02-06 22:15 ` Bjorn Helgaas
2017-02-07 6:21 ` Lukas Wunner
2017-02-07 16:04 ` Rafael J. Wysocki
2017-02-08 4:23 ` Lukas Wunner
2017-02-08 12:03 ` Rafael J. Wysocki
2017-02-08 18:04 ` Bjorn Helgaas
2017-02-08 17:57 ` Bjorn Helgaas
2017-02-09 4:32 ` Lukas Wunner
2017-02-12 14:57 ` Lukas Wunner
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=20170207062659.GC791@wunner.de \
--to=lukas@wunner.de \
--cc=helgaas@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--cc=rafael.j.wysocki@intel.com \
--cc=rjw@rjwysocki.net \
--cc=yinghai@kernel.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 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).