* Re: [PATCH v1] PCI: PM: Power up all devices during runtime resume
2022-04-06 19:00 [PATCH v1] PCI: PM: Power up all devices during runtime resume Rafael J. Wysocki
@ 2022-04-07 8:50 ` Mika Westerberg
2022-04-07 15:49 ` Bjorn Helgaas
2022-04-08 18:29 ` [PATCH v2] " Rafael J. Wysocki
2 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2022-04-07 8:50 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PCI, LKML, Linux ACPI, Bjorn Helgaas, Linux PM
On Wed, Apr 06, 2022 at 09:00:52PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> 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>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] PCI: PM: Power up all devices during runtime resume
2022-04-06 19:00 [PATCH v1] PCI: PM: Power up all devices during runtime resume Rafael J. Wysocki
2022-04-07 8:50 ` Mika Westerberg
@ 2022-04-07 15:49 ` Bjorn Helgaas
2022-04-08 18:21 ` Rafael J. Wysocki
2022-04-08 18:29 ` [PATCH v2] " Rafael J. Wysocki
2 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2022-04-07 15:49 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PCI, LKML, Linux ACPI, Mika Westerberg, Linux PM
On Wed, Apr 06, 2022 at 09:00:52PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> 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>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Applied with Mika's reviewed-by to pci/pm for v5.19, thanks!
> ---
> drivers/pci/pci-driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-pm/drivers/pci/pci-driver.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci-driver.c
> +++ linux-pm/drivers/pci/pci-driver.c
> @@ -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;
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] PCI: PM: Power up all devices during runtime resume
2022-04-07 15:49 ` Bjorn Helgaas
@ 2022-04-08 18:21 ` Rafael J. Wysocki
0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2022-04-08 18:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rafael J. Wysocki, Linux PCI, LKML, Linux ACPI, Mika Westerberg,
Linux PM
On Thu, Apr 7, 2022 at 5:49 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Wed, Apr 06, 2022 at 09:00:52PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > 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>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Applied with Mika's reviewed-by to pci/pm for v5.19, thanks!
This doesn't compile when CONFIG_PM_SLEEP in uset, sorry about this.
I'll send a v2, but if you prefer an incremental fix, please let me know.
>
> > ---
> > drivers/pci/pci-driver.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Index: linux-pm/drivers/pci/pci-driver.c
> > ===================================================================
> > --- linux-pm.orig/drivers/pci/pci-driver.c
> > +++ linux-pm/drivers/pci/pci-driver.c
> > @@ -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;
> >
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] PCI: PM: Power up all devices during runtime resume
2022-04-06 19:00 [PATCH v1] PCI: PM: Power up all devices during runtime resume Rafael J. Wysocki
2022-04-07 8:50 ` Mika Westerberg
2022-04-07 15:49 ` Bjorn Helgaas
@ 2022-04-08 18:29 ` Rafael J. Wysocki
2022-04-08 18:52 ` Bjorn Helgaas
2 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2022-04-08 18:29 UTC (permalink / raw)
To: Linux PCI, Bjorn Helgaas; +Cc: LKML, Linux ACPI, Mika Westerberg, Linux PM
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>
---
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;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] PCI: PM: Power up all devices during runtime resume
2022-04-08 18:29 ` [PATCH v2] " Rafael J. Wysocki
@ 2022-04-08 18:52 ` Bjorn Helgaas
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2022-04-08 18:52 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PCI, LKML, Linux ACPI, Mika Westerberg, Linux PM
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;
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread