From: Brian Norris <briannorris@xxxxxxxxxx>
We have asymmetry w.r.t. pwrctrl device creation and destruction.
pwrctrl devices are created by the host bridge, as part of scanning for
child devices, but they are destroyed by the child device. This causes
confusing behavior in cases like the following:
1. PCI device is removed (e.g., via /sys/bus/pci/devices/*/remove);
pwrctrl device is also destroyed
2. pwrctrl driver is removed (e.g., rmmod)
3. pwrctrl driver is reloaded
One could expect #3 to re-bind to the pwrctrl device and re-power the
device; but there's no device to bind to, so it remains off. Instead, we
require a forced rescan (/sys/bus/pci/devices/*/rescan) to recreate the
pwrctrl device(s) and restore power.
This asymmetry isn't required though; it makes more logical sense to
retain the pwrctrl devices even without the PCI device, since pwrctrl is
more of a logical ancestor than a child.
Additionally, Documentation/PCI/sysfs-pci.rst documents the behavior of
step #1 (remove):
The 'remove' file is used to remove the PCI device, by writing a
non-zero integer to the file. This does not involve any kind of
hot-plug functionality, e.g. powering off the device.
Instead, let's destroy a pwrctrl device only when its parent (the host
bridge) is destroyed.
We use of_platform_device_destroy(), since it's the logical inverse of
pwrctrl creation (of_platform_device_create()). It performs more or less
the same things pci_pwrctrl_unregister() did, with some added bonus of
ensuring these are OF_POPULATED devices.
Signed-off-by: Brian Norris <briannorris@xxxxxxxxxx>
Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
---
drivers/pci/probe.c | 4 ++++
drivers/pci/remove.c | 18 ------------------
2 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4b8693ec9e4c..ad6e7d05b9bc 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -5,6 +5,7 @@
#include <linux/kernel.h>
#include <linux/delay.h>
+#include <linux/device.h>
#include <linux/init.h>
#include <linux/pci.h>
#include <linux/msi.h>
@@ -627,6 +628,9 @@ static void pci_release_host_bridge_dev(struct device *dev)
{
struct pci_host_bridge *bridge = to_pci_host_bridge(dev);
+ /* Clean up any pwrctrl children. */
+ device_for_each_child(dev, NULL, of_platform_device_destroy);
+
if (bridge->release_fn)
bridge->release_fn(bridge);
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 445afdfa6498..58dbb41c4730 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -17,23 +17,6 @@ static void pci_free_resources(struct pci_dev *dev)
}
}
-static void pci_pwrctrl_unregister(struct device *dev)
-{
- struct device_node *np;
- struct platform_device *pdev;
-
- np = dev_of_node(dev);
- if (!np)
- return;
-
- pdev = of_find_device_by_node(np);
- if (!pdev)
- return;
-
- of_device_unregister(pdev);
- of_node_clear_flag(np, OF_POPULATED);
-}
-
static void pci_stop_dev(struct pci_dev *dev)
{
pci_pme_active(dev, false);
@@ -64,7 +47,6 @@ static void pci_destroy_dev(struct pci_dev *dev)
pci_doe_destroy(dev);
pcie_aspm_exit_link_state(dev);
pci_bridge_d3_update(dev);
- pci_pwrctrl_unregister(&dev->dev);
pci_free_resources(dev);
put_device(&dev->dev);
}
--
2.50.0.727.gbf7dc18ff4-goog
