2023-09-15 00:34:42

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 00/74] serial: wrappers for uart port lock

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

Provide and use wrapper functions for spin_[un]lock*(port->lock)
invocations so that the console mechanics can be applied later on at a
single place and does not require to copy the same logic all over the
drivers.

Patch 1 adds the wrapper functions.

Patches 2-74 switch all uart port locking call sites to use the new
wrappers. These patches were automatically generated using coccinelle.
The 2 used coccinelle scripts are included below and executed as
follows:

$ spatch --sp-file uartlock-1.cocci $FILE
$ spatch --sp-file uartlock-2.cocci --recursive-includes $FILE

This series brings no functional change.

Patches 2-74 contain identical commit message bodies. Feel free to
fold them into a single commit if that seems more reasonable.

Thomas Gleixner (74):
serial: core: Provide port lock wrappers
serial: core: Use lock wrappers
serial: 21285: Use port lock wrappers
serial: 8250_aspeed_vuart: Use port lock wrappers
serial: 8250_bcm7271: Use port lock wrappers
serial: 8250: Use port lock wrappers
serial: 8250_dma: Use port lock wrappers
serial: 8250_dw: Use port lock wrappers
serial: 8250_exar: Use port lock wrappers
serial: 8250_fsl: Use port lock wrappers
serial: 8250_mtk: Use port lock wrappers
serial: 8250_omap: Use port lock wrappers
serial: 8250_pci1xxxx: Use port lock wrappers
serial: altera_jtaguart: Use port lock wrappers
serial: altera_uart: Use port lock wrappers
serial: amba-pl010: Use port lock wrappers
serial: amba-pl011: Use port lock wrappers
serial: apb: Use port lock wrappers
serial: ar933x: Use port lock wrappers
serial: arc_uart: Use port lock wrappers
serial: atmel: Use port lock wrappers
serial: bcm63xx-uart: Use port lock wrappers
serial: cpm_uart: Use port lock wrappers
serial: digicolor: Use port lock wrappers
serial: dz: Use port lock wrappers
serial: linflexuart: Use port lock wrappers
serial: fsl_lpuart: Use port lock wrappers
serial: icom: Use port lock wrappers
serial: imx: Use port lock wrappers
serial: ip22zilog: Use port lock wrappers
serial: jsm: Use port lock wrappers
serial: liteuart: Use port lock wrappers
serial: lpc32xx_hs: Use port lock wrappers
serial: ma35d1: Use port lock wrappers
serial: mcf: Use port lock wrappers
serial: men_z135_uart: Use port lock wrappers
serial: meson: Use port lock wrappers
serial: milbeaut_usio: Use port lock wrappers
serial: mpc52xx: Use port lock wrappers
serial: mps2-uart: Use port lock wrappers
serial: msm: Use port lock wrappers
serial: mvebu-uart: Use port lock wrappers
serial: omap: Use port lock wrappers
serial: owl: Use port lock wrappers
serial: pch: Use port lock wrappers
serial: pic32: Use port lock wrappers
serial: pmac_zilog: Use port lock wrappers
serial: pxa: Use port lock wrappers
serial: qcom-geni: Use port lock wrappers
serial: rda: Use port lock wrappers
serial: rp2: Use port lock wrappers
serial: sa1100: Use port lock wrappers
serial: samsung_tty: Use port lock wrappers
serial: sb1250-duart: Use port lock wrappers
serial: sc16is7xx: Use port lock wrappers
serial: tegra: Use port lock wrappers
serial: core: Use port lock wrappers
serial: mctrl_gpio: Use port lock wrappers
serial: txx9: Use port lock wrappers
serial: sh-sci: Use port lock wrappers
serial: sifive: Use port lock wrappers
serial: sprd: Use port lock wrappers
serial: st-asc: Use port lock wrappers
serial: stm32: Use port lock wrappers
serial: sunhv: Use port lock wrappers
serial: sunplus-uart: Use port lock wrappers
serial: sunsab: Use port lock wrappers
serial: sunsu: Use port lock wrappers
serial: sunzilog: Use port lock wrappers
serial: timbuart: Use port lock wrappers
serial: uartlite: Use port lock wrappers
serial: ucc_uart: Use port lock wrappers
serial: vt8500: Use port lock wrappers
serial: xilinx_uartps: Use port lock wrappers

drivers/tty/serial/21285.c | 8 +-
drivers/tty/serial/8250/8250_aspeed_vuart.c | 6 +-
drivers/tty/serial/8250/8250_bcm7271.c | 28 +++---
drivers/tty/serial/8250/8250_core.c | 12 +--
drivers/tty/serial/8250/8250_dma.c | 8 +-
drivers/tty/serial/8250/8250_dw.c | 8 +-
drivers/tty/serial/8250/8250_exar.c | 4 +-
drivers/tty/serial/8250/8250_fsl.c | 6 +-
drivers/tty/serial/8250/8250_mtk.c | 8 +-
drivers/tty/serial/8250/8250_omap.c | 52 +++++-----
drivers/tty/serial/8250/8250_pci1xxxx.c | 8 +-
drivers/tty/serial/8250/8250_port.c | 100 ++++++++++----------
drivers/tty/serial/altera_jtaguart.c | 28 +++---
drivers/tty/serial/altera_uart.c | 20 ++--
drivers/tty/serial/amba-pl010.c | 20 ++--
drivers/tty/serial/amba-pl011.c | 72 +++++++-------
drivers/tty/serial/apbuart.c | 8 +-
drivers/tty/serial/ar933x_uart.c | 26 ++---
drivers/tty/serial/arc_uart.c | 16 ++--
drivers/tty/serial/atmel_serial.c | 24 ++---
drivers/tty/serial/bcm63xx_uart.c | 22 ++---
drivers/tty/serial/cpm_uart.c | 8 +-
drivers/tty/serial/digicolor-usart.c | 18 ++--
drivers/tty/serial/dz.c | 32 +++----
drivers/tty/serial/fsl_linflexuart.c | 26 ++---
drivers/tty/serial/fsl_lpuart.c | 88 ++++++++---------
drivers/tty/serial/icom.c | 26 ++---
drivers/tty/serial/imx.c | 84 ++++++++--------
drivers/tty/serial/ip22zilog.c | 36 +++----
drivers/tty/serial/jsm/jsm_neo.c | 4 +-
drivers/tty/serial/jsm/jsm_tty.c | 16 ++--
drivers/tty/serial/liteuart.c | 20 ++--
drivers/tty/serial/lpc32xx_hs.c | 26 ++---
drivers/tty/serial/ma35d1_serial.c | 22 ++---
drivers/tty/serial/mcf.c | 20 ++--
drivers/tty/serial/men_z135_uart.c | 8 +-
drivers/tty/serial/meson_uart.c | 30 +++---
drivers/tty/serial/milbeaut_usio.c | 16 ++--
drivers/tty/serial/mpc52xx_uart.c | 12 +--
drivers/tty/serial/mps2-uart.c | 16 ++--
drivers/tty/serial/msm_serial.c | 38 ++++----
drivers/tty/serial/mvebu-uart.c | 18 ++--
drivers/tty/serial/omap-serial.c | 38 ++++----
drivers/tty/serial/owl-uart.c | 26 ++---
drivers/tty/serial/pch_uart.c | 10 +-
drivers/tty/serial/pic32_uart.c | 20 ++--
drivers/tty/serial/pmac_zilog.c | 52 +++++-----
drivers/tty/serial/pxa.c | 30 +++---
drivers/tty/serial/qcom_geni_serial.c | 8 +-
drivers/tty/serial/rda-uart.c | 34 +++----
drivers/tty/serial/rp2.c | 20 ++--
drivers/tty/serial/sa1100.c | 20 ++--
drivers/tty/serial/samsung_tty.c | 50 +++++-----
drivers/tty/serial/sb1250-duart.c | 12 +--
drivers/tty/serial/sc16is7xx.c | 40 ++++----
drivers/tty/serial/serial-tegra.c | 32 +++----
drivers/tty/serial/serial_core.c | 88 ++++++++---------
drivers/tty/serial/serial_mctrl_gpio.c | 4 +-
drivers/tty/serial/serial_port.c | 4 +-
drivers/tty/serial/serial_txx9.c | 26 ++---
drivers/tty/serial/sh-sci.c | 68 ++++++-------
drivers/tty/serial/sifive.c | 16 ++--
drivers/tty/serial/sprd_serial.c | 30 +++---
drivers/tty/serial/st-asc.c | 18 ++--
drivers/tty/serial/stm32-usart.c | 38 ++++----
drivers/tty/serial/sunhv.c | 28 +++---
drivers/tty/serial/sunplus-uart.c | 26 ++---
drivers/tty/serial/sunsab.c | 34 +++----
drivers/tty/serial/sunsu.c | 46 ++++-----
drivers/tty/serial/sunzilog.c | 42 ++++----
drivers/tty/serial/timbuart.c | 8 +-
drivers/tty/serial/uartlite.c | 18 ++--
drivers/tty/serial/ucc_uart.c | 4 +-
drivers/tty/serial/vt8500_serial.c | 8 +-
drivers/tty/serial/xilinx_uartps.c | 56 +++++------
include/linux/serial_core.h | 91 ++++++++++++++++--
76 files changed, 1086 insertions(+), 1007 deletions(-)


base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
--
2.39.2

--------------8<--------------
// uartlock-1.cocci

@r1@
struct uart_port *U;
@@

-spin_lock(&U->lock)
+uart_port_lock(U)

@r2@
struct uart_port *U;
@@

-spin_lock_irq(&U->lock)
+uart_port_lock_irq(U)

@r3@
struct uart_port *U;
identifier F;
@@

-spin_lock_irqsave(&U->lock, F)
+uart_port_lock_irqsave(U, &F)

@r4@
struct uart_port *U;
@@

-spin_unlock(&U->lock)
+uart_port_unlock(U)

@r5@
struct uart_port *U;
@@

-spin_unlock_irq(&U->lock)
+uart_port_unlock_irq(U)

@r6@
struct uart_port *U;
identifier F;
@@

-spin_unlock_irqrestore(&U->lock, F)
+uart_port_unlock_irqrestore(U, F)

@r7@
struct uart_port *U;
@@

-spin_trylock(&U->lock)
+uart_port_trylock(U)

@r8@
struct uart_port *U;
identifier F;
@@

-spin_trylock_irqsave(&U->lock, F)
+uart_port_trylock_irqsave(U, &F)

--------------8<--------------
// uartlock-2.cocci

@r10@
type T1;
identifier U;
@@

T1 {
...
struct uart_port U;
...
};

@r11@
r10.T1 *E;
identifier r10.U;
@@

-spin_lock(&E->U.lock)
+uart_port_lock(&E->U)

@r12@
r10.T1 *E;
identifier r10.U;
@@

-spin_lock_irq(&E->U.lock)
+uart_port_lock_irq(&E->U)

@r13@
r10.T1 *E;
identifier r10.U;
identifier F;
@@

-spin_lock_irqsave(&E->U.lock, F)
+uart_port_lock_irqsave(&E->U, &F)

@r14@
r10.T1 *E;
identifier r10.U;
@@

-spin_unlock(&E->U.lock)
+uart_port_unlock(&E->U)

@r15@
r10.T1 *E;
identifier r10.U;
@@

-spin_unlock_irq(&E->U.lock)
+uart_port_unlock_irq(&E->U)

@r16@
r10.T1 *E;
identifier r10.U;
identifier F;
@@

-spin_unlock_irqrestore(&E->U.lock, F)
+uart_port_unlock_irqrestore(&E->U, F)

@r17@
r10.T1 *E;
identifier r10.U;
@@

-spin_trylock(&E->U.lock)
+uart_port_trylock(&E->U)

@r18@
r10.T1 *E;
identifier r10.U;
identifier F;
@@

-spin_trylock_irqsave(&E->U.lock, F)
+uart_port_trylock_irqsave(&E->U, &F)


2023-09-15 00:46:30

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 63/74] serial: st-asc: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/st-asc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index 92b9f6894006..a821f5d76a26 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -319,7 +319,7 @@ static irqreturn_t asc_interrupt(int irq, void *ptr)
struct uart_port *port = ptr;
u32 status;

- spin_lock(&port->lock);
+ uart_port_lock(port);

status = asc_in(port, ASC_STA);

@@ -334,7 +334,7 @@ static irqreturn_t asc_interrupt(int irq, void *ptr)
asc_transmit_chars(port);
}

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_HANDLED;
}
@@ -452,10 +452,10 @@ static void asc_pm(struct uart_port *port, unsigned int state,
* we can come to turning it off. Note this is not called with
* the port spinlock held.
*/
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
ctl = asc_in(port, ASC_CTL) & ~ASC_CTL_RUN;
asc_out(port, ASC_CTL, ctl);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
clk_disable_unprepare(ascport->clk);
break;
}
@@ -480,7 +480,7 @@ static void asc_set_termios(struct uart_port *port, struct ktermios *termios,
baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16);
cflag = termios->c_cflag;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* read control register */
ctrl_val = asc_in(port, ASC_CTL);
@@ -594,7 +594,7 @@ static void asc_set_termios(struct uart_port *port, struct ktermios *termios,
/* write final value and enable port */
asc_out(port, ASC_CTL, (ctrl_val | ASC_CTL_RUN));

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *asc_type(struct uart_port *port)
@@ -849,9 +849,9 @@ static void asc_console_write(struct console *co, const char *s, unsigned count)
if (port->sysrq)
locked = 0; /* asc_interrupt has already claimed the lock */
else if (oops_in_progress)
- locked = spin_trylock_irqsave(&port->lock, flags);
+ locked = uart_port_trylock_irqsave(port, &flags);
else
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/*
* Disable interrupts so we don't get the IRQ line bouncing
@@ -869,7 +869,7 @@ static void asc_console_write(struct console *co, const char *s, unsigned count)
asc_out(port, ASC_INTEN, intenable);

if (locked)
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int asc_console_setup(struct console *co, char *options)
--
2.39.2

2023-09-15 00:47:59

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 33/74] serial: lpc32xx_hs: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/lpc32xx_hs.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/lpc32xx_hs.c b/drivers/tty/serial/lpc32xx_hs.c
index b38fe4728c26..5149a947b7fe 100644
--- a/drivers/tty/serial/lpc32xx_hs.c
+++ b/drivers/tty/serial/lpc32xx_hs.c
@@ -140,15 +140,15 @@ static void lpc32xx_hsuart_console_write(struct console *co, const char *s,
if (up->port.sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock(&up->port.lock);
+ locked = uart_port_trylock(&up->port);
else
- spin_lock(&up->port.lock);
+ uart_port_lock(&up->port);

uart_console_write(&up->port, s, count, lpc32xx_hsuart_console_putchar);
wait_for_xmit_empty(&up->port);

if (locked)
- spin_unlock(&up->port.lock);
+ uart_port_unlock(&up->port);
local_irq_restore(flags);
}

@@ -298,7 +298,7 @@ static irqreturn_t serial_lpc32xx_interrupt(int irq, void *dev_id)
struct tty_port *tport = &port->state->port;
u32 status;

- spin_lock(&port->lock);
+ uart_port_lock(port);

/* Read UART status and clear latched interrupts */
status = readl(LPC32XX_HSUART_IIR(port->membase));
@@ -333,7 +333,7 @@ static irqreturn_t serial_lpc32xx_interrupt(int irq, void *dev_id)
__serial_lpc32xx_tx(port);
}

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_HANDLED;
}
@@ -404,14 +404,14 @@ static void serial_lpc32xx_break_ctl(struct uart_port *port,
unsigned long flags;
u32 tmp;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
tmp = readl(LPC32XX_HSUART_CTRL(port->membase));
if (break_state != 0)
tmp |= LPC32XX_HSU_BREAK;
else
tmp &= ~LPC32XX_HSU_BREAK;
writel(tmp, LPC32XX_HSUART_CTRL(port->membase));
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/* port->lock is not held. */
@@ -421,7 +421,7 @@ static int serial_lpc32xx_startup(struct uart_port *port)
unsigned long flags;
u32 tmp;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

__serial_uart_flush(port);

@@ -441,7 +441,7 @@ static int serial_lpc32xx_startup(struct uart_port *port)

lpc32xx_loopback_set(port->mapbase, 0); /* get out of loopback mode */

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

retval = request_irq(port->irq, serial_lpc32xx_interrupt,
0, MODNAME, port);
@@ -458,7 +458,7 @@ static void serial_lpc32xx_shutdown(struct uart_port *port)
u32 tmp;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

tmp = LPC32XX_HSU_TX_TL8B | LPC32XX_HSU_RX_TL32B |
LPC32XX_HSU_OFFSET(20) | LPC32XX_HSU_TMO_INACT_4B;
@@ -466,7 +466,7 @@ static void serial_lpc32xx_shutdown(struct uart_port *port)

lpc32xx_loopback_set(port->mapbase, 1); /* go to loopback mode */

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

free_irq(port->irq, port);
}
@@ -491,7 +491,7 @@ static void serial_lpc32xx_set_termios(struct uart_port *port,

quot = __serial_get_clock_div(port->uartclk, baud);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Ignore characters? */
tmp = readl(LPC32XX_HSUART_CTRL(port->membase));
@@ -505,7 +505,7 @@ static void serial_lpc32xx_set_termios(struct uart_port *port,

uart_update_timeout(port, termios->c_cflag, baud);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
--
2.39.2

2023-09-15 00:54:50

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 55/74] serial: sc16is7xx: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/sc16is7xx.c | 40 +++++++++++++++++-----------------
1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index f61d98e09dc3..9d8307f5fc36 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -667,9 +667,9 @@ static void sc16is7xx_handle_tx(struct uart_port *port)
}

if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
sc16is7xx_stop_tx(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
return;
}

@@ -695,13 +695,13 @@ static void sc16is7xx_handle_tx(struct uart_port *port)
sc16is7xx_fifo_write(port, to_send);
}

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
uart_write_wakeup(port);

if (uart_circ_empty(xmit))
sc16is7xx_stop_tx(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static unsigned int sc16is7xx_get_hwmctrl(struct uart_port *port)
@@ -733,7 +733,7 @@ static void sc16is7xx_update_mlines(struct sc16is7xx_one *one)

one->old_mctrl = status;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
if ((changed & TIOCM_RNG) && (status & TIOCM_RNG))
port->icount.rng++;
if (changed & TIOCM_DSR)
@@ -744,7 +744,7 @@ static void sc16is7xx_update_mlines(struct sc16is7xx_one *one)
uart_handle_cts_change(port, status & TIOCM_CTS);

wake_up_interruptible(&port->state->port.delta_msr_wait);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static bool sc16is7xx_port_irq(struct sc16is7xx_port *s, int portno)
@@ -823,9 +823,9 @@ static void sc16is7xx_tx_proc(struct kthread_work *ws)
sc16is7xx_handle_tx(port);
mutex_unlock(&s->efr_lock);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
sc16is7xx_ier_set(port, SC16IS7XX_IER_THRI_BIT);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void sc16is7xx_reconf_rs485(struct uart_port *port)
@@ -836,14 +836,14 @@ static void sc16is7xx_reconf_rs485(struct uart_port *port)
struct serial_rs485 *rs485 = &port->rs485;
unsigned long irqflags;

- spin_lock_irqsave(&port->lock, irqflags);
+ uart_port_lock_irqsave(port, &irqflags);
if (rs485->flags & SER_RS485_ENABLED) {
efcr |= SC16IS7XX_EFCR_AUTO_RS485_BIT;

if (rs485->flags & SER_RS485_RTS_AFTER_SEND)
efcr |= SC16IS7XX_EFCR_RTS_INVERT_BIT;
}
- spin_unlock_irqrestore(&port->lock, irqflags);
+ uart_port_unlock_irqrestore(port, irqflags);

sc16is7xx_port_update(port, SC16IS7XX_EFCR_REG, mask, efcr);
}
@@ -854,10 +854,10 @@ static void sc16is7xx_reg_proc(struct kthread_work *ws)
struct sc16is7xx_one_config config;
unsigned long irqflags;

- spin_lock_irqsave(&one->port.lock, irqflags);
+ uart_port_lock_irqsave(&one->port, &irqflags);
config = one->config;
memset(&one->config, 0, sizeof(one->config));
- spin_unlock_irqrestore(&one->port.lock, irqflags);
+ uart_port_unlock_irqrestore(&one->port, irqflags);

if (config.flags & SC16IS7XX_RECONF_MD) {
u8 mcr = 0;
@@ -963,18 +963,18 @@ static void sc16is7xx_throttle(struct uart_port *port)
* value set in MCR register. Stop reading data from RX FIFO so the
* AutoRTS feature will de-activate RTS output.
*/
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
sc16is7xx_ier_clear(port, SC16IS7XX_IER_RDI_BIT);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void sc16is7xx_unthrottle(struct uart_port *port)
{
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
sc16is7xx_ier_set(port, SC16IS7XX_IER_RDI_BIT);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static unsigned int sc16is7xx_tx_empty(struct uart_port *port)
@@ -1113,7 +1113,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
/* Setup baudrate generator */
baud = sc16is7xx_set_baud(port, baud);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Update timeout according to new baud rate */
uart_update_timeout(port, termios->c_cflag, baud);
@@ -1121,7 +1121,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
if (UART_ENABLE_MS(port, termios->c_cflag))
sc16is7xx_enable_ms(port);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int sc16is7xx_config_rs485(struct uart_port *port, struct ktermios *termios,
@@ -1208,9 +1208,9 @@ static int sc16is7xx_startup(struct uart_port *port)
sc16is7xx_port_write(port, SC16IS7XX_IER_REG, val);

/* Enable modem status polling */
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
sc16is7xx_enable_ms(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return 0;
}
--
2.39.2

2023-09-15 00:55:26

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 57/74] serial: core: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/serial_core.c | 88 ++++++++++++++++----------------
drivers/tty/serial/serial_port.c | 4 +-
2 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 7bdc21d5e13b..b32bbd7aa3d3 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -79,7 +79,7 @@ static inline void uart_port_deref(struct uart_port *uport)
({ \
struct uart_port *__uport = uart_port_ref(state); \
if (__uport) \
- spin_lock_irqsave(&__uport->lock, flags); \
+ uart_port_lock_irqsave(__uport, &flags); \
__uport; \
})

@@ -87,7 +87,7 @@ static inline void uart_port_deref(struct uart_port *uport)
({ \
struct uart_port *__uport = uport; \
if (__uport) { \
- spin_unlock_irqrestore(&__uport->lock, flags); \
+ uart_port_unlock_irqrestore(__uport, flags); \
uart_port_deref(__uport); \
} \
})
@@ -179,12 +179,12 @@ uart_update_mctrl(struct uart_port *port, unsigned int set, unsigned int clear)
unsigned long flags;
unsigned int old;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
old = port->mctrl;
port->mctrl = (old & ~clear) | set;
if (old != port->mctrl && !(port->rs485.flags & SER_RS485_ENABLED))
port->ops->set_mctrl(port, port->mctrl);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

#define uart_set_mctrl(port, set) uart_update_mctrl(port, set, 0)
@@ -219,7 +219,7 @@ static void uart_change_line_settings(struct tty_struct *tty, struct uart_state
/*
* Set modem status enables based on termios cflag
*/
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
if (termios->c_cflag & CRTSCTS)
uport->status |= UPSTAT_CTS_ENABLE;
else
@@ -240,7 +240,7 @@ static void uart_change_line_settings(struct tty_struct *tty, struct uart_state
else
__uart_start(state);
}
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);
}

/*
@@ -702,11 +702,11 @@ static void uart_send_xchar(struct tty_struct *tty, char ch)
if (port->ops->send_xchar)
port->ops->send_xchar(port, ch);
else {
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
port->x_char = ch;
if (ch)
port->ops->start_tx(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}
uart_port_deref(port);
}
@@ -1085,9 +1085,9 @@ static int uart_tiocmget(struct tty_struct *tty)

if (!tty_io_error(tty)) {
result = uport->mctrl;
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
result |= uport->ops->get_mctrl(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);
}
out:
mutex_unlock(&port->mutex);
@@ -1223,16 +1223,16 @@ static int uart_wait_modem_status(struct uart_state *state, unsigned long arg)
uport = uart_port_ref(state);
if (!uport)
return -EIO;
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
memcpy(&cprev, &uport->icount, sizeof(struct uart_icount));
uart_enable_ms(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);

add_wait_queue(&port->delta_msr_wait, &wait);
for (;;) {
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);

set_current_state(TASK_INTERRUPTIBLE);

@@ -1277,9 +1277,9 @@ static int uart_get_icount(struct tty_struct *tty,
uport = uart_port_ref(state);
if (!uport)
return -EIO;
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);
uart_port_deref(uport);

icount->cts = cnow.cts;
@@ -1422,9 +1422,9 @@ static int uart_get_rs485_config(struct uart_port *port,
unsigned long flags;
struct serial_rs485 aux;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
aux = port->rs485;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

if (copy_to_user(rs485, &aux, sizeof(aux)))
return -EFAULT;
@@ -1451,7 +1451,7 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
uart_sanitize_serial_rs485(port, &rs485);
uart_set_rs485_termination(port, &rs485);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
ret = port->rs485_config(port, &tty->termios, &rs485);
if (!ret) {
port->rs485 = rs485;
@@ -1460,7 +1460,7 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
if (!(rs485.flags & SER_RS485_ENABLED))
port->ops->set_mctrl(port, port->mctrl);
}
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
if (ret)
return ret;

@@ -1479,9 +1479,9 @@ static int uart_get_iso7816_config(struct uart_port *port,
if (!port->iso7816_config)
return -ENOTTY;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
aux = port->iso7816;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

if (copy_to_user(iso7816, &aux, sizeof(aux)))
return -EFAULT;
@@ -1510,9 +1510,9 @@ static int uart_set_iso7816_config(struct uart_port *port,
if (iso7816.reserved[i])
return -EINVAL;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
ret = port->iso7816_config(port, &iso7816);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
if (ret)
return ret;

@@ -1729,9 +1729,9 @@ static void uart_tty_port_shutdown(struct tty_port *port)
if (WARN(!uport, "detached port still initialized!\n"))
return;

- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
uport->ops->stop_rx(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);

uart_port_shutdown(port);

@@ -1745,10 +1745,10 @@ static void uart_tty_port_shutdown(struct tty_port *port)
/*
* Free the transmit buffer.
*/
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
buf = state->xmit.buf;
state->xmit.buf = NULL;
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);

