Hi Peter,

On 12/01/16 16:15, Peter Hurley wrote:

Hi Jon,

On 01/12/2016 02:33 AM, Jon Hunter wrote:

quoted

The 8250 early console assumes a default regshift for each iotype. This
does not work for all devices. For example, Tegra UARTs use a iotype of
UPIO_MEM with a regshift of 2 because the registers are 32-bit aligned
and permit byte access.

If the regshift is specified (for example, via device-tree), then use the
value provided and otherwise revert to the defaults assumed for each
iotype.

Signed-off-by: Jon Hunter <redacted>
---
Please note that today for Tegra, early console is supported by passing
the boot parameter "earlycon=uart8250,mmio32,0xXXXXXXXX". While this works
and we could use UPIO_MEM32 for tegra, this would mean changing all the
DT source files for Tegra to add the "reg-io-width" property. IMO it seems
better to make early console for 8250 work in the same way as the normal
8250 console and support regshift.

 drivers/tty/serial/8250/8250_early.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index af62131af21e..758054957788 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c

@@ -41,15 +41,15 @@ static unsigned int __init serial8250_early_in(struct uart_port *port, int offse
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
-		return readb(port->membase + offset);
+		return readb(port->membase + (offset << port->regshift));
 	case UPIO_MEM16:
-		return readw(port->membase + (offset << 1));
+		return readw(port->membase + (offset << port->regshift));
 	case UPIO_MEM32:
-		return readl(port->membase + (offset << 2));
+		return readl(port->membase + (offset << port->regshift));
 	case UPIO_MEM32BE:
-		return ioread32be(port->membase + (offset << 2));
+		return ioread32be(port->membase + (offset << port->regshift));
 	case UPIO_PORT:
-		return inb(port->iobase + offset);
+		return inb(port->iobase + (offset << port->regshift));
 	default:
 		return 0;
 	}

@@ -59,19 +59,19 @@ static void __init serial8250_early_out(struct uart_port *port, int offset, int
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
-		writeb(value, port->membase + offset);
+		writeb(value, port->membase + (offset << port->regshift));
 		break;
 	case UPIO_MEM16:
-		writew(value, port->membase + (offset << 1));
+		writew(value, port->membase + (offset << port->regshift));
 		break;
 	case UPIO_MEM32:
-		writel(value, port->membase + (offset << 2));
+		writel(value, port->membase + (offset << port->regshift));
 		break;
 	case UPIO_MEM32BE:
-		iowrite32be(value, port->membase + (offset << 2));
+		iowrite32be(value, port->membase + (offset << port->regshift));
 		break;
 	case UPIO_PORT:
-		outb(value, port->iobase + offset);
+		outb(value, port->iobase + (offset << port->regshift));
 		break;
 	}
 }

@@ -128,6 +128,22 @@ int __init early_serial8250_setup(struct earlycon_device *device,
 	if (!(device->port.membase || device->port.iobase))
 		return -ENODEV;
 
+	/*
+	 * If regshift is not specified, then assume the
+	 * following defaults for the below iotypes.
+	 */
+	if (!device->port.regshift) {
+		switch (device->port.iotype) {
+		case UPIO_MEM16:
+			device->port.regshift = 1;
+			break;
+		case UPIO_MEM32:
+		case UPIO_MEM32BE:
+			device->port.regshift = 2;
+			break;
+		}
+	}

Since the earlycon command line parsing sets port->regshift, the only possible
path requiring defaults would be DT, but 8250 DT should not have default
regshift based on the iotype; the 8250 port driver doesn't.

Ah, I see, I missed that. However, for command line parsing, I only see
regshift being set to 2 for mmio32 (and I guess it will be 0 otherwise).
Is mmio16 not supported for command line parsing?

Ok, I will drop the above hunk.

Cheers
Jon