[RFC/PATCH] of: Match PCI devices to OF nodes generically

[RFC/PATCH] of: Match PCI devices to OF nodes generically

[Benjamin Herrenschmidt]

powerpc has two different ways of matching PCI devices to their
corresponding OF node (if any) for historical reasons. The ppc64 one
does a scan looking for matching bus/dev/fn, while the ppc32 one does a
scan looking only for matching dev/fn on each level in order to be
agnostic to busses being renumbered (which Linux does on some
platforms).

This removes both and instead moves the matching code to the PCI core
itself. It's the most logical place to do it: when a pci_dev is created,
we know the parent and thus can do a single level scan for the matching
device_node (if any).

The benefit is that all archs now get the matching for free. There's one
hook the arch might want to provide to match a PHB bus to its device
node. A default weak implementation is provided that looks for the
parent device device node, but it's not entirely reliable on powerpc for
various reasons so powerpc provides its own.

Dave, I don't think I broke anything in sparc land but I haven't tested
(and have to admit haven't build-tested yet either).

Not-signed-off-yet. Tested on a few ppc's but not the whole range yet
either, mostly posted for comments.

 arch/powerpc/include/asm/pci-bridge.h |   31 +++++-----------------
 arch/powerpc/include/asm/pci.h        |    3 --
 arch/powerpc/include/asm/prom.h       |    1 
 arch/powerpc/kernel/pci-common.c      |   11 +++++--
 arch/powerpc/kernel/pci_32.c          |    6 ----
 arch/powerpc/kernel/pci_dn.c          |   47
----------------------------------
 arch/powerpc/kernel/pci_of_scan.c     |    9 ++----
 arch/sparc/kernel/pci.c               |    2 -
 drivers/pci/Makefile                  |    2 +
 drivers/pci/hotplug/rpadlpar_core.c   |    2 -
 drivers/pci/probe.c                   |    8 +++++
 include/linux/pci.h                   |   17 ++++++++++++
 12 files changed, 46 insertions(+), 93 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 5e156e0..a4c8e4a 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -169,18 +169,18 @@ static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
 	return bus->sysdata;
 }
 
-#ifndef CONFIG_PPC64
+static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
+{
+	return dev->dev.of_node;
+}
 
 static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 {
-	struct pci_controller *host;
-
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	host = pci_bus_to_host(bus);
-	return host ? host->dn : NULL;
+	return bus->dev.of_node;
 }
 
+#ifndef CONFIG_PPC64
+
 static inline int isa_vaddr_is_ioport(void __iomem *address)
 {
 	/* No specific ISA handling on ppc32 at this stage, it
@@ -223,17 +223,8 @@ struct pci_dn {
 /* Get the pointer to a device_node's pci_dn */
 #define PCI_DN(dn)	((struct pci_dn *) (dn)->data)
 
-extern struct device_node *fetch_dev_dn(struct pci_dev *dev);
 extern void * update_dn_pci_info(struct device_node *dn, void *data);
 
-/* Get a device_node from a pci_dev.  This code must be fast except
- * in the case where the sysdata is incorrect and needs to be fixed
- * up (this will only happen once). */
-static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
-}
-
 static inline int pci_device_from_OF_node(struct device_node *np,
 					  u8 *bus, u8 *devfn)
 {
@@ -244,14 +235,6 @@ static inline int pci_device_from_OF_node(struct device_node *np,
 	return 0;
 }
 
-static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
-{
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	else
-		return bus->dev.of_node; /* Must be root bus (PHB) */
-}
-
 /** Find the bus corresponding to the indicated device node */
 extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn);
 
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 7d77909..1f52268 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -179,8 +179,7 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
 extern struct pci_dev *of_create_pci_dev(struct device_node *node,
 					struct pci_bus *bus, int devfn);
 
-extern void of_scan_pci_bridge(struct device_node *node,
-				struct pci_dev *dev);
+extern void of_scan_pci_bridge(struct pci_dev *dev);
 
 extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
 extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index c189aa5..2b888cc 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -32,7 +32,6 @@ struct pci_dev;
 extern int pci_device_from_OF_node(struct device_node *node,
 				   u8* bus, u8* devfn);
 extern struct device_node* pci_busdev_to_OF_node(struct pci_bus *, int);
-extern struct device_node* pci_device_to_OF_node(struct pci_dev *);
 extern void pci_create_OF_bus_map(void);
 #endif
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 893af2a..47c516b 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1097,9 +1097,6 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
 		if (dev->is_added)
 			continue;
 
-		/* Setup OF node pointer in the device */
-		dev->dev.of_node = pci_device_to_OF_node(dev);
-
 		/* Fixup NUMA node as it may not be setup yet by the generic
 		 * code and is needed by the DMA init
 		 */
@@ -1685,6 +1682,13 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
 	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
 }
 
+struct device_node * pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	struct pci_controller *hose = bus->sysdata;
+
+	return of_node_get(hose->dn);
+}
+
 /**
  * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
  * @hose: Pointer to the PCI host controller instance structure
@@ -1705,7 +1709,6 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 			hose->global_number);
 		return;
 	}
-	bus->dev.of_node = of_node_get(node);
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index bedb370..700a826 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -276,12 +276,6 @@ pci_busdev_to_OF_node(struct pci_bus *bus, int devfn)
 }
 EXPORT_SYMBOL(pci_busdev_to_OF_node);
 
-struct device_node*
-pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return pci_busdev_to_OF_node(dev->bus, dev->devfn);
-}
-EXPORT_SYMBOL(pci_device_to_OF_node);
 
 static int
 find_OF_pci_device_filter(struct device_node* node, void* data)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index d225d99..8cb66a2 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -143,53 +143,6 @@ void __devinit pci_devs_phb_init_dynamic(struct pci_controller *phb)
 	traverse_pci_devices(dn, update_dn_pci_info, phb);
 }
 
-/*
- * Traversal func that looks for a <busno,devfcn> value.
- * If found, the pci_dn is returned (thus terminating the traversal).
- */
-static void *is_devfn_node(struct device_node *dn, void *data)
-{
-	int busno = ((unsigned long)data >> 8) & 0xff;
-	int devfn = ((unsigned long)data) & 0xff;
-	struct pci_dn *pci = dn->data;
-
-	if (pci && (devfn == pci->devfn) && (busno == pci->busno))
-		return dn;
-	return NULL;
-}
-
-/*
- * This is the "slow" path for looking up a device_node from a
- * pci_dev.  It will hunt for the device under its parent's
- * phb and then update of_node pointer.
- *
- * It may also do fixups on the actual device since this happens
- * on the first read/write.
- *
- * Note that it also must deal with devices that don't exist.
- * In this case it may probe for real hardware ("just in case")
- * and add a device_node to the device tree if necessary.
- *
- * Is this function necessary anymore now that dev->dev.of_node is
- * used to store the node pointer?
- *
- */
-struct device_node *fetch_dev_dn(struct pci_dev *dev)
-{
-	struct pci_controller *phb = dev->sysdata;
-	struct device_node *dn;
-	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
-
-	if (WARN_ON(!phb))
-		return NULL;
-
-	dn = traverse_pci_devices(phb->dn, is_devfn_node, (void *)searchval);
-	if (dn)
-		dev->dev.of_node = dn;
-	return dn;
-}
-EXPORT_SYMBOL(fetch_dev_dn);
-
 /** 
  * pci_devs_phb_init - Initialize phbs and pci devs under them.
  * 
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 1e89a72..fe0a5ad 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -202,9 +202,9 @@ EXPORT_SYMBOL(of_create_pci_dev);
  * this routine in turn call of_scan_bus() recusively to scan for more child
  * devices.
  */
-void __devinit of_scan_pci_bridge(struct device_node *node,
-				  struct pci_dev *dev)
+void __devinit of_scan_pci_bridge(struct pci_dev *dev)
 {
+	struct device_node *node = dev->dev.of_node;
 	struct pci_bus *bus;
 	const u32 *busrange, *ranges;
 	int len, i, mode;
@@ -238,7 +238,6 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
 	bus->primary = dev->bus->number;
 	bus->subordinate = busrange[1];
 	bus->bridge_ctl = 0;
-	bus->dev.of_node = of_node_get(node);
 
 	/* parse ranges property */
 	/* PCI #address-cells == 3 and #size-cells == 2 always */
@@ -335,9 +334,7 @@ static void __devinit __of_scan_bus(struct device_node *node,
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
-			struct device_node *child = pci_device_to_OF_node(dev);
-			if (child)
-				of_scan_pci_bridge(child, dev);
+			of_scan_pci_bridge(dev);
 		}
 	}
 }
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 713dc91..e539d23 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -284,7 +284,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 	dev->sysdata = node;
 	dev->dev.parent = bus->bridge;
 	dev->dev.bus = &pci_bus_type;
-	dev->dev.of_node = node;
+	dev->dev.of_node = of_node_get(node);
 	dev->devfn = devfn;
 	dev->multifunction = 0;		/* maybe a lie? */
 	set_pcie_port_type(dev);
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 98d61c8..d5c3cb9 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -70,4 +70,6 @@ obj-$(CONFIG_PCI_STUB) += pci-stub.o
 
 obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
 
+obj-$(CONFIG_OF) += of.o
+
 ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index 0830347..1d002b1 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -158,7 +158,7 @@ static void dlpar_pci_add_bus(struct device_node *dn)
 	/* Scan below the new bridge */
 	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 	    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
-		of_scan_pci_bridge(dn, dev);
+		of_scan_pci_bridge(dev);
 
 	/* Map IO space for child bus, which may or may not succeed */
 	pcibios_map_io_space(dev->subordinate);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 44cbbba..347349b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -89,6 +89,7 @@ static void release_pcibus_dev(struct device *dev)
 	if (pci_bus->bridge)
 		put_device(pci_bus->bridge);
 	pci_bus_remove_resources(pci_bus);
+	pci_release_bus_of_node(pci_bus);
 	kfree(pci_bus);
 }
 
@@ -624,7 +625,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 
 	child->self = bridge;
 	child->bridge = get_device(&bridge->dev);
-
+	pci_set_bus_of_node(child);	
 	pci_set_bus_speed(child);
 
 	/* Set up default resource pointers and names.. */
@@ -1074,6 +1075,7 @@ static void pci_release_dev(struct device *dev)
 
 	pci_dev = to_pci_dev(dev);
 	pci_release_capabilities(pci_dev);
+	pci_release_of_node(pci_dev);
 	kfree(pci_dev);
 }
 
@@ -1150,6 +1152,7 @@ struct pci_dev *alloc_pci_dev(void)
 }
 EXPORT_SYMBOL(alloc_pci_dev);
 
+
 /*
  * Read the config data for a PCI device, sanity-check it
  * and fill in the dev structure...
@@ -1193,6 +1196,8 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	dev->vendor = l & 0xffff;
 	dev->device = (l >> 16) & 0xffff;
 
+	pci_set_of_node(dev);
+
 	if (pci_setup_device(dev)) {
 		kfree(dev);
 		return NULL;
@@ -1445,6 +1450,7 @@ struct pci_bus * pci_create_bus(struct device *parent,
 		goto dev_reg_err;
 	b->bridge = get_device(dev);
 	device_enable_async_suspend(b->bridge);
+	pci_set_bus_of_node(b);
 
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 96f70d7..e5111da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1543,5 +1543,22 @@ int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt);
 int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off,
 			      unsigned int len, const char *kw);
 
+/* PCI <-> OF binding helpers */
+#ifdef  CONFIG_OF
+#include <linux/of.h>
+extern void pci_set_of_node(struct pci_dev *dev);
+extern void pci_release_of_node(struct pci_dev *dev);
+extern void pci_set_bus_of_node(struct pci_bus *bus);
+extern void pci_release_bus_of_node(struct pci_bus *bus);
+
+/* Arch may override this (weak) */
+extern struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus);
+
+
+#else /* CONFIG_OF */
+static void pci_locate_of_node(struct pci_dev *dev) { }
+static void pci_release_of_node(struct pci_dev *dev) { }
+#endif  /* CONFIG_OF */
+
 #endif /* __KERNEL__ */
 #endif /* LINUX_PCI_H */

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[Benjamin Herrenschmidt]

On Mon, 2011-04-04 at 12:04 +1000, Benjamin Herrenschmidt wrote:
> powerpc has two different ways of matching PCI devices to their
> corresponding OF node (if any) for historical reasons. The ppc64 one
> does a scan looking for matching bus/dev/fn, while the ppc32 one does a
> scan looking only for matching dev/fn on each level in order to be
> agnostic to busses being renumbered (which Linux does on some
> platforms).
> 
> This removes both and instead moves the matching code to the PCI core
> itself. It's the most logical place to do it: when a pci_dev is created,
> we know the parent and thus can do a single level scan for the matching
> device_node (if any).
> 
> The benefit is that all archs now get the matching for free. There's one
> hook the arch might want to provide to match a PHB bus to its device
> node. A default weak implementation is provided that looks for the
> parent device device node, but it's not entirely reliable on powerpc for
> various reasons so powerpc provides its own.
> 
> Dave, I don't think I broke anything in sparc land but I haven't tested
> (and have to admit haven't build-tested yet either).
> 
> Not-signed-off-yet. Tested on a few ppc's but not the whole range yet
> either, mostly posted for comments.

Nice, looks like I forgot to add the new drivers/pci/of.c file :-)
Here's a new patch. Also added linux-pci to the CC list.

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 5e156e0..a4c8e4a 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -169,18 +169,18 @@ static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
 	return bus->sysdata;
 }
 
-#ifndef CONFIG_PPC64
+static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
+{
+	return dev->dev.of_node;
+}
 
 static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 {
-	struct pci_controller *host;
-
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	host = pci_bus_to_host(bus);
-	return host ? host->dn : NULL;
+	return bus->dev.of_node;
 }
 
+#ifndef CONFIG_PPC64
+
 static inline int isa_vaddr_is_ioport(void __iomem *address)
 {
 	/* No specific ISA handling on ppc32 at this stage, it
@@ -223,17 +223,8 @@ struct pci_dn {
 /* Get the pointer to a device_node's pci_dn */
 #define PCI_DN(dn)	((struct pci_dn *) (dn)->data)
 
-extern struct device_node *fetch_dev_dn(struct pci_dev *dev);
 extern void * update_dn_pci_info(struct device_node *dn, void *data);
 
-/* Get a device_node from a pci_dev.  This code must be fast except
- * in the case where the sysdata is incorrect and needs to be fixed
- * up (this will only happen once). */
-static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
-}
-
 static inline int pci_device_from_OF_node(struct device_node *np,
 					  u8 *bus, u8 *devfn)
 {
@@ -244,14 +235,6 @@ static inline int pci_device_from_OF_node(struct device_node *np,
 	return 0;
 }
 
-static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
-{
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	else
-		return bus->dev.of_node; /* Must be root bus (PHB) */
-}
-
 /** Find the bus corresponding to the indicated device node */
 extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn);
 
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 7d77909..1f52268 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -179,8 +179,7 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
 extern struct pci_dev *of_create_pci_dev(struct device_node *node,
 					struct pci_bus *bus, int devfn);
 
-extern void of_scan_pci_bridge(struct device_node *node,
-				struct pci_dev *dev);
+extern void of_scan_pci_bridge(struct pci_dev *dev);
 
 extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
 extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index c189aa5..2b888cc 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -32,7 +32,6 @@ struct pci_dev;
 extern int pci_device_from_OF_node(struct device_node *node,
 				   u8* bus, u8* devfn);
 extern struct device_node* pci_busdev_to_OF_node(struct pci_bus *, int);
-extern struct device_node* pci_device_to_OF_node(struct pci_dev *);
 extern void pci_create_OF_bus_map(void);
 #endif
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 893af2a..47c516b 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1097,9 +1097,6 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
 		if (dev->is_added)
 			continue;
 
-		/* Setup OF node pointer in the device */
-		dev->dev.of_node = pci_device_to_OF_node(dev);
-
 		/* Fixup NUMA node as it may not be setup yet by the generic
 		 * code and is needed by the DMA init
 		 */
@@ -1685,6 +1682,13 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
 	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
 }
 
+struct device_node * pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	struct pci_controller *hose = bus->sysdata;
+
+	return of_node_get(hose->dn);
+}
+
 /**
  * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
  * @hose: Pointer to the PCI host controller instance structure
@@ -1705,7 +1709,6 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 			hose->global_number);
 		return;
 	}
-	bus->dev.of_node = of_node_get(node);
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index bedb370..700a826 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -276,12 +276,6 @@ pci_busdev_to_OF_node(struct pci_bus *bus, int devfn)
 }
 EXPORT_SYMBOL(pci_busdev_to_OF_node);
 
-struct device_node*
-pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return pci_busdev_to_OF_node(dev->bus, dev->devfn);
-}
-EXPORT_SYMBOL(pci_device_to_OF_node);
 
 static int
 find_OF_pci_device_filter(struct device_node* node, void* data)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index d225d99..8cb66a2 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -143,53 +143,6 @@ void __devinit pci_devs_phb_init_dynamic(struct pci_controller *phb)
 	traverse_pci_devices(dn, update_dn_pci_info, phb);
 }
 
-/*
- * Traversal func that looks for a <busno,devfcn> value.
- * If found, the pci_dn is returned (thus terminating the traversal).
- */
-static void *is_devfn_node(struct device_node *dn, void *data)
-{
-	int busno = ((unsigned long)data >> 8) & 0xff;
-	int devfn = ((unsigned long)data) & 0xff;
-	struct pci_dn *pci = dn->data;
-
-	if (pci && (devfn == pci->devfn) && (busno == pci->busno))
-		return dn;
-	return NULL;
-}
-
-/*
- * This is the "slow" path for looking up a device_node from a
- * pci_dev.  It will hunt for the device under its parent's
- * phb and then update of_node pointer.
- *
- * It may also do fixups on the actual device since this happens
- * on the first read/write.
- *
- * Note that it also must deal with devices that don't exist.
- * In this case it may probe for real hardware ("just in case")
- * and add a device_node to the device tree if necessary.
- *
- * Is this function necessary anymore now that dev->dev.of_node is
- * used to store the node pointer?
- *
- */
-struct device_node *fetch_dev_dn(struct pci_dev *dev)
-{
-	struct pci_controller *phb = dev->sysdata;
-	struct device_node *dn;
-	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
-
-	if (WARN_ON(!phb))
-		return NULL;
-
-	dn = traverse_pci_devices(phb->dn, is_devfn_node, (void *)searchval);
-	if (dn)
-		dev->dev.of_node = dn;
-	return dn;
-}
-EXPORT_SYMBOL(fetch_dev_dn);
-
 /** 
  * pci_devs_phb_init - Initialize phbs and pci devs under them.
  * 
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 1e89a72..fe0a5ad 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -202,9 +202,9 @@ EXPORT_SYMBOL(of_create_pci_dev);
  * this routine in turn call of_scan_bus() recusively to scan for more child
  * devices.
  */
-void __devinit of_scan_pci_bridge(struct device_node *node,
-				  struct pci_dev *dev)
+void __devinit of_scan_pci_bridge(struct pci_dev *dev)
 {
+	struct device_node *node = dev->dev.of_node;
 	struct pci_bus *bus;
 	const u32 *busrange, *ranges;
 	int len, i, mode;
@@ -238,7 +238,6 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
 	bus->primary = dev->bus->number;
 	bus->subordinate = busrange[1];
 	bus->bridge_ctl = 0;
-	bus->dev.of_node = of_node_get(node);
 
 	/* parse ranges property */
 	/* PCI #address-cells == 3 and #size-cells == 2 always */
@@ -335,9 +334,7 @@ static void __devinit __of_scan_bus(struct device_node *node,
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
-			struct device_node *child = pci_device_to_OF_node(dev);
-			if (child)
-				of_scan_pci_bridge(child, dev);
+			of_scan_pci_bridge(dev);
 		}
 	}
 }
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 713dc91..e539d23 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -284,7 +284,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 	dev->sysdata = node;
 	dev->dev.parent = bus->bridge;
 	dev->dev.bus = &pci_bus_type;
-	dev->dev.of_node = node;
+	dev->dev.of_node = of_node_get(node);
 	dev->devfn = devfn;
 	dev->multifunction = 0;		/* maybe a lie? */
 	set_pcie_port_type(dev);
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 98d61c8..d5c3cb9 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -70,4 +70,6 @@ obj-$(CONFIG_PCI_STUB) += pci-stub.o
 
 obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
 
+obj-$(CONFIG_OF) += of.o
+
 ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index 0830347..1d002b1 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -158,7 +158,7 @@ static void dlpar_pci_add_bus(struct device_node *dn)
 	/* Scan below the new bridge */
 	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 	    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
