On Thu, Apr 13, 2023 at 01:16:42PM +0300, Mika Westerberg wrote:
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5037,6 +5037,22 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
> }
> }
>
> + /*
> + * Everything above is handling the delays mandated by the PCIe r6.0
> + * sec 6.6.1.
> + *
> + * If the port supports active link reporting we now check one more
> + * time if the link is active and if not bail out early with the
> + * assumption that the device is not present anymore.
> + */
> + if (dev->link_active_reporting) {
> + u16 status;
> +
> + pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &status);
> + if (!(status & PCI_EXP_LNKSTA_DLLLA))
> + return -ENOTTY;
> + }
> +
> return pci_dev_wait(child, reset_type,
> PCIE_RESET_READY_POLL_MS - delay);
> }
Hm, shouldn't the added code live in the
if (pcie_get_speed_cap(dev) <= PCIE_SPEED_5_0GT)
branch? For the else branch (Gen3+ devices with > 5 GT/s),
we've already waited for the link to become active, so the
additional check seems superfluous. (But maybe I'm missing
something.)
I also note that this documentation change has been dropped
vis-à-vis v1 of the patch, not sure if that's intentional:
- * However, 100 ms is the minimum and the PCIe spec says the
- * software must allow at least 1s before it can determine that the
- * device that did not respond is a broken device. There is
- * evidence that 100 ms is not always enough, for example certain
- * Titan Ridge xHCI controller does not always respond to
- * configuration requests if we only wait for 100 ms (see
- * https://bugzilla.kernel.org/show_bug.cgi?id=203885).
+ * However, 100 ms is the minimum and the PCIe spec says the software
+ * must allow at least 1s before it can determine that the device that
+ * did not respond is a broken device. Also device can take longer than
+ * that to respond if it indicates so through Request Retry Status
+ * completions.
Thanks,
Lukas
