[PATCH 0/7] serial: various cleanups

[PATCH 0/7] serial: various cleanups

[Jiri Slaby]

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

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

[Jiri Slaby]

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 <jslaby@suse.cz>
---
 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

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

[Jiri Slaby]

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 <jslaby@suse.cz>
---
 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

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

[Jiri Slaby]

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 <jslaby@suse.cz>
---
 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

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

[Jiri Slaby]

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

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 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

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

[Jiri Slaby]

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 <agross@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 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

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

[Jiri Slaby]

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 <jslaby@suse.cz>
---
 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

[PATCH 7/7] serial: allow COMPILE_TEST for some drivers

[Jiri Slaby]

Some more serial drivers can be compile-tested under certain
circumstances (when building a specific architecture). So allow for
that.

This reduces the need of zillion mach/subarch-specific configs. And
since the 0day bot has only allmodconfig's for some archs, this
increases build coverage there too.

Note that cpm needs a minor update in the header, so that it drags in
at least some defines (CPM2 ones).

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/serial/Kconfig             | 6 +++---
 drivers/tty/serial/cpm_uart/cpm_uart.h | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 20cb103972fa..2d3eed53b43e 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -782,7 +782,7 @@ config SERIAL_PMACZILOG_CONSOLE
 
 config SERIAL_CPM
 	tristate "CPM SCC/SMC serial port support"
-	depends on CPM2 || CPM1
+	depends on CPM2 || CPM1 || (PPC32 && COMPILE_TEST)
 	select SERIAL_CORE
 	help
 	  This driver supports the SCC and SMC serial ports on Motorola 
@@ -806,7 +806,7 @@ config SERIAL_CPM_CONSOLE
 
 config SERIAL_PIC32
 	tristate "Microchip PIC32 serial support"
-	depends on MACH_PIC32
+	depends on MACH_PIC32 || (MIPS && COMPILE_TEST)
 	select SERIAL_CORE
 	help
 	  If you have a PIC32, this driver supports the serial ports.
@@ -1246,7 +1246,7 @@ config SERIAL_XILINX_PS_UART_CONSOLE
 
 config SERIAL_AR933X
 	tristate "AR933X serial port support"
-	depends on HAVE_CLK && ATH79
+	depends on (HAVE_CLK && ATH79) || (MIPS && COMPILE_TEST)
 	select SERIAL_CORE
 	select SERIAL_MCTRL_GPIO if GPIOLIB
 	help
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart.h b/drivers/tty/serial/cpm_uart/cpm_uart.h
index 6113b953ce25..8c582779cf22 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart.h
+++ b/drivers/tty/serial/cpm_uart/cpm_uart.h
@@ -19,6 +19,8 @@ struct gpio_desc;
 #include "cpm_uart_cpm2.h"
 #elif defined(CONFIG_CPM1)
 #include "cpm_uart_cpm1.h"
+#elif defined(CONFIG_COMPILE_TEST)
+#include "cpm_uart_cpm2.h"
 #endif
 
 #define SERIAL_CPM_MAJOR	204
-- 
2.36.0

Re: [PATCH 7/7] serial: allow COMPILE_TEST for some drivers

[Randy Dunlap]

Hi Jiri,

On 4/21/22 03:17, Jiri Slaby wrote:
> Some more serial drivers can be compile-tested under certain
> circumstances (when building a specific architecture). So allow for
> that.
> 
> This reduces the need of zillion mach/subarch-specific configs. And
> since the 0day bot has only allmodconfig's for some archs, this
> increases build coverage there too.
> 
> Note that cpm needs a minor update in the header, so that it drags in
> at least some defines (CPM2 ones).
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  drivers/tty/serial/Kconfig             | 6 +++---
>  drivers/tty/serial/cpm_uart/cpm_uart.h | 2 ++
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 20cb103972fa..2d3eed53b43e 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -782,7 +782,7 @@ config SERIAL_PMACZILOG_CONSOLE
>  
>  config SERIAL_CPM
>  	tristate "CPM SCC/SMC serial port support"
> -	depends on CPM2 || CPM1
> +	depends on CPM2 || CPM1 || (PPC32 && COMPILE_TEST)


This patch (now commit e3e7b13bffae) causes build errors
when neither CPM1 nor CPM2 is set but PPC32=y and COMPILE_TEST=y:

ERROR: modpost: "cpm_uart_freebuf" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined!
ERROR: modpost: "cpm_uart_allocbuf" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined!
ERROR: modpost: "cpm_line_cr_cmd" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined!
ERROR: modpost: "__cpm2_setbrg" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined!
ERROR: modpost: "cpm_uart_unmap_pram" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined!
ERROR: modpost: "cpm_uart_map_pram" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined!

>  	select SERIAL_CORE
>  	help
>  	  This driver supports the SCC and SMC serial ports on Motorola 
> @@ -806,7 +806,7 @@ config SERIAL_CPM_CONSOLE
>  
>  config SERIAL_PIC32
>  	tristate "Microchip PIC32 serial support"
> -	depends on MACH_PIC32
> +	depends on MACH_PIC32 || (MIPS && COMPILE_TEST)
>  	select SERIAL_CORE
>  	help
>  	  If you have a PIC32, this driver supports the serial ports.
> @@ -1246,7 +1246,7 @@ config SERIAL_XILINX_PS_UART_CONSOLE
>  
>  config SERIAL_AR933X
>  	tristate "AR933X serial port support"
> -	depends on HAVE_CLK && ATH79
> +	depends on (HAVE_CLK && ATH79) || (MIPS && COMPILE_TEST)
>  	select SERIAL_CORE
>  	select SERIAL_MCTRL_GPIO if GPIOLIB
>  	help
> diff --git a/drivers/tty/serial/cpm_uart/cpm_uart.h b/drivers/tty/serial/cpm_uart/cpm_uart.h
> index 6113b953ce25..8c582779cf22 100644
> --- a/drivers/tty/serial/cpm_uart/cpm_uart.h
> +++ b/drivers/tty/serial/cpm_uart/cpm_uart.h
> @@ -19,6 +19,8 @@ struct gpio_desc;
>  #include "cpm_uart_cpm2.h"
>  #elif defined(CONFIG_CPM1)
>  #include "cpm_uart_cpm1.h"
> +#elif defined(CONFIG_COMPILE_TEST)
> +#include "cpm_uart_cpm2.h"
>  #endif
>  
>  #define SERIAL_CPM_MAJOR	204

-- 
~Randy