From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Date: Fri, 1 Mar 2024 22:45:30 +0100 Subject: [PATCH 17/18] serial: pch: Remove eg20t_port::lock. The struct eg20t_port has a spinlock_t which is used for locking while access I/O of the device. Then there is the uart_portlock which is sometimes and nests within eg20t_port's lock. The uart_port lock is not used while using the struct in pch_uart_hal_read() which might be okay. Then both locks are used in pch_console_write() which looks odd especially the double try_lock part. All in all it looks like the uart_port's lock could replace eg20t_port's lock and simplify the code. Remove eg20t_port::lock and use uart_port's lock for the lock scope. Link: https://lore.kernel.org/r/20240301215246.891055-18-bigeasy@linutronix.de Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- drivers/tty/serial/pch_uart.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) Index: linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c =================================================================== @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:240 @ struct eg20t_port { #define IRQ_NAME_SIZE 17 char irq_name[IRQ_NAME_SIZE]; - - /* protect the eg20t_port private structure and io access to membase */ - spinlock_t lock; }; /** @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:1013 @ static irqreturn_t pch_uart_interrupt(in int next = 1; u8 msr; - spin_lock(&priv->lock); + uart_port_lock(&priv->port); handled = 0; while (next) { iid = pch_uart_hal_get_iid(priv); @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:1073 @ static irqreturn_t pch_uart_interrupt(in handled |= (unsigned int)ret; } - spin_unlock(&priv->lock); + uart_port_unlock(&priv->port); return IRQ_RETVAL(handled); } @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:1184 @ static void pch_uart_break_ctl(struct ua unsigned long flags; priv = container_of(port, struct eg20t_port, port); - spin_lock_irqsave(&priv->lock, flags); + uart_port_lock_irqsave(&priv->port, &flags); pch_uart_hal_set_break(priv, ctl); - spin_unlock_irqrestore(&priv->lock, flags); + uart_port_unlock_irqrestore(&priv->port, flags); } /* Grab any interrupt resources and initialise any low level driver state. */ @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:1336 @ static void pch_uart_set_termios(struct baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16); - spin_lock_irqsave(&priv->lock, flags); - uart_port_lock(port); + uart_port_lock_irqsave(port, &flags); uart_update_timeout(port, termios->c_cflag, baud); rtn = pch_uart_hal_set_line(priv, baud, parity, bits, stb); @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:1349 @ static void pch_uart_set_termios(struct tty_termios_encode_baud_rate(termios, baud, baud); out: - uart_port_unlock(port); - spin_unlock_irqrestore(&priv->lock, flags); + uart_port_unlock_irqrestore(port, flags); } static const char *pch_uart_type(struct uart_port *port) @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:1553 @ pch_console_write(struct console *co, co { struct eg20t_port *priv; unsigned long flags; - int priv_locked = 1; int port_locked = 1; u8 ier; @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:1562 @ pch_console_write(struct console *co, co local_irq_save(flags); if (priv->port.sysrq) { - /* call to uart_handle_sysrq_char already took the priv lock */ - priv_locked = 0; /* serial8250_handle_port() already took the port lock */ port_locked = 0; } else if (oops_in_progress) { - priv_locked = spin_trylock(&priv->lock); port_locked = uart_port_trylock(&priv->port); } else { - spin_lock(&priv->lock); uart_port_lock(&priv->port); } @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:1588 @ pch_console_write(struct console *co, co if (port_locked) uart_port_unlock(&priv->port); - if (priv_locked) - spin_unlock(&priv->lock); local_irq_restore(flags); } @ linux-6.8.2-rt10/drivers/tty/serial/pch_uart.c:1685 @ static struct eg20t_port *pch_uart_init_ pci_enable_msi(pdev); pci_set_master(pdev); - spin_lock_init(&priv->lock); - iobase = pci_resource_start(pdev, 0); mapbase = pci_resource_start(pdev, 1); priv->mapbase = mapbase;