* [PATCH v4] serial: max310x: drive RTS in software when hardware delays are too short
@ 2026-09-16 7:10 Tapio Reijonen
2026-09-23 10:39 ` Greg Kroah-Hartman
0 siblings, 1 reply; 2+ messages in thread
From: Tapio Reijonen @ 2026-09-16 7:10 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, Hugo Villeneuve, Tapio Reijonen,
Tapio Reijonen
max310x_rs485_config() rejected delay_rts_before_send and
delay_rts_after_send values larger than 0x0f with -ERANGE, which made
the UART core wipe port->rs485 in uart_rs485_config() and silently
disable RS485. The HDPIXDELAY register holds the setup and hold
delays in 4-bit-per-direction bit-times, so even values inside that
range only encode a fraction of a millisecond at typical baud rates
and the chip's hardware auto-RTS path cannot cover the millisecond
range the kernel UART layer expresses.
Add a software-driven RTS path that takes over whenever the hardware
cannot represent the requested timing:
* Cache the current baud rate and the per-character on-the-wire
duration in max310x_set_termios() so the decision below can use
them.
* max310x_set_rts_ctl_params() picks software or hardware timing:
- software if delay_rts_before_send or delay_rts_after_send in
milliseconds exceeds what 15 bit-times can encode at the
current baud, or if the requested RTS polarity cannot be
produced by the chip's auto-RTS engine;
- hardware otherwise, converting the millisecond delays to
bit-times (rounded up, capped at 15) and programming
MODE1.TRNSCVCTRL plus IRDA.RTSINVERT to drive RTS with the
requested polarity. RTS is left deasserted at idle; the chip's
auto-RTS engine owns the transceiver during transmission.
* When software timing is selected the RTS envelope is driven by a
single hrtimer, re-used for the before- and after-send phases (the
phase is tracked in tx_state), plus a single rts_work that toggles
RTS. max310x_start_tx() queues rts_work to assert RTS; rts_work
arms the timer for the before-send delay only after the RTS edge is
on the wire, so data is never shifted before RTS is asserted. The
timer expiry kicks tx_work to fill the chip FIFO; once that FIFO is
empty (max310x_handle_tx()) the same timer is re-armed for one
character duration plus the after-send delay, after which rts_work
releases RTS. The hold is armed only while tx_state is
MAX310X_TX_SEND, re-checked under port->lock because
max310x_handle_tx() runs from a worker that does not hold it and the
port can be shut down in the meantime. Using one timer and one
rts_work keeps the before-
and after-send phases mutually exclusive and the RTS toggles
ordered, which matters for back-to-back writes and on SMP.
* The LCR register carries the TX break and RTS bits next to the
termios bits, so max310x_set_termios() updates only the bits it owns
instead of writing the register absolutely, and
max310x_set_rts_ctl_params() settles RTS to the idle level only
while tx_state is MAX310X_TX_OFF. Both can run while a software
timed envelope is in flight - serial_core calls ->set_termios()
without port->lock, and max310x_rs485_config() schedules a
reconfigure on every TIOCSRS485 - and would otherwise release the
transceiver mid-character.
* max310x_shutdown() waits for transmission to finish before powering
the port down, so close() cannot truncate the final byte.
On the software path it waits out any in-flight RTS envelope
(bounded) so the last character and its after-send hold complete.
On the hardware path it lets the chip FIFO drain and the last
character clock out of the shift register: tx_empty only reports
the TX FIFO empty, not the shift register, so without this the
port could be powered down mid-character. The delay timer and
rts_work are then cancelled unconditionally: the software/hardware
decision is recomputed on every reconfigure, so a TIOCSRS485 can
clear sw_rts_during_tx while an envelope is still in flight, and
neither may outlive the port. That flag is published in a single
store for the same reason.
* max310x_rs485_config() now clamps the delays to the UART core's
RS485_MAX_RTS_DELAY (100 ms) instead of rejecting them, and
cancels the pending delay timer when RS485 is disabled.
* max310x_break_ctl() drives RTS manually for the break duration. The
chip's auto-RTS only asserts the transceiver while FIFO data is
shifting out, and a break is not FIFO data, so on the hardware path
it also disables auto-RTS for the break and restores it when the
break ends. The software path drives the configured RS485 RTS
polarity; on the hardware path IRDA.RTSINVERT already inverts the
RTS_ output stage, so break_state is driven as it is.
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
Changes in v4:
- max310x_shutdown() cancels the delay timer and rts_work
unconditionally. The software/hardware decision is recomputed on every
reconfigure, so a TIOCSRS485 can clear sw_rts_during_tx while an
envelope is still in flight; gating the cancellation on that flag left
the timer armed against a port that was about to be powered down.
- max310x_delayed_stop_tx() re-checks under port->lock and arms the
after-send hold only while tx_state is MAX310X_TX_SEND. It runs from a
worker that does not hold the lock, and it cleared cancel_tx_delay_tmr
unconditionally and tested tx_state != MAX310X_TX_WAIT_BEFORE_SEND,
which is also true for MAX310X_TX_OFF, so it could resurrect an
envelope that shutdown() had just cancelled.
- sw_rts_during_tx is computed into a local and published in a single
store, and read with READ_ONCE(). It was assigned false and only then
recomputed, so any concurrent reader could observe a transient false.
- Triage of the review findings that prompted the above:
https://lore.kernel.org/r/c15330e4-25e0-4c50-9f48-dbb8d51f5823@vaisala.com
- Link to v3: https://lore.kernel.org/r/20260915-max310x-rs485-sw-delay-v3-1-7d20a4a4ab52@vaisala.com
Changes in v3:
- max310x_set_termios() updates only the termios-owned LCR bits instead
of writing LCR absolutely. TX break and the software-timed RS485 RTS
live in the same register, and serial_core calls ->set_termios()
without port->lock, so a tcsetattr() concurrent with an in-flight
envelope released the transceiver mid-character.
- max310x_set_rts_ctl_params() only settles RTS to the idle level while
tx_state is MAX310X_TX_OFF. It runs from rs_work, which
max310x_rs485_config() schedules on every TIOCSRS485, and forcing the
idle level there unconditionally had the same mid-character effect.
- max310x_break_ctl() applies the configured RTS polarity on the
software path; a port with rs485-rts-active-low drove the wrong level
during a break. The hardware path keeps driving break_state as it is:
IRDA.RTSINVERT inverts the RTS_ output stage itself (MAX14830
datasheet, IrDA register bit 2), so both polarities already come out
right there.
- Add max310x_rts_level() for the flag-to-level selection, which is now
needed at four sites.
- Use uart_port_unlock()/uart_port_lock() at the two sites that drop
port->lock around hrtimer_cancel().
- Not done, although I said I would: skipping the before-send re-arm
when tx_state is already MAX310X_TX_SEND. Returning early there also
removes the interlock that keeps max310x_delayed_stop_tx() from arming
the after-send hold while a fresh write is still queued, which can
release RTS mid-character. The txlvl == 0 gate in
max310x_delayed_stop_tx() already keeps the timing correct; the cost
of leaving it alone is one spurious before-send delay mid-stream.
- Review that prompted the above, on the v2 resend:
https://lore.kernel.org/r/20260914-max310x-rs485-sw-delay-v2-1-9a7b681e64ce@vaisala.com
- Link to v2: https://lore.kernel.org/r/20260811-max310x-rs485-sw-delay-v2-1-e34283205789@vaisala.com
Changes in v2:
- Use tty_get_frame_size() instead of open-coding the frame size
(Jiri Slaby). The helper was left with a single expression and one
caller, so it is dropped and the calculation now lives in
max310x_set_termios(). tty_get_frame_size() additionally accounts for
ADDRB, which the open-coded version ignored.
- Use guard(spinlock_irqsave) instead of spin_lock_irqsave() and
spin_unlock_irqrestore() (Jiri Slaby), in max310x_rts_work_proc(),
max310x_tmr_tx() and max310x_delayed_stop_tx(). The bare
spin_lock()/spin_unlock() pairs in max310x_start_tx() and
max310x_rs485_config() are left alone: both are called with port->lock
held by the caller and only drop it around hrtimer_cancel(), which
guard() cannot express.
- Link to v1: https://lore.kernel.org/r/20260709-max310x-rs485-sw-delay-v1-1-454ac10b937a@vaisala.com
---
drivers/tty/serial/max310x.c | 408 +++++++++++++++++++++++++++++++++++++++----
1 file changed, 372 insertions(+), 36 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 022502986c5fcf1ff4de9328746ddc71677be730..c217857f5d32a85728f82ad52e02c3a9075eb0da 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -15,6 +15,7 @@
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/gpio/driver.h>
+#include <linux/hrtimer.h>
#include <linux/i2c.h>
#include <linux/kconfig.h>
#include <linux/module.h>
@@ -158,10 +159,17 @@
#define MAX310X_LCR_FORCEPARITY_BIT (1 << 5) /* 9-bit multidrop parity */
#define MAX310X_LCR_TXBREAK_BIT (1 << 6) /* TX break enable */
#define MAX310X_LCR_RTS_BIT (1 << 7) /* RTS pin control */
+/* LCR bits owned by termios; TX break and RTS are driven elsewhere */
+#define MAX310X_LCR_TERMIOS_MASK GENMASK(5, 0)
/* IRDA register bits */
#define MAX310X_IRDA_IRDAEN_BIT (1 << 0) /* IRDA mode enable */
#define MAX310X_IRDA_SIR_BIT (1 << 1) /* SIR mode enable */
+#define MAX310X_IRDA_RTSINVERT_BIT (1 << 2) /* Invert RTS output */
+
+/* HDPIXDELAY accessor macros */
+#define MAX310X_HDPIXDELAY_SETUP(val) (((val) & 0x0f) << 4)
+#define MAX310X_HDPIXDELAY_HOLD(val) ((val) & 0x0f)
/* Flow control trigger level register masks */
#define MAX310X_FLOWLVL_HALT_MASK GENMASK(3, 0) /* Flow control halt level */
@@ -290,12 +298,27 @@ struct max310x_devtype {
u8 power_bit; /* Bit for sleep or power-off mode (active high). */
};
+/* Software-timed RS485 RTS envelope phase */
+enum max310x_tx_state {
+ MAX310X_TX_OFF, /* idle, RTS released */
+ MAX310X_TX_WAIT_BEFORE_SEND, /* RTS asserted, before-send delay */
+ MAX310X_TX_SEND, /* data in flight, awaiting TX-empty */
+ MAX310X_TX_WAIT_AFTER_SEND, /* data drained, after-send hold */
+};
+
struct max310x_one {
struct uart_port port;
struct work_struct tx_work;
struct work_struct md_work;
struct work_struct rs_work;
+ struct work_struct rts_work;
+ struct hrtimer tx_delay_tmr;
struct regmap *regmap;
+ ktime_t one_character_duration;
+ unsigned int baud;
+ bool sw_rts_during_tx;
+ bool cancel_tx_delay_tmr;
+ enum max310x_tx_state tx_state;
u8 rx_buf[MAX310X_FIFO_SIZE];
};
@@ -680,6 +703,42 @@ static void max310x_batch_read(struct uart_port *port, u8 *rxbuf, unsigned int l
regmap_noinc_read(one->regmap, MAX310X_RHR_REG, rxbuf, len);
}
+static void max310x_rts_ctl(struct uart_port *port, bool rts_state)
+{
+ max310x_port_update(port, MAX310X_LCR_REG, MAX310X_LCR_RTS_BIT,
+ rts_state ? MAX310X_LCR_RTS_BIT : 0);
+}
+
+/* RTS level for the transmitting or the idle phase of an RS485 envelope */
+static bool max310x_rts_level(struct uart_port *port, bool active)
+{
+ return active ? (port->rs485.flags & SER_RS485_RTS_ON_SEND) :
+ (port->rs485.flags & SER_RS485_RTS_AFTER_SEND);
+}
+
+/*
+ * Drive the RS485 RTS line to match the current tx_state. This is the only
+ * place that touches RTS, and it reads tx_state rather than a fixed
+ * assert/deassert intent, so a newer assert is never clobbered by a stale
+ * release. It also arms the before-send timer once the RTS edge is on the wire,
+ * so data is never shifted before RTS is asserted.
+ */
+static void max310x_rts_work_proc(struct work_struct *ws)
+{
+ struct max310x_one *one = container_of(ws, struct max310x_one, rts_work);
+ struct uart_port *port = &one->port;
+ bool rts_on = READ_ONCE(one->tx_state) != MAX310X_TX_OFF;
+
+ max310x_rts_ctl(port, max310x_rts_level(port, rts_on));
+
+ guard(spinlock_irqsave)(&port->lock);
+ if (READ_ONCE(one->tx_state) == MAX310X_TX_WAIT_BEFORE_SEND &&
+ !one->cancel_tx_delay_tmr && !hrtimer_active(&one->tx_delay_tmr))
+ hrtimer_start(&one->tx_delay_tmr,
+ ms_to_ktime(port->rs485.delay_rts_before_send),
+ HRTIMER_MODE_REL);
+}
+
static void max310x_handle_rx(struct uart_port *port, unsigned int rxlen)
{
struct max310x_one *one = to_max310x_port(port);
@@ -776,6 +835,67 @@ static void max310x_handle_rx(struct uart_port *port, unsigned int rxlen)
tty_flip_buffer_push(&port->state->port);
}
+static enum hrtimer_restart max310x_tmr_tx(struct hrtimer *timer)
+{
+ struct max310x_one *one = container_of(timer, struct max310x_one,
+ tx_delay_tmr);
+
+ guard(spinlock_irqsave)(&one->port.lock);
+ if (!one->cancel_tx_delay_tmr) {
+ if (READ_ONCE(one->tx_state) == MAX310X_TX_WAIT_AFTER_SEND) {
+ /* After-send hold elapsed: drop RTS via the rts worker. */
+ WRITE_ONCE(one->tx_state, MAX310X_TX_OFF);
+ schedule_work(&one->rts_work);
+ } else {
+ WRITE_ONCE(one->tx_state, MAX310X_TX_SEND);
+ schedule_work(&one->tx_work);
+ }
+ }
+
+ return HRTIMER_NORESTART;
+}
+
+static void max310x_delayed_stop_tx(struct uart_port *port)
+{
+ struct max310x_one *one = to_max310x_port(port);
+ unsigned int txlvl;
+
+ if (READ_ONCE(one->tx_state) == MAX310X_TX_OFF)
+ return;
+
+ /*
+ * The kfifo can be empty while the chip TX FIFO is still draining, so arm
+ * the after-send hold only once the chip FIFO is empty too - the TX-empty
+ * interrupt re-invokes us then. Otherwise the hold starts early and RTS
+ * drops mid-character, clipping the last byte(s).
+ */
+ txlvl = max310x_port_read(port, MAX310X_TXFIFOLVL_REG);
+ if (txlvl)
+ return;
+
+ /*
+ * Runs from tx_work without port->lock, so re-check the state under it:
+ * shutdown() may have cancelled the envelope meanwhile. Only
+ * MAX310X_TX_SEND may arm the hold.
+ */
+ guard(spinlock_irqsave)(&one->port.lock);
+ if (one->cancel_tx_delay_tmr ||
+ READ_ONCE(one->tx_state) != MAX310X_TX_SEND)
+ return;
+
+ if (!hrtimer_active(&one->tx_delay_tmr)) {
+ /*
+ * Add one character for the byte still in the shift register -
+ * TX-empty fires as it enters, not as it leaves.
+ */
+ ktime_t delay = ktime_add_ms(one->one_character_duration,
+ port->rs485.delay_rts_after_send);
+
+ WRITE_ONCE(one->tx_state, MAX310X_TX_WAIT_AFTER_SEND);
+ hrtimer_start(&one->tx_delay_tmr, delay, HRTIMER_MODE_REL);
+ }
+}
+
static void max310x_handle_tx(struct uart_port *port)
{
struct tty_port *tport = &port->state->port;
@@ -787,8 +907,13 @@ static void max310x_handle_tx(struct uart_port *port)
return;
}
- if (kfifo_is_empty(&tport->xmit_fifo) || uart_tx_stopped(port))
+ if (kfifo_is_empty(&tport->xmit_fifo) || uart_tx_stopped(port)) {
+ struct max310x_one *one = to_max310x_port(port);
+
+ if (READ_ONCE(one->sw_rts_during_tx))
+ max310x_delayed_stop_tx(port);
return;
+ }
/*
* It's a circ buffer -- wrap around.
@@ -813,11 +938,48 @@ static void max310x_handle_tx(struct uart_port *port)
uart_write_wakeup(port);
}
+/*
+ * Begin a software-timed RTS envelope: set the before-send phase and queue the
+ * rts worker to assert RTS. tx_state is set synchronously here (start_tx() holds
+ * port.lock) so close()/shutdown can see an envelope is in flight; rts_work then
+ * asserts RTS and arms the before-send timer (see there).
+ */
+static void max310x_delayed_start_tx(struct uart_port *port)
+{
+ struct max310x_one *one = to_max310x_port(port);
+
+ WRITE_ONCE(one->tx_state, MAX310X_TX_WAIT_BEFORE_SEND);
+ one->cancel_tx_delay_tmr = false;
+ schedule_work(&one->rts_work);
+}
+
+/* called with port.lock taken and irqs off */
static void max310x_start_tx(struct uart_port *port)
{
struct max310x_one *one = to_max310x_port(port);
- schedule_work(&one->tx_work);
+ if (READ_ONCE(one->sw_rts_during_tx)) {
+ /*
+ * The before- and after-send phases share one delay timer. If an
+ * after-send release is pending, cancel it before starting a new
+ * TX so the just-asserted RTS is not yanked; re-arming the timer
+ * for the before-send phase then supersedes the release.
+ */
+ int res = 0;
+
+ if (READ_ONCE(one->tx_state) == MAX310X_TX_WAIT_AFTER_SEND)
+ res = hrtimer_try_to_cancel(&one->tx_delay_tmr);
+ if (unlikely(res == -1)) {
+ one->cancel_tx_delay_tmr = true;
+ uart_port_unlock(port);
+ hrtimer_cancel(&one->tx_delay_tmr);
+ uart_port_lock(port);
+ }
+
+ max310x_delayed_start_tx(port);
+ } else {
+ schedule_work(&one->tx_work);
+ }
}
static irqreturn_t max310x_port_irq(struct max310x_port *s, int portno)
@@ -843,7 +1005,7 @@ static irqreturn_t max310x_port_irq(struct max310x_port *s, int portno)
if (rxlen)
max310x_handle_rx(port, rxlen);
if (ists & MAX310X_IRQ_TXEMPTY_BIT)
- max310x_start_tx(port);
+ schedule_work(&s->p[portno].tx_work);
} while (1);
return res;
@@ -927,15 +1089,113 @@ static void max310x_set_mctrl(struct uart_port *port, unsigned int mctrl)
static void max310x_break_ctl(struct uart_port *port, int break_state)
{
+ struct max310x_one *one = to_max310x_port(port);
+
max310x_port_update(port, MAX310X_LCR_REG,
MAX310X_LCR_TXBREAK_BIT,
break_state ? MAX310X_LCR_TXBREAK_BIT : 0);
+
+ if (!(port->rs485.flags & SER_RS485_ENABLED))
+ return;
+
+ /*
+ * Drive RTS manually for the break duration. HW auto-RTS only asserts
+ * the transceiver while FIFO data is shifting out, and a break is not
+ * FIFO data, so on the HW path also disable auto-RTS for the break and
+ * restore it when the break ends. There IRDA.RTSINVERT already inverts
+ * the pin for an active-low RTS, so break_state is driven as it is;
+ * the software path applies the polarity itself.
+ */
+ if (READ_ONCE(one->sw_rts_during_tx)) {
+ max310x_rts_ctl(port, max310x_rts_level(port, break_state));
+ } else {
+ max310x_port_update(port, MAX310X_MODE1_REG,
+ MAX310X_MODE1_TRNSCVCTRL_BIT,
+ break_state ? 0 : MAX310X_MODE1_TRNSCVCTRL_BIT);
+ max310x_rts_ctl(port, break_state);
+ }
+}
+
+/*
+ * Pick hardware or software RTS timing for the current port. The chip can
+ * deliver up to 15 bit-times of setup/hold delay via HDPIXDELAY; anything
+ * longer (or any RTS polarity the chip cannot produce automatically) must
+ * be driven by software via tx_delay_tmr and rts_work.
+ */
+static void max310x_set_rts_ctl_params(struct max310x_one *one)
+{
+ const unsigned int max_bit_dly = 15;
+ struct uart_port *port = &one->port;
+ unsigned long max_hw_delay_ns = 0;
+ unsigned int setup = 0, hold = 0;
+ u8 mode1 = 0, irda = 0;
+ bool sw_rts = false;
+
+ if (!(port->rs485.flags & SER_RS485_ENABLED))
+ goto out;
+
+ if (one->baud)
+ max_hw_delay_ns = NSEC_PER_SEC / one->baud * max_bit_dly;
+
+ if ((u64)port->rs485.delay_rts_before_send * NSEC_PER_MSEC > max_hw_delay_ns ||
+ (u64)port->rs485.delay_rts_after_send * NSEC_PER_MSEC > max_hw_delay_ns ||
+ !!(port->rs485.flags & SER_RS485_RTS_ON_SEND) ==
+ !!(port->rs485.flags & SER_RS485_RTS_AFTER_SEND))
+ sw_rts = true;
+
+ if (sw_rts) {
+ setup = 0;
+ hold = 0;
+ goto out;
+ }
+
+ /* Convert milliseconds to bit-times, rounding up. */
+ setup = DIV_ROUND_UP(one->baud * port->rs485.delay_rts_before_send,
+ MSEC_PER_SEC);
+ hold = DIV_ROUND_UP(one->baud * port->rs485.delay_rts_after_send,
+ MSEC_PER_SEC);
+ setup = min(setup, max_bit_dly);
+ hold = min(hold, max_bit_dly);
+
+out:
+ /* Assign once; a transient false would be seen by other readers. */
+ WRITE_ONCE(one->sw_rts_during_tx, sw_rts);
+
+ max310x_port_write(port, MAX310X_HDPIXDELAY_REG,
+ MAX310X_HDPIXDELAY_SETUP(setup) |
+ MAX310X_HDPIXDELAY_HOLD(hold));
+
+ if (port->rs485.flags & SER_RS485_ENABLED) {
+ if (sw_rts) {
+ /*
+ * Only settle RTS at idle when no transmission owns it.
+ * A reconfigure while one is in flight - rs_work runs
+ * on every TIOCSRS485 - would otherwise release the
+ * transceiver mid-character.
+ */
+ if (READ_ONCE(one->tx_state) == MAX310X_TX_OFF)
+ max310x_rts_ctl(port,
+ max310x_rts_level(port, false));
+ } else {
+ mode1 = MAX310X_MODE1_TRNSCVCTRL_BIT;
+ if (!(port->rs485.flags & SER_RS485_RTS_ON_SEND))
+ irda = MAX310X_IRDA_RTSINVERT_BIT;
+ }
+ } else {
+ max310x_rts_ctl(port, 0);
+ }
+
+ max310x_port_update(port, MAX310X_MODE1_REG,
+ MAX310X_MODE1_TRNSCVCTRL_BIT, mode1);
+ max310x_port_update(port, MAX310X_IRDA_REG,
+ MAX310X_IRDA_RTSINVERT_BIT, irda);
}
static void max310x_set_termios(struct uart_port *port,
struct ktermios *termios,
const struct ktermios *old)
{
+ unsigned int frame_bits = tty_get_frame_size(termios->c_cflag);
unsigned int lcr = 0, flow = 0;
int baud;
@@ -969,8 +1229,12 @@ static void max310x_set_termios(struct uart_port *port,
if (termios->c_cflag & CSTOPB)
lcr |= MAX310X_LCR_STOPLEN_BIT; /* 2 stops */
- /* Update LCR register */
- max310x_port_write(port, MAX310X_LCR_REG, lcr);
+ /*
+ * Update LCR register. Leave the TX break and RTS bits alone: they are
+ * driven by break_ctl() and by the software-timed RS485 RTS, and an
+ * absolute write here would release the transceiver mid-character.
+ */
+ max310x_port_update(port, MAX310X_LCR_REG, MAX310X_LCR_TERMIOS_MASK, lcr);
/* Set read status mask */
port->read_status_mask = MAX310X_LSR_RXOVR_BIT;
@@ -1042,38 +1306,55 @@ static void max310x_set_termios(struct uart_port *port,
/* Update timeout according to new baud rate */
uart_update_timeout(port, termios->c_cflag, baud);
+
+ /*
+ * Cache the new baud rate and the time it takes to clock out one
+ * character so the RTS-timing decision in max310x_set_rts_ctl_params()
+ * and the post-TX delay in max310x_delayed_stop_tx() can use them.
+ */
+ to_max310x_port(port)->baud = baud;
+ to_max310x_port(port)->one_character_duration =
+ us_to_ktime(DIV_ROUND_UP(USEC_PER_SEC * frame_bits, baud));
+ max310x_set_rts_ctl_params(to_max310x_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;
- 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;
- }
-
- 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);
}
+/* called with port.lock taken and irqs off */
static int max310x_rs485_config(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
struct max310x_one *one = to_max310x_port(port);
- if ((rs485->delay_rts_before_send > 0x0f) ||
- (rs485->delay_rts_after_send > 0x0f))
- return -ERANGE;
+ rs485->delay_rts_before_send = min(rs485->delay_rts_before_send, 100U);
+ rs485->delay_rts_after_send = min(rs485->delay_rts_after_send, 100U);
+
+ /*
+ * Make sure no SW-timed RTS toggle survives an RS485 disable, even
+ * if the delay timer happens to be running right now.
+ */
+ if (!(rs485->flags & SER_RS485_ENABLED)) {
+ one->cancel_tx_delay_tmr = true;
+ if (hrtimer_try_to_cancel(&one->tx_delay_tmr) == -1) {
+ uart_port_unlock(port);
+ hrtimer_cancel(&one->tx_delay_tmr);
+ uart_port_lock(port);
+ }
+ WRITE_ONCE(one->tx_state, MAX310X_TX_OFF);
+ }
port->rs485 = *rs485;
@@ -1084,6 +1365,7 @@ static int max310x_rs485_config(struct uart_port *port, struct ktermios *termios
static int max310x_startup(struct uart_port *port)
{
+ struct max310x_one *one = to_max310x_port(port);
unsigned int val;
max310x_power(port, 1);
@@ -1098,21 +1380,20 @@ 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);
-
- if (port->rs485.flags & SER_RS485_ENABLED) {
- max310x_port_update(port, MAX310X_MODE1_REG,
- MAX310X_MODE1_TRNSCVCTRL_BIT,
- MAX310X_MODE1_TRNSCVCTRL_BIT);
+ /*
+ * Configure RTS timing (HW auto-RTS vs software-driven) and the
+ * RS485/RS232 mode bits. Don't hardcode HW auto-RTS here - let
+ * max310x_set_rts_ctl_params() pick HW or SW per the configured
+ * delays, otherwise the chip's auto-RTS would override the
+ * software RTS hold and the after-send delay is lost.
+ */
+ max310x_set_rts_ctl_params(one);
- 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:
@@ -1134,9 +1415,56 @@ static int max310x_startup(struct uart_port *port)
static void max310x_shutdown(struct uart_port *port)
{
+ struct max310x_one *one = to_max310x_port(port);
+
+ /*
+ * Drain any in-flight software-timed RTS envelope before the port is
+ * powered down, so the last character and its after-send hold complete
+ * - close() can reach shutdown with data still queued and a before-send
+ * delay pending. The loop ends when the envelope does (tx_state == OFF);
+ * the bound is just a worst-case safety cap.
+ */
+ if (READ_ONCE(one->sw_rts_during_tx)) {
+ unsigned int loops = port->rs485.delay_rts_before_send +
+ port->rs485.delay_rts_after_send +
+ DIV_ROUND_UP_ULL((kfifo_len(&port->state->port.xmit_fifo) +
+ port->fifosize) *
+ ktime_to_us(one->one_character_duration),
+ USEC_PER_MSEC);
+
+ while (READ_ONCE(one->tx_state) != MAX310X_TX_OFF && loops-- > 0)
+ fsleep(USEC_PER_MSEC);
+ } else {
+ /*
+ * HW auto-RTS path: the tty layer waits for tx_empty before
+ * close(), but tx_empty only reflects the chip TX FIFO - the
+ * last character may still be in the transmit shift register.
+ * Let the FIFO drain and the final character clock out before
+ * the port is powered down, otherwise close() truncates the last
+ * byte on the wire as the chip auto-RTS turnaround clips it.
+ */
+ unsigned int loops = port->fifosize + 1;
+
+ while (!max310x_tx_empty(port) && loops-- > 0)
+ fsleep(ktime_to_us(one->one_character_duration));
+ fsleep(ktime_to_us(one->one_character_duration));
+ }
+
+ /*
+ * Cancel unconditionally: sw_rts_during_tx can have turned false while
+ * an envelope was in flight, and neither may outlive the port.
+ */
+ one->cancel_tx_delay_tmr = true;
+ hrtimer_cancel(&one->tx_delay_tmr);
+ cancel_work_sync(&one->rts_work);
+ WRITE_ONCE(one->tx_state, MAX310X_TX_OFF);
+
/* Disable all interrupts */
max310x_port_write(port, MAX310X_IRQEN_REG, 0);
+ if (READ_ONCE(one->sw_rts_during_tx))
+ max310x_rts_ctl(port, max310x_rts_level(port, false));
+
max310x_power(port, 0);
}
@@ -1291,7 +1619,8 @@ static int max310x_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
#endif
static const struct serial_rs485 max310x_rs485_supported = {
- .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RX_DURING_TX,
+ .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND |
+ SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX,
.delay_rts_before_send = 1,
.delay_rts_after_send = 1,
};
@@ -1425,6 +1754,11 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
INIT_WORK(&s->p[i].md_work, max310x_md_proc);
/* Initialize queue for changing RS485 mode */
INIT_WORK(&s->p[i].rs_work, max310x_rs_proc);
+ /* Initialize queue for software-driven RTS toggling */
+ INIT_WORK(&s->p[i].rts_work, max310x_rts_work_proc);
+ hrtimer_setup(&s->p[i].tx_delay_tmr, max310x_tmr_tx,
+ CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ s->p[i].tx_state = MAX310X_TX_OFF;
}
#ifdef CONFIG_GPIOLIB
@@ -1535,6 +1869,8 @@ static void max310x_remove(struct device *dev)
int i;
for (i = 0; i < s->devtype->nr; i++) {
+ hrtimer_cancel(&s->p[i].tx_delay_tmr);
+ cancel_work_sync(&s->p[i].rts_work);
cancel_work_sync(&s->p[i].tx_work);
cancel_work_sync(&s->p[i].md_work);
cancel_work_sync(&s->p[i].rs_work);
---
base-commit: 9505146e885b1a842118aa6410f737290c4a5a32
change-id: 20260513-max310x-rs485-sw-delay-a306d783d529
Best regards,
--
Tapio Reijonen <tapio.reijonen@vaisala.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v4] serial: max310x: drive RTS in software when hardware delays are too short
2026-09-16 7:10 [PATCH v4] serial: max310x: drive RTS in software when hardware delays are too short Tapio Reijonen
@ 2026-09-23 10:39 ` Greg Kroah-Hartman
0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2026-09-23 10:39 UTC (permalink / raw)
To: Tapio Reijonen
Cc: Jiri Slaby, linux-kernel, linux-serial, Hugo Villeneuve, Tapio Reijonen
On Wed, Sep 16, 2026 at 07:10:54AM +0000, Tapio Reijonen wrote:
> max310x_rs485_config() rejected delay_rts_before_send and
> delay_rts_after_send values larger than 0x0f with -ERANGE, which made
> the UART core wipe port->rs485 in uart_rs485_config() and silently
> disable RS485. The HDPIXDELAY register holds the setup and hold
> delays in 4-bit-per-direction bit-times, so even values inside that
> range only encode a fraction of a millisecond at typical baud rates
> and the chip's hardware auto-RTS path cannot cover the millisecond
> range the kernel UART layer expresses.
>
> Add a software-driven RTS path that takes over whenever the hardware
> cannot represent the requested timing:
>
> * Cache the current baud rate and the per-character on-the-wire
> duration in max310x_set_termios() so the decision below can use
> them.
> * max310x_set_rts_ctl_params() picks software or hardware timing:
> - software if delay_rts_before_send or delay_rts_after_send in
> milliseconds exceeds what 15 bit-times can encode at the
> current baud, or if the requested RTS polarity cannot be
> produced by the chip's auto-RTS engine;
> - hardware otherwise, converting the millisecond delays to
> bit-times (rounded up, capped at 15) and programming
> MODE1.TRNSCVCTRL plus IRDA.RTSINVERT to drive RTS with the
> requested polarity. RTS is left deasserted at idle; the chip's
> auto-RTS engine owns the transceiver during transmission.
> * When software timing is selected the RTS envelope is driven by a
> single hrtimer, re-used for the before- and after-send phases (the
> phase is tracked in tx_state), plus a single rts_work that toggles
> RTS. max310x_start_tx() queues rts_work to assert RTS; rts_work
> arms the timer for the before-send delay only after the RTS edge is
> on the wire, so data is never shifted before RTS is asserted. The
> timer expiry kicks tx_work to fill the chip FIFO; once that FIFO is
> empty (max310x_handle_tx()) the same timer is re-armed for one
> character duration plus the after-send delay, after which rts_work
> releases RTS. The hold is armed only while tx_state is
> MAX310X_TX_SEND, re-checked under port->lock because
> max310x_handle_tx() runs from a worker that does not hold it and the
> port can be shut down in the meantime. Using one timer and one
> rts_work keeps the before-
> and after-send phases mutually exclusive and the RTS toggles
> ordered, which matters for back-to-back writes and on SMP.
> * The LCR register carries the TX break and RTS bits next to the
> termios bits, so max310x_set_termios() updates only the bits it owns
> instead of writing the register absolutely, and
> max310x_set_rts_ctl_params() settles RTS to the idle level only
> while tx_state is MAX310X_TX_OFF. Both can run while a software
> timed envelope is in flight - serial_core calls ->set_termios()
> without port->lock, and max310x_rs485_config() schedules a
> reconfigure on every TIOCSRS485 - and would otherwise release the
> transceiver mid-character.
> * max310x_shutdown() waits for transmission to finish before powering
> the port down, so close() cannot truncate the final byte.
> On the software path it waits out any in-flight RTS envelope
> (bounded) so the last character and its after-send hold complete.
> On the hardware path it lets the chip FIFO drain and the last
> character clock out of the shift register: tx_empty only reports
> the TX FIFO empty, not the shift register, so without this the
> port could be powered down mid-character. The delay timer and
> rts_work are then cancelled unconditionally: the software/hardware
> decision is recomputed on every reconfigure, so a TIOCSRS485 can
> clear sw_rts_during_tx while an envelope is still in flight, and
> neither may outlive the port. That flag is published in a single
> store for the same reason.
> * max310x_rs485_config() now clamps the delays to the UART core's
> RS485_MAX_RTS_DELAY (100 ms) instead of rejecting them, and
> cancels the pending delay timer when RS485 is disabled.
> * max310x_break_ctl() drives RTS manually for the break duration. The
> chip's auto-RTS only asserts the transceiver while FIFO data is
> shifting out, and a break is not FIFO data, so on the hardware path
> it also disables auto-RTS for the break and restores it when the
> break ends. The software path drives the configured RS485 RTS
> polarity; on the hardware path IRDA.RTSINVERT already inverts the
> RTS_ output stage, so break_state is driven as it is.
That's a lot to do in one commit. Why can't this be broken up into "do
only one thing at a time" patch series?
That might make all of this much more sane and managable and reviewable,
as it is, it is none of that.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-23 10:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16 7:10 [PATCH v4] serial: max310x: drive RTS in software when hardware delays are too short Tapio Reijonen
2026-09-23 10:39 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®