mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Tapio Reijonen <tapio.reijonen@vaisala.com>
Cc: Jiri Slaby <jirislaby@kernel.org>,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
	Hugo Villeneuve <hvilleneuve@dimonoff.com>,
	Tapio Reijonen <tapio.reijonen@kolumbus.fi>
Subject: Re: [PATCH v5 0/8] serial: max310x: RS485 delay and RTS fixes, software-timed delays
Date: Thu, 1 Oct 2026 10:35:45 +0200	[thread overview]
Message-ID: <2026100116-saint-idealize-32cf@gregkh> (raw)
In-Reply-To: <20260929-max310x-rs485-sw-delay-v5-0-ae46afa583f2@vaisala.com>

On Tue, Sep 29, 2026 at 09:37:54AM +0000, Tapio Reijonen wrote:
> 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-4 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; and close() truncating the final
> character because tx_empty() does not cover the transmit shift
> register. Patch 5 adds active-low RTS on the hardware path via
> IRDA.RTSINVERT. Patch 6 is preparation, and patch 7 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 8 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-8 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 25-scenario
> regression matrix covering both RTS paths, both polarities,
> RS485/RS232 mode round-trips, close-during-transmission, 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 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 8), 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/
> 
> ---
> Tapio Reijonen (8):
>       serial: max310x: don't clobber the TX break bit in set_termios
>       serial: max310x: assert the transceiver during a break
>       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 | 511 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 477 insertions(+), 34 deletions(-)
> ---
> base-commit: 9505146e885b1a842118aa6410f737290c4a5a32
> change-id: 20260513-max310x-rs485-sw-delay-a306d783d529
> 
> Best regards,
> -- 
> Tapio Reijonen <tapio.reijonen@vaisala.com>
> 

Did you forget the Assisted-by: tag for this series?

thanks,

greg k-h

  parent reply	other threads:[~2026-10-01  8:35 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-29  9:37 Tapio Reijonen
2026-09-29  9:37 ` [PATCH v5 1/8] serial: max310x: don't clobber the TX break bit in set_termios Tapio Reijonen
2026-09-29 13:40   ` Hugo Villeneuve
2026-09-29  9:37 ` [PATCH v5 2/8] serial: max310x: assert the transceiver during a break Tapio Reijonen
2026-09-29  9:37 ` [PATCH v5 3/8] serial: max310x: convert RS485 delays from milliseconds to bit-times Tapio Reijonen
2026-09-29 13:54   ` Hugo Villeneuve
2026-09-29  9:37 ` [PATCH v5 4/8] serial: max310x: wait for TX to drain before powering down in shutdown Tapio Reijonen
2026-09-29  9:37 ` [PATCH v5 5/8] serial: max310x: support active-low RTS on the hardware path Tapio Reijonen
2026-09-29  9:38 ` [PATCH v5 6/8] serial: max310x: schedule tx_work directly from the IRQ handler Tapio Reijonen
2026-09-29  9:38 ` [PATCH v5 7/8] serial: max310x: drive RTS in software when hardware delays are too short Tapio Reijonen
2026-09-29  9:38 ` [PATCH v5 8/8] serial: max310x: don't transmit while an RS485 reconfigure is pending Tapio Reijonen
2026-10-01  8:35 ` Greg Kroah-Hartman [this message]
2026-10-01  9:10   ` [PATCH v5 0/8] serial: max310x: RS485 delay and RTS fixes, software-timed delays Tapio Reijonen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026100116-saint-idealize-32cf@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=hvilleneuve@dimonoff.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=tapio.reijonen@kolumbus.fi \
    --cc=tapio.reijonen@vaisala.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®