Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes:
> On Thu, May 30, 2019 at 06:29:45PM +0300, Sergey Organov wrote:
>> imx_set_mctrl() cleared RTS on every call
>
>
>>
>> Signed-off-by: Sergey Organov <sorganov@xxxxxxxxx>
>> ---
>> drivers/tty/serial/imx.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 6577552..13face9 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -1648,7 +1648,6 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
>>
>> /* then, disable everything */
>> imx_uart_writel(sport, old_ucr2 & ~(UCR2_TXEN | UCR2_RXEN | UCR2_ATEN), UCR2);
>> - old_ucr2 &= (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN);
>>
>> /* custom-baudrate handling */
>> div = sport->port.uartclk / (baud * 16);
>> @@ -1686,7 +1685,8 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
>>
>> imx_uart_writel(sport, old_ucr1, UCR1);
>>
>> - /* set the parity, stop bits and data size */
>> + /* Set parity, stop bits, data size, etc. Keep bits we don't compute. */
>> + old_ucr2 &= (UCR2_CTS | UCR2_TXEN | UCR2_RXEN | UCR2_ATEN);
>
> I wonder if that fixes a certain usecase and breaks another.
>
> If I change the baud rate of the UART the sequence I actually want to
> have is:
>
> clear RTS (to not encourage the other side to send data)
> disable receiver
> reconfigure requested settings
> reenable receiver
> (maybe) reactivate RTS to signal being ready again
>
> So I think your approach is too simple.
This patch in fact should be dropped, but for different reason:
It will set back UCR2_CTS that was cleared by imx_uart_rts_active(),
i.e., it'd break RS485 mode.
... provided it's not broken anyway as:
static void imx_uart_rts_active(struct imx_port *sport, u32 *ucr2)
{
*ucr2 &= ~(UCR2_CTSC | UCR2_CTS);
sport->port.mctrl |= TIOCM_RTS;
mctrl_gpio_set(sport->gpios, sport->port.mctrl);
}
Seems to have inverted logic between TIOCM_RTS and UCR2_CTS (once again
intentional? with no comments?), compared to set_mctrl():
if (mctrl & TIOCM_RTS)
ucr2 |= UCR2_CTS;
Moreover, imx_uart_rts_active() is called from set_termios() before
spin_lock_irqsave(&sport->port.lock, flags);
is called, yet it changes sport->port.mctrl. A bug?
-- Sergey
