Messages in this thread | | | From | Bjorn Helgaas <> | Date | Fri, 4 May 2012 13:50:30 -0600 | Subject | Re: [RFC v2 3/5] PCIe, Add runtime PM support to PCIe port |
| |
On Fri, May 4, 2012 at 2:13 AM, Huang Ying <ying.huang@intel.com> wrote: > From: Zheng Yan <zheng.z.yan@intel.com> > > This patch adds runtime PM support to PCIe port. This is needed by > PCIe D3cold support, where PCIe device in slot may be powered on/off > by PCIe port.
I assume this works for integrated PCIe devices as well as those that are plugged into a slot and can be physically removed -- maybe the text "in slot" is superfluous?
> Because runtime suspend is broken for some chipset, a white list is > used to enable runtime PM support for only chipset known to work.
A whitelist requires perpetual maintenance. Every time a new working chipset comes out, you have to update the whitelist. That doesn't seem right.
> Signed-off-by: Zheng Yan <zheng.z.yan@intel.com> > Signed-off-by: Huang Ying <ying.huang@intel.com> > --- > drivers/pci/pci.c | 9 +++++++++ > drivers/pci/pcie/portdrv_pci.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 49 insertions(+) > > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1476,6 +1476,15 @@ bool pci_check_pme_status(struct pci_dev > */ > static int pci_pme_wakeup(struct pci_dev *dev, void *pme_poll_reset) > { > + struct pci_dev *bridge = dev->bus->self; > + > + /* > + * If bridge is in low power state, the configuration space of > + * subordinate devices may be not accessible > + */
This comment would make more sense as "If the upstream bridge is in a low power state, the configuration space of this device is not accessible."
> + if (bridge && bridge->current_state != PCI_D0) > + return 0; > + > if (pme_poll_reset && dev->pme_poll) > dev->pme_poll = false; > > --- a/drivers/pci/pcie/portdrv_pci.c > +++ b/drivers/pci/pcie/portdrv_pci.c > @@ -11,6 +11,7 @@ > #include <linux/kernel.h> > #include <linux/errno.h> > #include <linux/pm.h> > +#include <linux/pm_runtime.h> > #include <linux/init.h> > #include <linux/pcieport_if.h> > #include <linux/aer.h> > @@ -99,6 +100,27 @@ static int pcie_port_resume_noirq(struct > return 0; > } > > +#ifdef CONFIG_PM_RUNTIME > +static int pcie_port_runtime_suspend(struct device *dev) > +{ > + struct pci_dev *pdev = to_pci_dev(dev); > + > + pci_save_state(pdev); > + return 0; > +} > + > +static int pcie_port_runtime_resume(struct device *dev) > +{ > + struct pci_dev *pdev = to_pci_dev(dev); > + > + pci_restore_state(pdev); > + return 0; > +} > +#else > +#define pcie_port_runtime_suspend NULL > +#define pcie_port_runtime_resume NULL > +#endif > + > static const struct dev_pm_ops pcie_portdrv_pm_ops = { > .suspend = pcie_port_device_suspend, > .resume = pcie_port_device_resume, > @@ -107,6 +129,8 @@ static const struct dev_pm_ops pcie_port > .poweroff = pcie_port_device_suspend, > .restore = pcie_port_device_resume, > .resume_noirq = pcie_port_resume_noirq, > + .runtime_suspend = pcie_port_runtime_suspend, > + .runtime_resume = pcie_port_runtime_resume, > }; > > #define PCIE_PORTDRV_PM_OPS (&pcie_portdrv_pm_ops) > @@ -117,6 +141,18 @@ static const struct dev_pm_ops pcie_port > #endif /* !PM */ > > /* > + * PCIe port runtime suspend is broken for some chipset, so use a > + * white list to disable runtime PM for these chipset.
If you're *disabling* runtime PM for these chipsets, I'd call this a blacklist (and a blacklist makes more sense to me from a maintenance standpoint, because you only have to update it when new broken chips are discovered).
> + */ > +static const struct pci_device_id port_runtime_pm_pci_ids[] = { > + { PCI_VDEVICE(INTEL, 0x1e10), }, > + { PCI_VDEVICE(INTEL, 0x1e16), }, > + { PCI_VDEVICE(INTEL, 0x1e18), }, > + { PCI_VDEVICE(INTEL, 0x1e1c), }, > + { /* end: all zeroes */ } > +}; > + > +/* > * pcie_portdrv_probe - Probe PCI-Express port devices > * @dev: PCI-Express port device being probed > * > @@ -144,12 +180,16 @@ static int __devinit pcie_portdrv_probe( > return status; > > pci_save_state(dev); > + if (pci_match_id(port_runtime_pm_pci_ids, dev)) > + pm_runtime_put_noidle(&dev->dev); > > return 0; > } > > static void pcie_portdrv_remove(struct pci_dev *dev) > { > + if (pci_match_id(port_runtime_pm_pci_ids, dev)) > + pm_runtime_get_noresume(&dev->dev); > pcie_port_device_remove(dev); > pci_disable_device(dev); > } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |