linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing
@ 2013-04-11 15:26 Andrew Murray
  2013-04-11 15:26 ` [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC Andrew Murray
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Andrew Murray @ 2013-04-11 15:26 UTC (permalink / raw)
  To: rob.herring
  Cc: jgunthorpe, linux, siva.kallam, linux-pci, devicetree-discuss,
	jg1.han, Liviu.Dudau, linux-kernel, linux-samsung-soc, kgene.kim,
	bhelgaas, suren.reddy, linux-arm-kernel, monstr, benh, paulus,
	grant.likely, thomas.petazzoni, thierry.reding, thomas.abraham,
	arnd, linus.walleij, Andrew Murray

This patchset factors out duplicated code associated with parsing PCI
DT "ranges" properties across the architectures and introduces a
"ranges" parser. This parser "of_pci_range_parser" can be used directly
by ARM host bridge drivers enabling them to obtain ranges from device
trees.

I've included the Reviewed-by and Tested-by's received from v5 in this
patchset, earlier versions of this patchset (v3) have been tested-by:

Thierry Reding <thierry.reding@avionic-design.de>
Jingoo Han <jg1.han@samsung.com>

I believe a version of this patchset has also been tested through its
inclusion in Thomas Petazzoni's Armada 370 and Armada XP SoCs PCIe support by:

Linus Walleij <linus.walleij@linaro.org>

I've tested that this patchset builds and runs on ARM and that it builds on
PowerPC.

Compared to the v5 sent by Andrew Murray, the following changes have
been made:

 * Use of CONFIG_64BIT instead of CONFIG_[a32bitarch] as suggested by
   Rob Herring in drivers/of/of_pci.c

 * Added forward declaration of struct pci_controller in linux/of_pci.h
   to prevent compiler warning as suggested by Thomas Petazzoni

 * Improved error checking (!range check), removal of unnecessary be32_to_cpup
   call, improved formatting of struct of_pci_range_parser layout and
   replacement of macro with a static inline. All suggested by Rob Herring.

Compared to the v4 (incorrectly labelled v3) sent by Andrew Murray,
the following changes have been made:

 * Split the patch as suggested by Rob Herring

Compared to the v3 sent by Andrew Murray, the following changes have
been made:

 * Unify and move duplicate pci_process_bridge_OF_ranges functions to
   drivers/of/of_pci.c as suggested by Rob Herring

 * Fix potential build errors with Microblaze/MIPS

Compared to "[PATCH v5 01/17] of/pci: Provide support for parsing PCI DT
ranges property", the following changes have been made:

 * Correct use of IORESOURCE_* as suggested by Russell King

 * Improved interface and naming as suggested by Thierry Reding

Compared to the v2 sent by Andrew Murray, Thomas Petazzoni did:

 * Add a memset() on the struct of_pci_range_iter when starting the
   for loop in for_each_pci_range(). Otherwise, with an uninitialized
   of_pci_range_iter, of_pci_process_ranges() may crash.

 * Add parenthesis around 'res', 'np' and 'iter' in the
   for_each_of_pci_range macro definitions. Otherwise, passing
   something like &foobar as 'res' didn't work.

 * Rebased on top of 3.9-rc2, which required fixing a few conflicts in
   the Microblaze code.

v2:
  This follows on from suggestions made by Grant Likely
  (marc.info/?l=linux-kernel&m=136079602806328)

Andrew Murray (3):
  of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and
    PowerPC
  of/pci: Provide support for parsing PCI DT ranges property
  of/pci: mips: convert to common of_pci_range_parser

 arch/microblaze/include/asm/pci-bridge.h |    5 +-
 arch/microblaze/pci/pci-common.c         |  192 ------------------------------
 arch/mips/pci/pci.c                      |   50 +++------
 arch/powerpc/include/asm/pci-bridge.h    |    5 +-
 arch/powerpc/kernel/pci-common.c         |  192 ------------------------------
 drivers/of/address.c                     |   66 ++++++++++
 drivers/of/of_pci.c                      |  168 ++++++++++++++++++++++++++
 include/linux/of_address.h               |   46 +++++++
 include/linux/of_pci.h                   |    4 +
 9 files changed, 302 insertions(+), 426 deletions(-)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
  2013-04-11 15:26 [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing Andrew Murray
@ 2013-04-11 15:26 ` Andrew Murray
  2013-04-15 12:57   ` Thomas Petazzoni
  2013-04-11 15:26 ` [PATCH v6 2/3] of/pci: Provide support for parsing PCI DT ranges property Andrew Murray
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Andrew Murray @ 2013-04-11 15:26 UTC (permalink / raw)
  To: rob.herring
  Cc: jgunthorpe, linux, siva.kallam, linux-pci, devicetree-discuss,
	jg1.han, Liviu.Dudau, linux-kernel, linux-samsung-soc, kgene.kim,
	bhelgaas, suren.reddy, linux-arm-kernel, monstr, benh, paulus,
	grant.likely, thomas.petazzoni, thierry.reding, thomas.abraham,
	arnd, linus.walleij, Andrew Murray

The pci_process_bridge_OF_ranges function, used to parse the "ranges"
property of a PCI host device, is found in both Microblaze and PowerPC
architectures. These implementations are nearly identical. This patch
moves this common code to a common place.

Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Reviewed-by: Rob Herring <rob.herring@calxeda.com>
Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/microblaze/include/asm/pci-bridge.h |    5 +-
 arch/microblaze/pci/pci-common.c         |  192 ----------------------------
 arch/powerpc/include/asm/pci-bridge.h    |    5 +-
 arch/powerpc/kernel/pci-common.c         |  192 ----------------------------
 drivers/of/of_pci.c                      |  200 ++++++++++++++++++++++++++++++
 include/linux/of_pci.h                   |    4 +
 6 files changed, 206 insertions(+), 392 deletions(-)

diff --git a/arch/microblaze/include/asm/pci-bridge.h b/arch/microblaze/include/asm/pci-bridge.h
index cb5d397..5783cd6 100644
--- a/arch/microblaze/include/asm/pci-bridge.h
+++ b/arch/microblaze/include/asm/pci-bridge.h
@@ -10,6 +10,7 @@
 #include <linux/pci.h>
 #include <linux/list.h>
 #include <linux/ioport.h>
+#include <linux/of_pci.h>
 
 struct device_node;
 
@@ -132,10 +133,6 @@ extern void setup_indirect_pci(struct pci_controller *hose,
 extern struct pci_controller *pci_find_hose_for_OF_device(
 			struct device_node *node);
 
-/* Fill up host controller resources from the OF node */
-extern void pci_process_bridge_OF_ranges(struct pci_controller *hose,
-			struct device_node *dev, int primary);
-
 /* Allocate & free a PCI host bridge structure */
 extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
 extern void pcibios_free_controller(struct pci_controller *phb);
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 9ea521e..2735ad9 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -622,198 +622,6 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
 	*end = rsrc->end - offset;
 }
 
-/**
- * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
- * @hose: newly allocated pci_controller to be setup
- * @dev: device node of the host bridge
- * @primary: set if primary bus (32 bits only, soon to be deprecated)
- *
- * This function will parse the "ranges" property of a PCI host bridge device
- * node and setup the resource mapping of a pci controller based on its
- * content.
- *
- * Life would be boring if it wasn't for a few issues that we have to deal
- * with here:
- *
- *   - We can only cope with one IO space range and up to 3 Memory space
- *     ranges. However, some machines (thanks Apple !) tend to split their
- *     space into lots of small contiguous ranges. So we have to coalesce.
- *
- *   - We can only cope with all memory ranges having the same offset
- *     between CPU addresses and PCI addresses. Unfortunately, some bridges
- *     are setup for a large 1:1 mapping along with a small "window" which
- *     maps PCI address 0 to some arbitrary high address of the CPU space in
- *     order to give access to the ISA memory hole.
- *     The way out of here that I've chosen for now is to always set the
- *     offset based on the first resource found, then override it if we
- *     have a different offset and the previous was set by an ISA hole.
- *
- *   - Some busses have IO space not starting at 0, which causes trouble with
- *     the way we do our IO resource renumbering. The code somewhat deals with
- *     it for 64 bits but I would expect problems on 32 bits.
- *
- *   - Some 32 bits platforms such as 4xx can have physical space larger than
- *     32 bits so we need to use 64 bits values for the parsing
- */
-void pci_process_bridge_OF_ranges(struct pci_controller *hose,
-				  struct device_node *dev, int primary)
-{
-	const u32 *ranges;
-	int rlen;
-	int pna = of_n_addr_cells(dev);
-	int np = pna + 5;
-	int memno = 0, isa_hole = -1;
-	u32 pci_space;
-	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
-	unsigned long long isa_mb = 0;
-	struct resource *res;
-
-	pr_info("PCI host bridge %s %s ranges:\n",
-	       dev->full_name, primary ? "(primary)" : "");
-
-	/* Get ranges property */
-	ranges = of_get_property(dev, "ranges", &rlen);
-	if (ranges == NULL)
-		return;
-
-	/* Parse it */
-	pr_debug("Parsing ranges property...\n");
-	while ((rlen -= np * 4) >= 0) {
-		/* Read next ranges element */
-		pci_space = ranges[0];
-		pci_addr = of_read_number(ranges + 1, 2);
-		cpu_addr = of_translate_address(dev, ranges + 3);
-		size = of_read_number(ranges + pna + 3, 2);
-
-		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
-				pci_space, pci_addr);
-		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
-					cpu_addr, size);
-
-		ranges += np;
-
-		/* If we failed translation or got a zero-sized region
-		 * (some FW try to feed us with non sensical zero sized regions
-		 * such as power3 which look like some kind of attempt
-		 * at exposing the VGA memory hole)
-		 */
-		if (cpu_addr == OF_BAD_ADDR || size == 0)
-			continue;
-
-		/* Now consume following elements while they are contiguous */
-		for (; rlen >= np * sizeof(u32);
-		     ranges += np, rlen -= np * 4) {
-			if (ranges[0] != pci_space)
-				break;
-			pci_next = of_read_number(ranges + 1, 2);
-			cpu_next = of_translate_address(dev, ranges + 3);
-			if (pci_next != pci_addr + size ||
-			    cpu_next != cpu_addr + size)
-				break;
-			size += of_read_number(ranges + pna + 3, 2);
-		}
-
-		/* Act based on address space type */
-		res = NULL;
-		switch ((pci_space >> 24) & 0x3) {
-		case 1:		/* PCI IO space */
-			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
-			       cpu_addr, cpu_addr + size - 1, pci_addr);
-
-			/* We support only one IO range */
-			if (hose->pci_io_size) {
-				pr_info(" \\--> Skipped (too many) !\n");
-				continue;
-			}
-			/* On 32 bits, limit I/O space to 16MB */
-			if (size > 0x01000000)
-				size = 0x01000000;
-
-			/* 32 bits needs to map IOs here */
-			hose->io_base_virt = ioremap(cpu_addr, size);
-
-			/* Expect trouble if pci_addr is not 0 */
-			if (primary)
-				isa_io_base =
-					(unsigned long)hose->io_base_virt;
-			/* pci_io_size and io_base_phys always represent IO
-			 * space starting at 0 so we factor in pci_addr
-			 */
-			hose->pci_io_size = pci_addr + size;
-			hose->io_base_phys = cpu_addr - pci_addr;
-
-			/* Build resource */
-			res = &hose->io_resource;
-			res->flags = IORESOURCE_IO;
-			res->start = pci_addr;
-			break;
-		case 2:		/* PCI Memory space */
-		case 3:		/* PCI 64 bits Memory space */
-			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
-			       cpu_addr, cpu_addr + size - 1, pci_addr,
-			       (pci_space & 0x40000000) ? "Prefetch" : "");
-
-			/* We support only 3 memory ranges */
-			if (memno >= 3) {
-				pr_info(" \\--> Skipped (too many) !\n");
-				continue;
-			}
-			/* Handles ISA memory hole space here */
-			if (pci_addr == 0) {
-				isa_mb = cpu_addr;
-				isa_hole = memno;
-				if (primary || isa_mem_base == 0)
-					isa_mem_base = cpu_addr;
-				hose->isa_mem_phys = cpu_addr;
-				hose->isa_mem_size = size;
-			}
-
-			/* We get the PCI/Mem offset from the first range or
-			 * the, current one if the offset came from an ISA
-			 * hole. If they don't match, bugger.
-			 */
-			if (memno == 0 ||
-			    (isa_hole >= 0 && pci_addr != 0 &&
-			     hose->pci_mem_offset == isa_mb))
-				hose->pci_mem_offset = cpu_addr - pci_addr;
-			else if (pci_addr != 0 &&
-				 hose->pci_mem_offset != cpu_addr - pci_addr) {
-				pr_info(" \\--> Skipped (offset mismatch) !\n");
-				continue;
-			}
-
-			/* Build resource */
-			res = &hose->mem_resources[memno++];
-			res->flags = IORESOURCE_MEM;
-			if (pci_space & 0x40000000)
-				res->flags |= IORESOURCE_PREFETCH;
-			res->start = cpu_addr;
-			break;
-		}
-		if (res != NULL) {
-			res->name = dev->full_name;
-			res->end = res->start + size - 1;
-			res->parent = NULL;
-			res->sibling = NULL;
-			res->child = NULL;
-		}
-	}
-
-	/* If there's an ISA hole and the pci_mem_offset is -not- matching
-	 * the ISA hole offset, then we need to remove the ISA hole from
-	 * the resource list for that brige
-	 */
-	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
-		unsigned int next = isa_hole + 1;
-		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
-		if (next < memno)
-			memmove(&hose->mem_resources[isa_hole],
-				&hose->mem_resources[next],
-				sizeof(struct resource) * (memno - next));
-		hose->mem_resources[--memno].flags = 0;
-	}
-}
-
 /* Decide whether to display the domain number in /proc */
 int pci_proc_domain(struct pci_bus *bus)
 {
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 025a130..205bfba 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -10,6 +10,7 @@
 #include <linux/pci.h>
 #include <linux/list.h>
 #include <linux/ioport.h>
+#include <linux/of_pci.h>
 #include <asm-generic/pci-bridge.h>
 
 struct device_node;
@@ -231,10 +232,6 @@ extern int pcibios_map_io_space(struct pci_bus *bus);
 extern struct pci_controller *pci_find_hose_for_OF_device(
 			struct device_node* node);
 
-/* Fill up host controller resources from the OF node */
-extern void pci_process_bridge_OF_ranges(struct pci_controller *hose,
-			struct device_node *dev, int primary);
-
 /* Allocate & free a PCI host bridge structure */
 extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
 extern void pcibios_free_controller(struct pci_controller *phb);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index fa12ae4..6edf396 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -640,198 +640,6 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
 	*end = rsrc->end - offset;
 }
 
