Skip to contentSkip to conversation

serial: meson: series with smaller fixes and improvements

PATCH New Heiner Kallweit 2 participants 8 messages Apr 19, 2017 – Apr 27, 2017

Subsystems (2)
Arm/Amlogic Meson SoC support TTY layer and serial drivers
lore
Share & tools
lore

Heiner Kallweit

author
#0 lore raw

v2 cover serial: meson: series with smaller fixes and improvements

This series includes a few smaller fixes and improvements for the meson uart driver.

Changes in v2:
- change patch 3 based on review discussion
- change patch 6 to use port->name instead

Heiner Kallweit (6):
serial: meson: fix setting number of stop bits
serial: meson: remove dead code in meson_uart_change_speed
serial: meson: remove unneeded variable assignment in meson_serial_port_write
serial: meson: make use of uart_port member mapsize
serial: meson: remove use of flag UPF_IOREMAP
serial: meson: change interrupt description to of node name

 drivers/tty/serial/meson_uart.c | 68 ++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 42 deletions(-)
Signature · 1 line
2.12.2
author
#1 lore raw

PATCH v2 1/6 serial: meson: fix setting number of stop bits

The stop bit value as to be or'ed, so far this worked only just by chance because AML_UART_STOP_BIN_1SB is 0.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- no changes

---
 drivers/tty/serial/meson_uart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/meson_uart.c +1 -1
index 60f16795..e2e25da1 100644
@@ -355,7 +355,7 @@ static void meson_uart_set_termios(struct uart_port *port,
 	if (cflags & CSTOPB)
 		val |= AML_UART_STOP_BIN_2SB;
 	else
-		val &= ~AML_UART_STOP_BIN_1SB;
+		val |= AML_UART_STOP_BIN_1SB;
 
 	if (cflags & CRTSCTS)
 		val &= ~AML_UART_TWO_WIRE_EN;
Signature · 1 line
2.12.2
author
#2 lore raw

PATCH v2 2/6 serial: meson: remove dead code in meson_uart_change_speed

In reply to Heiner Kallweit

val is set in both branches of the if clause, therefore the two removed lines are dead code.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- no changes

---
 drivers/tty/serial/meson_uart.c | 2 --
 1 file changed, 2 deletions(-)
drivers/tty/serial/meson_uart.c +0 -2
index e2e25da1..22857f1e 100644
@@ -298,8 +298,6 @@ static void meson_uart_change_speed(struct uart_port *port, unsigned long baud)
 	while (!meson_uart_tx_empty(port))
 		cpu_relax();
 
-	val = readl(port->membase + AML_UART_REG5);
-	val &= ~AML_UART_BAUD_MASK;
 	if (port->uartclk == 24000000) {
 		val = ((port->uartclk / 3) / baud) - 1;
 		val |= AML_UART_BAUD_XTAL;
Signature · 1 line
2.12.2
author
#3 lore raw

PATCH v2 3/6 serial: meson: remove unneeded variable assignment in meson_serial_port_write

In reply to Heiner Kallweit

There's no need to set AML_UART_TX_EN in each call to meson_serial_port_write. In addition to meson_uart_startup set this flag in meson_serial_console_setup and meson_serial_early_console_setup.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- enable AML_UART_TX_EN in both console setup functions

---
 drivers/tty/serial/meson_uart.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/tty/serial/meson_uart.c +12 -1
index 22857f1e..e93a1e47 100644
@@ -124,6 +124,15 @@ static void meson_uart_stop_rx(struct uart_port *port)
 	writel(val, port->membase + AML_UART_CONTROL);
 }
 
+static void meson_uart_enable_tx_engine(struct uart_port *port)
+{
+	u32 val;
+
+	val = readl(port->membase + AML_UART_CONTROL);
+	val |= AML_UART_TX_EN;
+	writel(val, port->membase + AML_UART_CONTROL);
+}
+
 static void meson_uart_shutdown(struct uart_port *port)
 {
 	unsigned long flags;

@@ -497,7 +506,6 @@ static void meson_serial_port_write(struct uart_port *port, const char *s,
 	}
 
 	val = readl(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);
 

@@ -536,6 +544,8 @@ static int meson_serial_console_setup(struct console *co, char *options)
 	if (!port || !port->membase)
 		return -ENODEV;
 
+	meson_uart_enable_tx_engine(port);
+
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
 

@@ -574,6 +584,7 @@ meson_serial_early_console_setup(struct earlycon_device *device, const char *opt
 	if (!device->port.membase)
 		return -ENODEV;
 
+	meson_uart_enable_tx_engine(&device->port);
 	device->con->write = meson_serial_early_console_write;
 	return 0;
 }
Signature · 1 line
2.12.2
author
#4 lore raw

PATCH v2 4/6 serial: meson: make use of uart_port member mapsize

In reply to Heiner Kallweit

Member mapsize of struct uart_port is meant to store the resource size. By using it we can get rid of meson_uart_res_size().

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- no changes

---
 drivers/tty/serial/meson_uart.c | 29 +++++------------------------
 1 file changed, 5 insertions(+), 24 deletions(-)
drivers/tty/serial/meson_uart.c +5 -24
index e93a1e47..1220c9ac 100644
@@ -402,26 +402,11 @@ 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_release_mem_region(port->dev, port->mapbase,
+					port->mapsize);
 		devm_iounmap(port->dev, port->membase);
 		port->membase = NULL;
 	}

@@ -429,12 +414,7 @@ static void meson_uart_release_port(struct uart_port *port)
 
 static int meson_uart_request_port(struct uart_port *port)
 {
-	int size = meson_uart_res_size(port);
-
-	if (size < 0)
-		return size;
-
-	if (!devm_request_mem_region(port->dev, port->mapbase, size,
+	if (!devm_request_mem_region(port->dev, port->mapbase, port->mapsize,
 				     dev_name(port->dev))) {
 		dev_err(port->dev, "Memory region busy\n");
 		return -EBUSY;

@@ -443,7 +423,7 @@ static int meson_uart_request_port(struct uart_port *port)
 	if (port->flags & UPF_IOREMAP) {
 		port->membase = devm_ioremap_nocache(port->dev,
 						     port->mapbase,
-						     size);
+						     port->mapsize);
 		if (port->membase == NULL)
 			return -ENOMEM;
 	}

@@ -641,6 +621,7 @@ static int meson_uart_probe(struct platform_device *pdev)
 	port->uartclk = clk_get_rate(clk);
 	port->iotype = UPIO_MEM;
 	port->mapbase = res_mem->start;
+	port->mapsize = resource_size(res_mem);
 	port->irq = res_irq->start;
 	port->flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP | UPF_LOW_LATENCY;
 	port->dev = &pdev->dev;
Signature · 1 line
2.12.2
author
#5 lore raw

PATCH v2 5/6 serial: meson: remove use of flag UPF_IOREMAP

In reply to Heiner Kallweit

Flag UPF_IOREMAP is used by the 8250 subsystem only, it's not used by the serial core. Therefore I don't see any benefit in using it here.

In addition fix the order of calls in meson_uart_release_port. Unmapping needs to be done first, reversing call order in meson_uart_request_port.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- no changes

---
 drivers/tty/serial/meson_uart.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)
drivers/tty/serial/meson_uart.c +8 -14
index 1220c9ac..171eb673 100644
@@ -404,12 +404,9 @@ static int meson_uart_verify_port(struct uart_port *port,
 
 static void meson_uart_release_port(struct uart_port *port)
 {
-	if (port->flags & UPF_IOREMAP) {
-		devm_release_mem_region(port->dev, port->mapbase,
-					port->mapsize);
-		devm_iounmap(port->dev, port->membase);
-		port->membase = NULL;
-	}
+	devm_iounmap(port->dev, port->membase);
+	port->membase = NULL;
+	devm_release_mem_region(port->dev, port->mapbase, port->mapsize);
 }
 
 static int meson_uart_request_port(struct uart_port *port)

@@ -420,13 +417,10 @@ static int meson_uart_request_port(struct uart_port *port)
 		return -EBUSY;
 	}
 
-	if (port->flags & UPF_IOREMAP) {
-		port->membase = devm_ioremap_nocache(port->dev,
-						     port->mapbase,
-						     port->mapsize);
-		if (port->membase == NULL)
-			return -ENOMEM;
-	}
+	port->membase = devm_ioremap_nocache(port->dev, port->mapbase,
+					     port->mapsize);
+	if (!port->membase)
+		return -ENOMEM;
 
 	return 0;
 }

@@ -623,7 +617,7 @@ static int meson_uart_probe(struct platform_device *pdev)
 	port->mapbase = res_mem->start;
 	port->mapsize = resource_size(res_mem);
 	port->irq = res_irq->start;
-	port->flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP | UPF_LOW_LATENCY;
+	port->flags = UPF_BOOT_AUTOCONF | UPF_LOW_LATENCY;
 	port->dev = &pdev->dev;
 	port->line = pdev->id;
 	port->type = PORT_MESON;
Signature · 1 line
2.12.2
author
#6 lore raw

PATCH v2 6/6 serial: meson: change interrupt description to tty name

In reply to Heiner Kallweit

Change interrupt description from driver name to tty name (e.g. ttyAML0). If multiple serial ports are enabled this allows to determine which interrupt belongs to which port in /proc/interrupts.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- use new port->name instead of of node name

---
 drivers/tty/serial/meson_uart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/meson_uart.c +1 -1
index 171eb673..082e038e 100644
@@ -295,7 +295,7 @@ static int meson_uart_startup(struct uart_port *port)
 	writel(val, port->membase + AML_UART_MISC);
 
 	ret = request_irq(port->irq, meson_uart_interrupt, 0,
-			  meson_uart_type(port), port);
+			  port->name, port);
 
 	return ret;
 }
