On Mon, Nov 08, 2021 at 08:56:19PM -0600, Bjorn Helgaas wrote:

[+cc Greg: new device_is_bound() use]

On Mon, Nov 08, 2021 at 10:22:26PM +0100, Uwe Kleine-König wrote:

quoted

pci_pm_runtime_resume() exits early when the device to resume isn't
bound yet:

	if (!to_pci_driver(dev->driver))
		return 0;

This however isn't true when the device currently probes and
local_pci_probe() calls pm_runtime_get_sync() because then the driver
core already setup dev->driver. As a result the driver's resume callback
is called before the driver's probe function is called and so more often
than not required driver data isn't setup yet.

So replace the check for the device being unbound by a check that only
becomes true after .probe() succeeded.

I like the fact that this patch is short and simple.

But there are 30+ users of to_pci_driver().  This patch asserts that
*one* of them, pci_pm_runtime_resume(), is special and needs to test
device_is_bound() instead of using to_pci_driver().

It's special because the current PM implementation calls it via
pm_runtime_get_sync() before the driver's .probe() method.  That
connection is a little bit obscure and fragile.  What if the PM
implementation changes?

Maybe we just need a comment there about why it looks different than
the other PM interfaces?

I also notice that the only other uses of device_is_bound()
outside the driver core are in iommu_group_store_type() and
regulator_resolve_supply().  This patch seems like a reasonable use,
but I always look twice when we do something unique.

I agree that this looks really odd.  No one should care outside of the
driver core to call device_is_bound(), as if a driver is being called,
implicitly you know that the device is bound to that driver.

Why does the PCI core care if a device is bound to a pci driver at this
point in time?

But, this does feel like an odd use of to_pci_driver() here, what needs
to be known here, if a pci driver is in control of a device here or not?

thanks,

greg k-h