RE: 8250_dw DMA issue with BYT ?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Heikki, Jin

> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -143,7 +143,7 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
>          */
>         if (!state->xmit.buf) {
>                 /* This is protected by the per port mutex */
> -               page = get_zeroed_page(GFP_KERNEL);
> +               page = get_zeroed_page(GFP_KERNEL | GFP_DMA);
>                 if (!page)
>                        return -ENOMEM;

This patch doesn't fix my problem.


After analysis, here is my status:

The Bluetooth chip generates hardware errors due to unexpected
data. These unexpected data are caused by 8250 concurrent TX.

- Concurrent DMA TX (8250_dma.c):

After each DMA transfer, __dma_tx_complete() is called.
This function updates the circular buffer tail and calls
serial8250_tx_dma() if data remains.

However these operations are not protected against concurrent
call of serial8250_tx_dma() (via uart start_tx). So, this concurrent 
call may use non-updated tail index and may be called in parallel 
of an other serial8250_tx_dma().


- Concurrent Chars TX (8250_corec.c):

In the serial8250_handle_irq(), serial8250_tx_chars is called if
UART_LSR_THRE. However we should not call this function
if we are using the DMA. Moreover, for the same reason as above,
serial8250_tx_chars could be called with bad tail index or in parallel
of a serial8250_tx_dma.


I think two fixes are necessary:

to avoid tx chars usage:

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 69932b7..e124983 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1520,7 +1520,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
                        status = serial8250_rx_chars(up, status);
        }
        serial8250_modem_status(up);
-       if (status & UART_LSR_THRE)
+       if (!up->dma && (status & UART_LSR_THRE))
                serial8250_tx_chars(up);
 
        spin_unlock_irqrestore(&port->lock, flags);


to fix concurrent operations:

diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 7046769..4469190 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -20,6 +20,9 @@ static void __dma_tx_complete(void *param)
        struct uart_8250_port   *p = param;
        struct uart_8250_dma    *dma = p->dma;
        struct circ_buf         *xmit = &p->port.state->xmit;
+       unsigned long flags;
+
+       spin_lock_irqsave(&p->port.lock, flags);
 
        dma->tx_running = 0;
 
@@ -35,6 +38,8 @@ static void __dma_tx_complete(void *param)
 
        if (!uart_circ_empty(xmit) && !uart_tx_stopped(&p->port))
                serial8250_tx_dma(p);
+
+       spin_unlock_irqrestore(&p->port.lock, flags);
 }


The last one fixes TX issues but causes random freeze of my
platform (when uploading file via bluetooth). It seems to be a deadlock.
I have no kernel trace in my console, system is stuck.

To temporally workaround this, I simply moved "tx_running=0" after updating
tail index (in __dma_tx_complete) so that no dma_tx can happen before 
this update.

	xmit->tail += dma->tx_size;
	xmit->tail &= UART_XMIT_SIZE - 1;
	p->port.icount.tx += dma->tx_size;

	dma->tx_running = 0;


Regards,
Loic
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux