Re: [PATCH] PCI/PM: Wait longer after reset when active link reporting is supported

[Mika Westerberg <mika.westerberg@linux.intel.com>]

Hi Bjorn,

On Wed, Mar 22, 2023 at 05:16:24PM -0500, Bjorn Helgaas wrote:
> On Tue, Mar 21, 2023 at 11:50:31AM +0200, Mika Westerberg wrote:
> > The PCIe spec prescribes that a device may take up to 1 second to
> > recover from reset and this same delay is prescribed when coming out of
> > D3cold (as that involves reset too). The device may extend this 1 second
> > delay through Request Retry Status completions and we accommondate for
> > that in Linux with 60 second cap, only in reset code path, not in resume
> > code path.
> > 
> > However, a device has surfaced, namely Intel Titan Ridge xHCI, which
> > requires longer delay also in the resume code path. For this reason make
> > the resume code path to use this same extended delay than with the reset
> > path but only after the link has come up (active link reporting is
> > supported) so that we do not wait longer time for devices that have
> > become permanently innaccessible during system sleep, e.g because they
> > have been removed.
> > 
> > While there move the two constants from the pci.h header into pci.c as
> > these are not used outside of that file anymore.
> > 
> > Reported-by: Chris Chiu <chris.chiu@canonical.com>
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216728
> > Cc: Lukas Wunner <lukas@wunner.de>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Lukas just added the "timeout" parameter with ac91e6980563 ("PCI:
> Unify delay handling for reset and resume"), so I'm going to look for
> his ack for this.

Of course :)

> After ac91e6980563, we called pci_bridge_wait_for_secondary_bus() with
> timeouts of either:
> 
>   60s for reset (pci_bridge_secondary_bus_reset() or
>       dpc_reset_link()), or
> 
>    1s for resume (pci_pm_resume_noirq() or pci_pm_runtime_resume() via
>       pci_pm_bridge_power_up_actions())
> 
> If I'm reading this right, the main changes of this patch are:
> 
>   - For slow links (<= 5 GT/s), we sleep 100ms, then previously waited
>     up to 1s (resume) or 60s (reset) for the device to be ready.  Now
>     we will wait a max of 1s for both resume and reset.
> 
>   - For fast links (> 5 GT/s) we wait up to 100ms for the link to come
>     up and fail if it does not.  If the link did come up in 100ms, we
>     previously waited up to 1s (resume) or 60s (reset).  Now we will
>     wait up to 60s for both resume and reset.
> 
> So this *reduces* the time we wait for slow links after reset, and
> *increases* the time for fast links after resume.  Right?

Yes, this is correct.