[PATCH 0/2] serial: imx: Backport fixes for irq handling to v4.14

[PATCH 0/2] serial: imx: Backport fixes for irq handling to v4.14

[Schrempf Frieder]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

A customer of ours has problems with RS485 on i.MX6UL with the latest v4.14
kernel. They get an exception like below from time to time (the trace is
from an older kernel, but the problem also exists in v4.14.170).

As the cpuidle state 2 causes large delays for the interrupt that controls the
RS485 RTS signal (which can lead to collisions on the bus), cpuidle state 2 was
disabled on this system. This aspect might cause the exception happening more
often on this system than on other systems with default cpuidle settings.

Looking for solutions I found Uwe's patches that were applied in v4.17 being
mentioned here [1] and here [2]. In [1] Uwe notes that backporting these fixes
to v4.14 might not be trivial, but I tried and in my opinion found it not to be
too problematic either.

With the backported patches applied, our customer reports that the exceptions
stopped occuring. Given this and the fact that the problem seems to be known
and quite common, it would be nice to get this into the v4.14 stable tree. 

[1] https://patchwork.kernel.org/patch/11342831/
[2] https://community.nxp.com/thread/481000

Stack trace:

Unhandled fault: external abort on non-linefetch (0x1008) at 0x90928000
pgd = 8ce1c000
[90928000] *pgd=8d806811, *pte=021ec653, *ppte=021ec453
Internal error: : 1008 [#1] PREEMPT SMP ARM
Modules linked in: usb_f_ecm g_ether usb_f_rndis u_ether libcomposite xt_tcpudp iptable_filter ip_tables x_tables spidev
CPU: 0 PID: 277 Comm: mtiosSys5.elf Not tainted 4.14.89-exceet #4015
Hardware name: Freescale i.MX6 Ultralite (Device Tree)
task: 8da9de00 task.stack: 8cd50000
PC is at imx_rxint+0x58/0x298
LR is at _raw_spin_lock_irqsave+0x18/0x5c
pc : [<8044fa08>]    lr : [<80711208>]    psr: 20070193
sp : 8cd51ce0  ip : 8d400234  fp : 8da94010
r10: 80957900  r9 : 80c3e7ed  r8 : 00000004
r7 : 80c02d00  r6 : 00000000  r5 : 8dae49f0  r4 : 00000001
r3 : 0000e38e  r2 : 00021500  r1 : 90928000  r0 : 40070193
Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 8ce1c06a  DAC: 00000051
Process mtiosSys5.elf (pid: 277, stack limit = 0x8cd50210)
Stack: (0x8cd51ce0 to 0x8cd52000)
[...]
[<8044fa08>] (imx_rxint) from [<80450c1c>] (imx_int+0x124/0x20c)
[<80450c1c>] (imx_int) from [<8015ea94>] (__handle_irq_event_percpu+0x50/0x11c)
[<8015ea94>] (__handle_irq_event_percpu) from [<8015eb7c>] (handle_irq_event_percpu+0x1c/0x58)
[<8015eb7c>] (handle_irq_event_percpu) from [<8015ebf0>] (handle_irq_event+0x38/0x5c)
[<8015ebf0>] (handle_irq_event) from [<801621d4>] (handle_fasteoi_irq+0xb8/0x16c)
[<801621d4>] (handle_fasteoi_irq) from [<8015dd98>] (generic_handle_irq+0x24/0x34)
[<8015dd98>] (generic_handle_irq) from [<8015e2c0>] (__handle_domain_irq+0x7c/0xec)
[<8015e2c0>] (__handle_domain_irq) from [<80101448>] (gic_handle_irq+0x4c/0x90)
[<80101448>] (gic_handle_irq) from [<8010bf4c>] (__irq_svc+0x6c/0xa8)
[...]
[<8010bf4c>] (__irq_svc) from [<80711580>] (_raw_spin_unlock_irqrestore+0x20/0x54)
[<80711580>] (_raw_spin_unlock_irqrestore) from [<8044baf4>] (uart_write+0x110/0x178)
[<8044baf4>] (uart_write) from [<804339b8>] (n_tty_write+0x350/0x440)
[<804339b8>] (n_tty_write) from [<8042fbe8>] (tty_write+0x180/0x354)
[<8042fbe8>] (tty_write) from [<801f93bc>] (__vfs_write+0x1c/0x120)
[<801f93bc>] (__vfs_write) from [<801f9634>] (vfs_write+0xa4/0x168)
[<801f9634>] (vfs_write) from [<801f97f8>] (SyS_write+0x3c/0x90)
[<801f97f8>] (SyS_write) from [<80107900>] (ret_fast_syscall+0x0/0x54)
Code: e59b2074 e59b1008 e2822001 e58b2074 (e591a000)

Uwe Kleine-König (2):
  serial: imx: ensure that RX irqs are off if RX is off
  serial: imx: Only handle irqs that are actually enabled

 drivers/tty/serial/imx.c | 169 +++++++++++++++++++++++++++------------
 1 file changed, 117 insertions(+), 52 deletions(-)

-- 
2.17.1

[PATCH 2/2] serial: imx: Only handle irqs that are actually enabled

[Schrempf Frieder]

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

commit 437768962f754d9501e5ba4d98b1f2a89dc62028 upstream

Handling an irq that isn't enabled can have some undesired side effects.
Some of these are mentioned in the newly introduced code comment. Some
of the irq sources already had their handling right, some don't. Handle
them all in the same consistent way.

The change for USR1_RRDY and USR1_AGTIM drops the check for
dma_is_enabled. This is correct as UCR1_RRDYEN and UCR2_ATEN are always
off if dma is enabled.

Cc: <stable@vger.kernel.org> # v4.14.x
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Backport to v4.14]
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/tty/serial/imx.c | 53 +++++++++++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 31e1e32c62c9..969497599e88 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -843,14 +843,42 @@ static void imx_mctrl_check(struct imx_port *sport)
 static irqreturn_t imx_int(int irq, void *dev_id)
 {
 	struct imx_port *sport = dev_id;
-	unsigned int sts;
-	unsigned int sts2;
+	unsigned int usr1, usr2, ucr1, ucr2, ucr3, ucr4;
 	irqreturn_t ret = IRQ_NONE;
 
-	sts = readl(sport->port.membase + USR1);
-	sts2 = readl(sport->port.membase + USR2);
+	usr1 = readl(sport->port.membase + USR1);
+	usr2 = readl(sport->port.membase + USR2);
+	ucr1 = readl(sport->port.membase + UCR1);
+	ucr2 = readl(sport->port.membase + UCR2);
+	ucr3 = readl(sport->port.membase + UCR3);
+	ucr4 = readl(sport->port.membase + UCR4);
 
-	if (sts & (USR1_RRDY | USR1_AGTIM)) {
+	/*
+	 * Even if a condition is true that can trigger an irq only handle it if
+	 * the respective irq source is enabled. This prevents some undesired
+	 * actions, for example if a character that sits in the RX FIFO and that
+	 * should be fetched via DMA is tried to be fetched using PIO. Or the
+	 * receiver is currently off and so reading from URXD0 results in an
+	 * exception. So just mask the (raw) status bits for disabled irqs.
+	 */
+	if ((ucr1 & UCR1_RRDYEN) == 0)
+		usr1 &= ~USR1_RRDY;
+	if ((ucr2 & UCR2_ATEN) == 0)
+		usr1 &= ~USR1_AGTIM;
+	if ((ucr1 & UCR1_TXMPTYEN) == 0)
+		usr1 &= ~USR1_TRDY;
+	if ((ucr4 & UCR4_TCEN) == 0)
+		usr2 &= ~USR2_TXDC;
+	if ((ucr3 & UCR3_DTRDEN) == 0)
+		usr1 &= ~USR1_DTRD;
+	if ((ucr1 & UCR1_RTSDEN) == 0)
+		usr1 &= ~USR1_RTSD;
+	if ((ucr3 & UCR3_AWAKEN) == 0)
+		usr1 &= ~USR1_AWAKE;
+	if ((ucr4 & UCR4_OREN) == 0)
+		usr2 &= ~USR2_ORE;
+
+	if (usr1 & (USR1_RRDY | USR1_AGTIM)) {
 		if (sport->dma_is_enabled)
 			imx_dma_rxint(sport);
 		else
@@ -858,18 +886,15 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 		ret = IRQ_HANDLED;
 	}
 
-	if ((sts & USR1_TRDY &&
-	     readl(sport->port.membase + UCR1) & UCR1_TXMPTYEN) ||
-	    (sts2 & USR2_TXDC &&
-	     readl(sport->port.membase + UCR4) & UCR4_TCEN)) {
+	if ((usr1 & USR1_TRDY) || (usr2 & USR2_TXDC)) {
 		imx_txint(irq, dev_id);
 		ret = IRQ_HANDLED;
 	}
 
-	if (sts & USR1_DTRD) {
+	if (usr1 & USR1_DTRD) {
 		unsigned long flags;
 
-		if (sts & USR1_DTRD)
+		if (usr1 & USR1_DTRD)
 			writel(USR1_DTRD, sport->port.membase + USR1);
 
 		spin_lock_irqsave(&sport->port.lock, flags);
@@ -879,17 +904,17 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 		ret = IRQ_HANDLED;
 	}
 
-	if (sts & USR1_RTSD) {
+	if (usr1 & USR1_RTSD) {
 		imx_rtsint(irq, dev_id);
 		ret = IRQ_HANDLED;
 	}
 
-	if (sts & USR1_AWAKE) {
+	if (usr1 & USR1_AWAKE) {
 		writel(USR1_AWAKE, sport->port.membase + USR1);
 		ret = IRQ_HANDLED;
 	}
 
-	if (sts2 & USR2_ORE) {
+	if (usr2 & USR2_ORE) {
 		sport->port.icount.overrun++;
 		writel(USR2_ORE, sport->port.membase + USR2);
 		ret = IRQ_HANDLED;
-- 
2.17.1

[PATCH 1/2] serial: imx: ensure that RX irqs are off if RX is off

[Schrempf Frieder]

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

commit 76821e222c189b81d553b855ee7054340607eb46 upstream

Make sure that UCR1.RXDMAEN and UCR1.ATDMAEN (for the DMA case) and
UCR1.RRDYEN (for the PIO case) are off iff UCR1.RXEN is disabled. This
ensures that the fifo isn't read with RX disabled which results in an
exception.

Cc: <stable@vger.kernel.org> # v4.14.x
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Backport to v4.14]
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/tty/serial/imx.c | 116 ++++++++++++++++++++++++++-------------
 1 file changed, 78 insertions(+), 38 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index a81a5be0cf7a..31e1e32c62c9 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -80,7 +80,7 @@
 #define UCR1_IDEN	(1<<12) /* Idle condition interrupt */
 #define UCR1_ICD_REG(x) (((x) & 3) << 10) /* idle condition detect */
 #define UCR1_RRDYEN	(1<<9)	/* Recv ready interrupt enable */
-#define UCR1_RDMAEN	(1<<8)	/* Recv ready DMA enable */
+#define UCR1_RXDMAEN	(1<<8)	/* Recv ready DMA enable */
 #define UCR1_IREN	(1<<7)	/* Infrared interface enable */
 #define UCR1_TXMPTYEN	(1<<6)	/* Transimitter empty interrupt enable */
 #define UCR1_RTSDEN	(1<<5)	/* RTS delta interrupt enable */
@@ -352,6 +352,30 @@ static void imx_port_rts_auto(struct imx_port *sport, unsigned long *ucr2)
 	*ucr2 |= UCR2_CTSC;
 }
 
+/*
+ * interrupts disabled on entry
+ */
+static void imx_start_rx(struct uart_port *port)
+{
+	struct imx_port *sport = (struct imx_port *)port;
+	unsigned int ucr1, ucr2;
+
+	ucr1 = readl(port->membase + UCR1);
+	ucr2 = readl(port->membase + UCR2);
+
+	ucr2 |= UCR2_RXEN;
+
+	if (sport->dma_is_enabled) {
+		ucr1 |= UCR1_RXDMAEN | UCR1_ATDMAEN;
+	} else {
+		ucr1 |= UCR1_RRDYEN;
+	}
+
+	/* Write UCR2 first as it includes RXEN */
+	writel(ucr2, port->membase + UCR2);
+	writel(ucr1, port->membase + UCR1);
+}
+
 /*
  * interrupts disabled on entry
  */
@@ -378,9 +402,10 @@ static void imx_stop_tx(struct uart_port *port)
 			imx_port_rts_active(sport, &temp);
 		else
 			imx_port_rts_inactive(sport, &temp);
-		temp |= UCR2_RXEN;
 		writel(temp, port->membase + UCR2);
 
+		imx_start_rx(port);
+
 		temp = readl(port->membase + UCR4);
 		temp &= ~UCR4_TCEN;
 		writel(temp, port->membase + UCR4);
@@ -393,7 +418,7 @@ static void imx_stop_tx(struct uart_port *port)
 static void imx_stop_rx(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
-	unsigned long temp;
+	unsigned long ucr1, ucr2;
 
 	if (sport->dma_is_enabled && sport->dma_is_rxing) {
 		if (sport->port.suspended) {
@@ -404,12 +429,18 @@ static void imx_stop_rx(struct uart_port *port)
 		}
 	}
 
-	temp = readl(sport->port.membase + UCR2);
-	writel(temp & ~UCR2_RXEN, sport->port.membase + UCR2);
+	ucr1 = readl(sport->port.membase + UCR1);
+	ucr2 = readl(sport->port.membase + UCR2);
 
-	/* disable the `Receiver Ready Interrrupt` */
-	temp = readl(sport->port.membase + UCR1);
-	writel(temp & ~UCR1_RRDYEN, sport->port.membase + UCR1);
+	if (sport->dma_is_enabled) {
+		ucr1 &= ~(UCR1_RXDMAEN | UCR1_ATDMAEN);
+	} else {
+		ucr1 &= ~UCR1_RRDYEN;
+	}
+	writel(ucr1, port->membase + UCR1);
+
+	ucr2 &= ~UCR2_RXEN;
+	writel(ucr2, port->membase + UCR2);
 }
 
 /*
@@ -581,10 +612,11 @@ static void imx_start_tx(struct uart_port *port)
 			imx_port_rts_active(sport, &temp);
 		else
 			imx_port_rts_inactive(sport, &temp);
-		if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
-			temp &= ~UCR2_RXEN;
 		writel(temp, port->membase + UCR2);
 
+		if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
+			imx_stop_rx(port);
+
 		/* enable transmitter and shifter empty irq */
 		temp = readl(port->membase + UCR4);
 		temp |= UCR4_TCEN;
@@ -1206,7 +1238,7 @@ static void imx_enable_dma(struct imx_port *sport)
 
 	/* set UCR1 */
 	temp = readl(sport->port.membase + UCR1);
-	temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
+	temp |= UCR1_RXDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
 	writel(temp, sport->port.membase + UCR1);
 
 	temp = readl(sport->port.membase + UCR2);
@@ -1224,7 +1256,7 @@ static void imx_disable_dma(struct imx_port *sport)
 
 	/* clear UCR1 */
 	temp = readl(sport->port.membase + UCR1);
-	temp &= ~(UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN);
+	temp &= ~(UCR1_RXDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN);
 	writel(temp, sport->port.membase + UCR1);
 
 	/* clear UCR2 */
@@ -1289,11 +1321,9 @@ static int imx_startup(struct uart_port *port)
 	writel(USR1_RTSD | USR1_DTRD, sport->port.membase + USR1);
 	writel(USR2_ORE, sport->port.membase + USR2);
 
-	if (sport->dma_is_inited && !sport->dma_is_enabled)
-		imx_enable_dma(sport);
-
 	temp = readl(sport->port.membase + UCR1);
-	temp |= UCR1_RRDYEN | UCR1_UARTEN;
+	temp &= ~UCR1_RRDYEN;
+	temp |= UCR1_UARTEN;
 	if (sport->have_rtscts)
 			temp |= UCR1_RTSDEN;
 
@@ -1332,14 +1362,13 @@ static int imx_startup(struct uart_port *port)
 	 */
 	imx_enable_ms(&sport->port);
 
-	/*
-	 * Start RX DMA immediately instead of waiting for RX FIFO interrupts.
-	 * In our iMX53 the average delay for the first reception dropped from
-	 * approximately 35000 microseconds to 1000 microseconds.
-	 */
-	if (sport->dma_is_enabled) {
-		imx_disable_rx_int(sport);
+	if (sport->dma_is_inited) {
+		imx_enable_dma(sport);
 		start_rx_dma(sport);
+	} else {
+		temp = readl(sport->port.membase + UCR1);
+		temp |= UCR1_RRDYEN;
+		writel(temp, sport->port.membase + UCR1);
 	}
 
 	spin_unlock_irqrestore(&sport->port.lock, flags);
@@ -1386,7 +1415,8 @@ static void imx_shutdown(struct uart_port *port)
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 	temp = readl(sport->port.membase + UCR1);
-	temp &= ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN);
+	temp &= ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN |
+		  UCR1_RXDMAEN | UCR1_ATDMAEN);
 
 	writel(temp, sport->port.membase + UCR1);
 	spin_unlock_irqrestore(&sport->port.lock, flags);
@@ -1659,7 +1689,7 @@ static int imx_poll_init(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
 	unsigned long flags;
-	unsigned long temp;
+	unsigned long ucr1, ucr2;
 	int retval;
 
 	retval = clk_prepare_enable(sport->clk_ipg);
@@ -1673,16 +1703,29 @@ static int imx_poll_init(struct uart_port *port)
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
-	temp = readl(sport->port.membase + UCR1);
+	/*
+	 * Be careful about the order of enabling bits here. First enable the
+	 * receiver (UARTEN + RXEN) and only then the corresponding irqs.
+	 * This prevents that a character that already sits in the RX fifo is
+	 * triggering an irq but the try to fetch it from there results in an
+	 * exception because UARTEN or RXEN is still off.
+	 */
+	ucr1 = readl(port->membase + UCR1);
+	ucr2 = readl(port->membase + UCR2);
+
 	if (is_imx1_uart(sport))
-		temp |= IMX1_UCR1_UARTCLKEN;
-	temp |= UCR1_UARTEN | UCR1_RRDYEN;
-	temp &= ~(UCR1_TXMPTYEN | UCR1_RTSDEN);
-	writel(temp, sport->port.membase + UCR1);
+		ucr1 |= IMX1_UCR1_UARTCLKEN;
 
-	temp = readl(sport->port.membase + UCR2);
-	temp |= UCR2_RXEN;
-	writel(temp, sport->port.membase + UCR2);
+	ucr1 |= UCR1_UARTEN;
+	ucr1 &= ~(UCR1_TXMPTYEN | UCR1_RTSDEN | UCR1_RRDYEN);
+
+	ucr2 |= UCR2_RXEN;
+
+	writel(ucr1, sport->port.membase + UCR1);
+	writel(ucr2, sport->port.membase + UCR2);
+
+	/* now enable irqs */
+	writel(ucr1 | UCR1_RRDYEN, sport->port.membase + UCR1);
 
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 
@@ -1742,11 +1785,8 @@ static int imx_rs485_config(struct uart_port *port,
 
 	/* Make sure Rx is enabled in case Tx is active with Rx disabled */
 	if (!(rs485conf->flags & SER_RS485_ENABLED) ||
-	    rs485conf->flags & SER_RS485_RX_DURING_TX) {
-		temp = readl(sport->port.membase + UCR2);
-		temp |= UCR2_RXEN;
-		writel(temp, sport->port.membase + UCR2);
-	}
+	    rs485conf->flags & SER_RS485_RX_DURING_TX)
+		imx_start_rx(port);
 
 	port->rs485 = *rs485conf;
 
-- 
2.17.1

Re: [PATCH 0/2] serial: imx: Backport fixes for irq handling to v4.14

[gregkh]

On Mon, Feb 17, 2020 at 02:08:00PM +0000, Schrempf Frieder wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> A customer of ours has problems with RS485 on i.MX6UL with the latest v4.14
> kernel. They get an exception like below from time to time (the trace is
> from an older kernel, but the problem also exists in v4.14.170).
> 
> As the cpuidle state 2 causes large delays for the interrupt that controls the
> RS485 RTS signal (which can lead to collisions on the bus), cpuidle state 2 was
> disabled on this system. This aspect might cause the exception happening more
> often on this system than on other systems with default cpuidle settings.
> 
> Looking for solutions I found Uwe's patches that were applied in v4.17 being
> mentioned here [1] and here [2]. In [1] Uwe notes that backporting these fixes
> to v4.14 might not be trivial, but I tried and in my opinion found it not to be
> too problematic either.
> 
> With the backported patches applied, our customer reports that the exceptions
> stopped occuring. Given this and the fact that the problem seems to be known
> and quite common, it would be nice to get this into the v4.14 stable tree. 

Thanks for the backports, both now queued up.

greg k-h

Re: [PATCH 0/2] serial: imx: Backport fixes for irq handling to v4.14

[Uwe Kleine-König]

On Tue, Feb 18, 2020 at 05:50:08AM +0100, gregkh@linuxfoundation.org wrote:
> On Mon, Feb 17, 2020 at 02:08:00PM +0000, Schrempf Frieder wrote:
> > From: Frieder Schrempf <frieder.schrempf@kontron.de>
> > 
> > A customer of ours has problems with RS485 on i.MX6UL with the latest v4.14
> > kernel. They get an exception like below from time to time (the trace is
> > from an older kernel, but the problem also exists in v4.14.170).
> > 
> > As the cpuidle state 2 causes large delays for the interrupt that controls the
> > RS485 RTS signal (which can lead to collisions on the bus), cpuidle state 2 was
> > disabled on this system. This aspect might cause the exception happening more
> > often on this system than on other systems with default cpuidle settings.
> > 
> > Looking for solutions I found Uwe's patches that were applied in v4.17 being
> > mentioned here [1] and here [2]. In [1] Uwe notes that backporting these fixes
> > to v4.14 might not be trivial, but I tried and in my opinion found it not to be
> > too problematic either.
> > 
> > With the backported patches applied, our customer reports that the exceptions
> > stopped occuring. Given this and the fact that the problem seems to be known
> > and quite common, it would be nice to get this into the v4.14 stable tree. 
> 
> Thanks for the backports, both now queued up.

To complete these fixes you also want to backport

	101aa46bd221 serial: imx: fix a race condition in receive path

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH 0/2] serial: imx: Backport fixes for irq handling to v4.14

[gregkh]

On Tue, Feb 18, 2020 at 08:03:10AM +0100, Uwe Kleine-König wrote:
> On Tue, Feb 18, 2020 at 05:50:08AM +0100, gregkh@linuxfoundation.org wrote:
> > On Mon, Feb 17, 2020 at 02:08:00PM +0000, Schrempf Frieder wrote:
> > > From: Frieder Schrempf <frieder.schrempf@kontron.de>
> > > 
> > > A customer of ours has problems with RS485 on i.MX6UL with the latest v4.14
> > > kernel. They get an exception like below from time to time (the trace is
> > > from an older kernel, but the problem also exists in v4.14.170).
> > > 
> > > As the cpuidle state 2 causes large delays for the interrupt that controls the
> > > RS485 RTS signal (which can lead to collisions on the bus), cpuidle state 2 was
> > > disabled on this system. This aspect might cause the exception happening more
> > > often on this system than on other systems with default cpuidle settings.
> > > 
> > > Looking for solutions I found Uwe's patches that were applied in v4.17 being
> > > mentioned here [1] and here [2]. In [1] Uwe notes that backporting these fixes
> > > to v4.14 might not be trivial, but I tried and in my opinion found it not to be
> > > too problematic either.
> > > 
> > > With the backported patches applied, our customer reports that the exceptions
> > > stopped occuring. Given this and the fact that the problem seems to be known
> > > and quite common, it would be nice to get this into the v4.14 stable tree. 
> > 
> > Thanks for the backports, both now queued up.
> 
> To complete these fixes you also want to backport
> 
> 	101aa46bd221 serial: imx: fix a race condition in receive path

If so, it needs to also go to 4.19.y, and someone needs to provide a
working backport for both places :)

thanks,

greg k-h

Re: [PATCH 0/2] serial: imx: Backport fixes for irq handling to v4.14

[Schrempf Frieder]

On 18.02.20 08:17, gregkh@linuxfoundation.org wrote:
> On Tue, Feb 18, 2020 at 08:03:10AM +0100, Uwe Kleine-König wrote:
>> On Tue, Feb 18, 2020 at 05:50:08AM +0100, gregkh@linuxfoundation.org wrote:
>>> On Mon, Feb 17, 2020 at 02:08:00PM +0000, Schrempf Frieder wrote:
>>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>>
>>>> A customer of ours has problems with RS485 on i.MX6UL with the latest v4.14
>>>> kernel. They get an exception like below from time to time (the trace is
>>>> from an older kernel, but the problem also exists in v4.14.170).
>>>>
>>>> As the cpuidle state 2 causes large delays for the interrupt that controls the
>>>> RS485 RTS signal (which can lead to collisions on the bus), cpuidle state 2 was
>>>> disabled on this system. This aspect might cause the exception happening more
>>>> often on this system than on other systems with default cpuidle settings.
>>>>
>>>> Looking for solutions I found Uwe's patches that were applied in v4.17 being
>>>> mentioned here [1] and here [2]. In [1] Uwe notes that backporting these fixes
>>>> to v4.14 might not be trivial, but I tried and in my opinion found it not to be
>>>> too problematic either.
>>>>
>>>> With the backported patches applied, our customer reports that the exceptions
>>>> stopped occuring. Given this and the fact that the problem seems to be known
>>>> and quite common, it would be nice to get this into the v4.14 stable tree.
>>>
>>> Thanks for the backports, both now queued up.
>>
>> To complete these fixes you also want to backport
>>
>> 	101aa46bd221 serial: imx: fix a race condition in receive path
> 
> If so, it needs to also go to 4.19.y, and someone needs to provide a
> working backport for both places :)

I can try to come up with something. But I don't have a system that is 
affected by this (only single core) to test.

Best regards,
Frieder