From 672448ccf9b6a676f96f9352cbf91f4d35f4084a Mon Sep 17 00:00:00 2001 From: Rickard x Andersson Date: Wed, 21 Feb 2024 12:53:04 +0100 Subject: tty: serial: imx: Fix broken RS485 When about to transmit the function imx_uart_start_tx is called and in some RS485 configurations this function will call imx_uart_stop_rx. The problem is that imx_uart_stop_rx will enable loopback in order to release the RS485 bus, but when loopback is enabled transmitted data will just be looped to RX. This patch fixes the above problem by not enabling loopback when about to transmit. This driver now works well when used for RS485 half duplex master configurations. Fixes: 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal active high") Cc: stable Signed-off-by: Rickard x Andersson Tested-by: Christoph Niedermaier Link: https://lore.kernel.org/r/20240221115304.509811-1-rickaran@axis.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/imx.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 4aa72d5aeafb..e14813250616 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -462,8 +462,7 @@ static void imx_uart_stop_tx(struct uart_port *port) } } -/* called with port.lock taken and irqs off */ -static void imx_uart_stop_rx(struct uart_port *port) +static void imx_uart_stop_rx_with_loopback_ctrl(struct uart_port *port, bool loopback) { struct imx_port *sport = (struct imx_port *)port; u32 ucr1, ucr2, ucr4, uts; @@ -485,7 +484,7 @@ static void imx_uart_stop_rx(struct uart_port *port) /* See SER_RS485_ENABLED/UTS_LOOP comment in imx_uart_probe() */ if (port->rs485.flags & SER_RS485_ENABLED && port->rs485.flags & SER_RS485_RTS_ON_SEND && - sport->have_rtscts && !sport->have_rtsgpio) { + sport->have_rtscts && !sport->have_rtsgpio && loopback) { uts = imx_uart_readl(sport, imx_uart_uts_reg(sport)); uts |= UTS_LOOP; imx_uart_writel(sport, uts, imx_uart_uts_reg(sport)); @@ -497,6 +496,16 @@ static void imx_uart_stop_rx(struct uart_port *port) imx_uart_writel(sport, ucr2, UCR2); } +/* called with port.lock taken and irqs off */ +static void imx_uart_stop_rx(struct uart_port *port) +{ + /* + * Stop RX and enable loopback in order to make sure RS485 bus + * is not blocked. Se comment in imx_uart_probe(). + */ + imx_uart_stop_rx_with_loopback_ctrl(port, true); +} + /* called with port.lock taken and irqs off */ static void imx_uart_enable_ms(struct uart_port *port) { @@ -682,9 +691,14 @@ static void imx_uart_start_tx(struct uart_port *port) imx_uart_rts_inactive(sport, &ucr2); imx_uart_writel(sport, ucr2, UCR2); + /* + * Since we are about to transmit we can not stop RX + * with loopback enabled because that will make our + * transmitted data being just looped to RX. + */ if (!(port->rs485.flags & SER_RS485_RX_DURING_TX) && !port->rs485_rx_during_tx_gpio) - imx_uart_stop_rx(port); + imx_uart_stop_rx_with_loopback_ctrl(port, false); sport->tx_state = WAIT_AFTER_RTS; -- cgit From e5d6bd25f93d6ae158bb4cd04956cb497a85b8ef Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Thu, 22 Feb 2024 11:26:34 -0800 Subject: serial: 8250_dw: Do not reclock if already at correct rate When userspace opens the console, we call set_termios() passing a termios with the console's configured baud rate. Currently this causes dw8250_set_termios() to disable and then re-enable the UART clock at the same frequency as it was originally. This can cause corruption of any concurrent console output. Fix it by skipping the reclocking if we are already at the correct rate. Signed-off-by: Peter Collingbourne Fixes: 4e26b134bd17 ("serial: 8250_dw: clock rate handling for all ACPI platforms") Cc: stable@vger.kernel.org Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20240222192635.1050502-1-pcc@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_dw.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index 2d1f350a4bea..c1d43f040c43 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -357,9 +357,9 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios, long rate; int ret; - clk_disable_unprepare(d->clk); rate = clk_round_rate(d->clk, newrate); - if (rate > 0) { + if (rate > 0 && p->uartclk != rate) { + clk_disable_unprepare(d->clk); /* * Note that any clock-notifer worker will block in * serial8250_update_uartclk() until we are done. @@ -367,8 +367,8 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios, ret = clk_set_rate(d->clk, newrate); if (!ret) p->uartclk = rate; + clk_prepare_enable(d->clk); } - clk_prepare_enable(d->clk); dw8250_do_set_termios(p, termios, old); } -- cgit From 43066e32227ecde674e8ae1fcdd4a1ede67680c2 Mon Sep 17 00:00:00 2001 From: Yicong Yang Date: Mon, 26 Feb 2024 23:23:51 +0800 Subject: serial: port: Don't suspend if the port is still busy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We accidently met the issue that the bash prompt is not shown after the previous command done and until the next input if there's only one CPU (In our issue other CPUs are isolated by isolcpus=). Further analysis shows it's because the port entering runtime suspend even if there's still pending chars in the buffer and the pending chars will only be processed in next device resuming. We are using amba-pl011 and the problematic flow is like below: Bash                                         kworker tty_write()   file_tty_write()     n_tty_write()       uart_write()         __uart_start()           pm_runtime_get() // wakeup waker             queue_work()                                     pm_runtime_work()                                                rpm_resume()                                                 status = RPM_RESUMING                                                 serial_port_runtime_resume()                                                   port->ops->start_tx()                                                     pl011_tx_chars()                                                       uart_write_wakeup()         […]         __uart_start()           pm_runtime_get() < 0 // because runtime status = RPM_RESUMING                                // later data are not commit to the port driver                                                 status = RPM_ACTIVE                                                 rpm_idle() -> rpm_suspend() This patch tries to fix this by checking the port busy before entering runtime suspending. A runtime_suspend callback is added for the port driver. When entering runtime suspend the callback is invoked, if there's still pending chars in the buffer then flush the buffer. Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM") Cc: stable Reviewed-by: Tony Lindgren Reviewed-by: Andy Shevchenko Signed-off-by: Yicong Yang Link: https://lore.kernel.org/r/20240226152351.40924-1-yangyicong@huawei.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial_port.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c index 88975a4df306..72b6f4f326e2 100644 --- a/drivers/tty/serial/serial_port.c +++ b/drivers/tty/serial/serial_port.c @@ -46,8 +46,31 @@ out: return 0; } +static int serial_port_runtime_suspend(struct device *dev) +{ + struct serial_port_device *port_dev = to_serial_base_port_device(dev); + struct uart_port *port = port_dev->port; + unsigned long flags; + bool busy; + + if (port->flags & UPF_DEAD) + return 0; + + uart_port_lock_irqsave(port, &flags); + busy = __serial_port_busy(port); + if (busy) + port->ops->start_tx(port); + uart_port_unlock_irqrestore(port, flags); + + if (busy) + pm_runtime_mark_last_busy(dev); + + return busy ? -EBUSY : 0; +} + static DEFINE_RUNTIME_DEV_PM_OPS(serial_port_pm, - NULL, serial_port_runtime_resume, NULL); + serial_port_runtime_suspend, + serial_port_runtime_resume, NULL); static int serial_port_probe(struct device *dev) { -- cgit From 1581dafaf0d34bc9c428a794a22110d7046d186d Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Thu, 29 Feb 2024 17:15:27 -0500 Subject: vt: fix unicode buffer corruption when deleting characters This is the same issue that was fixed for the VGA text buffer in commit 39cdb68c64d8 ("vt: fix memory overlapping when deleting chars in the buffer"). The cure is also the same i.e. replace memcpy() with memmove() due to the overlaping buffers. Signed-off-by: Nicolas Pitre Fixes: 81732c3b2fed ("tty vt: Fix line garbage in virtual console on command line edition") Cc: stable Link: https://lore.kernel.org/r/sn184on2-3p0q-0qrq-0218-895349s4753o@syhkavp.arg Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/vt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 156efda7c80d..38a765eadbe2 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -381,7 +381,7 @@ static void vc_uniscr_delete(struct vc_data *vc, unsigned int nr) u32 *ln = vc->vc_uni_lines[vc->state.y]; unsigned int x = vc->state.x, cols = vc->vc_cols; - memcpy(&ln[x], &ln[x + nr], (cols - x - nr) * sizeof(*ln)); + memmove(&ln[x], &ln[x + nr], (cols - x - nr) * sizeof(*ln)); memset32(&ln[cols - nr], ' ', nr); } } -- cgit From 74cb7e0355fae9641f825afa389d3fba3b617714 Mon Sep 17 00:00:00 2001 From: Sherry Sun Date: Tue, 5 Mar 2024 09:57:06 +0800 Subject: tty: serial: fsl_lpuart: avoid idle preamble pending if CTS is enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the remote uart device is not connected or not enabled after booting up, the CTS line is high by default. At this time, if we enable the flow control when opening the device(for example, using “stty -F /dev/ttyLP4 crtscts” command), there will be a pending idle preamble(first writing 0 and then writing 1 to UARTCTRL_TE will queue an idle preamble) that cannot be sent out, resulting in the uart port fail to close(waiting for TX empty), so the user space stty will have to wait for a long time or forever. This is an LPUART IP bug(idle preamble has higher priority than CTS), here add a workaround patch to enable TX CTS after enabling UARTCTRL_TE, so that the idle preamble does not get stuck due to CTS is deasserted. Fixes: 380c966c093e ("tty: serial: fsl_lpuart: add 32-bit register interface support") Cc: stable Signed-off-by: Sherry Sun Reviewed-by: Alexander Sverdlin Link: https://lore.kernel.org/r/20240305015706.1050769-1-sherry.sun@nxp.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/fsl_lpuart.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index 5ddf110aedbe..bbcbc91482af 100644 --- a/drivers/tty/serial/fsl_lpuart.c +++ b/drivers/tty/serial/fsl_lpuart.c @@ -2345,9 +2345,12 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios, lpuart32_write(&sport->port, bd, UARTBAUD); lpuart32_serial_setbrg(sport, baud); - lpuart32_write(&sport->port, modem, UARTMODIR); - lpuart32_write(&sport->port, ctrl, UARTCTRL); + /* disable CTS before enabling UARTCTRL_TE to avoid pending idle preamble */ + lpuart32_write(&sport->port, modem & ~UARTMODIR_TXCTSE, UARTMODIR); /* restore control register */ + lpuart32_write(&sport->port, ctrl, UARTCTRL); + /* re-enable the CTS if needed */ + lpuart32_write(&sport->port, modem, UARTMODIR); if ((ctrl & (UARTCTRL_PE | UARTCTRL_M)) == UARTCTRL_PE) sport->is_cs7 = true; -- cgit From 3d9319c27ceb35fa3d2c8b15508967f3fc7e5b78 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Mon, 4 Mar 2024 17:49:53 -0800 Subject: Revert "tty: serial: simplify qcom_geni_serial_send_chunk_fifo()" This reverts commit 5c7e105cd156fc9adf5294a83623d7a40c15f9b9. As identified by KASAN, the simplification done by the cleanup patch was not legal. >From tracing through the code, it can be seen that we're transmitting from a 4096-byte circular buffer. We copy anywhere from 1-4 bytes from it each time. The simplification runs into trouble when we get near the end of the circular buffer. For instance, we might start out with xmit->tail = 4094 and we want to transfer 4 bytes. With the code before simplification this was no problem. We'd read buf[4094], buf[4095], buf[0], and buf[1]. With the new code we'll do a memcpy(&buf[4094], 4) which reads 2 bytes past the end of the buffer and then skips transmitting what's at buf[0] and buf[1]. KASAN isn't 100% consistent at reporting this for me, but to be extra confident in the analysis, I added traces of the tail and tx_bytes and then wrote a test program: while true; do echo -n "abcdefghijklmnopqrstuvwxyz0" > /dev/ttyMSM0 sleep .1 done I watched the traces over SSH and saw: qcom_geni_serial_send_chunk_fifo: 4093 4 qcom_geni_serial_send_chunk_fifo: 1 3 Which indicated that one byte should be missing. Sure enough the output that should have been: abcdefghijklmnopqrstuvwxyz0 In one case was actually missing a byte: abcdefghijklmnopqrstuvwyz0 Running "ls -al" on large directories also made the missing bytes obvious since columns didn't line up. While the original code may not be the most elegant, we only talking about copying up to 4 bytes here. Let's just go back to the code that worked. Fixes: 5c7e105cd156 ("tty: serial: simplify qcom_geni_serial_send_chunk_fifo()") Cc: stable Signed-off-by: Douglas Anderson Acked-by: Jiri Slaby Tested-by: Johan Hovold Link: https://lore.kernel.org/r/20240304174952.1.I920a314049b345efd1f69d708e7f74d2213d0b49@changeid Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/qcom_geni_serial.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index e63a8fbe63bd..99e08737f293 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -851,19 +851,21 @@ static void qcom_geni_serial_stop_tx(struct uart_port *uport) } static void qcom_geni_serial_send_chunk_fifo(struct uart_port *uport, - unsigned int remaining) + unsigned int chunk) { struct qcom_geni_serial_port *port = to_dev_port(uport); struct circ_buf *xmit = &uport->state->xmit; - unsigned int tx_bytes; + unsigned int tx_bytes, c, remaining = chunk; u8 buf[BYTES_PER_FIFO_WORD]; while (remaining) { memset(buf, 0, sizeof(buf)); tx_bytes = min(remaining, BYTES_PER_FIFO_WORD); - memcpy(buf, &xmit->buf[xmit->tail], tx_bytes); - uart_xmit_advance(uport, tx_bytes); + for (c = 0; c < tx_bytes ; c++) { + buf[c] = xmit->buf[xmit->tail]; + uart_xmit_advance(uport, 1); + } iowrite32_rep(uport->membase + SE_GENI_TX_FIFOn, buf, 1); -- cgit