Why check for PCI_PROBE_ONLY in pci_common_init_dev()

Commit Message

Benjamin Herrenschmidt June 13, 2019, 3:52 a.m. UTC

Hi !

So while trying to sort out & cleanup the business with PCI resource
allocation (and do the ground work to be able to revive 
https://lore.kernel.org/linux-pci/20150916085850.GA17510@red-moon/) I
stumbled upon this one:

arch/arm/kernel/bios32.c:pci_common_init_dev() checks for
PCI_PROBE_ONLY to decide whether to claim existing resources or
reallocate.

However, I can't see any code path leading to that function that would
have set that flag.

IE. Afaik, PCI_PROBE_ONLY is only set in a limited number of places
(short of a grep failure on my part):

arch/alpha/kernel/sys_marvel.c:	pci_set_flags(PCI_PROBE_ONLY);
arch/alpha/kernel/sys_titan.c:	pci_set_flags(PCI_PROBE_ONLY);
arch/mips/pci/pci-bcm1480.c:	pci_set_flags(PCI_PROBE_ONLY);
arch/mips/pci/pci-sb1250.c:	pci_set_flags(PCI_PROBE_ONLY);
arch/mips/pci/pci-virtio-guest.c:	pci_set_flags(PCI_PROBE_ONLY);
arch/mips/pci/pci-xlp.c:	pci_set_flags(PCI_PROBE_ONLY);
arch/mips/pci/pci-xlr.c:	pci_set_flags(PCI_PROBE_ONLY);
arch/mips/pci/pci-xtalk-bridge.c:	pci_set_flags(PCI_PROBE_ONLY);
arch/powerpc/platforms/maple/pci.c:	pci_add_flags(PCI_PROBE_ONLY);
arch/powerpc/platforms/pseries/setup.c:	pci_add_flags(PCI_PROBE_ONLY);
drivers/pci/of.c:		pci_add_flags(PCI_PROBE_ONLY);

The only one being of interest to arm32 here being the last one in
of_pci_check_probe_only().

Now that function is only called in two places:

arch/powerpc/platforms/pseries/setup.c: of_pci_check_probe_only();
drivers/pci/controller/pci-host-common.c:       of_pci_check_probe_only();

The only interesting call site here being pci-host-common.c, which
corresponds to the "new style" platform device based PCI host bridge
probing. Now those use pci_host_probe() in drivers/pci/probe.c, not the 
(legacy ?) arch/arm/kernel/bios32.c variant.

So unless I missed something, should I take out the PCI_PROBE_ONLY
case completely in the arm32 code as part of my series ?

ie:

index ed46ca69813d..f969a1a56ace 100644

Cheers,
Ben.

Comments

Russell King (Oracle) June 13, 2019, 10:06 p.m. UTC | #1

On Thu, Jun 13, 2019 at 01:52:34PM +1000, Benjamin Herrenschmidt wrote:
> Hi !
> 
> So while trying to sort out & cleanup the business with PCI resource
> allocation (and do the ground work to be able to revive 
> https://lore.kernel.org/linux-pci/20150916085850.GA17510@red-moon/) I
> stumbled upon this one:
> 
> arch/arm/kernel/bios32.c:pci_common_init_dev() checks for
> PCI_PROBE_ONLY to decide whether to claim existing resources or
> reallocate.
> 
> However, I can't see any code path leading to that function that would
> have set that flag.

Someone probably wanted it at some point but either that's been removed
or the code was never merged.  Either way, if no one is using it on
32-bit ARM, it can be killed.

Benjamin Herrenschmidt June 13, 2019, 10:10 p.m. UTC | #2

On Thu, 2019-06-13 at 23:06 +0100, Russell King - ARM Linux admin wrote:
> On Thu, Jun 13, 2019 at 01:52:34PM +1000, Benjamin Herrenschmidt wrote:
> > Hi !
> > 
> > So while trying to sort out & cleanup the business with PCI resource
> > allocation (and do the ground work to be able to revive 
> > https://lore.kernel.org/linux-pci/20150916085850.GA17510@red-moon/) I
> > stumbled upon this one:
> > 
> > arch/arm/kernel/bios32.c:pci_common_init_dev() checks for
> > PCI_PROBE_ONLY to decide whether to claim existing resources or
> > reallocate.
> > 
> > However, I can't see any code path leading to that function that would
> > have set that flag.
> 
> Someone probably wanted it at some point but either that's been removed
> or the code was never merged.  Either way, if no one is using it on
> 32-bit ARM, it can be killed.

It's not used by code path using that function. "New style" stuff using
drivers/pci/controller/* can still use the flag, but at least I can
simplify that code.

Thanks !

Cheers,
Ben.

Patch

--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -536,23 +536,13 @@  void pci_common_init_dev(struct device *parent, struct hw_pci *hw)
 
 	list_for_each_entry(sys, &head, node) {
 		struct pci_bus *bus = sys->bus;
+		struct pci_bus *child;
 
-		/*
-		 * We insert PCI resources into the iomem_resource and
-		 * ioport_resource trees in either pci_bus_claim_resources()
-		 * or pci_bus_assign_resources().
-		 */
-		if (pci_has_flag(PCI_PROBE_ONLY)) {
-			pci_bus_claim_resources(bus);
-		} else {
-			struct pci_bus *child;
+		pci_bus_size_bridges(bus);
+		pci_bus_assign_resources(bus);
 
-			pci_bus_size_bridges(bus);
-			pci_bus_assign_resources(bus);
-
-			list_for_each_entry(child, &bus->children, node)
-				pcie_bus_configure_settings(child);
-		}
+		list_for_each_entry(child, &bus->children, node)
+			pcie_bus_configure_settings(child);
 
 		pci_bus_add_devices(bus);
 	}