Skip to content

Commit

Permalink
Revert "PCI: Add missing link delays required by the PCIe spec"
Browse files Browse the repository at this point in the history 
Commit c2bf1fc ("PCI: Add missing link delays required by the PCIe
spec") turned out causing issues with some systems either by making them
unresponsive or slowing down runtime and system wide resume of PCIe
devices. While root cause for the unresponsiveness is still under
investigation given the amount of issues reported better to revert it
for now.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204413
Link: https://lore.kernel.org/linux-pci/SL2P216MB01878BBCD75F21D882AEEA2880C60@SL2P216MB0187.KORP216.PROD.OUTLOOK.COM/
Link: https://lore.kernel.org/linux-pci/2857501d-c167-547d-c57d-d5d24ea1f1dc@molgen.mpg.de/
Reported-by: Matthias Andree <matthias.andree@gmx.de>
Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reported-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  • Loading branch information
Mika Westerberg authored and Rafael J. Wysocki committed Aug 7, 2019
1 parent e21a712 commit 0617bde
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 86 deletions.
29 changes: 10 additions & 19 deletions drivers/pci/pci.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1025,10 +1025,15 @@ static void __pci_start_power_transition(struct pci_dev *dev, pci_power_t state)
if (state == PCI_D0) {
pci_platform_power_transition(dev, PCI_D0);
/*
* Mandatory power management transition delays are
* handled in the PCIe portdrv resume hooks.
* Mandatory power management transition delays, see
* PCI Express Base Specification Revision 2.0 Section
* 6.6.1: Conventional Reset. Do not delay for
* devices powered on/off by corresponding bridge,
* because have already delayed for the bridge.
*/
if (dev->runtime_d3cold) {
if (dev->d3cold_delay && !dev->imm_ready)
msleep(dev->d3cold_delay);
/*
* When powering on a bridge from D3cold, the
* whole hierarchy may be powered on into
Expand Down Expand Up @@ -4602,16 +4607,14 @@ static int pci_pm_reset(struct pci_dev *dev, int probe)

return pci_dev_wait(dev, "PM D3->D0", PCIE_RESET_READY_POLL_MS);
}

/**
* pcie_wait_for_link_delay - Wait until link is active or inactive
* pcie_wait_for_link - Wait until link is active or inactive
* @pdev: Bridge device
* @active: waiting for active or inactive?
* @delay: Delay to wait after link has become active (in ms)
*
* Use this to wait till link becomes active or inactive.
*/
bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active, int delay)
bool pcie_wait_for_link(struct pci_dev *pdev, bool active)
{
int timeout = 1000;
bool ret;
Expand Down Expand Up @@ -4648,25 +4651,13 @@ bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active, int delay)
timeout -= 10;
}
if (active && ret)
msleep(delay);
msleep(100);
else if (ret != active)
pci_info(pdev, "Data Link Layer Link Active not %s in 1000 msec\n",
active ? "set" : "cleared");
return ret == active;
}

/**
* pcie_wait_for_link - Wait until link is active or inactive
* @pdev: Bridge device
* @active: waiting for active or inactive?
*
* Use this to wait till link becomes active or inactive.
*/
bool pcie_wait_for_link(struct pci_dev *pdev, bool active)
{
return pcie_wait_for_link_delay(pdev, active, 100);
}

void pci_reset_secondary_bus(struct pci_dev *dev)
{
u16 ctrl;
Expand Down
1 change: 0 additions & 1 deletion drivers/pci/pci.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,6 @@ static inline int pci_dev_specific_disable_acs_redir(struct pci_dev *dev)
void pcie_do_recovery(struct pci_dev *dev, enum pci_channel_state state,
u32 service);

bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active, int delay);
bool pcie_wait_for_link(struct pci_dev *pdev, bool active);
#ifdef CONFIG_PCIEASPM
void pcie_aspm_init_link_state(struct pci_dev *pdev);
Expand Down
66 changes: 0 additions & 66 deletions drivers/pci/pcie/portdrv_core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/kernel.h>
#include <linux/delay.h>
#include <linux/errno.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
Expand Down Expand Up @@ -379,67 +378,6 @@ static int pm_iter(struct device *dev, void *data)
return 0;
}

static int get_downstream_delay(struct pci_bus *bus)
{
struct pci_dev *pdev;
int min_delay = 100;
int max_delay = 0;

list_for_each_entry(pdev, &bus->devices, bus_list) {
if (!pdev->imm_ready)
min_delay = 0;
else if (pdev->d3cold_delay < min_delay)
min_delay = pdev->d3cold_delay;
if (pdev->d3cold_delay > max_delay)
max_delay = pdev->d3cold_delay;
}

return max(min_delay, max_delay);
}

/*
* wait_for_downstream_link - Wait for downstream link to establish
* @pdev: PCIe port whose downstream link is waited
*
* Handle delays according to PCIe 4.0 section 6.6.1 before configuration
* access to the downstream component is permitted.
*
* This blocks PCI core resume of the hierarchy below this port until the
* link is trained. Should be called before resuming port services to
* prevent pciehp from starting to tear-down the hierarchy too soon.
*/
static void wait_for_downstream_link(struct pci_dev *pdev)
{
int delay;

if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT &&
pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM)
return;

if (pci_dev_is_disconnected(pdev))
return;

if (!pdev->subordinate || list_empty(&pdev->subordinate->devices) ||
!pdev->bridge_d3)
return;

delay = get_downstream_delay(pdev->subordinate);
if (!delay)
return;

dev_dbg(&pdev->dev, "waiting downstream link for %d ms\n", delay);

/*
* If downstream port does not support speeds greater than 5 GT/s
* need to wait 100ms. For higher speeds (gen3) we need to wait
* first for the data link layer to become active.
*/
if (pcie_get_speed_cap(pdev) <= PCIE_SPEED_5_0GT)
msleep(delay);
else
pcie_wait_for_link_delay(pdev, true, delay);
}

/**
* pcie_port_device_suspend - suspend port services associated with a PCIe port
* @dev: PCI Express port to handle
Expand All @@ -453,8 +391,6 @@ int pcie_port_device_suspend(struct device *dev)
int pcie_port_device_resume_noirq(struct device *dev)
{
size_t off = offsetof(struct pcie_port_service_driver, resume_noirq);

wait_for_downstream_link(to_pci_dev(dev));
return device_for_each_child(dev, &off, pm_iter);
}

Expand Down Expand Up @@ -485,8 +421,6 @@ int pcie_port_device_runtime_suspend(struct device *dev)
int pcie_port_device_runtime_resume(struct device *dev)
{
size_t off = offsetof(struct pcie_port_service_driver, runtime_resume);

wait_for_downstream_link(to_pci_dev(dev));
return device_for_each_child(dev, &off, pm_iter);
}
#endif /* PM */
Expand Down

0 comments on commit 0617bde

Please sign in to comment.