tty: serial: 8250: omap: synchronize rx_running
authorJohn Ogness <john.ogness@linutronix.de>
Mon, 27 Apr 2015 11:52:33 +0000 (13:52 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 6 May 2015 20:27:01 +0000 (22:27 +0200)
The rx_running flag should show if DMA is currently active. However
there is a window between when the flag is set/cleared and when
the DMA is started/stopped. Because the flag is queried from both
hard and soft irq contexts, the driver can make incorrect
decisions and do things like start a DMA transfer using a buffer
that is already setup to be used for a DMA transfer.

This patch adds a spinlock to synchronize the rx_running flag and
close the above mentioned window.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/tty/serial/8250/8250_omap.c

index 9289999cb7c62bb05b2a4b758fa76d5ce9413316..77c9f5aa83698646937f8aa9ed4c5d1579c73410 100644 (file)
@@ -98,6 +98,7 @@ struct omap8250_priv {
        struct pm_qos_request pm_qos_request;
        struct work_struct qos_work;
        struct uart_8250_dma omap8250_dma;
+       spinlock_t rx_dma_lock;
 };
 
 static u32 uart_read(struct uart_8250_port *up, u32 reg)
@@ -669,14 +670,21 @@ static int omap_8250_rx_dma(struct uart_8250_port *p, unsigned int iir);
 
 static void __dma_rx_do_complete(struct uart_8250_port *p, bool error)
 {
+       struct omap8250_priv    *priv = p->port.private_data;
        struct uart_8250_dma    *dma = p->dma;
        struct tty_port         *tty_port = &p->port.state->port;
        struct dma_tx_state     state;
        int                     count;
+       unsigned long           flags;
 
        dma_sync_single_for_cpu(dma->rxchan->device->dev, dma->rx_addr,
                                dma->rx_size, DMA_FROM_DEVICE);
 
+       spin_lock_irqsave(&priv->rx_dma_lock, flags);
+
+       if (!dma->rx_running)
+               goto unlock;
+
        dma->rx_running = 0;
        dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
        dmaengine_terminate_all(dma->rxchan);
@@ -685,6 +693,9 @@ static void __dma_rx_do_complete(struct uart_8250_port *p, bool error)
 
        tty_insert_flip_string(tty_port, dma->rx_buf, count);
        p->port.icount.rx += count;
+unlock:
+       spin_unlock_irqrestore(&priv->rx_dma_lock, flags);
+
        if (!error)
                omap_8250_rx_dma(p, 0);
 
@@ -696,28 +707,45 @@ static void __dma_rx_complete(void *param)
        __dma_rx_do_complete(param, false);
 }
 
+static void omap_8250_rx_dma_flush(struct uart_8250_port *p)
+{
+       struct omap8250_priv    *priv = p->port.private_data;
+       struct uart_8250_dma    *dma = p->dma;
+       unsigned long           flags;
+
+       spin_lock_irqsave(&priv->rx_dma_lock, flags);
+
+       if (!dma->rx_running) {
+               spin_unlock_irqrestore(&priv->rx_dma_lock, flags);
+               return;
+       }
+
+       dmaengine_pause(dma->rxchan);
+
+       spin_unlock_irqrestore(&priv->rx_dma_lock, flags);
+
+       __dma_rx_do_complete(p, true);
+}
+
 static int omap_8250_rx_dma(struct uart_8250_port *p, unsigned int iir)
 {
+       struct omap8250_priv            *priv = p->port.private_data;
        struct uart_8250_dma            *dma = p->dma;
+       int                             err = 0;
        struct dma_async_tx_descriptor  *desc;
+       unsigned long                   flags;
 
        switch (iir & 0x3f) {
        case UART_IIR_RLSI:
                /* 8250_core handles errors and break interrupts */
-               if (dma->rx_running) {
-                       dmaengine_pause(dma->rxchan);
-                       __dma_rx_do_complete(p, true);
-               }
+               omap_8250_rx_dma_flush(p);
                return -EIO;
        case UART_IIR_RX_TIMEOUT:
                /*
                 * If RCVR FIFO trigger level was not reached, complete the
                 * transfer and let 8250_core copy the remaining data.
                 */
-               if (dma->rx_running) {
-                       dmaengine_pause(dma->rxchan);
-                       __dma_rx_do_complete(p, true);
-               }
+               omap_8250_rx_dma_flush(p);
                return -ETIMEDOUT;
        case UART_IIR_RDI:
                /*
@@ -729,24 +757,25 @@ static int omap_8250_rx_dma(struct uart_8250_port *p, unsigned int iir)
                 * the DMA won't do anything soon so we have to cancel the DMA
                 * transfer and purge the FIFO manually.
                 */
-               if (dma->rx_running) {
-                       dmaengine_pause(dma->rxchan);
-                       __dma_rx_do_complete(p, true);
-               }
+               omap_8250_rx_dma_flush(p);
                return -ETIMEDOUT;
 
        default:
                break;
        }
 
+       spin_lock_irqsave(&priv->rx_dma_lock, flags);
+
        if (dma->rx_running)
-               return 0;
+               goto out;
 
        desc = dmaengine_prep_slave_single(dma->rxchan, dma->rx_addr,
                                           dma->rx_size, DMA_DEV_TO_MEM,
                                           DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
-       if (!desc)
-               return -EBUSY;
+       if (!desc) {
+               err = -EBUSY;
+               goto out;
+       }
 
        dma->rx_running = 1;
        desc->callback = __dma_rx_complete;
@@ -758,7 +787,9 @@ static int omap_8250_rx_dma(struct uart_8250_port *p, unsigned int iir)
                                   dma->rx_size, DMA_FROM_DEVICE);
 
        dma_async_issue_pending(dma->rxchan);
-       return 0;
+out:
+       spin_unlock_irqrestore(&priv->rx_dma_lock, flags);
+       return err;
 }
 
 static int omap_8250_tx_dma(struct uart_8250_port *p);
@@ -1065,6 +1096,8 @@ static int omap8250_probe(struct platform_device *pdev)
                           priv->latency);
        INIT_WORK(&priv->qos_work, omap8250_uart_qos_work);
 
+       spin_lock_init(&priv->rx_dma_lock);
+
        device_init_wakeup(&pdev->dev, true);
        pm_runtime_use_autosuspend(&pdev->dev);
        pm_runtime_set_autosuspend_delay(&pdev->dev, -1);