From: Nicolai Buchwitz <nb@tipi-net.de>
To: sashiko-reviews@lists.linux.dev
Cc: linux-serial@vger.kernel.org
Subject: Re: [PATCH v4 2/2] serial: amba-pl011: keep RS485 transmission running on reconfiguration
Date: Wed, 21 Oct 2026 14:19:00 +0200 [thread overview]
Message-ID: <a61f86ea6bec868c75753e3ddc1ff242@tipi-net.de> (raw)
In-Reply-To: <sashiko-outbox-180213@kernel.org>
On 21.10.2026 14:12, sashiko-bot@kernel.org wrote:
> [...]
> [Severity: Medium]
> During a TIOCSRS485 ioctl, uart_set_rs485_config() invokes
> port->rs485_config() before assigning the new configuration to
> port->rs485:
>
> drivers/tty/serial/serial_core.c:uart_set_rs485_config() {
> ...
> scoped_guard(uart_port_lock_irqsave, port) {
> ret = port->rs485_config(port, &tty->termios, &rs485);
> if (!ret) {
> port->rs485 = rs485;
> ...
> }
>
> In drivers/tty/serial/amba-pl011.c, pl011_rs485_config() calls
> pl011_rs485_tx_stop() -> pl011_rs485_tx_stop_now(), which reads
> port->rs485.flags rather than the new rs485->flags:
>
> drivers/tty/serial/amba-pl011.c:pl011_rs485_tx_stop_now() {
> ...
> if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> cr &= ~UART011_CR_RTS;
> else
> cr |= UART011_CR_RTS;
> ...
> }
>
> Doesn't this cause three regressions when configuring RS485 via
> TIOCSRS485?
>
> First, when RS485 is already enabled and idle (uap->rs485_tx_state ==
> OFF)
> and userspace changes SER_RS485_RTS_AFTER_SEND,
> pl011_rs485_tx_stop_now()
> programs UART011_CR_RTS using the old port->rs485.flags instead of the
> new
> rs485->flags.
>
> Second, because the else-if condition in pl011_rs485_config() checks
> port->rs485.flags & SER_RS485_ENABLED instead of
> rs485->flags & SER_RS485_ENABLED, newly enabling RS485 on an open, idle
> port
> (where pl011_tx_active(uap) is false and pl011_tx_empty(port) is true)
> skips
> both pl011_rs485_tx_resume() and pl011_rs485_tx_stop(), leaving
> UART011_CR_TXE enabled and UART011_CR_RTS at its previous non-RS485
> level.
>
> Third, when RS485 is reconfigured while a transmission or break is
> active
> (uap->rs485_tx_state != OFF), all branches in pl011_rs485_config() are
> skipped, so changes to SER_RS485_RTS_ON_SEND or SER_RS485_RX_DURING_TX
> in
> rs485->flags are not applied to REG_CR for the ongoing transmission.
>
> Should pl011_rs485_config() check rs485->flags & SER_RS485_ENABLED and
> apply
> the new rs485->flags when setting the idle or active REG_CR state?
All three predate this series, the branch read port->rs485 the same way
before.
The cover letter lists them as left for the RS485 rework, which fixes
them by
applying the new settings in pl011_rs485_config().
IMHO that's too big for these stable fixes.
Thanks,
Nicolai
prev parent reply other threads:[~2026-10-21 12:19 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-21 11:45 [PATCH v4 0/2] serial: amba-pl011: fix RS485 state on enable and reconfiguration Nicolai Buchwitz
2026-10-21 11:45 ` [PATCH v4 1/2] serial: amba-pl011: finish a transmission still running when RS485 is enabled Nicolai Buchwitz
2026-10-21 12:10 ` sashiko-bot
2026-10-21 11:45 ` [PATCH v4 2/2] serial: amba-pl011: keep RS485 transmission running on reconfiguration Nicolai Buchwitz
2026-10-21 12:12 ` sashiko-bot
2026-10-21 12:19 ` Nicolai Buchwitz [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=a61f86ea6bec868c75753e3ddc1ff242@tipi-net.de \
--to=nb@tipi-net.de \
--cc=linux-serial@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/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.