From dff6139015dc68e93be3822a7bd406a1d138628b Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Thu, 31 Mar 2022 22:40:03 -0500 Subject: PCI/ACPI: Allow D3 only if Root Port can signal and wake from D3 acpi_pci_bridge_d3(dev) returns "true" if "dev" is a hotplug bridge that can handle hotplug events while in D3. Previously this meant either: - "dev" has a _PS0 or _PR0 method (acpi_pci_power_manageable()), or - The Root Port above "dev" has a _DSD with a "HotPlugSupportInD3" property with value 1. This did not consider _PRW, which tells us about wakeup GPEs (ACPI v6.4, sec 7.3.13). Without a wakeup GPE, from an ACPI perspective the Root Port has no way of generating wakeup signals, so hotplug events will be lost if we use D3. Similarly, it did not consider _S0W, which tells us the deepest D-state from which a device can wake itself (sec 7.3.20). If _S0W tells us the device cannot wake from D3, hotplug events will again be lost if we use D3. Some platforms, e.g., AMD Yellow Carp, supply "HotPlugSupportInD3" without _PRW or with an _S0W that says the Root Port cannot wake from D3. On those platforms, we previously put bridges in D3hot, hotplug events were lost, and hotplugged devices would not be recognized without manually rescanning. Allow bridges to be put in D3 only if the Root Port can generate wakeup GPEs (wakeup.flags.valid), it can wake from D3 (_S0W), AND it has the "HotPlugSupportInD3" property. Neither Windows 10 nor Windows 11 puts the bridge in D3 when the firmware is configured this way, and this change aligns the handling of the situation to be the same. [bhelgaas: commit log, tidy "HotPlugSupportInD3" check and comment] Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/07_Power_and_Performance_Mgmt/device-power-management-objects.html?highlight=s0w#s0w-s0-device-wake-state Link: https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-pcie-root-ports-supporting-hot-plug-in-d3 Link: https://lore.kernel.org/r/20220401034003.3166-1-mario.limonciello@amd.com Fixes: 26ad34d510a87 ("PCI / ACPI: Whitelist D3 for more PCIe hotplug ports") Signed-off-by: Mario Limonciello Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki --- drivers/pci/pci-acpi.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 1f15ab7eabf8..3ae435beaf0a 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -974,9 +974,11 @@ bool acpi_pci_power_manageable(struct pci_dev *dev) bool acpi_pci_bridge_d3(struct pci_dev *dev) { - const union acpi_object *obj; - struct acpi_device *adev; struct pci_dev *rpdev; + struct acpi_device *adev; + acpi_status status; + unsigned long long state; + const union acpi_object *obj; if (acpi_pci_disabled || !dev->is_hotplug_bridge) return false; @@ -985,12 +987,6 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev) if (acpi_pci_power_manageable(dev)) return true; - /* - * The ACPI firmware will provide the device-specific properties through - * _DSD configuration object. Look for the 'HotPlugSupportInD3' property - * for the root port and if it is set we know the hierarchy behind it - * supports D3 just fine. - */ rpdev = pcie_find_root_port(dev); if (!rpdev) return false; @@ -999,11 +995,34 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev) if (!adev) return false; - if (acpi_dev_get_property(adev, "HotPlugSupportInD3", - ACPI_TYPE_INTEGER, &obj) < 0) + /* + * If the Root Port cannot signal wakeup signals at all, i.e., it + * doesn't supply a wakeup GPE via _PRW, it cannot signal hotplug + * events from low-power states including D3hot and D3cold. + */ + if (!adev->wakeup.flags.valid) return false; - return obj->integer.value == 1; + /* + * If the Root Port cannot wake itself from D3hot or D3cold, we + * can't use D3. + */ + status = acpi_evaluate_integer(adev->handle, "_S0W", NULL, &state); + if (ACPI_SUCCESS(status) && state < ACPI_STATE_D3_HOT) + return false; + + /* + * The "HotPlugSupportInD3" property in a Root Port _DSD indicates + * the Port can signal hotplug events while in D3. We assume any + * bridges *below* that Root Port can also signal hotplug events + * while in D3. + */ + if (!acpi_dev_get_property(adev, "HotPlugSupportInD3", + ACPI_TYPE_INTEGER, &obj) && + obj->integer.value == 1) + return true; + + return false; } int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state) -- cgit v1.3.1 From b2851926c6d9d977ff60f613aff95f4900b9620e Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Sat, 2 Apr 2022 12:11:56 +0200 Subject: PCI: hotplug: Clean up include files arch/powerpc/include/asm/prom.h includes some headers that it doesn't need itself. Add the missing headers to files that include prom.h so we can remove them from prom.h. Link: https://lore.kernel.org/r/79201f5fae8d003164ac36ed3be7789db1bc5ab4.1648833421.git.christophe.leroy@csgroup.eu Signed-off-by: Christophe Leroy Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/pnv_php.c | 1 + drivers/pci/hotplug/rpadlpar_core.c | 1 + drivers/pci/hotplug/rpaphp_core.c | 2 ++ drivers/pci/hotplug/rpaphp_pci.c | 1 + drivers/pci/hotplug/rpaphp_slot.c | 1 + 5 files changed, 6 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c index f4c2e6e01be0..881d420637bf 100644 --- a/drivers/pci/hotplug/pnv_php.c +++ b/drivers/pci/hotplug/pnv_php.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c index e6991ff67526..980bb3afd092 100644 --- a/drivers/pci/hotplug/rpadlpar_core.c +++ b/drivers/pci/hotplug/rpadlpar_core.c @@ -15,6 +15,7 @@ #include #include +#include #include #include #include diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c index 9887c9de08c3..491986197c47 100644 --- a/drivers/pci/hotplug/rpaphp_core.c +++ b/drivers/pci/hotplug/rpaphp_core.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -20,6 +21,7 @@ #include /* for eeh_add_device() */ #include /* rtas_call */ #include /* for pci_controller */ +#include #include "../pci.h" /* for pci_add_new_bus */ /* and pci_do_scan_bus */ #include "rpaphp.h" diff --git a/drivers/pci/hotplug/rpaphp_pci.c b/drivers/pci/hotplug/rpaphp_pci.c index c380bdacd146..630f77057c23 100644 --- a/drivers/pci/hotplug/rpaphp_pci.c +++ b/drivers/pci/hotplug/rpaphp_pci.c @@ -8,6 +8,7 @@ * Send feedback to * */ +#include #include #include diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c index 93b4a945c55d..779eab12e981 100644 --- a/drivers/pci/hotplug/rpaphp_slot.c +++ b/drivers/pci/hotplug/rpaphp_slot.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include -- cgit v1.3.1 From 03038d84ace72678a9944524508f218a00377dc0 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Tue, 5 Apr 2022 12:38:10 +0300 Subject: PCI/ASPM: Make Intel DG2 L1 acceptable latency unlimited Intel DG2 discrete graphics PCIe endpoints advertise L1 acceptable exit latency to be < 1us even though they can actually tolerate unlimited exit latencies just fine. Quirk the L1 acceptable exit latency for these endpoints to be unlimited so ASPM L1 can be enabled. [bhelgaas: use FIELD_GET/FIELD_PREP, wordsmith comment & commit log] Link: https://lore.kernel.org/r/20220405093810.76613-1-mika.westerberg@linux.intel.com Signed-off-by: Mika Westerberg Signed-off-by: Bjorn Helgaas Reviewed-by: Rodrigo Vivi --- drivers/pci/quirks.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index da829274fc66..41aeaa235132 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -12,6 +12,7 @@ * file, where their drivers can use them. */ +#include #include #include #include @@ -5895,3 +5896,49 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1533, rom_bar_overlap_defect); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect); + +#ifdef CONFIG_PCIEASPM +/* + * Several Intel DG2 graphics devices advertise that they can only tolerate + * 1us latency when transitioning from L1 to L0, which may prevent ASPM L1 + * from being enabled. But in fact these devices can tolerate unlimited + * latency. Override their Device Capabilities value to allow ASPM L1 to + * be enabled. + */ +static void aspm_l1_acceptable_latency(struct pci_dev *dev) +{ + u32 l1_lat = FIELD_GET(PCI_EXP_DEVCAP_L1, dev->devcap); + + if (l1_lat < 7) { + dev->devcap |= FIELD_PREP(PCI_EXP_DEVCAP_L1, 7); + pci_info(dev, "ASPM: overriding L1 acceptable latency from %#x to 0x7\n", + l1_lat); + } +} +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f80, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f81, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f82, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f83, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f84, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f85, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f86, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f87, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f88, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5690, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5691, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5692, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5693, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5694, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5695, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a0, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a1, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a2, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a3, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a4, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a5, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a6, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b0, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b1, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c0, aspm_l1_acceptable_latency); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c1, aspm_l1_acceptable_latency); +#endif -- cgit v1.3.1 From 0aa3a0937feeb91a0e4e438c3c063b749b194192 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 15 Mar 2022 09:58:29 +0300 Subject: PCI: cadence: Fix find_first_zero_bit() limit The ep->ob_region_map bitmap is a long and it has BITS_PER_LONG bits. Link: https://lore.kernel.org/r/20220315065829.GA13572@kili Fixes: 37dddf14f1ae ("PCI: cadence: Add EndPoint Controller driver for Cadence PCIe controller") Signed-off-by: Dan Carpenter Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/cadence/pcie-cadence-ep.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c index 88e05b9c2e5b..18e32b8ffd5e 100644 --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c @@ -187,8 +187,7 @@ static int cdns_pcie_ep_map_addr(struct pci_epc *epc, u8 fn, u8 vfn, struct cdns_pcie *pcie = &ep->pcie; u32 r; - r = find_first_zero_bit(&ep->ob_region_map, - sizeof(ep->ob_region_map) * BITS_PER_LONG); + r = find_first_zero_bit(&ep->ob_region_map, BITS_PER_LONG); if (r >= ep->max_regions - 1) { dev_err(&epc->dev, "no free outbound region\n"); return -EINVAL; -- cgit v1.3.1 From 096950e230b8d83645c7cf408b9f399f58c08b96 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 15 Mar 2022 09:59:44 +0300 Subject: PCI: rockchip: Fix find_first_zero_bit() limit The ep->ob_region_map bitmap is a long and it has BITS_PER_LONG bits. Link: https://lore.kernel.org/r/20220315065944.GB13572@kili Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller") Signed-off-by: Dan Carpenter Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/pcie-rockchip-ep.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c index 5fb9ce6e536e..d1a200b93b2b 100644 --- a/drivers/pci/controller/pcie-rockchip-ep.c +++ b/drivers/pci/controller/pcie-rockchip-ep.c @@ -264,8 +264,7 @@ static int rockchip_pcie_ep_map_addr(struct pci_epc *epc, u8 fn, u8 vfn, struct rockchip_pcie *pcie = &ep->rockchip; u32 r; - r = find_first_zero_bit(&ep->ob_region_map, - sizeof(ep->ob_region_map) * BITS_PER_LONG); + r = find_first_zero_bit(&ep->ob_region_map, BITS_PER_LONG); /* * Region 0 is reserved for configuration space and shouldn't * be used elsewhere per TRM, so leave it out. -- cgit v1.3.1 From 214e0d8fe4a813ae6ffd62bc2dfe7544c20914f4 Mon Sep 17 00:00:00 2001 From: Miaoqian Lin Date: Wed, 9 Mar 2022 09:19:52 +0000 Subject: PCI: mediatek: Fix refcount leak in mtk_pcie_subsys_powerup() The of_find_compatible_node() function returns a node pointer with refcount incremented, We should use of_node_put() on it when done Add the missing of_node_put() to release the refcount. Link: https://lore.kernel.org/r/20220309091953.5630-1-linmq006@gmail.com Fixes: 87e8657ba99c ("PCI: mediatek: Add new method to get shared pcie-cfg base address") Signed-off-by: Miaoqian Lin Signed-off-by: Lorenzo Pieralisi Reviewed-by: AngeloGioacchino Del Regno Reviewed-by: Miles Chen Acked-by: Rob Herring --- drivers/pci/controller/pcie-mediatek.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c index ddfbd4aebdec..be8bd919cb88 100644 --- a/drivers/pci/controller/pcie-mediatek.c +++ b/drivers/pci/controller/pcie-mediatek.c @@ -1008,6 +1008,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie) "mediatek,generic-pciecfg"); if (cfg_node) { pcie->cfg = syscon_node_to_regmap(cfg_node); + of_node_put(cfg_node); if (IS_ERR(pcie->cfg)) return PTR_ERR(pcie->cfg); } -- cgit v1.3.1 From 88557685cd72cf0db686a4ebff3fad4365cb6071 Mon Sep 17 00:00:00 2001 From: Jiantao Zhang Date: Wed, 9 Mar 2022 20:01:04 +0800 Subject: PCI: dwc: Fix setting error return on MSI DMA mapping failure When dma_mapping_error() returns error because of no enough memory, but dw_pcie_host_init() returns success, which will mislead the callers. Link: https://lore.kernel.org/r/30170911-0e2f-98ce-9266-70465b9073e5@huawei.com Fixes: 07940c369a6b ("PCI: dwc: Fix MSI page leakage in suspend/resume") Signed-off-by: Jianrong Zhang Signed-off-by: Jiantao Zhang Signed-off-by: Lorenzo Pieralisi Reviewed-by: Rob Herring --- drivers/pci/controller/dwc/pcie-designware-host.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 2fa86f32d964..9979302532b7 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -396,7 +396,8 @@ int dw_pcie_host_init(struct pcie_port *pp) sizeof(pp->msi_msg), DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); - if (dma_mapping_error(pci->dev, pp->msi_data)) { + ret = dma_mapping_error(pci->dev, pp->msi_data); + if (ret) { dev_err(pci->dev, "Failed to map MSI data\n"); pp->msi_data = 0; goto err_free_msi; -- cgit v1.3.1 From 571dda6ca5136c1213bd36b9f298d4487e2b6622 Mon Sep 17 00:00:00 2001 From: Jisheng Zhang Date: Sun, 26 Dec 2021 15:49:10 +0800 Subject: PCI: tegra194: Remove unnecessary MSI enable reg save and restore The integrated MSI Receiver enable register is always initialized in dw_pcie_setup_rc() which is also called in resume code path, so we don't need to save/restore the enable register during suspend/resume. Link: https://lore.kernel.org/r/20211226074910.2722-1-jszhang@kernel.org Signed-off-by: Jisheng Zhang Signed-off-by: Lorenzo Pieralisi Acked-by: Vidya Sagar --- drivers/pci/controller/dwc/pcie-tegra194.c | 9 --------- 1 file changed, 9 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index b1b5f836a806..cc2678490162 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -186,8 +186,6 @@ #define N_FTS_VAL 52 #define FTS_VAL 52 -#define PORT_LOGIC_MSI_CTRL_INT_0_EN 0x828 - #define GEN3_EQ_CONTROL_OFF 0x8a8 #define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_SHIFT 8 #define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK GENMASK(23, 8) @@ -2189,9 +2187,6 @@ static int tegra194_pcie_suspend_noirq(struct device *dev) if (!pcie->link_state) return 0; - /* Save MSI interrupt vector */ - pcie->msi_ctrl_int = dw_pcie_readl_dbi(&pcie->pci, - PORT_LOGIC_MSI_CTRL_INT_0_EN); tegra_pcie_downstream_dev_to_D0(pcie); tegra194_pcie_pme_turnoff(pcie); tegra_pcie_unconfig_controller(pcie); @@ -2223,10 +2218,6 @@ static int tegra194_pcie_resume_noirq(struct device *dev) if (ret < 0) goto fail_host_init; - /* Restore MSI interrupt vector */ - dw_pcie_writel_dbi(&pcie->pci, PORT_LOGIC_MSI_CTRL_INT_0_EN, - pcie->msi_ctrl_int); - return 0; fail_host_init: -- cgit v1.3.1 From 1af7c26c59ebcf241e865dd16dcc251e61472a37 Mon Sep 17 00:00:00 2001 From: Shlomo Pongratz Date: Sun, 10 Apr 2022 13:52:13 +0300 Subject: PCI/P2PDMA: Whitelist Intel Skylake-E Root Ports at any devfn In 7b94b53db34f ("PCI/P2PDMA: Add Intel Sky Lake-E Root Ports B, C, D to the whitelist"), Andrew Maier added Skylake-E 2031, 2032, and 2033 Root Ports to the pci_p2pdma_whitelist[], so we assume P2PDMA between devices below these ports works. Previously we only checked the whitelist for a device at devfn 00.0 on the root bus, which is often a "host bridge". But these Skylake Root Ports may be at any devfn and there may be no "host bridge" device. Generalize pci_host_bridge_dev() so we check the first device on the root bus, whether it is devfn 00.0 or a PCIe Root Port, against the whitelist. [bhelgaas: commit log, comment] Link: https://lore.kernel.org/r/20220410105213.690-2-shlomop@pliops.com Tested-by: Maor Gottlieb Signed-off-by: Shlomo Pongratz Signed-off-by: Bjorn Helgaas Cc: Andrew Maier --- drivers/pci/p2pdma.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index 30b1df3c9d2f..462b429ad243 100644 --- a/drivers/pci/p2pdma.c +++ b/drivers/pci/p2pdma.c @@ -326,15 +326,16 @@ static const struct pci_p2pdma_whitelist_entry { }; /* - * This lookup function tries to find the PCI device corresponding to a given - * host bridge. + * If the first device on host's root bus is either devfn 00.0 or a PCIe + * Root Port, return it. Otherwise return NULL. * - * It assumes the host bridge device is the first PCI device in the - * bus->devices list and that the devfn is 00.0. These assumptions should hold - * for all the devices in the whitelist above. + * We often use a devfn 00.0 "host bridge" in the pci_p2pdma_whitelist[] + * (though there is no PCI/PCIe requirement for such a device). On some + * platforms, e.g., Intel Skylake, there is no such host bridge device, and + * pci_p2pdma_whitelist[] may contain a Root Port at any devfn. * - * This function is equivalent to pci_get_slot(host->bus, 0), however it does - * not take the pci_bus_sem lock seeing __host_bridge_whitelist() must not + * This function is similar to pci_get_slot(host->bus, 0), but it does + * not take the pci_bus_sem lock since __host_bridge_whitelist() must not * sleep. * * For this to be safe, the caller should hold a reference to a device on the @@ -350,10 +351,14 @@ static struct pci_dev *pci_host_bridge_dev(struct pci_host_bridge *host) if (!root) return NULL; - if (root->devfn != PCI_DEVFN(0, 0)) - return NULL; - return root; + if (root->devfn == PCI_DEVFN(0, 0)) + return root; + + if (pci_pcie_type(root) == PCI_EXP_TYPE_ROOT_PORT) + return root; + + return NULL; } static bool __host_bridge_whitelist(struct pci_host_bridge *host, -- cgit v1.3.1 From 35662423fb879dd0f32b7beae71fc5f6d8abf45c Mon Sep 17 00:00:00 2001 From: Pali Rohár Date: Tue, 12 Apr 2022 11:49:45 +0200 Subject: PCI: Add function for parsing 'slot-power-limit-milliwatt' DT property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add function of_pci_get_slot_power_limit(), which parses the 'slot-power-limit-milliwatt' DT property, returning the value in milliwatts and in format ready for the PCIe Slot Capabilities Register. Link: https://lore.kernel.org/r/20220412094946.27069-4-pali@kernel.org Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Signed-off-by: Lorenzo Pieralisi Reviewed-by: Rob Herring Reviewed-by: Bjorn Helgaas --- drivers/pci/of.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci.h | 15 ++++++++++++ 2 files changed, 85 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/of.c b/drivers/pci/of.c index cb2e8351c2cc..6c1b81304665 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -633,3 +633,73 @@ int of_pci_get_max_link_speed(struct device_node *node) return max_link_speed; } EXPORT_SYMBOL_GPL(of_pci_get_max_link_speed); + +/** + * of_pci_get_slot_power_limit - Parses the "slot-power-limit-milliwatt" + * property. + * + * @node: device tree node with the slot power limit information + * @slot_power_limit_value: pointer where the value should be stored in PCIe + * Slot Capabilities Register format + * @slot_power_limit_scale: pointer where the scale should be stored in PCIe + * Slot Capabilities Register format + * + * Returns the slot power limit in milliwatts and if @slot_power_limit_value + * and @slot_power_limit_scale pointers are non-NULL, fills in the value and + * scale in format used by PCIe Slot Capabilities Register. + * + * If the property is not found or is invalid, returns 0. + */ +u32 of_pci_get_slot_power_limit(struct device_node *node, + u8 *slot_power_limit_value, + u8 *slot_power_limit_scale) +{ + u32 slot_power_limit_mw; + u8 value, scale; + + if (of_property_read_u32(node, "slot-power-limit-milliwatt", + &slot_power_limit_mw)) + slot_power_limit_mw = 0; + + /* Calculate Slot Power Limit Value and Slot Power Limit Scale */ + if (slot_power_limit_mw == 0) { + value = 0x00; + scale = 0; + } else if (slot_power_limit_mw <= 255) { + value = slot_power_limit_mw; + scale = 3; + } else if (slot_power_limit_mw <= 255*10) { + value = slot_power_limit_mw / 10; + scale = 2; + slot_power_limit_mw = slot_power_limit_mw / 10 * 10; + } else if (slot_power_limit_mw <= 255*100) { + value = slot_power_limit_mw / 100; + scale = 1; + slot_power_limit_mw = slot_power_limit_mw / 100 * 100; + } else if (slot_power_limit_mw <= 239*1000) { + value = slot_power_limit_mw / 1000; + scale = 0; + slot_power_limit_mw = slot_power_limit_mw / 1000 * 1000; + } else if (slot_power_limit_mw < 250*1000) { + value = 0xEF; + scale = 0; + slot_power_limit_mw = 239*1000; + } else if (slot_power_limit_mw <= 600*1000) { + value = 0xF0 + (slot_power_limit_mw / 1000 - 250) / 25; + scale = 0; + slot_power_limit_mw = slot_power_limit_mw / (1000*25) * (1000*25); + } else { + value = 0xFE; + scale = 0; + slot_power_limit_mw = 600*1000; + } + + if (slot_power_limit_value) + *slot_power_limit_value = value; + + if (slot_power_limit_scale) + *slot_power_limit_scale = scale; + + return slot_power_limit_mw; +} +EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 3d60cabde1a1..e10cdec6c56e 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -627,6 +627,9 @@ struct device_node; int of_pci_parse_bus_range(struct device_node *node, struct resource *res); int of_get_pci_domain_nr(struct device_node *node); int of_pci_get_max_link_speed(struct device_node *node); +u32 of_pci_get_slot_power_limit(struct device_node *node, + u8 *slot_power_limit_value, + u8 *slot_power_limit_scale); void pci_set_of_node(struct pci_dev *dev); void pci_release_of_node(struct pci_dev *dev); void pci_set_bus_of_node(struct pci_bus *bus); @@ -653,6 +656,18 @@ of_pci_get_max_link_speed(struct device_node *node) return -EINVAL; } +static inline u32 +of_pci_get_slot_power_limit(struct device_node *node, + u8 *slot_power_limit_value, + u8 *slot_power_limit_scale) +{ + if (slot_power_limit_value) + *slot_power_limit_value = 0; + if (slot_power_limit_scale) + *slot_power_limit_scale = 0; + return 0; +} + static inline void pci_set_of_node(struct pci_dev *dev) { } static inline void pci_release_of_node(struct pci_dev *dev) { } static inline void pci_set_bus_of_node(struct pci_bus *bus) { } -- cgit v1.3.1 From 0d5b8c298545c827ca9f2461b2655277ce0aef79 Mon Sep 17 00:00:00 2001 From: Pali Rohár Date: Tue, 12 Apr 2022 11:49:46 +0200 Subject: PCI: mvebu: Add support for sending Set_Slot_Power_Limit message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If DT supplies the 'slot-power-limit-milliwatt' property, program the value in the Slot Power Limit in the Slot Capabilities register and program the Root Port to send a Set_Slot_Power_Limit Message when the Link transitions to DL_Up. Link: https://lore.kernel.org/r/20220412094946.27069-5-pali@kernel.org Signed-off-by: Pali Rohár Signed-off-by: Lorenzo Pieralisi Reviewed-by: Rob Herring --- drivers/pci/controller/pci-mvebu.c | 97 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 5 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c index 8f76d4bda356..c1ffdb06c971 100644 --- a/drivers/pci/controller/pci-mvebu.c +++ b/drivers/pci/controller/pci-mvebu.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -66,6 +67,12 @@ #define PCIE_STAT_BUS 0xff00 #define PCIE_STAT_DEV 0x1f0000 #define PCIE_STAT_LINK_DOWN BIT(0) +#define PCIE_SSPL_OFF 0x1a0c +#define PCIE_SSPL_VALUE_SHIFT 0 +#define PCIE_SSPL_VALUE_MASK GENMASK(7, 0) +#define PCIE_SSPL_SCALE_SHIFT 8 +#define PCIE_SSPL_SCALE_MASK GENMASK(9, 8) +#define PCIE_SSPL_ENABLE BIT(16) #define PCIE_RC_RTSTA 0x1a14 #define PCIE_DEBUG_CTRL 0x1a60 #define PCIE_DEBUG_SOFT_RESET BIT(20) @@ -111,6 +118,8 @@ struct mvebu_pcie_port { struct mvebu_pcie_window iowin; u32 saved_pcie_stat; struct resource regs; + u8 slot_power_limit_value; + u8 slot_power_limit_scale; struct irq_domain *intx_irq_domain; raw_spinlock_t irq_lock; int intx_irq; @@ -239,7 +248,7 @@ static void mvebu_pcie_setup_wins(struct mvebu_pcie_port *port) static void mvebu_pcie_setup_hw(struct mvebu_pcie_port *port) { - u32 ctrl, lnkcap, cmd, dev_rev, unmask; + u32 ctrl, lnkcap, cmd, dev_rev, unmask, sspl; /* Setup PCIe controller to Root Complex mode. */ ctrl = mvebu_readl(port, PCIE_CTRL_OFF); @@ -292,6 +301,20 @@ static void mvebu_pcie_setup_hw(struct mvebu_pcie_port *port) /* Point PCIe unit MBUS decode windows to DRAM space. */ mvebu_pcie_setup_wins(port); + /* + * Program Root Port to automatically send Set_Slot_Power_Limit + * PCIe Message when changing status from Dl_Down to Dl_Up and valid + * slot power limit was specified. + */ + sspl = mvebu_readl(port, PCIE_SSPL_OFF); + sspl &= ~(PCIE_SSPL_VALUE_MASK | PCIE_SSPL_SCALE_MASK | PCIE_SSPL_ENABLE); + if (port->slot_power_limit_value) { + sspl |= port->slot_power_limit_value << PCIE_SSPL_VALUE_SHIFT; + sspl |= port->slot_power_limit_scale << PCIE_SSPL_SCALE_SHIFT; + sspl |= PCIE_SSPL_ENABLE; + } + mvebu_writel(port, sspl, PCIE_SSPL_OFF); + /* Mask all interrupt sources. */ mvebu_writel(port, ~PCIE_INT_ALL_MASK, PCIE_INT_UNMASK_OFF); @@ -628,9 +651,24 @@ mvebu_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge, (PCI_EXP_LNKSTA_DLLLA << 16) : 0); break; - case PCI_EXP_SLTCTL: - *value = PCI_EXP_SLTSTA_PDS << 16; + case PCI_EXP_SLTCTL: { + u16 slotctl = le16_to_cpu(bridge->pcie_conf.slotctl); + u16 slotsta = le16_to_cpu(bridge->pcie_conf.slotsta); + u32 val = 0; + /* + * When slot power limit was not specified in DT then + * ASPL_DISABLE bit is stored only in emulated config space. + * Otherwise reflect status of PCIE_SSPL_ENABLE bit in HW. + */ + if (!port->slot_power_limit_value) + val |= slotctl & PCI_EXP_SLTCTL_ASPL_DISABLE; + else if (!(mvebu_readl(port, PCIE_SSPL_OFF) & PCIE_SSPL_ENABLE)) + val |= PCI_EXP_SLTCTL_ASPL_DISABLE; + /* This callback is 32-bit and in high bits is slot status. */ + val |= slotsta << 16; + *value = val; break; + } case PCI_EXP_RTSTA: *value = mvebu_readl(port, PCIE_RC_RTSTA); @@ -774,6 +812,22 @@ mvebu_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge, mvebu_writel(port, new, PCIE_CAP_PCIEXP + PCI_EXP_LNKCTL); break; + case PCI_EXP_SLTCTL: + /* + * Allow to change PCIE_SSPL_ENABLE bit only when slot power + * limit was specified in DT and configured into HW. + */ + if ((mask & PCI_EXP_SLTCTL_ASPL_DISABLE) && + port->slot_power_limit_value) { + u32 sspl = mvebu_readl(port, PCIE_SSPL_OFF); + if (new & PCI_EXP_SLTCTL_ASPL_DISABLE) + sspl &= ~PCIE_SSPL_ENABLE; + else + sspl |= PCIE_SSPL_ENABLE; + mvebu_writel(port, sspl, PCIE_SSPL_OFF); + } + break; + case PCI_EXP_RTSTA: /* * PME Status bit in Root Status Register (PCIE_RC_RTSTA) @@ -868,8 +922,26 @@ static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port) /* * Older mvebu hardware provides PCIe Capability structure only in * version 1. New hardware provides it in version 2. + * Enable slot support which is emulated. */ - bridge->pcie_conf.cap = cpu_to_le16(pcie_cap_ver); + bridge->pcie_conf.cap = cpu_to_le16(pcie_cap_ver | PCI_EXP_FLAGS_SLOT); + + /* + * Set Presence Detect State bit permanently as there is no support for + * unplugging PCIe card from the slot. Assume that PCIe card is always + * connected in slot. + * + * Set physical slot number to port+1 as mvebu ports are indexed from + * zero and zero value is reserved for ports within the same silicon + * as Root Port which is not mvebu case. + * + * Also set correct slot power limit. + */ + bridge->pcie_conf.slotcap = cpu_to_le32( + FIELD_PREP(PCI_EXP_SLTCAP_SPLV, port->slot_power_limit_value) | + FIELD_PREP(PCI_EXP_SLTCAP_SPLS, port->slot_power_limit_scale) | + FIELD_PREP(PCI_EXP_SLTCAP_PSN, port->port+1)); + bridge->pcie_conf.slotsta = cpu_to_le16(PCI_EXP_SLTSTA_PDS); bridge->subsystem_vendor_id = ssdev_id & 0xffff; bridge->subsystem_id = ssdev_id >> 16; @@ -1191,6 +1263,7 @@ static int mvebu_pcie_parse_port(struct mvebu_pcie *pcie, { struct device *dev = &pcie->pdev->dev; enum of_gpio_flags flags; + u32 slot_power_limit; int reset_gpio, ret; u32 num_lanes; @@ -1291,6 +1364,15 @@ static int mvebu_pcie_parse_port(struct mvebu_pcie *pcie, port->reset_gpio = gpio_to_desc(reset_gpio); } + slot_power_limit = of_pci_get_slot_power_limit(child, + &port->slot_power_limit_value, + &port->slot_power_limit_scale); + if (slot_power_limit) + dev_info(dev, "%s: Slot power limit %u.%uW\n", + port->name, + slot_power_limit / 1000, + (slot_power_limit / 100) % 10); + port->clk = of_clk_get_by_name(child, NULL); if (IS_ERR(port->clk)) { dev_err(dev, "%s: cannot get clock\n", port->name); @@ -1588,7 +1670,7 @@ static int mvebu_pcie_remove(struct platform_device *pdev) { struct mvebu_pcie *pcie = platform_get_drvdata(pdev); struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie); - u32 cmd; + u32 cmd, sspl; int i; /* Remove PCI bus with all devices. */ @@ -1625,6 +1707,11 @@ static int mvebu_pcie_remove(struct platform_device *pdev) /* Free config space for emulated root bridge. */ pci_bridge_emul_cleanup(&port->bridge); + /* Disable sending Set_Slot_Power_Limit PCIe Message. */ + sspl = mvebu_readl(port, PCIE_SSPL_OFF); + sspl &= ~(PCIE_SSPL_VALUE_MASK | PCIE_SSPL_SCALE_MASK | PCIE_SSPL_ENABLE); + mvebu_writel(port, sspl, PCIE_SSPL_OFF); + /* Disable and clear BARs and windows. */ mvebu_pcie_disable_wins(port); -- cgit v1.3.1 From c049b4b37685ff5b179a7e062b919c31eb406214 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Wed, 20 Apr 2022 08:58:32 +0200 Subject: PCI: microchip: Add a missing semicolon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the driver is configured as a module (after allowing this by changing PCIE_MICROCHIP_HOST from bool to tristate) the missing semicolon makes the compiler very unhappy. While there isn't a real problem as MODULE_DEVICE_TABLE always evaluates to nothing for a built-in driver, do it right for consistency with other drivers. Link: https://lore.kernel.org/r/20220420065832.14173-1-u.kleine-koenig@pengutronix.de Signed-off-by: Uwe Kleine-König Signed-off-by: Lorenzo Pieralisi Acked-by: Daire McNamara --- drivers/pci/controller/pcie-microchip-host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c index 29d8e81e4181..4b1e130f88a3 100644 --- a/drivers/pci/controller/pcie-microchip-host.c +++ b/drivers/pci/controller/pcie-microchip-host.c @@ -1115,7 +1115,7 @@ static const struct of_device_id mc_pcie_of_match[] = { {}, }; -MODULE_DEVICE_TABLE(of, mc_pcie_of_match) +MODULE_DEVICE_TABLE(of, mc_pcie_of_match); static struct platform_driver mc_pcie_driver = { .probe = pci_host_common_probe, -- cgit v1.3.1 From 6086987bdeb5910778e6488b1cd6801701b4ef91 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 18 Apr 2022 15:44:16 +0100 Subject: PCI: versatile: Remove redundant variable retval Variable retval is being assigned a value that is never read, the variable is redundant and can be removed. Cleans up clang scan build warning: drivers/pci/controller/pci-versatile.c:37:10: warning: Although the value stored to 'retval' is used in the enclosing expression, the value is never actually read from 'retval' [deadcode.DeadStores] Link: https://lore.kernel.org/r/20220418144416.86121-1-colin.i.king@gmail.com Signed-off-by: Colin Ian King Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/pci-versatile.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/pci-versatile.c b/drivers/pci/controller/pci-versatile.c index 653d5d0ecf81..7991d334e0f1 100644 --- a/drivers/pci/controller/pci-versatile.c +++ b/drivers/pci/controller/pci-versatile.c @@ -31,10 +31,9 @@ static u32 pci_slot_ignore; static int __init versatile_pci_slot_ignore(char *str) { - int retval; int slot; - while ((retval = get_option(&str, &slot))) { + while (get_option(&str, &slot)) { if ((slot < 0) || (slot > 31)) pr_err("Illegal slot value: %d\n", slot); else -- cgit v1.3.1 From 18a94192e20de31e7e495d7c805c8930c42e99ef Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Wed, 20 Apr 2022 16:11:35 +0200 Subject: PCI/PM: Define pci_restore_standard_config() only for CONFIG_PM_SLEEP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pci_restore_standard_config() was defined under CONFIG_PM but called only by pci_pm_resume() (defined under CONFIG_SUSPEND) and pci_pm_restore() (defined under CONFIG_HIBERNATE_CALLBACKS). A configuration with only CONFIG_PM leads to a warning: drivers/pci/pci-driver.c:533:12: error: ‘pci_restore_standard_config’ defined but not used [-Werror=unused-function] CONFIG_PM_SLEEP depends on CONFIG_SUSPEND and CONFIG_HIBERNATE_CALLBACKS, so define pci_restore_standard_config() under that instead. Link: https://lore.kernel.org/r/20220420141135.444820-1-krzysztof.kozlowski@linaro.org Signed-off-by: Krzysztof Kozlowski Signed-off-by: Bjorn Helgaas --- drivers/pci/pci-driver.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 4ceeb75fc899..033ddb038170 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -522,9 +522,9 @@ static void pci_device_shutdown(struct device *dev) pci_clear_master(pci_dev); } -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP -/* Auxiliary functions used for system resume and run-time resume. */ +/* Auxiliary functions used for system resume */ /** * pci_restore_standard_config - restore standard config registers of PCI device @@ -544,6 +544,11 @@ static int pci_restore_standard_config(struct pci_dev *pci_dev) pci_pme_restore(pci_dev); return 0; } +#endif /* CONFIG_PM_SLEEP */ + +#ifdef CONFIG_PM + +/* Auxiliary functions used for system resume and run-time resume */ static void pci_pm_default_resume(struct pci_dev *pci_dev) { @@ -551,10 +556,6 @@ static void pci_pm_default_resume(struct pci_dev *pci_dev) pci_enable_wake(pci_dev, PCI_D0, false); } -#endif - -#ifdef CONFIG_PM_SLEEP - static void pci_pm_default_resume_early(struct pci_dev *pci_dev) { pci_power_up(pci_dev); @@ -563,6 +564,10 @@ static void pci_pm_default_resume_early(struct pci_dev *pci_dev) pci_pme_restore(pci_dev); } +#endif /* CONFIG_PM */ + +#ifdef CONFIG_PM_SLEEP + /* * Default "suspend" method for devices that have no driver provided suspend, * or not even a driver at all (second part). -- cgit v1.3.1 From 9a6058312ea941bac3b0ba8c963d7543fc42a288 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 8 Apr 2022 20:29:01 +0200 Subject: PCI/PM: Power up all devices during runtime resume Currently, endpoint devices may not be powered up entirely during runtime resume that follows a D3hot -> D0 transition of the parent bridge. Namely, even if the power state of an endpoint device, as indicated by its PCI_PM_CTRL register, is D0 after powering up its parent bridge, it may be still necessary to bring its ACPI companion into D0 and that should be done before accessing it. However, the current code assumes that reading the PCI_PM_CTRL register is sufficient to establish the endpoint device's power state, which may lead to problems. Address that by forcing a power-up of all PCI devices, including the platform firmware part of it, during runtime resume. Link: https://lore.kernel.org/linux-pm/11967527.O9o76ZdvQC@kreacher Fixes: 5775b843a619 ("PCI: Restore config space on runtime resume despite being unbound") Link: https://lore.kernel.org/r/2652115.mvXUDI8C0e@kreacher Reported-by: Abhishek Sahu Tested-by: Abhishek Sahu Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci-driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 033ddb038170..608ce87d0d48 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -1317,7 +1317,7 @@ static int pci_pm_runtime_resume(struct device *dev) * to a driver because although we left it in D0, it may have gone to * D3cold when the bridge above it runtime suspended. */ - pci_restore_standard_config(pci_dev); + pci_pm_default_resume_early(pci_dev); if (!pci_dev->driver) return 0; -- cgit v1.3.1 From 730643d33e2d9ae52d974201b5017d8c3efe5aa5 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 14 Apr 2022 15:04:13 +0200 Subject: PCI/PM: Resume subordinate bus in bus type callbacks Calling pci_resume_bus() on the secondary bus from pci_power_up() as it is done now is questionable, because it depends on the mandatory bridge power-up delays that are only covered by the PCI bus type PM callbacks. For this reason, move the subordinate bus resume to those callbacks too and use the observation that if a bridge is being powered-up during resume from system-wide suspend, it may be still desirable to runtime-resume its subordinate bus after completing the system-wide transition (in case the resume of the devices on that bus is skipped during it). Link: https://lore.kernel.org/r/3190097.aeNJFYEL58@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci-driver.c | 15 +++++++++++++-- drivers/pci/pci.c | 15 --------------- 2 files changed, 13 insertions(+), 17 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 608ce87d0d48..ee5868810a5b 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -564,6 +564,17 @@ static void pci_pm_default_resume_early(struct pci_dev *pci_dev) pci_pme_restore(pci_dev); } +static void pci_pm_bridge_power_up_actions(struct pci_dev *pci_dev) +{ + pci_bridge_wait_for_secondary_bus(pci_dev); + /* + * When powering on a bridge from D3cold, the whole hierarchy may be + * powered on into D0uninitialized state, resume them to give them a + * chance to suspend again + */ + pci_resume_bus(pci_dev->subordinate); +} + #endif /* CONFIG_PM */ #ifdef CONFIG_PM_SLEEP @@ -939,7 +950,7 @@ static int pci_pm_resume_noirq(struct device *dev) pcie_pme_root_status_cleanup(pci_dev); if (!skip_bus_pm && prev_state == PCI_D3cold) - pci_bridge_wait_for_secondary_bus(pci_dev); + pci_pm_bridge_power_up_actions(pci_dev); if (pci_has_legacy_pm_support(pci_dev)) return 0; @@ -1326,7 +1337,7 @@ static int pci_pm_runtime_resume(struct device *dev) pci_pm_default_resume(pci_dev); if (prev_state == PCI_D3cold) - pci_bridge_wait_for_secondary_bus(pci_dev); + pci_pm_bridge_power_up_actions(pci_dev); if (pm && pm->runtime_resume) error = pm->runtime_resume(dev); diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 9ecce435fb3f..e6c7f48e8b07 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1310,21 +1310,6 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout) int pci_power_up(struct pci_dev *dev) { pci_platform_power_transition(dev, PCI_D0); - - /* - * Mandatory power management transition delays are handled in - * pci_pm_resume_noirq() and pci_pm_runtime_resume() of the - * corresponding bridge. - */ - if (dev->runtime_d3cold) { - /* - * When powering on a bridge from D3cold, the whole hierarchy - * may be powered on into D0uninitialized state, resume them to - * give them a chance to suspend again - */ - pci_resume_bus(dev->subordinate); - } - return pci_raw_set_power_state(dev, PCI_D0); } -- cgit v1.3.1 From 8221ecd4e4620cf2f0e942cafcdecac1685f8f16 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 14 Apr 2022 15:04:27 +0200 Subject: PCI/PM: Drop the runtime_d3cold device flag The runtime_d3cold flag is not needed any more, so drop it. Link: https://lore.kernel.org/r/8077784.T7Z3S40VBb@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci-driver.c | 2 -- drivers/pci/pci.c | 3 --- include/linux/pci.h | 4 ---- 3 files changed, 9 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index ee5868810a5b..d76fab66a9c9 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -1342,8 +1342,6 @@ static int pci_pm_runtime_resume(struct device *dev) if (pm && pm->runtime_resume) error = pm->runtime_resume(dev); - pci_dev->runtime_d3cold = false; - return error; } diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e6c7f48e8b07..992417646128 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2703,8 +2703,6 @@ int pci_finish_runtime_suspend(struct pci_dev *dev) if (target_state == PCI_POWER_ERROR) return -EIO; - dev->runtime_d3cold = target_state == PCI_D3cold; - /* * There are systems (for example, Intel mobile chips since Coffee * Lake) where the power drawn while suspended can be significantly @@ -2722,7 +2720,6 @@ int pci_finish_runtime_suspend(struct pci_dev *dev) if (error) { pci_enable_wake(dev, target_state, false); pci_restore_ptm_state(dev); - dev->runtime_d3cold = false; } return error; diff --git a/include/linux/pci.h b/include/linux/pci.h index 60adf42460ab..3266ac08f8ec 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -379,10 +379,6 @@ struct pci_dev { unsigned int mmio_always_on:1; /* Disallow turning off io/mem decoding during BAR sizing */ unsigned int wakeup_prepared:1; - unsigned int runtime_d3cold:1; /* Whether go through runtime - D3cold, not set for devices - powered on/off by the - corresponding bridge */ unsigned int skip_bus_pm:1; /* Internal: Skip bus-level PM */ unsigned int ignore_hotplug:1; /* Ignore hotplug events */ unsigned int hotplug_user_indicators:1; /* SlotCtl indicators -- cgit v1.3.1 From 9c384ddd6eb2ee90d19cd01128748dec85fa36e3 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 14 Apr 2022 15:07:24 +0200 Subject: PCI/PM: Rearrange pci_update_current_state() Save one config space access in pci_update_current_state() by testing the retrieved PCI_PM_CTRL register value against PCI_POSSIBLE_ERROR() instead of invoking pci_device_is_present() separately. While at it, drop a pair of unnecessary parens. No expected functional impact. Link: https://lore.kernel.org/r/1917095.PYKUYFuaPT@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 992417646128..11f95ca84b22 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1201,14 +1201,17 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) */ void pci_update_current_state(struct pci_dev *dev, pci_power_t state) { - if (platform_pci_get_power_state(dev) == PCI_D3cold || - !pci_device_is_present(dev)) { + if (platform_pci_get_power_state(dev) == PCI_D3cold) { dev->current_state = PCI_D3cold; } else if (dev->pm_cap) { u16 pmcsr; pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); - dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK); + if (PCI_POSSIBLE_ERROR(pmcsr)) { + dev->current_state = PCI_D3cold; + return; + } + dev->current_state = pmcsr & PCI_PM_CTRL_STATE_MASK; } else { dev->current_state = state; } -- cgit v1.3.1 From 10aa5377fc8ae0a259692dd9b20593a79567e0ef Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:00:33 +0200 Subject: PCI/PM: Split pci_raw_set_power_state() The transitions from low-power states to D0 and the other way around are unnecessarily tangled in pci_raw_set_power_state() which makes it rather hard to follow. Moreover, the only caller of pci_raw_set_power_state() passing PCI_D0 as its state argument is pci_power_up(), so the code carrying out transitions into D0 can be put directly into that function. Accordingly, move the code handling transitions from low-power states into D0 directly into pci_power_up() and rename the remaining part of pci_raw_set_power_state() to pci_set_low_power_state(), because it only handles transitions into low-power state now. While at it, fix up some white space, update some comments and modify messages printed by pci_power_up() and pci_set_low_power_state() to be less confusing (which is the only expected functional impact of this change). Link: https://lore.kernel.org/r/13038676.uLZWGnKmhe@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 143 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 83 insertions(+), 60 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 11f95ca84b22..b5a8c798f4db 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1068,10 +1068,11 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev) } /** - * pci_raw_set_power_state - Use PCI PM registers to set the power state of - * given PCI device + * pci_set_low_power_state - Put a PCI device into a low-power state. * @dev: PCI device to handle. - * @state: PCI power state (D0, D1, D2, D3hot) to put the device into. + * @state: PCI power state (D1, D2, D3hot) to put the device into. + * + * Use the device's PCI_PM_CTRL register to put it into a low-power state. * * RETURN VALUE: * -EINVAL if the requested state is invalid. @@ -1080,10 +1081,9 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev) * 0 if device already is in the requested state. * 0 if device's power state has been successfully changed. */ -static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) +static int pci_set_low_power_state(struct pci_dev *dev, pci_power_t state) { u16 pmcsr; - bool need_restore = false; /* Check if we're already there */ if (dev->current_state == state) @@ -1092,7 +1092,7 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) if (!dev->pm_cap) return -EIO; - if (state < PCI_D0 || state > PCI_D3hot) + if (state < PCI_D1 || state > PCI_D3hot) return -EINVAL; /* @@ -1101,8 +1101,7 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) * we can go from D1 to D3, but we can't go directly from D3 to D1; * we'd have to go from D3 to D0, then to D1. */ - if (state != PCI_D0 && dev->current_state <= PCI_D3cold - && dev->current_state > state) { + if (dev->current_state <= PCI_D3cold && dev->current_state > state) { pci_err(dev, "invalid power transition (from %s to %s)\n", pci_power_name(dev->current_state), pci_power_name(state)); @@ -1116,70 +1115,30 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); if (PCI_POSSIBLE_ERROR(pmcsr)) { - pci_err(dev, "can't change power state from %s to %s (config space inaccessible)\n", + pci_err(dev, "Unable to change power state from %s to %s, device inaccessible\n", pci_power_name(dev->current_state), pci_power_name(state)); return -EIO; } - /* - * If we're (effectively) in D3, force entire word to 0. - * This doesn't affect PME_Status, disables PME_En, and - * sets PowerState to 0. - */ - switch (dev->current_state) { - case PCI_D0: - case PCI_D1: - case PCI_D2: - pmcsr &= ~PCI_PM_CTRL_STATE_MASK; - pmcsr |= state; - break; - case PCI_D3hot: - case PCI_D3cold: - case PCI_UNKNOWN: /* Boot-up */ - if ((pmcsr & PCI_PM_CTRL_STATE_MASK) == PCI_D3hot - && !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET)) - need_restore = true; - fallthrough; /* force to D0 */ - default: - pmcsr = 0; - break; - } + pmcsr &= ~PCI_PM_CTRL_STATE_MASK; + pmcsr |= state; /* Enter specified state */ pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr); - /* - * Mandatory power management transition delays; see PCI PM 1.1 - * 5.6.1 table 18 - */ - if (state == PCI_D3hot || dev->current_state == PCI_D3hot) + /* Mandatory power management transition delays; see PCI PM 1.2. */ + if (state == PCI_D3hot) pci_dev_d3_sleep(dev); - else if (state == PCI_D2 || dev->current_state == PCI_D2) + else if (state == PCI_D2) udelay(PCI_PM_D2_DELAY); pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); - dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK); + dev->current_state = pmcsr & PCI_PM_CTRL_STATE_MASK; if (dev->current_state != state) - pci_info_ratelimited(dev, "refused to change power state from %s to %s\n", - pci_power_name(dev->current_state), - pci_power_name(state)); - - /* - * According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT - * INTERFACE SPECIFICATION, REV. 1.2", a device transitioning - * from D3hot to D0 _may_ perform an internal reset, thereby - * going to "D0 Uninitialized" rather than "D0 Initialized". - * For example, at least some versions of the 3c905B and the - * 3c556B exhibit this behaviour. - * - * At least some laptop BIOSen (e.g. the Thinkpad T21) leave - * devices in a D3hot state at boot. Consequently, we need to - * restore at least the BARs so that the device will be - * accessible to its driver. - */ - if (need_restore) - pci_restore_bars(dev); + pci_info_ratelimited(dev, "Refused to change power state from %s to %s\n", + pci_power_name(dev->current_state), + pci_power_name(state)); if (dev->bus->self) pcie_aspm_pm_state_change(dev->bus->self); @@ -1312,8 +1271,72 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout) */ int pci_power_up(struct pci_dev *dev) { + bool need_restore = false; + u16 pmcsr; + pci_platform_power_transition(dev, PCI_D0); - return pci_raw_set_power_state(dev, PCI_D0); + + if (dev->current_state == PCI_D0) + return 0; + + if (!dev->pm_cap) + return -EIO; + + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); + if (PCI_POSSIBLE_ERROR(pmcsr)) { + pci_err(dev, "Unable to change power state from %s to D0, device inaccessible\n", + pci_power_name(dev->current_state)); + return -EIO; + } + + /* + * If we're (effectively) in D3, force entire word to 0. This doesn't + * affect PME_Status, disables PME_En, and sets PowerState to 0. + */ + if (dev->current_state >= PCI_D3hot) { + if ((pmcsr & PCI_PM_CTRL_STATE_MASK) == PCI_D3hot && + !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET)) + need_restore = true; + + pmcsr = 0; + } else { + pmcsr &= ~PCI_PM_CTRL_STATE_MASK; + } + + pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr); + + /* Mandatory transition delays; see PCI PM 1.2. */ + if (dev->current_state == PCI_D3hot) + pci_dev_d3_sleep(dev); + else if (dev->current_state == PCI_D2) + udelay(PCI_PM_D2_DELAY); + + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); + dev->current_state = pmcsr & PCI_PM_CTRL_STATE_MASK; + if (dev->current_state != PCI_D0) + pci_info_ratelimited(dev, "Refused to change power state from %s to D0\n", + pci_power_name(dev->current_state)); + + /* + * According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT + * INTERFACE SPECIFICATION, REV. 1.2", a device transitioning + * from D3hot to D0 _may_ perform an internal reset, thereby + * going to "D0 Uninitialized" rather than "D0 Initialized". + * For example, at least some versions of the 3c905B and the + * 3c556B exhibit this behaviour. + * + * At least some laptop BIOSen (e.g. the Thinkpad T21) leave + * devices in a D3hot state at boot. Consequently, we need to + * restore at least the BARs so that the device will be + * accessible to its driver. + */ + if (need_restore) + pci_restore_bars(dev); + + if (dev->bus->self) + pcie_aspm_pm_state_change(dev->bus->self); + + return 0; } /** @@ -1394,7 +1417,7 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state) * To put device in D3cold, we put device into D3hot in native * way, then put device into D3cold with platform ops */ - error = pci_raw_set_power_state(dev, state > PCI_D3hot ? + error = pci_set_low_power_state(dev, state > PCI_D3hot ? PCI_D3hot : state); if (pci_platform_power_transition(dev, state)) -- cgit v1.3.1 From 7957d201456f436557870cf8bbd47328a280c522 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:02:52 +0200 Subject: PCI/PM: Relocate pci_set_low_power_state() Because pci_set_power_state() is the only caller of pci_set_low_power_state(), put the latter next to the former. No functional impact. Link: https://lore.kernel.org/r/3202976.44csPzL39Z@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 158 +++++++++++++++++++++++++++--------------------------- 1 file changed, 79 insertions(+), 79 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b5a8c798f4db..f1bd87c5aa14 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1067,85 +1067,6 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev) return acpi_pci_bridge_d3(dev); } -/** - * pci_set_low_power_state - Put a PCI device into a low-power state. - * @dev: PCI device to handle. - * @state: PCI power state (D1, D2, D3hot) to put the device into. - * - * Use the device's PCI_PM_CTRL register to put it into a low-power state. - * - * RETURN VALUE: - * -EINVAL if the requested state is invalid. - * -EIO if device does not support PCI PM or its PM capabilities register has a - * wrong version, or device doesn't support the requested state. - * 0 if device already is in the requested state. - * 0 if device's power state has been successfully changed. - */ -static int pci_set_low_power_state(struct pci_dev *dev, pci_power_t state) -{ - u16 pmcsr; - - /* Check if we're already there */ - if (dev->current_state == state) - return 0; - - if (!dev->pm_cap) - return -EIO; - - if (state < PCI_D1 || state > PCI_D3hot) - return -EINVAL; - - /* - * Validate transition: We can enter D0 from any state, but if - * we're already in a low-power state, we can only go deeper. E.g., - * we can go from D1 to D3, but we can't go directly from D3 to D1; - * we'd have to go from D3 to D0, then to D1. - */ - if (dev->current_state <= PCI_D3cold && dev->current_state > state) { - pci_err(dev, "invalid power transition (from %s to %s)\n", - pci_power_name(dev->current_state), - pci_power_name(state)); - return -EINVAL; - } - - /* Check if this device supports the desired state */ - if ((state == PCI_D1 && !dev->d1_support) - || (state == PCI_D2 && !dev->d2_support)) - return -EIO; - - pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); - if (PCI_POSSIBLE_ERROR(pmcsr)) { - pci_err(dev, "Unable to change power state from %s to %s, device inaccessible\n", - pci_power_name(dev->current_state), - pci_power_name(state)); - return -EIO; - } - - pmcsr &= ~PCI_PM_CTRL_STATE_MASK; - pmcsr |= state; - - /* Enter specified state */ - pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr); - - /* Mandatory power management transition delays; see PCI PM 1.2. */ - if (state == PCI_D3hot) - pci_dev_d3_sleep(dev); - else if (state == PCI_D2) - udelay(PCI_PM_D2_DELAY); - - pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); - dev->current_state = pmcsr & PCI_PM_CTRL_STATE_MASK; - if (dev->current_state != state) - pci_info_ratelimited(dev, "Refused to change power state from %s to %s\n", - pci_power_name(dev->current_state), - pci_power_name(state)); - - if (dev->bus->self) - pcie_aspm_pm_state_change(dev->bus->self); - - return 0; -} - /** * pci_update_current_state - Read power state of given device and cache it * @dev: PCI device to handle. @@ -1363,6 +1284,85 @@ void pci_bus_set_current_state(struct pci_bus *bus, pci_power_t state) pci_walk_bus(bus, __pci_dev_set_current_state, &state); } +/** + * pci_set_low_power_state - Put a PCI device into a low-power state. + * @dev: PCI device to handle. + * @state: PCI power state (D1, D2, D3hot) to put the device into. + * + * Use the device's PCI_PM_CTRL register to put it into a low-power state. + * + * RETURN VALUE: + * -EINVAL if the requested state is invalid. + * -EIO if device does not support PCI PM or its PM capabilities register has a + * wrong version, or device doesn't support the requested state. + * 0 if device already is in the requested state. + * 0 if device's power state has been successfully changed. + */ +static int pci_set_low_power_state(struct pci_dev *dev, pci_power_t state) +{ + u16 pmcsr; + + /* Check if we're already there */ + if (dev->current_state == state) + return 0; + + if (!dev->pm_cap) + return -EIO; + + if (state < PCI_D1 || state > PCI_D3hot) + return -EINVAL; + + /* + * Validate transition: We can enter D0 from any state, but if + * we're already in a low-power state, we can only go deeper. E.g., + * we can go from D1 to D3, but we can't go directly from D3 to D1; + * we'd have to go from D3 to D0, then to D1. + */ + if (dev->current_state <= PCI_D3cold && dev->current_state > state) { + pci_err(dev, "invalid power transition (from %s to %s)\n", + pci_power_name(dev->current_state), + pci_power_name(state)); + return -EINVAL; + } + + /* Check if this device supports the desired state */ + if ((state == PCI_D1 && !dev->d1_support) + || (state == PCI_D2 && !dev->d2_support)) + return -EIO; + + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); + if (PCI_POSSIBLE_ERROR(pmcsr)) { + pci_err(dev, "Unable to change power state from %s to %s, device inaccessible\n", + pci_power_name(dev->current_state), + pci_power_name(state)); + return -EIO; + } + + pmcsr &= ~PCI_PM_CTRL_STATE_MASK; + pmcsr |= state; + + /* Enter specified state */ + pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr); + + /* Mandatory power management transition delays; see PCI PM 1.2. */ + if (state == PCI_D3hot) + pci_dev_d3_sleep(dev); + else if (state == PCI_D2) + udelay(PCI_PM_D2_DELAY); + + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); + dev->current_state = pmcsr & PCI_PM_CTRL_STATE_MASK; + if (dev->current_state != state) + pci_info_ratelimited(dev, "Refused to change power state from %s to %s\n", + pci_power_name(dev->current_state), + pci_power_name(state)); + + if (dev->bus->self) + pcie_aspm_pm_state_change(dev->bus->self); + + return 0; +} + /** * pci_set_power_state - Set the power state of a PCI device * @dev: PCI device to handle. -- cgit v1.3.1 From 1aa85bb14d8ed0ae4238617061924032c80dad37 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:04:07 +0200 Subject: PCI/PM: Set current_state to D3cold if the device is not accessible Make pci_power_up() and pci_set_low_power_state() change current_state to PCI_D3cold when the device is not accessible along the lines of pci_update_current_state(). Link: https://lore.kernel.org/r/10104376.nUPlyArG6x@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index f1bd87c5aa14..056e8284b5fd 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1207,6 +1207,7 @@ int pci_power_up(struct pci_dev *dev) if (PCI_POSSIBLE_ERROR(pmcsr)) { pci_err(dev, "Unable to change power state from %s to D0, device inaccessible\n", pci_power_name(dev->current_state)); + dev->current_state = PCI_D3cold; return -EIO; } @@ -1335,6 +1336,7 @@ static int pci_set_low_power_state(struct pci_dev *dev, pci_power_t state) pci_err(dev, "Unable to change power state from %s to %s, device inaccessible\n", pci_power_name(dev->current_state), pci_power_name(state)); + dev->current_state = PCI_D3cold; return -EIO; } -- cgit v1.3.1 From 6d8c016a55aca612802fc325eb0e659d1c5f255d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:05:15 +0200 Subject: PCI/PM: Unfold pci_platform_power_transition() in pci_power_up() Some actions carried out by pci_platform_power_transition(() in pci_power_up() are redundant, but before dealing with them, make pci_power_up() call the pci_platform_power_transition() code directly (and avoid a redundant check when pm_cap is unset while at it). Link: https://lore.kernel.org/r/1922486.PYKUYFuaPT@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 056e8284b5fd..bc4af86da3f9 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1194,8 +1194,15 @@ int pci_power_up(struct pci_dev *dev) { bool need_restore = false; u16 pmcsr; + int ret; - pci_platform_power_transition(dev, PCI_D0); + ret = platform_pci_set_power_state(dev, PCI_D0); + if (!ret) { + pci_update_current_state(dev, PCI_D0); + } else if (!dev->pm_cap) { /* Fall back to PCI_D0 */ + dev->current_state = PCI_D0; + return 0; + } if (dev->current_state == PCI_D0) return 0; -- cgit v1.3.1 From 0b59193548e63957101aae5e4fc47151fce4a629 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:09:12 +0200 Subject: PCI/PM: Do not call pci_update_current_state() from pci_power_up() Notice that calling pci_update_current_state() from pci_power_up() is redundant and may be harmful in some cases. First, if the device is in a low-power state before pci_power_up() gets called for it and platform_pci_set_power_state() successfully changes its power state to D0, pci_update_current_state() will update current_state to reflect that and pci_power_up() will return success right away without restoring the device's BARs or reconfiguring ASPM which may be necessary. This is arguably incorrect and definitely inconsistent with the case when platform_pci_set_power_state() returns an error (for example, because the device is not power-manageable by the platform firmware). Second, current_state should not be overwritten until the decision whether or not to restore the device's BARs is made, because that decision generally depends on its value. Again, calling pci_update_current_state() in pci_power_up() is not consistent with this observation. Next, pci_power_up() attempts to read from the device's PCI_PM_CTRL register regardless of the current_state value unless it is PCI_D0, including the case when pci_update_current_state() sets current_state to PCI_D3cold to indicate that the device is not accessible. If the register read is not successful, current_state will be set to PCI_D3cold anyway, so that pci_update_current_state() action is redundant. Further, if pci_update_current_state() reads the device's PCI_PM_CTRL register, pci_power_up() will repeat that read going forward and it is not necessary to update current_state in the meantime. Finally, if pm_cap is not set (in which case the PCI_PM_CTRL register is not present), the power state of the device should be determined with the help of the platform firmware or set to D0 if that's not possible and pci_update_current_state() does not do that. Accordingly, rearrange pci_power_up() so as to address the above shortcomings. Link: https://lore.kernel.org/r/3695055.kQq0lBPeGt@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index bc4af86da3f9..a5b93f85377a 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1192,23 +1192,24 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout) */ int pci_power_up(struct pci_dev *dev) { - bool need_restore = false; + bool need_restore; + pci_power_t state; u16 pmcsr; - int ret; - ret = platform_pci_set_power_state(dev, PCI_D0); - if (!ret) { - pci_update_current_state(dev, PCI_D0); - } else if (!dev->pm_cap) { /* Fall back to PCI_D0 */ - dev->current_state = PCI_D0; - return 0; - } + platform_pci_set_power_state(dev, PCI_D0); - if (dev->current_state == PCI_D0) - return 0; + if (!dev->pm_cap) { + state = platform_pci_get_power_state(dev); + if (state == PCI_UNKNOWN) + dev->current_state = PCI_D0; + else + dev->current_state = state; + + if (state == PCI_D0) + return 0; - if (!dev->pm_cap) return -EIO; + } pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); if (PCI_POSSIBLE_ERROR(pmcsr)) { @@ -1218,26 +1219,31 @@ int pci_power_up(struct pci_dev *dev) return -EIO; } + state = pmcsr & PCI_PM_CTRL_STATE_MASK; + + need_restore = (state == PCI_D3hot || dev->current_state >= PCI_D3hot) && + !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET); + + if (state == PCI_D0) { + dev->current_state = PCI_D0; + goto end; + } + /* * If we're (effectively) in D3, force entire word to 0. This doesn't * affect PME_Status, disables PME_En, and sets PowerState to 0. */ - if (dev->current_state >= PCI_D3hot) { - if ((pmcsr & PCI_PM_CTRL_STATE_MASK) == PCI_D3hot && - !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET)) - need_restore = true; - + if (state == PCI_D3hot) pmcsr = 0; - } else { + else pmcsr &= ~PCI_PM_CTRL_STATE_MASK; - } pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr); /* Mandatory transition delays; see PCI PM 1.2. */ - if (dev->current_state == PCI_D3hot) + if (state == PCI_D3hot) pci_dev_d3_sleep(dev); - else if (dev->current_state == PCI_D2) + else if (state == PCI_D2) udelay(PCI_PM_D2_DELAY); pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); @@ -1246,6 +1252,7 @@ int pci_power_up(struct pci_dev *dev) pci_info_ratelimited(dev, "Refused to change power state from %s to D0\n", pci_power_name(dev->current_state)); +end: /* * According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT * INTERFACE SPECIFICATION, REV. 1.2", a device transitioning -- cgit v1.3.1 From f0881d38c7eca3351f59d551604aaf74283c2e13 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:10:43 +0200 Subject: PCI/PM: Write 0 to PMCSR in pci_power_up() in all cases Make pci_power_up() write 0 to the device's PCI_PM_CTRL register in order to put it into D0 regardless of the power state returned by the previous read from that register which should not matter. Link: https://lore.kernel.org/r/5748066.MhkbZ0Pkbq@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index a5b93f85377a..5cce2cae0933 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1230,15 +1230,10 @@ int pci_power_up(struct pci_dev *dev) } /* - * If we're (effectively) in D3, force entire word to 0. This doesn't - * affect PME_Status, disables PME_En, and sets PowerState to 0. + * Force the entire word to 0. This doesn't affect PME_Status, disables + * PME_En, and sets PowerState to 0. */ - if (state == PCI_D3hot) - pmcsr = 0; - else - pmcsr &= ~PCI_PM_CTRL_STATE_MASK; - - pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr); + pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, 0); /* Mandatory transition delays; see PCI PM 1.2. */ if (state == PCI_D3hot) -- cgit v1.3.1 From e200904b275c63dae711fca542f5fb20d162eb26 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:13:00 +0200 Subject: PCI/PM: Split pci_power_up() One of the two callers of pci_power_up() invokes pci_update_current_state() and pci_restore_state() right after calling it, in which case running the part of it happening after the mandatory transition delays is redundant, so move that part out of it into a new function called pci_set_full_power_state() that will be invoked from pci_set_power_state() which is the other caller of pci_power_up(). Link: https://lore.kernel.org/r/1942150.usQuhbGJ8B@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 5cce2cae0933..44dcc848ff84 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1189,6 +1189,9 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout) /** * pci_power_up - Put the given device into D0 * @dev: PCI device to power up + * + * On success, return 0 or 1, depending on whether or not it is necessary to + * restore the device's BARs subsequently (1 is returned in that case). */ int pci_power_up(struct pci_dev *dev) { @@ -1224,10 +1227,8 @@ int pci_power_up(struct pci_dev *dev) need_restore = (state == PCI_D3hot || dev->current_state >= PCI_D3hot) && !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET); - if (state == PCI_D0) { - dev->current_state = PCI_D0; + if (state == PCI_D0) goto end; - } /* * Force the entire word to 0. This doesn't affect PME_Status, disables @@ -1241,13 +1242,41 @@ int pci_power_up(struct pci_dev *dev) else if (state == PCI_D2) udelay(PCI_PM_D2_DELAY); +end: + dev->current_state = PCI_D0; + if (need_restore) + return 1; + + return 0; +} + +/** + * pci_set_full_power_state - Put a PCI device into D0 and update its state + * @dev: PCI device to power up + * + * Call pci_power_up() to put @dev into D0, read from its PCI_PM_CTRL register + * to confirm the state change, restore its BARs if they might be lost and + * reconfigure ASPM in acordance with the new power state. + * + * If pci_restore_state() is going to be called right after a power state change + * to D0, it is more efficient to use pci_power_up() directly instead of this + * function. + */ +static int pci_set_full_power_state(struct pci_dev *dev) +{ + u16 pmcsr; + int ret; + + ret = pci_power_up(dev); + if (ret < 0) + return ret; + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); dev->current_state = pmcsr & PCI_PM_CTRL_STATE_MASK; if (dev->current_state != PCI_D0) pci_info_ratelimited(dev, "Refused to change power state from %s to D0\n", pci_power_name(dev->current_state)); -end: /* * According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT * INTERFACE SPECIFICATION, REV. 1.2", a device transitioning @@ -1261,7 +1290,7 @@ end: * restore at least the BARs so that the device will be * accessible to its driver. */ - if (need_restore) + if (ret > 0) pci_restore_bars(dev); if (dev->bus->self) @@ -1415,7 +1444,7 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state) return 0; if (state == PCI_D0) - return pci_power_up(dev); + return pci_set_full_power_state(dev); /* * This device is quirked not to be put into D3, so don't put it in -- cgit v1.3.1 From 0ce74a3b9c5255f641842df3c4c14fa8ea049a5a Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:14:24 +0200 Subject: PCI/PM: Do not restore BARs if device is not in D0 Do not attempt to restore the device's BARs in pci_set_full_power_state() if the actual current power state of the device is not D0. Link: https://lore.kernel.org/r/1849718.CQOukoFCf9@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 44dcc848ff84..3320453d2691 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1273,25 +1273,25 @@ static int pci_set_full_power_state(struct pci_dev *dev) pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); dev->current_state = pmcsr & PCI_PM_CTRL_STATE_MASK; - if (dev->current_state != PCI_D0) + if (dev->current_state != PCI_D0) { pci_info_ratelimited(dev, "Refused to change power state from %s to D0\n", pci_power_name(dev->current_state)); - - /* - * According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT - * INTERFACE SPECIFICATION, REV. 1.2", a device transitioning - * from D3hot to D0 _may_ perform an internal reset, thereby - * going to "D0 Uninitialized" rather than "D0 Initialized". - * For example, at least some versions of the 3c905B and the - * 3c556B exhibit this behaviour. - * - * At least some laptop BIOSen (e.g. the Thinkpad T21) leave - * devices in a D3hot state at boot. Consequently, we need to - * restore at least the BARs so that the device will be - * accessible to its driver. - */ - if (ret > 0) + } else if (ret > 0) { + /* + * According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT + * INTERFACE SPECIFICATION, REV. 1.2", a device transitioning + * from D3hot to D0 _may_ perform an internal reset, thereby + * going to "D0 Uninitialized" rather than "D0 Initialized". + * For example, at least some versions of the 3c905B and the + * 3c556B exhibit this behaviour. + * + * At least some laptop BIOSen (e.g. the Thinkpad T21) leave + * devices in a D3hot state at boot. Consequently, we need to + * restore at least the BARs so that the device will be + * accessible to its driver. + */ pci_restore_bars(dev); + } if (dev->bus->self) pcie_aspm_pm_state_change(dev->bus->self); -- cgit v1.3.1 From 0aacdc957401802bd2b94141a3d2c5f88c529e30 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:15:34 +0200 Subject: PCI/PM: Clean up pci_set_low_power_state() Make the following assorted non-essential changes in pci_set_low_power_state(): 1. Drop two redundant checks from it (the caller takes care of these conditions). 2. Change the log level of a messages printed by it to "debug", because it only indicates a programming mistake. Link: https://lore.kernel.org/r/2539071.Lt9SDvczpP@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 3320453d2691..b6ad2fa354f1 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1341,16 +1341,9 @@ static int pci_set_low_power_state(struct pci_dev *dev, pci_power_t state) { u16 pmcsr; - /* Check if we're already there */ - if (dev->current_state == state) - return 0; - if (!dev->pm_cap) return -EIO; - if (state < PCI_D1 || state > PCI_D3hot) - return -EINVAL; - /* * Validate transition: We can enter D0 from any state, but if * we're already in a low-power state, we can only go deeper. E.g., @@ -1358,7 +1351,7 @@ static int pci_set_low_power_state(struct pci_dev *dev, pci_power_t state) * we'd have to go from D3 to D0, then to D1. */ if (dev->current_state <= PCI_D3cold && dev->current_state > state) { - pci_err(dev, "invalid power transition (from %s to %s)\n", + pci_dbg(dev, "Invalid power transition (from %s to %s)\n", pci_power_name(dev->current_state), pci_power_name(state)); return -EINVAL; -- cgit v1.3.1 From 3cc2a2b2704f76702cdd417573a934502254276d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:16:50 +0200 Subject: PCI/PM: Rearrange pci_set_power_state() The part of pci_set_power_state() related to transitions into low-power states is unnecessary convoluted, so clearly divide it into the D3cold special case and the general case covering all of the other states. Also fix a potential issue with calling pci_bus_set_current_state() to set the current state of all devices on the subordinate bus to D3cold without checking if the power state of the parent bridge has really changed to D3cold. Link: https://lore.kernel.org/r/2139440.Mh6RI2rZIc@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b6ad2fa354f1..df886098bd60 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1446,19 +1446,25 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state) if (state >= PCI_D3hot && (dev->dev_flags & PCI_DEV_FLAGS_NO_D3)) return 0; - /* - * To put device in D3cold, we put device into D3hot in native - * way, then put device into D3cold with platform ops - */ - error = pci_set_low_power_state(dev, state > PCI_D3hot ? - PCI_D3hot : state); + if (state == PCI_D3cold) { + /* + * To put the device in D3cold, put it into D3hot in the native + * way, then put it into D3cold using platform ops. + */ + error = pci_set_low_power_state(dev, PCI_D3hot); - if (pci_platform_power_transition(dev, state)) - return error; + if (pci_platform_power_transition(dev, PCI_D3cold)) + return error; - /* Powering off a bridge may power off the whole hierarchy */ - if (state == PCI_D3cold) - pci_bus_set_current_state(dev->subordinate, PCI_D3cold); + /* Powering off a bridge may power off the whole hierarchy */ + if (dev->current_state == PCI_D3cold) + pci_bus_set_current_state(dev->subordinate, PCI_D3cold); + } else { + error = pci_set_low_power_state(dev, state); + + if (pci_platform_power_transition(dev, state)) + return error; + } return 0; } -- cgit v1.3.1 From 0f40ac35e4ecb16ab5bb672386a90e3cde13b186 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 May 2022 20:18:09 +0200 Subject: PCI/PM: Replace pci_set_power_state() in pci_pm_thaw_noirq() Calling pci_set_power_state() to put the given device into D0 in pci_pm_thaw_noirq() may cause it to restore the device's BARs, which is redundant before calling pci_restore_state(), so replace it with a direct pci_power_up() call followed by pci_update_current_state() if it returns a nonzero value, in analogy with pci_pm_default_resume_early(). Avoid code duplication by introducing a wrapper function to contain the repeating pattern and calling it in both places. Link: https://lore.kernel.org/r/3639079.MHq7AAxBmi@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas --- drivers/pci/pci-driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index d76fab66a9c9..2f3b69adfc9e 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -556,10 +556,15 @@ static void pci_pm_default_resume(struct pci_dev *pci_dev) pci_enable_wake(pci_dev, PCI_D0, false); } -static void pci_pm_default_resume_early(struct pci_dev *pci_dev) +static void pci_pm_power_up_and_verify_state(struct pci_dev *pci_dev) { pci_power_up(pci_dev); pci_update_current_state(pci_dev, PCI_D0); +} + +static void pci_pm_default_resume_early(struct pci_dev *pci_dev) +{ + pci_pm_power_up_and_verify_state(pci_dev); pci_restore_state(pci_dev); pci_pme_restore(pci_dev); } @@ -1084,7 +1089,7 @@ static int pci_pm_thaw_noirq(struct device *dev) * in case the driver's "freeze" callbacks put it into a low-power * state. */ - pci_set_power_state(pci_dev, PCI_D0); + pci_pm_power_up_and_verify_state(pci_dev); pci_restore_state(pci_dev); if (pci_has_legacy_pm_support(pci_dev)) -- cgit v1.3.1 From bc49681c96360e0ff9c6659e352eec3286b968ce Mon Sep 17 00:00:00 2001 From: Dmitry Baryshkov Date: Mon, 2 May 2022 16:19:38 +0530 Subject: PCI: qcom-ep: Move enable/disable resources code to common functions Remove code duplication by moving the code related to enabling/disabling the resources (PHY, CLK, Reset) to common functions so that they can be called from multiple places. [mani: renamed the functions and reworded the commit message] Link: https://lore.kernel.org/r/20220502104938.97033-1-manivannan.sadhasivam@linaro.org Signed-off-by: Dmitry Baryshkov Signed-off-by: Manivannan Sadhasivam Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/dwc/pcie-qcom-ep.c | 91 ++++++++++++++++--------------- 1 file changed, 46 insertions(+), 45 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c index 6ce8eddf3a37..ec99116ad05c 100644 --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c @@ -223,11 +223,8 @@ static void qcom_pcie_dw_stop_link(struct dw_pcie *pci) disable_irq(pcie_ep->perst_irq); } -static int qcom_pcie_perst_deassert(struct dw_pcie *pci) +static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep) { - struct qcom_pcie_ep *pcie_ep = to_pcie_ep(pci); - struct device *dev = pci->dev; - u32 val, offset; int ret; ret = clk_bulk_prepare_enable(ARRAY_SIZE(qcom_pcie_ep_clks), @@ -247,6 +244,38 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci) if (ret) goto err_phy_exit; + return 0; + +err_phy_exit: + phy_exit(pcie_ep->phy); +err_disable_clk: + clk_bulk_disable_unprepare(ARRAY_SIZE(qcom_pcie_ep_clks), + qcom_pcie_ep_clks); + + return ret; +} + +static void qcom_pcie_disable_resources(struct qcom_pcie_ep *pcie_ep) +{ + phy_power_off(pcie_ep->phy); + phy_exit(pcie_ep->phy); + clk_bulk_disable_unprepare(ARRAY_SIZE(qcom_pcie_ep_clks), + qcom_pcie_ep_clks); +} + +static int qcom_pcie_perst_deassert(struct dw_pcie *pci) +{ + struct qcom_pcie_ep *pcie_ep = to_pcie_ep(pci); + struct device *dev = pci->dev; + u32 val, offset; + int ret; + + ret = qcom_pcie_enable_resources(pcie_ep); + if (ret) { + dev_err(dev, "Failed to enable resources: %d\n", ret); + return ret; + } + /* Assert WAKE# to RC to indicate device is ready */ gpiod_set_value_cansleep(pcie_ep->wake, 1); usleep_range(WAKE_DELAY_US, WAKE_DELAY_US + 500); @@ -335,7 +364,7 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci) ret = dw_pcie_ep_init_complete(&pcie_ep->pci.ep); if (ret) { dev_err(dev, "Failed to complete initialization: %d\n", ret); - goto err_phy_power_off; + goto err_disable_resources; } /* @@ -355,13 +384,8 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci) return 0; -err_phy_power_off: - phy_power_off(pcie_ep->phy); -err_phy_exit: - phy_exit(pcie_ep->phy); -err_disable_clk: - clk_bulk_disable_unprepare(ARRAY_SIZE(qcom_pcie_ep_clks), - qcom_pcie_ep_clks); +err_disable_resources: + qcom_pcie_disable_resources(pcie_ep); return ret; } @@ -376,10 +400,7 @@ static void qcom_pcie_perst_assert(struct dw_pcie *pci) return; } - phy_power_off(pcie_ep->phy); - phy_exit(pcie_ep->phy); - clk_bulk_disable_unprepare(ARRAY_SIZE(qcom_pcie_ep_clks), - qcom_pcie_ep_clks); + qcom_pcie_disable_resources(pcie_ep); pcie_ep->link_status = QCOM_PCIE_EP_LINK_DISABLED; } @@ -643,43 +664,26 @@ static int qcom_pcie_ep_probe(struct platform_device *pdev) if (ret) return ret; - ret = clk_bulk_prepare_enable(ARRAY_SIZE(qcom_pcie_ep_clks), - qcom_pcie_ep_clks); - if (ret) + ret = qcom_pcie_enable_resources(pcie_ep); + if (ret) { + dev_err(dev, "Failed to enable resources: %d\n", ret); return ret; - - ret = qcom_pcie_ep_core_reset(pcie_ep); - if (ret) - goto err_disable_clk; - - ret = phy_init(pcie_ep->phy); - if (ret) - goto err_disable_clk; - - /* PHY needs to be powered on for dw_pcie_ep_init() */ - ret = phy_power_on(pcie_ep->phy); - if (ret) - goto err_phy_exit; + } ret = dw_pcie_ep_init(&pcie_ep->pci.ep); if (ret) { dev_err(dev, "Failed to initialize endpoint: %d\n", ret); - goto err_phy_power_off; + goto err_disable_resources; } ret = qcom_pcie_ep_enable_irq_resources(pdev, pcie_ep); if (ret) - goto err_phy_power_off; + goto err_disable_resources; return 0; -err_phy_power_off: - phy_power_off(pcie_ep->phy); -err_phy_exit: - phy_exit(pcie_ep->phy); -err_disable_clk: - clk_bulk_disable_unprepare(ARRAY_SIZE(qcom_pcie_ep_clks), - qcom_pcie_ep_clks); +err_disable_resources: + qcom_pcie_disable_resources(pcie_ep); return ret; } @@ -691,10 +695,7 @@ static int qcom_pcie_ep_remove(struct platform_device *pdev) if (pcie_ep->link_status == QCOM_PCIE_EP_LINK_DISABLED) return 0; - phy_power_off(pcie_ep->phy); - phy_exit(pcie_ep->phy); - clk_bulk_disable_unprepare(ARRAY_SIZE(qcom_pcie_ep_clks), - qcom_pcie_ep_clks); + qcom_pcie_disable_resources(pcie_ep); return 0; } -- cgit v1.3.1 From a6809941c1f17f455db2cf4ca19c6d8c8746ec25 Mon Sep 17 00:00:00 2001 From: Francesco Dolcini Date: Mon, 4 Apr 2022 10:15:09 +0200 Subject: PCI: imx6: Fix PERST# start-up sequence According to the PCIe standard the PERST# signal (reset-gpio in fsl,imx* compatible dts) should be kept asserted for at least 100 usec before the PCIe refclock is stable, should be kept asserted for at least 100 msec after the power rails are stable and the host should wait at least 100 msec after it is de-asserted before accessing the configuration space of any attached device. From PCIe CEM r2.0, sec 2.6.2 T-PVPERL: Power stable to PERST# inactive - 100 msec T-PERST-CLK: REFCLK stable before PERST# inactive - 100 usec. From PCIe r5.0, sec 6.6.1 With a Downstream Port that does not support Link speeds greater than 5.0 GT/s, software must wait a minimum of 100 ms before sending a Configuration Request to the device immediately below that Port. Failure to do so could prevent PCIe devices to be working correctly, and this was experienced with real devices. Move reset assert to imx6_pcie_assert_core_reset(), this way we ensure that PERST# is asserted before enabling any clock, move de-assert to the end of imx6_pcie_deassert_core_reset() after the clock is enabled and deemed stable and add a new delay of 100 msec just afterward. Link: https://lore.kernel.org/all/20220211152550.286821-1-francesco.dolcini@toradex.com Link: https://lore.kernel.org/r/20220404081509.94356-1-francesco.dolcini@toradex.com Fixes: bb38919ec56e ("PCI: imx6: Add support for i.MX6 PCIe controller") Signed-off-by: Francesco Dolcini Signed-off-by: Lorenzo Pieralisi Reviewed-by: Lucas Stach Acked-by: Richard Zhu --- drivers/pci/controller/dwc/pci-imx6.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 6619e3caffe2..7a285fb0f619 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -408,6 +408,11 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie) dev_err(dev, "failed to disable vpcie regulator: %d\n", ret); } + + /* Some boards don't have PCIe reset GPIO. */ + if (gpio_is_valid(imx6_pcie->reset_gpio)) + gpio_set_value_cansleep(imx6_pcie->reset_gpio, + imx6_pcie->gpio_active_high); } static unsigned int imx6_pcie_grp_offset(const struct imx6_pcie *imx6_pcie) @@ -540,15 +545,6 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie) /* allow the clocks to stabilize */ usleep_range(200, 500); - /* Some boards don't have PCIe reset GPIO. */ - if (gpio_is_valid(imx6_pcie->reset_gpio)) { - gpio_set_value_cansleep(imx6_pcie->reset_gpio, - imx6_pcie->gpio_active_high); - msleep(100); - gpio_set_value_cansleep(imx6_pcie->reset_gpio, - !imx6_pcie->gpio_active_high); - } - switch (imx6_pcie->drvdata->variant) { case IMX8MQ: reset_control_deassert(imx6_pcie->pciephy_reset); @@ -595,6 +591,15 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie) break; } + /* Some boards don't have PCIe reset GPIO. */ + if (gpio_is_valid(imx6_pcie->reset_gpio)) { + msleep(100); + gpio_set_value_cansleep(imx6_pcie->reset_gpio, + !imx6_pcie->gpio_active_high); + /* Wait for 100ms after PERST# deassertion (PCIe r5.0, 6.6.1) */ + msleep(100); + } + return; err_ref_clk: -- cgit v1.3.1 From 30097efa334a706f9021b9aee6efcddcfa44a78a Mon Sep 17 00:00:00 2001 From: Conor Dooley Date: Wed, 11 May 2022 10:55:05 +0100 Subject: PCI: microchip: Add missing chained_irq_enter()/exit() calls Two of the chained IRQ handlers miss their chained_irq_enter()/chained_irq_exit() calls, so add them in to avoid potentially lost interrupts. Reported by: Bjorn Helgaas Link: https://lore.kernel.org/linux-pci/87h76b8nxc.wl-maz@kernel.org Link: https://lore.kernel.org/r/20220511095504.2273799-1-conor.dooley@microchip.com Signed-off-by: Conor Dooley Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/pcie-microchip-host.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c index 4b1e130f88a3..fde3c80482a5 100644 --- a/drivers/pci/controller/pcie-microchip-host.c +++ b/drivers/pci/controller/pcie-microchip-host.c @@ -406,6 +406,7 @@ static void mc_pcie_enable_msi(struct mc_pcie *port, void __iomem *base) static void mc_handle_msi(struct irq_desc *desc) { struct mc_pcie *port = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); struct device *dev = port->dev; struct mc_msi *msi = &port->msi; void __iomem *bridge_base_addr = @@ -414,6 +415,8 @@ static void mc_handle_msi(struct irq_desc *desc) u32 bit; int ret; + chained_irq_enter(chip, desc); + status = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL); if (status & PM_MSI_INT_MSI_MASK) { status = readl_relaxed(bridge_base_addr + ISTATUS_MSI); @@ -424,6 +427,8 @@ static void mc_handle_msi(struct irq_desc *desc) bit); } } + + chained_irq_exit(chip, desc); } static void mc_msi_bottom_irq_ack(struct irq_data *data) @@ -563,6 +568,7 @@ static int mc_allocate_msi_domains(struct mc_pcie *port) static void mc_handle_intx(struct irq_desc *desc) { struct mc_pcie *port = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); struct device *dev = port->dev; void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; @@ -570,6 +576,8 @@ static void mc_handle_intx(struct irq_desc *desc) u32 bit; int ret; + chained_irq_enter(chip, desc); + status = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL); if (status & PM_MSI_INT_INTX_MASK) { status &= PM_MSI_INT_INTX_MASK; @@ -581,6 +589,8 @@ static void mc_handle_intx(struct irq_desc *desc) bit); } } + + chained_irq_exit(chip, desc); } static void mc_ack_intx_irq(struct irq_data *data) -- cgit v1.3.1 From 1d565935e3b9ccc682631e0bc6e415a7f48295d9 Mon Sep 17 00:00:00 2001 From: AngeloGioacchino Del Regno Date: Mon, 4 Apr 2022 16:48:58 +0200 Subject: PCI: mediatek-gen3: Assert resets to ensure expected init state The controller may have been left out of reset by the bootloader, in which case, before the powerup sequence, the controller will be found preconfigured with values that were set before booting the kernel: this produces a controller failure, with the result of a failure during the mtk_pcie_startup_port() sequence as the PCIe link never gets up. To ensure that we get a clean start in an expected state, assert both the PHY and MAC resets before executing the controller power-up sequence. Link: https://lore.kernel.org/r/20220404144858.92390-1-angelogioacchino.delregno@collabora.com Fixes: d3bf75b579b9 ("PCI: mediatek-gen3: Add MediaTek Gen3 driver for MT8192") Signed-off-by: AngeloGioacchino Del Regno Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/pcie-mediatek-gen3.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c index 3e8d70bfabc6..5d9fd36b02d1 100644 --- a/drivers/pci/controller/pcie-mediatek-gen3.c +++ b/drivers/pci/controller/pcie-mediatek-gen3.c @@ -838,6 +838,14 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie) if (err) return err; + /* + * The controller may have been left out of reset by the bootloader + * so make sure that we get a clean start by asserting resets here. + */ + reset_control_assert(pcie->phy_reset); + reset_control_assert(pcie->mac_reset); + usleep_range(10, 20); + /* Don't touch the hardware registers before power up */ err = mtk_pcie_power_up(pcie); if (err) -- cgit v1.3.1 From 431e7d2eece5b906578926d15ee22a70504c364d Mon Sep 17 00:00:00 2001 From: Peter Geis Date: Fri, 29 Apr 2022 08:38:28 -0400 Subject: PCI: rockchip-dwc: Reset core at driver probe The PCIe controller is in an unknown state at driver probe. This can lead to undesireable effects when the driver attempts to configure the controller. Prevent issues in the future by resetting the core during probe. Link: https://lore.kernel.org/r/20220429123832.2376381-3-pgwipeout@gmail.com Tested-by: Nicolas Frattaroli Signed-off-by: Peter Geis Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c index c9b341e55cbb..faedbd6ebc20 100644 --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c @@ -152,6 +152,11 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev, if (IS_ERR(rockchip->rst_gpio)) return PTR_ERR(rockchip->rst_gpio); + rockchip->rst = devm_reset_control_array_get_exclusive(&pdev->dev); + if (IS_ERR(rockchip->rst)) + return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst), + "failed to get reset lines\n"); + return 0; } @@ -182,18 +187,6 @@ static void rockchip_pcie_phy_deinit(struct rockchip_pcie *rockchip) phy_power_off(rockchip->phy); } -static int rockchip_pcie_reset_control_release(struct rockchip_pcie *rockchip) -{ - struct device *dev = rockchip->pci.dev; - - rockchip->rst = devm_reset_control_array_get_exclusive(dev); - if (IS_ERR(rockchip->rst)) - return dev_err_probe(dev, PTR_ERR(rockchip->rst), - "failed to get reset lines\n"); - - return reset_control_deassert(rockchip->rst); -} - static const struct dw_pcie_ops dw_pcie_ops = { .link_up = rockchip_pcie_link_up, .start_link = rockchip_pcie_start_link, @@ -222,6 +215,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev) if (ret) return ret; + ret = reset_control_assert(rockchip->rst); + if (ret) + return ret; + /* DON'T MOVE ME: must be enable before PHY init */ rockchip->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3"); if (IS_ERR(rockchip->vpcie3v3)) { @@ -241,7 +238,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev) if (ret) goto disable_regulator; - ret = rockchip_pcie_reset_control_release(rockchip); + ret = reset_control_deassert(rockchip->rst); if (ret) goto deinit_phy; -- cgit v1.3.1 From e8aae154df6121167e5b4f156cfc2402e651d2b1 Mon Sep 17 00:00:00 2001 From: Peter Geis Date: Fri, 29 Apr 2022 08:38:29 -0400 Subject: PCI: rockchip-dwc: Add legacy interrupt support The legacy interrupts on the rk356x PCIe controller are handled by a single muxed interrupt. Add IRQ domain support to the pcie-dw-rockchip driver to support the virtual domain. Link: https://lore.kernel.org/r/20220429123832.2376381-4-pgwipeout@gmail.com Signed-off-by: Peter Geis Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marc Zyngier --- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 96 ++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 2 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c index faedbd6ebc20..8c5bb9d7cc36 100644 --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c @@ -10,9 +10,12 @@ #include #include +#include +#include #include #include #include +#include #include #include #include @@ -26,6 +29,7 @@ */ #define HIWORD_UPDATE(mask, val) (((mask) << 16) | (val)) #define HIWORD_UPDATE_BIT(val) HIWORD_UPDATE(val, val) +#define HIWORD_DISABLE_BIT(val) HIWORD_UPDATE(val, ~val) #define to_rockchip_pcie(x) dev_get_drvdata((x)->dev) @@ -36,10 +40,12 @@ #define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP) #define PCIE_L0S_ENTRY 0x11 #define PCIE_CLIENT_GENERAL_CONTROL 0x0 +#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8 +#define PCIE_CLIENT_INTR_MASK_LEGACY 0x1c #define PCIE_CLIENT_GENERAL_DEBUG 0x104 -#define PCIE_CLIENT_HOT_RESET_CTRL 0x180 +#define PCIE_CLIENT_HOT_RESET_CTRL 0x180 #define PCIE_CLIENT_LTSSM_STATUS 0x300 -#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4) +#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4) #define PCIE_LTSSM_STATUS_MASK GENMASK(5, 0) struct rockchip_pcie { @@ -51,6 +57,7 @@ struct rockchip_pcie { struct reset_control *rst; struct gpio_desc *rst_gpio; struct regulator *vpcie3v3; + struct irq_domain *irq_domain; }; static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip, @@ -65,6 +72,78 @@ static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip, writel_relaxed(val, rockchip->apb_base + reg); } +static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc) +{ + struct irq_chip *chip = irq_desc_get_chip(desc); + struct rockchip_pcie *rockchip = irq_desc_get_handler_data(desc); + unsigned long reg, hwirq; + + chained_irq_enter(chip, desc); + + reg = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_STATUS_LEGACY); + + for_each_set_bit(hwirq, ®, 4) + generic_handle_domain_irq(rockchip->irq_domain, hwirq); + + chained_irq_exit(chip, desc); +} + +static void rockchip_intx_mask(struct irq_data *data) +{ + rockchip_pcie_writel_apb(irq_data_get_irq_chip_data(data), + HIWORD_UPDATE_BIT(BIT(data->hwirq)), + PCIE_CLIENT_INTR_MASK_LEGACY); +}; + +static void rockchip_intx_unmask(struct irq_data *data) +{ + rockchip_pcie_writel_apb(irq_data_get_irq_chip_data(data), + HIWORD_DISABLE_BIT(BIT(data->hwirq)), + PCIE_CLIENT_INTR_MASK_LEGACY); +}; + +static struct irq_chip rockchip_intx_irq_chip = { + .name = "INTx", + .irq_mask = rockchip_intx_mask, + .irq_unmask = rockchip_intx_unmask, + .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND, +}; + +static int rockchip_pcie_intx_map(struct irq_domain *domain, unsigned int irq, + irq_hw_number_t hwirq) +{ + irq_set_chip_and_handler(irq, &rockchip_intx_irq_chip, handle_level_irq); + irq_set_chip_data(irq, domain->host_data); + + return 0; +} + +static const struct irq_domain_ops intx_domain_ops = { + .map = rockchip_pcie_intx_map, +}; + +static int rockchip_pcie_init_irq_domain(struct rockchip_pcie *rockchip) +{ + struct device *dev = rockchip->pci.dev; + struct device_node *intc; + + intc = of_get_child_by_name(dev->of_node, "legacy-interrupt-controller"); + if (!intc) { + dev_err(dev, "missing child interrupt-controller node\n"); + return -EINVAL; + } + + rockchip->irq_domain = irq_domain_add_linear(intc, PCI_NUM_INTX, + &intx_domain_ops, rockchip); + of_node_put(intc); + if (!rockchip->irq_domain) { + dev_err(dev, "failed to get a INTx IRQ domain\n"); + return -EINVAL; + } + + return 0; +} + static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip) { rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM, @@ -111,7 +190,20 @@ static int rockchip_pcie_host_init(struct pcie_port *pp) { struct dw_pcie *pci = to_dw_pcie_from_pp(pp); struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); + struct device *dev = rockchip->pci.dev; u32 val = HIWORD_UPDATE_BIT(PCIE_LTSSM_ENABLE_ENHANCE); + int irq, ret; + + irq = of_irq_get_byname(dev->of_node, "legacy"); + if (irq < 0) + return irq; + + ret = rockchip_pcie_init_irq_domain(rockchip); + if (ret < 0) + dev_err(dev, "failed to init irq domain\n"); + + irq_set_chained_handler_and_data(irq, rockchip_pcie_legacy_int_handler, + rockchip); /* LTSSM enable control mode */ rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL); -- cgit v1.3.1 From a91ee0e9fca9d7501286cfbced9b30a33e52740a Mon Sep 17 00:00:00 2001 From: Yicong Yang Date: Mon, 4 Apr 2022 14:25:39 +0800 Subject: PCI: Avoid pci_dev_lock() AB/BA deadlock with sriov_numvfs_store() The sysfs sriov_numvfs_store() path acquires the device lock before the config space access lock: sriov_numvfs_store device_lock # A (1) acquire device lock sriov_configure vfio_pci_sriov_configure # (for example) vfio_pci_core_sriov_configure pci_disable_sriov sriov_disable pci_cfg_access_lock pci_wait_cfg # B (4) wait for dev->block_cfg_access == 0 Previously, pci_dev_lock() acquired the config space access lock before the device lock: pci_dev_lock pci_cfg_access_lock dev->block_cfg_access = 1 # B (2) set dev->block_cfg_access = 1 device_lock # A (3) wait for device lock Any path that uses pci_dev_lock(), e.g., pci_reset_function(), may deadlock with sriov_numvfs_store() if the operations occur in the sequence (1) (2) (3) (4). Avoid the deadlock by reversing the order in pci_dev_lock() so it acquires the device lock before the config space access lock, the same as the sriov_numvfs_store() path. [bhelgaas: combined and adapted commit log from Jay Zhou's independent subsequent posting: https://lore.kernel.org/r/20220404062539.1710-1-jianjay.zhou@huawei.com] Link: https://lore.kernel.org/linux-pci/1583489997-17156-1-git-send-email-yangyicong@hisilicon.com/ Also-posted-by: Jay Zhou Signed-off-by: Yicong Yang Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 9ecce435fb3f..61a6db1d21f6 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5103,19 +5103,19 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe) void pci_dev_lock(struct pci_dev *dev) { - pci_cfg_access_lock(dev); /* block PM suspend, driver probe, etc. */ device_lock(&dev->dev); + pci_cfg_access_lock(dev); } EXPORT_SYMBOL_GPL(pci_dev_lock); /* Return 1 on successful lock, 0 on contention */ int pci_dev_trylock(struct pci_dev *dev) { - if (pci_cfg_access_trylock(dev)) { - if (device_trylock(&dev->dev)) + if (device_trylock(&dev->dev)) { + if (pci_cfg_access_trylock(dev)) return 1; - pci_cfg_access_unlock(dev); + device_unlock(&dev->dev); } return 0; @@ -5124,8 +5124,8 @@ EXPORT_SYMBOL_GPL(pci_dev_trylock); void pci_dev_unlock(struct pci_dev *dev) { - device_unlock(&dev->dev); pci_cfg_access_unlock(dev); + device_unlock(&dev->dev); } EXPORT_SYMBOL_GPL(pci_dev_unlock); -- cgit v1.3.1 From 886e67100b904cb1b106ed1dfa8a60696aff519a Mon Sep 17 00:00:00 2001 From: Nirmal Patel Date: Wed, 11 May 2022 02:57:06 -0700 Subject: PCI: vmd: Assign VMD IRQ domain before enumeration During the boot process all the PCI devices are assigned default PCI-MSI IRQ domain including VMD endpoint devices. If interrupt-remapping is enabled by IOMMU, the PCI devices except VMD get new INTEL-IR-MSI IRQ domain. And VMD is supposed to create and assign a separate VMD-MSI IRQ domain for its child devices in order to support MSI-X remapping capabilities. Now when MSI-X remapping in VMD is disabled in order to improve performance, VMD skips VMD-MSI IRQ domain assignment process to its child devices. Thus the devices behind VMD get default PCI-MSI IRQ domain instead of INTEL-IR-MSI IRQ domain when VMD creates root bus and configures child devices. As a result host OS fails to boot and DMAR errors were observed when interrupt remapping was enabled on Intel Icelake CPUs. For instance: DMAR: DRHD: handling fault status reg 2 DMAR: [INTR-REMAP] Request device [0xe2:0x00.0] fault index 0xa00 [fault reason 0x25] Blocked a compatibility format interrupt request To fix this issue, dev_msi_info struct in dev struct maintains correct value of IRQ domain. VMD will use this information to assign proper IRQ domain to its child devices when it doesn't create a separate IRQ domain. Link: https://lore.kernel.org/r/20220511095707.25403-2-nirmal.patel@linux.intel.com Signed-off-by: Nirmal Patel Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/vmd.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index eb05cceab964..5015adc04d19 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -853,6 +853,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) vmd_attach_resources(vmd); if (vmd->irq_domain) dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain); + else + dev_set_msi_domain(&vmd->bus->dev, + dev_get_msi_domain(&vmd->dev->dev)); vmd_acpi_begin(); -- cgit v1.3.1 From c94f732e8001a860b42aa740b0a178a29907463c Mon Sep 17 00:00:00 2001 From: Nirmal Patel Date: Wed, 11 May 2022 02:57:07 -0700 Subject: PCI: vmd: Revert 2565e5b69c44 ("PCI: vmd: Do not disable MSI-X remapping if interrupt remapping is enabled by IOMMU.") Revert 2565e5b69c44 ("PCI: vmd: Do not disable MSI-X remapping if interrupt remapping is enabled by IOMMU.") The commit 2565e5b69c44 was added as a workaround to keep MSI-X remapping enabled if IOMMU enables interrupt remapping. VMD would keep running in low performance mode. There is no dependency between MSI-X remapping by VMD and interrupt remapping by IOMMU. Link: https://lore.kernel.org/r/20220511095707.25403-3-nirmal.patel@linux.intel.com Signed-off-by: Nirmal Patel Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/vmd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index 5015adc04d19..94a14a3d7e55 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -813,8 +812,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) * acceptable because the guest is usually CPU-limited and MSI * remapping doesn't become a performance bottleneck. */ - if (iommu_capable(vmd->dev->dev.bus, IOMMU_CAP_INTR_REMAP) || - !(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) || + if (!(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) || offset[0] || offset[1]) { ret = vmd_alloc_irqs(vmd); if (ret) -- cgit v1.3.1 From a1f67bc131c3935f325513cd153249fdbc22ac5b Mon Sep 17 00:00:00 2001 From: Christian Gmeiner Date: Thu, 12 May 2022 07:55:38 +0200 Subject: PCI: cadence: Allow PTM Responder to be enabled This enables the Controller [RP] to automatically respond with Response/ResponseD messages if CDNS_PCIE_LM_TPM_CTRL_PTMRSEN and PCI_PTM_CTRL_ENABLE bits are both set. Link: https://lore.kernel.org/r/20220512055539.1782437-1-christian.gmeiner@gmail.com Signed-off-by: Christian Gmeiner Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/cadence/pcie-cadence-host.c | 10 ++++++++++ drivers/pci/controller/cadence/pcie-cadence.h | 4 ++++ 2 files changed, 14 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c index fb96d37a135c..940c7dd701d6 100644 --- a/drivers/pci/controller/cadence/pcie-cadence-host.c +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c @@ -123,6 +123,14 @@ static int cdns_pcie_retrain(struct cdns_pcie *pcie) return ret; } +static void cdns_pcie_host_enable_ptm_response(struct cdns_pcie *pcie) +{ + u32 val; + + val = cdns_pcie_readl(pcie, CDNS_PCIE_LM_PTM_CTRL); + cdns_pcie_writel(pcie, CDNS_PCIE_LM_PTM_CTRL, val | CDNS_PCIE_LM_TPM_CTRL_PTMRSEN); +} + static int cdns_pcie_host_start_link(struct cdns_pcie_rc *rc) { struct cdns_pcie *pcie = &rc->pcie; @@ -501,6 +509,8 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc) if (rc->quirk_detect_quiet_flag) cdns_pcie_detect_quiet_min_delay_set(&rc->pcie); + cdns_pcie_host_enable_ptm_response(pcie); + ret = cdns_pcie_start_link(pcie); if (ret) { dev_err(dev, "Failed to start link\n"); diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h index c8a27b6290ce..1ffa8fa77a8a 100644 --- a/drivers/pci/controller/cadence/pcie-cadence.h +++ b/drivers/pci/controller/cadence/pcie-cadence.h @@ -116,6 +116,10 @@ #define LM_RC_BAR_CFG_APERTURE(bar, aperture) \ (((aperture) - 2) << ((bar) * 8)) +/* PTM Control Register */ +#define CDNS_PCIE_LM_PTM_CTRL (CDNS_PCIE_LM_BASE + 0x0da8) +#define CDNS_PCIE_LM_TPM_CTRL_PTMRSEN BIT(17) + /* * Endpoint Function Registers (PCI configuration space for endpoint functions) */ -- cgit v1.3.1 From 95b00f68209e2bc9f2ee9126afcebab451e0e9d8 Mon Sep 17 00:00:00 2001 From: Parshuram Thombare Date: Mon, 25 Oct 2021 05:31:15 -0700 Subject: PCI: cadence: Clear FLR in device capabilities register Clear FLR (Function Level Reset) from device capabilities registers for all physical functions. During FLR, the Margining Lane Status and Margining Lane Control registers should not be reset, as per PCIe specification. However, the controller incorrectly resets these registers upon FLR. This causes PCISIG compliance FLR test to fail. Hence preventing all functions from advertising FLR support if flag quirk_disable_flr is set. Link: https://lore.kernel.org/r/1635165075-89864-1-git-send-email-pthombar@cadence.com Signed-off-by: Parshuram Thombare Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/cadence/pci-j721e.c | 3 +++ drivers/pci/controller/cadence/pcie-cadence-ep.c | 18 +++++++++++++++++- drivers/pci/controller/cadence/pcie-cadence.h | 3 +++ 3 files changed, 23 insertions(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c index 768d33f9ebc8..a82f845cc4b5 100644 --- a/drivers/pci/controller/cadence/pci-j721e.c +++ b/drivers/pci/controller/cadence/pci-j721e.c @@ -69,6 +69,7 @@ struct j721e_pcie_data { enum j721e_pcie_mode mode; unsigned int quirk_retrain_flag:1; unsigned int quirk_detect_quiet_flag:1; + unsigned int quirk_disable_flr:1; u32 linkdown_irq_regfield; unsigned int byte_access_allowed:1; }; @@ -307,6 +308,7 @@ static const struct j721e_pcie_data j7200_pcie_rc_data = { static const struct j721e_pcie_data j7200_pcie_ep_data = { .mode = PCI_MODE_EP, .quirk_detect_quiet_flag = true, + .quirk_disable_flr = true, }; static const struct j721e_pcie_data am64_pcie_rc_data = { @@ -405,6 +407,7 @@ static int j721e_pcie_probe(struct platform_device *pdev) return -ENOMEM; ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag; + ep->quirk_disable_flr = data->quirk_disable_flr; cdns_pcie = &ep->pcie; cdns_pcie->dev = dev; diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c index 18e32b8ffd5e..b8b655d4047e 100644 --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c @@ -564,7 +564,8 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) struct cdns_pcie_ep *ep = epc_get_drvdata(epc); struct cdns_pcie *pcie = &ep->pcie; struct device *dev = pcie->dev; - int ret; + int max_epfs = sizeof(epc->function_num_map) * 8; + int ret, value, epf; /* * BIT(0) is hardwired to 1, hence function 0 is always enabled @@ -572,6 +573,21 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) */ cdns_pcie_writel(pcie, CDNS_PCIE_LM_EP_FUNC_CFG, epc->function_num_map); + if (ep->quirk_disable_flr) { + for (epf = 0; epf < max_epfs; epf++) { + if (!(epc->function_num_map & BIT(epf))) + continue; + + value = cdns_pcie_ep_fn_readl(pcie, epf, + CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + + PCI_EXP_DEVCAP); + value &= ~PCI_EXP_DEVCAP_FLR; + cdns_pcie_ep_fn_writel(pcie, epf, + CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + + PCI_EXP_DEVCAP, value); + } + } + ret = cdns_pcie_start_link(pcie); if (ret) { dev_err(dev, "Failed to start link\n"); diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h index 1ffa8fa77a8a..190786e47df9 100644 --- a/drivers/pci/controller/cadence/pcie-cadence.h +++ b/drivers/pci/controller/cadence/pcie-cadence.h @@ -127,6 +127,7 @@ #define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET 0x90 #define CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET 0xb0 +#define CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET 0xc0 #define CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET 0x200 /* @@ -361,6 +362,7 @@ struct cdns_pcie_epf { * minimize time between read and write * @epf: Structure to hold info about endpoint function * @quirk_detect_quiet_flag: LTSSM Detect Quiet min delay set as quirk + * @quirk_disable_flr: Disable FLR (Function Level Reset) quirk flag */ struct cdns_pcie_ep { struct cdns_pcie pcie; @@ -376,6 +378,7 @@ struct cdns_pcie_ep { spinlock_t lock; struct cdns_pcie_epf *epf; unsigned int quirk_detect_quiet_flag:1; + unsigned int quirk_disable_flr:1; }; -- cgit v1.3.1 From 203926da2bff8e172200a2f11c758987af112d4a Mon Sep 17 00:00:00 2001 From: Kuppuswamy Sathyanarayanan Date: Mon, 18 Apr 2022 15:02:37 +0000 Subject: PCI/AER: Clear MULTI_ERR_COR/UNCOR_RCV bits When a Root Port or Root Complex Event Collector receives an error Message e.g., ERR_COR, it sets PCI_ERR_ROOT_COR_RCV in the Root Error Status register and logs the Requester ID in the Error Source Identification register. If it receives a second ERR_COR Message before software clears PCI_ERR_ROOT_COR_RCV, hardware sets PCI_ERR_ROOT_MULTI_COR_RCV and the Requester ID is lost. In the following scenario, PCI_ERR_ROOT_MULTI_COR_RCV was never cleared: - hardware receives ERR_COR message - hardware sets PCI_ERR_ROOT_COR_RCV - aer_irq() entered - aer_irq(): status = pci_read_config_dword(PCI_ERR_ROOT_STATUS) - aer_irq(): now status == PCI_ERR_ROOT_COR_RCV - hardware receives second ERR_COR message - hardware sets PCI_ERR_ROOT_MULTI_COR_RCV - aer_irq(): pci_write_config_dword(PCI_ERR_ROOT_STATUS, status) - PCI_ERR_ROOT_COR_RCV is cleared; PCI_ERR_ROOT_MULTI_COR_RCV is set - aer_irq() entered again - aer_irq(): status = pci_read_config_dword(PCI_ERR_ROOT_STATUS) - aer_irq(): now status == PCI_ERR_ROOT_MULTI_COR_RCV - aer_irq() exits because PCI_ERR_ROOT_COR_RCV not set - PCI_ERR_ROOT_MULTI_COR_RCV is still set The same problem occurred with ERR_NONFATAL/ERR_FATAL Messages and PCI_ERR_ROOT_UNCOR_RCV and PCI_ERR_ROOT_MULTI_UNCOR_RCV. Fix the problem by queueing an AER event and clearing the Root Error Status bits when any of these bits are set: PCI_ERR_ROOT_COR_RCV PCI_ERR_ROOT_UNCOR_RCV PCI_ERR_ROOT_MULTI_COR_RCV PCI_ERR_ROOT_MULTI_UNCOR_RCV See the bugzilla link for details from Eric about how to reproduce this problem. [bhelgaas: commit log, move repro details to bugzilla] Fixes: e167bfcaa4cd ("PCI: aerdrv: remove magical ROOT_ERR_STATUS_MASKS") Link: https://bugzilla.kernel.org/show_bug.cgi?id=215992 Link: https://lore.kernel.org/r/20220418150237.1021519-1-sathyanarayanan.kuppuswamy@linux.intel.com Reported-by: Eric Badger Signed-off-by: Kuppuswamy Sathyanarayanan Signed-off-by: Bjorn Helgaas Reviewed-by: Ashok Raj --- drivers/pci/pcie/aer.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 9fa1f97e5b27..7952e5efd6cf 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -101,6 +101,11 @@ struct aer_stats { #define ERR_COR_ID(d) (d & 0xffff) #define ERR_UNCOR_ID(d) (d >> 16) +#define AER_ERR_STATUS_MASK (PCI_ERR_ROOT_UNCOR_RCV | \ + PCI_ERR_ROOT_COR_RCV | \ + PCI_ERR_ROOT_MULTI_COR_RCV | \ + PCI_ERR_ROOT_MULTI_UNCOR_RCV) + static int pcie_aer_disable; static pci_ers_result_t aer_root_reset(struct pci_dev *dev); @@ -1196,7 +1201,7 @@ static irqreturn_t aer_irq(int irq, void *context) struct aer_err_source e_src = {}; pci_read_config_dword(rp, aer + PCI_ERR_ROOT_STATUS, &e_src.status); - if (!(e_src.status & (PCI_ERR_ROOT_UNCOR_RCV|PCI_ERR_ROOT_COR_RCV))) + if (!(e_src.status & AER_ERR_STATUS_MASK)) return IRQ_NONE; pci_read_config_dword(rp, aer + PCI_ERR_ROOT_ERR_SRC, &e_src.id); -- cgit v1.3.1 From 7013654af694f6e1a2e699a6450ea50d309dd0e5 Mon Sep 17 00:00:00 2001 From: Daire McNamara Date: Tue, 17 May 2022 15:16:22 +0100 Subject: PCI: microchip: Fix potential race in interrupt handling Clear the MSI bit in ISTATUS_LOCAL register after reading it, but before reading and handling individual MSI bits from the ISTATUS_MSI register. This avoids a potential race where new MSI bits may be set on the ISTATUS_MSI register after it was read and be missed when the MSI bit in the ISTATUS_LOCAL register is cleared. ISTATUS_LOCAL is a read/write/clear register; the register's bits are set when the corresponding interrupt source is activated. Each source is independent and thus multiple sources may be active simultaneously. The processor can monitor and clear status bits. If one or more ISTATUS_LOCAL interrupt sources are active, the RootPort issues an interrupt towards the processor (on the AXI domain). Bit 28 of this register reports an MSI has been received by the RootPort. ISTATUS_MSI is a read/write/clear register. Bits 31-0 are asserted when an MSI with message number 31-0 is received by the RootPort. The processor must monitor and clear these bits. Effectively, Bit 28 of ISTATUS_LOCAL informs the processor that an MSI has arrived at the RootPort and ISTATUS_MSI informs the processor which MSI (in the range 0 - 31) needs handling. Reported by: Bjorn Helgaas Link: https://lore.kernel.org/linux-pci/20220127202000.GA126335@bhelgaas/ Link: https://lore.kernel.org/r/20220517141622.145581-1-daire.mcnamara@microchip.com Fixes: 6f15a9c9f941 ("PCI: microchip: Add Microchip PolarFire PCIe controller driver") Signed-off-by: Daire McNamara Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/pcie-microchip-host.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c index fde3c80482a5..dd5dba419047 100644 --- a/drivers/pci/controller/pcie-microchip-host.c +++ b/drivers/pci/controller/pcie-microchip-host.c @@ -419,6 +419,7 @@ static void mc_handle_msi(struct irq_desc *desc) status = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL); if (status & PM_MSI_INT_MSI_MASK) { + writel_relaxed(status & PM_MSI_INT_MSI_MASK, bridge_base_addr + ISTATUS_LOCAL); status = readl_relaxed(bridge_base_addr + ISTATUS_MSI); for_each_set_bit(bit, &status, msi->num_vectors) { ret = generic_handle_domain_irq(msi->dev_domain, bit); @@ -437,13 +438,8 @@ static void mc_msi_bottom_irq_ack(struct irq_data *data) void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; u32 bitpos = data->hwirq; - unsigned long status; writel_relaxed(BIT(bitpos), bridge_base_addr + ISTATUS_MSI); - status = readl_relaxed(bridge_base_addr + ISTATUS_MSI); - if (!status) - writel_relaxed(BIT(PM_MSI_INT_MSI_SHIFT), - bridge_base_addr + ISTATUS_LOCAL); } static void mc_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) -- cgit v1.3.1 From a935601eed18d739c11da5504b551c7c4754f2ec Mon Sep 17 00:00:00 2001 From: Bhupesh Sharma Date: Sat, 26 Mar 2022 11:38:10 +0530 Subject: PCI: qcom: Add SM8150 SoC support The PCIe IP (rev 1.5.0) on SM8150 SoC is similar to the one used on SM8250. Add SM8150 support, reusing the members of ops_1_9_0. Link: https://lore.kernel.org/r/20220326060810.1797516-3-bhupesh.sharma@linaro.org Signed-off-by: Bhupesh Sharma Signed-off-by: Lorenzo Pieralisi Signed-off-by: Bjorn Helgaas Reviewed-by: Dmitry Baryshkov Reviewed-by: Rob Herring Cc: Vinod Koul --- drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 6ab90891801d..375f27ab9403 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -1523,6 +1523,13 @@ static const struct qcom_pcie_cfg sdm845_cfg = { .has_tbu_clk = true, }; +static const struct qcom_pcie_cfg sm8150_cfg = { + /* sm8150 has qcom IP rev 1.5.0. However 1.5.0 ops are same as + * 1.9.0, so reuse the same. + */ + .ops = &ops_1_9_0, +}; + static const struct qcom_pcie_cfg sm8250_cfg = { .ops = &ops_1_9_0, .has_tbu_clk = true, @@ -1655,6 +1662,7 @@ static const struct of_device_id qcom_pcie_match[] = { { .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg }, { .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg }, { .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg }, + { .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg }, { .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg }, { .compatible = "qcom,pcie-sc8180x", .data = &sm8250_cfg }, { .compatible = "qcom,pcie-sm8450-pcie0", .data = &sm8450_pcie0_cfg }, -- cgit v1.3.1 From fdf6a2f533115ec5d4d9629178f8196331f1ac50 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 1 Apr 2022 15:33:51 +0200 Subject: PCI: qcom: Fix pipe clock imbalance Fix a clock imbalance introduced by ed8cc3b1fc84 ("PCI: qcom: Add support for SDM845 PCIe controller"), which enables the pipe clock both in init() and in post_init() but only disables in post_deinit(). Note that the pipe clock was also never disabled in the init() error paths and that enabling the clock before powering up the PHY looks questionable. Link: https://lore.kernel.org/r/20220401133351.10113-1-johan+linaro@kernel.org Fixes: ed8cc3b1fc84 ("PCI: qcom: Add support for SDM845 PCIe controller") Signed-off-by: Johan Hovold Signed-off-by: Lorenzo Pieralisi Signed-off-by: Bjorn Helgaas Reviewed-by: Bjorn Andersson Cc: stable@vger.kernel.org # 5.6 --- drivers/pci/controller/dwc/pcie-qcom.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 375f27ab9403..925324dece64 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -1238,12 +1238,6 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) goto err_disable_clocks; } - ret = clk_prepare_enable(res->pipe_clk); - if (ret) { - dev_err(dev, "cannot prepare/enable pipe clock\n"); - goto err_disable_clocks; - } - /* Wait for reset to complete, required on SM8450 */ usleep_range(1000, 1500); -- cgit v1.3.1 From 87d83b96c8d6c6c2d2096bd0bdba73bcf42b8ef0 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 1 Apr 2022 15:38:53 +0200 Subject: PCI: qcom: Fix runtime PM imbalance on probe errors Drop the leftover pm_runtime_disable() calls from the late probe error paths that would, for example, prevent runtime PM from being reenabled after a probe deferral. Link: https://lore.kernel.org/r/20220401133854.10421-2-johan+linaro@kernel.org Fixes: 6e5da6f7d824 ("PCI: qcom: Fix error handling in runtime PM support") Signed-off-by: Johan Hovold Signed-off-by: Lorenzo Pieralisi Signed-off-by: Bjorn Helgaas Reviewed-by: Manivannan Sadhasivam Acked-by: Stanimir Varbanov Cc: stable@vger.kernel.org # 4.20 Cc: Bjorn Andersson --- drivers/pci/controller/dwc/pcie-qcom.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 925324dece64..9191be96d627 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -1623,17 +1623,14 @@ static int qcom_pcie_probe(struct platform_device *pdev) pp->ops = &qcom_pcie_dw_ops; ret = phy_init(pcie->phy); - if (ret) { - pm_runtime_disable(&pdev->dev); + if (ret) goto err_pm_runtime_put; - } platform_set_drvdata(pdev, pcie); ret = dw_pcie_host_init(pp); if (ret) { dev_err(dev, "cannot initialize host\n"); - pm_runtime_disable(&pdev->dev); goto err_pm_runtime_put; } -- cgit v1.3.1 From 83013631f0f9961416abd812e228c8efbc2f6069 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 1 Apr 2022 15:38:54 +0200 Subject: PCI: qcom: Fix unbalanced PHY init on probe errors Undo the PHY initialisation (e.g. balance runtime PM) if host initialisation fails during probe. Link: https://lore.kernel.org/r/20220401133854.10421-3-johan+linaro@kernel.org Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver") Signed-off-by: Johan Hovold Signed-off-by: Lorenzo Pieralisi Signed-off-by: Bjorn Helgaas Reviewed-by: Manivannan Sadhasivam Acked-by: Stanimir Varbanov Cc: stable@vger.kernel.org # 4.5 --- drivers/pci/controller/dwc/pcie-qcom.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 9191be96d627..2e5464edc36e 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -1631,11 +1631,13 @@ static int qcom_pcie_probe(struct platform_device *pdev) ret = dw_pcie_host_init(pp); if (ret) { dev_err(dev, "cannot initialize host\n"); - goto err_pm_runtime_put; + goto err_phy_exit; } return 0; +err_phy_exit: + phy_exit(pcie->phy); err_pm_runtime_put: pm_runtime_put(dev); pm_runtime_disable(dev); -- cgit v1.3.1