[PATCH] tty: atmel_serial: use the correct RTS flag.

[PATCH] tty: atmel_serial: use the correct RTS flag.

[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

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

Re: [PATCH] tty: atmel_serial: use the correct RTS flag.

[Richard GENOUD]

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

Re: [PATCH] tty: atmel_serial: use the correct RTS flag.

[Alexander Dahl]

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
> 
> 

Re: [PATCH] tty: atmel_serial: use the correct RTS flag.

[Ilpo Järvinen]

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

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).

-- 
 i.

> > ---
> >  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
> > 
> > 
> 

Re: [PATCH] tty: atmel_serial: use the correct RTS flag.

[Richard GENOUD]

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 !