-		of_scan_pci_bridge(dn, dev);
+		of_scan_pci_bridge(dev);
 
 	/* Map IO space for child bus, which may or may not succeed */
 	pcibios_map_io_space(dev->subordinate);
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
new file mode 100644
index 0000000..306d852
--- /dev/null
+++ b/drivers/pci/of.c
@@ -0,0 +1,87 @@
+/*
+ * PCI <-> OF mapping helpers
+ *
+ * Copyright 2011 IBM Corp.
+ * 
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include "pci.h"
+
+static int pci_check_one_of_node(struct pci_dev *dev, struct device_node *node)
+{
+	unsigned int size;
+	const __be32 *reg = of_get_property(node, "reg", &size);
+
+	if (reg && size >= 4 && ((reg[0] >> 8) & 0xff) == dev->devfn) {
+		dev->dev.of_node = of_node_get(node);
+		return 1;
+	}
+	return 0;
+}
+
+void pci_set_of_node(struct pci_dev *dev)
+{
+	struct device_node *parent, *node, *node2;
+
+	parent = dev->bus->dev.of_node;
+	if (!parent)
+		return;
+	for_each_child_of_node(parent, node) {
+		if (pci_check_one_of_node(dev, node))
+			return;
+		/*
+		 * Some OFs create a parent node "multifunc-device" as
+		 * a fake root for all functions of a multi-function
+		 * device we go down them as well.
+		 */
+                if (!strcmp(node->name, "multifunc-device")) {
+			for_each_child_of_node(node, node2) {
+				if (pci_check_one_of_node(dev, node2))
+					return;
+			}
+                }
+	}
+}
+
+void pci_release_of_node(struct pci_dev *dev)
+{
+	of_node_put(dev->dev.of_node);
+	dev->dev.of_node = NULL;
+}
+
+void pci_set_bus_of_node(struct pci_bus *bus)
+{
+	if (bus->self == NULL)
+		bus->dev.of_node = pcibios_get_phb_of_node(bus);
+	else
+		bus->dev.of_node = of_node_get(bus->self->dev.of_node);
+}
+
+void pci_release_bus_of_node(struct pci_bus *bus)
+{
+	of_node_put(bus->dev.of_node);
+	bus->dev.of_node = NULL;
+}
+
+struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	/* This should only be called for PHBs */
+	if (WARN_ON(bus->self || bus->parent))
+		return NULL;
+
+	/* Look for a node pointer in either the intermediary device we
+	 * create above the root bus or it's own parent. Normally only
+	 * the later is populated.
+	 */
+	if (bus->bridge->of_node)
+		return of_node_get(bus->bridge->of_node);
+	if (bus->bridge->parent->of_node)
+		return of_node_get(bus->bridge->parent->of_node);
+	return NULL;
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 44cbbba..347349b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -89,6 +89,7 @@ static void release_pcibus_dev(struct device *dev)
 	if (pci_bus->bridge)
 		put_device(pci_bus->bridge);
 	pci_bus_remove_resources(pci_bus);
+	pci_release_bus_of_node(pci_bus);
 	kfree(pci_bus);
 }
 
@@ -624,7 +625,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 
 	child->self = bridge;
 	child->bridge = get_device(&bridge->dev);
-
+	pci_set_bus_of_node(child);	
 	pci_set_bus_speed(child);
 
 	/* Set up default resource pointers and names.. */
@@ -1074,6 +1075,7 @@ static void pci_release_dev(struct device *dev)
 
 	pci_dev = to_pci_dev(dev);
 	pci_release_capabilities(pci_dev);
+	pci_release_of_node(pci_dev);
 	kfree(pci_dev);
 }
 
@@ -1150,6 +1152,7 @@ struct pci_dev *alloc_pci_dev(void)
 }
 EXPORT_SYMBOL(alloc_pci_dev);
 
+
 /*
  * Read the config data for a PCI device, sanity-check it
  * and fill in the dev structure...
@@ -1193,6 +1196,8 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	dev->vendor = l & 0xffff;
 	dev->device = (l >> 16) & 0xffff;
 
+	pci_set_of_node(dev);
+
 	if (pci_setup_device(dev)) {
 		kfree(dev);
 		return NULL;
@@ -1445,6 +1450,7 @@ struct pci_bus * pci_create_bus(struct device *parent,
 		goto dev_reg_err;
 	b->bridge = get_device(dev);
 	device_enable_async_suspend(b->bridge);
+	pci_set_bus_of_node(b);
 
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 96f70d7..e5111da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1543,5 +1543,22 @@ int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt);
 int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off,
 			      unsigned int len, const char *kw);
 
+/* PCI <-> OF binding helpers */
+#ifdef  CONFIG_OF
+#include <linux/of.h>
+extern void pci_set_of_node(struct pci_dev *dev);
+extern void pci_release_of_node(struct pci_dev *dev);
+extern void pci_set_bus_of_node(struct pci_bus *bus);
+extern void pci_release_bus_of_node(struct pci_bus *bus);
+
+/* Arch may override this (weak) */
+extern struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus);
+
+
+#else /* CONFIG_OF */
+static void pci_locate_of_node(struct pci_dev *dev) { }
+static void pci_release_of_node(struct pci_dev *dev) { }
+#endif  /* CONFIG_OF */
+
 #endif /* __KERNEL__ */
 #endif /* LINUX_PCI_H */

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[Benjamin Herrenschmidt]

> Nice, looks like I forgot to add the new drivers/pci/of.c file :-)
> Here's a new patch. Also added linux-pci to the CC list.

And this one removes a lot more cruft from the powermac code while at
it, and moves the core matching logic to drivers/of/of_pci.c...

>From 917ea61d6afcf443ca467d0a6ffa00d5c6e21bb3 Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Mon, 4 Apr 2011 14:38:13 +1000
Subject: [PATCH] of: Match PCI devices to OF nodes dynamically

powerpc has two different ways of matching PCI devices to their
corresponding OF node (if any) for historical reasons. The ppc64 one
does a scan looking for matching bus/dev/fn, while the ppc32 one does a
scan looking only for matching dev/fn on each level in order to be
agnostic to busses being renumbered (which Linux does on some
platforms).

This removes both and instead moves the matching code to the PCI core
itself. It's the most logical place to do it: when a pci_dev is created,
we know the parent and thus can do a single level scan for the matching
device_node (if any).

The benefit is that all archs now get the matching for free. There's one
hook the arch might want to provide to match a PHB bus to its device
node. A default weak implementation is provided that looks for the
parent device device node, but it's not entirely reliable on powerpc for
various reasons so powerpc provides its own.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/include/asm/pci-bridge.h |   35 +++------
 arch/powerpc/include/asm/pci.h        |    3 +-
 arch/powerpc/include/asm/prom.h       |   14 ---
 arch/powerpc/kernel/pci-common.c      |   11 ++-
 arch/powerpc/kernel/pci_32.c          |  148 +++------------------------------
 arch/powerpc/kernel/pci_dn.c          |   47 -----------
 arch/powerpc/kernel/pci_of_scan.c     |    9 +--
 arch/powerpc/platforms/powermac/pci.c |    3 +-
 arch/sparc/kernel/pci.c               |    2 +-
 drivers/of/of_pci.c                   |   36 ++++++++
 drivers/pci/Makefile                  |    2 +
 drivers/pci/hotplug/rpadlpar_core.c   |    2 +-
 drivers/pci/of.c                      |   60 +++++++++++++
 drivers/pci/probe.c                   |    8 ++-
 include/linux/of_pci.h                |    5 +
 include/linux/pci.h                   |   17 ++++
 16 files changed, 165 insertions(+), 237 deletions(-)
 create mode 100644 drivers/pci/of.c

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 5e156e0..6912c45 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -169,18 +169,22 @@ static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
 	return bus->sysdata;
 }
 
-#ifndef CONFIG_PPC64
+static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
+{
+	return dev->dev.of_node;
+}
 
 static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 {
-	struct pci_controller *host;
-
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	host = pci_bus_to_host(bus);
-	return host ? host->dn : NULL;
+	return bus->dev.of_node;
 }
 
+#ifndef CONFIG_PPC64
+
+extern int pci_device_from_OF_node(struct device_node *node,
+				   u8* bus, u8* devfn);
+extern void pci_create_OF_bus_map(void);
+
 static inline int isa_vaddr_is_ioport(void __iomem *address)
 {
 	/* No specific ISA handling on ppc32 at this stage, it
@@ -223,17 +227,8 @@ struct pci_dn {
 /* Get the pointer to a device_node's pci_dn */
 #define PCI_DN(dn)	((struct pci_dn *) (dn)->data)
 
-extern struct device_node *fetch_dev_dn(struct pci_dev *dev);
 extern void * update_dn_pci_info(struct device_node *dn, void *data);
 
-/* Get a device_node from a pci_dev.  This code must be fast except
- * in the case where the sysdata is incorrect and needs to be fixed
- * up (this will only happen once). */
-static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
-}
-
 static inline int pci_device_from_OF_node(struct device_node *np,
 					  u8 *bus, u8 *devfn)
 {
@@ -244,14 +239,6 @@ static inline int pci_device_from_OF_node(struct device_node *np,
 	return 0;
 }
 
-static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
-{
-	if (bus->self)
-		return pci_device_to_OF_node(bus->self);
-	else
-		return bus->dev.of_node; /* Must be root bus (PHB) */
-}
-
 /** Find the bus corresponding to the indicated device node */
 extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn);
 
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 7d77909..1f52268 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -179,8 +179,7 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
 extern struct pci_dev *of_create_pci_dev(struct device_node *node,
 					struct pci_bus *bus, int devfn);
 
-extern void of_scan_pci_bridge(struct device_node *node,
-				struct pci_dev *dev);
+extern void of_scan_pci_bridge(struct pci_dev *dev);
 
 extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
 extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index c189aa5..b823536 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -22,20 +22,6 @@
 
 #define HAVE_ARCH_DEVTREE_FIXUPS
 
-#ifdef CONFIG_PPC32
-/*
- * PCI <-> OF matching functions
- * (XXX should these be here?)
- */
-struct pci_bus;
-struct pci_dev;
-extern int pci_device_from_OF_node(struct device_node *node,
-				   u8* bus, u8* devfn);
-extern struct device_node* pci_busdev_to_OF_node(struct pci_bus *, int);
-extern struct device_node* pci_device_to_OF_node(struct pci_dev *);
-extern void pci_create_OF_bus_map(void);
-#endif
-
 /*
  * OF address retreival & translation
  */
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 893af2a..47c516b 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1097,9 +1097,6 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
 		if (dev->is_added)
 			continue;
 
-		/* Setup OF node pointer in the device */
-		dev->dev.of_node = pci_device_to_OF_node(dev);
-
 		/* Fixup NUMA node as it may not be setup yet by the generic
 		 * code and is needed by the DMA init
 		 */
@@ -1685,6 +1682,13 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
 	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
 }
 
