[PATCH 0/3][POWERPC] Add PCI support for 44xEPx

[PATCH 0/3][POWERPC] Add PCI support for 44xEPx

[Vitaly Bordug]

The following series is a port of 440EPx (sequoia) PCI host bridge from
arch/ppc. It was initiated with some inconsistencies observed in pci_32.c
pci ranges parsing, so I've eventually merged pci_32 and pci_64 of
pci_process_bridge_OF_ranges() into one, that should be able to handle
64-bit phys addresses, including fancy 44x 36-bit addressing.

Actual PCI stuff was implemented more as a use-case for the upper, and is
more or less straight port from arch/ppc. It assumes 

[PATCH 0/4] PowerPC 440EPx: Initial Sequoia support take 2

from Valentine applied, and hereby is for for-2.6.24 branch of jwb's tree
(git pull git://git.infradead.org/users/jwboyer/powerpc.git for-2.6.24)

-- 
Sincerely, Vitaly

[PATCH 1/3] [POWERPC] Merge 32 and 64 bit pci_process_bridge_OF_ranges() instances

[Vitaly Bordug]

We are having 2 different instances of pci_process_bridge_OF_ranges(),
which makes describing 64-bit physical addresses in non PPC64 case
impossible.

This approach inherits pci space parsing, but has a new way to behave
equally good in both 32bit and 64bit environments. Currently validated
with 440EPx (sequoia) and mpc8349-eitx.

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Signed-off-by: Stefan Roese <sr@denx.de>

---

 arch/powerpc/kernel/pci-common.c |  159 ++++++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/pci_32.c     |  114 ---------------------------
 arch/powerpc/kernel/pci_64.c     |   94 ----------------------
 include/asm-powerpc/ppc-pci.h    |   16 ++++
 4 files changed, 175 insertions(+), 208 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 083cfbd..57cd039 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -478,3 +478,162 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
 	*start = rsrc->start - offset;
 	*end = rsrc->end - offset;
 }
+
+void __devinit pci_process_bridge_OF_ranges(struct pci_controller *hose,
+					    struct device_node *dev, int prim)
+{
+	static unsigned int static_lc_ranges[256];
+	const unsigned int *ranges;
+	unsigned int *lc_ranges;
+	unsigned int pci_space;
+	unsigned long size = 0;
+	int rlen = 0;
+	int orig_rlen, ranges_amnt, i;
+	int memno = 0;
+	struct resource *res;
+	int np, na = of_n_addr_cells(dev);
+	struct ranges_pci64_sz64 *ranges64 = NULL;
+	struct ranges_pci32_sz64 *ranges32 = NULL;
+	phys_addr_t pci_addr, cpu_phys_addr;
+
+	np = na + 5;
+
+	/* From "PCI Binding to 1275"
+	 * The ranges property is laid out as an array of elements,
+	 * each of which comprises:
+	 *   cells 0 - 2:       a PCI address
+	 *   cells 3 or 3+4:    a CPU physical address
+	 *                      (size depending on dev->n_addr_cells)
+	 *   cells 4+5 or 5+6:  the size of the range
+	 */
+	ranges = of_get_property(dev, "ranges", &rlen);
+	if (ranges == NULL)
+		return;
+	/* Sanity check, though hopefully that never happens */
+	if (rlen > sizeof(static_lc_ranges)) {
+		printk(KERN_WARNING "OF ranges property too large !\n");
+		rlen = sizeof(static_lc_ranges);
+	}
+
+	/* Let's work on a copy of the "ranges" property instead
+	 * of damaging the device-tree image in memory
+	 */
+	lc_ranges = static_lc_ranges;
+	memcpy(lc_ranges, ranges, rlen);
+	orig_rlen = rlen;
+
+	ranges = lc_ranges;
+	/* Map ranges to struct according to spec. */
+	if (na >= 2) {
+		ranges64 = (void *)ranges;
+		ranges_amnt = rlen / sizeof(*ranges64);
+	} else {
+		ranges32 = (void *)ranges;
+		ranges_amnt = rlen / sizeof(*ranges32);
+	}
+
+	hose->io_base_phys = 0;
+	for (i = 0; i < ranges_amnt; i++) {
+		res = NULL;
+		if (ranges64) {
+			if (ranges64[i].pci_space == 0)
+				continue;
+
+			pci_space = ranges64[i].pci_space;
+			pci_addr =
+			    (u64) ranges64[i].pci_addr[0] << 32 | ranges64[i].
+			    pci_addr[1];
+			cpu_phys_addr =
+			    of_translate_address(dev, ranges64[i].phys_addr);
+			/*
+			 * I haven't observed 2 significant size cells in kernel
+			 * code, so we're accounting only LSB of size part
+			 * from ranges. -vitb
+			 */
+			size = ranges64[i].size[1];
+#ifdef CONFIG_PPC64
+			if (ranges64[i].size[0])
+				size |= ranges64[i].size[0]<<32;
+#endif
+			DBG("Observed: pci %llx phys %llx size %x\n", pci_addr,
+			    cpu_phys_addr, size);
+		} else {
+			if (ranges32[i].pci_space == 0)
+				continue;
+
+			pci_space = (unsigned int)ranges32[i].pci_space;
+			pci_addr = (unsigned int)ranges32[i].pci_addr[1];
+			cpu_phys_addr = (unsigned int)ranges32[i].phys_addr[0];
+			size = ranges32[i].size[1];
+
+			DBG("Observed: pci %x phys %x size %x\n",
+			    (u32) pci_addr, (u32) cpu_phys_addr, size);
+		}
+
+		switch ((pci_space >> 24) & 0x3) {
+		case 1:	/* I/O space */
+#ifdef CONFIG_PPC32
+			/*
+			 * check from ppc32 pci implementation.
+			 * not sure if it is needed. -vitb
+			 */
+			if (pci_addr != 0)
+				break;
+#endif
+			/* limit I/O space to 16MB */
+			if (size > 0x01000000)
+				size = 0x01000000;
+
+			hose->io_base_phys = cpu_phys_addr - pci_addr;
+			/* handle from 0 to top of I/O window */
+#ifdef CONFIG_PPC64
+			hose->pci_io_size = pci_addr + size;
+#endif
+			hose->io_base_virt = ioremap(hose->io_base_phys, size);
+
+			if (prim)
+				isa_io_base = (unsigned long)hose->io_base_virt;
+
+			res = &hose->io_resource;
+			res->flags = IORESOURCE_IO;
+			res->start = pci_addr;
+			DBG("phb%d: IO 0x%llx -> 0x%llx\n", hose->global_number,
+			    (u64) res->start, (u64) (res->start + size - 1));
+			DBG("IO phys %llx IO virt %p\n",
+			    (u64) hose->io_base_phys, hose->io_base_virt);
+			break;
+		case 2:	/* memory space */
+			memno = 0;
+#ifdef CONFIG_PPC32
+			if ((pci_addr == 0) && (size <= (16 << 20))) {
+				/* 1st 16MB, i.e. ISA memory area */
+				if (prim)
+					isa_mem_base = cpu_phys_addr;
+				memno = 1;
+			}
+#endif
+			while (memno < 3 && hose->mem_resources[memno].flags)
+				++memno;
+
+			if (memno == 0)
+				hose->pci_mem_offset = cpu_phys_addr - pci_addr;
+			if (memno < 3) {
+				res = &hose->mem_resources[memno];
+				res->flags = IORESOURCE_MEM;
+				res->start = cpu_phys_addr;
+				DBG("phb%d: MEM 0x%llx -> 0x%llx\n",
+				    hose->global_number, res->start,
+				    res->start + size - 1);
+			}
+			break;
+		}
+		if (res != NULL) {
+			res->name = dev->full_name;
+			res->end = res->start + size - 1;
+			res->parent = NULL;
+			res->sibling = NULL;
+			res->child = NULL;
+		}
+	}
+}
+
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 0e2bee4..dc519e1 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -843,120 +843,6 @@ pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
 }
 EXPORT_SYMBOL(pci_device_from_OF_node);
 
-void __init
-pci_process_bridge_OF_ranges(struct pci_controller *hose,
-			   struct device_node *dev, int primary)
-{
-	static unsigned int static_lc_ranges[256] __initdata;
-	const unsigned int *dt_ranges;
-	unsigned int *lc_ranges, *ranges, *prev, size;
-	int rlen = 0, orig_rlen;
-	int memno = 0;
-	struct resource *res;
-	int np, na = of_n_addr_cells(dev);
-	np = na + 5;
-
-	/* First we try to merge ranges to fix a problem with some pmacs
-	 * that can have more than 3 ranges, fortunately using contiguous
-	 * addresses -- BenH
-	 */
-	dt_ranges = of_get_property(dev, "ranges", &rlen);
-	if (!dt_ranges)
-		return;
-	/* Sanity check, though hopefully that never happens */
-	if (rlen > sizeof(static_lc_ranges)) {
-		printk(KERN_WARNING "OF ranges property too large !\n");
-		rlen = sizeof(static_lc_ranges);
-	}
-	lc_ranges = static_lc_ranges;
-	memcpy(lc_ranges, dt_ranges, rlen);
-	orig_rlen = rlen;
-
-	/* Let's work on a copy of the "ranges" property instead of damaging
-	 * the device-tree image in memory
-	 */
-	ranges = lc_ranges;
-	prev = NULL;
-	while ((rlen -= np * sizeof(unsigned int)) >= 0) {
-		if (prev) {
-			if (prev[0] == ranges[0] && prev[1] == ranges[1] &&
-				(prev[2] + prev[na+4]) == ranges[2] &&
-				(prev[na+2] + prev[na+4]) == ranges[na+2]) {
-				prev[na+4] += ranges[na+4];
-				ranges[0] = 0;
-				ranges += np;
-				continue;
-			}
-		}
-		prev = ranges;
-		ranges += np;
-	}
-
-	/*
-	 * The ranges property is laid out as an array of elements,
-	 * each of which comprises:
-	 *   cells 0 - 2:	a PCI address
-	 *   cells 3 or 3+4:	a CPU physical address
-	 *			(size depending on dev->n_addr_cells)
-	 *   cells 4+5 or 5+6:	the size of the range
-	 */
-	ranges = lc_ranges;
-	rlen = orig_rlen;
-	while (ranges && (rlen -= np * sizeof(unsigned int)) >= 0) {
-		res = NULL;
-		size = ranges[na+4];
-		switch ((ranges[0] >> 24) & 0x3) {
-		case 1:		/* I/O space */
-			if (ranges[2] != 0)
-				break;
-			hose->io_base_phys = ranges[na+2];
-			/* limit I/O space to 16MB */
-			if (size > 0x01000000)
-				size = 0x01000000;
-			hose->io_base_virt = ioremap(ranges[na+2], size);
-			if (primary)
-				isa_io_base = (unsigned long) hose->io_base_virt;
-			res = &hose->io_resource;
-			res->flags = IORESOURCE_IO;
-			res->start = ranges[2];
-			DBG("PCI: IO 0x%llx -> 0x%llx\n",
-			    (u64)res->start, (u64)res->start + size - 1);
-			break;
-		case 2:		/* memory space */
-			memno = 0;
-			if (ranges[1] == 0 && ranges[2] == 0
-			    && ranges[na+4] <= (16 << 20)) {
-				/* 1st 16MB, i.e. ISA memory area */
-				if (primary)
-					isa_mem_base = ranges[na+2];
-				memno = 1;
-			}
-			while (memno < 3 && hose->mem_resources[memno].flags)
-				++memno;
-			if (memno == 0)
-				hose->pci_mem_offset = ranges[na+2] - ranges[2];
-			if (memno < 3) {
-				res = &hose->mem_resources[memno];
-				res->flags = IORESOURCE_MEM;
-				if(ranges[0] & 0x40000000)
-					res->flags |= IORESOURCE_PREFETCH;
-				res->start = ranges[na+2];
-				DBG("PCI: MEM[%d] 0x%llx -> 0x%llx\n", memno,
-				    (u64)res->start, (u64)res->start + size - 1);
-			}
-			break;
-		}
-		if (res != NULL) {
-			res->name = dev->full_name;
-			res->end = res->start + size - 1;
-			res->parent = NULL;
-			res->sibling = NULL;
-			res->child = NULL;
-		}
-		ranges += np;
-	}
-}
-
 /* We create the "pci-OF-bus-map" property now so it appears in the
  * /proc device tree
  */
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 291ffbc..68bce38 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -592,100 +592,6 @@ int pci_proc_domain(struct pci_bus *bus)
 	}
 }
 
-void __devinit pci_process_bridge_OF_ranges(struct pci_controller *hose,
-					    struct device_node *dev, int prim)
-{
-	const unsigned int *ranges;
-	unsigned int pci_space;
-	unsigned long size;
-	int rlen = 0;
-	int memno = 0;
-	struct resource *res;
-	int np, na = of_n_addr_cells(dev);
-	unsigned long pci_addr, cpu_phys_addr;
-
-	np = na + 5;
-
-	/* From "PCI Binding to 1275"
-	 * The ranges property is laid out as an array of elements,
-	 * each of which comprises:
-	 *   cells 0 - 2:	a PCI address
-	 *   cells 3 or 3+4:	a CPU physical address
-	 *			(size depending on dev->n_addr_cells)
-	 *   cells 4+5 or 5+6:	the size of the range
-	 */
-	ranges = of_get_property(dev, "ranges", &rlen);
-	if (ranges == NULL)
-		return;
-	hose->io_base_phys = 0;
-	while ((rlen -= np * sizeof(unsigned int)) >= 0) {
-		res = NULL;
-		pci_space = ranges[0];
-		pci_addr = ((unsigned long)ranges[1] << 32) | ranges[2];
-		cpu_phys_addr = of_translate_address(dev, &ranges[3]);
-		size = ((unsigned long)ranges[na+3] << 32) | ranges[na+4];
-		ranges += np;
-		if (size == 0)
-			continue;
-
-		/* Now consume following elements while they are contiguous */
-		while (rlen >= np * sizeof(unsigned int)) {
-			unsigned long addr, phys;
-
-			if (ranges[0] != pci_space)
-				break;
-			addr = ((unsigned long)ranges[1] << 32) | ranges[2];
-			phys = ranges[3];
-			if (na >= 2)
-				phys = (phys << 32) | ranges[4];
-			if (addr != pci_addr + size ||
-			    phys != cpu_phys_addr + size)
-				break;
-
-			size += ((unsigned long)ranges[na+3] << 32)
-				| ranges[na+4];
-			ranges += np;
-			rlen -= np * sizeof(unsigned int);
-		}
-
-		switch ((pci_space >> 24) & 0x3) {
-		case 1:		/* I/O space */
-			hose->io_base_phys = cpu_phys_addr - pci_addr;
-			/* handle from 0 to top of I/O window */
-			hose->pci_io_size = pci_addr + size;
-
-			res = &hose->io_resource;
-			res->flags = IORESOURCE_IO;
-			res->start = pci_addr;
-			DBG("phb%d: IO 0x%lx -> 0x%lx\n", hose->global_number,
-				    res->start, res->start + size - 1);
-			break;
-		case 2:		/* memory space */
-			memno = 0;
-			while (memno < 3 && hose->mem_resources[memno].flags)
-				++memno;
-
-			if (memno == 0)
-				hose->pci_mem_offset = cpu_phys_addr - pci_addr;
-			if (memno < 3) {
-				res = &hose->mem_resources[memno];
-				res->flags = IORESOURCE_MEM;
-				res->start = cpu_phys_addr;
-				DBG("phb%d: MEM 0x%lx -> 0x%lx\n", hose->global_number,
-					    res->start, res->start + size - 1);
-			}
-			break;
-		}
-		if (res != NULL) {
-			res->name = dev->full_name;
-			res->end = res->start + size - 1;
-			res->parent = NULL;
-			res->sibling = NULL;
-			res->child = NULL;
-		}
-	}
-}
-
 #ifdef CONFIG_HOTPLUG
 
 int pcibios_unmap_io_space(struct pci_bus *bus)
