serial: meson: series with smaller fixes and improvements
Subsystems (2)
cover serial: meson: series with smaller fixes and improvements
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(-)
Signature · 1 line
2.12.2
PATCH 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.
--- drivers/tty/serial/meson_uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/meson_uart.c +1 -1
@@ -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
PATCH 1/6 serial: meson: fix setting number of stop bits
Quoted 14 lines · Heiner Kallweit
The stop bit value as to be or'ed, so far this worked only just by chance because
AML_UART_STOP_BIN_1SBis 0.Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/tty/serial/meson_uart.c +0
-0
drivers/tty/serial/meson_uart.c static void meson_uart_set_termios(struct uart_port *port, · line 361
Quoted 8 lines
drivers/tty/serial/meson_uart.c +1
-1@@ -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
PATCH 1/6 serial: meson: fix setting number of stop bits
Am 17.04.2017 um 17:24 schrieb Neil Armstrong:
Quoted 20 lines · Heiner Kallweit
Quoted 19 lines
The stop bit value as to be or'ed, so far this worked only just by chance because
AML_UART_STOP_BIN_1SBis 0.Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/tty/serial/meson_uart.c +1
-1@@ -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 ;
Quoted 8 lines
drivers/tty/serial/meson_uart.c static void meson_uart_set_termios(struct uart_port *port, · line 361
Quoted 3 lines
drivers/tty/serial/meson_uart.c +0
-0if (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
Quoted 2 lines
Neil
Kevin Hilman
maintainerPATCH 1/6 serial: meson: fix setting number of stop bits
Quoted 27 lines · Heiner Kallweit
Am 17.04.2017 um 17:24 schrieb Neil Armstrong:
Quoted 26 lines · Heiner Kallweit
Quoted 14 lines
The stop bit value as to be or'ed, so far this worked only just by chance because
AML_UART_STOP_BIN_1SBis 0.Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/tty/serial/meson_uart.c +0
-0drivers/tty/serial/meson_uart.c static void meson_uart_set_termios(struct uart_port *port, · line 361
Quoted 8 lines
drivers/tty/serial/meson_uart.c +1
-1@@ -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,
Quoted 4 lines
Quoted 2 lines
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
PATCH 2/6 serial: meson: remove dead code in meson_uart_change_speed
In reply to Heiner Kallweitval is set in both branches of the if clause, therefore the two removed lines are dead code.
--- drivers/tty/serial/meson_uart.c | 2 -- 1 file changed, 2 deletions(-)
drivers/tty/serial/meson_uart.c +0 -2
@@ -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
PATCH 2/6 serial: meson: remove dead code in meson_uart_change_speed
Quoted 14 lines · 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]>--- drivers/tty/serial/meson_uart.c | 2 -- 1 file changed, 2 deletions(-)drivers/tty/serial/meson_uart.c +0
-0
drivers/tty/serial/meson_uart.c static void meson_uart_change_speed(struct uart_port *port, unsigned long baud) · line 303
Quoted 8 lines
drivers/tty/serial/meson_uart.c +0
-2@@ -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 ;
PATCH 2/6 serial: meson: remove dead code in meson_uart_change_speed
Am 17.04.2017 um 17:25 schrieb Neil Armstrong:
Quoted 17 lines · Heiner Kallweit
Quoted 16 lines
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]>--- drivers/tty/serial/meson_uart.c | 2 -- 1 file changed, 2 deletions(-)drivers/tty/serial/meson_uart.c +0
-0@@ -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 ();
Quoted 9 lines
drivers/tty/serial/meson_uart.c static void meson_uart_change_speed(struct uart_port *port, unsigned long baud) · line 303
Quoted 6 lines
drivers/tty/serial/meson_uart.c +0
-2- 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 <[email protected]>
I've double-checked that this was already done before I added the GXBB-enabling XTAL case:
Present since the beginning:
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,
Regards,
Andreas
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany GF: Felix Imend?rffer, Jane Smithard, Graham Norton HRB 21284 (AG N?rnberg)
PATCH 3/6 serial: meson: remove unneeded variable assignment in meson_serial_port_write
In reply to Heiner KallweitAML_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.
--- drivers/tty/serial/meson_uart.c | 1 - 1 file changed, 1 deletion(-)
drivers/tty/serial/meson_uart.c +0 -1
@@ -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 );
Signature · 1 line
2.12.2
PATCH 3/6 serial: meson: remove unneeded variable assignment in meson_serial_port_write
Quoted 3 lines · Heiner Kallweit
AML_UART_TX_ENis set inmeson_uart_startupand 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.
Quoted 18 lines
Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 1 - 1 file changed, 1 deletion(-)drivers/tty/serial/meson_uart.c +0
-1@@ -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 );
Signature · 2 lines
Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius
PATCH 3/6 serial: meson: remove unneeded variable assignment in meson_serial_port_write
Quoted 25 lines · Ben Dooks
Quoted 3 lines · Heiner Kallweit
AML_UART_TX_ENis set inmeson_uart_startupand 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.
Quoted 12 lines
Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 1 - 1 file changed, 1 deletion(-)drivers/tty/serial/meson_uart.c +0
-0@@ -497,7 +497,6 @@ static void meson_serial_port_write(struct uart_port *port, const char *s, }
Quoted 8 lines
drivers/tty/serial/meson_uart.c static void meson_serial_port_write(struct uart_port *port, const char *s, · line 502
Quoted 6 lines
drivers/tty/serial/meson_uart.c +0
-1val =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.
PATCH 3/6 serial: meson: remove unneeded variable assignment in meson_serial_port_write
Am 17.04.2017 um 17:27 schrieb Neil Armstrong:
Quoted 35 lines · Ben Dooks
Quoted 25 lines · Heiner Kallweit
Quoted 3 lines
AML_UART_TX_ENis set inmeson_uart_startupand 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.
Quoted 12 lines
Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 1 - 1 file changed, 1 deletion(-)drivers/tty/serial/meson_uart.c +0
-0@@ -497,7 +497,6 @@ static void meson_serial_port_write(struct uart_port *port, const char *s, }Quoted 8 lines
drivers/tty/serial/meson_uart.c static void meson_serial_port_write(struct uart_port *port, const char *s, · line 502
Quoted 6 lines
drivers/tty/serial/meson_uart.c +0
-1val =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 );
Quoted 3 lines
I'm ok to remove it, having it in
uart_startupis enough.Reviewed-by Neil Armstrong <[email protected]>
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
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany GF: Felix Imend?rffer, Jane Smithard, Graham Norton HRB 21284 (AG N?rnberg)
PATCH 3/6 serial: meson: remove unneeded variable assignment in meson_serial_port_write
Am 17.04.2017 um 17:37 schrieb Andreas F?rber:
Quoted 40 lines
Am 17.04.2017 um 17:27 schrieb Neil Armstrong:
Quoted 35 lines · Ben Dooks
Quoted 25 lines · Heiner Kallweit
Quoted 3 lines
AML_UART_TX_ENis set inmeson_uart_startupand 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.
Quoted 12 lines
Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 1 - 1 file changed, 1 deletion(-)drivers/tty/serial/meson_uart.c +0
-0@@ -497,7 +497,6 @@ static void meson_serial_port_write(struct uart_port *port, const char *s, }Quoted 8 lines
drivers/tty/serial/meson_uart.c static void meson_serial_port_write(struct uart_port *port, const char *s, · line 502
Quoted 6 lines
drivers/tty/serial/meson_uart.c +0
-1val =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 );Quoted 3 lines
I'm ok to remove it, having it in
uart_startupis enough.Reviewed-by Neil Armstrong <[email protected]>
Quoted 9 lines
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
PATCH 3/6 serial: meson: remove unneeded variable assignment in meson_serial_port_write
In reply to Andreas FärberQuoted 40 lines · Andreas F?rber
Am 17.04.2017 um 17:27 schrieb Neil Armstrong:
Quoted 35 lines · Ben Dooks
Quoted 25 lines · Heiner Kallweit
Quoted 3 lines
AML_UART_TX_ENis set inmeson_uart_startupand 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.
Quoted 12 lines
Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 1 - 1 file changed, 1 deletion(-)drivers/tty/serial/meson_uart.c +0
-0@@ -497,7 +497,6 @@ static void meson_serial_port_write(struct uart_port *port, const char *s, }Quoted 8 lines
drivers/tty/serial/meson_uart.c static void meson_serial_port_write(struct uart_port *port, const char *s, · line 502
Quoted 6 lines
drivers/tty/serial/meson_uart.c +0
-1val =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 );Quoted 3 lines
I'm ok to remove it, having it in
uart_startupis enough.Reviewed-by Neil Armstrong <[email protected]>
Quoted 9 lines
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
PATCH 4/6 serial: meson: make use of uart_port member mapsize
In reply to Heiner KallweitMember mapsize of struct uart_port is meant to store the resource size. By using it we can get rid of meson_uart_res_size().
--- drivers/tty/serial/meson_uart.c | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-)
drivers/tty/serial/meson_uart.c +5 -24
@@ -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;
Signature · 1 line
2.12.2
PATCH 4/6 serial: meson: make use of uart_port member mapsize
Quoted 65 lines · Heiner Kallweit
Member mapsize of struct
uart_portis meant to store the resource size. By using it we can get rid ofmeson_uart_res_size().Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-)drivers/tty/serial/meson_uart.c +4
-24@@ -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 ; }
drivers/tty/serial/meson_uart.c static int meson_uart_probe(struct platform_device *pdev) · line 615
Quoted 7 lines
drivers/tty/serial/meson_uart.c +1
-0@@ -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.
Signature · 2 lines
Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius
PATCH 4/6 serial: meson: make use of uart_port member mapsize
In reply to Heiner KallweitQuoted 66 lines · Heiner Kallweit
Member mapsize of struct
uart_portis meant to store the resource size. By using it we can get rid ofmeson_uart_res_size().Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-)drivers/tty/serial/meson_uart.c +4
-24@@ -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 ; }
drivers/tty/serial/meson_uart.c static int meson_uart_probe(struct platform_device *pdev) · line 615
Quoted 7 lines
drivers/tty/serial/meson_uart.c +1
-0@@ -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;
PATCH 5/6 serial: meson: remove use of flag UPF_IOREMAP
In reply to Heiner KallweitFlag 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.
--- drivers/tty/serial/meson_uart.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-)
drivers/tty/serial/meson_uart.c +8 -14
@@ -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 ;
Signature · 1 line
2.12.2
PATCH 5/6 serial: meson: remove use of flag UPF_IOREMAP
Quoted 53 lines · Heiner Kallweit
Flag
UPF_IOREMAPis 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 inmeson_uart_request_port.Signed-off-by Heiner Kallweit <[email protected]>--- drivers/tty/serial/meson_uart.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-)drivers/tty/serial/meson_uart.c +7
-13@@ -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 ; }
drivers/tty/serial/meson_uart.c static int meson_uart_probe(struct platform_device *pdev) · line 611
Quoted 8 lines
drivers/tty/serial/meson_uart.c +1
-1@@ -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 ;
PATCH 6/6 serial: meson: change interrupt description to of node name
In reply to Heiner KallweitChange 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.
--- drivers/tty/serial/meson_uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/meson_uart.c +1 -1
@@ -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; }
Signature · 1 line
2.12.2
PATCH 6/6 serial: meson: change interrupt description to of node name
Quoted 14 lines · Heiner Kallweit
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 <[email protected]>--- drivers/tty/serial/meson_uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/tty/serial/meson_uart.c +0
-0
drivers/tty/serial/meson_uart.c static int meson_uart_startup(struct uart_port *port) · line 292
Quoted 8 lines
drivers/tty/serial/meson_uart.c +1
-1@@ -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.
Signature · 2 lines
Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius
PATCH 6/6 serial: meson: change interrupt description to of node name
In reply to Heiner KallweitQuoted 15 lines · Heiner Kallweit
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 <[email protected]>--- drivers/tty/serial/meson_uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/tty/serial/meson_uart.c +0
-0
drivers/tty/serial/meson_uart.c static int meson_uart_startup(struct uart_port *port) · line 292
Quoted 8 lines
drivers/tty/serial/meson_uart.c +1
-1@@ -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; }