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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3 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.2.3