Skip to contentSkip to conversation

serial: MMIO32 support for 8250_early.c

PATCH In review Samium Gromoff 2 participants 5 messages Jun 23, 2010 – Jun 30, 2010 Documentation

lore
Share & tools
lore

Samium Gromoff

author
#0 lore raw

PATCH 1/1 serial: MMIO32 support for 8250_early.c

This should be fairly uncontroversial, but YMMV.

Available from master branch at git://git.feelingofgreen.ru/linux

commit 67e21ada6739e0730fb28f855abf4202a1bb3f8c

Author: Samium Gromoff <[email protected]>
Date:   Thu Jun 24 00:36:16 2010 +0400

Provide MMIO32 support in 8250_early (aka earlycon)

drivers/serial/8250_early.c +33 -16
index f279745..afb6dce 100644
@@ -48,18 +48,31 @@ static struct early_serial8250_device early_device;
 
 static unsigned int __init serial_in(struct uart_port *port, int offset)
 {
-	if (port->iotype == UPIO_MEM)
-		return readb(port->membase + offset);
-	else
-		return inb(port->iobase + offset);
+	switch (port->iotype) {
+		case UPIO_MEM:
+			return readb(port->membase + offset);
+		case UPIO_MEM32:
+			return readw(port->membase + (offset << 2));
+		case UPIO_PORT:
+			return inb(port->iobase + offset);
+		default:
+			return 0;
+	}
 }
 
 static void __init serial_out(struct uart_port *port, int offset, int value)
 {
-	if (port->iotype == UPIO_MEM)
-		writeb(value, port->membase + offset);
-	else
-		outb(value, port->iobase + offset);
+	switch (port->iotype) {
+		case UPIO_MEM:
+			writeb(value, port->membase + offset);
+			break;
+		case UPIO_MEM32:
+			writew(value, port->membase + (offset << 2));
+			break;
+		case UPIO_PORT:
+			outb(value, port->iobase + offset);
+			break;
+	}
 }
 
 #define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)

