diff mbox series

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

Message ID 20220422080404.27724-1-yangyicong@hisilicon.com (mailing list archive)
State Superseded
Headers show
Series [v4] PCI: Make sure the bus bridge powered on when scanning bus | expand

Commit Message

Yicong Yang April 22, 2022, 8:04 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().

Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
Change since v3:
- retain the pm_runtime_*() calls in pci_scan_bridge_extend() as Rafael points
  out that it's necessary when the brigde is in D3cold
Link: https://lore.kernel.org/linux-pci/20220414123736.34150-1-yangyicong@hisilicon.com/

Change since v2:
- just rebase it on v5.18-rc2
Link: https://lore.kernel.org/linux-pci/1601029386-4928-1-git-send-email-yangyicong@hisilicon.com/

Change since v1:
- 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 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Rafael J. Wysocki April 22, 2022, 11:31 a.m. UTC | #1
On Fri, Apr 22, 2022 at 10:06 AM Yicong Yang <yangyicong@hisilicon.com> wrote:
>
> 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.

I would rephrase this in the following way:

0000:80:00.0 is a Root Port and it is runtime-suspended, so
0000:81:00.1 is unreachable after a rescan.

>
> Make bridge powered on when scanning the child bus, by adding

"Power up the bridge when scanning the child bus and allow it to
suspend again by adding ..."

> pm_runtime_get_sync()/pm_runtime_put() in pci_scan_child_bus_extend().
>
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>

With the above addressed, please feel free to add

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Change since v3:
> - retain the pm_runtime_*() calls in pci_scan_bridge_extend() as Rafael points
>   out that it's necessary when the brigde is in D3cold
> Link: https://lore.kernel.org/linux-pci/20220414123736.34150-1-yangyicong@hisilicon.com/
>
> Change since v2:
> - just rebase it on v5.18-rc2
> Link: https://lore.kernel.org/linux-pci/1601029386-4928-1-git-send-email-yangyicong@hisilicon.com/
>
> Change since v1:
> - 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 | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 17a969942d37..b108e72b6586 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2859,11 +2859,20 @@ 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 *bridge = bus->self;
>         struct pci_dev *dev;
>         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);
> @@ -2976,6 +2985,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
> --
> 2.24.0
>
diff mbox series

Patch

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 17a969942d37..b108e72b6586 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2859,11 +2859,20 @@  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 *bridge = bus->self;
 	struct pci_dev *dev;
 	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);
@@ -2976,6 +2985,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