diff mbox series

tty: atmel_serial: use the correct RTS flag.

Message ID 20240808060637.19886-1-othacehe@gnu.org (mailing list archive)
State New, archived
Headers show
Series tty: atmel_serial: use the correct RTS flag. | expand

Commit Message

Mathieu Othacehe Aug. 8, 2024, 6:06 a.m. UTC
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 <othacehe@gnu.org>
---
 drivers/tty/serial/atmel_serial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard GENOUD Aug. 9, 2024, 4:45 p.m. UTC | #1
Hi,

Adding Ilpo to the list since he was the author.

Le 08/08/2024 à 08:06, Mathieu Othacehe a écrit :
> 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 <othacehe@gnu.org>
> ---
>   drivers/tty/serial/atmel_serial.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 0a90964d6d107..09b246c9e389e 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -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 Aug. 13, 2024, 7:59 a.m. UTC | #2
Hello Mathieu,

Am Thu, Aug 08, 2024 at 08:06:37AM +0200 schrieb Mathieu Othacehe:
> 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 <ada@thorsis.com>

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

> which is contradictory with what's really happening.
> 
> Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>
> ---
>  drivers/tty/serial/atmel_serial.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 0a90964d6d107..09b246c9e389e 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -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,
>  };
> -- 
> 2.45.2
> 
>
Ilpo Järvinen Aug. 13, 2024, 8:17 a.m. UTC | #3
On Tue, 13 Aug 2024, Alexander Dahl wrote:

> Hello Mathieu,
> 
> Am Thu, Aug 08, 2024 at 08:06:37AM +0200 schrieb Mathieu Othacehe:
> > 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 <ada@thorsis.com>
> 
> 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
> 
> > which is contradictory with what's really happening.
> > 
> > Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>

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).
Richard GENOUD Aug. 22, 2024, 2:06 p.m. UTC | #4
Le 13/08/2024 à 09:59, Alexander Dahl a écrit :
> Hello Mathieu,
> 
> Am Thu, Aug 08, 2024 at 08:06:37AM +0200 schrieb Mathieu Othacehe:
>> 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 <ada@thorsis.com>

Acked-by: Richard Genoud <richard.genoud@bootlin.com>

> 
> 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
> 
>> which is contradictory with what's really happening.
>>
>> Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>
>> ---
>>   drivers/tty/serial/atmel_serial.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index 0a90964d6d107..09b246c9e389e 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -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,
>>   };
>> -- 
>> 2.45.2
>>
>>
> 

Thanks !
diff mbox series

Patch

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 0a90964d6d107..09b246c9e389e 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -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,
 };