-/**
- * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
- * @hose: newly allocated pci_controller to be setup
- * @dev: device node of the host bridge
- * @primary: set if primary bus (32 bits only, soon to be deprecated)
- *
- * This function will parse the "ranges" property of a PCI host bridge device
- * node and setup the resource mapping of a pci controller based on its
- * content.
- *
- * Life would be boring if it wasn't for a few issues that we have to deal
- * with here:
- *
- *   - We can only cope with one IO space range and up to 3 Memory space
- *     ranges. However, some machines (thanks Apple !) tend to split their
- *     space into lots of small contiguous ranges. So we have to coalesce.
- *
- *   - We can only cope with all memory ranges having the same offset
- *     between CPU addresses and PCI addresses. Unfortunately, some bridges
- *     are setup for a large 1:1 mapping along with a small "window" which
- *     maps PCI address 0 to some arbitrary high address of the CPU space in
- *     order to give access to the ISA memory hole.
- *     The way out of here that I've chosen for now is to always set the
- *     offset based on the first resource found, then override it if we
- *     have a different offset and the previous was set by an ISA hole.
- *
- *   - Some busses have IO space not starting at 0, which causes trouble with
- *     the way we do our IO resource renumbering. The code somewhat deals with
- *     it for 64 bits but I would expect problems on 32 bits.
- *
- *   - Some 32 bits platforms such as 4xx can have physical space larger than
- *     32 bits so we need to use 64 bits values for the parsing
- */
-void pci_process_bridge_OF_ranges(struct pci_controller *hose,
-				  struct device_node *dev, int primary)
-{
-	const u32 *ranges;
-	int rlen;
-	int pna = of_n_addr_cells(dev);
-	int np = pna + 5;
-	int memno = 0, isa_hole = -1;
-	u32 pci_space;
-	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
-	unsigned long long isa_mb = 0;
-	struct resource *res;
-
-	printk(KERN_INFO "PCI host bridge %s %s ranges:\n",
-	       dev->full_name, primary ? "(primary)" : "");
-
-	/* Get ranges property */
-	ranges = of_get_property(dev, "ranges", &rlen);
-	if (ranges == NULL)
-		return;
-
-	/* Parse it */
-	while ((rlen -= np * 4) >= 0) {
-		/* Read next ranges element */
-		pci_space = ranges[0];
-		pci_addr = of_read_number(ranges + 1, 2);
-		cpu_addr = of_translate_address(dev, ranges + 3);
-		size = of_read_number(ranges + pna + 3, 2);
-		ranges += np;
-
-		/* If we failed translation or got a zero-sized region
-		 * (some FW try to feed us with non sensical zero sized regions
-		 * such as power3 which look like some kind of attempt at exposing
-		 * the VGA memory hole)
-		 */
-		if (cpu_addr == OF_BAD_ADDR || size == 0)
-			continue;
-
-		/* Now consume following elements while they are contiguous */
-		for (; rlen >= np * sizeof(u32);
-		     ranges += np, rlen -= np * 4) {
-			if (ranges[0] != pci_space)
-				break;
-			pci_next = of_read_number(ranges + 1, 2);
-			cpu_next = of_translate_address(dev, ranges + 3);
-			if (pci_next != pci_addr + size ||
-			    cpu_next != cpu_addr + size)
-				break;
-			size += of_read_number(ranges + pna + 3, 2);
-		}
-
-		/* Act based on address space type */
-		res = NULL;
-		switch ((pci_space >> 24) & 0x3) {
-		case 1:		/* PCI IO space */
-			printk(KERN_INFO
-			       "  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
-			       cpu_addr, cpu_addr + size - 1, pci_addr);
-
-			/* We support only one IO range */
-			if (hose->pci_io_size) {
-				printk(KERN_INFO
-				       " \\--> Skipped (too many) !\n");
-				continue;
-			}
-#ifdef CONFIG_PPC32
-			/* On 32 bits, limit I/O space to 16MB */
-			if (size > 0x01000000)
-				size = 0x01000000;
-
-			/* 32 bits needs to map IOs here */
-			hose->io_base_virt = ioremap(cpu_addr, size);
-
-			/* Expect trouble if pci_addr is not 0 */
-			if (primary)
-				isa_io_base =
-					(unsigned long)hose->io_base_virt;
-#endif /* CONFIG_PPC32 */
-			/* pci_io_size and io_base_phys always represent IO
-			 * space starting at 0 so we factor in pci_addr
-			 */
-			hose->pci_io_size = pci_addr + size;
-			hose->io_base_phys = cpu_addr - pci_addr;
-
-			/* Build resource */
-			res = &hose->io_resource;
-			res->flags = IORESOURCE_IO;
-			res->start = pci_addr;
-			break;
-		case 2:		/* PCI Memory space */
-		case 3:		/* PCI 64 bits Memory space */
-			printk(KERN_INFO
-			       " MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
-			       cpu_addr, cpu_addr + size - 1, pci_addr,
-			       (pci_space & 0x40000000) ? "Prefetch" : "");
-
-			/* We support only 3 memory ranges */
-			if (memno >= 3) {
-				printk(KERN_INFO
-				       " \\--> Skipped (too many) !\n");
-				continue;
-			}
-			/* Handles ISA memory hole space here */
-			if (pci_addr == 0) {
-				isa_mb = cpu_addr;
-				isa_hole = memno;
-				if (primary || isa_mem_base == 0)
-					isa_mem_base = cpu_addr;
-				hose->isa_mem_phys = cpu_addr;
-				hose->isa_mem_size = size;
-			}
-
-			/* We get the PCI/Mem offset from the first range or
-			 * the, current one if the offset came from an ISA
-			 * hole. If they don't match, bugger.
-			 */
-			if (memno == 0 ||
-			    (isa_hole >= 0 && pci_addr != 0 &&
-			     hose->pci_mem_offset == isa_mb))
-				hose->pci_mem_offset = cpu_addr - pci_addr;
-			else if (pci_addr != 0 &&
-				 hose->pci_mem_offset != cpu_addr - pci_addr) {
-				printk(KERN_INFO
-				       " \\--> Skipped (offset mismatch) !\n");
-				continue;
-			}
-
-			/* Build resource */
-			res = &hose->mem_resources[memno++];
-			res->flags = IORESOURCE_MEM;
-			if (pci_space & 0x40000000)
-				res->flags |= IORESOURCE_PREFETCH;
-			res->start = cpu_addr;
-			break;
-		}
-		if (res != NULL) {
-			res->name = dev->full_name;
-			res->end = res->start + size - 1;
-			res->parent = NULL;
-			res->sibling = NULL;
-			res->child = NULL;
-		}
-	}
-
-	/* If there's an ISA hole and the pci_mem_offset is -not- matching
-	 * the ISA hole offset, then we need to remove the ISA hole from
-	 * the resource list for that brige
-	 */
-	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
-		unsigned int next = isa_hole + 1;
-		printk(KERN_INFO " Removing ISA hole at 0x%016llx\n", isa_mb);
-		if (next < memno)
-			memmove(&hose->mem_resources[isa_hole],
-				&hose->mem_resources[next],
-				sizeof(struct resource) * (memno - next));
-		hose->mem_resources[--memno].flags = 0;
-	}
-}
-
 /* Decide whether to display the domain number in /proc */
 int pci_proc_domain(struct pci_bus *bus)
 {
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 13e37e2..1626172 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -4,6 +4,10 @@
 #include <linux/of_pci.h>
 #include <asm/prom.h>
 
+#if defined(CONFIG_PPC32) || defined(CONFIG_PPC64) || defined(CONFIG_MICROBLAZE)
+#include <asm/pci-bridge.h>
+#endif
+
 static inline int __of_pci_pci_compare(struct device_node *node,
 				       unsigned int devfn)
 {
@@ -40,3 +44,199 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(of_pci_find_child_device);
+
+/**
+ * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
+ * @hose: newly allocated pci_controller to be setup
+ * @dev: device node of the host bridge
+ * @primary: set if primary bus (32 bits only, soon to be deprecated)
+ *
+ * This function will parse the "ranges" property of a PCI host bridge device
+ * node and setup the resource mapping of a pci controller based on its
+ * content.
+ *
+ * Life would be boring if it wasn't for a few issues that we have to deal
+ * with here:
+ *
+ *   - We can only cope with one IO space range and up to 3 Memory space
+ *     ranges. However, some machines (thanks Apple !) tend to split their
+ *     space into lots of small contiguous ranges. So we have to coalesce.
+ *
+ *   - We can only cope with all memory ranges having the same offset
+ *     between CPU addresses and PCI addresses. Unfortunately, some bridges
+ *     are setup for a large 1:1 mapping along with a small "window" which
+ *     maps PCI address 0 to some arbitrary high address of the CPU space in
+ *     order to give access to the ISA memory hole.
+ *     The way out of here that I've chosen for now is to always set the
+ *     offset based on the first resource found, then override it if we
+ *     have a different offset and the previous was set by an ISA hole.
+ *
+ *   - Some busses have IO space not starting at 0, which causes trouble with
+ *     the way we do our IO resource renumbering. The code somewhat deals with
+ *     it for 64 bits but I would expect problems on 32 bits.
+ *
+ *   - Some 32 bits platforms such as 4xx can have physical space larger than
+ *     32 bits so we need to use 64 bits values for the parsing
+ */
+#if defined(CONFIG_PPC32) || defined(CONFIG_PPC64) || defined(CONFIG_MICROBLAZE)
+void pci_process_bridge_OF_ranges(struct pci_controller *hose,
+				  struct device_node *dev, int primary)
+{
+	const u32 *ranges;
+	int rlen;
+	int pna = of_n_addr_cells(dev);
+	int np = pna + 5;
+	int memno = 0, isa_hole = -1;
+	u32 pci_space;
+	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
+	unsigned long long isa_mb = 0;
+	struct resource *res;
+
+	pr_info("PCI host bridge %s %s ranges:\n",
+	       dev->full_name, primary ? "(primary)" : "");
+
+	/* Get ranges property */
+	ranges = of_get_property(dev, "ranges", &rlen);
+	if (ranges == NULL)
+		return;
+
+	/* Parse it */
+	pr_debug("Parsing ranges property...\n");
+	while ((rlen -= np * 4) >= 0) {
+		/* Read next ranges element */
+		pci_space = ranges[0];
+		pci_addr = of_read_number(ranges + 1, 2);
+		cpu_addr = of_translate_address(dev, ranges + 3);
+		size = of_read_number(ranges + pna + 3, 2);
+
+		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
+				pci_space, pci_addr);
+		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
+					cpu_addr, size);
+
+		ranges += np;
+
+		/* If we failed translation or got a zero-sized region
+		 * (some FW try to feed us with non sensical zero sized regions
+		 * such as power3 which look like some kind of attempt
+		 * at exposing the VGA memory hole)
+		 */
+		if (cpu_addr == OF_BAD_ADDR || size == 0)
+			continue;
+
+		/* Now consume following elements while they are contiguous */
+		for (; rlen >= np * sizeof(u32);
+		     ranges += np, rlen -= np * 4) {
+			if (ranges[0] != pci_space)
+				break;
+			pci_next = of_read_number(ranges + 1, 2);
+			cpu_next = of_translate_address(dev, ranges + 3);
+			if (pci_next != pci_addr + size ||
+			    cpu_next != cpu_addr + size)
+				break;
+			size += of_read_number(ranges + pna + 3, 2);
+		}
+
+		/* Act based on address space type */
+		res = NULL;
+		switch ((pci_space >> 24) & 0x3) {
+		case 1:		/* PCI IO space */
+			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
+			       cpu_addr, cpu_addr + size - 1, pci_addr);
+
+			/* We support only one IO range */
+			if (hose->pci_io_size) {
+				pr_info(" \\--> Skipped (too many) !\n");
+				continue;
+			}
+#if (!IS_ENABLED(CONFIG_64BIT))
+			/* On 32 bits, limit I/O space to 16MB */
+			if (size > 0x01000000)
+				size = 0x01000000;
+
+			/* 32 bits needs to map IOs here */
+			hose->io_base_virt = ioremap(cpu_addr, size);
+
+			/* Expect trouble if pci_addr is not 0 */
+			if (primary)
+				isa_io_base =
+					(unsigned long)hose->io_base_virt;
+#endif /* !CONFIG_64BIT */
+			/* pci_io_size and io_base_phys always represent IO
+			 * space starting at 0 so we factor in pci_addr
+			 */
+			hose->pci_io_size = pci_addr + size;
+			hose->io_base_phys = cpu_addr - pci_addr;
+
+			/* Build resource */
+			res = &hose->io_resource;
+			res->flags = IORESOURCE_IO;
+			res->start = pci_addr;
+			break;
+		case 2:		/* PCI Memory space */
+		case 3:		/* PCI 64 bits Memory space */
+			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
+			       cpu_addr, cpu_addr + size - 1, pci_addr,
+			       (pci_space & 0x40000000) ? "Prefetch" : "");
+
+			/* We support only 3 memory ranges */
+			if (memno >= 3) {
+				pr_info(" \\--> Skipped (too many) !\n");
+				continue;
+			}
+			/* Handles ISA memory hole space here */
+			if (pci_addr == 0) {
+				isa_mb = cpu_addr;
+				isa_hole = memno;
+				if (primary || isa_mem_base == 0)
+					isa_mem_base = cpu_addr;
+				hose->isa_mem_phys = cpu_addr;
+				hose->isa_mem_size = size;
+			}
+
+			/* We get the PCI/Mem offset from the first range or
+			 * the, current one if the offset came from an ISA
+			 * hole. If they don't match, bugger.
+			 */
+			if (memno == 0 ||
+			    (isa_hole >= 0 && pci_addr != 0 &&
+			     hose->pci_mem_offset == isa_mb))
+				hose->pci_mem_offset = cpu_addr - pci_addr;
+			else if (pci_addr != 0 &&
+				 hose->pci_mem_offset != cpu_addr - pci_addr) {
+				pr_info(" \\--> Skipped (offset mismatch) !\n");
+				continue;
+			}
+
+			/* Build resource */
+			res = &hose->mem_resources[memno++];
+			res->flags = IORESOURCE_MEM;
+			if (pci_space & 0x40000000)
+				res->flags |= IORESOURCE_PREFETCH;
+			res->start = cpu_addr;
+			break;
+		}
+		if (res != NULL) {
+			res->name = dev->full_name;
+			res->end = res->start + size - 1;
+			res->parent = NULL;
+			res->sibling = NULL;
+			res->child = NULL;
+		}
+	}
+
+	/* If there's an ISA hole and the pci_mem_offset is -not- matching
+	 * the ISA hole offset, then we need to remove the ISA hole from
+	 * the resource list for that brige
+	 */
+	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
+		unsigned int next = isa_hole + 1;
+		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
+		if (next < memno)
+			memmove(&hose->mem_resources[isa_hole],
+				&hose->mem_resources[next],
+				sizeof(struct resource) * (memno - next));
+		hose->mem_resources[--memno].flags = 0;
+	}
+}
+#endif
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index bb115de..33e8ead 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -11,4 +11,8 @@ struct device_node;
 struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
 
