Commit ca784104 authored by Mika Westerberg's avatar Mika Westerberg Committed by Bjorn Helgaas
Browse files

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.

d0751b98 ("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: default avatarBjorn Helgaas <bhelgaas@google.com>
Link: https://lore.kernel.org/r/20190822085553.62697-2-mika.westerberg@linux.intel.com


Signed-off-by: default avatarMika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
parent 984998e3

drivers/pci/pci.c

+1 −1
Original line number Diff line number Diff line
@@ -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)

drivers/pci/pcie/aspm.c

+4 −4
Original line number Diff line number Diff line
@@ -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;

drivers/pci/pcie/err.c

+1 −1
Original line number Diff line number Diff line
@@ -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",

drivers/pci/probe.c

+28 −20
Original line number Diff line number Diff line
@@ -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;

drivers/pci/vc.c

+3 −1
Original line number Diff line number Diff line
@@ -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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE