Re: [PATCH v4] PCI/PM: Prevent runtime suspend before devices are fully initialized

Rafael J. Wysocki
2026-01-18T11:53:35Z

On Sat, Jan 17, 2026 at 2:19 AM Brian Norris <briannorris@chromium.org> wrote:
>
> Hi Marek,
>
> On Thu, Jan 15, 2026 at 12:14:49PM +0100, Marek Szyprowski wrote:
> > On 14.01.2026 21:10, Brian Norris wrote:
> > > On Wed, Jan 14, 2026 at 10:46:41AM +0100, Marek Szyprowski wrote:
> > >> On 06.01.2026 23:27, Bjorn Helgaas wrote:
> > >>> On Thu, Oct 23, 2025 at 02:09:01PM -0700, Brian Norris wrote:
> > >>>> Today, it's possible for a PCI device to be created and
> > >>>> runtime-suspended before it is fully initialized. When that happens, the
> > >>>> device will remain in D0, but the suspend process may save an
> > >>>> intermediate version of that device's state -- for example, without
> > >>>> appropriate BAR configuration. When the device later resumes, we'll
> > >>>> restore invalid PCI state and the device may not function.
> > >>>>
> > >>>> Prevent runtime suspend for PCI devices by deferring pm_runtime_enable()
> > >>>> until we've fully initialized the device.
> > > ...
> > >> This patch landed recently in linux-next as commit c796513dc54e
> > >> ("PCI/PM: Prevent runtime suspend until devices are fully initialized").
> > >> In my tests I found that it sometimes causes the "pci 0000:01:00.0:
> > >> runtime PM trying to activate child device 0000:01:00.0 but parent
> > >> (0000:00:00.0) is not active" warning on Qualcomm Robotics RB5 board
> > >> (arch/arm64/boot/dts/qcom/qrb5165-rb5.dts). This in turn causes a
> > >> lockdep warning about console lock, but this is just a consequence of
> > >> the runtime pm warning. Reverting $subject patch on top of current
> > >> linux-next hides this warning.
> > >>
> > >> Here is a kernel log:
> > >>
> > >> pci 0000:01:00.0: [17cb:1101] type 00 class 0xff0000 PCIe Endpoint
> > >> pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x000fffff 64bit]
> > >> pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
> > >> pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth, limited by 5.0
> > >> GT/s PCIe x1 link at 0000:00:00.0 (capable of 7.876 Gb/s with 8.0 GT/s
> > >> PCIe x1 link)
> > >> pci 0000:01:00.0: Adding to iommu group 13
> > >> pci 0000:01:00.0: ASPM: default states L0s L1
> > >> pcieport 0000:00:00.0: bridge window [mem 0x60400000-0x604fffff]: assigned
> > >> pci 0000:01:00.0: BAR 0 [mem 0x60400000-0x604fffff 64bit]: assigned
> > >> pci 0000:01:00.0: runtime PM trying to activate child device
> > >> 0000:01:00.0 but parent (0000:00:00.0) is not active
> > > Thanks for the report. I'll try to look at reproducing this, or at least
> > > getting a better mental model of exactly why this might fail (or,
> > > "warn") this way. But if you have the time and desire to try things out
> > > for me, can you give v1 a try?
> > >
> > > https://lore.kernel.org/all/20251016155335.1.I60a53c170a8596661883bd2b4ef475155c7aa72b@changeid/
> > >
> > > I'm pretty sure it would not invoke the same problem.
> >
> > Right, this one works fine.
> >
> > > I also suspect v3
> > > might not, but I'm less sure:
> > >
> > > https://lore.kernel.org/all/20251022141434.v3.1.I60a53c170a8596661883bd2b4ef475155c7aa72b@changeid/
> > This one too, at least I was not able to reproduce any fail.
>
> Thanks for testing. I'm still not sure exactly how to reproduce your
> failure, but it seems as if the root port is being allowed to suspend
> before the endpoint is added to the system, and it remains so while the
> endpoint is about to probe. device_initial_probe() will be OK with
> respect to PM, since it will wake up the port if needed. But this
> particular code is not OK, since it doesn't ensure the parent device is
> active while preparing the endpoint power state.
>
> I suppose one way to "solve" that is (untested):
>
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -380,8 +380,12 @@ void pci_bus_add_device(struct pci_dev *dev)
> put_device(&pdev->dev);
> }
>
> + if (dev->dev.parent)
> + pm_runtime_get_sync(dev->dev.parent);
> pm_runtime_set_active(&dev->dev);
> pm_runtime_enable(&dev->dev);
> + if (dev->dev.parent)
> + pm_runtime_put(dev->dev.parent);
>
> if (!dn || of_device_is_available(dn))
> pci_dev_allow_binding(dev);
>
> Personally, I'm more inclined to go back to v1, since it prepares the
> runtime PM status when the device is first discovered. That way, its
> ancestors are still active, avoiding these sorts of problems. I'm
> frankly not sure of all the reasons Rafael recommended I make the
> v1->v3->v4 changes, and now that they cause problems, I'm inclined to
> question them again.
>
> Rafael, do you have any thoughts?

Yeah.

Move back pm_runtime_set_active(&dev->dev) back to pm_runtime_init()
because that would prevent the parent from suspending and keep
pm_runtime_enable() here because that would prevent the device itself
from suspending between pm_runtime_init() and this place.

And I would add comments in both places.