* [PATCH 1/6] tty: simplify throttling using guard()s
2025-04-25 11:13 [PATCH 0/6] tty: spring cleanup Jiri Slaby (SUSE)
@ 2025-04-25 11:13 ` Jiri Slaby (SUSE)
2025-04-25 11:13 ` [PATCH 2/6] tty: use lock guard()s in tty_io Jiri Slaby (SUSE)
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-04-25 11:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/6] tty: use lock guard()s in tty_io
2025-04-25 11:13 [PATCH 0/6] tty: spring cleanup Jiri Slaby (SUSE)
2025-04-25 11:13 ` [PATCH 1/6] tty: simplify throttling using guard()s Jiri Slaby (SUSE)
@ 2025-04-25 11:13 ` Jiri Slaby (SUSE)
2025-04-25 11:13 ` [PATCH 3/6] serial: switch uart_port::iotype to enum uart_iotype Jiri Slaby (SUSE)
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-04-25 11:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/6] serial: switch uart_port::iotype to enum uart_iotype
2025-04-25 11:13 [PATCH 0/6] tty: spring cleanup Jiri Slaby (SUSE)
2025-04-25 11:13 ` [PATCH 1/6] tty: simplify throttling using guard()s Jiri Slaby (SUSE)
2025-04-25 11:13 ` [PATCH 2/6] tty: use lock guard()s in tty_io Jiri Slaby (SUSE)
@ 2025-04-25 11:13 ` Jiri Slaby (SUSE)
2025-04-25 11:13 ` [PATCH 4/6] serial: rename local uart_port_lock() -> uart_port_ref_lock() Jiri Slaby (SUSE)
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-04-25 11:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/6] serial: rename local uart_port_lock() -> uart_port_ref_lock()
2025-04-25 11:13 [PATCH 0/6] tty: spring cleanup Jiri Slaby (SUSE)
` (2 preceding siblings ...)
2025-04-25 11:13 ` [PATCH 3/6] serial: switch uart_port::iotype to enum uart_iotype Jiri Slaby (SUSE)
@ 2025-04-25 11:13 ` Jiri Slaby (SUSE)
2025-04-25 11:13 ` [PATCH 5/6] serial: use uart_port_ref_lock() helper Jiri Slaby (SUSE)
2025-04-25 11:13 ` [PATCH 6/6] serial: 8250: unexport serial8250_rpm_*() functions Jiri Slaby (SUSE)
5 siblings, 0 replies; 7+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-04-25 11:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5/6] serial: use uart_port_ref_lock() helper
2025-04-25 11:13 [PATCH 0/6] tty: spring cleanup Jiri Slaby (SUSE)
` (3 preceding siblings ...)
2025-04-25 11:13 ` [PATCH 4/6] serial: rename local uart_port_lock() -> uart_port_ref_lock() Jiri Slaby (SUSE)
@ 2025-04-25 11:13 ` Jiri Slaby (SUSE)
2025-04-25 11:13 ` [PATCH 6/6] serial: 8250: unexport serial8250_rpm_*() functions Jiri Slaby (SUSE)
5 siblings, 0 replies; 7+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-04-25 11:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 6/6] serial: 8250: unexport serial8250_rpm_*() functions
2025-04-25 11:13 [PATCH 0/6] tty: spring cleanup Jiri Slaby (SUSE)
` (4 preceding siblings ...)
2025-04-25 11:13 ` [PATCH 5/6] serial: use uart_port_ref_lock() helper Jiri Slaby (SUSE)
@ 2025-04-25 11:13 ` Jiri Slaby (SUSE)
5 siblings, 0 replies; 7+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-04-25 11:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread