Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 8 Apr 2022 13:52:10 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc:     Linux PCI <linux-pci@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Linux ACPI <linux-acpi@vger.kernel.org>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        Linux PM <linux-pm@vger.kernel.org>
Subject: Re: [PATCH v2] PCI: PM: Power up all devices during runtime resume
Message-ID: <20220408185210.GA339319@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2652115.mvXUDI8C0e@kreacher>
Precedence: bulk

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