diff --git a/include/asm-powerpc/ppc-pci.h b/include/asm-powerpc/ppc-pci.h
index b847aa1..cd8ad87 100644
--- a/include/asm-powerpc/ppc-pci.h
+++ b/include/asm-powerpc/ppc-pci.h
@@ -15,6 +15,22 @@
 #include <linux/pci.h>
 #include <asm/pci-bridge.h>
 
+/* Address-cells and size-cells 2 */
+struct ranges_pci64_sz64 {
+	unsigned int pci_space;
+	unsigned int pci_addr[2];
+	unsigned int phys_addr[2];
+	unsigned int size[2];
+};
+
+/* Address-cells 1 */
+struct ranges_pci32_sz64 {
+	unsigned int pci_space;
+	unsigned int pci_addr[2];
+	unsigned int phys_addr[1];
+	unsigned int size[2];
+};
+
 extern unsigned long isa_io_base;
 
 extern void pci_setup_phb_io(struct pci_controller *hose, int primary);

Re: [PATCH 1/3] [POWERPC] Merge 32 and 64 bit pci_process_bridge_OF_ranges() instances

[David Gibson]

On Sat, Aug 25, 2007 at 01:29:47PM +0400, Vitaly Bordug wrote:
> 
> We are having 2 different instances of pci_process_bridge_OF_ranges(),
> which makes describing 64-bit physical addresses in non PPC64 case
> impossible.
> 
> This approach inherits pci space parsing, but has a new way to behave
> equally good in both 32bit and 64bit environments. Currently validated
> with 440EPx (sequoia) and mpc8349-eitx.
> 
> Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> Signed-off-by: Stefan Roese <sr@denx.de>

I like the idea, but I don't think this implementation is adequate
yet.

> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 083cfbd..57cd039 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -478,3 +478,162 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar,
>  	*start = rsrc->start - offset;
>  	*end = rsrc->end - offset;
>  }
> +
> +void __devinit pci_process_bridge_OF_ranges(struct pci_controller *hose,
> +					    struct device_node *dev, int prim)
> +{
> +	static unsigned int static_lc_ranges[256];
> +	const unsigned int *ranges;
> +	unsigned int *lc_ranges;
> +	unsigned int pci_space;
> +	unsigned long size = 0;

size can be 64-bit on 32-bit systems, at least in theory.

> +	int rlen = 0;
> +	int orig_rlen, ranges_amnt, i;
> +	int memno = 0;
> +	struct resource *res;
> +	int np, na = of_n_addr_cells(dev);
> +	struct ranges_pci64_sz64 *ranges64 = NULL;
> +	struct ranges_pci32_sz64 *ranges32 = NULL;
> +	phys_addr_t pci_addr, 

This is wrong: the PCI binding defines the PCI addresses to be 64-bit,
even if the CPU has 32-bit physical addresses.

cpu_phys_addr;
> +
> +	np = na + 5;
> +
> +	/* From "PCI Binding to 1275"
z> +	 * The ranges property is laid out as an array of elements,
> +	 * each of which comprises:
> +	 *   cells 0 - 2:       a PCI address
> +	 *   cells 3 or 3+4:    a CPU physical address
> +	 *                      (size depending on dev->n_addr_cells)
> +	 *   cells 4+5 or 5+6:  the size of the range
> +	 */
> +	ranges = of_get_property(dev, "ranges", &rlen);
> +	if (ranges == NULL)
> +		return;

if (!ranges) would be the usual idiom here.

> +	/* Sanity check, though hopefully that never happens */
> +	if (rlen > sizeof(static_lc_ranges)) {
> +		printk(KERN_WARNING "OF ranges property too large !\n");
> +		rlen = sizeof(static_lc_ranges);
> +	}
> +
> +	/* Let's work on a copy of the "ranges" property instead
> +	 * of damaging the device-tree image in memory
> +	 */
> +	lc_ranges = static_lc_ranges;
> +	memcpy(lc_ranges, ranges, rlen);
> +	orig_rlen = rlen;
> +
> +	ranges = lc_ranges;

You don't ever actually touch the ranges property in place, so there's
no need for this copy stuff.

> +	/* Map ranges to struct according to spec. */
> +	if (na >= 2) {
> +		ranges64 = (void *)ranges;
> +		ranges_amnt = rlen / sizeof(*ranges64);
> +	} else {
> +		ranges32 = (void *)ranges;
> +		ranges_amnt = rlen / sizeof(*ranges32);
> +	}
> +
> +	hose->io_base_phys = 0;
> +	for (i = 0; i < ranges_amnt; i++) {
> +		res = NULL;
> +		if (ranges64) {
> +			if (ranges64[i].pci_space == 0)
> +				continue;
> +
> +			pci_space = ranges64[i].pci_space;
> +			pci_addr =
> +			    (u64) ranges64[i].pci_addr[0] << 32 | ranges64[i].
> +			    pci_addr[1];

Why not just define the pci_addr field in your structures as u64?  You
would have to define the structure with attribute((packed)), I guess.

> +			cpu_phys_addr =
> +			    of_translate_address(dev, ranges64[i].phys_addr);
> +			/*
> +			 * I haven't observed 2 significant size cells in kernel
> +			 * code, so we're accounting only LSB of size part
> +			 * from ranges. -vitb
> +			 */
> +			size = ranges64[i].size[1];
> +#ifdef CONFIG_PPC64
> +			if (ranges64[i].size[0])
> +				size |= ranges64[i].size[0]<<32;
> +#endif
> +			DBG("Observed: pci %llx phys %llx size %x\n", pci_addr,
> +			    cpu_phys_addr, size);
> +		} else {
> +			if (ranges32[i].pci_space == 0)
> +				continue;
> +
> +			pci_space = (unsigned int)ranges32[i].pci_space;
> +			pci_addr = (unsigned int)ranges32[i].pci_addr[1];
> +			cpu_phys_addr = (unsigned int)ranges32[i].phys_addr[0];


We should really be using of_translate_address() in all cases - that's
what it's for, after all.

> +			size = ranges32[i].size[1];
> +
> +			DBG("Observed: pci %x phys %x size %x\n",
> +			    (u32) pci_addr, (u32) cpu_phys_addr, size);
> +		}

You don't have any equivalent of the code that exists in both
pre-existing versions to coalesce contiguous ranges.  You probably
want to use the 64-bit version, since that doesn't need a local copy
of the ranges.

> +
> +		switch ((pci_space >> 24) & 0x3) {
> +		case 1:	/* I/O space */
> +#ifdef CONFIG_PPC32
> +			/*
> +			 * check from ppc32 pci implementation.
> +			 * not sure if it is needed. -vitb
> +			 */
> +			if (pci_addr != 0)
> +				break;
> +#endif
> +			/* limit I/O space to 16MB */
> +			if (size > 0x01000000)
> +				size = 0x01000000;
> +
> +			hose->io_base_phys = cpu_phys_addr - pci_addr;
> +			/* handle from 0 to top of I/O window */
> +#ifdef CONFIG_PPC64
> +			hose->pci_io_size = pci_addr + size;
> +#endif
> +			hose->io_base_virt = ioremap(hose->io_base_phys, size);
> +
> +			if (prim)
> +				isa_io_base = (unsigned long)hose->io_base_virt;

The old 64-bit versions don't presently ioremap() or set isa_io_base.
I'd be worried about changing this semantic, at least without a rather
more widespread consolidation of the 32/64 bit PCI code.

> +
> +			res = &hose->io_resource;
> +			res->flags = IORESOURCE_IO;
> +			res->start = pci_addr;
> +			DBG("phb%d: IO 0x%llx -> 0x%llx\n", hose->global_number,
> +			    (u64) res->start, (u64) (res->start + size - 1));
> +			DBG("IO phys %llx IO virt %p\n",
> +			    (u64) hose->io_base_phys, hose->io_base_virt);
> +			break;
> +		case 2:	/* memory space */
> +			memno = 0;
> +#ifdef CONFIG_PPC32
> +			if ((pci_addr == 0) && (size <= (16 << 20))) {
> +				/* 1st 16MB, i.e. ISA memory area */
> +				if (prim)
> +					isa_mem_base = cpu_phys_addr;
> +				memno = 1;
> +			}
> +#endif
> +			while (memno < 3 && hose->mem_resources[memno].flags)
> +				++memno;
> +
> +			if (memno == 0)
> +				hose->pci_mem_offset = cpu_phys_addr - pci_addr;
> +			if (memno < 3) {
> +				res = &hose->mem_resources[memno];
> +				res->flags = IORESOURCE_MEM;
> +				res->start = cpu_phys_addr;
> +				DBG("phb%d: MEM 0x%llx -> 0x%llx\n",
> +				    hose->global_number, res->start,
> +				    res->start + size - 1);
> +			}
> +			break;
> +		}
> +		if (res != NULL) {
> +			res->name = dev->full_name;
> +			res->end = res->start + size - 1;
> +			res->parent = NULL;
> +			res->sibling = NULL;
> +			res->child = NULL;
> +		}
> +	}
> +}
> +
> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
> index 0e2bee4..dc519e1 100644
> --- a/arch/powerpc/kernel/pci_32.c
> +++ b/arch/powerpc/kernel/pci_32.c
> @@ -843,120 +843,6 @@ pci_device_from_OF_node(struct device_node* node, u8* bus, u8* devfn)
>  }
>  EXPORT_SYMBOL(pci_device_from_OF_node);
>  
> -void __init
> -pci_process_bridge_OF_ranges(struct pci_controller *hose,
> -			   struct device_node *dev, int primary)
> -{
> -	static unsigned int static_lc_ranges[256] __initdata;
> -	const unsigned int *dt_ranges;
> -	unsigned int *lc_ranges, *ranges, *prev, size;
> -	int rlen = 0, orig_rlen;
> -	int memno = 0;
> -	struct resource *res;
> -	int np, na = of_n_addr_cells(dev);
> -	np = na + 5;
> -
> -	/* First we try to merge ranges to fix a problem with some pmacs
> -	 * that can have more than 3 ranges, fortunately using contiguous
> -	 * addresses -- BenH
> -	 */
> -	dt_ranges = of_get_property(dev, "ranges", &rlen);
> -	if (!dt_ranges)
> -		return;
> -	/* Sanity check, though hopefully that never happens */
> -	if (rlen > sizeof(static_lc_ranges)) {
> -		printk(KERN_WARNING "OF ranges property too large !\n");
> -		rlen = sizeof(static_lc_ranges);
> -	}
> -	lc_ranges = static_lc_ranges;
> -	memcpy(lc_ranges, dt_ranges, rlen);
> -	orig_rlen = rlen;
> -
> -	/* Let's work on a copy of the "ranges" property instead of damaging
> -	 * the device-tree image in memory
> -	 */
> -	ranges = lc_ranges;
> -	prev = NULL;
> -	while ((rlen -= np * sizeof(unsigned int)) >= 0) {
> -		if (prev) {
> -			if (prev[0] == ranges[0] && prev[1] == ranges[1] &&
> -				(prev[2] + prev[na+4]) == ranges[2] &&
> -				(prev[na+2] + prev[na+4]) == ranges[na+2]) {
> -				prev[na+4] += ranges[na+4];
> -				ranges[0] = 0;
> -				ranges += np;
> -				continue;
> -			}
> -		}
> -		prev = ranges;
> -		ranges += np;
> -	}
> -
> -	/*
> -	 * The ranges property is laid out as an array of elements,
> -	 * each of which comprises:
> -	 *   cells 0 - 2:	a PCI address
> -	 *   cells 3 or 3+4:	a CPU physical address
> -	 *			(size depending on dev->n_addr_cells)
> -	 *   cells 4+5 or 5+6:	the size of the range
> -	 */
> -	ranges = lc_ranges;
> -	rlen = orig_rlen;
> -	while (ranges && (rlen -= np * sizeof(unsigned int)) >= 0) {
> -		res = NULL;
> -		size = ranges[na+4];
> -		switch ((ranges[0] >> 24) & 0x3) {
> -		case 1:		/* I/O space */
> -			if (ranges[2] != 0)
> -				break;
> -			hose->io_base_phys = ranges[na+2];
> -			/* limit I/O space to 16MB */
> -			if (size > 0x01000000)
> -				size = 0x01000000;
> -			hose->io_base_virt = ioremap(ranges[na+2], size);
> -			if (primary)
> -				isa_io_base = (unsigned long) hose->io_base_virt;
> -			res = &hose->io_resource;
> -			res->flags = IORESOURCE_IO;
> -			res->start = ranges[2];
> -			DBG("PCI: IO 0x%llx -> 0x%llx\n",
> -			    (u64)res->start, (u64)res->start + size - 1);
> -			break;
> -		case 2:		/* memory space */
> -			memno = 0;
> -			if (ranges[1] == 0 && ranges[2] == 0
> -			    && ranges[na+4] <= (16 << 20)) {
> -				/* 1st 16MB, i.e. ISA memory area */
> -				if (primary)
> -					isa_mem_base = ranges[na+2];
> -				memno = 1;
> -			}
> -			while (memno < 3 && hose->mem_resources[memno].flags)
> -				++memno;
> -			if (memno == 0)
> -				hose->pci_mem_offset = ranges[na+2] - ranges[2];
> -			if (memno < 3) {
> -				res = &hose->mem_resources[memno];
> -				res->flags = IORESOURCE_MEM;
> -				if(ranges[0] & 0x40000000)
> -					res->flags |= IORESOURCE_PREFETCH;
> -				res->start = ranges[na+2];
> -				DBG("PCI: MEM[%d] 0x%llx -> 0x%llx\n", memno,
> -				    (u64)res->start, (u64)res->start + size - 1);
> -			}
> -			break;
> -		}
> -		if (res != NULL) {
> -			res->name = dev->full_name;
> -			res->end = res->start + size - 1;
> -			res->parent = NULL;
> -			res->sibling = NULL;
> -			res->child = NULL;
> -		}
> -		ranges += np;
> -	}
> -}
> -
>  /* We create the "pci-OF-bus-map" property now so it appears in the
>   * /proc device tree
>   */
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 291ffbc..68bce38 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -592,100 +592,6 @@ int pci_proc_domain(struct pci_bus *bus)
>  	}
>  }
>  
> -void __devinit pci_process_bridge_OF_ranges(struct pci_controller *hose,
> -					    struct device_node *dev, int prim)
> -{
> -	const unsigned int *ranges;
> -	unsigned int pci_space;
> -	unsigned long size;
> -	int rlen = 0;
> -	int memno = 0;
> -	struct resource *res;
> -	int np, na = of_n_addr_cells(dev);
> -	unsigned long pci_addr, cpu_phys_addr;
> -
> -	np = na + 5;
> -
> -	/* From "PCI Binding to 1275"
> -	 * The ranges property is laid out as an array of elements,
> -	 * each of which comprises:
> -	 *   cells 0 - 2:	a PCI address
> -	 *   cells 3 or 3+4:	a CPU physical address
> -	 *			(size depending on dev->n_addr_cells)
> -	 *   cells 4+5 or 5+6:	the size of the range
> -	 */
> -	ranges = of_get_property(dev, "ranges", &rlen);
> -	if (ranges == NULL)
> -		return;
> -	hose->io_base_phys = 0;
> -	while ((rlen -= np * sizeof(unsigned int)) >= 0) {
> -		res = NULL;
> -		pci_space = ranges[0];
> -		pci_addr = ((unsigned long)ranges[1] << 32) | ranges[2];
> -		cpu_phys_addr = of_translate_address(dev, &ranges[3]);
> -		size = ((unsigned long)ranges[na+3] << 32) | ranges[na+4];
> -		ranges += np;
> -		if (size == 0)
> -			continue;
> -
> -		/* Now consume following elements while they are contiguous */
> -		while (rlen >= np * sizeof(unsigned int)) {
> -			unsigned long addr, phys;
> -
> -			if (ranges[0] != pci_space)
> -				break;
> -			addr = ((unsigned long)ranges[1] << 32) | ranges[2];
> -			phys = ranges[3];
> -			if (na >= 2)
> -				phys = (phys << 32) | ranges[4];
> -			if (addr != pci_addr + size ||
> -			    phys != cpu_phys_addr + size)
> -				break;
> -
> -			size += ((unsigned long)ranges[na+3] << 32)
> -				| ranges[na+4];
> -			ranges += np;
> -			rlen -= np * sizeof(unsigned int);
> -		}
> -
> -		switch ((pci_space >> 24) & 0x3) {
> -		case 1:		/* I/O space */
> -			hose->io_base_phys = cpu_phys_addr - pci_addr;
> -			/* handle from 0 to top of I/O window */
> -			hose->pci_io_size = pci_addr + size;
> -
> -			res = &hose->io_resource;
> -			res->flags = IORESOURCE_IO;
> -			res->start = pci_addr;
> -			DBG("phb%d: IO 0x%lx -> 0x%lx\n", hose->global_number,
> -				    res->start, res->start + size - 1);
> -			break;
> -		case 2:		/* memory space */
> -			memno = 0;
> -			while (memno < 3 && hose->mem_resources[memno].flags)
> -				++memno;
> -
> -			if (memno == 0)
> -				hose->pci_mem_offset = cpu_phys_addr - pci_addr;
> -			if (memno < 3) {
> -				res = &hose->mem_resources[memno];
> -				res->flags = IORESOURCE_MEM;
> -				res->start = cpu_phys_addr;
> -				DBG("phb%d: MEM 0x%lx -> 0x%lx\n", hose->global_number,
> -					    res->start, res->start + size - 1);
> -			}
> -			break;
> -		}
> -		if (res != NULL) {
> -			res->name = dev->full_name;
> -			res->end = res->start + size - 1;
> -			res->parent = NULL;
> -			res->sibling = NULL;
> -			res->child = NULL;
> -		}
> -	}
> -}
> -
>  #ifdef CONFIG_HOTPLUG
>  
>  int pcibios_unmap_io_space(struct pci_bus *bus)
> diff --git a/include/asm-powerpc/ppc-pci.h b/include/asm-powerpc/ppc-pci.h
> index b847aa1..cd8ad87 100644
> --- a/include/asm-powerpc/ppc-pci.h
> +++ b/include/asm-powerpc/ppc-pci.h
> @@ -15,6 +15,22 @@
>  #include <linux/pci.h>
>  #include <asm/pci-bridge.h>
>  
> +/* Address-cells and size-cells 2 */
> +struct ranges_pci64_sz64 {
> +	unsigned int pci_space;
> +	unsigned int pci_addr[2];
> +	unsigned int phys_addr[2];
> +	unsigned int size[2];
> +};
> +
> +/* Address-cells 1 */
> +struct ranges_pci32_sz64 {
> +	unsigned int pci_space;
> +	unsigned int pci_addr[2];
> +	unsigned int phys_addr[1];
> +	unsigned int size[2];
> +};
> +
>  extern unsigned long isa_io_base;
>  
>  extern void pci_setup_phb_io(struct pci_controller *hose, int primary);
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 1/3] [POWERPC] Merge 32 and 64 bit pci_process_bridge_OF_ranges() instances

[Vitaly Bordug]

On Mon, 27 Aug 2007 11:15:16 +1000
David Gibson wrote:

> On Sat, Aug 25, 2007 at 01:29:47PM +0400, Vitaly Bordug wrote:
> >=20
> > We are having 2 different instances of
> > pci_process_bridge_OF_ranges(), which makes describing 64-bit
> > physical addresses in non PPC64 case impossible.
> >=20
> > This approach inherits pci space parsing, but has a new way to
> > behave equally good in both 32bit and 64bit environments. Currently
> > validated with 440EPx (sequoia) and mpc8349-eitx.
> >=20
> > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > Signed-off-by: Stefan Roese <sr@denx.de>
>=20
> I like the idea, but I don't think this implementation is adequate
> yet.

OK, I'll try to address notes below, thanks for looking at it.
>=20
> > diff --git a/arch/powerpc/kernel/pci-common.c
> > b/arch/powerpc/kernel/pci-common.c index 083cfbd..57cd039 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -478,3 +478,162 @@ void pci_resource_to_user(const struct
> > pci_dev *dev, int bar, *start =3D rsrc->start - offset;
> >  	*end =3D rsrc->end - offset;
> >  }
> > +
> > +void __devinit pci_process_bridge_OF_ranges(struct pci_controller
> > *hose,
> > +					    struct device_node
> > *dev, int prim) +{
> > +	static unsigned int static_lc_ranges[256];
> > +	const unsigned int *ranges;
> > +	unsigned int *lc_ranges;
> > +	unsigned int pci_space;
> > +	unsigned long size =3D 0;
>=20
> size can be 64-bit on 32-bit systems, at least in theory.
>=20
hm, well, but later, we'll call ioremap (at least for IO region) that has s=
ize passed as ulong type. And,  such a size could be consumed by the code,o=
nly if resource_size_t is 64bit too. I'll look at it further.
> > +	int rlen =3D 0;
> > +	int orig_rlen, ranges_amnt, i;
> > +	int memno =3D 0;
> > +	struct resource *res;
> > +	int np, na =3D of_n_addr_cells(dev);
> > +	struct ranges_pci64_sz64 *ranges64 =3D NULL;
> > +	struct ranges_pci32_sz64 *ranges32 =3D NULL;
> > +	phys_addr_t pci_addr,=20
>=20
> This is wrong: the PCI binding defines the PCI addresses to be 64-bit,
> even if the CPU has 32-bit physical addresses.
>=20
hmm, ok
> cpu_phys_addr;
> > +
> > +	np =3D na + 5;
> > +
> > +	/* From "PCI Binding to 1275"
> z> +	 * The ranges property is laid out as an array of
> z> elements,
> > +	 * each of which comprises:
> > +	 *   cells 0 - 2:       a PCI address
> > +	 *   cells 3 or 3+4:    a CPU physical address
> > +	 *                      (size depending on
> > dev->n_addr_cells)
> > +	 *   cells 4+5 or 5+6:  the size of the range
> > +	 */
> > +	ranges =3D of_get_property(dev, "ranges", &rlen);
> > +	if (ranges =3D=3D NULL)
> > +		return;
>=20
> if (!ranges) would be the usual idiom here.
>=20
ok
> > +	/* Sanity check, though hopefully that never happens */
> > +	if (rlen > sizeof(static_lc_ranges)) {
> > +		printk(KERN_WARNING "OF ranges property too
> > large !\n");
> > +		rlen =3D sizeof(static_lc_ranges);
> > +	}
> > +
> > +	/* Let's work on a copy of the "ranges" property instead
> > +	 * of damaging the device-tree image in memory
> > +	 */
> > +	lc_ranges =3D static_lc_ranges;
> > +	memcpy(lc_ranges, ranges, rlen);
> > +	orig_rlen =3D rlen;
> > +
> > +	ranges =3D lc_ranges;
>=20
> You don't ever actually touch the ranges property in place, so there's
> no need for this copy stuff.
>=20
ok
> > +	/* Map ranges to struct according to spec. */
> > +	if (na >=3D 2) {
> > +		ranges64 =3D (void *)ranges;
> > +		ranges_amnt =3D rlen / sizeof(*ranges64);
> > +	} else {
> > +		ranges32 =3D (void *)ranges;
> > +		ranges_amnt =3D rlen / sizeof(*ranges32);
> > +	}
> > +
> > +	hose->io_base_phys =3D 0;
> > +	for (i =3D 0; i < ranges_amnt; i++) {
> > +		res =3D NULL;
> > +		if (ranges64) {
> > +			if (ranges64[i].pci_space =3D=3D 0)
> > +				continue;
> > +
> > +			pci_space =3D ranges64[i].pci_space;
> > +			pci_addr =3D
> > +			    (u64) ranges64[i].pci_addr[0] << 32 |
> > ranges64[i].
> > +			    pci_addr[1];
>=20
> Why not just define the pci_addr field in your structures as u64?  You
> would have to define the structure with attribute((packed)), I guess.
>=20
that was in the first approach, but it gets really interesting numbers (and=
 with contents nothing to do with real stuff),
on platforms that are pure 32bit. I mean,=20

u32 foo, foo1;
u64 foo64;

foo =3D bar[1];
foo1 =3D bar[2];
foo64 =3D ((u64)foo << 32) | foo1;

does not seem to work on 32bit board. I may need to write a clear testcase =
and submit it here, maybe I'm missing something
very obvious.

> > +			cpu_phys_addr =3D
> > +			    of_translate_address(dev,
> > ranges64[i].phys_addr);
> > +			/*
> > +			 * I haven't observed 2 significant size
> > cells in kernel
> > +			 * code, so we're accounting only LSB of
> > size part
> > +			 * from ranges. -vitb
> > +			 */
> > +			size =3D ranges64[i].size[1];
> > +#ifdef CONFIG_PPC64
> > +			if (ranges64[i].size[0])
> > +				size |=3D ranges64[i].size[0]<<32;
> > +#endif
> > +			DBG("Observed: pci %llx phys %llx size
> > %x\n", pci_addr,
> > +			    cpu_phys_addr, size);
> > +		} else {
> > +			if (ranges32[i].pci_space =3D=3D 0)
> > +				continue;
> > +
> > +			pci_space =3D (unsigned
> > int)ranges32[i].pci_space;
> > +			pci_addr =3D (unsigned
> > int)ranges32[i].pci_addr[1];
> > +			cpu_phys_addr =3D (unsigned
> > int)ranges32[i].phys_addr[0];
>=20
>=20
> We should really be using of_translate_address() in all cases - that's
> what it's for, after all.
>=20
being called, it gives 0 in 32bit branch of  if () {. Since, iirc,
32bit range parsing does not have it in original, variant,
I have it put as is. worths a brief investigation...
> > +			size =3D ranges32[i].size[1];
> > +
> > +			DBG("Observed: pci %x phys %x size %x\n",
> > +			    (u32) pci_addr, (u32) cpu_phys_addr,
> > size);
> > +		}
>=20
> You don't have any equivalent of the code that exists in both
> pre-existing versions to coalesce contiguous ranges.  You probably
> want to use the 64-bit version, since that doesn't need a local copy
> of the ranges.
>=20
correct. yet, the whole aim of the upper seems to use summary size of
contiguous block and that's it.
How would we recognize IORESOURCE_PREFETCH then? (I know, my code is missin=
g that prefetch regions so far, but anyway).
=46rom the code, it seems to declare each resource that is part of contiguous=
 block, with the size of entire block.

That's prolly from binding, but can you please elaborate the logic for bett=
er understanding?
> > +
> > +		switch ((pci_space >> 24) & 0x3) {
> > +		case 1:	/* I/O space */
> > +#ifdef CONFIG_PPC32
> > +			/*
> > +			 * check from ppc32 pci implementation.
> > +			 * not sure if it is needed. -vitb
> > +			 */
> > +			if (pci_addr !=3D 0)
> > +				break;
> > +#endif
> > +			/* limit I/O space to 16MB */
> > +			if (size > 0x01000000)
> > +				size =3D 0x01000000;
> > +
> > +			hose->io_base_phys =3D cpu_phys_addr -
> > pci_addr;
> > +			/* handle from 0 to top of I/O window */
> > +#ifdef CONFIG_PPC64
> > +			hose->pci_io_size =3D pci_addr + size;
> > +#endif
> > +			hose->io_base_virt =3D
> > ioremap(hose->io_base_phys, size); +
> > +			if (prim)
> > +				isa_io_base =3D (unsigned
> > long)hose->io_base_virt;
>=20
> The old 64-bit versions don't presently ioremap() or set isa_io_base.
> I'd be worried about changing this semantic, at least without a rather
> more widespread consolidation of the 32/64 bit PCI code.
>=20
Will ifdef it out.
> > +
> > +			res =3D &hose->io_resource;
> > +			res->flags =3D IORESOURCE_IO;
[snip]
--=20
Sincerely, Vitaly

Re: [PATCH 1/3] [POWERPC] Merge 32 and 64 bit pci_process_bridge_OF_ranges() instances

[David Gibson]

On Mon, Aug 27, 2007 at 10:31:36AM +0400, Vitaly Bordug wrote:
> On Mon, 27 Aug 2007 11:15:16 +1000
> David Gibson wrote:
> 
> > On Sat, Aug 25, 2007 at 01:29:47PM +0400, Vitaly Bordug wrote:
[snip]
> > > +void __devinit pci_process_bridge_OF_ranges(struct pci_controller
> > > *hose,
> > > +					    struct device_node
> > > *dev, int prim) +{
> > > +	static unsigned int static_lc_ranges[256];
> > > +	const unsigned int *ranges;
> > > +	unsigned int *lc_ranges;
> > > +	unsigned int pci_space;
> > > +	unsigned long size = 0;
> > 
> > size can be 64-bit on 32-bit systems, at least in theory.
> > 
> hm, well, but later, we'll call ioremap (at least for IO region) that has size passed as ulong type. And,  such a size could be consumed by the code,only if resource_size_t is 64bit too. I'll look at it further.

You should probably actually test for that condition though, rather
than blithely truncating.

[snip]
> > > +	/* Map ranges to struct according to spec. */
> > > +	if (na >= 2) {
> > > +		ranges64 = (void *)ranges;
> > > +		ranges_amnt = rlen / sizeof(*ranges64);
> > > +	} else {
> > > +		ranges32 = (void *)ranges;
> > > +		ranges_amnt = rlen / sizeof(*ranges32);
> > > +	}
> > > +
> > > +	hose->io_base_phys = 0;
> > > +	for (i = 0; i < ranges_amnt; i++) {
> > > +		res = NULL;
> > > +		if (ranges64) {
> > > +			if (ranges64[i].pci_space == 0)
> > > +				continue;
> > > +
> > > +			pci_space = ranges64[i].pci_space;
> > > +			pci_addr =
> > > +			    (u64) ranges64[i].pci_addr[0] << 32 |
> > > ranges64[i].
> > > +			    pci_addr[1];
> > 
> > Why not just define the pci_addr field in your structures as u64?  You
> > would have to define the structure with attribute((packed)), I guess.
> > 
> that was in the first approach, but it gets really interesting numbers (and with contents nothing to do with real stuff),
> on platforms that are pure 32bit. I mean, 

I'm guessing that's because you didn't put the ((packed)) in: without
it, gcc will align the 64-bit quantity to a 64-bit boundary, inserting
32-bits of padding into the structure between pci_space and pci_addr,
so that it doesn't actually line up with the ranges property.

> u32 foo, foo1;
> u64 foo64;
> 
> foo = bar[1];
> foo1 = bar[2];
> foo64 = ((u64)foo << 32) | foo1;
> 
> does not seem to work on 32bit board. I may need to write a clear testcase and submit it here, maybe I'm missing something
> very obvious.

!?!  shouldn't be anything wrong with that arithmetic...

[snip]
> > > +			cpu_phys_addr =
> > > +			    of_translate_address(dev,
> > > ranges64[i].phys_addr);
> > > +			/*
> > > +			 * I haven't observed 2 significant size
> > > cells in kernel
> > > +			 * code, so we're accounting only LSB of
> > > size part
> > > +			 * from ranges. -vitb
> > > +			 */
> > > +			size = ranges64[i].size[1];
> > > +#ifdef CONFIG_PPC64
> > > +			if (ranges64[i].size[0])
> > > +				size |= ranges64[i].size[0]<<32;
> > > +#endif
> > > +			DBG("Observed: pci %llx phys %llx size
> > > %x\n", pci_addr,
> > > +			    cpu_phys_addr, size);
> > > +		} else {
> > > +			if (ranges32[i].pci_space == 0)
> > > +				continue;
> > > +
> > > +			pci_space = (unsigned
> > > int)ranges32[i].pci_space;
> > > +			pci_addr = (unsigned
> > > int)ranges32[i].pci_addr[1];
> > > +			cpu_phys_addr = (unsigned
> > > int)ranges32[i].phys_addr[0];
> > 
> > 
> > We should really be using of_translate_address() in all cases - that's
> > what it's for, after all.
> > 
> being called, it gives 0 in 32bit branch of  if () {. Since, iirc,
> 32bit range parsing does not have it in original, variant,
> I have it put as is. worths a brief investigation...

Then we should fix of_translate_address() for 32-bit....

I don't think this patch is actually required for the 44x pci support,
yes?  It might be better to leave this until later - it's only a tiny
piece of all the consolidations we should do between ppc32 and ppc64
PCI code.  Currently there are a lot of silly, subtle differences
between them :(.

> > > +			size = ranges32[i].size[1];
> > > +
> > > +			DBG("Observed: pci %x phys %x size %x\n",
> > > +			    (u32) pci_addr, (u32) cpu_phys_addr,
> > > size);
> > > +		}
> > 
> > You don't have any equivalent of the code that exists in both
> > pre-existing versions to coalesce contiguous ranges.  You probably
> > want to use the 64-bit version, since that doesn't need a local copy
> > of the ranges.
> > 
> correct. yet, the whole aim of the upper seems to use summary size of
> contiguous block and that's it.
> How would we recognize IORESOURCE_PREFETCH then? (I know, my code is missing that prefetch regions so far, but anyway).
> >From the code, it seems to declare each resource that is part of contiguous block, with the size of entire block.
> 
> That's prolly from binding, but can you please elaborate the logic for better understanding?

The attributes such as PREFETCH are encoded into cell 0 of the 3-cell
OF PCI address.  So, ranges with different attributes won't appear as
contiguous in this space.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 1/3] [POWERPC] Merge 32 and 64 bit pci_process_bridge_OF_ranges() instances

[Vitaly Bordug]

On Mon, 27 Aug 2007 17:49:55 +1000
David Gibson wrote:

> On Mon, Aug 27, 2007 at 10:31:36AM +0400, Vitaly Bordug wrote:
> > On Mon, 27 Aug 2007 11:15:16 +1000
> > David Gibson wrote:
> > 
> > > On Sat, Aug 25, 2007 at 01:29:47PM +0400, Vitaly Bordug wrote:
> [snip]
> > > > +void __devinit pci_process_bridge_OF_ranges(struct
> > > > pci_controller *hose,
> > > > +					    struct device_node
> > > > *dev, int prim) +{
> > > > +	static unsigned int static_lc_ranges[256];
> > > > +	const unsigned int *ranges;
> > > > +	unsigned int *lc_ranges;
> > > > +	unsigned int pci_space;
> > > > +	unsigned long size = 0;
> > > 
> > > size can be 64-bit on 32-bit systems, at least in theory.
> > > 
> > hm, well, but later, we'll call ioremap (at least for IO region)
> > that has size passed as ulong type. And,  such a size could be
> > consumed by the code,only if resource_size_t is 64bit too. I'll
> > look at it further.
> 
> You should probably actually test for that condition though, rather
> than blithely truncating.
> 

ok, makes sense.

> [snip]
> > > > +	/* Map ranges to struct according to spec. */
> > > > +	if (na >= 2) {
> > > > +		ranges64 = (void *)ranges;
> > > > +		ranges_amnt = rlen / sizeof(*ranges64);
> > > > +	} else {
> > > > +		ranges32 = (void *)ranges;
> > > > +		ranges_amnt = rlen / sizeof(*ranges32);
> > > > +	}
> > > > +
> > > > +	hose->io_base_phys = 0;
> > > > +	for (i = 0; i < ranges_amnt; i++) {
> > > > +		res = NULL;
> > > > +		if (ranges64) {
> > > > +			if (ranges64[i].pci_space == 0)
> > > > +				continue;
> > > > +
> > > > +			pci_space = ranges64[i].pci_space;
> > > > +			pci_addr =
> > > > +			    (u64) ranges64[i].pci_addr[0] <<
> > > > 32 | ranges64[i].
> > > > +			    pci_addr[1];
> > > 
> > > Why not just define the pci_addr field in your structures as
> > > u64?  You would have to define the structure with
> > > attribute((packed)), I guess.
> > > 
> > that was in the first approach, but it gets really interesting
> > numbers (and with contents nothing to do with real stuff), on
> > platforms that are pure 32bit. I mean, 
> 
> I'm guessing that's because you didn't put the ((packed)) in: without
> it, gcc will align the 64-bit quantity to a 64-bit boundary, inserting
> 32-bits of padding into the structure between pci_space and pci_addr,
> so that it doesn't actually line up with the ranges property.
> 
will check, but sounds reasonable.

> > u32 foo, foo1;
> > u64 foo64;
> > 
> > foo = bar[1];
> > foo1 = bar[2];
> > foo64 = ((u64)foo << 32) | foo1;
> > 
> > does not seem to work on 32bit board. I may need to write a clear
> > testcase and submit it here, maybe I'm missing something very
> > obvious.
> 
> !?!  shouldn't be anything wrong with that arithmetic...
> 
Wrong example, I'll have real snippet if alignment trick won't work (and I hope it will)

> [snip]
> > > > +			cpu_phys_addr =
> > > > +			    of_translate_address(dev,
> > > > ranges64[i].phys_addr);
> > > > +			/*
> > > > +			 * I haven't observed 2 significant
> > > > size cells in kernel
> > > > +			 * code, so we're accounting only LSB
> > > > of size part
> > > > +			 * from ranges. -vitb
> > > > +			 */
> > > > +			size = ranges64[i].size[1];
> > > > +#ifdef CONFIG_PPC64
> > > > +			if (ranges64[i].size[0])
> > > > +				size |=
> > > > ranges64[i].size[0]<<32; +#endif
> > > > +			DBG("Observed: pci %llx phys %llx size
> > > > %x\n", pci_addr,
> > > > +			    cpu_phys_addr, size);
> > > > +		} else {
> > > > +			if (ranges32[i].pci_space == 0)
> > > > +				continue;
> > > > +
> > > > +			pci_space = (unsigned
> > > > int)ranges32[i].pci_space;
> > > > +			pci_addr = (unsigned
> > > > int)ranges32[i].pci_addr[1];
> > > > +			cpu_phys_addr = (unsigned
> > > > int)ranges32[i].phys_addr[0];
> > > 
> > > 
> > > We should really be using of_translate_address() in all cases -
> > > that's what it's for, after all.
> > > 
> > being called, it gives 0 in 32bit branch of  if () {. Since, iirc,
> > 32bit range parsing does not have it in original, variant,
> > I have it put as is. worths a brief investigation...
> 
> Then we should fix of_translate_address() for 32-bit....
> 
> I don't think this patch is actually required for the 44x pci support,
> yes?  It might be better to leave this until later - it's only a tiny
> piece of all the consolidations we should do between ppc32 and ppc64
> PCI code.  Currently there are a lot of silly, subtle differences
> between them :(.

Well, my point is:
	- pci_32.c ranges parsing code is confusing and silly in many
	ways

	- there is no obvious way to enable 64bit phys addressed
	handled correctly there.
	possible approaches may bring more problems than solve.

	- completely correct is going to be *hard* as David noted, but
	we can consider effective merge(with code flow similar to existing funcs) of existing
	bits a good starting point. Else, when a lot of stufpci_process_bridge_OF_rangesf would depend on 
	both functions, it would be too painful to rewrite.
	(speaking about pci_process_bridge_OF_ranges here)
	  

> 
> > > > +			size = ranges32[i].size[1];
> > > > +
> > > > +			DBG("Observed: pci %x phys %x size
> > > > %x\n",
> > > > +			    (u32) pci_addr, (u32)
> > > > cpu_phys_addr, size);
> > > > +		}
> > > 
> > > You don't have any equivalent of the code that exists in both
> > > pre-existing versions to coalesce contiguous ranges.  You probably
> > > want to use the 64-bit version, since that doesn't need a local
> > > copy of the ranges.
> > > 
> > correct. yet, the whole aim of the upper seems to use summary size
> > of contiguous block and that's it.
> > How would we recognize IORESOURCE_PREFETCH then? (I know, my code
> > is missing that prefetch regions so far, but anyway).
> > >From the code, it seems to declare each resource that is part of
> > >contiguous block, with the size of entire block.
> > 
> > That's prolly from binding, but can you please elaborate the logic
> > for better understanding?
> 
> The attributes such as PREFETCH are encoded into cell 0 of the 3-cell
> OF PCI address.  So, ranges with different attributes won't appear as
> contiguous in this space.
> 
ok, I think there's no problem to implement it in the new func then.

-- 
Sincerely, Vitaly

[PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Vitaly Bordug]

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Signed-off-by: Stefan Roese <sr@denx.de>

---

 arch/powerpc/boot/dts/sequoia.dts |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/sequoia.dts b/arch/powerpc/boot/dts/sequoia.dts
index ef6f41c..8eb258f 100644
--- a/arch/powerpc/boot/dts/sequoia.dts
+++ b/arch/powerpc/boot/dts/sequoia.dts
@@ -92,6 +92,32 @@
 		#size-cells = <1>;
 		ranges;
 		clock-frequency = <0>; /* Filled in by zImage */
+		
+		pci {
+			/* irqs are routed to irq67, dependless of devsel/PIRQx */
+			interrupt-map-mask = <0 0 0 0>;
+			interrupt-map = <0 0 0 0 &UIC2 3 8>;	
+
+			interrupt-parent = <&UIC2>;
+			interrupts = <3 8>;
+	    
+			bus-range = <0 0>;
+	
+			/* 
+			 * mem is at 80000000 set up indirectly
+			 * io is at 0001_e800_0000
+			 */
+			ranges = <02000000 0 80000000 1 80000000 0 10000000
+				01000000 0 00000000 1 e8000000 0 00100000>;
+
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+
+    			reg = <1 eec00000 40 1 ef400000 40>;  /* phb cfg, phb reg */
+			compatible = "ibm, 440epx";
+			device_type = "pci";
+		};
 
 		SDRAM0: sdram {
 			device_type = "memory-controller";

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Segher Boessenkool]

> +		pci {
> +    			reg = <1 eec00000 40 1 ef400000 40>;  /* phb cfg, phb reg */

First component of reg is the unit address, so: pci@1eec00000 .

"phb cfg" is how you access PCI configuration space?  It wouldn't
hurt to document that, either in a little binding or just here in
the code.

> +			bus-range = <0 0>;

Can't you have subordinate PCI busses?  Or are there no slots :-)

> +			/*
> +			 * mem is at 80000000 set up indirectly
> +			 * io is at 0001_e800_0000
> +			 */
> +			ranges = <02000000 0 80000000 1 80000000 0 10000000
> +				01000000 0 00000000 1 e8000000 0 00100000>;

Comment doesn't match code for the memory space.  What does "set
up indirectly" mean here?  Oh wait, you want to say that the host
addresses 1_8000_0000..1_8fff_ffff are translated to PCI addresses
8000_0000..8fff_ffff.

What about PCI DMA, is that identity mapped?

> +			#interrupt-cells = <1>;
> +			#size-cells = <2>;
> +			#address-cells = <3>;

The reverse order of these is more conventional.  Not that it
actually matters ;-)

> +			compatible = "ibm, 440epx";

Stray space.  And you need to say it is the PCI host, so something
like "ibm,440epx-pci".


Segher

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Vitaly Bordug]

On Sat, 25 Aug 2007 11:49:58 +0200
Segher Boessenkool wrote:

> > +		pci {
> > +    			reg = <1 eec00000 40 1 ef400000
> > 40>;  /* phb cfg, phb reg */
> 
> First component of reg is the unit address, so: pci@1eec00000 .
> 
> "phb cfg" is how you access PCI configuration space?  It wouldn't
> hurt to document that, either in a little binding or just here in
> the code.
> 
mmm, that was what my poor upper comment about, exactly.
do you mean that "PCI configuration space xxxx_xxxx, PCI register at xxxx_xxxx" will look more
appropriate?

> > +			bus-range = <0 0>;
> 
> Can't you have subordinate PCI busses?  Or are there no slots :-)
> 
Even if there are (and I dunno - Stefan did the HW validation and updates, since he has actual target),
the performance of such a beast would be low, with one shared irq for everybody...
> > +			/*
> > +			 * mem is at 80000000 set up indirectly
> > +			 * io is at 0001_e800_0000
> > +			 */
> > +			ranges = <02000000 0 80000000 1 80000000 0
> > 10000000
> > +				01000000 0 00000000 1 e8000000 0
> > 00100000>;
> 
> Comment doesn't match code for the memory space.  What does "set
> up indirectly" mean here?  Oh wait, you want to say that the host
> addresses 1_8000_0000..1_8fff_ffff are translated to PCI addresses
> 8000_0000..8fff_ffff.
> 
Yes, exactly.
> What about PCI DMA, is that identity mapped?
> 
Not thinking about it atm, should "just work" /*though it never really does*/ :)
> > +			#interrupt-cells = <1>;
> > +			#size-cells = <2>;
> > +			#address-cells = <3>;
> 
> The reverse order of these is more conventional.  Not that it
> actually matters ;-)

ok, I'll reorder them
> 
> > +			compatible = "ibm, 440epx";
> 
> Stray space.  And you need to say it is the PCI host, so something
> like "ibm,440epx-pci".
> 
OK, will have "amcc,.440epx-pci" here

-- 
Sincerely, Vitaly

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Segher Boessenkool]

>>> +		pci {
>>> +    			reg = <1 eec00000 40 1 ef400000
>>> 40>;  /* phb cfg, phb reg */
>>
>> First component of reg is the unit address, so: pci@1eec00000 .
>>
>> "phb cfg" is how you access PCI configuration space?  It wouldn't
>> hurt to document that, either in a little binding or just here in
>> the code.
>>
> mmm, that was what my poor upper comment about, exactly.
> do you mean that "PCI configuration space xxxx_xxxx, PCI register at 
> xxxx_xxxx" will look more
> appropriate?

I just mean you should document what the two "reg" regions for this
device are meant to represent.  "phb reg" isn't really verbose enough 
;-)

>>> +			bus-range = <0 0>;
>>
>> Can't you have subordinate PCI busses?  Or are there no slots :-)
>>
> Even if there are (and I dunno - Stefan did the HW validation and 
> updates, since he has actual target),
> the performance of such a beast would be low, with one shared irq for 
> everybody...

Sure, but this is about correctness, not performance.

>>> +			/*
>>> +			 * mem is at 80000000 set up indirectly
>>> +			 * io is at 0001_e800_0000
>>> +			 */
>>> +			ranges = <02000000 0 80000000 1 80000000 0
>>> 10000000
>>> +				01000000 0 00000000 1 e8000000 0
>>> 00100000>;
>>
>> Comment doesn't match code for the memory space.  What does "set
>> up indirectly" mean here?  Oh wait, you want to say that the host
>> addresses 1_8000_0000..1_8fff_ffff are translated to PCI addresses
>> 8000_0000..8fff_ffff.
>>
> Yes, exactly.

Great.  Could you please fix the comment to just say this, then?

>> What about PCI DMA, is that identity mapped?
>>
> Not thinking about it atm, should "just work" /*though it never really 
> does*/ :)

Okay, if it's identity mapped, you don't need any properties in the
device tree for it.  Good :-)


Segher

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[David Gibson]

On Sun, Aug 26, 2007 at 02:27:50PM +0400, Vitaly Bordug wrote:
> On Sat, 25 Aug 2007 11:49:58 +0200 Segher Boessenkool wrote:
[snip]
> > > +			compatible = "ibm, 440epx";
> > 
> > Stray space.  And you need to say it is the PCI host, so something
> > like "ibm,440epx-pci".
> > 
> OK, will have "amcc,.440epx-pci" here

Uh... now instead of a stray space you have a stray '.'

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Segher Boessenkool]

> +			compatible = "ibm, 440epx";

Oh, and shouldn't it be "amcc," anyway?


Segher

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Stefan Roese]

On Saturday 25 August 2007, Segher Boessenkool wrote:
> > +			compatible = "ibm, 440epx";
>
> Oh, and shouldn't it be "amcc," anyway?

Not sure here. It's still the PCI core used on the early 405 and 440 platforms 
build by IBM. If we use IBM on other core logic like UIC, EMAC etc, then we 
should use IBM here too.

Best regards,
Stefan

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[David Gibson]

On Sat, Aug 25, 2007 at 01:29:54PM +0400, Vitaly Bordug wrote:
> 
> Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> Signed-off-by: Stefan Roese <sr@denx.de>
> 
> ---
> 
>  arch/powerpc/boot/dts/sequoia.dts |   26 ++++++++++++++++++++++++++
>  1 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/sequoia.dts b/arch/powerpc/boot/dts/sequoia.dts
> index ef6f41c..8eb258f 100644
> --- a/arch/powerpc/boot/dts/sequoia.dts
> +++ b/arch/powerpc/boot/dts/sequoia.dts
> @@ -92,6 +92,32 @@
>  		#size-cells = <1>;
>  		ranges;
>  		clock-frequency = <0>; /* Filled in by zImage */
> +		
> +		pci {
> +			/* irqs are routed to irq67, dependless of devsel/PIRQx */
> +			interrupt-map-mask = <0 0 0 0>;
> +			interrupt-map = <0 0 0 0 &UIC2 3 8>;	
> +
> +			interrupt-parent = <&UIC2>;
> +			interrupts = <3 8>;
> +	    
> +			bus-range = <0 0>;
> +	
> +			/* 
> +			 * mem is at 80000000 set up indirectly
> +			 * io is at 0001_e800_0000
> +			 */
> +			ranges = <02000000 0 80000000 1 80000000 0 10000000
> +				01000000 0 00000000 1 e8000000 0 00100000>;
> +
> +			#interrupt-cells = <1>;
> +			#size-cells = <2>;
> +			#address-cells = <3>;
> +
> +    			reg = <1 eec00000 40 1 ef400000 40>;  /* phb cfg, phb reg */
> +			compatible = "ibm, 440epx";
> +			device_type = "pci";

I usually put device_type, compatible and reg at the top of the block,
to announce what the node actually is before giving all the details.

Also, apart from the stray space in the compatible, I'm guessing that
the 440EPx bridge is actually more-or-less like the PCI bridges on
other 4xx chips, so we should have a more general compatible string
too.

Is the 440EPx a vanilla PCI or a PCI-X bridge?  If the later that
should be reflected in the name and compatible as well.

> +		};
>  
>  		SDRAM0: sdram {
>  			device_type = "memory-controller";
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Stefan Roese]

On Monday 27 August 2007, David Gibson wrote:
> On Sat, Aug 25, 2007 at 01:29:54PM +0400, Vitaly Bordug wrote:
> > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > Signed-off-by: Stefan Roese <sr@denx.de>
> >
> > ---
> >
> >  arch/powerpc/boot/dts/sequoia.dts |   26 ++++++++++++++++++++++++++
> >  1 files changed, 26 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/sequoia.dts
> > b/arch/powerpc/boot/dts/sequoia.dts index ef6f41c..8eb258f 100644
> > --- a/arch/powerpc/boot/dts/sequoia.dts
> > +++ b/arch/powerpc/boot/dts/sequoia.dts
> > @@ -92,6 +92,32 @@
> >  		#size-cells = <1>;
> >  		ranges;
> >  		clock-frequency = <0>; /* Filled in by zImage */
> > +
> > +		pci {
> > +			/* irqs are routed to irq67, dependless of devsel/PIRQx */
> > +			interrupt-map-mask = <0 0 0 0>;
> > +			interrupt-map = <0 0 0 0 &UIC2 3 8>;
> > +
> > +			interrupt-parent = <&UIC2>;
> > +			interrupts = <3 8>;
> > +
> > +			bus-range = <0 0>;
> > +
> > +			/*
> > +			 * mem is at 80000000 set up indirectly
> > +			 * io is at 0001_e800_0000
> > +			 */
> > +			ranges = <02000000 0 80000000 1 80000000 0 10000000
> > +				01000000 0 00000000 1 e8000000 0 00100000>;
> > +
> > +			#interrupt-cells = <1>;
> > +			#size-cells = <2>;
> > +			#address-cells = <3>;
> > +
> > +    			reg = <1 eec00000 40 1 ef400000 40>;  /* phb cfg, phb reg */
> > +			compatible = "ibm, 440epx";
> > +			device_type = "pci";
>
> I usually put device_type, compatible and reg at the top of the block,
> to announce what the node actually is before giving all the details.
>
> Also, apart from the stray space in the compatible, I'm guessing that
> the 440EPx bridge is actually more-or-less like the PCI bridges on
> other 4xx chips, so we should have a more general compatible string
> too.

Yes, it is "more-or-less" like any other 4xx PCI core. So it really would make 
sense to define it more generally. Something like:

			compatible = "ibm,pci-440epx", "ibm,pci4xx";

or even:

			compatible = "ibm,pci-440epx", "ibm,pci";

?

> Is the 440EPx a vanilla PCI or a PCI-X bridge?  If the later that
> should be reflected in the name and compatible as well.

It's a vanilla PCI bridge.

Best regards,
Stefan

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[David Gibson]

On Mon, Aug 27, 2007 at 08:07:17AM +0200, Stefan Roese wrote:
[snip]
> > I usually put device_type, compatible and reg at the top of the block,
> > to announce what the node actually is before giving all the details.
> >
> > Also, apart from the stray space in the compatible, I'm guessing that
> > the 440EPx bridge is actually more-or-less like the PCI bridges on
> > other 4xx chips, so we should have a more general compatible string
> > too.
> 
> Yes, it is "more-or-less" like any other 4xx PCI core. So it really would make 
> sense to define it more generally. Something like:
> 
> 			compatible = "ibm,pci-440epx", "ibm,pci4xx";
> 
> or even:
> 
> 			compatible = "ibm,pci-440epx", "ibm,pci";
> 
> ?

Hrm.. "xx" is ugly, and "ibm,pci" isn't specific enough.  I think
we're better off just using the oldest similar chip.  Since this is
vanilla PCI, I think that makes it "ibm,pci-405gp"

> > Is the 440EPx a vanilla PCI or a PCI-X bridge?  If the later that
> > should be reflected in the name and compatible as well.
> 
> It's a vanilla PCI bridge.

Ok, so it is different from 440GP which is PCI-X.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Stefan Roese]

On Monday 27 August 2007, David Gibson wrote:
> On Mon, Aug 27, 2007 at 08:07:17AM +0200, Stefan Roese wrote:
> [snip]
>
> > > I usually put device_type, compatible and reg at the top of the block,
> > > to announce what the node actually is before giving all the details.
> > >
> > > Also, apart from the stray space in the compatible, I'm guessing that
> > > the 440EPx bridge is actually more-or-less like the PCI bridges on
> > > other 4xx chips, so we should have a more general compatible string
> > > too.
> >
> > Yes, it is "more-or-less" like any other 4xx PCI core. So it really would
> > make sense to define it more generally. Something like:
> >
> > 			compatible = "ibm,pci-440epx", "ibm,pci4xx";
> >
> > or even:
> >
> > 			compatible = "ibm,pci-440epx", "ibm,pci";
> >
> > ?
>
> Hrm.. "xx" is ugly, and "ibm,pci" isn't specific enough.  I think
> we're better off just using the oldest similar chip.  Since this is
> vanilla PCI, I think that makes it "ibm,pci-405gp"

Fine with me.

> > > Is the 440EPx a vanilla PCI or a PCI-X bridge?  If the later that
> > > should be reflected in the name and compatible as well.
> >
> > It's a vanilla PCI bridge.
>
> Ok, so it is different from 440GP which is PCI-X.

Yes. There are some common parts (IIRC), but there are differences. Perhaps 
both, IBM-PCI and IBM-PCI-X can be integrated into one source too, but I 
think we should start with PCI-only right now.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de
=====================================================================

Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Vitaly Bordug]

On Mon, 27 Aug 2007 11:54:17 +1000
David Gibson wrote:

> On Sat, Aug 25, 2007 at 01:29:54PM +0400, Vitaly Bordug wrote:
> > 
> > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > 
> > ---
> > 
> >  arch/powerpc/boot/dts/sequoia.dts |   26 ++++++++++++++++++++++++++
> >  1 files changed, 26 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/powerpc/boot/dts/sequoia.dts
> > b/arch/powerpc/boot/dts/sequoia.dts index ef6f41c..8eb258f 100644
> > --- a/arch/powerpc/boot/dts/sequoia.dts
> > +++ b/arch/powerpc/boot/dts/sequoia.dts
> > @@ -92,6 +92,32 @@
> >  		#size-cells = <1>;
> >  		ranges;
> >  		clock-frequency = <0>; /* Filled in by zImage */
> > +		
> > +		pci {
> > +			/* irqs are routed to irq67, dependless of
> > devsel/PIRQx */
> > +			interrupt-map-mask = <0 0 0 0>;
> > +			interrupt-map = <0 0 0 0 &UIC2 3
> > 8>; +
> > +			interrupt-parent = <&UIC2>;
> > +			interrupts = <3 8>;
> > +	    
> > +			bus-range = <0 0>;
> > +	
> > +			/* 
> > +			 * mem is at 80000000 set up indirectly
> > +			 * io is at 0001_e800_0000
> > +			 */
> > +			ranges = <02000000 0 80000000 1 80000000 0
> > 10000000
> > +				01000000 0 00000000 1 e8000000 0
> > 00100000>; +
> > +			#interrupt-cells = <1>;
> > +			#size-cells = <2>;
> > +			#address-cells = <3>;
> > +
> > +    			reg = <1 eec00000 40 1 ef400000
> > 40>;  /* phb cfg, phb reg */
> > +			compatible = "ibm, 440epx";
> > +			device_type = "pci";
> 
> I usually put device_type, compatible and reg at the top of the block,
> to announce what the node actually is before giving all the details.
> 
ok
> Also, apart from the stray space in the compatible, I'm guessing that
> the 440EPx bridge is actually more-or-less like the PCI bridges on
> other 4xx chips, so we should have a more general compatible string
> too.
> 
heh, with a sole PCI impl on 4xx it does not hurt to have more general
stuff here.
what is proposed, "amcc,4xx"?
> Is the 440EPx a vanilla PCI or a PCI-X bridge?  If the later that
> should be reflected in the name and compatible as well.
> 
"The 32-bit PCI bridge is compatible with he PCI Specification, Version
2.2 (it does not support 5V operation) "
so seems to be vanilla PCI.

> > +		};
> >  
> >  		SDRAM0: sdram {
> >  			device_type = "memory-controller";
> > 
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@ozlabs.org
> > https://ozlabs.org/mailman/listinfo/linuxppc-dev
> > 
> 


-- 
Sincerely, Vitaly

[PATCH 3/3] [POWERPC] Add PCI support for AMCC 440EPx (sequoia)

[Vitaly Bordug]

In fact, loosely move of arch/ppc bits, though regions are
set up using values from ranges property. This also adds
setup_indirect_pci_noremap() function to handle indirect
PCI without one more ioremap.

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Signed-off-by: Stefan Roese <sr@denx.de>

---

 arch/powerpc/platforms/44x/44x.h           |   28 ++++
 arch/powerpc/platforms/44x/Makefile        |    4 +
 arch/powerpc/platforms/44x/ppc440epx-pci.c |  192 ++++++++++++++++++++++++++++
 arch/powerpc/platforms/44x/sequoia.c       |   14 ++
 arch/powerpc/sysdev/indirect_pci.c         |   14 ++
 include/asm-powerpc/pci-bridge.h           |    2 
 6 files changed, 254 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/44x/44x.h b/arch/powerpc/platforms/44x/44x.h
index 42eabf8..d3845f9 100644
--- a/arch/powerpc/platforms/44x/44x.h
+++ b/arch/powerpc/platforms/44x/44x.h
@@ -1,8 +1,36 @@
 #ifndef __POWERPC_PLATFORMS_44X_44X_H
 #define __POWERPC_PLATFORMS_44X_44X_H
+#include <asm/pci-bridge.h>
+
+/* PCI support */
+#define PPC4xx_PCI_CFGA_OFFSET		0
+#define PPC4xx_PCI_CFGD_OFFSET		0x4
+
+#define PPC4xx_PCIL0_PMM0LA		0x000
+#define PPC4xx_PCIL0_PMM0MA		0x004
+#define PPC4xx_PCIL0_PMM0PCILA		0x008
+#define PPC4xx_PCIL0_PMM0PCIHA		0x00C
+#define PPC4xx_PCIL0_PMM1LA		0x010
+#define PPC4xx_PCIL0_PMM1MA		0x014
+#define PPC4xx_PCIL0_PMM1PCILA		0x018
+#define PPC4xx_PCIL0_PMM1PCIHA		0x01C
+#define PPC4xx_PCIL0_PMM2LA		0x020
+#define PPC4xx_PCIL0_PMM2MA		0x024
+#define PPC4xx_PCIL0_PMM2PCILA		0x028
+#define PPC4xx_PCIL0_PMM2PCIHA		0x02C
+#define PPC4xx_PCIL0_PTM1MS		0x030
+#define PPC4xx_PCIL0_PTM1LA		0x034
+#define PPC4xx_PCIL0_PTM2MS		0x038
+#define PPC4xx_PCIL0_PTM2LA		0x03C
 
 extern u8 as1_readb(volatile u8 __iomem  *addr);
 extern void as1_writeb(u8 data, volatile u8 __iomem *addr);
 extern void ppc44x_reset_system(char *cmd);
 
+#ifdef CONFIG_PCI
+int ppc440epx_exclude_device(struct pci_controller *hose,
+		u_char bus, u_char devfn);
+int ppc440epx_add_bridge(struct device_node *dev);
+#endif
+
 #endif /* __POWERPC_PLATFORMS_44X_44X_H */
diff --git a/arch/powerpc/platforms/44x/Makefile b/arch/powerpc/platforms/44x/Makefile
index 10ce674..d2a5278 100644
--- a/arch/powerpc/platforms/44x/Makefile
+++ b/arch/powerpc/platforms/44x/Makefile
@@ -2,3 +2,7 @@ obj-$(CONFIG_44x)	:= misc_44x.o
 obj-$(CONFIG_EBONY)	+= ebony.o
 obj-$(CONFIG_BAMBOO) += bamboo.o
 obj-$(CONFIG_SEQUOIA)	+= sequoia.o
+
+ifeq ($(CONFIG_PCI),y)
+obj-$(CONFIG_440EPX)   += ppc440epx-pci.o
+endif
diff --git a/arch/powerpc/platforms/44x/ppc440epx-pci.c b/arch/powerpc/platforms/44x/ppc440epx-pci.c
new file mode 100644
index 0000000..bd4a352
--- /dev/null
+++ b/arch/powerpc/platforms/44x/ppc440epx-pci.c
@@ -0,0 +1,192 @@
+/*
+ * PPC44x PCI host support
+ *
+ * Vitaly Bordug <vitb@kernel.crashing.org>
+ * Stefan Roese <sr@denx.de>
+ *
+ * Based on arch/ppc sequoia pci bits, that are
+ * Copyright 2006-2007 DENX Software Engineering, Stefan Roese <sr@denx.de>
+ *
+ * Based on bamboo.c from Wade Farnsworth <wfarnsworth@mvista.com>
+ *      Copyright 2004 MontaVista Software Inc.
+ *      Copyright 2006 AMCC
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/stddef.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/pci.h>
+#include <linux/delay.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/io.h>
+
+#include <mm/mmu_decl.h>
+
+#include <asm/system.h>
+#include <asm/atomic.h>
+#include <asm/pci-bridge.h>
+#include <asm/prom.h>
+
+#include "44x.h"
+
+#undef DEBUG
+
+#ifdef DEBUG
+#define DBG(x...) printk(x)
+#else
+#define DBG(x...)
+#endif
+
+#ifdef CONFIG_PCI
+
+int ppc440epx_exclude_device(struct pci_controller *hose, u_char bus,
+			   u_char devfn)
+{
+	if ((bus == hose->first_busno) && PCI_SLOT(devfn) == 0)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	return PCIBIOS_SUCCESSFUL;
+}
+
+inline void pci_writel(void *pci_reg_base, u32 offset, unsigned int val)
+{
+	writel(val, pci_reg_base + offset);
+}
+
+inline void pci_cfg_out(void *cfg, u32 offset, unsigned int value)
+{
+	out_le32((void *)((u32) cfg + offset), value);
+}
+
+inline u32 pci_cfg_in(void *cfg, u32 offset)
+{
+	return in_le32((void *)((u32) cfg + offset));
+}
+
+static void __init ppc440epx_setup_pci(struct pci_controller *hose,
+				     void *pci_reg_base, void *pci_cfg_base)
+{
+	unsigned long memory_size, pci_size = 0;
+	phys_addr_t pci_phys_base = 0;
+	phys_addr_t pci_pci_base = 0;
+	int i;
+
+	memory_size = total_memory;
+
+	/*
+	 * 440EPx has single memory region, we'll use it to configure phb
+	 */
+	for (i = 0; i < 3; i++)
+		if (hose->mem_resources[i].start) {
+			pci_phys_base = hose->mem_resources[i].start;
+			pci_pci_base = pci_phys_base - hose->pci_mem_offset;
+			pci_size =
+			    hose->mem_resources[i].end -
+			    hose->mem_resources[i].start;
+		}
+
+	if (pci_phys_base == 0) {
+		printk(KERN_ERR "PHB bridge memory region is not defined!\n");
+		return;
+	}
+
+	pci_cfg_out(pci_cfg_base, PPC4xx_PCI_CFGA_OFFSET,
+		    (pci_pci_base & 0xFFFFFFFF) | (PCI_COMMAND & 0xfc));
+	pci_cfg_out(pci_cfg_base, PPC4xx_PCI_CFGD_OFFSET,
+		    (pci_cfg_in(pci_cfg_base, PPC4xx_PCI_CFGD_OFFSET) |
+		     PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER));
+
+	/* Disable region first */
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0MA, 0);
+
+	/* PLB starting addr: 0x0000000180000000
+	 * We need just lower word to get the things work
+	 */
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0LA,
+		   pci_phys_base & 0xFFFFFFFF);
+
+	/* PCI start addr, 0x80000000 (PCI Address) */
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0PCILA,
+		   pci_pci_base & 0xFFFFFFFF);
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0PCIHA, 0);
+
+	/* Enable no pre-fetch, enable region */
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0MA,
+		   ((0xffffffff - pci_size) | 0x01));
+
+	/* Disable region one */
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1MA, 0);
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1LA, 0);
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1PCILA, 0);
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1PCIHA, 0);
+
+	/* Disable region two */
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1MA, 0);
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1LA, 0);
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1PCILA, 0);
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1PCIHA, 0);
+
+	/* Now configure the PCI->PLB windows, we only use PTM1
+	 *
+	 * For Inbound flow, set the window size to all available memory
+	 * This is required because if size is smaller,
+	 * then Eth/PCI DD would fail as PCI card not able to access
+	 * the memory allocated by DD.
+	 */
+
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PTM1MS, 0);
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PTM1LA, 0);
+
+	memory_size = 1 << fls(memory_size - 1);
+
+	/* Size low + Enabled */
+	pci_writel(pci_reg_base, PPC4xx_PCIL0_PTM1MS,
+		   (0xffffffff - (memory_size - 1)) | 0x1);
+	eieio();
+}
+
+int __init ppc440epx_add_bridge(struct device_node *dev)
+{
+	int len;
+	struct pci_controller *hose;
+	const int *bus_range;
+	int primary = 1;
+	void *reg, *cfg;
+
+	/* Fetch host bridge registers address */
+	reg = of_iomap(dev, 1);
+	cfg = of_iomap(dev, 0);
+
+	DBG("Adding PCI host bridge %s\n reg %p cfg %p\n", dev->full_name, reg,
+	    cfg);
+
+	/* Get bus range if any */
+	bus_range = of_get_property(dev, "bus-range", &len);
+	if (bus_range == NULL || len < 2 * sizeof(int))
+		printk(KERN_WARNING "Can't get bus-range for %s, assume"
+		       " bus 0\n", dev->full_name);
+
+	pci_assign_all_buses = 1;
+	hose = pcibios_alloc_controller(dev);
+	if (!hose)
+		return -ENOMEM;
+
+	hose->first_busno = bus_range ? bus_range[0] : 0;
+	hose->last_busno = bus_range ? bus_range[1] : 0xff;
+
+	setup_indirect_pci_noremap(hose, (u32) cfg,
+				   (u32) (cfg + PPC4xx_PCI_CFGD_OFFSET), 0);
+
+	/* Interpret the "ranges" property */
+	/* This also maps the I/O region and sets isa_io/mem_base */
+	pci_process_bridge_OF_ranges(hose, dev, primary);
+	ppc440epx_setup_pci(hose, reg, cfg);
+
+	return 0;
+}
+#endif
diff --git a/arch/powerpc/platforms/44x/sequoia.c b/arch/powerpc/platforms/44x/sequoia.c
index ca3c9cc..c3a638b 100644
--- a/arch/powerpc/platforms/44x/sequoia.c
+++ b/arch/powerpc/platforms/44x/sequoia.c
@@ -52,6 +52,20 @@ static int __init sequoia_probe(void)
 
 static void __init sequoia_setup_arch(void)
 {
+#ifdef CONFIG_PCI
+	struct device_node *np;
+#endif
+
+	if (ppc_md.progress)
+		ppc_md.progress("ppc44x_setup_arch()", 0);
+
+#ifdef CONFIG_PCI
+	for (np = NULL; (np = of_find_compatible_node(np,
+					"pci", "ibm, 440epx")) != NULL;)
+		ppc440epx_add_bridge(np);
+	ppc_md.pci_exclude_device = ppc440epx_exclude_device;
+#endif
+
 }
 
 define_machine(sequoia) {
diff --git a/arch/powerpc/sysdev/indirect_pci.c b/arch/powerpc/sysdev/indirect_pci.c
index b5d0682..9dab751 100644
--- a/arch/powerpc/sysdev/indirect_pci.c
+++ b/arch/powerpc/sysdev/indirect_pci.c
@@ -162,3 +162,17 @@ setup_indirect_pci(struct pci_controller* hose, u32 cfg_addr, u32 cfg_data, u32
 	hose->ops = &indirect_pci_ops;
 	hose->indirect_type = flags;
 }
+
+/*
+ * For some reason, ioremap does not handle 2-nd way mapping well,
+ * causing system check while trying to access config space later
+ */
+void __init
+setup_indirect_pci_noremap(struct pci_controller *hose, u32 cfg_addr,
+		u32 cfg_data, u32 flags)
+{
+	hose->cfg_addr = (void *)cfg_addr;
+	hose->cfg_data = (void *)cfg_data;
+	hose->ops = &indirect_pci_ops;
+	hose->indirect_type = flags;
+}
diff --git a/include/asm-powerpc/pci-bridge.h b/include/asm-powerpc/pci-bridge.h
index e909769..4bde6e5 100644
--- a/include/asm-powerpc/pci-bridge.h
+++ b/include/asm-powerpc/pci-bridge.h
@@ -99,6 +99,8 @@ extern int early_find_capability(struct pci_controller *hose, int bus,
 
 extern void setup_indirect_pci(struct pci_controller* hose,
 			       u32 cfg_addr, u32 cfg_data, u32 flags);
+extern void setup_indirect_pci_noremap(struct pci_controller *hose,
+			       u32 cfg_addr, u32 cfg_data, u32 flags);
 extern void setup_grackle(struct pci_controller *hose);
 extern void __init update_bridge_resource(struct pci_dev *dev,
 					  struct resource *res);

Re: [PATCH 3/3] [POWERPC] Add PCI support for AMCC 440EPx (sequoia)

[David Gibson]

On Sat, Aug 25, 2007 at 01:30:01PM +0400, Vitaly Bordug wrote:
> 
> In fact, loosely move of arch/ppc bits, though regions are
> set up using values from ranges property. This also adds
> setup_indirect_pci_noremap() function to handle indirect
> PCI without one more ioremap.
> 
> Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> Signed-off-by: Stefan Roese <sr@denx.de>
> 
> ---
> 
>  arch/powerpc/platforms/44x/44x.h           |   28 ++++
>  arch/powerpc/platforms/44x/Makefile        |    4 +
>  arch/powerpc/platforms/44x/ppc440epx-pci.c |  192 ++++++++++++++++++++++++++++
>  arch/powerpc/platforms/44x/sequoia.c       |   14 ++
>  arch/powerpc/sysdev/indirect_pci.c         |   14 ++
>  include/asm-powerpc/pci-bridge.h           |    2 
>  6 files changed, 254 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/44x/44x.h b/arch/powerpc/platforms/44x/44x.h
> index 42eabf8..d3845f9 100644
> --- a/arch/powerpc/platforms/44x/44x.h
> +++ b/arch/powerpc/platforms/44x/44x.h
> @@ -1,8 +1,36 @@
>  #ifndef __POWERPC_PLATFORMS_44X_44X_H
>  #define __POWERPC_PLATFORMS_44X_44X_H
> +#include <asm/pci-bridge.h>
> +
> +/* PCI support */
> +#define PPC4xx_PCI_CFGA_OFFSET		0
> +#define PPC4xx_PCI_CFGD_OFFSET		0x4
> +
> +#define PPC4xx_PCIL0_PMM0LA		0x000
> +#define PPC4xx_PCIL0_PMM0MA		0x004
> +#define PPC4xx_PCIL0_PMM0PCILA		0x008
> +#define PPC4xx_PCIL0_PMM0PCIHA		0x00C
> +#define PPC4xx_PCIL0_PMM1LA		0x010
> +#define PPC4xx_PCIL0_PMM1MA		0x014
> +#define PPC4xx_PCIL0_PMM1PCILA		0x018
> +#define PPC4xx_PCIL0_PMM1PCIHA		0x01C
> +#define PPC4xx_PCIL0_PMM2LA		0x020
> +#define PPC4xx_PCIL0_PMM2MA		0x024
> +#define PPC4xx_PCIL0_PMM2PCILA		0x028
> +#define PPC4xx_PCIL0_PMM2PCIHA		0x02C
> +#define PPC4xx_PCIL0_PTM1MS		0x030
> +#define PPC4xx_PCIL0_PTM1LA		0x034
> +#define PPC4xx_PCIL0_PTM2MS		0x038
> +#define PPC4xx_PCIL0_PTM2LA		0x03C
>  
>  extern u8 as1_readb(volatile u8 __iomem  *addr);
>  extern void as1_writeb(u8 data, volatile u8 __iomem *addr);
>  extern void ppc44x_reset_system(char *cmd);
>  
> +#ifdef CONFIG_PCI
> +int ppc440epx_exclude_device(struct pci_controller *hose,
> +		u_char bus, u_char devfn);
> +int ppc440epx_add_bridge(struct device_node *dev);
> +#endif
> +
>  #endif /* __POWERPC_PLATFORMS_44X_44X_H */
> diff --git a/arch/powerpc/platforms/44x/Makefile b/arch/powerpc/platforms/44x/Makefile
> index 10ce674..d2a5278 100644
> --- a/arch/powerpc/platforms/44x/Makefile
> +++ b/arch/powerpc/platforms/44x/Makefile
> @@ -2,3 +2,7 @@ obj-$(CONFIG_44x)	:= misc_44x.o
>  obj-$(CONFIG_EBONY)	+= ebony.o
>  obj-$(CONFIG_BAMBOO) += bamboo.o
>  obj-$(CONFIG_SEQUOIA)	+= sequoia.o
> +
> +ifeq ($(CONFIG_PCI),y)
> +obj-$(CONFIG_440EPX)   += ppc440epx-pci.o
> +endif
> diff --git a/arch/powerpc/platforms/44x/ppc440epx-pci.c b/arch/powerpc/platforms/44x/ppc440epx-pci.c
> new file mode 100644
> index 0000000..bd4a352
> --- /dev/null
> +++ b/arch/powerpc/platforms/44x/ppc440epx-pci.c
> @@ -0,0 +1,192 @@
> +/*
> + * PPC44x PCI host support
> + *
> + * Vitaly Bordug <vitb@kernel.crashing.org>
> + * Stefan Roese <sr@denx.de>
> + *
> + * Based on arch/ppc sequoia pci bits, that are
> + * Copyright 2006-2007 DENX Software Engineering, Stefan Roese <sr@denx.de>
> + *
> + * Based on bamboo.c from Wade Farnsworth <wfarnsworth@mvista.com>
> + *      Copyright 2004 MontaVista Software Inc.
> + *      Copyright 2006 AMCC
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */

Unless there really is something peculiar about the EPx bridge
compared to say the GP, EP and other 4xx bridges, this should have a
more general name.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 3/3] [POWERPC] Add PCI support for AMCC 440EPx (sequoia)

[Stefan Roese]

On Monday 27 August 2007, David Gibson wrote:
> On Sat, Aug 25, 2007 at 01:30:01PM +0400, Vitaly Bordug wrote:
> > In fact, loosely move of arch/ppc bits, though regions are
> > set up using values from ranges property. This also adds
> > setup_indirect_pci_noremap() function to handle indirect
> > PCI without one more ioremap.
> >
> > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > Signed-off-by: Stefan Roese <sr@denx.de>
> >
> > ---
> >
> >  arch/powerpc/platforms/44x/44x.h           |   28 ++++
> >  arch/powerpc/platforms/44x/Makefile        |    4 +
> >  arch/powerpc/platforms/44x/ppc440epx-pci.c |  192
> > ++++++++++++++++++++++++++++ arch/powerpc/platforms/44x/sequoia.c       |
> >   14 ++
> >  arch/powerpc/sysdev/indirect_pci.c         |   14 ++
> >  include/asm-powerpc/pci-bridge.h           |    2
> >  6 files changed, 254 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/44x/44x.h
> > b/arch/powerpc/platforms/44x/44x.h index 42eabf8..d3845f9 100644
> > --- a/arch/powerpc/platforms/44x/44x.h
> > +++ b/arch/powerpc/platforms/44x/44x.h
> > @@ -1,8 +1,36 @@
> >  #ifndef __POWERPC_PLATFORMS_44X_44X_H
> >  #define __POWERPC_PLATFORMS_44X_44X_H
> > +#include <asm/pci-bridge.h>
> > +
> > +/* PCI support */
> > +#define PPC4xx_PCI_CFGA_OFFSET		0
> > +#define PPC4xx_PCI_CFGD_OFFSET		0x4
> > +
> > +#define PPC4xx_PCIL0_PMM0LA		0x000
> > +#define PPC4xx_PCIL0_PMM0MA		0x004
> > +#define PPC4xx_PCIL0_PMM0PCILA		0x008
> > +#define PPC4xx_PCIL0_PMM0PCIHA		0x00C
> > +#define PPC4xx_PCIL0_PMM1LA		0x010
> > +#define PPC4xx_PCIL0_PMM1MA		0x014
> > +#define PPC4xx_PCIL0_PMM1PCILA		0x018
> > +#define PPC4xx_PCIL0_PMM1PCIHA		0x01C
> > +#define PPC4xx_PCIL0_PMM2LA		0x020
> > +#define PPC4xx_PCIL0_PMM2MA		0x024
> > +#define PPC4xx_PCIL0_PMM2PCILA		0x028
> > +#define PPC4xx_PCIL0_PMM2PCIHA		0x02C
> > +#define PPC4xx_PCIL0_PTM1MS		0x030
> > +#define PPC4xx_PCIL0_PTM1LA		0x034
> > +#define PPC4xx_PCIL0_PTM2MS		0x038
> > +#define PPC4xx_PCIL0_PTM2LA		0x03C
> >
> >  extern u8 as1_readb(volatile u8 __iomem  *addr);
> >  extern void as1_writeb(u8 data, volatile u8 __iomem *addr);
> >  extern void ppc44x_reset_system(char *cmd);
> >
> > +#ifdef CONFIG_PCI
> > +int ppc440epx_exclude_device(struct pci_controller *hose,
> > +		u_char bus, u_char devfn);
> > +int ppc440epx_add_bridge(struct device_node *dev);
> > +#endif
> > +
> >  #endif /* __POWERPC_PLATFORMS_44X_44X_H */
> > diff --git a/arch/powerpc/platforms/44x/Makefile
> > b/arch/powerpc/platforms/44x/Makefile index 10ce674..d2a5278 100644
> > --- a/arch/powerpc/platforms/44x/Makefile
> > +++ b/arch/powerpc/platforms/44x/Makefile
> > @@ -2,3 +2,7 @@ obj-$(CONFIG_44x)	:= misc_44x.o
> >  obj-$(CONFIG_EBONY)	+= ebony.o
> >  obj-$(CONFIG_BAMBOO) += bamboo.o
> >  obj-$(CONFIG_SEQUOIA)	+= sequoia.o
> > +
> > +ifeq ($(CONFIG_PCI),y)
> > +obj-$(CONFIG_440EPX)   += ppc440epx-pci.o
> > +endif
> > diff --git a/arch/powerpc/platforms/44x/ppc440epx-pci.c
> > b/arch/powerpc/platforms/44x/ppc440epx-pci.c new file mode 100644
> > index 0000000..bd4a352
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/44x/ppc440epx-pci.c
> > @@ -0,0 +1,192 @@
> > +/*
> > + * PPC44x PCI host support
> > + *
> > + * Vitaly Bordug <vitb@kernel.crashing.org>
> > + * Stefan Roese <sr@denx.de>
> > + *
> > + * Based on arch/ppc sequoia pci bits, that are
> > + * Copyright 2006-2007 DENX Software Engineering, Stefan Roese
> > <sr@denx.de> + *
> > + * Based on bamboo.c from Wade Farnsworth <wfarnsworth@mvista.com>
> > + *      Copyright 2004 MontaVista Software Inc.
> > + *      Copyright 2006 AMCC
> > + * This program is free software; you can redistribute  it and/or modify
> > it + * under  the terms of  the GNU General  Public License as published
> > by the + * Free Software Foundation;  either version 2 of the  License,
> > or (at your + * option) any later version.
> > + */
>
> Unless there really is something peculiar about the EPx bridge
> compared to say the GP, EP and other 4xx bridges, this should have a
> more general name.

We originally started naming this file sequoia-pci.c and changed it to be 
440EPx specific (just by renaming). But you are right of course. We should 
make it even more generic for 4xx PCI support. Perhaps we will overlook some 
problems with other 4xx platforms, but those should be solved when other 
platforms (Josh: 440ep and 405gp? ;)) will be added.

