Re: [v2 1/3] serial: 8250_dw: Add support for big-endian MMIO accesses

[Greg KH]

On Sun, Jul 26, 2015 at 07:54:36AM +0300, Noam Camus wrote:
> From: Noam Camus <noamc@xxxxxxxxxx>
> 
> Add support for UPIO_MEM32BE in addition to UPIO_MEM32.
> dw8250_serial_out32() main functionality was moved to new
> function called dw8250_check_LCR().
> 
> We use new 2 accessors similar to little endian, called
> dw8250_serial_out32be() and dw8250_serial_in32be().
> 
> Both little and big endian accessors use dw8250_check_LCR()
> for their dw8250_serial_out32{,be}().
> 
> Signed-off-by: Noam Camus <noamc@xxxxxxxxxx>
> ---
>  drivers/tty/serial/8250/8250_dw.c |   42 ++++++++++++++++++++++++++++++------
>  1 files changed, 35 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index d48b506..5c60ec8 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -173,15 +173,13 @@ static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
>  }
>  #endif /* CONFIG_64BIT */
>  
> -static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
> +static void dw8250_check_LCR(struct uart_port *p, int offset, int value)
>  {
>  	struct dw8250_data *d = p->private_data;
>  
>  	if (offset == UART_MCR)
>  		d->last_mcr = value;
>  
> -	writel(value, p->membase + (offset << p->regshift));

Why drop this write?

> -
>  	/* Make sure LCR write wasn't ignored */
>  	if (offset == UART_LCR) {
>  		int tries = 1000;
> @@ -190,7 +188,12 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
>  			if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
>  				return;
>  			dw8250_force_idle(p);
> -			writel(value, p->membase + (UART_LCR << p->regshift));
> +			if (p->iotype == UPIO_MEM32BE)
> +				iowrite32be(value,
> +					p->membase + (UART_LCR << p->regshift));
> +			else
> +				writel(value,
> +					p->membase + (UART_LCR << p->regshift));

Shouldn't this be hidden behind some other type of accessor?  Why is
this one writel() "special"?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/