From c9c13ba428ef90a9b408a6cdf874e14ab5754516 Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Sat, 28 Sep 2019 02:43:08 +0300 Subject: PCI: Add PCI_STD_NUM_BARS for the number of standard BARs Code that iterates over all standard PCI BARs typically uses PCI_STD_RESOURCE_END. However, that requires the unusual test "i <= PCI_STD_RESOURCE_END" rather than something the typical "i < PCI_STD_NUM_BARS". Add a definition for PCI_STD_NUM_BARS and change loops to use the more idiomatic C style to help avoid fencepost errors. Link: https://lore.kernel.org/r/20190927234026.23342-1-efremov@linux.com Link: https://lore.kernel.org/r/20190927234308.23935-1-efremov@linux.com Link: https://lore.kernel.org/r/20190916204158.6889-3-efremov@linux.com Signed-off-by: Denis Efremov Signed-off-by: Bjorn Helgaas Acked-by: Sebastian Ott # arch/s390/ Acked-by: Bartlomiej Zolnierkiewicz # video/fbdev/ Acked-by: Gustavo Pimentel # pci/controller/dwc/ Acked-by: Jack Wang # scsi/pm8001/ Acked-by: Martin K. Petersen # scsi/pm8001/ Acked-by: Ulf Hansson # memstick/ --- drivers/pci/pci.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e7982af9a5d8..184765f12848 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -674,7 +674,7 @@ struct resource *pci_find_resource(struct pci_dev *dev, struct resource *res) { int i; - for (i = 0; i < PCI_ROM_RESOURCE; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { struct resource *r = &dev->resource[i]; if (r->start && resource_contains(r, res)) @@ -3768,7 +3768,7 @@ void pci_release_selected_regions(struct pci_dev *pdev, int bars) { int i; - for (i = 0; i < 6; i++) + for (i = 0; i < PCI_STD_NUM_BARS; i++) if (bars & (1 << i)) pci_release_region(pdev, i); } @@ -3779,7 +3779,7 @@ static int __pci_request_selected_regions(struct pci_dev *pdev, int bars, { int i; - for (i = 0; i < 6; i++) + for (i = 0; i < PCI_STD_NUM_BARS; i++) if (bars & (1 << i)) if (__pci_request_region(pdev, i, res_name, excl)) goto err_out; @@ -3827,7 +3827,7 @@ EXPORT_SYMBOL(pci_request_selected_regions_exclusive); void pci_release_regions(struct pci_dev *pdev) { - pci_release_selected_regions(pdev, (1 << 6) - 1); + pci_release_selected_regions(pdev, (1 << PCI_STD_NUM_BARS) - 1); } EXPORT_SYMBOL(pci_release_regions); @@ -3846,7 +3846,8 @@ EXPORT_SYMBOL(pci_release_regions); */ int pci_request_regions(struct pci_dev *pdev, const char *res_name) { - return pci_request_selected_regions(pdev, ((1 << 6) - 1), res_name); + return pci_request_selected_regions(pdev, + ((1 << PCI_STD_NUM_BARS) - 1), res_name); } EXPORT_SYMBOL(pci_request_regions); @@ -3868,7 +3869,7 @@ EXPORT_SYMBOL(pci_request_regions); int pci_request_regions_exclusive(struct pci_dev *pdev, const char *res_name) { return pci_request_selected_regions_exclusive(pdev, - ((1 << 6) - 1), res_name); + ((1 << PCI_STD_NUM_BARS) - 1), res_name); } EXPORT_SYMBOL(pci_request_regions_exclusive); -- cgit v1.2.3 From af65d1ad416bc6e069ccb9e649faeda224248f96 Mon Sep 17 00:00:00 2001 From: "Patel, Mayurkumar" Date: Fri, 18 Oct 2019 16:52:21 +0000 Subject: PCI/AER: Save AER Capability for suspend/resume Previously we did not save and restore the AER configuration on suspend/resume, so the configuration may be lost after resume. Save the AER configuration during suspend and restore it during resume. [bhelgaas: commit log] Link: https://lore.kernel.org/r/92EBB4272BF81E4089A7126EC1E7B28492C3B007@IRSMSX101.ger.corp.intel.com Signed-off-by: Mayurkumar Patel Signed-off-by: Kuppuswamy Sathyanarayanan Signed-off-by: Bjorn Helgaas Reviewed-by: Andy Shevchenko --- drivers/pci/access.c | 2 +- drivers/pci/pci.c | 2 ++ drivers/pci/pci.h | 1 + drivers/pci/pcie/aer.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++-- include/linux/aer.h | 4 ++++ 5 files changed, 68 insertions(+), 3 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 2fccb5762c76..79c4a2ef269a 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -355,7 +355,7 @@ static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev) pcie_caps_reg(dev) & PCI_EXP_FLAGS_SLOT; } -static inline bool pcie_cap_has_rtctl(const struct pci_dev *dev) +bool pcie_cap_has_rtctl(const struct pci_dev *dev) { int type = pci_pcie_type(dev); diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e7982af9a5d8..d02636540ca1 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1361,6 +1361,7 @@ int pci_save_state(struct pci_dev *dev) pci_save_ltr_state(dev); pci_save_dpc_state(dev); + pci_save_aer_state(dev); return pci_save_vc_state(dev); } EXPORT_SYMBOL(pci_save_state); @@ -1474,6 +1475,7 @@ void pci_restore_state(struct pci_dev *dev) pci_restore_dpc_state(dev); pci_cleanup_aer_error_status_regs(dev); + pci_restore_aer_state(dev); pci_restore_config_space(dev); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 3f6947ee3324..b96988f90315 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -12,6 +12,7 @@ extern const unsigned char pcie_link_speed[]; extern bool pci_early_dump; bool pcie_cap_has_lnkctl(const struct pci_dev *dev); +bool pcie_cap_has_rtctl(const struct pci_dev *dev); /* Functions internal to the PCI core code */ diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index b45bc47d04fe..443512882be4 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -448,12 +448,70 @@ int pci_cleanup_aer_error_status_regs(struct pci_dev *dev) return 0; } +void pci_save_aer_state(struct pci_dev *dev) +{ + struct pci_cap_saved_state *save_state; + u32 *cap; + int pos; + + pos = dev->aer_cap; + if (!pos) + return; + + save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_ERR); + if (!save_state) + return; + + cap = &save_state->cap.data[0]; + pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, cap++); + pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, cap++); + pci_read_config_dword(dev, pos + PCI_ERR_COR_MASK, cap++); + pci_read_config_dword(dev, pos + PCI_ERR_CAP, cap++); + if (pcie_cap_has_rtctl(dev)) + pci_read_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, cap++); +} + +void pci_restore_aer_state(struct pci_dev *dev) +{ + struct pci_cap_saved_state *save_state; + u32 *cap; + int pos; + + pos = dev->aer_cap; + if (!pos) + return; + + save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_ERR); + if (!save_state) + return; + + cap = &save_state->cap.data[0]; + pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, *cap++); + pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, *cap++); + pci_write_config_dword(dev, pos + PCI_ERR_COR_MASK, *cap++); + pci_write_config_dword(dev, pos + PCI_ERR_CAP, *cap++); + if (pcie_cap_has_rtctl(dev)) + pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, *cap++); +} + void pci_aer_init(struct pci_dev *dev) { + int n; + dev->aer_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); + if (!dev->aer_cap) + return; - if (dev->aer_cap) - dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL); + dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL); + + /* + * We save/restore PCI_ERR_UNCOR_MASK, PCI_ERR_UNCOR_SEVER, + * PCI_ERR_COR_MASK, and PCI_ERR_CAP. Root and Root Complex Event + * Collectors also implement PCI_ERR_ROOT_COMMAND (PCIe r5.0, sec + * 7.8.4). + */ + n = pcie_cap_has_rtctl(dev) ? 5 : 4; + pci_add_ext_cap_save_buffer(dev, PCI_EXT_CAP_ID_ERR, sizeof(u32) * n); pci_cleanup_aer_error_status_regs(dev); } diff --git a/include/linux/aer.h b/include/linux/aer.h index 514bffa11dbb..fa19e01f418a 100644 --- a/include/linux/aer.h +++ b/include/linux/aer.h @@ -46,6 +46,8 @@ int pci_enable_pcie_error_reporting(struct pci_dev *dev); int pci_disable_pcie_error_reporting(struct pci_dev *dev); int pci_cleanup_aer_uncorrect_error_status(struct pci_dev *dev); int pci_cleanup_aer_error_status_regs(struct pci_dev *dev); +void pci_save_aer_state(struct pci_dev *dev); +void pci_restore_aer_state(struct pci_dev *dev); #else static inline int pci_enable_pcie_error_reporting(struct pci_dev *dev) { @@ -63,6 +65,8 @@ static inline int pci_cleanup_aer_error_status_regs(struct pci_dev *dev) { return -EINVAL; } +static inline void pci_save_aer_state(struct pci_dev *dev) {} +static inline void pci_restore_aer_state(struct pci_dev *dev) {} #endif void cper_print_aer(struct pci_dev *dev, int aer_severity, -- cgit v1.2.3 From 52525b7a3cf82adec5c6cf0ecbd23ff228badc94 Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Fri, 18 Oct 2019 15:38:47 +0800 Subject: PCI: Add a helper to check Power Resource Requirements _PR3 existence A driver may want to know the existence of _PR3, to choose different runtime suspend behavior. A user will be add in next patch. This is mostly the same as nouveau_pr3_present(). Signed-off-by: Kai-Heng Feng Acked-by: Bjorn Helgaas Link: https://lore.kernel.org/r/20191018073848.14590-1-kai.heng.feng@canonical.com Signed-off-by: Takashi Iwai --- drivers/pci/pci.c | 18 ++++++++++++++++++ include/linux/pci.h | 2 ++ 2 files changed, 20 insertions(+) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e7982af9a5d8..1df99d9e350e 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5856,6 +5856,24 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode, return 0; } +#ifdef CONFIG_ACPI +bool pci_pr3_present(struct pci_dev *pdev) +{ + struct acpi_device *adev; + + if (acpi_disabled) + return false; + + adev = ACPI_COMPANION(&pdev->dev); + if (!adev) + return false; + + return adev->power.flags.power_resources && + acpi_has_method(adev->handle, "_PR3"); +} +EXPORT_SYMBOL_GPL(pci_pr3_present); +#endif + /** * pci_add_dma_alias - Add a DMA devfn alias for a device * @dev: the PCI device for which alias is added diff --git a/include/linux/pci.h b/include/linux/pci.h index f9088c89a534..1d15c5d49cdd 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -2310,9 +2310,11 @@ struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus); void pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *)); +bool pci_pr3_present(struct pci_dev *pdev); #else static inline struct irq_domain * pci_host_bridge_acpi_msi_domain(struct pci_bus *bus) { return NULL; } +static bool pci_pr3_present(struct pci_dev *pdev) { return false; } #endif #ifdef CONFIG_EEH -- cgit v1.2.3 From d7b8a217521ca21e2c6391da88d4928c6ce1f539 Mon Sep 17 00:00:00 2001 From: Nicholas Johnson Date: Wed, 23 Oct 2019 12:12:29 +0000 Subject: PCI: Add "pci=hpmmiosize" and "pci=hpmmioprefsize" parameters The existing "pci=hpmemsize=nn[KMG]" kernel parameter overrides the default size of both the non-prefetchable and the prefetchable MMIO windows for hotplug bridges. Add "pci=hpmmiosize=nn[KMG]" to override the default size of only the non-prefetchable MMIO window. Add "pci=hpmmioprefsize=nn[KMG]" to override the default size of only the prefetchable MMIO window. Link: https://lore.kernel.org/r/SL2P216MB0187E4D0055791957B7E2660806B0@SL2P216MB0187.KORP216.PROD.OUTLOOK.COM Signed-off-by: Nicholas Johnson Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- Documentation/admin-guide/kernel-parameters.txt | 9 ++++++++- drivers/pci/pci.c | 20 ++++++++++++++++---- drivers/pci/pci.h | 3 ++- drivers/pci/setup-bus.c | 24 +++++++++++++----------- 4 files changed, 39 insertions(+), 17 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index c7ac2f3ac99f..62fbf7c0ff1b 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3492,8 +3492,15 @@ hpiosize=nn[KMG] The fixed amount of bus space which is reserved for hotplug bridge's IO window. Default size is 256 bytes. + hpmmiosize=nn[KMG] The fixed amount of bus space which is + reserved for hotplug bridge's MMIO window. + Default size is 2 megabytes. + hpmmioprefsize=nn[KMG] The fixed amount of bus space which is + reserved for hotplug bridge's MMIO_PREF window. + Default size is 2 megabytes. hpmemsize=nn[KMG] The fixed amount of bus space which is - reserved for hotplug bridge's memory window. + reserved for hotplug bridge's MMIO and + MMIO_PREF window. Default size is 2 megabytes. hpbussize=nn The minimum amount of additional bus numbers reserved for buses below a hotplug bridge. diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 184765f12848..66ff1ca5b688 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -85,10 +85,17 @@ unsigned long pci_cardbus_io_size = DEFAULT_CARDBUS_IO_SIZE; unsigned long pci_cardbus_mem_size = DEFAULT_CARDBUS_MEM_SIZE; #define DEFAULT_HOTPLUG_IO_SIZE (256) -#define DEFAULT_HOTPLUG_MEM_SIZE (2*1024*1024) -/* pci=hpmemsize=nnM,hpiosize=nn can override this */ +#define DEFAULT_HOTPLUG_MMIO_SIZE (2*1024*1024) +#define DEFAULT_HOTPLUG_MMIO_PREF_SIZE (2*1024*1024) +/* hpiosize=nn can override this */ unsigned long pci_hotplug_io_size = DEFAULT_HOTPLUG_IO_SIZE; -unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MEM_SIZE; +/* + * pci=hpmmiosize=nnM overrides non-prefetchable MMIO size, + * pci=hpmmioprefsize=nnM overrides prefetchable MMIO size; + * pci=hpmemsize=nnM overrides both + */ +unsigned long pci_hotplug_mmio_size = DEFAULT_HOTPLUG_MMIO_SIZE; +unsigned long pci_hotplug_mmio_pref_size = DEFAULT_HOTPLUG_MMIO_PREF_SIZE; #define DEFAULT_HOTPLUG_BUS_SIZE 1 unsigned long pci_hotplug_bus_size = DEFAULT_HOTPLUG_BUS_SIZE; @@ -6289,8 +6296,13 @@ static int __init pci_setup(char *str) pcie_ecrc_get_policy(str + 5); } else if (!strncmp(str, "hpiosize=", 9)) { pci_hotplug_io_size = memparse(str + 9, &str); + } else if (!strncmp(str, "hpmmiosize=", 11)) { + pci_hotplug_mmio_size = memparse(str + 11, &str); + } else if (!strncmp(str, "hpmmioprefsize=", 15)) { + pci_hotplug_mmio_pref_size = memparse(str + 15, &str); } else if (!strncmp(str, "hpmemsize=", 10)) { - pci_hotplug_mem_size = memparse(str + 10, &str); + pci_hotplug_mmio_size = memparse(str + 10, &str); + pci_hotplug_mmio_pref_size = pci_hotplug_mmio_size; } else if (!strncmp(str, "hpbussize=", 10)) { pci_hotplug_bus_size = simple_strtoul(str + 10, &str, 0); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 3f6947ee3324..9faa55a151cb 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -218,7 +218,8 @@ extern const struct device_type pci_dev_type; extern const struct attribute_group *pci_bus_groups[]; extern unsigned long pci_hotplug_io_size; -extern unsigned long pci_hotplug_mem_size; +extern unsigned long pci_hotplug_mmio_size; +extern unsigned long pci_hotplug_mmio_pref_size; extern unsigned long pci_hotplug_bus_size; /** diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index f1c51943bdfc..b5f9f57f436a 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -1178,7 +1178,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head) { struct pci_dev *dev; unsigned long mask, prefmask, type2 = 0, type3 = 0; - resource_size_t additional_mem_size = 0, additional_io_size = 0; + resource_size_t additional_io_size = 0, additional_mmio_size = 0, + additional_mmio_pref_size = 0; struct resource *b_res; int ret; @@ -1212,7 +1213,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head) pci_bridge_check_ranges(bus); if (bus->self->is_hotplug_bridge) { additional_io_size = pci_hotplug_io_size; - additional_mem_size = pci_hotplug_mem_size; + additional_mmio_size = pci_hotplug_mmio_size; + additional_mmio_pref_size = pci_hotplug_mmio_pref_size; } /* Fall through */ default: @@ -1230,9 +1232,9 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head) if (b_res[2].flags & IORESOURCE_MEM_64) { prefmask |= IORESOURCE_MEM_64; ret = pbus_size_mem(bus, prefmask, prefmask, - prefmask, prefmask, - realloc_head ? 0 : additional_mem_size, - additional_mem_size, realloc_head); + prefmask, prefmask, + realloc_head ? 0 : additional_mmio_pref_size, + additional_mmio_pref_size, realloc_head); /* * If successful, all non-prefetchable resources @@ -1254,9 +1256,9 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head) if (!type2) { prefmask &= ~IORESOURCE_MEM_64; ret = pbus_size_mem(bus, prefmask, prefmask, - prefmask, prefmask, - realloc_head ? 0 : additional_mem_size, - additional_mem_size, realloc_head); + prefmask, prefmask, + realloc_head ? 0 : additional_mmio_pref_size, + additional_mmio_pref_size, realloc_head); /* * If successful, only non-prefetchable resources @@ -1265,7 +1267,7 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head) if (ret == 0) mask = prefmask; else - additional_mem_size += additional_mem_size; + additional_mmio_size += additional_mmio_pref_size; type2 = type3 = IORESOURCE_MEM; } @@ -1285,8 +1287,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head) * prefetchable resource in a 64-bit prefetchable window. */ pbus_size_mem(bus, mask, IORESOURCE_MEM, type2, type3, - realloc_head ? 0 : additional_mem_size, - additional_mem_size, realloc_head); + realloc_head ? 0 : additional_mmio_size, + additional_mmio_size, realloc_head); break; } } -- cgit v1.2.3 From 7e24bc347e57992d532bc2ed700209b0fc0a4bf5 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Wed, 23 Oct 2019 17:40:52 -0500 Subject: PCI/PM: Apply D2 delay as milliseconds, not microseconds PCI_PM_D2_DELAY is defined as 200, which is milliseconds, but previously we used udelay(), which only waited for 200 microseconds. Use msleep() instead so we wait the correct amount of time. See PCIe r5.0, sec 5.9. Link: https://lore.kernel.org/r/20191101204558.210235-2-helgaas@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki --- drivers/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index a97e2571a527..4dfbde4f944e 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -886,7 +886,7 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) if (state == PCI_D3hot || dev->current_state == PCI_D3hot) pci_dev_d3_sleep(dev); else if (state == PCI_D2 || dev->current_state == PCI_D2) - udelay(PCI_PM_D2_DELAY); + msleep(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); -- cgit v1.2.3 From 993cc6d1bd3af734693a74a3ccc9445dd5b31f9c Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Mon, 28 Oct 2019 08:27:00 -0500 Subject: PCI/PM: Expand PM reset messages to mention D3hot (not just D3) pci_pm_reset() resets a device by putting it in D3hot and bringing it back to D0. Clarify related messages to mention "D3hot" explicitly instead of just "D3". Link: https://lore.kernel.org/r/20191101204558.210235-3-helgaas@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki --- drivers/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 4dfbde4f944e..9378fe5fe475 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4603,7 +4603,7 @@ static int pci_pm_reset(struct pci_dev *dev, int probe) pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, csr); pci_dev_d3_sleep(dev); - return pci_dev_wait(dev, "PM D3->D0", PCIE_RESET_READY_POLL_MS); + return pci_dev_wait(dev, "PM D3hot->D0", PCIE_RESET_READY_POLL_MS); } /** * pcie_wait_for_link - Wait until link is active or inactive -- cgit v1.2.3 From baef7f8e5e91f85ce7625c11370479f9f0778fae Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Wed, 16 Oct 2019 15:23:20 -0500 Subject: PCI/PM: Simplify pci_set_power_state() Check for the PCI_DEV_FLAGS_NO_D3 quirk early, before calling __pci_start_power_transition(). This way all the cases where we don't need to do anything at all are checked up front. This doesn't fix anything because if the caller requested D3hot or D3cold, __pci_start_power_transition() is a no-op. But calling it is pointless and makes the code harder to analyze. Link: https://lore.kernel.org/r/20191101204558.210235-4-helgaas@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki --- drivers/pci/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 9378fe5fe475..ea98c77a6512 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1117,8 +1117,6 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state) if (dev->current_state == state) return 0; - __pci_start_power_transition(dev, state); - /* * This device is quirked not to be put into D3, so don't put it in * D3 @@ -1126,6 +1124,8 @@ 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; + __pci_start_power_transition(dev, state); + /* * To put device in D3cold, we put device into D3hot in native * way, then put device into D3cold with platform ops -- cgit v1.2.3 From 81cfa5908fd6fe610b7c47b742fe30d6d897ba0f Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 5 Nov 2019 11:13:43 +0100 Subject: PCI/PM: Move power state update away from pci_power_up() Move the invocation of pci_update_current_state() from pci_power_up() to pci_pm_default_resume_early(), which is the only caller of that function. Preparatory change, no functional impact. Link: https://lore.kernel.org/r/37482337.udjOGdOKNb@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci-driver.c | 1 + drivers/pci/pci.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index e89fd90eaa93..08d3bdbc8c04 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -531,6 +531,7 @@ static void pci_pm_default_resume(struct pci_dev *pci_dev) static void pci_pm_default_resume_early(struct pci_dev *pci_dev) { pci_power_up(pci_dev); + pci_update_current_state(pci_dev, PCI_D0); pci_restore_state(pci_dev); pci_pme_restore(pci_dev); } diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index ea98c77a6512..11b4d9274d96 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1148,7 +1148,6 @@ void pci_power_up(struct pci_dev *dev) { __pci_start_power_transition(dev, PCI_D0); pci_raw_set_power_state(dev, PCI_D0); - pci_update_current_state(dev, PCI_D0); } /** -- cgit v1.2.3 From adfac8f6b7396b408fa9a8f40ea41112bebb980f Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 5 Nov 2019 11:27:49 +0100 Subject: PCI/PM: Use pci_power_up() in pci_set_power_state() Make it explicitly clear that the code to put devices into D0 in pci_set_power_state() and in pci_pm_default_resume_early() is the same by making the latter use pci_power_up() for transitions into D0. Code rearrangement, no intentional functional impact. Link: https://lore.kernel.org/r/2520019.OZ1nXS5aSj@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci.c | 25 +++++++++++++------------ drivers/pci/pci.h | 2 +- 2 files changed, 14 insertions(+), 13 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 11b4d9274d96..9f0977d0ac5a 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1032,6 +1032,16 @@ static void __pci_start_power_transition(struct pci_dev *dev, pci_power_t state) } } +/** + * pci_power_up - Put the given device into D0 + * @dev: PCI device to power up + */ +int pci_power_up(struct pci_dev *dev) +{ + __pci_start_power_transition(dev, PCI_D0); + return pci_raw_set_power_state(dev, PCI_D0); +} + /** * __pci_dev_set_current_state - Set current state of a PCI device * @dev: Device to handle @@ -1117,6 +1127,9 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state) if (dev->current_state == state) return 0; + if (state == PCI_D0) + return pci_power_up(dev); + /* * This device is quirked not to be put into D3, so don't put it in * D3 @@ -1124,8 +1137,6 @@ 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; - __pci_start_power_transition(dev, state); - /* * To put device in D3cold, we put device into D3hot in native * way, then put device into D3cold with platform ops @@ -1140,16 +1151,6 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state) } EXPORT_SYMBOL(pci_set_power_state); -/** - * pci_power_up - Put the given device into D0 forcibly - * @dev: PCI device to power up - */ -void pci_power_up(struct pci_dev *dev) -{ - __pci_start_power_transition(dev, PCI_D0); - pci_raw_set_power_state(dev, PCI_D0); -} - /** * pci_choose_state - Choose the power state of a PCI device * @dev: PCI device to be suspended diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 3f6947ee3324..7c68312c72f4 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -85,7 +85,7 @@ struct pci_platform_pm_ops { int pci_set_platform_pm(const struct pci_platform_pm_ops *ops); void pci_update_current_state(struct pci_dev *dev, pci_power_t state); void pci_refresh_power_state(struct pci_dev *dev); -void pci_power_up(struct pci_dev *dev); +int pci_power_up(struct pci_dev *dev); void pci_disable_enabled_device(struct pci_dev *dev); int pci_finish_runtime_suspend(struct pci_dev *dev); void pcie_clear_root_pme_status(struct pci_dev *dev); -- cgit v1.2.3 From dc2256b0735d03664a92a6cb94ea4e564dfa237b Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 5 Nov 2019 11:29:16 +0100 Subject: PCI/PM: Fold __pci_start_power_transition() into its caller Because pci_power_up() has become the only caller of __pci_start_power_transition(), there is no need for the latter to be a separate function any more, so fold it into the former, drop a redundant check and reduce the number of lines of code somewhat. Code rearrangement, no intentional functional impact. Link: https://lore.kernel.org/r/3458080.lsoDbfkST9@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci.c | 48 ++++++++++++++++++------------------------------ 1 file changed, 18 insertions(+), 30 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 9f0977d0ac5a..56bc79e33286 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1003,42 +1003,30 @@ void pci_wakeup_bus(struct pci_bus *bus) } /** - * __pci_start_power_transition - Start power transition of a PCI device - * @dev: PCI device to handle. - * @state: State to put the device into. + * pci_power_up - Put the given device into D0 + * @dev: PCI device to power up */ -static void __pci_start_power_transition(struct pci_dev *dev, pci_power_t state) +int pci_power_up(struct pci_dev *dev) { - if (state == PCI_D0) { - pci_platform_power_transition(dev, PCI_D0); + pci_platform_power_transition(dev, PCI_D0); + + /* + * Mandatory power management transition delays, see PCI Express Base + * Specification Revision 2.0 Section 6.6.1: Conventional Reset. Do not + * delay for devices powered on/off by corresponding bridge, because + * have already delayed for the bridge. + */ + if (dev->runtime_d3cold) { + if (dev->d3cold_delay && !dev->imm_ready) + msleep(dev->d3cold_delay); /* - * Mandatory power management transition delays, see - * PCI Express Base Specification Revision 2.0 Section - * 6.6.1: Conventional Reset. Do not delay for - * devices powered on/off by corresponding bridge, - * because have already delayed for the bridge. + * 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 */ - if (dev->runtime_d3cold) { - if (dev->d3cold_delay && !dev->imm_ready) - msleep(dev->d3cold_delay); - /* - * 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_wakeup_bus(dev->subordinate); - } + pci_wakeup_bus(dev->subordinate); } -} -/** - * pci_power_up - Put the given device into D0 - * @dev: PCI device to power up - */ -int pci_power_up(struct pci_dev *dev) -{ - __pci_start_power_transition(dev, PCI_D0); return pci_raw_set_power_state(dev, PCI_D0); } -- cgit v1.2.3 From d6aa37cd04fdafaf31ae89691e537535df43ca78 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 5 Nov 2019 11:30:36 +0100 Subject: PCI/PM: Avoid exporting __pci_complete_power_transition() Notice that radeon_set_suspend(), which is the only caller of __pci_complete_power_transition() outside of pci.c, really only cares about the pci_platform_power_transition() invoked by it, so export the latter instead of it, update the radeon driver to call pci_platform_power_transition() directly and make __pci_complete_power_transition() static. Code rearrangement, no intentional functional impact. Link: https://lore.kernel.org/r/1731661.ykamz2Tiuf@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci.c | 6 +++--- drivers/video/fbdev/aty/radeon_pm.c | 2 +- include/linux/pci.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 56bc79e33286..2f53234f9e91 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -963,7 +963,7 @@ void pci_refresh_power_state(struct pci_dev *dev) * @dev: PCI device to handle. * @state: State to put the device into. */ -static int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state) +int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state) { int error; @@ -979,6 +979,7 @@ static int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state) return error; } +EXPORT_SYMBOL_GPL(pci_platform_power_transition); /** * pci_wakeup - Wake up a PCI device @@ -1061,7 +1062,7 @@ void pci_bus_set_current_state(struct pci_bus *bus, pci_power_t state) * * This function should not be called directly by device drivers. */ -int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state) +static int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state) { int ret; @@ -1073,7 +1074,6 @@ int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state) pci_bus_set_current_state(dev->subordinate, PCI_D3cold); return ret; } -EXPORT_SYMBOL_GPL(__pci_complete_power_transition); /** * pci_set_power_state - Set the power state of a PCI device diff --git a/drivers/video/fbdev/aty/radeon_pm.c b/drivers/video/fbdev/aty/radeon_pm.c index 2dc5703eac51..7c4483c7f313 100644 --- a/drivers/video/fbdev/aty/radeon_pm.c +++ b/drivers/video/fbdev/aty/radeon_pm.c @@ -2593,7 +2593,7 @@ static void radeon_set_suspend(struct radeonfb_info *rinfo, int suspend) * calling pci_set_power_state() */ radeonfb_whack_power_state(rinfo, PCI_D2); - __pci_complete_power_transition(rinfo->pdev, PCI_D2); + pci_platform_power_transition(rinfo->pdev, PCI_D2); } else { printk(KERN_DEBUG "radeonfb (%s): switching to D0 state...\n", pci_name(rinfo->pdev)); diff --git a/include/linux/pci.h b/include/linux/pci.h index 9b0e35e09874..86976cccdfe3 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1228,7 +1228,7 @@ struct pci_cap_saved_state *pci_find_saved_ext_cap(struct pci_dev *dev, int pci_add_cap_save_buffer(struct pci_dev *dev, char cap, unsigned int size); int pci_add_ext_cap_save_buffer(struct pci_dev *dev, u16 cap, unsigned int size); -int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state); +int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state); int pci_set_power_state(struct pci_dev *dev, pci_power_t state); pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state); bool pci_pme_capable(struct pci_dev *dev, pci_power_t state); -- cgit v1.2.3 From 9c77e63bd8dcbb3b59294e9176c00c5fcab3c9c6 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 5 Nov 2019 17:32:08 +0100 Subject: PCI/PM: Fold __pci_complete_power_transition() into its caller Because pci_set_power_state() has become the only caller of __pci_complete_power_transition(), there is no need for the latter to be a separate function any more, so fold it into the former, drop a redundant check and reduce the number of lines of code somewhat. Code rearrangement, no intentional functional impact. Link: https://lore.kernel.org/r/15576968.k611qn3UU0@kreacher Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 2f53234f9e91..37fca1e51d16 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1055,26 +1055,6 @@ 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_complete_power_transition - Complete power transition of a PCI device - * @dev: PCI device to handle. - * @state: State to put the device into. - * - * This function should not be called directly by device drivers. - */ -static int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state) -{ - int ret; - - if (state <= PCI_D0) - return -EINVAL; - ret = pci_platform_power_transition(dev, state); - /* Power off the bridge may power off the whole hierarchy */ - if (!ret && state == PCI_D3cold) - pci_bus_set_current_state(dev->subordinate, PCI_D3cold); - return ret; -} - /** * pci_set_power_state - Set the power state of a PCI device * @dev: PCI device to handle. @@ -1132,10 +1112,14 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state) error = pci_raw_set_power_state(dev, state > PCI_D3hot ? PCI_D3hot : state); - if (!__pci_complete_power_transition(dev, state)) - error = 0; + if (pci_platform_power_transition(dev, state)) + return error; - 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); + + return 0; } EXPORT_SYMBOL(pci_set_power_state); -- cgit v1.2.3 From e43f15ea2f6d6858675fa1baa5cb624f17269af0 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Fri, 2 Aug 2019 18:47:22 -0500 Subject: PCI/PM: Decode D3cold power state correctly Use pci_power_name() to print pci_power_t correctly. This changes: "state 0" or "D0" to "D0" "state 1" or "D1" to "D1" "state 2" or "D2" to "D2" "state 3" or "D3" to "D3hot" "state 4" or "D4" to "D3cold" Changes dmesg logging only, no other functional change intended. Link: https://lore.kernel.org/r/20190822200551.129039-3-helgaas@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki Reviewed-by: Keith Busch Reviewed-by: Mika Westerberg --- drivers/pci/pci.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 37fca1e51d16..3376c663035d 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -834,14 +834,16 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) return -EINVAL; /* - * Validate current state: - * Can enter D0 from any state, but if we can only go deeper - * to sleep if we're already in a low power state + * 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 (state != PCI_D0 && dev->current_state <= PCI_D3cold && dev->current_state > state) { - pci_err(dev, "invalid power transition (from state %d to %d)\n", - 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; } @@ -891,8 +893,9 @@ 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); dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK); if (dev->current_state != state) - pci_info_ratelimited(dev, "Refused to change power state, currently in D%d\n", - dev->current_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 -- cgit v1.2.3 From 327ccbbcc1497bff6d6f543c1f37c43a1653671f Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 1 Aug 2019 11:50:56 -0500 Subject: PCI/PM: Return error when changing power state from D3cold pci_raw_set_power_state() uses the Power Management capability to change a device's power state. The capability is in config space, which is accessible in D0, D1, D2, and D3hot, but not in D3cold. If we call pci_raw_set_power_state() on a device that's in D3cold, config reads fail and return ~0 data, which we erroneously interpreted as "the device is in D3hot", leading to messages like this: pcieport 0000:03:00.0: Refused to change power state, currently in D3 The PCI_PM_CTRL has several RsvdP fields, so ~0 is never a valid register value. If we get that value, print a more informative message and return an error. Changing the power state of a device from D3cold must be done by a platform power management method or some other non-config space mechanism. Link: https://lore.kernel.org/r/20190822200551.129039-4-helgaas@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki Reviewed-by: Keith Busch Reviewed-by: Mika Westerberg --- drivers/pci/pci.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 3376c663035d..d6ff1a98bd8d 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -853,6 +853,12 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) return -EIO; pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); + if (pmcsr == (u16) ~0) { + pci_err(dev, "can't change power state from %s to %s (config space 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. -- cgit v1.2.3 From 4827d63891b6a839dac49c6ab62e61c4b011c4f2 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Tue, 12 Nov 2019 12:16:16 +0300 Subject: PCI/PM: Add pcie_wait_for_link_delay() Add pcie_wait_for_link_delay(). Similar to pcie_wait_for_link() but allows passing custom activation delay in milliseconds. Link: https://lore.kernel.org/r/20191112091617.70282-2-mika.westerberg@linux.intel.com Signed-off-by: Mika Westerberg Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki --- drivers/pci/pci.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d6ff1a98bd8d..11e4b495cfac 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4586,14 +4586,17 @@ static int pci_pm_reset(struct pci_dev *dev, int probe) return pci_dev_wait(dev, "PM D3hot->D0", PCIE_RESET_READY_POLL_MS); } + /** - * pcie_wait_for_link - Wait until link is active or inactive + * pcie_wait_for_link_delay - Wait until link is active or inactive * @pdev: Bridge device * @active: waiting for active or inactive? + * @delay: Delay to wait after link has become active (in ms) * * Use this to wait till link becomes active or inactive. */ -bool pcie_wait_for_link(struct pci_dev *pdev, bool active) +static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active, + int delay) { int timeout = 1000; bool ret; @@ -4630,13 +4633,25 @@ bool pcie_wait_for_link(struct pci_dev *pdev, bool active) timeout -= 10; } if (active && ret) - msleep(100); + msleep(delay); else if (ret != active) pci_info(pdev, "Data Link Layer Link Active not %s in 1000 msec\n", active ? "set" : "cleared"); return ret == active; } +/** + * pcie_wait_for_link - Wait until link is active or inactive + * @pdev: Bridge device + * @active: waiting for active or inactive? + * + * Use this to wait till link becomes active or inactive. + */ +bool pcie_wait_for_link(struct pci_dev *pdev, bool active) +{ + return pcie_wait_for_link_delay(pdev, active, 100); +} + void pci_reset_secondary_bus(struct pci_dev *dev) { u16 ctrl; -- cgit v1.2.3 From ad9001f2f41198784b0423646450ba2cb24793a3 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Tue, 12 Nov 2019 12:16:17 +0300 Subject: PCI/PM: Add missing link delays required by the PCIe spec Currently Linux does not follow PCIe spec regarding the required delays after reset. A concrete example is a Thunderbolt add-in-card that consists of a PCIe switch and two PCIe endpoints: +-1b.0-[01-6b]----00.0-[02-6b]--+-00.0-[03]----00.0 TBT controller +-01.0-[04-36]-- DS hotplug port +-02.0-[37]----00.0 xHCI controller \-04.0-[38-6b]-- DS hotplug port The root port (1b.0) and the PCIe switch downstream ports are all PCIe Gen3 so they support 8GT/s link speeds. We wait for the PCIe hierarchy to enter D3cold (runtime): pcieport 0000:00:1b.0: power state changed by ACPI to D3cold When it wakes up from D3cold, according to the PCIe 5.0 section 5.8 the PCIe switch is put to reset and its power is re-applied. This means that we must follow the rules in PCIe 5.0 section 6.6.1. For the PCIe Gen3 ports we are dealing with here, the following applies: With a Downstream Port that supports Link speeds greater than 5.0 GT/s, software must wait a minimum of 100 ms after Link training completes before sending a Configuration Request to the device immediately below that Port. Software can determine when Link training completes by polling the Data Link Layer Link Active bit or by setting up an associated interrupt (see Section 6.7.3.3). Translating this into the above topology we would need to do this (DLLLA stands for Data Link Layer Link Active): 0000:00:1b.0: wait for 100 ms after DLLLA is set before access to 0000:01:00.0 0000:02:00.0: wait for 100 ms after DLLLA is set before access to 0000:03:00.0 0000:02:02.0: wait for 100 ms after DLLLA is set before access to 0000:37:00.0 I've instrumented the kernel with some additional logging so we can see the actual delays performed: pcieport 0000:00:1b.0: power state changed by ACPI to D0 pcieport 0000:00:1b.0: waiting for D3cold delay of 100 ms pcieport 0000:00:1b.0: waiting for D3hot delay of 10 ms pcieport 0000:02:01.0: waiting for D3hot delay of 10 ms pcieport 0000:02:04.0: waiting for D3hot delay of 10 ms For the switch upstream port (01:00.0 reachable through 00:1b.0 root port) we wait for 100 ms but not taking into account the DLLLA requirement. We then wait 10 ms for D3hot -> D0 transition of the root port and the two downstream hotplug ports. This means that we deviate from what the spec requires. Performing the same check for system sleep (s2idle) transitions it turns out to be even worse. None of the mandatory delays are performed. If this would be S3 instead of s2idle then according to PCI FW spec 3.2 section 4.6.8. there is a specific _DSM that allows the OS to skip the delays but this platform does not provide the _DSM and does not go to S3 anyway so no firmware is involved that could already handle these delays. On this particular platform these delays are not actually needed because there is an additional delay as part of the ACPI power resource that is used to turn on power to the hierarchy but since that additional delay is not required by any of standards (PCIe, ACPI) it is not present in the Intel Ice Lake, for example where missing the mandatory delays causes pciehp to start tearing down the stack too early (links are not yet trained). Below is an example how it looks like when this happens: pcieport 0000:83:04.0: pciehp: Slot(4): Card not present pcieport 0000:87:04.0: PME# disabled pcieport 0000:83:04.0: pciehp: pciehp_unconfigure_device: domain:bus:dev = 0000:86:00 pcieport 0000:86:00.0: Refused to change power state, currently in D3 pcieport 0000:86:00.0: restoring config space at offset 0x3c (was 0xffffffff, writing 0x201ff) pcieport 0000:86:00.0: restoring config space at offset 0x38 (was 0xffffffff, writing 0x0) ... There is also one reported case (see the bugzilla link below) where the missing delay causes xHCI on a Titan Ridge controller fail to runtime resume when USB-C dock is plugged. This does not involve pciehp but instead the PCI core fails to runtime resume the xHCI device: pcieport 0000:04:02.0: restoring config space at offset 0xc (was 0x10000, writing 0x10020) pcieport 0000:04:02.0: restoring config space at offset 0x4 (was 0x100000, writing 0x100406) xhci_hcd 0000:39:00.0: Refused to change power state, currently in D3 xhci_hcd 0000:39:00.0: restoring config space at offset 0x3c (was 0xffffffff, writing 0x1ff) xhci_hcd 0000:39:00.0: restoring config space at offset 0x38 (was 0xffffffff, writing 0x0) ... Add a new function pci_bridge_wait_for_secondary_bus() that is called on PCI core resume and runtime resume paths accordingly if the bridge entered D3cold (and thus went through reset). This is second attempt to add the missing delays. The previous solution in c2bf1fc212f7 ("PCI: Add missing link delays required by the PCIe spec") was reverted because of two issues it caused: 1. One system become unresponsive after S3 resume due to PME service spinning in pcie_pme_work_fn(). The root port in question reports that the xHCI sent PME but the xHCI device itself does not have PME status set. The PME status bit is never cleared in the root port resulting the indefinite loop in pcie_pme_work_fn(). 2. Slows down resume if the root/downstream port does not support Data Link Layer Active Reporting because pcie_wait_for_link_delay() waits 1100 ms in that case. This version should avoid the above issues because we restrict the delay to happen only if the port went into D3cold. Link: https://lore.kernel.org/linux-pci/SL2P216MB01878BBCD75F21D882AEEA2880C60@SL2P216MB0187.KORP216.PROD.OUTLOOK.COM/ Link: https://bugzilla.kernel.org/show_bug.cgi?id=203885 Link: https://lore.kernel.org/r/20191112091617.70282-3-mika.westerberg@linux.intel.com Reported-by: Kai-Heng Feng Tested-by: Kai-Heng Feng Signed-off-by: Mika Westerberg Signed-off-by: Bjorn Helgaas --- drivers/pci/pci-driver.c | 11 +++- drivers/pci/pci.c | 128 ++++++++++++++++++++++++++++++++++++++++++++--- drivers/pci/pci.h | 1 + 3 files changed, 133 insertions(+), 7 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 08d3bdbc8c04..0454ca0e4e3f 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -890,6 +890,8 @@ static int pci_pm_resume_noirq(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + pci_power_t prev_state = pci_dev->current_state; + bool skip_bus_pm = pci_dev->skip_bus_pm; if (dev_pm_may_skip_resume(dev)) return 0; @@ -908,12 +910,15 @@ static int pci_pm_resume_noirq(struct device *dev) * configuration here and attempting to put them into D0 again is * pointless, so avoid doing that. */ - if (!(pci_dev->skip_bus_pm && pm_suspend_no_platform())) + if (!(skip_bus_pm && pm_suspend_no_platform())) pci_pm_default_resume_early(pci_dev); pci_fixup_device(pci_fixup_resume_early, pci_dev); pcie_pme_root_status_cleanup(pci_dev); + if (!skip_bus_pm && prev_state == PCI_D3cold) + pci_bridge_wait_for_secondary_bus(pci_dev); + if (pci_has_legacy_pm_support(pci_dev)) return 0; @@ -1299,6 +1304,7 @@ static int pci_pm_runtime_resume(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + pci_power_t prev_state = pci_dev->current_state; int error = 0; /* @@ -1314,6 +1320,9 @@ static int pci_pm_runtime_resume(struct device *dev) pci_fixup_device(pci_fixup_resume_early, pci_dev); pci_pm_default_resume(pci_dev); + if (prev_state == PCI_D3cold) + pci_bridge_wait_for_secondary_bus(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 11e4b495cfac..b9d91370856a 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1021,14 +1021,11 @@ int pci_power_up(struct pci_dev *dev) pci_platform_power_transition(dev, PCI_D0); /* - * Mandatory power management transition delays, see PCI Express Base - * Specification Revision 2.0 Section 6.6.1: Conventional Reset. Do not - * delay for devices powered on/off by corresponding bridge, because - * have already delayed for the bridge. + * 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) { - if (dev->d3cold_delay && !dev->imm_ready) - msleep(dev->d3cold_delay); /* * When powering on a bridge from D3cold, the whole hierarchy * may be powered on into D0uninitialized state, resume them to @@ -4652,6 +4649,125 @@ bool pcie_wait_for_link(struct pci_dev *pdev, bool active) return pcie_wait_for_link_delay(pdev, active, 100); } +/* + * Find maximum D3cold delay required by all the devices on the bus. The + * spec says 100 ms, but firmware can lower it and we allow drivers to + * increase it as well. + * + * Called with @pci_bus_sem locked for reading. + */ +static int pci_bus_max_d3cold_delay(const struct pci_bus *bus) +{ + const struct pci_dev *pdev; + int min_delay = 100; + int max_delay = 0; + + list_for_each_entry(pdev, &bus->devices, bus_list) { + if (pdev->d3cold_delay < min_delay) + min_delay = pdev->d3cold_delay; + if (pdev->d3cold_delay > max_delay) + max_delay = pdev->d3cold_delay; + } + + return max(min_delay, max_delay); +} + +/** + * pci_bridge_wait_for_secondary_bus - Wait for secondary bus to be accessible + * @dev: PCI bridge + * + * Handle necessary delays before access to the devices on the secondary + * side of the bridge are permitted after D3cold to D0 transition. + * + * For PCIe this means the delays in PCIe 5.0 section 6.6.1. For + * conventional PCI it means Tpvrh + Trhfa specified in PCI 3.0 section + * 4.3.2. + */ +void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev) +{ + struct pci_dev *child; + int delay; + + if (pci_dev_is_disconnected(dev)) + return; + + if (!pci_is_bridge(dev) || !dev->bridge_d3) + return; + + down_read(&pci_bus_sem); + + /* + * We only deal with devices that are present currently on the bus. + * For any hot-added devices the access delay is handled in pciehp + * board_added(). In case of ACPI hotplug the firmware is expected + * to configure the devices before OS is notified. + */ + if (!dev->subordinate || list_empty(&dev->subordinate->devices)) { + up_read(&pci_bus_sem); + return; + } + + /* Take d3cold_delay requirements into account */ + delay = pci_bus_max_d3cold_delay(dev->subordinate); + if (!delay) { + up_read(&pci_bus_sem); + return; + } + + child = list_first_entry(&dev->subordinate->devices, struct pci_dev, + bus_list); + up_read(&pci_bus_sem); + + /* + * Conventional PCI and PCI-X we need to wait Tpvrh + Trhfa before + * accessing the device after reset (that is 1000 ms + 100 ms). In + * practice this should not be needed because we don't do power + * management for them (see pci_bridge_d3_possible()). + */ + if (!pci_is_pcie(dev)) { + pci_dbg(dev, "waiting %d ms for secondary bus\n", 1000 + delay); + msleep(1000 + delay); + return; + } + + /* + * For PCIe downstream and root ports that do not support speeds + * greater than 5 GT/s need to wait minimum 100 ms. For higher + * speeds (gen3) we need to wait first for the data link layer to + * become active. + * + * However, 100 ms is the minimum and the PCIe spec says the + * software must allow at least 1s before it can determine that the + * device that did not respond is a broken device. There is + * evidence that 100 ms is not always enough, for example certain + * Titan Ridge xHCI controller does not always respond to + * configuration requests if we only wait for 100 ms (see + * https://bugzilla.kernel.org/show_bug.cgi?id=203885). + * + * Therefore we wait for 100 ms and check for the device presence. + * If it is still not present give it an additional 100 ms. + */ + if (!pcie_downstream_port(dev)) + return; + + if (pcie_get_speed_cap(dev) <= PCIE_SPEED_5_0GT) { + pci_dbg(dev, "waiting %d ms for downstream link\n", delay); + msleep(delay); + } else { + pci_dbg(dev, "waiting %d ms for downstream link, after activation\n", + delay); + if (!pcie_wait_for_link_delay(dev, true, delay)) { + /* Did not train, no need to wait any further */ + return; + } + } + + if (!pci_device_is_present(child)) { + pci_dbg(child, "waiting additional %d ms to become accessible\n", delay); + msleep(delay); + } +} + void pci_reset_secondary_bus(struct pci_dev *dev) { u16 ctrl; diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 7c68312c72f4..bf3c7e5de6fa 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -104,6 +104,7 @@ void pci_allocate_cap_save_buffers(struct pci_dev *dev); void pci_free_cap_save_buffers(struct pci_dev *dev); bool pci_bridge_d3_possible(struct pci_dev *dev); void pci_bridge_d3_update(struct pci_dev *dev); +void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev); static inline void pci_wakeup_event(struct pci_dev *dev) { -- cgit v1.2.3 From bae26849372b83c65da73d19ff58e987d70e6600 Mon Sep 17 00:00:00 2001 From: Vidya Sagar Date: Wed, 20 Nov 2019 10:47:42 +0530 Subject: PCI/PM: Move pci_dev_wait() definition earlier Move the definition of pci_dev_wait() above pci_power_up() so that it can be called from the latter with no change in functionality. This is a pure code move with no functional change. Link: https://lore.kernel.org/r/20191120051743.23124-1-vidyas@nvidia.com Signed-off-by: Vidya Sagar Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 82 +++++++++++++++++++++++++++---------------------------- 1 file changed, 41 insertions(+), 41 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b9d91370856a..d6b44ae8baa4 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1012,6 +1012,47 @@ void pci_wakeup_bus(struct pci_bus *bus) pci_walk_bus(bus, pci_wakeup, NULL); } +static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout) +{ + int delay = 1; + u32 id; + + /* + * After reset, the device should not silently discard config + * requests, but it may still indicate that it needs more time by + * responding to them with CRS completions. The Root Port will + * generally synthesize ~0 data to complete the read (except when + * CRS SV is enabled and the read was for the Vendor ID; in that + * case it synthesizes 0x0001 data). + * + * Wait for the device to return a non-CRS completion. Read the + * Command register instead of Vendor ID so we don't have to + * contend with the CRS SV value. + */ + pci_read_config_dword(dev, PCI_COMMAND, &id); + while (id == ~0) { + if (delay > timeout) { + pci_warn(dev, "not ready %dms after %s; giving up\n", + delay - 1, reset_type); + return -ENOTTY; + } + + if (delay > 1000) + pci_info(dev, "not ready %dms after %s; waiting\n", + delay - 1, reset_type); + + msleep(delay); + delay *= 2; + pci_read_config_dword(dev, PCI_COMMAND, &id); + } + + if (delay > 1000) + pci_info(dev, "ready %dms after %s\n", delay - 1, + reset_type); + + return 0; +} + /** * pci_power_up - Put the given device into D0 * @dev: PCI device to power up @@ -4406,47 +4447,6 @@ int pci_wait_for_pending_transaction(struct pci_dev *dev) } EXPORT_SYMBOL(pci_wait_for_pending_transaction); -static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout) -{ - int delay = 1; - u32 id; - - /* - * After reset, the device should not silently discard config - * requests, but it may still indicate that it needs more time by - * responding to them with CRS completions. The Root Port will - * generally synthesize ~0 data to complete the read (except when - * CRS SV is enabled and the read was for the Vendor ID; in that - * case it synthesizes 0x0001 data). - * - * Wait for the device to return a non-CRS completion. Read the - * Command register instead of Vendor ID so we don't have to - * contend with the CRS SV value. - */ - pci_read_config_dword(dev, PCI_COMMAND, &id); - while (id == ~0) { - if (delay > timeout) { - pci_warn(dev, "not ready %dms after %s; giving up\n", - delay - 1, reset_type); - return -ENOTTY; - } - - if (delay > 1000) - pci_info(dev, "not ready %dms after %s; waiting\n", - delay - 1, reset_type); - - msleep(delay); - delay *= 2; - pci_read_config_dword(dev, PCI_COMMAND, &id); - } - - if (delay > 1000) - pci_info(dev, "ready %dms after %s\n", delay - 1, - reset_type); - - return 0; -} - /** * pcie_has_flr - check if a device supports function level resets * @dev: device to check -- cgit v1.2.3 From bbd8810d399812f2016713565e4d8ff8f1508aa6 Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczynski Date: Tue, 3 Sep 2019 13:30:59 +0200 Subject: PCI: Remove unused includes and superfluous struct declaration Remove and from being included directly as part of the include/linux/of_pci.h, and remove superfluous declaration of struct of_phandle_args. Move users of include to include and directly rather than rely on both being included transitively through . Link: https://lore.kernel.org/r/20190903113059.2901-1-kw@linux.com Signed-off-by: Krzysztof Wilczynski Signed-off-by: Bjorn Helgaas Reviewed-by: Rob Herring --- drivers/iommu/of_iommu.c | 2 ++ drivers/irqchip/irq-gic-v2m.c | 1 + drivers/irqchip/irq-gic-v3-its-pci-msi.c | 1 + drivers/pci/controller/dwc/pcie-designware-host.c | 1 + drivers/pci/controller/pci-aardvark.c | 1 + drivers/pci/controller/pci-thunder-pem.c | 1 + drivers/pci/pci.c | 1 + drivers/pci/probe.c | 1 + include/linux/of_pci.h | 5 ++--- 9 files changed, 11 insertions(+), 3 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 614a93aa5305..026ad2b29dcd 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include #include diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c index e88e75c22b6a..fbec07d634ad 100644 --- a/drivers/irqchip/irq-gic-v2m.c +++ b/drivers/irqchip/irq-gic-v2m.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c index 229d586c3d7a..87711e0f8014 100644 --- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c +++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c @@ -5,6 +5,7 @@ */ #include +#include #include #include #include diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 0f36a926059a..43ce6de70374 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -10,6 +10,7 @@ #include #include +#include #include #include #include diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index fc0fe4d4de49..3a05f6ca95b0 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include diff --git a/drivers/pci/controller/pci-thunder-pem.c b/drivers/pci/controller/pci-thunder-pem.c index f127ce8bd4ef..9491e266b1ea 100644 --- a/drivers/pci/controller/pci-thunder-pem.c +++ b/drivers/pci/controller/pci-thunder-pem.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e7982af9a5d8..2d36995e4ea5 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 3d5271a7a849..7c5d68b807ef 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h index 21a89c4880fa..29658c0ee71f 100644 --- a/include/linux/of_pci.h +++ b/include/linux/of_pci.h @@ -2,11 +2,10 @@ #ifndef __OF_PCI_H #define __OF_PCI_H -#include -#include +#include +#include struct pci_dev; -struct of_phandle_args; struct device_node; #if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_PCI) -- cgit v1.2.3