* [PATCH 0/6] serial: meson: series with smaller fixes and improvements
@ 2017-04-16 20:02 Heiner Kallweit
2017-04-16 20:06 ` [PATCH 1/6] serial: meson: fix setting number of stop bits Heiner Kallweit
` (5 more replies)
0 siblings, 6 replies; 22+ messages in thread
From: Heiner Kallweit @ 2017-04-16 20:02 UTC (permalink / raw)
To: linus-amlogic
This series includes a few smaller fixes and improvements for the
meson uart driver.
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 | 52 ++++++++++-------------------------------
1 file changed, 12 insertions(+), 40 deletions(-)
--
2.12.2
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/6] serial: meson: fix setting number of stop bits
2017-04-16 20:02 [PATCH 0/6] serial: meson: series with smaller fixes and improvements Heiner Kallweit
@ 2017-04-16 20:06 ` Heiner Kallweit
2017-04-17 15:24 ` Neil Armstrong
2017-04-16 20:10 ` [PATCH 2/6] serial: meson: remove dead code in meson_uart_change_speed Heiner Kallweit
` (4 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Heiner Kallweit @ 2017-04-16 20:06 UTC (permalink / raw)
To: linus-amlogic
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 <hkallweit1@gmail.com>
---
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 60f16795..e2e25da1 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -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;
--
2.12.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/6] serial: meson: remove dead code in meson_uart_change_speed
2017-04-16 20:02 [PATCH 0/6] serial: meson: series with smaller fixes and improvements Heiner Kallweit
2017-04-16 20:06 ` [PATCH 1/6] serial: meson: fix setting number of stop bits Heiner Kallweit
@ 2017-04-16 20:10 ` Heiner Kallweit
2017-04-17 15:25 ` Neil Armstrong
2017-04-16 20:13 ` [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write Heiner Kallweit
` (3 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Heiner Kallweit @ 2017-04-16 20:10 UTC (permalink / raw)
To: linus-amlogic
val is set in both branches of the if clause, therefore the two
removed lines are dead code.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/tty/serial/meson_uart.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index e2e25da1..22857f1e 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -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;
--
2.12.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write
2017-04-16 20:02 [PATCH 0/6] serial: meson: series with smaller fixes and improvements Heiner Kallweit
2017-04-16 20:06 ` [PATCH 1/6] serial: meson: fix setting number of stop bits Heiner Kallweit
2017-04-16 20:10 ` [PATCH 2/6] serial: meson: remove dead code in meson_uart_change_speed Heiner Kallweit
@ 2017-04-16 20:13 ` Heiner Kallweit
2017-04-17 13:58 ` Ben Dooks
2017-04-16 20:15 ` [PATCH 4/6] serial: meson: make use of uart_port member mapsize Heiner Kallweit
` (2 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Heiner Kallweit @ 2017-04-16 20:13 UTC (permalink / raw)
To: linus-amlogic
AML_UART_TX_EN is set in meson_uart_startup and there's no place in
the driver where it gets cleared. Therefore we don't have to set it
here.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/tty/serial/meson_uart.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 22857f1e..714b29ad 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -497,7 +497,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);
--
2.12.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/6] serial: meson: make use of uart_port member mapsize
2017-04-16 20:02 [PATCH 0/6] serial: meson: series with smaller fixes and improvements Heiner Kallweit
` (2 preceding siblings ...)
2017-04-16 20:13 ` [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write Heiner Kallweit
@ 2017-04-16 20:15 ` Heiner Kallweit
2017-04-17 13:53 ` Ben Dooks
2017-04-17 15:28 ` Neil Armstrong
2017-04-16 20:21 ` [PATCH 5/6] serial: meson: remove use of flag UPF_IOREMAP Heiner Kallweit
2017-04-16 20:23 ` [PATCH 6/6] serial: meson: change interrupt description to of node name Heiner Kallweit
5 siblings, 2 replies; 22+ messages in thread
From: Heiner Kallweit @ 2017-04-16 20:15 UTC (permalink / raw)
To: linus-amlogic
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 <hkallweit1@gmail.com>
---
drivers/tty/serial/meson_uart.c | 29 +++++------------------------
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 714b29ad..8d2e7203 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -393,26 +393,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;
}
@@ -420,12 +405,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;
@@ -434,7 +414,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;
}
@@ -629,6 +609,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;
--
2.12.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 5/6] serial: meson: remove use of flag UPF_IOREMAP
2017-04-16 20:02 [PATCH 0/6] serial: meson: series with smaller fixes and improvements Heiner Kallweit
` (3 preceding siblings ...)
2017-04-16 20:15 ` [PATCH 4/6] serial: meson: make use of uart_port member mapsize Heiner Kallweit
@ 2017-04-16 20:21 ` Heiner Kallweit
2017-04-17 15:28 ` Neil Armstrong
2017-04-16 20:23 ` [PATCH 6/6] serial: meson: change interrupt description to of node name Heiner Kallweit
5 siblings, 1 reply; 22+ messages in thread
From: Heiner Kallweit @ 2017-04-16 20:21 UTC (permalink / raw)
To: linus-amlogic
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 <hkallweit1@gmail.com>
---
drivers/tty/serial/meson_uart.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 8d2e7203..c8b626e3 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -395,12 +395,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)
@@ -411,13 +408,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;
}
@@ -611,7 +605,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;
--
2.12.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 6/6] serial: meson: change interrupt description to of node name
2017-04-16 20:02 [PATCH 0/6] serial: meson: series with smaller fixes and improvements Heiner Kallweit
` (4 preceding siblings ...)
2017-04-16 20:21 ` [PATCH 5/6] serial: meson: remove use of flag UPF_IOREMAP Heiner Kallweit
@ 2017-04-16 20:23 ` Heiner Kallweit
2017-04-17 13:52 ` Ben Dooks
2017-04-17 15:29 ` Neil Armstrong
5 siblings, 2 replies; 22+ messages in thread
From: Heiner Kallweit @ 2017-04-16 20:23 UTC (permalink / raw)
To: linus-amlogic
Change interrupt description from driver name to of node name.
If multiple serial ports are enabled this allows to determine
which interrupt belongs to which port in /proc/interrupts.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
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 c8b626e3..d0682231 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -286,7 +286,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);
+ dev_name(port->dev), port);
return ret;
}
--
2.12.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 6/6] serial: meson: change interrupt description to of node name
2017-04-16 20:23 ` [PATCH 6/6] serial: meson: change interrupt description to of node name Heiner Kallweit
@ 2017-04-17 13:52 ` Ben Dooks
2017-04-17 15:29 ` Neil Armstrong
1 sibling, 0 replies; 22+ messages in thread
From: Ben Dooks @ 2017-04-17 13:52 UTC (permalink / raw)
To: linus-amlogic
On 16/04/17 21:23, Heiner Kallweit wrote:
> Change interrupt description from driver name to of node name.
> If multiple serial ports are enabled this allows to determine
> which interrupt belongs to which port in /proc/interrupts.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> 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 c8b626e3..d0682231 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -286,7 +286,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);
> + dev_name(port->dev), port);
>
> return ret;
> }
Good idea.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/6] serial: meson: make use of uart_port member mapsize
2017-04-16 20:15 ` [PATCH 4/6] serial: meson: make use of uart_port member mapsize Heiner Kallweit
@ 2017-04-17 13:53 ` Ben Dooks
2017-04-17 15:28 ` Neil Armstrong
1 sibling, 0 replies; 22+ messages in thread
From: Ben Dooks @ 2017-04-17 13:53 UTC (permalink / raw)
To: linus-amlogic
On 16/04/17 21:15, Heiner Kallweit wrote:
> 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 <hkallweit1@gmail.com>
> ---
> drivers/tty/serial/meson_uart.c | 29 +++++------------------------
> 1 file changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 714b29ad..8d2e7203 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -393,26 +393,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;
> }
> @@ -420,12 +405,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;
> @@ -434,7 +414,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;
> }
> @@ -629,6 +609,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;
Looks good.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write
2017-04-16 20:13 ` [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write Heiner Kallweit
@ 2017-04-17 13:58 ` Ben Dooks
2017-04-17 15:27 ` Neil Armstrong
0 siblings, 1 reply; 22+ messages in thread
From: Ben Dooks @ 2017-04-17 13:58 UTC (permalink / raw)
To: linus-amlogic
On 16/04/17 21:13, Heiner Kallweit wrote:
> AML_UART_TX_EN is set in meson_uart_startup and there's no place in
> the driver where it gets cleared. Therefore we don't have to set it
> here.
I think this is a leftover of the patches I did when trying to fix
the UART driver when using debian/systemd and having strange console
problems.
The only comment is it might be worth leaving this in as it isn't
a high cost to the driver and it is possible someone might accidentally
clear it later.
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/tty/serial/meson_uart.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 22857f1e..714b29ad 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -497,7 +497,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);
>
>
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/6] serial: meson: fix setting number of stop bits
2017-04-16 20:06 ` [PATCH 1/6] serial: meson: fix setting number of stop bits Heiner Kallweit
@ 2017-04-17 15:24 ` Neil Armstrong
2017-04-17 15:54 ` Heiner Kallweit
0 siblings, 1 reply; 22+ messages in thread
From: Neil Armstrong @ 2017-04-17 15:24 UTC (permalink / raw)
To: linus-amlogic
On 04/16/2017 10:06 PM, Heiner Kallweit wrote:
> 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 <hkallweit1@gmail.com>
> ---
> 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 60f16795..e2e25da1 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -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;
>
Hi,
in fact you can totally drop the else statement since it's a no-op in both cases.
Neil
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/6] serial: meson: remove dead code in meson_uart_change_speed
2017-04-16 20:10 ` [PATCH 2/6] serial: meson: remove dead code in meson_uart_change_speed Heiner Kallweit
@ 2017-04-17 15:25 ` Neil Armstrong
2017-04-17 15:54 ` Andreas Färber
0 siblings, 1 reply; 22+ messages in thread
From: Neil Armstrong @ 2017-04-17 15:25 UTC (permalink / raw)
To: linus-amlogic
On 04/16/2017 10:10 PM, Heiner Kallweit wrote:
> val is set in both branches of the if clause, therefore the two
> removed lines are dead code.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/tty/serial/meson_uart.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index e2e25da1..22857f1e 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -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;
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write
2017-04-17 13:58 ` Ben Dooks
@ 2017-04-17 15:27 ` Neil Armstrong
2017-04-17 15:37 ` Andreas Färber
0 siblings, 1 reply; 22+ messages in thread
From: Neil Armstrong @ 2017-04-17 15:27 UTC (permalink / raw)
To: linus-amlogic
On 04/17/2017 03:58 PM, Ben Dooks wrote:
> On 16/04/17 21:13, Heiner Kallweit wrote:
>> AML_UART_TX_EN is set in meson_uart_startup and there's no place in
>> the driver where it gets cleared. Therefore we don't have to set it
>> here.
>
> I think this is a leftover of the patches I did when trying to fix
> the UART driver when using debian/systemd and having strange console
> problems.
>
> The only comment is it might be worth leaving this in as it isn't
> a high cost to the driver and it is possible someone might accidentally
> clear it later.
>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/tty/serial/meson_uart.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>> index 22857f1e..714b29ad 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -497,7 +497,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);
>>
>>
>
>
I'm ok to remove it, having it in uart_startup is enough.
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/6] serial: meson: make use of uart_port member mapsize
2017-04-16 20:15 ` [PATCH 4/6] serial: meson: make use of uart_port member mapsize Heiner Kallweit
2017-04-17 13:53 ` Ben Dooks
@ 2017-04-17 15:28 ` Neil Armstrong
1 sibling, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2017-04-17 15:28 UTC (permalink / raw)
To: linus-amlogic
On 04/16/2017 10:15 PM, Heiner Kallweit wrote:
> 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 <hkallweit1@gmail.com>
> ---
> drivers/tty/serial/meson_uart.c | 29 +++++------------------------
> 1 file changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 714b29ad..8d2e7203 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -393,26 +393,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;
> }
> @@ -420,12 +405,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;
> @@ -434,7 +414,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;
> }
> @@ -629,6 +609,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;
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 5/6] serial: meson: remove use of flag UPF_IOREMAP
2017-04-16 20:21 ` [PATCH 5/6] serial: meson: remove use of flag UPF_IOREMAP Heiner Kallweit
@ 2017-04-17 15:28 ` Neil Armstrong
0 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2017-04-17 15:28 UTC (permalink / raw)
To: linus-amlogic
On 04/16/2017 10:21 PM, Heiner Kallweit wrote:
> 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 <hkallweit1@gmail.com>
> ---
> drivers/tty/serial/meson_uart.c | 22 ++++++++--------------
> 1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 8d2e7203..c8b626e3 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -395,12 +395,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)
> @@ -411,13 +408,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;
> }
> @@ -611,7 +605,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;
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 6/6] serial: meson: change interrupt description to of node name
2017-04-16 20:23 ` [PATCH 6/6] serial: meson: change interrupt description to of node name Heiner Kallweit
2017-04-17 13:52 ` Ben Dooks
@ 2017-04-17 15:29 ` Neil Armstrong
1 sibling, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2017-04-17 15:29 UTC (permalink / raw)
To: linus-amlogic
On 04/16/2017 10:23 PM, Heiner Kallweit wrote:
> Change interrupt description from driver name to of node name.
> If multiple serial ports are enabled this allows to determine
> which interrupt belongs to which port in /proc/interrupts.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> 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 c8b626e3..d0682231 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -286,7 +286,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);
> + dev_name(port->dev), port);
>
> return ret;
> }
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write
2017-04-17 15:27 ` Neil Armstrong
@ 2017-04-17 15:37 ` Andreas Färber
2017-04-17 16:03 ` Heiner Kallweit
2017-04-19 8:39 ` Neil Armstrong
0 siblings, 2 replies; 22+ messages in thread
From: Andreas Färber @ 2017-04-17 15:37 UTC (permalink / raw)
To: linus-amlogic
Am 17.04.2017 um 17:27 schrieb Neil Armstrong:
> On 04/17/2017 03:58 PM, Ben Dooks wrote:
>> On 16/04/17 21:13, Heiner Kallweit wrote:
>>> AML_UART_TX_EN is set in meson_uart_startup and there's no place in
>>> the driver where it gets cleared. Therefore we don't have to set it
>>> here.
>>
>> I think this is a leftover of the patches I did when trying to fix
>> the UART driver when using debian/systemd and having strange console
>> problems.
>>
>> The only comment is it might be worth leaving this in as it isn't
>> a high cost to the driver and it is possible someone might accidentally
>> clear it later.
>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>> drivers/tty/serial/meson_uart.c | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>>> index 22857f1e..714b29ad 100644
>>> --- a/drivers/tty/serial/meson_uart.c
>>> +++ b/drivers/tty/serial/meson_uart.c
>>> @@ -497,7 +497,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);
>>>
>>>
>>
>>
>
> I'm ok to remove it, having it in uart_startup is enough.
>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Have you checked the earlycon code paths? That does not run through the
standard setup function, so it is dependent on what the firmware set
last. I believe I am calling into this function through some wrapper.
Since we're writing the register anyway, it shouldn't hurt to leave this in.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/6] serial: meson: fix setting number of stop bits
2017-04-17 15:24 ` Neil Armstrong
@ 2017-04-17 15:54 ` Heiner Kallweit
2017-04-21 22:03 ` Kevin Hilman
0 siblings, 1 reply; 22+ messages in thread
From: Heiner Kallweit @ 2017-04-17 15:54 UTC (permalink / raw)
To: linus-amlogic
Am 17.04.2017 um 17:24 schrieb Neil Armstrong:
> On 04/16/2017 10:06 PM, Heiner Kallweit wrote:
>> 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 <hkallweit1@gmail.com>
>> ---
>> 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 60f16795..e2e25da1 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -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;
>>
>
> Hi,
>
> in fact you can totally drop the else statement since it's a no-op in both cases.
>
That's right. However I think leaving this in makes the code better understandable.
And most likely the compiler will remove this no-op anyway.
Heiner
> Neil
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/6] serial: meson: remove dead code in meson_uart_change_speed
2017-04-17 15:25 ` Neil Armstrong
@ 2017-04-17 15:54 ` Andreas Färber
0 siblings, 0 replies; 22+ messages in thread
From: Andreas Färber @ 2017-04-17 15:54 UTC (permalink / raw)
To: linus-amlogic
Am 17.04.2017 um 17:25 schrieb Neil Armstrong:
> On 04/16/2017 10:10 PM, Heiner Kallweit wrote:
>> val is set in both branches of the if clause, therefore the two
>> removed lines are dead code.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/tty/serial/meson_uart.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>> index e2e25da1..22857f1e 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -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;
>>
>
>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
I've double-checked that this was already done before I added the
GXBB-enabling XTAL case:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/tty/serial/meson_uart.c?id=146f3808e08faabba46ea9574133a66aa4a9468d
Present since the beginning:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/tty/serial/meson_uart.c?id=ff7693d079e58fb62d735b7b8085b53fcfb74528
In general it is worth pointing out that reading a register may
sometimes be required - I hope that BayLibre have checked against the
manuals that this is not the case here? Assuming it is okay,
Reviewed-by: Andreas F?rber <afaerber@suse.de>
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write
2017-04-17 15:37 ` Andreas Färber
@ 2017-04-17 16:03 ` Heiner Kallweit
2017-04-19 8:39 ` Neil Armstrong
1 sibling, 0 replies; 22+ messages in thread
From: Heiner Kallweit @ 2017-04-17 16:03 UTC (permalink / raw)
To: linus-amlogic
Am 17.04.2017 um 17:37 schrieb Andreas F?rber:
> Am 17.04.2017 um 17:27 schrieb Neil Armstrong:
>> On 04/17/2017 03:58 PM, Ben Dooks wrote:
>>> On 16/04/17 21:13, Heiner Kallweit wrote:
>>>> AML_UART_TX_EN is set in meson_uart_startup and there's no place in
>>>> the driver where it gets cleared. Therefore we don't have to set it
>>>> here.
>>>
>>> I think this is a leftover of the patches I did when trying to fix
>>> the UART driver when using debian/systemd and having strange console
>>> problems.
>>>
>>> The only comment is it might be worth leaving this in as it isn't
>>> a high cost to the driver and it is possible someone might accidentally
>>> clear it later.
>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> ---
>>>> drivers/tty/serial/meson_uart.c | 1 -
>>>> 1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>>>> index 22857f1e..714b29ad 100644
>>>> --- a/drivers/tty/serial/meson_uart.c
>>>> +++ b/drivers/tty/serial/meson_uart.c
>>>> @@ -497,7 +497,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);
>>>>
>>>>
>>>
>>>
>>
>> I'm ok to remove it, having it in uart_startup is enough.
>>
>> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>
> Have you checked the earlycon code paths? That does not run through the
> standard setup function, so it is dependent on what the firmware set
> last. I believe I am calling into this function through some wrapper.
>
> Since we're writing the register anyway, it shouldn't hurt to leave this in.
>
> Regards,
> Andreas
>
Provided that everybody is fine with the other patches of this series:
Can you simply omit patch 3 when applying the series or would you prefer
sending an updated series w/o patch 3?
Heiner
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write
2017-04-17 15:37 ` Andreas Färber
2017-04-17 16:03 ` Heiner Kallweit
@ 2017-04-19 8:39 ` Neil Armstrong
1 sibling, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2017-04-19 8:39 UTC (permalink / raw)
To: linus-amlogic
On 04/17/2017 05:37 PM, Andreas F?rber wrote:
> Am 17.04.2017 um 17:27 schrieb Neil Armstrong:
>> On 04/17/2017 03:58 PM, Ben Dooks wrote:
>>> On 16/04/17 21:13, Heiner Kallweit wrote:
>>>> AML_UART_TX_EN is set in meson_uart_startup and there's no place in
>>>> the driver where it gets cleared. Therefore we don't have to set it
>>>> here.
>>>
>>> I think this is a leftover of the patches I did when trying to fix
>>> the UART driver when using debian/systemd and having strange console
>>> problems.
>>>
>>> The only comment is it might be worth leaving this in as it isn't
>>> a high cost to the driver and it is possible someone might accidentally
>>> clear it later.
>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> ---
>>>> drivers/tty/serial/meson_uart.c | 1 -
>>>> 1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>>>> index 22857f1e..714b29ad 100644
>>>> --- a/drivers/tty/serial/meson_uart.c
>>>> +++ b/drivers/tty/serial/meson_uart.c
>>>> @@ -497,7 +497,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);
>>>>
>>>>
>>>
>>>
>>
>> I'm ok to remove it, having it in uart_startup is enough.
>>
>> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>
> Have you checked the earlycon code paths? That does not run through the
> standard setup function, so it is dependent on what the firmware set
> last. I believe I am calling into this function through some wrapper.
>
> Since we're writing the register anyway, it shouldn't hurt to leave this in.
>
> Regards,
> Andreas
>
Indeed, it's on the earlycon path, but it's a hack and should be moved inside meson_serial_early_console_setup() and meson_serial_console_setup() instead.
Writing TX_EN at each meson_serial_port_write() is really unnecessary...
Neil
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/6] serial: meson: fix setting number of stop bits
2017-04-17 15:54 ` Heiner Kallweit
@ 2017-04-21 22:03 ` Kevin Hilman
0 siblings, 0 replies; 22+ messages in thread
From: Kevin Hilman @ 2017-04-21 22:03 UTC (permalink / raw)
To: linus-amlogic
Heiner Kallweit <hkallweit1@gmail.com> writes:
> Am 17.04.2017 um 17:24 schrieb Neil Armstrong:
>> On 04/16/2017 10:06 PM, Heiner Kallweit wrote:
>>> 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 <hkallweit1@gmail.com>
>>> ---
>>> 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 60f16795..e2e25da1 100644
>>> --- a/drivers/tty/serial/meson_uart.c
>>> +++ b/drivers/tty/serial/meson_uart.c
>>> @@ -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;
>>>
>>
>> Hi,
>>
>> in fact you can totally drop the else statement since it's a no-op in both cases.
>>
> That's right. However I think leaving this in makes the code better understandable.
> And most likely the compiler will remove this no-op anyway.
Agreed. I like it for readability.
Kevin
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-04-21 22:03 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-16 20:02 [PATCH 0/6] serial: meson: series with smaller fixes and improvements Heiner Kallweit
2017-04-16 20:06 ` [PATCH 1/6] serial: meson: fix setting number of stop bits Heiner Kallweit
2017-04-17 15:24 ` Neil Armstrong
2017-04-17 15:54 ` Heiner Kallweit
2017-04-21 22:03 ` Kevin Hilman
2017-04-16 20:10 ` [PATCH 2/6] serial: meson: remove dead code in meson_uart_change_speed Heiner Kallweit
2017-04-17 15:25 ` Neil Armstrong
2017-04-17 15:54 ` Andreas Färber
2017-04-16 20:13 ` [PATCH 3/6] serial: meson: remove unneeded variable assignment in meson_serial_port_write Heiner Kallweit
2017-04-17 13:58 ` Ben Dooks
2017-04-17 15:27 ` Neil Armstrong
2017-04-17 15:37 ` Andreas Färber
2017-04-17 16:03 ` Heiner Kallweit
2017-04-19 8:39 ` Neil Armstrong
2017-04-16 20:15 ` [PATCH 4/6] serial: meson: make use of uart_port member mapsize Heiner Kallweit
2017-04-17 13:53 ` Ben Dooks
2017-04-17 15:28 ` Neil Armstrong
2017-04-16 20:21 ` [PATCH 5/6] serial: meson: remove use of flag UPF_IOREMAP Heiner Kallweit
2017-04-17 15:28 ` Neil Armstrong
2017-04-16 20:23 ` [PATCH 6/6] serial: meson: change interrupt description to of node name Heiner Kallweit
2017-04-17 13:52 ` Ben Dooks
2017-04-17 15:29 ` Neil Armstrong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.