2022-04-22 14:34:50

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 0/7] serial: various cleanups

This is a set of simple cleanups for various drivers. All should be
straightforward.

The last two patches enable compilation on 0-day bot also for three more
drivers. So we should have a better coverage. Actually the last but one
is the result of the last one -- 0-day bot already complained to me.

Jiri Slaby (7):
serial: sunplus-uart: mark sunplus_console_ports[] static
serial: xilinx_uartps: return early in cdns_uart_handle_tx()
serial: xilinx_uartps: cache xmit in cdns_uart_handle_tx()
serial: zs: use NULL as a pointer, not 0
serial: qcom: use check for empty instead of pending
serial: pic32: make SERIAL_PIC32_CONSOLE depend on SERIAL_PIC32=y
serial: allow COMPILE_TEST for some drivers

drivers/tty/serial/Kconfig | 8 ++---
drivers/tty/serial/cpm_uart/cpm_uart.h | 2 ++
drivers/tty/serial/qcom_geni_serial.c | 2 +-
drivers/tty/serial/sunplus-uart.c | 2 +-
drivers/tty/serial/xilinx_uartps.c | 46 +++++++++-----------------
drivers/tty/serial/zs.c | 2 +-
6 files changed, 25 insertions(+), 37 deletions(-)

--
2.36.0


2022-04-22 17:19:31

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 3/7] serial: xilinx_uartps: cache xmit in cdns_uart_handle_tx()

Cache port->state->xmit into a local variable (xmit) in
cdns_uart_handle_tx(). This reduces length of some lines there
significantly. I.e. makes the code more readable.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/xilinx_uartps.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index b84ae9c07c3b..9e01fe6c0ab8 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -313,29 +313,26 @@ static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus)
static void cdns_uart_handle_tx(void *dev_id)
{
struct uart_port *port = (struct uart_port *)dev_id;
+ struct circ_buf *xmit = &port->state->xmit;
unsigned int numbytes;

- if (uart_circ_empty(&port->state->xmit)) {
+ if (uart_circ_empty(xmit)) {
writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IDR);
return;
}

numbytes = port->fifosize;
- while (numbytes && !uart_circ_empty(&port->state->xmit) &&
- !(readl(port->membase + CDNS_UART_SR) &
- CDNS_UART_SR_TXFULL)) {
+ while (numbytes && !uart_circ_empty(xmit) &&
+ !(readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXFULL)) {

- writel(port->state->xmit.buf[port->state->xmit.tail],
- port->membase + CDNS_UART_FIFO);
+ writel(xmit->buf[xmit->tail], port->membase + CDNS_UART_FIFO);

port->icount.tx++;
- port->state->xmit.tail = (port->state->xmit.tail + 1) &
- (UART_XMIT_SIZE - 1);
-
+ xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
numbytes--;
}

- if (uart_circ_chars_pending(&port->state->xmit) < WAKEUP_CHARS)
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
uart_write_wakeup(port);
}

--
2.36.0

2022-04-22 17:55:26

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 4/7] serial: zs: use NULL as a pointer, not 0

struct uart_port::membase is declared as a pointer. So it should be
initialized by NULL, not zero constant.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/zs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index 70969bf9d82c..5bc58591665a 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -981,7 +981,7 @@ static const char *zs_type(struct uart_port *uport)
static void zs_release_port(struct uart_port *uport)
{
iounmap(uport->membase);
- uport->membase = 0;
+ uport->membase = NULL;
release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
}

--
2.36.0

2022-04-22 19:10:42

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 1/7] serial: sunplus-uart: mark sunplus_console_ports[] static

It's used only in the driver, so declare it as local to silence this
correct sparse warning:
sunplus-uart.c:501:26: warning: symbol 'sunplus_console_ports' was not declared. Should it be static?

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/sunplus-uart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sunplus-uart.c b/drivers/tty/serial/sunplus-uart.c
index 9f15922e681b..60c73662f955 100644
--- a/drivers/tty/serial/sunplus-uart.c
+++ b/drivers/tty/serial/sunplus-uart.c
@@ -498,7 +498,7 @@ static const struct uart_ops sunplus_uart_ops = {
};

#ifdef CONFIG_SERIAL_SUNPLUS_CONSOLE
-struct sunplus_uart_port *sunplus_console_ports[SUP_UART_NR];
+static struct sunplus_uart_port *sunplus_console_ports[SUP_UART_NR];

