Re: [PATCH] PCI: Drop unnecessary retries when restoring BARs

On Tue, May 05, 2026 at 12:43:17PM +0200, Rafael J. Wysocki wrote:
> On Mon, May 4, 2026 at 11:17 PM Bjorn Helgaas <[email protected]> wrote:
> ...

> > If pci_power_up() is doing D3cold -> D0, main power is initially
> > off, so platform_pci_set_power_state(PCI_D0) would turn on main
> > power, which is a Fundamental Reset leaving the device in
> > D0uninitialized.  In general the device needs at least 100 ms
> > after that reset before any access, e.g., before the PMCSR read.
> 
> Yes, it does in general, but that should be covered by
> platform_pci_set_power_state() because on some platforms the delay
> can be shorter.

It's surprising to me that platform_pci_set_power_state() takes
care of that delay because there's so much PCIe infrastructure
(dependencies on link speed, RRS polling, Immediate Readiness,
Readiness Notifications, etc), much of which sounds more like
OS-level support than firmware support.

But if platform_pci_set_power_state() does guarantee that the device
is Configuration-Ready, we should document that somewhere.

> > If pci_power_up() is doing D3hot -> D0, the device already has
> > main power, so I suppose platform_pci_set_power_state(PCI_D0)
> > doesn't do anything.
> 
> It may or may not.  Some platforms supply AML to run during D3hot ->
> D0 transitions (and the ACPI spec allows that).

If the device started in D3hot, it never lost main power, and I
assumed platform_pci_set_power_state(PCI_D0) might just leave it in
D3hot.  It sounds like it takes it all the way to D0, although the
subsequent code that checks the PMCSR state does suggest that the
device might *not* be in D0:

    platform_pci_set_power_state(dev, PCI_D0);

    pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
    state = pmcsr & PCI_PM_CTRL_STATE_MASK;
    if (state == PCI_D0)
      goto end;

    pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, 0);
    if (state == PCI_D3hot)
      pci_dev_d3_sleep(dev);
    ...

  end:

If platform_pci_set_power_state(PCI_D0) puts the device in D0 and
takes care of all the delays, "state" should always be PCI_D0, and we
shouldn't need the D3hot and D2 delays here.

Bjorn