-----Original Message-----
From: Andy Duan
Sent: Tuesday, June 13, 2017 11:09 AM
To: A.s. Dong; linux-serial at vger.kernel.org
Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
gregkh at linuxfoundation.org; jslaby at suse.com; stefan at agner.ch; Mingkai Hu;
Y.b. Lu; nikita.yoush at cogentembedded.com; andy.shevchenko at gmail.com;
dongas86 at gmail.com; A.s. Dong
Subject: RE: [PATCH V3 3/7] tty: serial: lpuart: add little endian 32 bit
register support

From: Dong Aisheng <aisheng.dong@nxp.com> Sent: Monday, June 12, 2017
11:37 PM

quoted

To: linux-serial at vger.kernel.org
Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
gregkh at linuxfoundation.org; jslaby at suse.com; Andy Duan
[off-list ref]; stefan at agner.ch; Mingkai Hu
[off-list ref]; Y.b. Lu [off-list ref];
nikita.yoush at cogentembedded.com; andy.shevchenko at gmail.com;
dongas86 at gmail.com; A.S. Dong [off-list ref]
Subject: [PATCH V3 3/7] tty: serial: lpuart: add little endian 32 bit
register support

Use standard port->iotype to distinguish endian difference. Note as we
read/write register by checking iotype dynamically, we need to
initialize the iotype correctly for earlycon as well to avoid a break.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <redacted> (supporter:TTY LAYER)
Cc: Stefan Agner <stefan@agner.ch>
Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Cc: Fugang Duan <redacted>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

ChangeLog:
v2->v3:
* Instead of using global var, use standard port->iotype to distinguish
  endian difference.
v1->v2:
* No changes
---
drivers/tty/serial/fsl_lpuart.c | 43
+++++++++++++++++++++++++++----------
----
1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c

b/drivers/tty/serial/fsl_lpuart.c index
ed9db2f..bbf47a0 100644

--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c

@@ -280,15 +280,29 @@ MODULE_DEVICE_TABLE(of, lpuart_dt_ids);

/* Forward declare this for the dma callbacks*/  static void
lpuart_dma_tx_complete(void *arg);

-static inline u32 lpuart32_read(struct uart_port *port, u32 reg_off) -{
-	return ioread32be(port->membase + reg_off);
+static inline u32 lpuart32_read(struct uart_port *port, u32 off) {
+	switch (port->iotype) {
+	case UPIO_MEM32:
+		return readl(port->membase + off);
+	case UPIO_MEM32BE:
+		return ioread32be(port->membase + off);
+	default:
+		return 0;
+	};
}

static inline void lpuart32_write(struct uart_port *port, u32 val,
-				  u32 reg_off)
+				  u32 off)
{
-	iowrite32be(val, port->membase + reg_off);
+	switch (port->iotype) {
+	case UPIO_MEM32:
+		writel(val, port->membase + off);
+		break;
+	case UPIO_MEM32BE:
+		iowrite32be(val, port->membase + off);
+		break;
+	};
}

static void lpuart_stop_tx(struct uart_port *port) @@ -602,7 +616,7 @@
static irqreturn_t lpuart_txint(int irq, void *dev_id)

	spin_lock_irqsave(&sport->port.lock, flags);
	if (sport->port.x_char) {
-		if (sport->port.iotype & UPIO_MEM32BE)
+		if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
			lpuart32_write(&sport->port, sport->port.x_char,

UARTDATA);

quoted

		else
			writeb(sport->port.x_char, sport->port.membase + UARTDR);

@@

quoted

-610,14 +624,14 @@ static irqreturn_t lpuart_txint(int irq, void
*dev_id)
	}

	if (uart_circ_empty(xmit) || uart_tx_stopped(&sport->port)) {
-		if (sport->port.iotype & UPIO_MEM32BE)
+		if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))

Can use one macro instead of sport->port.iotype & (UPIO_MEM32 |
UPIO_MEM32BE ?

UPIO_MEM32 and UPIO_MEM32BE are serial core definitions.
If we use one macro for it, then the macro seems be better in serial core.
But I don't think it's quite necessary.

Explicit using also make the code look clearer.

Regards
Dong Aisheng

quoted

			lpuart32_stop_tx(&sport->port);
		else
			lpuart_stop_tx(&sport->port);
		goto out;
	}

-	if (sport->port.iotype & UPIO_MEM32BE)
+	if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
		lpuart32_transmit_buffer(sport);
	else
		lpuart_transmit_buffer(sport);

@@ -1890,12 +1904,12 @@ static int __init lpuart_console_setup(struct

console *co, char *options)
	if (options)
		uart_parse_options(options, &baud, &parity, &bits, &flow);
	else
-		if (sport->port.iotype & UPIO_MEM32BE)
+		if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
			lpuart32_console_get_options(sport, &baud, &parity,

&bits);

quoted

		else
			lpuart_console_get_options(sport, &baud, &parity, &bits);

-	if (sport->port.iotype & UPIO_MEM32BE)
+	if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
		lpuart32_setup_watermark(sport);
	else
		lpuart_setup_watermark(sport);

@@ -1954,6 +1968,7 @@ static int __init

lpuart32_early_console_setup(struct
earlycon_device *device,
	if (!device->port.membase)
		return -ENODEV;

+	device->port.iotype = UPIO_MEM32BE;
	device->con->write = lpuart32_early_write;
	return 0;
}

@@ -2015,7 +2030,7 @@ static int lpuart_probe(struct platform_device

*pdev)

quoted

	}
	sport->port.irq = ret;
	sport->port.iotype = sdata->iotype;
-	if (sport->port.iotype & UPIO_MEM32BE)
+	if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
		sport->port.ops = &lpuart32_pops;
	else
		sport->port.ops = &lpuart_pops;

@@ -2042,7 +2057,7 @@ static int lpuart_probe(struct platform_device

*pdev)

	platform_set_drvdata(pdev, &sport->port);

-	if (sport->port.iotype & UPIO_MEM32BE)
+	if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
		lpuart_reg.cons = LPUART32_CONSOLE;
	else
		lpuart_reg.cons = LPUART_CONSOLE;

@@ -2095,7 +2110,7 @@ static int lpuart_suspend(struct device *dev)

	struct lpuart_port *sport = dev_get_drvdata(dev);
	unsigned long temp;

-	if (sport->port.iotype & UPIO_MEM32BE) {
+	if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE)) {
		/* disable Rx/Tx and interrupts */
		temp = lpuart32_read(&sport->port, UARTCTRL);
		temp &= ~(UARTCTRL_TE | UARTCTRL_TIE | UARTCTRL_TCIE); @@ -

2146,7

quoted

+2161,7 @@ static int lpuart_resume(struct device *dev)
	if (sport->port.suspended && !sport->port.irq_wake)
		clk_prepare_enable(sport->clk);

-	if (sport->port.iotype & UPIO_MEM32BE) {
+	if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE)) {
		lpuart32_setup_watermark(sport);
		temp = lpuart32_read(&sport->port, UARTCTRL);
		temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE |
--
2.7.4