* [PATCH] serial: Revert RS485 polarity change on UART open
@ 2022-03-29 8:50 Matthias Schiffer
2022-03-29 10:03 ` Lukas Wunner
0 siblings, 1 reply; 10+ messages in thread
From: Matthias Schiffer @ 2022-03-29 8:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Lukas Wunner
Cc: Russell King, linux-serial, linux-kernel, Matthias Schiffer
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] serial: Revert RS485 polarity change on UART open 2022-03-29 8:50 [PATCH] serial: Revert RS485 polarity change on UART open Matthias Schiffer @ 2022-03-29 10:03 ` Lukas Wunner 2022-03-29 10:39 ` Matthias Schiffer 0 siblings, 1 reply; 10+ messages in thread From: Lukas Wunner @ 2022-03-29 10:03 UTC (permalink / raw) To: Matthias Schiffer Cc: Greg Kroah-Hartman, Jiri Slaby, Russell King, linux-serial, linux-kernel, Ilpo Järvinen, Lino Sanfilippo [cc += Ilpo, Lino] On Tue, Mar 29, 2022 at 10:50:50AM +0200, Matthias Schiffer wrote: > 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: [...] > [(*) 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.] Since RTS is often just a GPIO on a pin controller that's configured to function as RTS, my expectation would be that the same rules apply to RTS polarity as those that apply to *any* GPIO. According to Documentation/devicetree/bindings/gpio/gpio.txt: "A gpio-specifier should contain a flag indicating the GPIO polarity; active- high or active-low. If it does, the following best practices should be followed: The gpio-specifier's polarity flag should represent the physical level at the ^^^^^^^^^^^^^^ GPIO controller that achieves (or represents, for inputs) a logically asserted value at the device." > At least the 8250 and the i.MX UART drivers interpret rs485-rts-active-low Which 8250 driver are you referring to specifically? When developing d3b3404df318, I tested with 8250_bcm2835aux.c and amba-pl011.c. Both worked exactly the way they should. If imx.c and others have historically interpreted rs485-rts-active-low to mean that the physical level is "high" when active, then we could just amend imx_uart_probe() such that after calling uart_get_rs485_mode(), the SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND bits are flipped. Would that work for you? I'll go through the drivers to check which ones are affected. I'm sorry that you're seeing breakage, it's surprising to me that these different interpretations of rs485-rts-active-low exist. Thanks, Lukas ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] serial: Revert RS485 polarity change on UART open 2022-03-29 10:03 ` Lukas Wunner @ 2022-03-29 10:39 ` Matthias Schiffer 2022-03-29 12:55 ` David Laight 2022-06-27 8:40 ` Matthias Schiffer 0 siblings, 2 replies; 10+ messages in thread From: Matthias Schiffer @ 2022-03-29 10:39 UTC (permalink / raw) To: Lukas Wunner Cc: Greg Kroah-Hartman, Jiri Slaby, Russell King, linux-serial, linux-kernel, Ilpo Järvinen, Lino Sanfilippo On Tue, 2022-03-29 at 12:03 +0200, Lukas Wunner wrote: > [cc += Ilpo, Lino] > > On Tue, Mar 29, 2022 at 10:50:50AM +0200, Matthias Schiffer wrote: > > 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: > [...] > > [(*) 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.] Hi Lukas, > > Since RTS is often just a GPIO on a pin controller that's configured > to function as RTS, my expectation would be that the same rules apply > to RTS polarity as those that apply to *any* GPIO. > > According to Documentation/devicetree/bindings/gpio/gpio.txt: > > "A gpio-specifier should contain a flag indicating the GPIO polarity; > active- > high or active-low. If it does, the following best practices should > be > followed: > The gpio-specifier's polarity flag should represent the physical > level at the > ^^^^^^^^^^^^ > ^^ > GPIO controller that achieves (or represents, for inputs) a > logically asserted > value at the device." Yes, that would make sense to me as well, but as described, this is not how the majority of drivers that I looked at works at the moment :( I'm not particularly attached to any of the interpretations, but it would be great to have some consistency here. And if the majority of drivers does it the "wrong" way, maybe we should accept that to keep the breakage as small as possible? > > > > At least the 8250 and the i.MX UART drivers interpret rs485-rts- > > active-low > > Which 8250 driver are you referring to specifically? When developing > d3b3404df318, I tested with 8250_bcm2835aux.c and amba-pl011.c. Both > worked exactly the way they should. I tested with 8250_omap.c, which does not implement the RS485 handling itself, but refers to the generic code in 8250_port.c. In fact, my first attempt to get the RS485 to work on my device was the following (which matches the originally intended interpretation of the polarity flag, but breaks existing users): --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -1460,9 +1460,9 @@ void serial8250_em485_stop_tx(struct uart_8250_port *p) unsigned char mcr = serial8250_in_MCR(p); if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) - mcr |= UART_MCR_RTS; - else mcr &= ~UART_MCR_RTS; + else + mcr |= UART_MCR_RTS; serial8250_out_MCR(p, mcr); /* @@ -1611,9 +1611,9 @@ void serial8250_em485_start_tx(struct uart_8250_port *up) serial8250_stop_rx(&up->port); if (up->port.rs485.flags & SER_RS485_RTS_ON_SEND) - mcr |= UART_MCR_RTS; - else mcr &= ~UART_MCR_RTS; + else + mcr |= UART_MCR_RTS; serial8250_out_MCR(up, mcr); } > > If imx.c and others have historically interpreted rs485-rts-active- > low > to mean that the physical level is "high" when active, then we could > just > amend imx_uart_probe() such that after calling uart_get_rs485_mode(), > the SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND bits are > flipped. Would that work for you? > I guess that would work. The fact that even the different variants of the 8250 are implemented inconsistently makes this especially ugly... It certainly puts a damper on the efforts to make the handling of RS485 in serial drivers more generic. > I'll go through the drivers to check which ones are affected. I'm > sorry > that you're seeing breakage, it's surprising to me that these > different > interpretations of rs485-rts-active-low exist. > > Thanks, > > Lukas Regards, Matthias ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] serial: Revert RS485 polarity change on UART open 2022-03-29 10:39 ` Matthias Schiffer @ 2022-03-29 12:55 ` David Laight 2022-03-29 13:02 ` Matthias Schiffer 2022-06-27 8:40 ` Matthias Schiffer 1 sibling, 1 reply; 10+ messages in thread From: David Laight @ 2022-03-29 12:55 UTC (permalink / raw) To: 'Matthias Schiffer', Lukas Wunner Cc: Greg Kroah-Hartman, Jiri Slaby, Russell King, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, Ilpo Järvinen, Lino Sanfilippo From: Matthias Schiffer > Sent: 29 March 2022 11:39 ... > I guess that would work. The fact that even the different > variants of the 8250 are implemented inconsistently makes this > especially ugly... It certainly puts a damper on the efforts to make > the handling of RS485 in serial drivers more generic. One thing to remember is that RS232 (IIRC really V.38) line driver chips are typically inverting. So the modem signals on a TTL level output will have the opposite polarity to that required on the actual connector. Normally a UART will have an 'active high' register bit for a modem signal that drives and 'active low' pin so you get the correct polarity with an inverting line driver. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] serial: Revert RS485 polarity change on UART open 2022-03-29 12:55 ` David Laight @ 2022-03-29 13:02 ` Matthias Schiffer 2022-03-29 13:19 ` David Laight 0 siblings, 1 reply; 10+ messages in thread From: Matthias Schiffer @ 2022-03-29 13:02 UTC (permalink / raw) To: David Laight Cc: Greg Kroah-Hartman, Jiri Slaby, Russell King, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, Ilpo Järvinen, Lino Sanfilippo, Lukas Wunner On Tue, 2022-03-29 at 12:55 +0000, David Laight wrote: > From: Matthias Schiffer > > Sent: 29 March 2022 11:39 > ... > > I guess that would work. The fact that even the different > > variants of the 8250 are implemented inconsistently makes this > > especially ugly... It certainly puts a damper on the efforts to > > make > > the handling of RS485 in serial drivers more generic. > > One thing to remember is that RS232 (IIRC really V.38) line driver > chips are typically inverting. > > So the modem signals on a TTL level output will have the > opposite polarity to that required on the actual connector. > > Normally a UART will have an 'active high' register bit for > a modem signal that drives and 'active low' pin so you get > the correct polarity with an inverting line driver. > > David > Indeed. As far as I can tell, this property of UARTs is what got us into this mess: Some people interpreted SER_RS485_RTS_ON_SEND as "set the RTS flag in the MCR register on send", while other thought it should mean "set the RTS pin to high on send", leading to opposite behaviours in different UART drivers (and even different UART variants in the same driver, in the case of the 8250 family). Regards, Matthias ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] serial: Revert RS485 polarity change on UART open 2022-03-29 13:02 ` Matthias Schiffer @ 2022-03-29 13:19 ` David Laight 2022-03-29 13:36 ` (EXT) " Matthias Schiffer 2022-05-11 10:22 ` m.brock 0 siblings, 2 replies; 10+ messages in thread From: David Laight @ 2022-03-29 13:19 UTC (permalink / raw) To: 'Matthias Schiffer' Cc: Greg Kroah-Hartman, Jiri Slaby, Russell King, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, Ilpo Järvinen, Lino Sanfilippo, Lukas Wunner From: Matthias Schiffer > Sent: 29 March 2022 14:03 > > On Tue, 2022-03-29 at 12:55 +0000, David Laight wrote: > > From: Matthias Schiffer > > > Sent: 29 March 2022 11:39 > > ... > > > I guess that would work. The fact that even the different > > > variants of the 8250 are implemented inconsistently makes this > > > especially ugly... It certainly puts a damper on the efforts to > > > make > > > the handling of RS485 in serial drivers more generic. > > > > One thing to remember is that RS232 (IIRC really V.38) line driver > > chips are typically inverting. > > > > So the modem signals on a TTL level output will have the > > opposite polarity to that required on the actual connector. > > > > Normally a UART will have an 'active high' register bit for > > a modem signal that drives and 'active low' pin so you get > > the correct polarity with an inverting line driver. > > > > David > > > > Indeed. As far as I can tell, this property of UARTs is what got us > into this mess: Some people interpreted SER_RS485_RTS_ON_SEND as "set > the RTS flag in the MCR register on send", while other thought it > should mean "set the RTS pin to high on send", leading to opposite > behaviours in different UART drivers (and even different UART variants > in the same driver, in the case of the 8250 family). Hmmm... A complete mess. The 'RTS pin' that needs to go high is the one on the (typically) 'D' connector after the inverting line driver. Not the pin on the uart package. I'd expect TTL level serial interfaces to require active low modem signals. If RS485 is trying to do half duplex using RTS (request to send) and CTS (clear to send) you've typically got bigger problems than asserting RTS before a transmit. The real problem is removing RTS once the last transmit data bit (the stop bit) has left the UART pin. I've used local loopback (tx to rx) to detect that in the past. Of course, if it is just doing flow control that should use RFS (ready for sending) to indicate space in the receive fifo but using the RTS pin instead that is a different matter. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: (EXT) RE: [PATCH] serial: Revert RS485 polarity change on UART open 2022-03-29 13:19 ` David Laight @ 2022-03-29 13:36 ` Matthias Schiffer 2022-05-11 10:22 ` m.brock 1 sibling, 0 replies; 10+ messages in thread From: Matthias Schiffer @ 2022-03-29 13:36 UTC (permalink / raw) To: David Laight Cc: Greg Kroah-Hartman, Jiri Slaby, Russell King, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, Ilpo Järvinen, Lino Sanfilippo, Lukas Wunner On Tue, 2022-03-29 at 13:19 +0000, David Laight wrote: > From: Matthias Schiffer > > Sent: 29 March 2022 14:03 > > > > On Tue, 2022-03-29 at 12:55 +0000, David Laight wrote: > > > From: Matthias Schiffer > > > > Sent: 29 March 2022 11:39 > > > ... > > > > I guess that would work. The fact that even the different > > > > variants of the 8250 are implemented inconsistently makes this > > > > especially ugly... It certainly puts a damper on the efforts to > > > > make > > > > the handling of RS485 in serial drivers more generic. > > > > > > One thing to remember is that RS232 (IIRC really V.38) line > > > driver > > > chips are typically inverting. > > > > > > So the modem signals on a TTL level output will have the > > > opposite polarity to that required on the actual connector. > > > > > > Normally a UART will have an 'active high' register bit for > > > a modem signal that drives and 'active low' pin so you get > > > the correct polarity with an inverting line driver. > > > > > > David > > > > > > > Indeed. As far as I can tell, this property of UARTs is what got us > > into this mess: Some people interpreted SER_RS485_RTS_ON_SEND as > > "set > > the RTS flag in the MCR register on send", while other thought it > > should mean "set the RTS pin to high on send", leading to opposite > > behaviours in different UART drivers (and even different UART > > variants > > in the same driver, in the case of the 8250 family). > > Hmmm... A complete mess. > The 'RTS pin' that needs to go high is the one on the (typically) 'D' > connector after the inverting line driver. > Not the pin on the uart package. > I'd expect TTL level serial interfaces to require active low > modem signals. > > If RS485 is trying to do half duplex using RTS (request to send) > and CTS (clear to send) you've typically got bigger problems > than asserting RTS before a transmit. > The real problem is removing RTS once the last transmit data bit > (the stop bit) has left the UART pin. > I've used local loopback (tx to rx) to detect that in the past. > > Of course, if it is just doing flow control that should use RFS > (ready for sending) to indicate space in the receive fifo but > using the RTS pin instead that is a different matter. > > David > I'm aware of the difficulties of deasserting RTS after the transmission is complete, but that's completely orthogonal to the issue I'm trying to solve right now :) Regards, Matthias ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] serial: Revert RS485 polarity change on UART open 2022-03-29 13:19 ` David Laight 2022-03-29 13:36 ` (EXT) " Matthias Schiffer @ 2022-05-11 10:22 ` m.brock 1 sibling, 0 replies; 10+ messages in thread From: m.brock @ 2022-05-11 10:22 UTC (permalink / raw) To: David Laight Cc: 'Matthias Schiffer', Greg Kroah-Hartman, Jiri Slaby, Russell King, linux-serial, linux-kernel, Ilpo Järvinen, Lino Sanfilippo, Lukas Wunner David Laight wrote on 2022-03-29 15:19: > From: Matthias Schiffer >> Sent: 29 March 2022 14:03 >> >> On Tue, 2022-03-29 at 12:55 +0000, David Laight wrote: >> > From: Matthias Schiffer >> > > Sent: 29 March 2022 11:39 >> > ... >> > > I guess that would work. The fact that even the different >> > > variants of the 8250 are implemented inconsistently makes this >> > > especially ugly... It certainly puts a damper on the efforts to >> > > make the handling of RS485 in serial drivers more generic. >> > >> > One thing to remember is that RS232 (IIRC really V.38) line driver >> > chips are typically inverting. >> > >> > So the modem signals on a TTL level output will have the >> > opposite polarity to that required on the actual connector. >> > >> > Normally a UART will have an 'active high' register bit for >> > a modem signal that drives and 'active low' pin so you get >> > the correct polarity with an inverting line driver. >> > >> > David >> > >> >> Indeed. As far as I can tell, this property of UARTs is what got us >> into this mess: Some people interpreted SER_RS485_RTS_ON_SEND as "set >> the RTS flag in the MCR register on send", while other thought it >> should mean "set the RTS pin to high on send", leading to opposite >> behaviours in different UART drivers (and even different UART variants >> in the same driver, in the case of the 8250 family). > > Hmmm... A complete mess. > The 'RTS pin' that needs to go high is the one on the (typically) 'D' > connector after the inverting line driver. > Not the pin on the uart package. > I'd expect TTL level serial interfaces to require active low > modem signals. A typical 'D' connector for RS485? I've seen all sorts of connectors and I can't say one of them is typical. You're assuming there are RS232 level shifters in play, but usually there aren't and the TTL UART signals are directly connected to an RS485 transceiver. For RS485 there is no outside RTS pin, so you will have to talk about the UART TTL pin. https://www.kernel.org/doc/Documentation/serial/serial-rs485.txt is unfortunately not entirely clear: /* Set logical level for RTS pin equal to 1 when sending: */ rs485conf.flags |= SER_RS485_RTS_ON_SEND; Most (all?) UART chips have an RTSn, RTS# or /RTS pin which means RTS inverted. So the "RTS pin" can't be set to logic level 1, but RTS can. IMHO it would be best to let SER_RS485_RTS_ON_SEND mean that RTS is asserted during sending. That means that RTSn should be 0V. When the UART chip is connected directly to an RS485 driver with RTSn connected to DE, you need SER_RS485_RTS_AFTER_SEND so RTS=0 and RTSn=DE=1 during sending. When a UART is not in RS485 mode and unopened it usually has RTS=0, RTSn=1 which would make any directly connected RS485 driver claim the bus. This is probably undesired behaviour and thus asks for an extra inverter between RTSn and DE. With this added inverter one would need SER_RS485_RTS_ON_SEND. https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/serial/rs485.yaml isn't much better either: rs485-rts-active-low: description: drive RTS low when sending (default is high). Was this meant as "drive the RTSn pin low when sending"? Finally, can these two methods be synchronized in their wording? E.g. can we just drop "rs485-rts-active-low" and replace it with "rs485-rts-on-send" and "rs485-rts-after-send"? Maarten ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] serial: Revert RS485 polarity change on UART open 2022-03-29 10:39 ` Matthias Schiffer 2022-03-29 12:55 ` David Laight @ 2022-06-27 8:40 ` Matthias Schiffer 2022-06-29 17:01 ` Lukas Wunner 1 sibling, 1 reply; 10+ messages in thread From: Matthias Schiffer @ 2022-06-27 8:40 UTC (permalink / raw) To: Lukas Wunner Cc: Greg Kroah-Hartman, Jiri Slaby, Russell King, linux-serial, linux-kernel, Ilpo Järvinen, Lino Sanfilippo On Tue, 2022-03-29 at 12:39 +0200, Matthias Schiffer wrote: > On Tue, 2022-03-29 at 12:03 +0200, Lukas Wunner wrote: > > [cc += Ilpo, Lino] > > > > On Tue, Mar 29, 2022 at 10:50:50AM +0200, Matthias Schiffer wrote: > > > 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: > > [...] > > > [(*) 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.] > > Hi Lukas, > > > Since RTS is often just a GPIO on a pin controller that's configured > > to function as RTS, my expectation would be that the same rules apply > > to RTS polarity as those that apply to *any* GPIO. > > > > According to Documentation/devicetree/bindings/gpio/gpio.txt: > > > > "A gpio-specifier should contain a flag indicating the GPIO polarity; > > active- > > high or active-low. If it does, the following best practices should > > be > > followed: > > The gpio-specifier's polarity flag should represent the physical > > level at the > > ^^^^^^^^^^^^ > > ^^ > > GPIO controller that achieves (or represents, for inputs) a > > logically asserted > > value at the device." > > Yes, that would make sense to me as well, but as described, this is not > how the majority of drivers that I looked at works at the moment :( > > I'm not particularly attached to any of the interpretations, but it > would be great to have some consistency here. And if the majority of > drivers does it the "wrong" way, maybe we should accept that to keep > the breakage as small as possible? > > > > > > At least the 8250 and the i.MX UART drivers interpret rs485-rts- > > > active-low > > > > Which 8250 driver are you referring to specifically? When developing > > d3b3404df318, I tested with 8250_bcm2835aux.c and amba-pl011.c. Both > > worked exactly the way they should. > > I tested with 8250_omap.c, which does not implement the RS485 handling > itself, but refers to the generic code in 8250_port.c. In fact, my > first attempt to get the RS485 to work on my device was the following > (which matches the originally intended interpretation of the polarity > flag, but breaks existing users): > > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > @@ -1460,9 +1460,9 @@ void serial8250_em485_stop_tx(struct > uart_8250_port *p) > unsigned char mcr = serial8250_in_MCR(p); > > if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) > - mcr |= UART_MCR_RTS; > - else > mcr &= ~UART_MCR_RTS; > + else > + mcr |= UART_MCR_RTS; > serial8250_out_MCR(p, mcr); > > /* > @@ -1611,9 +1611,9 @@ void serial8250_em485_start_tx(struct > uart_8250_port *up) > serial8250_stop_rx(&up->port); > > if (up->port.rs485.flags & SER_RS485_RTS_ON_SEND) > - mcr |= UART_MCR_RTS; > - else > mcr &= ~UART_MCR_RTS; > + else > + mcr |= UART_MCR_RTS; > serial8250_out_MCR(up, mcr); > } > > > If imx.c and others have historically interpreted rs485-rts-active- > > low > > to mean that the physical level is "high" when active, then we could > > just > > amend imx_uart_probe() such that after calling uart_get_rs485_mode(), > > the SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND bits are > > flipped. Would that work for you? > > > > I guess that would work. The fact that even the different > variants of the 8250 are implemented inconsistently makes this > especially ugly... It certainly puts a damper on the efforts to make > the handling of RS485 in serial drivers more generic. > > > > I'll go through the drivers to check which ones are affected. I'm > > sorry > > that you're seeing breakage, it's surprising to me that these > > different > > interpretations of rs485-rts-active-low exist. > > Thanks, > > > > Lukas > Hi Lukas, do you know if there has been any progress on this issue? I see that there has been quite a bit of activity in the RS485 code in linux-next, but I didn't have time to check if that has any effect on the polarity issue so far. Thanks, Matthias ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] serial: Revert RS485 polarity change on UART open 2022-06-27 8:40 ` Matthias Schiffer @ 2022-06-29 17:01 ` Lukas Wunner 0 siblings, 0 replies; 10+ messages in thread From: Lukas Wunner @ 2022-06-29 17:01 UTC (permalink / raw) To: Matthias Schiffer Cc: Greg Kroah-Hartman, Jiri Slaby, Russell King, linux-serial, linux-kernel, Ilpo Järvinen, Lino Sanfilippo, m.brock, David Laight On Mon, Jun 27, 2022 at 10:40:36AM +0200, Matthias Schiffer wrote: > On Tue, 2022-03-29 at 12:39 +0200, Matthias Schiffer wrote: > > On Tue, 2022-03-29 at 12:03 +0200, Lukas Wunner wrote: > > > On Tue, Mar 29, 2022 at 10:50:50AM +0200, Matthias Schiffer wrote: > > > > 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: > > > [...] > > > > [(*) 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.] > > do you know if there has been any progress on this issue? I see that > there has been quite a bit of activity in the RS485 code in linux-next, > but I didn't have time to check if that has any effect on the polarity > issue so far. No, the issue has not been resolved yet but at least I have inspected all drivers now and compiled a list of their behavior: 8250_bcm2835aux: identity (RTS in software) 8250_dwlib: identity (RTS in hardware, DE active high after POR) inverse (RTS in software em485) 8250_exar: identity (RTS in hardware) 8250_fintek: inverse (RTS in hardware) 8250_lpc18xx: identity (RTS in hardware) 8250_of: inverse (RTS in software em485) [ns8250, ns16550, lpc3220, da830, wpcm450] 8250_omap: inverse (RTS in software em485) 8250_pci: identity (RTS in hardware) amba-pl011: identity (RTS in software) ar933x_uart: identity (RTS as gpio) atmel_serial: identity (RTS in hardware, only high supported) fsl_lpuart: inverse (RTS in hardware) imx: identity (RTS as gpio or CTS in software) max310x: identity (RTS in hardware) mcf: identity (RTS in hardware) omap-serial: identity (RTS as gpio) sc16is7xx: inverse (RTS in hardware) stm32-usart: identity (RTS in hardware) identity (RTS as gpio) Where "identity" means that SER_RS485_RTS_ON_SEND results in "high" voltage level and "inverse" means it results in "low" voltage level. Unless I've made any mistakes here, it looks like the majority of drivers use "identity". The only ones using inverse polarity are 8250-compatible UARTs using em485 software emulation (with the exception of bcm2835aux), plus three UART drivers which use hardware-driven RTS. When you reported the issue, you claimed that i.MX, OMAP and STM32 drivers use inverse polarity. I was only able to confirm that for OMAP (if 8250_omap.c is used, not the older omap-serial.c). When you refer to i.MX, I suppose you're referring to fsl_lpuart.c (which uses hardware-controlled RTS assertion on newer i.MX SoCs), not imx.c, right? The latter offers two options to drive an RS-485 transceiver, either through CTS or through an mctrl_gpio. imx_uart_rts_active() clears the CTS bit in the UCR2 register. According to page 4679 of iMX53RM.pdf, that means "high" voltage level, hence "identity". And if an mctrl_gpio is used, mctrl_gpio_set() sets the GPIO to "active", which likewise means "high" voltage level unless you've specified the GPIO as active-low in the devicetree: https://www.nxp.com/docs/en/reference-manual/iMX53RM.pdf As for stm32-usart.c, the driver offers either hardware-controlled RTS assertion or mctrl_gpio. The hardware-controlled variant clears the DEP bit in the USART_CR3 register if SER_RS485_RTS_ON_SEND is set. According to page 1392 of the STM32 documentation this means that "DE signal is active high", so it seems to me that this driver uses "identity and not "inverse" as claimed: https://www.st.com/resource/en/reference_manual/rm0351-stm32l47xxx-stm32l48xxx-stm32l49xxx-and-stm32l4axxx-advanced-armbased-32bit-mcus-stmicroelectronics.pdf Anyway, I'll try to come up with a fix for the affected em485 drivers by next week. The "inverse" drivers with hardware-controlled RTS should not be affected by d3b3404df318 (unless I'm missing something). Thanks! Lukas ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-06-29 17:02 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-03-29 8:50 [PATCH] serial: Revert RS485 polarity change on UART open Matthias Schiffer 2022-03-29 10:03 ` Lukas Wunner 2022-03-29 10:39 ` Matthias Schiffer 2022-03-29 12:55 ` David Laight 2022-03-29 13:02 ` Matthias Schiffer 2022-03-29 13:19 ` David Laight 2022-03-29 13:36 ` (EXT) " Matthias Schiffer 2022-05-11 10:22 ` m.brock 2022-06-27 8:40 ` Matthias Schiffer 2022-06-29 17:01 ` Lukas Wunner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).