+struct device_node * pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	struct pci_controller *hose = bus->sysdata;
+
+	return of_node_get(hose->dn);
+}
+
 /**
  * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
  * @hose: Pointer to the PCI host controller instance structure
@@ -1705,7 +1709,6 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 			hose->global_number);
 		return;
 	}
-	bus->dev.of_node = of_node_get(node);
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index bedb370..ca6bcb7 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -167,150 +167,26 @@ pcibios_make_OF_bus_map(void)
 #endif
 }
 
-typedef int (*pci_OF_scan_iterator)(struct device_node* node, void* data);
-
-static struct device_node*
-scan_OF_pci_childs(struct device_node *parent, pci_OF_scan_iterator filter, void* data)
-{
-	struct device_node *node;
-	struct device_node* sub_node;
-
-	for_each_child_of_node(parent, node) {
-		const unsigned int *class_code;
-	
-		if (filter(node, data)) {
-			of_node_put(node);
-			return node;
-		}
-
-		/* For PCI<->PCI bridges or CardBus bridges, we go down
-		 * Note: some OFs create a parent node "multifunc-device" as
-		 * a fake root for all functions of a multi-function device,
-		 * we go down them as well.
-		 */
-		class_code = of_get_property(node, "class-code", NULL);
-		if ((!class_code || ((*class_code >> 8) != PCI_CLASS_BRIDGE_PCI &&
-			(*class_code >> 8) != PCI_CLASS_BRIDGE_CARDBUS)) &&
-			strcmp(node->name, "multifunc-device"))
-			continue;
-		sub_node = scan_OF_pci_childs(node, filter, data);
-		if (sub_node) {
-			of_node_put(node);
-			return sub_node;
-		}
-	}
-	return NULL;
-}
-
-static struct device_node *scan_OF_for_pci_dev(struct device_node *parent,
-					       unsigned int devfn)
-{
-	struct device_node *np, *cnp;
-	const u32 *reg;
-	unsigned int psize;
-
-	for_each_child_of_node(parent, np) {
-		reg = of_get_property(np, "reg", &psize);
-                if (reg && psize >= 4 && ((reg[0] >> 8) & 0xff) == devfn)
-			return np;
-
-		/* Note: some OFs create a parent node "multifunc-device" as
-		 * a fake root for all functions of a multi-function device,
-		 * we go down them as well. */
-                if (!strcmp(np->name, "multifunc-device")) {
-                        cnp = scan_OF_for_pci_dev(np, devfn);
-                        if (cnp)
-                                return cnp;
-                }
-	}
-	return NULL;
-}
-
-
-static struct device_node *scan_OF_for_pci_bus(struct pci_bus *bus)
-{
-	struct device_node *parent, *np;
-
-	/* Are we a root bus ? */
-	if (bus->self == NULL || bus->parent == NULL) {
-		struct pci_controller *hose = pci_bus_to_host(bus);
-		if (hose == NULL)
-			return NULL;
-		return of_node_get(hose->dn);
-	}
-
-	/* not a root bus, we need to get our parent */
-	parent = scan_OF_for_pci_bus(bus->parent);
-	if (parent == NULL)
-		return NULL;
-
-	/* now iterate for children for a match */
-	np = scan_OF_for_pci_dev(parent, bus->self->devfn);
-	of_node_put(parent);
-
-	return np;
-}
-
-/*
- * Scans the OF tree for a device node matching a PCI device
- */
-struct device_node *
-pci_busdev_to_OF_node(struct pci_bus *bus, int devfn)
-{
-	struct device_node *parent, *np;
-
-	pr_debug("pci_busdev_to_OF_node(%d,0x%x)\n", bus->number, devfn);
-	parent = scan_OF_for_pci_bus(bus);
-	if (parent == NULL)
-		return NULL;
-	pr_debug(" parent is %s\n", parent ? parent->full_name : "<NULL>");
-	np = scan_OF_for_pci_dev(parent, devfn);
-	of_node_put(parent);
-	pr_debug(" result is %s\n", np ? np->full_name : "<NULL>");
-
-	/* XXX most callers don't release the returned node
-	 * mostly because ppc64 doesn't increase the refcount,
-	 * we need to fix that.
-	 */
-	return np;
-}
-EXPORT_SYMBOL(pci_busdev_to_OF_node);
-
-struct device_node*
-pci_device_to_OF_node(struct pci_dev *dev)
-{
-	return pci_busdev_to_OF_node(dev->bus, dev->devfn);
-}
-EXPORT_SYMBOL(pci_device_to_OF_node);
-
-static int
-find_OF_pci_device_filter(struct device_node* node, void* data)
-{
-	return ((void *)node == data);
-}
 
 /*
  * Returns the PCI device matching a given OF node
  */
-int
-pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
+int pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
 {
-	const unsigned int *reg;
-	struct pci_controller* hose;
 	struct pci_dev* dev = NULL;
-	
-	/* Make sure it's really a PCI device */
-	hose = pci_find_hose_for_OF_device(node);
-	if (!hose || !hose->dn)
-		return -ENODEV;
-	if (!scan_OF_pci_childs(hose->dn,
-			find_OF_pci_device_filter, (void *)node))
+	const __be32 *reg;
+	int size;
+
+	/* Check if it might have a chance to be a PCI device */
+	if (!pci_find_hose_for_OF_device(node))
 		return -ENODEV;
-	reg = of_get_property(node, "reg", NULL);
-	if (!reg)
+
+	reg = of_get_property(node, "reg", &size);
+	if (!reg || size < 5 * sizeof(u32))
 		return -ENODEV;
-	*bus = (reg[0] >> 16) & 0xff;
-	*devfn = ((reg[0] >> 8) & 0xff);
+	
+	*bus = (be32_to_cpup(&reg[0]) >> 16) & 0xff;
+	*devfn = (be32_to_cpup(&reg[0]) >> 8) & 0xff;
 
 	/* Ok, here we need some tweak. If we have already renumbered
 	 * all busses, we can't rely on the OF bus number any more.
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index d225d99..8cb66a2 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -143,53 +143,6 @@ void __devinit pci_devs_phb_init_dynamic(struct pci_controller *phb)
 	traverse_pci_devices(dn, update_dn_pci_info, phb);
 }
 
-/*
- * Traversal func that looks for a <busno,devfcn> value.
- * If found, the pci_dn is returned (thus terminating the traversal).
- */
-static void *is_devfn_node(struct device_node *dn, void *data)
-{
-	int busno = ((unsigned long)data >> 8) & 0xff;
-	int devfn = ((unsigned long)data) & 0xff;
-	struct pci_dn *pci = dn->data;
-
-	if (pci && (devfn == pci->devfn) && (busno == pci->busno))
-		return dn;
-	return NULL;
-}
-
-/*
- * This is the "slow" path for looking up a device_node from a
- * pci_dev.  It will hunt for the device under its parent's
- * phb and then update of_node pointer.
- *
- * It may also do fixups on the actual device since this happens
- * on the first read/write.
- *
- * Note that it also must deal with devices that don't exist.
- * In this case it may probe for real hardware ("just in case")
- * and add a device_node to the device tree if necessary.
- *
- * Is this function necessary anymore now that dev->dev.of_node is
- * used to store the node pointer?
- *
- */
-struct device_node *fetch_dev_dn(struct pci_dev *dev)
-{
-	struct pci_controller *phb = dev->sysdata;
-	struct device_node *dn;
-	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
-
-	if (WARN_ON(!phb))
-		return NULL;
-
-	dn = traverse_pci_devices(phb->dn, is_devfn_node, (void *)searchval);
-	if (dn)
-		dev->dev.of_node = dn;
-	return dn;
-}
-EXPORT_SYMBOL(fetch_dev_dn);
-
 /** 
  * pci_devs_phb_init - Initialize phbs and pci devs under them.
  * 
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 1e89a72..fe0a5ad 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -202,9 +202,9 @@ EXPORT_SYMBOL(of_create_pci_dev);
  * this routine in turn call of_scan_bus() recusively to scan for more child
  * devices.
  */
-void __devinit of_scan_pci_bridge(struct device_node *node,
-				  struct pci_dev *dev)
+void __devinit of_scan_pci_bridge(struct pci_dev *dev)
 {
+	struct device_node *node = dev->dev.of_node;
 	struct pci_bus *bus;
 	const u32 *busrange, *ranges;
 	int len, i, mode;
@@ -238,7 +238,6 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
 	bus->primary = dev->bus->number;
 	bus->subordinate = busrange[1];
 	bus->bridge_ctl = 0;
-	bus->dev.of_node = of_node_get(node);
 
 	/* parse ranges property */
 	/* PCI #address-cells == 3 and #size-cells == 2 always */
@@ -335,9 +334,7 @@ static void __devinit __of_scan_bus(struct device_node *node,
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
-			struct device_node *child = pci_device_to_OF_node(dev);
-			if (child)
-				of_scan_pci_bridge(child, dev);
+			of_scan_pci_bridge(dev);
 		}
 	}
 }
diff --git a/arch/powerpc/platforms/powermac/pci.c b/arch/powerpc/platforms/powermac/pci.c
index ab68989..68a18da 100644
--- a/arch/powerpc/platforms/powermac/pci.c
+++ b/arch/powerpc/platforms/powermac/pci.c
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/bootmem.h>
 #include <linux/irq.h>
+#include <linux/of_pci.h>
 
 #include <asm/sections.h>
 #include <asm/io.h>
@@ -235,7 +236,7 @@ static int chaos_validate_dev(struct pci_bus *bus, int devfn, int offset)
 
 	if (offset >= 0x100)
 		return  PCIBIOS_BAD_REGISTER_NUMBER;
-	np = pci_busdev_to_OF_node(bus, devfn);
+	np = of_pci_find_child_device(bus->dev.of_node, devfn);
 	if (np == NULL)
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 713dc91..e539d23 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -284,7 +284,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 	dev->sysdata = node;
 	dev->dev.parent = bus->bridge;
 	dev->dev.bus = &pci_bus_type;
-	dev->dev.of_node = node;
+	dev->dev.of_node = of_node_get(node);
 	dev->devfn = devfn;
 	dev->multifunction = 0;		/* maybe a lie? */
 	set_pcie_port_type(dev);
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index ac1ec54..9d179c4 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -90,3 +90,39 @@ int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq)
 	return of_irq_map_raw(ppnode, &lspec_be, 1, laddr, out_irq);
 }
 EXPORT_SYMBOL_GPL(of_irq_map_pci);
+
+
+static inline int __of_pci_pci_compare(struct device_node *node, unsigned int devfn)
+{
+	unsigned int size;
+	const __be32 *reg = of_get_property(node, "reg", &size);
+
+	if (!reg || size < 5 * sizeof(__be32))
+		return 0;
+	return ((be32_to_cpup(&reg[0]) >> 8) & 0xff) == devfn;
+}
+
+struct device_node *of_pci_find_child_device(struct device_node *parent,
+					     unsigned int devfn)
+{
+	struct device_node *node, *node2;
+	
+	for_each_child_of_node(parent, node) {
+		if (__of_pci_pci_compare(node, devfn))
+			return node;
+		/*
+		 * Some OFs create a parent node "multifunc-device" as
+		 * a fake root for all functions of a multi-function
+		 * device we go down them as well.
+		 */
+                if (!strcmp(node->name, "multifunc-device")) {
+			for_each_child_of_node(node, node2) {
+				if (__of_pci_pci_compare(node2, devfn)) {
+					of_node_put(node);
+					return node2;
+				}
+			}
+                }
+	}
+	return NULL;
+}
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 98d61c8..d5c3cb9 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -70,4 +70,6 @@ obj-$(CONFIG_PCI_STUB) += pci-stub.o
 
 obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
 
+obj-$(CONFIG_OF) += of.o
+
 ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index 0830347..1d002b1 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -158,7 +158,7 @@ static void dlpar_pci_add_bus(struct device_node *dn)
 	/* Scan below the new bridge */
 	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 	    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
