On 24.08.2022 15:02, Ilpo Järvinen wrote:
> On Wed, 24 Aug 2022, Sergiu Moga wrote:
>
>> Whenever the atmel_rs485_config driver method would be called,
>> the USART mode is reset to normal mode before even checking if
>> RS485 flag is set, thus resulting in losing the previous USART
>> mode in the case where the checking fails. Some tools, such as
>> `linux-serial-test`, lead to the driver calling this method
>> when doing the setup of the serial port: after setting the port
>> mode (Hardware Flow Control, Normal Mode, RS485 Mode, etc.),
>> `linux-serial-test` tries to enable/disable RS485 depending on
>> the commandline arguments passed. If we were to, for example, enable
>> Hardware Flow Control through `linux-serial-test`, the tool would
>> make the driver set the corresponding bit to 1 (ATMEL_US_USMODE_HWHS
>> bit in the ATMEL_US_MR register) through the atmel_set_termios method
>> and then proceed to disabling RS485. This, in turn, causes the
>> ATMEL_US_USMODE_HWHS bit of the ATMEL_US_MR mode register to be unset
>> and, if the checking for RS485 fails, leads to having the mode set
>> back to the ATMEL_US_USMODE_NORMAL normal mode. Since in hardware
>> flow control mode the meanings of the ATMEL_US_RTSDIS and
>> ATMEL_US_RTSEN bits are swapped, this leads to our endpoint leaving
>> the RTS line to high when wanting to receive, which is the opposite
>> of what the other endpoint is expecting in order to start transmitting.
>> This fix ensures that this reset is done only if the checking for RS485
>> succeeds.
> Could you please try to split this long paragraph to a slightly shorter
> bits such that it would be easier to read.
Sure, will do :).
>> Fixes: e8faff7330a35 ("ARM: 6092/1: atmel_serial: support for RS485 communications")
>> Signed-off-by: Sergiu Moga <sergiu.moga@xxxxxxxxxxxxx>
>> ---
>> drivers/tty/serial/atmel_serial.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index 0a0b46ee0955..c29b1fb48694 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -298,9 +298,6 @@ static int atmel_config_rs485(struct uart_port *port, struct ktermios *termios,
>>
>> mode = atmel_uart_readl(port, ATMEL_US_MR);
>>
>> - /* Resetting serial mode to RS232 (0x0) */
>> - mode &= ~ATMEL_US_USMODE;
>> -
>> if (rs485conf->flags & SER_RS485_ENABLED) {
>> dev_dbg(port->dev, "Setting UART to RS485\n");
>> if (rs485conf->flags & SER_RS485_RX_DURING_TX)
>> @@ -310,6 +307,7 @@ static int atmel_config_rs485(struct uart_port *port, struct ktermios *termios,
>>
>> atmel_uart_writel(port, ATMEL_US_TTGR,
>> rs485conf->delay_rts_after_send);
>> + mode &= ~ATMEL_US_USMODE;
>> mode |= ATMEL_US_USMODE_RS485;
>> } else {
>> dev_dbg(port->dev, "Setting UART to RS232\n");
>>
> Makes sense.
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
>
> Unrelated to this patch but I came across it while reviewing yours... Do
> you BTW have any idea why atmel_serial_probe() sets ATMEL_US_USMODE_NORMAL
> inside rs485_enabled block? I'd have expected it wanted to do
> ATMEL_US_USMODE_RS485 there too like is done in atmel_config_rs485().
>
A quick git blame in an older version of the driver shows this commit:
5dfbd1d734ef5415bc47b034df7433ba21e40e7b
with the following commit message:
```
atmel_serial: fix RTS high after initialization in RS485 mode
When working in RS485 mode, the atmel_serial driver keeps RTS high after
the initialization of the serial port. It goes low only after the first
character has been sent.
```
If I am to remove the line in question (delete it entirely) and do a
simple test, the serial interface seems to continue to work fine in
RS485 mode. This is for some of the newer IPs, I am not so sure about
the older ones though, so it is probably a good idea to not risk it.
> --
> i.
Regards,
Sergiu
