Re: [PATCH] serial: imx: fix TX stop function not setting state to OFF in RS232-mode

[Robert Hodaszi <robert.hodaszi@digi.com>]

On 2021. 04. 10. 10:33, Greg Kroah-Hartman wrote:
> [EXTERNAL EMAIL]
>
> On Fri, Apr 02, 2021 at 01:05:48PM +0200, Robert Hodaszi wrote:
>> From: Robert Hodaszi <robert.hodaszi@digi.com>
>>
>> If the mode had been changed to RS-485 after at least one character had
>> been sent in RS-232 mode, the RS-485 transmission was not working.
>>
>> Commit cb1a609236096c278ecbfb7be678a693a70283f1 ("serial: imx: implement
>> rts delaying for rs485") added a TX state variable to keep track, when
>> it needs to enable / disable the RS-485 transmitter.
>>
>> In RS-232 mode, the start TX function just sets the state to SEND, and
>> the stop function supposed to set it to OFF.
>>
>> In RS-485 mode, the start TX function expects the state to be either
>> OFF, WAIT_AFTER_SEND, or WAIT_AFTER RTS. It cannot do anything if the
>> state is set to SEND, expects a stop first.
>>
>> But stop TX function in RS-232 mode usually didn't set the state to OFF,
>> as it first checked if the shifter is empty, and if not, it just
>> returned, waiting for a TransmitComplete interrupt, which is only
>> enabled in RS-485 mode. So the stop function was never called again.
>>
>> That check, and the subsequent code part is not needed for RS-232, it
>> just have to set the TX state to OFF.
>>
>> Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com>
>> ---
>>  drivers/tty/serial/imx.c | 46 +++++++++++++++++++++-------------------
>>  1 file changed, 24 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 8257597d034d..511badce3edd 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -443,6 +443,12 @@ static void imx_uart_stop_tx(struct uart_port *port)
>>      ucr1 = imx_uart_readl(sport, UCR1);
>>      imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1);
>>
>> +    if (!(port->rs485.flags & SER_RS485_ENABLED)) {
>> +        /* Set the TX state in non-RS485 mode, nothing else to do */
>> +        sport->tx_state = OFF;
>> +        return;
>> +    }
>> +
>>      usr2 = imx_uart_readl(sport, USR2);
>>      if (!(usr2 & USR2_TXDC)) {
>>          /* The shifter is still busy, so retry once TC triggers */
>> @@ -453,33 +459,29 @@ static void imx_uart_stop_tx(struct uart_port *port)
>>      ucr4 &= ~UCR4_TCEN;
>>      imx_uart_writel(sport, ucr4, UCR4);
>>
>> -    /* in rs485 mode disable transmitter */
>> -    if (port->rs485.flags & SER_RS485_ENABLED) {
>> -        if (sport->tx_state == SEND) {
>> -            sport->tx_state = WAIT_AFTER_SEND;
>> -            start_hrtimer_ms(&sport->trigger_stop_tx,
>> -                     port->rs485.delay_rts_after_send);
>> -            return;
>> -        }
>> +    if (sport->tx_state == SEND) {
>> +        sport->tx_state = WAIT_AFTER_SEND;
>> +        start_hrtimer_ms(&sport->trigger_stop_tx,
>> +                    port->rs485.delay_rts_after_send);
>> +        return;
>> +    }
>>
>> -        if (sport->tx_state == WAIT_AFTER_RTS ||
>> -            sport->tx_state == WAIT_AFTER_SEND) {
>> -            u32 ucr2;
>> +    if (sport->tx_state == WAIT_AFTER_RTS ||
>> +        sport->tx_state == WAIT_AFTER_SEND) {
>> +        /* in rs485 mode disable transmitter */
>> +        u32 ucr2;
>>
>> - hrtimer_try_to_cancel(&sport->trigger_start_tx);
>> +        hrtimer_try_to_cancel(&sport->trigger_start_tx);
>>
>> -            ucr2 = imx_uart_readl(sport, UCR2);
>> -            if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
>> -                imx_uart_rts_active(sport, &ucr2);
>> -            else
>> -                imx_uart_rts_inactive(sport, &ucr2);
>> -            imx_uart_writel(sport, ucr2, UCR2);
>> +        ucr2 = imx_uart_readl(sport, UCR2);
>> +        if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
>> +            imx_uart_rts_active(sport, &ucr2);
>> +        else
>> +            imx_uart_rts_inactive(sport, &ucr2);
>> +        imx_uart_writel(sport, ucr2, UCR2);
>>
>> -            imx_uart_start_rx(port);
>> +        imx_uart_start_rx(port);
>>
>> -            sport->tx_state = OFF;
>> -        }
>> -    } else {
>>          sport->tx_state = OFF;
>>      }
>>  }
>> --
>> 2.27.0
> Your patch is corrupted, all the tabs are turned to spaces and it can
> not be applied :(
>
> Please fix up and resend.
>
> thanks,
>
> greg k-h

Sorry, for that (and for the slow reply)! My latest Thunderbird setup
seems not playing very. Went back to my older setup, and sent out a v2.
Hopefully, that will be OK.

Thanks,
Robert Hodaszi