-		of_scan_pci_bridge(dn, dev);
+		of_scan_pci_bridge(dev);
 
 	/* Map IO space for child bus, which may or may not succeed */
 	pcibios_map_io_space(dev->subordinate);
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
new file mode 100644
index 0000000..fff7270
--- /dev/null
+++ b/drivers/pci/of.c
@@ -0,0 +1,60 @@
+/*
+ * PCI <-> OF mapping helpers
+ *
+ * Copyright 2011 IBM Corp.
+ * 
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/of_pci.h>
+#include "pci.h"
+
+void pci_set_of_node(struct pci_dev *dev)
+{
+	if (!dev->bus->dev.of_node)
+		return;
+	dev->dev.of_node = of_pci_find_child_device(dev->bus->dev.of_node,
+						    dev->devfn);
+}
+
+void pci_release_of_node(struct pci_dev *dev)
+{
+	of_node_put(dev->dev.of_node);
+	dev->dev.of_node = NULL;
+}
+
+void pci_set_bus_of_node(struct pci_bus *bus)
+{
+	if (bus->self == NULL)
+		bus->dev.of_node = pcibios_get_phb_of_node(bus);
+	else
+		bus->dev.of_node = of_node_get(bus->self->dev.of_node);
+}
+
+void pci_release_bus_of_node(struct pci_bus *bus)
+{
+	of_node_put(bus->dev.of_node);
+	bus->dev.of_node = NULL;
+}
+
+struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	/* This should only be called for PHBs */
+	if (WARN_ON(bus->self || bus->parent))
+		return NULL;
+
+	/* Look for a node pointer in either the intermediary device we
+	 * create above the root bus or it's own parent. Normally only
+	 * the later is populated.
+	 */
+	if (bus->bridge->of_node)
+		return of_node_get(bus->bridge->of_node);
+	if (bus->bridge->parent->of_node)
+		return of_node_get(bus->bridge->parent->of_node);
+	return NULL;
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 44cbbba..347349b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -89,6 +89,7 @@ static void release_pcibus_dev(struct device *dev)
 	if (pci_bus->bridge)
 		put_device(pci_bus->bridge);
 	pci_bus_remove_resources(pci_bus);
+	pci_release_bus_of_node(pci_bus);
 	kfree(pci_bus);
 }
 
@@ -624,7 +625,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 
 	child->self = bridge;
 	child->bridge = get_device(&bridge->dev);
-
+	pci_set_bus_of_node(child);	
 	pci_set_bus_speed(child);
 
 	/* Set up default resource pointers and names.. */
@@ -1074,6 +1075,7 @@ static void pci_release_dev(struct device *dev)
 
 	pci_dev = to_pci_dev(dev);
 	pci_release_capabilities(pci_dev);
+	pci_release_of_node(pci_dev);
 	kfree(pci_dev);
 }
 
@@ -1150,6 +1152,7 @@ struct pci_dev *alloc_pci_dev(void)
 }
 EXPORT_SYMBOL(alloc_pci_dev);
 
+
 /*
  * Read the config data for a PCI device, sanity-check it
  * and fill in the dev structure...
@@ -1193,6 +1196,8 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	dev->vendor = l & 0xffff;
 	dev->device = (l >> 16) & 0xffff;
 
+	pci_set_of_node(dev);
+
 	if (pci_setup_device(dev)) {
 		kfree(dev);
 		return NULL;
@@ -1445,6 +1450,7 @@ struct pci_bus * pci_create_bus(struct device *parent,
 		goto dev_reg_err;
 	b->bridge = get_device(dev);
 	device_enable_async_suspend(b->bridge);
+	pci_set_bus_of_node(b);
 
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 85a27b6..f93e217 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -6,4 +6,9 @@
 struct pci_dev;
 struct of_irq;
 int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq);
+
+struct device_node;
+struct device_node *of_pci_find_child_device(struct device_node *parent,
+					     unsigned int devfn);
+
 #endif
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 96f70d7..e5111da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1543,5 +1543,22 @@ int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt);
 int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off,
 			      unsigned int len, const char *kw);
 
+/* PCI <-> OF binding helpers */
+#ifdef  CONFIG_OF
+#include <linux/of.h>
+extern void pci_set_of_node(struct pci_dev *dev);
+extern void pci_release_of_node(struct pci_dev *dev);
+extern void pci_set_bus_of_node(struct pci_bus *bus);
+extern void pci_release_bus_of_node(struct pci_bus *bus);
+
+/* Arch may override this (weak) */
+extern struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus);
+
+
+#else /* CONFIG_OF */
+static void pci_locate_of_node(struct pci_dev *dev) { }
+static void pci_release_of_node(struct pci_dev *dev) { }
+#endif  /* CONFIG_OF */
+
 #endif /* __KERNEL__ */
 #endif /* LINUX_PCI_H */

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[David Miller]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Mon, 04 Apr 2011 13:27:10 +1000

> +struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
> +{
> +	/* This should only be called for PHBs */
> +	if (WARN_ON(bus->self || bus->parent))
> +		return NULL;

This WARN_ON() will always trigger on sparc, because we use the OF
device tree object at the "parent" of the PCI bus devices we create
for the PCI controller domains.

I'm really surprised you don't link the PCI bus roots into the rest of
the global device hierarchy on powerpc.

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[Bjorn Helgaas]

On Mon, Apr 4, 2011 at 1:37 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:

> powerpc has two different ways of matching PCI devices to their
> corresponding OF node (if any) for historical reasons. The ppc64 one
> does a scan looking for matching bus/dev/fn, while the ppc32 one does a
> scan looking only for matching dev/fn on each level in order to be
> agnostic to busses being renumbered (which Linux does on some
> platforms).
>
> This removes both and instead moves the matching code to the PCI core
> itself. It's the most logical place to do it: when a pci_dev is created,
> we know the parent and thus can do a single level scan for the matching
> device_node (if any).

Some of this is reminiscent of the ACPI/PCI binding we do on x86/ia64,
e.g., acpi_get_pci_dev() and the stuff in drivers/acpi/glue.c.  Have
you looked at that to see if there's any hope of covering both OF and
ACPI with something more generic?

Bjorn

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[Benjamin Herrenschmidt]

On Mon, 2011-04-04 at 00:48 -0700, David Miller wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Mon, 04 Apr 2011 13:27:10 +1000
> 
> > +struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
> > +{
> > +	/* This should only be called for PHBs */
> > +	if (WARN_ON(bus->self || bus->parent))
> > +		return NULL;
> 
> This WARN_ON() will always trigger on sparc, because we use the OF
> device tree object at the "parent" of the PCI bus devices we create
> for the PCI controller domains.
> 
> I'm really surprised you don't link the PCI bus roots into the rest of
> the global device hierarchy on powerpc.

But in the above test bus->parent is the "struct pci_bus *" parent, not
the "struct device *" nor "struct device_node *" parent... That
shouldn't be linked to anything on a PHB.

To answer your other point, we do link PHBs on some platforms, not
others. Historical stuff here. Most of our platforms discover PCI
bridges very early from setup_arch() before we have any struct device
around. Mostly because that's how we always did and there's some
subtle/nasty corner cases to deal with if we change that.

Cheers,
Ben.

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[Benjamin Herrenschmidt]

On Mon, 2011-04-04 at 09:25 -0600, Bjorn Helgaas wrote:
> 
> Some of this is reminiscent of the ACPI/PCI binding we do on x86/ia64,
> e.g., acpi_get_pci_dev() and the stuff in drivers/acpi/glue.c.  Have
> you looked at that to see if there's any hope of covering both OF and
> ACPI with something more generic?

I haven't but I will have a look. Thanks.

Cheers,
Ben.

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[David Miller]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 05 Apr 2011 07:03:17 +1000

> On Mon, 2011-04-04 at 00:48 -0700, David Miller wrote:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Date: Mon, 04 Apr 2011 13:27:10 +1000
>> 
>> > +struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
>> > +{
>> > +	/* This should only be called for PHBs */
>> > +	if (WARN_ON(bus->self || bus->parent))
>> > +		return NULL;
>> 
>> This WARN_ON() will always trigger on sparc, because we use the OF
>> device tree object at the "parent" of the PCI bus devices we create
>> for the PCI controller domains.
>> 
>> I'm really surprised you don't link the PCI bus roots into the rest of
>> the global device hierarchy on powerpc.
> 
> But in the above test bus->parent is the "struct pci_bus *" parent, not
> the "struct device *" nor "struct device_node *" parent... That
> shouldn't be linked to anything on a PHB.

Aha, ok, then it shouldn't trigger on sparc, thanks for explaining.

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[Benjamin Herrenschmidt]

On Mon, 2011-04-04 at 09:25 -0600, Bjorn Helgaas wrote:
> Some of this is reminiscent of the ACPI/PCI binding we do on x86/ia64,
> e.g., acpi_get_pci_dev() and the stuff in drivers/acpi/glue.c.  Have
> you looked at that to see if there's any hope of covering both OF and
> ACPI with something more generic?

Hrm, the ACPI stuff is quite different (to some extent akin to what
power used to do) form what I can tell. You basically traverse the ACPI
tree do perform a matching after the fact.

The idea with my patch is really to populate things as they get
discovered, which makes the code much simpler. Since we have the pointer
to the OF device node in the generic struct device nowadays, if we
populate things that way, by the time we discover a device we already
have at hand the OF device node of the parent bus, so it's a
single/simpler one level search to locate the device itself and populate
it's node as well, done once for all.

I suppose you could do something similar for ACPI, but I wouldn't try to
make a "common infrastructure" at that point, especially since there is
the possibility on x86 to have both OF device-trees and ACPI :-)

Note that I don't really provide a direct/good equivalent of your
acpi_get_pci_dev() ... the matching is mostly used the other way around,
ie a driver for a pci_dev wanting to peek a some properties in the
corresponding OF device_node. ppc32 has some reverse mapping stuff but
it's pretty crappy (and on my to-fixup list for after that patch goes
in) and in fact it would be reasonably easy from now on to implement it
as well, but so far there is no real demand.

Cheers,
Ben.

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[Grant Likely]

On Mon, Apr 04, 2011 at 05:37:44PM +1000, Benjamin Herrenschmidt wrote:
> 
> > Nice, looks like I forgot to add the new drivers/pci/of.c file :-)
> > Here's a new patch. Also added linux-pci to the CC list.
> 
> And this one removes a lot more cruft from the powermac code while at
> it, and moves the core matching logic to drivers/of/of_pci.c...
> 
> From 917ea61d6afcf443ca467d0a6ffa00d5c6e21bb3 Mon Sep 17 00:00:00 2001
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Mon, 4 Apr 2011 14:38:13 +1000
> Subject: [PATCH] of: Match PCI devices to OF nodes dynamically
> 
> powerpc has two different ways of matching PCI devices to their
> corresponding OF node (if any) for historical reasons. The ppc64 one
> does a scan looking for matching bus/dev/fn, while the ppc32 one does a
> scan looking only for matching dev/fn on each level in order to be
> agnostic to busses being renumbered (which Linux does on some
> platforms).
> 
> This removes both and instead moves the matching code to the PCI core
> itself. It's the most logical place to do it: when a pci_dev is created,
> we know the parent and thus can do a single level scan for the matching
> device_node (if any).
> 
> The benefit is that all archs now get the matching for free. There's one
> hook the arch might want to provide to match a PHB bus to its device
> node. A default weak implementation is provided that looks for the
> parent device device node, but it's not entirely reliable on powerpc for
> various reasons so powerpc provides its own.

