PCI: Get rid of dev->has_secondary_link flag

In some systems, the Device/Port Type in the PCI Express Capabilities
register incorrectly identifies upstream ports as downstream ports.

d0751b98dfa3 ("PCI: Add dev->has_secondary_link to track downstream PCIe
links") addressed this by adding pci_dev.has_secondary_link, which is set
for downstream ports.  But this is confusing because pci_pcie_type()
sometimes gives the wrong answer, and it's not obvious that we should use
pci_dev.has_secondary_link instead.

Reduce the confusion by correcting the type of the port itself so that
pci_pcie_type() returns the actual type regardless of what the Device/Port
Type register claims it is.  Update the users to call pci_pcie_type() and
pcie_downstream_port() accordingly, and remove pci_dev.has_secondary_link
completely.

Link: https://lore.kernel.org/linux-pci/20190703133953.GK128603@google.com/
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://lore.kernel.org/r/20190822085553.62697-2-mika.westerberg@linux.intel.com
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 29ed5ec1ac27bf1c1d0b2d4028123a992594a513..97e7f6e0821e522fe421100fb0abd4ab44992a1a 100644 (file)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3576,7 +3576,7 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
                }
 
                /* Ensure upstream ports don't block AtomicOps on egress */
-               if (!bridge->has_secondary_link) {
+               if (pci_pcie_type(bridge) == PCI_EXP_TYPE_UPSTREAM) {
                        pcie_capability_read_dword(bridge, PCI_EXP_DEVCTL2,
                                                   &ctl2);
                        if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_EGRESS_BLOCK)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index e44af7f4d37ff48c050daeaeab33326ec1a52067..dcda96818eb63ac153f38f7c16cca87990c515af 100644 (file)
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -913,10 +913,10 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 
        /*
         * We allocate pcie_link_state for the component on the upstream
-        * end of a Link, so there's nothing to do unless this device has a
-        * Link on its secondary side.
+        * end of a Link, so there's nothing to do unless this device is
+        * downstream port.
         */
-       if (!pdev->has_secondary_link)
+       if (!pcie_downstream_port(pdev))
                return;
 
        /* VIA has a strange chipset, root port is under a bridge */
@@ -1070,7 +1070,7 @@ static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
        if (!pci_is_pcie(pdev))
                return 0;
 
-       if (pdev->has_secondary_link)
+       if (pcie_downstream_port(pdev))
                parent = pdev;
        if (!parent || !parent->link_state)
                return -EINVAL;

diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index 773197a12568e1e9d2ebcb1dd4e707de059ccec5..b0e6048a9208d1eef95134a83b35ae17b8f78cf0 100644 (file)
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -166,7 +166,7 @@ static pci_ers_result_t reset_link(struct pci_dev *dev, u32 service)
        driver = pcie_port_find_service(dev, service);
        if (driver && driver->reset_link) {
                status = driver->reset_link(dev);
-       } else if (dev->has_secondary_link) {
+       } else if (pcie_downstream_port(dev)) {
                status = default_reset_link(dev);
        } else {
                pci_printk(KERN_DEBUG, dev, "no link-reset support at upstream device %s\n",

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index cdf34a3c531a594def845b9ed12e124194b86861..3bfa57d58402bd8a09a5a346025fa9edb03015e3 100644 (file)
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1431,26 +1431,38 @@ void set_pcie_port_type(struct pci_dev *pdev)
        pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, &reg16);
        pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD;
 
+       parent = pci_upstream_bridge(pdev);
+       if (!parent)
+               return;
+
        /*
-        * A Root Port or a PCI-to-PCIe bridge is always the upstream end
-        * of a Link.  No PCIe component has two Links.  Two Links are
-        * connected by a Switch that has a Port on each Link and internal
-        * logic to connect the two Ports.
+        * Some systems do not identify their upstream/downstream ports
+        * correctly so detect impossible configurations here and correct
+        * the port type accordingly.
         */
        type = pci_pcie_type(pdev);
-       if (type == PCI_EXP_TYPE_ROOT_PORT ||
-           type == PCI_EXP_TYPE_PCIE_BRIDGE)
-               pdev->has_secondary_link = 1;
-       else if (type == PCI_EXP_TYPE_UPSTREAM ||
-                type == PCI_EXP_TYPE_DOWNSTREAM) {
-               parent = pci_upstream_bridge(pdev);
-
+       if (type == PCI_EXP_TYPE_DOWNSTREAM) {
                /*
-                * Usually there's an upstream device (Root Port or Switch
-                * Downstream Port), but we can't assume one exists.
+                * If pdev claims to be downstream port but the parent
+                * device is also downstream port assume pdev is actually
+                * upstream port.
                 */
-               if (parent && !parent->has_secondary_link)
-                       pdev->has_secondary_link = 1;
+               if (pcie_downstream_port(parent)) {
+                       pci_info(pdev, "claims to be downstream port but is acting as upstream port, correcting type\n");
+                       pdev->pcie_flags_reg &= ~PCI_EXP_FLAGS_TYPE;
+                       pdev->pcie_flags_reg |= PCI_EXP_TYPE_UPSTREAM;
+               }
+       } else if (type == PCI_EXP_TYPE_UPSTREAM) {
+               /*
+                * If pdev claims to be upstream port but the parent
+                * device is also upstream port assume pdev is actually
+                * downstream port.
+                */
+               if (pci_pcie_type(parent) == PCI_EXP_TYPE_UPSTREAM) {
+                       pci_info(pdev, "claims to be upstream port but is acting as downstream port, correcting type\n");
+                       pdev->pcie_flags_reg &= ~PCI_EXP_FLAGS_TYPE;
+                       pdev->pcie_flags_reg |= PCI_EXP_TYPE_DOWNSTREAM;
+               }
        }
 }
 
@@ -2488,12 +2500,8 @@ static int only_one_child(struct pci_bus *bus)
         * A PCIe Downstream Port normally leads to a Link with only Device
         * 0 on it (PCIe spec r3.1, sec 7.3.1).  As an optimization, scan
         * only for Device 0 in that situation.
-        *
-        * Checking has_secondary_link is a hack to identify Downstream
-        * Ports because sometimes Switches are configured such that the
-        * PCIe Port Type labels are backwards.
         */
-       if (bridge && pci_is_pcie(bridge) && bridge->has_secondary_link)
+       if (bridge && pci_is_pcie(bridge) && pcie_downstream_port(bridge))
                return 1;
 
        return 0;

diff --git a/drivers/pci/vc.c b/drivers/pci/vc.c
index 5acd9c02683af78dd96fffa7d8613c7fc9d582fd..9ae9fb9339e80b6136c3281bcd55cd7acbfba429 100644 (file)
--- a/drivers/pci/vc.c
+++ b/drivers/pci/vc.c
@@ -13,6 +13,8 @@
 #include <linux/pci_regs.h>
 #include <linux/types.h>
 
+#include "pci.h"
+
 /**
  * pci_vc_save_restore_dwords - Save or restore a series of dwords
  * @dev: device
@@ -105,7 +107,7 @@ static void pci_vc_enable(struct pci_dev *dev, int pos, int res)
        struct pci_dev *link = NULL;
 
        /* Enable VCs from the downstream device */
-       if (!dev->has_secondary_link)
+       if (!pci_is_pcie(dev) || !pcie_downstream_port(dev))
                return;
 
        ctrl_pos = pos + PCI_VC_RES_CTRL + (res * PCI_CAP_VC_PER_VC_SIZEOF);

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9e700d9f9f287d0db0bf770cd5c4ee2a564db94c..0153ede227e55efc4ca42b0adc03d278e080f913 100644 (file)
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -418,7 +418,6 @@ struct pci_dev {
        unsigned int    broken_intx_masking:1;  /* INTx masking can't be used */
        unsigned int    io_window_1k:1;         /* Intel bridge 1K I/O windows */
        unsigned int    irq_managed:1;
-       unsigned int    has_secondary_link:1;
        unsigned int    non_compliant_bars:1;   /* Broken BARs; ignore them */
        unsigned int    is_probed:1;            /* Device probing in progress */
        unsigned int    link_active_reporting:1;/* Device capable of reporting link active */