[PATCH] Use resource_size_t for serial port IO addresses

[PATCH] Use resource_size_t for serial port IO addresses

[Josh Boyer]

This is a resend of a patch David sent out on May 7.  Without it, the
PowerPC 44x port in 2.6.22 and on is broken.  I've rebased it off of
Linus' current tree.  Please consider pushing this soon.

josh


At present, various parts of the serial code use unsigned long to
define resource addresses.  This is a problem, because some 32-bit
platforms have physical addresses larger than 32-bits, and have mmio
serial uarts located above the 4GB point.

This patch changes the type of mapbase in both struct uart_port and
struct plat_serial8250_port to resource_size_t, which can be
configured to be 64 bits on such platforms.  The mapbase in
serial_struct can't safely be changed, because that structure is user
visible.

Signed-off-by: David Gibson <dwg@au1.ibm.com>

---
 drivers/serial/8250.c        |    5 +++--
 drivers/serial/8250_early.c  |   16 +++++++++-------
 drivers/serial/serial_core.c |    9 +++++----
 include/linux/serial_8250.h  |    2 +-
 include/linux/serial_core.h  |    2 +-
 5 files changed, 19 insertions(+), 15 deletions(-)

--- linux-2.6.orig/include/linux/serial_core.h
+++ linux-2.6/include/linux/serial_core.h
@@ -284,7 +284,7 @@ struct uart_port {
 	const struct uart_ops	*ops;
 	unsigned int		custom_divisor;
 	unsigned int		line;			/* port index */
-	unsigned long		mapbase;		/* for ioremap */
+	resource_size_t		mapbase;		/* for ioremap */
 	struct device		*dev;			/* parent device */
 	unsigned char		hub6;			/* this should be in the 8250 driver */
 	unsigned char		unused[3];
--- linux-2.6.orig/drivers/serial/serial_core.c
+++ linux-2.6/drivers/serial/serial_core.c
@@ -626,7 +626,7 @@ static int uart_get_info(struct uart_sta
 	tmp.hub6	    = port->hub6;
 	tmp.io_type         = port->iotype;
 	tmp.iomem_reg_shift = port->regshift;
-	tmp.iomem_base      = (void *)port->mapbase;
+	tmp.iomem_base      = (void *)(unsigned long)port->mapbase;
 
 	if (copy_to_user(retinfo, &tmp, sizeof(*retinfo)))
 		return -EFAULT;
@@ -1666,10 +1666,11 @@ static int uart_line_info(char *buf, str
 		return 0;
 
 	mmio = port->iotype >= UPIO_MEM;
-	ret = sprintf(buf, "%d: uart:%s %s%08lX irq:%d",
+	ret = sprintf(buf, "%d: uart:%s %s%08llX irq:%d",
 			port->line, uart_type(port),
 			mmio ? "mmio:0x" : "port:",
-			mmio ? port->mapbase : (unsigned long) port->iobase,
+			mmio ? (unsigned long long)port->mapbase
+		             : (unsigned long long) port->iobase,
 			port->irq);
 
 	if (port->type == PORT_UNKNOWN) {
@@ -2063,7 +2064,7 @@ uart_report_port(struct uart_driver *drv
 	case UPIO_TSI:
 	case UPIO_DWAPB:
 		snprintf(address, sizeof(address),
-			 "MMIO 0x%lx", port->mapbase);
+			 "MMIO 0x%llx", (unsigned long long)port->mapbase);
 		break;
 	default:
 		strlcpy(address, "*unknown*", sizeof(address));
--- linux-2.6.orig/drivers/serial/8250_early.c
+++ linux-2.6/drivers/serial/8250_early.c
@@ -145,8 +145,9 @@ static int __init parse_options(struct e
 		port->mapbase = simple_strtoul(options + 5, &options, 0);
 		port->membase = ioremap(port->mapbase, mapsize);
 		if (!port->membase) {
-			printk(KERN_ERR "%s: Couldn't ioremap 0x%lx\n",
-				__FUNCTION__, port->mapbase);
+			printk(KERN_ERR "%s: Couldn't ioremap 0x%llx\n",
+				__FUNCTION__,
+			       (unsigned long long)port->mapbase);
 			return -ENOMEM;
 		}
 		mmio = 1;
@@ -168,9 +169,10 @@ static int __init parse_options(struct e
 			device->baud);
 	}
 
-	printk(KERN_INFO "Early serial console at %s 0x%lx (options '%s')\n",
+	printk(KERN_INFO "Early serial console at %s 0x%llx (options '%s')\n",
 		mmio ? "MMIO" : "I/O port",
-		mmio ? port->mapbase : (unsigned long) port->iobase,
+		mmio ? (unsigned long long) port->mapbase
+	             : (unsigned long long) port->iobase,
 		device->options);
 	return 0;
 }
@@ -236,10 +238,10 @@ static int __init early_uart_console_swi
 	mmio = (port->iotype == UPIO_MEM);
 	line = serial8250_start_console(port, device->options);
 	if (line < 0)
-		printk("No ttyS device at %s 0x%lx for console\n",
+		printk("No ttyS device at %s 0x%llx for console\n",
 			mmio ? "MMIO" : "I/O port",
-			mmio ? port->mapbase :
-			    (unsigned long) port->iobase);
+			mmio ? (unsigned long long) port->mapbase
+		             : (unsigned long long) port->iobase);
 
 	unregister_console(&early_uart_console);
 	if (mmio)
--- linux-2.6.orig/include/linux/serial_8250.h
+++ linux-2.6/include/linux/serial_8250.h
@@ -20,7 +20,7 @@
 struct plat_serial8250_port {
 	unsigned long	iobase;		/* io base address */
 	void __iomem	*membase;	/* ioremap cookie or NULL */
-	unsigned long	mapbase;	/* resource base */
+	resource_size_t	mapbase;	/* resource base */
 	unsigned int	irq;		/* interrupt number */
 	unsigned int	uartclk;	/* UART clock rate */
 	unsigned char	regshift;	/* register shift */
--- linux-2.6.orig/drivers/serial/8250.c
+++ linux-2.6/drivers/serial/8250.c
@@ -2664,8 +2664,9 @@ static int __devinit serial8250_probe(st
 		ret = serial8250_register_port(&port);
 		if (ret < 0) {
 			dev_err(&dev->dev, "unable to register port at index %d "
-				"(IO%lx MEM%lx IRQ%d): %d\n", i,
-				p->iobase, p->mapbase, p->irq, ret);
+				"(IO%lx MEM%llx IRQ%d): %d\n", i,
+				p->iobase, (unsigned long long)p->mapbase,
+				p->irq, ret);
 		}
 	}
 	return 0;

Re: [PATCH] Use resource_size_t for serial port IO addresses

[Andrew Morton]

On Fri, 13 Jul 2007 09:02:16 -0500
Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:

> This is a resend of a patch David sent out on May 7.  Without it, the
> PowerPC 44x port in 2.6.22 and on is broken.  I've rebased it off of
> Linus' current tree.  Please consider pushing this soon.
> 
> josh
> 
> 
> At present, various parts of the serial code use unsigned long to
> define resource addresses.  This is a problem, because some 32-bit
> platforms have physical addresses larger than 32-bits, and have mmio
> serial uarts located above the 4GB point.
> 
> This patch changes the type of mapbase in both struct uart_port and
> struct plat_serial8250_port to resource_size_t, which can be
> configured to be 64 bits on such platforms.  The mapbase in
> serial_struct can't safely be changed, because that structure is user
> visible.

This is something we should do, but I have recollections of Russell
identifying problems with this patch, or at least an earlier version of it?

> Signed-off-by: David Gibson <dwg@au1.ibm.com>

This should have had Signed-off-by:you as well.

Re: [PATCH] Use resource_size_t for serial port IO addresses

[Josh Boyer]

On Fri, 2007-07-13 at 12:02 -0700, Andrew Morton wrote:
> On Fri, 13 Jul 2007 09:02:16 -0500
> Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> 
> > This is a resend of a patch David sent out on May 7.  Without it, the
> > PowerPC 44x port in 2.6.22 and on is broken.  I've rebased it off of
> > Linus' current tree.  Please consider pushing this soon.
> > 
> > josh
> > 
> > 
> > At present, various parts of the serial code use unsigned long to
> > define resource addresses.  This is a problem, because some 32-bit
> > platforms have physical addresses larger than 32-bits, and have mmio
> > serial uarts located above the 4GB point.
> > 
> > This patch changes the type of mapbase in both struct uart_port and
> > struct plat_serial8250_port to resource_size_t, which can be
> > configured to be 64 bits on such platforms.  The mapbase in
> > serial_struct can't safely be changed, because that structure is user
> > visible.
> 
> This is something we should do, but I have recollections of Russell
> identifying problems with this patch, or at least an earlier version of it?

I can't recall myself if there were problems or not.  I thought David
had worked that out with Russell, but I could be mistaken.

> > Signed-off-by: David Gibson <dwg@au1.ibm.com>
> 
> This should have had Signed-off-by:you as well.

Oops, for the rebase I suppose, yes.  Is the one below enough or do you
want me to resubmit?

Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

josh

Re: [PATCH] Use resource_size_t for serial port IO addresses

[Russell King]

On Fri, Jul 13, 2007 at 12:02:26PM -0700, Andrew Morton wrote:
> On Fri, 13 Jul 2007 09:02:16 -0500
> Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> 
> > This is a resend of a patch David sent out on May 7.  Without it, the
> > PowerPC 44x port in 2.6.22 and on is broken.  I've rebased it off of
> > Linus' current tree.  Please consider pushing this soon.
> > 
> > josh
> > 
> > 
> > At present, various parts of the serial code use unsigned long to
> > define resource addresses.  This is a problem, because some 32-bit
> > platforms have physical addresses larger than 32-bits, and have mmio
> > serial uarts located above the 4GB point.
> > 
> > This patch changes the type of mapbase in both struct uart_port and
> > struct plat_serial8250_port to resource_size_t, which can be
> > configured to be 64 bits on such platforms.  The mapbase in
> > serial_struct can't safely be changed, because that structure is user
> > visible.
> 
> This is something we should do, but I have recollections of Russell
> identifying problems with this patch, or at least an earlier version of it?

Basically, there's two patches.  This one (let's call this patch A)
and another to prevent users being surprised (let's call that patch B).

I didn't have any real objections to patch A, provided something like
patch B was merged.  I did have objections against patch B, and I was
intermittently working on a revised solution.

However, for whatever reason [*], during the last merge window patch B
got merged and patch A got dropped, and as a result I've now given up
with my revised solution, and TBH, I no longer care.


* - probably a result of the patches having been worked on several
months prior to the merge window, then me forgetting the discussions
that were had, and then objecting to the wrong patch with wrong
reasoning.  I really hate this "work on patches, wait months, forget
all about them, then try to work out what the hell happened" approach.
It causes mistakes like this.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH] Use resource_size_t for serial port IO addresses

[Josh Boyer]

On Sun, 2007-07-15 at 12:06 +0100, Russell King wrote:
> > This is something we should do, but I have recollections of Russell
> > identifying problems with this patch, or at least an earlier version of it?
> 
> Basically, there's two patches.  This one (let's call this patch A)
> and another to prevent users being surprised (let's call that patch B).
> 
> I didn't have any real objections to patch A, provided something like
> patch B was merged.  I did have objections against patch B, and I was
> intermittently working on a revised solution.
> 
> However, for whatever reason [*], during the last merge window patch B
> got merged and patch A got dropped, and as a result I've now given up
> with my revised solution, and TBH, I no longer care.

Patch B in this case was commit abb4a2390.  Since that has already been
merged, can we please merge this patch into 2.6.23?  I'd really like to
avoid 44x not working in yet another kernel, so if this patch can't be
merged I'll have to come up with some alternate solution soon.

josh

Re: [PATCH] Use resource_size_t for serial port IO addresses

[Andrew Morton]

On Mon, 23 Jul 2007 09:08:37 -0500
Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:

> On Sun, 2007-07-15 at 12:06 +0100, Russell King wrote:
> > > This is something we should do, but I have recollections of Russell
> > > identifying problems with this patch, or at least an earlier version of it?
> > 
> > Basically, there's two patches.  This one (let's call this patch A)
> > and another to prevent users being surprised (let's call that patch B).
> > 
> > I didn't have any real objections to patch A, provided something like
> > patch B was merged.  I did have objections against patch B, and I was
> > intermittently working on a revised solution.
> > 
> > However, for whatever reason [*], during the last merge window patch B
> > got merged and patch A got dropped, and as a result I've now given up
> > with my revised solution, and TBH, I no longer care.
> 
> Patch B in this case was commit abb4a2390.  Since that has already been
> merged, can we please merge this patch into 2.6.23?  I'd really like to
> avoid 44x not working in yet another kernel, so if this patch can't be
> merged I'll have to come up with some alternate solution soon.
> 

I still have a large pile of not-completely-obviously-ready patches to go
through, of which this is one.

There _were_ issues with this patch when it first turned up, but I failed
to record what they were.

Oh well, here's hoping it got fixed.

Re: [PATCH] Use resource_size_t for serial port IO addresses

[David Gibson]

On Mon, Jul 23, 2007 at 12:34:11PM -0700, Andrew Morton wrote:
> On Mon, 23 Jul 2007 09:08:37 -0500
> Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> 
> > On Sun, 2007-07-15 at 12:06 +0100, Russell King wrote:
> > > > This is something we should do, but I have recollections of Russell
> > > > identifying problems with this patch, or at least an earlier version of it?
> > > 
> > > Basically, there's two patches.  This one (let's call this patch A)
> > > and another to prevent users being surprised (let's call that patch B).
> > > 
> > > I didn't have any real objections to patch A, provided something like
> > > patch B was merged.  I did have objections against patch B, and I was
> > > intermittently working on a revised solution.
> > > 
> > > However, for whatever reason [*], during the last merge window patch B
> > > got merged and patch A got dropped, and as a result I've now given up
> > > with my revised solution, and TBH, I no longer care.
> > 
> > Patch B in this case was commit abb4a2390.  Since that has already been
> > merged, can we please merge this patch into 2.6.23?  I'd really like to
> > avoid 44x not working in yet another kernel, so if this patch can't be
> > merged I'll have to come up with some alternate solution soon.
> > 
> 
> I still have a large pile of not-completely-obviously-ready patches to go
> through, of which this is one.
> 
> There _were_ issues with this patch when it first turned up, but I failed
> to record what they were.

Heh.  Nor did I ever hear what they might be.

> 
> Oh well, here's hoping it got fixed.
> _______________________________________________
> 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