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 v4] serial: max310x: drive RTS in software when hardware delays are too short
Date: Wed, 23 Sep 2026 12:39:10 +0200 [thread overview]
Message-ID: <2026092326-truth-unweave-c773@gregkh> (raw)
In-Reply-To: <20260916-max310x-rs485-sw-delay-v4-1-d37a694342cf@vaisala.com>
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
prev parent reply other threads:[~2026-09-23 10:39 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-16 7:10 Tapio Reijonen
2026-09-23 10:39 ` Greg Kroah-Hartman [this message]
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=2026092326-truth-unweave-c773@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®