Re: [PATCH RFC 4/7] serial: imx: set_termios(): do not enable autoRTS if RTS is unset

[Sergey Organov <sorganov@gmail.com>]

Hi Sasha,

Sascha Hauer <s.hauer@pengutronix.de> writes:

> Hi Sergey,
>
> On Fri, Jun 14, 2019 at 03:11:31PM +0300, Sergey Organov wrote:
>> set_termios() shouldn't set UCR2_CTSC bit if UCR2_CTS (=TIOCM_RTS) is
>> cleared. Added corresponding check in imx_uart_rts_auto() to fix this.
>> 
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  drivers/tty/serial/imx.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 17e2322..8ee910f 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -405,7 +405,8 @@ static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
>>  /* called with port.lock taken and irqs caller dependent */
>>  static void imx_uart_rts_auto(struct imx_port *sport, u32 *ucr2)
>>  {
>> -	*ucr2 |= UCR2_CTSC;
>> +	if (*ucr2 & UCR2_CTS)
>> +		*ucr2 |= UCR2_CTSC;
>>  }
>
> *ucr2 is set like this in imx_uart_set_termios():
>
> 	ucr2 = UCR2_SRST | UCR2_IRTS;
> 	if ((termios->c_cflag & CSIZE) == CS8)
> 		ucr2 |= UCR2_WS;
> 	...
> 	imx_uart_rts_auto(sport, &ucr2);
>
> So the UCR2_CTS bit is never set, hence UCR2_CTSC will never be set.
> You meant to pass in the actual register value of the UCR2 register.
>
> This is shifted around a bit in the following patches, as an end result
> we have:
>
> 	old_ucr2 = imx_uart_readl(sport, UCR2);
> 	ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN | UCR2_CTSC);

This is rather the typo problem in my patches right here: it should have
been:

> 	ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN | UCR2_CTS);

as we need to preserve RTS bit state USR2_CTS, not hardware handshake bit
UCR2_CCTS.

> 	...
> 	if (ucr2 & UCR2_CTS)
> 		ucr2 |= UCR2_CTSC;
>
> Again the test can never be true, it should probably be if (old_ucr2 &
> UCR2_CTS).

No, I believe it's different mistake on my part, see above.

>
> With this issue and the one Lothar has found fixed this series works for
> me.
>
> With these issues fixed I'd be happy to test this series and apply it in
> favour of my patch.

Thanks a lot for reviewing and volunteering to test! It's even more
appreciated as I can't easily test either on recent kernels and/or
without heavy patching of the kernel, and patching would diminish
applicability of my test results to mainstream kernel.

I think I'll better re-roll the series with these 2 corrections, right?

-- Sergey.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel