Re: [PATCH v4 1/3] serial: imx: set_termios(): do not enable autoRTS if RTS is unset

[Sergey Organov <sorganov@gmail.com>]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

> On Fri, Jul 19, 2019 at 11:47:52AM +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.
>
> This is not understandable unless you read the reference manual.
>
> In terms understandable without manual, this patch does:
>
> 	Don't let the receiver hardware control the RTS output if it was
> 	requested to be inactive.

I'll fix it, thanks!

>
>> Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
>> Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
>> 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 57d6e6b..95d7984 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;
>
> I think this patch is wrong or the commit log is insufficient.
> imx_uart_rts_auto() has only a single caller and there ucr2 & UCR2_CTS is
> never true. And CTSC isn't restored anywhere, is it?

This is rebase to 'tty-next' branch, and you need to look at it in that
context. There, ucr2 & UCR2_CTS does already make sense, due to previous
fix that is already there.

Alternatively, look at v3 of the patches, but you basically already did.
It's that the first part of v3 patch series has been already accepted
upstream, and this is the rest of those patches.

Thanks!

-- Sergey

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