RE: [EXT] Re: iMX6/UART imprecise external abort

[Andy Duan <fugang.duan@nxp.com>]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Sent: Wednesday, January 8, 2020 7:23 PM
> Hello,
> 
> On Tue, Jan 07, 2020 at 11:24:06PM +0100, Uwe Kleine-König wrote:
> > I will look into this again tomorrow when I'm well rested and create a
> > patch.
> 
> Here you go, for now without proper commit log etc.pp.
> 
> Please test if this fixes your problems.
> 
> I currently don't have the setup to trigger this bug, but normal console usage
> still works for me.

I also have not the environment to reproduce the issue.
@Andre Renaud, can you try Uwe's patch ?


Regards,
Andy
> 
> Best regards
> Uwe
> 
> -------->8--------
> From 025a72c6de6df8b71414378a0297568df371bd73 Mon Sep 17 00:00:00
> 2001
> From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?=
> <u.kleine-koenig@pengutronix.de>
> Date: Wed, 8 Jan 2020 09:47:20 +0100
> Subject: [PATCH RFT] serial: imx: fix a race condition in receive path
> 
> ---
>  drivers/tty/serial/imx.c | 52 ++++++++++++++++++++++++++++++----------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index
> a9e20e6c63ad..679b2de27c4d 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -700,22 +700,33 @@ static void imx_uart_start_tx(struct uart_port
> *port)
>         }
>  }
> 
> -static irqreturn_t imx_uart_rtsint(int irq, void *dev_id)
> +static irqreturn_t __imx_uart_rtsint(int irq, void *dev_id)
>  {
>         struct imx_port *sport = dev_id;
>         u32 usr1;
> 
> -       spin_lock(&sport->port.lock);
> -
>         imx_uart_writel(sport, USR1_RTSD, USR1);
>         usr1 = imx_uart_readl(sport, USR1) & USR1_RTSS;
>         uart_handle_cts_change(&sport->port, !!usr1);
>         wake_up_interruptible(&sport->port.state->port.delta_msr_wait);
> 
> -       spin_unlock(&sport->port.lock);
>         return IRQ_HANDLED;
>  }
> 
> +static irqreturn_t imx_uart_rtsint(int irq, void *dev_id) {
> +       struct imx_port *sport = dev_id;
> +       irqreturn_t ret;
> +
> +       spin_lock(&sport->port.lock);
> +
> +       ret = __imx_uart_rtsint(irq, dev_id);
> +
> +       spin_unlock(&sport->port.lock);
> +
> +       return ret;
> +}
> +
>  static irqreturn_t imx_uart_txint(int irq, void *dev_id)  {
>         struct imx_port *sport = dev_id; @@ -726,14 +737,12 @@ static
> irqreturn_t imx_uart_txint(int irq, void *dev_id)
>         return IRQ_HANDLED;
>  }
> 
> -static irqreturn_t imx_uart_rxint(int irq, void *dev_id)
> +static irqreturn_t __imx_uart_rxint(int irq, void *dev_id)
>  {
>         struct imx_port *sport = dev_id;
>         unsigned int rx, flg, ignored = 0;
>         struct tty_port *port = &sport->port.state->port;
> 
> -       spin_lock(&sport->port.lock);
> -
>         while (imx_uart_readl(sport, USR2) & USR2_RDR) {
>                 u32 usr2;
> 
> @@ -792,11 +801,26 @@ static irqreturn_t imx_uart_rxint(int irq, void
> *dev_id)
>         }
> 
>  out:
> -       spin_unlock(&sport->port.lock);
>         tty_flip_buffer_push(port);
> +
>         return IRQ_HANDLED;
>  }
> 
> +static irqreturn_t imx_uart_rxint(int irq, void *dev_id) {
> +       struct imx_port *sport = dev_id;
> +       struct tty_port *port = &sport->port.state->port;
> +       irqreturn_t ret;
> +
> +       spin_lock(&sport->port.lock);
> +
> +       ret = __imx_uart_rxint(irq, dev_id);
> +
> +       spin_unlock(&sport->port.lock);
> +
> +       return ret;
> +}
> +
>  static void imx_uart_clear_rx_errors(struct imx_port *sport);
> 
>  /*
> @@ -855,6 +879,8 @@ static irqreturn_t imx_uart_int(int irq, void *dev_id)
>         unsigned int usr1, usr2, ucr1, ucr2, ucr3, ucr4;
>         irqreturn_t ret = IRQ_NONE;
> 
> +       spin_lock(&sport->port.lock);
> +
>         usr1 = imx_uart_readl(sport, USR1);
>         usr2 = imx_uart_readl(sport, USR2);
>         ucr1 = imx_uart_readl(sport, UCR1); @@ -888,27 +914,25 @@
> static irqreturn_t imx_uart_int(int irq, void *dev_id)
>                 usr2 &= ~USR2_ORE;
> 
>         if (usr1 & (USR1_RRDY | USR1_AGTIM)) {
> -               imx_uart_rxint(irq, dev_id);
> +               __imx_uart_rxint(irq, dev_id);
>                 ret = IRQ_HANDLED;
>         }
> 
>         if ((usr1 & USR1_TRDY) || (usr2 & USR2_TXDC)) {
> -               imx_uart_txint(irq, dev_id);
> +               imx_uart_transmit_buffer(sport);
>                 ret = IRQ_HANDLED;
>         }
> 
>         if (usr1 & USR1_DTRD) {
>                 imx_uart_writel(sport, USR1_DTRD, USR1);
> 
> -               spin_lock(&sport->port.lock);
>                 imx_uart_mctrl_check(sport);
> -               spin_unlock(&sport->port.lock);
> 
>                 ret = IRQ_HANDLED;
>         }
> 
>         if (usr1 & USR1_RTSD) {
> -               imx_uart_rtsint(irq, dev_id);
> +               __imx_uart_rtsint(irq, dev_id);
>                 ret = IRQ_HANDLED;
>         }
> 
> @@ -923,6 +947,8 @@ static irqreturn_t imx_uart_int(int irq, void *dev_id)
>                 ret = IRQ_HANDLED;
>         }
> 
> +       spin_unlock(&sport->port.lock);
> +
>         return ret;
>  }
> 
> --
> 2.24.0
> 
> 
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König
> |
> Industrial Linux Solutions                 |
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> pengutronix.de%2F&amp;data=02%7C01%7Cfugang.duan%40nxp.com%7Ca7
> 00379685894ff5ceeb08d7942d28d8%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C637140793993226335&amp;sdata=KH2pKxhQxNLv6jV8ZST
> j7UuZHYDAnzP8%2BTkHNTr5T4Y%3D&amp;reserved=0 |

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