Re: [PATCH v6 05/11] PCI: PM: Disable device wakeups when halting system through S4 flow

[Bjorn Helgaas <helgaas@kernel.org>]

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:
> 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?

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

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

> Tested-by: Eric Naim <dnaim@cachyos.org>
> 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/
> ---
>  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?

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

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