[PATCH 0/6] tty: spring cleanup

[PATCH 0/6] tty: spring cleanup

[Jiri Slaby (SUSE)]

Hi,

this is another series of various cleanup in tty of issues which popped
up at me during a larger rework.

Jiri Slaby (SUSE) (6):
  tty: simplify throttling using guard()s
  tty: use lock guard()s in tty_io
  serial: switch uart_port::iotype to enum uart_iotype
  serial: rename local uart_port_lock() -> uart_port_ref_lock()
  serial: use uart_port_ref_lock() helper
  serial: 8250: unexport serial8250_rpm_*() functions

 drivers/tty/serial/8250/8250.h       |  6 --
 drivers/tty/serial/8250/8250_core.c  |  2 +-
 drivers/tty/serial/8250/8250_early.c |  2 +
 drivers/tty/serial/8250/8250_port.c  | 16 ++---
 drivers/tty/serial/8250/8250_rsa.c   |  2 +
 drivers/tty/serial/amba-pl011.c      |  2 +-
 drivers/tty/serial/fsl_lpuart.c      |  5 +-
 drivers/tty/serial/samsung_tty.c     |  4 ++
 drivers/tty/serial/serial_core.c     | 97 ++++++++++++++--------------
 drivers/tty/tty_io.c                 | 96 ++++++++++++---------------
 drivers/tty/tty_ioctl.c              | 50 +++++++-------
 include/linux/serial_core.h          | 30 +++++----
 12 files changed, 149 insertions(+), 163 deletions(-)

-- 
2.49.0

[PATCH 1/6] tty: simplify throttling using guard()s

[Jiri Slaby (SUSE)]

tty_throttle_safe() and tty_unthrottle_safe can be made less convoluted
using guard()s. Switch them.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty_ioctl.c | 50 +++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 85de90eebc7b..90c70d8d14e3 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -122,21 +122,19 @@ EXPORT_SYMBOL(tty_unthrottle);
  */
 bool tty_throttle_safe(struct tty_struct *tty)
 {
-	bool ret = true;
-
-	mutex_lock(&tty->throttle_mutex);
-	if (!tty_throttled(tty)) {
-		if (tty->flow_change != TTY_THROTTLE_SAFE)
-			ret = false;
-		else {
-			set_bit(TTY_THROTTLED, &tty->flags);
-			if (tty->ops->throttle)
-				tty->ops->throttle(tty);
-		}
-	}
-	mutex_unlock(&tty->throttle_mutex);
+	guard(mutex)(&tty->throttle_mutex);
 
-	return ret;
+	if (tty_throttled(tty))
+		return true;
+
+	if (tty->flow_change != TTY_THROTTLE_SAFE)
+		return false;
+
+	set_bit(TTY_THROTTLED, &tty->flags);
+	if (tty->ops->throttle)
+		tty->ops->throttle(tty);
+
+	return true;
 }
 
 /**
@@ -152,21 +150,19 @@ bool tty_throttle_safe(struct tty_struct *tty)
  */
 bool tty_unthrottle_safe(struct tty_struct *tty)
 {
-	bool ret = true;
+	guard(mutex)(&tty->throttle_mutex);
 
-	mutex_lock(&tty->throttle_mutex);
-	if (tty_throttled(tty)) {
-		if (tty->flow_change != TTY_UNTHROTTLE_SAFE)
-			ret = false;
-		else {
-			clear_bit(TTY_THROTTLED, &tty->flags);
-			if (tty->ops->unthrottle)
-				tty->ops->unthrottle(tty);
-		}
-	}
-	mutex_unlock(&tty->throttle_mutex);
+	if (!tty_throttled(tty))
+		return true;
 
-	return ret;
+	if (tty->flow_change != TTY_UNTHROTTLE_SAFE)
+		return false;
+
+	clear_bit(TTY_THROTTLED, &tty->flags);
+	if (tty->ops->unthrottle)
+		tty->ops->unthrottle(tty);
+
+	return true;
 }
 
 /**
-- 
2.49.0

[PATCH 2/6] tty: use lock guard()s in tty_io

[Jiri Slaby (SUSE)]

guard()s and scoped_guard()s express more clearly what is protected by
locks. And also makes the code cleaner as it can return immediately in
case of short returns.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty_io.c | 96 ++++++++++++++++++--------------------------
 1 file changed, 40 insertions(+), 56 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index ca9b7d7bad2b..e2d92cf70eb7 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -276,11 +276,10 @@ static void check_tty_count(struct tty_struct *tty, const char *routine)
 	struct list_head *p;
 	int count = 0, kopen_count = 0;
 
-	spin_lock(&tty->files_lock);
-	list_for_each(p, &tty->tty_files) {
-		count++;
-	}
-	spin_unlock(&tty->files_lock);
+	scoped_guard(spinlock, &tty->files_lock)
+		list_for_each(p, &tty->tty_files)
+			count++;
+
 	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
 	    tty->driver->subtype == PTY_TYPE_SLAVE &&
 	    tty->link && tty->link->count)
@@ -378,7 +377,7 @@ EXPORT_SYMBOL_GPL(tty_dev_name_to_number);
  */
 struct tty_driver *tty_find_polling_driver(char *name, int *line)
 {
-	struct tty_driver *p, *res = NULL;
+	struct tty_driver *p;
 	int tty_line = 0;
 	int len;
 	char *str, *stp;
@@ -392,7 +391,8 @@ struct tty_driver *tty_find_polling_driver(char *name, int *line)
 	len = str - name;
 	tty_line = simple_strtoul(str, &str, 10);
 
-	mutex_lock(&tty_mutex);
+	guard(mutex)(&tty_mutex);
+
 	/* Search through the tty devices to look for a match */
 	list_for_each_entry(p, &tty_drivers, tty_drivers) {
 		if (!len || strncmp(name, p->name, len) != 0)
@@ -405,14 +405,12 @@ struct tty_driver *tty_find_polling_driver(char *name, int *line)
 
 		if (tty_line >= 0 && tty_line < p->num && p->ops &&
 		    p->ops->poll_init && !p->ops->poll_init(p, tty_line, stp)) {
-			res = tty_driver_kref_get(p);
 			*line = tty_line;
-			break;
+			return tty_driver_kref_get(p);
 		}
 	}
-	mutex_unlock(&tty_mutex);
 
-	return res;
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(tty_find_polling_driver);
 #endif
@@ -531,16 +529,15 @@ EXPORT_SYMBOL_GPL(tty_wakeup);
  */
 static struct file *tty_release_redirect(struct tty_struct *tty)
 {
-	struct file *f = NULL;
+	guard(spinlock)(&redirect_lock);
 
-	spin_lock(&redirect_lock);
 	if (redirect && file_tty(redirect) == tty) {
-		f = redirect;
+		struct file *f = redirect;
 		redirect = NULL;
+		return f;
 	}
-	spin_unlock(&redirect_lock);
 
-	return f;
+	return NULL;
 }
 
 /**
@@ -765,11 +762,8 @@ void __stop_tty(struct tty_struct *tty)
  */
 void stop_tty(struct tty_struct *tty)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&tty->flow.lock, flags);
