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
next prev parent reply other threads:[~2026-09-23 10:39 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-16 7:10 [PATCH v4] serial: max310x: drive RTS in software when hardware delays are too short Tapio Reijonen
2026-09-16 7:26 ` sashiko-bot
2026-09-23 10:39 ` Greg Kroah-Hartman [this message]
2026-09-29 7:13 ` 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=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.