On Wed, Oct 02, 2024 at 06:11:16AM +0200, Marek Vasut wrote:
> When sending data using DMA at high baudrate (4 Mbdps in local test case) to
> a device with small RX buffer which keeps asserting RTS after every received
> byte, it is possible that the iMX UART driver would not recognize the falling
> edge of RTS input signal and get stuck, unable to transmit any more data.
>
> This condition happens when the following sequence of events occur:
> - imx_uart_mctrl_check() is called at some point and takes a snapshot of UART
> control signal status into sport->old_status using imx_uart_get_hwmctrl().
> The RTSS/TIOCM_CTS bit is of interest here (*).
> - DMA transfer occurs, the remote device asserts RTS signal after each byte.
> The i.MX UART driver recognizes each such RTS signal change, raises an
> interrupt with USR1 register RTSD bit set, which leads to invocation of
> __imx_uart_rtsint(), which calls uart_handle_cts_change().
> - If the RTS signal is deasserted, uart_handle_cts_change() clears
> port->hw_stopped and unblocks the port for further data transfers.
> - If the RTS is asserted, uart_handle_cts_change() sets port->hw_stopped
> and blocks the port for further data transfers. This may occur as the
> last interrupt of a transfer, which means port->hw_stopped remains set
> and the port remains blocked (**).
> - Any further data transfer attempts will trigger imx_uart_mctrl_check(),
> which will read current status of UART control signals by calling
> imx_uart_get_hwmctrl() (***) and compare it with sport->old_status .
> - If current status differs from sport->old_status for RTS signal,
> uart_handle_cts_change() is called and possibly unblocks the port
> by clearing port->hw_stopped .
> - If current status does not differ from sport->old_status for RTS
> signal, no action occurs. This may occur in case prior snapshot (*)
> was taken before any transfer so the RTS is deasserted, current
> snapshot (***) was taken after a transfer and therefore RTS is
> deasserted again, which means current status and sport->old_status
> are identical. In case (**) triggered when RTS got asserted, and
> made port->hw_stopped set, the port->hw_stopped will remain set
> because no change on RTS line is recognized by this driver and
> uart_handle_cts_change() is not called from here to unblock the
> port->hw_stopped.
>
> Update sport->old_status in __imx_uart_rtsint() accordingly to make
> imx_uart_mctrl_check() detect such RTS change. Note that TIOCM_CAR
> and TIOCM_RI bits in sport->old_status do not suffer from this problem.
Why is that? Just because these don't stop transmission?
> Fixes: ceca629e0b48 ("[ARM] 2971/1: i.MX uart handle rts irq")
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> ---
> drivers/tty/serial/imx.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 67d4a72eda770..3ad7f42790ef9 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -762,6 +762,10 @@ static irqreturn_t __imx_uart_rtsint(int irq, void *dev_id)
>
> imx_uart_writel(sport, USR1_RTSD, USR1);
> usr1 = imx_uart_readl(sport, USR1) & USR1_RTSS;
> + if (usr1 & USR1_RTSS)
> + sport->old_status |= TIOCM_CTS;
> + else
> + sport->old_status &= ~TIOCM_CTS;
> uart_handle_cts_change(&sport->port, usr1);
> wake_up_interruptible(&sport->port.state->port.delta_msr_wait);
I didn't grab the whole picture, but I think this deserves a code
comment.
Would it make sense to replace the current code in __imx_uart_rtsint by
a call to imx_uart_mctrl_check()?
Best regards
Uwe
Attachment:
signature.asc
Description: PGP signature
