On 9/8/2025 4:34 PM, Bjorn Helgaas wrote:

In subject, s|PCI: PM:|PCI/PM:| to follow previous practice.

👍

On Sun, Aug 17, 2025 at 09:00:55PM -0500, Mario Limonciello (AMD) wrote:

quoted

PCI devices can be programmed as a wakeup source from low power states
by sysfs.  However when using the S4 flow to go into S5 these wakeup
sources should be disabled to avoid what users would perceive as
spurious wakeup events.

Is the "can be programmed vis sysfs" part relevant here?

No, I can drop that part of the sentence.

I think S4 and S5 are ACPI sleep states not applicable to all
platforms.  Is it relevant that we got here via ACPI?

But a non-ACPI system would still run the exact same callbacks for 
hibernation wouldn't it?

In general you can 's,S4,hibernation,; s,s5,shutdown,' and it would be 
accurate.

I assume non-ACPI systems can also exercise this path.  Is there a way
to describe this scenario in a way that would apply to all systems?

I'm not sure what "using the S4 flow to go in to S5" means.

It means to run the hibernate related device callbacks instead of the 
shutdown related device callbacks.

I suppose anywhere in the series that is not ACPI specific (besides this 
patch) I should clarify this as well.

It would be nice to have a spec reference or some sort of rationale
for the requirement to disable all wakeup sources in SYSTEM_HALT and
SYSTEM_POWER_OFF.

I didn't observe it in the PCI path, but I did see that in USB that if 
you leave them enabled they stay enabled when the system is shutdown.
For example if a USB mouse was connected and a wakeup source it could 
wakeup from S5 too.

So this patch was aiming for congruence between the two subsystems.

I would hypothesize that means that a dock could wake a system from S5 
on a hotplug event, which I don't believe is intended behavior.

quoted

Tested-by: Eric Naim <redacted>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v5:
  * Re-order
  * Add tags
v4:
  * https://lore.kernel.org/linux-pci/20250616175019.3471583-1-superm1@kernel.org/ (local)
---
  drivers/pci/pci-driver.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 63665240ae87f..f201d298d7173 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c

@@ -1139,6 +1139,10 @@ static int pci_pm_poweroff(struct device *dev)
  	struct pci_dev *pci_dev = to_pci_dev(dev);
  	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
  
+	if (device_may_wakeup(dev) &&
+	    (system_state == SYSTEM_HALT || system_state == SYSTEM_POWER_OFF))
+		device_set_wakeup_enable(dev, false);

I guess the suggestion is that we can't wake up at all from
SYSTEM_HALT or SYSTEM_POWER_OFF?  Would both be considered S5?

Correct.

Does this mean we need a physical power button push to start up again?
I guess ACPI r6.5, sec 16.1.5 kind of suggests that: "hardware does
allow a transition to S0 due to power button press or a Remote Start."

Correct.

quoted

  	if (pci_has_legacy_pm_support(pci_dev))
  		return pci_legacy_suspend(dev, PMSG_HIBERNATE);
  
-- 
2.43.0