Awesome.  I'm looking at doing pretty much exactly the same thing for
USB and platform_devices on SoCs.  I'm glad to see this for pci.

> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  arch/powerpc/include/asm/pci-bridge.h |   35 +++------
>  arch/powerpc/include/asm/pci.h        |    3 +-
>  arch/powerpc/include/asm/prom.h       |   14 ---
>  arch/powerpc/kernel/pci-common.c      |   11 ++-
>  arch/powerpc/kernel/pci_32.c          |  148 +++------------------------------
>  arch/powerpc/kernel/pci_dn.c          |   47 -----------
>  arch/powerpc/kernel/pci_of_scan.c     |    9 +--
>  arch/powerpc/platforms/powermac/pci.c |    3 +-
>  arch/sparc/kernel/pci.c               |    2 +-
>  drivers/of/of_pci.c                   |   36 ++++++++
>  drivers/pci/Makefile                  |    2 +
>  drivers/pci/hotplug/rpadlpar_core.c   |    2 +-
>  drivers/pci/of.c                      |   60 +++++++++++++
>  drivers/pci/probe.c                   |    8 ++-
>  include/linux/of_pci.h                |    5 +
>  include/linux/pci.h                   |   17 ++++
>  16 files changed, 165 insertions(+), 237 deletions(-)
>  create mode 100644 drivers/pci/of.c
> 
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 5e156e0..6912c45 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -169,18 +169,22 @@ static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
>  	return bus->sysdata;
>  }
>  
> -#ifndef CONFIG_PPC64
> +static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
> +{
> +	return dev->dev.of_node;
> +}
>  
>  static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
>  {
> -	struct pci_controller *host;
> -
> -	if (bus->self)
> -		return pci_device_to_OF_node(bus->self);
> -	host = pci_bus_to_host(bus);
> -	return host ? host->dn : NULL;
> +	return bus->dev.of_node;
>  }

Should these two inlines move to include/linux/of_pci.h?  Microblaze
defines it differently, but I don't think it should be.  Sparc also
has a different variant in arch/sparc/kernel/pci.c, but not in
arch/sparc/kernel/pcic.c.  It looks to me like this should be the
common case unless a specific platform needs otherwise.

