From 73cb3a35f94db723c0211ad099bce55b2155e3f0 Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Tue, 23 Apr 2024 16:08:19 +0300 Subject: PCI: Wait for Link Training==0 before starting Link retrain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes were made in link retraining logic independent of each other. The commit e7e39756363a ("PCI/ASPM: Avoid link retraining race") added a check to pcie_retrain_link() to ensure no Link Training is currently active to address the Implementation Note in PCIe r6.1 sec 7.5.3.7. At that time pcie_wait_for_retrain() only checked for the Link Training (LT) bit being cleared. The commit 680e9c47a229 ("PCI: Add support for polling DLLLA to pcie_retrain_link()") generalized pcie_wait_for_retrain() into pcie_wait_for_link_status() which can wait either for LT or the Data Link Layer Link Active (DLLLA) bit with 'use_lt' argument and supporting waiting for either cleared or set using 'active' argument. In the merge commit 1abb47390350 ("Merge branch 'pci/enumeration'"), those two divergent branches converged. The merge changed LT bit checking added in the commit e7e39756363a ("PCI/ASPM: Avoid link retraining race") to now wait for completion of any ongoing Link Training using DLLLA bit being set if 'use_lt' is false. When 'use_lt' is false, the pseudo-code steps of what occurs in pcie_retrain_link(): 1. Wait for DLLLA==1 2. Trigger link to retrain 3. Wait for DLLLA==1 Step 3 waits for the link to come up from the retraining triggered by Step 2. As Step 1 is supposed to wait for any ongoing retraining to end, using DLLLA also for it does not make sense because link training being active is still indicated using LT bit, not with DLLLA. Correct the pcie_wait_for_link_status() parameters in Step 1 to only wait for LT==0 to ensure there is no ongoing Link Training. This only impacts the Target Speed quirk, which is the only case where waiting for DLLLA bit is used. It currently works in the problematic case by means of link training getting initiated by hardware repeatedly and respecting the new link parameters set by the caller, which then make training succeed and bring the link up, setting DLLLA and causing pcie_wait_for_link_status() to return success. We are not supposed to rely on luck and need to make sure that LT transitioned through the inactive state though before we initiate link training by hand via RL (Retrain Link) bit. Fixes: 1abb47390350 ("Merge branch 'pci/enumeration'") Link: https://lore.kernel.org/r/20240423130820.43824-1-ilpo.jarvinen@linux.intel.com Signed-off-by: Ilpo Järvinen Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e5f243dd4288..70b8c87055cb 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4629,7 +4629,7 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt) * avoid LTSSM race as recommended in Implementation Note at the * end of PCIe r6.0.1 sec 7.5.3.7. */ - rc = pcie_wait_for_link_status(pdev, use_lt, !use_lt); + rc = pcie_wait_for_link_status(pdev, true, false); if (rc) return rc; -- cgit From cdc6c4abcb313be1b7118b6e86eb99a85a626578 Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Tue, 23 Apr 2024 16:08:20 +0300 Subject: PCI: Clarify intent of LT wait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clarify the comment relating to the LT wait and the purpose of the check that implements the implementation note in PCIe r6.1 sec 7.5.3.7. Suggested-by: Maciej W. Rozycki Link: https://lore.kernel.org/r/20240423130820.43824-2-ilpo.jarvinen@linux.intel.com Signed-off-by: Ilpo Järvinen Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 70b8c87055cb..5a25facb3ce7 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4625,9 +4625,10 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt) /* * Ensure the updated LNKCTL parameters are used during link - * training by checking that there is no ongoing link training to - * avoid LTSSM race as recommended in Implementation Note at the - * end of PCIe r6.0.1 sec 7.5.3.7. + * training by checking that there is no ongoing link training that + * may have started before link parameters were changed, so as to + * avoid LTSSM race as recommended in Implementation Note at the end + * of PCIe r6.1 sec 7.5.3.7. */ rc = pcie_wait_for_link_status(pdev, true, false); if (rc) -- cgit From 844177a80753fc173131f3e591124c8dcbc89812 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sat, 23 Mar 2024 18:16:36 +0100 Subject: PCI: Remove unused pci_enable_device_io() After the last user was removed, remove this PCI core function. It's very unlikely that we'll see a new device requiring io space access, even though memory space access is supported. Link: https://lore.kernel.org/r/213ebf62-53a3-42b7-8518-ecd5cd6d6b08@gmail.com Signed-off-by: Heiner Kallweit Signed-off-by: Bjorn Helgaas Reviewed-by: Damien Le Moal --- drivers/pci/pci.c | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 5a25facb3ce7..d3fabfca7e6f 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2110,20 +2110,6 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags) return err; } -/** - * pci_enable_device_io - Initialize a device for use with IO space - * @dev: PCI device to be initialized - * - * Initialize device before it's used by a driver. Ask low-level code - * to enable I/O resources. Wake up the device if it was suspended. - * Beware, this function can fail. - */ -int pci_enable_device_io(struct pci_dev *dev) -{ - return pci_enable_device_flags(dev, IORESOURCE_IO); -} -EXPORT_SYMBOL(pci_enable_device_io); - /** * pci_enable_device_mem - Initialize a device for use with Memory space * @dev: PCI device to be initialized -- cgit From 6613443ffc49d03e27f0404978f685c4eac43fba Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Thu, 8 Feb 2024 15:23:21 +0200 Subject: PCI: Do not wait for disconnected devices when resuming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On runtime resume, pci_dev_wait() is called: pci_pm_runtime_resume() pci_pm_bridge_power_up_actions() pci_bridge_wait_for_secondary_bus() pci_dev_wait() While a device is runtime suspended along with its PCI hierarchy, the device could get disconnected. In such case, the link will not come up no matter how long pci_dev_wait() waits for it. Besides the above mentioned case, there could be other ways to get the device disconnected while pci_dev_wait() is waiting for the link to come up. Make pci_dev_wait() exit if the device is already disconnected to avoid unnecessary delay. The use cases of pci_dev_wait() boil down to two: 1. Waiting for the device after reset 2. pci_bridge_wait_for_secondary_bus() The callers in both cases seem to benefit from propagating the disconnection as error even if device disconnection would be more analoguous to the case where there is no device in the first place which return 0 from pci_dev_wait(). In the case 2, it results in unnecessary marking of the devices disconnected again but that is just harmless extra work. Also make sure compiler does not become too clever with dev->error_state and use READ_ONCE() to force a fetch for the up-to-date value. Link: https://lore.kernel.org/r/20240208132322.4811-1-ilpo.jarvinen@linux.intel.com Reported-by: Mika Westerberg Tested-by: Mika Westerberg Signed-off-by: Ilpo Järvinen Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d3fabfca7e6f..a9848316c623 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1277,6 +1277,11 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout) for (;;) { u32 id; + if (pci_dev_is_disconnected(dev)) { + pci_dbg(dev, "disconnected; not waiting\n"); + return -ENOTTY; + } + pci_read_config_dword(dev, PCI_COMMAND, &id); if (!PCI_POSSIBLE_ERROR(id)) break; -- cgit