All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] serial: Fix incorrect rs485 polarity on uart open
@ 2021-12-18  9:58 Lukas Wunner
  2021-12-20  6:28 ` Jiri Slaby
  2022-08-04 14:38 ` Roosen Henri
  0 siblings, 2 replies; 10+ messages in thread
From: Lukas Wunner @ 2021-12-18  9:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Su Bao Cheng, baocheng_su, Jan Kiszka, Chao Zeng, linux-serial,
	Lino Sanfilippo, Philipp Rosenberger, Rafael Gago Castano

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.

Fixes: a6845e1e1b78 ("serial: core: Consider rs485 settings to drive RTS")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v4.14+
Cc: Rafael Gago Castano <rgc@hms.se>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Su Bao Cheng <baocheng.su@siemens.com>
---
 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);
 	}
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial: Fix incorrect rs485 polarity on uart open
  2021-12-18  9:58 [PATCH] serial: Fix incorrect rs485 polarity on uart open Lukas Wunner
@ 2021-12-20  6:28 ` Jiri Slaby
  2021-12-20  6:30   ` Jiri Slaby
  2022-08-04 14:38 ` Roosen Henri
  1 sibling, 1 reply; 10+ messages in thread
From: Jiri Slaby @ 2021-12-20  6:28 UTC (permalink / raw)
  To: Lukas Wunner, Greg Kroah-Hartman
  Cc: Su Bao Cheng, baocheng_su, Jan Kiszka, Chao Zeng, linux-serial,
	Lino Sanfilippo, Philipp Rosenberger, Rafael Gago Castano

On 18. 12. 21, 10:58, 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.
> 
> Fixes: a6845e1e1b78 ("serial: core: Consider rs485 settings to drive RTS")
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: stable@vger.kernel.org # v4.14+
> Cc: Rafael Gago Castano <rgc@hms.se>

Rafael, can you ack/test this, please?

> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Su Bao Cheng <baocheng.su@siemens.com>
> ---
>   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);
>   	}
>   }


-- 
js
suse labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial: Fix incorrect rs485 polarity on uart open
  2021-12-20  6:28 ` Jiri Slaby
@ 2021-12-20  6:30   ` Jiri Slaby
  2021-12-27 13:17     ` Lukas Wunner
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Slaby @ 2021-12-20  6:30 UTC (permalink / raw)
  To: Lukas Wunner, Greg Kroah-Hartman
  Cc: Su Bao Cheng, baocheng_su, Jan Kiszka, Chao Zeng, linux-serial,
	Lino Sanfilippo, Philipp Rosenberger, rafael.gago

On 20. 12. 21, 7:28, Jiri Slaby wrote:
> On 18. 12. 21, 10:58, 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.
>>
>> Fixes: a6845e1e1b78 ("serial: core: Consider rs485 settings to drive 
>> RTS")
>> Signed-off-by: Lukas Wunner <lukas@wunner.de>
>> Cc: stable@vger.kernel.org # v4.14+
>> Cc: Rafael Gago Castano <rgc@hms.se>
> 
> Rafael, can you ack/test this, please?

Definitely on that e-mail:
  550 5.4.1 Recipient address rejected: Access denied. AS(201806281) 
[DB5EUR03FT039.eop-EUR03.prod.protection.outlook.com]

Trying rafael.gago@gmail.com from the Author field.

>> Cc: Jan Kiszka <jan.kiszka@siemens.com>
>> Cc: Su Bao Cheng <baocheng.su@siemens.com>
>> ---
>>   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);
>>       }
>>   }
> 
> 


-- 
js
suse labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial: Fix incorrect rs485 polarity on uart open
  2021-12-20  6:30   ` Jiri Slaby
@ 2021-12-27 13:17     ` Lukas Wunner
  0 siblings, 0 replies; 10+ messages in thread
