mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v6 0/9] serial: max310x: RS485 delay and RTS fixes, software-timed delays
@ 2026-10-04 12:16 Tapio Reijonen
  2026-10-04 12:16 ` [PATCH v6 1/9] serial: max310x: don't clobber the TX break bit in set_termios Tapio Reijonen
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Tapio Reijonen @ 2026-10-04 12:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, linux-serial, Hugo Villeneuve, Tapio Reijonen,
	Tapio Reijonen

The MAX310X hardware can express at most 15 bit-times of RS485 RTS
setup/hold delay, while struct serial_rs485 expresses the delays in
milliseconds. The driver rejected anything above 0x0f with -ERANGE,
upon which uart_rs485_config() wipes port->rs485 and silently disables
RS485 - a device tree asking for a 20 ms setup delay boots with RS485
off and an unusable bus. The values that were accepted got written
into HDPIXDELAY unconverted, milliseconds as bit-times.

Patches 1-5 fix pre-existing bugs found on the way: a termios write
clobbering an active break; breaks never reaching the wire on RS485
ports because auto-RTS only drives the transceiver for FIFO data; the
milliseconds-as-bit-times unit bug (patch 3 first centralizes the
transceiver register programming with no functional change, patch 4
then converts the units); and close() truncating the final character
because tx_empty() does not cover the transmit shift register. Patch 6
adds active-low RTS on the hardware path via IRDA.RTSINVERT. Patch 7
is preparation, and patch 8 adds the software-timed RTS path that
takes over whenever the hardware cannot represent the requested
timing, clamping the delays to the UART core's maximum instead of
rejecting them. Patch 9 fixes a reconfigure-versus-write race the
asynchronous rs485 config application has had since 2016, which the
software path would have made worse.

v4 was all of this in a single patch; Greg asked for it to be broken
up into one change at a time [1]. Splitting it meant re-verifying each
patch in isolation on hardware, and that re-verification found two
bugs v4 contained: a set_termios() or TIOCSRS485 during an active
break released the transceiver mid-break while the break bookkeeping
still looked correct (prevented by the tx_break ownership guard in
patches 2 and 3), and the patch-9 race, where a TIOCSRS485 followed
immediately by a write could put an entire transfer on the wire with
the transceiver released.

Tested on a MAX14830 (SPI, i.MX6SX) driving RS485 transceivers: for
each patch the bug it fixes was first reproduced on the wire with a
logic analyzer against the kernel one patch earlier, then shown fixed.
The complete series additionally passed an automated regression
matrix, grown during the v5/v6 review rounds to more than 40
scenarios: both RTS paths, both polarities, RS485/RS232 mode
round-trips, close/hangup/SIGKILL landing in every envelope phase,
output stop and flush mid-burst, mid-transfer path switches in both
directions, XON/XOFF injection at idle and mid-burst, and termios/
TIOCSRS485 disturbances landing in every envelope phase (setup, data,
hold, break) - each scenario checked both on the wire and against the
driver's reported state.

Changes in v6:
- every patch now carries the Assisted-by tag, which v5 omitted [2]
- patch 1: the LCR clobber is now described as "overwrites the whole
  LCR register" in the message and as a whole-register write in the
  comments (Hugo)
- the character-time caching and the shutdown drain-wait comments now
  state their reasons in place: both values must be current before
  the RS485 helper runs, and the tty layer's wait-until-sent is
  bounded by closing_wait and absent on hangup (Hugo)
- the old patch 3 is split in two: patch 3 only introduces
  max310x_set_rts_ctl_params() and folds the three copies of the
  HDPIXDELAY/MODE1 programming into it, patch 4 then only adds the
  millisecond-to-bit-time conversion (Hugo); break_ctl() keeps a
  single max310x_rts_ctl() call at the end instead of duplicating it
  in both branches (Hugo)
- one_char_duration_us is renamed to char_time_us and the shutdown
  drain bounds to tries (Hugo)
- fix: disabling RS485 during an active break left the transceiver
  driving the bus indefinitely - break-off only ran its restore while
  RS485 was still enabled. Only the break assertion is now gated on
  RS485 being enabled; the break-off restore always runs and derives
  the register state from the current configuration (patch 2)
- fix: a reconfigure that moves the port off the hardware RTS path
  while a transmission is still in flight - a termios change or
  TIOCSRS485 pushing a delay above what the hardware can time at the
  new rate - released the transceiver mid-transfer, and the rest of
  the data was shifted out with the bus undriven, with no error
  reported. The helper now takes such a transfer over: it enters the
  software envelope and keeps RTS driven across the handover, and the
  normal drain path arms the after-send hold (patch 8)
- fix: a break that begins while the previous envelope's after-send
  hold is still armed - TIOCSBRK waits for the output to drain, which
  lands it exactly there - was cut short when the hold expired and
  released the transceiver mid-break. The RTS worker now derives the
  line state from break ownership as well, so the expiry re-asserts
  instead of releasing (patch 8)
- fix: an XON/XOFF character deferred by the reconfigure-pending gate
  was never transmitted when nothing else was queued, and the re-check
  after start_tx()'s dropped lock did not cover a freshly posted
  reconfigure (patch 9)
- fix: a delay of exactly 15 bit-times - the largest the chip can
  time - was routed to the software path, because the hardware
  ceiling was computed in nanoseconds from the truncated per-bit
  time (49999995 ns where 50 ms at 300 baud needs 50000000). The
  selection now converts to bit-times first and compares against the
  field maximum, which is exact at every baud rate (patch 8)
- fix: a close() racing the after-send hold expiry could leave the
  transceiver driving the bus: shutdown's manual RTS release reaches
  the register, but the RTS output stage is clocked by the UART
  channel clock that the power-off stops - stopped too soon, the pin
  stays asserted until the next open (register confirmed released,
  pin confirmed high, in every observed case). shutdown() now gives
  the release one character time, at least 100 us, before powering
  down; measured propagation is below 50 us at 1200 baud, and the
  delay-free alternative (handing the pin to the auto-RTS engine) was
  tried and rejected on the wire - for an active-low RTS every
  register ordering of that handover drives the pin to the asserted
  level mid-sequence (patch 8)
- fix: a reconfigure moving the port onto the hardware path
  mid-envelope stranded the software envelope state: nothing reset it
  in that direction, and the TX-empty hold arming ran only on the
  software path. A later reconfigure off the hardware path trusted
  the stale state, skipped the in-flight-transfer adoption, and
  released the transceiver under the running transfer. The TX-empty
  unwind now runs on both paths, the reconfigure asserts RTS for any
  transfer it believes exists before auto-RTS is disabled, and the
  hold arms only once no transmittable data remains (patch 8)
- the seven fixes above came out of further review and hardware
  testing of v5: each was first demonstrated on the wire, then shown
  fixed, and the full regression matrix was re-run on the result
- Link to v5: https://lore.kernel.org/r/20260929-max310x-rs485-sw-delay-v5-0-ae46afa583f2@vaisala.com

Changes in v5, beyond the split:
- teardown interlock (tx_teardown): shutdown() and the rs485-disable
  path set it under port->lock, and start_tx() checks it on entry and
  again after retaking the dropped lock, so a racing write can no
  longer re-arm the delay timer or queue RTS work against a port being
  torn down (addresses the remaining review-bot findings on v4)
- shutdown() also cancels tx_work, previously only cancelled in
  remove()
- the per-character duration is stored as unsigned int microseconds
  instead of ktime_t: single-copy atomic on 32-bit, so a torn read of
  the 64-bit value is gone by construction
- the TXEMPTY handling documents that the interrupt latches on the
  FIFO becoming empty, so a stale interrupt cannot pump data during an
  RTS setup delay
- new in v5: the tx_break ownership guard (patches 2/3) and the
  reconfigure-pending gate (patch 9), both found during the per-patch
  hardware re-testing described above
- also new in v5, from a review pass over the split series: startup()
  clears a latched break (nothing clears TXBREAK when a port is closed
  with a break still asserted - 8250 does the same); a reconfigure
  arriving during a break is now deferred and applied at break-end
  instead of partially dropped; the rs485-config worker runs under
  port->mutex so its break-guarded register writes cannot straddle a
  break edge; the termios-path idle settle re-checks tx_state after
  writing and requeues rts_work if an envelope started meanwhile; and
  the hardware-delay ceiling is computed in u64

[1] https://lore.kernel.org/all/2026092326-truth-unweave-c773@gregkh/
[2] https://lore.kernel.org/all/2026100116-saint-idealize-32cf@gregkh/

---
Tapio Reijonen (9):
      serial: max310x: don't clobber the TX break bit in set_termios
      serial: max310x: assert the transceiver during a break
      serial: max310x: centralize the RS485 transceiver programming
      serial: max310x: convert RS485 delays from milliseconds to bit-times
      serial: max310x: wait for TX to drain before powering down in shutdown
      serial: max310x: support active-low RTS on the hardware path
      serial: max310x: schedule tx_work directly from the IRQ handler
      serial: max310x: drive RTS in software when hardware delays are too short
      serial: max310x: don't transmit while an RS485 reconfigure is pending

 drivers/tty/serial/max310x.c | 583 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 549 insertions(+), 34 deletions(-)
---
base-commit: 9505146e885b1a842118aa6410f737290c4a5a32
change-id: 20260513-max310x-rs485-sw-delay-a306d783d529

Best regards,
-- 
Tapio Reijonen <tapio.reijonen@vaisala.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-10-05 15:45 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-04 12:16 [PATCH v6 0/9] serial: max310x: RS485 delay and RTS fixes, software-timed delays Tapio Reijonen
2026-10-04 12:16 ` [PATCH v6 1/9] serial: max310x: don't clobber the TX break bit in set_termios Tapio Reijonen
2026-10-05 15:45   ` Hugo Villeneuve
2026-10-04 12:16 ` [PATCH v6 2/9] serial: max310x: assert the transceiver during a break Tapio Reijonen
2026-10-04 12:16 ` [PATCH v6 3/9] serial: max310x: centralize the RS485 transceiver programming Tapio Reijonen
2026-10-04 12:16 ` [PATCH v6 4/9] serial: max310x: convert RS485 delays from milliseconds to bit-times Tapio Reijonen
2026-10-04 12:16 ` [PATCH v6 5/9] serial: max310x: wait for TX to drain before powering down in shutdown Tapio Reijonen
     [not found]   ` <20261004122835.92E741F000FF@smtp.kernel.org>
2026-10-05  8:19     ` Tapio Reijonen
2026-10-05 10:12   ` Maarten Brock
2026-10-05 10:51     ` Tapio Reijonen
2026-10-04 12:16 ` [PATCH v6 6/9] serial: max310x: support active-low RTS on the hardware path Tapio Reijonen
2026-10-04 12:16 ` [PATCH v6 7/9] serial: max310x: schedule tx_work directly from the IRQ handler Tapio Reijonen
2026-10-04 12:16 ` [PATCH v6 8/9] serial: max310x: drive RTS in software when hardware delays are too short Tapio Reijonen
2026-10-04 12:16 ` [PATCH v6 9/9] serial: max310x: don't transmit while an RS485 reconfigure is pending Tapio Reijonen

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®