On Sun, Jul 26, 2015 at 07:54:36AM +0300, Noam Camus wrote:
> From: Noam Camus <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
From: Greg KH [mailto:[email protected]]
Sent: Tuesday, August 04, 2015 2:43 AM
> > - writel(value, p->membase + (offset << p->regshift));
> Why drop this write?
This was not dropped, it is now part of dw8250_serial_out32().
Now it is called before updating last_mcr.
> > - 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"?
I will add inner level accessors into "struct dw8250_data" for in32/out32. new accessors will be used in few places in this driver that still uses writel/readl without considering iotype.
Noam-