Signature · 1 line
2.12.2
#7 lore raw

PATCH v2 6/6 serial: meson: change interrupt description to tty name

Quoted 35 lines · Heiner Kallweit

Change interrupt description from driver name to tty name (e.g. ttyAML0). If multiple serial ports are enabled this allows to determine which interrupt belongs to which port in /proc/interrupts.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- use new port->name instead of of node name

---
 drivers/tty/serial/meson_uart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/meson_uart.c +1 -1
index 171eb673..082e038e 100644
@@ -295,7 +295,7 @@ static int meson_uart_startup(struct uart_port *port)
 	writel(val, port->membase + AML_UART_MISC);
 
 	ret = request_irq(port->irq, meson_uart_interrupt, 0,
-			  meson_uart_type(port), port);
+			  port->name, port);
 
 	return ret;
 }
Signature · 8 lines
2.12.2



_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Hi Heiner Kallweit:

there is no member of 'name' in 'struct uart_port'
or did I miss something obviously?
(btw, I'm applying these patches on top of khilman's tree: branch v4.12/integ)

as I got this error:

drivers/tty/serial/meson_uart.c: In function ?meson_uart_startup?:
drivers/tty/serial/meson_uart.c:298:10: error: ?struct uart_port? has no member named ?name?

      port->name, port);                         
          ^~

make[3]: *** [scripts/Makefile.build:294: drivers/tty/serial/meson_uart.o] Error 1 make[3]: *** Waiting for unfinished jobs....

Signature · 3 lines
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55