| Message ID | ec4761d5e7f7a3258d9a37284571b9eb1c2e0a4a.1672511017.git.lukas@wunner.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI reset delay fixes | expand |
On 12/31/22 10:33 AM, Lukas Wunner wrote: > If a PCI bridge is suspended to D3cold upon entering system sleep, > resuming it entails a Fundamental Reset per PCIe r6.0 sec 5.8. > > The delay prescribed after a Fundamental Reset in PCIe r6.0 sec 6.6.1 > is sought to be observed by: > > pci_pm_resume_noirq() > pci_pm_bridge_power_up_actions() > pci_bridge_wait_for_secondary_bus() > > However, pci_bridge_wait_for_secondary_bus() bails out if the bridge_d3 > flag is not set. That flag indicates whether a bridge is allowed to > suspend to D3cold at *runtime*. > > Hence *no* delay is observed on resume from system sleep if runtime > D3cold is forbidden. That doesn't make any sense, so drop the bridge_d3 > check from pci_bridge_wait_for_secondary_bus(). > > The purpose of the bridge_d3 check was probably to avoid delays if a > bridge remained in D0 during suspend. However the sole caller of > pci_bridge_wait_for_secondary_bus(), pci_pm_bridge_power_up_actions(), > is only invoked if the previous power state was D3cold. Hence the > additional bridge_d3 check seems superfluous. > > Fixes: ad9001f2f411 ("PCI/PM: Add missing link delays required by the PCIe spec") > Tested-by: Ravi Kishore Koppuravuri <ravi.kishore.koppuravuri@intel.com> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > Cc: stable@vger.kernel.org # v5.5+ > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > --- Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > drivers/pci/pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index fba95486caaf..f43f3e84f634 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -4964,7 +4964,7 @@ void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev) > if (pci_dev_is_disconnected(dev)) > return; > > - if (!pci_is_bridge(dev) || !dev->bridge_d3) > + if (!pci_is_bridge(dev)) > return; > > down_read(&pci_bus_sem);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index fba95486caaf..f43f3e84f634 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4964,7 +4964,7 @@ void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev) if (pci_dev_is_disconnected(dev)) return; - if (!pci_is_bridge(dev) || !dev->bridge_d3) + if (!pci_is_bridge(dev)) return; down_read(&pci_bus_sem);
If a PCI bridge is suspended to D3cold upon entering system sleep, resuming it entails a Fundamental Reset per PCIe r6.0 sec 5.8. The delay prescribed after a Fundamental Reset in PCIe r6.0 sec 6.6.1 is sought to be observed by: pci_pm_resume_noirq() pci_pm_bridge_power_up_actions() pci_bridge_wait_for_secondary_bus() However, pci_bridge_wait_for_secondary_bus() bails out if the bridge_d3 flag is not set. That flag indicates whether a bridge is allowed to suspend to D3cold at *runtime*. Hence *no* delay is observed on resume from system sleep if runtime D3cold is forbidden. That doesn't make any sense, so drop the bridge_d3 check from pci_bridge_wait_for_secondary_bus(). The purpose of the bridge_d3 check was probably to avoid delays if a bridge remained in D0 during suspend. However the sole caller of pci_bridge_wait_for_secondary_bus(), pci_pm_bridge_power_up_actions(), is only invoked if the previous power state was D3cold. Hence the additional bridge_d3 check seems superfluous. Fixes: ad9001f2f411 ("PCI/PM: Add missing link delays required by the PCIe spec") Tested-by: Ravi Kishore Koppuravuri <ravi.kishore.koppuravuri@intel.com> Signed-off-by: Lukas Wunner <lukas@wunner.de> Cc: stable@vger.kernel.org # v5.5+ Cc: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)