+struct pci_controller;
+void pci_process_bridge_OF_ranges(struct pci_controller *hose,
+			struct device_node *dev, int primary);
+
 #endif
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 2/3] of/pci: Provide support for parsing PCI DT ranges property
  2013-04-11 15:26 [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing Andrew Murray
  2013-04-11 15:26 ` [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC Andrew Murray
@ 2013-04-11 15:26 ` Andrew Murray
  2013-04-11 15:26 ` [PATCH v6 3/3] of/pci: mips: convert to common of_pci_range_parser Andrew Murray
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Andrew Murray @ 2013-04-11 15:26 UTC (permalink / raw)
  To: rob.herring
  Cc: jgunthorpe, linux, siva.kallam, linux-pci, devicetree-discuss,
	jg1.han, Liviu.Dudau, linux-kernel, linux-samsung-soc, kgene.kim,
	bhelgaas, suren.reddy, linux-arm-kernel, monstr, benh, paulus,
	grant.likely, thomas.petazzoni, thierry.reding, thomas.abraham,
	arnd, linus.walleij, Andrew Murray

This patch factors out common implementation patterns to reduce overall kernel
code and provide a means for host bridge drivers to directly obtain struct
resources from the DT's ranges property without relying on architecture specific
DT handling. This will make it easier to write archiecture independent host bridge
drivers and mitigate against further duplication of DT parsing code.

This patch can be used in the following way:

	struct of_pci_range_parser parser;
	struct of_pci_range range;

	if (of_pci_range_parser(&parser, np))
		; //no ranges property

	for_each_of_pci_range(&parser, &range) {

		/*
			directly access properties of the address range, e.g.:
			range.pci_space, range.pci_addr, range.cpu_addr,
			range.size, range.flags

			alternatively obtain a struct resource, e.g.:
			struct resource res;
			of_pci_range_to_resource(&range, np, &res);
		*/
	}

Additionally the implementation takes care of adjacent ranges and merges them
into a single range (as was the case with powerpc and microblaze).

Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Reviewed-by: Rob Herring <rob.herring@calxeda.com>
Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/of/address.c       |   66 ++++++++++++++++++++++++++
 drivers/of/of_pci.c        |  112 ++++++++++++++++----------------------------
 include/linux/of_address.h |   46 ++++++++++++++++++
 3 files changed, 152 insertions(+), 72 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 04da786..d3c4f2f 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -227,6 +227,72 @@ int of_pci_address_to_resource(struct device_node *dev, int bar,
 	return __of_address_to_resource(dev, addrp, size, flags, NULL, r);
 }
 EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
+
+int of_pci_range_parser(struct of_pci_range_parser *parser,
+			struct device_node *node)
+{
+	const int na = 3, ns = 2;
+	int rlen;
+
+	parser->node = node;
+	parser->pna = of_n_addr_cells(node);
+	parser->np = parser->pna + na + ns;
+
+	parser->range = of_get_property(node, "ranges", &rlen);
+	if (parser->range == NULL)
+		return -ENOENT;
+
+	parser->end = parser->range + rlen / sizeof(__be32);
+
+	return 0;
+}
+
+struct of_pci_range *of_pci_process_ranges(struct of_pci_range_parser *parser,
+						struct of_pci_range *range)
+{
+	const int na = 3, ns = 2;
+
+	if (!range)
+		return NULL;
+
+	if (!parser->range || parser->range + parser->np > parser->end)
+		return NULL;
+
+	range->pci_space = parser->range[0];
+	range->flags = of_bus_pci_get_flags(parser->range);
+	range->pci_addr = of_read_number(parser->range + 1, ns);
+	range->cpu_addr = of_translate_address(parser->node,
+				parser->range + na);
+	range->size = of_read_number(parser->range + parser->pna + na, ns);
+
+	parser->range += parser->np;
+
+	/* Now consume following elements while they are contiguous */
+	while (parser->range + parser->np <= parser->end) {
+		u32 flags, pci_space;
+		u64 pci_addr, cpu_addr, size;
+
+		pci_space = be32_to_cpup(parser->range);
+		flags = of_bus_pci_get_flags(parser->range);
+		pci_addr = of_read_number(parser->range + 1, ns);
+		cpu_addr = of_translate_address(parser->node,
+				parser->range + na);
+		size = of_read_number(parser->range + parser->pna + na, ns);
+
+		if (flags != range->flags)
+			break;
+		if (pci_addr != range->pci_addr + range->size ||
+		    cpu_addr != range->cpu_addr + range->size)
+			break;
+
+		range->size += size;
+		parser->range += parser->np;
+	}
+
+	return range;
+}
+EXPORT_SYMBOL_GPL(of_pci_process_ranges);
+
 #endif /* CONFIG_PCI */
 
 /*
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 1626172..3e428a1 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -82,67 +82,43 @@ EXPORT_SYMBOL_GPL(of_pci_find_child_device);
 void pci_process_bridge_OF_ranges(struct pci_controller *hose,
 				  struct device_node *dev, int primary)
 {
-	const u32 *ranges;
-	int rlen;
-	int pna = of_n_addr_cells(dev);
-	int np = pna + 5;
 	int memno = 0, isa_hole = -1;
-	u32 pci_space;
-	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
 	unsigned long long isa_mb = 0;
 	struct resource *res;
+	struct of_pci_range range;
+	struct of_pci_range_parser parser;
+	u32 res_type;
 
 	pr_info("PCI host bridge %s %s ranges:\n",
 	       dev->full_name, primary ? "(primary)" : "");
 
-	/* Get ranges property */
-	ranges = of_get_property(dev, "ranges", &rlen);
-	if (ranges == NULL)
+	/* Check for ranges property */
+	if (of_pci_range_parser(&parser, dev))
 		return;
 
-	/* Parse it */
 	pr_debug("Parsing ranges property...\n");
-	while ((rlen -= np * 4) >= 0) {
+	for_each_of_pci_range(&parser, &range) {
 		/* Read next ranges element */
-		pci_space = ranges[0];
-		pci_addr = of_read_number(ranges + 1, 2);
-		cpu_addr = of_translate_address(dev, ranges + 3);
-		size = of_read_number(ranges + pna + 3, 2);
-
-		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
-				pci_space, pci_addr);
-		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
-					cpu_addr, size);
-
-		ranges += np;
+		pr_debug("pci_space: 0x%08x pci_addr: 0x%016llx ",
+				range.pci_space, range.pci_addr);
+		pr_debug("cpu_addr: 0x%016llx size: 0x%016llx\n",
+				range.cpu_addr, range.size);
 
 		/* If we failed translation or got a zero-sized region
 		 * (some FW try to feed us with non sensical zero sized regions
 		 * such as power3 which look like some kind of attempt
 		 * at exposing the VGA memory hole)
 		 */
-		if (cpu_addr == OF_BAD_ADDR || size == 0)
+		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
 			continue;
 
-		/* Now consume following elements while they are contiguous */
-		for (; rlen >= np * sizeof(u32);
-		     ranges += np, rlen -= np * 4) {
-			if (ranges[0] != pci_space)
-				break;
-			pci_next = of_read_number(ranges + 1, 2);
-			cpu_next = of_translate_address(dev, ranges + 3);
-			if (pci_next != pci_addr + size ||
-			    cpu_next != cpu_addr + size)
-				break;
-			size += of_read_number(ranges + pna + 3, 2);
-		}
-
 		/* Act based on address space type */
 		res = NULL;
-		switch ((pci_space >> 24) & 0x3) {
-		case 1:		/* PCI IO space */
+		res_type = range.flags & IORESOURCE_TYPE_BITS;
+		if (res_type == IORESOURCE_IO) {
 			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
-			       cpu_addr, cpu_addr + size - 1, pci_addr);
+				range.cpu_addr, range.cpu_addr + range.size - 1,
+				range.pci_addr);
 
 			/* We support only one IO range */
 			if (hose->pci_io_size) {
@@ -151,11 +127,12 @@ void pci_process_bridge_OF_ranges(struct pci_controller *hose,
 			}
 #if (!IS_ENABLED(CONFIG_64BIT))
 			/* On 32 bits, limit I/O space to 16MB */
-			if (size > 0x01000000)
-				size = 0x01000000;
+			if (range.size > 0x01000000)
+				range.size = 0x01000000;
 
 			/* 32 bits needs to map IOs here */
-			hose->io_base_virt = ioremap(cpu_addr, size);
+			hose->io_base_virt = ioremap(range.cpu_addr,
+						range.size);
 
 			/* Expect trouble if pci_addr is not 0 */
 			if (primary)
@@ -165,19 +142,18 @@ void pci_process_bridge_OF_ranges(struct pci_controller *hose,
 			/* pci_io_size and io_base_phys always represent IO
 			 * space starting at 0 so we factor in pci_addr
 			 */
-			hose->pci_io_size = pci_addr + size;
-			hose->io_base_phys = cpu_addr - pci_addr;
+			hose->pci_io_size = range.pci_addr + range.size;
+			hose->io_base_phys = range.cpu_addr - range.pci_addr;
 
 			/* Build resource */
 			res = &hose->io_resource;
-			res->flags = IORESOURCE_IO;
-			res->start = pci_addr;
-			break;
-		case 2:		/* PCI Memory space */
-		case 3:		/* PCI 64 bits Memory space */
+			range.cpu_addr = range.pci_addr;
+		} else if (res_type == IORESOURCE_MEM) {
 			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
-			       cpu_addr, cpu_addr + size - 1, pci_addr,
-			       (pci_space & 0x40000000) ? "Prefetch" : "");
+				range.cpu_addr, range.cpu_addr + range.size - 1,
+				range.pci_addr,
+				(range.pci_space & 0x40000000) ?
+				"Prefetch" : "");
 
 			/* We support only 3 memory ranges */
 			if (memno >= 3) {
@@ -185,13 +161,13 @@ void pci_process_bridge_OF_ranges(struct pci_controller *hose,
 				continue;
 			}
 			/* Handles ISA memory hole space here */
-			if (pci_addr == 0) {
-				isa_mb = cpu_addr;
+			if (range.pci_addr == 0) {
+				isa_mb = range.cpu_addr;
 				isa_hole = memno;
 				if (primary || isa_mem_base == 0)
-					isa_mem_base = cpu_addr;
-				hose->isa_mem_phys = cpu_addr;
-				hose->isa_mem_size = size;
+					isa_mem_base = range.cpu_addr;
+				hose->isa_mem_phys = range.cpu_addr;
+				hose->isa_mem_size = range.size;
 			}
 
 			/* We get the PCI/Mem offset from the first range or
@@ -199,30 +175,22 @@ void pci_process_bridge_OF_ranges(struct pci_controller *hose,
 			 * hole. If they don't match, bugger.
 			 */
 			if (memno == 0 ||
-			    (isa_hole >= 0 && pci_addr != 0 &&
+			(isa_hole >= 0 && range.pci_addr != 0 &&
 			     hose->pci_mem_offset == isa_mb))
-				hose->pci_mem_offset = cpu_addr - pci_addr;
-			else if (pci_addr != 0 &&
-				 hose->pci_mem_offset != cpu_addr - pci_addr) {
+				hose->pci_mem_offset = range.cpu_addr -
+							range.pci_addr;
+			else if (range.pci_addr != 0 &&
+				hose->pci_mem_offset != range.cpu_addr -
+							range.pci_addr) {
 				pr_info(" \\--> Skipped (offset mismatch) !\n");
 				continue;
 			}
 
 			/* Build resource */
 			res = &hose->mem_resources[memno++];
-			res->flags = IORESOURCE_MEM;
-			if (pci_space & 0x40000000)
-				res->flags |= IORESOURCE_PREFETCH;
-			res->start = cpu_addr;
-			break;
-		}
-		if (res != NULL) {
-			res->name = dev->full_name;
-			res->end = res->start + size - 1;
-			res->parent = NULL;
-			res->sibling = NULL;
-			res->child = NULL;
 		}
+		if (res != NULL)
+			of_pci_range_to_resource(&range, dev, res);
 	}
 
 	/* If there's an ISA hole and the pci_mem_offset is -not- matching
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 0506eb5..9702255 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -4,6 +4,36 @@
 #include <linux/errno.h>
 #include <linux/of.h>
 
+struct of_pci_range_parser {
+	struct device_node *node;
+	const __be32 *range;
+	const __be32 *end;
+	int np;
+	int pna;
+};
+
+struct of_pci_range {
+	u32 pci_space;
+	u64 pci_addr;
+	u64 cpu_addr;
+	u64 size;
+	u32 flags;
+};
+
+#define for_each_of_pci_range(parser, range) \
+	for (; of_pci_process_ranges(parser, range);)
+
+static inline void of_pci_range_to_resource(struct of_pci_range *range,
+					    struct device_node *np,
+					    struct resource *res)
+{
+	res->flags = range->flags;
+	res->start = range->cpu_addr;
+	res->end = range->cpu_addr + range->size - 1;
+	res->parent = res->child = res->sibling = NULL;
+	res->name = np->full_name;
+}
+
 #ifdef CONFIG_OF_ADDRESS
 extern u64 of_translate_address(struct device_node *np, const __be32 *addr);
 extern bool of_can_translate_address(struct device_node *dev);
@@ -27,6 +57,10 @@ static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; }
 #define pci_address_to_pio pci_address_to_pio
 #endif
 
+int of_pci_range_parser(struct of_pci_range_parser *parser,
+			struct device_node *node);
+struct of_pci_range *of_pci_process_ranges(struct of_pci_range_parser *parser,
+						struct of_pci_range *range);
 #else /* CONFIG_OF_ADDRESS */
 #ifndef of_address_to_resource
 static inline int of_address_to_resource(struct device_node *dev, int index,
@@ -53,6 +87,18 @@ static inline const __be32 *of_get_address(struct device_node *dev, int index,
 {
 	return NULL;
 }
+
+int of_pci_range_parser(struct of_pci_range_parser *parser,
+			struct device_noed *node)
+{
+	return -1;
+}
+
+struct of_pci_range *of_pci_process_ranges(struct of_pci_range_parser *parser,
+						struct of_pci_range *range)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF_ADDRESS */
 
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 3/3] of/pci: mips: convert to common of_pci_range_parser
  2013-04-11 15:26 [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing Andrew Murray
  2013-04-11 15:26 ` [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC Andrew Murray
  2013-04-11 15:26 ` [PATCH v6 2/3] of/pci: Provide support for parsing PCI DT ranges property Andrew Murray
@ 2013-04-11 15:26 ` Andrew Murray
  2013-04-11 16:55 ` [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing Linus Walleij
  2013-04-15 17:56 ` Jason Cooper
  4 siblings, 0 replies; 12+ messages in thread
From: Andrew Murray @ 2013-04-11 15:26 UTC (permalink / raw)
  To: rob.herring
  Cc: jgunthorpe, linux, siva.kallam, linux-pci, devicetree-discuss,
	jg1.han, Liviu.Dudau, linux-kernel, linux-samsung-soc, kgene.kim,
	bhelgaas, suren.reddy, linux-arm-kernel, monstr, benh, paulus,
	grant.likely, thomas.petazzoni, thierry.reding, thomas.abraham,
	arnd, linus.walleij, Andrew Murray

This patch converts the pci_load_of_ranges function to use the new common
of_pci_range_parser.

Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Reviewed-by: Rob Herring <rob.herring@calxeda.com>
---
 arch/mips/pci/pci.c |   50 ++++++++++++++++----------------------------------
 1 files changed, 16 insertions(+), 34 deletions(-)

diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 0872f12..bee49a4 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -122,51 +122,33 @@ static void pcibios_scanbus(struct pci_controller *hose)
 #ifdef CONFIG_OF
 void pci_load_of_ranges(struct pci_controller *hose, struct device_node *node)
 {
-	const __be32 *ranges;
-	int rlen;
-	int pna = of_n_addr_cells(node);
-	int np = pna + 5;
+	struct of_pci_range_range range;
+	struct of_pci_range_parser parser;
+	u32 res_type;
 
 	pr_info("PCI host bridge %s ranges:\n", node->full_name);
-	ranges = of_get_property(node, "ranges", &rlen);
-	if (ranges == NULL)
-		return;
 	hose->of_node = node;
 
-	while ((rlen -= np * 4) >= 0) {
-		u32 pci_space;
+	if (of_pci_range_parser(&parser, node))
+		return;
+
+	for_each_of_pci_range(&parser, &range) {
 		struct resource *res = NULL;
-		u64 addr, size;
-
-		pci_space = be32_to_cpup(&ranges[0]);
-		addr = of_translate_address(node, ranges + 3);
-		size = of_read_number(ranges + pna + 3, 2);
-		ranges += np;
-		switch ((pci_space >> 24) & 0x3) {
-		case 1:		/* PCI IO space */
+
+		res_type = range.flags & IORESOURCE_TYPE_BITS;
+		if (res_type == IORESOURCE_IO) {
 			pr_info("  IO 0x%016llx..0x%016llx\n",
-					addr, addr + size - 1);
+				range.addr, range.addr + range.size - 1);
 			hose->io_map_base =
-				(unsigned long)ioremap(addr, size);
+				(unsigned long)ioremap(range.addr, range.size);
 			res = hose->io_resource;
-			res->flags = IORESOURCE_IO;
-			break;
-		case 2:		/* PCI Memory space */
-		case 3:		/* PCI 64 bits Memory space */
+		} else if (res_type == IORESOURCE_MEM) {
 			pr_info(" MEM 0x%016llx..0x%016llx\n",
-					addr, addr + size - 1);
+				range.addr, range.addr + range.size - 1);
 			res = hose->mem_resource;
-			res->flags = IORESOURCE_MEM;
-			break;
-		}
-		if (res != NULL) {
-			res->start = addr;
-			res->name = node->full_name;
-			res->end = res->start + size - 1;
-			res->parent = NULL;
-			res->sibling = NULL;
-			res->child = NULL;
 		}
+		if (res != NULL)
+			of_pci_range_to_resource(&range, node, res);
 	}
 }
 #endif
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing
  2013-04-11 15:26 [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing Andrew Murray
                   ` (2 preceding siblings ...)
  2013-04-11 15:26 ` [PATCH v6 3/3] of/pci: mips: convert to common of_pci_range_parser Andrew Murray
@ 2013-04-11 16:55 ` Linus Walleij
  2013-04-15 17:56 ` Jason Cooper
  4 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2013-04-11 16:55 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Rob Herring, Jason Gunthorpe, Russell King - ARM Linux,
	siva.kallam, linux-pci, devicetree-discuss, Jingoo Han,
	Liviu.Dudau, linux-kernel, linux-samsung-soc, Kukjin Kim,
	bhelgaas, suren.reddy, linux-arm-kernel, Michal Simek,
	Benjamin Herrenschmidt, Paul Mackerras, Grant Likely,
	Thomas Petazzoni, Thierry Reding, Thomas Abraham, Arnd Bergmann

On Thu, Apr 11, 2013 at 5:26 PM, Andrew Murray <Andrew.Murray@arm.com> wrote:

> This patchset factors out duplicated code associated with parsing PCI
> DT "ranges" properties across the architectures and introduces a
> "ranges" parser. This parser "of_pci_range_parser" can be used directly
> by ARM host bridge drivers enabling them to obtain ranges from device
> trees.
>
> I've included the Reviewed-by and Tested-by's received from v5 in this
> patchset, earlier versions of this patchset (v3) have been tested-by:
>
> Thierry Reding <thierry.reding@avionic-design.de>
> Jingoo Han <jg1.han@samsung.com>
>
> I believe a version of this patchset has also been tested through its
> inclusion in Thomas Petazzoni's Armada 370 and Armada XP SoCs PCIe support by:
>
> Linus Walleij <linus.walleij@linaro.org>

Yes you can add my Tested-by tag.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
  2013-04-11 15:26 ` [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC Andrew Murray
@ 2013-04-15 12:57   ` Thomas Petazzoni
  2013-04-15 15:32     ` Benjamin Herrenschmidt
  2013-04-15 16:50     ` Michal Simek
  0 siblings, 2 replies; 12+ messages in thread
From: Thomas Petazzoni @ 2013-04-15 12:57 UTC (permalink / raw)
  To: monstr, benh
  Cc: Andrew Murray, rob.herring, jgunthorpe, linux, siva.kallam,
	linux-pci, devicetree-discuss, jg1.han, Liviu.Dudau,
	linux-kernel, linux-samsung-soc, kgene.kim, bhelgaas,
	suren.reddy, linux-arm-kernel, paulus, grant.likely,
	thierry.reding, thomas.abraham, arnd, linus.walleij

Michal, Ben,

Would you have some time to look at this patch and give your comments
and/or ACK ? Since it touches the PowerPC and Microblaze core code, we
need your agreement to merge this code, and quite a bit of code pending
for 3.10 depends on this patch.

Rob, alternatively, could we imagine doing a different version of the
'of/pci: Provide support for parsing PCI DT ranges property' that
introduces the new API only, leaving the PowerPC and Microblaze rework
as follow-up efforts, so that all the PCIe drivers that depend on this
patch can get in for 3.10 ? I'd find it pretty sad if the Marvell PCIe
driver that has been worked on since 4+ months does not get into 3.10
just because this patch cannot be merged.

Thanks!

Thomas

On Thu, 11 Apr 2013 16:26:07 +0100, Andrew Murray wrote:
> The pci_process_bridge_OF_ranges function, used to parse the "ranges"
> property of a PCI host device, is found in both Microblaze and PowerPC
> architectures. These implementations are nearly identical. This patch
> moves this common code to a common place.
> 
> Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> Reviewed-by: Rob Herring <rob.herring@calxeda.com>
> Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/microblaze/include/asm/pci-bridge.h |    5 +-
>  arch/microblaze/pci/pci-common.c         |  192 ----------------------------
>  arch/powerpc/include/asm/pci-bridge.h    |    5 +-
>  arch/powerpc/kernel/pci-common.c         |  192 ----------------------------
>  drivers/of/of_pci.c                      |  200 ++++++++++++++++++++++++++++++
>  include/linux/of_pci.h                   |    4 +
>  6 files changed, 206 insertions(+), 392 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/pci-bridge.h b/arch/microblaze/include/asm/pci-bridge.h
> index cb5d397..5783cd6 100644
> --- a/arch/microblaze/include/asm/pci-bridge.h
> +++ b/arch/microblaze/include/asm/pci-bridge.h
> @@ -10,6 +10,7 @@
>  #include <linux/pci.h>
>  #include <linux/list.h>
>  #include <linux/ioport.h>
> +#include <linux/of_pci.h>
>  
>  struct device_node;
>  
> @@ -132,10 +133,6 @@ extern void setup_indirect_pci(struct pci_controller *hose,
>  extern struct pci_controller *pci_find_hose_for_OF_device(
>  			struct device_node *node);
>  
> -/* Fill up host controller resources from the OF node */
> -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> -			struct device_node *dev, int primary);
> -
>  /* Allocate & free a PCI host bridge structure */
>  extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
>  extern void pcibios_free_controller(struct pci_controller *phb);
> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> index 9ea521e..2735ad9 100644
> --- a/arch/microblaze/pci/pci-common.c
> +++ b/arch/microblaze/pci/pci-common.c
> @@ -622,198 +622,6 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
>  	*end = rsrc->end - offset;
>  }
>  
> -/**
> - * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
> - * @hose: newly allocated pci_controller to be setup
> - * @dev: device node of the host bridge
> - * @primary: set if primary bus (32 bits only, soon to be deprecated)
> - *
> - * This function will parse the "ranges" property of a PCI host bridge device
> - * node and setup the resource mapping of a pci controller based on its
> - * content.
> - *
> - * Life would be boring if it wasn't for a few issues that we have to deal
> - * with here:
> - *
> - *   - We can only cope with one IO space range and up to 3 Memory space
> - *     ranges. However, some machines (thanks Apple !) tend to split their
> - *     space into lots of small contiguous ranges. So we have to coalesce.
> - *
> - *   - We can only cope with all memory ranges having the same offset
> - *     between CPU addresses and PCI addresses. Unfortunately, some bridges
> - *     are setup for a large 1:1 mapping along with a small "window" which
> - *     maps PCI address 0 to some arbitrary high address of the CPU space in
> - *     order to give access to the ISA memory hole.
> - *     The way out of here that I've chosen for now is to always set the
> - *     offset based on the first resource found, then override it if we
> - *     have a different offset and the previous was set by an ISA hole.
> - *
> - *   - Some busses have IO space not starting at 0, which causes trouble with
> - *     the way we do our IO resource renumbering. The code somewhat deals with
> - *     it for 64 bits but I would expect problems on 32 bits.
> - *
> - *   - Some 32 bits platforms such as 4xx can have physical space larger than
> - *     32 bits so we need to use 64 bits values for the parsing
> - */
> -void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> -				  struct device_node *dev, int primary)
> -{
> -	const u32 *ranges;
> -	int rlen;
> -	int pna = of_n_addr_cells(dev);
> -	int np = pna + 5;
> -	int memno = 0, isa_hole = -1;
> -	u32 pci_space;
> -	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
> -	unsigned long long isa_mb = 0;
> -	struct resource *res;
> -
> -	pr_info("PCI host bridge %s %s ranges:\n",
> -	       dev->full_name, primary ? "(primary)" : "");
> -
> -	/* Get ranges property */
> -	ranges = of_get_property(dev, "ranges", &rlen);
> -	if (ranges == NULL)
> -		return;
> -
> -	/* Parse it */
> -	pr_debug("Parsing ranges property...\n");
> -	while ((rlen -= np * 4) >= 0) {
> -		/* Read next ranges element */
> -		pci_space = ranges[0];
> -		pci_addr = of_read_number(ranges + 1, 2);
> -		cpu_addr = of_translate_address(dev, ranges + 3);
> -		size = of_read_number(ranges + pna + 3, 2);
> -
> -		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
> -				pci_space, pci_addr);
> -		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
> -					cpu_addr, size);
> -
> -		ranges += np;
> -
> -		/* If we failed translation or got a zero-sized region
> -		 * (some FW try to feed us with non sensical zero sized regions
> -		 * such as power3 which look like some kind of attempt
> -		 * at exposing the VGA memory hole)
> -		 */
> -		if (cpu_addr == OF_BAD_ADDR || size == 0)
> -			continue;
> -
> -		/* Now consume following elements while they are contiguous */
> -		for (; rlen >= np * sizeof(u32);
> -		     ranges += np, rlen -= np * 4) {
> -			if (ranges[0] != pci_space)
> -				break;
> -			pci_next = of_read_number(ranges + 1, 2);
> -			cpu_next = of_translate_address(dev, ranges + 3);
> -			if (pci_next != pci_addr + size ||
> -			    cpu_next != cpu_addr + size)
> -				break;
> -			size += of_read_number(ranges + pna + 3, 2);
> -		}
> -
> -		/* Act based on address space type */
> -		res = NULL;
> -		switch ((pci_space >> 24) & 0x3) {
> -		case 1:		/* PCI IO space */
> -			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
> -			       cpu_addr, cpu_addr + size - 1, pci_addr);
> -
> -			/* We support only one IO range */
> -			if (hose->pci_io_size) {
> -				pr_info(" \\--> Skipped (too many) !\n");
> -				continue;
> -			}
> -			/* On 32 bits, limit I/O space to 16MB */
> -			if (size > 0x01000000)
> -				size = 0x01000000;
> -
> -			/* 32 bits needs to map IOs here */
> -			hose->io_base_virt = ioremap(cpu_addr, size);
> -
> -			/* Expect trouble if pci_addr is not 0 */
> -			if (primary)
> -				isa_io_base =
> -					(unsigned long)hose->io_base_virt;
> -			/* pci_io_size and io_base_phys always represent IO
> -			 * space starting at 0 so we factor in pci_addr
> -			 */
> -			hose->pci_io_size = pci_addr + size;
> -			hose->io_base_phys = cpu_addr - pci_addr;
> -
> -			/* Build resource */
> -			res = &hose->io_resource;
> -			res->flags = IORESOURCE_IO;
> -			res->start = pci_addr;
> -			break;
> -		case 2:		/* PCI Memory space */
> -		case 3:		/* PCI 64 bits Memory space */
> -			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
> -			       cpu_addr, cpu_addr + size - 1, pci_addr,
> -			       (pci_space & 0x40000000) ? "Prefetch" : "");
> -
> -			/* We support only 3 memory ranges */
> -			if (memno >= 3) {
> -				pr_info(" \\--> Skipped (too many) !\n");
> -				continue;
> -			}
> -			/* Handles ISA memory hole space here */
> -			if (pci_addr == 0) {
> -				isa_mb = cpu_addr;
> -				isa_hole = memno;
> -				if (primary || isa_mem_base == 0)
> -					isa_mem_base = cpu_addr;
> -				hose->isa_mem_phys = cpu_addr;
> -				hose->isa_mem_size = size;
> -			}
> -
> -			/* We get the PCI/Mem offset from the first range or
> -			 * the, current one if the offset came from an ISA
> -			 * hole. If they don't match, bugger.
> -			 */
> -			if (memno == 0 ||
> -			    (isa_hole >= 0 && pci_addr != 0 &&
> -			     hose->pci_mem_offset == isa_mb))
> -				hose->pci_mem_offset = cpu_addr - pci_addr;
> -			else if (pci_addr != 0 &&
> -				 hose->pci_mem_offset != cpu_addr - pci_addr) {
> -				pr_info(" \\--> Skipped (offset mismatch) !\n");
> -				continue;
> -			}
> -
> -			/* Build resource */
> -			res = &hose->mem_resources[memno++];
> -			res->flags = IORESOURCE_MEM;
> -			if (pci_space & 0x40000000)
> -				res->flags |= IORESOURCE_PREFETCH;
> -			res->start = cpu_addr;
> -			break;
> -		}
> -		if (res != NULL) {
> -			res->name = dev->full_name;
> -			res->end = res->start + size - 1;
> -			res->parent = NULL;
> -			res->sibling = NULL;
> -			res->child = NULL;
> -		}
> -	}
> -
> -	/* If there's an ISA hole and the pci_mem_offset is -not- matching
> -	 * the ISA hole offset, then we need to remove the ISA hole from
> -	 * the resource list for that brige
> -	 */
> -	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
> -		unsigned int next = isa_hole + 1;
> -		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
> -		if (next < memno)
> -			memmove(&hose->mem_resources[isa_hole],
> -				&hose->mem_resources[next],
> -				sizeof(struct resource) * (memno - next));
> -		hose->mem_resources[--memno].flags = 0;
> -	}
> -}
> -
>  /* Decide whether to display the domain number in /proc */
>  int pci_proc_domain(struct pci_bus *bus)
>  {
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 025a130..205bfba 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -10,6 +10,7 @@
>  #include <linux/pci.h>
>  #include <linux/list.h>
>  #include <linux/ioport.h>
> +#include <linux/of_pci.h>
>  #include <asm-generic/pci-bridge.h>
>  
>  struct device_node;
> @@ -231,10 +232,6 @@ extern int pcibios_map_io_space(struct pci_bus *bus);
>  extern struct pci_controller *pci_find_hose_for_OF_device(
>  			struct device_node* node);
>  
> -/* Fill up host controller resources from the OF node */
> -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> -			struct device_node *dev, int primary);
> -
>  /* Allocate & free a PCI host bridge structure */
>  extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
>  extern void pcibios_free_controller(struct pci_controller *phb);
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index fa12ae4..6edf396 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -640,198 +640,6 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
>  	*end = rsrc->end - offset;
>  }
>  
> -/**
> - * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
> - * @hose: newly allocated pci_controller to be setup
> - * @dev: device node of the host bridge
> - * @primary: set if primary bus (32 bits only, soon to be deprecated)
> - *
> - * This function will parse the "ranges" property of a PCI host bridge device
> - * node and setup the resource mapping of a pci controller based on its
> - * content.
> - *
> - * Life would be boring if it wasn't for a few issues that we have to deal
> - * with here:
> - *
> - *   - We can only cope with one IO space range and up to 3 Memory space
> - *     ranges. However, some machines (thanks Apple !) tend to split their
> - *     space into lots of small contiguous ranges. So we have to coalesce.
> - *
> - *   - We can only cope with all memory ranges having the same offset
> - *     between CPU addresses and PCI addresses. Unfortunately, some bridges
> - *     are setup for a large 1:1 mapping along with a small "window" which
> - *     maps PCI address 0 to some arbitrary high address of the CPU space in
> - *     order to give access to the ISA memory hole.
> - *     The way out of here that I've chosen for now is to always set the
> - *     offset based on the first resource found, then override it if we
> - *     have a different offset and the previous was set by an ISA hole.
> - *
> - *   - Some busses have IO space not starting at 0, which causes trouble with
> - *     the way we do our IO resource renumbering. The code somewhat deals with
> - *     it for 64 bits but I would expect problems on 32 bits.
> - *
> - *   - Some 32 bits platforms such as 4xx can have physical space larger than
> - *     32 bits so we need to use 64 bits values for the parsing
> - */
> -void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> -				  struct device_node *dev, int primary)
> -{
> -	const u32 *ranges;
> -	int rlen;
> -	int pna = of_n_addr_cells(dev);
> -	int np = pna + 5;
> -	int memno = 0, isa_hole = -1;
> -	u32 pci_space;
> -	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
> -	unsigned long long isa_mb = 0;
> -	struct resource *res;
> -
> -	printk(KERN_INFO "PCI host bridge %s %s ranges:\n",
> -	       dev->full_name, primary ? "(primary)" : "");
> -
> -	/* Get ranges property */
> -	ranges = of_get_property(dev, "ranges", &rlen);
> -	if (ranges == NULL)
> -		return;
> -
> -	/* Parse it */
> -	while ((rlen -= np * 4) >= 0) {
> -		/* Read next ranges element */
> -		pci_space = ranges[0];
> -		pci_addr = of_read_number(ranges + 1, 2);
> -		cpu_addr = of_translate_address(dev, ranges + 3);
> -		size = of_read_number(ranges + pna + 3, 2);
> -		ranges += np;
> -
> -		/* If we failed translation or got a zero-sized region
> -		 * (some FW try to feed us with non sensical zero sized regions
> -		 * such as power3 which look like some kind of attempt at exposing
> -		 * the VGA memory hole)
> -		 */
> -		if (cpu_addr == OF_BAD_ADDR || size == 0)
> -			continue;
> -
> -		/* Now consume following elements while they are contiguous */
> -		for (; rlen >= np * sizeof(u32);
> -		     ranges += np, rlen -= np * 4) {
> -			if (ranges[0] != pci_space)
> -				break;
> -			pci_next = of_read_number(ranges + 1, 2);
> -			cpu_next = of_translate_address(dev, ranges + 3);
> -			if (pci_next != pci_addr + size ||
> -			    cpu_next != cpu_addr + size)
> -				break;
> -			size += of_read_number(ranges + pna + 3, 2);
> -		}
> -
> -		/* Act based on address space type */
> -		res = NULL;
> -		switch ((pci_space >> 24) & 0x3) {
> -		case 1:		/* PCI IO space */
> -			printk(KERN_INFO
> -			       "  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
> -			       cpu_addr, cpu_addr + size - 1, pci_addr);
> -
> -			/* We support only one IO range */
> -			if (hose->pci_io_size) {
> -				printk(KERN_INFO
> -				       " \\--> Skipped (too many) !\n");
> -				continue;
> -			}
> -#ifdef CONFIG_PPC32
> -			/* On 32 bits, limit I/O space to 16MB */
> -			if (size > 0x01000000)
> -				size = 0x01000000;
> -
> -			/* 32 bits needs to map IOs here */
> -			hose->io_base_virt = ioremap(cpu_addr, size);
> -
> -			/* Expect trouble if pci_addr is not 0 */
> -			if (primary)
> -				isa_io_base =
> -					(unsigned long)hose->io_base_virt;
> -#endif /* CONFIG_PPC32 */
> -			/* pci_io_size and io_base_phys always represent IO
> -			 * space starting at 0 so we factor in pci_addr
> -			 */
> -			hose->pci_io_size = pci_addr + size;
> -			hose->io_base_phys = cpu_addr - pci_addr;
> -
> -			/* Build resource */
> -			res = &hose->io_resource;
> -			res->flags = IORESOURCE_IO;
> -			res->start = pci_addr;
> -			break;
> -		case 2:		/* PCI Memory space */
> -		case 3:		/* PCI 64 bits Memory space */
> -			printk(KERN_INFO
> -			       " MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
> -			       cpu_addr, cpu_addr + size - 1, pci_addr,
> -			       (pci_space & 0x40000000) ? "Prefetch" : "");
> -
> -			/* We support only 3 memory ranges */
> -			if (memno >= 3) {
> -				printk(KERN_INFO
> -				       " \\--> Skipped (too many) !\n");
> -				continue;
> -			}
> -			/* Handles ISA memory hole space here */
> -			if (pci_addr == 0) {
> -				isa_mb = cpu_addr;
> -				isa_hole = memno;
> -				if (primary || isa_mem_base == 0)
> -					isa_mem_base = cpu_addr;
> -				hose->isa_mem_phys = cpu_addr;
> -				hose->isa_mem_size = size;
> -			}
> -
> -			/* We get the PCI/Mem offset from the first range or
> -			 * the, current one if the offset came from an ISA
> -			 * hole. If they don't match, bugger.
> -			 */
> -			if (memno == 0 ||
> -			    (isa_hole >= 0 && pci_addr != 0 &&
> -			     hose->pci_mem_offset == isa_mb))
> -				hose->pci_mem_offset = cpu_addr - pci_addr;
> -			else if (pci_addr != 0 &&
> -				 hose->pci_mem_offset != cpu_addr - pci_addr) {
> -				printk(KERN_INFO
> -				       " \\--> Skipped (offset mismatch) !\n");
> -				continue;
> -			}
> -
> -			/* Build resource */
> -			res = &hose->mem_resources[memno++];
> -			res->flags = IORESOURCE_MEM;
> -			if (pci_space & 0x40000000)
> -				res->flags |= IORESOURCE_PREFETCH;
> -			res->start = cpu_addr;
> -			break;
> -		}
> -		if (res != NULL) {
> -			res->name = dev->full_name;
> -			res->end = res->start + size - 1;
> -			res->parent = NULL;
> -			res->sibling = NULL;
> -			res->child = NULL;
> -		}
> -	}
> -
> -	/* If there's an ISA hole and the pci_mem_offset is -not- matching
> -	 * the ISA hole offset, then we need to remove the ISA hole from
> -	 * the resource list for that brige
> -	 */
> -	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
> -		unsigned int next = isa_hole + 1;
> -		printk(KERN_INFO " Removing ISA hole at 0x%016llx\n", isa_mb);
> -		if (next < memno)
> -			memmove(&hose->mem_resources[isa_hole],
> -				&hose->mem_resources[next],
> -				sizeof(struct resource) * (memno - next));
> -		hose->mem_resources[--memno].flags = 0;
> -	}
> -}
> -
>  /* Decide whether to display the domain number in /proc */
>  int pci_proc_domain(struct pci_bus *bus)
>  {
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index 13e37e2..1626172 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -4,6 +4,10 @@
>  #include <linux/of_pci.h>
>  #include <asm/prom.h>
>  
> +#if defined(CONFIG_PPC32) || defined(CONFIG_PPC64) || defined(CONFIG_MICROBLAZE)
> +#include <asm/pci-bridge.h>
> +#endif
> +
>  static inline int __of_pci_pci_compare(struct device_node *node,
>  				       unsigned int devfn)
>  {
> @@ -40,3 +44,199 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
>  	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(of_pci_find_child_device);
> +
> +/**
> + * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
> + * @hose: newly allocated pci_controller to be setup
> + * @dev: device node of the host bridge
> + * @primary: set if primary bus (32 bits only, soon to be deprecated)
> + *
> + * This function will parse the "ranges" property of a PCI host bridge device
> + * node and setup the resource mapping of a pci controller based on its
> + * content.
> + *
> + * Life would be boring if it wasn't for a few issues that we have to deal
> + * with here:
> + *
> + *   - We can only cope with one IO space range and up to 3 Memory space
> + *     ranges. However, some machines (thanks Apple !) tend to split their
> + *     space into lots of small contiguous ranges. So we have to coalesce.
> + *
> + *   - We can only cope with all memory ranges having the same offset
> + *     between CPU addresses and PCI addresses. Unfortunately, some bridges
> + *     are setup for a large 1:1 mapping along with a small "window" which
> + *     maps PCI address 0 to some arbitrary high address of the CPU space in
> + *     order to give access to the ISA memory hole.
> + *     The way out of here that I've chosen for now is to always set the
> + *     offset based on the first resource found, then override it if we
> + *     have a different offset and the previous was set by an ISA hole.
> + *
> + *   - Some busses have IO space not starting at 0, which causes trouble with
> + *     the way we do our IO resource renumbering. The code somewhat deals with
> + *     it for 64 bits but I would expect problems on 32 bits.
> + *
> + *   - Some 32 bits platforms such as 4xx can have physical space larger than
> + *     32 bits so we need to use 64 bits values for the parsing
> + */
> +#if defined(CONFIG_PPC32) || defined(CONFIG_PPC64) || defined(CONFIG_MICROBLAZE)
> +void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> +				  struct device_node *dev, int primary)
> +{
> +	const u32 *ranges;
> +	int rlen;
> +	int pna = of_n_addr_cells(dev);
> +	int np = pna + 5;
> +	int memno = 0, isa_hole = -1;
> +	u32 pci_space;
> +	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
> +	unsigned long long isa_mb = 0;
> +	struct resource *res;
> +
> +	pr_info("PCI host bridge %s %s ranges:\n",
> +	       dev->full_name, primary ? "(primary)" : "");
> +
> +	/* Get ranges property */
> +	ranges = of_get_property(dev, "ranges", &rlen);
> +	if (ranges == NULL)
> +		return;
> +
> +	/* Parse it */
> +	pr_debug("Parsing ranges property...\n");
> +	while ((rlen -= np * 4) >= 0) {
> +		/* Read next ranges element */
> +		pci_space = ranges[0];
> +		pci_addr = of_read_number(ranges + 1, 2);
> +		cpu_addr = of_translate_address(dev, ranges + 3);
> +		size = of_read_number(ranges + pna + 3, 2);
> +
> +		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
> +				pci_space, pci_addr);
> +		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
> +					cpu_addr, size);
> +
> +		ranges += np;
> +
> +		/* If we failed translation or got a zero-sized region
> +		 * (some FW try to feed us with non sensical zero sized regions
> +		 * such as power3 which look like some kind of attempt
> +		 * at exposing the VGA memory hole)
> +		 */
> +		if (cpu_addr == OF_BAD_ADDR || size == 0)
> +			continue;
> +
> +		/* Now consume following elements while they are contiguous */
> +		for (; rlen >= np * sizeof(u32);
> +		     ranges += np, rlen -= np * 4) {
> +			if (ranges[0] != pci_space)
> +				break;
> +			pci_next = of_read_number(ranges + 1, 2);
> +			cpu_next = of_translate_address(dev, ranges + 3);
> +			if (pci_next != pci_addr + size ||
> +			    cpu_next != cpu_addr + size)
> +				break;
> +			size += of_read_number(ranges + pna + 3, 2);
> +		}
> +
> +		/* Act based on address space type */
> +		res = NULL;
> +		switch ((pci_space >> 24) & 0x3) {
> +		case 1:		/* PCI IO space */
> +			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
> +			       cpu_addr, cpu_addr + size - 1, pci_addr);
> +
> +			/* We support only one IO range */
> +			if (hose->pci_io_size) {
> +				pr_info(" \\--> Skipped (too many) !\n");
> +				continue;
> +			}
> +#if (!IS_ENABLED(CONFIG_64BIT))
> +			/* On 32 bits, limit I/O space to 16MB */
> +			if (size > 0x01000000)
> +				size = 0x01000000;
> +
> +			/* 32 bits needs to map IOs here */
> +			hose->io_base_virt = ioremap(cpu_addr, size);
> +
> +			/* Expect trouble if pci_addr is not 0 */
> +			if (primary)
> +				isa_io_base =
> +					(unsigned long)hose->io_base_virt;
> +#endif /* !CONFIG_64BIT */
> +			/* pci_io_size and io_base_phys always represent IO
> +			 * space starting at 0 so we factor in pci_addr
> +			 */
> +			hose->pci_io_size = pci_addr + size;
> +			hose->io_base_phys = cpu_addr - pci_addr;
> +
> +			/* Build resource */
> +			res = &hose->io_resource;
> +			res->flags = IORESOURCE_IO;
> +			res->start = pci_addr;
> +			break;
> +		case 2:		/* PCI Memory space */
> +		case 3:		/* PCI 64 bits Memory space */
> +			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
> +			       cpu_addr, cpu_addr + size - 1, pci_addr,
> +			       (pci_space & 0x40000000) ? "Prefetch" : "");
> +
> +			/* We support only 3 memory ranges */
> +			if (memno >= 3) {
> +				pr_info(" \\--> Skipped (too many) !\n");
> +				continue;
> +			}
> +			/* Handles ISA memory hole space here */
> +			if (pci_addr == 0) {
> +				isa_mb = cpu_addr;
> +				isa_hole = memno;
> +				if (primary || isa_mem_base == 0)
> +					isa_mem_base = cpu_addr;
> +				hose->isa_mem_phys = cpu_addr;
> +				hose->isa_mem_size = size;
> +			}
> +
> +			/* We get the PCI/Mem offset from the first range or
> +			 * the, current one if the offset came from an ISA
> +			 * hole. If they don't match, bugger.
> +			 */
> +			if (memno == 0 ||
> +			    (isa_hole >= 0 && pci_addr != 0 &&
> +			     hose->pci_mem_offset == isa_mb))
> +				hose->pci_mem_offset = cpu_addr - pci_addr;
> +			else if (pci_addr != 0 &&
> +				 hose->pci_mem_offset != cpu_addr - pci_addr) {
> +				pr_info(" \\--> Skipped (offset mismatch) !\n");
> +				continue;
> +			}
> +
> +			/* Build resource */
> +			res = &hose->mem_resources[memno++];
> +			res->flags = IORESOURCE_MEM;
> +			if (pci_space & 0x40000000)
> +				res->flags |= IORESOURCE_PREFETCH;
> +			res->start = cpu_addr;
> +			break;
> +		}
> +		if (res != NULL) {
> +			res->name = dev->full_name;
> +			res->end = res->start + size - 1;
> +			res->parent = NULL;
> +			res->sibling = NULL;
> +			res->child = NULL;
> +		}
> +	}
> +
> +	/* If there's an ISA hole and the pci_mem_offset is -not- matching
> +	 * the ISA hole offset, then we need to remove the ISA hole from
> +	 * the resource list for that brige
> +	 */
> +	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
> +		unsigned int next = isa_hole + 1;
> +		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
> +		if (next < memno)
> +			memmove(&hose->mem_resources[isa_hole],
> +				&hose->mem_resources[next],
> +				sizeof(struct resource) * (memno - next));
> +		hose->mem_resources[--memno].flags = 0;
> +	}
> +}
> +#endif
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index bb115de..33e8ead 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -11,4 +11,8 @@ struct device_node;
>  struct device_node *of_pci_find_child_device(struct device_node *parent,
>  					     unsigned int devfn);
>  
> +struct pci_controller;
> +void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> +			struct device_node *dev, int primary);
> +
>  #endif



