Hi, On Tue, Apr 04, 2023 at 04:36:55PM -0500, Bjorn Helgaas wrote: > Hi Mika, > > I need some help because I have a hard time applying sec 6.6.1. > > On Tue, Apr 04, 2023 at 08:27:14AM +0300, Mika Westerberg wrote: > > In order speed up reset and resume time of devices behind slow links, > > decrease the wait time to 1s. This should give enough time for them to > > respond. > > Is there some spec language behind this? In sec 6.6.1, I see that all > devices "must be able to receive a Configuration Request and return a > Successful Completion". > > A preceding rule says devices with slow links must enter LTSSM Detect > within 20ms, but I don't see a direct connection from that to a > shorter wait time. I think this (PCIe 5.0 p. 553): "Following a Conventional Reset of a device, within 1.0 s the device must be able to receive a Configuration Request and return a Successful Completion if the Request is valid." > > While doing this, instead of looking at the speed we check if > > the port supports active link reporting. > > Why check dev->link_active_reporting (i.e., PCI_EXP_LNKCAP_DLLLARC) > instead of the link speed described by the spec? This is what Sathyanarayanan suggested in the previous version comments. > DLLLARC is required for fast links, but it's not prohibited for slower > links and it's *required* for hotplug ports with slow links, so > dev->link_active_reporting is not completely determined by link speed. > > IIUC, the current code basically has these cases: > > 1) All devices on secondary bus have zero D3cold delay: > return immediately; no delay at all > > 2) Non-PCIe bridge: > sleep 1000ms > sleep 100ms (typical, depends on downstream devices) > > 3) Speed <= 5 GT/s: > sleep 100ms (typical) > sleep up to 59.9s (typical) waiting for valid config read > > 4) Speed > 5 GT/s (DLLLARC required): > sleep 20ms > sleep up to 1000ms waiting for DLLLA > sleep 100ms (typical) > sleep up to 59.9s (typical) waiting for valid config read > > This patch changes cases 3) and 4) to: > > 3) DLLLARC not supported: > sleep 100ms (typical) > sleep up to 1.0s (typical) waiting for valid config read > > 4) DLLLARC supported: > no change in wait times, ~60s total > > And testing dev->link_active_reporting instead of speed means slow > hotplug ports (and possibly other slow ports that implement DLLLARC) > that previously were in case 3) will now be in case 4). Yes, and we do that because if the device gets unplugged while we were in susppend we don't want to wait for the total 60s for it to become ready. That's what the DLLLARC can tell us (for ports that support it). For the ports that do not we want to give the device some time but not to wait for that 60s so we wait for the 1s as the "minimum" requirement from the spec before it can be determined "broken".