+	guard(spinlock_irqsave)(&tty->flow.lock);
 	__stop_tty(tty);
-	spin_unlock_irqrestore(&tty->flow.lock, flags);
 }
 EXPORT_SYMBOL(stop_tty);
 
@@ -796,11 +790,8 @@ void __start_tty(struct tty_struct *tty)
  */
 void start_tty(struct tty_struct *tty)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&tty->flow.lock, flags);
+	guard(spinlock_irqsave)(&tty->flow.lock);
 	__start_tty(tty);
-	spin_unlock_irqrestore(&tty->flow.lock, flags);
 }
 EXPORT_SYMBOL(start_tty);
 
@@ -809,7 +800,8 @@ static void tty_update_time(struct tty_struct *tty, bool mtime)
 	time64_t sec = ktime_get_real_seconds();
 	struct tty_file_private *priv;
 
-	spin_lock(&tty->files_lock);
+	guard(spinlock)(&tty->files_lock);
+
 	list_for_each_entry(priv, &tty->tty_files, list) {
 		struct inode *inode = file_inode(priv->file);
 		struct timespec64 time = mtime ? inode_get_mtime(inode) : inode_get_atime(inode);
@@ -827,7 +819,6 @@ static void tty_update_time(struct tty_struct *tty, bool mtime)
 				inode_set_atime(inode, sec, 0);
 		}
 	}
-	spin_unlock(&tty->files_lock);
 }
 
 /*
@@ -2314,13 +2305,12 @@ static int tiocsti(struct tty_struct *tty, u8 __user *p)
  */
 static int tiocgwinsz(struct tty_struct *tty, struct winsize __user *arg)
 {
-	int err;
+	guard(mutex)(&tty->winsize_mutex);
 
-	mutex_lock(&tty->winsize_mutex);
-	err = copy_to_user(arg, &tty->winsize, sizeof(*arg));
-	mutex_unlock(&tty->winsize_mutex);
+	if (copy_to_user(arg, &tty->winsize, sizeof(*arg)))
+		return -EFAULT;
 
-	return err ? -EFAULT : 0;
+	return 0;
 }
 
 /**
@@ -2335,10 +2325,10 @@ int tty_do_resize(struct tty_struct *tty, struct winsize *ws)
 {
 	struct pid *pgrp;
 
-	/* Lock the tty */
-	mutex_lock(&tty->winsize_mutex);
+	guard(mutex)(&tty->winsize_mutex);
+
 	if (!memcmp(ws, &tty->winsize, sizeof(*ws)))
-		goto done;
+		return 0;
 
 	/* Signal the foreground process group */
 	pgrp = tty_get_pgrp(tty);
@@ -2347,8 +2337,7 @@ int tty_do_resize(struct tty_struct *tty, struct winsize *ws)
 	put_pid(pgrp);
 
 	tty->winsize = *ws;
-done:
-	mutex_unlock(&tty->winsize_mutex);
+
 	return 0;
 }
 EXPORT_SYMBOL(tty_do_resize);
@@ -2409,13 +2398,14 @@ static int tioccons(struct file *file)
 		return -EBADF;
 	if (!(file->f_mode & FMODE_CAN_WRITE))
 		return -EINVAL;
-	spin_lock(&redirect_lock);
-	if (redirect) {
-		spin_unlock(&redirect_lock);
+
+	guard(spinlock)(&redirect_lock);
+
+	if (redirect)
 		return -EBUSY;
-	}
+
 	redirect = get_file(file);
-	spin_unlock(&redirect_lock);
+
 	return 0;
 }
 
@@ -3028,11 +3018,9 @@ void __do_SAK(struct tty_struct *tty)
 	struct task_struct *g, *p;
 	struct pid *session;
 	int i;
-	unsigned long flags;
 
-	spin_lock_irqsave(&tty->ctrl.lock, flags);
-	session = get_pid(tty->ctrl.session);
-	spin_unlock_irqrestore(&tty->ctrl.lock, flags);
+	scoped_guard(spinlock_irqsave, &tty->ctrl.lock)
+		session = get_pid(tty->ctrl.session);
 
 	tty_ldisc_flush(tty);
 
@@ -3055,7 +3043,7 @@ void __do_SAK(struct tty_struct *tty)
 					PIDTYPE_SID);
 			continue;
 		}
-		task_lock(p);
+		guard(task_lock)(p);
 		i = iterate_fd(p->files, 0, this_tty, tty);
 		if (i != 0) {
 			tty_notice(tty, "SAK: killed process %d (%s): by fd#%d\n",
@@ -3063,7 +3051,6 @@ void __do_SAK(struct tty_struct *tty)
 			group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p,
 					PIDTYPE_SID);
 		}
-		task_unlock(p);
 	}
 	read_unlock(&tasklist_lock);
 	put_pid(session);
@@ -3465,9 +3452,8 @@ int tty_register_driver(struct tty_driver *driver)
 			goto err_unreg_char;
 	}
 
