[PATCH v5.10] serial: core: Initialize rs485 RTS polarity already on probe

[PATCH v5.10] serial: core: Initialize rs485 RTS polarity already on probe

[Dominique Martinet]

From: Lukas Wunner <lukas@wunner.de>

Core part of commit 2dd8a74fddd21b95dcc60a2d3c9eaec993419d69 upstream:
the PL011 driver does not support RS485 in the 5.10 tree yet so drop
that bit.

RTS polarity of rs485-enabled ports is currently initialized on uart
open via:

tty_port_open()
  tty_port_block_til_ready()
    tty_port_raise_dtr_rts()  # if (C_BAUD(tty))
      uart_dtr_rts()
        uart_port_dtr_rts()

There's at least three problems here:

First, if no baud rate is set, RTS polarity is not initialized.
That's the right thing to do for rs232, but not for rs485, which
requires that RTS is deasserted unconditionally.

Second, if the DeviceTree property "linux,rs485-enabled-at-boot-time" is
present, RTS should be deasserted as early as possible, i.e. on probe.
Otherwise it may remain asserted until first open.

Third, even though RTS is deasserted on open and close, it may
subsequently be asserted by uart_throttle(), uart_unthrottle() or
uart_set_termios() because those functions aren't rs485-aware.
(Only uart_tiocmset() is.)

To address these issues, move RTS initialization from uart_port_dtr_rts()
to uart_configure_port().  Prevent subsequent modification of RTS
polarity by moving the existing rs485 check from uart_tiocmget() to
uart_update_mctrl().

That way, RTS is initialized on probe and then remains unmodified unless
the uart transmits data.  If rs485 is enabled at runtime (instead of at
boot) through a TIOCSRS485 ioctl(), RTS is initialized by the uart
driver's ->rs485_config() callback and then likewise remains unmodified.

The PL011 driver initializes RTS on uart open and prevents subsequent
modification in its ->set_mctrl() callback.  That code is obsoleted by
the present commit, so drop it.

Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Su Bao Cheng <baocheng.su@siemens.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Link: https://lore.kernel.org/r/2d2acaf3a69e89b7bf687c912022b11fd29dfa1e.1642909284.git.lukas@wunner.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable@vger.kernel.org # 5.10
Reported-by: Daisuke Mizobuchi <mizo@atmark-techno.com>
Tested-by: Daisuke Mizobuchi <mizo@atmark-techno.com>
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
---
Notes:
 - as said in commit message, I've dropped the PL011 part of the
original patch as it is orthogonal to this change. We need this
serial core fix for the imx serial tty driver.

- I wasn't really sure what to do with tags in the commit message,
everything below the 'Cc: stable' tag apply to the backport:
Mizobuchi-san tested the backport on the 5.10 branch with the imx
driver.

- I'm not quite sure how far back it is relevant, for imx I assume
rs485 is broken since 58362d5be352 ("serial: imx: implement handshaking
using gpios with the mctrl_gpio helper") (4.5), and we did have that
problem all the way back in our older 4.9 product tree... but core
support back then wasn't as extensive for RS485 so we have a different
imx specific workaround there.

 - I do not use 5.15 but either version of this patch apply cleanly
there; I'd assume it'd be more appropriate to get the original
2dd8a74fddd21b cherry-picked in this case for 5.15.

Thanks!


 drivers/tty/serial/serial_core.c | 34 +++++++++++---------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 19f0c5db11e3..32d09d024f6c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -144,6 +144,11 @@ uart_update_mctrl(struct uart_port *port, unsigned int set, unsigned int clear)
 	unsigned long flags;
 	unsigned int old;
 
+	if (port->rs485.flags & SER_RS485_ENABLED) {
+		set &= ~TIOCM_RTS;
+		clear &= ~TIOCM_RTS;
+	}
+
 	spin_lock_irqsave(&port->lock, flags);
 	old = port->mctrl;
 	port->mctrl = (old & ~clear) | set;
@@ -157,23 +162,10 @@ uart_update_mctrl(struct uart_port *port, unsigned int set, unsigned int clear)
 
 static void uart_port_dtr_rts(struct uart_port *uport, int raise)
 {
-	int rs485_on = uport->rs485_config &&
-		(uport->rs485.flags & SER_RS485_ENABLED);
-	int RTS_after_send = !!(uport->rs485.flags & SER_RS485_RTS_AFTER_SEND);
-
-	if (raise) {
-		if (rs485_on && RTS_after_send) {
-			uart_set_mctrl(uport, TIOCM_DTR);
-			uart_clear_mctrl(uport, TIOCM_RTS);
-		} else {
-			uart_set_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
-		}
-	} else {
-		unsigned int clear = TIOCM_DTR;
-
-		clear |= (!rs485_on || RTS_after_send) ? TIOCM_RTS : 0;
-		uart_clear_mctrl(uport, clear);
-	}
+	if (raise)
+		uart_set_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
+	else
+		uart_clear_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
 }
 
 /*
@@ -1116,11 +1108,6 @@ uart_tiocmset(struct tty_struct *tty, unsigned int set, unsigned int clear)
 		goto out;
 
 	if (!tty_io_error(tty)) {
-		if (uport->rs485.flags & SER_RS485_ENABLED) {
-			set &= ~TIOCM_RTS;
-			clear &= ~TIOCM_RTS;
-		}
-
 		uart_update_mctrl(uport, set, clear);
 		ret = 0;
 	}
@@ -2429,6 +2416,9 @@ 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->mctrl |= TIOCM_RTS;
 		port->ops->set_mctrl(port, port->mctrl);
 		spin_unlock_irqrestore(&port->lock, flags);
 
-- 
2.35.1

Re: [PATCH v5.10] serial: core: Initialize rs485 RTS polarity already on probe

[Greg Kroah-Hartman]

On Thu, Jun 23, 2022 at 09:58:58AM +0900, Dominique Martinet wrote:
> From: Lukas Wunner <lukas@wunner.de>
> 
> Core part of commit 2dd8a74fddd21b95dcc60a2d3c9eaec993419d69 upstream:
> the PL011 driver does not support RS485 in the 5.10 tree yet so drop
> that bit.
> 
> RTS polarity of rs485-enabled ports is currently initialized on uart
> open via:
> 
> tty_port_open()
>   tty_port_block_til_ready()
>     tty_port_raise_dtr_rts()  # if (C_BAUD(tty))
>       uart_dtr_rts()
>         uart_port_dtr_rts()
> 
> There's at least three problems here:
> 
> First, if no baud rate is set, RTS polarity is not initialized.
> That's the right thing to do for rs232, but not for rs485, which
> requires that RTS is deasserted unconditionally.
> 
> Second, if the DeviceTree property "linux,rs485-enabled-at-boot-time" is
> present, RTS should be deasserted as early as possible, i.e. on probe.
> Otherwise it may remain asserted until first open.
> 
> Third, even though RTS is deasserted on open and close, it may
> subsequently be asserted by uart_throttle(), uart_unthrottle() or
> uart_set_termios() because those functions aren't rs485-aware.
> (Only uart_tiocmset() is.)
> 
> To address these issues, move RTS initialization from uart_port_dtr_rts()
> to uart_configure_port().  Prevent subsequent modification of RTS
> polarity by moving the existing rs485 check from uart_tiocmget() to
> uart_update_mctrl().
> 
> That way, RTS is initialized on probe and then remains unmodified unless
> the uart transmits data.  If rs485 is enabled at runtime (instead of at
> boot) through a TIOCSRS485 ioctl(), RTS is initialized by the uart
> driver's ->rs485_config() callback and then likewise remains unmodified.
> 
> The PL011 driver initializes RTS on uart open and prevents subsequent
> modification in its ->set_mctrl() callback.  That code is obsoleted by
> the present commit, so drop it.
> 
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Su Bao Cheng <baocheng.su@siemens.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Link: https://lore.kernel.org/r/2d2acaf3a69e89b7bf687c912022b11fd29dfa1e.1642909284.git.lukas@wunner.de
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable@vger.kernel.org # 5.10
> Reported-by: Daisuke Mizobuchi <mizo@atmark-techno.com>
> Tested-by: Daisuke Mizobuchi <mizo@atmark-techno.com>
> Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
> ---
> Notes:
>  - as said in commit message, I've dropped the PL011 part of the
> original patch as it is orthogonal to this change. We need this
> serial core fix for the imx serial tty driver.
> 
> - I wasn't really sure what to do with tags in the commit message,
> everything below the 'Cc: stable' tag apply to the backport:
> Mizobuchi-san tested the backport on the 5.10 branch with the imx
> driver.
> 
> - I'm not quite sure how far back it is relevant, for imx I assume
> rs485 is broken since 58362d5be352 ("serial: imx: implement handshaking
> using gpios with the mctrl_gpio helper") (4.5), and we did have that
> problem all the way back in our older 4.9 product tree... but core
> support back then wasn't as extensive for RS485 so we have a different
> imx specific workaround there.
> 
>  - I do not use 5.15 but either version of this patch apply cleanly
> there; I'd assume it'd be more appropriate to get the original
> 2dd8a74fddd21b cherry-picked in this case for 5.15.

Now queued up, thanks.

greg k-h