From 37540db221e1ca94d9a57632238d1a62043205b3 Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczyński Date: Thu, 13 Jul 2023 12:18:39 -0500 Subject: MAINTAINERS: Add Manivannan Sadhasivam as DesignWare PCIe driver maintainer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manivannan has been actively reviewing patches and testing changes related to the DesignWare core driver and other DWC-based PCIe drivers for a while now. Add Manivannan as a maintainer for the Synopsys DesignWare driver to make his role and contributions official. Thank you Manivannan! For all the help with DWC! Signed-off-by: Krzysztof Wilczyński Signed-off-by: Bjorn Helgaas Acked-by: Manivannan Sadhasivam --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3be1bdfe8ecc7f..76bdef7ba1c682 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16285,6 +16285,7 @@ F: drivers/pci/controller/dwc/pci-exynos.c PCI DRIVER FOR SYNOPSYS DESIGNWARE M: Jingoo Han M: Gustavo Pimentel +M: Manivannan Sadhasivam L: linux-pci@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml -- cgit 1.2.3-korg From c5097b9869a136349d8404715dc8aabb7570a762 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Thu, 6 Jul 2023 10:26:10 +0200 Subject: Revert "PCI: dwc: Wait for link up only if link is started" This reverts commit da56a1bfbab55189595e588f1d984bdfb5cf5924. Bjorn Andersson, Fabio Estevam, Xiaolei Wang, and Jon Hunter reported that da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started") broke controller probing by returning an error in case the link does not come up during host initialisation, for example when the slot is empty. As explained in commit 886a9c134755 ("PCI: dwc: Move link handling into common code") and as indicated by the comment "Ignore errors, the link may come up later" in the code, waiting for link up and ignoring errors is the intended behaviour: Let's standardize this to succeed as there are usecases where devices (and the link) appear later even without hotplug. For example, a reconfigured FPGA device. Reverting the offending commit specifically fixes a regression on Qualcomm platforms like the Lenovo ThinkPad X13s which no longer reach the interconnect sync state if a slot does not have a device populated (e.g. an optional modem). Note that enabling asynchronous probing by default as was done for Qualcomm platforms by commit c0e1eb441b1d ("PCI: qcom: Enable async probe by default"), should take care of any related boot time concerns. Finally, note that the intel-gw driver is the only driver currently not providing a .start_link() callback and instead starts the link in its .host_init() callback, which may avoid an additional one-second timeout during probe by making the link-up wait conditional. If anyone cares, that can be done in a follow-up patch with a proper motivation. [bhelgaas: add Fabio Estevam, Xiaolei Wang, Jon Hunter reports] Fixes: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started") Link: https://lore.kernel.org/r/20230706082610.26584-1-johan+linaro@kernel.org Reported-by: Bjorn Andersson Signed-off-by: Johan Hovold Reported-by: Fabio Estevam Link: https://lore.kernel.org/r/20230704122635.1362156-1-festevam@gmail.com/ Reported-by: Xiaolei Wang Link: https://lore.kernel.org/r/20230705010624.3912934-1-xiaolei.wang@windriver.com/ Reported-by: Jon Hunter Link: https://lore.kernel.org/r/6ca287a1-6c7c-7b90-9022-9e73fb82b564@nvidia.com Signed-off-by: Bjorn Helgaas Reviewed-by: Manivannan Sadhasivam Cc: Sajid Dalvi Cc: Ajay Agarwal --- drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++--------- drivers/pci/controller/dwc/pcie-designware.c | 20 +++++++------------- drivers/pci/controller/dwc/pcie-designware.h | 1 - 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index cf61733bf78d24..9952057c8819c8 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -485,20 +485,15 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) if (ret) goto err_remove_edma; - if (dw_pcie_link_up(pci)) { - dw_pcie_print_link_status(pci); - } else { + if (!dw_pcie_link_up(pci)) { ret = dw_pcie_start_link(pci); if (ret) goto err_remove_edma; - - if (pci->ops && pci->ops->start_link) { - ret = dw_pcie_wait_for_link(pci); - if (ret) - goto err_stop_link; - } } + /* Ignore errors, the link may come up later */ + dw_pcie_wait_for_link(pci); + bridge->sysdata = pp; ret = pci_host_probe(bridge); diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index c87848cd8686c0..1f2ee71da4da2a 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -644,20 +644,9 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index) dw_pcie_writel_atu(pci, dir, index, PCIE_ATU_REGION_CTRL2, 0); } -void dw_pcie_print_link_status(struct dw_pcie *pci) -{ - u32 offset, val; - - offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); - val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA); - - dev_info(pci->dev, "PCIe Gen.%u x%u link up\n", - FIELD_GET(PCI_EXP_LNKSTA_CLS, val), - FIELD_GET(PCI_EXP_LNKSTA_NLW, val)); -} - int dw_pcie_wait_for_link(struct dw_pcie *pci) { + u32 offset, val; int retries; /* Check if the link is up or not */ @@ -673,7 +662,12 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci) return -ETIMEDOUT; } - dw_pcie_print_link_status(pci); + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); + val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA); + + dev_info(pci->dev, "PCIe Gen.%u x%u link up\n", + FIELD_GET(PCI_EXP_LNKSTA_CLS, val), + FIELD_GET(PCI_EXP_LNKSTA_NLW, val)); return 0; } diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 61566064080196..79713ce075cc1b 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -429,7 +429,6 @@ void dw_pcie_setup(struct dw_pcie *pci); void dw_pcie_iatu_detect(struct dw_pcie *pci); int dw_pcie_edma_detect(struct dw_pcie *pci); void dw_pcie_edma_remove(struct dw_pcie *pci); -void dw_pcie_print_link_status(struct dw_pcie *pci); static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val) { -- cgit 1.2.3-korg From 3bfc37d92687b4b19056998cebc02f94fbc81427 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Fri, 4 Aug 2023 11:54:43 -0500 Subject: Revert "PCI: mvebu: Mark driver as BROKEN" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit b3574f579ece ("PCI: mvebu: Mark driver as BROKEN") made it impossible to enable the pci-mvebu driver. The driver does have known problems, but as Russell and Uwe reported, it does work in some configurations, so removing it broke some working setups. Revert b3574f579ece so pci-mvebu is available. Reported-by: Russell King (Oracle) Link: https://lore.kernel.org/r/ZMzicVQEyHyZzBOc@shell.armlinux.org.uk Reported-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230804134622.pmbymxtzxj2yfhri@pengutronix.de Signed-off-by: Bjorn Helgaas --- drivers/pci/controller/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig index 8d49bad7f84728..0859be86e71834 100644 --- a/drivers/pci/controller/Kconfig +++ b/drivers/pci/controller/Kconfig @@ -179,7 +179,6 @@ config PCI_MVEBU depends on MVEBU_MBUS depends on ARM depends on OF - depends on BROKEN select PCI_BRIDGE_EMUL help Add support for Marvell EBU PCIe controller. This PCIe controller -- cgit 1.2.3-korg From cc22522fd55e257c86d340ae9aedc122e705a435 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Wed, 26 Jul 2023 14:35:18 +0200 Subject: PCI: acpiphp: Use pci_assign_unassigned_bridge_resources() only for non-root bus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 40613da52b13 ("PCI: acpiphp: Reassign resources on bridge if necessary") changed acpiphp hotplug to use pci_assign_unassigned_bridge_resources() which depends on bridge being available, however enable_slot() can be called without bridge associated: 1. Legitimate case of hotplug on root bus (widely used in virt world) 2. A (misbehaving) firmware, that sends ACPI Bus Check notifications to non existing root ports (Dell Inspiron 7352/0W6WV0), which end up at enable_slot(..., bridge = 0) where bus has no bridge assigned to it. acpihp doesn't know that it's a bridge, and bus specific 'PCI subsystem' can't augment ACPI context with bridge information since the PCI device to get this data from is/was not available. Issue is easy to reproduce with QEMU's 'pc' machine, which supports PCI hotplug on hostbridge slots. To reproduce, boot kernel at commit 40613da52b13 in VM started with following CLI (assuming guest root fs is installed on sda1 partition): # qemu-system-x86_64 -M pc -m 1G -enable-kvm -cpu host \ -monitor stdio -serial file:serial.log \ -kernel arch/x86/boot/bzImage \ -append "root=/dev/sda1 console=ttyS0" \ guest_disk.img Once guest OS is fully booted at qemu prompt: (qemu) device_add e1000 (check serial.log) it will cause NULL pointer dereference at: void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) { struct pci_bus *parent = bridge->subordinate; BUG: kernel NULL pointer dereference, address: 0000000000000018 ? pci_assign_unassigned_bridge_resources+0x1f/0x260 enable_slot+0x21f/0x3e0 acpiphp_hotplug_notify+0x13d/0x260 acpi_device_hotplug+0xbc/0x540 acpi_hotplug_work_fn+0x15/0x20 process_one_work+0x1f7/0x370 worker_thread+0x45/0x3b0 The issue was discovered on Dell Inspiron 7352/0W6WV0 laptop with following sequence: 1. Suspend to RAM 2. Wake up with the same backtrace being observed: 3. 2nd suspend to RAM attempt makes laptop freeze Fix it by using __pci_bus_assign_resources() instead of pci_assign_unassigned_bridge_resources() as we used to do, but only in case when bus doesn't have a bridge associated (to cover for the case of ACPI event on hostbridge or non existing root port). That lets us keep hotplug on root bus working like it used to and at the same time keeps resource reassignment usable on root ports (and other 1st level bridges) that was fixed by 40613da52b13. Fixes: 40613da52b13 ("PCI: acpiphp: Reassign resources on bridge if necessary") Link: https://lore.kernel.org/r/20230726123518.2361181-2-imammedo@redhat.com Reported-by: Woody Suwalski Tested-by: Woody Suwalski Tested-by: Michal Koutný Link: https://lore.kernel.org/r/11fc981c-af49-ce64-6b43-3e282728bd1a@gmail.com Signed-off-by: Igor Mammedov Signed-off-by: Bjorn Helgaas Acked-by: Rafael J. Wysocki Acked-by: Michael S. Tsirkin --- drivers/pci/hotplug/acpiphp_glue.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 328d1e41601476..601129772b2d50 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -498,6 +498,7 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge) acpiphp_native_scan_bridge(dev); } } else { + LIST_HEAD(add_list); int max, pass; acpiphp_rescan_slot(slot); @@ -511,10 +512,15 @@ static void enable_slot(struct acpiphp_slot *slot, bool bridge) if (pass && dev->subordinate) { check_hotplug_bridge(slot, dev); pcibios_resource_survey_bus(dev->subordinate); + if (pci_is_root_bus(bus)) + __pci_bus_size_bridges(dev->subordinate, &add_list); } } } - pci_assign_unassigned_bridge_resources(bus->self); + if (pci_is_root_bus(bus)) + __pci_bus_assign_resources(bus, &add_list, NULL); + else + pci_assign_unassigned_bridge_resources(bus->self); } acpiphp_sanitize_bus(bus); -- cgit 1.2.3-korg