| Message ID | 20220714231609.3962051-1-rajvi.jingar@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] PCI/PM: refactor pci_pm_suspend_noirq() | expand |
On Fri, Jul 15, 2022 at 1:16 AM Rajvi Jingar <rajvi.jingar@linux.intel.com> wrote: > > pci_dev->ptm_enabled needs to be maintained to reflect the current PTM > state of the device. In pci_ptm_disable(), clear ptm_enabled from > 'struct pci_dev' on disabling PTM state for the device. > In pci_restore_ptm_state(), set dev->ptm_enabled based on the restored > PTM state of the device. > > Also, perform ptm_enabled check in pci_ptm_disable() to avoid config > space access in case if PTM is already disabled for the device. > > Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com> > --- > v1->v2: > - add ptm_enabled check in pci_ptm_disable(). > - set the dev->ptm_enabled value in pci_restore_ptm_state(). > --- > drivers/pci/pcie/ptm.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c > index 368a254e3124..8dc7d2285a0d 100644 > --- a/drivers/pci/pcie/ptm.c > +++ b/drivers/pci/pcie/ptm.c > @@ -34,6 +34,9 @@ void pci_disable_ptm(struct pci_dev *dev) > int ptm; > u16 ctrl; > > + if (!dev->ptm_enabled) > + return; > + > if (!pci_is_pcie(dev)) > return; After the check you've added this doesn't need to be checked any more, because ptm_enabled will not be set if pci_is_pcie(dev) is not true AFAICS. Otherwise the patch looks good to me, so with the above addressed, please feel free to add Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> to it. Thanks! > > @@ -44,6 +47,7 @@ void pci_disable_ptm(struct pci_dev *dev) > pci_read_config_word(dev, ptm + PCI_PTM_CTRL, &ctrl); > ctrl &= ~(PCI_PTM_CTRL_ENABLE | PCI_PTM_CTRL_ROOT); > pci_write_config_word(dev, ptm + PCI_PTM_CTRL, ctrl); > + dev->ptm_enabled = 0; > } > > void pci_save_ptm_state(struct pci_dev *dev) > @@ -83,6 +87,7 @@ void pci_restore_ptm_state(struct pci_dev *dev) > > cap = (u16 *)&save_state->cap.data[0]; > pci_write_config_word(dev, ptm + PCI_PTM_CTRL, *cap); > + dev->ptm_enabled = !!(*cap & PCI_PTM_CTRL_ENABLE); > } > > void pci_ptm_init(struct pci_dev *dev) > -- > 2.25.1 >
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 49238ddd39ee..1f64de3e5280 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -867,20 +867,14 @@ static int pci_pm_suspend_noirq(struct device *dev) } } - if (pci_dev->skip_bus_pm) { + if (!pci_dev->state_saved) { + pci_save_state(pci_dev); /* - * Either the device is a bridge with a child in D0 below it, or - * the function is running for the second time in a row without - * going through full resume, which is possible only during - * suspend-to-idle in a spurious wakeup case. The device should - * be in D0 at this point, but if it is a bridge, it may be - * necessary to save its state. + * If the device is a bridge with a child in D0 below it, it needs to + * stay in D0, so check skip_bus_pm to avoid putting it into a + * low-power state in that case. */ - if (!pci_dev->state_saved) - pci_save_state(pci_dev); - } else if (!pci_dev->state_saved) { - pci_save_state(pci_dev); - if (pci_power_manageable(pci_dev)) + if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev)) pci_prepare_to_sleep(pci_dev); }