Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes:
[...]
> 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.
This looks almost correct, except there is no sport->crtscts. Are you
going to add it? Other drivers use port->status & UPSTAT_AUTORTS for
that, and maybe it'd better to follow, except it seems you then need to
manage UPSTAT_AUTORTS yourself, no help from top-level :-(
However, I figure that it could rather be calculated right here as:
!(ucr2 & UCR2_IRTS)
as autoCTS is set if and only if autoRTS is also set, and UCR2_IRTS
otherwise remains static.
... and then set_termios() should still be fixed, as it clears UCR2_CTS
bit on every invocation, but differently from my original patch (see
separate mail on that patch).
I'll try to come-up with new separate patches for this particular issue
(I mean RTS/CTS handshake handling).
-- Sergey