From: Lukas Wunner @ 2021-12-27 13:17 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Greg Kroah-Hartman, Su Bao Cheng, baocheng_su, Jan Kiszka,
	Chao Zeng, linux-serial, Lino Sanfilippo, Philipp Rosenberger,
	rafael.gago, rafael_gago_81, rafael.gago, rafael.gago

On Mon, Dec 20, 2021 at 07:30:52AM +0100, Jiri Slaby wrote:
> On 20. 12. 21, 7:28, Jiri Slaby wrote:
> > On 18. 12. 21, 10:58, 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.
> > > 
> > > Fixes: a6845e1e1b78 ("serial: core: Consider rs485 settings to drive
> > > RTS")
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > Cc: stable@vger.kernel.org # v4.14+
> > > Cc: Rafael Gago Castano <rgc@hms.se>
> > 
> > Rafael, can you ack/test this, please?
> 
> Definitely on that e-mail:
>  550 5.4.1 Recipient address rejected: Access denied. AS(201806281)
> [DB5EUR03FT039.eop-EUR03.prod.protection.outlook.com]
> 
> Trying rafael.gago@gmail.com from the Author field.

A bit of GitHub sleuthing turned up the following alternative addresses:

rafael_gago_81@hotmail.com
rafael.gago@zenuity.com
rafael.gago@zenseact.com

Unfortunately none of them is responsive.  I was hoping that the Siemens
folks might be willing to attest correctness of the patch.

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial: Fix incorrect rs485 polarity on uart open
  2021-12-18  9:58 [PATCH] serial: Fix incorrect rs485 polarity on uart open Lukas Wunner
  2021-12-20  6:28 ` Jiri Slaby
@ 2022-08-04 14:38 ` Roosen Henri
  2022-08-04 15:52   ` Lukas Wunner
  1 sibling, 1 reply; 10+ messages in thread
From: Roosen Henri @ 2022-08-04 14:38 UTC (permalink / raw)
  To: jirislaby, lukas, gregkh
  Cc: chao.zeng, LinoSanfilippo, baocheng_su, p.rosenberger,
	baocheng.su, rgc, linux-serial, jan.kiszka

[-- Attachment #1: Type: text/plain, Size: 3931 bytes --]

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@wunner.de>
Cc: stable@vger.kernel.org # v4.14+
Cc: Rafael Gago Castano <rgc@hms.se>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Su Bao Cheng <baocheng.su@siemens.com>
---
 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 #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 7961 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial: Fix incorrect rs485 polarity on uart open
  2022-08-04 14:38 ` Roosen Henri
@ 2022-08-04 15:52   ` Lukas Wunner
  2022-08-04 16:11     ` Roosen Henri
  0 siblings, 1 reply; 10+ messages in thread
From: Lukas Wunner @ 2022-08-04 15:52 UTC (permalink / raw)
  To: Roosen Henri
  Cc: jirislaby, gregkh, chao.zeng, LinoSanfilippo, baocheng_su,
	p.rosenberger, baocheng.su, rgc, linux-serial, jan.kiszka

On Thu, Aug 04, 2022 at 02:38:23PM +0000, Roosen Henri wrote:
> unfortunately this commit, which has been backported to v5.4.x, seems
> to break RS485 functionality on our iMX boards.

What exactly broke?  Are you seeing incorrect polarity after opening
the tty but correct polarity after the first send?  Or is polarity
always incorrect?

There have been reports about incorrect polarity after open and
before the first send as a result of that committ, but they only
concern a subset of drivers using the "em485" software emulation
in 8250_port.c:

https://lore.kernel.org/linux-serial/20220329085050.311408-1-matthias.schiffer@ew.tq-group.com/

imx.c does not use the em485 software emulation but rather a
dedicated GPIO, and the way you've set it up looks correct to me.

Some transceivers have both a DE and a !DE pin.  Is the GPIO
connected to the former?

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial: Fix incorrect rs485 polarity on uart open
  2022-08-04 15:52   ` Lukas Wunner
@ 2022-08-04 16:11     ` Roosen Henri
  2022-08-05  8:18       ` Lukas Wunner
  0 siblings, 1 reply; 10+ messages in thread
From: Roosen Henri @ 2022-08-04 16:11 UTC (permalink / raw)
  To: lukas
  Cc: chao.zeng, LinoSanfilippo, baocheng_su, gregkh, p.rosenberger,
	baocheng.su, jirislaby, rgc, linux-serial, jan.kiszka

[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]

On Thu, 2022-08-04 at 17:52 +0200, Lukas Wunner wrote:
> On Thu, Aug 04, 2022 at 02:38:23PM +0000, Roosen Henri wrote:
> > unfortunately this commit, which has been backported to v5.4.x,
> > seems
> > to break RS485 functionality on our iMX boards.
> 
> What exactly broke?  Are you seeing incorrect polarity after opening
> the tty but correct polarity after the first send?  Or is polarity
> always incorrect?

I'm not sure about the state before opening, I have to measure that
when I'm back at the office.

After opening, the polarity is always incorrect (inverted) when this
patch is applied. We open the tty and try to receive, which now fails
because the rs485 transmitter is on.

Before the patch, the transmitter was only on during sending (before
and after opening). This is the way it should be, because a system
should only 'be on the bus' during transmit. In all other cases, like
for instance at startup (closed tty), the transmitter should be off, so
other devices can take the bus to send.

> 
> There have been reports about incorrect polarity after open and
> before the first send as a result of that committ, but they only
> concern a subset of drivers using the "em485" software emulation
> in 8250_port.c:
> 
>  
> https://lore.kernel.org/linux-serial/20220329085050.311408-1-matthias.schiffer@ew.tq-group.com/
> 
> imx.c does not use the em485 software emulation but rather a
> dedicated GPIO, and the way you've set it up looks correct to me.
> 
> Some transceivers have both a DE and a !DE pin.  Is the GPIO
> connected to the former?

The board design uses the GPIO directly to the DE and also directly to
the !RE (see SN65HVD82 datasheet).

> 
> Thanks,
> 
> Lukas


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 7961 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial: Fix incorrect rs485 polarity on uart open
  2022-08-04 16:11     ` Roosen Henri
@ 2022-08-05  8:18       ` Lukas Wunner
       [not found]         ` <33b4b797-cd4a-436e-8e03-4bc5d7dd69ff.d000f9d8-ed3e-456c-b23a-5a86eac57608.c023ef82-f7f7-4a43-92f1-8642a4f822e1@emailsignatures365.codetwo.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Lukas Wunner @ 2022-08-05  8:18 UTC (permalink / raw)
  To: Roosen Henri
  Cc: chao.zeng, LinoSanfilippo, baocheng_su, gregkh, p.rosenberger,
	baocheng.su, jirislaby, rgc, linux-serial, jan.kiszka

On Thu, Aug 04, 2022 at 04:11:26PM +0000, Roosen Henri wrote:
> On Thu, 2022-08-04 at 17:52 +0200, Lukas Wunner wrote:
> > On Thu, Aug 04, 2022 at 02:38:23PM +0000, Roosen Henri wrote:
> > > unfortunately this commit, which has been backported to v5.4.x,
> > > seems
> > > to break RS485 functionality on our iMX boards.
> > 
> > What exactly broke?  Are you seeing incorrect polarity after opening
> > the tty but correct polarity after the first send? Or is polarity
> > always incorrect?
> 
> I'm not sure about the state before opening, I have to measure that
> when I'm back at the office.
> 
> After opening, the polarity is always incorrect (inverted) when this
> patch is applied. We open the tty and try to receive, which now fails
> because the rs485 transmitter is on.

Does reception work after you send something?

Please provide the contents of the pinctrl_uart2_rs485 node in your DT.

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial: Fix incorrect rs485 polarity on uart open
       [not found]           ` <33b4b797-cd4a-436e-8e03-4bc5d7dd69ff.0668d69a-b7f6-4fc6-94cd-e3b904d4ce63.488ca449-4f1c-444b-8de0-1344d3aeb879@emailsignatures365.codetwo.com>
@ 2022-08-05 11:20             ` Roosen Henri
  2022-08-11  6:18               ` Lukas Wunner
  0 siblings, 1 reply; 10+ messages in thread
From: Roosen Henri @ 2022-08-05 11:20 UTC (permalink / raw)
  To: lukas
  Cc: chao.zeng, LinoSanfilippo, baocheng_su, gregkh, p.rosenberger,
	baocheng.su, jirislaby, rgc, linux-serial, jan.kiszka

[-- Attachment #1: Type: text/plain, Size: 3049 bytes --]


-- 

Henri Roosen | Entwicklung Software

GINZINGER ELECTRONIC SYSTEMS GMBH

Tel.: +43 7723 5422 161

Mail: Henri.Roosen@ginzinger.com
Web: www.ginzinger.com

On Fri, 2022-08-05 at 10:18 +0200, Lukas Wunner wrote:
> On Thu, Aug 04, 2022 at 04:11:26PM +0000, Roosen Henri wrote:
> > On Thu, 2022-08-04 at 17:52 +0200, Lukas Wunner wrote:
> > > On Thu, Aug 04, 2022 at 02:38:23PM +0000, Roosen Henri wrote:
> > > > unfortunately this commit, which has been backported to v5.4.x,
> > > > seems
> > > > to break RS485 functionality on our iMX boards.
> > > 
> > > What exactly broke?  Are you seeing incorrect polarity after
> > > opening
> > > the tty but correct polarity after the first send? Or is polarity
> > > always incorrect?
> > 
> > I'm not sure about the state before opening, I have to measure that
> > when I'm back at the office.
> > 
> > After opening, the polarity is always incorrect (inverted) when
> > this
> > patch is applied. We open the tty and try to receive, which now
> > fails
> > because the rs485 transmitter is on.
> 
> Does reception work after you send something?

No, it does not. This is because RTS is high during reception.

I'll provide you some oscilloscope screenshots off-list, but in words,
with this patch:
- the RTS is low at boot
- there is a small glitch (high-low pulse) when configuring the speed
  using 'stty -F /dev/ttymxc2 230400 raw -echo'
- RTS goes high when receiving, using 'cat /dev/ttymxc2'
- after stopping the cat command, RTS stays high
- RTS goes low for a few microseconds then high, just before
  transmitting the first data using 'echo hello > /dev/ttymxc2'
- RTS stays high while sending data
- RTS goes low after sending data, and stays low

Without this patch, RTS is only high during data transmission.

> 
> Please provide the contents of the pinctrl_uart2_rs485 node in your
> DT.

#define UART_PAD_CTRL (                 \
		PAD_CTL_ENABLED |       \
		PAD_CTL_SION_OFF |      \
		PAD_CTL_HYS_ON |        \
		PAD_CTL_PUS_100K_UP |   \
		PAD_CTL_PUE_ON |        \
		PAD_CTL_PKE_ON |        \
		PAD_CTL_ODE_OFF |       \
		PAD_CTL_SPEED_MED |     \
		PAD_CTL_DSE_40ohm |     \
		PAD_CTL_SRE_FAST)

pinctrl_uart2_rs485: uart2rs485grp {
	fsl,pins = <
		MX6UL_PAD_UART2_RX_DATA__UART2_DCE_RX   UART_PAD_CTRL
		MX6UL_PAD_UART2_TX_DATA__UART2_DCE_TX   UART_PAD_CTRL
		MX6UL_PAD_UART2_RTS_B__GPIO1_IO23       UART_PAD_CTRL
	>;
};

> 
> Thanks,
> 
> Lukas



Ginzinger electronic systems GmbH
Gewerbepark 3
4950 Altheim

www.ginzinger.com <https://www.ginzinger.com/>
Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis

UID-Nr.: ATU66521089

Diese Nachricht ist vertraulich und darf nicht an andere Personen weitergegeben oder von diesen verwendet werden. Verständigen Sie uns, wenn Sie irrtümlich eine Mitteilung empfangen haben.
This message is confidential. It may not be disclosed to, or used by, anyone other than the addressee. If you receive this message by mistake, please advise the sender.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 7961 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial: Fix incorrect rs485 polarity on uart open
  2022-08-05 11:20             ` Roosen Henri
@ 2022-08-11  6:18               ` Lukas Wunner
  0 siblings, 0 replies; 10+ messages in thread
From: Lukas Wunner @ 2022-08-11  6:18 UTC (permalink / raw)
  To: Roosen Henri
  Cc: chao.zeng, LinoSanfilippo, baocheng_su, gregkh, p.rosenberger,
	baocheng.su, jirislaby, rgc, linux-serial, jan.kiszka

On Fri, Aug 05, 2022 at 11:20:24AM +0000, Roosen Henri wrote:
> On Fri, 2022-08-05 at 10:18 +0200, Lukas Wunner wrote:
> > On Thu, Aug 04, 2022 at 04:11:26PM +0000, Roosen Henri wrote:
> > > On Thu, 2022-08-04 at 17:52 +0200, Lukas Wunner wrote:
> > > > On Thu, Aug 04, 2022 at 02:38:23PM +0000, Roosen Henri wrote:
> > > > > unfortunately this commit, which has been backported to v5.4.x,
> > > > > seems
> > > > > to break RS485 functionality on our iMX boards.

Thanks for the report and sorry for the breakage.

I see what the problem is.  The serial core assumes that RTS in mctrl
has inverted semantics and that doesn't hold for mctrl_gpio.

I guess the takeaway is that deasserting RS485 Transmit Enable is
really driver-specific.  In particular, imx.c has support for using
CTS to drive Transmit Enable and the serial core can't deassert that
on probe because it doesn't know about this driver-specific feature.

In the case of imx.c, the driver already deasserts Transmit Enable via:
imx_uart_probe()
  uart_rs485_config()
    imx_uart_rs485_config()

(Those function names refer to current mainline, uart_rs485_config()
will be newly introduced in v6.0.)

Thus, just deleting deassertion from uart_configure_port() should fix
the issue for imx.c:

-		if (port->rs485.flags & SER_RS485_ENABLED &&
-		    !(port->rs485.flags & SER_RS485_RTS_AFTER_SEND))
-			port->mctrl |= TIOCM_RTS;

I need to go through all other rs485-capable drivers to check whether
the same is true for them or which ones need to be amended.  Please
stand by.

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-08-11  6:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-18  9:58 [PATCH] serial: Fix incorrect rs485 polarity on uart open Lukas Wunner
2021-12-20  6:28 ` Jiri Slaby
2021-12-20  6:30   ` Jiri Slaby
2021-12-27 13:17     ` Lukas Wunner
2022-08-04 14:38 ` Roosen Henri
2022-08-04 15:52   ` Lukas Wunner
2022-08-04 16:11     ` Roosen Henri
2022-08-05  8:18       ` Lukas Wunner
     [not found]         ` <33b4b797-cd4a-436e-8e03-4bc5d7dd69ff.d000f9d8-ed3e-456c-b23a-5a86eac57608.c023ef82-f7f7-4a43-92f1-8642a4f822e1@emailsignatures365.codetwo.com>
     [not found]           ` <33b4b797-cd4a-436e-8e03-4bc5d7dd69ff.0668d69a-b7f6-4fc6-94cd-e3b904d4ce63.488ca449-4f1c-444b-8de0-1344d3aeb879@emailsignatures365.codetwo.com>
2022-08-05 11:20             ` Roosen Henri
2022-08-11  6:18               ` Lukas Wunner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.