Re: [PATCH 2/3] serial: 8250_early: Add support for regshift

[Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>]

Hi Jon,

On 01/12/2016 02:33 AM, Jon Hunter wrote:
> 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 <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> 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.

Regards,
Peter Hurley

> +
>  	if (!device->baud) {
>  		struct uart_port *port = &device->port;
>  		unsigned int ier;
>