Re: [PATCH 6.13] PCI/pwrctrl: Skip NULL of_node when unregistering

[Lukas Wunner <lukas@wunner.de>]

On Sun, Dec 01, 2024 at 01:51:08PM +0530, Manivannan Sadhasivam wrote:
> The idea of pci_pwrctrl_unregister() is to remove the pwrctl device when the
> associated PCI device gets removed. When this happens, the pwrctl driver will
> turn off the power to the corresponding PCI device

After pci_pwrctrl_unregister() is called from pci_stop_dev(),
the device may be accessed by one of the calls in pci_destroy_dev().

E.g. pci_doe_destroy() may set the DOE Abort bit in the DOE Control
Register if a DOE exchange is ongoing.  One cannot assume that no
such exchange is ongoing merely because the device was unbound from
its driver.  The PCI core may have legitimate reasons to perform a DOE
exchange or generally access config space even after the device has been
unbound.  And IIUC, those accesses will fail if pwrctrl has powered the
device down.

Another example is pcie_aspm_exit_link_state(), which will adjust ASPM
settings on device removal.

So it seems to me that the call to pci_pwrctrl_unregister() needs to be
moved to pci_doe_destroy().  However I'm worried that will break the
symmetry with pci_pwrctrl_create_devices(), which is only called in the
!pci_dev_is_added() case.

Thanks,

Lukas