From e2cbfebe8ceda68ff1d860b4ca138d74003207c5 Mon Sep 17 00:00:00 2001 From: Miroslav Ondra Date: Sat, 21 Dec 2024 20:11:04 +0100 Subject: [PATCH 1/3] serial: amba-pl011: Fix RTS handling in RS485 mode commit 2c1fd53af21b8cb13878b054894d33d3383eb1f3 upstream. Data loss on serial line was observed during communication through serial ports ttyAMA1 and ttyAMA2 interconnected via RS485 transcievers. Both ports are in one BCM2711 (Compute Module CM40) and they share the same interrupt line. The problem is caused by long waiting for tx queue flush in the function pl011_rs485_tx_stop. Udelay or mdelay are used to wait. The function is called from the interrupt handler. If multiple devices share a single interrupt line, late processing of pending interrupts and data loss may occur. When operation of both devices are synchronous, collisions are quite often. This rework is based on the method used in tty/serial/imx.c Use hrtimer instead of udelay and mdelay calls. Replace simple bool variable rs485_tx_started by 4-state variable rs485_tx_state. Tested-by: Lino Sanfilippo Signed-off-by: Miroslav Ondra Link: https://lore.kernel.org/r/20241221-amba-rts-v3-1-d3d444681419@faster.cz Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/amba-pl011.c | 126 ++++++++++++++++++++++++-------- 1 file changed, 96 insertions(+), 30 deletions(-) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index 1e24bbc1f00679..3cb58086b38df9 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -262,6 +262,13 @@ struct pl011_dmatx_data { bool queued; }; +enum pl011_rs485_tx_state { + OFF, + WAIT_AFTER_RTS, + SEND, + WAIT_AFTER_SEND, +}; + /* * We wrap our port structure around the generic uart_port. */ @@ -275,8 +282,10 @@ struct uart_amba_port { unsigned int fifosize; /* vendor-specific */ unsigned int fixed_baud; /* vendor-set fixed baud rate */ char type[12]; - bool rs485_tx_started; - unsigned int rs485_tx_drain_interval; /* usecs */ + ktime_t rs485_tx_drain_interval; /* nano */ + enum pl011_rs485_tx_state rs485_tx_state; + struct hrtimer trigger_start_tx; + struct hrtimer trigger_stop_tx; #ifdef CONFIG_DMA_ENGINE /* DMA stuff */ unsigned int dmacr; /* dma control reg */ @@ -1281,30 +1290,31 @@ static inline bool pl011_dma_rx_running(struct uart_amba_port *uap) static void pl011_rs485_tx_stop(struct uart_amba_port *uap) { - /* - * To be on the safe side only time out after twice as many iterations - * as fifo size. - */ - const int MAX_TX_DRAIN_ITERS = uap->port.fifosize * 2; struct uart_port *port = &uap->port; - int i = 0; u32 cr; - /* Wait until hardware tx queue is empty */ - while (!pl011_tx_empty(port)) { - if (i > MAX_TX_DRAIN_ITERS) { - dev_warn(port->dev, - "timeout while draining hardware tx queue\n"); - break; - } + if (uap->rs485_tx_state == SEND) + uap->rs485_tx_state = WAIT_AFTER_SEND; - udelay(uap->rs485_tx_drain_interval); - i++; + if (uap->rs485_tx_state == WAIT_AFTER_SEND) { + /* Schedule hrtimer if tx queue not empty */ + if (!pl011_tx_empty(port)) { + hrtimer_start(&uap->trigger_stop_tx, + uap->rs485_tx_drain_interval, + HRTIMER_MODE_REL); + return; + } + if (port->rs485.delay_rts_after_send > 0) { + hrtimer_start(&uap->trigger_stop_tx, + ms_to_ktime(port->rs485.delay_rts_after_send), + HRTIMER_MODE_REL); + return; + } + /* Continue without any delay */ + } else if (uap->rs485_tx_state == WAIT_AFTER_RTS) { + hrtimer_try_to_cancel(&uap->trigger_start_tx); } - if (port->rs485.delay_rts_after_send) - mdelay(port->rs485.delay_rts_after_send); - cr = pl011_read(uap, REG_CR); if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) @@ -1317,7 +1327,7 @@ static void pl011_rs485_tx_stop(struct uart_amba_port *uap) cr |= UART011_CR_RXE; pl011_write(cr, uap, REG_CR); - uap->rs485_tx_started = false; + uap->rs485_tx_state = OFF; } static void pl011_stop_tx(struct uart_port *port) @@ -1325,11 +1335,18 @@ static void pl011_stop_tx(struct uart_port *port) struct uart_amba_port *uap = container_of(port, struct uart_amba_port, port); + if (port->rs485.flags & SER_RS485_ENABLED && + uap->rs485_tx_state == WAIT_AFTER_RTS) { + pl011_rs485_tx_stop(uap); + return; + } + uap->im &= ~UART011_TXIM; pl011_write(uap->im, uap, REG_IMSC); pl011_dma_tx_stop(uap); - if ((port->rs485.flags & SER_RS485_ENABLED) && uap->rs485_tx_started) + if (port->rs485.flags & SER_RS485_ENABLED && + uap->rs485_tx_state != OFF) pl011_rs485_tx_stop(uap); } @@ -1349,10 +1366,19 @@ static void pl011_rs485_tx_start(struct uart_amba_port *uap) struct uart_port *port = &uap->port; u32 cr; + if (uap->rs485_tx_state == WAIT_AFTER_RTS) { + uap->rs485_tx_state = SEND; + return; + } + if (uap->rs485_tx_state == WAIT_AFTER_SEND) { + hrtimer_try_to_cancel(&uap->trigger_stop_tx); + uap->rs485_tx_state = SEND; + return; + } + /* uap->rs485_tx_state == OFF */ /* Enable transmitter */ cr = pl011_read(uap, REG_CR); cr |= UART011_CR_TXE; - /* Disable receiver if half-duplex */ if (!(port->rs485.flags & SER_RS485_RX_DURING_TX)) cr &= ~UART011_CR_RXE; @@ -1364,10 +1390,14 @@ static void pl011_rs485_tx_start(struct uart_amba_port *uap) pl011_write(cr, uap, REG_CR); - if (port->rs485.delay_rts_before_send) - mdelay(port->rs485.delay_rts_before_send); - - uap->rs485_tx_started = true; + if (port->rs485.delay_rts_before_send > 0) { + uap->rs485_tx_state = WAIT_AFTER_RTS; + hrtimer_start(&uap->trigger_start_tx, + ms_to_ktime(port->rs485.delay_rts_before_send), + HRTIMER_MODE_REL); + } else { + uap->rs485_tx_state = SEND; + } } static void pl011_start_tx(struct uart_port *port) @@ -1376,13 +1406,44 @@ static void pl011_start_tx(struct uart_port *port) container_of(port, struct uart_amba_port, port); if ((uap->port.rs485.flags & SER_RS485_ENABLED) && - !uap->rs485_tx_started) + uap->rs485_tx_state != SEND) { pl011_rs485_tx_start(uap); + if (uap->rs485_tx_state == WAIT_AFTER_RTS) + return; + } if (!pl011_dma_tx_start(uap)) pl011_start_tx_pio(uap); } +static enum hrtimer_restart pl011_trigger_start_tx(struct hrtimer *t) +{ + struct uart_amba_port *uap = + container_of(t, struct uart_amba_port, trigger_start_tx); + unsigned long flags; + + uart_port_lock_irqsave(&uap->port, &flags); + if (uap->rs485_tx_state == WAIT_AFTER_RTS) + pl011_start_tx(&uap->port); + uart_port_unlock_irqrestore(&uap->port, flags); + + return HRTIMER_NORESTART; +} + +static enum hrtimer_restart pl011_trigger_stop_tx(struct hrtimer *t) +{ + struct uart_amba_port *uap = + container_of(t, struct uart_amba_port, trigger_stop_tx); + unsigned long flags; + + uart_port_lock_irqsave(&uap->port, &flags); + if (uap->rs485_tx_state == WAIT_AFTER_SEND) + pl011_rs485_tx_stop(uap); + uart_port_unlock_irqrestore(&uap->port, flags); + + return HRTIMER_NORESTART; +} + static void pl011_stop_rx(struct uart_port *port) { struct uart_amba_port *uap = @@ -1979,7 +2040,7 @@ static void pl011_shutdown(struct uart_port *port) pl011_dma_shutdown(uap); - if ((port->rs485.flags & SER_RS485_ENABLED) && uap->rs485_tx_started) + if ((port->rs485.flags & SER_RS485_ENABLED && uap->rs485_tx_state != OFF)) pl011_rs485_tx_stop(uap); free_irq(uap->port.irq, uap); @@ -2124,7 +2185,7 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios, * with the given baud rate. We use this as the poll interval when we * wait for the tx queue to empty. */ - uap->rs485_tx_drain_interval = DIV_ROUND_UP(bits * 1000 * 1000, baud); + uap->rs485_tx_drain_interval = ns_to_ktime(DIV_ROUND_UP(bits * NSEC_PER_SEC, baud)); pl011_setup_status_masks(port, termios); @@ -2838,6 +2899,11 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id) } } + hrtimer_init(&uap->trigger_start_tx, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + hrtimer_init(&uap->trigger_stop_tx, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + uap->trigger_start_tx.function = pl011_trigger_start_tx; + uap->trigger_stop_tx.function = pl011_trigger_stop_tx; + ret = pl011_setup_port(&dev->dev, uap, &dev->res, portnr); if (ret) return ret; From 1e70af9152422168203c182a018164ccfb9ff904 Mon Sep 17 00:00:00 2001 From: Dave Stevenson Date: Tue, 20 Jan 2026 16:12:39 +0000 Subject: [PATCH 2/3] serial: pl011: Initialise the hrtimers for RS485 on pl011_axi pl011_axi_probe was missing the equivlent hrtimer initialisation from commit 2c1fd53af21b ("serial: amba-pl011: Fix RTS handling in RS485 mode") and commit 8cb44188b986 ("serial: amba-pl011: Switch to use hrtimer_setup()") resulting in the kernel blowing up as soon as pl011_rs485_stop_tx tried to use them. Add the hrtimer initialisation. Fixes: 120c89e1051a ("serial: pl011: rp1 uart support") Signed-off-by: Dave Stevenson --- drivers/tty/serial/amba-pl011.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index 3cb58086b38df9..a60a7135cdbaef 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -3098,6 +3098,11 @@ static int pl011_axi_probe(struct platform_device *pdev) r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + hrtimer_init(&uap->trigger_start_tx, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + hrtimer_init(&uap->trigger_stop_tx, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + uap->trigger_start_tx.function = pl011_trigger_start_tx; + uap->trigger_stop_tx.function = pl011_trigger_stop_tx; + ret = pl011_setup_port(&pdev->dev, uap, r, portnr); if (ret) return ret; From 0ba9aeec5507f97294aaefbe36eb81b6e6d1ba43 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Sat, 24 Jan 2026 15:44:44 +0100 Subject: [PATCH 3/3] serial: pl011: Add configurable bounded polling for RS485 TX drain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hrtimer-based TX drain introduced in commit 2c1fd53af21b ("serial: amba-pl011: Fix RTS handling in RS485 mode") can cause RX errors in fast request/response RS485 protocols. The hrtimer callback latency (often 50-100µs per iteration) delays RX re-enable, causing the first bytes of slave responses to be lost. Add a configurable bounded polling mode that spins for TX completion with a timeout, falling back to hrtimer only if the timeout is exceeded. This is controlled by two new device tree properties: rs485-tx-drain-poll - enable bounded polling mode rs485-tx-drain-timeout-us - explicit timeout in microseconds (optional) When rs485-tx-drain-poll is set without an explicit timeout, the timeout is auto-calculated as (fifo_size + 1) * character_time, accounting for the FIFO depth plus one character in the shift register. This adapts automatically to baud rate changes. The default behavior (no DT properties) remains unchanged, preserving the pure hrtimer approach for systems where shared IRQ latency is the primary concern. Signed-off-by: Nicolai Buchwitz --- drivers/tty/serial/amba-pl011.c | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index a60a7135cdbaef..67c5faaf895727 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -286,6 +286,8 @@ struct uart_amba_port { enum pl011_rs485_tx_state rs485_tx_state; struct hrtimer trigger_start_tx; struct hrtimer trigger_stop_tx; + bool rs485_tx_drain_poll; /* use bounded polling */ + unsigned int rs485_tx_drain_timeout_us; /* explicit override, 0 = auto */ #ifdef CONFIG_DMA_ENGINE /* DMA stuff */ unsigned int dmacr; /* dma control reg */ @@ -1288,6 +1290,19 @@ static inline bool pl011_dma_rx_running(struct uart_amba_port *uap) #define pl011_dma_flush_buffer NULL #endif +static unsigned int pl011_rs485_tx_drain_timeout(struct uart_amba_port *uap) +{ + if (uap->rs485_tx_drain_timeout_us > 0) + return uap->rs485_tx_drain_timeout_us; + + /* + * Auto-calculate based on FIFO depth plus one character + * in the shift register. + */ + return div_u64(ktime_to_ns(uap->rs485_tx_drain_interval) * + (uap->fifosize + 1), NSEC_PER_USEC); +} + static void pl011_rs485_tx_stop(struct uart_amba_port *uap) { struct uart_port *port = &uap->port; @@ -1297,6 +1312,19 @@ static void pl011_rs485_tx_stop(struct uart_amba_port *uap) uap->rs485_tx_state = WAIT_AFTER_SEND; if (uap->rs485_tx_state == WAIT_AFTER_SEND) { + if (uap->rs485_tx_drain_poll) { + /* Bounded spin with auto-calculated or explicit timeout */ + unsigned int timeout_us = pl011_rs485_tx_drain_timeout(uap); + ktime_t deadline = ktime_add_us(ktime_get(), timeout_us); + + while (!pl011_tx_empty(port)) { + if (ktime_after(ktime_get(), deadline)) { + /* Timeout - fall back to hrtimer */ + break; + } + cpu_relax(); + } + } /* Schedule hrtimer if tx queue not empty */ if (!pl011_tx_empty(port)) { hrtimer_start(&uap->trigger_stop_tx, @@ -2904,6 +2932,11 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id) uap->trigger_start_tx.function = pl011_trigger_start_tx; uap->trigger_stop_tx.function = pl011_trigger_stop_tx; + uap->rs485_tx_drain_poll = device_property_read_bool(&dev->dev, + "rs485-tx-drain-poll"); + device_property_read_u32(&dev->dev, "rs485-tx-drain-timeout-us", + &uap->rs485_tx_drain_timeout_us); + ret = pl011_setup_port(&dev->dev, uap, &dev->res, portnr); if (ret) return ret; @@ -3103,6 +3136,11 @@ static int pl011_axi_probe(struct platform_device *pdev) uap->trigger_start_tx.function = pl011_trigger_start_tx; uap->trigger_stop_tx.function = pl011_trigger_stop_tx; + uap->rs485_tx_drain_poll = device_property_read_bool(&pdev->dev, + "rs485-tx-drain-poll"); + device_property_read_u32(&pdev->dev, "rs485-tx-drain-timeout-us", + &uap->rs485_tx_drain_timeout_us); + ret = pl011_setup_port(&pdev->dev, uap, r, portnr); if (ret) return ret;