[PATCH RESEND v4 0/9] PCI: Fixes and cleanups for native PCIe and ACPI hotplug

[PATCH RESEND v4 0/9] PCI: Fixes and cleanups for native PCIe and ACPI hotplug

[Mika Westerberg]

[Resending this with linux-pci ML address corrected]

Hi,

When Thunderbolt host router is configured to be in native enumeration mode
it is only present in the system if there is something connected to the
ports. This pretty much follows how the BIOS assisted mode works.

In native enumeration mode the Thunderbolt host controller (NHI) and USB
host controller (xHCI) are not hot-added using native PCIe hotplug but
instead they will be hotplugged via BIOS triggered ACPI Notify() to the
root port. This is done to preserve resources since NHI and xHCI only need
1 MB of MMIO space and no additional buses. Currently Linux does not
support this very well and ends up failing the hotplug in one way or
another. More detailed explanation is in changelog of patch [4/9].

This series fixes this issue and in addition includes fixes for few other
issues found during testing on a system that has Thunderbolt controller in
native PCIe enumeration mode. However, the fixes here are not in any way
Thunderbolt specific and should be applicable to other systems as well.

The previous versions of the patch series can be found here:

  v3: https://www.spinics.net/lists/linux-acpi/msg80876.html
  v2: https://www.spinics.net/lists/linux-pci/msg69186.html
  v1: https://www.spinics.net/lists/linux-acpi/msg80607.html

Changes from v3:

  - Added Andy's tag to patches [1-5]
  - Improved changelog and subject line of patch [1/9] to match better what
    it is trying to solve
  - Improved comment in patch [1/9]
  - Improved changelog of patches [2/9] and [4/9] accordingly to match
    better why they are needed
  - Added cleanup patches [6-9/9]

Changes from v2:

  - Added Rafael's tag to patch [1/5]
  - Updated changelog of patch [1/5] to include more details about how the
    problem can be observed from dmesg and lspci output

Changes from v1:

  - Drop 'cmax - max ?: 1' and use similar construct than we use in second
    pass loop in patch [1/5]
  - Drop unnecessary parentheses in patch [1/5]
  - Added Rafael's tag to patches [2-5/5]

Mika Westerberg (9):
  PCI: Take all bridges into account when calculating bus numbers for extension
  PCI: Take bridge window alignment into account when distributing resources
  PCI: pciehp: Clear Presence Detect and Data Link Layer Status Changed on resume
  ACPI / hotplug / PCI: Do not scan all bridges when native PCIe hotplug is used
  ACPI / hotplug / PCI: Mark stale PCI devices disconnected
  PCI: Move resource distribution for a single bridge outside of the loop
  PCI: Document return value of pci_scan_bridge() and pci_scan_bridge_extend()
  PCI: Improve "partially hidden behind bridge" log message
  ACPI / hotplug / PCI: Drop unnecessary parentheses

 drivers/pci/hotplug/acpiphp.h      |  1 +
 drivers/pci/hotplug/acpiphp_glue.c | 82 ++++++++++++++++++++++++++++++--------
 drivers/pci/hotplug/pciehp.h       |  2 +-
 drivers/pci/hotplug/pciehp_core.c  |  2 +-
 drivers/pci/hotplug/pciehp_hpc.c   | 13 +++++-
 drivers/pci/probe.c                | 33 +++++++++------
 drivers/pci/setup-bus.c            | 69 ++++++++++++++++++++++++++------
 7 files changed, 158 insertions(+), 44 deletions(-)

-- 
2.16.2

[PATCH RESEND v4 1/9] PCI: Take all bridges into account when calculating bus numbers for extension

[Mika Westerberg]

When distributing extra buses between hotplug bridges for future
extension we currently fail to take into account the fact that there
might be non-hotplug bridges on the bus after the hotplug bridges. In
this particular system we have following topology:

  01:00.0 --+- 02:00.0 -- Thunderbolt host controller
            +- 02:01.0 (HotPlug+)
            \- 02:02.0 -- xHCI host controller

Now, pci_scan_child_bus_extend() is supposed to distribute remaining bus
numbers to the hotplug bridge at 02:01.0 but only after all bridges on
that bus have been been accounted for. Since we don't check upfront that
there will be another non-hotplug bridge after the hotplug bridge
02:01.0 it will inadvertently extend over the bus space of the
non-hotplug bridge:

  pci 0000:01:00.0: PCI bridge to [bus 02-39]
  ...
  pci_bus 0000:04: [bus 04-39] extended by 0x35
  pci_bus 0000:04: bus scan returning with max=39
  pci_bus 0000:04: busn_res: [bus 04-39] end is updated to 39
  pci 0000:02:02.0: scanning [bus 00-00] behind bridge, pass 1
  pci_bus 0000:3a: scanning bus
  pci_bus 0000:3a: bus scan returning with max=3a
  pci_bus 0000:3a: busn_res: [bus 3a] end is updated to 3a
  pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:02 [bus 02-39]
  pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:01 [bus 01-39]
  pci_bus 0000:02: bus scan returning with max=3a
  pci_bus 0000:02: busn_res: [bus 02-39] end can not be updated to 3a

Resulting 'lspci -t' output looks like this:

  +-1b.0-[01-39]----00.0-[02-3a]--+-00.0-[03]----00.0
                             ^^   +-01.0-[04-39]--
                                  \-02.0-[3a]----00.0
                                          ^^
The xHCI host controller (3a:00.0) behind downstream bridge at 02:02.0
is not usable anymore.

To fix this reserve at least one bus for each bridge during scanning of
already configured bridges. Then we use this information in the second
scan to correct the available extra bus space for hotplug bridges.

After this change the 'lspci -t' output is what is expected:

  +-1b.0-[01-39]----00.0-[02-39]--+-00.0-[03]----00.0
                                  +-01.0-[04-38]--
                                  \-02.0-[39]----00.0

Fixes: 1c02ea810065 ("PCI: Distribute available buses to hotplug-capable bridges")
Reported-by: Mario Limonciello <mario.limonciello@dell.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/pci/probe.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ef5377438a1e..8c4715df62e7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2561,7 +2561,14 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
 	for_each_pci_bridge(dev, bus) {
 		cmax = max;
 		max = pci_scan_bridge_extend(bus, dev, max, 0, 0);
-		used_buses += cmax - max;
+		/*
+		 * Reserve one bus for each bridge now to avoid
+		 * extending hotplug bridges too much during the second
+		 * scan below.
+		 */
+		used_buses++;
+		if (cmax - max > 1)
+			used_buses += cmax - max - 1;
 	}
 
 	/* Scan bridges that need to be reconfigured */
@@ -2584,12 +2591,14 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
 			 * bridges if any.
 			 */
 			buses = available_buses / hotplug_bridges;
-			buses = min(buses, available_buses - used_buses);
+			buses = min(buses, available_buses - used_buses + 1);
 		}
 
 		cmax = max;
 		max = pci_scan_bridge_extend(bus, dev, cmax, buses, 1);
-		used_buses += max - cmax;
+		/* One bus is already accounted so don't add it again */
+		if (max - cmax > 1)
+			used_buses += max - cmax - 1;
 	}
 
 	/*
-- 
2.16.2

[PATCH RESEND v4 2/9] PCI: Take bridge window alignment into account when distributing resources

[Mika Westerberg]

When hot-adding a PCIe switch the way we currently distribute resources
does not always work well because devices connected to the switch might
need to have their MMIO resources aligned to something else than the
default 1 MB boundary. For example Intel Gigabit ET2 quad port server
adapter includes PCIe switch leading to 4 x GbE NIC devices that want
to have their MMIO resources aligned to 2 MB boundary instead.

The current resource distribution code does not take this alignment into
account and might try to add too much resources for the extension
hotplug bridge(s). The resulting bridge window is too big which makes
the resource assignment operation fail, and we are left with a bridge
window with minimal amount (1 MB) of MMIO space.

Here is what happens when an Intel Gigabit ET2 quad port server adapter
is hot-added:

  pci 0000:39:00.0: BAR 14: assigned [mem 0x53300000-0x6a0fffff]
                                          ^^^^^^^^^^
  pci 0000:3a:01.0: BAR 14: assigned [mem 0x53400000-0x547fffff]
                                          ^^^^^^^^^^
The above shows that the downstream bridge (3a:01.0) window is aligned
to 2 MB instead of 1 MB as is the upstream bridge (39:00.0) window. The
remaining MMIO space (0x15a00000) is assigned to the hotplug bridge
(3a:04.0) but it fails:

  pci 0000:3a:04.0: BAR 14: no space for [mem size 0x15a00000]
  pci 0000:3a:04.0: BAR 14: failed to assign [mem size 0x15a00000]

The MMIO resource is calculated as follows:

  start = 0x54800000
  end = 0x54800000 + 0x15a00000 - 1 = 0x6a1fffff

This results bridge window [mem 0x54800000 - 0x6a1fffff] and it ends
after the upstream bridge window [mem 0x53300000-0x6a0fffff] explaining
the above failure. Because of this Linux falls back to the default
allocation of 1 MB as can be seen from 'lspci' output:

 39:00.0 Memory behind bridge: 53300000-6a0fffff [size=366M]
   3a:01.0 Memory behind bridge: 53400000-547fffff [size=20M]
   3a:04.0 Memory behind bridge: 53300000-533fffff [size=1M]

The hotplug bridge 3a:04.0 only occupies 1 MB MMIO window which is
clearly not enough for extending the PCIe topology later if more devices
are to be hot-added.

Fix this by substracting properly aligned non-hotplug downstream bridge
window size from the remaining resources used for extension. After this
change the resource allocation looks like:

  39:00.0 Memory behind bridge: 53300000-6a0fffff [size=366M]
    3a:01.0 Memory behind bridge: 53400000-547fffff [size=20M]
    3a:04.0 Memory behind bridge: 54800000-6a0fffff [size=345M]

This matches the expectation. All the extra MMIO resource space (345 MB)
is allocated to the extension hotplug bridge (3a:04.0).

Fixes: 1a5767725cec ("PCI: Distribute available resources to hotplug-capable bridges")
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/pci/setup-bus.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 3cce29a069e6..f1e6172734f0 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1882,6 +1882,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 	resource_size_t available_mmio, resource_size_t available_mmio_pref)
 {
 	resource_size_t remaining_io, remaining_mmio, remaining_mmio_pref;
+	resource_size_t io_start, mmio_start, mmio_pref_start;
 	unsigned int normal_bridges = 0, hotplug_bridges = 0;
 	struct resource *io_res, *mmio_res, *mmio_pref_res;
 	struct pci_dev *dev, *bridge = bus->self;
@@ -1946,11 +1947,16 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 			remaining_mmio_pref -= resource_size(res);
 	}
 
+	io_start = io_res->start;
+	mmio_start = mmio_res->start;
+	mmio_pref_start = mmio_pref_res->start;
+
 	/*
 	 * Go over devices on this bus and distribute the remaining
 	 * resource space between hotplug bridges.
 	 */
 	for_each_pci_bridge(dev, bus) {
+		resource_size_t align;
 		struct pci_bus *b;
 
 		b = dev->subordinate;
@@ -1968,7 +1974,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 				available_io, available_mmio,
 				available_mmio_pref);
 		} else if (dev->is_hotplug_bridge) {
-			resource_size_t align, io, mmio, mmio_pref;
+			resource_size_t io, mmio, mmio_pref;
 
 			/*
 			 * Distribute available extra resources equally
@@ -1981,11 +1987,13 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 			io = div64_ul(available_io, hotplug_bridges);
 			io = min(ALIGN(io, align), remaining_io);
 			remaining_io -= io;
+			io_start += io;
 
 			align = pci_resource_alignment(bridge, mmio_res);
 			mmio = div64_ul(available_mmio, hotplug_bridges);
 			mmio = min(ALIGN(mmio, align), remaining_mmio);
 			remaining_mmio -= mmio;
+			mmio_start += mmio;
 
 			align = pci_resource_alignment(bridge, mmio_pref_res);
 			mmio_pref = div64_ul(available_mmio_pref,
@@ -1993,9 +2001,40 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 			mmio_pref = min(ALIGN(mmio_pref, align),
 					remaining_mmio_pref);
 			remaining_mmio_pref -= mmio_pref;
+			mmio_pref_start += mmio_pref;
 
 			pci_bus_distribute_available_resources(b, add_list, io,
 							       mmio, mmio_pref);
+		} else {
+			/*
+			 * For normal bridges, track start of the parent
+			 * bridge window to make sure we align the
+			 * remaining space which is distributed to the
+			 * hotplug bridges properly.
+			 */
+			resource_size_t aligned;
+			struct resource *res;
+
+			res = &dev->resource[PCI_BRIDGE_RESOURCES + 0];
+			io_start += resource_size(res);
+			aligned = ALIGN(io_start,
+					pci_resource_alignment(dev, res));
+			if (aligned > io_start)
+				remaining_io -= aligned - io_start;
+
+			res = &dev->resource[PCI_BRIDGE_RESOURCES + 1];
+			mmio_start += resource_size(res);
+			aligned = ALIGN(mmio_start,
+					pci_resource_alignment(dev, res));
+			if (aligned > mmio_start)
+				remaining_mmio -= aligned - mmio_start;
+
+			res = &dev->resource[PCI_BRIDGE_RESOURCES + 2];
+			mmio_pref_start += resource_size(res);
+			aligned = ALIGN(mmio_pref_start,
+					pci_resource_alignment(dev, res));
+			if (aligned > mmio_pref_start)
+				remaining_mmio_pref -= aligned - mmio_pref_start;
 		}
 	}
 }
