Re: [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper

[Rafael J. Wysocki (Intel)]

On Mon, Aug 31, 2026 at 10:56 PM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
>
> From: "Mario Limonciello (AMD)" <superm1@xxxxxxxxxx>
>
> In order to unify suspend and hibernate codepaths without code duplication
> the common code should be in common helpers.  Move it from
> pci_pm_suspend_noirq() into a helper.  No intended functional changes.
>
> Tested-by: Eric Naim <dnaim@xxxxxxxxxxx>
> Signed-off-by: Mario Limonciello (AMD) <superm1@xxxxxxxxxx>
> ---
>  drivers/pci/pci-driver.c | 75 +++++++++++++++++++++++++---------------
>  1 file changed, 47 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index e16aa59dd7ac8..e3d352c7b31d4 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -818,6 +818,50 @@ static void pci_pm_complete(struct device *dev)
>
>  #endif /* !CONFIG_PM_SLEEP */
>
> +#if defined(CONFIG_SUSPEND)
> +/**
> + * pci_pm_suspend_noirq_common - prepare a device to enter a low-power state
> + * @pci_dev: pci device
> + * @skip_bus_pm: pointer to a boolean indicating whether to skip bus PM
> + *
> + * Save the device state and decide whether bus-level power management should
> + * be skipped, reporting the decision back to the caller via @skip_bus_pm.
> + */
> +static void pci_pm_suspend_noirq_common(struct pci_dev *pci_dev, bool *skip_bus_pm)

Change it to bool.

> +{
> +       if (!pci_dev->state_saved) {
> +               pci_save_state(pci_dev);
> +
> +               /*
> +                * If the device is a bridge with a child in D0 below it,
> +                * it needs to stay in D0, so check skip_bus_pm to avoid
> +                * putting it into a low-power state in that case.
> +                */
> +               if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
> +                       pci_prepare_to_sleep(pci_dev);
> +       }
> +
> +       pci_dbg(pci_dev, "PCI PM: Sleep power state: %s\n",
> +               pci_power_name(pci_dev->current_state));
> +
> +       if (pci_dev->current_state == PCI_D0) {
> +               pci_dev->skip_bus_pm = true;
> +               /*
> +                * Per PCI PM r1.2, table 6-1, a bridge must be in D0 if any
> +                * downstream device is in D0, so avoid changing the power state
> +                * of the parent bridge by setting the skip_bus_pm flag for it.
> +                */
> +               if (pci_dev->bus->self)
> +                       pci_dev->bus->self->skip_bus_pm = true;
> +       }
> +
> +       if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
> +               pci_dbg(pci_dev, "PCI PM: Skipped\n");
> +               *skip_bus_pm = true;

And return true here.

> +       }

And false here.

Then you won't need the second argument.

> +}
> +#endif /* CONFIG_SUSPEND */
> +
>  #ifdef CONFIG_SUSPEND
>  static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
>  {
> @@ -907,6 +951,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
>  {
>         struct pci_dev *pci_dev = to_pci_dev(dev);
>         const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +       bool skip_bus_pm = false;
>
>         if (dev_pm_skip_suspend(dev))
>                 return 0;
> @@ -937,36 +982,10 @@ static int pci_pm_suspend_noirq(struct device *dev)
>                 }
>         }
>
> -       if (!pci_dev->state_saved) {
> -               pci_save_state(pci_dev);
> -
> -               /*
> -                * If the device is a bridge with a child in D0 below it,
> -                * it needs to stay in D0, so check skip_bus_pm to avoid
> -                * putting it into a low-power state in that case.
> -                */
> -               if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
> -                       pci_prepare_to_sleep(pci_dev);
> -       }
> -
> -       pci_dbg(pci_dev, "PCI PM: Suspend power state: %s\n",
> -               pci_power_name(pci_dev->current_state));
> +       pci_pm_suspend_noirq_common(pci_dev, &skip_bus_pm);
>
> -       if (pci_dev->current_state == PCI_D0) {
> -               pci_dev->skip_bus_pm = true;
> -               /*
> -                * Per PCI PM r1.2, table 6-1, a bridge must be in D0 if any
> -                * downstream device is in D0, so avoid changing the power state
> -                * of the parent bridge by setting the skip_bus_pm flag for it.
> -                */
> -               if (pci_dev->bus->self)
> -                       pci_dev->bus->self->skip_bus_pm = true;
> -       }
> -
> -       if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
> -               pci_dbg(pci_dev, "PCI PM: Skipped\n");
> +       if (skip_bus_pm)
>                 goto Fixup;
> -       }
>
>  set_unknown:
>         pci_pm_set_unknown_state(pci_dev);
> --
> 2.43.0
>