Re: [PATCH v2] PCI: PM: Power up all devices during runtime resume

Bjorn Helgaas
2022-04-08T18:52:16Z

On Fri, Apr 08, 2022 at 08:29:01PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: [PATCH] PCI: PM: Power up all devices during runtime resume
>
> Currently, endpoint devices may not be powered up entirely during
> runtime resume that follows a D3hot -> D0 transition of the parent
> bridge.
>
> Namely, even if the power state of an endpoint device, as indicated
> by its PCI_PM_CTRL register, is D0 after powering up its parent
> bridge, it may be still necessary to bring its ACPI companion into
> D0 and that should be done before accessing it. However, the current
> code assumes that reading the PCI_PM_CTRL register is sufficient to
> establish the endpoint device's power state, which may lead to
> problems.
>
> Address that by forcing a power-up of all PCI devices, including the
> platform firmware part of it, during runtime resume.
>
> Link: https://lore.kernel.org/linux-pm/11967527.O9o76ZdvQC@kreacher
> Fixes: 5775b843a619 ("PCI: Restore config space on runtime resume despite being unbound")
> Reported-by: Abhishek Sahu <abhsahu@nvidia.com>
> Tested-by: Abhishek Sahu <abhsahu@nvidia.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

I replaced the v1 patch with this one on pci/pm, thanks!

> ---
>
> v1 -> v2:
> * Move pci_pm_default_resume_early() away from #ifdef CONFIG_PM_SLEEP.
> * Add R-by from Mika.
>
> ---
> drivers/pci/pci-driver.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> Index: linux-pm/drivers/pci/pci-driver.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci-driver.c
> +++ linux-pm/drivers/pci/pci-driver.c
> @@ -551,10 +551,6 @@ static void pci_pm_default_resume(struct
> pci_enable_wake(pci_dev, PCI_D0, false);
> }
>
> -#endif
> -
> -#ifdef CONFIG_PM_SLEEP
> -
> static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
> {
> pci_power_up(pci_dev);
> @@ -563,6 +559,10 @@ static void pci_pm_default_resume_early(
> pci_pme_restore(pci_dev);
> }
>
> +#endif
> +
> +#ifdef CONFIG_PM_SLEEP
> +
> /*
> * Default "suspend" method for devices that have no driver provided suspend,
> * or not even a driver at all (second part).
> @@ -1312,7 +1312,7 @@ static int pci_pm_runtime_resume(struct
> * to a driver because although we left it in D0, it may have gone to
> * D3cold when the bridge above it runtime suspended.
> */
> - pci_restore_standard_config(pci_dev);
> + pci_pm_default_resume_early(pci_dev);
>
> if (!pci_dev->driver)
> return 0;
>
>
>