All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] serial: fsl_lpuart: fix DMA issues for 3.19
@ 2015-01-10  0:08 ` Stefan Agner
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  0:08 UTC (permalink / raw)
  To: gregkh
  Cc: jslaby, jingchang.lu, shawn.guo, linux-serial, linux-arm-kernel,
	linux-kernel, stefan

This is patch 1 and 3 of the first version of this patchset.

Changes since v1:
- Reproduced on v3.19-rc3 and rebased ontop of that
- Make sure that RDMAS is cleared on startup

Stefan Agner (2):
  serial: fsl_lpuart: delete timer on shutdown
  serial: fsl_lpuart: avoid new transfer while DMA is running

 drivers/tty/serial/fsl_lpuart.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

-- 
2.2.1


^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v2 0/2] serial: fsl_lpuart: fix DMA issues for 3.19
@ 2015-01-10  0:08 ` Stefan Agner
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  0:08 UTC (permalink / raw)
  To: linux-arm-kernel

This is patch 1 and 3 of the first version of this patchset.

Changes since v1:
- Reproduced on v3.19-rc3 and rebased ontop of that
- Make sure that RDMAS is cleared on startup

Stefan Agner (2):
  serial: fsl_lpuart: delete timer on shutdown
  serial: fsl_lpuart: avoid new transfer while DMA is running

 drivers/tty/serial/fsl_lpuart.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

-- 
2.2.1

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v2 1/2] serial: fsl_lpuart: delete timer on shutdown
  2015-01-10  0:08 ` Stefan Agner
@ 2015-01-10  0:08   ` Stefan Agner
  -1 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  0:08 UTC (permalink / raw)
  To: gregkh
  Cc: jslaby, jingchang.lu, shawn.guo, linux-serial, linux-arm-kernel,
	linux-kernel, stefan

If the serial port gets closed while a RX transfer is in progress,
the timer might fire after the serial port shutdown finished. This
leads in a NULL pointer dereference:

[    7.508324] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[    7.516590] pgd = 86348000
[    7.519445] [00000000] *pgd=86179831, *pte=00000000, *ppte=00000000
[    7.526145] Internal error: Oops: 17 [#1] ARM
[    7.530611] Modules linked in:
[    7.533876] CPU: 0 PID: 123 Comm: systemd Not tainted 3.19.0-rc3-00004-g5b11ea7 #1778
[    7.541827] Hardware name: Freescale Vybrid VF610 (Device Tree)
[    7.547862] task: 861c3400 ti: 86ac8000 task.ti: 86ac8000
[    7.553392] PC is at lpuart_timer_func+0x24/0xf8
[    7.558127] LR is at lpuart_timer_func+0x20/0xf8
[    7.562857] pc : [<802df99c>]    lr : [<802df998>]    psr: 600b0113
[    7.562857] sp : 86ac9b90  ip : 86ac9b90  fp : 86ac9bbc
[    7.574467] r10: 80817180  r9 : 80817b98  r8 : 80817998
[    7.579803] r7 : 807acee0  r6 : 86989000  r5 : 00000100  r4 : 86997210
[    7.586444] r3 : 86ac8000  r2 : 86ac9bc0  r1 : 86997210  r0 : 00000000
[    7.593085] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[    7.600341] Control: 10c5387d  Table: 86348059  DAC: 00000015
[    7.606203] Process systemd (pid: 123, stack limit = 0x86ac8230)

Setup the timer on UART startup which allows to delete the timer
unconditionally on shutdown. This also saves the initialization
on each transfer.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/tty/serial/fsl_lpuart.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index e7cde3a..29932b7 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -506,9 +506,6 @@ static inline void lpuart_prepare_rx(struct lpuart_port *sport)
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
-	init_timer(&sport->lpuart_timer);
-	sport->lpuart_timer.function = lpuart_timer_func;
-	sport->lpuart_timer.data = (unsigned long)sport;
 	sport->lpuart_timer.expires = jiffies + sport->dma_rx_timeout;
 	add_timer(&sport->lpuart_timer);
 
@@ -1106,6 +1103,8 @@ static int lpuart_startup(struct uart_port *port)
 		sport->lpuart_dma_use = false;
 	} else {
 		sport->lpuart_dma_use = true;
+		setup_timer(&sport->lpuart_timer, lpuart_timer_func,
+			    (unsigned long)sport);
 		temp = readb(port->membase + UARTCR5);
 		writeb(temp | UARTCR5_TDMAS, port->membase + UARTCR5);
 	}
@@ -1180,6 +1179,8 @@ static void lpuart_shutdown(struct uart_port *port)
 	devm_free_irq(port->dev, port->irq, sport);
 
 	if (sport->lpuart_dma_use) {
+		del_timer_sync(&sport->lpuart_timer);
+
 		lpuart_dma_tx_free(port);
 		lpuart_dma_rx_free(port);
 	}
-- 
2.2.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v2 1/2] serial: fsl_lpuart: delete timer on shutdown
@ 2015-01-10  0:08   ` Stefan Agner
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  0:08 UTC (permalink / raw)
  To: linux-arm-kernel

If the serial port gets closed while a RX transfer is in progress,
the timer might fire after the serial port shutdown finished. This
leads in a NULL pointer dereference:

[    7.508324] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[    7.516590] pgd = 86348000
[    7.519445] [00000000] *pgd=86179831, *pte=00000000, *ppte=00000000
[    7.526145] Internal error: Oops: 17 [#1] ARM
[    7.530611] Modules linked in:
[    7.533876] CPU: 0 PID: 123 Comm: systemd Not tainted 3.19.0-rc3-00004-g5b11ea7 #1778
[    7.541827] Hardware name: Freescale Vybrid VF610 (Device Tree)
[    7.547862] task: 861c3400 ti: 86ac8000 task.ti: 86ac8000
[    7.553392] PC is at lpuart_timer_func+0x24/0xf8
[    7.558127] LR is at lpuart_timer_func+0x20/0xf8
[    7.562857] pc : [<802df99c>]    lr : [<802df998>]    psr: 600b0113
[    7.562857] sp : 86ac9b90  ip : 86ac9b90  fp : 86ac9bbc
[    7.574467] r10: 80817180  r9 : 80817b98  r8 : 80817998
[    7.579803] r7 : 807acee0  r6 : 86989000  r5 : 00000100  r4 : 86997210
[    7.586444] r3 : 86ac8000  r2 : 86ac9bc0  r1 : 86997210  r0 : 00000000
[    7.593085] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[    7.600341] Control: 10c5387d  Table: 86348059  DAC: 00000015
[    7.606203] Process systemd (pid: 123, stack limit = 0x86ac8230)

Setup the timer on UART startup which allows to delete the timer
unconditionally on shutdown. This also saves the initialization
on each transfer.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/tty/serial/fsl_lpuart.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index e7cde3a..29932b7 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -506,9 +506,6 @@ static inline void lpuart_prepare_rx(struct lpuart_port *sport)
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
-	init_timer(&sport->lpuart_timer);
-	sport->lpuart_timer.function = lpuart_timer_func;
-	sport->lpuart_timer.data = (unsigned long)sport;
 	sport->lpuart_timer.expires = jiffies + sport->dma_rx_timeout;
 	add_timer(&sport->lpuart_timer);
 
@@ -1106,6 +1103,8 @@ static int lpuart_startup(struct uart_port *port)
 		sport->lpuart_dma_use = false;
 	} else {
 		sport->lpuart_dma_use = true;
+		setup_timer(&sport->lpuart_timer, lpuart_timer_func,
+			    (unsigned long)sport);
 		temp = readb(port->membase + UARTCR5);
 		writeb(temp | UARTCR5_TDMAS, port->membase + UARTCR5);
 	}
@@ -1180,6 +1179,8 @@ static void lpuart_shutdown(struct uart_port *port)
 	devm_free_irq(port->dev, port->irq, sport);
 
 	if (sport->lpuart_dma_use) {
+		del_timer_sync(&sport->lpuart_timer);
+
 		lpuart_dma_tx_free(port);
 		lpuart_dma_rx_free(port);
 	}
-- 
2.2.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v2 2/2] serial: fsl_lpuart: avoid new transfer while DMA is running
  2015-01-10  0:08 ` Stefan Agner
@ 2015-01-10  0:08   ` Stefan Agner
  -1 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  0:08 UTC (permalink / raw)
  To: gregkh
  Cc: jslaby, jingchang.lu, shawn.guo, linux-serial, linux-arm-kernel,
	linux-kernel, stefan

When the UART is in DMA receive mode (RDMAS set) and one character
just arrived while another interrupt is handled (e.g. TX), the RDRF
(receiver data register full flag) is set due to the water level of
1. But since the DMA will take care of this character, there is no
need to handle it by calling lpuart_prepare_rx. Handling it leads to
adding the RX timeout timer twice:

[   74.336698] Kernel BUG at 80053070 [verbose debug info unavailable]
[   74.342999] Internal error: Oops - BUG: 0 [#1] ARM0:00.00 khungtaskd
[   74.347817] Modules linked in:    0 S  0.0  0.0   0:00.00 writeback
[   74.350926] CPU: 0 PID: 0 Comm: swapper Not tainted 3.19.0-rc3-00001-g39d78e2 #1788
[   74.358617] Hardware name: Freescale Vybrid VF610 (Device Tree)t
[   74.364563] task: 807a7678 ti: 8079c000 task.ti: 8079c000 kblockd
[   74.370002] PC is at add_timer+0x24/0x28.0  0.0   0:00.09 kworker/u2:1
[   74.373960] LR is at lpuart_int+0x15c/0x3d8
[   74.378171] pc : [<80053070>]    lr : [<802e0d88>]    psr: a0010193
[   74.378171] sp : 8079de10  ip : 8079de20  fp : 8079de1c
[   74.389694] r10: 807d44c0  r9 : 8688c300  r8 : 00000013
[   74.394943] r7 : 20010193  r6 : 00000000  r5 : 000000a0  r4 : 86997210
[   74.401498] r3 : ffffa7da  r2 : 80817868  r1 : 86997210  r0 : 86997344
[   74.408052] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[   74.415489] Control: 10c5387d  Table: 8611c059  DAC: 00000015
[   74.421265] Process swapper (pid: 0, stack limit = 0x8079c230)
...

Solve this by only execute the receiver path (lpuart_prepare_rx) if
the DMA receive mode (RDMAS) is not set. Also, make sure the flag is
cleared on initialization, in case it has been left set.

This can be best reproduced using UART as a serial console, then
running top while dd'ing data into the terminal.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/tty/serial/fsl_lpuart.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 29932b7..e95c497 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -755,18 +755,18 @@ out:
 static irqreturn_t lpuart_int(int irq, void *dev_id)
 {
 	struct lpuart_port *sport = dev_id;
-	unsigned char sts;
+	unsigned char sts, crdma;
 
 	sts = readb(sport->port.membase + UARTSR1);
+	crdma = readb(sport->port.membase + UARTCR5);
 
-	if (sts & UARTSR1_RDRF) {
+	if (sts & UARTSR1_RDRF && !(crdma & UARTCR5_RDMAS)) {
 		if (sport->lpuart_dma_use)
 			lpuart_prepare_rx(sport);
 		else
 			lpuart_rxint(irq, dev_id);
 	}
-	if (sts & UARTSR1_TDRE &&
-		!(readb(sport->port.membase + UARTCR5) & UARTCR5_TDMAS)) {
+	if (sts & UARTSR1_TDRE && !(crdma & UARTCR5_TDMAS)) {
 		if (sport->lpuart_dma_use)
 			lpuart_pio_tx(sport);
 		else
@@ -1106,6 +1106,7 @@ static int lpuart_startup(struct uart_port *port)
 		setup_timer(&sport->lpuart_timer, lpuart_timer_func,
 			    (unsigned long)sport);
 		temp = readb(port->membase + UARTCR5);
+		temp &= ~UARTCR5_RDMAS;
 		writeb(temp | UARTCR5_TDMAS, port->membase + UARTCR5);
 	}
 
-- 
2.2.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v2 2/2] serial: fsl_lpuart: avoid new transfer while DMA is running
@ 2015-01-10  0:08   ` Stefan Agner
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  0:08 UTC (permalink / raw)
  To: linux-arm-kernel

When the UART is in DMA receive mode (RDMAS set) and one character
just arrived while another interrupt is handled (e.g. TX), the RDRF
(receiver data register full flag) is set due to the water level of
1. But since the DMA will take care of this character, there is no
need to handle it by calling lpuart_prepare_rx. Handling it leads to
adding the RX timeout timer twice:

[   74.336698] Kernel BUG at 80053070 [verbose debug info unavailable]
[   74.342999] Internal error: Oops - BUG: 0 [#1] ARM0:00.00 khungtaskd
[   74.347817] Modules linked in:    0 S  0.0  0.0   0:00.00 writeback
[   74.350926] CPU: 0 PID: 0 Comm: swapper Not tainted 3.19.0-rc3-00001-g39d78e2 #1788
[   74.358617] Hardware name: Freescale Vybrid VF610 (Device Tree)t
[   74.364563] task: 807a7678 ti: 8079c000 task.ti: 8079c000 kblockd
[   74.370002] PC is at add_timer+0x24/0x28.0  0.0   0:00.09 kworker/u2:1
[   74.373960] LR is at lpuart_int+0x15c/0x3d8
[   74.378171] pc : [<80053070>]    lr : [<802e0d88>]    psr: a0010193
[   74.378171] sp : 8079de10  ip : 8079de20  fp : 8079de1c
[   74.389694] r10: 807d44c0  r9 : 8688c300  r8 : 00000013
[   74.394943] r7 : 20010193  r6 : 00000000  r5 : 000000a0  r4 : 86997210
[   74.401498] r3 : ffffa7da  r2 : 80817868  r1 : 86997210  r0 : 86997344
[   74.408052] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[   74.415489] Control: 10c5387d  Table: 8611c059  DAC: 00000015
[   74.421265] Process swapper (pid: 0, stack limit = 0x8079c230)
...

Solve this by only execute the receiver path (lpuart_prepare_rx) if
the DMA receive mode (RDMAS) is not set. Also, make sure the flag is
cleared on initialization, in case it has been left set.

This can be best reproduced using UART as a serial console, then
running top while dd'ing data into the terminal.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/tty/serial/fsl_lpuart.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 29932b7..e95c497 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -755,18 +755,18 @@ out:
 static irqreturn_t lpuart_int(int irq, void *dev_id)
 {
 	struct lpuart_port *sport = dev_id;
-	unsigned char sts;
+	unsigned char sts, crdma;
 
 	sts = readb(sport->port.membase + UARTSR1);
+	crdma = readb(sport->port.membase + UARTCR5);
 
-	if (sts & UARTSR1_RDRF) {
+	if (sts & UARTSR1_RDRF && !(crdma & UARTCR5_RDMAS)) {
 		if (sport->lpuart_dma_use)
 			lpuart_prepare_rx(sport);
 		else
 			lpuart_rxint(irq, dev_id);
 	}
-	if (sts & UARTSR1_TDRE &&
-		!(readb(sport->port.membase + UARTCR5) & UARTCR5_TDMAS)) {
+	if (sts & UARTSR1_TDRE && !(crdma & UARTCR5_TDMAS)) {
 		if (sport->lpuart_dma_use)
 			lpuart_pio_tx(sport);
 		else
@@ -1106,6 +1106,7 @@ static int lpuart_startup(struct uart_port *port)
 		setup_timer(&sport->lpuart_timer, lpuart_timer_func,
 			    (unsigned long)sport);
 		temp = readb(port->membase + UARTCR5);
+		temp &= ~UARTCR5_RDMAS;
 		writeb(temp | UARTCR5_TDMAS, port->membase + UARTCR5);
 	}
 
-- 
2.2.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] serial: fsl_lpuart: delete timer on shutdown
  2015-01-10  0:08   ` Stefan Agner
@ 2015-01-10  0:48     ` Greg KH
  -1 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2015-01-10  0:48 UTC (permalink / raw)
  To: Stefan Agner
  Cc: jslaby, jingchang.lu, shawn.guo, linux-serial, linux-arm-kernel,
	linux-kernel

On Sat, Jan 10, 2015 at 01:08:58AM +0100, Stefan Agner wrote:
> If the serial port gets closed while a RX transfer is in progress,
> the timer might fire after the serial port shutdown finished. This
> leads in a NULL pointer dereference:
> 
> [    7.508324] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [    7.516590] pgd = 86348000
> [    7.519445] [00000000] *pgd=86179831, *pte=00000000, *ppte=00000000
> [    7.526145] Internal error: Oops: 17 [#1] ARM
> [    7.530611] Modules linked in:
> [    7.533876] CPU: 0 PID: 123 Comm: systemd Not tainted 3.19.0-rc3-00004-g5b11ea7 #1778
> [    7.541827] Hardware name: Freescale Vybrid VF610 (Device Tree)
> [    7.547862] task: 861c3400 ti: 86ac8000 task.ti: 86ac8000
> [    7.553392] PC is at lpuart_timer_func+0x24/0xf8
> [    7.558127] LR is at lpuart_timer_func+0x20/0xf8
> [    7.562857] pc : [<802df99c>]    lr : [<802df998>]    psr: 600b0113
> [    7.562857] sp : 86ac9b90  ip : 86ac9b90  fp : 86ac9bbc
> [    7.574467] r10: 80817180  r9 : 80817b98  r8 : 80817998
> [    7.579803] r7 : 807acee0  r6 : 86989000  r5 : 00000100  r4 : 86997210
> [    7.586444] r3 : 86ac8000  r2 : 86ac9bc0  r1 : 86997210  r0 : 00000000
> [    7.593085] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
> [    7.600341] Control: 10c5387d  Table: 86348059  DAC: 00000015
> [    7.606203] Process systemd (pid: 123, stack limit = 0x86ac8230)
> 
> Setup the timer on UART startup which allows to delete the timer
> unconditionally on shutdown. This also saves the initialization
> on each transfer.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Do stable kernels care about this as well, or is this just a regression
from 3.18?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v2 1/2] serial: fsl_lpuart: delete timer on shutdown
@ 2015-01-10  0:48     ` Greg KH
  0 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2015-01-10  0:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Jan 10, 2015 at 01:08:58AM +0100, Stefan Agner wrote:
> If the serial port gets closed while a RX transfer is in progress,
> the timer might fire after the serial port shutdown finished. This
> leads in a NULL pointer dereference:
> 
> [    7.508324] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [    7.516590] pgd = 86348000
> [    7.519445] [00000000] *pgd=86179831, *pte=00000000, *ppte=00000000
> [    7.526145] Internal error: Oops: 17 [#1] ARM
> [    7.530611] Modules linked in:
> [    7.533876] CPU: 0 PID: 123 Comm: systemd Not tainted 3.19.0-rc3-00004-g5b11ea7 #1778
> [    7.541827] Hardware name: Freescale Vybrid VF610 (Device Tree)
> [    7.547862] task: 861c3400 ti: 86ac8000 task.ti: 86ac8000
> [    7.553392] PC is at lpuart_timer_func+0x24/0xf8
> [    7.558127] LR is at lpuart_timer_func+0x20/0xf8
> [    7.562857] pc : [<802df99c>]    lr : [<802df998>]    psr: 600b0113
> [    7.562857] sp : 86ac9b90  ip : 86ac9b90  fp : 86ac9bbc
> [    7.574467] r10: 80817180  r9 : 80817b98  r8 : 80817998
> [    7.579803] r7 : 807acee0  r6 : 86989000  r5 : 00000100  r4 : 86997210
> [    7.586444] r3 : 86ac8000  r2 : 86ac9bc0  r1 : 86997210  r0 : 00000000
> [    7.593085] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
> [    7.600341] Control: 10c5387d  Table: 86348059  DAC: 00000015
> [    7.606203] Process systemd (pid: 123, stack limit = 0x86ac8230)
> 
> Setup the timer on UART startup which allows to delete the timer
> unconditionally on shutdown. This also saves the initialization
> on each transfer.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Do stable kernels care about this as well, or is this just a regression
from 3.18?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] serial: fsl_lpuart: avoid new transfer while DMA is running
  2015-01-10  0:08   ` Stefan Agner
@ 2015-01-10  0:49     ` Greg KH
  -1 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2015-01-10  0:49 UTC (permalink / raw)
  To: Stefan Agner
  Cc: jslaby, jingchang.lu, shawn.guo, linux-serial, linux-arm-kernel,
	linux-kernel

On Sat, Jan 10, 2015 at 01:08:59AM +0100, Stefan Agner wrote:
> When the UART is in DMA receive mode (RDMAS set) and one character
> just arrived while another interrupt is handled (e.g. TX), the RDRF
> (receiver data register full flag) is set due to the water level of
> 1. But since the DMA will take care of this character, there is no
> need to handle it by calling lpuart_prepare_rx. Handling it leads to
> adding the RX timeout timer twice:
> 
> [   74.336698] Kernel BUG at 80053070 [verbose debug info unavailable]
> [   74.342999] Internal error: Oops - BUG: 0 [#1] ARM0:00.00 khungtaskd
> [   74.347817] Modules linked in:    0 S  0.0  0.0   0:00.00 writeback
> [   74.350926] CPU: 0 PID: 0 Comm: swapper Not tainted 3.19.0-rc3-00001-g39d78e2 #1788
> [   74.358617] Hardware name: Freescale Vybrid VF610 (Device Tree)t
> [   74.364563] task: 807a7678 ti: 8079c000 task.ti: 8079c000 kblockd
> [   74.370002] PC is at add_timer+0x24/0x28.0  0.0   0:00.09 kworker/u2:1
> [   74.373960] LR is at lpuart_int+0x15c/0x3d8
> [   74.378171] pc : [<80053070>]    lr : [<802e0d88>]    psr: a0010193
> [   74.378171] sp : 8079de10  ip : 8079de20  fp : 8079de1c
> [   74.389694] r10: 807d44c0  r9 : 8688c300  r8 : 00000013
> [   74.394943] r7 : 20010193  r6 : 00000000  r5 : 000000a0  r4 : 86997210
> [   74.401498] r3 : ffffa7da  r2 : 80817868  r1 : 86997210  r0 : 86997344
> [   74.408052] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> [   74.415489] Control: 10c5387d  Table: 8611c059  DAC: 00000015
> [   74.421265] Process swapper (pid: 0, stack limit = 0x8079c230)
> ...
> 
> Solve this by only execute the receiver path (lpuart_prepare_rx) if
> the DMA receive mode (RDMAS) is not set. Also, make sure the flag is
> cleared on initialization, in case it has been left set.
> 
> This can be best reproduced using UART as a serial console, then
> running top while dd'ing data into the terminal.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Same stable question as before.


^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v2 2/2] serial: fsl_lpuart: avoid new transfer while DMA is running
@ 2015-01-10  0:49     ` Greg KH
  0 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2015-01-10  0:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Jan 10, 2015 at 01:08:59AM +0100, Stefan Agner wrote:
> When the UART is in DMA receive mode (RDMAS set) and one character
> just arrived while another interrupt is handled (e.g. TX), the RDRF
> (receiver data register full flag) is set due to the water level of
> 1. But since the DMA will take care of this character, there is no
> need to handle it by calling lpuart_prepare_rx. Handling it leads to
> adding the RX timeout timer twice:
> 
> [   74.336698] Kernel BUG at 80053070 [verbose debug info unavailable]
> [   74.342999] Internal error: Oops - BUG: 0 [#1] ARM0:00.00 khungtaskd
> [   74.347817] Modules linked in:    0 S  0.0  0.0   0:00.00 writeback
> [   74.350926] CPU: 0 PID: 0 Comm: swapper Not tainted 3.19.0-rc3-00001-g39d78e2 #1788
> [   74.358617] Hardware name: Freescale Vybrid VF610 (Device Tree)t
> [   74.364563] task: 807a7678 ti: 8079c000 task.ti: 8079c000 kblockd
> [   74.370002] PC is at add_timer+0x24/0x28.0  0.0   0:00.09 kworker/u2:1
> [   74.373960] LR is at lpuart_int+0x15c/0x3d8
> [   74.378171] pc : [<80053070>]    lr : [<802e0d88>]    psr: a0010193
> [   74.378171] sp : 8079de10  ip : 8079de20  fp : 8079de1c
> [   74.389694] r10: 807d44c0  r9 : 8688c300  r8 : 00000013
> [   74.394943] r7 : 20010193  r6 : 00000000  r5 : 000000a0  r4 : 86997210
> [   74.401498] r3 : ffffa7da  r2 : 80817868  r1 : 86997210  r0 : 86997344
> [   74.408052] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> [   74.415489] Control: 10c5387d  Table: 8611c059  DAC: 00000015
> [   74.421265] Process swapper (pid: 0, stack limit = 0x8079c230)
> ...
> 
> Solve this by only execute the receiver path (lpuart_prepare_rx) if
> the DMA receive mode (RDMAS) is not set. Also, make sure the flag is
> cleared on initialization, in case it has been left set.
> 
> This can be best reproduced using UART as a serial console, then
> running top while dd'ing data into the terminal.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Same stable question as before.

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] serial: fsl_lpuart: delete timer on shutdown
  2015-01-10  0:48     ` Greg KH
@ 2015-01-10  8:09       ` Stefan Agner
  -1 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  8:09 UTC (permalink / raw)
  To: Greg KH
  Cc: jslaby, jingchang.lu, shawn.guo, linux-serial, linux-arm-kernel,
	linux-kernel

On 2015-01-10 01:48, Greg KH wrote:
> On Sat, Jan 10, 2015 at 01:08:58AM +0100, Stefan Agner wrote:
>> If the serial port gets closed while a RX transfer is in progress,
>> the timer might fire after the serial port shutdown finished. This
>> leads in a NULL pointer dereference:
>>
>> [    7.508324] Unable to handle kernel NULL pointer dereference at virtual address 00000000
>> [    7.516590] pgd = 86348000
>> [    7.519445] [00000000] *pgd=86179831, *pte=00000000, *ppte=00000000
>> [    7.526145] Internal error: Oops: 17 [#1] ARM
>> [    7.530611] Modules linked in:
>> [    7.533876] CPU: 0 PID: 123 Comm: systemd Not tainted 3.19.0-rc3-00004-g5b11ea7 #1778
>> [    7.541827] Hardware name: Freescale Vybrid VF610 (Device Tree)
>> [    7.547862] task: 861c3400 ti: 86ac8000 task.ti: 86ac8000
>> [    7.553392] PC is at lpuart_timer_func+0x24/0xf8
>> [    7.558127] LR is at lpuart_timer_func+0x20/0xf8
>> [    7.562857] pc : [<802df99c>]    lr : [<802df998>]    psr: 600b0113
>> [    7.562857] sp : 86ac9b90  ip : 86ac9b90  fp : 86ac9bbc
>> [    7.574467] r10: 80817180  r9 : 80817b98  r8 : 80817998
>> [    7.579803] r7 : 807acee0  r6 : 86989000  r5 : 00000100  r4 : 86997210
>> [    7.586444] r3 : 86ac8000  r2 : 86ac9bc0  r1 : 86997210  r0 : 00000000
>> [    7.593085] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
>> [    7.600341] Control: 10c5387d  Table: 86348059  DAC: 00000015
>> [    7.606203] Process systemd (pid: 123, stack limit = 0x86ac8230)
>>
>> Setup the timer on UART startup which allows to delete the timer
>> unconditionally on shutdown. This also saves the initialization
>> on each transfer.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/tty/serial/fsl_lpuart.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> Do stable kernels care about this as well, or is this just a regression
> from 3.18?

This is an issue since DMA support for lpuart was included in the
kernel, which was in 3.14.

--
Stefan


^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v2 1/2] serial: fsl_lpuart: delete timer on shutdown
@ 2015-01-10  8:09       ` Stefan Agner
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  8:09 UTC (permalink / raw)
  To: linux-arm-kernel

On 2015-01-10 01:48, Greg KH wrote:
> On Sat, Jan 10, 2015 at 01:08:58AM +0100, Stefan Agner wrote:
>> If the serial port gets closed while a RX transfer is in progress,
>> the timer might fire after the serial port shutdown finished. This
>> leads in a NULL pointer dereference:
>>
>> [    7.508324] Unable to handle kernel NULL pointer dereference at virtual address 00000000
>> [    7.516590] pgd = 86348000
>> [    7.519445] [00000000] *pgd=86179831, *pte=00000000, *ppte=00000000
>> [    7.526145] Internal error: Oops: 17 [#1] ARM
>> [    7.530611] Modules linked in:
>> [    7.533876] CPU: 0 PID: 123 Comm: systemd Not tainted 3.19.0-rc3-00004-g5b11ea7 #1778
>> [    7.541827] Hardware name: Freescale Vybrid VF610 (Device Tree)
>> [    7.547862] task: 861c3400 ti: 86ac8000 task.ti: 86ac8000
>> [    7.553392] PC is at lpuart_timer_func+0x24/0xf8
>> [    7.558127] LR is at lpuart_timer_func+0x20/0xf8
>> [    7.562857] pc : [<802df99c>]    lr : [<802df998>]    psr: 600b0113
>> [    7.562857] sp : 86ac9b90  ip : 86ac9b90  fp : 86ac9bbc
>> [    7.574467] r10: 80817180  r9 : 80817b98  r8 : 80817998
>> [    7.579803] r7 : 807acee0  r6 : 86989000  r5 : 00000100  r4 : 86997210
>> [    7.586444] r3 : 86ac8000  r2 : 86ac9bc0  r1 : 86997210  r0 : 00000000
>> [    7.593085] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
>> [    7.600341] Control: 10c5387d  Table: 86348059  DAC: 00000015
>> [    7.606203] Process systemd (pid: 123, stack limit = 0x86ac8230)
>>
>> Setup the timer on UART startup which allows to delete the timer
>> unconditionally on shutdown. This also saves the initialization
>> on each transfer.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/tty/serial/fsl_lpuart.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> Do stable kernels care about this as well, or is this just a regression
> from 3.18?

This is an issue since DMA support for lpuart was included in the
kernel, which was in 3.14.

--
Stefan

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] serial: fsl_lpuart: avoid new transfer while DMA is running
  2015-01-10  0:49     ` Greg KH
@ 2015-01-10  8:12       ` Stefan Agner
  -1 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  8:12 UTC (permalink / raw)
  To: Greg KH
  Cc: jslaby, jingchang.lu, shawn.guo, linux-serial, linux-arm-kernel,
	linux-kernel

On 2015-01-10 01:49, Greg KH wrote:
> On Sat, Jan 10, 2015 at 01:08:59AM +0100, Stefan Agner wrote:
>> When the UART is in DMA receive mode (RDMAS set) and one character
>> just arrived while another interrupt is handled (e.g. TX), the RDRF
>> (receiver data register full flag) is set due to the water level of
>> 1. But since the DMA will take care of this character, there is no
>> need to handle it by calling lpuart_prepare_rx. Handling it leads to
>> adding the RX timeout timer twice:
>>
>> [   74.336698] Kernel BUG at 80053070 [verbose debug info unavailable]
>> [   74.342999] Internal error: Oops - BUG: 0 [#1] ARM0:00.00 khungtaskd
>> [   74.347817] Modules linked in:    0 S  0.0  0.0   0:00.00 writeback
>> [   74.350926] CPU: 0 PID: 0 Comm: swapper Not tainted 3.19.0-rc3-00001-g39d78e2 #1788
>> [   74.358617] Hardware name: Freescale Vybrid VF610 (Device Tree)t
>> [   74.364563] task: 807a7678 ti: 8079c000 task.ti: 8079c000 kblockd
>> [   74.370002] PC is at add_timer+0x24/0x28.0  0.0   0:00.09 kworker/u2:1
>> [   74.373960] LR is at lpuart_int+0x15c/0x3d8
>> [   74.378171] pc : [<80053070>]    lr : [<802e0d88>]    psr: a0010193
>> [   74.378171] sp : 8079de10  ip : 8079de20  fp : 8079de1c
>> [   74.389694] r10: 807d44c0  r9 : 8688c300  r8 : 00000013
>> [   74.394943] r7 : 20010193  r6 : 00000000  r5 : 000000a0  r4 : 86997210
>> [   74.401498] r3 : ffffa7da  r2 : 80817868  r1 : 86997210  r0 : 86997344
>> [   74.408052] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
>> [   74.415489] Control: 10c5387d  Table: 8611c059  DAC: 00000015
>> [   74.421265] Process swapper (pid: 0, stack limit = 0x8079c230)
>> ...
>>
>> Solve this by only execute the receiver path (lpuart_prepare_rx) if
>> the DMA receive mode (RDMAS) is not set. Also, make sure the flag is
>> cleared on initialization, in case it has been left set.
>>
>> This can be best reproduced using UART as a serial console, then
>> running top while dd'ing data into the terminal.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/tty/serial/fsl_lpuart.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> Same stable question as before.

The same here, an issue since 3.14....

--
Stefan



^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v2 2/2] serial: fsl_lpuart: avoid new transfer while DMA is running
@ 2015-01-10  8:12       ` Stefan Agner
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Agner @ 2015-01-10  8:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 2015-01-10 01:49, Greg KH wrote:
> On Sat, Jan 10, 2015 at 01:08:59AM +0100, Stefan Agner wrote:
>> When the UART is in DMA receive mode (RDMAS set) and one character
>> just arrived while another interrupt is handled (e.g. TX), the RDRF
>> (receiver data register full flag) is set due to the water level of
>> 1. But since the DMA will take care of this character, there is no
>> need to handle it by calling lpuart_prepare_rx. Handling it leads to
>> adding the RX timeout timer twice:
>>
>> [   74.336698] Kernel BUG at 80053070 [verbose debug info unavailable]
>> [   74.342999] Internal error: Oops - BUG: 0 [#1] ARM0:00.00 khungtaskd
>> [   74.347817] Modules linked in:    0 S  0.0  0.0   0:00.00 writeback
>> [   74.350926] CPU: 0 PID: 0 Comm: swapper Not tainted 3.19.0-rc3-00001-g39d78e2 #1788
>> [   74.358617] Hardware name: Freescale Vybrid VF610 (Device Tree)t
>> [   74.364563] task: 807a7678 ti: 8079c000 task.ti: 8079c000 kblockd
>> [   74.370002] PC is at add_timer+0x24/0x28.0  0.0   0:00.09 kworker/u2:1
>> [   74.373960] LR is at lpuart_int+0x15c/0x3d8
>> [   74.378171] pc : [<80053070>]    lr : [<802e0d88>]    psr: a0010193
>> [   74.378171] sp : 8079de10  ip : 8079de20  fp : 8079de1c
>> [   74.389694] r10: 807d44c0  r9 : 8688c300  r8 : 00000013
>> [   74.394943] r7 : 20010193  r6 : 00000000  r5 : 000000a0  r4 : 86997210
>> [   74.401498] r3 : ffffa7da  r2 : 80817868  r1 : 86997210  r0 : 86997344
>> [   74.408052] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
>> [   74.415489] Control: 10c5387d  Table: 8611c059  DAC: 00000015
>> [   74.421265] Process swapper (pid: 0, stack limit = 0x8079c230)
>> ...
>>
>> Solve this by only execute the receiver path (lpuart_prepare_rx) if
>> the DMA receive mode (RDMAS) is not set. Also, make sure the flag is
>> cleared on initialization, in case it has been left set.
>>
>> This can be best reproduced using UART as a serial console, then
>> running top while dd'ing data into the terminal.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/tty/serial/fsl_lpuart.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> Same stable question as before.

The same here, an issue since 3.14....

--
Stefan

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-01-10  8:13 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-10  0:08 [PATCH v2 0/2] serial: fsl_lpuart: fix DMA issues for 3.19 Stefan Agner
2015-01-10  0:08 ` Stefan Agner
2015-01-10  0:08 ` [PATCH v2 1/2] serial: fsl_lpuart: delete timer on shutdown Stefan Agner
2015-01-10  0:08   ` Stefan Agner
2015-01-10  0:48   ` Greg KH
2015-01-10  0:48     ` Greg KH
2015-01-10  8:09     ` Stefan Agner
2015-01-10  8:09       ` Stefan Agner
2015-01-10  0:08 ` [PATCH v2 2/2] serial: fsl_lpuart: avoid new transfer while DMA is running Stefan Agner
2015-01-10  0:08   ` Stefan Agner
2015-01-10  0:49   ` Greg KH
2015-01-10  0:49     ` Greg KH
2015-01-10  8:12     ` Stefan Agner
2015-01-10  8:12       ` Stefan Agner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.