free_page((unsigned long)buf);

@@ -1891,10 +1891,10 @@ static bool uart_carrier_raised(struct tty_port *port)
*/
if (WARN_ON(!uport))
return true;
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
uart_enable_ms(uport);
mctrl = uport->ops->get_mctrl(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);
uart_port_deref(uport);

return mctrl & TIOCM_CAR;
@@ -2011,9 +2011,9 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
pm_state = state->pm_state;
if (pm_state != UART_PM_STATE_ON)
uart_change_pm(state, UART_PM_STATE_ON);
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
status = uport->ops->get_mctrl(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);
if (pm_state != UART_PM_STATE_ON)
uart_change_pm(state, pm_state);

@@ -2352,9 +2352,9 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
*/
if (!console_suspend_enabled && uart_console(uport)) {
if (uport->ops->start_rx) {
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
uport->ops->stop_rx(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);
}
goto unlock;
}
@@ -2369,7 +2369,7 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
tty_port_set_suspended(port, true);
tty_port_set_initialized(port, false);

- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
ops->stop_tx(uport);
if (!(uport->rs485.flags & SER_RS485_ENABLED))
ops->set_mctrl(uport, 0);
@@ -2377,7 +2377,7 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
mctrl = uport->mctrl;
uport->mctrl = 0;
ops->stop_rx(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);

/*
* Wait for the transmitter to empty.
@@ -2449,9 +2449,9 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
uart_change_pm(state, UART_PM_STATE_ON);
uport->ops->set_termios(uport, &termios, NULL);
if (!console_suspend_enabled && uport->ops->start_rx) {
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
uport->ops->start_rx(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);
}
if (console_suspend_enabled)
console_start(uport->cons);
@@ -2462,10 +2462,10 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
int ret;

uart_change_pm(state, UART_PM_STATE_ON);
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
if (!(uport->rs485.flags & SER_RS485_ENABLED))
ops->set_mctrl(uport, 0);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);
if (console_suspend_enabled || !uart_console(uport)) {
/* Protected by port mutex for now */
struct tty_struct *tty = port->tty;
@@ -2474,13 +2474,13 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
if (ret == 0) {
if (tty)
uart_change_line_settings(tty, state, NULL);
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(uport);
if (!(uport->rs485.flags & SER_RS485_ENABLED))
ops->set_mctrl(uport, uport->mctrl);
else
uart_rs485_config(uport);
ops->start_tx(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(uport);
tty_port_set_initialized(port, true);
} else {
/*
@@ -2583,13 +2583,13 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
* keep the DTR setting that is set in uart_set_options()
* We probably don't need a spinlock around this, but
*/
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
port->mctrl &= TIOCM_DTR;
if (!(port->rs485.flags & SER_RS485_ENABLED))
port->ops->set_mctrl(port, port->mctrl);
else
uart_rs485_config(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

/*
* If this driver supports console, and it hasn't been
diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c
index 862423237007..88975a4df306 100644
--- a/drivers/tty/serial/serial_port.c
+++ b/drivers/tty/serial/serial_port.c
@@ -35,10 +35,10 @@ static int serial_port_runtime_resume(struct device *dev)
goto out;

/* Flush any pending TX for the port */
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
if (__serial_port_busy(port))
port->ops->start_tx(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

out:
pm_runtime_mark_last_busy(dev);
--
2.39.2

2023-09-15 00:58:27

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 41/74] serial: msm: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/msm_serial.c | 38 ++++++++++++++++-----------------
1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index 90953e679e38..597264b546fd 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -444,7 +444,7 @@ static void msm_complete_tx_dma(void *args)
unsigned int count;
u32 val;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Already stopped */
if (!dma->count)
@@ -476,7 +476,7 @@ static void msm_complete_tx_dma(void *args)

msm_handle_tx(port);
done:
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
@@ -549,7 +549,7 @@ static void msm_complete_rx_dma(void *args)
unsigned long flags;
u32 val;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Already stopped */
if (!dma->count)
@@ -587,16 +587,16 @@ static void msm_complete_rx_dma(void *args)
if (!(port->read_status_mask & MSM_UART_SR_RX_BREAK))
flag = TTY_NORMAL;

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
sysrq = uart_handle_sysrq_char(port, dma->virt[i]);
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
if (!sysrq)
tty_insert_flip_char(tport, dma->virt[i], flag);
}

msm_start_rx_dma(msm_port);
done:
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

if (count)
tty_flip_buffer_push(tport);
@@ -762,9 +762,9 @@ static void msm_handle_rx_dm(struct uart_port *port, unsigned int misr)
if (!(port->read_status_mask & MSM_UART_SR_RX_BREAK))
flag = TTY_NORMAL;

- spin_unlock(&port->lock);
+ uart_port_unlock(port);
sysrq = uart_handle_sysrq_char(port, buf[i]);
- spin_lock(&port->lock);
+ uart_port_lock(port);
if (!sysrq)
tty_insert_flip_char(tport, buf[i], flag);
}
@@ -824,9 +824,9 @@ static void msm_handle_rx(struct uart_port *port)
else if (sr & MSM_UART_SR_PAR_FRAME_ERR)
flag = TTY_FRAME;

- spin_unlock(&port->lock);
+ uart_port_unlock(port);
sysrq = uart_handle_sysrq_char(port, c);
- spin_lock(&port->lock);
+ uart_port_lock(port);
if (!sysrq)
tty_insert_flip_char(tport, c, flag);
}
@@ -951,7 +951,7 @@ static irqreturn_t msm_uart_irq(int irq, void *dev_id)
unsigned int misr;
u32 val;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
misr = msm_read(port, MSM_UART_MISR);
msm_write(port, 0, MSM_UART_IMR); /* disable interrupt */

@@ -983,7 +983,7 @@ static irqreturn_t msm_uart_irq(int irq, void *dev_id)
msm_handle_delta_cts(port);

msm_write(port, msm_port->imr, MSM_UART_IMR); /* restore interrupt */
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return IRQ_HANDLED;
}
@@ -1128,13 +1128,13 @@ static int msm_set_baud_rate(struct uart_port *port, unsigned int baud,
unsigned long flags, rate;

flags = *saved_flags;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

entry = msm_find_best_baud(port, baud, &rate);
clk_set_rate(msm_port->clk, rate);
baud = rate / 16 / entry->divisor;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
*saved_flags = flags;
port->uartclk = rate;

@@ -1266,7 +1266,7 @@ static void msm_set_termios(struct uart_port *port, struct ktermios *termios,
unsigned long flags;
unsigned int baud, mr;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

if (dma->chan) /* Terminate if any */
msm_stop_dma(port, dma);
@@ -1338,7 +1338,7 @@ static void msm_set_termios(struct uart_port *port, struct ktermios *termios,
/* Try to use DMA */
msm_start_rx_dma(msm_port);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *msm_type(struct uart_port *port)
@@ -1620,9 +1620,9 @@ static void __msm_console_write(struct uart_port *port, const char *s,
if (port->sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock(&port->lock);
+ locked = uart_port_trylock(port);
else
- spin_lock(&port->lock);
+ uart_port_lock(port);

if (is_uartdm)
msm_reset_dm_count(port, count);
@@ -1661,7 +1661,7 @@ static void __msm_console_write(struct uart_port *port, const char *s,
}

if (locked)
- spin_unlock(&port->lock);
+ uart_port_unlock(port);

local_irq_restore(flags);
}
--
2.39.2

2023-09-15 00:58:34

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 52/74] serial: sa1100: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/sa1100.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index ad011f1e2f4d..be7bcd75d9f4 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -115,9 +115,9 @@ static void sa1100_timeout(struct timer_list *t)
unsigned long flags;

if (sport->port.state) {
- spin_lock_irqsave(&sport->port.lock, flags);
+ uart_port_lock_irqsave(&sport->port, &flags);
sa1100_mctrl_check(sport);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ uart_port_unlock_irqrestore(&sport->port, flags);

mod_timer(&sport->timer, jiffies + MCTRL_TIMEOUT);
}
@@ -247,7 +247,7 @@ static irqreturn_t sa1100_int(int irq, void *dev_id)
struct sa1100_port *sport = dev_id;
unsigned int status, pass_counter = 0;

- spin_lock(&sport->port.lock);
+ uart_port_lock(&sport->port);
status = UART_GET_UTSR0(sport);
status &= SM_TO_UTSR0(sport->port.read_status_mask) | ~UTSR0_TFS;
do {
@@ -276,7 +276,7 @@ static irqreturn_t sa1100_int(int irq, void *dev_id)
status &= SM_TO_UTSR0(sport->port.read_status_mask) |
~UTSR0_TFS;
} while (status & (UTSR0_TFS | UTSR0_RFS | UTSR0_RID));
- spin_unlock(&sport->port.lock);
+ uart_port_unlock(&sport->port);

return IRQ_HANDLED;
}
@@ -321,14 +321,14 @@ static void sa1100_break_ctl(struct uart_port *port, int break_state)
unsigned long flags;
unsigned int utcr3;

- spin_lock_irqsave(&sport->port.lock, flags);
+ uart_port_lock_irqsave(&sport->port, &flags);
utcr3 = UART_GET_UTCR3(sport);
if (break_state == -1)
utcr3 |= UTCR3_BRK;
else
utcr3 &= ~UTCR3_BRK;
UART_PUT_UTCR3(sport, utcr3);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ uart_port_unlock_irqrestore(&sport->port, flags);
}

static int sa1100_startup(struct uart_port *port)
@@ -354,9 +354,9 @@ static int sa1100_startup(struct uart_port *port)
/*
* Enable modem status interrupts
*/
- spin_lock_irq(&sport->port.lock);
+ uart_port_lock_irq(&sport->port);
sa1100_enable_ms(&sport->port);
- spin_unlock_irq(&sport->port.lock);
+ uart_port_unlock_irq(&sport->port);

return 0;
}
@@ -423,7 +423,7 @@ sa1100_set_termios(struct uart_port *port, struct ktermios *termios,

del_timer_sync(&sport->timer);

- spin_lock_irqsave(&sport->port.lock, flags);
+ uart_port_lock_irqsave(&sport->port, &flags);

sport->port.read_status_mask &= UTSR0_TO_SM(UTSR0_TFS);
sport->port.read_status_mask |= UTSR1_TO_SM(UTSR1_ROR);
@@ -485,7 +485,7 @@ sa1100_set_termios(struct uart_port *port, struct ktermios *termios,
if (UART_ENABLE_MS(&sport->port, termios->c_cflag))
sa1100_enable_ms(&sport->port);

- spin_unlock_irqrestore(&sport->port.lock, flags);
+ uart_port_unlock_irqrestore(&sport->port, flags);
}

static const char *sa1100_type(struct uart_port *port)
--
2.39.2

2023-09-15 00:59:12

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 42/74] serial: mvebu-uart: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/mvebu-uart.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index ea924e9b913b..0255646bc175 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -187,9 +187,9 @@ static unsigned int mvebu_uart_tx_empty(struct uart_port *port)
unsigned long flags;
unsigned int st;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
st = readl(port->membase + UART_STAT);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return (st & STAT_TX_EMP) ? TIOCSER_TEMT : 0;
}
@@ -249,14 +249,14 @@ static void mvebu_uart_break_ctl(struct uart_port *port, int brk)
unsigned int ctl;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
ctl = readl(port->membase + UART_CTRL(port));
if (brk == -1)
ctl |= CTRL_SND_BRK_SEQ;
else
ctl &= ~CTRL_SND_BRK_SEQ;
writel(ctl, port->membase + UART_CTRL(port));
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
@@ -540,7 +540,7 @@ static void mvebu_uart_set_termios(struct uart_port *port,
unsigned long flags;
unsigned int baud, min_baud, max_baud;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

port->read_status_mask = STAT_RX_RDY(port) | STAT_OVR_ERR |
STAT_TX_RDY(port) | STAT_TX_FIFO_FUL;
@@ -589,7 +589,7 @@ static void mvebu_uart_set_termios(struct uart_port *port,
uart_update_timeout(port, termios->c_cflag, baud);
}

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *mvebu_uart_type(struct uart_port *port)
@@ -735,9 +735,9 @@ static void mvebu_uart_console_write(struct console *co, const char *s,
int locked = 1;

if (oops_in_progress)
- locked = spin_trylock_irqsave(&port->lock, flags);
+ locked = uart_port_trylock_irqsave(port, &flags);
else
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

ier = readl(port->membase + UART_CTRL(port)) & CTRL_BRK_INT;
intr = readl(port->membase + UART_INTR(port)) &
@@ -758,7 +758,7 @@ static void mvebu_uart_console_write(struct console *co, const char *s,
}

if (locked)
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int mvebu_uart_console_setup(struct console *co, char *options)
--
2.39.2

2023-09-15 01:01:08

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 04/74] serial: 8250_aspeed_vuart: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 4a9e71b2dbbc..021949f252f8 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -288,9 +288,9 @@ static void aspeed_vuart_set_throttle(struct uart_port *port, bool throttle)
struct uart_8250_port *up = up_to_u8250p(port);
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
__aspeed_vuart_set_throttle(up, throttle);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void aspeed_vuart_throttle(struct uart_port *port)
@@ -340,7 +340,7 @@ static int aspeed_vuart_handle_irq(struct uart_port *port)
if (iir & UART_IIR_NO_INT)
return 0;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

lsr = serial_port_in(port, UART_LSR);

--
2.39.2

2023-09-15 01:05:45

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 01/74] serial: core: Provide port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

Provide wrapper functions for spin_[un]lock*(port->lock) invocations so
that the console mechanics can be applied later on at a single place and
does not require to copy the same logic all over the drivers.

Signed-off-by: Thomas Gleixner <[email protected]>
---
include/linux/serial_core.h | 79 +++++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index bb6f073bc159..f1d5c0d1568c 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -588,6 +588,85 @@ struct uart_port {
void *private_data; /* generic platform data pointer */
};

+/**
+ * uart_port_lock - Lock the UART port
+ * @up: Pointer to UART port structure
+ */
+static inline void uart_port_lock(struct uart_port *up)
+{
+ spin_lock(&up->lock);
+}
+
+/**
+ * uart_port_lock_irq - Lock the UART port and disable interrupts
+ * @up: Pointer to UART port structure
+ */
+static inline void uart_port_lock_irq(struct uart_port *up)
+{
+ spin_lock_irq(&up->lock);
+}
+
+/**
+ * uart_port_lock_irqsave - Lock the UART port, save and disable interrupts
+ * @up: Pointer to UART port structure
+ * @flags: Pointer to interrupt flags storage
+ */
+static inline void uart_port_lock_irqsave(struct uart_port *up, unsigned long *flags)
+{
+ spin_lock_irqsave(&up->lock, *flags);
+}
+
+/**
+ * uart_port_trylock - Try to lock the UART port
+ * @up: Pointer to UART port structure
+ *
+ * Returns: True if lock was acquired, false otherwise
+ */
+static inline bool uart_port_trylock(struct uart_port *up)
+{
+ return spin_trylock(&up->lock);
+}
+
+/**
+ * uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
+ * @up: Pointer to UART port structure
+ * @flags: Pointer to interrupt flags storage
+ *
+ * Returns: True if lock was acquired, false otherwise
+ */
+static inline bool uart_port_trylock_irqsave(struct uart_port *up, unsigned long *flags)
+{
+ return spin_trylock_irqsave(&up->lock, *flags);
+}
+
+/**
+ * uart_port_unlock - Unlock the UART port
+ * @up: Pointer to UART port structure
+ */
+static inline void uart_port_unlock(struct uart_port *up)
+{
+ spin_unlock(&up->lock);
+}
+
+/**
+ * uart_port_unlock_irq - Unlock the UART port and re-enable interrupts
+ * @up: Pointer to UART port structure
+ */
+static inline void uart_port_unlock_irq(struct uart_port *up)
+{
+ spin_unlock_irq(&up->lock);
+}
+
+/**
+ * uart_port_lock_irqrestore - Unlock the UART port, restore interrupts
+ * @up: Pointer to UART port structure
+ * @flags: The saved interrupt flags for restore
+ */
+static inline void uart_port_unlock_irqrestore(struct uart_port *up, unsigned long flags)
+{
+ spin_unlock_irqrestore(&up->lock, flags);
+}
+
static inline int serial_port_in(struct uart_port *up, int offset)
{
return up->serial_in(up, offset);
--
2.39.2

2023-09-15 01:16:39

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 23/74] serial: cpm_uart: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/cpm_uart.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/cpm_uart.c b/drivers/tty/serial/cpm_uart.c
index 626423022d62..be4af6eda4c2 100644
--- a/drivers/tty/serial/cpm_uart.c
+++ b/drivers/tty/serial/cpm_uart.c
@@ -569,7 +569,7 @@ static void cpm_uart_set_termios(struct uart_port *port,
if ((termios->c_cflag & CREAD) == 0)
port->read_status_mask &= ~BD_SC_EMPTY;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

if (IS_SMC(pinfo)) {
unsigned int bits = tty_get_frame_size(termios->c_cflag);
@@ -609,7 +609,7 @@ static void cpm_uart_set_termios(struct uart_port *port,
clk_set_rate(pinfo->clk, baud);
else
cpm_setbrg(pinfo->brg - 1, baud);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *cpm_uart_type(struct uart_port *port)
@@ -1386,9 +1386,9 @@ static void cpm_uart_console_write(struct console *co, const char *s,
cpm_uart_early_write(pinfo, s, count, true);
local_irq_restore(flags);
} else {
- spin_lock_irqsave(&pinfo->port.lock, flags);
+ uart_port_lock_irqsave(&pinfo->port, &flags);
cpm_uart_early_write(pinfo, s, count, true);
- spin_unlock_irqrestore(&pinfo->port.lock, flags);
+ uart_port_unlock_irqrestore(&pinfo->port, flags);
}
}

--
2.39.2

2023-09-15 01:18:11

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 11/74] serial: 8250_mtk: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/8250/8250_mtk.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index 74da5676ce67..23457daae8a1 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -102,7 +102,7 @@ static void mtk8250_dma_rx_complete(void *param)
if (data->rx_status == DMA_RX_SHUTDOWN)
return;

- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);

dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
total = dma->rx_size - state.residue;
@@ -128,7 +128,7 @@ static void mtk8250_dma_rx_complete(void *param)

mtk8250_rx_dma(up);

- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);
}

static void mtk8250_rx_dma(struct uart_8250_port *up)
@@ -368,7 +368,7 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios,
* Ok, we're now changing the port state. Do it with
* interrupts disabled.
*/
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/*
* Update the per-port timeout.
@@ -416,7 +416,7 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios,
if (uart_console(port))
up->port.cons->cflag = termios->c_cflag;

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);
--
2.39.2

2023-09-15 01:22:07

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 69/74] serial: sunzilog: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/sunzilog.c | 42 +++++++++++++++++------------------
1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/sunzilog.c b/drivers/tty/serial/sunzilog.c
index c8c71c56264c..d3b5e864b727 100644
--- a/drivers/tty/serial/sunzilog.c
+++ b/drivers/tty/serial/sunzilog.c
@@ -531,7 +531,7 @@ static irqreturn_t sunzilog_interrupt(int irq, void *dev_id)
struct tty_port *port;
unsigned char r3;

- spin_lock(&up->port.lock);
+ uart_port_lock(&up->port);
r3 = read_zsreg(channel, R3);

/* Channel A */
@@ -548,7 +548,7 @@ static irqreturn_t sunzilog_interrupt(int irq, void *dev_id)
if (r3 & CHATxIP)
sunzilog_transmit_chars(up, channel);
}
- spin_unlock(&up->port.lock);
+ uart_port_unlock(&up->port);

if (port)
tty_flip_buffer_push(port);
@@ -557,7 +557,7 @@ static irqreturn_t sunzilog_interrupt(int irq, void *dev_id)
up = up->next;
channel = ZILOG_CHANNEL_FROM_PORT(&up->port);

- spin_lock(&up->port.lock);
+ uart_port_lock(&up->port);
port = NULL;
if (r3 & (CHBEXT | CHBTxIP | CHBRxIP)) {
writeb(RES_H_IUS, &channel->control);
@@ -571,7 +571,7 @@ static irqreturn_t sunzilog_interrupt(int irq, void *dev_id)
if (r3 & CHBTxIP)
sunzilog_transmit_chars(up, channel);
}
- spin_unlock(&up->port.lock);
+ uart_port_unlock(&up->port);

if (port)
tty_flip_buffer_push(port);
@@ -604,11 +604,11 @@ static unsigned int sunzilog_tx_empty(struct uart_port *port)
unsigned char status;
unsigned int ret;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