-	mutex_lock(&tty_mutex);
-	list_add(&driver->tty_drivers, &tty_drivers);
-	mutex_unlock(&tty_mutex);
+	scoped_guard(mutex, &tty_mutex)
+		list_add(&driver->tty_drivers, &tty_drivers);
 
 	if (!(driver->flags & TTY_DRIVER_DYNAMIC_DEV)) {
 		for (i = 0; i < driver->num; i++) {
@@ -3486,9 +3472,8 @@ int tty_register_driver(struct tty_driver *driver)
 	for (i--; i >= 0; i--)
 		tty_unregister_device(driver, i);
 
-	mutex_lock(&tty_mutex);
-	list_del(&driver->tty_drivers);
-	mutex_unlock(&tty_mutex);
+	scoped_guard(mutex, &tty_mutex)
+		list_del(&driver->tty_drivers);
 
 err_unreg_char:
 	unregister_chrdev_region(dev, driver->num);
@@ -3507,9 +3492,8 @@ void tty_unregister_driver(struct tty_driver *driver)
 {
 	unregister_chrdev_region(MKDEV(driver->major, driver->minor_start),
 				driver->num);
-	mutex_lock(&tty_mutex);
-	list_del(&driver->tty_drivers);
-	mutex_unlock(&tty_mutex);
+	scoped_guard(mutex, &tty_mutex)
+		list_del(&driver->tty_drivers);
 }
 EXPORT_SYMBOL(tty_unregister_driver);
 
-- 
2.49.0

[PATCH 3/6] serial: switch uart_port::iotype to enum uart_iotype

[Jiri Slaby (SUSE)]

The inline-defined constants look weird. Instead, define a proper enum
for them and type uart_port::iotype as that enum. This allows for proper
checking in switch-case labels (somewhere, a default or UPIO_UNKNOWN
label needs to be added/handled).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/serial/8250/8250_core.c  |  2 +-
 drivers/tty/serial/8250/8250_early.c |  2 ++
 drivers/tty/serial/8250/8250_port.c  |  4 ++++
 drivers/tty/serial/8250/8250_rsa.c   |  2 ++
 drivers/tty/serial/amba-pl011.c      |  2 +-
 drivers/tty/serial/fsl_lpuart.c      |  5 ++++-
 drivers/tty/serial/samsung_tty.c     |  4 ++++
 drivers/tty/serial/serial_core.c     |  8 ++++----
 include/linux/serial_core.h          | 30 +++++++++++++++-------------
 9 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 5a56f853cf6d..68994a964321 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -461,7 +461,7 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
 				  char *options)
 {
 	char match[] = "uart";	/* 8250-specific earlycon name */
-	unsigned char iotype;
+	enum uart_iotype iotype;
 	resource_size_t addr;
 	int i;
 
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 842422921765..dc0371857ecb 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -77,6 +77,8 @@ static void serial8250_early_out(struct uart_port *port, int offset, int value)
 		outb(value, port->iobase + offset);
 		break;
 #endif
+	default:
+		break;
 	}
 }
 
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 8ac452cea36c..8d9bb91d4bae 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2993,6 +2993,8 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
 		if (!request_region(port->iobase, size, "serial"))
 			return -EBUSY;
 		return 0;
+	case UPIO_UNKNOWN:
+		break;
 	}
 
 	return 0;
@@ -3025,6 +3027,8 @@ static void serial8250_release_std_resource(struct uart_8250_port *up)
 	case UPIO_PORT:
 		release_region(port->iobase, size);
 		break;
+	case UPIO_UNKNOWN:
+		break;
 	}
 }
 
diff --git a/drivers/tty/serial/8250/8250_rsa.c b/drivers/tty/serial/8250/8250_rsa.c
index 82f2593b4c59..4c8b9671bd41 100644
--- a/drivers/tty/serial/8250/8250_rsa.c
+++ b/drivers/tty/serial/8250/8250_rsa.c
@@ -43,6 +43,8 @@ static void rsa8250_release_resource(struct uart_8250_port *up)
 	case UPIO_PORT:
 		release_region(port->iobase + offset, size);
 		break;
+	default:
+		break;
 	}
 }
 
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 11d65097578c..421ac22555df 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2476,7 +2476,7 @@ static int pl011_console_setup(struct console *co, char *options)
 static int pl011_console_match(struct console *co, char *name, int idx,
 			       char *options)
 {
-	unsigned char iotype;
+	enum uart_iotype iotype;
 	resource_size_t addr;
 	int i;
 
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index fe5aed99d55a..dff6a6c57b5f 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -403,6 +403,8 @@ static inline void lpuart32_write(struct uart_port *port, u32 val,
 	case UPIO_MEM32BE:
 		iowrite32be(val, port->membase + off);
 		break;
+	default:
+		break;
 	}
 }
 
@@ -563,8 +565,9 @@ static dma_addr_t lpuart_dma_datareg_addr(struct lpuart_port *sport)
 		return sport->port.mapbase + UARTDATA;
 	case UPIO_MEM32BE:
 		return sport->port.mapbase + UARTDATA + sizeof(u32) - 1;
+	default:
+		return sport->port.mapbase + UARTDR;
 	}
-	return sport->port.mapbase + UARTDR;
 }
 
 static int lpuart_dma_tx_request(struct uart_port *port)
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 210fff7164c1..73e2866febc1 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -190,6 +190,8 @@ static void wr_reg(const struct uart_port *port, u32 reg, u32 val)
 	case UPIO_MEM32:
 		writel_relaxed(val, portaddr(port, reg));
 		break;
+	default:
+		break;
 	}
 }
 
