Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jirislaby@kernel.org>,
        Lukas Wunner <lukas@wunner.de>
Cc:     Russell King <linux@armlinux.org.uk>, linux-serial@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Subject: [PATCH] serial: Revert RS485 polarity change on UART open
Date:   Tue, 29 Mar 2022 10:50:50 +0200
Message-Id: <20220329085050.311408-1-matthias.schiffer@ew.tq-group.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

While the change of the RS485 polarity in
commit d3b3404df318 ("serial: Fix incorrect rs485 polarity on uart open")
might have made sense based on the original intention of the
rs485-rts-active-low flag (*), this is not how it is implemented in
various drivers:

At least the 8250 and the i.MX UART drivers interpret rs485-rts-active-low
the other way around. For these drivers, said change broke the initial
state of the RTS pin after opening the UART, making it impossible to
receive data until after the first send.

I considered fixing up the drivers to match the polarity used for the
initial state, however doing so would break all existing Device Trees
that configure RS485 for one of these drivers. Reverting the change
makes RS485 work correctly again on these devices.

[(*) My understanding of the mentioned commit's description is that
rs485-rts-active-low should have referred to the electical signal level
of the RTS pin, rather than the logical RTS state as understood by the
UART controller.]

Fixes: d3b3404df318 ("serial: Fix incorrect rs485 polarity on uart open")
Fixes: 2dd8a74fddd2 ("serial: core: Initialize rs485 RTS polarity already on probe")
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---

Skimming through a few other drivers with RS485 support, many other
implementation are in agreement with the 8250 and i.MX drivers. This seems
to be the case for the OMAP and the STM32 drivers. The PL011 driver on the
other hand seems to disagree, so it will need to be fixed if the intention
is to make all drivers consistent (preferable by someone who is familiar
with that hardware and can test the change).

It is unfortunate that different drivers disagree here, as any fix to
make things more consistent will break some users. One way to avoid that
would be to deprecate the rs485-rts-active-low flag altogether and replace
it with something new that is implemented consistently...

I can also propose a clarification for the DT binding documentation, if
this revert is how we want to proceed with the issue.

 drivers/tty/serial/serial_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 6a8963caf954..c1c8bd712a59 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2390,7 +2390,7 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
 		spin_lock_irqsave(&port->lock, flags);
 		port->mctrl &= TIOCM_DTR;
 		if (port->rs485.flags & SER_RS485_ENABLED &&
-		    !(port->rs485.flags & SER_RS485_RTS_AFTER_SEND))
+		    !(port->rs485.flags & SER_RS485_RTS_ON_SEND))
 			port->mctrl |= TIOCM_RTS;
 		port->ops->set_mctrl(port, port->mctrl);
 		spin_unlock_irqrestore(&port->lock, flags);
-- 
2.25.1