LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/5] tty/serial: at91: fix bugs when using multiple serials
@ 2015-02-26  6:55 Leilei Zhao
  2015-02-26  6:55 ` [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA Leilei Zhao
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Leilei Zhao @ 2015-02-26  6:55 UTC (permalink / raw)
  To: nicolas.ferre, gregkh, linux-serial, jslaby
  Cc: linux-kernel, linux-arm-kernel, Leilei Zhao

The series of patches fix bugs when using multiple serial ports at the same time:
 - The using rx ring buffer in DMA is inconsistent with its allocation.
 - The serial port can't send and receive data when it's opened the second time
   and the later if it switches to PIO when DMA channel is not available.
 - The serial port can lead to kernel crash when data is sending and receiving
   and the program is killed.
 
The patches were made from branch tty-next of gregkh/tty.git repository, and 
tested on a SAMA5D36 VB boards with 5 serial ports enabled.

Leilei Zhao (5):
  tty/serial: at91: correct check of buf used in DMA
  tty/serial: at91: correct buffer size used in DMA
  tty/serial: at91: revise the return type of atmel_init_property
  tty/serial: at91: set ops in property init each time
  tty/serial: at91: correct the usage of tasklet

 drivers/tty/serial/atmel_serial.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA
  2015-02-26  6:55 [PATCH 0/5] tty/serial: at91: fix bugs when using multiple serials Leilei Zhao
@ 2015-02-26  6:55 ` Leilei Zhao
  2015-02-26  9:29   ` Jiri Slaby
  2015-02-26  6:55 ` [PATCH 2/5] tty/serial: at91: correct buffer size " Leilei Zhao
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Leilei Zhao @ 2015-02-26  6:55 UTC (permalink / raw)
  To: nicolas.ferre, gregkh, linux-serial, jslaby
  Cc: linux-kernel, linux-arm-kernel, Leilei Zhao

We only use buf of ring In DMA rx function while using buf of xmit
in DMA tx function. So here we need definitively to check the buf
of ring which is corresponding to DMA rx function.

Signed-off-by: Leilei Zhao <leilei.zhao@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/tty/serial/atmel_serial.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 846552b..460903c 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1027,7 +1027,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
 	spin_lock_init(&atmel_port->lock_rx);
 	sg_init_table(&atmel_port->sg_rx, 1);
 	/* UART circular rx buffer is an aligned page. */
-	BUG_ON((int)port->state->xmit.buf & ~PAGE_MASK);
+	BUG_ON((int)ring->buf & ~PAGE_MASK);
 	sg_set_page(&atmel_port->sg_rx,
 		    virt_to_page(ring->buf),
 		    ATMEL_SERIAL_RINGSIZE,
-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/5] tty/serial: at91: correct buffer size used in DMA
  2015-02-26  6:55 [PATCH 0/5] tty/serial: at91: fix bugs when using multiple serials Leilei Zhao
  2015-02-26  6:55 ` [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA Leilei Zhao
@ 2015-02-26  6:55 ` Leilei Zhao
  2015-02-26  6:55 ` [PATCH 3/5] tty/serial: at91: revise the return type of atmel_init_property Leilei Zhao
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Leilei Zhao @ 2015-02-26  6:55 UTC (permalink / raw)
  To: nicolas.ferre, gregkh, linux-serial, jslaby
  Cc: linux-kernel, linux-arm-kernel, Leilei Zhao

The buffer size set in DMA is inconsistent with its allocation.
So keep them consistent here. The structure atmel_uart_char is
used in PIO mode with its meaning. But here in DMA, all of the
buffer is treated as general char.

Signed-off-by: Leilei Zhao <leilei.zhao@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/tty/serial/atmel_serial.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 460903c..6a4d44d 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1030,7 +1030,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
 	BUG_ON((int)ring->buf & ~PAGE_MASK);
 	sg_set_page(&atmel_port->sg_rx,
 		    virt_to_page(ring->buf),
-		    ATMEL_SERIAL_RINGSIZE,
+		    sizeof(struct atmel_uart_char) * ATMEL_SERIAL_RINGSIZE,
 		    (int)ring->buf & ~PAGE_MASK);
 	nent = dma_map_sg(port->dev,
 			  &atmel_port->sg_rx,
-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/5] tty/serial: at91: revise the return type of atmel_init_property
  2015-02-26  6:55 [PATCH 0/5] tty/serial: at91: fix bugs when using multiple serials Leilei Zhao
  2015-02-26  6:55 ` [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA Leilei Zhao
  2015-02-26  6:55 ` [PATCH 2/5] tty/serial: at91: correct buffer size " Leilei Zhao
@ 2015-02-26  6:55 ` Leilei Zhao
  2015-02-26  6:55 ` [PATCH 4/5] tty/serial: at91: set ops in property init each time Leilei Zhao
  2015-02-26  6:55 ` [PATCH 5/5] tty/serial: at91: correct the usage of tasklet Leilei Zhao
  4 siblings, 0 replies; 8+ messages in thread
From: Leilei Zhao @ 2015-02-26  6:55 UTC (permalink / raw)
  To: nicolas.ferre, gregkh, linux-serial, jslaby
  Cc: linux-kernel, linux-arm-kernel, Leilei Zhao

The function of atmel_init_property is to set the work manner of
atmel serial ports according to the property in device trees.
If DMA or PDC is not set or something goes wrong in getting property,
the work manner will switch to general PIO mode, thus there will
not be any failure case in this function. It's actually a procedure.
So changing the return type from int to void.

Signed-off-by: Leilei Zhao <leilei.zhao@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/tty/serial/atmel_serial.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 6a4d44d..30a62cd 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1534,7 +1534,7 @@ static void atmel_tasklet_func(unsigned long data)
 	spin_unlock(&port->lock);
 }
 
-static int atmel_init_property(struct atmel_uart_port *atmel_port,
+static void atmel_init_property(struct atmel_uart_port *atmel_port,
 				struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -1575,7 +1575,6 @@ static int atmel_init_property(struct atmel_uart_port *atmel_port,
 		atmel_port->use_dma_tx  = false;
 	}
 
-	return 0;
 }
 
 static void atmel_init_rs485(struct uart_port *port,
@@ -2235,8 +2234,8 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
 	struct uart_port *port = &atmel_port->uart;
 	struct atmel_uart_data *pdata = dev_get_platdata(&pdev->dev);
 
-	if (!atmel_init_property(atmel_port, pdev))
-		atmel_set_ops(port);
+	atmel_init_property(atmel_port, pdev);
+	atmel_set_ops(port);
 
 	atmel_init_rs485(port, pdev);
 
-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 4/5] tty/serial: at91: set ops in property init each time
  2015-02-26  6:55 [PATCH 0/5] tty/serial: at91: fix bugs when using multiple serials Leilei Zhao
                   ` (2 preceding siblings ...)
  2015-02-26  6:55 ` [PATCH 3/5] tty/serial: at91: revise the return type of atmel_init_property Leilei Zhao
@ 2015-02-26  6:55 ` Leilei Zhao
  2015-02-26  6:55 ` [PATCH 5/5] tty/serial: at91: correct the usage of tasklet Leilei Zhao
  4 siblings, 0 replies; 8+ messages in thread
From: Leilei Zhao @ 2015-02-26  6:55 UTC (permalink / raw)
  To: nicolas.ferre, gregkh, linux-serial, jslaby
  Cc: linux-kernel, linux-arm-kernel, Leilei Zhao

The property in device tree will be reading each time when tty is opened,
so the ops of serial port should be set after that instead of setting once
in probe. Otherwise, the ops of serial port is inconsistent with the state
of serial work manner. For example, the atmel serial driver can't work when
switching to PIO mode due to DMA channel is not available.

Signed-off-by: Leilei Zhao <leilei.zhao@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/tty/serial/atmel_serial.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 30a62cd..8d28210 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1759,6 +1759,7 @@ static int atmel_startup(struct uart_port *port)
 	 * Initialize DMA (if necessary)
 	 */
 	atmel_init_property(atmel_port, pdev);
+	atmel_set_ops(port);
 
 	if (atmel_port->prepare_rx) {
 		retval = atmel_port->prepare_rx(port);
-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 5/5] tty/serial: at91: correct the usage of tasklet
  2015-02-26  6:55 [PATCH 0/5] tty/serial: at91: fix bugs when using multiple serials Leilei Zhao
                   ` (3 preceding siblings ...)
  2015-02-26  6:55 ` [PATCH 4/5] tty/serial: at91: set ops in property init each time Leilei Zhao
@ 2015-02-26  6:55 ` Leilei Zhao
  4 siblings, 0 replies; 8+ messages in thread
From: Leilei Zhao @ 2015-02-26  6:55 UTC (permalink / raw)
  To: nicolas.ferre, gregkh, linux-serial, jslaby
  Cc: linux-kernel, linux-arm-kernel, Leilei Zhao

The tasklet may be scheduled and executed after serial port
was shutdown, for example, DMA rx callback will schedule the
tasklet while serial port is shutting down, especially serial
port is sending and receiving data in a higher baud rate and
it's killed by external program. In this case, tasklet_kill
can only clear the current scheduling out, so tasklet should
be disabled to prevent being executed in later scheduling.
Otherwise, the tasklet executed after serial port was shutdown
can lead to kernel crash.

Signed-off-by: Leilei Zhao <leilei.zhao@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/tty/serial/atmel_serial.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 8d28210..39ec278 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1755,6 +1755,8 @@ static int atmel_startup(struct uart_port *port)
 	if (retval)
 		goto free_irq;
 
+	tasklet_enable(&atmel_port->tasklet);
+
 	/*
 	 * Initialize DMA (if necessary)
 	 */
@@ -1858,6 +1860,7 @@ static void atmel_shutdown(struct uart_port *port)
 	 * Clear out any scheduled tasklets before
 	 * we destroy the buffers
 	 */
+	tasklet_disable(&atmel_port->tasklet);
 	tasklet_kill(&atmel_port->tasklet);
 
 	/*
@@ -2251,6 +2254,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
 
 	tasklet_init(&atmel_port->tasklet, atmel_tasklet_func,
 			(unsigned long)port);
+	tasklet_disable(&atmel_port->tasklet);
 
 	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
 
-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA
  2015-02-26  6:55 ` [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA Leilei Zhao
@ 2015-02-26  9:29   ` Jiri Slaby
  2015-02-27  7:38     ` Zhao, Leilei
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Slaby @ 2015-02-26  9:29 UTC (permalink / raw)
  To: Leilei Zhao, nicolas.ferre, gregkh, linux-serial
  Cc: linux-kernel, linux-arm-kernel

On 02/26/2015, 07:55 AM, Leilei Zhao wrote:
> We only use buf of ring In DMA rx function while using buf of xmit
> in DMA tx function. So here we need definitively to check the buf
> of ring which is corresponding to DMA rx function.
> 
> Signed-off-by: Leilei Zhao <leilei.zhao@atmel.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>  drivers/tty/serial/atmel_serial.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 846552b..460903c 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1027,7 +1027,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>  	spin_lock_init(&atmel_port->lock_rx);
>  	sg_init_table(&atmel_port->sg_rx, 1);
>  	/* UART circular rx buffer is an aligned page. */
> -	BUG_ON((int)port->state->xmit.buf & ~PAGE_MASK);
> +	BUG_ON((int)ring->buf & ~PAGE_MASK);

Please use PAGE_ALIGNED. This will also make the bad (int) cast to dismiss.

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA
  2015-02-26  9:29   ` Jiri Slaby
@ 2015-02-27  7:38     ` Zhao, Leilei
  0 siblings, 0 replies; 8+ messages in thread
From: Zhao, Leilei @ 2015-02-27  7:38 UTC (permalink / raw)
  To: Jiri Slaby, Ferre, Nicolas, gregkh, linux-serial
  Cc: linux-kernel, linux-arm-kernel

Thanks

Best Regards
Zhao Leilei

-----Original Message-----
From: Jiri Slaby [mailto:jirislaby@gmail.com] On Behalf Of Jiri Slaby
Sent: Thursday, February 26, 2015 17:30
To: Zhao, Leilei; Ferre, Nicolas; gregkh@linuxfoundation.org; linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA

On 02/26/2015, 07:55 AM, Leilei Zhao wrote:
> We only use buf of ring In DMA rx function while using buf of xmit in 
> DMA tx function. So here we need definitively to check the buf of ring 
> which is corresponding to DMA rx function.
> 
> Signed-off-by: Leilei Zhao <leilei.zhao@atmel.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>  drivers/tty/serial/atmel_serial.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c 
> b/drivers/tty/serial/atmel_serial.c
> index 846552b..460903c 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1027,7 +1027,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>  	spin_lock_init(&atmel_port->lock_rx);
>  	sg_init_table(&atmel_port->sg_rx, 1);
>  	/* UART circular rx buffer is an aligned page. */
> -	BUG_ON((int)port->state->xmit.buf & ~PAGE_MASK);
> +	BUG_ON((int)ring->buf & ~PAGE_MASK);

Please use PAGE_ALIGNED. This will also make the bad (int) cast to dismiss.

thanks,
--
js
suse labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-02-27  7:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-26  6:55 [PATCH 0/5] tty/serial: at91: fix bugs when using multiple serials Leilei Zhao
2015-02-26  6:55 ` [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA Leilei Zhao
2015-02-26  9:29   ` Jiri Slaby
2015-02-27  7:38     ` Zhao, Leilei
2015-02-26  6:55 ` [PATCH 2/5] tty/serial: at91: correct buffer size " Leilei Zhao
2015-02-26  6:55 ` [PATCH 3/5] tty/serial: at91: revise the return type of atmel_init_property Leilei Zhao
2015-02-26  6:55 ` [PATCH 4/5] tty/serial: at91: set ops in property init each time Leilei Zhao
2015-02-26  6:55 ` [PATCH 5/5] tty/serial: at91: correct the usage of tasklet Leilei Zhao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).