>  
> +#ifndef CONFIG_PPC64
> +
> +extern int pci_device_from_OF_node(struct device_node *node,
> +				   u8* bus, u8* devfn);
> +extern void pci_create_OF_bus_map(void);
> +
>  static inline int isa_vaddr_is_ioport(void __iomem *address)
>  {
>  	/* No specific ISA handling on ppc32 at this stage, it
> @@ -223,17 +227,8 @@ struct pci_dn {
>  /* Get the pointer to a device_node's pci_dn */
>  #define PCI_DN(dn)	((struct pci_dn *) (dn)->data)
>  
> -extern struct device_node *fetch_dev_dn(struct pci_dev *dev);
>  extern void * update_dn_pci_info(struct device_node *dn, void *data);
>  
> -/* Get a device_node from a pci_dev.  This code must be fast except
> - * in the case where the sysdata is incorrect and needs to be fixed
> - * up (this will only happen once). */
> -static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
> -{
> -	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
> -}
> -
>  static inline int pci_device_from_OF_node(struct device_node *np,
>  					  u8 *bus, u8 *devfn)
>  {
> @@ -244,14 +239,6 @@ static inline int pci_device_from_OF_node(struct device_node *np,
>  	return 0;
>  }
>  
> -static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
> -{
> -	if (bus->self)
> -		return pci_device_to_OF_node(bus->self);
> -	else
> -		return bus->dev.of_node; /* Must be root bus (PHB) */
> -}
> -

Yay!

>  /** Find the bus corresponding to the indicated device node */
>  extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn);
>  
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index 7d77909..1f52268 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -179,8 +179,7 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
>  extern struct pci_dev *of_create_pci_dev(struct device_node *node,
>  					struct pci_bus *bus, int devfn);
>  
> -extern void of_scan_pci_bridge(struct device_node *node,
> -				struct pci_dev *dev);
> +extern void of_scan_pci_bridge(struct pci_dev *dev);
>  
>  extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
>  extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index c189aa5..b823536 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -22,20 +22,6 @@
>  
>  #define HAVE_ARCH_DEVTREE_FIXUPS
>  
> -#ifdef CONFIG_PPC32
> -/*
> - * PCI <-> OF matching functions
> - * (XXX should these be here?)
> - */
> -struct pci_bus;
> -struct pci_dev;
> -extern int pci_device_from_OF_node(struct device_node *node,
> -				   u8* bus, u8* devfn);
> -extern struct device_node* pci_busdev_to_OF_node(struct pci_bus *, int);
> -extern struct device_node* pci_device_to_OF_node(struct pci_dev *);
> -extern void pci_create_OF_bus_map(void);
> -#endif
> -
>  /*
>   * OF address retreival & translation
>   */
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 893af2a..47c516b 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1097,9 +1097,6 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
>  		if (dev->is_added)
>  			continue;
>  
> -		/* Setup OF node pointer in the device */
> -		dev->dev.of_node = pci_device_to_OF_node(dev);
> -
>  		/* Fixup NUMA node as it may not be setup yet by the generic
>  		 * code and is needed by the DMA init
>  		 */
> @@ -1685,6 +1682,13 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
>  	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
>  }
>  
> +struct device_node * pcibios_get_phb_of_node(struct pci_bus *bus)
> +{
> +	struct pci_controller *hose = bus->sysdata;
> +
> +	return of_node_get(hose->dn);
> +}
> +
>  /**
>   * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
>   * @hose: Pointer to the PCI host controller instance structure
> @@ -1705,7 +1709,6 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>  			hose->global_number);
>  		return;
>  	}
> -	bus->dev.of_node = of_node_get(node);
>  	bus->secondary = hose->first_busno;
>  	hose->bus = bus;
>  
> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
> index bedb370..ca6bcb7 100644
> --- a/arch/powerpc/kernel/pci_32.c
> +++ b/arch/powerpc/kernel/pci_32.c
> @@ -167,150 +167,26 @@ pcibios_make_OF_bus_map(void)
>  #endif
>  }
>  
> -typedef int (*pci_OF_scan_iterator)(struct device_node* node, void* data);
> -
> -static struct device_node*
> -scan_OF_pci_childs(struct device_node *parent, pci_OF_scan_iterator filter, void* data)
> -{
> -	struct device_node *node;
> -	struct device_node* sub_node;
> -
> -	for_each_child_of_node(parent, node) {
> -		const unsigned int *class_code;
> -	
> -		if (filter(node, data)) {
> -			of_node_put(node);
> -			return node;
> -		}
> -
> -		/* For PCI<->PCI bridges or CardBus bridges, we go down
> -		 * Note: some OFs create a parent node "multifunc-device" as
> -		 * a fake root for all functions of a multi-function device,
> -		 * we go down them as well.
> -		 */
> -		class_code = of_get_property(node, "class-code", NULL);
> -		if ((!class_code || ((*class_code >> 8) != PCI_CLASS_BRIDGE_PCI &&
> -			(*class_code >> 8) != PCI_CLASS_BRIDGE_CARDBUS)) &&
> -			strcmp(node->name, "multifunc-device"))
> -			continue;
> -		sub_node = scan_OF_pci_childs(node, filter, data);
> -		if (sub_node) {
> -			of_node_put(node);
> -			return sub_node;
> -		}
> -	}
> -	return NULL;
> -}
> -
> -static struct device_node *scan_OF_for_pci_dev(struct device_node *parent,
> -					       unsigned int devfn)
> -{
> -	struct device_node *np, *cnp;
> -	const u32 *reg;
> -	unsigned int psize;
> -
> -	for_each_child_of_node(parent, np) {
> -		reg = of_get_property(np, "reg", &psize);
> -                if (reg && psize >= 4 && ((reg[0] >> 8) & 0xff) == devfn)
> -			return np;
> -
> -		/* Note: some OFs create a parent node "multifunc-device" as
> -		 * a fake root for all functions of a multi-function device,
> -		 * we go down them as well. */
> -                if (!strcmp(np->name, "multifunc-device")) {
> -                        cnp = scan_OF_for_pci_dev(np, devfn);
> -                        if (cnp)
> -                                return cnp;
> -                }
> -	}
> -	return NULL;
> -}
> -
> -
> -static struct device_node *scan_OF_for_pci_bus(struct pci_bus *bus)
> -{
> -	struct device_node *parent, *np;
> -
> -	/* Are we a root bus ? */
> -	if (bus->self == NULL || bus->parent == NULL) {
> -		struct pci_controller *hose = pci_bus_to_host(bus);
> -		if (hose == NULL)
> -			return NULL;
> -		return of_node_get(hose->dn);
> -	}
> -
> -	/* not a root bus, we need to get our parent */
> -	parent = scan_OF_for_pci_bus(bus->parent);
> -	if (parent == NULL)
> -		return NULL;
> -
> -	/* now iterate for children for a match */
> -	np = scan_OF_for_pci_dev(parent, bus->self->devfn);
> -	of_node_put(parent);
> -
> -	return np;
> -}
> -
> -/*
> - * Scans the OF tree for a device node matching a PCI device
> - */
> -struct device_node *
> -pci_busdev_to_OF_node(struct pci_bus *bus, int devfn)
> -{
> -	struct device_node *parent, *np;
> -
> -	pr_debug("pci_busdev_to_OF_node(%d,0x%x)\n", bus->number, devfn);
> -	parent = scan_OF_for_pci_bus(bus);
> -	if (parent == NULL)
> -		return NULL;
> -	pr_debug(" parent is %s\n", parent ? parent->full_name : "<NULL>");
> -	np = scan_OF_for_pci_dev(parent, devfn);
> -	of_node_put(parent);
> -	pr_debug(" result is %s\n", np ? np->full_name : "<NULL>");
> -
> -	/* XXX most callers don't release the returned node
> -	 * mostly because ppc64 doesn't increase the refcount,
> -	 * we need to fix that.
> -	 */
> -	return np;
> -}
> -EXPORT_SYMBOL(pci_busdev_to_OF_node);
> -
> -struct device_node*
> -pci_device_to_OF_node(struct pci_dev *dev)
> -{
> -	return pci_busdev_to_OF_node(dev->bus, dev->devfn);
> -}
> -EXPORT_SYMBOL(pci_device_to_OF_node);
> -
> -static int
> -find_OF_pci_device_filter(struct device_node* node, void* data)
> -{
> -	return ((void *)node == data);
> -}
>  
>  /*
>   * Returns the PCI device matching a given OF node
>   */
> -int
> -pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
> +int pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
>  {
> -	const unsigned int *reg;
> -	struct pci_controller* hose;
>  	struct pci_dev* dev = NULL;
> -	
> -	/* Make sure it's really a PCI device */
> -	hose = pci_find_hose_for_OF_device(node);
> -	if (!hose || !hose->dn)
> -		return -ENODEV;
> -	if (!scan_OF_pci_childs(hose->dn,
> -			find_OF_pci_device_filter, (void *)node))
> +	const __be32 *reg;
> +	int size;
> +
> +	/* Check if it might have a chance to be a PCI device */
> +	if (!pci_find_hose_for_OF_device(node))
>  		return -ENODEV;
> -	reg = of_get_property(node, "reg", NULL);
> -	if (!reg)
> +
> +	reg = of_get_property(node, "reg", &size);
> +	if (!reg || size < 5 * sizeof(u32))
>  		return -ENODEV;
> -	*bus = (reg[0] >> 16) & 0xff;
> -	*devfn = ((reg[0] >> 8) & 0xff);
> +	
> +	*bus = (be32_to_cpup(&reg[0]) >> 16) & 0xff;
> +	*devfn = (be32_to_cpup(&reg[0]) >> 8) & 0xff;
>  
>  	/* Ok, here we need some tweak. If we have already renumbered
>  	 * all busses, we can't rely on the OF bus number any more.
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index d225d99..8cb66a2 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -143,53 +143,6 @@ void __devinit pci_devs_phb_init_dynamic(struct pci_controller *phb)
>  	traverse_pci_devices(dn, update_dn_pci_info, phb);
>  }
>  
> -/*
> - * Traversal func that looks for a <busno,devfcn> value.
> - * If found, the pci_dn is returned (thus terminating the traversal).
> - */
> -static void *is_devfn_node(struct device_node *dn, void *data)
> -{
> -	int busno = ((unsigned long)data >> 8) & 0xff;
> -	int devfn = ((unsigned long)data) & 0xff;
> -	struct pci_dn *pci = dn->data;
> -
> -	if (pci && (devfn == pci->devfn) && (busno == pci->busno))
> -		return dn;
> -	return NULL;
> -}
> -
> -/*
> - * This is the "slow" path for looking up a device_node from a
> - * pci_dev.  It will hunt for the device under its parent's
> - * phb and then update of_node pointer.
> - *
> - * It may also do fixups on the actual device since this happens
> - * on the first read/write.
> - *
> - * Note that it also must deal with devices that don't exist.
> - * In this case it may probe for real hardware ("just in case")
> - * and add a device_node to the device tree if necessary.
> - *
> - * Is this function necessary anymore now that dev->dev.of_node is
> - * used to store the node pointer?
> - *
> - */
> -struct device_node *fetch_dev_dn(struct pci_dev *dev)
> -{
> -	struct pci_controller *phb = dev->sysdata;
> -	struct device_node *dn;
> -	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
> -
> -	if (WARN_ON(!phb))
> -		return NULL;
> -
> -	dn = traverse_pci_devices(phb->dn, is_devfn_node, (void *)searchval);
> -	if (dn)
> -		dev->dev.of_node = dn;
> -	return dn;
> -}
> -EXPORT_SYMBOL(fetch_dev_dn);
> -
>  /** 
>   * pci_devs_phb_init - Initialize phbs and pci devs under them.
>   * 
> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
> index 1e89a72..fe0a5ad 100644
> --- a/arch/powerpc/kernel/pci_of_scan.c
> +++ b/arch/powerpc/kernel/pci_of_scan.c
> @@ -202,9 +202,9 @@ EXPORT_SYMBOL(of_create_pci_dev);
>   * this routine in turn call of_scan_bus() recusively to scan for more child
>   * devices.
>   */
> -void __devinit of_scan_pci_bridge(struct device_node *node,
> -				  struct pci_dev *dev)
> +void __devinit of_scan_pci_bridge(struct pci_dev *dev)
>  {
> +	struct device_node *node = dev->dev.of_node;
>  	struct pci_bus *bus;
>  	const u32 *busrange, *ranges;
>  	int len, i, mode;
> @@ -238,7 +238,6 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
>  	bus->primary = dev->bus->number;
>  	bus->subordinate = busrange[1];
>  	bus->bridge_ctl = 0;
> -	bus->dev.of_node = of_node_get(node);
>  
>  	/* parse ranges property */
>  	/* PCI #address-cells == 3 and #size-cells == 2 always */
> @@ -335,9 +334,7 @@ static void __devinit __of_scan_bus(struct device_node *node,
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
>  		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
>  		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
> -			struct device_node *child = pci_device_to_OF_node(dev);
> -			if (child)
> -				of_scan_pci_bridge(child, dev);
> +			of_scan_pci_bridge(dev);
>  		}
>  	}
>  }
> diff --git a/arch/powerpc/platforms/powermac/pci.c b/arch/powerpc/platforms/powermac/pci.c
> index ab68989..68a18da 100644
> --- a/arch/powerpc/platforms/powermac/pci.c
> +++ b/arch/powerpc/platforms/powermac/pci.c
> @@ -17,6 +17,7 @@
>  #include <linux/init.h>
>  #include <linux/bootmem.h>
>  #include <linux/irq.h>
> +#include <linux/of_pci.h>
>  
>  #include <asm/sections.h>
>  #include <asm/io.h>
> @@ -235,7 +236,7 @@ static int chaos_validate_dev(struct pci_bus *bus, int devfn, int offset)
>  
>  	if (offset >= 0x100)
>  		return  PCIBIOS_BAD_REGISTER_NUMBER;
> -	np = pci_busdev_to_OF_node(bus, devfn);
> +	np = of_pci_find_child_device(bus->dev.of_node, devfn);
>  	if (np == NULL)
>  		return PCIBIOS_DEVICE_NOT_FOUND;
>  
> diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
> index 713dc91..e539d23 100644
> --- a/arch/sparc/kernel/pci.c
> +++ b/arch/sparc/kernel/pci.c
> @@ -284,7 +284,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
>  	dev->sysdata = node;
>  	dev->dev.parent = bus->bridge;
>  	dev->dev.bus = &pci_bus_type;
> -	dev->dev.of_node = node;
> +	dev->dev.of_node = of_node_get(node);
>  	dev->devfn = devfn;
>  	dev->multifunction = 0;		/* maybe a lie? */
>  	set_pcie_port_type(dev);
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index ac1ec54..9d179c4 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -90,3 +90,39 @@ int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq)
>  	return of_irq_map_raw(ppnode, &lspec_be, 1, laddr, out_irq);
>  }
>  EXPORT_SYMBOL_GPL(of_irq_map_pci);
> +
> +
> +static inline int __of_pci_pci_compare(struct device_node *node, unsigned int devfn)
> +{
> +	unsigned int size;
> +	const __be32 *reg = of_get_property(node, "reg", &size);
> +
> +	if (!reg || size < 5 * sizeof(__be32))
> +		return 0;
> +	return ((be32_to_cpup(&reg[0]) >> 8) & 0xff) == devfn;
> +}
> +
> +struct device_node *of_pci_find_child_device(struct device_node *parent,
> +					     unsigned int devfn)
> +{
> +	struct device_node *node, *node2;
> +	
> +	for_each_child_of_node(parent, node) {
> +		if (__of_pci_pci_compare(node, devfn))
> +			return node;
> +		/*
> +		 * Some OFs create a parent node "multifunc-device" as
> +		 * a fake root for all functions of a multi-function
> +		 * device we go down them as well.
> +		 */
> +                if (!strcmp(node->name, "multifunc-device")) {
> +			for_each_child_of_node(node, node2) {
> +				if (__of_pci_pci_compare(node2, devfn)) {
> +					of_node_put(node);
> +					return node2;
> +				}
> +			}
> +                }
> +	}
> +	return NULL;
> +}
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index 98d61c8..d5c3cb9 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -70,4 +70,6 @@ obj-$(CONFIG_PCI_STUB) += pci-stub.o
>  
>  obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
>  
> +obj-$(CONFIG_OF) += of.o
> +
>  ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
> diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
> index 0830347..1d002b1 100644
> --- a/drivers/pci/hotplug/rpadlpar_core.c
> +++ b/drivers/pci/hotplug/rpadlpar_core.c
> @@ -158,7 +158,7 @@ static void dlpar_pci_add_bus(struct device_node *dn)
>  	/* Scan below the new bridge */
>  	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
>  	    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
> -		of_scan_pci_bridge(dn, dev);
> +		of_scan_pci_bridge(dev);
>  
>  	/* Map IO space for child bus, which may or may not succeed */
>  	pcibios_map_io_space(dev->subordinate);
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> new file mode 100644
> index 0000000..fff7270
> --- /dev/null
> +++ b/drivers/pci/of.c

Should this be consolidated with drivers/of/of_pci.c?  I don't have
strong opinions about where the result lives, but I don't think they
should be split up.

> @@ -0,0 +1,60 @@
> +/*
> + * PCI <-> OF mapping helpers
> + *
> + * Copyright 2011 IBM Corp.
> + * 
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +#include <linux/of_pci.h>
> +#include "pci.h"
> +
> +void pci_set_of_node(struct pci_dev *dev)
> +{
> +	if (!dev->bus->dev.of_node)
> +		return;
> +	dev->dev.of_node = of_pci_find_child_device(dev->bus->dev.of_node,
> +						    dev->devfn);
> +}
> +
> +void pci_release_of_node(struct pci_dev *dev)
> +{
> +	of_node_put(dev->dev.of_node);
> +	dev->dev.of_node = NULL;
> +}
> +
> +void pci_set_bus_of_node(struct pci_bus *bus)
> +{
> +	if (bus->self == NULL)
> +		bus->dev.of_node = pcibios_get_phb_of_node(bus);
> +	else
> +		bus->dev.of_node = of_node_get(bus->self->dev.of_node);
> +}
> +
> +void pci_release_bus_of_node(struct pci_bus *bus)
> +{
> +	of_node_put(bus->dev.of_node);
> +	bus->dev.of_node = NULL;
> +}
> +
> +struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
> +{
> +	/* This should only be called for PHBs */
> +	if (WARN_ON(bus->self || bus->parent))
> +		return NULL;
> +
> +	/* Look for a node pointer in either the intermediary device we
> +	 * create above the root bus or it's own parent. Normally only
> +	 * the later is populated.
> +	 */
> +	if (bus->bridge->of_node)
> +		return of_node_get(bus->bridge->of_node);
> +	if (bus->bridge->parent->of_node)
> +		return of_node_get(bus->bridge->parent->of_node);
> +	return NULL;
> +}
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 44cbbba..347349b 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -89,6 +89,7 @@ static void release_pcibus_dev(struct device *dev)
>  	if (pci_bus->bridge)
>  		put_device(pci_bus->bridge);
>  	pci_bus_remove_resources(pci_bus);
> +	pci_release_bus_of_node(pci_bus);
>  	kfree(pci_bus);
>  }
>  
> @@ -624,7 +625,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  
>  	child->self = bridge;
>  	child->bridge = get_device(&bridge->dev);
> -
> +	pci_set_bus_of_node(child);	

