Re: [PATCH 1/2] PCI: Ensure error recoverability at all times

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Nov 21, 2025 at 06:40:23PM +0100, Lukas Wunner wrote:
> On Thu, Nov 13, 2025 at 10:15:56AM -0600, Bjorn Helgaas wrote:
> > On Thu, Nov 13, 2025 at 10:38:09AM +0100, Lukas Wunner wrote:
> > > On Wed, Nov 12, 2025 at 04:38:31PM -0600, Bjorn Helgaas wrote:
> > > > On Sun, Oct 12, 2025 at 03:25:01PM +0200, Lukas Wunner wrote:
> > > > It would be nice if there were a few more
> > > > words about pci_save_state() and pci_restore_state() in
> > > > Documentation/.
> > > > 
> > > > pci_save_state() isn't mentioned at all in Documentation/PCI
> > > 
> > > Right, it's documented in the Documentation/power directory. :)
> > 
> > Yes, in the pci.rst I mentioned, but it mostly uses the "saves the
> > device's standard configuration registers" wording.
> > 
> > I'm just wishing for a more concrete mention of "pci_save_state()",
> > since that's where the critical "state_saved" flag is updated.
> 
> Hm, Documentation/power/pci.rst does contain this:
> 
>    "Then, pci_save_state(), pci_prepare_to_sleep(), and
>     pci_set_power_state() should be used to save the device’s
>     standard configuration registers, to prepare it for system wakeup
>     (if necessary), and to put it into a low-power state, respectively."
> 
> I'm struggling to find a better way to phrase it.

The part that seems confusing to me is that pci_save_state() is the
switch that turns off PCI core power management.  The state save part
is not obviously connected with the power management part, at least
from the function name.

But that paragraph goes on to say:

  Moreover, if the driver calls pci_save_state(), the PCI subsystem
  will not execute either pci_prepare_to_sleep(), or
  pci_set_power_state() for its device, so the driver is then
  responsible for handling the device as appropriate.

so the doc actually *does* mention the connection, and although
pci_prepare_to_sleep() and pci_set_power_state() sound sort of like
internal functions, I guess that makes sense because drivers doing
their own power management would be using things like that.

I do raise my eyebrows a bit at them though; there are only seven
drivers that use pci_prepare_to_sleep(), which makes me a little
suspicious -- what is so unique about those drivers?

A bunch of drivers use pci_set_power_state().  Many seem to be rolling
their own PM resets.  Several others use it in suspend/resume for
reasons that aren't obvious to me but are likely legit.

> > And I'm not sure Documentation/ includes anything about the idea of
> > a driver using pci_save_state() to capture the state it wants to
> > restore after an error.
> 
> Right, while pci_save_state() usage is mentioned in the PCI power
> management documentation, it's not mentioned at all in the error
> recovery context.  So I'm proposing this amendment:
> 
> https://lore.kernel.org/r/077596ba70202be0e43fdad3bb9b93d356cbe4ec.1763746079.git.lukas@wunner.de/

Thanks!

Bjorn