So what should it be called? arch/powerpc/syslib/ppc4xx_pci.c ?

Best regards,
Stefan

Re: [PATCH 3/3] [POWERPC] Add PCI support for AMCC 440EPx (sequoia)

[Josh Boyer]

On Mon, 27 Aug 2007 08:21:35 +0200
Stefan Roese <sr@denx.de> wrote:

> > > +++ b/arch/powerpc/platforms/44x/ppc440epx-pci.c
> > > @@ -0,0 +1,192 @@
> > > +/*
> > > + * PPC44x PCI host support
> > > + *
> > > + * Vitaly Bordug <vitb@kernel.crashing.org>
> > > + * Stefan Roese <sr@denx.de>
> > > + *
> > > + * Based on arch/ppc sequoia pci bits, that are
> > > + * Copyright 2006-2007 DENX Software Engineering, Stefan Roese
> > > <sr@denx.de> + *
> > > + * Based on bamboo.c from Wade Farnsworth <wfarnsworth@mvista.com>
> > > + *      Copyright 2004 MontaVista Software Inc.
> > > + *      Copyright 2006 AMCC
> > > + * This program is free software; you can redistribute  it and/or modify
> > > it + * under  the terms of  the GNU General  Public License as published
> > > by the + * Free Software Foundation;  either version 2 of the  License,
> > > or (at your + * option) any later version.
> > > + */
> >
> > Unless there really is something peculiar about the EPx bridge
> > compared to say the GP, EP and other 4xx bridges, this should have a
> > more general name.
> 
> We originally started naming this file sequoia-pci.c and changed it to be 
> 440EPx specific (just by renaming). But you are right of course. We should 
> make it even more generic for 4xx PCI support. Perhaps we will overlook some 
> problems with other 4xx platforms, but those should be solved when other 
> platforms (Josh: 440ep and 405gp? ;)) will be added.
> 
> So what should it be called? arch/powerpc/syslib/ppc4xx_pci.c ?

That would work, yes.  I'd be happy to test out 440ep, 440gp, and 405gp
once the patches are reworked a bit per David and Segher's comments.

josh

Re: [PATCH 3/3] [POWERPC] Add PCI support for AMCC 440EPx (sequoia)

[David Gibson]

On Mon, Aug 27, 2007 at 08:21:35AM +0200, Stefan Roese wrote:
> On Monday 27 August 2007, David Gibson wrote:
> > On Sat, Aug 25, 2007 at 01:30:01PM +0400, Vitaly Bordug wrote:
[snip]
> > Unless there really is something peculiar about the EPx bridge
> > compared to say the GP, EP and other 4xx bridges, this should have a
> > more general name.
> 
> We originally started naming this file sequoia-pci.c and changed it to be 
> 440EPx specific (just by renaming). But you are right of course. We should 
> make it even more generic for 4xx PCI support. Perhaps we will overlook some 
> problems with other 4xx platforms, but those should be solved when other 
> platforms (Josh: 440ep and 405gp? ;)) will be added.
> 
> So what should it be called? arch/powerpc/syslib/ppc4xx_pci.c ?

Sounds ok, except that it's sysdev, not syslib in arch/powerpc of
course.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 3/3] [POWERPC] Add PCI support for AMCC 440EPx (sequoia)

[Vitaly Bordug]

On Mon, 27 Aug 2007 11:57:19 +1000
David Gibson wrote:

> > + * Free Software Foundation;  either version 2 of the  License, or
> > (at your
> > + * option) any later version.
> > + */  
> 
> Unless there really is something peculiar about the EPx bridge
> compared to say the GP, EP and other 4xx bridges, this should have a
> more general name.
well, I'd rather extend the name once/when it would be validated to work on those GR, EP etc.
But, if preferred, it does not hurt to change all the prefixes to ppc4xx_ with such a sole implementation,
and fork() if required later.

opinions?
-- 
Sincerely, Vitaly

Re: [PATCH 3/3] [POWERPC] Add PCI support for AMCC 440EPx (sequoia)

[Stefan Roese]

On Monday 27 August 2007, Vitaly Bordug wrote:
> On Mon, 27 Aug 2007 11:57:19 +1000
>
> David Gibson wrote:
> > > + * Free Software Foundation;  either version 2 of the  License, or
> > > (at your
> > > + * option) any later version.
> > > + */
> >
> > Unless there really is something peculiar about the EPx bridge
> > compared to say the GP, EP and other 4xx bridges, this should have a
> > more general name.
>
> well, I'd rather extend the name once/when it would be validated to work on
> those GR, EP etc. But, if preferred, it does not hurt to change all the
> prefixes to ppc4xx_ with such a sole implementation, and fork() if required
> later.
>
> opinions?

I would prefer to move to a more generic name now (as already suggest). 
Perhaps somebody (Josh?) has a chance to test this on Bamboo (440EP) soon.

Best regards,
Stefan

Re: [PATCH 3/3] [POWERPC] Add PCI support for AMCC 440EPx (sequoia)

[Valentine Barshak]

Vitaly Bordug wrote:
> In fact, loosely move of arch/ppc bits, though regions are
> set up using values from ranges property. This also adds
> setup_indirect_pci_noremap() function to handle indirect
> PCI without one more ioremap.
> 
> Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> Signed-off-by: Stefan Roese <sr@denx.de>
> 
> ---
> 
>  arch/powerpc/platforms/44x/44x.h           |   28 ++++
>  arch/powerpc/platforms/44x/Makefile        |    4 +
>  arch/powerpc/platforms/44x/ppc440epx-pci.c |  192 ++++++++++++++++++++++++++++
>  arch/powerpc/platforms/44x/sequoia.c       |   14 ++
>  arch/powerpc/sysdev/indirect_pci.c         |   14 ++
>  include/asm-powerpc/pci-bridge.h           |    2 
>  6 files changed, 254 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/44x/44x.h b/arch/powerpc/platforms/44x/44x.h
> index 42eabf8..d3845f9 100644
> --- a/arch/powerpc/platforms/44x/44x.h
> +++ b/arch/powerpc/platforms/44x/44x.h
> @@ -1,8 +1,36 @@
>  #ifndef __POWERPC_PLATFORMS_44X_44X_H
>  #define __POWERPC_PLATFORMS_44X_44X_H
> +#include <asm/pci-bridge.h>
> +
> +/* PCI support */
> +#define PPC4xx_PCI_CFGA_OFFSET		0
> +#define PPC4xx_PCI_CFGD_OFFSET		0x4
> +
> +#define PPC4xx_PCIL0_PMM0LA		0x000
> +#define PPC4xx_PCIL0_PMM0MA		0x004
> +#define PPC4xx_PCIL0_PMM0PCILA		0x008
> +#define PPC4xx_PCIL0_PMM0PCIHA		0x00C
> +#define PPC4xx_PCIL0_PMM1LA		0x010
> +#define PPC4xx_PCIL0_PMM1MA		0x014
> +#define PPC4xx_PCIL0_PMM1PCILA		0x018
> +#define PPC4xx_PCIL0_PMM1PCIHA		0x01C
> +#define PPC4xx_PCIL0_PMM2LA		0x020
> +#define PPC4xx_PCIL0_PMM2MA		0x024
> +#define PPC4xx_PCIL0_PMM2PCILA		0x028
> +#define PPC4xx_PCIL0_PMM2PCIHA		0x02C
> +#define PPC4xx_PCIL0_PTM1MS		0x030
> +#define PPC4xx_PCIL0_PTM1LA		0x034
> +#define PPC4xx_PCIL0_PTM2MS		0x038
> +#define PPC4xx_PCIL0_PTM2LA		0x03C
>  
>  extern u8 as1_readb(volatile u8 __iomem  *addr);
>  extern void as1_writeb(u8 data, volatile u8 __iomem *addr);
>  extern void ppc44x_reset_system(char *cmd);
>  
> +#ifdef CONFIG_PCI
> +int ppc440epx_exclude_device(struct pci_controller *hose,
> +		u_char bus, u_char devfn);
> +int ppc440epx_add_bridge(struct device_node *dev);
> +#endif
> +
>  #endif /* __POWERPC_PLATFORMS_44X_44X_H */
> diff --git a/arch/powerpc/platforms/44x/Makefile b/arch/powerpc/platforms/44x/Makefile
> index 10ce674..d2a5278 100644
> --- a/arch/powerpc/platforms/44x/Makefile
> +++ b/arch/powerpc/platforms/44x/Makefile
> @@ -2,3 +2,7 @@ obj-$(CONFIG_44x)	:= misc_44x.o
>  obj-$(CONFIG_EBONY)	+= ebony.o
>  obj-$(CONFIG_BAMBOO) += bamboo.o
>  obj-$(CONFIG_SEQUOIA)	+= sequoia.o
> +
> +ifeq ($(CONFIG_PCI),y)
> +obj-$(CONFIG_440EPX)   += ppc440epx-pci.o
> +endif
> diff --git a/arch/powerpc/platforms/44x/ppc440epx-pci.c b/arch/powerpc/platforms/44x/ppc440epx-pci.c
> new file mode 100644
> index 0000000..bd4a352
> --- /dev/null
> +++ b/arch/powerpc/platforms/44x/ppc440epx-pci.c
> @@ -0,0 +1,192 @@
> +/*
> + * PPC44x PCI host support
> + *
> + * Vitaly Bordug <vitb@kernel.crashing.org>
> + * Stefan Roese <sr@denx.de>
> + *
> + * Based on arch/ppc sequoia pci bits, that are
> + * Copyright 2006-2007 DENX Software Engineering, Stefan Roese <sr@denx.de>
> + *
> + * Based on bamboo.c from Wade Farnsworth <wfarnsworth@mvista.com>
> + *      Copyright 2004 MontaVista Software Inc.
> + *      Copyright 2006 AMCC
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/stddef.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/pci.h>
> +#include <linux/delay.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +
> +#include <mm/mmu_decl.h>
> +
> +#include <asm/system.h>
> +#include <asm/atomic.h>
> +#include <asm/pci-bridge.h>
> +#include <asm/prom.h>
> +
> +#include "44x.h"
> +
> +#undef DEBUG
> +
> +#ifdef DEBUG
> +#define DBG(x...) printk(x)
> +#else
> +#define DBG(x...)
> +#endif
> +
> +#ifdef CONFIG_PCI
> +
> +int ppc440epx_exclude_device(struct pci_controller *hose, u_char bus,
> +			   u_char devfn)
> +{
> +	if ((bus == hose->first_busno) && PCI_SLOT(devfn) == 0)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +	return PCIBIOS_SUCCESSFUL;
> +}
> +
> +inline void pci_writel(void *pci_reg_base, u32 offset, unsigned int val)
> +{
> +	writel(val, pci_reg_base + offset);
> +}
> +
> +inline void pci_cfg_out(void *cfg, u32 offset, unsigned int value)
> +{
> +	out_le32((void *)((u32) cfg + offset), value);
> +}
> +
> +inline u32 pci_cfg_in(void *cfg, u32 offset)
> +{
> +	return in_le32((void *)((u32) cfg + offset));
> +}
> +
> +static void __init ppc440epx_setup_pci(struct pci_controller *hose,
> +				     void *pci_reg_base, void *pci_cfg_base)
> +{
> +	unsigned long memory_size, pci_size = 0;
> +	phys_addr_t pci_phys_base = 0;
> +	phys_addr_t pci_pci_base = 0;
> +	int i;
> +
> +	memory_size = total_memory;
> +
> +	/*
> +	 * 440EPx has single memory region, we'll use it to configure phb
> +	 */
> +	for (i = 0; i < 3; i++)
> +		if (hose->mem_resources[i].start) {
> +			pci_phys_base = hose->mem_resources[i].start;
> +			pci_pci_base = pci_phys_base - hose->pci_mem_offset;
> +			pci_size =
> +			    hose->mem_resources[i].end -
> +			    hose->mem_resources[i].start;
> +		}
> +
> +	if (pci_phys_base == 0) {
> +		printk(KERN_ERR "PHB bridge memory region is not defined!\n");
> +		return;
> +	}
> +
> +	pci_cfg_out(pci_cfg_base, PPC4xx_PCI_CFGA_OFFSET,
> +		    (pci_pci_base & 0xFFFFFFFF) | (PCI_COMMAND & 0xfc));
> +	pci_cfg_out(pci_cfg_base, PPC4xx_PCI_CFGD_OFFSET,
> +		    (pci_cfg_in(pci_cfg_base, PPC4xx_PCI_CFGD_OFFSET) |
> +		     PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER));
> +
> +	/* Disable region first */
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0MA, 0);
> +
> +	/* PLB starting addr: 0x0000000180000000
> +	 * We need just lower word to get the things work
> +	 */
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0LA,
> +		   pci_phys_base & 0xFFFFFFFF);
> +
> +	/* PCI start addr, 0x80000000 (PCI Address) */
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0PCILA,
> +		   pci_pci_base & 0xFFFFFFFF);
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0PCIHA, 0);
> +
> +	/* Enable no pre-fetch, enable region */
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM0MA,
> +		   ((0xffffffff - pci_size) | 0x01));
> +
> +	/* Disable region one */
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1MA, 0);
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1LA, 0);
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1PCILA, 0);
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1PCIHA, 0);
> +
> +	/* Disable region two */
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1MA, 0);
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1LA, 0);
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1PCILA, 0);
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PMM1PCIHA, 0);
> +
> +	/* Now configure the PCI->PLB windows, we only use PTM1
> +	 *
> +	 * For Inbound flow, set the window size to all available memory
> +	 * This is required because if size is smaller,
> +	 * then Eth/PCI DD would fail as PCI card not able to access
> +	 * the memory allocated by DD.
> +	 */
> +
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PTM1MS, 0);
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PTM1LA, 0);
> +
> +	memory_size = 1 << fls(memory_size - 1);
> +
> +	/* Size low + Enabled */
> +	pci_writel(pci_reg_base, PPC4xx_PCIL0_PTM1MS,
> +		   (0xffffffff - (memory_size - 1)) | 0x1);
> +	eieio();
> +}
> +
> +int __init ppc440epx_add_bridge(struct device_node *dev)
> +{
> +	int len;
> +	struct pci_controller *hose;
> +	const int *bus_range;
> +	int primary = 1;
> +	void *reg, *cfg;
> +
> +	/* Fetch host bridge registers address */
> +	reg = of_iomap(dev, 1);
> +	cfg = of_iomap(dev, 0);
> +
> +	DBG("Adding PCI host bridge %s\n reg %p cfg %p\n", dev->full_name, reg,
> +	    cfg);
> +
> +	/* Get bus range if any */
> +	bus_range = of_get_property(dev, "bus-range", &len);
> +	if (bus_range == NULL || len < 2 * sizeof(int))
> +		printk(KERN_WARNING "Can't get bus-range for %s, assume"
> +		       " bus 0\n", dev->full_name);
> +
> +	pci_assign_all_buses = 1;
> +	hose = pcibios_alloc_controller(dev);
> +	if (!hose)
> +		return -ENOMEM;
> +
> +	hose->first_busno = bus_range ? bus_range[0] : 0;
> +	hose->last_busno = bus_range ? bus_range[1] : 0xff;
> +
> +	setup_indirect_pci_noremap(hose, (u32) cfg,
> +				   (u32) (cfg + PPC4xx_PCI_CFGD_OFFSET), 0);
> +
> +	/* Interpret the "ranges" property */
> +	/* This also maps the I/O region and sets isa_io/mem_base */
> +	pci_process_bridge_OF_ranges(hose, dev, primary);
> +	ppc440epx_setup_pci(hose, reg, cfg);
> +
> +	return 0;
> +}
> +#endif
> diff --git a/arch/powerpc/platforms/44x/sequoia.c b/arch/powerpc/platforms/44x/sequoia.c
> index ca3c9cc..c3a638b 100644
> --- a/arch/powerpc/platforms/44x/sequoia.c
> +++ b/arch/powerpc/platforms/44x/sequoia.c
> @@ -52,6 +52,20 @@ static int __init sequoia_probe(void)
>  
>  static void __init sequoia_setup_arch(void)
>  {
> +#ifdef CONFIG_PCI
> +	struct device_node *np;
> +#endif
> +
> +	if (ppc_md.progress)
> +		ppc_md.progress("ppc44x_setup_arch()", 0);
> +
> +#ifdef CONFIG_PCI
> +	for (np = NULL; (np = of_find_compatible_node(np,
> +					"pci", "ibm, 440epx")) != NULL;)
> +		ppc440epx_add_bridge(np);
> +	ppc_md.pci_exclude_device = ppc440epx_exclude_device;
> +#endif
> +
>  }
>  
>  define_machine(sequoia) {
> diff --git a/arch/powerpc/sysdev/indirect_pci.c b/arch/powerpc/sysdev/indirect_pci.c
> index b5d0682..9dab751 100644
> --- a/arch/powerpc/sysdev/indirect_pci.c
> +++ b/arch/powerpc/sysdev/indirect_pci.c
> @@ -162,3 +162,17 @@ setup_indirect_pci(struct pci_controller* hose, u32 cfg_addr, u32 cfg_data, u32
>  	hose->ops = &indirect_pci_ops;
>  	hose->indirect_type = flags;
>  }
> +
> +/*
> + * For some reason, ioremap does not handle 2-nd way mapping well,
> + * causing system check while trying to access config space later
> + */
> +void __init
> +setup_indirect_pci_noremap(struct pci_controller *hose, u32 cfg_addr,
> +		u32 cfg_data, u32 flags)
> +{
> +	hose->cfg_addr = (void *)cfg_addr;
> +	hose->cfg_data = (void *)cfg_data;
> +	hose->ops = &indirect_pci_ops;
> +	hose->indirect_type = flags;
> +}
> diff --git a/include/asm-powerpc/pci-bridge.h b/include/asm-powerpc/pci-bridge.h
> index e909769..4bde6e5 100644
> --- a/include/asm-powerpc/pci-bridge.h
> +++ b/include/asm-powerpc/pci-bridge.h
> @@ -99,6 +99,8 @@ extern int early_find_capability(struct pci_controller *hose, int bus,
>  
>  extern void setup_indirect_pci(struct pci_controller* hose,
>  			       u32 cfg_addr, u32 cfg_data, u32 flags);
> +extern void setup_indirect_pci_noremap(struct pci_controller *hose,
> +			       u32 cfg_addr, u32 cfg_data, u32 flags);
>  extern void setup_grackle(struct pci_controller *hose);
>  extern void __init update_bridge_resource(struct pci_dev *dev,
>  					  struct resource *res);
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

The patches seem to work fine on Sequoia, just
pci_iomap() function has to be fixed to support 64-bit resources.
I'll submit a couple of patches soon.
Thanks,
Valentine.