@@ -137,15 +150,20 @@ static int __init parse_options(struct early_serial8250_device *device,
 								char *options)
 {
 	struct uart_port *port = &device->port;
-	int mmio, length;
+	int mmio, mmio32, length;
 
 	if (!options)
 		return -ENODEV;
 
 	port->uartclk = BASE_BAUD * 16;
-	if (!strncmp(options, "mmio,", 5)) {
-		port->iotype = UPIO_MEM;
-		port->mapbase = simple_strtoul(options + 5, &options, 0);
+
+	mmio = !strncmp(options, "mmio,", 5);
+	mmio32 = !strncmp(options, "mmio32,", 7);
+	if (mmio || mmio32) {
+		port->iotype = (mmio ? UPIO_MEM : UPIO_MEM32);
+		port->mapbase = simple_strtoul(options + (mmio ? 5 : 7), &options, 0);
+		if (mmio32)
+			port->regshift = 2;
 #ifdef CONFIG_FIX_EARLYCON_MEM
 		set_fixmap_nocache(FIX_EARLYCON_MEM_BASE,
 					port->mapbase & PAGE_MASK);

@@ -161,7 +179,6 @@ static int __init parse_options(struct early_serial8250_device *device,
 			return -ENOMEM;
 		}
 #endif
-		mmio = 1;
 	} else if (!strncmp(options, "io,", 3)) {
 		port->iotype = UPIO_PORT;
 		port->iobase = simple_strtoul(options + 3, &options, 0);

@@ -182,9 +199,9 @@ static int __init parse_options(struct early_serial8250_device *device,
 	}
 
 	printk(KERN_INFO "Early serial console at %s 0x%llx (options '%s')\n",
-		mmio ? "MMIO" : "I/O port",
-		mmio ? (unsigned long long) port->mapbase
-		     : (unsigned long long) port->iobase,
+		mmio ? "MMIO" : mmio32 ? "MMIO32" : "I/O port",
+		(mmio | mmio32) ? (unsigned long long) port->mapbase
+		                : (unsigned long long) port->iobase,
 		device->options);
 	return 0;
 }
Signature · 5 lines
regards,
  Samium Gromoff
--
"Actually I made up the term 'object-oriented', and I can tell you I
did not have C++ in mind." - Alan Kay (OOPSLA 1997 Keynote)
#1 lore raw
Quoted 9 lines · On Wed, 23 Jun 2010 23:10:20 +0400 Samium Gromoff…

This should be fairly uncontroversial, but YMMV.

Available from master branch at git://git.feelingofgreen.ru/linux

commit 67e21ada6739e0730fb28f855abf4202a1bb3f8c

Author: Samium Gromoff <[email protected]>
Date:   Thu Jun 24 00:36:16 2010 +0400

Provide MMIO32 support in 8250_early (aka earlycon)

Please update Documentation/kernel-parameters.txt.

drivers/serial/8250_early.c static int __init parse_options(struct early_serial8250_device *device, · line 205

Quoted 13 lines

...

drivers/serial/8250_early.c +3 -3
@@ -182,9 +199,9 @@ static int __init parse_options(struct early_serial8250_device *device,
 	}
 
 	printk(KERN_INFO "Early serial console at %s 0x%llx (options '%s')\n",
-		mmio ? "MMIO" : "I/O port",
-		mmio ? (unsigned long long) port->mapbase
-		     : (unsigned long long) port->iobase,
+		mmio ? "MMIO" : mmio32 ? "MMIO32" : "I/O port",
+		(mmio | mmio32) ? (unsigned long long) port->mapbase
+		                : (unsigned long long) port->iobase,
 		device->options);

That printk has just got out of control. We should at the very least parenthesise that ?:?: thing before someone sees it and dies. Or just rip it up and reimplement it. Note that mapbase can be printed with %p and iobase with %pr, but the code munges them both into ull's and uses %llx.

author
#2 lore raw
Quoted 20 lines · Andrew Morton
Quoted 9 lines · On Wed, 23 Jun 2010 23:10:20 +0400 Samium Gromoff…

This should be fairly uncontroversial, but YMMV.

Available from master branch at git://git.feelingofgreen.ru/linux

commit 67e21ada6739e0730fb28f855abf4202a1bb3f8c

Author: Samium Gromoff <[email protected]>
Date:   Thu Jun 24 00:36:16 2010 +0400

Provide MMIO32 support in 8250_early (aka earlycon)

Please update Documentation/kernel-parameters.txt.

Quoted 5 lines

...

(quoted diff) +0 -0
@@ -182,9 +199,9 @@ static int __init parse_options(struct early_serial8250_device *device,
 	}
Quoted 14 lines

Review static int __init parse_options(struct early_serial8250_device *device, · line 205

Quoted 8 lines
(quoted diff) +3 -3
 	printk(KERN_INFO "Early serial console at %s 0x%llx (options '%s')\n",
-		mmio ? "MMIO" : "I/O port",
-		mmio ? (unsigned long long) port->mapbase
-		     : (unsigned long long) port->iobase,
+		mmio ? "MMIO" : mmio32 ? "MMIO32" : "I/O port",
+		(mmio | mmio32) ? (unsigned long long) port->mapbase
+		                : (unsigned long long) port->iobase,
 		device->options);

That printk has just got out of control. We should at the very least parenthesise that ?:?: thing before someone sees it and dies. Or just rip it up and reimplement it. Note that mapbase can be printed with %p and iobase with %pr, but the code munges them both into ull's and uses %llx.

Isn't the

    cond1 ? case1 [ : condx ? casex ]* ... : case-else

constuct kind of idiomatic? Well, I guess not very much, then..

Missing Common Lisp's powerful FORMAT function, with an embedded DSL for conditional sub-directives and such -- the resulting substitute is duplicate-ish..

So, the branch is again updated, and the delta is:

========================================================================

commit eb6a4309f9f1c56c6b600739c16d06ea1f1e74dd

Author: Samium Gromoff <[email protected]>
Date:   Wed Jun 30 23:07:46 2010 +0400

Provide MMIO32 support in 8250_early (aka earlycon)

Signed-off-by Samium Gromoff <[email protected]>
Documentation/kernel-parameters.txt +3 -0
index 1808f11..0bb9939 100644
@@ -686,8 +686,11 @@ and is between 256 and 4096 characters. It is defined in the file
 	earlycon=	[KNL] Output early console device and options.
 		uart[8250],io,<addr>[,options]
 		uart[8250],mmio,<addr>[,options]
+		uart[8250],mmio32,<addr>[,options]
 			Start an early, polled-mode console on the 8250/16550
 			UART at the specified I/O port or MMIO address.
+			MMIO inter-register address stride is either 8bit (mmio)
+                        or 32bit (mmio32).
 			The options are the same as for ttyS, above.
 
 	earlyprintk=	[X86,SH,BLACKFIN]
drivers/serial/8250_early.c +41 -19

105 lines not on this page · raw message

Signature · 5 lines
regards,
  Samium Gromoff
--
"Actually I made up the term 'object-oriented', and I can tell you I
did not have C++ in mind." - Alan Kay (OOPSLA 1997 Keynote)
#3 lore raw
Quoted 1 line · On Wed, 30 Jun 2010 23:12:43 +0400 Samium Gromoff…

So, the branch is again updated, and the delta is:

This gives checkpatch a complete heart attack. Please, use it?

author
#4 lore raw
Quoted 6 lines · Andrew Morton
Quoted 1 line · On Wed, 30 Jun 2010 23:12:43 +0400 Samium Gromoff…

So, the branch is again updated, and the delta is:

This gives checkpatch a complete heart attack. Please, use it?

Done. There's a warning, but it belongs to the original code, so not touching it..

= = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = commit ae1a741d1aab1a67dfe1c0ef2b874fcf838b858e

Author: Samium Gromoff <[email protected]>
Date:   Thu Jul 1 01:23:45 2010 +0400

Provide MMIO32 support in 8250_early (aka earlycon)

Signed-off-by Samium Gromoff <[email protected]>
Documentation/kernel-parameters.txt +3 -0
index 1808f11..0bb9939 100644
@@ -686,8 +686,11 @@ and is between 256 and 4096 characters. It is defined in the file
 	earlycon=	[KNL] Output early console device and options.
 		uart[8250],io,<addr>[,options]
 		uart[8250],mmio,<addr>[,options]
+		uart[8250],mmio32,<addr>[,options]
 			Start an early, polled-mode console on the 8250/16550
 			UART at the specified I/O port or MMIO address.
+			MMIO inter-register address stride is either 8bit (mmio)
+                        or 32bit (mmio32).
 			The options are the same as for ttyS, above.
 
 	earlyprintk=	[X86,SH,BLACKFIN]
drivers/serial/8250_early.c +38 -15

102 lines not on this page · raw message

Signature · 5 lines
regards,
  Samium Gromoff
--
"Actually I made up the term 'object-oriented', and I can tell you I
did not have C++ in mind." - Alan Kay (OOPSLA 1997 Keynote)