Re: [PATCH] 8250: add a UPIO_DWAPB32 for 32 bit accesses (v2)

[Greg KH <greg@kroah.com>]

On Mon, Nov 22, 2010 at 02:26:47PM +0000, Jamie Iles wrote:
> Some platforms contain a Synopsys DesignWare APB UART that is attached
> to a 32-bit APB bus where sub-word accesses are not allowed. Add a new
> IO type (UPIO_DWAPB32) that performs 32 bit acccesses to the UART.
> 
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> ---
>  drivers/serial/8250.c        |   18 ++++++++++++++++--
>  drivers/serial/serial_core.c |    2 ++
>  include/linux/serial_core.h  |    1 +
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index 09a5508..244a982 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -464,7 +464,12 @@ static void dwapb_serial_out(struct uart_port *p, int offset, int value)
>  		struct uart_8250_port *up = (struct uart_8250_port *)p;
>  		up->lcr = value;
>  	}
> -	writeb(value, p->membase + offset);
> +
> +	if (UPIO_DWAPB == p->iotype)

I understand why people feel they have to write code like this, but
please, don't, put the variable test the other way around.  The complier
will complain if you accidentally use '=' instead of '=='


> +		writeb(value, p->membase + offset);
> +	else
> +		writel(value, p->membase + offset);
> +

As this is on the "fast path" for writing data out, isn't it going to
slow down the UPIO_DWAPB users as well?

thanks,

greg k-h