From patchwork Thu Aug 22 08:55:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mika Westerberg X-Patchwork-Id: 11108839 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id BFE261398 for ; Thu, 22 Aug 2019 08:55:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 905632173E for ; Thu, 22 Aug 2019 08:55:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732799AbfHVIz6 (ORCPT ); Thu, 22 Aug 2019 04:55:58 -0400 Received: from mga14.intel.com ([192.55.52.115]:18188 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730948AbfHVIz6 (ORCPT ); Thu, 22 Aug 2019 04:55:58 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Aug 2019 01:55:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,416,1559545200"; d="scan'208";a="169698837" Received: from black.fi.intel.com ([10.237.72.28]) by orsmga007.jf.intel.com with ESMTP; 22 Aug 2019 01:55:54 -0700 Received: by black.fi.intel.com (Postfix, from userid 1001) id A0B63FC; Thu, 22 Aug 2019 11:55:53 +0300 (EEST) From: Mika Westerberg To: Bjorn Helgaas Cc: Russell Currey , Sam Bobroff , Oliver O'Halloran , Andy Shevchenko , stefan.maetje@esd.eu, Patrick Talbert , "David S . Miller" , Lukas Wunner , Frederick Lawler , "Rafael J. Wysocki" , Heiner Kallweit , Yijing Wang , Robert White , Mika Westerberg , linux-pci@vger.kernel.org Subject: [PATCH 1/2] PCI: Make pcie_downstream_port() available outside of access.c Date: Thu, 22 Aug 2019 11:55:52 +0300 Message-Id: <20190822085553.62697-1-mika.westerberg@linux.intel.com> X-Mailer: git-send-email 2.23.0.rc1 MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org This helper function is useful in other places where code needs to determine whether the PCIe port is downstream so make it available outside of access.c. Signed-off-by: Mika Westerberg Reviewed-by: Andy Shevchenko --- drivers/pci/access.c | 9 --------- drivers/pci/pci.h | 9 +++++++++ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 544922f097c0..2fccb5762c76 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -336,15 +336,6 @@ static inline int pcie_cap_version(const struct pci_dev *dev) return pcie_caps_reg(dev) & PCI_EXP_FLAGS_VERS; } -static bool pcie_downstream_port(const struct pci_dev *dev) -{ - int type = pci_pcie_type(dev); - - return type == PCI_EXP_TYPE_ROOT_PORT || - type == PCI_EXP_TYPE_DOWNSTREAM || - type == PCI_EXP_TYPE_PCIE_BRIDGE; -} - bool pcie_cap_has_lnkctl(const struct pci_dev *dev) { int type = pci_pcie_type(dev); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 9a83fcf612ca..ae8d839dca4f 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -118,6 +118,15 @@ static inline bool pci_power_manageable(struct pci_dev *pci_dev) return !pci_has_subordinate(pci_dev) || pci_dev->bridge_d3; } +static inline bool pcie_downstream_port(const struct pci_dev *dev) +{ + int type = pci_pcie_type(dev); + + return type == PCI_EXP_TYPE_ROOT_PORT || + type == PCI_EXP_TYPE_DOWNSTREAM || + type == PCI_EXP_TYPE_PCIE_BRIDGE; +} + int pci_vpd_init(struct pci_dev *dev); void pci_vpd_release(struct pci_dev *dev); void pcie_vpd_create_sysfs_dev_files(struct pci_dev *dev); From patchwork Thu Aug 22 08:55:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mika Westerberg X-Patchwork-Id: 11108841 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3931414DB for ; Thu, 22 Aug 2019 08:56:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 16459214DA for ; Thu, 22 Aug 2019 08:56:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730948AbfHVIz7 (ORCPT ); Thu, 22 Aug 2019 04:55:59 -0400 Received: from mga04.intel.com ([192.55.52.120]:47058 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732798AbfHVIz7 (ORCPT ); Thu, 22 Aug 2019 04:55:59 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Aug 2019 01:55:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,416,1559545200"; d="scan'208";a="354208258" Received: from black.fi.intel.com ([10.237.72.28]) by orsmga005.jf.intel.com with ESMTP; 22 Aug 2019 01:55:54 -0700 Received: by black.fi.intel.com (Postfix, from userid 1001) id AC7CC96; Thu, 22 Aug 2019 11:55:53 +0300 (EEST) From: Mika Westerberg To: Bjorn Helgaas Cc: Russell Currey , Sam Bobroff , Oliver O'Halloran , Andy Shevchenko , stefan.maetje@esd.eu, Patrick Talbert , "David S . Miller" , Lukas Wunner , Frederick Lawler , "Rafael J. Wysocki" , Heiner Kallweit , Yijing Wang , Robert White , Mika Westerberg , linux-pci@vger.kernel.org Subject: [PATCH 2/2] PCI: Get rid of dev->has_secondary_link flag Date: Thu, 22 Aug 2019 11:55:53 +0300 Message-Id: <20190822085553.62697-2-mika.westerberg@linux.intel.com> X-Mailer: git-send-email 2.23.0.rc1 In-Reply-To: <20190822085553.62697-1-mika.westerberg@linux.intel.com> References: <20190822085553.62697-1-mika.westerberg@linux.intel.com> MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org In order to solve a problem on a system where PCIe switch upstream port incorrectly identifies itself as downstream port commit d0751b98dfa3 ("PCI: Add dev->has_secondary_link to track downstream PCIe links") introduced a flag to struct pci_dev that can be used to determine which side of the port the link is. The problem with this approach is that each time caller wants to determine whether the port is upstream or downstream, it may need to look at the dev->has_secondary_link as well. Making the calling code unnecessary complicated. Instead of this, we can correct the type of the port itself so that pci_pcie_type() returns the actual type regardless of what the port claims it is. Then update the users to call pci_pcie_type() and pcie_downstream_port() accordingly. This allows us to remove dev->has_secondary_flag completely. Link: https://lore.kernel.org/linux-pci/20190703133953.GK128603@google.com/ Suggested-by: Bjorn Helgaas Signed-off-by: Mika Westerberg Reviewed-by: Andy Shevchenko --- I haven't been able to test this properly since I don't have access to any system having the issue. I tried on a couple of systems without the issue and did not see any problems, though. @Robert, as the reporter of the original issue if you still have access to any of these systems I would appreciate if you could give this patch series a try. drivers/pci/pci.c | 2 +- drivers/pci/pcie/aspm.c | 8 +++---- drivers/pci/pcie/err.c | 2 +- drivers/pci/probe.c | 48 ++++++++++++++++++++++++----------------- drivers/pci/vc.c | 4 +++- include/linux/pci.h | 1 - 6 files changed, 37 insertions(+), 28 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 9ac50710f1d4..3c0672f1dfe7 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3577,7 +3577,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 464f8f92653f..2776d34be693 100644 --- 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 773197a12568..b0e6048a9208 100644 --- 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 a3c7338fad86..01752a63cd15 100644 --- 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, ®16); 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; + } } } @@ -2764,12 +2776,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 5acd9c02683a..9ae9fb9339e8 100644 --- a/drivers/pci/vc.c +++ b/drivers/pci/vc.c @@ -13,6 +13,8 @@ #include #include +#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 82e4cd1b7ac3..2f8990246316 100644 --- 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 */