On Fri, 16 Sep 2022, Lukas Wunner wrote:
> When a UART port is newly registered, uart_configure_port() seeks to
> deassert RS485 Transmit Enable by setting the RTS bit in port->mctrl.
> However a number of UART drivers interpret a set RTS bit as *assertion*
> instead of deassertion: Affected drivers include those using
> serial8250_em485_config() (except 8250_bcm2835aux.c) and some using
> mctrl_gpio (e.g. imx.c).
>
> Since the interpretation of the RTS bit is driver-specific, it is not
> suitable as a means to centrally deassert Transmit Enable in the serial
> core. Instead, the serial core must call on drivers to deassert it in
> their driver-specific way. One way to achieve that is to call
> ->rs485_config(). It implicitly deasserts Transmit Enable.
>
> So amend uart_configure_port() and uart_resume_port() to invoke
> uart_rs485_config(). That allows removing calls to uart_rs485_config()
> from drivers' ->probe() hooks and declaring the function static.
>
> Skip any invocation of ->set_mctrl() if RS485 is enabled. RS485 has no
> hardware flow control, so the modem control lines are irrelevant and
> need not be touched. When leaving RS485 mode, reset the modem control
> lines to the state stored in port->mctrl. That way, UARTs which are
> muxed between RS485 and RS232 transceivers drive the lines correctly
> when switched to RS232. (serial8250_do_startup() historically raises
> the OUT1 modem signal because otherwise interrupts are not signaled on
> ancient PC UARTs, but I believe that no longer applies to modern,
> RS485-capable UARTs and is thus safe to be skipped.)
>
> imx.c modifies port->mctrl whenever Transmit Enable is asserted and
> deasserted. Stop it from doing that so port->mctrl reflects the RS232
> line state.
>
> 8250_omap.c deasserts Transmit Enable on ->runtime_resume() by calling
> ->set_mctrl(). Because that is now a no-op in RS485 mode, amend the
> function to call serial8250_em485_stop_tx().
>
> fsl_lpuart.c retrieves and applies the RS485 device tree properties
> after registering the UART port. Because applying now happens on
> registration in uart_configure_port(), move retrieval of the properties
> ahead of uart_add_one_port().
>
> Fixes: d3b3404df318 ("serial: Fix incorrect rs485 polarity on uart open")
> Reported-by: Matthias Schiffer <matthias.schiffer@xxxxxxxxxxxxxxx>
> Link: https://lore.kernel.org/all/20220329085050.311408-1-matthias.schiffer@xxxxxxxxxxxxxxx/
> Reported-by: Roosen Henri <Henri.Roosen@xxxxxxxxxxxxx>
> Link: https://lore.kernel.org/all/8f538a8903795f22f9acc94a9a31b03c9c4ccacb.camel@xxxxxxxxxxxxx/
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # v4.14+
> ---
> Based on v6.0-rc3 + this dependency:
> https://lore.kernel.org/linux-serial/72fb646c1b0b11c989850c55f52f9ff343d1b2fa.1662884345.git.lukas@xxxxxxxxx/
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 907c5ff..a6f03b1 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -600,7 +600,7 @@ void serial8250_rpm_put(struct uart_8250_port *p)
> static int serial8250_em485_init(struct uart_8250_port *p)
> {
> if (p->em485)
> - return 0;
> + goto deassert_rts;
>
> p->em485 = kmalloc(sizeof(struct uart_8250_em485), GFP_ATOMIC);
> if (!p->em485)
> @@ -616,6 +616,7 @@ static int serial8250_em485_init(struct uart_8250_port *p)
> p->em485->active_timer = NULL;
> p->em485->tx_stopped = true;
>
> +deassert_rts:
> p->rs485_stop_tx(p);
if (p->em485->tx_stopped)
p->rs485_stop_tx(p);
?
Because if p->em485->tx_stopped is false and serial8250_em485_init() is
called again (the comment above it says it is safe to do so), it would
stop tx at wrong point.
--
i.
