Re: [PATCH v4 1/2] PCI/PM: refactor pci_pm_suspend_noirq()

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Apr 14, 2022 at 07:53:25PM +0200, Rafael J. Wysocki wrote:
> On 3/25/2022 8:50 PM, Rajvi Jingar wrote:
> > The state of the device is saved during pci_pm_suspend_noirq(), if it
> > has not already been saved, regardless of the skip_bus_pm flag value. So
> > skip_bus_pm check is removed before saving the device state.
> > 
> > Signed-off-by: Rajvi Jingar <rajvi.jingar@intel.com>
> > Suggested-by: David E. Box <david.e.box@linux.intel.com>
> 
> Sorry for the delay here.
> 
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Rajvi, can you post these again?  It looks like they didn't make it to
the linux-pci mailing list, and I can't find them in the lore archives:

  https://lore.kernel.org/all/?q=f%3Arajvi.jingar

Maybe some formatting issue that vger didn't like?  Some possible
issues here:

  http://vger.kernel.org/majordomo-info.html

They should appear on the list so others can comment and so the lore
URL can be included in the commit when applying.

Please incorporate Rafael's reviewed-by when you repost.

> > ---
> >   v1 -> v2: add comments to the changes
> >   v2 -> v3: move changelog after "---" marker
> >   v3 -> v4: add "---" marker after changelog
> > ---
> >   drivers/pci/pci-driver.c | 18 ++++++------------
> >   1 file changed, 6 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index 4ceeb75fc899..8b55a90126a2 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -845,20 +845,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);
> >   	}
> 
>