Re: [PATCH] PCI: add hibernation hooks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 20, 2013 at 8:41 AM, Sebastian Ott
<sebott@xxxxxxxxxxxxxxxxxx> wrote:
> Hello Bjorn,
>
> I'm currently trying to implement hibernate support for pci on s390. To access
> the config space of a pci function on s390 the function has to be enabled first.
> So I need some kind of callback to achieve this before the pci core or the device
> driver tries to access the function after a resume from hibernate.
>
> Would you be OK with the following patch?

I think doing something like this is fine.

What would it look like if we provided empty weak definitions of the
eight functions you need to hook: pcibios_pm_freeze(),
pcibios_pm_freeze_noirq(), etc.?  Then the callers wouldn't have to do
a test before making the call.  Not sure which would be better, but it
might be more obvious that this is a build-time binding rather than a
dynamic binding.

Should we check return values from the arch code?

> ---
> PCI: add hibernation hooks
>
> 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.
>
> Signed-off-by: Sebastian Ott <sebott@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/pci/pci-driver.c |   30 ++++++++++++++++++++++++++++++
>  include/linux/pci.h      |    4 ++++
>  2 files changed, 34 insertions(+)
>
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -763,6 +763,13 @@ static int pci_pm_resume(struct device *
>
>  #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;

For example:

int __weak pcibios_pm_freeze(struct device *) { return 0; }

>  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 *
>                         return error;
>         }
>
> +       if (pcibios_pm_ops.freeze)
> +               pcibios_pm_ops.freeze(dev);

    return pcibios_pm_freeze(dev);

>         return 0;
>  }
>
> @@ -811,6 +821,9 @@ static int pci_pm_freeze_noirq(struct de
>
>         pci_pm_set_unknown_state(pci_dev);
>
> +       if (pcibios_pm_ops.freeze_noirq)
> +               pcibios_pm_ops.freeze_noirq(dev);
> +
>         return 0;
>  }
>
> @@ -820,6 +833,9 @@ static int pci_pm_thaw_noirq(struct devi
>         struct device_driver *drv = dev->driver;
>         int error = 0;
>
> +       if (pcibios_pm_ops.thaw_noirq)
> +               pcibios_pm_ops.thaw_noirq(dev);
> +
>         if (pci_has_legacy_pm_support(pci_dev))
>                 return pci_legacy_resume_early(dev);
>
> @@ -837,6 +853,9 @@ static int pci_pm_thaw(struct device *de
>         const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>         int error = 0;
>
> +       if (pcibios_pm_ops.thaw)
> +               pcibios_pm_ops.thaw(dev);
> +
>         if (pci_has_legacy_pm_support(pci_dev))
>                 return pci_legacy_resume(dev);
>
> @@ -878,6 +897,9 @@ static int pci_pm_poweroff(struct device
>   Fixup:
>         pci_fixup_device(pci_fixup_suspend, pci_dev);
>
> +       if (pcibios_pm_ops.poweroff)
> +               pcibios_pm_ops.poweroff(dev);
> +
>         return 0;
>  }
>
> @@ -911,6 +933,9 @@ static int pci_pm_poweroff_noirq(struct
>         if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI)
>                 pci_write_config_word(pci_dev, PCI_COMMAND, 0);
>
> +       if (pcibios_pm_ops.poweroff_noirq)
> +               pcibios_pm_ops.poweroff_noirq(dev);
> +
>         return 0;
>  }
>
> @@ -920,6 +945,9 @@ static int pci_pm_restore_noirq(struct d
>         struct device_driver *drv = dev->driver;
>         int error = 0;
>
> +       if (pcibios_pm_ops.restore_noirq)
> +               pcibios_pm_ops.restore_noirq(dev);
> +
>         pci_pm_default_resume_early(pci_dev);
>
>         if (pci_has_legacy_pm_support(pci_dev))
> @@ -937,6 +965,8 @@ static int pci_pm_restore(struct device
>         const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>         int error = 0;
>
> +       if (pcibios_pm_ops.restore)
> +               pcibios_pm_ops.restore(dev);
>         /*
>          * This is necessary for the hibernation error path in which restore is
>          * called without restoring the standard config registers of the device.
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1648,6 +1648,10 @@ int pcibios_set_pcie_reset_state(struct
>  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);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux