From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:56122 "EHLO e06smtp17.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753813Ab3HVVWD (ORCPT ); Thu, 22 Aug 2013 17:22:03 -0400 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 22 Aug 2013 22:17:01 +0100 Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 499A217D8057 for ; Thu, 22 Aug 2013 22:23:53 +0100 (BST) Received: from d06av05.portsmouth.uk.ibm.com (d06av05.portsmouth.uk.ibm.com [9.149.37.229]) by b06cxnps4074.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r7MLLjhS47906956 for ; Thu, 22 Aug 2013 21:21:45 GMT Received: from d06av05.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r7MLLu7s001728 for ; Thu, 22 Aug 2013 15:21:56 -0600 Date: Thu, 22 Aug 2013 23:21:54 +0200 (CEST) From: Sebastian Ott To: Bjorn Helgaas cc: "Rafael J. Wysocki" , "linux-pci@vger.kernel.org" Subject: Re: [PATCH] PCI: add hibernation hooks In-Reply-To: <20130822202206.GA1200@google.com> Message-ID: References: <3503956.Yk8myngYqt@vostro.rjw.lan> <3498248.Me8u1JmYkr@vostro.rjw.lan> <20130822202206.GA1200@google.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-pci-owner@vger.kernel.org List-ID: On Thu, 22 Aug 2013, Bjorn Helgaas wrote: > On Thu, Aug 22, 2013 at 12:06:43AM +0200, Rafael J. Wysocki wrote: > > -> the prevailing convention for representing platform options like that > > seems to be to use a pointer to an object containing callback pointers. This > > is what we do for pci_platform_pm and for suspend_ops, hibernation_ops etc. > > I think it is just more flexible, because it allows the pointer to be left > > unset if there is an initialization error of some sort or a command line > > option disabling something is set etc. > > Good points. I put merged the error checking into the original patch > and put it in my pci/misc branch. Here's the result; let me know if > anything needs to be tweaked. Great, thank you! Sebastian > > > PCI: Add pcibios_pm_ops for optional arch-specific hibernate functionality > > From: Sebastian Ott > > Platforms may want to provide architecture-specific functionality when > a PCI device is doing a hibernate transition. Add a weak symbol > pcibios_pm_ops that architectures can override to do so. > > [bhelgaas: fold in return value checks from v2 patch] > Signed-off-by: Sebastian Ott > Signed-off-by: Bjorn Helgaas > --- > drivers/pci/pci-driver.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > include/linux/pci.h | 4 ++++ > 2 files changed, 47 insertions(+) > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index e6515e2..98f7b9b 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -763,6 +763,13 @@ static int pci_pm_resume(struct device *dev) > > #ifdef CONFIG_HIBERNATE_CALLBACKS > > + > +/* > + * pcibios_pm_ops - provide arch-specific hooks when a PCI device is doing > + * a hibernate transition > + */ > +struct dev_pm_ops __weak pcibios_pm_ops; > + > static int pci_pm_freeze(struct device *dev) > { > struct pci_dev *pci_dev = to_pci_dev(dev); > @@ -786,6 +793,9 @@ static int pci_pm_freeze(struct device *dev) > return error; > } > > + if (pcibios_pm_ops.freeze) > + return pcibios_pm_ops.freeze(dev); > + > return 0; > } > > @@ -811,6 +821,9 @@ static int pci_pm_freeze_noirq(struct device *dev) > > pci_pm_set_unknown_state(pci_dev); > > + if (pcibios_pm_ops.freeze_noirq) > + return pcibios_pm_ops.freeze_noirq(dev); > + > return 0; > } > > @@ -820,6 +833,12 @@ static int pci_pm_thaw_noirq(struct device *dev) > struct device_driver *drv = dev->driver; > int error = 0; > > + if (pcibios_pm_ops.thaw_noirq) { > + error = pcibios_pm_ops.thaw_noirq(dev); > + if (error) > + return error; > + } > + > if (pci_has_legacy_pm_support(pci_dev)) > return pci_legacy_resume_early(dev); > > @@ -837,6 +856,12 @@ static int pci_pm_thaw(struct device *dev) > const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > int error = 0; > > + if (pcibios_pm_ops.thaw) { > + error = pcibios_pm_ops.thaw(dev); > + if (error) > + return error; > + } > + > if (pci_has_legacy_pm_support(pci_dev)) > return pci_legacy_resume(dev); > > @@ -878,6 +903,9 @@ static int pci_pm_poweroff(struct device *dev) > Fixup: > pci_fixup_device(pci_fixup_suspend, pci_dev); > > + if (pcibios_pm_ops.poweroff) > + return pcibios_pm_ops.poweroff(dev); > + > return 0; > } > > @@ -911,6 +939,9 @@ static int pci_pm_poweroff_noirq(struct device *dev) > if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI) > pci_write_config_word(pci_dev, PCI_COMMAND, 0); > > + if (pcibios_pm_ops.poweroff_noirq) > + return pcibios_pm_ops.poweroff_noirq(dev); > + > return 0; > } > > @@ -920,6 +951,12 @@ static int pci_pm_restore_noirq(struct device *dev) > struct device_driver *drv = dev->driver; > int error = 0; > > + if (pcibios_pm_ops.restore_noirq) { > + error = pcibios_pm_ops.restore_noirq(dev); > + if (error) > + return error; > + } > + > pci_pm_default_resume_early(pci_dev); > > if (pci_has_legacy_pm_support(pci_dev)) > @@ -937,6 +974,12 @@ static int pci_pm_restore(struct device *dev) > const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > int error = 0; > > + if (pcibios_pm_ops.restore) { > + error = pcibios_pm_ops.restore(dev); > + if (error) > + return error; > + } > + > /* > * This is necessary for the hibernation error path in which restore is > * called without restoring the standard config registers of the device. > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 0fd1f15..89ed123 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1648,6 +1648,10 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, > int pcibios_add_device(struct pci_dev *dev); > void pcibios_release_device(struct pci_dev *dev); > > +#ifdef CONFIG_HIBERNATE_CALLBACKS > +extern struct dev_pm_ops pcibios_pm_ops; > +#endif > + > #ifdef CONFIG_PCI_MMCONFIG > void __init pci_mmcfg_early_init(void); > void __init pci_mmcfg_late_init(void); > >