[PATCH 0/5] tty/serial: at91: fix bugs when using multiple serials

[PATCH 0/5] tty/serial: at91: fix bugs when using multiple serials

[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

[PATCH 1/5] tty/serial: at91: correct check of buf used in DMA

[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

[PATCH 2/5] tty/serial: at91: correct buffer size used in DMA

[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

[PATCH 3/5] tty/serial: at91: revise the return type of atmel_init_property

[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

[PATCH 4/5] tty/serial: at91: set ops in property init each time

[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

[PATCH 5/5] tty/serial: at91: correct the usage of tasklet

[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

Re: [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA

[Jiri Slaby]

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

RE: [PATCH 1/5] tty/serial: at91: correct check of buf used in DMA

[Zhao, Leilei]

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