status = sunzilog_read_channel_status(port);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

if (status & Tx_BUF_EMP)
ret = TIOCSER_TEMT;
@@ -764,7 +764,7 @@ static void sunzilog_break_ctl(struct uart_port *port, int break_state)
else
clear_bits |= SND_BRK;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

new_reg = (up->curregs[R5] | set_bits) & ~clear_bits;
if (new_reg != up->curregs[R5]) {
@@ -774,7 +774,7 @@ static void sunzilog_break_ctl(struct uart_port *port, int break_state)
write_zsreg(channel, R5, up->curregs[R5]);
}

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void __sunzilog_startup(struct uart_sunzilog_port *up)
@@ -800,9 +800,9 @@ static int sunzilog_startup(struct uart_port *port)
if (ZS_IS_CONS(up))
return 0;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
__sunzilog_startup(up);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
return 0;
}

@@ -840,7 +840,7 @@ static void sunzilog_shutdown(struct uart_port *port)
if (ZS_IS_CONS(up))
return;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

channel = ZILOG_CHANNEL_FROM_PORT(port);

@@ -853,7 +853,7 @@ static void sunzilog_shutdown(struct uart_port *port)
up->curregs[R5] &= ~SND_BRK;
sunzilog_maybe_update_regs(up, channel);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/* Shared by TTY driver and serial console setup. The port lock is held
@@ -945,7 +945,7 @@ sunzilog_set_termios(struct uart_port *port, struct ktermios *termios,

baud = uart_get_baud_rate(port, termios, old, 1200, 76800);

- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);

brg = BPS_TO_BRG(baud, ZS_CLOCK / ZS_CLOCK_DIVISOR);

@@ -962,7 +962,7 @@ sunzilog_set_termios(struct uart_port *port, struct ktermios *termios,

uart_update_timeout(port, termios->c_cflag, baud);

- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);
}

static const char *sunzilog_type(struct uart_port *port)
@@ -1201,15 +1201,15 @@ sunzilog_console_write(struct console *con, const char *s, unsigned int count)
int locked = 1;

if (up->port.sysrq || oops_in_progress)
- locked = spin_trylock_irqsave(&up->port.lock, flags);
+ locked = uart_port_trylock_irqsave(&up->port, &flags);
else
- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);

uart_console_write(&up->port, s, count, sunzilog_putchar);
udelay(2);

if (locked)
- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);
}

static int __init sunzilog_console_setup(struct console *con, char *options)
@@ -1244,7 +1244,7 @@ static int __init sunzilog_console_setup(struct console *con, char *options)

brg = BPS_TO_BRG(baud, ZS_CLOCK / ZS_CLOCK_DIVISOR);

- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);

up->curregs[R15] |= BRKIE;
sunzilog_convert_to_zs(up, con->cflag, 0, brg);
@@ -1252,7 +1252,7 @@ static int __init sunzilog_console_setup(struct console *con, char *options)
sunzilog_set_mctrl(&up->port, TIOCM_DTR | TIOCM_RTS);
__sunzilog_startup(up);

- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);

return 0;
}
@@ -1333,7 +1333,7 @@ static void sunzilog_init_hw(struct uart_sunzilog_port *up)

channel = ZILOG_CHANNEL_FROM_PORT(&up->port);

- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);
if (ZS_IS_CHANNEL_A(up)) {
write_zsreg(channel, R9, FHWRES);
ZSDELAY_LONG();
@@ -1383,7 +1383,7 @@ static void sunzilog_init_hw(struct uart_sunzilog_port *up)
write_zsreg(channel, R9, up->curregs[R9]);
}

- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);

#ifdef CONFIG_SERIO
if (up->flags & (SUNZILOG_FLAG_CONS_KEYB |
--
2.39.2

2023-09-15 01:36:51

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 18/74] serial: apb: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/apbuart.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index d7658f380838..716cb014c028 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -133,7 +133,7 @@ static irqreturn_t apbuart_int(int irq, void *dev_id)
struct uart_port *port = dev_id;
unsigned int status;

- spin_lock(&port->lock);
+ uart_port_lock(port);

status = UART_GET_STATUS(port);
if (status & UART_STATUS_DR)
@@ -141,7 +141,7 @@ static irqreturn_t apbuart_int(int irq, void *dev_id)
if (status & UART_STATUS_THE)
apbuart_tx_chars(port);

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_HANDLED;
}
@@ -228,7 +228,7 @@ static void apbuart_set_termios(struct uart_port *port,
if (termios->c_cflag & CRTSCTS)
cr |= UART_CTRL_FL;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Update the per-port timeout. */
uart_update_timeout(port, termios->c_cflag, baud);
@@ -251,7 +251,7 @@ static void apbuart_set_termios(struct uart_port *port,
UART_PUT_SCAL(port, quot);
UART_PUT_CTRL(port, cr);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *apbuart_type(struct uart_port *port)
--
2.39.2

2023-09-15 01:38:21

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 59/74] serial: txx9: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/serial_txx9.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c
index be08fb6f749c..eaa980722455 100644
--- a/drivers/tty/serial/serial_txx9.c
+++ b/drivers/tty/serial/serial_txx9.c
@@ -335,13 +335,13 @@ static irqreturn_t serial_txx9_interrupt(int irq, void *dev_id)
unsigned int status;

while (1) {
- spin_lock(&up->lock);
+ uart_port_lock(up);
status = sio_in(up, TXX9_SIDISR);
if (!(sio_in(up, TXX9_SIDICR) & TXX9_SIDICR_TIE))
status &= ~TXX9_SIDISR_TDIS;
if (!(status & (TXX9_SIDISR_TDIS | TXX9_SIDISR_RDIS |
TXX9_SIDISR_TOUT))) {
- spin_unlock(&up->lock);
+ uart_port_unlock(up);
break;
}

@@ -353,7 +353,7 @@ static irqreturn_t serial_txx9_interrupt(int irq, void *dev_id)
sio_mask(up, TXX9_SIDISR,
TXX9_SIDISR_TDIS | TXX9_SIDISR_RDIS |
TXX9_SIDISR_TOUT);
- spin_unlock(&up->lock);
+ uart_port_unlock(up);

if (pass_counter++ > PASS_LIMIT)
break;
@@ -367,9 +367,9 @@ static unsigned int serial_txx9_tx_empty(struct uart_port *up)
unsigned long flags;
unsigned int ret;

- spin_lock_irqsave(&up->lock, flags);
+ uart_port_lock_irqsave(up, &flags);
ret = (sio_in(up, TXX9_SICISR) & TXX9_SICISR_TXALS) ? TIOCSER_TEMT : 0;
- spin_unlock_irqrestore(&up->lock, flags);
+ uart_port_unlock_irqrestore(up, flags);

return ret;
}
@@ -399,12 +399,12 @@ static void serial_txx9_break_ctl(struct uart_port *up, int break_state)
{
unsigned long flags;

- spin_lock_irqsave(&up->lock, flags);
+ uart_port_lock_irqsave(up, &flags);
if (break_state == -1)
sio_set(up, TXX9_SIFLCR, TXX9_SIFLCR_TBRK);
else
sio_mask(up, TXX9_SIFLCR, TXX9_SIFLCR_TBRK);
- spin_unlock_irqrestore(&up->lock, flags);
+ uart_port_unlock_irqrestore(up, flags);
}

#if defined(CONFIG_SERIAL_TXX9_CONSOLE) || defined(CONFIG_CONSOLE_POLL)
@@ -517,9 +517,9 @@ static int serial_txx9_startup(struct uart_port *up)
/*
* Now, initialize the UART
*/
- spin_lock_irqsave(&up->lock, flags);
+ uart_port_lock_irqsave(up, &flags);
serial_txx9_set_mctrl(up, up->mctrl);
- spin_unlock_irqrestore(&up->lock, flags);
+ uart_port_unlock_irqrestore(up, flags);

/* Enable RX/TX */
sio_mask(up, TXX9_SIFLCR, TXX9_SIFLCR_RSDE | TXX9_SIFLCR_TSDE);
@@ -541,9 +541,9 @@ static void serial_txx9_shutdown(struct uart_port *up)
*/
sio_out(up, TXX9_SIDICR, 0); /* disable all intrs */

- spin_lock_irqsave(&up->lock, flags);
+ uart_port_lock_irqsave(up, &flags);
serial_txx9_set_mctrl(up, up->mctrl);
- spin_unlock_irqrestore(&up->lock, flags);
+ uart_port_unlock_irqrestore(up, flags);

/*
* Disable break condition
@@ -625,7 +625,7 @@ serial_txx9_set_termios(struct uart_port *up, struct ktermios *termios,
* Ok, we're now changing the port state. Do it with
* interrupts disabled.
*/
- spin_lock_irqsave(&up->lock, flags);
+ uart_port_lock_irqsave(up, &flags);

/*
* Update the per-port timeout.
@@ -676,7 +676,7 @@ serial_txx9_set_termios(struct uart_port *up, struct ktermios *termios,
sio_out(up, TXX9_SIFCR, fcr);

serial_txx9_set_mctrl(up, up->mctrl);
- spin_unlock_irqrestore(&up->lock, flags);
+ uart_port_unlock_irqrestore(up, flags);
}

static void
--
2.39.2

2023-09-15 01:47:15

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 48/74] serial: pxa: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/pxa.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 73c60f5ea027..46e70e155aab 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -225,14 +225,14 @@ static inline irqreturn_t serial_pxa_irq(int irq, void *dev_id)
iir = serial_in(up, UART_IIR);
if (iir & UART_IIR_NO_INT)
return IRQ_NONE;
- spin_lock(&up->port.lock);
+ uart_port_lock(&up->port);
lsr = serial_in(up, UART_LSR);
if (lsr & UART_LSR_DR)
receive_chars(up, &lsr);
check_modem_status(up);
if (lsr & UART_LSR_THRE)
transmit_chars(up);
- spin_unlock(&up->port.lock);
+ uart_port_unlock(&up->port);
return IRQ_HANDLED;
}

@@ -242,9 +242,9 @@ static unsigned int serial_pxa_tx_empty(struct uart_port *port)
unsigned long flags;
unsigned int ret;

- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);
ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);

return ret;
}
@@ -295,13 +295,13 @@ static void serial_pxa_break_ctl(struct uart_port *port, int break_state)
struct uart_pxa_port *up = (struct uart_pxa_port *)port;
unsigned long flags;

- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);
if (break_state == -1)
up->lcr |= UART_LCR_SBC;
else
up->lcr &= ~UART_LCR_SBC;
serial_out(up, UART_LCR, up->lcr);
- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);
}

static int serial_pxa_startup(struct uart_port *port)
@@ -346,10 +346,10 @@ static int serial_pxa_startup(struct uart_port *port)
*/
serial_out(up, UART_LCR, UART_LCR_WLEN8);

- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);
up->port.mctrl |= TIOCM_OUT2;
serial_pxa_set_mctrl(&up->port, up->port.mctrl);
- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);

/*
* Finally, enable interrupts. Note: Modem status interrupts
@@ -383,10 +383,10 @@ static void serial_pxa_shutdown(struct uart_port *port)
up->ier = 0;
serial_out(up, UART_IER, 0);

- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);
up->port.mctrl &= ~TIOCM_OUT2;
serial_pxa_set_mctrl(&up->port, up->port.mctrl);
- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);

/*
* Disable break condition and FIFOs
@@ -434,7 +434,7 @@ serial_pxa_set_termios(struct uart_port *port, struct ktermios *termios,
* Ok, we're now changing the port state. Do it with
* interrupts disabled.
*/
- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);

/*
* Ensure the port will be enabled.
@@ -504,7 +504,7 @@ serial_pxa_set_termios(struct uart_port *port, struct ktermios *termios,
up->lcr = cval; /* Save LCR */
serial_pxa_set_mctrl(&up->port, up->port.mctrl);
serial_out(up, UART_FCR, fcr);
- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);
}

static void
@@ -608,9 +608,9 @@ serial_pxa_console_write(struct console *co, const char *s, unsigned int count)
if (up->port.sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock(&up->port.lock);
+ locked = uart_port_trylock(&up->port);
else
- spin_lock(&up->port.lock);
+ uart_port_lock(&up->port);

/*
* First save the IER then disable the interrupts
@@ -628,7 +628,7 @@ serial_pxa_console_write(struct console *co, const char *s, unsigned int count)
serial_out(up, UART_IER, ier);

if (locked)
- spin_unlock(&up->port.lock);
+ uart_port_unlock(&up->port);
local_irq_restore(flags);
clk_disable(up->clk);

--
2.39.2

2023-09-15 02:00:10

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 60/74] serial: sh-sci: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/sh-sci.c | 68 ++++++++++++++++++-------------------
1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index a560b729fa3b..84ab434c94ba 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1205,7 +1205,7 @@ static void sci_dma_tx_complete(void *arg)

dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

uart_xmit_advance(port, s->tx_dma_len);

@@ -1229,7 +1229,7 @@ static void sci_dma_tx_complete(void *arg)
}
}

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/* Locking: called with port lock held */
@@ -1320,7 +1320,7 @@ static void sci_dma_rx_complete(void *arg)
dev_dbg(port->dev, "%s(%d) active cookie %d\n", __func__, port->line,
s->active_rx);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

active = sci_dma_rx_find_active(s);
if (active >= 0)
@@ -1347,20 +1347,20 @@ static void sci_dma_rx_complete(void *arg)

dma_async_issue_pending(chan);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
dev_dbg(port->dev, "%s: cookie %d #%d, new active cookie %d\n",
__func__, s->cookie_rx[active], active, s->active_rx);
return;

fail:
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
dev_warn(port->dev, "Failed submitting Rx DMA descriptor\n");
/* Switch to PIO */
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
dmaengine_terminate_async(chan);
sci_dma_rx_chan_invalidate(s);
sci_dma_rx_reenable_irq(s);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void sci_dma_tx_release(struct sci_port *s)
@@ -1409,13 +1409,13 @@ static int sci_dma_rx_submit(struct sci_port *s, bool port_lock_held)
fail:
/* Switch to PIO */
if (!port_lock_held)
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
if (i)
dmaengine_terminate_async(chan);
sci_dma_rx_chan_invalidate(s);
sci_start_rx(port);
if (!port_lock_held)
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
return -EAGAIN;
}

@@ -1437,14 +1437,14 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
* transmit till the end, and then the rest. Take the port lock to get a
* consistent xmit buffer state.
*/
- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(port);
head = xmit->head;
tail = xmit->tail;
buf = s->tx_dma_addr + tail;
s->tx_dma_len = CIRC_CNT_TO_END(head, tail, UART_XMIT_SIZE);
if (!s->tx_dma_len) {
/* Transmit buffer has been flushed */
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);
return;
}

@@ -1452,7 +1452,7 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
DMA_MEM_TO_DEV,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!desc) {
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);
dev_warn(port->dev, "Failed preparing Tx DMA descriptor\n");
goto switch_to_pio;
}
@@ -1464,12 +1464,12 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
desc->callback_param = s;
s->cookie_tx = dmaengine_submit(desc);
if (dma_submit_error(s->cookie_tx)) {
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);
dev_warn(port->dev, "Failed submitting Tx DMA descriptor\n");
goto switch_to_pio;
}

- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);
dev_dbg(port->dev, "%s: %p: %d...%d, cookie %d\n",
__func__, xmit->buf, tail, head, s->cookie_tx);

@@ -1477,10 +1477,10 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
return;

switch_to_pio:
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
s->chan_tx = NULL;
sci_start_tx(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
return;
}

@@ -1497,17 +1497,17 @@ static enum hrtimer_restart sci_dma_rx_timer_fn(struct hrtimer *t)

dev_dbg(port->dev, "DMA Rx timed out\n");

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

active = sci_dma_rx_find_active(s);
if (active < 0) {
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
return HRTIMER_NORESTART;
}

status = dmaengine_tx_status(s->chan_rx, s->active_rx, &state);
if (status == DMA_COMPLETE) {
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
dev_dbg(port->dev, "Cookie %d #%d has already completed\n",
s->active_rx, active);

@@ -1525,7 +1525,7 @@ static enum hrtimer_restart sci_dma_rx_timer_fn(struct hrtimer *t)
*/
status = dmaengine_tx_status(s->chan_rx, s->active_rx, &state);
if (status == DMA_COMPLETE) {
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
dev_dbg(port->dev, "Transaction complete after DMA engine was stopped");
return HRTIMER_NORESTART;
}
@@ -1546,7 +1546,7 @@ static enum hrtimer_restart sci_dma_rx_timer_fn(struct hrtimer *t)

sci_dma_rx_reenable_irq(s);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return HRTIMER_NORESTART;
}
@@ -1770,9 +1770,9 @@ static irqreturn_t sci_tx_interrupt(int irq, void *ptr)
struct uart_port *port = ptr;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
sci_transmit_chars(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return IRQ_HANDLED;
}
@@ -1786,11 +1786,11 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
if (port->type != PORT_SCI)
return sci_tx_interrupt(irq, ptr);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
ctrl = serial_port_in(port, SCSCR);
ctrl &= ~(SCSCR_TE | SCSCR_TEIE);
serial_port_out(port, SCSCR, ctrl);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return IRQ_HANDLED;
}
@@ -2187,7 +2187,7 @@ static void sci_break_ctl(struct uart_port *port, int break_state)
return;
}

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
scsptr = serial_port_in(port, SCSPTR);
scscr = serial_port_in(port, SCSCR);

@@ -2201,7 +2201,7 @@ static void sci_break_ctl(struct uart_port *port, int break_state)

serial_port_out(port, SCSPTR, scsptr);
serial_port_out(port, SCSCR, scscr);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int sci_startup(struct uart_port *port)
@@ -2233,7 +2233,7 @@ static void sci_shutdown(struct uart_port *port)
s->autorts = false;
mctrl_gpio_disable_ms(to_sci_port(port)->gpios);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
sci_stop_rx(port);
sci_stop_tx(port);
/*
@@ -2243,7 +2243,7 @@ static void sci_shutdown(struct uart_port *port)
scr = serial_port_in(port, SCSCR);
serial_port_out(port, SCSCR, scr &
(SCSCR_CKE1 | SCSCR_CKE0 | s->hscif_tot));
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

#ifdef CONFIG_SERIAL_SH_SCI_DMA
if (s->chan_rx_saved) {
@@ -2545,7 +2545,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
serial_port_out(port, SCCKS, sccks);
}

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

sci_reset(port);

@@ -2667,7 +2667,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
if ((termios->c_cflag & CREAD) != 0)
sci_start_rx(port);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

sci_port_disable(s);

@@ -3052,9 +3052,9 @@ static void serial_console_write(struct console *co, const char *s,
if (port->sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock_irqsave(&port->lock, flags);
+ locked = uart_port_trylock_irqsave(port, &flags);
else
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* first save SCSCR then disable interrupts, keep clock source */
ctrl = serial_port_in(port, SCSCR);
@@ -3074,7 +3074,7 @@ static void serial_console_write(struct console *co, const char *s,
serial_port_out(port, SCSCR, ctrl);

if (locked)
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int serial_console_setup(struct console *co, char *options)
--
2.39.2

2023-09-15 02:41:52

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 58/74] serial: mctrl_gpio: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/serial_mctrl_gpio.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 7d5aaa8d422b..e51ca593ab86 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -184,7 +184,7 @@ static irqreturn_t mctrl_gpio_irq_handle(int irq, void *context)

mctrl_gpio_get(gpios, &mctrl);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

mctrl_diff = mctrl ^ gpios->mctrl_prev;
gpios->mctrl_prev = mctrl;
@@ -205,7 +205,7 @@ static irqreturn_t mctrl_gpio_irq_handle(int irq, void *context)
wake_up_interruptible(&port->state->port.delta_msr_wait);
}

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return IRQ_HANDLED;
}
--
2.39.2

2023-09-15 02:51:50

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 64/74] serial: stm32: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/stm32-usart.c | 38 ++++++++++++++++----------------
1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 5e9cf0c48813..8c51ec9433d6 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -537,7 +537,7 @@ static void stm32_usart_rx_dma_complete(void *arg)
unsigned int size;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
size = stm32_usart_receive_chars(port, false);
uart_unlock_and_check_sysrq_irqrestore(port, flags);
if (size)
@@ -643,9 +643,9 @@ static void stm32_usart_tx_dma_complete(void *arg)
stm32_usart_tx_dma_terminate(stm32port);