-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
  2013-04-15 12:57   ` Thomas Petazzoni
@ 2013-04-15 15:32     ` Benjamin Herrenschmidt
  2013-04-15 16:50     ` Michal Simek
  1 sibling, 0 replies; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2013-04-15 15:32 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: monstr, Andrew Murray, rob.herring, jgunthorpe, linux,
	siva.kallam, linux-pci, devicetree-discuss, jg1.han, Liviu.Dudau,
	linux-kernel, linux-samsung-soc, kgene.kim, bhelgaas,
	suren.reddy, linux-arm-kernel, paulus, grant.likely,
	thierry.reding, thomas.abraham, arnd, linus.walleij

On Mon, 2013-04-15 at 14:57 +0200, Thomas Petazzoni wrote:
> Michal, Ben,
> 
> Would you have some time to look at this patch and give your comments
> and/or ACK ? Since it touches the PowerPC and Microblaze core code, we
> need your agreement to merge this code, and quite a bit of code pending
> for 3.10 depends on this patch.

I'm currently still on vacation. I will be able to look at this after
I'm back in about a week.

> Rob, alternatively, could we imagine doing a different version of the
> 'of/pci: Provide support for parsing PCI DT ranges property' that
> introduces the new API only, leaving the PowerPC and Microblaze rework
> as follow-up efforts, so that all the PCIe drivers that depend on this
> patch can get in for 3.10 ? I'd find it pretty sad if the Marvell PCIe
> driver that has been worked on since 4+ months does not get into 3.10
> just because this patch cannot be merged.

Cheers,
Ben.

> Thanks!
> 
> Thomas
> 
> On Thu, 11 Apr 2013 16:26:07 +0100, Andrew Murray wrote:
> > The pci_process_bridge_OF_ranges function, used to parse the "ranges"
> > property of a PCI host device, is found in both Microblaze and PowerPC
> > architectures. These implementations are nearly identical. This patch
> > moves this common code to a common place.
> > 
> > Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > Reviewed-by: Rob Herring <rob.herring@calxeda.com>
> > Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > ---
> >  arch/microblaze/include/asm/pci-bridge.h |    5 +-
> >  arch/microblaze/pci/pci-common.c         |  192 ----------------------------
> >  arch/powerpc/include/asm/pci-bridge.h    |    5 +-
> >  arch/powerpc/kernel/pci-common.c         |  192 ----------------------------
> >  drivers/of/of_pci.c                      |  200 ++++++++++++++++++++++++++++++
> >  include/linux/of_pci.h                   |    4 +
> >  6 files changed, 206 insertions(+), 392 deletions(-)
> > 
> > diff --git a/arch/microblaze/include/asm/pci-bridge.h b/arch/microblaze/include/asm/pci-bridge.h
> > index cb5d397..5783cd6 100644
> > --- a/arch/microblaze/include/asm/pci-bridge.h
> > +++ b/arch/microblaze/include/asm/pci-bridge.h
> > @@ -10,6 +10,7 @@
> >  #include <linux/pci.h>
> >  #include <linux/list.h>
> >  #include <linux/ioport.h>
> > +#include <linux/of_pci.h>
> >  
> >  struct device_node;
> >  
> > @@ -132,10 +133,6 @@ extern void setup_indirect_pci(struct pci_controller *hose,
> >  extern struct pci_controller *pci_find_hose_for_OF_device(
> >  			struct device_node *node);
> >  
> > -/* Fill up host controller resources from the OF node */
> > -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> > -			struct device_node *dev, int primary);
> > -
> >  /* Allocate & free a PCI host bridge structure */
> >  extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
> >  extern void pcibios_free_controller(struct pci_controller *phb);
> > diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> > index 9ea521e..2735ad9 100644
> > --- a/arch/microblaze/pci/pci-common.c
> > +++ b/arch/microblaze/pci/pci-common.c
> > @@ -622,198 +622,6 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
> >  	*end = rsrc->end - offset;
> >  }
> >  
> > -/**
> > - * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
> > - * @hose: newly allocated pci_controller to be setup
> > - * @dev: device node of the host bridge
> > - * @primary: set if primary bus (32 bits only, soon to be deprecated)
> > - *
> > - * This function will parse the "ranges" property of a PCI host bridge device
> > - * node and setup the resource mapping of a pci controller based on its
> > - * content.
> > - *
> > - * Life would be boring if it wasn't for a few issues that we have to deal
> > - * with here:
> > - *
> > - *   - We can only cope with one IO space range and up to 3 Memory space
> > - *     ranges. However, some machines (thanks Apple !) tend to split their
> > - *     space into lots of small contiguous ranges. So we have to coalesce.
> > - *
> > - *   - We can only cope with all memory ranges having the same offset
> > - *     between CPU addresses and PCI addresses. Unfortunately, some bridges
> > - *     are setup for a large 1:1 mapping along with a small "window" which
> > - *     maps PCI address 0 to some arbitrary high address of the CPU space in
> > - *     order to give access to the ISA memory hole.
> > - *     The way out of here that I've chosen for now is to always set the
> > - *     offset based on the first resource found, then override it if we
> > - *     have a different offset and the previous was set by an ISA hole.
> > - *
> > - *   - Some busses have IO space not starting at 0, which causes trouble with
> > - *     the way we do our IO resource renumbering. The code somewhat deals with
> > - *     it for 64 bits but I would expect problems on 32 bits.
> > - *
> > - *   - Some 32 bits platforms such as 4xx can have physical space larger than
> > - *     32 bits so we need to use 64 bits values for the parsing
> > - */
> > -void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> > -				  struct device_node *dev, int primary)
> > -{
> > -	const u32 *ranges;
> > -	int rlen;
> > -	int pna = of_n_addr_cells(dev);
> > -	int np = pna + 5;
> > -	int memno = 0, isa_hole = -1;
> > -	u32 pci_space;
> > -	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
> > -	unsigned long long isa_mb = 0;
> > -	struct resource *res;
> > -
> > -	pr_info("PCI host bridge %s %s ranges:\n",
> > -	       dev->full_name, primary ? "(primary)" : "");
> > -
> > -	/* Get ranges property */
> > -	ranges = of_get_property(dev, "ranges", &rlen);
> > -	if (ranges == NULL)
> > -		return;
> > -
> > -	/* Parse it */
> > -	pr_debug("Parsing ranges property...\n");
> > -	while ((rlen -= np * 4) >= 0) {
> > -		/* Read next ranges element */
> > -		pci_space = ranges[0];
> > -		pci_addr = of_read_number(ranges + 1, 2);
> > -		cpu_addr = of_translate_address(dev, ranges + 3);
> > -		size = of_read_number(ranges + pna + 3, 2);
> > -
> > -		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
> > -				pci_space, pci_addr);
> > -		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
> > -					cpu_addr, size);
> > -
> > -		ranges += np;
> > -
> > -		/* If we failed translation or got a zero-sized region
> > -		 * (some FW try to feed us with non sensical zero sized regions
> > -		 * such as power3 which look like some kind of attempt
> > -		 * at exposing the VGA memory hole)
> > -		 */
> > -		if (cpu_addr == OF_BAD_ADDR || size == 0)
> > -			continue;
> > -
> > -		/* Now consume following elements while they are contiguous */
> > -		for (; rlen >= np * sizeof(u32);
> > -		     ranges += np, rlen -= np * 4) {
> > -			if (ranges[0] != pci_space)
> > -				break;
> > -			pci_next = of_read_number(ranges + 1, 2);
> > -			cpu_next = of_translate_address(dev, ranges + 3);
> > -			if (pci_next != pci_addr + size ||
> > -			    cpu_next != cpu_addr + size)
> > -				break;
> > -			size += of_read_number(ranges + pna + 3, 2);
> > -		}
> > -
> > -		/* Act based on address space type */
> > -		res = NULL;
> > -		switch ((pci_space >> 24) & 0x3) {
> > -		case 1:		/* PCI IO space */
> > -			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
> > -			       cpu_addr, cpu_addr + size - 1, pci_addr);
> > -
> > -			/* We support only one IO range */
> > -			if (hose->pci_io_size) {
> > -				pr_info(" \\--> Skipped (too many) !\n");
> > -				continue;
> > -			}
> > -			/* On 32 bits, limit I/O space to 16MB */
> > -			if (size > 0x01000000)
> > -				size = 0x01000000;
> > -
> > -			/* 32 bits needs to map IOs here */
> > -			hose->io_base_virt = ioremap(cpu_addr, size);
> > -
> > -			/* Expect trouble if pci_addr is not 0 */
> > -			if (primary)
> > -				isa_io_base =
> > -					(unsigned long)hose->io_base_virt;
> > -			/* pci_io_size and io_base_phys always represent IO
> > -			 * space starting at 0 so we factor in pci_addr
> > -			 */
> > -			hose->pci_io_size = pci_addr + size;
> > -			hose->io_base_phys = cpu_addr - pci_addr;
> > -
> > -			/* Build resource */
> > -			res = &hose->io_resource;
> > -			res->flags = IORESOURCE_IO;
> > -			res->start = pci_addr;
> > -			break;
> > -		case 2:		/* PCI Memory space */
> > -		case 3:		/* PCI 64 bits Memory space */
> > -			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
> > -			       cpu_addr, cpu_addr + size - 1, pci_addr,
> > -			       (pci_space & 0x40000000) ? "Prefetch" : "");
> > -
> > -			/* We support only 3 memory ranges */
> > -			if (memno >= 3) {
> > -				pr_info(" \\--> Skipped (too many) !\n");
> > -				continue;
> > -			}
> > -			/* Handles ISA memory hole space here */
> > -			if (pci_addr == 0) {
> > -				isa_mb = cpu_addr;
> > -				isa_hole = memno;
> > -				if (primary || isa_mem_base == 0)
> > -					isa_mem_base = cpu_addr;
> > -				hose->isa_mem_phys = cpu_addr;
> > -				hose->isa_mem_size = size;
> > -			}
> > -
> > -			/* We get the PCI/Mem offset from the first range or
> > -			 * the, current one if the offset came from an ISA
> > -			 * hole. If they don't match, bugger.
> > -			 */
> > -			if (memno == 0 ||
> > -			    (isa_hole >= 0 && pci_addr != 0 &&
> > -			     hose->pci_mem_offset == isa_mb))
> > -				hose->pci_mem_offset = cpu_addr - pci_addr;
> > -			else if (pci_addr != 0 &&
> > -				 hose->pci_mem_offset != cpu_addr - pci_addr) {
> > -				pr_info(" \\--> Skipped (offset mismatch) !\n");
> > -				continue;
> > -			}
> > -
> > -			/* Build resource */
> > -			res = &hose->mem_resources[memno++];
> > -			res->flags = IORESOURCE_MEM;
> > -			if (pci_space & 0x40000000)
> > -				res->flags |= IORESOURCE_PREFETCH;
> > -			res->start = cpu_addr;
> > -			break;
> > -		}
> > -		if (res != NULL) {
> > -			res->name = dev->full_name;
> > -			res->end = res->start + size - 1;
> > -			res->parent = NULL;
> > -			res->sibling = NULL;
> > -			res->child = NULL;
> > -		}
> > -	}
> > -
> > -	/* If there's an ISA hole and the pci_mem_offset is -not- matching
> > -	 * the ISA hole offset, then we need to remove the ISA hole from
> > -	 * the resource list for that brige
> > -	 */
> > -	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
> > -		unsigned int next = isa_hole + 1;
> > -		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
> > -		if (next < memno)
> > -			memmove(&hose->mem_resources[isa_hole],
> > -				&hose->mem_resources[next],
> > -				sizeof(struct resource) * (memno - next));
> > -		hose->mem_resources[--memno].flags = 0;
> > -	}
> > -}
> > -
> >  /* Decide whether to display the domain number in /proc */
> >  int pci_proc_domain(struct pci_bus *bus)
> >  {
> > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> > index 025a130..205bfba 100644
> > --- a/arch/powerpc/include/asm/pci-bridge.h
> > +++ b/arch/powerpc/include/asm/pci-bridge.h
> > @@ -10,6 +10,7 @@
> >  #include <linux/pci.h>
> >  #include <linux/list.h>
> >  #include <linux/ioport.h>
> > +#include <linux/of_pci.h>
> >  #include <asm-generic/pci-bridge.h>
> >  
> >  struct device_node;
> > @@ -231,10 +232,6 @@ extern int pcibios_map_io_space(struct pci_bus *bus);
> >  extern struct pci_controller *pci_find_hose_for_OF_device(
> >  			struct device_node* node);
> >  
> > -/* Fill up host controller resources from the OF node */
> > -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> > -			struct device_node *dev, int primary);
> > -
> >  /* Allocate & free a PCI host bridge structure */
> >  extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
> >  extern void pcibios_free_controller(struct pci_controller *phb);
> > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> > index fa12ae4..6edf396 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -640,198 +640,6 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
> >  	*end = rsrc->end - offset;
> >  }
> >  
> > -/**
> > - * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
> > - * @hose: newly allocated pci_controller to be setup
> > - * @dev: device node of the host bridge
> > - * @primary: set if primary bus (32 bits only, soon to be deprecated)
> > - *
> > - * This function will parse the "ranges" property of a PCI host bridge device
> > - * node and setup the resource mapping of a pci controller based on its
> > - * content.
> > - *
> > - * Life would be boring if it wasn't for a few issues that we have to deal
> > - * with here:
> > - *
> > - *   - We can only cope with one IO space range and up to 3 Memory space
> > - *     ranges. However, some machines (thanks Apple !) tend to split their
> > - *     space into lots of small contiguous ranges. So we have to coalesce.
> > - *
> > - *   - We can only cope with all memory ranges having the same offset
> > - *     between CPU addresses and PCI addresses. Unfortunately, some bridges
> > - *     are setup for a large 1:1 mapping along with a small "window" which
> > - *     maps PCI address 0 to some arbitrary high address of the CPU space in
> > - *     order to give access to the ISA memory hole.
> > - *     The way out of here that I've chosen for now is to always set the
> > - *     offset based on the first resource found, then override it if we
> > - *     have a different offset and the previous was set by an ISA hole.
> > - *
> > - *   - Some busses have IO space not starting at 0, which causes trouble with
> > - *     the way we do our IO resource renumbering. The code somewhat deals with
> > - *     it for 64 bits but I would expect problems on 32 bits.
> > - *
> > - *   - Some 32 bits platforms such as 4xx can have physical space larger than
> > - *     32 bits so we need to use 64 bits values for the parsing
> > - */
> > -void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> > -				  struct device_node *dev, int primary)
> > -{
> > -	const u32 *ranges;
> > -	int rlen;
> > -	int pna = of_n_addr_cells(dev);
> > -	int np = pna + 5;
> > -	int memno = 0, isa_hole = -1;
> > -	u32 pci_space;
> > -	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
> > -	unsigned long long isa_mb = 0;
> > -	struct resource *res;
> > -
> > -	printk(KERN_INFO "PCI host bridge %s %s ranges:\n",
> > -	       dev->full_name, primary ? "(primary)" : "");
> > -
> > -	/* Get ranges property */
> > -	ranges = of_get_property(dev, "ranges", &rlen);
> > -	if (ranges == NULL)
> > -		return;
> > -
> > -	/* Parse it */
> > -	while ((rlen -= np * 4) >= 0) {
> > -		/* Read next ranges element */
> > -		pci_space = ranges[0];
> > -		pci_addr = of_read_number(ranges + 1, 2);
> > -		cpu_addr = of_translate_address(dev, ranges + 3);
> > -		size = of_read_number(ranges + pna + 3, 2);
> > -		ranges += np;
> > -
> > -		/* If we failed translation or got a zero-sized region
> > -		 * (some FW try to feed us with non sensical zero sized regions
> > -		 * such as power3 which look like some kind of attempt at exposing
> > -		 * the VGA memory hole)
> > -		 */
> > -		if (cpu_addr == OF_BAD_ADDR || size == 0)
> > -			continue;
> > -
> > -		/* Now consume following elements while they are contiguous */
> > -		for (; rlen >= np * sizeof(u32);
> > -		     ranges += np, rlen -= np * 4) {
> > -			if (ranges[0] != pci_space)
> > -				break;
> > -			pci_next = of_read_number(ranges + 1, 2);
> > -			cpu_next = of_translate_address(dev, ranges + 3);
> > -			if (pci_next != pci_addr + size ||
> > -			    cpu_next != cpu_addr + size)
> > -				break;
> > -			size += of_read_number(ranges + pna + 3, 2);
> > -		}
> > -
> > -		/* Act based on address space type */
> > -		res = NULL;
> > -		switch ((pci_space >> 24) & 0x3) {
> > -		case 1:		/* PCI IO space */
> > -			printk(KERN_INFO
> > -			       "  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
> > -			       cpu_addr, cpu_addr + size - 1, pci_addr);
> > -
> > -			/* We support only one IO range */
> > -			if (hose->pci_io_size) {
> > -				printk(KERN_INFO
> > -				       " \\--> Skipped (too many) !\n");
> > -				continue;
> > -			}
> > -#ifdef CONFIG_PPC32
> > -			/* On 32 bits, limit I/O space to 16MB */
> > -			if (size > 0x01000000)
> > -				size = 0x01000000;
> > -
> > -			/* 32 bits needs to map IOs here */
> > -			hose->io_base_virt = ioremap(cpu_addr, size);
> > -
> > -			/* Expect trouble if pci_addr is not 0 */
> > -			if (primary)
> > -				isa_io_base =
> > -					(unsigned long)hose->io_base_virt;
> > -#endif /* CONFIG_PPC32 */
> > -			/* pci_io_size and io_base_phys always represent IO
> > -			 * space starting at 0 so we factor in pci_addr
> > -			 */
> > -			hose->pci_io_size = pci_addr + size;
> > -			hose->io_base_phys = cpu_addr - pci_addr;
> > -
> > -			/* Build resource */
> > -			res = &hose->io_resource;
> > -			res->flags = IORESOURCE_IO;
> > -			res->start = pci_addr;
> > -			break;
> > -		case 2:		/* PCI Memory space */
> > -		case 3:		/* PCI 64 bits Memory space */
> > -			printk(KERN_INFO
> > -			       " MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
> > -			       cpu_addr, cpu_addr + size - 1, pci_addr,
> > -			       (pci_space & 0x40000000) ? "Prefetch" : "");
> > -
> > -			/* We support only 3 memory ranges */
> > -			if (memno >= 3) {
> > -				printk(KERN_INFO
> > -				       " \\--> Skipped (too many) !\n");
> > -				continue;
> > -			}
> > -			/* Handles ISA memory hole space here */
> > -			if (pci_addr == 0) {
> > -				isa_mb = cpu_addr;
> > -				isa_hole = memno;
> > -				if (primary || isa_mem_base == 0)
> > -					isa_mem_base = cpu_addr;
> > -				hose->isa_mem_phys = cpu_addr;
> > -				hose->isa_mem_size = size;
> > -			}
> > -
> > -			/* We get the PCI/Mem offset from the first range or
> > -			 * the, current one if the offset came from an ISA
> > -			 * hole. If they don't match, bugger.
> > -			 */
> > -			if (memno == 0 ||
> > -			    (isa_hole >= 0 && pci_addr != 0 &&
> > -			     hose->pci_mem_offset == isa_mb))
> > -				hose->pci_mem_offset = cpu_addr - pci_addr;
> > -			else if (pci_addr != 0 &&
> > -				 hose->pci_mem_offset != cpu_addr - pci_addr) {
> > -				printk(KERN_INFO
> > -				       " \\--> Skipped (offset mismatch) !\n");
> > -				continue;
> > -			}
> > -
> > -			/* Build resource */
> > -			res = &hose->mem_resources[memno++];
> > -			res->flags = IORESOURCE_MEM;
> > -			if (pci_space & 0x40000000)
> > -				res->flags |= IORESOURCE_PREFETCH;
> > -			res->start = cpu_addr;
> > -			break;
> > -		}
> > -		if (res != NULL) {
> > -			res->name = dev->full_name;
> > -			res->end = res->start + size - 1;
> > -			res->parent = NULL;
> > -			res->sibling = NULL;
> > -			res->child = NULL;
> > -		}
> > -	}
> > -
> > -	/* If there's an ISA hole and the pci_mem_offset is -not- matching
> > -	 * the ISA hole offset, then we need to remove the ISA hole from
> > -	 * the resource list for that brige
> > -	 */
> > -	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
> > -		unsigned int next = isa_hole + 1;
> > -		printk(KERN_INFO " Removing ISA hole at 0x%016llx\n", isa_mb);
> > -		if (next < memno)
> > -			memmove(&hose->mem_resources[isa_hole],
> > -				&hose->mem_resources[next],
> > -				sizeof(struct resource) * (memno - next));
> > -		hose->mem_resources[--memno].flags = 0;
> > -	}
> > -}
> > -
> >  /* Decide whether to display the domain number in /proc */
> >  int pci_proc_domain(struct pci_bus *bus)
> >  {
> > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> > index 13e37e2..1626172 100644
> > --- a/drivers/of/of_pci.c
> > +++ b/drivers/of/of_pci.c
> > @@ -4,6 +4,10 @@
> >  #include <linux/of_pci.h>
> >  #include <asm/prom.h>
> >  
> > +#if defined(CONFIG_PPC32) || defined(CONFIG_PPC64) || defined(CONFIG_MICROBLAZE)
> > +#include <asm/pci-bridge.h>
> > +#endif
> > +
> >  static inline int __of_pci_pci_compare(struct device_node *node,
> >  				       unsigned int devfn)
> >  {
> > @@ -40,3 +44,199 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
> >  	return NULL;
> >  }
> >  EXPORT_SYMBOL_GPL(of_pci_find_child_device);
> > +
> > +/**
> > + * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
> > + * @hose: newly allocated pci_controller to be setup
> > + * @dev: device node of the host bridge
> > + * @primary: set if primary bus (32 bits only, soon to be deprecated)
> > + *
> > + * This function will parse the "ranges" property of a PCI host bridge device
> > + * node and setup the resource mapping of a pci controller based on its
> > + * content.
> > + *
> > + * Life would be boring if it wasn't for a few issues that we have to deal
> > + * with here:
> > + *
> > + *   - We can only cope with one IO space range and up to 3 Memory space
> > + *     ranges. However, some machines (thanks Apple !) tend to split their
> > + *     space into lots of small contiguous ranges. So we have to coalesce.
> > + *
> > + *   - We can only cope with all memory ranges having the same offset
> > + *     between CPU addresses and PCI addresses. Unfortunately, some bridges
> > + *     are setup for a large 1:1 mapping along with a small "window" which
> > + *     maps PCI address 0 to some arbitrary high address of the CPU space in
> > + *     order to give access to the ISA memory hole.
> > + *     The way out of here that I've chosen for now is to always set the
> > + *     offset based on the first resource found, then override it if we
> > + *     have a different offset and the previous was set by an ISA hole.
> > + *
> > + *   - Some busses have IO space not starting at 0, which causes trouble with
> > + *     the way we do our IO resource renumbering. The code somewhat deals with
> > + *     it for 64 bits but I would expect problems on 32 bits.
> > + *
> > + *   - Some 32 bits platforms such as 4xx can have physical space larger than
> > + *     32 bits so we need to use 64 bits values for the parsing
> > + */
> > +#if defined(CONFIG_PPC32) || defined(CONFIG_PPC64) || defined(CONFIG_MICROBLAZE)
> > +void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> > +				  struct device_node *dev, int primary)
> > +{
> > +	const u32 *ranges;
> > +	int rlen;
> > +	int pna = of_n_addr_cells(dev);
> > +	int np = pna + 5;
> > +	int memno = 0, isa_hole = -1;
> > +	u32 pci_space;
> > +	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
> > +	unsigned long long isa_mb = 0;
> > +	struct resource *res;
> > +
> > +	pr_info("PCI host bridge %s %s ranges:\n",
> > +	       dev->full_name, primary ? "(primary)" : "");
> > +
> > +	/* Get ranges property */
> > +	ranges = of_get_property(dev, "ranges", &rlen);
> > +	if (ranges == NULL)
> > +		return;
> > +
> > +	/* Parse it */
> > +	pr_debug("Parsing ranges property...\n");
> > +	while ((rlen -= np * 4) >= 0) {
> > +		/* Read next ranges element */
> > +		pci_space = ranges[0];
> > +		pci_addr = of_read_number(ranges + 1, 2);
> > +		cpu_addr = of_translate_address(dev, ranges + 3);
> > +		size = of_read_number(ranges + pna + 3, 2);
> > +
> > +		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
> > +				pci_space, pci_addr);
> > +		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
> > +					cpu_addr, size);
> > +
> > +		ranges += np;
> > +
> > +		/* If we failed translation or got a zero-sized region
> > +		 * (some FW try to feed us with non sensical zero sized regions
> > +		 * such as power3 which look like some kind of attempt
> > +		 * at exposing the VGA memory hole)
> > +		 */
> > +		if (cpu_addr == OF_BAD_ADDR || size == 0)
> > +			continue;
> > +
> > +		/* Now consume following elements while they are contiguous */
> > +		for (; rlen >= np * sizeof(u32);
> > +		     ranges += np, rlen -= np * 4) {
> > +			if (ranges[0] != pci_space)
> > +				break;
> > +			pci_next = of_read_number(ranges + 1, 2);
> > +			cpu_next = of_translate_address(dev, ranges + 3);
> > +			if (pci_next != pci_addr + size ||
> > +			    cpu_next != cpu_addr + size)
> > +				break;
> > +			size += of_read_number(ranges + pna + 3, 2);
> > +		}
> > +
> > +		/* Act based on address space type */
> > +		res = NULL;
> > +		switch ((pci_space >> 24) & 0x3) {
> > +		case 1:		/* PCI IO space */
> > +			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
> > +			       cpu_addr, cpu_addr + size - 1, pci_addr);
> > +
> > +			/* We support only one IO range */
> > +			if (hose->pci_io_size) {
> > +				pr_info(" \\--> Skipped (too many) !\n");
> > +				continue;
> > +			}
> > +#if (!IS_ENABLED(CONFIG_64BIT))
> > +			/* On 32 bits, limit I/O space to 16MB */
> > +			if (size > 0x01000000)
> > +				size = 0x01000000;
> > +
> > +			/* 32 bits needs to map IOs here */
> > +			hose->io_base_virt = ioremap(cpu_addr, size);
> > +
> > +			/* Expect trouble if pci_addr is not 0 */
> > +			if (primary)
> > +				isa_io_base =
> > +					(unsigned long)hose->io_base_virt;
> > +#endif /* !CONFIG_64BIT */
> > +			/* pci_io_size and io_base_phys always represent IO
> > +			 * space starting at 0 so we factor in pci_addr
> > +			 */
> > +			hose->pci_io_size = pci_addr + size;
> > +			hose->io_base_phys = cpu_addr - pci_addr;
> > +
> > +			/* Build resource */
> > +			res = &hose->io_resource;
> > +			res->flags = IORESOURCE_IO;
> > +			res->start = pci_addr;
> > +			break;
> > +		case 2:		/* PCI Memory space */
> > +		case 3:		/* PCI 64 bits Memory space */
> > +			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
> > +			       cpu_addr, cpu_addr + size - 1, pci_addr,
> > +			       (pci_space & 0x40000000) ? "Prefetch" : "");
> > +
> > +			/* We support only 3 memory ranges */
> > +			if (memno >= 3) {
> > +				pr_info(" \\--> Skipped (too many) !\n");
> > +				continue;
> > +			}
> > +			/* Handles ISA memory hole space here */
> > +			if (pci_addr == 0) {
> > +				isa_mb = cpu_addr;
> > +				isa_hole = memno;
> > +				if (primary || isa_mem_base == 0)
> > +					isa_mem_base = cpu_addr;
> > +				hose->isa_mem_phys = cpu_addr;
> > +				hose->isa_mem_size = size;
> > +			}
> > +
> > +			/* We get the PCI/Mem offset from the first range or
> > +			 * the, current one if the offset came from an ISA
> > +			 * hole. If they don't match, bugger.
> > +			 */
> > +			if (memno == 0 ||
> > +			    (isa_hole >= 0 && pci_addr != 0 &&
> > +			     hose->pci_mem_offset == isa_mb))
> > +				hose->pci_mem_offset = cpu_addr - pci_addr;
> > +			else if (pci_addr != 0 &&
> > +				 hose->pci_mem_offset != cpu_addr - pci_addr) {
> > +				pr_info(" \\--> Skipped (offset mismatch) !\n");
> > +				continue;
> > +			}
> > +
> > +			/* Build resource */
> > +			res = &hose->mem_resources[memno++];
> > +			res->flags = IORESOURCE_MEM;
> > +			if (pci_space & 0x40000000)
> > +				res->flags |= IORESOURCE_PREFETCH;
> > +			res->start = cpu_addr;
> > +			break;
> > +		}
> > +		if (res != NULL) {
> > +			res->name = dev->full_name;
> > +			res->end = res->start + size - 1;
> > +			res->parent = NULL;
> > +			res->sibling = NULL;
> > +			res->child = NULL;
> > +		}
> > +	}
> > +
> > +	/* If there's an ISA hole and the pci_mem_offset is -not- matching
> > +	 * the ISA hole offset, then we need to remove the ISA hole from
> > +	 * the resource list for that brige
> > +	 */
> > +	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
> > +		unsigned int next = isa_hole + 1;
> > +		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
> > +		if (next < memno)
> > +			memmove(&hose->mem_resources[isa_hole],
> > +				&hose->mem_resources[next],
> > +				sizeof(struct resource) * (memno - next));
> > +		hose->mem_resources[--memno].flags = 0;
> > +	}
> > +}
> > +#endif
> > diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> > index bb115de..33e8ead 100644
> > --- a/include/linux/of_pci.h
> > +++ b/include/linux/of_pci.h
> > @@ -11,4 +11,8 @@ struct device_node;
> >  struct device_node *of_pci_find_child_device(struct device_node *parent,
> >  					     unsigned int devfn);
> >  
> > +struct pci_controller;
> > +void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> > +			struct device_node *dev, int primary);
> > +
> >  #endif
> 
> 
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
  2013-04-15 12:57   ` Thomas Petazzoni
  2013-04-15 15:32     ` Benjamin Herrenschmidt
@ 2013-04-15 16:50     ` Michal Simek
  1 sibling, 0 replies; 12+ messages in thread
From: Michal Simek @ 2013-04-15 16:50 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: benh, Andrew Murray, rob.herring, jgunthorpe, linux, siva.kallam,
	linux-pci, devicetree-discuss, jg1.han, Liviu.Dudau,
	linux-kernel, linux-samsung-soc, kgene.kim, bhelgaas,
	suren.reddy, linux-arm-kernel, paulus, grant.likely,
	thierry.reding, thomas.abraham, arnd, linus.walleij

Hi Thomas and Andrew,

First of all I would recommend you to add your tree to 0-day testing
system where you can easily catch up compilation failures for
microblaze and others.

Getting this compilation failure:
drivers/of/of_pci.c: In function 'pci_process_bridge_OF_ranges':
drivers/of/of_pci.c:88:22: error: storage size of 'range' isn't known
drivers/of/of_pci.c:89:29: error: storage size of 'parser' isn't known
drivers/of/of_pci.c:96:2: error: implicit declaration of function 'of_pci_range_parser' [-Werror=implicit-function-declaration]
drivers/of/of_pci.c:100:2: error: implicit declaration of function 'for_each_of_pci_range' [-Werror=implicit-function-declaration]
drivers/of/of_pci.c:100:41: error: expected ';' before '{' token
drivers/of/of_pci.c:90:6: warning: unused variable 'res_type' [-Wunused-variable]
drivers/of/of_pci.c:89:29: warning: unused variable 'parser' [-Wunused-variable]
drivers/of/of_pci.c:88:22: warning: unused variable 'range' [-Wunused-variable]
drivers/of/of_pci.c:87:19: warning: unused variable 'res' [-Wunused-variable]
drivers/of/of_pci.c:86:21: warning: unused variable 'isa_mb' [-Wunused-variable]
drivers/of/of_pci.c:85:17: warning: unused variable 'isa_hole' [-Wunused-variable]
drivers/of/of_pci.c:85:6: warning: unused variable 'memno' [-Wunused-variable]

which is caused missing linux/of_addresss.h in of_pci.c.
PowerPC probably won't have this problem because you have this header in asm/prom.h
but based on the same comment it is better to add it directly to the files.

That's why please add this header to this patch.

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 3e428a1..f30887e 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -1,6 +1,7 @@
 #include <linux/kernel.h>
 #include <linux/export.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_pci.h>
 #include <asm/prom.h>


When you fix it:
Acked-by: Michal Simek <monstr@monstr.eu>

Thanks,
Michal



On 04/15/2013 02:57 PM, Thomas Petazzoni wrote:
> Michal, Ben,
> 
> Would you have some time to look at this patch and give your comments
> and/or ACK ? Since it touches the PowerPC and Microblaze core code, we
> need your agreement to merge this code, and quite a bit of code pending
> for 3.10 depends on this patch.
> 
> Rob, alternatively, could we imagine doing a different version of the
> 'of/pci: Provide support for parsing PCI DT ranges property' that
> introduces the new API only, leaving the PowerPC and Microblaze rework
> as follow-up efforts, so that all the PCIe drivers that depend on this
> patch can get in for 3.10 ? I'd find it pretty sad if the Marvell PCIe
> driver that has been worked on since 4+ months does not get into 3.10
> just because this patch cannot be merged.
> 
> Thanks!
> 
> Thomas
> 
> On Thu, 11 Apr 2013 16:26:07 +0100, Andrew Murray wrote:
>> The pci_process_bridge_OF_ranges function, used to parse the "ranges"
>> property of a PCI host device, is found in both Microblaze and PowerPC
>> architectures. These implementations are nearly identical. This patch
>> moves this common code to a common place.
>>
>> Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
>> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>> Reviewed-by: Rob Herring <rob.herring@calxeda.com>
>> Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> ---
>>  arch/microblaze/include/asm/pci-bridge.h |    5 +-
>>  arch/microblaze/pci/pci-common.c         |  192 ----------------------------
>>  arch/powerpc/include/asm/pci-bridge.h    |    5 +-
>>  arch/powerpc/kernel/pci-common.c         |  192 ----------------------------
>>  drivers/of/of_pci.c                      |  200 ++++++++++++++++++++++++++++++
>>  include/linux/of_pci.h                   |    4 +
>>  6 files changed, 206 insertions(+), 392 deletions(-)
>>
>> diff --git a/arch/microblaze/include/asm/pci-bridge.h b/arch/microblaze/include/asm/pci-bridge.h
>> index cb5d397..5783cd6 100644
>> --- a/arch/microblaze/include/asm/pci-bridge.h
>> +++ b/arch/microblaze/include/asm/pci-bridge.h
>> @@ -10,6 +10,7 @@
>>  #include <linux/pci.h>
>>  #include <linux/list.h>
>>  #include <linux/ioport.h>
>> +#include <linux/of_pci.h>
>>  
>>  struct device_node;
>>  
>> @@ -132,10 +133,6 @@ extern void setup_indirect_pci(struct pci_controller *hose,
>>  extern struct pci_controller *pci_find_hose_for_OF_device(
>>  			struct device_node *node);
>>  
>> -/* Fill up host controller resources from the OF node */
>> -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose,
>> -			struct device_node *dev, int primary);
>> -
>>  /* Allocate & free a PCI host bridge structure */
>>  extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
>>  extern void pcibios_free_controller(struct pci_controller *phb);
>> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
>> index 9ea521e..2735ad9 100644
>> --- a/arch/microblaze/pci/pci-common.c
>> +++ b/arch/microblaze/pci/pci-common.c
>> @@ -622,198 +622,6 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
>>  	*end = rsrc->end - offset;
>>  }
>>  
>> -/**
>> - * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
>> - * @hose: newly allocated pci_controller to be setup
>> - * @dev: device node of the host bridge
>> - * @primary: set if primary bus (32 bits only, soon to be deprecated)
>> - *
>> - * This function will parse the "ranges" property of a PCI host bridge device
>> - * node and setup the resource mapping of a pci controller based on its
>> - * content.
>> - *
>> - * Life would be boring if it wasn't for a few issues that we have to deal
>> - * with here:
>> - *
>> - *   - We can only cope with one IO space range and up to 3 Memory space
>> - *     ranges. However, some machines (thanks Apple !) tend to split their
>> - *     space into lots of small contiguous ranges. So we have to coalesce.
>> - *
>> - *   - We can only cope with all memory ranges having the same offset
>> - *     between CPU addresses and PCI addresses. Unfortunately, some bridges
>> - *     are setup for a large 1:1 mapping along with a small "window" which
>> - *     maps PCI address 0 to some arbitrary high address of the CPU space in
>> - *     order to give access to the ISA memory hole.
>> - *     The way out of here that I've chosen for now is to always set the
>> - *     offset based on the first resource found, then override it if we
>> - *     have a different offset and the previous was set by an ISA hole.
>> - *
>> - *   - Some busses have IO space not starting at 0, which causes trouble with
>> - *     the way we do our IO resource renumbering. The code somewhat deals with
>> - *     it for 64 bits but I would expect problems on 32 bits.
>> - *
>> - *   - Some 32 bits platforms such as 4xx can have physical space larger than
>> - *     32 bits so we need to use 64 bits values for the parsing
>> - */
>> -void pci_process_bridge_OF_ranges(struct pci_controller *hose,
>> -				  struct device_node *dev, int primary)
>> -{
>> -	const u32 *ranges;
>> -	int rlen;
>> -	int pna = of_n_addr_cells(dev);
>> -	int np = pna + 5;
>> -	int memno = 0, isa_hole = -1;
>> -	u32 pci_space;
>> -	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
>> -	unsigned long long isa_mb = 0;
>> -	struct resource *res;
>> -
>> -	pr_info("PCI host bridge %s %s ranges:\n",
>> -	       dev->full_name, primary ? "(primary)" : "");
>> -
>> -	/* Get ranges property */
>> -	ranges = of_get_property(dev, "ranges", &rlen);
>> -	if (ranges == NULL)
>> -		return;
>> -
>> -	/* Parse it */
>> -	pr_debug("Parsing ranges property...\n");
>> -	while ((rlen -= np * 4) >= 0) {
>> -		/* Read next ranges element */
>> -		pci_space = ranges[0];
>> -		pci_addr = of_read_number(ranges + 1, 2);
>> -		cpu_addr = of_translate_address(dev, ranges + 3);
>> -		size = of_read_number(ranges + pna + 3, 2);
>> -
>> -		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
>> -				pci_space, pci_addr);
>> -		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
>> -					cpu_addr, size);
>> -
>> -		ranges += np;
>> -
>> -		/* If we failed translation or got a zero-sized region
>> -		 * (some FW try to feed us with non sensical zero sized regions
>> -		 * such as power3 which look like some kind of attempt
>> -		 * at exposing the VGA memory hole)
>> -		 */
>> -		if (cpu_addr == OF_BAD_ADDR || size == 0)
>> -			continue;
>> -
>> -		/* Now consume following elements while they are contiguous */
>> -		for (; rlen >= np * sizeof(u32);
>> -		     ranges += np, rlen -= np * 4) {
>> -			if (ranges[0] != pci_space)
>> -				break;
>> -			pci_next = of_read_number(ranges + 1, 2);
>> -			cpu_next = of_translate_address(dev, ranges + 3);
>> -			if (pci_next != pci_addr + size ||
>> -			    cpu_next != cpu_addr + size)
>> -				break;
>> -			size += of_read_number(ranges + pna + 3, 2);
>> -		}
>> -
>> -		/* Act based on address space type */
>> -		res = NULL;
>> -		switch ((pci_space >> 24) & 0x3) {
>> -		case 1:		/* PCI IO space */
>> -			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
>> -			       cpu_addr, cpu_addr + size - 1, pci_addr);
>> -
>> -			/* We support only one IO range */
>> -			if (hose->pci_io_size) {
>> -				pr_info(" \\--> Skipped (too many) !\n");
>> -				continue;
>> -			}
>> -			/* On 32 bits, limit I/O space to 16MB */
>> -			if (size > 0x01000000)
>> -				size = 0x01000000;
>> -
>> -			/* 32 bits needs to map IOs here */
>> -			hose->io_base_virt = ioremap(cpu_addr, size);
>> -
>> -			/* Expect trouble if pci_addr is not 0 */
>> -			if (primary)
>> -				isa_io_base =
>> -					(unsigned long)hose->io_base_virt;
>> -			/* pci_io_size and io_base_phys always represent IO
>> -			 * space starting at 0 so we factor in pci_addr
>> -			 */
>> -			hose->pci_io_size = pci_addr + size;
>> -			hose->io_base_phys = cpu_addr - pci_addr;
>> -
>> -			/* Build resource */
>> -			res = &hose->io_resource;
>> -			res->flags = IORESOURCE_IO;
>> -			res->start = pci_addr;
>> -			break;
>> -		case 2:		/* PCI Memory space */
>> -		case 3:		/* PCI 64 bits Memory space */
>> -			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
>> -			       cpu_addr, cpu_addr + size - 1, pci_addr,
>> -			       (pci_space & 0x40000000) ? "Prefetch" : "");
>> -
>> -			/* We support only 3 memory ranges */
>> -			if (memno >= 3) {
>> -				pr_info(" \\--> Skipped (too many) !\n");
>> -				continue;
>> -			}
>> -			/* Handles ISA memory hole space here */
>> -			if (pci_addr == 0) {
>> -				isa_mb = cpu_addr;
>> -				isa_hole = memno;
>> -				if (primary || isa_mem_base == 0)
>> -					isa_mem_base = cpu_addr;
>> -				hose->isa_mem_phys = cpu_addr;
>> -				hose->isa_mem_size = size;
>> -			}
>> -
>> -			/* We get the PCI/Mem offset from the first range or
>> -			 * the, current one if the offset came from an ISA
>> -			 * hole. If they don't match, bugger.
>> -			 */
>> -			if (memno == 0 ||
>> -			    (isa_hole >= 0 && pci_addr != 0 &&
>> -			     hose->pci_mem_offset == isa_mb))
>> -				hose->pci_mem_offset = cpu_addr - pci_addr;
>> -			else if (pci_addr != 0 &&
>> -				 hose->pci_mem_offset != cpu_addr - pci_addr) {
>> -				pr_info(" \\--> Skipped (offset mismatch) !\n");
>> -				continue;
>> -			}
>> -
>> -			/* Build resource */
>> -			res = &hose->mem_resources[memno++];
>> -			res->flags = IORESOURCE_MEM;
>> -			if (pci_space & 0x40000000)
>> -				res->flags |= IORESOURCE_PREFETCH;
>> -			res->start = cpu_addr;
>> -			break;
>> -		}
>> -		if (res != NULL) {
>> -			res->name = dev->full_name;
>> -			res->end = res->start + size - 1;
>> -			res->parent = NULL;
>> -			res->sibling = NULL;
>> -			res->child = NULL;
>> -		}
>> -	}
>> -
>> -	/* If there's an ISA hole and the pci_mem_offset is -not- matching
>> -	 * the ISA hole offset, then we need to remove the ISA hole from
>> -	 * the resource list for that brige
>> -	 */
>> -	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
>> -		unsigned int next = isa_hole + 1;
>> -		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
>> -		if (next < memno)
>> -			memmove(&hose->mem_resources[isa_hole],
>> -				&hose->mem_resources[next],
>> -				sizeof(struct resource) * (memno - next));
>> -		hose->mem_resources[--memno].flags = 0;
>> -	}
>> -}
>> -
>>  /* Decide whether to display the domain number in /proc */
>>  int pci_proc_domain(struct pci_bus *bus)
>>  {
>> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>> index 025a130..205bfba 100644
>> --- a/arch/powerpc/include/asm/pci-bridge.h
>> +++ b/arch/powerpc/include/asm/pci-bridge.h
>> @@ -10,6 +10,7 @@
>>  #include <linux/pci.h>
>>  #include <linux/list.h>
>>  #include <linux/ioport.h>
>> +#include <linux/of_pci.h>
>>  #include <asm-generic/pci-bridge.h>
>>  
>>  struct device_node;
>> @@ -231,10 +232,6 @@ extern int pcibios_map_io_space(struct pci_bus *bus);
>>  extern struct pci_controller *pci_find_hose_for_OF_device(
>>  			struct device_node* node);
>>  
>> -/* Fill up host controller resources from the OF node */
>> -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose,
>> -			struct device_node *dev, int primary);
>> -
>>  /* Allocate & free a PCI host bridge structure */
>>  extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
>>  extern void pcibios_free_controller(struct pci_controller *phb);
>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>> index fa12ae4..6edf396 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -640,198 +640,6 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
>>  	*end = rsrc->end - offset;
>>  }
>>  
>> -/**
>> - * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
>> - * @hose: newly allocated pci_controller to be setup
>> - * @dev: device node of the host bridge
>> - * @primary: set if primary bus (32 bits only, soon to be deprecated)
>> - *
>> - * This function will parse the "ranges" property of a PCI host bridge device
>> - * node and setup the resource mapping of a pci controller based on its
>> - * content.
>> - *
>> - * Life would be boring if it wasn't for a few issues that we have to deal
>> - * with here:
>> - *
>> - *   - We can only cope with one IO space range and up to 3 Memory space
>> - *     ranges. However, some machines (thanks Apple !) tend to split their
>> - *     space into lots of small contiguous ranges. So we have to coalesce.
>> - *
>> - *   - We can only cope with all memory ranges having the same offset
>> - *     between CPU addresses and PCI addresses. Unfortunately, some bridges
>> - *     are setup for a large 1:1 mapping along with a small "window" which
>> - *     maps PCI address 0 to some arbitrary high address of the CPU space in
>> - *     order to give access to the ISA memory hole.
>> - *     The way out of here that I've chosen for now is to always set the
>> - *     offset based on the first resource found, then override it if we
>> - *     have a different offset and the previous was set by an ISA hole.
>> - *
>> - *   - Some busses have IO space not starting at 0, which causes trouble with
>> - *     the way we do our IO resource renumbering. The code somewhat deals with
>> - *     it for 64 bits but I would expect problems on 32 bits.
>> - *
>> - *   - Some 32 bits platforms such as 4xx can have physical space larger than
>> - *     32 bits so we need to use 64 bits values for the parsing
>> - */
>> -void pci_process_bridge_OF_ranges(struct pci_controller *hose,
>> -				  struct device_node *dev, int primary)
>> -{
>> -	const u32 *ranges;
>> -	int rlen;
>> -	int pna = of_n_addr_cells(dev);
>> -	int np = pna + 5;
>> -	int memno = 0, isa_hole = -1;
>> -	u32 pci_space;
>> -	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
>> -	unsigned long long isa_mb = 0;
>> -	struct resource *res;
>> -
>> -	printk(KERN_INFO "PCI host bridge %s %s ranges:\n",
>> -	       dev->full_name, primary ? "(primary)" : "");
>> -
>> -	/* Get ranges property */
>> -	ranges = of_get_property(dev, "ranges", &rlen);
>> -	if (ranges == NULL)
>> -		return;
>> -
>> -	/* Parse it */
>> -	while ((rlen -= np * 4) >= 0) {
>> -		/* Read next ranges element */
>> -		pci_space = ranges[0];
>> -		pci_addr = of_read_number(ranges + 1, 2);
>> -		cpu_addr = of_translate_address(dev, ranges + 3);
>> -		size = of_read_number(ranges + pna + 3, 2);
>> -		ranges += np;
>> -
>> -		/* If we failed translation or got a zero-sized region
>> -		 * (some FW try to feed us with non sensical zero sized regions
>> -		 * such as power3 which look like some kind of attempt at exposing
>> -		 * the VGA memory hole)
>> -		 */
>> -		if (cpu_addr == OF_BAD_ADDR || size == 0)
>> -			continue;
>> -
>> -		/* Now consume following elements while they are contiguous */
>> -		for (; rlen >= np * sizeof(u32);
>> -		     ranges += np, rlen -= np * 4) {
>> -			if (ranges[0] != pci_space)
>> -				break;
>> -			pci_next = of_read_number(ranges + 1, 2);
>> -			cpu_next = of_translate_address(dev, ranges + 3);
>> -			if (pci_next != pci_addr + size ||
>> -			    cpu_next != cpu_addr + size)
>> -				break;
>> -			size += of_read_number(ranges + pna + 3, 2);
>> -		}
>> -
>> -		/* Act based on address space type */
>> -		res = NULL;
>> -		switch ((pci_space >> 24) & 0x3) {
>> -		case 1:		/* PCI IO space */
>> -			printk(KERN_INFO
>> -			       "  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
>> -			       cpu_addr, cpu_addr + size - 1, pci_addr);
>> -
>> -			/* We support only one IO range */
>> -			if (hose->pci_io_size) {
>> -				printk(KERN_INFO
>> -				       " \\--> Skipped (too many) !\n");
>> -				continue;
>> -			}
>> -#ifdef CONFIG_PPC32
>> -			/* On 32 bits, limit I/O space to 16MB */
>> -			if (size > 0x01000000)
>> -				size = 0x01000000;
>> -
>> -			/* 32 bits needs to map IOs here */
>> -			hose->io_base_virt = ioremap(cpu_addr, size);
>> -
>> -			/* Expect trouble if pci_addr is not 0 */
>> -			if (primary)
>> -				isa_io_base =
>> -					(unsigned long)hose->io_base_virt;
>> -#endif /* CONFIG_PPC32 */
>> -			/* pci_io_size and io_base_phys always represent IO
>> -			 * space starting at 0 so we factor in pci_addr
>> -			 */
>> -			hose->pci_io_size = pci_addr + size;
>> -			hose->io_base_phys = cpu_addr - pci_addr;
>> -
>> -			/* Build resource */
>> -			res = &hose->io_resource;
>> -			res->flags = IORESOURCE_IO;
>> -			res->start = pci_addr;
>> -			break;
>> -		case 2:		/* PCI Memory space */
>> -		case 3:		/* PCI 64 bits Memory space */
>> -			printk(KERN_INFO
>> -			       " MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
>> -			       cpu_addr, cpu_addr + size - 1, pci_addr,
>> -			       (pci_space & 0x40000000) ? "Prefetch" : "");
>> -
>> -			/* We support only 3 memory ranges */
>> -			if (memno >= 3) {
>> -				printk(KERN_INFO
>> -				       " \\--> Skipped (too many) !\n");
>> -				continue;
>> -			}
>> -			/* Handles ISA memory hole space here */
>> -			if (pci_addr == 0) {
>> -				isa_mb = cpu_addr;
>> -				isa_hole = memno;
>> -				if (primary || isa_mem_base == 0)
>> -					isa_mem_base = cpu_addr;
>> -				hose->isa_mem_phys = cpu_addr;
>> -				hose->isa_mem_size = size;
>> -			}
>> -
>> -			/* We get the PCI/Mem offset from the first range or
>> -			 * the, current one if the offset came from an ISA
>> -			 * hole. If they don't match, bugger.
>> -			 */
>> -			if (memno == 0 ||
>> -			    (isa_hole >= 0 && pci_addr != 0 &&
>> -			     hose->pci_mem_offset == isa_mb))
>> -				hose->pci_mem_offset = cpu_addr - pci_addr;
>> -			else if (pci_addr != 0 &&
>> -				 hose->pci_mem_offset != cpu_addr - pci_addr) {
>> -				printk(KERN_INFO
>> -				       " \\--> Skipped (offset mismatch) !\n");
>> -				continue;
>> -			}
>> -
>> -			/* Build resource */
>> -			res = &hose->mem_resources[memno++];
>> -			res->flags = IORESOURCE_MEM;
>> -			if (pci_space & 0x40000000)
>> -				res->flags |= IORESOURCE_PREFETCH;
>> -			res->start = cpu_addr;
>> -			break;
>> -		}
>> -		if (res != NULL) {
>> -			res->name = dev->full_name;
>> -			res->end = res->start + size - 1;
>> -			res->parent = NULL;
>> -			res->sibling = NULL;
>> -			res->child = NULL;
>> -		}
>> -	}
>> -
>> -	/* If there's an ISA hole and the pci_mem_offset is -not- matching
>> -	 * the ISA hole offset, then we need to remove the ISA hole from
>> -	 * the resource list for that brige
>> -	 */
>> -	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
>> -		unsigned int next = isa_hole + 1;
>> -		printk(KERN_INFO " Removing ISA hole at 0x%016llx\n", isa_mb);
>> -		if (next < memno)
>> -			memmove(&hose->mem_resources[isa_hole],
>> -				&hose->mem_resources[next],
>> -				sizeof(struct resource) * (memno - next));
>> -		hose->mem_resources[--memno].flags = 0;
>> -	}
>> -}
>> -
>>  /* Decide whether to display the domain number in /proc */
>>  int pci_proc_domain(struct pci_bus *bus)
>>  {
>> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
>> index 13e37e2..1626172 100644
>> --- a/drivers/of/of_pci.c
>> +++ b/drivers/of/of_pci.c
>> @@ -4,6 +4,10 @@
>>  #include <linux/of_pci.h>
>>  #include <asm/prom.h>
>>  
>> +#if defined(CONFIG_PPC32) || defined(CONFIG_PPC64) || defined(CONFIG_MICROBLAZE)
>> +#include <asm/pci-bridge.h>
>> +#endif
>> +
>>  static inline int __of_pci_pci_compare(struct device_node *node,
>>  				       unsigned int devfn)
>>  {
>> @@ -40,3 +44,199 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
>>  	return NULL;
>>  }
>>  EXPORT_SYMBOL_GPL(of_pci_find_child_device);
>> +
>> +/**
>> + * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
>> + * @hose: newly allocated pci_controller to be setup
>> + * @dev: device node of the host bridge
>> + * @primary: set if primary bus (32 bits only, soon to be deprecated)
>> + *
>> + * This function will parse the "ranges" property of a PCI host bridge device
>> + * node and setup the resource mapping of a pci controller based on its
>> + * content.
>> + *
>> + * Life would be boring if it wasn't for a few issues that we have to deal
>> + * with here:
>> + *
>> + *   - We can only cope with one IO space range and up to 3 Memory space
>> + *     ranges. However, some machines (thanks Apple !) tend to split their
>> + *     space into lots of small contiguous ranges. So we have to coalesce.
>> + *
>> + *   - We can only cope with all memory ranges having the same offset
>> + *     between CPU addresses and PCI addresses. Unfortunately, some bridges
>> + *     are setup for a large 1:1 mapping along with a small "window" which
>> + *     maps PCI address 0 to some arbitrary high address of the CPU space in
>> + *     order to give access to the ISA memory hole.
>> + *     The way out of here that I've chosen for now is to always set the
>> + *     offset based on the first resource found, then override it if we
>> + *     have a different offset and the previous was set by an ISA hole.
>> + *
>> + *   - Some busses have IO space not starting at 0, which causes trouble with
>> + *     the way we do our IO resource renumbering. The code somewhat deals with
>> + *     it for 64 bits but I would expect problems on 32 bits.
>> + *
>> + *   - Some 32 bits platforms such as 4xx can have physical space larger than
>> + *     32 bits so we need to use 64 bits values for the parsing
>> + */
>> +#if defined(CONFIG_PPC32) || defined(CONFIG_PPC64) || defined(CONFIG_MICROBLAZE)
>> +void pci_process_bridge_OF_ranges(struct pci_controller *hose,
>> +				  struct device_node *dev, int primary)
>> +{
>> +	const u32 *ranges;
>> +	int rlen;
>> +	int pna = of_n_addr_cells(dev);
>> +	int np = pna + 5;
>> +	int memno = 0, isa_hole = -1;
>> +	u32 pci_space;
>> +	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
>> +	unsigned long long isa_mb = 0;
>> +	struct resource *res;
>> +
>> +	pr_info("PCI host bridge %s %s ranges:\n",
>> +	       dev->full_name, primary ? "(primary)" : "");
>> +
>> +	/* Get ranges property */
>> +	ranges = of_get_property(dev, "ranges", &rlen);
>> +	if (ranges == NULL)
>> +		return;
>> +
>> +	/* Parse it */
>> +	pr_debug("Parsing ranges property...\n");
>> +	while ((rlen -= np * 4) >= 0) {
>> +		/* Read next ranges element */
>> +		pci_space = ranges[0];
>> +		pci_addr = of_read_number(ranges + 1, 2);
>> +		cpu_addr = of_translate_address(dev, ranges + 3);
>> +		size = of_read_number(ranges + pna + 3, 2);
>> +
>> +		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
>> +				pci_space, pci_addr);
>> +		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
>> +					cpu_addr, size);
>> +
>> +		ranges += np;
>> +
>> +		/* If we failed translation or got a zero-sized region
>> +		 * (some FW try to feed us with non sensical zero sized regions
>> +		 * such as power3 which look like some kind of attempt
>> +		 * at exposing the VGA memory hole)
>> +		 */
>> +		if (cpu_addr == OF_BAD_ADDR || size == 0)
>> +			continue;
>> +
>> +		/* Now consume following elements while they are contiguous */
>> +		for (; rlen >= np * sizeof(u32);
>> +		     ranges += np, rlen -= np * 4) {
>> +			if (ranges[0] != pci_space)
>> +				break;
>> +			pci_next = of_read_number(ranges + 1, 2);
>> +			cpu_next = of_translate_address(dev, ranges + 3);
>> +			if (pci_next != pci_addr + size ||
>> +			    cpu_next != cpu_addr + size)
>> +				break;
>> +			size += of_read_number(ranges + pna + 3, 2);
>> +		}
>> +
>> +		/* Act based on address space type */
>> +		res = NULL;
>> +		switch ((pci_space >> 24) & 0x3) {
>> +		case 1:		/* PCI IO space */
>> +			pr_info("  IO 0x%016llx..0x%016llx -> 0x%016llx\n",
>> +			       cpu_addr, cpu_addr + size - 1, pci_addr);
>> +
>> +			/* We support only one IO range */
>> +			if (hose->pci_io_size) {
>> +				pr_info(" \\--> Skipped (too many) !\n");
>> +				continue;
>> +			}
>> +#if (!IS_ENABLED(CONFIG_64BIT))
>> +			/* On 32 bits, limit I/O space to 16MB */
>> +			if (size > 0x01000000)
>> +				size = 0x01000000;
>> +
>> +			/* 32 bits needs to map IOs here */
>> +			hose->io_base_virt = ioremap(cpu_addr, size);
>> +
>> +			/* Expect trouble if pci_addr is not 0 */
>> +			if (primary)
>> +				isa_io_base =
>> +					(unsigned long)hose->io_base_virt;
>> +#endif /* !CONFIG_64BIT */
>> +			/* pci_io_size and io_base_phys always represent IO
>> +			 * space starting at 0 so we factor in pci_addr
>> +			 */
>> +			hose->pci_io_size = pci_addr + size;
>> +			hose->io_base_phys = cpu_addr - pci_addr;
>> +
>> +			/* Build resource */
>> +			res = &hose->io_resource;
>> +			res->flags = IORESOURCE_IO;
>> +			res->start = pci_addr;
>> +			break;
>> +		case 2:		/* PCI Memory space */
>> +		case 3:		/* PCI 64 bits Memory space */
>> +			pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
>> +			       cpu_addr, cpu_addr + size - 1, pci_addr,
>> +			       (pci_space & 0x40000000) ? "Prefetch" : "");
>> +
>> +			/* We support only 3 memory ranges */
>> +			if (memno >= 3) {
>> +				pr_info(" \\--> Skipped (too many) !\n");
>> +				continue;
>> +			}
>> +			/* Handles ISA memory hole space here */
>> +			if (pci_addr == 0) {
>> +				isa_mb = cpu_addr;
>> +				isa_hole = memno;
>> +				if (primary || isa_mem_base == 0)
>> +					isa_mem_base = cpu_addr;
>> +				hose->isa_mem_phys = cpu_addr;
>> +				hose->isa_mem_size = size;
>> +			}
>> +
>> +			/* We get the PCI/Mem offset from the first range or
>> +			 * the, current one if the offset came from an ISA
>> +			 * hole. If they don't match, bugger.
>> +			 */
>> +			if (memno == 0 ||
>> +			    (isa_hole >= 0 && pci_addr != 0 &&
>> +			     hose->pci_mem_offset == isa_mb))
>> +				hose->pci_mem_offset = cpu_addr - pci_addr;
>> +			else if (pci_addr != 0 &&
>> +				 hose->pci_mem_offset != cpu_addr - pci_addr) {
>> +				pr_info(" \\--> Skipped (offset mismatch) !\n");
>> +				continue;
>> +			}
>> +
>> +			/* Build resource */
>> +			res = &hose->mem_resources[memno++];
>> +			res->flags = IORESOURCE_MEM;
>> +			if (pci_space & 0x40000000)
>> +				res->flags |= IORESOURCE_PREFETCH;
>> +			res->start = cpu_addr;
>> +			break;
>> +		}
>> +		if (res != NULL) {
>> +			res->name = dev->full_name;
>> +			res->end = res->start + size - 1;
>> +			res->parent = NULL;
>> +			res->sibling = NULL;
>> +			res->child = NULL;
>> +		}
>> +	}
>> +
>> +	/* If there's an ISA hole and the pci_mem_offset is -not- matching
>> +	 * the ISA hole offset, then we need to remove the ISA hole from
>> +	 * the resource list for that brige
>> +	 */
>> +	if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
>> +		unsigned int next = isa_hole + 1;
>> +		pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
>> +		if (next < memno)
>> +			memmove(&hose->mem_resources[isa_hole],
>> +				&hose->mem_resources[next],
>> +				sizeof(struct resource) * (memno - next));
>> +		hose->mem_resources[--memno].flags = 0;
>> +	}
>> +}
>> +#endif
>> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
>> index bb115de..33e8ead 100644
>> --- a/include/linux/of_pci.h
>> +++ b/include/linux/of_pci.h
>> @@ -11,4 +11,8 @@ struct device_node;
>>  struct device_node *of_pci_find_child_device(struct device_node *parent,
>>  					     unsigned int devfn);
>>  
>> +struct pci_controller;
>> +void pci_process_bridge_OF_ranges(struct pci_controller *hose,
>> +			struct device_node *dev, int primary);
>> +
>>  #endif
> 
> 
> 


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID:
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing
  2013-04-11 15:26 [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing Andrew Murray
                   ` (3 preceding siblings ...)
  2013-04-11 16:55 ` [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing Linus Walleij
@ 2013-04-15 17:56 ` Jason Cooper
       [not found]   ` <CACRpkdbSd8pRG5pUS0ymqXDSp=ZhoQa2ctVuRA0cCmUcX5qFAA@mail.gmail.com>
  4 siblings, 1 reply; 12+ messages in thread
From: Jason Cooper @ 2013-04-15 17:56 UTC (permalink / raw)
  To: Andrew Murray
  Cc: rob.herring, siva.kallam, linux-pci, linus.walleij,
	thierry.reding, Liviu.Dudau, grant.likely, paulus,
	linux-samsung-soc, linux, jg1.han, jgunthorpe, thomas.abraham,
	benh, arnd, devicetree-discuss, kgene.kim, bhelgaas,
	linux-arm-kernel, thomas.petazzoni, monstr, linux-kernel,
	suren.reddy

Andrew Murray, Thomas,

On Thu, Apr 11, 2013 at 04:26:06PM +0100, Andrew Murray wrote:
> This patchset factors out duplicated code associated with parsing PCI
> DT "ranges" properties across the architectures and introduces a
> "ranges" parser. This parser "of_pci_range_parser" can be used directly
> by ARM host bridge drivers enabling them to obtain ranges from device
> trees.
> 
> I've included the Reviewed-by and Tested-by's received from v5 in this
> patchset, earlier versions of this patchset (v3) have been tested-by:
> 
> Thierry Reding <thierry.reding@avionic-design.de>
> Jingoo Han <jg1.han@samsung.com>
> 
> I believe a version of this patchset has also been tested through its
> inclusion in Thomas Petazzoni's Armada 370 and Armada XP SoCs PCIe support by:
> 
> Linus Walleij <linus.walleij@linaro.org>
> 
> I've tested that this patchset builds and runs on ARM and that it builds on
> PowerPC.
> 
> Compared to the v5 sent by Andrew Murray, the following changes have
> been made:
> 
>  * Use of CONFIG_64BIT instead of CONFIG_[a32bitarch] as suggested by
>    Rob Herring in drivers/of/of_pci.c
> 
>  * Added forward declaration of struct pci_controller in linux/of_pci.h
>    to prevent compiler warning as suggested by Thomas Petazzoni
> 
>  * Improved error checking (!range check), removal of unnecessary be32_to_cpup
>    call, improved formatting of struct of_pci_range_parser layout and
>    replacement of macro with a static inline. All suggested by Rob Herring.
> 
> Compared to the v4 (incorrectly labelled v3) sent by Andrew Murray,
> the following changes have been made:
> 
>  * Split the patch as suggested by Rob Herring
> 
> Compared to the v3 sent by Andrew Murray, the following changes have
> been made:
> 
>  * Unify and move duplicate pci_process_bridge_OF_ranges functions to
>    drivers/of/of_pci.c as suggested by Rob Herring
> 
>  * Fix potential build errors with Microblaze/MIPS
> 
> Compared to "[PATCH v5 01/17] of/pci: Provide support for parsing PCI DT
> ranges property", the following changes have been made:
> 
>  * Correct use of IORESOURCE_* as suggested by Russell King
> 
>  * Improved interface and naming as suggested by Thierry Reding
> 
> Compared to the v2 sent by Andrew Murray, Thomas Petazzoni did:
> 
>  * Add a memset() on the struct of_pci_range_iter when starting the
>    for loop in for_each_pci_range(). Otherwise, with an uninitialized
>    of_pci_range_iter, of_pci_process_ranges() may crash.
> 
>  * Add parenthesis around 'res', 'np' and 'iter' in the
>    for_each_of_pci_range macro definitions. Otherwise, passing
>    something like &foobar as 'res' didn't work.
> 
>  * Rebased on top of 3.9-rc2, which required fixing a few conflicts in
>    the Microblaze code.
> 
> v2:
>   This follows on from suggestions made by Grant Likely
>   (marc.info/?l=linux-kernel&m=136079602806328)
> 
> Andrew Murray (3):
>   of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and
>     PowerPC
>   of/pci: Provide support for parsing PCI DT ranges property
>   of/pci: mips: convert to common of_pci_range_parser
> 
>  arch/microblaze/include/asm/pci-bridge.h |    5 +-
>  arch/microblaze/pci/pci-common.c         |  192 ------------------------------
>  arch/mips/pci/pci.c                      |   50 +++------
>  arch/powerpc/include/asm/pci-bridge.h    |    5 +-
>  arch/powerpc/kernel/pci-common.c         |  192 ------------------------------
>  drivers/of/address.c                     |   66 ++++++++++
>  drivers/of/of_pci.c                      |  168 ++++++++++++++++++++++++++
>  include/linux/of_address.h               |   46 +++++++
>  include/linux/of_pci.h                   |    4 +
>  9 files changed, 302 insertions(+), 426 deletions(-)

As agreed with Rob Herring, series applied to mvebu/drivers to support
mvebu pcie driver.

thx,

Jason.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing
       [not found]   ` <CACRpkdbSd8pRG5pUS0ymqXDSp=ZhoQa2ctVuRA0cCmUcX5qFAA@mail.gmail.com>
@ 2013-04-16  7:12     ` Benjamin Herrenschmidt
       [not found]       ` <20130416112951.GZ28693@titan.lakedaemon.net>
  0 siblings, 1 reply; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2013-04-16  7:12 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jason Cooper, Andrew Murray, Rob Herring, siva.kallam, linux-pci,
	Thierry Reding, Liviu.Dudau, Grant Likely, Paul Mackerras,
	linux-samsung-soc, Russell King - ARM Linux, Jingoo Han,
	Jason Gunthorpe, Thomas Abraham, Arnd Bergmann,
	devicetree-discuss, Kukjin Kim, bhelgaas, linux-arm-kernel,
	Thomas Petazzoni, Michal Simek, linux-kernel, suren.reddy

On Mon, 2013-04-15 at 20:29 +0200, Linus Walleij wrote:
> > As agreed with Rob Herring, series applied to mvebu/drivers to
> support
> > mvebu pcie driver.
> 
> Will this hit ARM SoC soon-ish so I can base a pull request for the
> Integrator on this stuff as well?

Do not send this series upstream (or even in -next) until it has been at
least build-tested and acked by the affected architectures. AFAIK it
breaks microblaze build (simple missing #include) and isn't yet tested
on powerpc (I'm on vacation, maybe somebody else on linuxppc-dev can do
it though).

Ben.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing
       [not found]       ` <20130416112951.GZ28693@titan.lakedaemon.net>
@ 2013-04-16 11:31         ` Thomas Petazzoni
  2013-04-16 11:37           ` Jason Cooper
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Petazzoni @ 2013-04-16 11:31 UTC (permalink / raw)
  To: Jason Cooper
  Cc: Benjamin Herrenschmidt, Linus Walleij, Andrew Murray,
	Rob Herring, siva.kallam, linux-pci, Thierry Reding, Liviu.Dudau,
	Grant Likely, Paul Mackerras, linux-samsung-soc,
	Russell King - ARM Linux, Jingoo Han, Jason Gunthorpe,
	Thomas Abraham, Arnd Bergmann, devicetree-discuss, Kukjin Kim,
	bhelgaas, linux-arm-kernel, Michal Simek, linux-kernel,
	suren.reddy

Dear Jason Cooper,

On Tue, 16 Apr 2013 07:29:51 -0400, Jason Cooper wrote:

> > Do not send this series upstream (or even in -next) until it has been at
> > least build-tested and acked by the affected architectures. AFAIK it
> > breaks microblaze build (simple missing #include) and isn't yet tested
> > on powerpc (I'm on vacation, maybe somebody else on linuxppc-dev can do
> > it though).
> 
> I added the missing include reference by Michal Simek for the branch at
> mvebu/drivers.

Some other build issues were reported on x86 during the night (thanks
to automated build tests), and Andrew Murray has just sent a new
version v7 of the patch set addressing those issues, as well as the
missing include.

Also, please note that Andrew's patch set contains three patches, but
the Marvell PCIe driver only requires PATCH 1/3 and PATCH 2/3.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing
  2013-04-16 11:31         ` Thomas Petazzoni
@ 2013-04-16 11:37           ` Jason Cooper
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Cooper @ 2013-04-16 11:37 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Benjamin Herrenschmidt, Linus Walleij, Andrew Murray,
	Rob Herring, siva.kallam, linux-pci, Thierry Reding, Liviu.Dudau,
	Grant Likely, Paul Mackerras, linux-samsung-soc,
	Russell King - ARM Linux, Jingoo Han, Jason Gunthorpe,
	Thomas Abraham, Arnd Bergmann, devicetree-discuss, Kukjin Kim,
	bhelgaas, linux-arm-kernel, Michal Simek, linux-kernel,
	suren.reddy

On Tue, Apr 16, 2013 at 01:31:40PM +0200, Thomas Petazzoni wrote:
> Dear Jason Cooper,
> 
> On Tue, 16 Apr 2013 07:29:51 -0400, Jason Cooper wrote:
> 
> > > Do not send this series upstream (or even in -next) until it has been at
> > > least build-tested and acked by the affected architectures. AFAIK it
> > > breaks microblaze build (simple missing #include) and isn't yet tested
> > > on powerpc (I'm on vacation, maybe somebody else on linuxppc-dev can do
> > > it though).
> > 
> > I added the missing include reference by Michal Simek for the branch at
> > mvebu/drivers.
> 
> Some other build issues were reported on x86 during the night (thanks
> to automated build tests), and Andrew Murray has just sent a new
> version v7 of the patch set addressing those issues, as well as the
> missing include.
> 
> Also, please note that Andrew's patch set contains three patches, but
> the Marvell PCIe driver only requires PATCH 1/3 and PATCH 2/3.

Ok.  Thanks Andrew!  Do you (AndrewM) want me to keep the series
together?  Or drop 3/3 from my tree?  I can do either, I just need to
know.

thx,

Jason.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-04-16 11:37 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-11 15:26 [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing Andrew Murray
2013-04-11 15:26 ` [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC Andrew Murray
2013-04-15 12:57   ` Thomas Petazzoni
2013-04-15 15:32     ` Benjamin Herrenschmidt
2013-04-15 16:50     ` Michal Simek
2013-04-11 15:26 ` [PATCH v6 2/3] of/pci: Provide support for parsing PCI DT ranges property Andrew Murray
2013-04-11 15:26 ` [PATCH v6 3/3] of/pci: mips: convert to common of_pci_range_parser Andrew Murray
2013-04-11 16:55 ` [PATCH v6 0/3] of/pci: Provide common support for PCI DT parsing Linus Walleij
2013-04-15 17:56 ` Jason Cooper
     [not found]   ` <CACRpkdbSd8pRG5pUS0ymqXDSp=ZhoQa2ctVuRA0cCmUcX5qFAA@mail.gmail.com>
2013-04-16  7:12     ` Benjamin Herrenschmidt
     [not found]       ` <20130416112951.GZ28693@titan.lakedaemon.net>
2013-04-16 11:31         ` Thomas Petazzoni
2013-04-16 11:37           ` Jason Cooper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).