[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250822145605.18172-10-ilpo.jarvinen@linux.intel.com>
Date: Fri, 22 Aug 2025 17:55:50 +0300
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Andreas Larsson <andreas@...sler.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
"David S. Miller" <davem@...emloft.net>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
linux-m68k@...ts.linux-m68k.org,
linux-mips@...r.kernel.org,
linux-pci@...r.kernel.org,
sparclinux@...r.kernel.org,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Christian König <christian.koenig@....com>,
Yinghai Lu <yinghai@...nel.org>,
Igor Mammedov <imammedo@...hat.com>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Krzysztof Wilczyński <kw@...ux.com>,
linux-kernel@...r.kernel.org
Cc: Michał Winiarski <michal.winiarski@...el.com>,
linuxppc-dev@...ts.ozlabs.org,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: [PATCH 09/24] PCI: Enable bridge even if bridge window fails to assign
A normal PCI bridge has multiple bridge windows and not all of them are
always required by devices underneath the bridge. If Root Port or
bridge does not have a device underneath, no bridge windows get
assigned. Yet, pci_enable_resources() is set to fail indiscriminantly
on any resource assignment failure if the resource is not known to be
optional.
In practice, the code in pci_enable_resources() is currently largely
dormant. The kernel sets resource flags to zero for any unused bridge
window and resets flags to zero in case of an resource assignment
failure, which short-circuits pci_enable_resources() because of this
check:
if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
continue;
However, an upcoming change to resource flags will alter how bridge
window flags resource flags behave activating these long dormants
checks in pci_enable_resources().
While complex logic could be built to selectively enable a bridge only
under some conditions, a few versions of such logic were tried during
development of this change and none of them worked satisfactorily.
Thus, I just gave up and decided to enable any bridge regardless of the
bridge windows as there seems to be no clear benefit from not enabling
it but a major downside as pcieport will not be probed for the bridge
if it's not enabled.
Therefore, change pci_enable_resources() to not check if bridge window
resources remain unassigned. Resource assignment failures are pretty
noisy already so there is no need to log that for bridge windows in
pci_enable_resources().
Ignoring bridge window failures hopefully prevents an obvious sources
of regressions when the upcoming change that no longer clears resource
flags for bridge windows is enacted. I've hit this problem even during
my own testing on multiple occassions so I expect it to be a quite
common problem.
This can always be revisited later if somebody thinks the enable check
for bridges is not strict enough, but expect a mind-boggling number of
regressions from such a change.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
---
drivers/pci/setup-res.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 0468c058b598..4e0e60256f04 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -527,22 +527,26 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
if (pci_resource_is_optional(dev, i))
continue;
- if (r->flags & IORESOURCE_UNSET) {
- pci_err(dev, "%s %pR: not assigned; can't enable device\n",
- r_name, r);
- return -EINVAL;
+ if (i < PCI_BRIDGE_RESOURCES) {
+ if (r->flags & IORESOURCE_UNSET) {
+ pci_err(dev, "%s %pR: not assigned; can't enable device\n",
+ r_name, r);
+ return -EINVAL;
+ }
+
+ if (!r->parent) {
+ pci_err(dev, "%s %pR: not claimed; can't enable device\n",
+ r_name, r);
+ return -EINVAL;
+ }
}
- if (!r->parent) {
- pci_err(dev, "%s %pR: not claimed; can't enable device\n",
- r_name, r);
- return -EINVAL;
+ if (r->parent) {
+ if (r->flags & IORESOURCE_IO)
+ cmd |= PCI_COMMAND_IO;
+ if (r->flags & IORESOURCE_MEM)
+ cmd |= PCI_COMMAND_MEMORY;
}
-
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
}
if (cmd != old_cmd) {
--
2.39.5
Powered by blists - more mailing lists