/* Let's see if we have pending data to send */
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
stm32_usart_transmit_chars(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void stm32_usart_tx_interrupt_enable(struct uart_port *port)
@@ -889,7 +889,7 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
if (!stm32_port->throttled) {
if (((sr & USART_SR_RXNE) && !stm32_usart_rx_dma_started(stm32_port)) ||
((sr & USART_SR_ERR_MASK) && stm32_usart_rx_dma_started(stm32_port))) {
- spin_lock(&port->lock);
+ uart_port_lock(port);
size = stm32_usart_receive_chars(port, false);
uart_unlock_and_check_sysrq(port);
if (size)
@@ -898,14 +898,14 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
}

if ((sr & USART_SR_TXE) && !(stm32_port->tx_ch)) {
- spin_lock(&port->lock);
+ uart_port_lock(port);
stm32_usart_transmit_chars(port);
- spin_unlock(&port->lock);
+ uart_port_unlock(port);
}

/* Receiver timeout irq for DMA RX */
if (stm32_usart_rx_dma_started(stm32_port) && !stm32_port->throttled) {
- spin_lock(&port->lock);
+ uart_port_lock(port);
size = stm32_usart_receive_chars(port, false);
uart_unlock_and_check_sysrq(port);
if (size)
@@ -993,7 +993,7 @@ static void stm32_usart_throttle(struct uart_port *port)
const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/*
* Pause DMA transfer, so the RX data gets queued into the FIFO.
@@ -1006,7 +1006,7 @@ static void stm32_usart_throttle(struct uart_port *port)
stm32_usart_clr_bits(port, ofs->cr3, stm32_port->cr3_irq);

stm32_port->throttled = true;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/* Unthrottle the remote, the input buffer can now accept data. */
@@ -1016,7 +1016,7 @@ static void stm32_usart_unthrottle(struct uart_port *port)
const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
stm32_usart_set_bits(port, ofs->cr1, stm32_port->cr1_irq);
if (stm32_port->cr3_irq)
stm32_usart_set_bits(port, ofs->cr3, stm32_port->cr3_irq);
@@ -1030,7 +1030,7 @@ static void stm32_usart_unthrottle(struct uart_port *port)
if (stm32_port->rx_ch)
stm32_usart_rx_dma_start_or_resume(port);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/* Receive stop */
@@ -1158,7 +1158,7 @@ static void stm32_usart_set_termios(struct uart_port *port,

baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 8);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

ret = readl_relaxed_poll_timeout_atomic(port->membase + ofs->isr,
isr,
@@ -1349,7 +1349,7 @@ static void stm32_usart_set_termios(struct uart_port *port,
writel_relaxed(cr1, port->membase + ofs->cr1);

stm32_usart_set_bits(port, ofs->cr1, BIT(cfg->uart_enable_bit));
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

/* Handle modem control interrupts */
if (UART_ENABLE_MS(port, termios->c_cflag))
@@ -1399,9 +1399,9 @@ static void stm32_usart_pm(struct uart_port *port, unsigned int state,
pm_runtime_get_sync(port->dev);
break;
case UART_PM_STATE_OFF:
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
stm32_usart_clr_bits(port, ofs->cr1, BIT(cfg->uart_enable_bit));
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
pm_runtime_put_sync(port->dev);
break;
}
@@ -1884,9 +1884,9 @@ static void stm32_usart_console_write(struct console *co, const char *s,
int locked = 1;

if (oops_in_progress)
- locked = spin_trylock_irqsave(&port->lock, flags);
+ locked = uart_port_trylock_irqsave(port, &flags);
else
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Save and disable interrupts, enable the transmitter */
old_cr1 = readl_relaxed(port->membase + ofs->cr1);
@@ -1900,7 +1900,7 @@ static void stm32_usart_console_write(struct console *co, const char *s,
writel_relaxed(old_cr1, port->membase + ofs->cr1);

if (locked)
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int stm32_usart_console_setup(struct console *co, char *options)
@@ -2035,7 +2035,7 @@ static int __maybe_unused stm32_usart_serial_en_wakeup(struct uart_port *port,
* low-power mode.
*/
if (stm32_port->rx_ch) {
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
/* Poll data from DMA RX buffer if any */
if (!stm32_usart_rx_dma_pause(stm32_port))
size += stm32_usart_receive_chars(port, true);
--
2.39.2

2023-09-15 02:55:31

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 26/74] serial: linflexuart: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/fsl_linflexuart.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/fsl_linflexuart.c b/drivers/tty/serial/fsl_linflexuart.c
index 249cb380c3c6..7fa809a405e8 100644
--- a/drivers/tty/serial/fsl_linflexuart.c
+++ b/drivers/tty/serial/fsl_linflexuart.c
@@ -203,7 +203,7 @@ static irqreturn_t linflex_txint(int irq, void *dev_id)
struct circ_buf *xmit = &sport->state->xmit;
unsigned long flags;

- spin_lock_irqsave(&sport->lock, flags);
+ uart_port_lock_irqsave(sport, &flags);

if (sport->x_char) {
linflex_put_char(sport, sport->x_char);
@@ -217,7 +217,7 @@ static irqreturn_t linflex_txint(int irq, void *dev_id)

linflex_transmit_buffer(sport);
out:
- spin_unlock_irqrestore(&sport->lock, flags);
+ uart_port_unlock_irqrestore(sport, flags);
return IRQ_HANDLED;
}

@@ -230,7 +230,7 @@ static irqreturn_t linflex_rxint(int irq, void *dev_id)
unsigned char rx;
bool brk;

- spin_lock_irqsave(&sport->lock, flags);
+ uart_port_lock_irqsave(sport, &flags);

status = readl(sport->membase + UARTSR);
while (status & LINFLEXD_UARTSR_RMB) {
@@ -266,7 +266,7 @@ static irqreturn_t linflex_rxint(int irq, void *dev_id)
}
}

- spin_unlock_irqrestore(&sport->lock, flags);
+ uart_port_unlock_irqrestore(sport, flags);

tty_flip_buffer_push(port);

@@ -369,11 +369,11 @@ static int linflex_startup(struct uart_port *port)
int ret = 0;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

linflex_setup_watermark(port);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

ret = devm_request_irq(port->dev, port->irq, linflex_int, 0,
DRIVER_NAME, port);
@@ -386,14 +386,14 @@ static void linflex_shutdown(struct uart_port *port)
unsigned long ier;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* disable interrupts */
ier = readl(port->membase + LINIER);
ier &= ~(LINFLEXD_LINIER_DRIE | LINFLEXD_LINIER_DTIE);
writel(ier, port->membase + LINIER);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

devm_free_irq(port->dev, port->irq, port);
}
@@ -474,7 +474,7 @@ linflex_set_termios(struct uart_port *port, struct ktermios *termios,
cr &= ~LINFLEXD_UARTCR_PCE;
}

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

port->read_status_mask = 0;

@@ -507,7 +507,7 @@ linflex_set_termios(struct uart_port *port, struct ktermios *termios,

writel(cr1, port->membase + LINCR1);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *linflex_type(struct uart_port *port)
@@ -646,14 +646,14 @@ linflex_console_write(struct console *co, const char *s, unsigned int count)
if (sport->sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock_irqsave(&sport->lock, flags);
+ locked = uart_port_trylock_irqsave(sport, &flags);
else
- spin_lock_irqsave(&sport->lock, flags);
+ uart_port_lock_irqsave(sport, &flags);

linflex_string_write(sport, s, count);

if (locked)
- spin_unlock_irqrestore(&sport->lock, flags);
+ uart_port_unlock_irqrestore(sport, flags);
}

/*
--
2.39.2

2023-09-15 02:59:22

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 12/74] serial: 8250_omap: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/8250/8250_omap.c | 52 ++++++++++++++---------------
1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 26dd089d8e82..9ea38e2a6e23 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -401,7 +401,7 @@ static void omap_8250_set_termios(struct uart_port *port,
* interrupts disabled.
*/
pm_runtime_get_sync(port->dev);
- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(port);

/*
* Update the per-port timeout.
@@ -504,7 +504,7 @@ static void omap_8250_set_termios(struct uart_port *port,
}
omap8250_restore_regs(up);

- spin_unlock_irq(&up->port.lock);
+ uart_port_unlock_irq(&up->port);
pm_runtime_mark_last_busy(port->dev);
pm_runtime_put_autosuspend(port->dev);

@@ -529,7 +529,7 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state,
pm_runtime_get_sync(port->dev);

/* Synchronize UART_IER access against the console. */
- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(port);

serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
efr = serial_in(up, UART_EFR);
@@ -541,7 +541,7 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state,
serial_out(up, UART_EFR, efr);
serial_out(up, UART_LCR, 0);

- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);

pm_runtime_mark_last_busy(port->dev);
pm_runtime_put_autosuspend(port->dev);
@@ -660,7 +660,7 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id)
unsigned long delay;

/* Synchronize UART_IER access against the console. */
- spin_lock(&port->lock);
+ uart_port_lock(port);
up->ier = port->serial_in(port, UART_IER);
if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) {
port->ops->stop_rx(port);
@@ -670,7 +670,7 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id)
*/
cancel_delayed_work(&up->overrun_backoff);
}
- spin_unlock(&port->lock);
+ uart_port_unlock(port);

delay = msecs_to_jiffies(up->overrun_backoff_time_ms);
schedule_delayed_work(&up->overrun_backoff, delay);
@@ -717,10 +717,10 @@ static int omap_8250_startup(struct uart_port *port)
}

/* Synchronize UART_IER access against the console. */
- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(port);
up->ier = UART_IER_RLSI | UART_IER_RDI;
serial_out(up, UART_IER, up->ier);
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);

#ifdef CONFIG_PM
up->capabilities |= UART_CAP_RPM;
@@ -733,9 +733,9 @@ static int omap_8250_startup(struct uart_port *port)
serial_out(up, UART_OMAP_WER, priv->wer);

if (up->dma && !(priv->habit & UART_HAS_EFR2)) {
- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(port);
up->dma->rx_dma(up);
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);
}

enable_irq(up->port.irq);
@@ -761,10 +761,10 @@ static void omap_8250_shutdown(struct uart_port *port)
serial_out(up, UART_OMAP_EFR2, 0x0);

/* Synchronize UART_IER access against the console. */
- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(port);
up->ier = 0;
serial_out(up, UART_IER, 0);
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);
disable_irq_nosync(up->port.irq);
dev_pm_clear_wake_irq(port->dev);

@@ -789,10 +789,10 @@ static void omap_8250_throttle(struct uart_port *port)

pm_runtime_get_sync(port->dev);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
port->ops->stop_rx(port);
priv->throttled = true;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

pm_runtime_mark_last_busy(port->dev);
pm_runtime_put_autosuspend(port->dev);
@@ -807,14 +807,14 @@ static void omap_8250_unthrottle(struct uart_port *port)
pm_runtime_get_sync(port->dev);

/* Synchronize UART_IER access against the console. */
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
priv->throttled = false;
if (up->dma)
up->dma->rx_dma(up);
up->ier |= UART_IER_RLSI | UART_IER_RDI;
port->read_status_mask |= UART_LSR_DR;
serial_out(up, UART_IER, up->ier);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

pm_runtime_mark_last_busy(port->dev);
pm_runtime_put_autosuspend(port->dev);
@@ -958,7 +958,7 @@ static void __dma_rx_complete(void *param)
unsigned long flags;

/* Synchronize UART_IER access against the console. */
- spin_lock_irqsave(&p->port.lock, flags);
+ uart_port_lock_irqsave(&p->port, &flags);

/*
* If the tx status is not DMA_COMPLETE, then this is a delayed
@@ -967,7 +967,7 @@ static void __dma_rx_complete(void *param)
*/
if (dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state) !=
DMA_COMPLETE) {
- spin_unlock_irqrestore(&p->port.lock, flags);
+ uart_port_unlock_irqrestore(&p->port, flags);
return;
}
__dma_rx_do_complete(p);
@@ -978,7 +978,7 @@ static void __dma_rx_complete(void *param)
omap_8250_rx_dma(p);
}

- spin_unlock_irqrestore(&p->port.lock, flags);
+ uart_port_unlock_irqrestore(&p->port, flags);
}

static void omap_8250_rx_dma_flush(struct uart_8250_port *p)
@@ -1083,7 +1083,7 @@ static void omap_8250_dma_tx_complete(void *param)
dma_sync_single_for_cpu(dma->txchan->device->dev, dma->tx_addr,
UART_XMIT_SIZE, DMA_TO_DEVICE);

- spin_lock_irqsave(&p->port.lock, flags);
+ uart_port_lock_irqsave(&p->port, &flags);

dma->tx_running = 0;

@@ -1112,7 +1112,7 @@ static void omap_8250_dma_tx_complete(void *param)
serial8250_set_THRI(p);
}

- spin_unlock_irqrestore(&p->port.lock, flags);
+ uart_port_unlock_irqrestore(&p->port, flags);
}

static int omap_8250_tx_dma(struct uart_8250_port *p)
@@ -1278,7 +1278,7 @@ static int omap_8250_dma_handle_irq(struct uart_port *port)
return IRQ_HANDLED;
}

- spin_lock(&port->lock);
+ uart_port_lock(port);

status = serial_port_in(port, UART_LSR);

@@ -1761,15 +1761,15 @@ static int omap8250_runtime_resume(struct device *dev)
up = serial8250_get_port(priv->line);

if (up && omap8250_lost_context(up)) {
- spin_lock_irq(&up->port.lock);
+ uart_port_lock_irq(&up->port);
omap8250_restore_regs(up);
- spin_unlock_irq(&up->port.lock);
+ uart_port_unlock_irq(&up->port);
}

if (up && up->dma && up->dma->rxchan && !(priv->habit & UART_HAS_EFR2)) {
- spin_lock_irq(&up->port.lock);
+ uart_port_lock_irq(&up->port);
omap_8250_rx_dma(up);
- spin_unlock_irq(&up->port.lock);
+ uart_port_unlock_irq(&up->port);
}

priv->latency = priv->calc_latency;
--
2.39.2

2023-09-15 03:00:34

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 46/74] serial: pic32: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/pic32_uart.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index e308d5022b3f..3a95bf5d55d3 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -243,7 +243,7 @@ static void pic32_uart_break_ctl(struct uart_port *port, int ctl)
struct pic32_sport *sport = to_pic32_sport(port);
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

if (ctl)
pic32_uart_writel(sport, PIC32_SET(PIC32_UART_STA),
@@ -252,7 +252,7 @@ static void pic32_uart_break_ctl(struct uart_port *port, int ctl)
pic32_uart_writel(sport, PIC32_CLR(PIC32_UART_STA),
PIC32_UART_STA_UTXBRK);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/* get port type in string format */
@@ -274,7 +274,7 @@ static void pic32_uart_do_rx(struct uart_port *port)
*/
max_count = PIC32_UART_RX_FIFO_DEPTH;

- spin_lock(&port->lock);
+ uart_port_lock(port);

tty = &port->state->port;

@@ -331,7 +331,7 @@ static void pic32_uart_do_rx(struct uart_port *port)

} while (--max_count);

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

tty_flip_buffer_push(tty);
}
@@ -410,9 +410,9 @@ static irqreturn_t pic32_uart_tx_interrupt(int irq, void *dev_id)
struct uart_port *port = dev_id;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
pic32_uart_do_tx(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return IRQ_HANDLED;
}
@@ -580,9 +580,9 @@ static void pic32_uart_shutdown(struct uart_port *port)
unsigned long flags;

/* disable uart */
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
pic32_uart_dsbl_and_mask(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
clk_disable_unprepare(sport->clk);

/* free all 3 interrupts for this UART */
@@ -604,7 +604,7 @@ static void pic32_uart_set_termios(struct uart_port *port,
unsigned int quot;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* disable uart and mask all interrupts while changing speed */
pic32_uart_dsbl_and_mask(port);
@@ -672,7 +672,7 @@ static void pic32_uart_set_termios(struct uart_port *port,
/* enable uart */
pic32_uart_en_and_unmask(port);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/* serial core request to claim uart iomem */
--
2.39.2

2023-09-15 03:17:23

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 62/74] serial: sprd: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/sprd_serial.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index f328fa57231f..f257525f9299 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -247,7 +247,7 @@ static void sprd_complete_tx_dma(void *data)
struct circ_buf *xmit = &port->state->xmit;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
dma_unmap_single(port->dev, sp->tx_dma.phys_addr,
sp->tx_dma.trans_len, DMA_TO_DEVICE);

@@ -260,7 +260,7 @@ static void sprd_complete_tx_dma(void *data)
sprd_tx_dma_config(port))
sp->tx_dma.trans_len = 0;

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int sprd_uart_dma_submit(struct uart_port *port,
@@ -429,13 +429,13 @@ static void sprd_complete_rx_dma(void *data)
enum dma_status status;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

status = dmaengine_tx_status(sp->rx_dma.chn,
sp->rx_dma.cookie, &state);
if (status != DMA_COMPLETE) {
sprd_stop_rx(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
return;
}

@@ -449,7 +449,7 @@ static void sprd_complete_rx_dma(void *data)
if (sprd_start_dma_rx(port))
sprd_stop_rx(port);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int sprd_start_dma_rx(struct uart_port *port)
@@ -638,12 +638,12 @@ static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
struct uart_port *port = dev_id;
unsigned int ims;

- spin_lock(&port->lock);
+ uart_port_lock(port);

ims = serial_in(port, SPRD_IMSR);

if (!ims) {
- spin_unlock(&port->lock);
+ uart_port_unlock(port);
return IRQ_NONE;
}

@@ -660,7 +660,7 @@ static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
if (ims & SPRD_IMSR_TX_FIFO_EMPTY)
sprd_tx(port);

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_HANDLED;
}
@@ -727,13 +727,13 @@ static int sprd_startup(struct uart_port *port)
serial_out(port, SPRD_CTL1, fc);

/* enable interrupt */
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
ien = serial_in(port, SPRD_IEN);
ien |= SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
if (!sp->rx_dma.enable)
ien |= SPRD_IEN_RX_FULL;
serial_out(port, SPRD_IEN, ien);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return 0;
}
@@ -793,7 +793,7 @@ static void sprd_set_termios(struct uart_port *port, struct ktermios *termios,
lcr |= SPRD_LCR_EVEN_PAR;
}

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* update the per-port timeout */
uart_update_timeout(port, termios->c_cflag, baud);
@@ -837,7 +837,7 @@ static void sprd_set_termios(struct uart_port *port, struct ktermios *termios,
fc |= RX_TOUT_THLD_DEF | RX_HFC_THLD_DEF;
serial_out(port, SPRD_CTL1, fc);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
@@ -974,9 +974,9 @@ static void sprd_console_write(struct console *co, const char *s,
if (port->sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock_irqsave(&port->lock, flags);
+ locked = uart_port_trylock_irqsave(port, &flags);
else
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

uart_console_write(port, s, count, sprd_console_putchar);

@@ -984,7 +984,7 @@ static void sprd_console_write(struct console *co, const char *s,
wait_for_xmitr(port);

if (locked)
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int sprd_console_setup(struct console *co, char *options)
--
2.39.2

2023-09-15 03:17:33

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 37/74] serial: meson: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/meson_uart.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 790d910dafa5..45cc23e9e399 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -129,14 +129,14 @@ static void meson_uart_shutdown(struct uart_port *port)

free_irq(port->irq, port);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

val = readl(port->membase + AML_UART_CONTROL);
val &= ~AML_UART_RX_EN;
val &= ~(AML_UART_RX_INT_EN | AML_UART_TX_INT_EN);
writel(val, port->membase + AML_UART_CONTROL);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void meson_uart_start_tx(struct uart_port *port)
@@ -238,7 +238,7 @@ static irqreturn_t meson_uart_interrupt(int irq, void *dev_id)
{
struct uart_port *port = (struct uart_port *)dev_id;

- spin_lock(&port->lock);
+ uart_port_lock(port);

if (!(readl(port->membase + AML_UART_STATUS) & AML_UART_RX_EMPTY))
meson_receive_chars(port);
@@ -248,7 +248,7 @@ static irqreturn_t meson_uart_interrupt(int irq, void *dev_id)
meson_uart_start_tx(port);
}

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_HANDLED;
}
@@ -284,7 +284,7 @@ static int meson_uart_startup(struct uart_port *port)
u32 val;
int ret = 0;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

val = readl(port->membase + AML_UART_CONTROL);
val |= AML_UART_CLEAR_ERR;
@@ -301,7 +301,7 @@ static int meson_uart_startup(struct uart_port *port)
val = (AML_UART_RECV_IRQ(1) | AML_UART_XMIT_IRQ(port->fifosize / 2));
writel(val, port->membase + AML_UART_MISC);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

ret = request_irq(port->irq, meson_uart_interrupt, 0,
port->name, port);
@@ -341,7 +341,7 @@ static void meson_uart_set_termios(struct uart_port *port,
unsigned long flags;
u32 val;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

cflags = termios->c_cflag;
iflags = termios->c_iflag;
@@ -401,7 +401,7 @@ static void meson_uart_set_termios(struct uart_port *port,
AML_UART_FRAME_ERR;

uart_update_timeout(port, termios->c_cflag, baud);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int meson_uart_verify_port(struct uart_port *port,
@@ -460,14 +460,14 @@ static int meson_uart_poll_get_char(struct uart_port *port)
u32 c;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

if (readl(port->membase + AML_UART_STATUS) & AML_UART_RX_EMPTY)
c = NO_POLL_CHAR;
else
c = readl(port->membase + AML_UART_RFIFO);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return c;
}
@@ -478,7 +478,7 @@ static void meson_uart_poll_put_char(struct uart_port *port, unsigned char c)
u32 reg;
int ret;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Wait until FIFO is empty or timeout */
ret = readl_poll_timeout_atomic(port->membase + AML_UART_STATUS, reg,
@@ -502,7 +502,7 @@ static void meson_uart_poll_put_char(struct uart_port *port, unsigned char c)
dev_err(port->dev, "Timeout waiting for UART TX EMPTY\n");

out:
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

#endif /* CONFIG_CONSOLE_POLL */
@@ -559,9 +559,9 @@ static void meson_serial_port_write(struct uart_port *port, const char *s,
if (port->sysrq) {
locked = 0;
} else if (oops_in_progress) {
- locked = spin_trylock(&port->lock);
+ locked = uart_port_trylock(port);
} else {
- spin_lock(&port->lock);
+ uart_port_lock(port);
locked = 1;
}

@@ -573,7 +573,7 @@ static void meson_serial_port_write(struct uart_port *port, const char *s,
writel(val, port->membase + AML_UART_CONTROL);

if (locked)
- spin_unlock(&port->lock);
+ uart_port_unlock(port);
local_irq_restore(flags);
}

--
2.39.2

2023-09-15 03:34:46

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH tty v1 41/74] serial: msm: Use port lock wrappers

On Thu, Sep 14, 2023 at 08:43:58PM +0206, John Ogness wrote:
> From: Thomas Gleixner <[email protected]>
>
> When a serial port is used for kernel console output, then all
> modifications to the UART registers which are done from other contexts,
> e.g. getty, termios, are interference points for the kernel console.
>
> So far this has been ignored and the printk output is based on the
> principle of hope. The rework of the console infrastructure which aims to
> support threaded and atomic consoles, requires to mark sections which
> modify the UART registers as unsafe. This allows the atomic write function
> to make informed decisions and eventually to restore operational state. It
> also allows to prevent the regular UART code from modifying UART registers
> while printk output is in progress.
>
> All modifications of UART registers are guarded by the UART port lock,
> which provides an obvious synchronization point with the console
> infrastructure.
>
> To avoid adding this functionality to all UART drivers, wrap the
> spin_[un]lock*() invocations for uart_port::lock into helper functions
> which just contain the spin_[un]lock*() invocations for now. In a
> subsequent step these helpers will gain the console synchronization
> mechanisms.
>
> Converted with coccinelle. No functional change.
>
> Signed-off-by: Thomas Gleixner <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

2023-09-15 03:35:50

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 40/74] serial: mps2-uart: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/mps2-uart.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
index ea5a7911cb15..2a4c09f3a834 100644
--- a/drivers/tty/serial/mps2-uart.c
+++ b/drivers/tty/serial/mps2-uart.c
@@ -188,12 +188,12 @@ static irqreturn_t mps2_uart_rxirq(int irq, void *data)
if (unlikely(!(irqflag & UARTn_INT_RX)))
return IRQ_NONE;

- spin_lock(&port->lock);
+ uart_port_lock(port);

mps2_uart_write8(port, UARTn_INT_RX, UARTn_INT);
mps2_uart_rx_chars(port);

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_HANDLED;
}
@@ -206,12 +206,12 @@ static irqreturn_t mps2_uart_txirq(int irq, void *data)
if (unlikely(!(irqflag & UARTn_INT_TX)))
return IRQ_NONE;

- spin_lock(&port->lock);
+ uart_port_lock(port);

mps2_uart_write8(port, UARTn_INT_TX, UARTn_INT);
mps2_uart_tx_chars(port);

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_HANDLED;
}
@@ -222,7 +222,7 @@ static irqreturn_t mps2_uart_oerrirq(int irq, void *data)
struct uart_port *port = data;
u8 irqflag = mps2_uart_read8(port, UARTn_INT);

- spin_lock(&port->lock);
+ uart_port_lock(port);

if (irqflag & UARTn_INT_RX_OVERRUN) {
struct tty_port *tport = &port->state->port;
@@ -244,7 +244,7 @@ static irqreturn_t mps2_uart_oerrirq(int irq, void *data)
handled = IRQ_HANDLED;
}

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return handled;
}
@@ -356,12 +356,12 @@ mps2_uart_set_termios(struct uart_port *port, struct ktermios *termios,

bauddiv = DIV_ROUND_CLOSEST(port->uartclk, baud);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

uart_update_timeout(port, termios->c_cflag, baud);
mps2_uart_write32(port, bauddiv, UARTn_BAUDDIV);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);
--
2.39.2

2023-09-15 04:38:32

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 53/74] serial: samsung_tty: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/samsung_tty.c | 50 ++++++++++++++++----------------
1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 07fb8a9dac63..ee51a0368a55 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -248,7 +248,7 @@ static void s3c24xx_serial_rx_enable(struct uart_port *port)
unsigned int ucon, ufcon;
int count = 10000;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

while (--count && !s3c24xx_serial_txempty_nofifo(port))
udelay(100);
@@ -262,7 +262,7 @@ static void s3c24xx_serial_rx_enable(struct uart_port *port)
wr_regl(port, S3C2410_UCON, ucon);

ourport->rx_enabled = 1;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void s3c24xx_serial_rx_disable(struct uart_port *port)
@@ -271,14 +271,14 @@ static void s3c24xx_serial_rx_disable(struct uart_port *port)
unsigned long flags;
unsigned int ucon;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

ucon = rd_regl(port, S3C2410_UCON);
ucon &= ~S3C2410_UCON_RXIRQMODE;
wr_regl(port, S3C2410_UCON, ucon);

ourport->rx_enabled = 0;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void s3c24xx_serial_stop_tx(struct uart_port *port)
@@ -344,7 +344,7 @@ static void s3c24xx_serial_tx_dma_complete(void *args)
dma->tx_transfer_addr, dma->tx_size,
DMA_TO_DEVICE);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

uart_xmit_advance(port, count);
ourport->tx_in_progress = 0;
@@ -353,7 +353,7 @@ static void s3c24xx_serial_tx_dma_complete(void *args)
uart_write_wakeup(port);

s3c24xx_serial_start_next_tx(ourport);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void enable_tx_dma(struct s3c24xx_uart_port *ourport)
@@ -619,7 +619,7 @@ static void s3c24xx_serial_rx_dma_complete(void *args)
received = dma->rx_bytes_requested - state.residue;
async_tx_ack(dma->rx_desc);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

if (received)
s3c24xx_uart_copy_rx_to_tty(ourport, t, received);
@@ -631,7 +631,7 @@ static void s3c24xx_serial_rx_dma_complete(void *args)

s3c64xx_start_rx_dma(ourport);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void s3c64xx_start_rx_dma(struct s3c24xx_uart_port *ourport)
@@ -722,7 +722,7 @@ static irqreturn_t s3c24xx_serial_rx_chars_dma(void *dev_id)
utrstat = rd_regl(port, S3C2410_UTRSTAT);
rd_regl(port, S3C2410_UFSTAT);

- spin_lock(&port->lock);
+ uart_port_lock(port);

if (!(utrstat & S3C2410_UTRSTAT_TIMEOUT)) {
s3c64xx_start_rx_dma(ourport);
@@ -751,7 +751,7 @@ static irqreturn_t s3c24xx_serial_rx_chars_dma(void *dev_id)
wr_regl(port, S3C2410_UTRSTAT, S3C2410_UTRSTAT_TIMEOUT);

finish:
- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_HANDLED;
}
@@ -849,9 +849,9 @@ static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
struct s3c24xx_uart_port *ourport = dev_id;
struct uart_port *port = &ourport->port;

- spin_lock(&port->lock);
+ uart_port_lock(port);
s3c24xx_serial_rx_drain_fifo(ourport);
- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_HANDLED;
}
@@ -932,11 +932,11 @@ static irqreturn_t s3c24xx_serial_tx_irq(int irq, void *id)
struct s3c24xx_uart_port *ourport = id;
struct uart_port *port = &ourport->port;

- spin_lock(&port->lock);
+ uart_port_lock(port);

s3c24xx_serial_tx_chars(ourport);

- spin_unlock(&port->lock);
+ uart_port_unlock(port);
return IRQ_HANDLED;
}

@@ -1033,7 +1033,7 @@ static void s3c24xx_serial_break_ctl(struct uart_port *port, int break_state)
unsigned long flags;
unsigned int ucon;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

ucon = rd_regl(port, S3C2410_UCON);

@@ -1044,7 +1044,7 @@ static void s3c24xx_serial_break_ctl(struct uart_port *port, int break_state)

wr_regl(port, S3C2410_UCON, ucon);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p)
@@ -1303,7 +1303,7 @@ static int s3c64xx_serial_startup(struct uart_port *port)
ourport->rx_enabled = 1;
ourport->tx_enabled = 0;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

ufcon = rd_regl(port, S3C2410_UFCON);
ufcon |= S3C2410_UFCON_RESETRX | S5PV210_UFCON_RXTRIG8;
@@ -1313,7 +1313,7 @@ static int s3c64xx_serial_startup(struct uart_port *port)

enable_rx_pio(ourport);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

/* Enable Rx Interrupt */
s3c24xx_clear_bit(port, S3C64XX_UINTM_RXD, S3C64XX_UINTM);
@@ -1341,7 +1341,7 @@ static int apple_s5l_serial_startup(struct uart_port *port)
ourport->rx_enabled = 1;
ourport->tx_enabled = 0;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

ufcon = rd_regl(port, S3C2410_UFCON);
ufcon |= S3C2410_UFCON_RESETRX | S5PV210_UFCON_RXTRIG8;
@@ -1351,7 +1351,7 @@ static int apple_s5l_serial_startup(struct uart_port *port)

enable_rx_pio(ourport);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

/* Enable Rx Interrupt */
s3c24xx_set_bit(port, APPLE_S5L_UCON_RXTHRESH_ENA, S3C2410_UCON);
@@ -1626,7 +1626,7 @@ static void s3c24xx_serial_set_termios(struct uart_port *port,
ulcon |= S3C2410_LCON_PNONE;
}

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

dev_dbg(port->dev,
"setting ulcon to %08x, brddiv to %d, udivslot %08x\n",
@@ -1684,7 +1684,7 @@ static void s3c24xx_serial_set_termios(struct uart_port *port,
if ((termios->c_cflag & CREAD) == 0)
port->ignore_status_mask |= RXSTAT_DUMMY_READ;

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *s3c24xx_serial_type(struct uart_port *port)
@@ -2376,14 +2376,14 @@ s3c24xx_serial_console_write(struct console *co, const char *s,
if (cons_uart->sysrq)
locked = false;
else if (oops_in_progress)
- locked = spin_trylock_irqsave(&cons_uart->lock, flags);
+ locked = uart_port_trylock_irqsave(cons_uart, &flags);
else
- spin_lock_irqsave(&cons_uart->lock, flags);
+ uart_port_lock_irqsave(cons_uart, &flags);

uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar);

if (locked)
- spin_unlock_irqrestore(&cons_uart->lock, flags);
+ uart_port_unlock_irqrestore(cons_uart, flags);
}

/* Shouldn't be __init, as it can be instantiated from other module */
--
2.39.2

2023-09-15 05:53:49

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 17/74] serial: amba-pl011: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/amba-pl011.c | 72 ++++++++++++++++-----------------
1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 3dc9b0fcab1c..41eabad4c94b 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -345,9 +345,9 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap)
flag = TTY_FRAME;
}

- spin_unlock(&uap->port.lock);
+ uart_port_unlock(&uap->port);
sysrq = uart_handle_sysrq_char(&uap->port, ch & 255);
- spin_lock(&uap->port.lock);
+ uart_port_lock(&uap->port);

if (!sysrq)
uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
@@ -550,7 +550,7 @@ static void pl011_dma_tx_callback(void *data)
unsigned long flags;
u16 dmacr;

- spin_lock_irqsave(&uap->port.lock, flags);
+ uart_port_lock_irqsave(&uap->port, &flags);
if (uap->dmatx.queued)
dma_unmap_sg(dmatx->chan->device->dev, &dmatx->sg, 1,
DMA_TO_DEVICE);
@@ -571,7 +571,7 @@ static void pl011_dma_tx_callback(void *data)
if (!(dmacr & UART011_TXDMAE) || uart_tx_stopped(&uap->port) ||
uart_circ_empty(&uap->port.state->xmit)) {
uap->dmatx.queued = false;
- spin_unlock_irqrestore(&uap->port.lock, flags);
+ uart_port_unlock_irqrestore(&uap->port, flags);
return;
}

@@ -582,7 +582,7 @@ static void pl011_dma_tx_callback(void *data)
*/
pl011_start_tx_pio(uap);

- spin_unlock_irqrestore(&uap->port.lock, flags);
+ uart_port_unlock_irqrestore(&uap->port, flags);
}

/*
@@ -1009,7 +1009,7 @@ static void pl011_dma_rx_callback(void *data)
* routine to flush out the secondary DMA buffer while
* we immediately trigger the next DMA job.
*/
- spin_lock_irq(&uap->port.lock);
+ uart_port_lock_irq(&uap->port);
/*
* Rx data can be taken by the UART interrupts during
* the DMA irq handler. So we check the residue here.
@@ -1025,7 +1025,7 @@ static void pl011_dma_rx_callback(void *data)
ret = pl011_dma_rx_trigger_dma(uap);

pl011_dma_rx_chars(uap, pending, lastbuf, false);
- spin_unlock_irq(&uap->port.lock);
+ uart_port_unlock_irq(&uap->port);
/*
* Do this check after we picked the DMA chars so we don't
* get some IRQ immediately from RX.
@@ -1091,11 +1091,11 @@ static void pl011_dma_rx_poll(struct timer_list *t)
if (jiffies_to_msecs(jiffies - dmarx->last_jiffies)
> uap->dmarx.poll_timeout) {

- spin_lock_irqsave(&uap->port.lock, flags);
+ uart_port_lock_irqsave(&uap->port, &flags);
pl011_dma_rx_stop(uap);
uap->im |= UART011_RXIM;
pl011_write(uap->im, uap, REG_IMSC);
- spin_unlock_irqrestore(&uap->port.lock, flags);
+ uart_port_unlock_irqrestore(&uap->port, flags);

uap->dmarx.running = false;
dmaengine_terminate_all(rxchan);
@@ -1191,10 +1191,10 @@ static void pl011_dma_shutdown(struct uart_amba_port *uap)
while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
cpu_relax();

- spin_lock_irq(&uap->port.lock);
+ uart_port_lock_irq(&uap->port);
uap->dmacr &= ~(UART011_DMAONERR | UART011_RXDMAE | UART011_TXDMAE);
pl011_write(uap->dmacr, uap, REG_DMACR);
- spin_unlock_irq(&uap->port.lock);
+ uart_port_unlock_irq(&uap->port);

if (uap->using_tx_dma) {
/* In theory, this should already be done by pl011_dma_flush_buffer */
@@ -1374,9 +1374,9 @@ static void pl011_throttle_rx(struct uart_port *port)
{
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
pl011_stop_rx(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void pl011_enable_ms(struct uart_port *port)
@@ -1394,7 +1394,7 @@ __acquires(&uap->port.lock)
{
pl011_fifo_to_tty(uap);

- spin_unlock(&uap->port.lock);
+ uart_port_unlock(&uap->port);
tty_flip_buffer_push(&uap->port.state->port);
/*
* If we were temporarily out of DMA mode for a while,
@@ -1419,7 +1419,7 @@ __acquires(&uap->port.lock)
#endif
}
}
- spin_lock(&uap->port.lock);
+ uart_port_lock(&uap->port);
}

static bool pl011_tx_char(struct uart_amba_port *uap, unsigned char c,
@@ -1555,7 +1555,7 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
unsigned int status, pass_counter = AMBA_ISR_PASS_LIMIT;
int handled = 0;

- spin_lock_irqsave(&uap->port.lock, flags);
+ uart_port_lock_irqsave(&uap->port, &flags);
status = pl011_read(uap, REG_RIS) & uap->im;
if (status) {
do {
@@ -1585,7 +1585,7 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
handled = 1;
}

- spin_unlock_irqrestore(&uap->port.lock, flags);
+ uart_port_unlock_irqrestore(&uap->port, flags);

return IRQ_RETVAL(handled);
}
@@ -1657,14 +1657,14 @@ static void pl011_break_ctl(struct uart_port *port, int break_state)
unsigned long flags;
unsigned int lcr_h;

- spin_lock_irqsave(&uap->port.lock, flags);
+ uart_port_lock_irqsave(&uap->port, &flags);
lcr_h = pl011_read(uap, REG_LCRH_TX);
if (break_state == -1)
lcr_h |= UART01x_LCRH_BRK;
else
lcr_h &= ~UART01x_LCRH_BRK;
pl011_write(lcr_h, uap, REG_LCRH_TX);
- spin_unlock_irqrestore(&uap->port.lock, flags);
+ uart_port_unlock_irqrestore(&uap->port, flags);
}

#ifdef CONFIG_CONSOLE_POLL
@@ -1803,7 +1803,7 @@ static void pl011_enable_interrupts(struct uart_amba_port *uap)
unsigned long flags;
unsigned int i;

- spin_lock_irqsave(&uap->port.lock, flags);
+ uart_port_lock_irqsave(&uap->port, &flags);

/* Clear out any spuriously appearing RX interrupts */
pl011_write(UART011_RTIS | UART011_RXIS, uap, REG_ICR);
@@ -1825,7 +1825,7 @@ static void pl011_enable_interrupts(struct uart_amba_port *uap)
if (!pl011_dma_rx_running(uap))
uap->im |= UART011_RXIM;
pl011_write(uap->im, uap, REG_IMSC);
- spin_unlock_irqrestore(&uap->port.lock, flags);
+ uart_port_unlock_irqrestore(&uap->port, flags);
}

static void pl011_unthrottle_rx(struct uart_port *port)
@@ -1833,7 +1833,7 @@ static void pl011_unthrottle_rx(struct uart_port *port)
struct uart_amba_port *uap = container_of(port, struct uart_amba_port, port);
unsigned long flags;

- spin_lock_irqsave(&uap->port.lock, flags);
+ uart_port_lock_irqsave(&uap->port, &flags);

uap->im = UART011_RTIM;
if (!pl011_dma_rx_running(uap))
@@ -1841,7 +1841,7 @@ static void pl011_unthrottle_rx(struct uart_port *port)

pl011_write(uap->im, uap, REG_IMSC);

- spin_unlock_irqrestore(&uap->port.lock, flags);
+ uart_port_unlock_irqrestore(&uap->port, flags);
}

static int pl011_startup(struct uart_port *port)
@@ -1861,7 +1861,7 @@ static int pl011_startup(struct uart_port *port)

pl011_write(uap->vendor->ifls, uap, REG_IFLS);

- spin_lock_irq(&uap->port.lock);
+ uart_port_lock_irq(&uap->port);

cr = pl011_read(uap, REG_CR);
cr &= UART011_CR_RTS | UART011_CR_DTR;
@@ -1872,7 +1872,7 @@ static int pl011_startup(struct uart_port *port)

pl011_write(cr, uap, REG_CR);

- spin_unlock_irq(&uap->port.lock);
+ uart_port_unlock_irq(&uap->port);

/*
* initialise the old status of the modem signals
@@ -1933,12 +1933,12 @@ static void pl011_disable_uart(struct uart_amba_port *uap)
unsigned int cr;

uap->port.status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS);
- spin_lock_irq(&uap->port.lock);
+ uart_port_lock_irq(&uap->port);
cr = pl011_read(uap, REG_CR);
cr &= UART011_CR_RTS | UART011_CR_DTR;
cr |= UART01x_CR_UARTEN | UART011_CR_TXE;
pl011_write(cr, uap, REG_CR);
- spin_unlock_irq(&uap->port.lock);
+ uart_port_unlock_irq(&uap->port);

/*
* disable break condition and fifos
@@ -1950,14 +1950,14 @@ static void pl011_disable_uart(struct uart_amba_port *uap)

static void pl011_disable_interrupts(struct uart_amba_port *uap)
{
- spin_lock_irq(&uap->port.lock);
+ uart_port_lock_irq(&uap->port);

/* mask all interrupts and clear all pending ones */
uap->im = 0;
pl011_write(uap->im, uap, REG_IMSC);
pl011_write(0xffff, uap, REG_ICR);

- spin_unlock_irq(&uap->port.lock);
+ uart_port_unlock_irq(&uap->port);
}

static void pl011_shutdown(struct uart_port *port)
@@ -2102,7 +2102,7 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,

bits = tty_get_frame_size(termios->c_cflag);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/*
* Update the per-port timeout.
@@ -2176,7 +2176,7 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
old_cr |= UART011_CR_RXE;
pl011_write(old_cr, uap, REG_CR);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void
@@ -2194,10 +2194,10 @@ sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
termios->c_cflag &= ~(CMSPAR | CRTSCTS);
termios->c_cflag |= CS8 | CLOCAL;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
uart_update_timeout(port, CS8, uap->fixed_baud);
pl011_setup_status_masks(port, termios);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *pl011_type(struct uart_port *port)
@@ -2336,9 +2336,9 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
if (uap->port.sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock(&uap->port.lock);
+ locked = uart_port_trylock(&uap->port);
else
- spin_lock(&uap->port.lock);
+ uart_port_lock(&uap->port);

/*
* First save the CR then disable the interrupts
@@ -2364,7 +2364,7 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
pl011_write(old_cr, uap, REG_CR);

if (locked)
- spin_unlock(&uap->port.lock);
+ uart_port_unlock(&uap->port);
local_irq_restore(flags);

clk_disable(uap->clk);
--
2.39.2

2023-09-15 06:26:13

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 14/74] serial: altera_jtaguart: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/altera_jtaguart.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
index 5fab4c978891..7090b251dd4d 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -147,14 +147,14 @@ static irqreturn_t altera_jtaguart_interrupt(int irq, void *data)
isr = (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) >>
ALTERA_JTAGUART_CONTROL_RI_OFF) & port->read_status_mask;

- spin_lock(&port->lock);
+ uart_port_lock(port);

if (isr & ALTERA_JTAGUART_CONTROL_RE_MSK)
altera_jtaguart_rx_chars(port);
if (isr & ALTERA_JTAGUART_CONTROL_WE_MSK)
altera_jtaguart_tx_chars(port);

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return IRQ_RETVAL(isr);
}
@@ -180,14 +180,14 @@ static int altera_jtaguart_startup(struct uart_port *port)
return ret;
}

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Enable RX interrupts now */
port->read_status_mask = ALTERA_JTAGUART_CONTROL_RE_MSK;
writel(port->read_status_mask,
port->membase + ALTERA_JTAGUART_CONTROL_REG);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return 0;
}
@@ -196,14 +196,14 @@ static void altera_jtaguart_shutdown(struct uart_port *port)
{
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Disable all interrupts now */
port->read_status_mask = 0;
writel(port->read_status_mask,
port->membase + ALTERA_JTAGUART_CONTROL_REG);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

free_irq(port->irq, port);
}
@@ -264,33 +264,33 @@ static void altera_jtaguart_console_putc(struct uart_port *port, unsigned char c
unsigned long flags;
u32 status;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
while (!altera_jtaguart_tx_space(port, &status)) {
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

if ((status & ALTERA_JTAGUART_CONTROL_AC_MSK) == 0) {
return; /* no connection activity */
}

cpu_relax();
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
}
writel(c, port->membase + ALTERA_JTAGUART_DATA_REG);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}
#else
static void altera_jtaguart_console_putc(struct uart_port *port, unsigned char c)
{
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
while (!altera_jtaguart_tx_space(port, NULL)) {
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
cpu_relax();
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
}
writel(c, port->membase + ALTERA_JTAGUART_DATA_REG);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}
#endif

--
2.39.2

2023-09-15 06:39:55

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH tty v1 11/74] serial: 8250_mtk: Use port lock wrappers

On Fri, Sep 15, 2023 at 2:38 AM John Ogness <[email protected]> wrote:
>
> From: Thomas Gleixner <[email protected]>
>
> When a serial port is used for kernel console output, then all
> modifications to the UART registers which are done from other contexts,
> e.g. getty, termios, are interference points for the kernel console.
>
> So far this has been ignored and the printk output is based on the
> principle of hope. The rework of the console infrastructure which aims to
> support threaded and atomic consoles, requires to mark sections which
> modify the UART registers as unsafe. This allows the atomic write function
> to make informed decisions and eventually to restore operational state. It
> also allows to prevent the regular UART code from modifying UART registers
> while printk output is in progress.
>
> All modifications of UART registers are guarded by the UART port lock,
> which provides an obvious synchronization point with the console
> infrastructure.
>
> To avoid adding this functionality to all UART drivers, wrap the
> spin_[un]lock*() invocations for uart_port::lock into helper functions
> which just contain the spin_[un]lock*() invocations for now. In a
> subsequent step these helpers will gain the console synchronization
> mechanisms.
>
> Converted with coccinelle. No functional change.
>
> Signed-off-by: Thomas Gleixner <[email protected]>

Replacements look correct.

Reviewed-by: Chen-Yu Tsai <[email protected]>

2023-09-15 07:42:57

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 25/74] serial: dz: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/dz.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index 667f52e83277..6df7af9edc1c 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -268,9 +268,9 @@ static inline void dz_transmit_chars(struct dz_mux *mux)
}
/* If nothing to do or stopped or hardware stopped. */
if (uart_circ_empty(xmit) || uart_tx_stopped(&dport->port)) {
- spin_lock(&dport->port.lock);
+ uart_port_lock(&dport->port);
dz_stop_tx(&dport->port);
- spin_unlock(&dport->port.lock);
+ uart_port_unlock(&dport->port);
return;
}

@@ -287,9 +287,9 @@ static inline void dz_transmit_chars(struct dz_mux *mux)

/* Are we are done. */
if (uart_circ_empty(xmit)) {
- spin_lock(&dport->port.lock);
+ uart_port_lock(&dport->port);
dz_stop_tx(&dport->port);
- spin_unlock(&dport->port.lock);
+ uart_port_unlock(&dport->port);
}
}

@@ -415,14 +415,14 @@ static int dz_startup(struct uart_port *uport)
return ret;
}

- spin_lock_irqsave(&dport->port.lock, flags);
+ uart_port_lock_irqsave(&dport->port, &flags);

/* Enable interrupts. */
tmp = dz_in(dport, DZ_CSR);
tmp |= DZ_RIE | DZ_TIE;
dz_out(dport, DZ_CSR, tmp);

- spin_unlock_irqrestore(&dport->port.lock, flags);
+ uart_port_unlock_irqrestore(&dport->port, flags);

return 0;
}
@@ -443,9 +443,9 @@ static void dz_shutdown(struct uart_port *uport)
int irq_guard;
u16 tmp;

- spin_lock_irqsave(&dport->port.lock, flags);
+ uart_port_lock_irqsave(&dport->port, &flags);
dz_stop_tx(&dport->port);
- spin_unlock_irqrestore(&dport->port.lock, flags);
+ uart_port_unlock_irqrestore(&dport->port, flags);

irq_guard = atomic_add_return(-1, &mux->irq_guard);
if (!irq_guard) {
@@ -491,14 +491,14 @@ static void dz_break_ctl(struct uart_port *uport, int break_state)
unsigned long flags;
unsigned short tmp, mask = 1 << dport->port.line;

- spin_lock_irqsave(&uport->lock, flags);
+ uart_port_lock_irqsave(uport, &flags);
tmp = dz_in(dport, DZ_TCR);
if (break_state)
tmp |= mask;
else
tmp &= ~mask;
dz_out(dport, DZ_TCR, tmp);
- spin_unlock_irqrestore(&uport->lock, flags);
+ uart_port_unlock_irqrestore(uport, flags);
}

