LKML Archive on
help / color / mirror / Atom feed
From: Kai-Heng Feng <>
To: "Rafael J. Wysocki" <>
Cc: Bjorn Helgaas <>,
	Alan Stern <>,
	Rafael Wysocki <>,, LKML <>,
	Mathias Nyman <>,
Subject: Re: [PATCH] PCI / PM: Don't runtime suspend when device only supports wakeup from D0
Date: Fri, 5 Jul 2019 21:51:39 +0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <2067449.jKPth8Qelp@kreacher>

at 17:39, Rafael J. Wysocki <> wrote:

> On Friday, July 5, 2019 9:02:01 AM CEST Kai-Heng Feng wrote:
>> at 19:57, Bjorn Helgaas <> wrote:
>>> On Mon, May 27, 2019 at 11:57:47AM -0500, Bjorn Helgaas wrote:
>>>> On Thu, May 23, 2019 at 12:39:23PM +0800, Kai-Heng Feng wrote:
>>>>> at 04:52, Bjorn Helgaas <> wrote:
>>>>>> On Wed, May 22, 2019 at 02:39:56PM -0400, Alan Stern wrote:
>>>>>>> On Wed, 22 May 2019, Bjorn Helgaas wrote:
>>>>>>>> On Wed, May 22, 2019 at 11:46:25PM +0800, Kai Heng Feng wrote:
>>>>>>>>>> On May 22, 2019, at 9:48 PM, Bjorn Helgaas <>
>>>>>>>>>> wrote:
>>>>>>>>>> On Wed, May 22, 2019 at 11:42:14AM +0800, Kai Heng Feng wrote:
>>>>>>>>>>> at 6:23 AM, Bjorn Helgaas <> wrote:
>>>>>>>>>>>> On Wed, May 22, 2019 at 12:31:04AM +0800, Kai-Heng Feng wrote:
>>>>>>>>>>>>> There's an xHC device that doesn't wake when
>>>>>>>>>>>>> a USB device gets plugged
>>>>>>>>>>>>> to its USB port. The driver's own runtime
>>>>>>>>>>>>> suspend callback was called,
>>>>>>>>>>>>> PME signaling was enabled, but it stays at PCI D0.
>>>>>>>>>> ...
>>>>>>>>>> And I guess this patch basically means we wouldn't call
>>>>>>>>>> the driver's suspend callback if we're merely going to
>>>>>>>>>> stay at D0, so the driver would have no idea anything
>>>>>>>>>> happened.  That might match Documentation/power/pci.txt
>>>>>>>>>> better, because it suggests that the suspend callback is
>>>>>>>>>> related to putting a device in a low-power state, and D0
>>>>>>>>>> is not a low-power state.
>>>>>>>>> Yes, the patch is to let the device stay at D0 and don’t run
>>>>>>>>> driver’s own runtime suspend routine.
>>>>>>>>> I guess I’ll just proceed to send a V2 with updated commit message?
>>>>>>>> Now that I understand what "runtime suspended to D0" means, help me
>>>>>>>> understand what's actually wrong.
>>>>>>> Kai's point is that the xhci-hcd driver thinks the device is now
>>>>>>> in runtime suspend, because the runtime_suspend method has been
>>>>>>> executed.  But in fact the device is still in D0, and as a
>>>>>>> result, PME signalling may not work correctly.
>>>>>> The device claims to be able to signal PME from D0 (this is from the
>>>>>> lspci
>>>>>> in
>>>>>>   00:10.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI Controller (rev 20) (prog-if 30 [XHCI])
>>>>>>     Capabilities: [50] Power Management version 3
>>>>>>       Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
>>>>>> From the xHCI spec r1.0, sec, it looks like a connect
>>>>>> detected while in D0 should assert PME# if enabled (and WCE is
>>>>>> set).
>>>>> I think section is about S3 wake up, no S0 we are
>>>>> discussing here.
>>>> S0 and S3 are system-level ideas and have no meaning to an individual
>>>> PCI device.  The xHC is a PCI device and can't tell whether the system
>>>> as a whole is in S0 or S3.  If a PCI device claims to be able to
>>>> generate PME while in D0, that applies regardless of the system state.
>>>> xHCI r1.0, sec A.1 says "The host controller should be capable of
>>>> asserting PME# when in any supported device state."  In sec 4.19.2,
>>>> Figure 42 says PME# should be asserted whenever PMCSR.PME_En=1 and
>>>> WCE=1 and a connection is detected.
>>>> Figure 42 also shows that CSC (Connect Status Change) and related bits
>>>> feed into Port Status Change Event Generation.  So I assume the xhci
>>>> driver normally detects connect/disconnect via CSC, but the runtime
>>>> suspend method makes it use PME# instead?
>>>> And the way your patch works is by avoiding that xhci runtime suspend
>>>> method, so it *always* uses CSC and never uses PME#?  If that's the
>>>> case, we're just papering over a problem without really understanding
>>>> it.
>>>> I'm wondering if this platform has a firmware defect.  Here's my
>>>> thinking.  The xHC is a Root Complex Integrated Endpoint, so its PME
>>>> signaling is a little unusual.
>>>> The typical scenario is that a PCIe device is below a Root Port.  In
>>>> that case, it would send a PME Message upstream to the Root Port.  Per
>>>> PCIe r4.0, sec 6.1.6, when configured for native PME support (for ACPI
>>>> systems, I assume this means "when firmware has granted PME control to
>>>> the OS via _OSC"), the Root Port would generate a normal PCI INTx or
>>>> MSI interrupt:
>>>>   PCI Express-aware software can enable a mode where the Root Complex
>>>>   signals PME via an interrupt. When configured for native PME
>>>>   support, a Root Port receives the PME Message and sets the PME
>>>>   Status bit in its Root Status register. If software has set the PME
>>>>   Interrupt Enable bit in the Root Control register to 1b, the Root
>>>>   Port then generates an interrupt.
>>>> But on this platform the xHC is a Root Complex Integrated Endpoint, so
>>>> there is no Root Port upstream from it, and that mechanism can't be
>>>> used.  Per PCIe r4.0, sec, RCiEPs signal PME via "the same
>>>> mechanism as PCI systems" or via Root Complex Event Collectors:
>>>>   An RCiEP must signal PME and error conditions through the same
>>>>   mechanisms used on PCI systems. If a Root Complex Event Collector is
>>>>   implemented, an RCiEP may optionally signal PME and error conditions
>>>>   through a Root Complex Event Collector.
>>>> This platform has no Root Complex Event Collectors, so the xHC should
>>>> signal PME via the same mechanism as PCI systems, i.e., asserting a
>>>> PME# signal.  I think this means the OS cannot use native PCIe PME
>>>> control because it doesn't know what interrupt PME# is connected to.
>>>> The PCI Firmware Spec r3.2, sec 4.5.1 (also quoted in ACPI v6.2, sec
>>>>, says:
>>>>   PCI Express Native Power Management Events control
>>>>   The firmware sets this bit to 1 to grant control over PCI Express
>>>>   native power management event interrupts (PMEs). If firmware
>>>>   allows the operating system control of this feature, then in the
>>>>   context of the _OSC method, it must ensure that all PMEs are
>>>>   routed to root port interrupts as described in the PCI Express
>>>>   Base Specification.
>>>> This platform cannot route all PMEs to Root Port interrupts because
>>>> the xHC RCiEP cannot report PME via a Root Port, so I think its _OSC
>>>> method should not grant control of PCIe Native Power Management Events
>>>> to the OS, and I think that would mean we have to use the ACPI
>>>> mechanism for PME on this platform.
>>>> Can you confirm or deny any of this line of reasoning?  I'm wondering
>>>> if there's something wrong with the platform's _OSC, so Linux thinks
>>>> it can use native PME, but that doesn't work for this device.
>>>>> It’s a platform in development so the name can’t be disclosed.
>>>> Please attach a complete dmesg log to the bugzilla.  You can remove
>>>> identifying details like the platform name, but I want to see the
>>>> results of the _OSC negotiation.
>>> Thanks for the dmesg log
>>> (  It shows:
>>>   acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI HPX-Type3]
>>>   acpi PNP0A08:00: _OSC: platform does not support [SHPCHotplug LTR]
>>>   acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME AER PCIeCapability]
>>> I think it is incorrect for the platform to give the OS native control
>>> over PME because the OS has no way to know how the RCiEP PMEs are
>>> routed.  But it would be interesting to know how BIOSes on other
>>> platforms with RCiEPs handle this, and I did post a question to the
>>> PCI-SIG to see if there's any guidance there.
>> Is there any update from PCI-SIG?
>> I really think we don’t need wakeup capability in D0 because D0 is a
>> working state.
> Well, in theory, devices may stay in D0 over suspend-to-idle and they may  
> need to
> signal wakeup then.  Using PME for that would be kind of handy (if it  
> worked) as it
> would allow special handling of in-band IRQs to be avoided in that case.

That makes sense but doesn’t apply to this case.
This patch only avoids D0 runtime suspend, suspend-to-idle will call  
system-wide suspend routine which still enables D0 PME.

It’ll be great if you can review my v3 patch here:


  reply	other threads:[~2019-07-05 13:51 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-21 16:31 Kai-Heng Feng
2019-05-21 22:23 ` Bjorn Helgaas
2019-05-22  3:42   ` Kai Heng Feng
2019-05-22 13:48     ` Bjorn Helgaas
2019-05-22 15:46       ` Kai Heng Feng
2019-05-22 18:11         ` Bjorn Helgaas
2019-05-22 18:39           ` Alan Stern
2019-05-22 18:53             ` Lukas Wunner
2019-05-22 19:05               ` Kai Heng Feng
2019-05-22 20:52             ` Bjorn Helgaas
2019-05-23  4:39               ` Kai-Heng Feng
2019-05-27 16:57                 ` Bjorn Helgaas
2019-06-05 11:57                   ` Bjorn Helgaas
2019-07-05  7:02                     ` Kai-Heng Feng
2019-07-05  9:39                       ` Rafael J. Wysocki
2019-07-05 13:51                         ` Kai-Heng Feng [this message]
2019-07-09 13:45                       ` Bjorn Helgaas
2019-09-02 13:47                         ` Kai-Heng Feng

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] PCI / PM: Don'\''t runtime suspend when device only supports wakeup from D0' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).