AMLogic Meson serial driver fixes

AMLogic Meson serial driver fixes

[Ben Dooks]

Whilst testing the 4.3 kernel on an AMlogic AM805 (Odroid-C1) with
a systemd based system, the console has issues with missing parts
of the output.

This is a set of fixes, and a few cleanups which makes the output
when the console is set to ttyAML0 usable.

[PATCH 1/9] ARM: meson: serial: release region on port release

[Ben Dooks]

The meson_uart_release_port() unmaps the register area but does not release
it. The meson_uart_request_port() calls devm_request_mem_region so the
release should call devm_release_mem_region() for that area so that anyt
subsequent use of these calls will work.

This fixes an issue where the addition of reset code before registering
the uart stops the console from working.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/tty/serial/meson_uart.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 0fc83c9..b9f0829 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -367,9 +367,26 @@ static int meson_uart_verify_port(struct uart_port *port,
 	return ret;
 }
 
+static int meson_uart_res_size(struct uart_port *port)
+{
+	struct platform_device *pdev = to_platform_device(port->dev);
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(port->dev, "cannot obtain I/O memory region");
+		return -ENODEV;
+	}
+
+	return resource_size(res);
+}
+
 static void meson_uart_release_port(struct uart_port *port)
 {
+	int size = meson_uart_res_size(port);
+
 	if (port->flags & UPF_IOREMAP) {
+		devm_release_mem_region(port->dev, port->mapbase, size);
 		devm_iounmap(port->dev, port->membase);
 		port->membase = NULL;
 	}
@@ -377,16 +394,10 @@ static void meson_uart_release_port(struct uart_port *port)
 
 static int meson_uart_request_port(struct uart_port *port)
 {
-	struct platform_device *pdev = to_platform_device(port->dev);
-	struct resource *res;
-	int size;
+	int size = meson_uart_res_size(port);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "cannot obtain I/O memory region");
-		return -ENODEV;
-	}
-	size = resource_size(res);
+	if (size < 0)
+		return size;
 
 	if (!devm_request_mem_region(port->dev, port->mapbase, size,
 				     dev_name(port->dev))) {
-- 
2.6.2

[PATCH 2/9] ARM: meson: serial: don't reset port on uart startup

[Ben Dooks]

When the uart startup entry is called, do not reset the port as this
could cause issues with anything left in the FIFO from a previous operation
such as a console write. Move the hardware reset to probe time and simply
clear the errors before enabling the port.

This fixes the issue where the console could become corrupted as there
where characters left in the output or output fifo when a user process
such as systemd would open/close the uart to transmit characters.

For example, you get:
    [    3.252263] systemd[1]: Dete

instead of:
    [    3.338801] systemd[1]: Detected architecture arm.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/tty/serial/meson_uart.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index b9f0829..b87eb97 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -241,10 +241,9 @@ static const char *meson_uart_type(struct uart_port *port)
 	return (port->type == PORT_MESON) ? "meson_uart" : NULL;
 }
 
-static int meson_uart_startup(struct uart_port *port)
+static void meson_uart_reset(struct uart_port *port)
 {
 	u32 val;
-	int ret = 0;
 
 	val = readl(port->membase + AML_UART_CONTROL);
 	val |= (AML_UART_RX_RST | AML_UART_TX_RST | AML_UART_CLR_ERR);
@@ -252,6 +251,18 @@ static int meson_uart_startup(struct uart_port *port)
 
 	val &= ~(AML_UART_RX_RST | AML_UART_TX_RST | AML_UART_CLR_ERR);
 	writel(val, port->membase + AML_UART_CONTROL);
+}
+
+static int meson_uart_startup(struct uart_port *port)
+{
+	u32 val;
+	int ret = 0;
+
+	val = readl(port->membase + AML_UART_CONTROL);
+	val |= AML_UART_CLR_ERR;
+	writel(val, port->membase + AML_UART_CONTROL);
+	val &= ~AML_UART_CLR_ERR;
+	writel(val, port->membase + AML_UART_CONTROL);
 
 	val |= (AML_UART_RX_EN | AML_UART_TX_EN);
 	writel(val, port->membase + AML_UART_CONTROL);
@@ -581,6 +592,12 @@ static int meson_uart_probe(struct platform_device *pdev)
 	meson_ports[pdev->id] = port;
 	platform_set_drvdata(pdev, port);
 
+	/* reset port before registering (and possibly registering console) */
+	if (meson_uart_request_port(port) >= 0) {
+		meson_uart_reset(port);
+		meson_uart_release_port(port);
+	}
+
 	ret = uart_add_one_port(&meson_uart_driver, port);
 	if (ret)
 		meson_ports[pdev->id] = NULL;
-- 
2.6.2

[PATCH 3/9] ARM: meson: serial: tx_empty fails to check for transmitter busy

[Ben Dooks]

The tx_empty() uart_op should only return empty if both the transmit fifo
and the transmit state-machine are both idle. Add a test for the hardware's
XMIT_BUSY flag.

Note, this is possibly related to an issue where the port is being shutdown
with paritally transmitted characters in it.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reported-by: Edward Cragg <edward.cragg@codethink.co.uk>
---
 drivers/tty/serial/meson_uart.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index b87eb97..54d1b95 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -57,6 +57,7 @@
 #define AML_UART_RX_EMPTY		BIT(20)
 #define AML_UART_TX_FULL		BIT(21)
 #define AML_UART_TX_EMPTY		BIT(22)
+#define AML_UART_XMIT_BUSY		BIT(25)
 #define AML_UART_ERR			(AML_UART_PARITY_ERR | \
 					 AML_UART_FRAME_ERR  | \
 					 AML_UART_TX_FIFO_WERR)
@@ -100,7 +101,8 @@ static unsigned int meson_uart_tx_empty(struct uart_port *port)
 	u32 val;
 
 	val = readl(port->membase + AML_UART_STATUS);
-	return (val & AML_UART_TX_EMPTY) ? TIOCSER_TEMT : 0;
+	val &= (AML_UART_TX_EMPTY | AML_UART_XMIT_BUSY);
+	return (val == AML_UART_TX_EMPTY) ? TIOCSER_TEMT : 0;
 }
 
 static void meson_uart_stop_tx(struct uart_port *port)
-- 
2.6.2

[PATCH 4/9] ARM: meson: serial: ensure console port uart enabled

[Ben Dooks]

Ensure the UART's transmitter is enabled when meson_console_putchar is
called. If not, then the console output is corrupt (the hardware seems
to try and send /something/ even if the TX is disabled).

This fixes corrupt console output on events such as trying to reboot the
system since the console tx may be called after drivers shutdown method has
been called.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reported-by: Edward Cragg <edward.cragg@codethink.co.uk>
--
---
 drivers/tty/serial/meson_uart.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 54d1b95..c7bad2b 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -472,6 +472,7 @@ static void meson_serial_console_write(struct console *co, const char *s,
 	struct uart_port *port;
 	unsigned long flags;
 	int locked;
+	u32 val;
 
 	port = meson_ports[co->index];
 	if (!port)
@@ -487,6 +488,9 @@ static void meson_serial_console_write(struct console *co, const char *s,
 		locked = 1;
 	}
 
+	val = readl(port->membase + AML_UART_CONTROL);
+	writel(val | AML_UART_TX_EN, port->membase + AML_UART_CONTROL);
+
 	uart_console_write(port, s, count, meson_console_putchar);
 
 	if (locked)
-- 
2.6.2

[PATCH 5/9] ARM: meson: serial: only disable tx irq on stop

[Ben Dooks]

Since disabling the transmit state machine still allows characters to
be transmitted when written to the UART write FIFO, simply disable the
transmit interrupt when the UART port is stopped.

This has not shown an improvement with the console issues when running
systemd, but seems like it should be done.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reported-by: Edward Cragg <ed.cragg@codethink.co.uk>
---
---
 drivers/tty/serial/meson_uart.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index c7bad2b..9327efd 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -110,7 +110,7 @@ static void meson_uart_stop_tx(struct uart_port *port)
 	u32 val;
 
 	val = readl(port->membase + AML_UART_CONTROL);
-	val &= ~AML_UART_TX_EN;
+	val &= ~AML_UART_TX_INT_EN;
 	writel(val, port->membase + AML_UART_CONTROL);
 }
 
@@ -133,7 +133,7 @@ static void meson_uart_shutdown(struct uart_port *port)
 	spin_lock_irqsave(&port->lock, flags);
 
 	val = readl(port->membase + AML_UART_CONTROL);
-	val &= ~(AML_UART_RX_EN | AML_UART_TX_EN);
+	val &= ~AML_UART_RX_EN;
 	val &= ~(AML_UART_RX_INT_EN | AML_UART_TX_INT_EN);
 	writel(val, port->membase + AML_UART_CONTROL);
 
-- 
2.6.2

[PATCH 6/9] ARM: meson: serial: use meson_uart_tx_empty() to wait for empty

[Ben Dooks]

Use the meson_uart_tx_empty() instead of a direct read of the status
register. This is easier to read and will ensure the UART's transmit
state machine is idle when trying to update the baud rate.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/tty/serial/meson_uart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 9327efd..d3f2c96 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -285,7 +285,7 @@ static void meson_uart_change_speed(struct uart_port *port, unsigned long baud)
 {
 	u32 val;
 
-	while (!(readl(port->membase + AML_UART_STATUS) & AML_UART_TX_EMPTY))
+	while (!meson_uart_tx_empty(port))
 		cpu_relax();
 
 	val = readl(port->membase + AML_UART_REG5);
-- 
2.6.2

[PATCH 7/9] ARM: meson: serial: disable rx/tx irqs during console write

[Ben Dooks]

As an attempt to stop issues with bad console output, ensure that both the
rx and tx interrupts are disabled during the console write to avoid any
problems with console and non-console being called together.

This should help with the SMP case as it should stop other cores being
signalled during the console write.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/tty/serial/meson_uart.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index d3f2c96..12436cc 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -472,7 +472,7 @@ static void meson_serial_console_write(struct console *co, const char *s,
 	struct uart_port *port;
 	unsigned long flags;
 	int locked;
-	u32 val;
+	u32 val, tmp;
 
 	port = meson_ports[co->index];
 	if (!port)
@@ -489,9 +489,12 @@ static void meson_serial_console_write(struct console *co, const char *s,
 	}
 
 	val = readl(port->membase + AML_UART_CONTROL);
-	writel(val | AML_UART_TX_EN, port->membase + AML_UART_CONTROL);
+	val |= AML_UART_TX_EN;
+	tmp = val & ~(AML_UART_TX_INT_EN | AML_UART_RX_INT_EN);
+	writel(tmp, port->membase + AML_UART_CONTROL);
 
 	uart_console_write(port, s, count, meson_console_putchar);
+	writel(val, port->membase + AML_UART_CONTROL);
 
 	if (locked)
 		spin_unlock(&port->lock);
-- 
2.6.2

[PATCH 8/9] ARM: meson: serial: ensure tx irq on if more work to do

[Ben Dooks]

The tx_stop() call turns the interrupt off, but the tx_start() does not
check if the interrupt is enabled. Switch it back on if there is more
work to do.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/tty/serial/meson_uart.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 12436cc..6c36526 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -144,6 +144,7 @@ static void meson_uart_start_tx(struct uart_port *port)
 {
 	struct circ_buf *xmit = &port->state->xmit;
 	unsigned int ch;
+	u32 val;
 
 	if (uart_tx_stopped(port)) {
 		meson_uart_stop_tx(port);
@@ -167,6 +168,12 @@ static void meson_uart_start_tx(struct uart_port *port)
 		port->icount.tx++;
 	}
 
+	if (!uart_circ_empty(xmit)) {
+		val = readl(port->membase + AML_UART_CONTROL);
+		val |= AML_UART_TX_INT_EN;
+		writel(val, port->membase + AML_UART_CONTROL);
+	}
+
 	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
 }
-- 
2.6.2

[PATCH 9/9] ARM: meson: serial: check for tx-irq enabled in irq code

[Ben Dooks]

Ensure that if the interrupt handler is entered then only try and do tx
work if the tx irq is enabled.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/tty/serial/meson_uart.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 6c36526..b12a37b 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -237,8 +237,10 @@ static irqreturn_t meson_uart_interrupt(int irq, void *dev_id)
 	if (!(readl(port->membase + AML_UART_STATUS) & AML_UART_RX_EMPTY))
 		meson_receive_chars(port);
 
-	if (!(readl(port->membase + AML_UART_STATUS) & AML_UART_TX_FULL))
-		meson_uart_start_tx(port);
+	if (!(readl(port->membase + AML_UART_STATUS) & AML_UART_TX_FULL)) {
+		if (readl(port->membase + AML_UART_CONTROL) & AML_UART_TX_INT_EN)
+			meson_uart_start_tx(port);
+	}
 
 	spin_unlock(&port->lock);
 
-- 
2.6.2

Re: [linux-meson] AMLogic Meson serial driver fixes

[Carlo Caione]

On Wed, Nov 18, 2015 at 3:41 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> Whilst testing the 4.3 kernel on an AMlogic AM805 (Odroid-C1) with
> a systemd based system, the console has issues with missing parts
> of the output.
>
> This is a set of fixes, and a few cleanups which makes the output
> when the console is set to ttyAML0 usable.
>

nit: next time use '--cover-letter' to generate the cover.

For whole patchset:

Tested-by: Carlo Caione <carlo@endlessm.com>

Thanks!

-- 
Carlo Caione