tty: atmel_serial: use the correct RTS flag.

PATCH In review Mathieu Othacehe 4 participants 5 messages Aug 8, 2024 – Aug 22, 2024

Subsystems (3)

TTY layer and serial drivers Arm/Microchip (AT91) SoC support Microchip AT91 serial driver

lore

Share & tools

lore

Mathieu Othacehe

author

2024-08-08 06:06 UTC #0 lore raw

PATCH 1/1 tty: atmel_serial: use the correct RTS flag.

In RS485 mode, the RTS pin is driven high by hardware when the transmitter is operating. This behaviour cannot be changed. This means that the driver should claim that it supports SER_RS485_RTS_ON_SEND and not SER_RS485_RTS_AFTER_SEND.

Otherwise, when configuring the port with the SER_RS485_RTS_ON_SEND, one get the following warning:

kern.warning kernel: atmel_usart_serial atmel_usart_serial.2.auto:
ttyS1 (1): invalid RTS setting, using RTS_AFTER_SEND instead

which is contradictory with what's really happening.

Signed-off-by Mathieu Othacehe <[email protected]>

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

drivers/tty/serial/atmel_serial.c +1 -1

index 0a90964d6d107..09b246c9e389e 100644
@@ -2514,7 +2514,7 @@ static const struct uart_ops atmel_pops = {
 };
 
 static const struct serial_rs485 atmel_rs485_supported = {
-	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX,
+	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
 	.delay_rts_after_send = 1,
 };

Signature · 1 line

2.45.2

Richard GENOUD

maintainer

2024-08-09 16:46 UTC #1 lore raw

Hi,

Adding Ilpo to the list since he was the author.

Le 08/08/2024 à 08:06, Mathieu Othacehe a écrit :

Quoted 23 lines

In RS485 mode, the RTS pin is driven high by hardware when the transmitter is operating. This behaviour cannot be changed. This means that the driver should claim that it supports SER_RS485_RTS_ON_SEND and not SER_RS485_RTS_AFTER_SEND.
Otherwise, when configuring the port with the SER_RS485_RTS_ON_SEND, one get the following warning:
kern.warning kernel: atmel_usart_serial atmel_usart_serial.2.auto:
ttyS1 (1): invalid RTS setting, using RTS_AFTER_SEND instead
which is contradictory with what's really happening.
Signed-off-by Mathieu Othacehe <[email protected]>
---
  drivers/tty/serial/atmel_serial.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/atmel_serial.c +0 -0
index 0a90964d6d107..09b246c9e389e 100644