static void sunplus_uart_console_putchar(struct uart_port *port,
unsigned char ch)
--
2.36.0

2022-04-22 20:44:24

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 5/7] serial: qcom: use check for empty instead of pending

The code wants to know if the circ buffer is empty, so use the proper
macro.

No functional change intended, just saner function name used for that
use case.

Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/qcom_geni_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 1543a6028856..f4961022d7d0 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -507,7 +507,7 @@ static void qcom_geni_serial_console_write(struct console *co, const char *s,
*/
qcom_geni_serial_poll_tx_done(uport);

- if (uart_circ_chars_pending(&uport->state->xmit)) {
+ if (!uart_circ_empty(&uport->state->xmit)) {
irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
writel(irq_en | M_TX_FIFO_WATERMARK_EN,
uport->membase + SE_GENI_M_IRQ_EN);
--
2.36.0

2022-04-22 20:59:47

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 2/7] serial: xilinx_uartps: return early in cdns_uart_handle_tx()

Return from the true branch of the 'if'. This saves one indentation
level and makes the code more readable.

The two comments about what obvious code does are removed too.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/xilinx_uartps.c | 47 ++++++++++++------------------
1 file changed, 18 insertions(+), 29 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 250a1d888eeb..b84ae9c07c3b 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -317,37 +317,26 @@ static void cdns_uart_handle_tx(void *dev_id)

if (uart_circ_empty(&port->state->xmit)) {
writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IDR);
- } else {
- numbytes = port->fifosize;
- while (numbytes && !uart_circ_empty(&port->state->xmit) &&
- !(readl(port->membase + CDNS_UART_SR) &
- CDNS_UART_SR_TXFULL)) {
- /*
- * Get the data from the UART circular buffer
- * and write it to the cdns_uart's TX_FIFO
- * register.
- */
- writel(
- port->state->xmit.buf[port->state->xmit.tail],
- port->membase + CDNS_UART_FIFO);
-
- port->icount.tx++;
-
- /*
- * Adjust the tail of the UART buffer and wrap
- * the buffer if it reaches limit.
- */
- port->state->xmit.tail =
- (port->state->xmit.tail + 1) &
- (UART_XMIT_SIZE - 1);
-
- numbytes--;
- }
+ return;
+ }

- if (uart_circ_chars_pending(
- &port->state->xmit) < WAKEUP_CHARS)
- uart_write_wakeup(port);
+ numbytes = port->fifosize;
+ while (numbytes && !uart_circ_empty(&port->state->xmit) &&
+ !(readl(port->membase + CDNS_UART_SR) &
+ CDNS_UART_SR_TXFULL)) {
+
+ writel(port->state->xmit.buf[port->state->xmit.tail],
+ port->membase + CDNS_UART_FIFO);
+
+ port->icount.tx++;
+ port->state->xmit.tail = (port->state->xmit.tail + 1) &
+ (UART_XMIT_SIZE - 1);
+
+ numbytes--;
}
+
+ if (uart_circ_chars_pending(&port->state->xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
}

/**
--
2.36.0

2022-04-22 21:12:33

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 6/7] serial: pic32: make SERIAL_PIC32_CONSOLE depend on SERIAL_PIC32=y

pic32_uart contains this:
#ifdef CONFIG_SERIAL_PIC32_CONSOLE
...
console_initcall(pic32_console_init);
...
core_initcall(pic32_late_console_init);
...
#endif
...
arch_initcall(pic32_uart_init);

When the driver is built as module, all three above become
module_init(). So if SERIAL_PIC32_CONSOLE is set while SERIAL_PIC32=m,
it results in the following build error:
In file included from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from include/linux/platform_device.h:13,
from drivers/tty/serial/pic32_uart.c:12:
include/linux/module.h:131:49: error: redefinition of '__inittest'

So make sure SERIAL_PIC32_CONSOLE can be set only when SERIAL_PIC32=y --
similar as for other drivers.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index dbac90e2e209..20cb103972fa 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -817,7 +817,7 @@ config SERIAL_PIC32

config SERIAL_PIC32_CONSOLE
bool "PIC32 serial console support"
- depends on SERIAL_PIC32
+ depends on SERIAL_PIC32=y
select SERIAL_CORE_CONSOLE
help
If you have a PIC32, this driver supports the putting a console on one
--
2.36.0