Hi All,
This series supersedes the one at
https://lore.kernel.org/linux-pm/4198163.ejJDZkT8p0@kreacher
It addresses some potential issues related to PCI device transitions from
low-power states into D0 and makes the related code more straightforward
and so easier to follow.
Please refer to the patch changelogs for details.
Thanks!
From: Rafael J. Wysocki <[email protected]>
This flag is not needed any more, so drop it.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/pci/pci-driver.c | 2 --
drivers/pci/pci.c | 3 ---
include/linux/pci.h | 4 ----
3 files changed, 9 deletions(-)
Index: linux-pm/drivers/pci/pci-driver.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -1337,8 +1337,6 @@ static int pci_pm_runtime_resume(struct
if (pm && pm->runtime_resume)
error = pm->runtime_resume(dev);
- pci_dev->runtime_d3cold = false;
-
return error;
}
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -2703,8 +2703,6 @@ int pci_finish_runtime_suspend(struct pc
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 pc
if (error) {
pci_enable_wake(dev, target_state, false);
pci_restore_ptm_state(dev);
- dev->runtime_d3cold = false;
}
return error;
Index: linux-pm/include/linux/pci.h
===================================================================
--- linux-pm.orig/include/linux/pci.h
+++ linux-pm/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
From: Rafael J. Wysocki <[email protected]>
Save one config space access in pci_update_current_state() by
testing the retireved 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.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/pci/pci.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1201,14 +1201,17 @@ static int pci_raw_set_power_state(struc
*/
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;
}
From: Rafael J. Wysocki <[email protected]>
Because pci_set_power_state() is the only caller of
pci_set_low_power_state(), move the latter next to the former.
No functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/pci/pci.c | 174 +++++++++++++++++++++++++++---------------------------
1 file changed, 87 insertions(+), 87 deletions(-)
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1068,93 +1068,6 @@ static inline bool platform_pci_bridge_d
}
/**
- * pci_set_low_power_state - Program the given device into a low-power state
- * @dev: PCI device to handle.
- * @state: PCI power state (D1, D2, D3hot) to put the device into.
- *
- * 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, "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.
- * This doesn't affect PME_Status, disables PME_En, and
- * sets PowerState to 0.
- */
- if (dev->current_state <= PCI_D2) {
- 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)
- 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.
* @state: State to cache in case the device doesn't have the PM capability
@@ -1391,6 +1304,93 @@ static int pci_set_full_power_state(stru
if (dev->bus->self)
pcie_aspm_pm_state_change(dev->bus->self);
+
+ return 0;
+}
+
+/**
+ * pci_set_low_power_state - Program the given device into a low-power state
+ * @dev: PCI device to handle.
+ * @state: PCI power state (D1, D2, D3hot) to put the device into.
+ *
+ * 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, "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.
+ * This doesn't affect PME_Status, disables PME_En, and
+ * sets PowerState to 0.
+ */
+ if (dev->current_state <= PCI_D2) {
+ 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)
+ 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;
}
Hi All,
On Saturday, April 9, 2022 3:03:14 PM CEST Rafael J. Wysocki wrote:
> Hi All,
>
> This series supersedes the one at
>
> https://lore.kernel.org/linux-pm/4198163.ejJDZkT8p0@kreacher
>
> It addresses some potential issues related to PCI device transitions from
> low-power states into D0 and makes the related code more straightforward
> and so easier to follow.
>
> Please refer to the patch changelogs for details.
Here's a v2 of this patch series which is being sent, because I realized that
one of the checks in pci_power_up() added by patch [4/7] in v1 was redundant
and can be dropped, but that affected the last 3 patches in the series and
then I noticed that more improvements were possible and hence the new patches
[2/9].
Thanks!
From: Rafael J. Wysocki <[email protected]>
Because pci_set_power_state() is the only caller of
pci_set_low_power_state(), move the latter next to the former.
No functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v1 -> v2:
* Take v1 -> v2 difference in the previous patch into account.
---
drivers/pci/pci.c | 160 +++++++++++++++++++++++++++---------------------------
1 file changed, 80 insertions(+), 80 deletions(-)
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1068,86 +1068,6 @@ static inline bool platform_pci_bridge_d
}
/**
- * pci_set_low_power_state - Program the given device into a low-power state
- * @dev: PCI device to handle.
- * @state: PCI power state (D1, D2, D3hot) to put the device into.
- *
- * 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, "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;
- }
-
- 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)
- 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.
* @state: State to cache in case the device doesn't have the PM capability
@@ -1384,6 +1304,86 @@ static int pci_set_full_power_state(stru
if (dev->bus->self)
pcie_aspm_pm_state_change(dev->bus->self);
+
+ return 0;
+}
+
+/**
+ * pci_set_low_power_state - Program the given device into a low-power state
+ * @dev: PCI device to handle.
+ * @state: PCI power state (D1, D2, D3hot) to put the device into.
+ *
+ * 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, "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;
+ }
+
+ 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)
+ 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;
}
On Monday, April 11, 2022 4:17:41 PM CEST Rafael J. Wysocki wrote:
> Hi All,
>
> On Saturday, April 9, 2022 3:03:14 PM CEST Rafael J. Wysocki wrote:
> > Hi All,
> >
> > This series supersedes the one at
> >
> > https://lore.kernel.org/linux-pm/4198163.ejJDZkT8p0@kreacher
> >
> > It addresses some potential issues related to PCI device transitions from
> > low-power states into D0 and makes the related code more straightforward
> > and so easier to follow.
> >
> > Please refer to the patch changelogs for details.
>
> Here's a v2 of this patch series which is being sent, because I realized that
> one of the checks in pci_power_up() added by patch [4/7] in v1 was redundant
> and can be dropped, but that affected the last 3 patches in the series and
> then I noticed that more improvements were possible and hence the new patches
> [2/9].
Here's a v3 of this series with some minor review comments addressed and R-by
tags from Mika added.
Thanks!
From: Rafael J. Wysocki <[email protected]>
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.
Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
---
v1 -> v3:
* Fixed typo in the changelog (missing word "bus").
* Added R-by from Mika.
---
drivers/pci/pci.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1452,19 +1452,25 @@ int pci_set_power_state(struct pci_dev *
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;
}
From: Rafael J. Wysocki <[email protected]>
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. Modify error messages printed by it to make them more consistent
with the messages printed by pci_power_up() and
pci_set_full_power_state().
3. Change the log level of one of the messages to "debug", because
it only indicates a programming mistake.
4. Make it return -ENODEV (instead of -EIO) when the device is not
accessible.
Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
---
v1 -> v3:
* Added R-by from Mika.
---
drivers/pci/pci.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1345,16 +1345,9 @@ static int pci_set_low_power_state(struc
{
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.,
@@ -1362,7 +1355,7 @@ static int pci_set_low_power_state(struc
* 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;
@@ -1375,10 +1368,10 @@ static int pci_set_low_power_state(struc
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;
+ return -ENODEV;
}
pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
From: Rafael J. Wysocki <[email protected]>
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 nonzeor 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.
Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
---
v2 -> v3:
* Added R-by from Mika.
---
drivers/pci/pci-driver.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Index: linux-pm/drivers/pci/pci-driver.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -551,11 +551,15 @@ static void pci_pm_default_resume(struct
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)
{
if (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);
}
@@ -1080,7 +1084,7 @@ static int pci_pm_thaw_noirq(struct devi
* 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))
On Thu, Apr 14, 2022 at 03:00:23PM +0200, Rafael J. Wysocki wrote:
> On Monday, April 11, 2022 4:17:41 PM CEST Rafael J. Wysocki wrote:
> > Hi All,
> >
> > On Saturday, April 9, 2022 3:03:14 PM CEST Rafael J. Wysocki wrote:
> > > Hi All,
> > >
> > > This series supersedes the one at
> > >
> > > https://lore.kernel.org/linux-pm/4198163.ejJDZkT8p0@kreacher
> > >
> > > It addresses some potential issues related to PCI device transitions from
> > > low-power states into D0 and makes the related code more straightforward
> > > and so easier to follow.
> > >
> > > Please refer to the patch changelogs for details.
> >
> > Here's a v2 of this patch series which is being sent, because I realized that
> > one of the checks in pci_power_up() added by patch [4/7] in v1 was redundant
> > and can be dropped, but that affected the last 3 patches in the series and
> > then I noticed that more improvements were possible and hence the new patches
> > [2/9].
>
> Here's a v3 of this series with some minor review comments addressed and R-by
> tags from Mika added.
Applied to pci/pm for v5.19, thanks, Rafael!
On Wed, Apr 20, 2022 at 6:25 PM Bjorn Helgaas <[email protected]> wrote:
>
> On Thu, Apr 14, 2022 at 03:00:23PM +0200, Rafael J. Wysocki wrote:
> > On Monday, April 11, 2022 4:17:41 PM CEST Rafael J. Wysocki wrote:
> > > Hi All,
> > >
> > > On Saturday, April 9, 2022 3:03:14 PM CEST Rafael J. Wysocki wrote:
> > > > Hi All,
> > > >
> > > > This series supersedes the one at
> > > >
> > > > https://lore.kernel.org/linux-pm/4198163.ejJDZkT8p0@kreacher
> > > >
> > > > It addresses some potential issues related to PCI device transitions from
> > > > low-power states into D0 and makes the related code more straightforward
> > > > and so easier to follow.
> > > >
> > > > Please refer to the patch changelogs for details.
> > >
> > > Here's a v2 of this patch series which is being sent, because I realized that
> > > one of the checks in pci_power_up() added by patch [4/7] in v1 was redundant
> > > and can be dropped, but that affected the last 3 patches in the series and
> > > then I noticed that more improvements were possible and hence the new patches
> > > [2/9].
> >
> > Here's a v3 of this series with some minor review comments addressed and R-by
> > tags from Mika added.
>
> Applied to pci/pm for v5.19, thanks, Rafael!
Thank you!