Skip to contentSkip to conversation

Misc serial cleanups

PATCH New Peter Hurley 1 participant 9 messages Jan 11, 2016

Subsystems (5)
MIPS TTY layer and serial drivers m68k architecture Alpha port +1 more subsystems
lore
Share & tools
lore

Peter Hurley

author
#0 lore raw

v2 cover Misc serial cleanups

Hi Greg,

Apologies for the earlier mixed series; this is the proper series. This series contains accumulated cleanups for the serial core.

Several of these patches were in earlier series that were not applied so I split the simpler ones into this series to form a basis for follow-on series (especially the "Fix unsafe uart port access" series and another that addresses the tty_port->flags problem).

Regards,

Regards,

Peter Hurley (8):
serial: core: Fold __uart_put_char() into caller
serial: core: Fold do_uart_get_info() into caller
serial: core: Use tty->index for port # in debug messages
serial: Fix ASYNC_* => UPF_* flags misuse
serial: core: Cleanup uart_open() exit
serial: core: Remove cast from void ptr in uart_open()
serial: core: Unfold < 80 char lines
serial: core: Perform RTS signalling before soft flow ctrl

 arch/alpha/include/asm/serial.h      |  8 ++--
 arch/frv/include/asm/serial.h        |  2 +-
 arch/m68k/include/asm/serial.h       |  8 ++--
 arch/mips/pmcs-msp71xx/msp_serial.c  |  2 +-
 arch/mn10300/include/asm/serial.h    | 10 ++---
 arch/xtensa/platforms/xt2000/setup.c |  2 +-
 drivers/tty/serial/serial_core.c     | 71 ++++++++++++++----------------------
 drivers/tty/serial/serial_ks8695.c   |  2 +-
 8 files changed, 44 insertions(+), 61 deletions(-)
Signature · 1 line
2.7.0
author
#1 lore raw

PATCH v2 1/8 serial: core: Fold __uart_put_char() into caller

uart_put_char() is the required interface; manually inline __uart_put_char().

Signed-off-by Peter Hurley <[email protected]>
---
 drivers/tty/serial/serial_core.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)
drivers/tty/serial/serial_core.c +5 -9
index 3698ada..048b175 100644
@@ -483,12 +483,15 @@ static void uart_change_speed(struct tty_struct *tty, struct uart_state *state,
 	spin_unlock_irq(&uport->lock);
 }
 
-static inline int __uart_put_char(struct uart_port *port,
-				struct circ_buf *circ, unsigned char c)
+static int uart_put_char(struct tty_struct *tty, unsigned char c)
 {
+	struct uart_state *state = tty->driver_data;
+	struct uart_port *port = state->uart_port;
+	struct circ_buf *circ;
 	unsigned long flags;
 	int ret = 0;
 
+	circ = &state->xmit;
 	if (!circ->buf)
 		return 0;
 

@@ -502,13 +505,6 @@ static inline int __uart_put_char(struct uart_port *port,
 	return ret;
 }
 
-static int uart_put_char(struct tty_struct *tty, unsigned char ch)
-{
-	struct uart_state *state = tty->driver_data;
-
-	return __uart_put_char(state->uart_port, &state->xmit, ch);
-}
-
 static void uart_flush_chars(struct tty_struct *tty)
 {
 	uart_start(tty);
Signature · 1 line
2.7.0
author
#2 lore raw

PATCH v2 2/8 serial: core: Fold do_uart_get_info() into caller

In reply to Peter Hurley

do_uart_get_info() has a single caller: uart_get_info(). Manually inline do_uart_get_info().

Signed-off-by Peter Hurley <[email protected]>
---
 drivers/tty/serial/serial_core.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)
drivers/tty/serial/serial_core.c +7 -11
index 048b175..6dfb583 100644
@@ -671,14 +671,18 @@ static void uart_unthrottle(struct tty_struct *tty)
 		uart_set_mctrl(port, TIOCM_RTS);
 }
 