drivers/tty/serial/atmel_serial.c static const struct uart_ops atmel_pops = { · line 2520

Quoted 8 lines

drivers/tty/serial/atmel_serial.c +1 -1

@@ -2514,7 +2514,7 @@ static const struct uart_ops atmel_pops = {
  };
 
  static const struct serial_rs485 atmel_rs485_supported = {
-	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX,
+	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RX_DURING_TX,
  	.delay_rts_before_send = 1,
  	.delay_rts_after_send = 1,
  };

Regards,
Ricahrd

Alexander Dahl

Tested-by on v1 1/1

2024-08-13 08:01 UTC #2 lore raw

In reply to Mathieu Othacehe

Hello Mathieu,

Am Thu, Aug 08, 2024 at 08:06:37AM +0200 schrieb Mathieu Othacehe:

Quoted 10 lines

In RS485 mode, the RTS pin is driven high by hardware when the transmitter is operating. This behaviour cannot be changed. This means that the driver should claim that it supports SER_RS485_RTS_ON_SEND and not SER_RS485_RTS_AFTER_SEND.
Otherwise, when configuring the port with the SER_RS485_RTS_ON_SEND, one get the following warning:
kern.warning kernel: atmel_usart_serial atmel_usart_serial.2.auto:
ttyS1 (1): invalid RTS setting, using RTS_AFTER_SEND instead

I've seen this warning already, when migrating a sam9x60 based board from LTS kernel 6.1 to 6.6, so thanks for taking care of this.

I can confirm after applying the patch on top of 6.6.44 the warning is gone, and RS-485 communication still works on our platform, so …

Tested-by Alexander Dahl <[email protected]>

Does this deserve a Fixes tag for the change which introduced struct serial_rs485 to the atmel serial driver? Then it should be this:

Fixes af47c491e3c7 ("serial: atmel: Fill in rs485_supported")

Greets
Alex

Quoted 24 lines

which is contradictory with what's really happening.

Signed-off-by Mathieu Othacehe <[email protected]>

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

drivers/tty/serial/atmel_serial.c +1 -1

index 0a90964d6d107..09b246c9e389e 100644
@@ -2514,7 +2514,7 @@ static const struct uart_ops atmel_pops = {
 };
 
 static const struct serial_rs485 atmel_rs485_supported = {
-	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX,
+	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
 	.delay_rts_after_send = 1,
 };

Signature · 1 line

2.45.2

Ilpo Järvinen

2024-08-13 08:17 UTC #3 lore raw

Quoted 22 lines · Alexander Dahl

Hello Mathieu,
Am Thu, Aug 08, 2024 at 08:06:37AM +0200 schrieb Mathieu Othacehe:
Quoted 10 lines
In RS485 mode, the RTS pin is driven high by hardware when the transmitter is operating. This behaviour cannot be changed. This means that the driver should claim that it supports SER_RS485_RTS_ON_SEND and not SER_RS485_RTS_AFTER_SEND.
Otherwise, when configuring the port with the SER_RS485_RTS_ON_SEND, one get the following warning:
kern.warning kernel: atmel_usart_serial atmel_usart_serial.2.auto:
ttyS1 (1): invalid RTS setting, using RTS_AFTER_SEND instead
I've seen this warning already, when migrating a sam9x60 based board from LTS kernel 6.1 to 6.6, so thanks for taking care of this.
I can confirm after applying the patch on top of 6.6.44 the warning is gone, and RS-485 communication still works on our platform, so …
Tested-by Alexander Dahl <[email protected]>

Quoted 11 lines

Does this deserve a Fixes tag for the change which introduced struct serial_rs485 to the atmel serial driver? Then it should be this:
Fixes af47c491e3c7 ("serial: atmel: Fill in rs485_supported")
Greets
Alex
Quoted 3 lines
which is contradictory with what's really happening.
Signed-off-by Mathieu Othacehe <[email protected]>

This got (more) broken by one of the changes made into uart_sanitize_serial_rs485() even if af47c491e3c7 used a wrong flag and must be fixed. The code in af47c491e3c7:uart_sanitize_serial_rs485() picks RTS_ON_SEND if no RTS_*_SEND flag is specified by the userspace code.

The behavior change was due to 4afeced55baa ("serial: core: fix sanitizing check for RTS settings") that enforced flags differently (4afeced55baa makes very much sense but because of the wrong flag in atmel driver, it ended up breaking atmel).

Signature · 1 line

i.

Quoted 22 lines

Quoted 21 lines

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

drivers/tty/serial/atmel_serial.c +1 -1

index 0a90964d6d107..09b246c9e389e 100644
@@ -2514,7 +2514,7 @@ static const struct uart_ops atmel_pops = {
 };
 
 static const struct serial_rs485 atmel_rs485_supported = {
-	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX,
+	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
 	.delay_rts_after_send = 1,
 };

Signature · 1 line

2.45.2

Richard GENOUD

maintainer Acked-by

2024-08-22 14:06 UTC #4 lore raw

In reply to Alexander Dahl

Le 13/08/2024 à 09:59, Alexander Dahl a écrit :

Quoted 14 lines

Hello Mathieu,
Am Thu, Aug 08, 2024 at 08:06:37AM +0200 schrieb Mathieu Othacehe:
Quoted 10 lines
In RS485 mode, the RTS pin is driven high by hardware when the transmitter is operating. This behaviour cannot be changed. This means that the driver should claim that it supports SER_RS485_RTS_ON_SEND and not SER_RS485_RTS_AFTER_SEND.
Otherwise, when configuring the port with the SER_RS485_RTS_ON_SEND, one get the following warning:
kern.warning kernel: atmel_usart_serial atmel_usart_serial.2.auto:
ttyS1 (1): invalid RTS setting, using RTS_AFTER_SEND instead

Quoted 7 lines

I've seen this warning already, when migrating a sam9x60 based board from LTS kernel 6.1 to 6.6, so thanks for taking care of this.
I can confirm after applying the patch on top of 6.6.44 the warning is gone, and RS-485 communication still works on our platform, so …
Tested-by Alexander Dahl <[email protected]>

Acked-by Richard Genoud <[email protected]>

Quoted 23 lines

Does this deserve a Fixes tag for the change which introduced struct serial_rs485 to the atmel serial driver? Then it should be this:
Fixes af47c491e3c7 ("serial: atmel: Fill in rs485_supported")
Greets
Alex
Quoted 14 lines
which is contradictory with what's really happening.
Signed-off-by Mathieu Othacehe <[email protected]>
---
  drivers/tty/serial/atmel_serial.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/atmel_serial.c +0 -0
index 0a90964d6d107..09b246c9e389e 100644
@@ -2514,7 +2514,7 @@ static const struct uart_ops atmel_pops = {
  };
 

Quoted 11 lines

Quoted 10 lines

drivers/tty/serial/atmel_serial.c +1 -1

  static const struct serial_rs485 atmel_rs485_supported = {
-	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX,
+	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RX_DURING_TX,
  	.delay_rts_before_send = 1,
  	.delay_rts_after_send = 1,
  };

Signature · 1 line

2.45.2

Thanks !