Re: [BUGFIX 3/4] PCI/PM: Fix config reg access for D3cold and bridge suspending

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Friday, August 03, 2012, Alan Stern wrote:
> On Fri, 3 Aug 2012, Huang Ying wrote:
> 
> > This patch fixes the following bug:
> > 
> > http://marc.info/?l=linux-pci&m=134338059022620&w=2
> > 
> > Where lspci does not work properly if a device and the corresponding
> > parent bridge (such as PCIe port) is suspended.  This is because the
> > device configuration space registers will be not accessible if the
> > corresponding parent bridge is suspended or the device is put into
> > D3cold state.
> > 
> > To solve the issue, the bridge/PCIe port connected to the device is
> > put into active state before read/write configuration space registers.
> > If the device is in D3cold state, it will be put into active state
> > too.
> > 
> > To avoid resume/suspend PCIe port for each configuration register
> > read/write, a small delay is added before the PCIe port to go
> > suspended.
> 
> 
> > +static void
> > +pci_config_pm_runtime_put(struct pci_dev *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device *parent = dev->parent;
> > +
> > +	pm_runtime_put(dev);
> > +	if (parent)
> > +		pm_runtime_put(parent);
> > +}
> 
> This is just the sort of thing Rafael and I have been talking about.  
> Why do an asynchronous put, going to all the trouble of using the 
> workqueue, if the idle routine is just going to call 
> pm_schedule_suspend()?

If that's PCI, it will call pm_runtime_suspend().  That probably _should_ be
pm_schedule_suspend(), but it isn't at the moment.

> Why not call pm_runtime_put_sync() instead?

I guess because the caller doesn't care whether or not the devices will be
suspended immediately and we seem to have agreed already that the added
workqueue overhead is minimal.

If the _idle() routine were to call pm_schedule_suspend(), though, I'd
agree that the overhead would be absolutely unnecessary.

Thanks,
Rafael