On Friday, September 27, 2019 11:02:02 AM CEST Daniel Drake wrote: > This delay is needed to fix resume from s2idle of the XHCI controller on > AMD Ryzen SoCs, where a 20ms delay is required (this will be quirked > in a followup patch), to avoid this failure: > > xhci_hcd 0000:03:00.4: WARN: xHC restore state timeout > xhci_hcd 0000:03:00.4: PCI post-resume error -110! > > The D3 delay is already being performed in a runtime resume from D3cold, > through the following sequence of events: > > pci_pm_runtime_resume > -> pci_restore_standard_config > -> pci_set_power_state(D0) > -> __pci_start_power_transition > -> pci_platform_power_transition > -> pci_update_current_state > > At this point, the device has been set to D0 at the platform level, > so pci_update_current_state() reads pmcsr and updates dev->current_state > to D3hot. Now when we reach pci_raw_set_power_state() the D3 delay will > be applied. > > However, the D3cold resume from s2idle path is somewhat different, and > we arrive at the same function without hitting pci_update_current_state() > along the way: > pci_pm_resume_noirq > -> pci_pm_default_resume_early > -> pci_power_up > -> pci_raw_set_power_state > > As dev->current_state is D3cold, the D3 delay is skipped and the XHCI > controllers fail to be powered up. > > Apply the D3 delay in the s2idle resume case too, in order to fix > USB functionality after resume. > > Link: http://lkml.kernel.org/r/CAD8Lp47Vh69gQjROYG69=waJgL7hs1PwnLonL9+27S_TcRhixA@xxxxxxxxxxxxxx > Signed-off-by: Daniel Drake <drake@xxxxxxxxxxxx> > --- > drivers/pci/pci.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index e7982af9a5d8..ab15fa5eda2c 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -883,7 +883,8 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) > * Mandatory power management transition delays; see PCI PM 1.1 > * 5.6.1 table 18 > */ > - if (state == PCI_D3hot || dev->current_state == PCI_D3hot) > + if (state == PCI_D3hot || dev->current_state == PCI_D3hot > + || dev->current_state == PCI_D3cold) > pci_dev_d3_sleep(dev); > else if (state == PCI_D2 || dev->current_state == PCI_D2) > udelay(PCI_PM_D2_DELAY); > So I think that we can use pci_restore_standard_config() in the system resume patch too, which should address the issue as well. Basically, there is no reason for the PM-runtime and system-wide resume code paths to be different in that respect. Something like this (untested): --- drivers/pci/pci-driver.c | 11 ++--------- drivers/pci/pci.c | 13 ------------- drivers/pci/pci.h | 1 - 3 files changed, 2 insertions(+), 23 deletions(-) Index: linux-pm/drivers/pci/pci-driver.c =================================================================== --- linux-pm.orig/drivers/pci/pci-driver.c +++ linux-pm/drivers/pci/pci-driver.c @@ -521,13 +521,6 @@ static int pci_restore_standard_config(s #ifdef CONFIG_PM_SLEEP -static void pci_pm_default_resume_early(struct pci_dev *pci_dev) -{ - pci_power_up(pci_dev); - pci_restore_state(pci_dev); - pci_pme_restore(pci_dev); -} - /* * Default "suspend" method for devices that have no driver provided suspend, * or not even a driver at all (second part). @@ -938,7 +931,7 @@ static int pci_pm_resume_noirq(struct de * pointless, so avoid doing that. */ if (!(pci_dev->skip_bus_pm && pm_suspend_no_platform())) - pci_pm_default_resume_early(pci_dev); + pci_restore_standard_config(pci_dev); pci_fixup_device(pci_fixup_resume_early, pci_dev); @@ -1213,7 +1206,7 @@ static int pci_pm_restore_noirq(struct d return error; } - pci_pm_default_resume_early(pci_dev); + pci_restore_standard_config(pci_dev); pci_fixup_device(pci_fixup_resume_early, pci_dev); if (pci_has_legacy_pm_support(pci_dev)) Index: linux-pm/drivers/pci/pci.c =================================================================== --- linux-pm.orig/drivers/pci/pci.c +++ linux-pm/drivers/pci/pci.c @@ -959,19 +959,6 @@ void pci_refresh_power_state(struct pci_ } /** - * pci_power_up - Put the given device into D0 forcibly - * @dev: PCI device to power up - */ -void pci_power_up(struct pci_dev *dev) -{ - if (platform_pci_power_manageable(dev)) - platform_pci_set_power_state(dev, PCI_D0); - - pci_raw_set_power_state(dev, PCI_D0); - pci_update_current_state(dev, PCI_D0); -} - -/** * pci_platform_power_transition - Use platform to change device power state * @dev: PCI device to handle. * @state: State to put the device into. Index: linux-pm/drivers/pci/pci.h =================================================================== --- linux-pm.orig/drivers/pci/pci.h +++ linux-pm/drivers/pci/pci.h @@ -85,7 +85,6 @@ struct pci_platform_pm_ops { int pci_set_platform_pm(const struct pci_platform_pm_ops *ops); void pci_update_current_state(struct pci_dev *dev, pci_power_t state); void pci_refresh_power_state(struct pci_dev *dev); -void pci_power_up(struct pci_dev *dev); void pci_disable_enabled_device(struct pci_dev *dev); int pci_finish_runtime_suspend(struct pci_dev *dev); void pcie_clear_root_pme_status(struct pci_dev *dev);