@@ -2713,6 +2715,8 @@ static void wr_reg_barrier(const struct uart_port *port, u32 reg, u32 val)
 	case UPIO_MEM32:
 		writel(val, portaddr(port, reg));
 		break;
+	default:
+		break;
 	}
 }
 
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 88669972d9a0..5bc145643385 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2178,8 +2178,8 @@ uart_get_console(struct uart_port *ports, int nr, struct console *co)
  *
  * Returns: 0 on success or -%EINVAL on failure
  */
-int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr,
-			char **options)
+int uart_parse_earlycon(char *p, enum uart_iotype *iotype,
+			resource_size_t *addr, char **options)
 {
 	if (strncmp(p, "mmio,", 5) == 0) {
 		*iotype = UPIO_MEM;
@@ -3289,9 +3289,9 @@ bool uart_match_port(const struct uart_port *port1,
 	case UPIO_AU:
 	case UPIO_TSI:
 		return port1->mapbase == port2->mapbase;
+	default:
+		return false;
 	}
-
-	return false;
 }
 EXPORT_SYMBOL(uart_match_port);
 
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 743b4afaad4c..914b5e97e056 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -427,6 +427,18 @@ struct uart_icount {
 typedef u64 __bitwise upf_t;
 typedef unsigned int __bitwise upstat_t;
 
+enum uart_iotype {
+	UPIO_UNKNOWN	= -1,
+	UPIO_PORT	= SERIAL_IO_PORT,	/* 8b I/O port access */
+	UPIO_HUB6	= SERIAL_IO_HUB6,	/* Hub6 ISA card */
+	UPIO_MEM	= SERIAL_IO_MEM,	/* driver-specific */
+	UPIO_MEM32	= SERIAL_IO_MEM32,	/* 32b little endian */
+	UPIO_AU		= SERIAL_IO_AU,		/* Au1x00 and RT288x type IO */
+	UPIO_TSI	= SERIAL_IO_TSI,	/* Tsi108/109 type IO */
+	UPIO_MEM32BE	= SERIAL_IO_MEM32BE,	/* 32b big endian */
+	UPIO_MEM16	= SERIAL_IO_MEM16,	/* 16b little endian */
+};
+
 struct uart_port {
 	spinlock_t		lock;			/* port lock */
 	unsigned long		iobase;			/* in/out[bwl] */
@@ -469,23 +481,13 @@ struct uart_port {
 	unsigned char		x_char;			/* xon/xoff char */
 	unsigned char		regshift;		/* reg offset shift */
 
-	unsigned char		iotype;			/* io access style */
-
-#define UPIO_UNKNOWN		((unsigned char)~0U)	/* UCHAR_MAX */
-#define UPIO_PORT		(SERIAL_IO_PORT)	/* 8b I/O port access */
-#define UPIO_HUB6		(SERIAL_IO_HUB6)	/* Hub6 ISA card */
-#define UPIO_MEM		(SERIAL_IO_MEM)		/* driver-specific */
-#define UPIO_MEM32		(SERIAL_IO_MEM32)	/* 32b little endian */
-#define UPIO_AU			(SERIAL_IO_AU)		/* Au1x00 and RT288x type IO */
-#define UPIO_TSI		(SERIAL_IO_TSI)		/* Tsi108/109 type IO */
-#define UPIO_MEM32BE		(SERIAL_IO_MEM32BE)	/* 32b big endian */
-#define UPIO_MEM16		(SERIAL_IO_MEM16)	/* 16b little endian */
-
 	unsigned char		quirks;			/* internal quirks */
 
 	/* internal quirks must be updated while holding port mutex */
 #define UPQ_NO_TXEN_TEST	BIT(0)
 
+	enum uart_iotype	iotype;			/* io access style */
+
 	unsigned int		read_status_mask;	/* driver specific */
 	unsigned int		ignore_status_mask;	/* driver specific */
 	struct uart_state	*state;			/* pointer to parent state */
@@ -1101,8 +1103,8 @@ static inline bool uart_console_registered(struct uart_port *port)
 
 struct uart_port *uart_get_console(struct uart_port *ports, int nr,
 				   struct console *c);
-int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr,
-			char **options);
+int uart_parse_earlycon(char *p, enum uart_iotype *iotype,
+			resource_size_t *addr, char **options);
 void uart_parse_options(const char *options, int *baud, int *parity, int *bits,
 			int *flow);
 int uart_set_options(struct uart_port *port, struct console *co, int baud,
-- 
2.49.0

[PATCH 4/6] serial: rename local uart_port_lock() -> uart_port_ref_lock()

[Jiri Slaby (SUSE)]

uart_port_lock() and uart_port_unlock() are (at the same time) defined
as:
* functions in include/linux/serial_core.h
* macros in drivers/tty/serial/serial_core.c

The former are sane uart port lock wrappers.

The latter _lock() does something completely different: it inspects
a uart_state, obtains a uart_port from it, and increases its reference
count. And if that all succeeded, the port is locked too.

Similarly, the _unlock() counterpart first unlocks and then decrements
the refcount too.

This state is REALLY CONFUSING.

So rename the latter (local .c macros):
* uart_port_lock() -> uart_port_ref_lock(), and
* uart_port_unlock() -> uart_port_unlock_deref().

Now, the forbidden while-at-it part: convert from a macro to an inline
-- do it here as the passed 'flags' have to be pointer to ulong now. So
we avoid doubled changes on identical LOCs.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/serial/serial_core.c | 75 ++++++++++++++++----------------
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 5bc145643385..52e764be42c4 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -75,22 +75,23 @@ static inline void uart_port_deref(struct uart_port *uport)
 		wake_up(&uport->state->remove_wait);
 }
 
-#define uart_port_lock(state, flags)					\
-	({								\
-		struct uart_port *__uport = uart_port_ref(state);	\
-		if (__uport)						\
-			uart_port_lock_irqsave(__uport, &flags);	\
-		__uport;						\
-	})
-
-#define uart_port_unlock(uport, flags)					\
-	({								\
-		struct uart_port *__uport = uport;			\
-		if (__uport) {						\
-			uart_port_unlock_irqrestore(__uport, flags);	\
-			uart_port_deref(__uport);			\
-		}							\
-	})
+static inline struct uart_port *uart_port_ref_lock(struct uart_state *state, unsigned long *flags)
+{
+	struct uart_port *uport = uart_port_ref(state);
+
+	if (uport)
+		uart_port_lock_irqsave(uport, flags);
+
+	return uport;
+}
+
+static inline void uart_port_unlock_deref(struct uart_port *uport, unsigned long flags)
+{
+	if (uport) {
+		uart_port_unlock_irqrestore(uport, flags);
+		uart_port_deref(uport);
+	}
+}
 
 static inline struct uart_port *uart_port_check(struct uart_state *state)
 {
@@ -127,10 +128,10 @@ static void uart_stop(struct tty_struct *tty)
 	struct uart_port *port;
 	unsigned long flags;
 
-	port = uart_port_lock(state, flags);
+	port = uart_port_ref_lock(state, &flags);
 	if (port)
 		port->ops->stop_tx(port);
-	uart_port_unlock(port, flags);
+	uart_port_unlock_deref(port, flags);
 }
 
 static void __uart_start(struct uart_state *state)
@@ -168,9 +169,9 @@ static void uart_start(struct tty_struct *tty)
 	struct uart_port *port;
 	unsigned long flags;
 
-	port = uart_port_lock(state, flags);
+	port = uart_port_ref_lock(state, &flags);
 	__uart_start(state);
-	uart_port_unlock(port, flags);
+	uart_port_unlock_deref(port, flags);
 }
 
 static void
@@ -258,14 +259,14 @@ static int uart_alloc_xmit_buf(struct tty_port *port)
 	if (!page)
 		return -ENOMEM;
 
-	uport = uart_port_lock(state, flags);
+	uport = uart_port_ref_lock(state, &flags);
 	if (!state->port.xmit_buf) {
 		state->port.xmit_buf = (unsigned char *)page;
 		kfifo_init(&state->port.xmit_fifo, state->port.xmit_buf,
 				PAGE_SIZE);
-		uart_port_unlock(uport, flags);
+		uart_port_unlock_deref(uport, flags);
 	} else {
-		uart_port_unlock(uport, flags);
+		uart_port_unlock_deref(uport, flags);
 		/*
 		 * Do not free() the page under the port lock, see
 		 * uart_free_xmit_buf().
@@ -289,11 +290,11 @@ static void uart_free_xmit_buf(struct tty_port *port)
 	 * console driver may need to allocate/free a debug object, which
 	 * can end up in printk() recursion.
 	 */
-	uport = uart_port_lock(state, flags);
+	uport = uart_port_ref_lock(state, &flags);
 	xmit_buf = port->xmit_buf;
 	port->xmit_buf = NULL;
 	INIT_KFIFO(port->xmit_fifo);
-	uart_port_unlock(uport, flags);
+	uart_port_unlock_deref(uport, flags);
 
 	free_page((unsigned long)xmit_buf);
 }
@@ -592,15 +593,15 @@ static int uart_put_char(struct tty_struct *tty, u8 c)
 	unsigned long flags;
 	int ret = 0;
 
-	port = uart_port_lock(state, flags);
+	port = uart_port_ref_lock(state, &flags);
 	if (!state->port.xmit_buf) {
-		uart_port_unlock(port, flags);
+		uart_port_unlock_deref(port, flags);
 		return 0;
 	}
 
 	if (port)
 		ret = kfifo_put(&state->port.xmit_fifo, c);
-	uart_port_unlock(port, flags);
+	uart_port_unlock_deref(port, flags);
 	return ret;
 }
 
@@ -623,9 +624,9 @@ static ssize_t uart_write(struct tty_struct *tty, const u8 *buf, size_t count)
 	if (WARN_ON(!state))
 		return -EL3HLT;
 
-	port = uart_port_lock(state, flags);
+	port = uart_port_ref_lock(state, &flags);
 	if (!state->port.xmit_buf) {
-		uart_port_unlock(port, flags);
+		uart_port_unlock_deref(port, flags);
 		return 0;
 	}
 
@@ -633,7 +634,7 @@ static ssize_t uart_write(struct tty_struct *tty, const u8 *buf, size_t count)
 		ret = kfifo_in(&state->port.xmit_fifo, buf, count);
 
 	__uart_start(state);
-	uart_port_unlock(port, flags);
+	uart_port_unlock_deref(port, flags);
 	return ret;
 }
 
@@ -644,9 +645,9 @@ static unsigned int uart_write_room(struct tty_struct *tty)
 	unsigned long flags;
 	unsigned int ret;
 
-	port = uart_port_lock(state, flags);
+	port = uart_port_ref_lock(state, &flags);
 	ret = kfifo_avail(&state->port.xmit_fifo);
-	uart_port_unlock(port, flags);
+	uart_port_unlock_deref(port, flags);
 	return ret;
 }
 
@@ -657,9 +658,9 @@ static unsigned int uart_chars_in_buffer(struct tty_struct *tty)
 	unsigned long flags;
 	unsigned int ret;
 
-	port = uart_port_lock(state, flags);
+	port = uart_port_ref_lock(state, &flags);
 	ret = kfifo_len(&state->port.xmit_fifo);
-	uart_port_unlock(port, flags);
+	uart_port_unlock_deref(port, flags);
 	return ret;
 }
 
@@ -678,13 +679,13 @@ static void uart_flush_buffer(struct tty_struct *tty)
 
 	pr_debug("uart_flush_buffer(%d) called\n", tty->index);
 
-	port = uart_port_lock(state, flags);
+	port = uart_port_ref_lock(state, &flags);
 	if (!port)
 		return;
 	kfifo_reset(&state->port.xmit_fifo);
 	if (port->ops->flush_buffer)
 		port->ops->flush_buffer(port);
-	uart_port_unlock(port, flags);
+	uart_port_unlock_deref(port, flags);
 	tty_port_tty_wakeup(&state->port);
 }
 
-- 
2.49.0

[PATCH 5/6] serial: use uart_port_ref_lock() helper

[Jiri Slaby (SUSE)]

uart_get_icount() and uart_carrier_raised() open code
uart_port_ref_lock(). Use the helper instead.

The difference is we use _irqsave() variants of a spinlock now. But
that's "safer" than _irq().

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/serial/serial_core.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 52e764be42c4..1f7708a91fc6 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1276,14 +1276,13 @@ static int uart_get_icount(struct tty_struct *tty,
 	struct uart_state *state = tty->driver_data;
 	struct uart_icount cnow;
 	struct uart_port *uport;
+	unsigned long flags;
 
-	uport = uart_port_ref(state);
+	uport = uart_port_ref_lock(state, &flags);
 	if (!uport)
 		return -EIO;
-	uart_port_lock_irq(uport);
 	memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
-	uart_port_unlock_irq(uport);
-	uart_port_deref(uport);
+	uart_port_unlock_deref(uport, flags);
 
 	icount->cts         = cnow.cts;
 	icount->dsr         = cnow.dsr;
@@ -1915,9 +1914,10 @@ static bool uart_carrier_raised(struct tty_port *port)
 {
 	struct uart_state *state = container_of(port, struct uart_state, port);
 	struct uart_port *uport;
+	unsigned long flags;
 	int mctrl;
 
-	uport = uart_port_ref(state);
+	uport = uart_port_ref_lock(state, &flags);
 	/*
 	 * Should never observe uport == NULL since checks for hangup should
 	 * abort the tty_port_block_til_ready() loop before checking for carrier
@@ -1926,11 +1926,9 @@ static bool uart_carrier_raised(struct tty_port *port)
 	 */
 	if (WARN_ON(!uport))
 		return true;
-	uart_port_lock_irq(uport);
 	uart_enable_ms(uport);
 	mctrl = uport->ops->get_mctrl(uport);
-	uart_port_unlock_irq(uport);
-	uart_port_deref(uport);
+	uart_port_unlock_deref(uport, flags);
 
 	return mctrl & TIOCM_CAR;
 }
-- 
2.49.0

[PATCH 6/6] serial: 8250: unexport serial8250_rpm_*() functions

[Jiri Slaby (SUSE)]

Since commit 8700a7ea5519 (serial: 8250_omap: Drop
pm_runtime_irq_safe()), all the serial8250_rpm_*() functions are used
solely in 8250_port.

Unexport them.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/serial/8250/8250.h      |  6 ------
 drivers/tty/serial/8250/8250_port.c | 12 ++++--------
 2 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index b861585ca02a..18530c31a598 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -223,12 +223,6 @@ static inline bool serial8250_clear_THRI(struct uart_8250_port *up)
 struct uart_8250_port *serial8250_setup_port(int index);
 struct uart_8250_port *serial8250_get_port(int line);
 
-void serial8250_rpm_get(struct uart_8250_port *p);
-void serial8250_rpm_put(struct uart_8250_port *p);
-
-void serial8250_rpm_get_tx(struct uart_8250_port *p);
-void serial8250_rpm_put_tx(struct uart_8250_port *p);
-
 int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
 			    struct serial_rs485 *rs485);
 void serial8250_em485_start_tx(struct uart_8250_port *p, bool toggle_ier);
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 8d9bb91d4bae..6d7b8c4667c9 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -517,22 +517,20 @@ void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p)
 }
 EXPORT_SYMBOL_GPL(serial8250_clear_and_reinit_fifos);
 
-void serial8250_rpm_get(struct uart_8250_port *p)
+static void serial8250_rpm_get(struct uart_8250_port *p)
 {
 	if (!(p->capabilities & UART_CAP_RPM))
 		return;
 	pm_runtime_get_sync(p->port.dev);
 }
-EXPORT_SYMBOL_GPL(serial8250_rpm_get);
 
-void serial8250_rpm_put(struct uart_8250_port *p)
+static void serial8250_rpm_put(struct uart_8250_port *p)
 {
 	if (!(p->capabilities & UART_CAP_RPM))
 		return;
 	pm_runtime_mark_last_busy(p->port.dev);
 	pm_runtime_put_autosuspend(p->port.dev);
 }
-EXPORT_SYMBOL_GPL(serial8250_rpm_put);
 
 /**
  *	serial8250_em485_init() - put uart_8250_port into rs485 emulating
@@ -647,7 +645,7 @@ EXPORT_SYMBOL_GPL(serial8250_em485_config);
  * once and disable_runtime_pm_tx() will still disable RPM because the fifo is
  * empty and the HW can idle again.
  */
-void serial8250_rpm_get_tx(struct uart_8250_port *p)
+static void serial8250_rpm_get_tx(struct uart_8250_port *p)
 {
 	unsigned char rpm_active;
 
@@ -659,9 +657,8 @@ void serial8250_rpm_get_tx(struct uart_8250_port *p)
 		return;
 	pm_runtime_get_sync(p->port.dev);
 }
-EXPORT_SYMBOL_GPL(serial8250_rpm_get_tx);
 
-void serial8250_rpm_put_tx(struct uart_8250_port *p)
+static void serial8250_rpm_put_tx(struct uart_8250_port *p)
 {
 	unsigned char rpm_active;
 
@@ -674,7 +671,6 @@ void serial8250_rpm_put_tx(struct uart_8250_port *p)
 	pm_runtime_mark_last_busy(p->port.dev);
 	pm_runtime_put_autosuspend(p->port.dev);
 }
-EXPORT_SYMBOL_GPL(serial8250_rpm_put_tx);
 
 /*
  * IER sleep support.  UARTs which have EFRs need the "extended
-- 
2.49.0