If I'm reading this correctly, the only time an of_node can be
associated with a pci_bus is here and pci_alloc_bus.  In both cases
that makes it safe for release_pcibus_dev() to remove it.  Am I
correct?

oh, and trailing whitespace.  :-)

>  	pci_set_bus_speed(child);
>  
>  	/* Set up default resource pointers and names.. */
> @@ -1074,6 +1075,7 @@ static void pci_release_dev(struct device *dev)
>  
>  	pci_dev = to_pci_dev(dev);
>  	pci_release_capabilities(pci_dev);
> +	pci_release_of_node(pci_dev);
>  	kfree(pci_dev);
>  }
>  
> @@ -1150,6 +1152,7 @@ struct pci_dev *alloc_pci_dev(void)
>  }
>  EXPORT_SYMBOL(alloc_pci_dev);
>  
> +
>  /*
>   * Read the config data for a PCI device, sanity-check it
>   * and fill in the dev structure...

Unrelated whitespace change

> @@ -1193,6 +1196,8 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
>  	dev->vendor = l & 0xffff;
>  	dev->device = (l >> 16) & 0xffff;
>  
> +	pci_set_of_node(dev);
> +
>  	if (pci_setup_device(dev)) {
>  		kfree(dev);
>  		return NULL;
> @@ -1445,6 +1450,7 @@ struct pci_bus * pci_create_bus(struct device *parent,
>  		goto dev_reg_err;
>  	b->bridge = get_device(dev);
>  	device_enable_async_suspend(b->bridge);
> +	pci_set_bus_of_node(b);

Similarly here; so device node linkage to pci devices will always use
the same mechanism except for in of_create_pci_dev() for sparc, right?

>  
>  	if (!parent)
>  		set_dev_node(b->bridge, pcibus_to_node(b));
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 85a27b6..f93e217 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -6,4 +6,9 @@
>  struct pci_dev;
>  struct of_irq;
>  int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq);
> +
> +struct device_node;
> +struct device_node *of_pci_find_child_device(struct device_node *parent,
> +					     unsigned int devfn);
> +
>  #endif
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 96f70d7..e5111da 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1543,5 +1543,22 @@ int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt);
>  int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off,
>  			      unsigned int len, const char *kw);
>  
> +/* PCI <-> OF binding helpers */
> +#ifdef  CONFIG_OF
> +#include <linux/of.h>

linux/of.h is safe to include when CONFIG_OF is not selected; it can
live at the top of the file with the rest of the includes.

> +extern void pci_set_of_node(struct pci_dev *dev);
> +extern void pci_release_of_node(struct pci_dev *dev);
> +extern void pci_set_bus_of_node(struct pci_bus *bus);
> +extern void pci_release_bus_of_node(struct pci_bus *bus);

Looks to me like these functions still get called when
!defined(CONFIG_OF).

> +
> +/* Arch may override this (weak) */
> +extern struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus);
> +
> +
> +#else /* CONFIG_OF */
> +static void pci_locate_of_node(struct pci_dev *dev) { }
> +static void pci_release_of_node(struct pci_dev *dev) { }
> +#endif  /* CONFIG_OF */
> +
>  #endif /* __KERNEL__ */
>  #endif /* LINUX_PCI_H */
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH] of: Match PCI devices to OF nodes generically

[Benjamin Herrenschmidt]

> > The benefit is that all archs now get the matching for free. There's one
> > hook the arch might want to provide to match a PHB bus to its device
> > node. A default weak implementation is provided that looks for the
> > parent device device node, but it's not entirely reliable on powerpc for
> > various reasons so powerpc provides its own.
> 
> Awesome.  I'm looking at doing pretty much exactly the same thing for
> USB and platform_devices on SoCs.  I'm glad to see this for pci.

Yeah, I figured it would come in handy :-)

I looked a bit at USB and the main issue is that it's unclear in the
binding whether the root hub is exposed in the device-tree as a hub or
not. The tendency from what I can see of Apple and Sun produced trees is
that it's -not- there.

> > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> > index 5e156e0..6912c45 100644
> > --- a/arch/powerpc/include/asm/pci-bridge.h
> > +++ b/arch/powerpc/include/asm/pci-bridge.h
> > @@ -169,18 +169,22 @@ static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
> >  	return bus->sysdata;
> >  }
> >  
> > -#ifndef CONFIG_PPC64
> > +static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
> > +{
> > +	return dev->dev.of_node;
> > +}
> >  
> >  static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
> >  {
> > -	struct pci_controller *host;
> > -
> > -	if (bus->self)
> > -		return pci_device_to_OF_node(bus->self);
> > -	host = pci_bus_to_host(bus);
> > -	return host ? host->dn : NULL;
> > +	return bus->dev.of_node;
> >  }
> 
> Should these two inlines move to include/linux/of_pci.h?  Microblaze
> defines it differently, but I don't think it should be.  Sparc also
> has a different variant in arch/sparc/kernel/pci.c, but not in
> arch/sparc/kernel/pcic.c.  It looks to me like this should be the
> common case unless a specific platform needs otherwise.

I didn't like the function name and though it would be better to
deprecate it and have users peek at the struct device -> of_node
instead, but if you think it's worthwhile I can factor that out.

I haven't looked at microblaze PCI code yet so I might be breaking
it... it looks like an old variant of the ppc32 one :-) They should
never have copied the OF node map stuff for example or the bus
renumbering for that matter :-)

The main thing is to get the node of the PHB. The "default"
weak function looks for the parent struct device you pass to
pci_create_bus() but it's often NULL if you detect your PHBs
very early like we do on most powerpc platforms.

That's why I have my override, and from what I can tell, microblaze
would need to copy it over too (and probably get rid of a lot of crap
they copied from me that they really really don't need :-)

 .../...

> > --- a/drivers/pci/hotplug/rpadlpar_core.c
> > +++ b/drivers/pci/hotplug/rpadlpar_core.c
> > @@ -158,7 +158,7 @@ static void dlpar_pci_add_bus(struct device_node *dn)
> >  	/* Scan below the new bridge */
> >  	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
> >  	    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
> > -		of_scan_pci_bridge(dn, dev);
> > +		of_scan_pci_bridge(dev);
> >  
> >  	/* Map IO space for child bus, which may or may not succeed */
> >  	pcibios_map_io_space(dev->subordinate);
> > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > new file mode 100644
> > index 0000000..fff7270
> > --- /dev/null
> > +++ b/drivers/pci/of.c
> 
> Should this be consolidated with drivers/of/of_pci.c?  I don't have
> strong opinions about where the result lives, but I don't think they
> should be split up.

I assume your comment refers to the rest of the code in that file (ie to
the file name above and not to the rpadlpar hunk :-)

This is my rationale for the split:

 - Bits in drivers/of/of_pci.c contain basic routines to parse
PCI device "reg" properties and locate devices in the device-tree
that don't involve the Linux PCI layer. This can be used by the later
(as it is below) but it can also be used by early boot platform code
that might need to locate PCI devices before the linux PCI probe
happens, that sort of thing. It's pretty clear that those functions
don't rely on anything from the linux PCI code

 - Bits in drivers/pci/of.c that contain the glue to actually populate
the pci_dev's and which use the parsers in the previous file. These
functions are strictly hooks that are called by the linux PCI layer to
populate it's nodes (along with the weak overridable one for getting
to the PHB device node).

There's a clear distinction here I believe and I think it's worth
keeping. I don't like having drivers/pci/probe.c call into
drivers/of/of_pci.c directly basically. The glue layer that way is more
obvious.

  .../...

> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 44cbbba..347349b 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -89,6 +89,7 @@ static void release_pcibus_dev(struct device *dev)
> >  	if (pci_bus->bridge)
> >  		put_device(pci_bus->bridge);
> >  	pci_bus_remove_resources(pci_bus);
> > +	pci_release_bus_of_node(pci_bus);
> >  	kfree(pci_bus);
> >  }
> >  
> > @@ -624,7 +625,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> >  
> >  	child->self = bridge;
> >  	child->bridge = get_device(&bridge->dev);
> > -
> > +	pci_set_bus_of_node(child);	
> 
> If I'm reading this correctly, the only time an of_node can be
> associated with a pci_bus is here and pci_alloc_bus.  In both cases
> that makes it safe for release_pcibus_dev() to remove it.  Am I
> correct?

Well, there is also pci_create_bus() but yes, basically. Also
of_node_put() is safe vs. NULL afaik.

> oh, and trailing whitespace.  :-)

Yeah, those will kill me eventually :-)

  .../...

> > @@ -1193,6 +1196,8 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
> >  	dev->vendor = l & 0xffff;
> >  	dev->device = (l >> 16) & 0xffff;
> >  
> > +	pci_set_of_node(dev);
> > +
> >  	if (pci_setup_device(dev)) {
> >  		kfree(dev);
> >  		return NULL;
> > @@ -1445,6 +1450,7 @@ struct pci_bus * pci_create_bus(struct device *parent,
> >  		goto dev_reg_err;
> >  	b->bridge = get_device(dev);
> >  	device_enable_async_suspend(b->bridge);
> > +	pci_set_bus_of_node(b);
> 
> Similarly here; so device node linkage to pci devices will always use
> the same mechanism except for in of_create_pci_dev() for sparc, right?

And of_create_pci_dev() for powerpc :-)

But yes, basically. If you have a platform that "generates" struct
pci_dev without calling pci_scan_device() then it's your responsibility
to populate it.

But again, put() should be NULL safe so the ->release callback souldn't
have a problem if the node isn't populated.

> linux/of.h is safe to include when CONFIG_OF is not selected; it can
> live at the top of the file with the rest of the includes.
> 
> > +extern void pci_set_of_node(struct pci_dev *dev);
> > +extern void pci_release_of_node(struct pci_dev *dev);
> > +extern void pci_set_bus_of_node(struct pci_bus *bus);
> > +extern void pci_release_bus_of_node(struct pci_bus *bus);
> 
> Looks to me like these functions still get called when
> !defined(CONFIG_OF).

Yes, I just forgot to "update" the empty inlines (you can see them
below but they have different/obsolete names :-)

I'll fix all of that up.

Thanks for the review !

Cheers,
Ben.

> > +
> > +/* Arch may override this (weak) */
> > +extern struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus);
> > +
> > +
> > +#else /* CONFIG_OF */
> > +static void pci_locate_of_node(struct pci_dev *dev) { }
> > +static void pci_release_of_node(struct pci_dev *dev) { }
> > +#endif  /* CONFIG_OF */
> > +
> >  #endif /* __KERNEL__ */
> >  #endif /* LINUX_PCI_H */
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/