static int dz_encode_baud_rate(unsigned int baud)
@@ -608,7 +608,7 @@ static void dz_set_termios(struct uart_port *uport, struct ktermios *termios,
if (termios->c_cflag & CREAD)
cflag |= DZ_RXENAB;

- spin_lock_irqsave(&dport->port.lock, flags);
+ uart_port_lock_irqsave(&dport->port, &flags);

uart_update_timeout(uport, termios->c_cflag, baud);

@@ -631,7 +631,7 @@ static void dz_set_termios(struct uart_port *uport, struct ktermios *termios,
if (termios->c_iflag & IGNBRK)
dport->port.ignore_status_mask |= DZ_BREAK;

- spin_unlock_irqrestore(&dport->port.lock, flags);
+ uart_port_unlock_irqrestore(&dport->port, flags);
}

/*
@@ -645,12 +645,12 @@ static void dz_pm(struct uart_port *uport, unsigned int state,
struct dz_port *dport = to_dport(uport);
unsigned long flags;

- spin_lock_irqsave(&dport->port.lock, flags);
+ uart_port_lock_irqsave(&dport->port, &flags);
if (state < 3)
dz_start_tx(&dport->port);
else
dz_stop_tx(&dport->port);
- spin_unlock_irqrestore(&dport->port.lock, flags);
+ uart_port_unlock_irqrestore(&dport->port, flags);
}


@@ -811,7 +811,7 @@ static void dz_console_putchar(struct uart_port *uport, unsigned char ch)
unsigned short csr, tcr, trdy, mask;
int loops = 10000;

- spin_lock_irqsave(&dport->port.lock, flags);
+ uart_port_lock_irqsave(&dport->port, &flags);
csr = dz_in(dport, DZ_CSR);
dz_out(dport, DZ_CSR, csr & ~DZ_TIE);
tcr = dz_in(dport, DZ_TCR);
@@ -819,7 +819,7 @@ static void dz_console_putchar(struct uart_port *uport, unsigned char ch)
mask = tcr;
dz_out(dport, DZ_TCR, mask);
iob();
- spin_unlock_irqrestore(&dport->port.lock, flags);
+ uart_port_unlock_irqrestore(&dport->port, flags);

do {
trdy = dz_in(dport, DZ_CSR);
--
2.39.2

2023-09-15 07:47:09

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 74/74] serial: xilinx_uartps: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/xilinx_uartps.c | 56 +++++++++++++++---------------
1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 2e5e86a00a77..9c13dac1d4d1 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -346,7 +346,7 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
struct uart_port *port = (struct uart_port *)dev_id;
unsigned int isrstatus;

- spin_lock(&port->lock);
+ uart_port_lock(port);

/* Read the interrupt status register to determine which
* interrupt(s) is/are active and clear them.
@@ -369,7 +369,7 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
!(readl(port->membase + CDNS_UART_CR) & CDNS_UART_CR_RX_DIS))
cdns_uart_handle_rx(dev_id, isrstatus);

- spin_unlock(&port->lock);
+ uart_port_unlock(port);
return IRQ_HANDLED;
}

@@ -506,14 +506,14 @@ static int cdns_uart_clk_notifier_cb(struct notifier_block *nb,
return NOTIFY_BAD;
}

- spin_lock_irqsave(&cdns_uart->port->lock, flags);
+ uart_port_lock_irqsave(cdns_uart->port, &flags);

/* Disable the TX and RX to set baud rate */
ctrl_reg = readl(port->membase + CDNS_UART_CR);
ctrl_reg |= CDNS_UART_CR_TX_DIS | CDNS_UART_CR_RX_DIS;
writel(ctrl_reg, port->membase + CDNS_UART_CR);

- spin_unlock_irqrestore(&cdns_uart->port->lock, flags);
+ uart_port_unlock_irqrestore(cdns_uart->port, flags);

return NOTIFY_OK;
}
@@ -523,7 +523,7 @@ static int cdns_uart_clk_notifier_cb(struct notifier_block *nb,
* frequency.
*/

- spin_lock_irqsave(&cdns_uart->port->lock, flags);
+ uart_port_lock_irqsave(cdns_uart->port, &flags);

locked = 1;
port->uartclk = ndata->new_rate;
@@ -533,7 +533,7 @@ static int cdns_uart_clk_notifier_cb(struct notifier_block *nb,
fallthrough;
case ABORT_RATE_CHANGE:
if (!locked)
- spin_lock_irqsave(&cdns_uart->port->lock, flags);
+ uart_port_lock_irqsave(cdns_uart->port, &flags);

/* Set TX/RX Reset */
ctrl_reg = readl(port->membase + CDNS_UART_CR);
@@ -555,7 +555,7 @@ static int cdns_uart_clk_notifier_cb(struct notifier_block *nb,
ctrl_reg |= CDNS_UART_CR_TX_EN | CDNS_UART_CR_RX_EN;
writel(ctrl_reg, port->membase + CDNS_UART_CR);

- spin_unlock_irqrestore(&cdns_uart->port->lock, flags);
+ uart_port_unlock_irqrestore(cdns_uart->port, flags);

return NOTIFY_OK;
default:
@@ -652,7 +652,7 @@ static void cdns_uart_break_ctl(struct uart_port *port, int ctl)
unsigned int status;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

status = readl(port->membase + CDNS_UART_CR);

@@ -664,7 +664,7 @@ static void cdns_uart_break_ctl(struct uart_port *port, int ctl)
writel(CDNS_UART_CR_STOPBRK | status,
port->membase + CDNS_UART_CR);
}
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/**
@@ -683,7 +683,7 @@ static void cdns_uart_set_termios(struct uart_port *port,
unsigned long flags;
unsigned int ctrl_reg, mode_reg;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Disable the TX and RX to set baud rate */
ctrl_reg = readl(port->membase + CDNS_UART_CR);
@@ -794,7 +794,7 @@ static void cdns_uart_set_termios(struct uart_port *port,
cval &= ~CDNS_UART_MODEMCR_FCM;
writel(cval, port->membase + CDNS_UART_MODEMCR);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/**
@@ -813,7 +813,7 @@ static int cdns_uart_startup(struct uart_port *port)

is_brk_support = cdns_uart->quirks & CDNS_UART_RXBS_SUPPORT;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Disable the TX and RX */
writel(CDNS_UART_CR_TX_DIS | CDNS_UART_CR_RX_DIS,
@@ -861,7 +861,7 @@ static int cdns_uart_startup(struct uart_port *port)
writel(readl(port->membase + CDNS_UART_ISR),
port->membase + CDNS_UART_ISR);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

ret = request_irq(port->irq, cdns_uart_isr, 0, CDNS_UART_NAME, port);
if (ret) {
@@ -889,7 +889,7 @@ static void cdns_uart_shutdown(struct uart_port *port)
int status;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Disable interrupts */
status = readl(port->membase + CDNS_UART_IMR);
@@ -900,7 +900,7 @@ static void cdns_uart_shutdown(struct uart_port *port)
writel(CDNS_UART_CR_TX_DIS | CDNS_UART_CR_RX_DIS,
port->membase + CDNS_UART_CR);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

free_irq(port->irq, port);
}
@@ -1050,7 +1050,7 @@ static int cdns_uart_poll_get_char(struct uart_port *port)
int c;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Check if FIFO is empty */
if (readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_RXEMPTY)
@@ -1058,7 +1058,7 @@ static int cdns_uart_poll_get_char(struct uart_port *port)
else /* Read a character */
c = (unsigned char) readl(port->membase + CDNS_UART_FIFO);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return c;
}
@@ -1067,7 +1067,7 @@ static void cdns_uart_poll_put_char(struct uart_port *port, unsigned char c)
{
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Wait until FIFO is empty */
while (!(readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXEMPTY))
@@ -1080,7 +1080,7 @@ static void cdns_uart_poll_put_char(struct uart_port *port, unsigned char c)
while (!(readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXEMPTY))
cpu_relax();

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}
#endif

@@ -1232,9 +1232,9 @@ static void cdns_uart_console_write(struct console *co, const char *s,
if (port->sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock_irqsave(&port->lock, flags);
+ locked = uart_port_trylock_irqsave(port, &flags);
else
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* save and disable interrupt */
imr = readl(port->membase + CDNS_UART_IMR);
@@ -1257,7 +1257,7 @@ static void cdns_uart_console_write(struct console *co, const char *s,
writel(imr, port->membase + CDNS_UART_IER);

if (locked)
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/**
@@ -1325,7 +1325,7 @@ static int cdns_uart_suspend(struct device *device)
if (console_suspend_enabled && uart_console(port) && may_wake) {
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
/* Empty the receive FIFO 1st before making changes */
while (!(readl(port->membase + CDNS_UART_SR) &
CDNS_UART_SR_RXEMPTY))
@@ -1334,7 +1334,7 @@ static int cdns_uart_suspend(struct device *device)
writel(1, port->membase + CDNS_UART_RXWM);
/* disable RX timeout interrups */
writel(CDNS_UART_IXR_TOUT, port->membase + CDNS_UART_IDR);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/*
@@ -1372,7 +1372,7 @@ static int cdns_uart_resume(struct device *device)
return ret;
}

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Set TX/RX Reset */
ctrl_reg = readl(port->membase + CDNS_UART_CR);
@@ -1392,14 +1392,14 @@ static int cdns_uart_resume(struct device *device)

clk_disable(cdns_uart->uartclk);
clk_disable(cdns_uart->pclk);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
} else {
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
/* restore original rx trigger level */
writel(rx_trigger_level, port->membase + CDNS_UART_RXWM);
/* enable RX timeout interrupt */
writel(CDNS_UART_IXR_TOUT, port->membase + CDNS_UART_IER);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

return uart_resume_port(cdns_uart->cdns_uart_driver, port);
--
2.39.2

2023-09-15 08:07:40

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH tty v1 37/74] serial: meson: Use port lock wrappers

On 14/09/2023 20:37, John Ogness wrote:
> From: Thomas Gleixner <[email protected]>
>
> When a serial port is used for kernel console output, then all
> modifications to the UART registers which are done from other contexts,
> e.g. getty, termios, are interference points for the kernel console.
>
> So far this has been ignored and the printk output is based on the
> principle of hope. The rework of the console infrastructure which aims to
> support threaded and atomic consoles, requires to mark sections which
> modify the UART registers as unsafe. This allows the atomic write function
> to make informed decisions and eventually to restore operational state. It
> also allows to prevent the regular UART code from modifying UART registers
> while printk output is in progress.
>
> All modifications of UART registers are guarded by the UART port lock,
> which provides an obvious synchronization point with the console
> infrastructure.
>
> To avoid adding this functionality to all UART drivers, wrap the
> spin_[un]lock*() invocations for uart_port::lock into helper functions
> which just contain the spin_[un]lock*() invocations for now. In a
> subsequent step these helpers will gain the console synchronization
> mechanisms.
>
> Converted with coccinelle. No functional change.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> drivers/tty/serial/meson_uart.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 790d910dafa5..45cc23e9e399 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -129,14 +129,14 @@ static void meson_uart_shutdown(struct uart_port *port)
>
> free_irq(port->irq, port);
>
> - spin_lock_irqsave(&port->lock, flags);
> + uart_port_lock_irqsave(port, &flags);
>
> val = readl(port->membase + AML_UART_CONTROL);
> val &= ~AML_UART_RX_EN;
> val &= ~(AML_UART_RX_INT_EN | AML_UART_TX_INT_EN);
> writel(val, port->membase + AML_UART_CONTROL);
>
> - spin_unlock_irqrestore(&port->lock, flags);
> + uart_port_unlock_irqrestore(port, flags);
> }
>
> static void meson_uart_start_tx(struct uart_port *port)
> @@ -238,7 +238,7 @@ static irqreturn_t meson_uart_interrupt(int irq, void *dev_id)
> {
> struct uart_port *port = (struct uart_port *)dev_id;
>
> - spin_lock(&port->lock);
> + uart_port_lock(port);
>
> if (!(readl(port->membase + AML_UART_STATUS) & AML_UART_RX_EMPTY))
> meson_receive_chars(port);
> @@ -248,7 +248,7 @@ static irqreturn_t meson_uart_interrupt(int irq, void *dev_id)
> meson_uart_start_tx(port);
> }
>
> - spin_unlock(&port->lock);
> + uart_port_unlock(port);
>
> return IRQ_HANDLED;
> }
> @@ -284,7 +284,7 @@ static int meson_uart_startup(struct uart_port *port)
> u32 val;
> int ret = 0;
>
> - spin_lock_irqsave(&port->lock, flags);
> + uart_port_lock_irqsave(port, &flags);
>
> val = readl(port->membase + AML_UART_CONTROL);
> val |= AML_UART_CLEAR_ERR;
> @@ -301,7 +301,7 @@ static int meson_uart_startup(struct uart_port *port)
> val = (AML_UART_RECV_IRQ(1) | AML_UART_XMIT_IRQ(port->fifosize / 2));
> writel(val, port->membase + AML_UART_MISC);
>
> - spin_unlock_irqrestore(&port->lock, flags);
> + uart_port_unlock_irqrestore(port, flags);
>
> ret = request_irq(port->irq, meson_uart_interrupt, 0,
> port->name, port);
> @@ -341,7 +341,7 @@ static void meson_uart_set_termios(struct uart_port *port,
> unsigned long flags;
> u32 val;
>
> - spin_lock_irqsave(&port->lock, flags);
> + uart_port_lock_irqsave(port, &flags);
>
> cflags = termios->c_cflag;
> iflags = termios->c_iflag;
> @@ -401,7 +401,7 @@ static void meson_uart_set_termios(struct uart_port *port,
> AML_UART_FRAME_ERR;
>
> uart_update_timeout(port, termios->c_cflag, baud);
> - spin_unlock_irqrestore(&port->lock, flags);
> + uart_port_unlock_irqrestore(port, flags);
> }
>
> static int meson_uart_verify_port(struct uart_port *port,
> @@ -460,14 +460,14 @@ static int meson_uart_poll_get_char(struct uart_port *port)
> u32 c;
> unsigned long flags;
>
> - spin_lock_irqsave(&port->lock, flags);
> + uart_port_lock_irqsave(port, &flags);
>
> if (readl(port->membase + AML_UART_STATUS) & AML_UART_RX_EMPTY)
> c = NO_POLL_CHAR;
> else
> c = readl(port->membase + AML_UART_RFIFO);
>
> - spin_unlock_irqrestore(&port->lock, flags);
> + uart_port_unlock_irqrestore(port, flags);
>
> return c;
> }
> @@ -478,7 +478,7 @@ static void meson_uart_poll_put_char(struct uart_port *port, unsigned char c)
> u32 reg;
> int ret;
>
> - spin_lock_irqsave(&port->lock, flags);
> + uart_port_lock_irqsave(port, &flags);
>
> /* Wait until FIFO is empty or timeout */
> ret = readl_poll_timeout_atomic(port->membase + AML_UART_STATUS, reg,
> @@ -502,7 +502,7 @@ static void meson_uart_poll_put_char(struct uart_port *port, unsigned char c)
> dev_err(port->dev, "Timeout waiting for UART TX EMPTY\n");
>
> out:
> - spin_unlock_irqrestore(&port->lock, flags);
> + uart_port_unlock_irqrestore(port, flags);
> }
>
> #endif /* CONFIG_CONSOLE_POLL */
> @@ -559,9 +559,9 @@ static void meson_serial_port_write(struct uart_port *port, const char *s,
> if (port->sysrq) {
> locked = 0;
> } else if (oops_in_progress) {
> - locked = spin_trylock(&port->lock);
> + locked = uart_port_trylock(port);
> } else {
> - spin_lock(&port->lock);
> + uart_port_lock(port);
> locked = 1;
> }
>
> @@ -573,7 +573,7 @@ static void meson_serial_port_write(struct uart_port *port, const char *s,
> writel(val, port->membase + AML_UART_CONTROL);
>
> if (locked)
> - spin_unlock(&port->lock);
> + uart_port_unlock(port);
> local_irq_restore(flags);
> }
>

Acked-by: Neil Armstrong <[email protected]>

2023-09-15 09:14:10

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH tty v1 00/74] serial: wrappers for uart port lock

On Thu, 14 Sep 2023, John Ogness wrote:

> When a serial port is used for kernel console output, then all
> modifications to the UART registers which are done from other contexts,
> e.g. getty, termios, are interference points for the kernel console.
>
> So far this has been ignored and the printk output is based on the
> principle of hope. The rework of the console infrastructure which aims to
> support threaded and atomic consoles, requires to mark sections which
> modify the UART registers as unsafe. This allows the atomic write function
> to make informed decisions and eventually to restore operational state. It
> also allows to prevent the regular UART code from modifying UART registers
> while printk output is in progress.

Hi John,

Would this also be useful to enable printing to console while under port's
lock (by postponing the output until the lock is released)?

E.g., 8250_dw.c has had this commented out since the dawn on time:
/*
* FIXME: this deadlocks if port->lock is already held
* dev_err(p->dev, "Couldn't set LCR to %d\n", value);
*/

--
i.


> All modifications of UART registers are guarded by the UART port lock,
> which provides an obvious synchronization point with the console
> infrastructure.
>
> Provide and use wrapper functions for spin_[un]lock*(port->lock)
> invocations so that the console mechanics can be applied later on at a
> single place and does not require to copy the same logic all over the
> drivers.
>
> Patch 1 adds the wrapper functions.
>
> Patches 2-74 switch all uart port locking call sites to use the new
> wrappers. These patches were automatically generated using coccinelle.
> The 2 used coccinelle scripts are included below and executed as
> follows:
>
> $ spatch --sp-file uartlock-1.cocci $FILE
> $ spatch --sp-file uartlock-2.cocci --recursive-includes $FILE
>
> This series brings no functional change.
>
> Patches 2-74 contain identical commit message bodies. Feel free to
> fold them into a single commit if that seems more reasonable.
>
> Thomas Gleixner (74):
> serial: core: Provide port lock wrappers
> serial: core: Use lock wrappers
> serial: 21285: Use port lock wrappers
> serial: 8250_aspeed_vuart: Use port lock wrappers
> serial: 8250_bcm7271: Use port lock wrappers
> serial: 8250: Use port lock wrappers
> serial: 8250_dma: Use port lock wrappers
> serial: 8250_dw: Use port lock wrappers
> serial: 8250_exar: Use port lock wrappers
> serial: 8250_fsl: Use port lock wrappers
> serial: 8250_mtk: Use port lock wrappers
> serial: 8250_omap: Use port lock wrappers
> serial: 8250_pci1xxxx: Use port lock wrappers
> serial: altera_jtaguart: Use port lock wrappers
> serial: altera_uart: Use port lock wrappers
> serial: amba-pl010: Use port lock wrappers
> serial: amba-pl011: Use port lock wrappers
> serial: apb: Use port lock wrappers
> serial: ar933x: Use port lock wrappers
> serial: arc_uart: Use port lock wrappers
> serial: atmel: Use port lock wrappers
> serial: bcm63xx-uart: Use port lock wrappers
> serial: cpm_uart: Use port lock wrappers
> serial: digicolor: Use port lock wrappers
> serial: dz: Use port lock wrappers
> serial: linflexuart: Use port lock wrappers
> serial: fsl_lpuart: Use port lock wrappers
> serial: icom: Use port lock wrappers
> serial: imx: Use port lock wrappers
> serial: ip22zilog: Use port lock wrappers
> serial: jsm: Use port lock wrappers
> serial: liteuart: Use port lock wrappers
> serial: lpc32xx_hs: Use port lock wrappers
> serial: ma35d1: Use port lock wrappers
> serial: mcf: Use port lock wrappers
> serial: men_z135_uart: Use port lock wrappers
> serial: meson: Use port lock wrappers
> serial: milbeaut_usio: Use port lock wrappers
> serial: mpc52xx: Use port lock wrappers
> serial: mps2-uart: Use port lock wrappers
> serial: msm: Use port lock wrappers
> serial: mvebu-uart: Use port lock wrappers
> serial: omap: Use port lock wrappers
> serial: owl: Use port lock wrappers
> serial: pch: Use port lock wrappers
> serial: pic32: Use port lock wrappers
> serial: pmac_zilog: Use port lock wrappers
> serial: pxa: Use port lock wrappers
> serial: qcom-geni: Use port lock wrappers
> serial: rda: Use port lock wrappers
> serial: rp2: Use port lock wrappers
> serial: sa1100: Use port lock wrappers
> serial: samsung_tty: Use port lock wrappers
> serial: sb1250-duart: Use port lock wrappers
> serial: sc16is7xx: Use port lock wrappers
> serial: tegra: Use port lock wrappers
> serial: core: Use port lock wrappers
> serial: mctrl_gpio: Use port lock wrappers
> serial: txx9: Use port lock wrappers
> serial: sh-sci: Use port lock wrappers
> serial: sifive: Use port lock wrappers
> serial: sprd: Use port lock wrappers
> serial: st-asc: Use port lock wrappers
> serial: stm32: Use port lock wrappers
> serial: sunhv: Use port lock wrappers
> serial: sunplus-uart: Use port lock wrappers
> serial: sunsab: Use port lock wrappers
> serial: sunsu: Use port lock wrappers
> serial: sunzilog: Use port lock wrappers
> serial: timbuart: Use port lock wrappers
> serial: uartlite: Use port lock wrappers
> serial: ucc_uart: Use port lock wrappers
> serial: vt8500: Use port lock wrappers
> serial: xilinx_uartps: Use port lock wrappers
>
> drivers/tty/serial/21285.c | 8 +-
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 6 +-
> drivers/tty/serial/8250/8250_bcm7271.c | 28 +++---
> drivers/tty/serial/8250/8250_core.c | 12 +--
> drivers/tty/serial/8250/8250_dma.c | 8 +-
> drivers/tty/serial/8250/8250_dw.c | 8 +-
> drivers/tty/serial/8250/8250_exar.c | 4 +-
> drivers/tty/serial/8250/8250_fsl.c | 6 +-
> drivers/tty/serial/8250/8250_mtk.c | 8 +-
> drivers/tty/serial/8250/8250_omap.c | 52 +++++-----
> drivers/tty/serial/8250/8250_pci1xxxx.c | 8 +-
> drivers/tty/serial/8250/8250_port.c | 100 ++++++++++----------
> drivers/tty/serial/altera_jtaguart.c | 28 +++---
> drivers/tty/serial/altera_uart.c | 20 ++--
> drivers/tty/serial/amba-pl010.c | 20 ++--
> drivers/tty/serial/amba-pl011.c | 72 +++++++-------
> drivers/tty/serial/apbuart.c | 8 +-
> drivers/tty/serial/ar933x_uart.c | 26 ++---
> drivers/tty/serial/arc_uart.c | 16 ++--
> drivers/tty/serial/atmel_serial.c | 24 ++---
> drivers/tty/serial/bcm63xx_uart.c | 22 ++---
> drivers/tty/serial/cpm_uart.c | 8 +-
> drivers/tty/serial/digicolor-usart.c | 18 ++--
> drivers/tty/serial/dz.c | 32 +++----
> drivers/tty/serial/fsl_linflexuart.c | 26 ++---
> drivers/tty/serial/fsl_lpuart.c | 88 ++++++++---------
> drivers/tty/serial/icom.c | 26 ++---
> drivers/tty/serial/imx.c | 84 ++++++++--------
> drivers/tty/serial/ip22zilog.c | 36 +++----
> drivers/tty/serial/jsm/jsm_neo.c | 4 +-
> drivers/tty/serial/jsm/jsm_tty.c | 16 ++--
> drivers/tty/serial/liteuart.c | 20 ++--
> drivers/tty/serial/lpc32xx_hs.c | 26 ++---
> drivers/tty/serial/ma35d1_serial.c | 22 ++---
> drivers/tty/serial/mcf.c | 20 ++--
> drivers/tty/serial/men_z135_uart.c | 8 +-
> drivers/tty/serial/meson_uart.c | 30 +++---
> drivers/tty/serial/milbeaut_usio.c | 16 ++--
> drivers/tty/serial/mpc52xx_uart.c | 12 +--
> drivers/tty/serial/mps2-uart.c | 16 ++--
> drivers/tty/serial/msm_serial.c | 38 ++++----
> drivers/tty/serial/mvebu-uart.c | 18 ++--
> drivers/tty/serial/omap-serial.c | 38 ++++----
> drivers/tty/serial/owl-uart.c | 26 ++---
> drivers/tty/serial/pch_uart.c | 10 +-
> drivers/tty/serial/pic32_uart.c | 20 ++--
> drivers/tty/serial/pmac_zilog.c | 52 +++++-----
> drivers/tty/serial/pxa.c | 30 +++---
> drivers/tty/serial/qcom_geni_serial.c | 8 +-
> drivers/tty/serial/rda-uart.c | 34 +++----
> drivers/tty/serial/rp2.c | 20 ++--
> drivers/tty/serial/sa1100.c | 20 ++--
> drivers/tty/serial/samsung_tty.c | 50 +++++-----
> drivers/tty/serial/sb1250-duart.c | 12 +--
> drivers/tty/serial/sc16is7xx.c | 40 ++++----
> drivers/tty/serial/serial-tegra.c | 32 +++----
> drivers/tty/serial/serial_core.c | 88 ++++++++---------
> drivers/tty/serial/serial_mctrl_gpio.c | 4 +-
> drivers/tty/serial/serial_port.c | 4 +-
> drivers/tty/serial/serial_txx9.c | 26 ++---
> drivers/tty/serial/sh-sci.c | 68 ++++++-------
> drivers/tty/serial/sifive.c | 16 ++--
> drivers/tty/serial/sprd_serial.c | 30 +++---
> drivers/tty/serial/st-asc.c | 18 ++--
> drivers/tty/serial/stm32-usart.c | 38 ++++----
> drivers/tty/serial/sunhv.c | 28 +++---
> drivers/tty/serial/sunplus-uart.c | 26 ++---
> drivers/tty/serial/sunsab.c | 34 +++----
> drivers/tty/serial/sunsu.c | 46 ++++-----
> drivers/tty/serial/sunzilog.c | 42 ++++----
> drivers/tty/serial/timbuart.c | 8 +-
> drivers/tty/serial/uartlite.c | 18 ++--
> drivers/tty/serial/ucc_uart.c | 4 +-
> drivers/tty/serial/vt8500_serial.c | 8 +-
> drivers/tty/serial/xilinx_uartps.c | 56 +++++------
> include/linux/serial_core.h | 91 ++++++++++++++++--
> 76 files changed, 1086 insertions(+), 1007 deletions(-)
>
>
> base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
>

2023-09-15 10:32:21

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 56/74] serial: tegra: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/serial-tegra.c | 32 +++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index d4ec943cb8e9..6d4006b41975 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -411,7 +411,7 @@ static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
divisor = DIV_ROUND_CLOSEST(rate, baud * 16);
}

- spin_lock_irqsave(&tup->uport.lock, flags);
+ uart_port_lock_irqsave(&tup->uport, &flags);
lcr = tup->lcr_shadow;
lcr |= UART_LCR_DLAB;
tegra_uart_write(tup, lcr, UART_LCR);
@@ -424,7 +424,7 @@ static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)

/* Dummy read to ensure the write is posted */
tegra_uart_read(tup, UART_SCR);
- spin_unlock_irqrestore(&tup->uport.lock, flags);
+ uart_port_unlock_irqrestore(&tup->uport, flags);

tup->current_baud = baud;

@@ -522,13 +522,13 @@ static void tegra_uart_tx_dma_complete(void *args)
dmaengine_tx_status(tup->tx_dma_chan, tup->tx_cookie, &state);
count = tup->tx_bytes_requested - state.residue;
async_tx_ack(tup->tx_dma_desc);
- spin_lock_irqsave(&tup->uport.lock, flags);
+ uart_port_lock_irqsave(&tup->uport, &flags);
uart_xmit_advance(&tup->uport, count);
tup->tx_in_progress = 0;
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
uart_write_wakeup(&tup->uport);
tegra_uart_start_next_tx(tup);
- spin_unlock_irqrestore(&tup->uport.lock, flags);
+ uart_port_unlock_irqrestore(&tup->uport, flags);
}