-- 
2.16.2

[PATCH RESEND v4 3/9] PCI: pciehp: Clear Presence Detect and Data Link Layer Status Changed on resume

[Mika Westerberg]

After suspend/resume cycle the Presence Detect or Data Link Layer Status
Changed bits might be set and if we don't clear them those events will
not fire anymore and nothing happens for instance when a device is now
hot-unplugged.

Fix this by clearing those bits in a newly introduced function
pcie_reenable_notification(). This should be fine because immediately
after we check if the adapter is still present by reading directly from
the status register.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/pci/hotplug/pciehp.h      |  2 +-
 drivers/pci/hotplug/pciehp_core.c |  2 +-
 drivers/pci/hotplug/pciehp_hpc.c  | 13 ++++++++++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index 636ed8f4b869..5abf864eae35 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -120,7 +120,7 @@ struct controller *pcie_init(struct pcie_device *dev);
 int pcie_init_notification(struct controller *ctrl);
 int pciehp_enable_slot(struct slot *p_slot);
 int pciehp_disable_slot(struct slot *p_slot);
-void pcie_enable_notification(struct controller *ctrl);
+void pcie_reenable_notification(struct controller *ctrl);
 int pciehp_power_on_slot(struct slot *slot);
 void pciehp_power_off_slot(struct slot *slot);
 void pciehp_get_power_status(struct slot *slot, u8 *status);
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 332b723ff9e6..44a6a63802d5 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -283,7 +283,7 @@ static int pciehp_resume(struct pcie_device *dev)
 	ctrl = get_service_data(dev);
 
 	/* reinitialize the chipset's event detection logic */
-	pcie_enable_notification(ctrl);
+	pcie_reenable_notification(ctrl);
 
 	slot = ctrl->slot;
 
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 18a42f8f5dc5..98ea75aa32c7 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -659,7 +659,7 @@ static irqreturn_t pcie_isr(int irq, void *dev_id)
 	return handled;
 }
 
-void pcie_enable_notification(struct controller *ctrl)
+static void pcie_enable_notification(struct controller *ctrl)
 {
 	u16 cmd, mask;
 
@@ -697,6 +697,17 @@ void pcie_enable_notification(struct controller *ctrl)
 		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, cmd);
 }
 
+void pcie_reenable_notification(struct controller *ctrl)
+{
+	/*
+	 * Clear both Presence and Data Link Layer Changed to make sure
+	 * those events still fire after we have re-enabled them.
+	 */
+	pcie_capability_write_word(ctrl->pcie->port, PCI_EXP_SLTSTA,
+				   PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC);
+	pcie_enable_notification(ctrl);
+}
+
 static void pcie_disable_notification(struct controller *ctrl)
 {
 	u16 mask;
-- 
2.16.2

[PATCH RESEND v4 4/9] ACPI / hotplug / PCI: Do not scan all bridges when native PCIe hotplug is used

[Mika Westerberg]

When a system is using native PCIe hotplug for Thunderbolt and the
controller is not enabled for full RTD3 (runtime D3) it will be only
present in the system when there is a device connected. This pretty much
follows the BIOS assisted hotplug behaviour.

Thunderbolt host router integrated PCIe switch has two additional PCIe
downstream bridges that lead to NHI (Thunderbolt host controller) and xHCI
(USB 3 host controller) respectively. These downstream bridges are not
marked being hotplug capable. Reason for that is to preserve resources.
Otherwise the OS would distribute remaining resources between all
downstream bridges making these two bridges consume precious resources
of the actual hotplug bridges.

Now, because these two bridges are not marked being hotplug capable the OS
will not enable hotplug interrupt for them either and will not receive
interrupt when devices behind them are hot-added. Solution to this is
that the BIOS sends ACPI Notify() to the root port let the OS know it
needs to rescan for added and/or removed devices.

Here is how the mechanism is supposed to work when a Thunderbolt
endpoint is connected to one of the ports. In case of a standard USB-C
device only the xHCI is hot-added otherwise steps are the same.

1. Initially there is only the PCIe root port that is controlled by
   the pciehp driver

  00:1b.0 (Hotplug+) --

2. Then we get native PCIe hotplug interrupt and once it is handled the
   topology looks as following

  00:1b.0 (Hotplug+) -- 01:00.0 --+- 02:00.0 --
                                  +- 02:01.0 (HotPlug+)
                                  \- 02:02.0 --

3. Bridges 02:00.0 and 02:02.0 are not marked as hotplug capable and
   they don't have anything behind them currently. Bridge 02:01.0 is
   hotplug capable and used for extending the topology. At this point
   the required PCIe devices are enabled and ACPI Notify() is sent to
   the root port. The resulting topology is expected to look like

  00:1b.0 (Hotplug+) -- 01:00.0 --+- 02:00.0 -- Thunderbolt host controller
                                  +- 02:01.0 (HotPlug+)
                                  \- 02:02.0 -- xHCI host controller

However, the current ACPI hotplug implementation scans the whole 00:1b.0
hotplug slot and everything behind it regardless whether native PCIe is
used or not, and it expects that the BIOS has configured bridge
resources upfront. If that's not the case it assigns resources using
minimal allocation (everything currently found just barely fit)
preventing future extension. In addition to that, if there is another
native PCIe hotplug going on we may find the new PCIe switch only
partially ready (all links are not fully trained yet) confusing pciehp
when it finally starts to enumerate for new devices.

To make this work better with the native PCIe hotplug driver (pciehp),
we let it handle all slot management and resource allocation for hotplug
bridges and restrict ACPI hotplug to non-hotplug bridges.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/pci/hotplug/acpiphp.h      |  1 +
 drivers/pci/hotplug/acpiphp_glue.c | 73 ++++++++++++++++++++++++++++++--------
 2 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index e438a2d734f2..8dcfd623ef1d 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -158,6 +158,7 @@ struct acpiphp_attention_info
 
 #define SLOT_ENABLED		(0x00000001)
 #define SLOT_IS_GOING_AWAY	(0x00000002)
+#define SLOT_IS_NATIVE		(0x00000004)
 
 /* function flags */
 
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index e2198a2feeca..43afeac7f5f3 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -282,6 +282,9 @@ static acpi_status acpiphp_add_context(acpi_handle handle, u32 lvl, void *data,
 	slot->device = device;
 	INIT_LIST_HEAD(&slot->funcs);
 
+	if (pdev && pciehp_is_native(pdev))
+		slot->flags |= SLOT_IS_NATIVE;
+
 	list_add_tail(&slot->node, &bridge->slots);
 
 	/*
@@ -291,7 +294,7 @@ static acpi_status acpiphp_add_context(acpi_handle handle, u32 lvl, void *data,
 	 * expose slots to user space in those cases.
 	 */
 	if ((acpi_pci_check_ejectable(pbus, handle) || is_dock_device(adev))
-	    && !(pdev && pdev->is_hotplug_bridge && pciehp_is_native(pdev))) {
+	    && !(slot->flags & SLOT_IS_NATIVE && pdev->is_hotplug_bridge)) {
 		unsigned long long sun;
 		int retval;
 
@@ -430,6 +433,29 @@ static int acpiphp_rescan_slot(struct acpiphp_slot *slot)
 	return pci_scan_slot(slot->bus, PCI_DEVFN(slot->device, 0));
 }
 
+static void acpiphp_native_scan_bridge(struct pci_dev *bridge)
+{
+	struct pci_bus *bus = bridge->subordinate;
+	struct pci_dev *dev;
+	int max;
+
+	if (!bus)
+		return;
+
+	max = bus->busn_res.start;
+	/* Scan already configured non-hotplug bridges */
+	for_each_pci_bridge(dev, bus) {
+		if (!dev->is_hotplug_bridge)
+			max = pci_scan_bridge(bus, dev, max, 0);
+	}
+
+	/* Scan non-hotplug bridges that need to be reconfigured */
+	for_each_pci_bridge(dev, bus) {
+		if (!dev->is_hotplug_bridge)
+			max = pci_scan_bridge(bus, dev, max, 1);
+	}
+}
+
 /**
  * enable_slot - enable, configure a slot
  * @slot: slot to be enabled
@@ -442,25 +468,42 @@ static void enable_slot(struct acpiphp_slot *slot)
 	struct pci_dev *dev;
 	struct pci_bus *bus = slot->bus;
 	struct acpiphp_func *func;
-	int max, pass;
-	LIST_HEAD(add_list);
 
-	acpiphp_rescan_slot(slot);
-	max = acpiphp_max_busnr(bus);
-	for (pass = 0; pass < 2; pass++) {
+	if (slot->flags & SLOT_IS_NATIVE) {
+		/*
+		 * If native PCIe hotplug is used, it will take care of
+		 * hotplug slot management and resource allocation for
+		 * hotplug bridges. However, ACPI hotplug may still be
+		 * used for non-hotplug bridges to bring in additional
+		 * devices such as Thunderbolt host controller.
+		 */
 		for_each_pci_bridge(dev, bus) {
-			if (PCI_SLOT(dev->devfn) != slot->device)
-				continue;
-
-			max = pci_scan_bridge(bus, dev, max, pass);
-			if (pass && dev->subordinate) {
-				check_hotplug_bridge(slot, dev);
-				pcibios_resource_survey_bus(dev->subordinate);
-				__pci_bus_size_bridges(dev->subordinate, &add_list);
+			if (PCI_SLOT(dev->devfn) == slot->device)
+				acpiphp_native_scan_bridge(dev);
+		}
+		pci_assign_unassigned_bridge_resources(bus->self);
+	} else {
+		LIST_HEAD(add_list);
+		int max, pass;
+
+		acpiphp_rescan_slot(slot);
+		max = acpiphp_max_busnr(bus);
+		for (pass = 0; pass < 2; pass++) {
+			for_each_pci_bridge(dev, bus) {
+				if (PCI_SLOT(dev->devfn) != slot->device)
+					continue;
+
+				max = pci_scan_bridge(bus, dev, max, pass);
+				if (pass && dev->subordinate) {
+					check_hotplug_bridge(slot, dev);
+					pcibios_resource_survey_bus(dev->subordinate);
+					__pci_bus_size_bridges(dev->subordinate,
+							       &add_list);
+				}
 			}
 		}
+		__pci_bus_assign_resources(bus, &add_list, NULL);
 	}
-	__pci_bus_assign_resources(bus, &add_list, NULL);
 
 	acpiphp_sanitize_bus(bus);
 	pcie_bus_configure_settings(bus);
-- 
2.16.2

[PATCH RESEND v4 5/9] ACPI / hotplug / PCI: Mark stale PCI devices disconnected

[Mika Westerberg]

Following PCIehp mark the unplugged PCI devices disconnected. This makes
sure PCI core code leaves the now missing hardware registers alone.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pci/hotplug/acpiphp_glue.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 43afeac7f5f3..5d7be06bba7f 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -642,6 +642,11 @@ static void trim_stale_devices(struct pci_dev *dev)
 		alive = pci_device_is_present(dev);
 
 	if (!alive) {
+		pci_dev_set_disconnected(dev, NULL);
+		if (pci_has_subordinate(dev))
+			pci_walk_bus(dev->subordinate, pci_dev_set_disconnected,
+				     NULL);
+
 		pci_stop_and_remove_bus_device(dev);
 		if (adev)
 			acpi_bus_trim(adev);
-- 
2.16.2

[PATCH RESEND v4 6/9] PCI: Move resource distribution for a single bridge outside of the loop

[Mika Westerberg]

There is a special case where there is only single bridge on the bus. In
that case we just assign all resources to it. Currently this is done as
a part of the resource distribution loop but it does not have to be
there, and moving it outside actually improves readability.

While there we can add hotplug_bridges == 1 && normal_bridges == 0 to
the same block because they are dealt the same way.

Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/setup-bus.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index f1e6172734f0..b5eefd20d78d 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1951,6 +1951,22 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 	mmio_start = mmio_res->start;
 	mmio_pref_start = mmio_pref_res->start;
 
+	/*
+	 * There is only one bridge on the bus so it gets all available
+	 * resources which it can then distribute to the possible
+	 * hotplug bridges below.
+	 */
+	if ((hotplug_bridges == 0 && normal_bridges == 1) ||
+	    (hotplug_bridges == 1 && normal_bridges == 0)) {
+		dev = list_first_entry(&bus->devices, struct pci_dev, bus_list);
+		if (dev->subordinate) {
+			pci_bus_distribute_available_resources(dev->subordinate,
+				add_list, available_io, available_mmio,
+				available_mmio_pref);
+		}
+		return;
+	}
+
 	/*
 	 * Go over devices on this bus and distribute the remaining
 	 * resource space between hotplug bridges.
@@ -1963,17 +1979,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 		if (!b)
 			continue;
 
-		if (!hotplug_bridges && normal_bridges == 1) {
-			/*
-			 * There is only one bridge on the bus (upstream
-			 * port) so it gets all available resources
-			 * which it can then distribute to the possible
-			 * hotplug bridges below.
-			 */
-			pci_bus_distribute_available_resources(b, add_list,
-				available_io, available_mmio,
-				available_mmio_pref);
-		} else if (dev->is_hotplug_bridge) {
+		if (dev->is_hotplug_bridge) {
 			resource_size_t io, mmio, mmio_pref;
 
 			/*
-- 
2.16.2

[PATCH RESEND v4 7/9] PCI: Document return value of pci_scan_bridge() and pci_scan_bridge_extend()

[Mika Westerberg]

It is not immediately clear what the two functions actually return so
add kernel-doc comment explaining it a bit better.

Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/probe.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8c4715df62e7..079eea6680bb 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -983,6 +983,8 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
  * already configured by the BIOS and after we are done with all of
  * them, we proceed to assigning numbers to the remaining buses in
  * order to avoid overlaps between old and new bus numbers.
+ *
+ * Return: New subordinate number covering all buses behind this bridge.
  */
 static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
 				  int max, unsigned int available_buses,
@@ -1215,6 +1217,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
  * already configured by the BIOS and after we are done with all of
  * them, we proceed to assigning numbers to the remaining buses in
  * order to avoid overlaps between old and new bus numbers.
+ *
+ * Return: New subordinate number covering all buses behind this bridge.
  */
 int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
 {
-- 
2.16.2

[PATCH RESEND v4 8/9] PCI: Improve "partially hidden behind bridge" log message

[Mika Westerberg]

There is a sanity checker in the end of pci_scan_child_bus_extend() that
tries to detect badly configured bridges. For example given the below
topology:

  +-1b.0-[01-39]----00.0-[02-3a]--+-00.0-[03]----00.0
                                  +-01.0-[04-39]--
                                  \-02.0-[3a]----00.0

The sanity checker notices this and logs following messages:

  pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:02 [bus 02-39]
  pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:01 [bus 01-39]

This is not really helpful to users and the information above is not
even correct (0000:02 is a bus not bridge). Make this a bit more
understandable by changing the sanity checker to log following message
in place of the above two messages:

  pci 0000:02:02.0: devices behind bridge are unusable because we cannot assign [bus 3a] for them

While there update the comment on top of the sanity checker block to
make it clear that it is not only restricted to CardBus.

Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/probe.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 079eea6680bb..70b07a0e00b8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1175,20 +1175,16 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
 		(is_cardbus ? "PCI CardBus %04x:%02x" : "PCI Bus %04x:%02x"),
 		pci_domain_nr(bus), child->number);
 
-	/* Has only triggered on CardBus, fixup is in yenta_socket */
+	/* Check that all devices are accessible */
 	while (bus->parent) {
 		if ((child->busn_res.end > bus->busn_res.end) ||
 		    (child->number > bus->busn_res.end) ||
 		    (child->number < bus->number) ||
 		    (child->busn_res.end < bus->number)) {
-			dev_info(&child->dev, "%pR %s hidden behind%s bridge %s %pR\n",
-				&child->busn_res,
-				(bus->number > child->busn_res.end &&
-				 bus->busn_res.end < child->number) ?
-					"wholly" : "partially",
-				bus->self->transparent ? " transparent" : "",
-				dev_name(&bus->dev),
-				&bus->busn_res);
+			dev_info(&dev->dev,
+				 "devices behind bridge are unusable because we cannot assign %pR for them\n",
+				 &child->busn_res);
+			break;
 		}
 		bus = bus->parent;
 	}
-- 
2.16.2

[PATCH RESEND v4 9/9] ACPI / hotplug / PCI: Drop unnecessary parentheses

[Mika Westerberg]

There is no need for them and it makes the code look uglier than it
should. Remove them.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/hotplug/acpiphp_glue.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 5d7be06bba7f..ce8716c022f0 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -524,7 +524,7 @@ static void enable_slot(struct acpiphp_slot *slot)
 		if (!dev) {
 			/* Do not set SLOT_ENABLED flag if some funcs
 			   are not added. */
-			slot->flags &= (~SLOT_ENABLED);
+			slot->flags &= ~SLOT_ENABLED;
 			continue;
 		}
 	}
@@ -553,7 +553,7 @@ static void disable_slot(struct acpiphp_slot *slot)
 	list_for_each_entry(func, &slot->funcs, sibling)
 		acpi_bus_trim(func_to_acpi_device(func));
 
-	slot->flags &= (~SLOT_ENABLED);
+	slot->flags &= ~SLOT_ENABLED;
 }
 
 static bool slot_no_hotplug(struct acpiphp_slot *slot)
-- 
2.16.2

Re: [PATCH RESEND v4 3/9] PCI: pciehp: Clear Presence Detect and Data Link Layer Status Changed on resume

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has also determined it's probably a bug fixing patch. (score: 43.1022)

The bot has tested the following trees: v4.16, v4.15.15, v4.14.32, v4.9.92, v4.4.126.

v4.16: Build OK!
v4.15.15: Build OK!
v4.14.32: Build OK!
v4.9.92: Build OK!
v4.4.126: Build OK!

Please let us know if you'd like to have this patch included in a stable tree.

--
Thanks,
Sasha

Re: [PATCH RESEND v4 1/9] PCI: Take all bridges into account when calculating bus numbers for extension

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 1c02ea810065 PCI: Distribute available buses to hotplug-capable bridges.

The bot has also determined it's probably a bug fixing patch. (score: 93.5345)

The bot has tested the following trees: v4.16, v4.15.15.

v4.16: Build OK!
v4.15.15: Build OK!

--
Thanks,
Sasha

Re: [PATCH RESEND v4 2/9] PCI: Take bridge window alignment into account when distributing resources

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 1a5767725cec PCI: Distribute available resources to hotplug-capable bridges.

The bot has also determined it's probably a bug fixing patch. (score: 7.3001)

The bot has tested the following trees: v4.16, v4.15.15.

v4.16: Build OK!
v4.15.15: Build OK!

--
Thanks,
Sasha

Re: [PATCH RESEND v4 4/9] ACPI / hotplug / PCI: Do not scan all bridges when native PCIe hotplug is used

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.16, v4.15.15, v4.14.32, v4.9.92, v4.4.126.

v4.16: Build OK!
v4.15.15: Build OK!
v4.14.32: Failed to apply! Possible dependencies:
    66476ebea943 ("ACPI / hotplug / PCI: Do not scan all bridges when native PCIe hotplug is used")

v4.9.92: Failed to apply! Possible dependencies:
    437eb7bf7b28 ("ACPI / hotplug / PCI: Make device_is_managed_by_native_pciehp() public")
    66476ebea943 ("ACPI / hotplug / PCI: Do not scan all bridges when native PCIe hotplug is used")

v4.4.126: Failed to apply! Possible dependencies:
    437eb7bf7b28 ("ACPI / hotplug / PCI: Make device_is_managed_by_native_pciehp() public")
    66476ebea943 ("ACPI / hotplug / PCI: Do not scan all bridges when native PCIe hotplug is used")


Please let us know if you'd like to have this patch included in a stable tree.

--
Thanks,
Sasha

Re: [PATCH RESEND v4 6/9] PCI: Move resource distribution for a single bridge outside of the loop

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed by the -stable helper bot and determined
to be a high probability candidate for -stable trees. (score: 13.3696)

The bot has tested the following trees: v4.16, v4.15.15, v4.14.32, v4.9.92, v4.4.126.

v4.16: Failed to apply! Possible dependencies:
    b4e4f66ae0d5 ("PCI: Take bridge window alignment into account when distributing resources")

v4.15.15: Failed to apply! Possible dependencies:
    b4e4f66ae0d5 ("PCI: Take bridge window alignment into account when distributing resources")

v4.14.32: Failed to apply! Possible dependencies:
    2e4a9cf5b125 ("PCI: Move resource distribution for a single bridge outside of the loop")
    b4e4f66ae0d5 ("PCI: Take bridge window alignment into account when distributing resources")

v4.9.92: Failed to apply! Possible dependencies:
    2e4a9cf5b125 ("PCI: Move resource distribution for a single bridge outside of the loop")
    b4e4f66ae0d5 ("PCI: Take bridge window alignment into account when distributing resources")

v4.4.126: Failed to apply! Possible dependencies:
    2e4a9cf5b125 ("PCI: Move resource distribution for a single bridge outside of the loop")
    b4e4f66ae0d5 ("PCI: Take bridge window alignment into account when distributing resources")


Please let us know if you'd like to have this patch included in a stable tree.

--
Thanks,
Sasha

Re: [PATCH RESEND v4 7/9] PCI: Document return value of pci_scan_bridge() and pci_scan_bridge_extend()

[Rafael J. Wysocki]

On Tuesday, April 3, 2018 4:52:23 PM CEST Mika Westerberg wrote:
> It is not immediately clear what the two functions actually return so
> add kernel-doc comment explaining it a bit better.
> 
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/probe.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8c4715df62e7..079eea6680bb 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -983,6 +983,8 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
>   * already configured by the BIOS and after we are done with all of
>   * them, we proceed to assigning numbers to the remaining buses in
>   * order to avoid overlaps between old and new bus numbers.
> + *
> + * Return: New subordinate number covering all buses behind this bridge.
>   */
>  static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>  				  int max, unsigned int available_buses,
> @@ -1215,6 +1217,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>   * already configured by the BIOS and after we are done with all of
>   * them, we proceed to assigning numbers to the remaining buses in
>   * order to avoid overlaps between old and new bus numbers.
> + *
> + * Return: New subordinate number covering all buses behind this bridge.
>   */
>  int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>  {
> 

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

Re: [PATCH RESEND v4 6/9] PCI: Move resource distribution for a single bridge outside of the loop

[Rafael J. Wysocki]

On Tuesday, April 3, 2018 4:52:22 PM CEST Mika Westerberg wrote:
> There is a special case where there is only single bridge on the bus. In
> that case we just assign all resources to it. Currently this is done as
> a part of the resource distribution loop but it does not have to be
> there, and moving it outside actually improves readability.
> 
> While there we can add hotplug_bridges == 1 && normal_bridges == 0 to
> the same block because they are dealt the same way.
> 
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/setup-bus.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index f1e6172734f0..b5eefd20d78d 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1951,6 +1951,22 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
>  	mmio_start = mmio_res->start;
>  	mmio_pref_start = mmio_pref_res->start;
>  
> +	/*
> +	 * There is only one bridge on the bus so it gets all available
> +	 * resources which it can then distribute to the possible
> +	 * hotplug bridges below.
> +	 */
> +	if ((hotplug_bridges == 0 && normal_bridges == 1) ||
> +	    (hotplug_bridges == 1 && normal_bridges == 0)) {
> +		dev = list_first_entry(&bus->devices, struct pci_dev, bus_list);
> +		if (dev->subordinate) {
> +			pci_bus_distribute_available_resources(dev->subordinate,
> +				add_list, available_io, available_mmio,
> +				available_mmio_pref);
> +		}
> +		return;
> +	}
> +
>  	/*
>  	 * Go over devices on this bus and distribute the remaining
>  	 * resource space between hotplug bridges.
> @@ -1963,17 +1979,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
>  		if (!b)
>  			continue;
>  
> -		if (!hotplug_bridges && normal_bridges == 1) {
> -			/*
> -			 * There is only one bridge on the bus (upstream
> -			 * port) so it gets all available resources
> -			 * which it can then distribute to the possible
> -			 * hotplug bridges below.
> -			 */
> -			pci_bus_distribute_available_resources(b, add_list,
> -				available_io, available_mmio,
> -				available_mmio_pref);
> -		} else if (dev->is_hotplug_bridge) {
> +		if (dev->is_hotplug_bridge) {
>  			resource_size_t io, mmio, mmio_pref;
>  
>  			/*
> 

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

Re: [PATCH RESEND v4 8/9] PCI: Improve "partially hidden behind bridge" log message

[Rafael J. Wysocki]

On Tuesday, April 3, 2018 4:52:24 PM CEST Mika Westerberg wrote:
> There is a sanity checker in the end of pci_scan_child_bus_extend() that
> tries to detect badly configured bridges. For example given the below
> topology:
> 
>   +-1b.0-[01-39]----00.0-[02-3a]--+-00.0-[03]----00.0
>                                   +-01.0-[04-39]--
>                                   \-02.0-[3a]----00.0
> 
> The sanity checker notices this and logs following messages:
> 
>   pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:02 [bus 02-39]
>   pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:01 [bus 01-39]
> 
> This is not really helpful to users and the information above is not
> even correct (0000:02 is a bus not bridge). Make this a bit more
> understandable by changing the sanity checker to log following message
> in place of the above two messages:
> 
>   pci 0000:02:02.0: devices behind bridge are unusable because we cannot assign [bus 3a] for them

FWIW, I prefer to use passive voice in messages, like

 devices behind bridge are unusable, because [bus 3a] cannot be assigned for them

> While there update the comment on top of the sanity checker block to
> make it clear that it is not only restricted to CardBus.
> 
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/probe.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 079eea6680bb..70b07a0e00b8 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1175,20 +1175,16 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>  		(is_cardbus ? "PCI CardBus %04x:%02x" : "PCI Bus %04x:%02x"),
>  		pci_domain_nr(bus), child->number);
>  
> -	/* Has only triggered on CardBus, fixup is in yenta_socket */
> +	/* Check that all devices are accessible */
>  	while (bus->parent) {
>  		if ((child->busn_res.end > bus->busn_res.end) ||
>  		    (child->number > bus->busn_res.end) ||
>  		    (child->number < bus->number) ||
>  		    (child->busn_res.end < bus->number)) {
> -			dev_info(&child->dev, "%pR %s hidden behind%s bridge %s %pR\n",
> -				&child->busn_res,
> -				(bus->number > child->busn_res.end &&
> -				 bus->busn_res.end < child->number) ?
> -					"wholly" : "partially",
> -				bus->self->transparent ? " transparent" : "",
> -				dev_name(&bus->dev),
> -				&bus->busn_res);
> +			dev_info(&dev->dev,
> +				 "devices behind bridge are unusable because we cannot assign %pR for them\n",
> +				 &child->busn_res);
> +			break;
>  		}
>  		bus = bus->parent;
>  	}
> 

Re: [PATCH RESEND v4 9/9] ACPI / hotplug / PCI: Drop unnecessary parentheses

[Rafael J. Wysocki]

On Tuesday, April 3, 2018 4:52:25 PM CEST Mika Westerberg wrote:
> There is no need for them and it makes the code look uglier than it
> should. Remove them.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/hotplug/acpiphp_glue.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 5d7be06bba7f..ce8716c022f0 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -524,7 +524,7 @@ static void enable_slot(struct acpiphp_slot *slot)
>  		if (!dev) {
>  			/* Do not set SLOT_ENABLED flag if some funcs
>  			   are not added. */
> -			slot->flags &= (~SLOT_ENABLED);
> +			slot->flags &= ~SLOT_ENABLED;
>  			continue;
>  		}
>  	}
> @@ -553,7 +553,7 @@ static void disable_slot(struct acpiphp_slot *slot)
>  	list_for_each_entry(func, &slot->funcs, sibling)
>  		acpi_bus_trim(func_to_acpi_device(func));
>  
> -	slot->flags &= (~SLOT_ENABLED);
> +	slot->flags &= ~SLOT_ENABLED;
>  }
>  
>  static bool slot_no_hotplug(struct acpiphp_slot *slot)
> 

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

Re: [PATCH RESEND v4 8/9] PCI: Improve "partially hidden behind bridge" log message

[Mika Westerberg]

On Tue, Apr 10, 2018 at 05:04:53PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, April 3, 2018 4:52:24 PM CEST Mika Westerberg wrote:
> > There is a sanity checker in the end of pci_scan_child_bus_extend() that
> > tries to detect badly configured bridges. For example given the below
> > topology:
> > 
> >   +-1b.0-[01-39]----00.0-[02-3a]--+-00.0-[03]----00.0
> >                                   +-01.0-[04-39]--
> >                                   \-02.0-[3a]----00.0
> > 
> > The sanity checker notices this and logs following messages:
> > 
> >   pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:02 [bus 02-39]
> >   pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:01 [bus 01-39]
> > 
> > This is not really helpful to users and the information above is not
> > even correct (0000:02 is a bus not bridge). Make this a bit more
> > understandable by changing the sanity checker to log following message
> > in place of the above two messages:
> > 
> >   pci 0000:02:02.0: devices behind bridge are unusable because we cannot assign [bus 3a] for them
> 
> FWIW, I prefer to use passive voice in messages, like
> 
>  devices behind bridge are unusable, because [bus 3a] cannot be assigned for them

Works for me :)

I'll update the message accordingly then.