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 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

      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®