static int tegra_uart_start_tx_dma(struct tegra_uart_port *tup,
@@ -598,13 +598,13 @@ static unsigned int tegra_uart_tx_empty(struct uart_port *u)
unsigned int ret = 0;
unsigned long flags;

- spin_lock_irqsave(&u->lock, flags);
+ uart_port_lock_irqsave(u, &flags);
if (!tup->tx_in_progress) {
unsigned long lsr = tegra_uart_read(tup, UART_LSR);
if ((lsr & TX_EMPTY_STATUS) == TX_EMPTY_STATUS)
ret = TIOCSER_TEMT;
}
- spin_unlock_irqrestore(&u->lock, flags);
+ uart_port_unlock_irqrestore(u, flags);
return ret;
}

@@ -727,7 +727,7 @@ static void tegra_uart_rx_dma_complete(void *args)
struct dma_tx_state state;
enum dma_status status;

- spin_lock_irqsave(&u->lock, flags);
+ uart_port_lock_irqsave(u, &flags);

status = dmaengine_tx_status(tup->rx_dma_chan, tup->rx_cookie, &state);

@@ -749,7 +749,7 @@ static void tegra_uart_rx_dma_complete(void *args)
set_rts(tup, true);

done:
- spin_unlock_irqrestore(&u->lock, flags);
+ uart_port_unlock_irqrestore(u, flags);
}

static void tegra_uart_terminate_rx_dma(struct tegra_uart_port *tup)
@@ -836,7 +836,7 @@ static irqreturn_t tegra_uart_isr(int irq, void *data)
bool is_rx_int = false;
unsigned long flags;

- spin_lock_irqsave(&u->lock, flags);
+ uart_port_lock_irqsave(u, &flags);
while (1) {
iir = tegra_uart_read(tup, UART_IIR);
if (iir & UART_IIR_NO_INT) {
@@ -852,7 +852,7 @@ static irqreturn_t tegra_uart_isr(int irq, void *data)
} else if (is_rx_start) {
tegra_uart_start_rx_dma(tup);
}
- spin_unlock_irqrestore(&u->lock, flags);
+ uart_port_unlock_irqrestore(u, flags);
return IRQ_HANDLED;
}

@@ -969,11 +969,11 @@ static void tegra_uart_hw_deinit(struct tegra_uart_port *tup)
}
}

- spin_lock_irqsave(&tup->uport.lock, flags);
+ uart_port_lock_irqsave(&tup->uport, &flags);
/* Reset the Rx and Tx FIFOs */
tegra_uart_fifo_reset(tup, UART_FCR_CLEAR_XMIT | UART_FCR_CLEAR_RCVR);
tup->current_baud = 0;
- spin_unlock_irqrestore(&tup->uport.lock, flags);
+ uart_port_unlock_irqrestore(&tup->uport, flags);

tup->rx_in_progress = 0;
tup->tx_in_progress = 0;
@@ -1292,7 +1292,7 @@ static void tegra_uart_set_termios(struct uart_port *u,
int ret;

max_divider *= 16;
- spin_lock_irqsave(&u->lock, flags);
+ uart_port_lock_irqsave(u, &flags);

/* Changing configuration, it is safe to stop any rx now */
if (tup->rts_active)
@@ -1341,7 +1341,7 @@ static void tegra_uart_set_termios(struct uart_port *u,
baud = uart_get_baud_rate(u, termios, oldtermios,
parent_clk_rate/max_divider,
parent_clk_rate/16);
- spin_unlock_irqrestore(&u->lock, flags);
+ uart_port_unlock_irqrestore(u, flags);
ret = tegra_set_baudrate(tup, baud);
if (ret < 0) {
dev_err(tup->uport.dev, "Failed to set baud rate\n");
@@ -1349,7 +1349,7 @@ static void tegra_uart_set_termios(struct uart_port *u,
}
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);
- spin_lock_irqsave(&u->lock, flags);
+ uart_port_lock_irqsave(u, &flags);

/* Flow control */
if (termios->c_cflag & CRTSCTS) {
@@ -1382,7 +1382,7 @@ static void tegra_uart_set_termios(struct uart_port *u,
if (termios->c_iflag & IGNBRK)
tup->uport.ignore_status_mask |= UART_LSR_BI;

- spin_unlock_irqrestore(&u->lock, flags);
+ uart_port_unlock_irqrestore(u, flags);
}

static const char *tegra_uart_type(struct uart_port *u)
--
2.39.2

2023-09-15 13:21:20

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 28/74] serial: icom: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/icom.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/icom.c b/drivers/tty/serial/icom.c
index 819f957b6b84..a75eafbcbea3 100644
--- a/drivers/tty/serial/icom.c
+++ b/drivers/tty/serial/icom.c
@@ -929,7 +929,7 @@ static inline void check_modem_status(struct icom_port *icom_port)
char delta_status;
unsigned char status;

- spin_lock(&icom_port->uart_port.lock);
+ uart_port_lock(&icom_port->uart_port);

/*modem input register */
status = readb(&icom_port->dram->isr);
@@ -951,7 +951,7 @@ static inline void check_modem_status(struct icom_port *icom_port)
port.delta_msr_wait);
old_status = status;
}
- spin_unlock(&icom_port->uart_port.lock);
+ uart_port_unlock(&icom_port->uart_port);
}

static void xmit_interrupt(u16 port_int_reg, struct icom_port *icom_port)
@@ -1093,7 +1093,7 @@ static void process_interrupt(u16 port_int_reg,
struct icom_port *icom_port)
{

- spin_lock(&icom_port->uart_port.lock);
+ uart_port_lock(&icom_port->uart_port);
trace(icom_port, "INTERRUPT", port_int_reg);

if (port_int_reg & (INT_XMIT_COMPLETED | INT_XMIT_DISABLED))
@@ -1102,7 +1102,7 @@ static void process_interrupt(u16 port_int_reg,
if (port_int_reg & INT_RCV_COMPLETED)
recv_interrupt(port_int_reg, icom_port);

- spin_unlock(&icom_port->uart_port.lock);
+ uart_port_unlock(&icom_port->uart_port);
}

static irqreturn_t icom_interrupt(int irq, void *dev_id)
@@ -1186,14 +1186,14 @@ static unsigned int icom_tx_empty(struct uart_port *port)
int ret;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
if (le16_to_cpu(icom_port->statStg->xmit[0].flags) &
SA_FLAGS_READY_TO_XMIT)
ret = TIOCSER_TEMT;
else
ret = 0;

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
return ret;
}

@@ -1276,7 +1276,7 @@ static void icom_send_xchar(struct uart_port *port, char ch)

/* wait .1 sec to send char */
for (index = 0; index < 10; index++) {
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
xdata = readb(&icom_port->dram->xchar);
if (xdata == 0x00) {
trace(icom_port, "QUICK_WRITE", 0);
@@ -1284,10 +1284,10 @@ static void icom_send_xchar(struct uart_port *port, char ch)

/* flush write operation */
xdata = readb(&icom_port->dram->xchar);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
break;
}
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
msleep(10);
}
}
@@ -1307,7 +1307,7 @@ static void icom_break(struct uart_port *port, int break_state)
unsigned char cmdReg;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
trace(icom_port, "BREAK", 0);
cmdReg = readb(&icom_port->dram->CmdReg);
if (break_state == -1) {
@@ -1315,7 +1315,7 @@ static void icom_break(struct uart_port *port, int break_state)
} else {
writeb(cmdReg & ~CMD_SND_BREAK, &icom_port->dram->CmdReg);
}
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int icom_open(struct uart_port *port)
@@ -1365,7 +1365,7 @@ static void icom_set_termios(struct uart_port *port, struct ktermios *termios,
unsigned long offset;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
trace(icom_port, "CHANGE_SPEED", 0);

cflag = termios->c_cflag;
@@ -1516,7 +1516,7 @@ static void icom_set_termios(struct uart_port *port, struct ktermios *termios,
trace(icom_port, "XR_ENAB", 0);
writeb(CMD_XMIT_RCV_ENABLE, &icom_port->dram->CmdReg);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *icom_type(struct uart_port *port)
--
2.39.2

2023-09-15 13:43:38

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 36/74] serial: men_z135_uart: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/men_z135_uart.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/men_z135_uart.c b/drivers/tty/serial/men_z135_uart.c
index d2502aaa3e8c..8048fa542fc4 100644
--- a/drivers/tty/serial/men_z135_uart.c
+++ b/drivers/tty/serial/men_z135_uart.c
@@ -392,7 +392,7 @@ static irqreturn_t men_z135_intr(int irq, void *data)
if (!irq_id)
goto out;

- spin_lock(&port->lock);
+ uart_port_lock(port);
/* It's save to write to IIR[7:6] RXC[9:8] */
iowrite8(irq_id, port->membase + MEN_Z135_STAT_REG);

@@ -418,7 +418,7 @@ static irqreturn_t men_z135_intr(int irq, void *data)
handled = true;
}

- spin_unlock(&port->lock);
+ uart_port_unlock(port);
out:
return IRQ_RETVAL(handled);
}
@@ -708,7 +708,7 @@ static void men_z135_set_termios(struct uart_port *port,

baud = uart_get_baud_rate(port, termios, old, 0, uart_freq / 16);

- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(port);
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);

@@ -716,7 +716,7 @@ static void men_z135_set_termios(struct uart_port *port,
iowrite32(bd_reg, port->membase + MEN_Z135_BAUD_REG);

uart_update_timeout(port, termios->c_cflag, baud);
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);
}

static const char *men_z135_type(struct uart_port *port)
--
2.39.2

2023-09-15 14:33:43

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 45/74] serial: pch: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/pch_uart.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index cc83b772b7ca..436cc6d52a11 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -1347,7 +1347,7 @@ static void pch_uart_set_termios(struct uart_port *port,
baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);

spin_lock_irqsave(&priv->lock, flags);
- spin_lock(&port->lock);
+ uart_port_lock(port);

uart_update_timeout(port, termios->c_cflag, baud);
rtn = pch_uart_hal_set_line(priv, baud, parity, bits, stb);
@@ -1360,7 +1360,7 @@ static void pch_uart_set_termios(struct uart_port *port,
tty_termios_encode_baud_rate(termios, baud, baud);

out:
- spin_unlock(&port->lock);
+ uart_port_unlock(port);
spin_unlock_irqrestore(&priv->lock, flags);
}

@@ -1581,10 +1581,10 @@ pch_console_write(struct console *co, const char *s, unsigned int count)
port_locked = 0;
} else if (oops_in_progress) {
priv_locked = spin_trylock(&priv->lock);
- port_locked = spin_trylock(&priv->port.lock);
+ port_locked = uart_port_trylock(&priv->port);
} else {
spin_lock(&priv->lock);
- spin_lock(&priv->port.lock);
+ uart_port_lock(&priv->port);
}

/*
@@ -1604,7 +1604,7 @@ pch_console_write(struct console *co, const char *s, unsigned int count)
iowrite8(ier, priv->membase + UART_IER);

if (port_locked)
- spin_unlock(&priv->port.lock);
+ uart_port_unlock(&priv->port);
if (priv_locked)
spin_unlock(&priv->lock);
local_irq_restore(flags);
--
2.39.2

2023-09-15 17:04:13

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 35/74] serial: mcf: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/mcf.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/mcf.c b/drivers/tty/serial/mcf.c
index 1666ce012e5e..91b15243f6c6 100644
--- a/drivers/tty/serial/mcf.c
+++ b/drivers/tty/serial/mcf.c
@@ -135,12 +135,12 @@ static void mcf_break_ctl(struct uart_port *port, int break_state)
{
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
if (break_state == -1)
writeb(MCFUART_UCR_CMDBREAKSTART, port->membase + MCFUART_UCR);
else
writeb(MCFUART_UCR_CMDBREAKSTOP, port->membase + MCFUART_UCR);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/****************************************************************************/
@@ -150,7 +150,7 @@ static int mcf_startup(struct uart_port *port)
struct mcf_uart *pp = container_of(port, struct mcf_uart, port);
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Reset UART, get it into known state... */
writeb(MCFUART_UCR_CMDRESETRX, port->membase + MCFUART_UCR);
@@ -164,7 +164,7 @@ static int mcf_startup(struct uart_port *port)
pp->imr = MCFUART_UIR_RXREADY;
writeb(pp->imr, port->membase + MCFUART_UIMR);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

return 0;
}
@@ -176,7 +176,7 @@ static void mcf_shutdown(struct uart_port *port)
struct mcf_uart *pp = container_of(port, struct mcf_uart, port);
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Disable all interrupts now */
pp->imr = 0;
@@ -186,7 +186,7 @@ static void mcf_shutdown(struct uart_port *port)
writeb(MCFUART_UCR_CMDRESETRX, port->membase + MCFUART_UCR);
writeb(MCFUART_UCR_CMDRESETTX, port->membase + MCFUART_UCR);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/****************************************************************************/
@@ -252,7 +252,7 @@ static void mcf_set_termios(struct uart_port *port, struct ktermios *termios,
mr2 |= MCFUART_MR2_TXCTS;
}

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
if (port->rs485.flags & SER_RS485_ENABLED) {
dev_dbg(port->dev, "Setting UART to RS485\n");
mr2 |= MCFUART_MR2_TXRTS;
@@ -273,7 +273,7 @@ static void mcf_set_termios(struct uart_port *port, struct ktermios *termios,
port->membase + MCFUART_UCSR);
writeb(MCFUART_UCR_RXENABLE | MCFUART_UCR_TXENABLE,
port->membase + MCFUART_UCR);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

/****************************************************************************/
@@ -350,7 +350,7 @@ static irqreturn_t mcf_interrupt(int irq, void *data)

isr = readb(port->membase + MCFUART_UISR) & pp->imr;

- spin_lock(&port->lock);
+ uart_port_lock(port);
if (isr & MCFUART_UIR_RXREADY) {
mcf_rx_chars(pp);
ret = IRQ_HANDLED;
@@ -359,7 +359,7 @@ static irqreturn_t mcf_interrupt(int irq, void *data)
mcf_tx_chars(pp);
ret = IRQ_HANDLED;
}
- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return ret;
}
--
2.39.2

2023-09-15 17:37:07

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [PATCH tty v1 00/74] serial: wrappers for uart port lock

On Fri, 15 Sep 2023, Thomas Gleixner wrote:

> >> Patches 2-74 switch all uart port locking call sites to use the new
> >> wrappers. These patches were automatically generated using coccinelle.
> >
> > Hmm, no need to do this for drivers/tty/serial/zs.c?
>
> zs.c does not use port lock at all. It has like a couple of other
> drivers a local homebrewn spinlock.

Ah, right, that's because there are registers shared between two ports
within one SCC, so the spinlock has to be shared as well.

This also indicates that dz.c is wrong and shouldn't be using a per-port
spinlock as the DZ has a shared register set between all the four ports
(it's a serial line multiplexer rather than a set discrete ports; up to 8
lines are architecturally supported, though we don't have support in Linux
for systems having those), e.g. there's only one 16-bit receiver buffer
register for all the four ports, supplying the 8-bit character received
along with the receive status and the number of the line this data arrived
on, and similarly there's only one transmit data register, which supplies
data to the next enabled line whose transmit buffer needs servicing, and
the chip routes the data itself. Therefore the driver ought to use a
shared spinlock too.

I guess it wasn't noticed so far because DZ devices aren't that common
(and my usual test machine is currently broken too, pending an SRAM chip
replacement, hopefully in the next few weeks) and then hardly ever more
than one serial line has been used at a time with these devices. It looks
like the first issue for me to look into once the machine has been fixed.

Maybe dz.c shouldn't be touched by this series then? (Though obviously
both drivers will have to be eventually adapted for the ultimate console
rework.)

Thanks for your input, as it turns out it has had an unexpected outcome.

Maciej

2023-09-15 17:56:32

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 05/74] serial: 8250_bcm7271: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/8250/8250_bcm7271.c | 28 +++++++++++++-------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c
index aa5aff046756..ff0662c68725 100644
--- a/drivers/tty/serial/8250/8250_bcm7271.c
+++ b/drivers/tty/serial/8250/8250_bcm7271.c
@@ -567,7 +567,7 @@ static irqreturn_t brcmuart_isr(int irq, void *dev_id)
if (interrupts == 0)
return IRQ_NONE;

- spin_lock_irqsave(&up->lock, flags);
+ uart_port_lock_irqsave(up, &flags);

/* Clear all interrupts */
udma_writel(priv, REGS_DMA_ISR, UDMA_INTR_CLEAR, interrupts);
@@ -581,7 +581,7 @@ static irqreturn_t brcmuart_isr(int irq, void *dev_id)
if ((rval | tval) == 0)
dev_warn(dev, "Spurious interrupt: 0x%x\n", interrupts);

- spin_unlock_irqrestore(&up->lock, flags);
+ uart_port_unlock_irqrestore(up, flags);
return IRQ_HANDLED;
}

@@ -608,10 +608,10 @@ static int brcmuart_startup(struct uart_port *port)
*
* Synchronize UART_IER access against the console.
*/
- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(port);
up->ier &= ~UART_IER_RDI;
serial_port_out(port, UART_IER, up->ier);
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(port);

priv->tx_running = false;
priv->dma.rx_dma = NULL;
@@ -629,7 +629,7 @@ static void brcmuart_shutdown(struct uart_port *port)
struct brcmuart_priv *priv = up->port.private_data;
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
priv->shutdown = true;
if (priv->dma_enabled) {
stop_rx_dma(up);
@@ -645,7 +645,7 @@ static void brcmuart_shutdown(struct uart_port *port)
*/
up->dma = NULL;

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
serial8250_do_shutdown(port);
}

@@ -788,7 +788,7 @@ static int brcmuart_handle_irq(struct uart_port *p)
* interrupt but there is no data ready.
*/
if (((iir & UART_IIR_ID) == UART_IIR_RX_TIMEOUT) && !(priv->shutdown)) {
- spin_lock_irqsave(&p->lock, flags);
+ uart_port_lock_irqsave(p, &flags);
status = serial_port_in(p, UART_LSR);
if ((status & UART_LSR_DR) == 0) {

@@ -813,7 +813,7 @@ static int brcmuart_handle_irq(struct uart_port *p)

handled = 1;
}
- spin_unlock_irqrestore(&p->lock, flags);
+ uart_port_unlock_irqrestore(p, flags);
if (handled)
return 1;
}
@@ -831,7 +831,7 @@ static enum hrtimer_restart brcmuart_hrtimer_func(struct hrtimer *t)
if (priv->shutdown)
return HRTIMER_NORESTART;

- spin_lock_irqsave(&p->lock, flags);
+ uart_port_lock_irqsave(p, &flags);
status = serial_port_in(p, UART_LSR);

/*
@@ -855,7 +855,7 @@ static enum hrtimer_restart brcmuart_hrtimer_func(struct hrtimer *t)
status |= UART_MCR_RTS;
serial_port_out(p, UART_MCR, status);
}
- spin_unlock_irqrestore(&p->lock, flags);
+ uart_port_unlock_irqrestore(p, flags);
return HRTIMER_NORESTART;
}

@@ -1154,10 +1154,10 @@ static int __maybe_unused brcmuart_suspend(struct device *dev)
* This will prevent resume from enabling RTS before the
* baud rate has been restored.
*/
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
priv->saved_mctrl = port->mctrl;
port->mctrl &= ~TIOCM_RTS;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

serial8250_suspend_port(priv->line);
clk_disable_unprepare(priv->baud_mux_clk);
@@ -1196,10 +1196,10 @@ static int __maybe_unused brcmuart_resume(struct device *dev)

if (priv->saved_mctrl & TIOCM_RTS) {
/* Restore RTS */
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
port->mctrl |= TIOCM_RTS;
port->ops->set_mctrl(port, port->mctrl);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

return 0;
--
2.39.2

2023-09-15 19:39:52

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 51/74] serial: rp2: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/rp2.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/rp2.c b/drivers/tty/serial/rp2.c
index de220ac8ca54..d46a81cddfcd 100644
--- a/drivers/tty/serial/rp2.c
+++ b/drivers/tty/serial/rp2.c
@@ -276,9 +276,9 @@ static unsigned int rp2_uart_tx_empty(struct uart_port *port)
* But the TXEMPTY bit doesn't seem to work unless the TX IRQ is
* enabled.
*/
- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port, &flags);
tx_fifo_bytes = readw(up->base + RP2_TX_FIFO_COUNT);
- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port, flags);

return tx_fifo_bytes ? 0 : TIOCSER_TEMT;
}
@@ -323,10 +323,10 @@ static void rp2_uart_break_ctl(struct uart_port *port, int break_state)
{
unsigned long flags;

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
rp2_rmw(port_to_up(port), RP2_TXRX_CTL, RP2_TXRX_CTL_BREAK_m,
break_state ? RP2_TXRX_CTL_BREAK_m : 0);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void rp2_uart_enable_ms(struct uart_port *port)
@@ -383,7 +383,7 @@ static void rp2_uart_set_termios(struct uart_port *port, struct ktermios *new,
if (tty_termios_baud_rate(new))
tty_termios_encode_baud_rate(new, baud, baud);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* ignore all characters if CREAD is not set */
port->ignore_status_mask = (new->c_cflag & CREAD) ? 0 : RP2_DUMMY_READ;
@@ -391,7 +391,7 @@ static void rp2_uart_set_termios(struct uart_port *port, struct ktermios *new,
__rp2_uart_set_termios(up, new->c_cflag, new->c_iflag, baud_div);
uart_update_timeout(port, new->c_cflag, baud);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static void rp2_rx_chars(struct rp2_uart_port *up)
@@ -440,7 +440,7 @@ static void rp2_ch_interrupt(struct rp2_uart_port *up)
{
u32 status;

- spin_lock(&up->port.lock);
+ uart_port_lock(&up->port);

/*
* The IRQ status bits are clear-on-write. Other status bits in
@@ -456,7 +456,7 @@ static void rp2_ch_interrupt(struct rp2_uart_port *up)
if (status & RP2_CHAN_STAT_MS_CHANGED_MASK)
wake_up_interruptible(&up->port.state->port.delta_msr_wait);

- spin_unlock(&up->port.lock);
+ uart_port_unlock(&up->port);
}

static int rp2_asic_interrupt(struct rp2_card *card, unsigned int asic_id)
@@ -516,10 +516,10 @@ static void rp2_uart_shutdown(struct uart_port *port)

rp2_uart_break_ctl(port, 0);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
rp2_mask_ch_irq(up, up->idx, 0);
rp2_rmw(up, RP2_CHAN_STAT, 0, 0);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *rp2_uart_type(struct uart_port *port)
--
2.39.2

2023-09-16 04:07:53

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH tty v1 01/74] serial: core: Provide port lock wrappers

On Thu, 14 Sep 2023, John Ogness wrote:

> From: Thomas Gleixner <[email protected]>
>
> When a serial port is used for kernel console output, then all
> modifications to the UART registers which are done from other contexts,
> e.g. getty, termios, are interference points for the kernel console.
>
> So far this has been ignored and the printk output is based on the
> principle of hope. The rework of the console infrastructure which aims to
> support threaded and atomic consoles, requires to mark sections which
> modify the UART registers as unsafe. This allows the atomic write function
> to make informed decisions and eventually to restore operational state. It
> also allows to prevent the regular UART code from modifying UART registers
> while printk output is in progress.
>
> All modifications of UART registers are guarded by the UART port lock,
> which provides an obvious synchronization point with the console
> infrastructure.
>
> Provide wrapper functions for spin_[un]lock*(port->lock) invocations so
> that the console mechanics can be applied later on at a single place and
> does not require to copy the same logic all over the drivers.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> include/linux/serial_core.h | 79 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index bb6f073bc159..f1d5c0d1568c 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -588,6 +588,85 @@ struct uart_port {
> void *private_data; /* generic platform data pointer */
> };
>
> +/**
> + * uart_port_lock - Lock the UART port
> + * @up: Pointer to UART port structure
> + */
> +static inline void uart_port_lock(struct uart_port *up)
> +{
> + spin_lock(&up->lock);
> +}
> +
> +/**
> + * uart_port_lock_irq - Lock the UART port and disable interrupts
> + * @up: Pointer to UART port structure
> + */
> +static inline void uart_port_lock_irq(struct uart_port *up)
> +{
> + spin_lock_irq(&up->lock);
> +}
> +
> +/**
> + * uart_port_lock_irqsave - Lock the UART port, save and disable interrupts
> + * @up: Pointer to UART port structure
> + * @flags: Pointer to interrupt flags storage
> + */
> +static inline void uart_port_lock_irqsave(struct uart_port *up, unsigned long *flags)
> +{
> + spin_lock_irqsave(&up->lock, *flags);
> +}
> +
> +/**
> + * uart_port_trylock - Try to lock the UART port
> + * @up: Pointer to UART port structure
> + *
> + * Returns: True if lock was acquired, false otherwise
> + */
> +static inline bool uart_port_trylock(struct uart_port *up)
> +{
> + return spin_trylock(&up->lock);
> +}
> +
> +/**
> + * uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
> + * @up: Pointer to UART port structure
> + * @flags: Pointer to interrupt flags storage
> + *
> + * Returns: True if lock was acquired, false otherwise
> + */
> +static inline bool uart_port_trylock_irqsave(struct uart_port *up, unsigned long *flags)
> +{
> + return spin_trylock_irqsave(&up->lock, *flags);
> +}
> +
> +/**
> + * uart_port_unlock - Unlock the UART port
> + * @up: Pointer to UART port structure
> + */
> +static inline void uart_port_unlock(struct uart_port *up)
> +{
> + spin_unlock(&up->lock);
> +}
> +
> +/**
> + * uart_port_unlock_irq - Unlock the UART port and re-enable interrupts
> + * @up: Pointer to UART port structure
> + */
> +static inline void uart_port_unlock_irq(struct uart_port *up)
> +{
> + spin_unlock_irq(&up->lock);
> +}
> +
> +/**
> + * uart_port_lock_irqrestore - Unlock the UART port, restore interrupts
> + * @up: Pointer to UART port structure
> + * @flags: The saved interrupt flags for restore
> + */
> +static inline void uart_port_unlock_irqrestore(struct uart_port *up, unsigned long flags)
> +{
> + spin_unlock_irqrestore(&up->lock, flags);
> +}
> +
> static inline int serial_port_in(struct uart_port *up, int offset)
> {
> return up->serial_in(up, offset);
>

Reviewed-by: Ilpo J?rvinen <[email protected]>

--
i.

2023-09-16 07:44:40

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 13/74] serial: 8250_pci1xxxx: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/8250/8250_pci1xxxx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci1xxxx.c b/drivers/tty/serial/8250/8250_pci1xxxx.c
index a3b25779d921..53e238c8cc89 100644
--- a/drivers/tty/serial/8250/8250_pci1xxxx.c
+++ b/drivers/tty/serial/8250/8250_pci1xxxx.c
@@ -225,10 +225,10 @@ static bool pci1xxxx_port_suspend(int line)
if (port->suspended == 0 && port->dev) {
wakeup_mask = readb(up->port.membase + UART_WAKE_MASK_REG);

- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
port->mctrl &= ~TIOCM_OUT2;
port->ops->set_mctrl(port, port->mctrl);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);

ret = (wakeup_mask & UART_WAKE_SRCS) != UART_WAKE_SRCS;
}
@@ -251,10 +251,10 @@ static void pci1xxxx_port_resume(int line)
writeb(UART_WAKE_SRCS, port->membase + UART_WAKE_REG);

if (port->suspended == 0) {
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);
port->mctrl |= TIOCM_OUT2;
port->ops->set_mctrl(port, port->mctrl);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}
mutex_unlock(&tport->mutex);
}
--
2.39.2

2023-09-16 13:12:18

by John Ogness

[permalink] [raw]
Subject: [PATCH tty v1 39/74] serial: mpc52xx: Use port lock wrappers

From: Thomas Gleixner <[email protected]>

When a serial port is used for kernel console output, then all
modifications to the UART registers which are done from other contexts,
e.g. getty, termios, are interference points for the kernel console.

So far this has been ignored and the printk output is based on the
principle of hope. The rework of the console infrastructure which aims to
support threaded and atomic consoles, requires to mark sections which
modify the UART registers as unsafe. This allows the atomic write function
to make informed decisions and eventually to restore operational state. It
also allows to prevent the regular UART code from modifying UART registers
while printk output is in progress.

All modifications of UART registers are guarded by the UART port lock,
which provides an obvious synchronization point with the console
infrastructure.

To avoid adding this functionality to all UART drivers, wrap the
spin_[un]lock*() invocations for uart_port::lock into helper functions
which just contain the spin_[un]lock*() invocations for now. In a
subsequent step these helpers will gain the console synchronization
mechanisms.

Converted with coccinelle. No functional change.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/tty/serial/mpc52xx_uart.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 916507b8f31d..a252465e745f 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -1096,14 +1096,14 @@ static void
mpc52xx_uart_break_ctl(struct uart_port *port, int ctl)
{
unsigned long flags;
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

if (ctl == -1)
psc_ops->command(port, MPC52xx_PSC_START_BRK);
else
psc_ops->command(port, MPC52xx_PSC_STOP_BRK);

- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static int
@@ -1214,7 +1214,7 @@ mpc52xx_uart_set_termios(struct uart_port *port, struct ktermios *new,
}

/* Get the lock */
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(port, &flags);

/* Do our best to flush TX & RX, so we don't lose anything */
/* But we don't wait indefinitely ! */
@@ -1250,7 +1250,7 @@ mpc52xx_uart_set_termios(struct uart_port *port, struct ktermios *new,
psc_ops->command(port, MPC52xx_PSC_RX_ENABLE);

/* We're all set, release the lock */
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(port, flags);
}

static const char *
@@ -1477,11 +1477,11 @@ mpc52xx_uart_int(int irq, void *dev_id)
struct uart_port *port = dev_id;
irqreturn_t ret;

- spin_lock(&port->lock);
+ uart_port_lock(port);

ret = psc_ops->handle_irq(port);

- spin_unlock(&port->lock);
+ uart_port_unlock(port);

return ret;
}
--
2.39.2

2023-09-17 09:27:08

by John Ogness

[permalink] [raw]
Subject: Re: [PATCH tty v1 00/74] serial: wrappers for uart port lock

On 2023-09-15, Ilpo Järvinen <[email protected]> wrote:
> Would this also be useful to enable printing to console while under
> port's lock (by postponing the output until the lock is released)?
>
> E.g., 8250_dw.c has had this commented out since the dawn on time:
> /*
> * FIXME: this deadlocks if port->lock is already held
> * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> */

Yes, this will fix such issues. However, only for consoles that are
converted to the new NBCON console type.

Good news, the 8250 driver will be the flagship driver that is converted
as part of the rework. So this particular issue will be solved then. I
will try to remember this so that I can remove the FIXME in the series.

Thanks for mentioning it.

John

2023-09-17 11:07:19

by John Ogness

[permalink] [raw]
Subject: Re: [PATCH tty v1 00/74] serial: wrappers for uart port lock

On 2023-09-15, "Maciej W. Rozycki" <[email protected]> wrote:
> Maybe dz.c shouldn't be touched by this series then?

Correct. This series is only wrapping the uart port lock, which dz.c is
not using.

> Though obviously both drivers will have to be eventually adapted for
> the ultimate console rework.

Correct.

Thanks for clarifying how the hardware works.

John

2023-09-18 10:06:40

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH tty v1 00/74] serial: wrappers for uart port lock

On Mon, Sep 18, 2023 at 10:29:30AM +0206, John Ogness wrote:
> On 2023-09-14, John Ogness <[email protected]> wrote:
> > Provide and use wrapper functions for spin_[un]lock*(port->lock)
> > invocations so that the console mechanics can be applied later on at a
> > single place and does not require to copy the same logic all over the
> > drivers.
>
> For the full 74-patch series:
>
> Signed-off-by: John Ogness <[email protected]>
>
> Sorry that my SoB was missing from the initial posting.

Thanks for this, I'll rebuild my tree with this added.

greg k-h

2023-09-18 11:43:35

by John Ogness

[permalink] [raw]
Subject: Re: [PATCH tty v1 00/74] serial: wrappers for uart port lock

On 2023-09-14, John Ogness <[email protected]> wrote:
> Provide and use wrapper functions for spin_[un]lock*(port->lock)
> invocations so that the console mechanics can be applied later on at a
> single place and does not require to copy the same logic all over the
> drivers.

For the full 74-patch series:

Signed-off-by: John Ogness <[email protected]>

Sorry that my SoB was missing from the initial posting.

John Ogness

2023-09-19 19:21:05

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH tty v1 01/74] serial: core: Provide port lock wrappers

On Thu 2023-09-14 20:43:18, John Ogness wrote:
> From: Thomas Gleixner <[email protected]>
>
> When a serial port is used for kernel console output, then all
> modifications to the UART registers which are done from other contexts,
> e.g. getty, termios, are interference points for the kernel console.
>
> So far this has been ignored and the printk output is based on the
> principle of hope. The rework of the console infrastructure which aims to
> support threaded and atomic consoles, requires to mark sections which
> modify the UART registers as unsafe. This allows the atomic write function
> to make informed decisions and eventually to restore operational state. It
> also allows to prevent the regular UART code from modifying UART registers
> while printk output is in progress.
>
> All modifications of UART registers are guarded by the UART port lock,
> which provides an obvious synchronization point with the console
> infrastructure.
>
> Provide wrapper functions for spin_[un]lock*(port->lock) invocations so
> that the console mechanics can be applied later on at a single place and
> does not require to copy the same logic all over the drivers.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> include/linux/serial_core.h | 79 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index bb6f073bc159..f1d5c0d1568c 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> +/**
> + * uart_port_lock_irqsave - Lock the UART port, save and disable interrupts
> + * @up: Pointer to UART port structure
> + * @flags: Pointer to interrupt flags storage
> + */
> +static inline void uart_port_lock_irqsave(struct uart_port *up, unsigned long *flags)
> +{
> + spin_lock_irqsave(&up->lock, *flags);
> +}

IMHO, it would have been better to pass the flags variable directly
via a macro as it is done in most *_lock_*_irqsafe() APIs. I mean
something like:

/**
* uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
* @up: Pointer to UART port structure
* @flags: Interrupt flags storage
*
* Returns: True if lock was acquired, false otherwise
*/
#define uart_port_lock_irqsave(up, flags) \
({ \
local_irq_save(flags); \
uart_port_lock(lock) \
})

> +
> +/**
> + * uart_port_trylock - Try to lock the UART port
> + * @up: Pointer to UART port structure
> + *
> + * Returns: True if lock was acquired, false otherwise
> + */
> +static inline bool uart_port_trylock(struct uart_port *up)
> +{
> + return spin_trylock(&up->lock);
> +}
> +
> +/**
> + * uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
> + * @up: Pointer to UART port structure
> + * @flags: Pointer to interrupt flags storage
> + *
> + * Returns: True if lock was acquired, false otherwise
> + */
> +static inline bool uart_port_trylock_irqsave(struct uart_port *up, unsigned long *flags)
> +{
> + return spin_trylock_irqsave(&up->lock, *flags);
> +}

Similar here:

/**
* uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
* @up: Pointer to UART port structure
* @flags: Interrupt flags storage
*
* Returns: True if lock was acquired, false otherwise
*/
#define uart_port_trylock_irqsave(up, flags) \
({ \
bool __ret; \
\
local_irq_save(flags); \
__ret = uart_port_trylock(lock) \
if (!__ret) \
local_irq_restore(flags); \
__ret; \
})

I do not resist on this rather cosmetic change. The current code seems
to be doing what is expected. Feel free to keep it and use:

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr

PS: I am sorry for the late review. I have made a quick look on Monday
and it looked straightforward. I have got this idea today when
having a closer look.

2023-09-19 23:36:05

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH tty v1 01/74] serial: core: Provide port lock wrappers

On Tue, Sep 19 2023 at 16:24, Petr Mladek wrote:
> On Thu 2023-09-14 20:43:18, John Ogness wrote:
> IMHO, it would have been better to pass the flags variable directly
> via a macro as it is done in most *_lock_*_irqsafe() APIs. I mean
> something like:
>
> /**
> * uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
> * @up: Pointer to UART port structure
> * @flags: Interrupt flags storage
> *
> * Returns: True if lock was acquired, false otherwise
> */
> #define uart_port_lock_irqsave(up, flags) \
> ({ \
> local_irq_save(flags); \
> uart_port_lock(lock) \
> })

It's worse.

1) Macros are not type safe by themself and rely on type safety
of the inner workings.

2) Macros are bad for grep as you can't search for a 'struct foo *'
argument. Even semantic parsers have their problems with macro
constructs. I just learned that again when doing this.

3) Macros are just horrible to read

4) If you want to out of line the wrapper later, then you still
have to keep the macro around because the 'flags' argument is by
value and not a pointer.

From a code generation point of view it's completely irrelevant whether
you have a macro or an inline. That was different 25 years ago, but who
cares about museum compilers today.

Thanks,

tglx

2023-09-20 05:46:34

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH tty v1 01/74] serial: core: Provide port lock wrappers

On Tue, Sep 19, 2023 at 04:24:48PM +0200, Petr Mladek wrote:
> On Thu 2023-09-14 20:43:18, John Ogness wrote:
> > From: Thomas Gleixner <[email protected]>
> >
> > When a serial port is used for kernel console output, then all
> > modifications to the UART registers which are done from other contexts,
> > e.g. getty, termios, are interference points for the kernel console.
> >
> > So far this has been ignored and the printk output is based on the
> > principle of hope. The rework of the console infrastructure which aims to
> > support threaded and atomic consoles, requires to mark sections which
> > modify the UART registers as unsafe. This allows the atomic write function
> > to make informed decisions and eventually to restore operational state. It
> > also allows to prevent the regular UART code from modifying UART registers
> > while printk output is in progress.
> >
> > All modifications of UART registers are guarded by the UART port lock,
> > which provides an obvious synchronization point with the console
> > infrastructure.
> >
> > Provide wrapper functions for spin_[un]lock*(port->lock) invocations so
> > that the console mechanics can be applied later on at a single place and
> > does not require to copy the same logic all over the drivers.
> >
> > Signed-off-by: Thomas Gleixner <[email protected]>
> > ---
> > include/linux/serial_core.h | 79 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 79 insertions(+)
> >
> > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> > index bb6f073bc159..f1d5c0d1568c 100644
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > +/**
> > + * uart_port_lock_irqsave - Lock the UART port, save and disable interrupts
> > + * @up: Pointer to UART port structure
> > + * @flags: Pointer to interrupt flags storage
> > + */
> > +static inline void uart_port_lock_irqsave(struct uart_port *up, unsigned long *flags)
> > +{
> > + spin_lock_irqsave(&up->lock, *flags);
> > +}
>
> IMHO, it would have been better to pass the flags variable directly
> via a macro as it is done in most *_lock_*_irqsafe() APIs. I mean
> something like:
>
> /**
> * uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
> * @up: Pointer to UART port structure
> * @flags: Interrupt flags storage
> *
> * Returns: True if lock was acquired, false otherwise
> */
> #define uart_port_lock_irqsave(up, flags) \
> ({ \
> local_irq_save(flags); \
> uart_port_lock(lock) \
> })
>
> > +
> > +/**
> > + * uart_port_trylock - Try to lock the UART port
> > + * @up: Pointer to UART port structure
> > + *
> > + * Returns: True if lock was acquired, false otherwise
> > + */
> > +static inline bool uart_port_trylock(struct uart_port *up)
> > +{
> > + return spin_trylock(&up->lock);
> > +}
> > +
> > +/**
> > + * uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
> > + * @up: Pointer to UART port structure
> > + * @flags: Pointer to interrupt flags storage
> > + *
> > + * Returns: True if lock was acquired, false otherwise
> > + */
> > +static inline bool uart_port_trylock_irqsave(struct uart_port *up, unsigned long *flags)
> > +{
> > + return spin_trylock_irqsave(&up->lock, *flags);
> > +}
>
> Similar here:
>
> /**
> * uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
> * @up: Pointer to UART port structure
> * @flags: Interrupt flags storage
> *
> * Returns: True if lock was acquired, false otherwise
> */
> #define uart_port_trylock_irqsave(up, flags) \
> ({ \
> bool __ret; \
> \
> local_irq_save(flags); \
> __ret = uart_port_trylock(lock) \
> if (!__ret) \
> local_irq_restore(flags); \
> __ret; \
> })

What is the difference here of a macro vs. an inline function going to
do for the resulting binary? The important thing is now we have wrapper
functions, people can tweak them all they want to see if we can get
better results :)

thanks for the review!

greg k-h

2023-09-21 10:37:21

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH tty v1 01/74] serial: core: Provide port lock wrappers

Adding Peter into Cc who also has a big experience with locking APIs.
Well, I think that he would not care ;-)

On Tue 2023-09-19 21:51:12, Thomas Gleixner wrote:
> On Tue, Sep 19 2023 at 16:24, Petr Mladek wrote:
> > On Thu 2023-09-14 20:43:18, John Ogness wrote:
> > IMHO, it would have been better to pass the flags variable directly
> > via a macro as it is done in most *_lock_*_irqsafe() APIs. I mean
> > something like:
> >
> > /**
> > * uart_port_trylock_irqsave - Try to lock the UART port, save and disable interrupts
> > * @up: Pointer to UART port structure
> > * @flags: Interrupt flags storage
> > *
> > * Returns: True if lock was acquired, false otherwise
> > */
> > #define uart_port_lock_irqsave(up, flags) \
> > ({ \
> > local_irq_save(flags); \
> > uart_port_lock(lock) \
> > })
>
> It's worse.
>
> 1) Macros are not type safe by themself and rely on type safety
> of the inner workings.
>
> 2) Macros are bad for grep as you can't search for a 'struct foo *'
> argument. Even semantic parsers have their problems with macro
> constructs. I just learned that again when doing this.
>
> 3) Macros are just horrible to read
>
> 4) If you want to out of line the wrapper later, then you still
> have to keep the macro around because the 'flags' argument is by
> value and not a pointer.
>
> >From a code generation point of view it's completely irrelevant whether
> you have a macro or an inline. That was different 25 years ago, but who
> cares about museum compilers today.

I probably was not clear enough. The difference and the motivation
is to pass the "flags" variable instead of pointer "*flags".

Both most common APIs, local_irq_save(flags) and
spin_lock_irqsave(lock, flags) pass the variable. Also most
subsystem specific wrappers do so.

IMHO, some consistency makes the life easier for developers,
especially for frequently used API. And the patchset seems to be
adding >350 users of the uart_port_lock_*irqsave() API.

I do not know. Maybe using macros was bad decision for local_irq*()
and spin_lock*() API. Anyway, we are going to live with
the uart_port_lock*() API for a looong time. And I want to be sure
that we do not create (small) headaches for future generations.

OK, maybe this particular difference is not important enough.
As I said, I do not resist on the change.

Best Regards,
Petr