On Saturday, August 04, 2012, Rafael J. Wysocki wrote: > 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.
Sorry, I should have had a closer look at pcie_port_runtime_idle() before replying. You're right, pm_runtime_put_sync() should be used for the parent. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/