i2c: rcar: fix concurrency issue related to ICDMAER
authorHiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Sun, 3 Mar 2019 15:03:13 +0000 (16:03 +0100)
committerWolfram Sang <wsa@the-dreams.de>
Tue, 12 Mar 2019 13:09:16 +0000 (14:09 +0100)
This patch fixes the problem that an interrupt may set up a new I2C
message and the DMA callback overwrites this setup.

By disabling the DMA Enable Register(ICDMAER), rcar_i2c_dma_unmap()
enables interrupts for register settings (such as Master Control
Register(ICMCR)) and advances the I2C transfer sequence.

If an interrupt occurs immediately after ICDMAER is disabled, the
callback handler later continues and overwrites the previous settings
from the interrupt. So, disable ICDMAER at the end of the callback to
ensure other interrupts are masked until then.

Note that this driver needs to work lock-free because there are IP cores
with a HW race condition which prevent us from using a spinlock in the
interrupt handler.

Reproduction test:
1. Add a delay after disabling ICDMAER. (It is expected to generate an
   interrupt of rcar_i2c_irq())

    void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv)
    {
        ...
        rcar_i2c_write(priv, ICDMAER, 0);
        usleep_range(500, 800)
        ...
        priv->dma_direction = DMA_NONE;
    }

2. Execute DMA transfers

 $ i2ctransfer -y 4 w9@0x6a 1 1+ r16

3. A log message of BUG_ON() will be displayed.

Fixes: 73e8b0528346 ("i2c: rcar: add DMA support")
Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[wsa: updated test case to be more reliable, added note to comment]
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
drivers/i2c/busses/i2c-rcar.c

index dd52a06..ce59e14 100644 (file)
@@ -363,9 +363,6 @@ static void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv)
        struct dma_chan *chan = priv->dma_direction == DMA_FROM_DEVICE
                ? priv->dma_rx : priv->dma_tx;
 
-       /* Disable DMA Master Received/Transmitted */
-       rcar_i2c_write(priv, ICDMAER, 0);
-
        dma_unmap_single(chan->device->dev, sg_dma_address(&priv->sg),
                         sg_dma_len(&priv->sg), priv->dma_direction);
 
@@ -375,6 +372,9 @@ static void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv)
                priv->flags |= ID_P_NO_RXDMA;
 
        priv->dma_direction = DMA_NONE;
+
+       /* Disable DMA Master Received/Transmitted, must be last! */
+       rcar_i2c_write(priv, ICDMAER, 0);
 }
 
 static void rcar_i2c_cleanup_dma(struct rcar_i2c_priv *priv)