-static void do_uart_get_info(struct tty_port *port,
-			struct serial_struct *retinfo)
+static void uart_get_info(struct tty_port *port, struct serial_struct *retinfo)
 {
 	struct uart_state *state = container_of(port, struct uart_state, port);
 	struct uart_port *uport = state->uart_port;
 
 	memset(retinfo, 0, sizeof(*retinfo));
 
+	/*
+	 * Ensure the state we copy is consistent and no hardware changes
+	 * occur as we go
+	 */
+	mutex_lock(&port->mutex);
 	retinfo->type	    = uport->type;
 	retinfo->line	    = uport->line;
 	retinfo->port	    = uport->iobase;

@@ -697,15 +701,6 @@ static void do_uart_get_info(struct tty_port *port,
 	retinfo->io_type         = uport->iotype;
 	retinfo->iomem_reg_shift = uport->regshift;
 	retinfo->iomem_base      = (void *)(unsigned long)uport->mapbase;
-}
-
-static void uart_get_info(struct tty_port *port,
-			struct serial_struct *retinfo)
-{
-	/* Ensure the state we copy is consistent and no hardware changes
-	   occur as we go */
-	mutex_lock(&port->mutex);
-	do_uart_get_info(port, retinfo);
 	mutex_unlock(&port->mutex);
 }
 

@@ -713,6 +708,7 @@ static int uart_get_info_user(struct tty_port *port,
 			 struct serial_struct __user *retinfo)
 {
 	struct serial_struct tmp;
+
 	uart_get_info(port, &tmp);
 
 	if (copy_to_user(retinfo, &tmp, sizeof(*retinfo)))
Signature · 1 line
2.7.0
author
#3 lore raw

PATCH v2 3/8 serial: core: Use tty->index for port # in debug messages

In reply to Peter Hurley

The uart port may have already been removed by uart_remove_one_port(); use equivalent tty->index (which is always valid in these contexts) instead.

Signed-off-by Peter Hurley <[email protected]>
---
 drivers/tty/serial/serial_core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/tty/serial/serial_core.c +2 -3
index 6dfb583..f8c28a7 100644
@@ -1381,8 +1381,7 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
 
 	uport = state->uart_port;
 	port = &state->port;
-
-	pr_debug("uart_close(%d) called\n", uport ? uport->line : -1);
+	pr_debug("uart_close(%d) called\n", tty->index);
 
 	if (!port->count || tty_port_close_start(port, tty, filp) == 0)
 		return;

@@ -1500,7 +1499,7 @@ static void uart_hangup(struct tty_struct *tty)
 	struct tty_port *port = &state->port;
 	unsigned long flags;
 
-	pr_debug("uart_hangup(%d)\n", state->uart_port->line);
+	pr_debug("uart_hangup(%d)\n", tty->index);
 
 	mutex_lock(&port->mutex);
 	if (port->flags & ASYNC_NORMAL_ACTIVE) {
Signature · 1 line
2.7.0
author
#4 lore raw

PATCH v2 4/8 serial: Fix ASYNC_* => UPF_* flags misuse

In reply to Peter Hurley

The UPF_* flags are the correct values to use for struct uart_port and struct old_serial_port/SERIAL_PORT_DFNS.

Signed-off-by Peter Hurley <[email protected]>
---
 arch/alpha/include/asm/serial.h      |  8 ++++----
 arch/frv/include/asm/serial.h        |  2 +-
 arch/m68k/include/asm/serial.h       |  8 ++++----
 arch/mips/pmcs-msp71xx/msp_serial.c  |  2 +-
 arch/mn10300/include/asm/serial.h    | 10 +++++-----
 arch/xtensa/platforms/xt2000/setup.c |  2 +-
 drivers/tty/serial/serial_ks8695.c   |  2 +-
 7 files changed, 17 insertions(+), 17 deletions(-)
arch/alpha/include/asm/serial.h +4 -4
index 22909b8..e31557f 100644
@@ -14,11 +14,11 @@
 
 /* Standard COM flags (except for COM4, because of the 8514 problem) */
 #ifdef CONFIG_SERIAL_8250_DETECT_IRQ
-#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST | ASYNC_AUTO_IRQ)
-#define STD_COM4_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_AUTO_IRQ)
+#define STD_COM_FLAGS (UPF_BOOT_AUTOCONF | UPF_SKIP_TEST | UPF_AUTO_IRQ)
+#define STD_COM4_FLAGS (UPF_BOOT_AUTOCONF | UPF_AUTO_IRQ)
 #else
-#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST)
-#define STD_COM4_FLAGS ASYNC_BOOT_AUTOCONF
+#define STD_COM_FLAGS (UPF_BOOT_AUTOCONF | UPF_SKIP_TEST)
+#define STD_COM4_FLAGS UPF_BOOT_AUTOCONF
 #endif
 
 #define SERIAL_PORT_DFNS			\
arch/frv/include/asm/serial.h +1 -1
index dbb8259..bce0d0d 100644
@@ -13,6 +13,6 @@
  */
 #define BASE_BAUD 0
 
-#define STD_COM_FLAGS		ASYNC_BOOT_AUTOCONF
+#define STD_COM_FLAGS		UPF_BOOT_AUTOCONF
 
 #define SERIAL_PORT_DFNS
arch/m68k/include/asm/serial.h +4 -4
index 06d0cb1..6d44970 100644
@@ -18,11 +18,11 @@
 
 /* Standard COM flags (except for COM4, because of the 8514 problem) */
 #ifdef CONFIG_SERIAL_8250_DETECT_IRQ
-#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST | ASYNC_AUTO_IRQ)
-#define STD_COM4_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_AUTO_IRQ)
+#define STD_COM_FLAGS (UPF_BOOT_AUTOCONF | UPF_SKIP_TEST | UPF_AUTO_IRQ)
+#define STD_COM4_FLAGS (UPF_BOOT_AUTOCONF | UPF_AUTO_IRQ)
 #else
-#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST)
-#define STD_COM4_FLAGS ASYNC_BOOT_AUTOCONF
+#define STD_COM_FLAGS (UPF_BOOT_AUTOCONF | UPF_SKIP_TEST)
+#define STD_COM4_FLAGS UPF_BOOT_AUTOCONF
 #endif
 
 #ifdef CONFIG_ISA
arch/mips/pmcs-msp71xx/msp_serial.c +1 -1
index d304be2..8e6e8db 100644
@@ -110,7 +110,7 @@ void __init msp_serial_setup(void)
 	up.uartclk	= uartclk;
 	up.regshift	= 2;
 	up.iotype	= UPIO_MEM;
-	up.flags	= ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST;
+	up.flags	= UPF_BOOT_AUTOCONF | UPF_SKIP_TEST;
 	up.type		= PORT_16550A;
 	up.line		= 0;
 	up.serial_out	= msp_serial_out;
arch/mn10300/include/asm/serial.h +5 -5
index c199021..594ebff 100644
@@ -14,15 +14,15 @@
 
 /* Standard COM flags (except for COM4, because of the 8514 problem) */
 #ifdef CONFIG_SERIAL_8250_DETECT_IRQ
-#define STD_COM_FLAGS	(ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST | ASYNC_AUTO_IRQ)
-#define STD_COM4_FLAGS	(ASYNC_BOOT_AUTOCONF | ASYNC_AUTO_IRQ)
+#define STD_COM_FLAGS	(UPF_BOOT_AUTOCONF | UPF_SKIP_TEST | UPF_AUTO_IRQ)
+#define STD_COM4_FLAGS	(UPF_BOOT_AUTOCONF | UPF_AUTO_IRQ)
 #else
-#define STD_COM_FLAGS	(ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST)
-#define STD_COM4_FLAGS	ASYNC_BOOT_AUTOCONF
+#define STD_COM_FLAGS	(UPF_BOOT_AUTOCONF | UPF_SKIP_TEST)
+#define STD_COM4_FLAGS	UPF_BOOT_AUTOCONF
 #endif
 
 #ifdef CONFIG_SERIAL_8250_MANY_PORTS
-#define FOURPORT_FLAGS	ASYNC_FOURPORT
+#define FOURPORT_FLAGS	UPF_FOURPORT
 #define ACCENT_FLAGS	0
 #define BOCA_FLAGS	0
 #define HUB6_FLAGS	0
arch/xtensa/platforms/xt2000/setup.c +1 -1
index 8767896..5f4bd71 100644
@@ -113,7 +113,7 @@ void platform_heartbeat(void)
 }
 
 //#define RS_TABLE_SIZE 2
-//#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF|ASYNC_SKIP_TEST)
+//#define STD_COM_FLAGS (UPF_BOOT_AUTOCONF|UPF_SKIP_TEST)
 
 #define _SERIAL_PORT(_base,_irq)					\
 {									\
drivers/tty/serial/serial_ks8695.c +1 -1
index b4decf8..57f1523 100644
@@ -554,7 +554,7 @@ static struct uart_port ks8695uart_ports[SERIAL_KS8695_NR] = {
 		.uartclk	= KS8695_CLOCK_RATE * 16,
 		.fifosize	= 16,
 		.ops		= &ks8695uart_pops,
-		.flags		= ASYNC_BOOT_AUTOCONF,
+		.flags		= UPF_BOOT_AUTOCONF,
 		.line		= 0,
 	}
 };
Signature · 1 line
2.7.0
author
#5 lore raw

PATCH v2 5/8 serial: core: Cleanup uart_open() exit

In reply to Peter Hurley

If aborting uart_open() unsuccessfully, retval is non-zero, so the existing fall-through exit is equivalent.

Signed-off-by Peter Hurley <[email protected]>
---
 drivers/tty/serial/serial_core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/tty/serial/serial_core.c +1 -4
index f8c28a7..f77be14 100644
@@ -1622,15 +1622,12 @@ static int uart_open(struct tty_struct *tty, struct file *filp)
 	/*
 	 * If we succeeded, wait until the port is ready.
 	 */
+err_unlock:
 	mutex_unlock(&port->mutex);
 	if (retval == 0)
 		retval = tty_port_block_til_ready(port, tty, filp);
-
 end:
 	return retval;
-err_unlock:
-	mutex_unlock(&port->mutex);
-	goto end;
 }
 
 static const char *uart_type(struct uart_port *port)
Signature · 1 line
2.7.0
author
#6 lore raw

PATCH v2 6/8 serial: core: Remove cast from void ptr in uart_open()

In reply to Peter Hurley

void * promotes to any pointer type; remove type cast.

Signed-off-by Peter Hurley <[email protected]>
---
 drivers/tty/serial/serial_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/serial_core.c +1 -1
index f77be14..530b892 100644
@@ -1580,7 +1580,7 @@ static void uart_dtr_rts(struct tty_port *port, int onoff)
  */
 static int uart_open(struct tty_struct *tty, struct file *filp)
 {
-	struct uart_driver *drv = (struct uart_driver *)tty->driver->driver_state;
+	struct uart_driver *drv = tty->driver->driver_state;
 	int retval, line = tty->index;
 	struct uart_state *state = drv->state + line;
 	struct tty_port *port = &state->port;
Signature · 1 line
2.7.0
author
#7 lore raw

PATCH v2 7/8 serial: core: Unfold < 80 char lines

In reply to Peter Hurley
Signed-off-by Peter Hurley <[email protected]>
---
 drivers/tty/serial/serial_core.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)
drivers/tty/serial/serial_core.c +5 -10
index 530b892..3031982 100644
@@ -1686,17 +1686,13 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
 		seq_printf(m, " tx:%d rx:%d",
 				uport->icount.tx, uport->icount.rx);
 		if (uport->icount.frame)
-			seq_printf(m, " fe:%d",
-				uport->icount.frame);
+			seq_printf(m, " fe:%d",	uport->icount.frame);
 		if (uport->icount.parity)
-			seq_printf(m, " pe:%d",
-				uport->icount.parity);
+			seq_printf(m, " pe:%d",	uport->icount.parity);
 		if (uport->icount.brk)
-			seq_printf(m, " brk:%d",
-				uport->icount.brk);
+			seq_printf(m, " brk:%d", uport->icount.brk);
 		if (uport->icount.overrun)
-			seq_printf(m, " oe:%d",
-				uport->icount.overrun);
+			seq_printf(m, " oe:%d", uport->icount.overrun);
 
 #define INFOBIT(bit, str) \
 	if (uport->mctrl & (bit)) \

@@ -1731,8 +1727,7 @@ static int uart_proc_show(struct seq_file *m, void *v)
 	struct uart_driver *drv = ttydrv->driver_state;
 	int i;
 
-	seq_printf(m, "serinfo:1.0 driver%s%s revision:%s\n",
-			"", "", "");
+	seq_printf(m, "serinfo:1.0 driver%s%s revision:%s\n", "", "", "");
 	for (i = 0; i < drv->nr; i++)
 		uart_line_info(m, drv, i);
 	return 0;
Signature · 1 line
2.7.0
author
#8 lore raw

PATCH v2 8/8 serial: core: Perform RTS signalling before soft flow ctrl

In reply to Peter Hurley

When throttling, time is of the essence; try RTS signalling before soft flow control, which will take longer.

Signed-off-by Peter Hurley <[email protected]>
---
 drivers/tty/serial/serial_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/tty/serial/serial_core.c +6 -6
index 3031982..0baa231 100644
@@ -641,11 +641,11 @@ static void uart_throttle(struct tty_struct *tty)
 		mask &= ~port->status;
 	}
 
-	if (mask & UPSTAT_AUTOXOFF)
-		uart_send_xchar(tty, STOP_CHAR(tty));
-
 	if (mask & UPSTAT_AUTORTS)
 		uart_clear_mctrl(port, TIOCM_RTS);
+
+	if (mask & UPSTAT_AUTOXOFF)
+		uart_send_xchar(tty, STOP_CHAR(tty));
 }
 
 static void uart_unthrottle(struct tty_struct *tty)

@@ -664,11 +664,11 @@ static void uart_unthrottle(struct tty_struct *tty)
 		mask &= ~port->status;
 	}
 
-	if (mask & UPSTAT_AUTOXOFF)
-		uart_send_xchar(tty, START_CHAR(tty));
-
 	if (mask & UPSTAT_AUTORTS)
 		uart_set_mctrl(port, TIOCM_RTS);
+
+	if (mask & UPSTAT_AUTOXOFF)
+		uart_send_xchar(tty, START_CHAR(tty));
 }
 
 static void uart_get_info(struct tty_port *port, struct serial_struct *retinfo)
Signature · 1 line
2.7.0