From 56051740bf9720335a1a56e87a99e3491c33ce54 Mon Sep 17 00:00:00 2001 From: Tapio Reijonen Date: Thu, 8 Oct 2026 09:48:26 +0000 Subject: [PATCH] serial: max310x: centralize the RS485 transceiver programming The HDPIXDELAY RTS delays and the MODE1 auto-transceiver enable are programmed in three places - the rs485-config worker, startup() and break_ctl()'s break-off restore - each with its own copy of the register sequence. Move the sequence into a helper, max310x_set_rts_ctl_params(), together with the tx_break guard that keeps the MODE1 write away from a break in progress; the break-off restore reapplies the current configuration through the same helper. No functional change intended. The copies differed only in that startup() clamped the delays to the 4-bit field while the worker wrote them unclamped; the helper clamps, and the difference is unreachable while rs485_config() still rejects delays above 15 ms with -ERANGE. Break-off additionally rewrites HDPIXDELAY with the values it already holds, a no-op on the wire. Assisted-by: Claude:claude-fable-5 Signed-off-by: Tapio Reijonen Link: https://patch.msgid.link/20261008-max310x-rs485-sw-delay-v8-3-6ba881f81e1f@vaisala.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/max310x.c | 85 ++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c index dddd8ed97881..bf2dcf63be88 100644 --- a/drivers/tty/serial/max310x.c +++ b/drivers/tty/serial/max310x.c @@ -932,6 +932,38 @@ static void max310x_set_mctrl(struct uart_port *port, unsigned int mctrl) schedule_work(&one->md_work); } +/* + * Program the chip's RS485 RTS timing (HDPIXDELAY) and the auto + * transceiver control (MODE1) from the current configuration. The + * rs485-config worker, startup() and break_ctl()'s break-off restore + * each carried their own copy of this sequence. + */ +static void max310x_set_rts_ctl_params(struct max310x_one *one) +{ + struct uart_port *port = &one->port; + unsigned int delay; + u8 mode1 = 0; + + delay = (clamp(port->rs485.delay_rts_before_send, 0U, 15U) << 4) | + clamp(port->rs485.delay_rts_after_send, 0U, 15U); + max310x_port_write(port, MAX310X_HDPIXDELAY_REG, delay); + + if (port->rs485.flags & SER_RS485_ENABLED) + mode1 = MAX310X_MODE1_TRNSCVCTRL_BIT; + + /* + * A break owns the transceiver: break_ctl() disabled auto-RTS and + * drives RTS manually, and restores it from the current + * configuration when the break ends. Touching MODE1 here would + * release the transceiver mid-break. + */ + if (one->tx_break) + return; + + max310x_port_update(port, MAX310X_MODE1_REG, + MAX310X_MODE1_TRNSCVCTRL_BIT, mode1); +} + static void max310x_break_ctl(struct uart_port *port, int break_state) { struct max310x_one *one = to_max310x_port(port); @@ -954,10 +986,7 @@ static void max310x_break_ctl(struct uart_port *port, int break_state) max310x_port_update(port, MAX310X_MODE1_REG, MAX310X_MODE1_TRNSCVCTRL_BIT, 0); } else { - max310x_port_update(port, MAX310X_MODE1_REG, - MAX310X_MODE1_TRNSCVCTRL_BIT, - (port->rs485.flags & SER_RS485_ENABLED) ? - MAX310X_MODE1_TRNSCVCTRL_BIT : 0); + max310x_set_rts_ctl_params(one); } max310x_rts_ctl(port, break_state); } @@ -1081,36 +1110,23 @@ static void max310x_set_termios(struct uart_port *port, static void max310x_rs_proc(struct work_struct *ws) { struct max310x_one *one = container_of(ws, struct max310x_one, rs_work); - unsigned int delay, mode1 = 0, mode2 = 0; + unsigned int mode2 = 0; /* * Serialize against break_ctl() and set_termios(), which run under - * port->mutex: the tx_break test below and the MODE1 write must not + * port->mutex: the tx_break-guarded register writes must not * straddle a break starting or ending. */ guard(mutex)(&one->port.state->port.mutex); - delay = (one->port.rs485.delay_rts_before_send << 4) | - one->port.rs485.delay_rts_after_send; - max310x_port_write(&one->port, MAX310X_HDPIXDELAY_REG, delay); + max310x_set_rts_ctl_params(one); - if (one->port.rs485.flags & SER_RS485_ENABLED) { - mode1 = MAX310X_MODE1_TRNSCVCTRL_BIT; + if (one->port.rs485.flags & SER_RS485_ENABLED && + !(one->port.rs485.flags & SER_RS485_RX_DURING_TX)) + mode2 = MAX310X_MODE2_ECHOSUPR_BIT; - if (!(one->port.rs485.flags & SER_RS485_RX_DURING_TX)) - mode2 = MAX310X_MODE2_ECHOSUPR_BIT; - } - - /* - * A break owns the transceiver: break_ctl() disabled auto-RTS and - * drives RTS manually, and restores it when the break ends. Leave - * MODE1 alone meanwhile or the break goes undriven mid-way. - */ - if (!one->tx_break) - max310x_port_update(&one->port, MAX310X_MODE1_REG, - MAX310X_MODE1_TRNSCVCTRL_BIT, mode1); max310x_port_update(&one->port, MAX310X_MODE2_REG, - MAX310X_MODE2_ECHOSUPR_BIT, mode2); + MAX310X_MODE2_ECHOSUPR_BIT, mode2); } static int max310x_rs485_config(struct uart_port *port, struct ktermios *termios, @@ -1154,21 +1170,14 @@ static int max310x_startup(struct uart_port *port) max310x_port_update(port, MAX310X_MODE2_REG, MAX310X_MODE2_FIFORST_BIT, 0); - /* Configure mode1/mode2 to have rs485/rs232 enabled at startup */ - val = (clamp(port->rs485.delay_rts_before_send, 0U, 15U) << 4) | - clamp(port->rs485.delay_rts_after_send, 0U, 15U); - max310x_port_write(port, MAX310X_HDPIXDELAY_REG, val); + /* Configure the RS485 RTS timing and the RS485/RS232 mode bits. */ + max310x_set_rts_ctl_params(one); - if (port->rs485.flags & SER_RS485_ENABLED) { - max310x_port_update(port, MAX310X_MODE1_REG, - MAX310X_MODE1_TRNSCVCTRL_BIT, - MAX310X_MODE1_TRNSCVCTRL_BIT); - - if (!(port->rs485.flags & SER_RS485_RX_DURING_TX)) - max310x_port_update(port, MAX310X_MODE2_REG, - MAX310X_MODE2_ECHOSUPR_BIT, - MAX310X_MODE2_ECHOSUPR_BIT); - } + if (port->rs485.flags & SER_RS485_ENABLED && + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) + max310x_port_update(port, MAX310X_MODE2_REG, + MAX310X_MODE2_ECHOSUPR_BIT, + MAX310X_MODE2_ECHOSUPR_BIT); /* * Configure flow control levels: