diff mbox series

[v3] PCI: Make sure the bus bridge powered on when scanning bus

Message ID 1616147193-13186-1-git-send-email-yangyicong@hisilicon.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show
Series [v3] PCI: Make sure the bus bridge powered on when scanning bus | expand

Commit Message

Yicong Yang March 19, 2021, 9:46 a.m. UTC
When the bus bridge is runtime suspended, we'll fail to rescan
the devices through sysfs as we cannot access the configuration
space correctly when the bridge is in D3hot.
It can be reproduced like:

$ echo 1 > /sys/bus/pci/devices/0000:80:00.0/0000:81:00.1/remove
$ echo 1 > /sys/bus/pci/devices/0000:80:00.0/pci_bus/0000:81/rescan

0000:80:00.0 is root port and is runtime suspended and we cannot
get 0000:81:00.1 after rescan.

Make bridge powered on when scanning the child bus, by adding
pm_runtime_get_sync()/pm_runtime_put() in pci_scan_child_bus_extend().

A similar issue is met and solved by
d963f6512e15 ("PCI: Power on bridges before scanning new devices")
which rescan the devices through /sys/bus/pci/devices/0000:80:00.0/rescan.
The callstack is like:

dev_rescan_restore()
  pci_rescan_bus()
    pci_scan_bridge_extend()
      pci_scan_child_bus_extend() /* will wake up the bridge with this patch */

With this patch the issue is also resolved, so let's remove the calls of
pm_runtime_*() in pci_scan_bridge_extend().

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
Change since v2:
- rebase on v5.12-rc3
Link: https://lore.kernel.org/linux-pci/1601029386-4928-1-git-send-email-yangyicong@hisilicon.com/

Change since v1:
address the comments from Bjorn:
- use an intermediate variable *bridge as suggested
- remove the pm_runtime_*() calls in pci_scan_bridge_extend()
Link: https://lore.kernel.org/linux-pci/1596022223-4765-1-git-send-email-yangyicong@hisilicon.com/

 drivers/pci/probe.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
diff mbox series

Patch

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 953f15a..6ad3c48 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1224,12 +1224,6 @@  static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
 	u8 fixed_sec, fixed_sub;
 	int next_busnr;
 
-	/*
-	 * Make sure the bridge is powered on to be able to access config
-	 * space of devices below it.
-	 */
-	pm_runtime_get_sync(&dev->dev);
-
 	pci_read_config_dword(dev, PCI_PRIMARY_BUS, &buses);
 	primary = buses & 0xFF;
 	secondary = (buses >> 8) & 0xFF;
@@ -1431,8 +1425,6 @@  static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
 out:
 	pci_write_config_word(dev, PCI_BRIDGE_CONTROL, bctl);
 
-	pm_runtime_put(&dev->dev);
-
 	return max;
 }
 
@@ -2797,11 +2789,19 @@  static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
 	unsigned int used_buses, normal_bridges = 0, hotplug_bridges = 0;
 	unsigned int start = bus->busn_res.start;
 	unsigned int devfn, fn, cmax, max = start;
-	struct pci_dev *dev;
+	struct pci_dev *dev, *bridge = bus->self;
 	int nr_devs;
 
 	dev_dbg(&bus->dev, "scanning bus\n");
 
+	/*
+	 * Make sure the bus bridge is powered on, otherwise we may not be
+	 * able to scan the devices as we may fail to access the configuration
+	 * space of subordinates.
+	 */
+	if (bridge)
+		pm_runtime_get_sync(&bridge->dev);
+
 	/* Go find them, Rover! */
 	for (devfn = 0; devfn < 256; devfn += 8) {
 		nr_devs = pci_scan_slot(bus, devfn);
@@ -2914,6 +2914,9 @@  static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
 		}
 	}
 
+	if (bridge)
+		pm_runtime_put(&bridge->dev);
+
 	/*
 	 * We've scanned the bus and so we know all about what's on
 	 * the other side of any bridges that may be on this bus plus