On Sat, 2021-12-18 at 10:58 +0100, Lukas Wunner wrote: Commit a6845e1e1b78 ("serial: core: Consider rs485 settings to drive RTS") sought to deassert RTS when opening an rs485-enabled uart port. That way, the transceiver does not occupy the bus until it transmits data. Unfortunately, the commit mixed up the logic and *asserted* RTS instead of *deasserting* it: The commit amended uart_port_dtr_rts(), which raises DTR and RTS when opening an rs232 port. "Raising" actually means lowering the signal that's coming out of the uart, because an rs232 transceiver not only changes a signal's voltage level, it also *inverts* the signal. See the simplified schematic in the MAX232 datasheet for an example: https://www.ti.com/lit/ds/symlink/max232.pdf So, to raise RTS on an rs232 port, TIOCM_RTS is *set* in port->mctrl and that results in the signal being driven low. In contrast to rs232, the signal level for rs485 Transmit Enable is the identity, not the inversion: If the transceiver expects a "high" RTS signal for Transmit Enable, the signal coming out of the uart must also be high, so TIOCM_RTS must be *cleared* in port->mctrl. The commit did the exact opposite, but it's easy to see why given the confusing semantics of rs232 and rs485. Fix it. Hi Lukas, unfortunately this commit, which has been backported to v5.4.x, seems to break RS485 functionality on our iMX boards. In my opinion, we have a correct dts setup: we're using a GPIO to control the RS485 transceiver Driver Enable pin, which is specified as active high. The iMX GPIO pin directly drives the tranceiver DE pin. Therefore, we did not specify rs485-rts-active-low, so according to the dts-binding documentation RTS should be driven high when sending (default). Also the GPIO is registered as GPIO_ACTIVE_HIGH at rts- gpios. &uart2 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_uart2_rs485>; rts-gpios = <&gpio1 23 GPIO_ACTIVE_HIGH>; linux,rs485-enabled-at-boot-time; status = "okay"; }; So, I'm unsure now whether your commit is wrong, or whether there was a workaround in the iMX driver which now needs to be undone, or whether I made a wrong assumption at the dts setup. Could you please share your opinion on this. Thanks, Henri Fixes: a6845e1e1b78 ("serial: core: Consider rs485 settings to drive RTS") Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx> Cc: stable@xxxxxxxxxxxxxxx # v4.14+ Cc: Rafael Gago Castano <rgc@xxxxxx> Cc: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> Cc: Su Bao Cheng <baocheng.su@xxxxxxxxxxx> --- drivers/tty/serial/serial_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 29f4781..259f28e 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -162,7 +162,7 @@ static void uart_port_dtr_rts(struct uart_port *uport, int raise) int RTS_after_send = !!(uport->rs485.flags & SER_RS485_RTS_AFTER_SEND); if (raise) { - if (rs485_on && !RTS_after_send) { + if (rs485_on && RTS_after_send) { uart_set_mctrl(uport, TIOCM_DTR); uart_clear_mctrl(uport, TIOCM_RTS); } else { @@ -171,7 +171,7 @@ static void uart_port_dtr_rts(struct uart_port *uport, int raise) } else { unsigned int clear = TIOCM_DTR; - clear |= (!rs485_on || !RTS_after_send) ? TIOCM_RTS : 0; + clear |= (!rs485_on || RTS_after_send) ? TIOCM_RTS : 0; uart_clear_mctrl(uport, clear); } }
Attachment:
smime.p7s
Description: S/MIME cryptographic signature