Hello,
On Thu, May 30, 2019 at 06:29:44PM +0300, Sergey Organov wrote:
> imx_set_mctrl() stop fiddling with UCR2_CTSC bit
>
> 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 e9e812a..6577552 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -967,9 +967,9 @@ static void imx_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> u32 ucr2;
>
> ucr2 = imx_uart_readl(sport, UCR2);
> - ucr2 &= ~(UCR2_CTS | UCR2_CTSC);
> + ucr2 &= ~UCR2_CTS;
> if (mctrl & TIOCM_RTS)
> - ucr2 |= UCR2_CTS | UCR2_CTSC;
> + ucr2 |= UCR2_CTS;
> imx_uart_writel(sport, ucr2, UCR2);
> }
I'm sure this patch is wrong. And your change log fails to point out
what you want to achieve.
Independant of your patch I discussed a problem in imx_uart_set_mctrl()
with Sascha and Russell (both added to Cc:) earlier this week. In the
current implementation there are actually two problems.
Currently imx_uart_set_mctrl does:
if TIOCM_RTS is set:
let the receiver control the RTS signal
else:
set RTS inactive
The bigger problem is that if the UART is configured not to use
handshaking (CRTSCTS unset) the mode "let the receiver control the RTS
signal" should not be used.
The smaller (and irrelevant for correctness) problem is that setting
UCR2_CTS is a no-op when UCR2_CTSC is also set.
We think the right thing to do is:
ucr2 = imx_uart_readl(sport, UCR2);
ucr2 &= ~(UCR2_CTS | UCR2_CTSC);
if (mctrl & TIOCM_RTS) {
if (sport->crtscts)
/* let the receiver control RTS */
ucr2 |= UCR2_CTSC;
else
/* Force RTS active */
ucr2 |= UCR2_CTS;
} else {
/* Force RTS inactive, i.e. CTS=0, CTSC=0 */
}
imx_uart_writel(sport, ucr2, UCR2);
but AFAICT this isn't tested yet to an end in the use case that Sascha
currently has and so there isn't a complete patch available yet.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
