[PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

The current pci_assign_unassigned_resources() code doesn't work properly
on 32 bits platforms with 64 bits resources. The main reason is the use
of unsigned long in various places instead of resource_size_t.

This fixes it, along with some tricks to avoid casting to 64 bits on
platforms that don't need it in every printk around.

This is a pre-requisite for making powerpc use the generic code instead of
its own half-useful implementation.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

This version fixes some stupid warnings when using 32 bits resources

 drivers/pci/pci.h       |   11 +++++++++++
 drivers/pci/setup-bus.c |   32 +++++++++++++++++---------------
 drivers/pci/setup-res.c |    5 ++---
 include/linux/pci.h     |    4 ++--
 4 files changed, 32 insertions(+), 20 deletions(-)

Index: linux-work/drivers/pci/pci.h
===================================================================
--- linux-work.orig/drivers/pci/pci.h	2007-12-05 11:55:49.000000000 +1100
+++ linux-work/drivers/pci/pci.h	2007-12-05 13:37:45.000000000 +1100
@@ -91,3 +91,14 @@ pci_match_one_device(const struct pci_de
 }
 
 struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
+
+#ifdef CONFIG_RESOURCES_64BIT
+#define RESOURCE_ORDER(order)	(1ULL << (order))
+#define RES_PR			"%016llx"
+#else
+#define RESOURCE_ORDER(order)	(1UL << (order))
+#define RES_PR			"%08x"
+#endif
+
+#define RANGE_PR		RES_PR "-" RES_PR
+
Index: linux-work/drivers/pci/setup-bus.c
===================================================================
--- linux-work.orig/drivers/pci/setup-bus.c	2007-12-05 11:55:49.000000000 +1100
+++ linux-work/drivers/pci/setup-bus.c	2007-12-05 12:04:35.000000000 +1100
@@ -26,6 +26,7 @@
 #include <linux/cache.h>
 #include <linux/slab.h>
 
+#include "pci.h"
 
 #define DEBUG_CONFIG 1
 #if DEBUG_CONFIG
@@ -89,8 +90,9 @@ void pci_setup_cardbus(struct pci_bus *b
 		 * The IO resource is allocated a range twice as large as it
 		 * would normally need.  This allows us to set both IO regs.
 		 */
-		printk("  IO window: %08lx-%08lx\n",
-			region.start, region.end);
+		printk(KERN_INFO "  IO window: 0x%08lx-0x%08lx\n",
+		       (unsigned long)region.start,
+		       (unsigned long)region.end);
 		pci_write_config_dword(bridge, PCI_CB_IO_BASE_0,
 					region.start);
 		pci_write_config_dword(bridge, PCI_CB_IO_LIMIT_0,
@@ -99,7 +101,7 @@ void pci_setup_cardbus(struct pci_bus *b
 
 	pcibios_resource_to_bus(bridge, &region, bus->resource[1]);
 	if (bus->resource[1]->flags & IORESOURCE_IO) {
-		printk("  IO window: %08lx-%08lx\n",
+		printk(KERN_INFO "  IO window: "RANGE_PR"\n",
 			region.start, region.end);
 		pci_write_config_dword(bridge, PCI_CB_IO_BASE_1,
 					region.start);
@@ -109,7 +111,7 @@ void pci_setup_cardbus(struct pci_bus *b
 
 	pcibios_resource_to_bus(bridge, &region, bus->resource[2]);
 	if (bus->resource[2]->flags & IORESOURCE_MEM) {
-		printk("  PREFETCH window: %08lx-%08lx\n",
+		printk(KERN_INFO "  PREFETCH window: "RANGE_PR"\n",
 			region.start, region.end);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_0,
 					region.start);
@@ -119,7 +121,7 @@ void pci_setup_cardbus(struct pci_bus *b
 
 	pcibios_resource_to_bus(bridge, &region, bus->resource[3]);
 	if (bus->resource[3]->flags & IORESOURCE_MEM) {
-		printk("  MEM window: %08lx-%08lx\n",
+		printk(KERN_INFO "  MEM window: "RANGE_PR"\n",
 			region.start, region.end);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_1,
 					region.start);
@@ -159,7 +161,8 @@ pci_setup_bridge(struct pci_bus *bus)
 		/* Set up upper 16 bits of I/O base/limit. */
 		io_upper16 = (region.end & 0xffff0000) | (region.start >> 16);
 		DBG(KERN_INFO "  IO window: %04lx-%04lx\n",
-				region.start, region.end);
+		    (unsigned long)region.start,
+		    (unsigned long)region.end);
 	}
 	else {
 		/* Clear upper 16 bits of I/O base/limit. */
@@ -180,7 +183,7 @@ pci_setup_bridge(struct pci_bus *bus)
 	if (bus->resource[1]->flags & IORESOURCE_MEM) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
-		DBG(KERN_INFO "  MEM window: %08lx-%08lx\n",
+		DBG(KERN_INFO "  MEM window: "RANGE_PR"\n",
 				region.start, region.end);
 	}
 	else {
@@ -199,7 +202,7 @@ pci_setup_bridge(struct pci_bus *bus)
 	if (bus->resource[2]->flags & IORESOURCE_PREFETCH) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
-		DBG(KERN_INFO "  PREFETCH window: %08lx-%08lx\n",
+		DBG(KERN_INFO "  PREFETCH window: "RANGE_PR"\n",
 				region.start, region.end);
 	}
 	else {
@@ -323,8 +326,8 @@ static void pbus_size_io(struct pci_bus 
 static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, unsigned long type)
 {
 	struct pci_dev *dev;
-	unsigned long min_align, align, size;
-	unsigned long aligns[12];	/* Alignments from 1Mb to 2Gb */
+	resource_size_t min_align, align, size;
+	resource_size_t aligns[12];	/* Alignments from 1Mb to 2Gb */
 	int order, max_order;
 	struct resource *b_res = find_free_bus_resource(bus, type);
 
@@ -340,7 +343,7 @@ static int pbus_size_mem(struct pci_bus 
 		
 		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 			struct resource *r = &dev->resource[i];
-			unsigned long r_size;
+			resource_size_t r_size;
 
 			if (r->parent || (r->flags & mask) != type)
 				continue;
@@ -350,10 +353,9 @@ static int pbus_size_mem(struct pci_bus 
 			order = __ffs(align) - 20;
 			if (order > 11) {
 				printk(KERN_WARNING "PCI: region %s/%d "
-				       "too large: %llx-%llx\n",
+				       "too large: " RANGE_PR "\n",
 					pci_name(dev), i,
-					(unsigned long long)r->start,
-					(unsigned long long)r->end);
+					r->start, r->end);
 				r->flags = 0;
 				continue;
 			}
@@ -372,7 +374,7 @@ static int pbus_size_mem(struct pci_bus 
 	align = 0;
 	min_align = 0;
 	for (order = 0; order <= max_order; order++) {
-		unsigned long align1 = 1UL << (order + 20);
+		resource_size_t align1 = RESOURCE_ORDER(order + 20);
 
 		if (!align)
 			min_align = align1;
Index: linux-work/drivers/pci/setup-res.c
===================================================================
--- linux-work.orig/drivers/pci/setup-res.c	2007-12-05 11:55:49.000000000 +1100
+++ linux-work/drivers/pci/setup-res.c	2007-12-05 12:04:35.000000000 +1100
@@ -51,9 +51,8 @@ pci_update_resource(struct pci_dev *dev,
 
 	pcibios_resource_to_bus(dev, &region, res);
 
-	pr_debug("  got res [%llx:%llx] bus [%lx:%lx] flags %lx for "
-		 "BAR %d of %s\n", (unsigned long long)res->start,
-		 (unsigned long long)res->end,
+	pr_debug("  got res ["RANGE_PR"] bus ["RANGE_PR"] flags %lx for "
+		 "BAR %d of %s\n", res->start, res->end,
 		 region.start, region.end, res->flags, resno, pci_name(dev));
 
 	new = region.start | (res->flags & PCI_REGION_FLAG_MASK);
Index: linux-work/include/linux/pci.h
===================================================================
--- linux-work.orig/include/linux/pci.h	2007-12-05 11:55:49.000000000 +1100
+++ linux-work/include/linux/pci.h	2007-12-05 12:04:35.000000000 +1100
@@ -314,8 +314,8 @@ struct pci_raw_ops {
 extern struct pci_raw_ops *raw_pci_ops;
 
 struct pci_bus_region {
-	unsigned long start;
-	unsigned long end;
+	resource_size_t start;
+	resource_size_t end;
 };
 
 struct pci_dynids {

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

On Wed, 2007-12-05 at 17:40 +1100, Benjamin Herrenschmidt wrote:
> The current pci_assign_unassigned_resources() code doesn't work properly
> on 32 bits platforms with 64 bits resources. The main reason is the use
> of unsigned long in various places instead of resource_size_t.
> 
> This fixes it, along with some tricks to avoid casting to 64 bits on
> platforms that don't need it in every printk around.
> 
> This is a pre-requisite for making powerpc use the generic code instead of
> its own half-useful implementation.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> This version fixes some stupid warnings when using 32 bits resources

 ... and has warnings on 64 bits platforms... GRRRR

This whole issue of printk vs. resource_size_t is a terrible mess :-(

Part of the problem is that resource_size_t can be either u32 or u64..

that is  it can be either unsigned int, unsigned long or unsigned long
long... and we have no way to reliably printk that.

Any clever idea before I start pushing filthy macros up linux/types.h ?

Cheers,
Ben.

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Greg KH]

On Thu, Dec 06, 2007 at 02:22:27PM +1100, Benjamin Herrenschmidt wrote:
> 
> On Wed, 2007-12-05 at 17:40 +1100, Benjamin Herrenschmidt wrote:
> > The current pci_assign_unassigned_resources() code doesn't work properly
> > on 32 bits platforms with 64 bits resources. The main reason is the use
> > of unsigned long in various places instead of resource_size_t.
> > 
> > This fixes it, along with some tricks to avoid casting to 64 bits on
> > platforms that don't need it in every printk around.
> > 
> > This is a pre-requisite for making powerpc use the generic code instead of
> > its own half-useful implementation.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> > 
> > This version fixes some stupid warnings when using 32 bits resources
> 
>  ... and has warnings on 64 bits platforms... GRRRR
> 
> This whole issue of printk vs. resource_size_t is a terrible mess :-(
> 
> Part of the problem is that resource_size_t can be either u32 or u64..
> 
> that is  it can be either unsigned int, unsigned long or unsigned long
> long... and we have no way to reliably printk that.

We do this already just fine.  Take a look in the kernel, I think we
just always cast it to long long to be uniform.

> Any clever idea before I start pushing filthy macros up linux/types.h ?

I don't think any macros are needed.

thanks,

greg k-h

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

On Wed, 2007-12-05 at 22:39 -0800, Greg KH wrote:
> > that is  it can be either unsigned int, unsigned long or unsigned
> long
> > long... and we have no way to reliably printk that.
> 
> We do this already just fine.  Take a look in the kernel, I think we
> just always cast it to long long to be uniform.

I wanted to avoid that for two reasons:

 - casts are fugly
 - it adds support code to cast & handle 64 bits to 32 bits platforms
   that wouldn't normally need it

Now, if you really think that's the way to go, I'll respin with casts
(I've used cast in subsequent patches merging bits & pieces of the
powerpc 32 and 64 bits PCI code too in fact).

I was just hoping somebody had a better idea, like a way to add a new
format specifier to printk without losing gcc type checking :-)

Cheers,
Ben.

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Geert Uytterhoeven]

On Thu, 6 Dec 2007, Benjamin Herrenschmidt wrote:
> On Wed, 2007-12-05 at 22:39 -0800, Greg KH wrote:
> > > that is  it can be either unsigned int, unsigned long or unsigned
> > long
> > > long... and we have no way to reliably printk that.
> > 
> > We do this already just fine.  Take a look in the kernel, I think we
> > just always cast it to long long to be uniform.
> 
> I wanted to avoid that for two reasons:
> 
>  - casts are fugly
>  - it adds support code to cast & handle 64 bits to 32 bits platforms
>    that wouldn't normally need it

Indeed. I still have a few places to fix it up in the Zorro bus code and
drivers. Everything is 32 bit, but allyesconfig sets CONFIG_RESOURCES_64BIT...
In some way I liked the recent suggestion to make CONFIG_RESOURCES_64BIT
depend on something...

> Now, if you really think that's the way to go, I'll respin with casts
> (I've used cast in subsequent patches merging bits & pieces of the
> powerpc 32 and 64 bits PCI code too in fact).

And casting to `unsigned long' for Zorro bus may sound fine, except what if
one day we'll need 64-bit resources for one platform?

> I was just hoping somebody had a better idea, like a way to add a new
> format specifier to printk without losing gcc type checking :-)

#define PRI* (...) ;-)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Matthew Wilcox]

On Thu, Dec 06, 2007 at 06:58:54PM +1100, Benjamin Herrenschmidt wrote:
> I was just hoping somebody had a better idea, like a way to add a new
> format specifier to printk without losing gcc type checking :-)

It's been discussed before.  Some of the solutions discussed:

 - Add something like PRI_RES which can be concatenated into a printk.
   Ugly.
 - Patch gcc to allow user-definable types.  I think OpenBSD has a patch
   for this.  Then we have to get that patch propagated to all the
   people who compile the kernel.  Unappetising.
 - Disable gcc's printk checking, teach sparse to typecheck printk.
   Most people don't run sparse yet.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Kumar Gala]

On Dec 6, 2007, at 2:16 AM, Matthew Wilcox wrote:

> On Thu, Dec 06, 2007 at 06:58:54PM +1100, Benjamin Herrenschmidt  
> wrote:
>> I was just hoping somebody had a better idea, like a way to add a new
>> format specifier to printk without losing gcc type checking :-)
>
> It's been discussed before.  Some of the solutions discussed:
>
> - Add something like PRI_RES which can be concatenated into a printk.
>   Ugly.
> - Patch gcc to allow user-definable types.  I think OpenBSD has a  
> patch
>   for this.  Then we have to get that patch propagated to all the
>   people who compile the kernel.  Unappetising.
> - Disable gcc's printk checking, teach sparse to typecheck printk.
>   Most people don't run sparse yet.

How does gcc deal with glibc extension to allow people to add their  
own specifiers?

http://www.gnu.org/software/libc/manual/html_node/Customizing-Printf.html

- k

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Andreas Schwab]

Kumar Gala <galak@kernel.crashing.org> writes:

> How does gcc deal with glibc extension to allow people to add their  
> own specifiers?

It doesn't, gcc only knows about the standard specifiers.  But it also
defines attributes that check other formats, like its own asm_fprintf
format specs.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

> It's been discussed before.  Some of the solutions discussed:
> 
>  - Add something like PRI_RES which can be concatenated into a printk.
>    Ugly.
>  - Patch gcc to allow user-definable types.  I think OpenBSD has a patch
>    for this.  Then we have to get that patch propagated to all the
>    people who compile the kernel.  Unappetising.
>  - Disable gcc's printk checking, teach sparse to typecheck printk.
>    Most people don't run sparse yet.

So you seem to dislike all 3, which one do you dislike the less ?

Cheers,
Ben.

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Greg KH]

On Thu, Dec 06, 2007 at 06:58:54PM +1100, Benjamin Herrenschmidt wrote:
> 
> On Wed, 2007-12-05 at 22:39 -0800, Greg KH wrote:
> > > that is  it can be either unsigned int, unsigned long or unsigned
> > long
> > > long... and we have no way to reliably printk that.
> > 
> > We do this already just fine.  Take a look in the kernel, I think we
> > just always cast it to long long to be uniform.
> 
> I wanted to avoid that for two reasons:
> 
>  - casts are fugly
>  - it adds support code to cast & handle 64 bits to 32 bits platforms
>    that wouldn't normally need it

But that is how we already handle this today, in numerous places in the
kernel for this very type.

So, you can disagree that this is what we need to do, and if so, feel
free to fix up a whole lot of files in the tree :)

thanks,

greg k-h

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

On Thu, 2007-12-06 at 17:00 -0800, Greg KH wrote:
> But that is how we already handle this today, in numerous places in
> the
> kernel for this very type.
> 
> So, you can disagree that this is what we need to do, and if so, feel
> free to fix up a whole lot of files in the tree :)

Heh, ok, allright, I'll respin with the casts.

Cheers,
Ben.

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

On Mon, 2007-12-10 at 17:15 +1100, Benjamin Herrenschmidt wrote:
> The current pci_assign_unassigned_resources() code doesn't work properly
> on 32 bits platforms with 64 bits resources. The main reason is the use
> of unsigned long in various places instead of resource_size_t.
> 
> This fixes it, along with some tricks to avoid casting to 64 bits on
> platforms that don't need it in every printk around.
> 
> This is a pre-requisite for making powerpc use the generic code instead of
> its own half-useful implementation.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> This version now uses casts as Greg asked for and adds proper setup
> of the prefetchable base & limit "upper" registers when using 64 bits
> resources.

Crap ! Ignore it. I forgot to quilt ref again... won't build.

Sending it again, with a separate patch fixing a warning in setup-res.c
that isn't directly related to the changes to setup-bus.c

Cheers,
Ben.

[PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

The current pci_assign_unassigned_resources() code doesn't work properly
on 32 bits platforms with 64 bits resources. The main reason is the use
of unsigned long in various places instead of resource_size_t.

This fixes it, along with some tricks to avoid casting to 64 bits on
platforms that don't need it in every printk around.

This is a pre-requisite for making powerpc use the generic code instead of
its own half-useful implementation.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

This version now uses casts as Greg asked for and adds proper setup
of the prefetchable base & limit "upper" registers when using 64 bits
resources.

 drivers/pci/setup-bus.c |   59 +++++++++++++++++++++++++++++-------------------
 include/linux/pci.h     |    4 +--
 2 files changed, 38 insertions(+), 25 deletions(-)

Index: linux-work/drivers/pci/setup-bus.c
===================================================================
--- linux-work.orig/drivers/pci/setup-bus.c	2007-12-10 16:56:51.000000000 +1100
+++ linux-work/drivers/pci/setup-bus.c	2007-12-10 17:12:27.000000000 +1100
@@ -89,8 +89,9 @@ void pci_setup_cardbus(struct pci_bus *b
 		 * The IO resource is allocated a range twice as large as it
 		 * would normally need.  This allows us to set both IO regs.
 		 */
-		printk("  IO window: %08lx-%08lx\n",
-			region.start, region.end);
+		printk(KERN_INFO "  IO window: 0x%08lx-0x%08lx\n",
+		       (unsigned long)region.start,
+		       (unsigned long)region.end);
 		pci_write_config_dword(bridge, PCI_CB_IO_BASE_0,
 					region.start);
 		pci_write_config_dword(bridge, PCI_CB_IO_LIMIT_0,
@@ -99,8 +100,9 @@ void pci_setup_cardbus(struct pci_bus *b
 
 	pcibios_resource_to_bus(bridge, &region, bus->resource[1]);
 	if (bus->resource[1]->flags & IORESOURCE_IO) {
-		printk("  IO window: %08lx-%08lx\n",
-			region.start, region.end);
+		printk(KERN_INFO "  IO window: 0x%08lx-0x%08lx\n",
+		       (unsigned long)region.start,
+		       (unsigned long)region.end);
 		pci_write_config_dword(bridge, PCI_CB_IO_BASE_1,
 					region.start);
 		pci_write_config_dword(bridge, PCI_CB_IO_LIMIT_1,
@@ -109,8 +111,9 @@ void pci_setup_cardbus(struct pci_bus *b
 
 	pcibios_resource_to_bus(bridge, &region, bus->resource[2]);
 	if (bus->resource[2]->flags & IORESOURCE_MEM) {
-		printk("  PREFETCH window: %08lx-%08lx\n",
-			region.start, region.end);
+		printk(KERN_INFO "  PREFETCH window: 0x%08lx-0x%08lx\n",
+		       (unsigned long)region.start,
+		       (unsigned long)region.end);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_0,
 					region.start);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_LIMIT_0,
@@ -119,8 +122,9 @@ void pci_setup_cardbus(struct pci_bus *b
 
 	pcibios_resource_to_bus(bridge, &region, bus->resource[3]);
 	if (bus->resource[3]->flags & IORESOURCE_MEM) {
-		printk("  MEM window: %08lx-%08lx\n",
-			region.start, region.end);
+		printk(KERN_INFO "  MEM window: 0x%08lx-0x%08lx\n",
+		       (unsigned long)region.start,
+		       (unsigned long)region.end);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_1,
 					region.start);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_LIMIT_1,
@@ -145,7 +149,7 @@ pci_setup_bridge(struct pci_bus *bus)
 {
 	struct pci_dev *bridge = bus->self;
 	struct pci_bus_region region;
-	u32 l, io_upper16;
+	u32 l, bu, lu, io_upper16;
 
 	DBG(KERN_INFO "PCI: Bridge: %s\n", pci_name(bridge));
 
@@ -159,7 +163,8 @@ pci_setup_bridge(struct pci_bus *bus)
 		/* Set up upper 16 bits of I/O base/limit. */
 		io_upper16 = (region.end & 0xffff0000) | (region.start >> 16);
 		DBG(KERN_INFO "  IO window: %04lx-%04lx\n",
-				region.start, region.end);
+		    (unsigned long)region.start,
+		    (unsigned long)region.end);
 	}
 	else {
 		/* Clear upper 16 bits of I/O base/limit. */
@@ -180,8 +185,9 @@ pci_setup_bridge(struct pci_bus *bus)
 	if (bus->resource[1]->flags & IORESOURCE_MEM) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
-		DBG(KERN_INFO "  MEM window: %08lx-%08lx\n",
-				region.start, region.end);
+		DBG(KERN_INFO "  MEM window: 0x%08lx-0x%08lx\n",
+		    (unsigned long)region.start,
+		    (unsigned long)region.end);
 	}
 	else {
 		l = 0x0000fff0;
@@ -195,12 +201,18 @@ pci_setup_bridge(struct pci_bus *bus)
 	pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, 0);
 
 	/* Set up PREF base/limit. */
+	bu = lu = 0;
 	pcibios_resource_to_bus(bridge, &region, bus->resource[2]);
 	if (bus->resource[2]->flags & IORESOURCE_PREFETCH) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
-		DBG(KERN_INFO "  PREFETCH window: %08lx-%08lx\n",
-				region.start, region.end);
+#ifdef CONFIG_RESOURCES_64BIT
+		bu = region.start >> 32;
+		lu = region.end >> 32;
+#endif
+		DBG(KERN_INFO "  PREFETCH window: 0x%016llx-0x%016llx\n",
+		    (unsigned long long)region.start,
+		    (unsigned long long)region.end);
 	}
 	else {
 		l = 0x0000fff0;
@@ -208,8 +220,9 @@ pci_setup_bridge(struct pci_bus *bus)
 	}
 	pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, l);
 
-	/* Clear out the upper 32 bits of PREF base. */
-	pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, 0);
+	/* Set the upper 32 bits of PREF base & limit. */
+	pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
+	pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
 
 	pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl);
 }
@@ -323,8 +336,8 @@ static void pbus_size_io(struct pci_bus 
 static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, unsigned long type)
 {
 	struct pci_dev *dev;
-	unsigned long min_align, align, size;
-	unsigned long aligns[12];	/* Alignments from 1Mb to 2Gb */
+	resource_size_t min_align, align, size;
+	resource_size_t aligns[12];	/* Alignments from 1Mb to 2Gb */
 	int order, max_order;
 	struct resource *b_res = find_free_bus_resource(bus, type);
 
@@ -340,7 +353,7 @@ static int pbus_size_mem(struct pci_bus 
 		
 		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 			struct resource *r = &dev->resource[i];
-			unsigned long r_size;
+			resource_size_t r_size;
 
 			if (r->parent || (r->flags & mask) != type)
 				continue;
@@ -350,10 +363,10 @@ static int pbus_size_mem(struct pci_bus 
 			order = __ffs(align) - 20;
 			if (order > 11) {
 				printk(KERN_WARNING "PCI: region %s/%d "
-				       "too large: %llx-%llx\n",
+				       "too large: 0x%016llx-0x%016llx\n",
 					pci_name(dev), i,
-					(unsigned long long)r->start,
-					(unsigned long long)r->end);
+				       (unsigned long long)r->start,
+				       (unsigned long long)r->end);
 				r->flags = 0;
 				continue;
 			}
@@ -372,7 +385,7 @@ static int pbus_size_mem(struct pci_bus 
 	align = 0;
 	min_align = 0;
 	for (order = 0; order <= max_order; order++) {
-		unsigned long align1 = 1UL << (order + 20);
+		resource_size_t align1 = RESOURCE_ORDER(order + 20);
 
 		if (!align)
 			min_align = align1;
Index: linux-work/include/linux/pci.h
===================================================================
--- linux-work.orig/include/linux/pci.h	2007-12-10 16:56:51.000000000 +1100
+++ linux-work/include/linux/pci.h	2007-12-10 16:57:20.000000000 +1100
@@ -314,8 +314,8 @@ struct pci_raw_ops {
 extern struct pci_raw_ops *raw_pci_ops;
 
 struct pci_bus_region {
-	unsigned long start;
-	unsigned long end;
+	resource_size_t start;
+	resource_size_t end;
 };
 
 struct pci_dynids {

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

On Tue, 2007-12-04 at 17:08 +1100, Benjamin Herrenschmidt wrote:
> The current pci_assign_unassigned_resources() code doesn't work properly
> on 32 bits platforms with 64 bits resources. The main reason is the use
> of unsigned long in various places instead of resource_size_t.
> 
> This fixes it, along with some tricks to avoid casting to 64 bits on
> platforms that don't need it in every printk around.
> 
> This is a pre-requisite for making powerpc use the generic code instead of
> its own half-useful implementation.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Drop it for now, stupid warnings without 64 bits resources... I'll fix
that.

Ben.

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

On Tue, 2007-12-04 at 13:39 +0100, Geert Uytterhoeven wrote:
> 
> Can we please have them in <linux/ioport.h>? They look very useful to
> me
> elsewhere (other bus drivers, device drivers), too.
> 
> What about naming the printf format specifier macros more like in C99,
> e.g.
> PRI*?

That's a can of worms I just didn't want to open...

Ben.

Re: [PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2306 bytes --]

On Tue, 4 Dec 2007, Benjamin Herrenschmidt wrote:
> The current pci_assign_unassigned_resources() code doesn't work properly
> on 32 bits platforms with 64 bits resources. The main reason is the use
> of unsigned long in various places instead of resource_size_t.
> 
> This fixes it, along with some tricks to avoid casting to 64 bits on
> platforms that don't need it in every printk around.
> 
> This is a pre-requisite for making powerpc use the generic code instead of
> its own half-useful implementation.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
>  drivers/pci/pci.h       |   11 +++++++++++
>  drivers/pci/setup-bus.c |   32 +++++++++++++++++---------------
>  drivers/pci/setup-res.c |    5 ++---
>  include/linux/pci.h     |    4 ++--
>  4 files changed, 32 insertions(+), 20 deletions(-)
> 
> Index: linux-work/drivers/pci/pci.h
> ===================================================================
> --- linux-work.orig/drivers/pci/pci.h	2007-12-04 17:00:43.000000000 +1100
> +++ linux-work/drivers/pci/pci.h	2007-12-04 17:02:11.000000000 +1100
> @@ -91,3 +91,14 @@ pci_match_one_device(const struct pci_de
>  }
>  
>  struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
> +
> +#ifdef CONFIG_RESOURCES_64BIT
> +#define RESOURCE_ORDER(order)	(1ULL << (order))
> +#define RES_PR			"%016llx"
> +#else
> +#define RESOURCE_ORDER(order)	(1UL << (order))
> +#define RES_PR			"%08lx"
> +#endif
> +
> +#define RANGE_PR		RES_PR "-" RES_PR

Can we please have them in <linux/ioport.h>? They look very useful to me
elsewhere (other bus drivers, device drivers), too.

What about naming the printf format specifier macros more like in C99, e.g.
PRI*?

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

[PATCH] pci: Fix bus resource assignment on 32 bits with 64b resources

[Benjamin Herrenschmidt]

The current pci_assign_unassigned_resources() code doesn't work properly
on 32 bits platforms with 64 bits resources. The main reason is the use
of unsigned long in various places instead of resource_size_t.

This fixes it, along with some tricks to avoid casting to 64 bits on
platforms that don't need it in every printk around.

This is a pre-requisite for making powerpc use the generic code instead of
its own half-useful implementation.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 drivers/pci/pci.h       |   11 +++++++++++
 drivers/pci/setup-bus.c |   32 +++++++++++++++++---------------
 drivers/pci/setup-res.c |    5 ++---
 include/linux/pci.h     |    4 ++--
 4 files changed, 32 insertions(+), 20 deletions(-)

Index: linux-work/drivers/pci/pci.h
===================================================================
--- linux-work.orig/drivers/pci/pci.h	2007-12-04 17:00:43.000000000 +1100
+++ linux-work/drivers/pci/pci.h	2007-12-04 17:02:11.000000000 +1100
@@ -91,3 +91,14 @@ pci_match_one_device(const struct pci_de
 }
 
 struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
+
+#ifdef CONFIG_RESOURCES_64BIT
+#define RESOURCE_ORDER(order)	(1ULL << (order))
+#define RES_PR			"%016llx"
+#else
+#define RESOURCE_ORDER(order)	(1UL << (order))
+#define RES_PR			"%08lx"
+#endif
+
+#define RANGE_PR		RES_PR "-" RES_PR
+
Index: linux-work/drivers/pci/setup-bus.c
===================================================================
--- linux-work.orig/drivers/pci/setup-bus.c	2007-12-04 17:00:43.000000000 +1100
+++ linux-work/drivers/pci/setup-bus.c	2007-12-04 17:04:23.000000000 +1100
@@ -26,6 +26,7 @@
 #include <linux/cache.h>
 #include <linux/slab.h>
 
+#include "pci.h"
 
 #define DEBUG_CONFIG 1
 #if DEBUG_CONFIG
@@ -89,8 +90,9 @@ void pci_setup_cardbus(struct pci_bus *b
 		 * The IO resource is allocated a range twice as large as it
 		 * would normally need.  This allows us to set both IO regs.
 		 */
-		printk("  IO window: %08lx-%08lx\n",
-			region.start, region.end);
+		printk(KERN_INFO "  IO window: 0x%08lx-0x%08lx\n",
+		       (unsigned long)region.start,
+		       (unsigned long)region.end);
 		pci_write_config_dword(bridge, PCI_CB_IO_BASE_0,
 					region.start);
 		pci_write_config_dword(bridge, PCI_CB_IO_LIMIT_0,
@@ -99,7 +101,7 @@ void pci_setup_cardbus(struct pci_bus *b
 
 	pcibios_resource_to_bus(bridge, &region, bus->resource[1]);
 	if (bus->resource[1]->flags & IORESOURCE_IO) {
-		printk("  IO window: %08lx-%08lx\n",
+		printk(KERN_INFO "  IO window: "RANGE_PR"\n",
 			region.start, region.end);
 		pci_write_config_dword(bridge, PCI_CB_IO_BASE_1,
 					region.start);
@@ -109,7 +111,7 @@ void pci_setup_cardbus(struct pci_bus *b
 
 	pcibios_resource_to_bus(bridge, &region, bus->resource[2]);
 	if (bus->resource[2]->flags & IORESOURCE_MEM) {
-		printk("  PREFETCH window: %08lx-%08lx\n",
+		printk(KERN_INFO "  PREFETCH window: "RANGE_PR"\n",
 			region.start, region.end);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_0,
 					region.start);
@@ -119,7 +121,7 @@ void pci_setup_cardbus(struct pci_bus *b
 
 	pcibios_resource_to_bus(bridge, &region, bus->resource[3]);
 	if (bus->resource[3]->flags & IORESOURCE_MEM) {
-		printk("  MEM window: %08lx-%08lx\n",
+		printk(KERN_INFO "  MEM window: "RANGE_PR"\n",
 			region.start, region.end);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_1,
 					region.start);
@@ -159,7 +161,8 @@ pci_setup_bridge(struct pci_bus *bus)
 		/* Set up upper 16 bits of I/O base/limit. */
 		io_upper16 = (region.end & 0xffff0000) | (region.start >> 16);
 		DBG(KERN_INFO "  IO window: %04lx-%04lx\n",
-				region.start, region.end);
+		    (unsigned long)region.start,
+		    (unsigned long)region.end);
 	}
 	else {
 		/* Clear upper 16 bits of I/O base/limit. */
@@ -180,7 +183,7 @@ pci_setup_bridge(struct pci_bus *bus)
 	if (bus->resource[1]->flags & IORESOURCE_MEM) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
-		DBG(KERN_INFO "  MEM window: %08lx-%08lx\n",
+		DBG(KERN_INFO "  MEM window: "RANGE_PR"\n",
 				region.start, region.end);
 	}
 	else {
@@ -199,7 +202,7 @@ pci_setup_bridge(struct pci_bus *bus)
 	if (bus->resource[2]->flags & IORESOURCE_PREFETCH) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
-		DBG(KERN_INFO "  PREFETCH window: %08lx-%08lx\n",
+		DBG(KERN_INFO "  PREFETCH window: "RANGE_PR"\n",
 				region.start, region.end);
 	}
 	else {
@@ -323,8 +326,8 @@ static void pbus_size_io(struct pci_bus 
 static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, unsigned long type)
 {
 	struct pci_dev *dev;
-	unsigned long min_align, align, size;
-	unsigned long aligns[12];	/* Alignments from 1Mb to 2Gb */
+	resource_size_t min_align, align, size;
+	resource_size_t aligns[12];	/* Alignments from 1Mb to 2Gb */
 	int order, max_order;
 	struct resource *b_res = find_free_bus_resource(bus, type);
 
@@ -340,7 +343,7 @@ static int pbus_size_mem(struct pci_bus 
 		
 		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 			struct resource *r = &dev->resource[i];
-			unsigned long r_size;
+			resource_size_t r_size;
 
 			if (r->parent || (r->flags & mask) != type)
 				continue;
@@ -350,10 +353,9 @@ static int pbus_size_mem(struct pci_bus 
 			order = __ffs(align) - 20;
 			if (order > 11) {
 				printk(KERN_WARNING "PCI: region %s/%d "
-				       "too large: %llx-%llx\n",
+				       "too large: " RANGE_PR "\n",
 					pci_name(dev), i,
-					(unsigned long long)r->start,
-					(unsigned long long)r->end);
+					r->start, r->end);
 				r->flags = 0;
 				continue;
 			}
@@ -372,7 +374,7 @@ static int pbus_size_mem(struct pci_bus 
 	align = 0;
 	min_align = 0;
 	for (order = 0; order <= max_order; order++) {
-		unsigned long align1 = 1UL << (order + 20);
+		resource_size_t align1 = RESOURCE_ORDER(order + 20);
 
 		if (!align)
 			min_align = align1;
Index: linux-work/drivers/pci/setup-res.c
===================================================================
--- linux-work.orig/drivers/pci/setup-res.c	2007-12-04 17:00:43.000000000 +1100
+++ linux-work/drivers/pci/setup-res.c	2007-12-04 17:02:11.000000000 +1100
@@ -51,9 +51,8 @@ pci_update_resource(struct pci_dev *dev,
 
 	pcibios_resource_to_bus(dev, &region, res);
 
-	pr_debug("  got res [%llx:%llx] bus [%lx:%lx] flags %lx for "
-		 "BAR %d of %s\n", (unsigned long long)res->start,
-		 (unsigned long long)res->end,
+	pr_debug("  got res ["RANGE_PR"] bus ["RANGE_PR"] flags %lx for "
+		 "BAR %d of %s\n", res->start, res->end,
 		 region.start, region.end, res->flags, resno, pci_name(dev));
 
 	new = region.start | (res->flags & PCI_REGION_FLAG_MASK);
Index: linux-work/include/linux/pci.h
===================================================================
--- linux-work.orig/include/linux/pci.h	2007-12-04 17:00:43.000000000 +1100
+++ linux-work/include/linux/pci.h	2007-12-04 17:02:11.000000000 +1100
@@ -314,8 +314,8 @@ struct pci_raw_ops {
 extern struct pci_raw_ops *raw_pci_ops;
 
 struct pci_bus_region {
-	unsigned long start;
-	unsigned long end;
+	resource_size_t start;
+	resource_size_t end;
 };
 
 struct pci_dynids {