| Message ID | 20220707172205.3205110-2-rajvi.jingar@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] PCI/PM: refactor pci_pm_suspend_noirq() | expand |
On Thu, Jul 7, 2022 at 7:22 PM Rajvi Jingar <rajvi.jingar@linux.intel.com> wrote: > > Set ptm_enabled from 'struct pci_dev' to 0 in pci_ptm_disable() to I would set "Clear" here instead of "Set", but moreover -> > save the disabled PTM state for the device. > > Fixes: a697f072f5da ("PCI: Disable PTM during suspend to save power") > Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com> > --- > drivers/pci/pcie/ptm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c > index 368a254e3124..746e29779c27 100644 > --- a/drivers/pci/pcie/ptm.c > +++ b/drivers/pci/pcie/ptm.c > @@ -44,6 +44,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; -> I don't think that this is going to work without setting dev->ptm_enabled in pci_restore_ptm_state() if the restored state turns out to be "enabled". > } > > void pci_save_ptm_state(struct pci_dev *dev) > --
On Thu, Jul 7, 2022 at 8:27 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Thu, Jul 7, 2022 at 7:22 PM Rajvi Jingar > <rajvi.jingar@linux.intel.com> wrote: > > > > Set ptm_enabled from 'struct pci_dev' to 0 in pci_ptm_disable() to > > I would set "Clear" here instead of "Set", but moreover -> s/set/say/ sorry. > > save the disabled PTM state for the device. > > > > Fixes: a697f072f5da ("PCI: Disable PTM during suspend to save power") > > Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com> > > --- > > drivers/pci/pcie/ptm.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c > > index 368a254e3124..746e29779c27 100644 > > --- a/drivers/pci/pcie/ptm.c > > +++ b/drivers/pci/pcie/ptm.c > > @@ -44,6 +44,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; > > -> I don't think that this is going to work without setting > dev->ptm_enabled in pci_restore_ptm_state() if the restored state > turns out to be "enabled". And arguably, this function doesn't need to do anything if dev->ptm_enabled is 0 already and this is the only check it needs to do, because dev->ptm_enabled will only be set for PCIe devices that have PTM. > > } > > > > void pci_save_ptm_state(struct pci_dev *dev) > > --
diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c index 368a254e3124..746e29779c27 100644 --- a/drivers/pci/pcie/ptm.c +++ b/drivers/pci/pcie/ptm.c @@ -44,6 +44,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)
Set ptm_enabled from 'struct pci_dev' to 0 in pci_ptm_disable() to save the disabled PTM state for the device. Fixes: a697f072f5da ("PCI: Disable PTM during suspend to save power") Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com> --- drivers/pci/pcie/ptm.c | 1 + 1 file changed, 1 insertion(+)